On Mon, 2021-05-17 at 11:13 +0300, Andy Shevchenko wrote:

On Mon, May 17, 2021 at 12:40 AM Sander Vanheule [off-list ref] wrote:

quoted

On Wed, 2021-05-12 at 18:29 +0300, Andy Shevchenko wrote:

quoted

On Tuesday, May 11, 2021, Sander Vanheule [off-list ref] wrote:

...

quoted

quoted

 * > > > 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

quoted

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.

quoted

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.

OK, then I can just iterate over all child fwnodes.

quoted

 * 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?

The chip appears to be intended for use with ethernet switches. The registers
are organised to into a few groups, to provide 2 or 3 status LEDs per switch
port:

 * "LED0" group for 32 ports,
 * "LED1" group for 32 ports,
 * "LED2" group for 24 ports

The number of LEDs that can be used depends on the output mode, so I use a two-
part <#PORT #LED> address, resembling how this is defined by Realtek.

A single linear LED address space would get awkward gaps in bi-color mode (where
only the lower 24 ports can be used), but would still require addresses to be
able to specify which LED is where. For example in case the user want to link
them to a phy trigger for a specific switch port.

Best,
Sander