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? Thanks, -- Dominique
