diff mbox series

[-next,005/491] ARM/UNIPHIER ARCHITECTURE: Use fallthrough;

Message ID dae0878058223a42c77d725b8d7c5845a7ef9dc0.1583896348.git.joe@perches.com
State New, archived
Headers show
Series None | expand

Commit Message

Joe Perches March 11, 2020, 4:51 a.m. UTC
Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/tty/serial/8250/8250_uniphier.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Masahiro Yamada March 11, 2020, 5:15 a.m. UTC | #1
On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
>
> Convert the various uses of fallthrough comments to fallthrough;
>
> Done via script
> Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
>
> Signed-off-by: Joe Perches <joe@perches.com>


Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>


But, I think the patch subject should be prefixed:
"serial: 8250_uniphier:"



> ---
>  drivers/tty/serial/8250/8250_uniphier.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> index e0b73a5..a2978ab 100644
> --- a/drivers/tty/serial/8250/8250_uniphier.c
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -75,7 +75,7 @@ static unsigned int uniphier_serial_in(struct uart_port *p, int offset)
>                 break;
>         case UART_LCR:
>                 valshift = 8;
> -               /* fall through */
> +               fallthrough;
>         case UART_MCR:
>                 offset = UNIPHIER_UART_LCR_MCR;
>                 break;
> @@ -101,7 +101,7 @@ static void uniphier_serial_out(struct uart_port *p, int offset, int value)
>         case UART_SCR:
>                 /* No SCR for this hardware.  Use CHAR as a scratch register */
>                 valshift = 8;
> -               /* fall through */
> +               fallthrough;
>         case UART_FCR:
>                 offset = UNIPHIER_UART_CHAR_FCR;
>                 break;
> @@ -109,7 +109,7 @@ static void uniphier_serial_out(struct uart_port *p, int offset, int value)
>                 valshift = 8;
>                 /* Divisor latch access bit does not exist. */
>                 value &= ~UART_LCR_DLAB;
> -               /* fall through */
> +               fallthrough;
>         case UART_MCR:
>                 offset = UNIPHIER_UART_LCR_MCR;
>                 break;
> --
> 2.24.0
>
Joe Perches March 11, 2020, 2:31 p.m. UTC | #2
On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > Convert the various uses of fallthrough comments to fallthrough;
> > 
> > Done via script
> > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 
> But, I think the patch subject should be prefixed:
> "serial: 8250_uniphier:"

Yeah thanks, that's difficult to script though.
Greg KH March 12, 2020, 8:56 a.m. UTC | #3
On Wed, Mar 11, 2020 at 07:31:07AM -0700, Joe Perches wrote:
> On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> > On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > > Convert the various uses of fallthrough comments to fallthrough;
> > > 
> > > Done via script
> > > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > > 
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > 
> > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > 
> > 
> > But, I think the patch subject should be prefixed:
> > "serial: 8250_uniphier:"
> 
> Yeah thanks, that's difficult to script though.
> 
> 

Kernel development is hard :)
Masahiro Yamada March 12, 2020, 9:02 a.m. UTC | #4
On Thu, Mar 12, 2020 at 5:56 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Mar 11, 2020 at 07:31:07AM -0700, Joe Perches wrote:
> > On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> > > On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > > > Convert the various uses of fallthrough comments to fallthrough;
> > > >
> > > > Done via script
> > > > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > > >
> > > > Signed-off-by: Joe Perches <joe@perches.com>
> > >
> > > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > >
> > >
> > > But, I think the patch subject should be prefixed:
> > > "serial: 8250_uniphier:"
> >
> > Yeah thanks, that's difficult to script though.
> >
> >
>
> Kernel development is hard :)


It is strange to process this per-platform
and to send out a giant series that
consists of 491 patches.

This is very trivial conversion.

I think it is better to have a single patch
to convert all files under drivers/tty/serial/,
with the patch subject "serial:".
Joe Perches March 12, 2020, 9:03 a.m. UTC | #5
On Thu, 2020-03-12 at 09:56 +0100, Greg Kroah-Hartman wrote:
> On Wed, Mar 11, 2020 at 07:31:07AM -0700, Joe Perches wrote:
> > On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> > > On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > > > Convert the various uses of fallthrough comments to fallthrough;
> > > > 
> > > > Done via script
> > > > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > > > 
> > > > Signed-off-by: Joe Perches <joe@perches.com>
> > > 
> > > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > 
> > > 
> > > But, I think the patch subject should be prefixed:
> > > "serial: 8250_uniphier:"
> > 
> > Yeah thanks, that's difficult to script though.
> > 
> > 
> 
> Kernel development is hard :)

Not really, kernel development processes are pretty stupid sometimes.
Greg KH March 12, 2020, 9:36 a.m. UTC | #6
On Thu, Mar 12, 2020 at 06:02:19PM +0900, Masahiro Yamada wrote:
> On Thu, Mar 12, 2020 at 5:56 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Mar 11, 2020 at 07:31:07AM -0700, Joe Perches wrote:
> > > On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> > > > On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > > > > Convert the various uses of fallthrough comments to fallthrough;
> > > > >
> > > > > Done via script
> > > > > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > > > >
> > > > > Signed-off-by: Joe Perches <joe@perches.com>
> > > >
> > > > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > >
> > > >
> > > > But, I think the patch subject should be prefixed:
> > > > "serial: 8250_uniphier:"
> > >
> > > Yeah thanks, that's difficult to script though.
> > >
> > >
> >
> > Kernel development is hard :)
> 
> 
> It is strange to process this per-platform
> and to send out a giant series that
> consists of 491 patches.
> 
> This is very trivial conversion.
> 
> I think it is better to have a single patch
> to convert all files under drivers/tty/serial/,
> with the patch subject "serial:".

