[PATCH] gpio: bd70528: remove unneeded break

[PATCH] gpio: bd70528: remove unneeded break

[trix]

From: Tom Rix <trix@redhat.com>

A break is not needed if it is preceded by a return

Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/gpio/gpio-bd70528.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-bd70528.c
index 45b3da8da336..931e5765fe92 100644
--- a/drivers/gpio/gpio-bd70528.c
+++ b/drivers/gpio/gpio-bd70528.c
@@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
 					  GPIO_OUT_REG(offset),
 					  BD70528_GPIO_DRIVE_MASK,
 					  BD70528_GPIO_OPEN_DRAIN);
-		break;
 	case PIN_CONFIG_DRIVE_PUSH_PULL:
 		return regmap_update_bits(bdgpio->chip.regmap,
 					  GPIO_OUT_REG(offset),
 					  BD70528_GPIO_DRIVE_MASK,
 					  BD70528_GPIO_PUSH_PULL);
-		break;
 	case PIN_CONFIG_INPUT_DEBOUNCE:
 		return bd70528_set_debounce(bdgpio, offset,
 					    pinconf_to_config_argument(config));
-		break;
 	default:
 		break;
 	}
-- 
2.18.1

Re: [PATCH] gpio: bd70528: remove unneeded break

[Vaittinen, Matti]

Thanks Tom,

On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:
> From: Tom Rix <trix@redhat.com>
> 
> A break is not needed if it is preceded by a return
> 
> Signed-off-by: Tom Rix <trix@redhat.com>
> ---
>  drivers/gpio/gpio-bd70528.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-
> bd70528.c
> index 45b3da8da336..931e5765fe92 100644
> --- a/drivers/gpio/gpio-bd70528.c
> +++ b/drivers/gpio/gpio-bd70528.c
> @@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct
> gpio_chip *chip, unsigned int offset,
>  					  GPIO_OUT_REG(offset),
>  					  BD70528_GPIO_DRIVE_MASK,
>  					  BD70528_GPIO_OPEN_DRAIN);
> -		break;
My personal taste is also to omit these breaks but I am pretty sure I
saw some tooling issuing a warning about falling through the switch-
case back when I wrote this. Most probably checkpatch didn't like that
back then. Anyways - if you have no warnings from any of the tools -
this indeed looks better (to me) without the break :)

>  	case PIN_CONFIG_DRIVE_PUSH_PULL:
>  		return regmap_update_bits(bdgpio->chip.regmap,
>  					  GPIO_OUT_REG(offset),
>  					  BD70528_GPIO_DRIVE_MASK,
>  					  BD70528_GPIO_PUSH_PULL);
> -		break;
>  	case PIN_CONFIG_INPUT_DEBOUNCE:
>  		return bd70528_set_debounce(bdgpio, offset,
>  					    pinconf_to_config_argument(
> config));
> -		break;
>  	default:

Actually - my personal taste would be to also get rid of the empty
default here - but I guess it was also added to make some tool happy...

>  		break;
>  	}

Acked-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Re: [PATCH] gpio: bd70528: remove unneeded break

[Andy Shevchenko]

On Tue, Oct 20, 2020 at 2:26 PM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
> On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:

> > -             break;
> My personal taste is also to omit these breaks but I am pretty sure I
> saw some tooling issuing a warning about falling through the switch-
> case back when I wrote this. Most probably checkpatch didn't like that
> back then. Anyways - if you have no warnings from any of the tools -
> this indeed looks better (to me) without the break :)

JFYI: it's a clang which actually *is* complaining for an extra break.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] gpio: bd70528: remove unneeded break

[Vaittinen, Matti]

