Re: [PATCH 0/5] RTL8231 GPIO expander support

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Sander Vanheule <sander@svanheule.net>
Cc: Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
	Lee Jones <lee.jones@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/5] RTL8231 GPIO expander support
Date: Mon, 17 May 2021 11:13:53 +0300	[thread overview]
Message-ID: <CAHp75VdS63n55b3RG7ZL1=uVgB=7ZiBtTOmV0t+5hw6xKoLFSg@mail.gmail.com> (raw)
In-Reply-To: <b7d39a26bc3a90de4d8e4fdbf8f944de43d12a56.camel@svanheule.net>

On Mon, May 17, 2021 at 12:40 AM Sander Vanheule <sander@svanheule.net> wrote:
> On Wed, 2021-05-12 at 18:29 +0300, Andy Shevchenko wrote:
> > On Tuesday, May 11, 2021, Sander Vanheule <sander@svanheule.net> wrote:

...

> > I have several comments to the series, but I may give them next week.
> >
> > Just couple here:
> > 1. If subsystem provides a regmap API I would suggest to use it, I.o.w. try
> > again to understand what is wrong with MDIO case.
>
> Are you referring to the MDIO regmap interface, or the GPIO regmap interface?

MDIO

> For the MDIO regmap interface, I have been able to resolve the Kconfig
> dependency issue. So I can reintroduce that, if that's preferred over the
> solution in this v1.
>
> With an extra patch, I was able to use the gpio-regmap interface, dropping most
> of the GPIO code. The current gpio-regmap implementation makes the assumption
> that an output value can be set while a pin is configured as an input. That
> assumption is invalid for this chip, so I had to provide an extra flag for
> gpio_regmap_config, similar to how this is handled in gpio-mmio.
>
>
> > 2. Please, switch to fwnode API in LED driver
>
> Since you had the same comment on my previous patch set, I had already tried to
> this this into account as much as possible.
>
> There's a few things I couldn't find the fwnode-equivalent for:
>  * I use of_node_name_prefix to enforce the naming required by the binding. I
>    could just walk over all (available) child nodes, which would be mostly
>    equivalent.

AFAIU the LED traditional bindings is that you define LED compatible
nodes and all child nodes of it are the one-per-LED ones, there
shouldn't be others.

>  * To get the address of an LED child node, I use of_get_address, since this
>    appeared to provide what I want to do: get the address of the node. I know
>    next to nothing about ACPI. Does the equivalent exist there? Or am I taking
>    the wrong approach?

What are the means of an address in this case?

> I have updated patches ready, if you would rather just review a v2.

-- 
With Best Regards,
Andy Shevchenko