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

[Samuel Holland]

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.

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,
>>   };
>>     /**
>>
>