I agree.
Joe Perches March 12, 2020, 9:37 a.m. UTC | #7
On Thu, 2020-03-12 at 18:02 +0900, Masahiro Yamada wrote:
> On Thu, Mar 12, 2020 at 5:56 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Wed, Mar 11, 2020 at 07:31:07AM -0700, Joe Perches wrote:
> > > On Wed, 2020-03-11 at 14:15 +0900, Masahiro Yamada wrote:
> > > > On Wed, Mar 11, 2020 at 2:07 PM Joe Perches <joe@perches.com> wrote:
> > > > > Convert the various uses of fallthrough comments to fallthrough;
> > > > > 
> > > > > Done via script
> > > > > Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> > > > > 
> > > > > Signed-off-by: Joe Perches <joe@perches.com>
> > > > 
> > > > Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > > 
> > > > 
> > > > But, I think the patch subject should be prefixed:
> > > > "serial: 8250_uniphier:"
> > > 
> > > Yeah thanks, that's difficult to script though.
> > > 
> > > 
> > 
> > Kernel development is hard :)
> 
> It is strange to process this per-platform
> and to send out a giant series that
> consists of 491 patches.
> 
> This is very trivial conversion.
> 
> I think it is better to have a single patch
> to convert all files under drivers/tty/serial/,
> with the patch subject "serial:".

The difficulty there is determining where these
subsystem spanning blocks should begin and end.

That could not be done for instance with drivers/net.

As I have suggested a few times, better still
would be to have a mechanism for scripted patches
applied possibly as single treewide patch.

Likely applied only at an -rc1.

The stated negatives to a treewide mechanism
have been difficulty to backport to -stable.

Perhaps a mechanism like

	git format-patch --stdout <treewide_commit_to_backport> | \
	git apply --include=<specific_files>

with some automated rewrite of the treewide patch subject
then commit could help.
Theodore Y. Ts'o March 12, 2020, 1:47 p.m. UTC | #8
On Thu, Mar 12, 2020 at 02:37:31AM -0700, Joe Perches wrote:
> As I have suggested a few times, better still
> would be to have a mechanism for scripted patches
> applied possibly as single treewide patch.
> 
> Likely applied only at an -rc1.
> 
> The stated negatives to a treewide mechanism
> have been difficulty to backport to -stable.

Any time we do a massive, disruptive change to the code base, it's
going to cause problems to -stable.  It means that bug fix patches
won't necessarily auto-apply, and some will require manual fixups
afterwards

Given that this change doesn't really fix any bugs, I'd have to ask
the question --- is it *worth* it?  We really need to apply a certain
amount of cost/benefit analysis around this.

If it were really important, the thing we could do is to apply a
single treewide patch at some point after the merge window.  I'd
suggest after -rc2, myself, but reasonable people can differ.  And
then, if it were *really* important we could run the same script on
the stable kernels.

But for changing "/* fallthrough */" to "fallthrough;"

Does this ***really*** matter?  Why are we tying ourselves up in knots
trying to do this all at once?

					- Ted
Joe Perches March 12, 2020, 2:15 p.m. UTC | #9
On Thu, 2020-03-12 at 09:47 -0400, Theodore Y. Ts'o wrote:
> On Thu, Mar 12, 2020 at 02:37:31AM -0700, Joe Perches wrote:
> > As I have suggested a few times, better still
> > would be to have a mechanism for scripted patches
> > applied possibly as single treewide patch.
> > 
> > Likely applied only at an -rc1.
> > 
> > The stated negatives to a treewide mechanism
> > have been difficulty to backport to -stable.
> 
> Any time we do a massive, disruptive change to the code base, it's
> going to cause problems to -stable.  It means that bug fix patches
> won't necessarily auto-apply, and some will require manual fixups
> afterwards

That's mostly a tools problem than a real problem.

> Given that this change doesn't really fix any bugs, I'd have to ask
> the question --- is it *worth* it?  We really need to apply a certain
> amount of cost/benefit analysis around this.
> 
> If it were really important, the thing we could do is to apply a
> single treewide patch at some point after the merge window.  I'd
> suggest after -rc2, myself, but reasonable people can differ.  And
> then, if it were *really* important we could run the same script on
> the stable kernels.
> 
> But for changing "/* fallthrough */" to "fallthrough;"
> 
> Does this ***really*** matter?

That depends a bit on whether clang is your
compiler of choice.

> Why are we tying ourselves up in knots
> trying to do this all at once?

Discretely or treewide, all at once or done over time,
the impact problem to backports is the same.
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index e0b73a5..a2978ab 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -75,7 +75,7 @@  static unsigned int uniphier_serial_in(struct uart_port *p, int offset)
 		break;
 	case UART_LCR:
 		valshift = 8;
-		/* fall through */
+		fallthrough;
 	case UART_MCR:
 		offset = UNIPHIER_UART_LCR_MCR;
 		break;
@@ -101,7 +101,7 @@  static void uniphier_serial_out(struct uart_port *p, int offset, int value)
 	case UART_SCR:
 		/* No SCR for this hardware.  Use CHAR as a scratch register */
 		valshift = 8;
-		/* fall through */
+		fallthrough;
 	case UART_FCR:
 		offset = UNIPHIER_UART_CHAR_FCR;
 		break;
@@ -109,7 +109,7 @@  static void uniphier_serial_out(struct uart_port *p, int offset, int value)
 		valshift = 8;
 		/* Divisor latch access bit does not exist. */
 		value &= ~UART_LCR_DLAB;
-		/* fall through */
+		fallthrough;
 	case UART_MCR:
 		offset = UNIPHIER_UART_LCR_MCR;
 		break;