[PATCH v1 1/2] serial: 8250: Let serial core initialise spin lock

[PATCH v1 1/2] serial: 8250: Let serial core initialise spin lock

[Andy Shevchenko]

Since the serial core handles spin lock initialisation,
let the driver rely on it.

Depends-on: f743061a85f5 ("serial: core: Initialise spin lock before use in uart_configure_port()")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 09475695effd..b2d18189d3d9 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3194,7 +3194,6 @@ void serial8250_init_port(struct uart_8250_port *up)
 {
 	struct uart_port *port = &up->port;
 
-	spin_lock_init(&port->lock);
 	port->ops = &serial8250_pops;
 	port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
 
-- 
2.27.0

[PATCH v1 2/2] serial: 8250: Explicitly show we initialise ISA ports only once

[Andy Shevchenko]

serial8250_isa_init_ports() uses home grown approach to make itself
a singleton. Instead, explicitly show that we initialise ISA ports
once by providing a helper function which calls the original function
via DO_ONCE() macro.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index cae61d1ebec5..9c0d6693f745 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -22,6 +22,7 @@
 #include <linux/console.h>
 #include <linux/sysrq.h>
 #include <linux/delay.h>
+#include <linux/once.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/ratelimit.h>
@@ -495,13 +496,8 @@ static inline void serial8250_apply_quirks(struct uart_8250_port *up)
 static void __init serial8250_isa_init_ports(void)
 {
 	struct uart_8250_port *up;
-	static int first = 1;
 	int i, irqflag = 0;
 
-	if (!first)
-		return;
-	first = 0;
-
 	if (nr_uarts > UART_NR)
 		nr_uarts = UART_NR;
 
@@ -555,6 +551,11 @@ static void __init serial8250_isa_init_ports(void)
 	}
 }
 
+static void __init serial8250_isa_init_ports_once(void)
+{
+	DO_ONCE(serial8250_isa_init_ports);
+}
+
 static void __init
 serial8250_register_ports(struct uart_driver *drv, struct device *dev)
 {
@@ -686,7 +687,7 @@ static int __init univ8250_console_init(void)
 	if (nr_uarts == 0)
 		return -ENODEV;
 
-	serial8250_isa_init_ports();
+	serial8250_isa_init_ports_once();
 	register_console(&univ8250_console);
 	return 0;
 }
@@ -719,7 +720,7 @@ int __init early_serial_setup(struct uart_port *port)
 	if (port->line >= ARRAY_SIZE(serial8250_ports) || nr_uarts == 0)
 		return -ENODEV;
 
-	serial8250_isa_init_ports();
+	serial8250_isa_init_ports_once();
 	p = &serial8250_ports[port->line].port;
 	p->iobase       = port->iobase;
 	p->membase      = port->membase;
@@ -1170,7 +1171,7 @@ static int __init serial8250_init(void)
 	if (nr_uarts == 0)
 		return -ENODEV;
 
-	serial8250_isa_init_ports();
+	serial8250_isa_init_ports_once();
 
 	pr_info("Serial: 8250/16550 driver, %d ports, IRQ sharing %sabled\n",
 		nr_uarts, share_irqs ? "en" : "dis");
-- 
2.27.0

Re: [PATCH v1 2/2] serial: 8250: Explicitly show we initialise ISA ports only once

[Greg Kroah-Hartman]

On Fri, Jul 31, 2020 at 03:37:33PM +0300, Andy Shevchenko wrote:
> serial8250_isa_init_ports() uses home grown approach to make itself
> a singleton. Instead, explicitly show that we initialise ISA ports
> once by providing a helper function which calls the original function
> via DO_ONCE() macro.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_core.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index cae61d1ebec5..9c0d6693f745 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -22,6 +22,7 @@
>  #include <linux/console.h>
>  #include <linux/sysrq.h>
>  #include <linux/delay.h>
> +#include <linux/once.h>
>  #include <linux/platform_device.h>
>  #include <linux/tty.h>
>  #include <linux/ratelimit.h>
> @@ -495,13 +496,8 @@ static inline void serial8250_apply_quirks(struct uart_8250_port *up)
>  static void __init serial8250_isa_init_ports(void)
>  {
>  	struct uart_8250_port *up;
> -	static int first = 1;
>  	int i, irqflag = 0;
>  
> -	if (!first)
> -		return;
> -	first = 0;
> -
>  	if (nr_uarts > UART_NR)
>  		nr_uarts = UART_NR;
>  
> @@ -555,6 +551,11 @@ static void __init serial8250_isa_init_ports(void)
>  	}
>  }
>  
> +static void __init serial8250_isa_init_ports_once(void)
> +{
> +	DO_ONCE(serial8250_isa_init_ports);

DO_ONCE() is there for fast-path code, where the code is then patched
out of the kernel after it is run.  That's not the case here and is not
needed, please just do this "normally".

thanks,

greg k-h