| Message ID | 20201107032924.25044-2-bernard@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drivers/tty: delete break after return or goto | expand |
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 <bernard@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). Best regards Uwe
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Date: 2020-11-07 22:01:29 To: Bernard Zhao <bernard@vivo.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,Jiri Slaby <jirislaby@kernel.org>,Shawn Guo <shawnguo@kernel.org>,Sascha Hauer <s.hauer@pengutronix.de>,Pengutronix Kernel Team <kernel@pengutronix.de>,Fabio Estevam <festevam@gmail.com>,NXP Linux Team <linux-imx@nxp.com>,linux-kernel@vger.kernel.org,linux-serial@vger.kernel.org,linux-arm-kernel@lists.infradead.org,opensource.kernel@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@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/ |
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); }
Delete break after return, which will never run. Signed-off-by: Bernard Zhao <bernard@vivo.com> --- drivers/tty/serial/imx.c | 5 ----- 1 file changed, 5 deletions(-)