| Message ID | dae0878058223a42c77d725b8d7c5845a7ef9dc0.1583896348.git.joe@perches.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
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 >
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.
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 :)
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:".
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.
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.
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.
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
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 --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;
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(-)