Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1308838 > unrolled thread

[PATCH v6 00/15] tty: serial: 8250: Fix checkpatch warnings

Started byAnton Wuerfel <anton.wuerfel@fau.de>
First post2016-01-13 22:40 +0100
Last post2016-01-14 10:20 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 00/15] tty: serial: 8250: Fix checkpatch warnings Anton Wuerfel <anton.wuerfel@fau.de> - 2016-01-13 22:40 +0100
    [PATCH v6 14/15] tty: serial: 8250: Fix indentation warnings Anton Wuerfel <anton.wuerfel@fau.de> - 2016-01-13 22:40 +0100
    [PATCH v6 02/15] tty: serial: 8250: Replace spaces with tabs Anton Wuerfel <anton.wuerfel@fau.de> - 2016-01-13 22:40 +0100
    [PATCH v6 13/15] tty: serial: 8250: Merge duplicate conditions Anton Wuerfel <anton.wuerfel@fau.de> - 2016-01-13 22:40 +0100
    [PATCH v6 08/15] tty: serial: 8250: Add parentheses to macro Anton Wuerfel <anton.wuerfel@fau.de> - 2016-01-13 22:40 +0100
    Re: [PATCH v6 00/15] tty: serial: 8250: Fix checkpatch warnings Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-14 10:20 +0100

#1308838 — [PATCH v6 00/15] tty: serial: 8250: Fix checkpatch warnings

FromAnton Wuerfel <anton.wuerfel@fau.de>
Date2016-01-13 22:40 +0100
Subject[PATCH v6 00/15] tty: serial: 8250: Fix checkpatch warnings
Message-ID<qQBj4-5yt-5@gated-at.bofh.it>
Hello Andy,

> Also, please thin out the Cc list, I would recommend remove just
> ordinary commiters (with maybe few exceptions) and leave maintainers. 
> Moreover some addresses are bounced.
I thinned out the Cc list and left over only maintainers and major
contributors to the code.

> Yep!
> However, you forgot to fix switch-case as well.
Phillip and me are kind of git magicians. We can make code disappear during
rebase :-) The piece of code was part of a commit, but it never made it into
the final patch file.
 
I hope, there are no major issues with our patch set.

Regards,
Anton Wuerfel
Phillip Raffeck

This patch set fixes several checkpatch warnings in tty/serial/8250.

Patch 1/15:  Adds missing spaces (mainly to function/macro headers)
Patch 2/15:  Replace spaces with tabs wherever possible
Patch 3/15:  Slight patch which moves an opening curly brace
Patch 4/15:  Fixes multiline comment style
Patch 5/15:  Removes else blocks after return statements
Patch 6/15:  Slight patch which moves EXPORT_SYMBOL macro to correct
position
Patch 7/15:  Slight patch which removes an unneccessary line continuation
Patch 8/15:  Slight patch which adds parentheses to a macro definition
Patch 9/15:  Merges user-visible multiline strings to a single line
Patch 10/15: Replace printk by dev_* or pr_* where appropriate
Patch 11/15: Remove orphaned debug macro
Patch 12/15: Fix warnings in dev_dbg by adding two casts
Patch 13/15: Fixes code indentation
Patch 14/15: Add a generic port macro
Patch 15/15: Refactor a switch/case statement

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 (15):
Phillip Raffeck (15):
  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: Correct conversion specifiers
  tty: serial: 8250: Merge duplicate conditions
  tty: serial: 8250: Fix indentation warnings
  tty: serial: 8250: Add generic port init macro
  tty: serial: 8250: Suitably replace printk
  tty: serial: 8250: Remove SERIAL_DEBUG_PNP macro

 drivers/tty/serial/8250/8250.h               | 12 ++++
 drivers/tty/serial/8250/8250_accent.c        | 13 +---
 drivers/tty/serial/8250/8250_acorn.c         |  2 +-
 drivers/tty/serial/8250/8250_boca.c          | 41 ++++++-------
 drivers/tty/serial/8250/8250_core.c          | 15 ++---
 drivers/tty/serial/8250/8250_exar_st16c554.c | 17 ++----
 drivers/tty/serial/8250/8250_fourport.c      | 28 ++++-----
 drivers/tty/serial/8250/8250_gsc.c           |  7 ++-
 drivers/tty/serial/8250/8250_hp300.c         | 27 +++++----
 drivers/tty/serial/8250/8250_hub6.c          |  2 +-
 drivers/tty/serial/8250/8250_ingenic.c       | 12 ++--
 drivers/tty/serial/8250/8250_pci.c           | 60 ++++++++++---------
 drivers/tty/serial/8250/8250_pnp.c           | 28 +++++----
 drivers/tty/serial/8250/8250_port.c          | 47 ++++++++-------
 drivers/tty/serial/8250/serial_cs.c          | 90 +++++++++++++++-------------
 15 files changed, 203 insertions(+), 198 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1308839 — [PATCH v6 14/15] tty: serial: 8250: Fix indentation warnings

