[PATCH 3/4] gpio: sunxi: Implement .set_flags

[Andre Przywara]

On Fri, 5 Nov 2021 16:46:07 -0500
Samuel Holland <samuel at sholland.org> wrote:

> On 11/5/21 9:43 AM, Heinrich Schuchardt wrote:
> > On 10/21/21 06:52, Samuel Holland wrote:  
> >> This, along with gpio_flags_xlate(), allows the GPIO driver to handle
> >> pull-up/down flags provided by consumer drivers or in the device tree.
> >>
> >> Signed-off-by: Samuel Holland <samuel at sholland.org>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >> ---
> >>
> >>   drivers/gpio/sunxi_gpio.c | 62 +++++++++++++++++----------------------
> >>   1 file changed, 27 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c
> >> index cdbc40d48f..92fee0118d 100644
> >> --- a/drivers/gpio/sunxi_gpio.c
> >> +++ b/drivers/gpio/sunxi_gpio.c
> >> @@ -139,27 +139,6 @@ int sunxi_name_to_gpio(const char *name)
> >>       return ret ? ret : gpio;
> >>   }
> >>   -static int sunxi_gpio_direction_input(struct udevice *dev, unsigned
> >> offset)
> >> -{
> >> -    struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> -
> >> -    sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_INPUT);
> >> -
> >> -    return 0;
> >> -}
> >> -
> >> -static int sunxi_gpio_direction_output(struct udevice *dev, unsigned
> >> offset,
> >> -                       int value)
> >> -{
> >> -    struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> -    u32 num = GPIO_NUM(offset);
> >> -
> >> -    sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_OUTPUT);
> >> -    clrsetbits_le32(&plat->regs->dat, 1 << num, value ? (1 << num) : 0);
> >> -
> >> -    return 0;
> >> -}
> >> -
> >>   static int sunxi_gpio_get_value(struct udevice *dev, unsigned offset)
> >>   {
> >>       struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> @@ -172,16 +151,6 @@ static int sunxi_gpio_get_value(struct udevice
> >> *dev, unsigned offset)
> >>       return dat & 0x1;
> >>   }
> >>   -static int sunxi_gpio_set_value(struct udevice *dev, unsigned offset,
> >> -                int value)
> >> -{
> >> -    struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> -    u32 num = GPIO_NUM(offset);
> >> -
> >> -    clrsetbits_le32(&plat->regs->dat, 1 << num, value ? (1 << num) : 0);
> >> -    return 0;
> >> -}
> >> -
> >>   static int sunxi_gpio_get_function(struct udevice *dev, unsigned
> >> offset)
> >>   {
> >>       struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> @@ -205,18 +174,41 @@ static int sunxi_gpio_xlate(struct udevice *dev,
> >> struct gpio_desc *desc,
> >>       if (ret)
> >>           return ret;
> >>       desc->offset = args->args[1];
> >> -    desc->flags = args->args[2] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW
> >> : 0;
> >> +    desc->flags = gpio_flags_xlate(args->args[2]);
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +static int sunxi_gpio_set_flags(struct udevice *dev, unsigned int
> >> offset,
> >> +                ulong flags)
> >> +{
> >> +    struct sunxi_gpio_plat *plat = dev_get_plat(dev);
> >> +
> >> +    if (flags & GPIOD_IS_OUT) {
> >> +        u32 value = !!(flags & GPIOD_IS_OUT_ACTIVE);
> >> +        u32 num = GPIO_NUM(offset);
> >> +
> >> +        clrsetbits_le32(&plat->regs->dat, 1 << num, value << num);
> >> +        sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_OUTPUT);
> >> +    } else if (flags & GPIOD_IS_IN) {
> >> +        u32 pull = 0;
> >> +
> >> +        if (flags & GPIOD_PULL_UP)
> >> +            pull = 1;
> >> +        else if (flags & GPIOD_PULL_DOWN)
> >> +            pull = 2;
> >> +        sunxi_gpio_set_pull_bank(plat->regs, offset, pull);
> >> +        sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_INPUT);
> >> +    }
> >>         return 0;
> >>   }
> >>     static const struct dm_gpio_ops gpio_sunxi_ops = {
> >> -    .direction_input    = sunxi_gpio_direction_input,
> >> -    .direction_output    = sunxi_gpio_direction_output,  
> > 
> > Removing these operations is not related to your commit message.
> > 
> > dm_gpio_set_value() seems to be using them.  
> 
> It does not use any of these operations when set_flags is provided. The
> same applies to _dm_gpio_set_flags().
> 
> asm-generic/gpio.h says about set_flags:
> > This method is required and should be implemented by new drivers. At
> > some point, it will supersede direction_input() and
> > direction_output(), which wil be removed.  
> 
> So I believe it is intended to replace the other three operations.

That's what I get from the code and this header file comment as well.

Reviewed-by: Andre Przywara <andre.przywara at arm.com>

Cheers,
Andre

> 
> Regards,
> Samuel
> 
> >>       .get_value        = sunxi_gpio_get_value,
> >> -    .set_value        = sunxi_gpio_set_value,  
> > 
> > Same here.
> > 
> > Best regards
> > 
> > Heinrich
> >   
> >>       .get_function        = sunxi_gpio_get_function,
> >>       .xlate            = sunxi_gpio_xlate,
> >> +    .set_flags        = sunxi_gpio_set_flags,
> >>   };
> >>     /**
> >>  
> >   
>