linux-kernel - Re: [PATCH] gpio: bd70528: remove unneeded break

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 21 Oct 2020 07:25:24 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "joe@...ches.com" <joe@...ches.com>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "trix@...hat.com" <trix@...hat.com>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>
CC:     linux-power <linux-power@...rohmeurope.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "apw@...onical.com" <apw@...onical.com>
Subject: Re: [PATCH] gpio: bd70528: remove unneeded break

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@...hat.com wrote:
> > > > From: Tom Rix <trix@...hat.com>
> > > > 
> > > > A break is not needed if it is preceded by a return
> > > > 
> > > > Signed-off-by: Tom Rix <trix@...hat.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@...alhost 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
> > 


View attachment "0001-gpio-add-dummy.patch" of type "text/x-patch" (7230 bytes)

Download attachment "checkpatch.pl" of type "application/x-perl" (207571 bytes)