FromAnton Wuerfel <anton.wuerfel@fau.de>
Date2016-01-13 22:40 +0100
Subject[PATCH v6 14/15] tty: serial: 8250: Fix indentation warnings
Message-ID<qQBj6-5yt-49@gated-at.bofh.it>
In reply to#1308838
Checkpatch complains about incorrect indentation of switch/case statements.
This patch fixes the corresponding warnings. Additionally some indentation
is changed to match the correct format specified in the Linux Kernel
Coding Style.

Signed-off-by: Anton Würfel <anton.wuerfel@fau.de>
Signed-off-by: Phillip Raffeck <phillip.raffeck@fau.de>
Cc: linux-kernel@i4.cs.fau.de
---
 drivers/tty/serial/8250/8250_pci.c  | 16 ++++++++--------
 drivers/tty/serial/8250/8250_pnp.c  |  4 ++--
 drivers/tty/serial/8250/8250_port.c |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index b22e5a5..40aa5e7 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -842,15 +842,15 @@ static int pci_netmos_init(struct pci_dev *dev)
 		return 0;
 
 	switch (dev->device) { /* FALLTHROUGH on all */
-		case PCI_DEVICE_ID_NETMOS_9904:
-		case PCI_DEVICE_ID_NETMOS_9912:
-		case PCI_DEVICE_ID_NETMOS_9922:
-		case PCI_DEVICE_ID_NETMOS_9900:
-			num_serial = pci_netmos_9900_numports(dev);
-			break;
+	case PCI_DEVICE_ID_NETMOS_9904:
+	case PCI_DEVICE_ID_NETMOS_9912:
+	case PCI_DEVICE_ID_NETMOS_9922:
+	case PCI_DEVICE_ID_NETMOS_9900:
+		num_serial = pci_netmos_9900_numports(dev);
+		break;
 
-		default:
-			break;
+	default:
+		break;
 	}
 
 	if (num_serial == 0) {
diff --git a/drivers/tty/serial/8250/8250_pnp.c b/drivers/tty/serial/8250/8250_pnp.c
index 92f4412..79bcdf2 100644
--- a/drivers/tty/serial/8250/8250_pnp.c
+++ b/drivers/tty/serial/8250/8250_pnp.c
@@ -427,8 +427,8 @@ static int check_resources(struct pnp_dev *dev)
 static int serial_pnp_guess_board(struct pnp_dev *dev)
 {
 	if (!(check_name(pnp_dev_name(dev)) ||
-		(dev->card && check_name(dev->card->name))))
-			return -ENODEV;
+	    (dev->card && check_name(dev->card->name))))
+		return -ENODEV;
 
 	if (check_resources(dev))
 		return 0;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 25fd6b4..9fe814f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2236,9 +2236,9 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
 		serial_port_out(port, 0x2, quot_frac);
 }
 
-static unsigned int
-serial8250_get_baud_rate(struct uart_port *port, struct ktermios *termios,
-			 struct ktermios *old)
+static unsigned int serial8250_get_baud_rate(struct uart_port *port,
+					     struct ktermios *termios,
+					     struct ktermios *old)
 {
 	unsigned int tolerance = port->uartclk / 100;
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1308840 — [PATCH v6 02/15] tty: serial: 8250: Replace spaces with tabs

FromAnton Wuerfel <anton.wuerfel@fau.de>
Date2016-01-13 22:40 +0100
Subject[PATCH v6 02/15] tty: serial: 8250: Replace spaces with tabs
Message-ID<qQBj6-5yt-53@gated-at.bofh.it>
In reply to#1308838
Indentation is changed to match the correct format of using tabs instead
of spaces wherever possible.

Signed-off-by: Anton Würfel <anton.wuerfel@fau.de>
Signed-off-by: Phillip Raffeck <phillip.raffeck@fau.de>
Cc: linux-kernel@i4.cs.fau.de
---
 drivers/tty/serial/8250/8250_pci.c  | 12 ++++++------
 drivers/tty/serial/8250/8250_port.c |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index ecd5a32..534ca18 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -721,7 +721,7 @@ static int pci_ni8430_init(struct pci_dev *dev)
 	 */
 	pcibios_resource_to_bus(dev->bus, &region, &dev->resource[bar]);
 	device_window = ((region.start + MITE_IOWBSR1_WIN_OFFSET) & 0xffffff00)
-	                | MITE_IOWBSR1_WENAB | MITE_IOWBSR1_WSIZE;
+			| MITE_IOWBSR1_WENAB | MITE_IOWBSR1_WSIZE;
 	writel(device_window, p + MITE_IOWBSR1);
 
 	/* Set window access to go to RAMSEL IO address space */
@@ -1763,7 +1763,7 @@ xr17v35x_has_slave(struct serial_private *priv)
 	const int dev_id = priv->dev->device;
 
 	return ((dev_id == PCI_DEVICE_ID_EXAR_XR17V4358) ||
-	        (dev_id == PCI_DEVICE_ID_EXAR_XR17V8358));
+		(dev_id == PCI_DEVICE_ID_EXAR_XR17V8358));
 }
 
 static int
