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

From: Johan Hovold <johan@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Johan Hovold <johan@kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	linux-usb <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	patong.mxl@gmail.com,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Angelo Dureghello <angelo.dureghello@timesys.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH v5 2/3] usb: serial: xr_serial: Add gpiochip support
Date: Mon, 14 Dec 2020 09:58:40 +0100	[thread overview]
Message-ID: <X9cpQO3IV4IgX1dh@localhost> (raw)
In-Reply-To: <CACRpkdZ6MUzRe9m=NrqA_5orhZXDtWj+qoFMHX7v6Zjsx-rVGg@mail.gmail.com>

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