* [PATCH v3] gpio: omap: handle pin config bias flags
@ 2020-07-17 19:40 Drew Fustini
2020-07-20 22:38 ` Gustavo A. R. Silva
0 siblings, 1 reply; 4+ messages in thread
From: Drew Fustini @ 2020-07-17 19:40 UTC (permalink / raw)
To: Tony Lindgren, Haojian Zhuang, Linus Walleij, Grygorii Strashko,
linux-omap, linux-gpio, Jason Kridner, Robert Nelson
Cc: Drew Fustini
Modify omap_gpio_set_config() to handle pin config bias flags by calling
gpiochip_generic_config().
The pin group for the gpio line must have the corresponding pinconf
properties:
PIN_CONFIG_BIAS_PULL_UP requires "pinctrl-single,bias-pullup"
PIN_CONFIG_BIAS_PULL_DOWN requires "pinctrl-single,bias-pulldown"
This is necessary for pcs_pinconf_set() to find the requested bias
parameter in the PIN_MAP_TYPE_CONFIGS_GROUP pinctrl map.
Signed-off-by: Drew Fustini <drew@beagleboard.org>
Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Link: https://lore.kernel.org/r/20200715213738.1640030-1-drew@beagleboard.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-omap.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
v3 changes:
- adjust the braces to match the correct coding style
- note: I originally re-submitted this as v2 by accident when it should
have been v3. Sorry for the noise.
v2 changes:
- simplify handling of -ENOTSUPP return value per Grygorii's suggestion
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index b8e2ecc3eade..0ccb31de0b67 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -896,12 +896,18 @@ static int omap_gpio_set_config(struct gpio_chip *chip, unsigned offset,
unsigned long config)
{
u32 debounce;
+ int ret = -ENOTSUPP;
+
+ if ((pinconf_to_config_param(config) == PIN_CONFIG_BIAS_DISABLE) ||
+ (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_UP) ||
+ (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_DOWN)) {
+ ret = gpiochip_generic_config(chip, offset, config);
+ } else if (pinconf_to_config_param(config) == PIN_CONFIG_INPUT_DEBOUNCE) {
+ debounce = pinconf_to_config_argument(config);
+ ret = omap_gpio_debounce(chip, offset, debounce);
+ }
- if (pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE)
- return -ENOTSUPP;
-
- debounce = pinconf_to_config_argument(config);
- return omap_gpio_debounce(chip, offset, debounce);
+ return ret;
}
static void omap_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] gpio: omap: handle pin config bias flags
2020-07-17 19:40 [PATCH v3] gpio: omap: handle pin config bias flags Drew Fustini
@ 2020-07-20 22:38 ` Gustavo A. R. Silva
2020-07-21 10:02 ` Drew Fustini
0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2020-07-20 22:38 UTC (permalink / raw)
To: Drew Fustini, Tony Lindgren, Haojian Zhuang, Linus Walleij,
Grygorii Strashko, linux-omap, linux-gpio, Jason Kridner,
Robert Nelson
Hi Drew,
Somehow I ran into this patch in Linus' tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=75dec56710dfafd37daa95e756c5d1840932ba90
Please, see some comments below...
On 7/17/20 14:40, Drew Fustini wrote:
> Modify omap_gpio_set_config() to handle pin config bias flags by calling
> gpiochip_generic_config().
>
> The pin group for the gpio line must have the corresponding pinconf
> properties:
>
> PIN_CONFIG_BIAS_PULL_UP requires "pinctrl-single,bias-pullup"
> PIN_CONFIG_BIAS_PULL_DOWN requires "pinctrl-single,bias-pulldown"
>
> This is necessary for pcs_pinconf_set() to find the requested bias
> parameter in the PIN_MAP_TYPE_CONFIGS_GROUP pinctrl map.
>
> Signed-off-by: Drew Fustini <drew@beagleboard.org>
> Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Link: https://lore.kernel.org/r/20200715213738.1640030-1-drew@beagleboard.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpio-omap.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> v3 changes:
> - adjust the braces to match the correct coding style
> - note: I originally re-submitted this as v2 by accident when it should
> have been v3. Sorry for the noise.
>
> v2 changes:
> - simplify handling of -ENOTSUPP return value per Grygorii's suggestion
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index b8e2ecc3eade..0ccb31de0b67 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -896,12 +896,18 @@ static int omap_gpio_set_config(struct gpio_chip *chip, unsigned offset,
> unsigned long config)
> {
> u32 debounce;
> + int ret = -ENOTSUPP;
> +
> + if ((pinconf_to_config_param(config) == PIN_CONFIG_BIAS_DISABLE) ||
> + (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_UP) ||
> + (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_DOWN)) {
> + ret = gpiochip_generic_config(chip, offset, config);
> + } else if (pinconf_to_config_param(config) == PIN_CONFIG_INPUT_DEBOUNCE) {
> + debounce = pinconf_to_config_argument(config);
> + ret = omap_gpio_debounce(chip, offset, debounce);
> + }
>
> - if (pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE)
> - return -ENOTSUPP;
> -
> - debounce = pinconf_to_config_argument(config);
> - return omap_gpio_debounce(chip, offset, debounce);
> + return ret;
> }
>
> static void omap_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
>
Maybe next time you could consider coding something like this, instead:
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8dd86b9fae53..7fbe0c9e1fc1 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -899,16 +899,18 @@ static int omap_gpio_set_config(struct gpio_chip *chip, unsigned offset,
u32 debounce;
int ret = -ENOTSUPP;
- if ((pinconf_to_config_param(config) == PIN_CONFIG_BIAS_DISABLE) ||
- (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_UP) ||
- (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_DOWN))
- {
+ switch (pinconf_to_config_param(config)) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ case PIN_CONFIG_BIAS_PULL_UP:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
ret = gpiochip_generic_config(chip, offset, config);
- }
- else if (pinconf_to_config_param(config) == PIN_CONFIG_INPUT_DEBOUNCE)
- {
+ break;
+ case PIN_CONFIG_INPUT_DEBOUNCE:
debounce = pinconf_to_config_argument(config);
ret = omap_gpio_debounce(chip, offset, debounce);
+ break;
+ default:
+ break;
}
return ret;
It looks a bit more readable and cleaner. :)
Thanks
--
Gustavo
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] gpio: omap: handle pin config bias flags
2020-07-20 22:38 ` Gustavo A. R. Silva
@ 2020-07-21 10:02 ` Drew Fustini
2020-07-21 22:07 ` Linus Walleij
0 siblings, 1 reply; 4+ messages in thread
From: Drew Fustini @ 2020-07-21 10:02 UTC (permalink / raw)
To: Gustavo A. R. Silva, Linus Walleij
Cc: Tony Lindgren, Haojian Zhuang, Grygorii Strashko, linux-omap,
linux-gpio, Jason Kridner, Robert Nelson
On Mon, Jul 20, 2020 at 05:38:51PM -0500, Gustavo A. R. Silva wrote:
> Hi Drew,
>
> Somehow I ran into this patch in Linus' tree:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=for-next&id=75dec56710dfafd37daa95e756c5d1840932ba90
>
> Please, see some comments below...
>
> On 7/17/20 14:40, Drew Fustini wrote:
> > Modify omap_gpio_set_config() to handle pin config bias flags by calling
> > gpiochip_generic_config().
> >
> > The pin group for the gpio line must have the corresponding pinconf
> > properties:
> >
> > PIN_CONFIG_BIAS_PULL_UP requires "pinctrl-single,bias-pullup"
> > PIN_CONFIG_BIAS_PULL_DOWN requires "pinctrl-single,bias-pulldown"
> >
> > This is necessary for pcs_pinconf_set() to find the requested bias
> > parameter in the PIN_MAP_TYPE_CONFIGS_GROUP pinctrl map.
> >
> > Signed-off-by: Drew Fustini <drew@beagleboard.org>
> > Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > Link: https://lore.kernel.org/r/20200715213738.1640030-1-drew@beagleboard.org
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > drivers/gpio/gpio-omap.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > v3 changes:
> > - adjust the braces to match the correct coding style
> > - note: I originally re-submitted this as v2 by accident when it should
> > have been v3. Sorry for the noise.
> >
> > v2 changes:
> > - simplify handling of -ENOTSUPP return value per Grygorii's suggestion
> >
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index b8e2ecc3eade..0ccb31de0b67 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -896,12 +896,18 @@ static int omap_gpio_set_config(struct gpio_chip *chip, unsigned offset,
> > unsigned long config)
> > {
> > u32 debounce;
> > + int ret = -ENOTSUPP;
> > +
> > + if ((pinconf_to_config_param(config) == PIN_CONFIG_BIAS_DISABLE) ||
> > + (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_UP) ||
> > + (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_DOWN)) {
> > + ret = gpiochip_generic_config(chip, offset, config);
> > + } else if (pinconf_to_config_param(config) == PIN_CONFIG_INPUT_DEBOUNCE) {
> > + debounce = pinconf_to_config_argument(config);
> > + ret = omap_gpio_debounce(chip, offset, debounce);
> > + }
> >
> > - if (pinconf_to_config_param(config) != PIN_CONFIG_INPUT_DEBOUNCE)
> > - return -ENOTSUPP;
> > -
> > - debounce = pinconf_to_config_argument(config);
> > - return omap_gpio_debounce(chip, offset, debounce);
> > + return ret;
> > }
> >
> > static void omap_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> >
>
> Maybe next time you could consider coding something like this, instead:
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 8dd86b9fae53..7fbe0c9e1fc1 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -899,16 +899,18 @@ static int omap_gpio_set_config(struct gpio_chip *chip, unsigned offset,
> u32 debounce;
> int ret = -ENOTSUPP;
>
> - if ((pinconf_to_config_param(config) == PIN_CONFIG_BIAS_DISABLE) ||
> - (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_UP) ||
> - (pinconf_to_config_param(config) == PIN_CONFIG_BIAS_PULL_DOWN))
> - {
> + switch (pinconf_to_config_param(config)) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + case PIN_CONFIG_BIAS_PULL_UP:
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> ret = gpiochip_generic_config(chip, offset, config);
> - }
> - else if (pinconf_to_config_param(config) == PIN_CONFIG_INPUT_DEBOUNCE)
> - {
> + break;
> + case PIN_CONFIG_INPUT_DEBOUNCE:
> debounce = pinconf_to_config_argument(config);
> ret = omap_gpio_debounce(chip, offset, debounce);
> + break;
> + default:
> + break;
> }
>
> return ret;
>
> It looks a bit more readable and cleaner. :)
>
> Thanks
> --
> Gustavo
Gustavo - thanks very much for the feedback. I appreciate getting these
insights into best practices.
Linus - should I submit a patch?
I'm not sure if it is better to limit churn, or make sure the code is
structured as best is possible.
Thanks,
Drew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] gpio: omap: handle pin config bias flags
2020-07-21 10:02 ` Drew Fustini
@ 2020-07-21 22:07 ` Linus Walleij
0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2020-07-21 22:07 UTC (permalink / raw)
To: Drew Fustini
Cc: Gustavo A. R. Silva, Tony Lindgren, Haojian Zhuang,
Grygorii Strashko, Linux-OMAP, open list:GPIO SUBSYSTEM,
Jason Kridner, Robert Nelson
On Tue, Jul 21, 2020 at 12:02 PM Drew Fustini <drew@beagleboard.org> wrote:
> Linus - should I submit a patch?
Yeps, normally always submit a patch to fix something that is applied
already.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-21 22:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17 19:40 [PATCH v3] gpio: omap: handle pin config bias flags Drew Fustini
2020-07-20 22:38 ` Gustavo A. R. Silva
2020-07-21 10:02 ` Drew Fustini
2020-07-21 22:07 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).