linux-kernel - Re: [PATCH v5 2/3] usb: serial: xr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdavm7GG8HdV1xk0W_b1EzUmvF0kKAGnp0u6t42NAWa9iA@mail.gmail.com>
Date:   Tue, 8 Dec 2020 13:41:52 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Johan Hovold <johan@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>
Cc:     Manivannan Sadhasivam <mani@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-usb <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        patong.mxl@...il.com,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Angelo Dureghello <angelo.dureghello@...esys.com>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] usb: serial: xr_serial: Add gpiochip support

On Tue, Dec 8, 2020 at 10:57 AM Johan Hovold <johan@...nel.org> wrote:
> [Me]

> > A better approach might be to create an array of names
> > prepended with something device-unique like the USB
> > bus topology? Or do we need a helper to help naming the
> > GPIOs? What would be helpful here?
> >
> > name = kasprintf(GFP_KERNEL, "%s-NAME", topology_str);
>
> Well we started discussing this back when we only had the sysfs
> interface which suffered from the same problem. I thought the chardev
> interface was supposed to get rid of the assumption of a flat name
> space? Perhaps in v3 of the ABI. ;P

It's "mostly true" that the line names are unique per-chip actually,
because people don't like the nasty warning message. I wonder
if anything would really break if I go in and make a patch to
enforce it, since all drivers passing ->names in the gpiochip
are in the kernel we can check them all.

If the names are unique-per-chip, we can add a restriction like this
with the requirement:

depends on !GPIO_SYSFS

so it can't even be compiled in if someone is using the sysfs.

That should solve the situation where people are (ab)using
the sysfs and getting name collisions as a result.

Then it should be fine for any driver to provide a names array
provided all the names are unique on that gpiochip.

I doubt it would break anything, but let's see what Geert says.
He has some special usecases in the gpio-aggregator driver
which will incidentally look for just linenames when
aggregating gpios, but I feel it is a bit thick for it to work
with multiple hot-pluggable GPIO chips as well, I don't think
that is its usecase. (We all want to be perfect but...)

> But what about any other non-pluggable
> IC, which provides a few named GPIO lines and of which there could be
> more than one in a system?

I think if there are such, and the lines are unique per-chip
we should make the drivers depend on !GPIO_SYSFS.

> The topology is already encoded in sysfs and it seems backwards to have
> each and every gpio driver reconstruct it.

I agree.

I think if this driver already has unique line-names per-gpiochip
we could actually make it depend on !GPIO_SYSFS and
just add the names.

Yours,
Linus Walleij