On Tue, 2020-10-20 at 13:07 +0300, Matti Vaittinen wrote:
> Thanks Tom,
> 
> On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:
> > From: Tom Rix <trix@redhat.com>
> > 
> > A break is not needed if it is preceded by a return
> > 
> > Signed-off-by: Tom Rix <trix@redhat.com>
> > ---
> >  drivers/gpio/gpio-bd70528.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-
> > bd70528.c
> > index 45b3da8da336..931e5765fe92 100644
> > --- a/drivers/gpio/gpio-bd70528.c
> > +++ b/drivers/gpio/gpio-bd70528.c
> > @@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct
> > gpio_chip *chip, unsigned int offset,
> >  					  GPIO_OUT_REG(offset),
> >  					  BD70528_GPIO_DRIVE_MASK,
> >  					  BD70528_GPIO_OPEN_DRAIN);
> > -		break;
> My personal taste is also to omit these breaks but I am pretty sure I
> saw some tooling issuing a warning about falling through the switch-
> case back when I wrote this. Most probably checkpatch didn't like
> that
> back then.

I did a test and removed the breaks. Then I copied the modified file to
drivers/gpio/dummy.c
Next I committed this dummy.c in git, ran git-format-patch -s and
finally ran the checkpatch on this... Following was produced:


[mvaittin@localhost linux]$ scripts/checkpatch.pl 0001-gpio-add-
dummy.patch 
Traceback (most recent call last):
  File "scripts/spdxcheck.py", line 6, in <module>
    from ply import lex, yacc
ImportError: No module named ply
WARNING: added, moved or deleted file(s), does MAINTAINERS need
updating?
#15: 
new file mode 100644

WARNING: Possible switch case/default not preceded by break or
fallthrough comment
#91: FILE: drivers/gpio/dummy.c:72:
+	case PIN_CONFIG_DRIVE_PUSH_PULL:

WARNING: Possible switch case/default not preceded by break or
fallthrough comment
#96: FILE: drivers/gpio/dummy.c:77:
+	case PIN_CONFIG_INPUT_DEBOUNCE:

total: 0 errors, 3 warnings, 229 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-
inplace.

0001-gpio-add-dummy.patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

I guess that explains the odd "fallthrough" comments you mentioned in
another email. I guess the checkpatch should be fixed before you put
too much effort in clean-up...


And for peeps who have not been following - following function triggers
the checkpatch error above:

static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int
offset,
				   unsigned long config)
{
	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);

	switch (pinconf_to_config_param(config)) {
	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
		return regmap_update_bits(bdgpio->chip.regmap,
					  GPIO_OUT_REG(offset),
					  BD70528_GPIO_DRIVE_MASK,
					  BD70528_GPIO_OPEN_DRAIN);
	case PIN_CONFIG_DRIVE_PUSH_PULL:
		return regmap_update_bits(bdgpio->chip.regmap,
					  GPIO_OUT_REG(offset),
					  BD70528_GPIO_DRIVE_MASK,
					  BD70528_GPIO_PUSH_PULL);
	case PIN_CONFIG_INPUT_DEBOUNCE:
		return bd70528_set_debounce(bdgpio, offset,
					    pinconf_to_config_argument(
config));
	default:
		break;
	}
	return -ENOTSUPP;
}


Best Regards
	Matti Vaittinen

Re: [PATCH] gpio: bd70528: remove unneeded break

[Vaittinen, Matti]

On Tue, 2020-10-20 at 14:46 +0300, Andy Shevchenko wrote:
> On Tue, Oct 20, 2020 at 2:26 PM Vaittinen, Matti
> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:
> > > -             break;
> > My personal taste is also to omit these breaks but I am pretty sure
> > I
> > saw some tooling issuing a warning about falling through the
> > switch-
> > case back when I wrote this. Most probably checkpatch didn't like
> > that
> > back then. Anyways - if you have no warnings from any of the tools
> > -
> > this indeed looks better (to me) without the break :)
> 
> JFYI: it's a clang which actually *is* complaining for an extra
> break.
> 
Oh. I just replied before seeing this. So actually, checkpatch
complains about missing break and clang about existing break. I'm
getting much more nagging at work than at home!

Best Regards
	Matti

Re: [PATCH] gpio: bd70528: remove unneeded break

[Joe Perches]

