On Fri, 5 Nov 2021 16:46:07 -0500
Samuel Holland <sam...@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 <sam...@sholland.org>
> >> Reviewed-by: Simon Glass <s...@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.przyw...@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,
> >> };
> >> /**
> >>
> >
>
