Message-ID: <20240401174756.0000786a@Huawei.com>
Date: Mon, 1 Apr 2024 17:47:56 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Dominique Martinet <dominique.martinet@...ark-techno.com>
CC: Jonathan Cameron <jic23@...nel.org>, 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

On Mon, 1 Apr 2024 17:18:31 +0900
Dominique Martinet <dominique.martinet@...ark-techno.com> wrote:

> 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?

Yes.  Just somewhere other than /dev because I agree that feels wrong.
> 
> > 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.

Agreed - this does rely on that wildcard.

> 
> 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.

Agreed they have changed in the past. Mostly a case of fixing up
devices that didn't have their parents set (there are lots of those left
- that reminds me that I was cleaning up perf drivers that do this wrong
last year and only got part way through it :(

We will keep the /sys/bus/iio/devices/ path the same and continue to
support all paths below there (there are already compatibility links
in place for buffers for example from when we added multiple buffer support).
However, sysfs docs are pretty blunt on not relying on reliable paths for
classes (and the IIO bus is effectively a class from this point of view).

> 
> 
> 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.

Good luck.  If you have time it might be good to hear what you end up
with!

Thanks,

Jonathan

> 
> 
> Thanks,

Powered by blists - more mailing lists