Re: [PATCH v2 00/11] tty: serial: 8250: Fix checkpatch warnings

From: Peter Hurley
Date: Fri Dec 18 2015 - 10:17:29 EST


On 12/18/2015 03:21 AM, Anton Wuerfel wrote:
> Hello Peter,
>
> thanks for your feedback. We updated the patches according to your remarks.

Not a problem; on the contrary, thanks for the patches.

Just a procedural note: it's customary to address the patches to
the maintainer(s) and cc others and the list. It's not really a problem here
because Greg picks up everything but that's not true for other subsystems.
Just thought I'd let you know.

Regards,
Peter Hurley


> This patch set fixes several checkpatch warnings in tty/serial/8250.
>
> Patch 1/11: Adds missing spaces (mainly to function/macro headers)
> Patch 2/11: Replace spaces with tabs wherever possible
> Patch 3/11: Slight patch which moves an opening curly brace
> Patch 4/11: Fixes multiline comment style
> Patch 5/11: Removes else blocks after return statements
> Patch 6/11: Slight patch which moves EXPORT_SYMBOL macro to correct position
> Patch 7/11: Slight patch which removes an unneccessary line continuation
> Patch 8/11: Slight patch which adds parentheses to a macro definition
> Patch 9/11: Merges user-visible multiline strings to a single line
> Patch 10/11: Replaces printk by corresponding variant of pr_*
> Patch 11/11: Fixes code indentation
>
> Remaining checkpatch warnings after applying this patch series:
>
> -line over 80 characters
> This error mostly occurs in serial_cs.c, which contains long-lined
> macro calls. However, splitting these calls into multiple lines would
> not increase readability.
>
> -externs should be avoided in .c files
> This occurs in 8250_hp300.c. There is no corresponding header file
> the extern statement could be moved to. It could be moved to 8250.h
> but this would affect other .c files.
>
> -Use #include <linux/*.h> instead of <asm/*.h>
> This warning has been left open for more experienced kernel hackers.
> This patch series is about style issues. We do not intend to alter
> the code behavior.
>
> -struct uart_ops should normally be const
> This warning only occurs in 8250_core.c. The corresponding struct
> cannot be declared as const because it is altered in
> serial8250_isa_init_ports(). Maybe a checkpatch exception should
> be added for this particular warning.
>
> -quoted string split across lines
> These strings were ignored because they otherwise would exceed
> 80 characters in a single line. These particular strings use
> format specifiers, which break the ability to grep for them anyway.
>
> Anton Wuerfel (11):
> Phillip Raffeck (11):
> tty: serial: 8250: Fix whitespace errors
> tty: serial: 8250: Replace spaces with tabs
> tty: serial: 8250: Fix braces after struct
> tty: serial: 8250: Fix multiline comment style
> tty: serial: 8250: Remove else after return
> tty: serial: 8250: Move EXPORT_SYMBOL to function
> tty: serial: 8250: Fix line continuation warning
> tty: serial: 8250: Add parentheses to macro
> tty: serial: 8250: Fix multi-line strings
> tty: serial: 8250: Replace printk by pr_*
> tty: serial: 8250: Fix indentation warnings
>
> drivers/tty/serial/8250/8250_accent.c | 2 +-
> drivers/tty/serial/8250/8250_acorn.c | 2 +-
> drivers/tty/serial/8250/8250_boca.c | 2 +-
> drivers/tty/serial/8250/8250_core.c | 15 ++---
> drivers/tty/serial/8250/8250_exar_st16c554.c | 2 +-
> drivers/tty/serial/8250/8250_fourport.c | 2 +-
> drivers/tty/serial/8250/8250_gsc.c | 6 +-
> drivers/tty/serial/8250/8250_hp300.c | 25 ++++----
> drivers/tty/serial/8250/8250_hub6.c | 2 +-
> drivers/tty/serial/8250/8250_ingenic.c | 12 ++--
> drivers/tty/serial/8250/8250_pci.c | 49 +++++++--------
> drivers/tty/serial/8250/8250_pnp.c | 16 ++---
> drivers/tty/serial/8250/8250_port.c | 50 ++++++++--------
> drivers/tty/serial/8250/serial_cs.c | 89 +++++++++++++++-------------
> 14 files changed, 142 insertions(+), 132 deletions(-)
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/