Received-SPF: pass (google.com: domain of linux-kernel+bounces-104600-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240228051254.3988329-1-dominique.martinet@atmark-techno.com>
 <7f03bb12-0976-4cb7-9ca9-4e4e28170bdd@linaro.org> <Zd7hSOw3_zosyrn3@atmark-techno.com>
 <daed8ada-9e01-41ad-82af-5da5cbbc865c@linaro.org> <Zd7qz1Qte8HWieF_@atmark-techno.com>
 <20240228142441.00002a79@Huawei.com> <Zd_zB_ymxkx0HB3q@atmark-techno.com> <ZfPg-nMANUtBlr6S@atmark-techno.com>
In-Reply-To: <ZfPg-nMANUtBlr6S@atmark-techno.com>
From: David Lechner <dlechner@baylibre.com>
Date: Fri, 15 Mar 2024 10:53:36 -0500
Message-ID: <CAMknhBG_kJx8JPvTBQo7zpy3mFAkUjZpRY3DLBfXt+39nRJWiA@mail.gmail.com>
Subject: Re: [PATCH] iio: industrialio-core: look for aliases to request
 device index
To: Dominique Martinet <dominique.martinet@atmark-techno.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Jonathan Cameron <jic23@kernel.org>, 
	Syunya Ohshio <syunya.ohshio@atmark-techno.com>, =?UTF-8?Q?Guido_G=C3=BCnther?= <agx@sigxcpu.org>, 
	Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 15, 2024 at 12:58=E2=80=AFAM Dominique Martinet
<dominique.martinet@atmark-techno.com> 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 (regi=
sters
> > > 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), bu=
t
> > 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 i=
n
> > it as current software expect, but add a brand new directory with a lin=
k
> > 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.