[PATCH] serial: earlycon: prefer EARLYCON_DECLARE() variant

[PATCH] serial: earlycon: prefer EARLYCON_DECLARE() variant

[Michael Walle]

If a driver exposes early consoles with EARLYCON_DECLARE() and
OF_EARLYCON_DECLARE(), pefer the non-OF variant if the user specifies it
by
  earlycon=<driver>,<options>

The rationale behind this is that some drivers register multiple setup
functions under the same driver name. Eg.

OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart", lpuart_early_console_setup);
OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart", lpuart32_early_console_setup);
OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart", lpuart32_imx_early_console_setup);
EARLYCON_DECLARE(lpuart, lpuart_early_console_setup);
EARLYCON_DECLARE(lpuart32, lpuart32_early_console_setup);

It depends on the order of the entries which console_setup() actually
gets called. To make things worse, I guess it also depends on the
compiler how these are ordered. Thus always prefer the EARLYCON_DECLARE()
ones.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/tty/serial/earlycon.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index c14873b67803..2ae9190b64bb 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -170,6 +170,7 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
 int __init setup_earlycon(char *buf)
 {
 	const struct earlycon_id **p_match;
+	bool empty_compatible = true;
 
 	if (!buf || !buf[0])
 		return -EINVAL;
@@ -177,6 +178,7 @@ int __init setup_earlycon(char *buf)
 	if (early_con.flags & CON_ENABLED)
 		return -EALREADY;
 
+again:
 	for (p_match = __earlycon_table; p_match < __earlycon_table_end;
 	     p_match++) {
 		const struct earlycon_id *match = *p_match;
@@ -185,6 +187,10 @@ int __init setup_earlycon(char *buf)
 		if (strncmp(buf, match->name, len))
 			continue;
 
+		/* prefer entries with empty compatible */
+		if (empty_compatible && *match->compatible)
+			continue;
+
 		if (buf[len]) {
 			if (buf[len] != ',')
 				continue;
@@ -195,6 +201,11 @@ int __init setup_earlycon(char *buf)
 		return register_earlycon(buf, match);
 	}
 
+	if (empty_compatible) {
+		empty_compatible = false;
+		goto again;
+	}
+
 	return -ENOENT;
 }
 
-- 
2.20.1

Re: [PATCH] serial: earlycon: prefer EARLYCON_DECLARE() variant

[Michael Walle]

Hi Greg,


Am 2020-02-20 18:46, schrieb Michael Walle:
> If a driver exposes early consoles with EARLYCON_DECLARE() and
> OF_EARLYCON_DECLARE(), pefer the non-OF variant if the user specifies 
> it
> by
>   earlycon=<driver>,<options>
> 
> The rationale behind this is that some drivers register multiple setup
> functions under the same driver name. Eg.
> 
> OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart", 
> lpuart_early_console_setup);
> OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart",
> lpuart32_early_console_setup);
> OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart",
> lpuart32_imx_early_console_setup);
> EARLYCON_DECLARE(lpuart, lpuart_early_console_setup);
> EARLYCON_DECLARE(lpuart32, lpuart32_early_console_setup);
> 
> It depends on the order of the entries which console_setup() actually
> gets called. To make things worse, I guess it also depends on the
> compiler how these are ordered. Thus always prefer the 
> EARLYCON_DECLARE()
> ones.

Do you have an opinon on this proposal?

-michael

> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/tty/serial/earlycon.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/tty/serial/earlycon.c 
> b/drivers/tty/serial/earlycon.c
> index c14873b67803..2ae9190b64bb 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -170,6 +170,7 @@ static int __init register_earlycon(char *buf,
> const struct earlycon_id *match)
>  int __init setup_earlycon(char *buf)
>  {
>  	const struct earlycon_id **p_match;
> +	bool empty_compatible = true;
> 
>  	if (!buf || !buf[0])
>  		return -EINVAL;
> @@ -177,6 +178,7 @@ int __init setup_earlycon(char *buf)
>  	if (early_con.flags & CON_ENABLED)
>  		return -EALREADY;
> 
> +again:
>  	for (p_match = __earlycon_table; p_match < __earlycon_table_end;
>  	     p_match++) {
>  		const struct earlycon_id *match = *p_match;
> @@ -185,6 +187,10 @@ int __init setup_earlycon(char *buf)
>  		if (strncmp(buf, match->name, len))
>  			continue;
> 
> +		/* prefer entries with empty compatible */
> +		if (empty_compatible && *match->compatible)
> +			continue;
> +
>  		if (buf[len]) {
>  			if (buf[len] != ',')
>  				continue;
> @@ -195,6 +201,11 @@ int __init setup_earlycon(char *buf)
>  		return register_earlycon(buf, match);
>  	}
> 
> +	if (empty_compatible) {
> +		empty_compatible = false;
> +		goto again;
> +	}
> +
>  	return -ENOENT;
>  }

Re: [PATCH] serial: earlycon: prefer EARLYCON_DECLARE() variant

[Greg Kroah-Hartman]

On Fri, Feb 28, 2020 at 11:26:36AM +0100, Michael Walle wrote:
> Hi Greg,
> 
> 
> Am 2020-02-20 18:46, schrieb Michael Walle:
> > If a driver exposes early consoles with EARLYCON_DECLARE() and
> > OF_EARLYCON_DECLARE(), pefer the non-OF variant if the user specifies it
> > by
> >   earlycon=<driver>,<options>
> > 
> > The rationale behind this is that some drivers register multiple setup
> > functions under the same driver name. Eg.
> > 
> > OF_EARLYCON_DECLARE(lpuart, "fsl,vf610-lpuart",
> > lpuart_early_console_setup);
> > OF_EARLYCON_DECLARE(lpuart32, "fsl,ls1021a-lpuart",
> > lpuart32_early_console_setup);
> > OF_EARLYCON_DECLARE(lpuart32, "fsl,imx7ulp-lpuart",
> > lpuart32_imx_early_console_setup);
> > EARLYCON_DECLARE(lpuart, lpuart_early_console_setup);
> > EARLYCON_DECLARE(lpuart32, lpuart32_early_console_setup);
> > 
> > It depends on the order of the entries which console_setup() actually
> > gets called. To make things worse, I guess it also depends on the
> > compiler how these are ordered. Thus always prefer the
> > EARLYCON_DECLARE()
> > ones.
> 
> Do you have an opinon on this proposal?

It's only been a week, please give me a chance to catch up on serial
patches...

thanks,

greg k-h