lkml.org 
[lkml]   [2021]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 0/5] RTL8231 GPIO expander support
    From
    Date
    On Wed, 2021-05-12 at 18:29 +0300, Andy Shevchenko wrote:
    >
    >
    > On Tuesday, May 11, 2021, Sander Vanheule <sander@svanheule.net> wrote:
    > > 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

    \
     
     \ /
      Last update: 2021-05-16 23:41    [W:4.269 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site