Re: [PATCH v4 09/16] gpio: support ROHM BD71815 GPOs

From: Matti Vaittinen
Date: Thu Mar 25 2021 - 06:32:53 EST


Hello Linus,

On Thu, 2021-03-25 at 10:35 +0100, Linus Walleij wrote:
> On Wed, Mar 24, 2021 at 8:29 AM Matti Vaittinen
> <matti.vaittinen@xxxxxxxxxxxxxxxxx> wrote:
>
> > Support GPO(s) found from ROHM BD71815 power management IC. The IC
> > has two
> > GPO pins but only one is properly documented in data-sheet. The
> > driver
> > exposes by default only the documented GPO. The second GPO is
> > connected to
> > E5 pin and is marked as GND in data-sheet. Control for this
> > undocumented
> > pin can be enabled using a special DT property.
> >
> > This driver is derived from work by Peter Yang <
> > yanglsh@xxxxxxxxxxxxxxx>
> > although not so much of original is left.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > ---
> > Changes since v3:
> > - No changes
>
> This looks OK to me:
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>
> It could potentially (like the other Rohm GPIO MFD PMIC drivers)
> make some use of the gpio regmap library, but we have some
> pending changes for that so look into it after the next merge
> window.
>
> I.e. for your TODO: look at the GPIO_REGMAP helper.

I just took a quick peek at gpio_regmap and it looks pretty good to me!

Any particular reason why gpio_regmap is not just part of gpio_chip? I
guess providing the 'gpio_regmap_direction_*()', 'gpio_regmap_get()',
'gpio_regmap_set()' as exported helpers and leaving calling the
(devm_)gpiochip_add_data() to IC driver would have allowed more
flexibility. Drivers could then use the gpio_regamap features which fit
the IC (by providing pointers to helper functions in gpio_chip) - and
handle potential oddball-features by using pointers to some customized
functions in gpio_chip.

Anyways, definitely worth getting familiar with! Thanks for the pointer
:]

Best Regards,
Matti Vaittinen