Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 14 Dec 2020 09:58:40 +0100
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
Message-ID: <X9cpQO3IV4IgX1dh@localhost>
References: <20201122170822.21715-3-mani@kernel.org>
 <CACRpkdbY-aZB1BAD=JkZAHA+OQvpH12AD3tLAp6Nf1hwr74s9A@mail.gmail.com>
 <X8ZmfbQp7/BGgxec@localhost>
 <CACRpkdZJdxqxUEQaKUHctHRSQAUpYZJtuxonwVd_ZFAsLBbKrA@mail.gmail.com>
 <X89OOUOG0x0SSxXA@localhost>
 <CACRpkdavm7GG8HdV1xk0W_b1EzUmvF0kKAGnp0u6t42NAWa9iA@mail.gmail.com>
 <X9DsWahl6UDwZwBn@localhost>
 <CACRpkdYm-j9QcK8hgNrC33KruWE17Q0F4+T=UanE7PCEZEtu6w@mail.gmail.com>
 <X9HiGaIzk4UaZG7i@localhost>
 <CACRpkdZ6MUzRe9m=NrqA_5orhZXDtWj+qoFMHX7v6Zjsx-rVGg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACRpkdZ6MUzRe9m=NrqA_5orhZXDtWj+qoFMHX7v6Zjsx-rVGg@mail.gmail.com>
Precedence: bulk

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