| Message ID | 20211021045258.30757-4-samuel@sholland.org |
|---|---|
| State | Accepted |
| Commit | 35ae126c16a6a9149edc6638faaa247f67b8a400 |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | gpio: sunxi: Handle pin configuration flags | expand |
On Wed, 20 Oct 2021 at 22:53, Samuel Holland <samuel@sholland.org> 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@sholland.org> > --- > > drivers/gpio/sunxi_gpio.c | 62 +++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 35 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org>
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@sholland.org> > Reviewed-by: Simon Glass <sjg@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_set_gpio_value() seems to be using them. > .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, > }; > > /** >
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@sholland.org> >> Reviewed-by: Simon Glass <sjg@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, >> }; >> /** >> >
On Fri, 5 Nov 2021 16:46:07 -0500 Samuel Holland <samuel@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@sholland.org> > >> Reviewed-by: Simon Glass <sjg@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@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, > >> }; > >> /** > >> > > >
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, .get_value = sunxi_gpio_get_value, - .set_value = sunxi_gpio_set_value, .get_function = sunxi_gpio_get_function, .xlate = sunxi_gpio_xlate, + .set_flags = sunxi_gpio_set_flags, }; /**
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@sholland.org> --- drivers/gpio/sunxi_gpio.c | 62 +++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 35 deletions(-)