@@ -1863,8 +1863,8 @@ pci_fastcom335_setup(struct serial_private *priv,
 
 static int
 pci_wch_ch353_setup(struct serial_private *priv,
-                    const struct pciserial_board *board,
-                    struct uart_8250_port *port, int idx)
+		    const struct pciserial_board *board,
+		    struct uart_8250_port *port, int idx)
 {
 	port->port.flags |= UPF_FIXED_TYPE;
 	port->port.type = PORT_16550A;
@@ -1873,8 +1873,8 @@ pci_wch_ch353_setup(struct serial_private *priv,
 
 static int
 pci_wch_ch38x_setup(struct serial_private *priv,
-                    const struct pciserial_board *board,
-                    struct uart_8250_port *port, int idx)
+		    const struct pciserial_board *board,
+		    struct uart_8250_port *port, int idx)
 {
 	port->port.flags |= UPF_FIXED_TYPE;
 	port->port.type = PORT_16850;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 7f3bd7b..f2e588e 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -52,7 +52,7 @@
 #define DEBUG_AUTOCONF(fmt...)	do { } while (0)
 #endif
 
-#define BOTH_EMPTY 	(UART_LSR_TEMT | UART_LSR_THRE)
+#define BOTH_EMPTY	(UART_LSR_TEMT | UART_LSR_THRE)
 
 /*
  * Here we define the default xmit fifo size used for each type of UART.
@@ -2254,7 +2254,7 @@ serial8250_get_baud_rate(struct uart_port *port, struct ktermios *termios,
 
 void
 serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
-		          struct ktermios *old)
+			  struct ktermios *old)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 	unsigned char cval;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1308841 — [PATCH v6 13/15] tty: serial: 8250: Merge duplicate conditions

FromAnton Wuerfel <anton.wuerfel@fau.de>
Date2016-01-13 22:40 +0100
Subject[PATCH v6 13/15] tty: serial: 8250: Merge duplicate conditions
Message-ID<qQBj6-5yt-55@gated-at.bofh.it>
In reply to#1308838
This patch refactors a switch case statement by merging an if condition
in the default case into an identical condition right after the switch
statement.
This comes with a slight change in behaviour: If pci_netmos_9900_numports
returns 0, an additional warning is printed.

Signed-off-by: Anton Würfel <anton.wuerfel@fau.de>
Signed-off-by: Phillip Raffeck <phillip.raffeck@fau.de>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-kernel@i4.cs.fau.de
---
 drivers/tty/serial/8250/8250_pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index fbd4222..b22e5a5 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -850,13 +850,13 @@ static int pci_netmos_init(struct pci_dev *dev)
 			break;
 
 		default:
-			if (num_serial == 0 ) {
-				moan_device("unknown NetMos/Mostech device", dev);
-			}
+			break;
 	}
 
-	if (num_serial == 0)
+	if (num_serial == 0) {
+		moan_device("unknown NetMos/Mostech device", dev);
 		return -ENODEV;
+	}
 
 	return num_serial;
 }
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1308842 — [PATCH v6 08/15] tty: serial: 8250: Add parentheses to macro

FromAnton Wuerfel <anton.wuerfel@fau.de>
Date2016-01-13 22:40 +0100
Subject[PATCH v6 08/15] tty: serial: 8250: Add parentheses to macro
Message-ID<qQBj6-5yt-51@gated-at.bofh.it>
In reply to#1308838
This patch fixes a checkpatch warning caused by missing parentheses
in the definition of a macro.
Furthermore redundant parentheses are removed in an assignment.

Signed-off-by: Anton Würfel <anton.wuerfel@fau.de>
Signed-off-by: Phillip Raffeck <phillip.raffeck@fau.de>
Cc: linux-kernel@i4.cs.fau.de
---
 drivers/tty/serial/8250/8250_core.c | 2 +-
 drivers/tty/serial/8250/8250_pci.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 15082ae..d042ad5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -686,7 +686,7 @@ static int __init univ8250_console_init(void)
 }
 console_initcall(univ8250_console_init);
 
-#define SERIAL8250_CONSOLE	&univ8250_console
+#define SERIAL8250_CONSOLE	(&univ8250_console)
 #else
 #define SERIAL8250_CONSOLE	NULL
 #endif
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 8543719..fbd4222 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -803,7 +803,7 @@ static int pci_netmos_9900_numports(struct pci_dev *dev)
 	unsigned int pi;
 	unsigned short sub_serports;
 
-	pi = (c & 0xff);
+	pi = c & 0xff;
 
 	if (pi == 2)
 		return 1;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1309102

FromAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Date2016-01-14 10:20 +0100
Message-ID<qQMeu-4Y2-9@gated-at.bofh.it>
In reply to#1308838
On Wed, 2016-01-13 at 22:29 +0100, Anton Wuerfel wrote:
> Hello Andy,
> 
> > Also, please thin out the Cc list, I would recommend remove just
> > ordinary commiters (with maybe few exceptions) and leave
> > maintainers. 
> > Moreover some addresses are bounced.
> I thinned out the Cc list and left over only maintainers and major
> contributors to the code.
> 
> > Yep!
> > However, you forgot to fix switch-case as well.
> Phillip and me are kind of git magicians. We can make code disappear
> during
> rebase :-) The piece of code was part of a commit, but it never made
> it into
> the final patch file.
>  
> I hope, there are no major issues with our patch set.

There are comments regarding patch 12, otherwise FWIW:

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


> 
> Regards,
> Anton Wuerfel
> Phillip Raffeck
> 
> This patch set fixes several checkpatch warnings in tty/serial/8250.
> 
> Patch 1/15:  Adds missing spaces (mainly to function/macro headers)
> Patch 2/15:  Replace spaces with tabs wherever possible
> Patch 3/15:  Slight patch which moves an opening curly brace
> Patch 4/15:  Fixes multiline comment style
> Patch 5/15:  Removes else blocks after return statements
> Patch 6/15:  Slight patch which moves EXPORT_SYMBOL macro to correct
> position
> Patch 7/15:  Slight patch which removes an unneccessary line
> continuation
> Patch 8/15:  Slight patch which adds parentheses to a macro
> definition
> Patch 9/15:  Merges user-visible multiline strings to a single line
> Patch 10/15: Replace printk by dev_* or pr_* where appropriate
> Patch 11/15: Remove orphaned debug macro
> Patch 12/15: Fix warnings in dev_dbg by adding two casts
> Patch 13/15: Fixes code indentation
> Patch 14/15: Add a generic port macro
> Patch 15/15: Refactor a switch/case statement
> 
> 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 (15):
> Phillip Raffeck (15):
>   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: Correct conversion specifiers
>   tty: serial: 8250: Merge duplicate conditions
>   tty: serial: 8250: Fix indentation warnings
>   tty: serial: 8250: Add generic port init macro
>   tty: serial: 8250: Suitably replace printk
>   tty: serial: 8250: Remove SERIAL_DEBUG_PNP macro
> 
>  drivers/tty/serial/8250/8250.h               | 12 ++++
>  drivers/tty/serial/8250/8250_accent.c        | 13 +---
>  drivers/tty/serial/8250/8250_acorn.c         |  2 +-
>  drivers/tty/serial/8250/8250_boca.c          | 41 ++++++-------
>  drivers/tty/serial/8250/8250_core.c          | 15 ++---
>  drivers/tty/serial/8250/8250_exar_st16c554.c | 17 ++----
>  drivers/tty/serial/8250/8250_fourport.c      | 28 ++++-----
>  drivers/tty/serial/8250/8250_gsc.c           |  7 ++-
>  drivers/tty/serial/8250/8250_hp300.c         | 27 +++++----
>  drivers/tty/serial/8250/8250_hub6.c          |  2 +-
>  drivers/tty/serial/8250/8250_ingenic.c       | 12 ++--
>  drivers/tty/serial/8250/8250_pci.c           | 60 ++++++++++------
> ---
>  drivers/tty/serial/8250/8250_pnp.c           | 28 +++++----
>  drivers/tty/serial/8250/8250_port.c          | 47 ++++++++-------
>  drivers/tty/serial/8250/serial_cs.c          | 90 +++++++++++++++---
> ----------
>  15 files changed, 203 insertions(+), 198 deletions(-)
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web