On Tue, 2020-10-20 at 11:48 +0000, Vaittinen, Matti wrote:
> On Tue, 2020-10-20 at 13:07 +0300, Matti Vaittinen wrote:
> > Thanks Tom,
> > 
> > On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:
> > > From: Tom Rix <trix@redhat.com>
> > > 
> > > A break is not needed if it is preceded by a return
> > > 
> > > Signed-off-by: Tom Rix <trix@redhat.com>
> > > ---
> > >  drivers/gpio/gpio-bd70528.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-
> > > bd70528.c
> > > index 45b3da8da336..931e5765fe92 100644
> > > --- a/drivers/gpio/gpio-bd70528.c
> > > +++ b/drivers/gpio/gpio-bd70528.c
> > > @@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct
> > > gpio_chip *chip, unsigned int offset,
> > >  					  GPIO_OUT_REG(offset),
> > >  					  BD70528_GPIO_DRIVE_MASK,
> > >  					  BD70528_GPIO_OPEN_DRAIN);
> > > -		break;
> > My personal taste is also to omit these breaks but I am pretty sure I
> > saw some tooling issuing a warning about falling through the switch-
> > case back when I wrote this. Most probably checkpatch didn't like
> > that
> > back then.
> 
> I did a test and removed the breaks. Then I copied the modified file to
> drivers/gpio/dummy.c
> Next I committed this dummy.c in git, ran git-format-patch -s and
> finally ran the checkpatch on this... Following was produced:
> 
> 
> [mvaittin@localhost linux]$ scripts/checkpatch.pl 0001-gpio-add-
> dummy.patch 
> Traceback (most recent call last):
>   File "scripts/spdxcheck.py", line 6, in <module>
>     from ply import lex, yacc
> ImportError: No module named ply
> WARNING: added, moved or deleted file(s), does MAINTAINERS need
> updating?
> #15: 
> new file mode 100644
> 
> WARNING: Possible switch case/default not preceded by break or
> fallthrough comment
> #91: FILE: drivers/gpio/dummy.c:72:
> +	case PIN_CONFIG_DRIVE_PUSH_PULL:
> 
> WARNING: Possible switch case/default not preceded by break or
> fallthrough comment
> #96: FILE: drivers/gpio/dummy.c:77:
> +	case PIN_CONFIG_INPUT_DEBOUNCE:
> 
> total: 0 errors, 3 warnings, 229 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-
> inplace.
> 
> 0001-gpio-add-dummy.patch has style problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> I guess that explains the odd "fallthrough" comments you mentioned in
> another email. I guess the checkpatch should be fixed before you put
> too much effort in clean-up...
> 
> 
> And for peeps who have not been following - following function triggers
> the checkpatch error above:

Huh?  what version of checkpatch are you using?
Send it to me please.

> static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int
> offset,
> 				   unsigned long config)
> {
> 	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> 
> 	switch (pinconf_to_config_param(config)) {
> 	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> 		return regmap_update_bits(bdgpio->chip.regmap,
> 					  GPIO_OUT_REG(offset),
> 					  BD70528_GPIO_DRIVE_MASK,
> 					  BD70528_GPIO_OPEN_DRAIN);
> 	case PIN_CONFIG_DRIVE_PUSH_PULL:
> 		return regmap_update_bits(bdgpio->chip.regmap,
> 					  GPIO_OUT_REG(offset),
> 					  BD70528_GPIO_DRIVE_MASK,
> 					  BD70528_GPIO_PUSH_PULL);
> 	case PIN_CONFIG_INPUT_DEBOUNCE:
> 		return bd70528_set_debounce(bdgpio, offset,
> 					    pinconf_to_config_argument(
> config));
> 	default:
> 		break;
> 	}
> 	return -ENOTSUPP;
> }
> 
> 
> Best Regards
> 	Matti Vaittinen
> 

Re: [PATCH] gpio: bd70528: remove unneeded break

[Vaittinen, Matti]

