Re:Re: [PATCH 1/2] tty/serial: delete break after return

[Bernard]

From: "Uwe Kleine-König" <u.kleine-koenig at pengutronix.de>
Date: 2020-11-07 22:01:29
To:  Bernard Zhao <bernard at vivo.com>
Cc:  Greg Kroah-Hartman <gregkh at linuxfoundation.org>,Jiri Slaby <jirislaby at kernel.org>,Shawn Guo <shawnguo at kernel.org>,Sascha Hauer <s.hauer at pengutronix.de>,Pengutronix Kernel Team <kernel at pengutronix.de>,Fabio Estevam <festevam at gmail.com>,NXP Linux Team <linux-imx at nxp.com>,linux-kernel at vger.kernel.org,linux-serial at vger.kernel.org,linux-arm-kernel at lists.infradead.org,opensource.kernel at vivo.com
Subject: Re: [PATCH 1/2] tty/serial: delete break after return>Hello,
>
>the Subject is wrong, it should use a prefix similar to "serial: imx:".
>It's a good idea to check previous patches to the same file to pick a
>suitable prefix. (E.g. git log --oneline drivers/tty/serial/imx.c)

Hi, Uwe:

Thank you for your suggestion, I will make a more accurate subject in my future patches.

>On Fri, Nov 06, 2020 at 07:29:23PM -0800, Bernard Zhao wrote:
>> Delete break after return, which will never run.
>> 
>> Signed-off-by: Bernard Zhao <bernard at vivo.com>
>> ---
>>  drivers/tty/serial/imx.c | 5 -----
>>  1 file changed, 5 deletions(-)
>> 
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 1731d9728865..09703079db7b 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -320,7 +320,6 @@ static u32 imx_uart_readl(struct imx_port *sport, u32 offset)
>>  	switch (offset) {
>>  	case UCR1:
>>  		return sport->ucr1;
>> -		break;
>>  	case UCR2:
>>  		/*
>>  		 * UCR2_SRST is the only bit in the cached registers that might
>> @@ -331,16 +330,12 @@ static u32 imx_uart_readl(struct imx_port *sport, u32 offset)
>>  		if (!(sport->ucr2 & UCR2_SRST))
>>  			sport->ucr2 = readl(sport->port.membase + offset);
>>  		return sport->ucr2;
>> -		break;
>>  	case UCR3:
>>  		return sport->ucr3;
>> -		break;
>>  	case UCR4:
>>  		return sport->ucr4;
>> -		break;
>>  	case UFCR:
>>  		return sport->ufcr;
>> -		break;
>
>you're the third to send this patch since October 20:
>
>	https://lore.kernel.org/r/20201026125142.21105-1-zhangqilong3@huawei.com
>	https://lore.kernel.org/r/20201020130709.28096-1-trix@redhat.com
>
>My comment for these was:
>
>> this might be subjective, but I like the break being there for clearity.
>> So I object to make a patch to remove them. In case I'm outvoted I'd at
>> least want empty lines instead.
>
>Zhang Qilong wrote he found the patch opportunity by manual code
>inspection, I would have expected that there is a tool that identifies a
>break after a return. If you had tool support, please mention the tool
>in the commit log (if you really want to keep following the patch's
>idea).

For this part:
I wrote a python script to check if there is a break after a return or goto.
The script currently has some issues in its handling of special characters, the search result is not precise enough. It is still under debugging. 
These are the only places which  the script checks out in path: driver/tty.
I also checked other path(like: driver/vme/, driver/video, driver/usb), these paths also have a certain number of similar issues. And I will try to submit these patch later.
Thanks!

BR//Bernard

>Best regards
>Uwe
>
>-- 
>Pengutronix e.K.                           | Uwe Kleine-König            |
>Industrial Linux Solutions                 | https://www.pengutronix.de/ |