Re: [PATCH v5 2/3] usb: serial: xr_serial: Add gpiochip support

[Johan Hovold <johan@kernel.org>]

On Sat, Dec 12, 2020 at 01:03:32AM +0100, Linus Walleij wrote:
> On Thu, Dec 10, 2020 at 9:53 AM Johan Hovold <johan@kernel.org> wrote:
> > On Wed, Dec 09, 2020 at 05:25:32PM +0100, Linus Walleij wrote:
> 
> > I just replied to that thread, but to summarize, you can't rely on
> > having the sysfs code detect collisions since that will trigger a bunch
> > of nasty warnings and backtraces. We also don't want the sysfs interface
> > for a specific USB device to depend on probe order (only the first one
> > plugged in gets to use the line names). And adding line names now could
> > in fact be what breaks currently working scripts.
> 
> Yes the sysfs ABI is very volatile and easy to break.
> 
> As pointed out in the other reply, sysfs base GPIO number is all
> wibbly-wobbly on anything hot-pluggable so in a way I feel it
> is the right thing to disallow sysfs altogether on hotpluggable
> devices.

No, the gpio numbers will of course vary, but since gpiolib exports the
base number for the chip, a scripts can easily determine the right gpio
number as base + offset.

Having probe order break that by sometimes exporting the line using it's
name is what would be a problem.

> > Just did a super quick check and it seems libgpiod still assumes a flat
> > name space. For example, gpiod_chip_find_line() returns only the first
> > line found that matches a name. Shouldn't be impossible to extend, but
> > just want to make sure this flat namespace assumption hasn't been to
> > heavily relied upon.
> 
> The unique way to identify a GPIO is gpiochip instance (with
> topology from sysfs) and then a line number on that chip.
> This is done e.g. in the example tool
> tools/gpio/gpio-hammer.c
> 
> As you can see the tool doesn't use these line names.
>
> The line names are really like symbolic links or something.
> But they are indeed in a flat namespace so we should try to
> at least make them unique if it turns out people love to use
> these.

Not necessarily, they could be unique per chip as we're discussing here
with respect to hotpluggable controllers. We just can't have it both
ways.

> As it is now system designers mostly use device tree to assign
> line names and they try to make these unique because they don't
> like the nasty warnings from gpiolib.
>
> If I google for the phrase "Detected name collision for GPIO name"
> I just find the code, our discussions and some USB serial devices
> warning about this so far.
> 
> Maybe we should just make a patch to disallow it?

That would make it impossible to provide name lines on hotpluggable
controllers, which would be nice to support.

Johan