[-- Attachment #1: Type: text/plain, Size: 4440 bytes --]

Hello Joe & All,

On Tue, 2020-10-20 at 11:36 -0700, Joe Perches wrote:
> On Tue, 2020-10-20 at 11:48 +0000, Vaittinen, Matti wrote:
> > On Tue, 2020-10-20 at 13:07 +0300, Matti Vaittinen wrote:
> > > Thanks Tom,
> > > 
> > > On Mon, 2020-10-19 at 12:33 -0700, trix@redhat.com wrote:
> > > > From: Tom Rix <trix@redhat.com>
> > > > 
> > > > A break is not needed if it is preceded by a return
> > > > 
> > > > Signed-off-by: Tom Rix <trix@redhat.com>
> > > > ---
> > > >  drivers/gpio/gpio-bd70528.c | 3 ---
> > > >  1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpio/gpio-bd70528.c b/drivers/gpio/gpio-
> > > > bd70528.c
> > > > index 45b3da8da336..931e5765fe92 100644
> > > > --- a/drivers/gpio/gpio-bd70528.c
> > > > +++ b/drivers/gpio/gpio-bd70528.c
> > > > @@ -71,17 +71,14 @@ static int bd70528_gpio_set_config(struct
> > > > gpio_chip *chip, unsigned int offset,
> > > >  					  GPIO_OUT_REG(offset),
> > > >  					  BD70528_GPIO_DRIVE_MA
> > > > SK,
> > > >  					  BD70528_GPIO_OPEN_DRA
> > > > IN);
> > > > -		break;
> > > My personal taste is also to omit these breaks but I am pretty
> > > sure I
> > > saw some tooling issuing a warning about falling through the
> > > switch-
> > > case back when I wrote this. Most probably checkpatch didn't like
> > > that
> > > back then.
> > 
> > I did a test and removed the breaks. Then I copied the modified
> > file to
> > drivers/gpio/dummy.c
> > Next I committed this dummy.c in git, ran git-format-patch -s and
> > finally ran the checkpatch on this... Following was produced:
> > 
> > 
> > [mvaittin@localhost linux]$ scripts/checkpatch.pl 0001-gpio-add-
> > dummy.patch 
> > Traceback (most recent call last):
> >   File "scripts/spdxcheck.py", line 6, in <module>
> >     from ply import lex, yacc
> > ImportError: No module named ply
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need
> > updating?
> > #15: 
> > new file mode 100644
> > 
> > WARNING: Possible switch case/default not preceded by break or
> > fallthrough comment
> > #91: FILE: drivers/gpio/dummy.c:72:
> > +	case PIN_CONFIG_DRIVE_PUSH_PULL:
> > 
> > WARNING: Possible switch case/default not preceded by break or
> > fallthrough comment
> > #96: FILE: drivers/gpio/dummy.c:77:
> > +	case PIN_CONFIG_INPUT_DEBOUNCE:
> > 
> > total: 0 errors, 3 warnings, 229 lines checked
> > 
> > NOTE: For some of the reported defects, checkpatch may be able to
> >       mechanically convert to the typical style using --fix or --
> > fix-
> > inplace.
> > 
> > 0001-gpio-add-dummy.patch has style problems, please review.
> > 
> > NOTE: If any of the errors are false positives, please report
> >       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > 
> > I guess that explains the odd "fallthrough" comments you mentioned
> > in
> > another email. I guess the checkpatch should be fixed before you
> > put
> > too much effort in clean-up...
> > 
> > 
> > And for peeps who have not been following - following function
> > triggers
> > the checkpatch error above:
> 
> Huh?  what version of checkpatch are you using?
> Send it to me please.

I was actually accidentally running the checkpatch from stable release
5.4 yesterday. I did today run it from my development repo which
currently was based on regulator-v5.8. I didn't test the latest
versions.

Please find my version of checkpatch and the patch to trigger the
warning attached.

> 
> > static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned
> > int
> > offset,
> > 				   unsigned long config)
> > {
> > 	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
> > 
> > 	switch (pinconf_to_config_param(config)) {
> > 	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> > 		return regmap_update_bits(bdgpio->chip.regmap,
> > 					  GPIO_OUT_REG(offset),
> > 					  BD70528_GPIO_DRIVE_MASK,
> > 					  BD70528_GPIO_OPEN_DRAIN);
> > 	case PIN_CONFIG_DRIVE_PUSH_PULL:
> > 		return regmap_update_bits(bdgpio->chip.regmap,
> > 					  GPIO_OUT_REG(offset),
> > 					  BD70528_GPIO_DRIVE_MASK,
> > 					  BD70528_GPIO_PUSH_PULL);
> > 	case PIN_CONFIG_INPUT_DEBOUNCE:
> > 		return bd70528_set_debounce(bdgpio, offset,
> > 					    pinconf_to_config_argument(
> > config));
> > 	default:
> > 		break;
> > 	}
> > 	return -ENOTSUPP;
> > }
> > 
> > 
> > Best Regards
> > 	Matti Vaittinen
> > 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gpio-add-dummy.patch --]
[-- Type: text/x-patch; name="0001-gpio-add-dummy.patch", Size: 7230 bytes --]

