RE: [PATCH v2 07/10] gpio: bd9571mwv: Add BD9574MWF support

[Yoshihiro Shimoda]

Hi Matti-san,

Thank you for your review!

> From: Vaittinen, Matti, Sent: Friday, December 11, 2020 9:55 PM
> 
> On Fri, 2020-12-11 at 20:27 +0900, Yoshihiro Shimoda wrote:
> > Add support for BD9574MWF which is silimar chip with BD9571MWV.
> > Note that BD9574MWF has an additional feature, but doesn't
> > support it for now.
> 
> nit:
> Perhaps mention which feature?

BD9574MWF GPIO[01] have 4 functions like below.
 1) GPIO, 2) "RECOV_GPOUT", 3) "FREQSEL", 4) "RTC_IN"

It seems "pinctrl" features though and I don't know
these features in detail for now.

> And I think you mean the driver does not support it yet?

You're correct. Now this driver only support the 1) GPIO.

> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> 
> FWIW:
> Reviewed-By: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>

Thank you for your review!

> > ---
> >  drivers/gpio/gpio-bd9571mwv.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-bd9571mwv.c b/drivers/gpio/gpio-
> > bd9571mwv.c
> > index 0e5395f..df6102b 100644
> > --- a/drivers/gpio/gpio-bd9571mwv.c
> > +++ b/drivers/gpio/gpio-bd9571mwv.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  /*
> > - * ROHM BD9571MWV-M GPIO driver
> > + * ROHM BD9571MWV-M and BD9574MWF-M GPIO driver
> >   *
> >   * Copyright (C) 2017 Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> >   *
> > @@ -10,6 +10,7 @@
> >   */
> >
> >  #include <linux/gpio/driver.h>
> > +#include <linux/mfd/rohm-generic.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >
> > @@ -118,7 +119,8 @@ static int bd9571mwv_gpio_probe(struct
> > platform_device *pdev)
> >  }
> >
> >  static const struct platform_device_id bd9571mwv_gpio_id_table[] = {
> > -	{ "bd9571mwv-gpio", },
> > +	{ "bd9571mwv-gpio", ROHM_CHIP_TYPE_BD9571 },
> > +	{ "bd9574mwf-gpio", ROHM_CHIP_TYPE_BD9574 },
> 
> I guess these CHIP_TYPES are used by subsequent patches?
> 
> I guess this means the existing functionality in both chips is same,
> right? (GPIO register addresses etc? - I don't have BD9571 data-sheet
> so I can't check)

Yes, the existing functionality in both chips is same.
GPIO register addresses and bits are the same.
Note that BD9574MWF has one more register, but the driver
doesn't use it for now.

Best regards,
Yoshihiro Shimoda