Delete break after return, which will never run.
Signed-off-by: Bernard Zhao <[email protected]>
---
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;
default:
return readl(sport->port.membase + offset);
}
--
2.29.0
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)
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 <[email protected]>
> ---
> 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/[email protected]
https://lore.kernel.org/r/[email protected]
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).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
From: "Uwe Kleine-König" <[email protected]>
Date: 2020-11-07 22:01:29
To: Bernard Zhao <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>,Jiri Slaby <[email protected]>,Shawn Guo <[email protected]>,Sascha Hauer <[email protected]>,Pengutronix Kernel Team <[email protected]>,Fabio Estevam <[email protected]>,NXP Linux Team <[email protected]>,[email protected],[email protected],[email protected],[email protected]
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 <[email protected]>
>> ---
>> 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/[email protected]
> https://lore.kernel.org/r/[email protected]
>
>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/ |