From 5e70eba5d3b0e8e0505378400c417fb4060463c4 Mon Sep 17 00:00:00 2001
From: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Date: Tue, 20 Oct 2020 14:35:16 +0300
Subject: [PATCH] gpio: add dummy

this description is likely to not make any sense.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/gpio/dummy.c | 229 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 229 insertions(+)
 create mode 100644 drivers/gpio/dummy.c

diff --git a/drivers/gpio/dummy.c b/drivers/gpio/dummy.c
new file mode 100644
index 000000000000..75c1bae94514
--- /dev/null
+++ b/drivers/gpio/dummy.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 ROHM Semiconductors
+// gpio-bd70528.c ROHM BD70528MWV gpio driver
+
+#include <linux/gpio/driver.h>
+#include <linux/mfd/rohm-bd70528.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define GPIO_IN_REG(offset) (BD70528_REG_GPIO1_IN + (offset) * 2)
+#define GPIO_OUT_REG(offset) (BD70528_REG_GPIO1_OUT + (offset) * 2)
+
+struct bd70528_gpio {
+	struct rohm_regmap_dev chip;
+	struct gpio_chip gpio;
+};
+
+static int bd70528_set_debounce(struct bd70528_gpio *bdgpio,
+				unsigned int offset, unsigned int debounce)
+{
+	u8 val;
+
+	switch (debounce) {
+	case 0:
+		val = BD70528_DEBOUNCE_DISABLE;
+		break;
+	case 1 ... 15000:
+		val = BD70528_DEBOUNCE_15MS;
+		break;
+	case 15001 ... 30000:
+		val = BD70528_DEBOUNCE_30MS;
+		break;
+	case 30001 ... 50000:
+		val = BD70528_DEBOUNCE_50MS;
+		break;
+	default:
+		dev_err(bdgpio->chip.dev,
+			"Invalid debounce value %u\n", debounce);
+		return -EINVAL;
+	}
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_IN_REG(offset),
+				 BD70528_DEBOUNCE_MASK, val);
+}
+
+static int bd70528_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+	int val, ret;
+
+	/* Do we need to do something to IRQs here? */
+	ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
+	if (ret) {
+		dev_err(bdgpio->chip.dev, "Could not read gpio direction\n");
+		return ret;
+	}
+
+	return !(val & BD70528_GPIO_OUT_EN_MASK);
+}
+
+static int bd70528_gpio_set_config(struct gpio_chip *chip, unsigned int offset,
+				   unsigned long config)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		return regmap_update_bits(bdgpio->chip.regmap,
+					  GPIO_OUT_REG(offset),
+					  BD70528_GPIO_DRIVE_MASK,
+					  BD70528_GPIO_OPEN_DRAIN);
+	case PIN_CONFIG_DRIVE_PUSH_PULL:
+		return regmap_update_bits(bdgpio->chip.regmap,
+					  GPIO_OUT_REG(offset),
+					  BD70528_GPIO_DRIVE_MASK,
+					  BD70528_GPIO_PUSH_PULL);
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		return bd70528_set_debounce(bdgpio, offset,
+					    pinconf_to_config_argument(config));
+	default:
+		break;
+	}
+	return -ENOTSUPP;
+}
+
+static int bd70528_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	/* Do we need to do something to IRQs here? */
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				 BD70528_GPIO_OUT_EN_MASK,
+				 BD70528_GPIO_OUT_DISABLE);
+}
+
+static void bd70528_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+	int ret;
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+	u8 val = (value) ? BD70528_GPIO_OUT_HI : BD70528_GPIO_OUT_LO;
+
+	ret = regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				 BD70528_GPIO_OUT_MASK, val);
+	if (ret)
+		dev_err(bdgpio->chip.dev, "Could not set gpio to %d\n", value);
+}
+
+static int bd70528_direction_output(struct gpio_chip *chip, unsigned int offset,
+				    int value)
+{
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	bd70528_gpio_set(chip, offset, value);
+	return regmap_update_bits(bdgpio->chip.regmap, GPIO_OUT_REG(offset),
+				 BD70528_GPIO_OUT_EN_MASK,
+				 BD70528_GPIO_OUT_ENABLE);
+}
+
+#define GPIO_IN_STATE_MASK(offset) (BD70528_GPIO_IN_STATE_BASE << (offset))
+
+static int bd70528_gpio_get_o(struct bd70528_gpio *bdgpio, unsigned int offset)
+{
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(bdgpio->chip.regmap, GPIO_OUT_REG(offset), &val);
+	if (!ret)
+		ret = !!(val & BD70528_GPIO_OUT_MASK);
+	else
+		dev_err(bdgpio->chip.dev, "GPIO (out) state read failed\n");
+
+	return ret;
+}
+
+static int bd70528_gpio_get_i(struct bd70528_gpio *bdgpio, unsigned int offset)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(bdgpio->chip.regmap, BD70528_REG_GPIO_STATE, &val);
+
+	if (!ret)
+		ret = !(val & GPIO_IN_STATE_MASK(offset));
+	else
+		dev_err(bdgpio->chip.dev, "GPIO (in) state read failed\n");
+
+	return ret;
+}
+
+static int bd70528_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	int ret;
+	struct bd70528_gpio *bdgpio = gpiochip_get_data(chip);
+
+	/*
+	 * There is a race condition where someone might be changing the
+	 * GPIO direction after we get it but before we read the value. But
+	 * application design where GPIO direction may be changed just when
+	 * we read GPIO value would be pointless as reader could not know
+	 * whether the returned high/low state is caused by input or output.
+	 * Or then there must be other ways to mitigate the issue. Thus
+	 * locking would make no sense.
+	 */
+	ret = bd70528_get_direction(chip, offset);
+	if (ret == 0)
+		ret = bd70528_gpio_get_o(bdgpio, offset);
+	else if (ret == 1)
+		ret = bd70528_gpio_get_i(bdgpio, offset);
+	else
+		dev_err(bdgpio->chip.dev, "failed to read GPIO direction\n");
+
+	return ret;
+}
+
+static int bd70528_probe(struct platform_device *pdev)
+{
+	struct bd70528_gpio *bdgpio;
+	struct rohm_regmap_dev *bd70528;
+	int ret;
+
+	bd70528 = dev_get_drvdata(pdev->dev.parent);
+	if (!bd70528) {
+		dev_err(&pdev->dev, "No MFD driver data\n");
+		return -EINVAL;
+	}
+
+	bdgpio = devm_kzalloc(&pdev->dev, sizeof(*bdgpio),
+			      GFP_KERNEL);
+	if (!bdgpio)
+		return -ENOMEM;
+	bdgpio->chip.dev = &pdev->dev;
+	bdgpio->gpio.parent = pdev->dev.parent;
+	bdgpio->gpio.label = "bd70528-gpio";
+	bdgpio->gpio.owner = THIS_MODULE;
+	bdgpio->gpio.get_direction = bd70528_get_direction;
+	bdgpio->gpio.direction_input = bd70528_direction_input;
+	bdgpio->gpio.direction_output = bd70528_direction_output;
+	bdgpio->gpio.set_config = bd70528_gpio_set_config;
+	bdgpio->gpio.can_sleep = true;
+	bdgpio->gpio.get = bd70528_gpio_get;
+	bdgpio->gpio.set = bd70528_gpio_set;
+	bdgpio->gpio.ngpio = 4;
+	bdgpio->gpio.base = -1;
+#ifdef CONFIG_OF_GPIO
+	bdgpio->gpio.of_node = pdev->dev.parent->of_node;
+#endif
+	bdgpio->chip.regmap = bd70528->regmap;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &bdgpio->gpio,
+				     bdgpio);
+	if (ret)
+		dev_err(&pdev->dev, "gpio_init: Failed to add bd70528-gpio\n");
+
+	return ret;
+}
+
+static struct platform_driver bd70528_gpio = {
+	.driver = {
+		.name = "bd70528-gpio"
+	},
+	.probe = bd70528_probe,
+};
+
+module_platform_driver(bd70528_gpio);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD70528 voltage regulator driver");
+MODULE_LICENSE("GPL");
-- 
2.21.3


