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

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

quoted

The RTL8231 GPIO and LED expander can be configured for use as an MDIO or
SMI
bus device. Currently only the MDIO mode is supported, although SMI mode
support should be fairly straightforward, once an SMI bus driver is
available.

Provided features by the RTL8231:
  - Up to 37 GPIOs
    - Configurable drive strength: 8mA or 4mA (currently unsupported)
    - Input debouncing on high GPIOs (currently unsupported)
  - Up to 88 LEDs in multiple scan matrix groups
    - On, off, or one of six toggling intervals
    - "single-color mode": 2×36 single color LEDs + 8 bi-color LEDs
    - "bi-color mode": (12 + 2×6) bi-color LEDs + 24 single color LEDs
  - Up to one PWM output (currently unsupported)
    - Fixed duty cycle, 8 selectable frequencies (1.2kHz - 4.8kHz)

There remain some log warnings when probing the device, possibly due to the
way
I'm using the MFD subsystem. Would it be possible to avoid these?
[    2.602242] rtl8231-pinctrl: Failed to locate of_node [id: -2]
[    2.609380] rtl8231-pinctrl rtl8231-pinctrl.0.auto: no of_node; not
parsing pinctrl DT

When no 'leds' sub-node is specified:
[    2.922262] rtl8231-leds: Failed to locate of_node [id: -2]
[    2.967149] rtl8231-leds rtl8231-leds.1.auto: no of_node; not parsing
pinctrl DT
[    2.975673] rtl8231-leds rtl8231-leds.1.auto: scan mode missing or
invalid
[    2.983531] rtl8231-leds: probe of rtl8231-leds.1.auto failed with error
-22

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?

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


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


Best,
Sander