linux-kernel - Re: [PATCH] iio: industrialio-core: look for aliases to request device index

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 15 Mar 2024 14:47:38 +0900
From: Dominique Martinet <dominique.martinet@...ark-techno.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
	Jonathan Cameron <jic23@...nel.org>,
	Syunya Ohshio <syunya.ohshio@...ark-techno.com>,
	Guido Günther <agx@...xcpu.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: industrialio-core: look for aliases to request
 device index

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