[-- Attachment #3: checkpatch.pl --]
[-- Type: application/x-perl, Size: 207571 bytes --]

Re: [PATCH] gpio: bd70528: remove unneeded break

[Joe Perches]

On Wed, 2020-10-21 at 07:25 +0000, Vaittinen, Matti wrote:
> Hello Joe & All,
> On Tue, 2020-10-20 at 11:36 -0700, Joe Perches wrote:
> > On Tue, 2020-10-20 at 11:48 +0000, Vaittinen, Matti wrote:
[]
> > > And for peeps who have not been following - following function
> > > triggers the checkpatch error above:
> > 
> > Huh?  what version of checkpatch are you using?
> > Send it to me please.
[]
> Please find my version of checkpatch and the patch to trigger the
> warning attached.

Thanks.  This test wasn't particularly useful
(and had some false positives) and was removed by

commit ef3c005c0eb07a60949191bc6ee407d5f43cc502
Author: Joe Perches <joe@perches.com>
Date:   Tue Aug 11 18:35:19 2020 -0700

    checkpatch: remove missing switch/case break test
    
    This test doesn't work well and newer compilers are much better
    at emitting this warning.
    
    Signed-off-by: Joe Perches <joe@perches.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Cc: Cambda Zhu <cambda@linux.alibaba.com>
    Link: http://lkml.kernel.org/r/7e25090c79f6a69d502ab8219863300790192fe2.camel@perches.com
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Re: [PATCH] gpio: bd70528: remove unneeded break

[Vaittinen, Matti]

Hello,

On Wed, 2020-10-21 at 07:39 -0700, Joe Perches wrote:
> On Wed, 2020-10-21 at 07:25 +0000, Vaittinen, Matti wrote:
> > Hello Joe & All,
> > On Tue, 2020-10-20 at 11:36 -0700, Joe Perches wrote:
> > > On Tue, 2020-10-20 at 11:48 +0000, Vaittinen, Matti wrote:
> []
> > > > And for peeps who have not been following - following function
> > > > triggers the checkpatch error above:
> > > 
> > > Huh?  what version of checkpatch are you using?
> > > Send it to me please.
> []
> > Please find my version of checkpatch and the patch to trigger the
> > warning attached.
> 
> Thanks.  This test wasn't particularly useful
> (and had some false positives) and was removed by
> 
> commit ef3c005c0eb07a60949191bc6ee407d5f43cc502
> Author: Joe Perches <joe@perches.com>
> Date:   Tue Aug 11 18:35:19 2020 -0700
> 
>     checkpatch: remove missing switch/case break test
>     
>     This test doesn't work well and newer compilers are much better
>     at emitting this warning.
>     
>     Signed-off-by: Joe Perches <joe@perches.com>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Cc: Cambda Zhu <cambda@linux.alibaba.com>
>     Link: 
> http://lkml.kernel.org/r/7e25090c79f6a69d502ab8219863300790192fe2.camel@perches.com
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 

Thanks for checking this Joe!
And note to self - always check with newest kernel... ;)

(Sorry for bothering)

Br,
	Matti Vaittinen