Re: [PATCH v2] gpio: mt7621: support gpio-line-names property

From: Linus Walleij <linus.walleij@linaro.org>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc: "Andy Shevchenko" <andy.shevchenko@gmail.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"John Thomson" <git@johnthomson.fastmail.com.au>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	NeilBrown <neil@brown.name>,
	"René van Dorst" <opensource@vdorst.com>,
	"Nicholas Mc Guire" <hofrat@osadl.org>
Subject: Re: [PATCH v2] gpio: mt7621: support gpio-line-names property
Date: Fri, 2 Jul 2021 12:18:12 +0200	[thread overview]
Message-ID: <CACRpkdaqSoyDUn3dVuVgzRK_7AabdY=1FzAnhHZzPs3qS+GfsA@mail.gmail.com> (raw)
In-Reply-To: <CAMhs-H_Ne4W79Awbmo6w_68X+h0-ybjvzNsbh=XuHMPJJ8-hDQ@mail.gmail.com>

On Fri, Jul 2, 2021 at 11:40 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
> On Fri, Jul 2, 2021 at 11:27 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

> > > There are no child nodes, all the stuff is in the same parent node
> > > and, as I said, belongs to the same device but internally uses three
> > > gpiochips.
> >
> > And it can't be split into three children in the overlay?
>
> Original code before this being mainlined was using three children and
> I was told in the review that three children were not allowed:
>
> See https://patchwork.ozlabs.org/project/linux-gpio/patch/1527924610-13135-3-git-send-email-sergio.paracuellos@gmail.com/#1932827

Yeah this is one of those unfortunate cases where the DT representation
(or ACPI for that matter) of the device and the Linux internal representation
differs.

> > Let's assume it can't, then the GPIO library function should be
> > refactored in a way that it takes parameters like base index for the
> > names and tries to satisfy the caller.
>
> Bartosz, Linus, any thoughts on this?

This would be ideal, is there a reasonably simple way to get to this helper?

In gpiolib.c devprop_gpiochip_set_names() need to be refactored to
take a base number, devprop_gpiochip_set_names_base() that
function exposed in <linux/gpio/driver.h> and then the old function
devprop_gpiochip_set_names() wrapped in the new
one so all old users continue to work without modification.
Sprinkle some kerneldoc on top so we do not make mistakes
in the future.

This should work I think.

Any similar drivers (several gpio_chip per FW node) that want to
set line names need to do the same thing.

Sorry that you ran into this, I hate it when I'm first at hairy stuff
like this.

Yours,
Linus Walleij