On Fri, Mar 15, 2024 at 12:58 AM Dominique Martinet <dominique.martinet@xxxxxxxxxxxxxxxxx> wrote: > > Hi Jonathan, > > Dominique Martinet wrote on Thu, Feb 29, 2024 at 11:59:19AM +0900: > > Jonathan Cameron wrote on Wed, Feb 28, 2024 at 02:24:41PM +0000: > > > A given IIO device driver may create multiple sysfs directories (registers > > > device + one or more triggers), so I'm not sure how this would work. > > > > Thanks for pointing this out, the driver I'm using doesn't seem to > > create extra triggers (iio_trigger_alloc doesn't seem to be called), but > > the current patch would only affect iio_device_register, so presumably > > would have no impact for these extra directories. > > So my device doesn't have any "built-in" trigger if that's a thing (let > alone multiple), but I've played with iio-trig-sysfs and also had a look > at what's in the tree's dts, and as far as I can see the 'name' > (/sys/bus/iio/devices/trigger*/name, also used when registering a > trigger for a device) seems to be fixed by the driver with parameters of > the dts (e.g. 'reg'), so if there are multiple triggers and one wants > something in the triggerX directory they're supposed to check all the > names? > > > So as far as I can see, I keep thinking it's orthogonal: > - devices get a link as /sys/bus/iio/devices/iio:deviceX ; which contains: > * 'name', set by driver (some have an index but many are constant), and > does not have to be unique, > * 'label' contains whatever was set as label if set > * 'of_node', a symlink to the device tree node which is what we > currently use to differentiate devices in our code > - triggers get /sys/bus/iio/devices/triggerX, which has a 'name' file > that probably must be unique (as it's can be written in device's > trigger/current_trigger to select it) > > > I'm sure we can make something work out while preserving compatibility, > > the patch I sent might not be great but it wouldn't bother anyone not > > using said aliases. > > > > aliases are apparently not appropriate for this (still not sure why?), > > but if for example labels are better we could keep the current > > iio:deviceX path (/sys/bus/iio/devices/iio:device0) with a label file in > > it as current software expect, but add a brand new directory with a link > > named as per the label itself (for example /sys/class/iio/<label>; > > my understanding is that /sys/class is meant for "easier" links and we > > don't have anything iio-related there yet) > > I've looked at this /sys/class/iio idea (could use the label or fallback > to iio:deviceX for devices, and name for triggers), but /sys/class seems > to be entierly managed by the linux core driver framework so that > doesn't leave much room for compromise... > The links there use the device name (so iio:deviceX for devices), and if > creating such a link fails it'll also fail the whole device creation > (cdev_device_add() -> device_add() -> device_add_class_symlinks()), so > my evil plan is foiled. (/sys/bus/iio/devices links are also > automatically created by device_add() -> bus_add_device() from the > device name) > > > I guess we could manage another new directory somewhere or haphazardly > create extra redundant links in the current bus directory, but that's > not exactly something I'd consider workable given there is no possible > deprecation path down the road, so ultimately I still think the aliases > patch I sent is amongst the least bad options we have here: > - there's currently no alias for iio so it won't break anything; > - even if one adds some on a device with multiple iio devices all that > can happen is some indices will be shuffled, but paths will still be > compatible with all current applications. > > > Did you have time to think about this or another possible way forward? > How about using udev rules to create symlinks for each device based on the label attribute? No changes to the kernel are needed.
