Re: [PATCH v9 5/8] gpio: Initial support for ROHM bd70528 GPIO block

[Matti Vaittinen]

Thanks a bunch Linus,

On Thu, Feb 14, 2019 at 09:00:33AM +0100, Linus Walleij wrote:
> On Wed, Feb 13, 2019 at 2:34 PM Matti Vaittinen
> <matti.vaittinen@xxxxxxxxxxxxxxxxx> wrote:
> 
> > ROHM BD70528 PMIC has 4 GPIO pins. Allow them to be
> > controlled by GPIO framework.
> >
> > IRQs are handled by regmap-irq and GPIO driver is not
> > aware of the irq usage.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> 
> (...)
> 
> Just spotted this:
> 
> > +#include <linux/gpio/driver.h>
> > +#include <linux/gpio.h>
> 
> A driver should only need <linux/gpio/driver.h>
> the <linux/gpio.h> should not be used at all in new code,
> it is a legacy header.

Allright. I'll do v10 and drop the header. Thanks for pointing this out

> > +static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
> > +                                  unsigned long config)
> > +{
> > +       struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> > +
> > +       switch (pinconf_to_config_param(config)) {
> > +       case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> > +               return regmap_update_bits(bdgpio->chip.regmap,
> > +                                         GPIO_OUT_REG(offset),
> > +                                         BD70528_GPIO_DRIVE_MASK,
> > +                                         BD70528_GPIO_OPEN_DRAIN);
> > +               break;
> > +       case PIN_CONFIG_DRIVE_PUSH_PULL:
> > +               return regmap_update_bits(bdgpio->chip.regmap,
> > +                                         GPIO_OUT_REG(offset),
> > +                                         BD70528_GPIO_DRIVE_MASK,
> > +                                         BD70528_GPIO_PUSH_PULL);
> > +               break;
> > +       case PIN_CONFIG_INPUT_DEBOUNCE:
> > +               return bd70528_set_debounce(bdgpio, offset,
> > +                                           pinconf_to_config_argument(config));
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +       return -ENOTSUPP;
> > +}
> 
> BTW I just merged code from Thomas Petazzoni that make it possible to
> also support pull-up and pull-down from GPIO here if you want to enable
> it.

Thanks for heads up Linus! I'll check it out although I don't want to
add up more changes in this series... But my initial feeling is that
this will deserve another patch (set) when initial support for BD70528
is available. (This answer is not locked, it may be I will fit the
change in this series if I am not buried under other duties ;] )

Br,
	Matti Vaittinen


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes.  Just then, he vanished ~~~