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: Mon, 1 Apr 2024 17:18:31 +0900
From: Dominique Martinet <dominique.martinet@...ark-techno.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	David Lechner <dlechner@...libre.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.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

Jonathan Cameron wrote on Sun, Mar 31, 2024 at 03:20:42PM +0100:
> Hi, got back to this finally...

Thank you for taking the time to express your thoughts!

> So the problems compared to other 'alias' users is that IIO is a bit more
> complex than for example LEDs.  A single DT node/compatible (or equivalent) can
> result in a 1+ IIO devices and 1+ triggers.

Right. I'm no longer really arguing for it at this point, but for
comparison in the patch I sent, the alias sets the start of the idr for
the device index, so if you have a driver that creates two IIO devices
you could just "reserve" two for this DT node and assuming the order
within this node is constant you'd still get constant numbering, so I
think it still somewhat holds up here.

For triggers though the numbers are separate and it wouldn't make sense
to use the same alias, if we wanted a coherent design with this we'd
need to add a second alias (such as iio_trigger = ..), but that makes
much less sense to me given they're also likely to be dynamically
instancied via configfs from what I've seen; I wouldn't want to do this
kind of mapping, so I agree with you.

> So I've messed around a bit and can think of various possible options to make
> this simpler.
> 1) Use a tmpfs mount and link from that.
>    Now we 'could' put an alias directory somewhere under /sys/bus/iio/ that
>    is a mount point created via sysfs_create_mount_point() - I abused the
>    /sys/kernel/debug directory to test this (unmounted debugfs and mounted
>    a tmpfs).  That would provide somewhere in sysfs that allows suitable
>    links. However, this is unusual so likely to be controversial.

Agreed that's probably not something we want to put our hands into.

> 2) Alternatively the relevant platform could create one of these somewhere
>    outside of sysfs and use udev rules to create the links.

I'm not sure I understood this one, something akin to the udev rules
I've showed that made links to the /sys iio device in /dev?
"relevant platform" here would be vendors?

> 3) Stick to the oddity of doing it under /dev/
> 4) Access the things in the first place via more stable paths?
>   /sys/bus/i2c/devices/i2c-0/0-0008/iio\:device?/ etc 
>    Relying on the alias support for i2c bus numbering to make that stable should work
>    and if you are sure there will only be one entry (most devices) that matches
>    the wild card, should be easy enough to use in scripts.
> 
> My personal preference is this last option.  Basically if you want stable paths
> don't use /sys/bus/iio/devices/ to get them.

Hmm, I wouldn't call that path stable given the '?' portion can change,
but at least that certainly is a single glob away so it's definitely
simpler than checking every labels all the time.

My second nitpick with this is that while these paths are stable for a
given kernel version, but we've had some paths changes over many years
(not sure if it was 3.14 or 4.9 but one of these perhaps didn't have
/sys/devices/platform yet? and things got moved there at some point with
some subtle name changes, breaking a couple of scripts).
OTOH /sys/bus/iio/devices feels less likely to change, but I guess this
is something that'd come up on tests when preparing a new kernel anyway,
so this is probably acceptable; I'm just thinking out loud.

With that said I can't think of anything that'd work everywhere either,
so I guess we can stick to the status-quo and I'll rediscuss what we
want to do with coworkers.

Thanks,
-- 
Dominique