Re: [PATCH v3 1/3] iio: configfs: Add configfs support to IIO

[Daniel Baluta]

On Thu, Apr 9, 2015 at 9:12 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 09/04/15 18:40, Jonathan Cameron wrote:
>> On 09/04/15 18:15, Jonathan Cameron wrote:
>>> On 06/04/15 15:18, Daniel Baluta wrote:
>>>> This module is the core of IIO configfs. It creates the "iio" subsystem under
>>>> configfs mount point root, with one default group for "triggers".
>>>>
>>>> It offers basic interface for registering software trigger types. Next patches
>>>> will add "hrtimer" and "sysfs" trigger types. To add a new trigger type we must
>>>> create a new kernel module which implements iio_configfs_trigger.h interface.
>>>>
>>>> See Documentation/iio/iio_configfs.txt for more details on how configfs
>>>> support for IIO works.
>>>>
>>>> Signed-off-by: Daniel Baluta <daniel.baluta@xxxxxxxxx>
>>> Looks good and is actually a fair bit simpler than I expected which is nice!
>>> As an ideal aim, I'd like there to be no need for the core to have any
>>> idea what triggers exist and can be registered (beyond preventing naming
>>> clashes etc). Actually, not much would be needed to implement that I think, just
>>> using a list and looking up in it (we aren't on a particularly fast path so
>>> don't need anything clever) instead of a simple array.  A touch of locking
>>> probably to avoid two many simultaneous users of that list...
>>>
>>> Hmm. having read through the patches, are we fundamentally limited to having to
>>> specify the entire directory structure before bringing up configfs?
>> As I read more on this, I think the definition of a 'subsystem' in configfs is
>> very restricted. It means a tight group of devices sharing a prior
>> known interface with no ability to add extras later.

It looked fairly easy for me to add new attributes to an existing
interface. But this
is also my first experience with configfs.

>> It's sort of like the difference between a class and a bus in sysfs (but more limited
>> actually that's a rubbish analogy).  Thus either we have to do as here
>> and have the core know all about everything it might want to setup in advance
>> of registering the iio configfs subsystem or we need to make a whole load of
>> different configfs subsystems. Basically boils down to one per module.  We lose
>> the grouping convenience of the current structure but gain in separation.
>>
>> So option 1 we have effectively to predefine anything that might turn up
>> in a module...
>>
>> /config/iio/triggers/hrtimer/instance1/..
>> /config/iio/triggers/sysfs/instance2/..
>> /config/iio/virtualdevs/hwmon/iiohwmoninstance1/..
>> /config/iio/virtualdevs/input/iioinputinstance1/..
>> /config/iio/maps/channelmapisntance1/..
>> etc (actually fine for the maps as they are coming from the core anyway
>> and will be known in advance)

We predefine the files that would appear inside instace1/instance2. But we
do not predefine the instances. Also, we can easaily add new attributes for
a given type of trigger type ("hrtimer", "sysfs", etc).

These instances directories would be created with mkdir.

>>
>> Option 2 we have complete flexibility as its down to the relevant drivers
>>
>> /config/iio-trigger-hrtimer/instance/..
>> /config/iio-trigger-sysfs/instance/..
>> /config/iio-virtual-hwmon/instance/..
>> /config/iio-virtual-input/instance/..
>>
>> but we lose the grouping by directory which is a conceptual shame.
>>
>> Am I missing something important here?  (pretty new to configfs!)

Not sure exactly :), also my first experience with configs. But I woul prefer
option 1.

>>
>> Just for others not familiar with IIO who might read this. We have a number
>> of userspace created elements but each type is provided by a different module,
>> hence it's a real pain having to have them all defined at configfs subsystem
>> initialization as it adds tight coupling we don't otherwise have!
>
> For reference of others the usb gadget code is a good example of how to handle
> these sort of cross dependencies..
>
> the approach used for function drivers there would give us the cleaner option of
>
> /config/iio/triggers/hrtimer-instance1
> /config/iio/triggers/hrtimer-instance2
> /config/iio/triggers/sysfs-instance1
> /config/iio/virtualdevs/hwmon-instance1
> etc

I will look again at the gadget code, this was also my source of inspiration
since there are not so many users of configfs.

Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/