Re: [PATCH v1 2/4] gpio: dwapb: Read GPIO base from gpio-base property

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Rob Herring <robh+dt@kernel.org>,
	Lee Jones <lee.jones@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Hoan Tran <hoan@os.amperecomputing.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v1 2/4] gpio: dwapb: Read GPIO base from gpio-base property
Date: Mon, 2 Aug 2021 18:52:28 +0300	[thread overview]
Message-ID: <CAHp75Vcz=vkaGObUcOOTZA51pHPoMc50RpPBLg4fgZgHdUStRA@mail.gmail.com> (raw)
In-Reply-To: <20210802135839.4clqd34npppwasyh@mobilestation>

On Mon, Aug 2, 2021 at 5:14 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> On Mon, Jul 26, 2021 at 03:54:34PM +0300, Andy Shevchenko wrote:
> > For backward compatibility with some legacy devices introduce
> > a new (*) property gpio-base to read GPIO base. This will allow
> > further cleanup of the driver.

Thanks for the review! My answers below.

> > *) Note, it's not new for GPIO library since mockup driver is
> >    using it already.
>
> You are right but I don't think it's a good idea to advertise the
> pure Linux-internal property "gpio-base" to any use-case like OF
> and ACPI FW nodes.

I don't want to advertise them, actually (that's why no bindings are
modified). Perhaps introducing a paragraph in the GPIO documentation
about this (and / or in GPIO generic bindings) that gpio-base property
is solely for internal use and should't be used in actual DTs?

>  Especially seeing we don't have it described in the
> DT-bindings and noting that the mockup driver is dedicated for the
> GPIO tests only. What about restricting the property usage for the
> SW-nodes only by adding an additional check: is_software_node() here?

I don't think we need this. But if you think it's better this way just
to avoid usage of this property outside of internal properties, I'm
fine to add. Perhaps we may issue a warning and continue? (see also
above)

> @Linus, @Bartosz, @Rob, what do you think about that?

-- 
With Best Regards,
Andy Shevchenko