[PATCH 0/3] Implement ACPI SPCR v3 support and minor earlycon cleanup

[PATCH 0/3] Implement ACPI SPCR v3 support and minor earlycon cleanup

[Raul E Rangel]

Getting `earlycon` working on AMD platforms is a bit tricky because the
uartclk is 48MHz. If the uartclk isn't specified, we default to
115200*16 which results in an incorrect divisor computation.

This patch set adds the uartclk to the `console=` documentation,
prints a notice to the user that it's using a default uartclk, and
implements support for SPCR v3 so that the uartclk can be provided
by the firmware.


Raul E Rangel (3):
  Documentation: kernel-parameters: Add uartclk to console=uart
  earlycon: Print a notice when uartclk is unknown
  ACPI: SPCR: Add support for rev 3

 Documentation/admin-guide/kernel-parameters.txt | 16 ++++++----------
 drivers/acpi/spcr.c                             |  5 ++++-
 drivers/tty/serial/earlycon.c                   |  6 +++++-
 include/acpi/actbl3.h                           |  6 +++---
 4 files changed, 18 insertions(+), 15 deletions(-)

-- 
2.46.0.662.g92d0881bb0-goog

[PATCH 1/3] Documentation: kernel-parameters: Add uartclk to console=uart

[Raul E Rangel]

The console=uartXXXXX parameter is just an alias for earlycon=uartXXXX.
This means it also accepts the uartclk parameter.

Signed-off-by: Raul E Rangel <rrangel@chromium.org>

---

 Documentation/admin-guide/kernel-parameters.txt | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bc1036e73bc0b4..cd3d44db60457d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -788,20 +788,16 @@
 			Documentation/networking/netconsole.rst for an
 			alternative.
 
-		uart[8250],io,<addr>[,options]
-		uart[8250],mmio,<addr>[,options]
-		uart[8250],mmio16,<addr>[,options]
-		uart[8250],mmio32,<addr>[,options]
+		uart[8250],io,<addr>[,options[,uartclk]]
+		uart[8250],mmio,<addr>[,options[,uartclk]]
+		uart[8250],mmio16,<addr>[,options[,uartclk]]
+		uart[8250],mmio32,<addr>[,options[,uartclk]]
 		uart[8250],0x<addr>[,options]
 			Start an early, polled-mode console on the 8250/16550
 			UART at the specified I/O port or MMIO address,
 			switching to the matching ttyS device later.
-			MMIO inter-register address stride is either 8-bit
-			(mmio), 16-bit (mmio16), or 32-bit (mmio32).
-			If none of [io|mmio|mmio16|mmio32], <addr> is assumed
-			to be equivalent to 'mmio'. 'options' are specified in
-			the same format described for ttyS above; if unspecified,
-			the h/w is not re-initialized.
+
+			See the documentation for earlycon=uart
 
 		hvc<n>	Use the hypervisor console device <n>. This is for
 			both Xen and PowerPC hypervisors.
-- 
2.46.0.662.g92d0881bb0-goog

Re: [PATCH 1/3] Documentation: kernel-parameters: Add uartclk to console=uart

[Petr Mladek]

On Thu 2024-09-12 11:36:19, Raul E Rangel wrote:
> The console=uartXXXXX parameter is just an alias for earlycon=uartXXXX.
> This means it also accepts the uartclk parameter.
> 
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH 2/3] earlycon: Print a notice when uartclk is unknown

[Raul E Rangel]

When trying to construct an earlycon=uart parameter it's hard to debug
why it's not working.  In my specific case it was because the default
uartclk earlycon assumes doesn't match my hardware. This change adds a
notice so that the user is made aware of that this assumption is being
made. This should hopefully lead to them adding a <uartclk> option to
their earlycon parameter.

Booting with `console=uart,mmio32,0xfedc9000,115200n8`:
[    0.000000] earlycon: uart: Unknown uartclk, assuming 1843200hz
[    0.000000] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200n8')

Signed-off-by: Raul E Rangel <rrangel@chromium.org>

---

 drivers/tty/serial/earlycon.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index ab9af37f6cda35..5a8fe0cb3b1986 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -145,8 +145,12 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
 		buf = NULL;
 
 	spin_lock_init(&port->lock);
-	if (!port->uartclk)
+	if (!port->uartclk) {
 		port->uartclk = BASE_BAUD * 16;
+		if (early_console_dev.baud)
+			pr_notice("%s: Unknown uartclk, assuming %dhz",
+				  match->name, port->uartclk);
+	}
 	if (port->mapbase)
 		port->membase = earlycon_map(port->mapbase, 64);
 
-- 
2.46.0.662.g92d0881bb0-goog

Re: [PATCH 2/3] earlycon: Print a notice when uartclk is unknown

[Greg Kroah-Hartman]

On Thu, Sep 12, 2024 at 11:36:20AM -0600, Raul E Rangel wrote:
> When trying to construct an earlycon=uart parameter it's hard to debug
> why it's not working.  In my specific case it was because the default
> uartclk earlycon assumes doesn't match my hardware. This change adds a
> notice so that the user is made aware of that this assumption is being
> made. This should hopefully lead to them adding a <uartclk> option to
> their earlycon parameter.
> 
> Booting with `console=uart,mmio32,0xfedc9000,115200n8`:
> [    0.000000] earlycon: uart: Unknown uartclk, assuming 1843200hz
> [    0.000000] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200n8')
> 
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 2/3] earlycon: Print a notice when uartclk is unknown

[Petr Mladek]

On Thu 2024-09-12 11:36:20, Raul E Rangel wrote:
> When trying to construct an earlycon=uart parameter it's hard to debug
> why it's not working.  In my specific case it was because the default
> uartclk earlycon assumes doesn't match my hardware. This change adds a
> notice so that the user is made aware of that this assumption is being
> made. This should hopefully lead to them adding a <uartclk> option to
> their earlycon parameter.
> 
> Booting with `console=uart,mmio32,0xfedc9000,115200n8`:
> [    0.000000] earlycon: uart: Unknown uartclk, assuming 1843200hz
> [    0.000000] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200n8')
> 
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH 3/3] ACPI: SPCR: Add support for rev 3

[Raul E Rangel]

Revision 3 supports specifying the UART input clock. This allows for
proper computation of the UART divisor when the baud rate is specified.

The earlycon code can accept the following format (See `parse_options`
in `earlycon.c`.):
* <name>,io|mmio|mmio32|mmio32be,<addr>,<baud>,<uartclk>,<options>

This change makes it so the uartclk is passed along if it's defined in
the SPCR table.

Booting with `earlycon` and a SPCR v3 table that has the uartclk and
baud defined:
[    0.028251] ACPI: SPCR: console: uart,mmio32,0xfedc9000,115200,48000000
[    0.028267] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200,48000000')
[    0.028272] printk: legacy bootconsole [uart0] enabled

Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table

Signed-off-by: Raul E Rangel <rrangel@chromium.org>

---

 drivers/acpi/spcr.c   | 5 ++++-
 include/acpi/actbl3.h | 6 +++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index cd36a97b0ea2c7..67ae42afcc59ef 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -209,9 +209,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
 	if (!baud_rate) {
 		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
 			 table->serial_port.address);
-	} else {
+	} else if (table->header.revision <= 2 || !table->uartclk) {
 		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
 			 table->serial_port.address, baud_rate);
+	} else {
+		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d,%d", uart, iotype,
+			 table->serial_port.address, baud_rate, table->uartclk);
 	}
 
 	pr_info("console: %s\n", opts);
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index 8f775e3a08fdfb..afe45a2379866a 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -92,10 +92,10 @@ struct acpi_table_slit {
 /*******************************************************************************
  *
  * SPCR - Serial Port Console Redirection table
- *        Version 2
+ *        Version 3
  *
  * Conforms to "Serial Port Console Redirection Table",
- * Version 1.03, August 10, 2015
+ * Version 1.08, October 7, 2021
  *
  ******************************************************************************/
 
@@ -120,7 +120,7 @@ struct acpi_table_spcr {
 	u8 pci_function;
 	u32 pci_flags;
 	u8 pci_segment;
-	u32 reserved2;
+	u32 uartclk;
 };
 
 /* Masks for pci_flags field above */
-- 
2.46.0.662.g92d0881bb0-goog

Re: [PATCH 3/3] ACPI: SPCR: Add support for rev 3

[Petr Mladek]

On Thu 2024-09-12 11:36:21, Raul E Rangel wrote:
> Revision 3 supports specifying the UART input clock. This allows for
> proper computation of the UART divisor when the baud rate is specified.
> 
> The earlycon code can accept the following format (See `parse_options`
> in `earlycon.c`.):
> * <name>,io|mmio|mmio32|mmio32be,<addr>,<baud>,<uartclk>,<options>
> 
> This change makes it so the uartclk is passed along if it's defined in
> the SPCR table.
> 
> Booting with `earlycon` and a SPCR v3 table that has the uartclk and
> baud defined:
> [    0.028251] ACPI: SPCR: console: uart,mmio32,0xfedc9000,115200,48000000
> [    0.028267] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200,48000000')
> [    0.028272] printk: legacy bootconsole [uart0] enabled
> 
> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> 
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH 3/3] ACPI: SPCR: Add support for rev 3

[Rafael J. Wysocki]

On Thu, Sep 12, 2024 at 7:39 PM Raul E Rangel <rrangel@chromium.org> wrote:
>
> Revision 3 supports specifying the UART input clock. This allows for
> proper computation of the UART divisor when the baud rate is specified.
>
> The earlycon code can accept the following format (See `parse_options`
> in `earlycon.c`.):
> * <name>,io|mmio|mmio32|mmio32be,<addr>,<baud>,<uartclk>,<options>
>
> This change makes it so the uartclk is passed along if it's defined in
> the SPCR table.
>
> Booting with `earlycon` and a SPCR v3 table that has the uartclk and
> baud defined:
> [    0.028251] ACPI: SPCR: console: uart,mmio32,0xfedc9000,115200,48000000
> [    0.028267] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200,48000000')
> [    0.028272] printk: legacy bootconsole [uart0] enabled
>
> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
>
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>
>
> ---
>
>  drivers/acpi/spcr.c   | 5 ++++-
>  include/acpi/actbl3.h | 6 +++---
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index cd36a97b0ea2c7..67ae42afcc59ef 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -209,9 +209,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
>         if (!baud_rate) {
>                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
>                          table->serial_port.address);
> -       } else {
> +       } else if (table->header.revision <= 2 || !table->uartclk) {
>                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
>                          table->serial_port.address, baud_rate);
> +       } else {
> +               snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d,%d", uart, iotype,
> +                        table->serial_port.address, baud_rate, table->uartclk);
>         }
>
>         pr_info("console: %s\n", opts);
> diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> index 8f775e3a08fdfb..afe45a2379866a 100644
> --- a/include/acpi/actbl3.h
> +++ b/include/acpi/actbl3.h

The part of the patch below is outdated - SPCR v4 is supported already.

Please rebase on the current mainline kernel source.

> @@ -92,10 +92,10 @@ struct acpi_table_slit {
>  /*******************************************************************************
>   *
>   * SPCR - Serial Port Console Redirection table
> - *        Version 2
> + *        Version 3
>   *
>   * Conforms to "Serial Port Console Redirection Table",
> - * Version 1.03, August 10, 2015
> + * Version 1.08, October 7, 2021
>   *
>   ******************************************************************************/
>
> @@ -120,7 +120,7 @@ struct acpi_table_spcr {
>         u8 pci_function;
>         u32 pci_flags;
>         u8 pci_segment;
> -       u32 reserved2;
> +       u32 uartclk;
>  };
>
>  /* Masks for pci_flags field above */
> --
> 2.46.0.662.g92d0881bb0-goog
>

Re: [PATCH 3/3] ACPI: SPCR: Add support for rev 3

[Raul Rangel]

On Wed, Oct 2, 2024 at 12:13 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Sep 12, 2024 at 7:39 PM Raul E Rangel <rrangel@chromium.org> wrote:
> >
> > Revision 3 supports specifying the UART input clock. This allows for
> > proper computation of the UART divisor when the baud rate is specified.
> >
> > The earlycon code can accept the following format (See `parse_options`
> > in `earlycon.c`.):
> > * <name>,io|mmio|mmio32|mmio32be,<addr>,<baud>,<uartclk>,<options>
> >
> > This change makes it so the uartclk is passed along if it's defined in
> > the SPCR table.
> >
> > Booting with `earlycon` and a SPCR v3 table that has the uartclk and
> > baud defined:
> > [    0.028251] ACPI: SPCR: console: uart,mmio32,0xfedc9000,115200,48000000
> > [    0.028267] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200,48000000')
> > [    0.028272] printk: legacy bootconsole [uart0] enabled
> >
> > Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> >
> > Signed-off-by: Raul E Rangel <rrangel@chromium.org>
> >
> > ---
> >
> >  drivers/acpi/spcr.c   | 5 ++++-
> >  include/acpi/actbl3.h | 6 +++---
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > index cd36a97b0ea2c7..67ae42afcc59ef 100644
> > --- a/drivers/acpi/spcr.c
> > +++ b/drivers/acpi/spcr.c
> > @@ -209,9 +209,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
> >         if (!baud_rate) {
> >                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> >                          table->serial_port.address);
> > -       } else {
> > +       } else if (table->header.revision <= 2 || !table->uartclk) {
> >                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> >                          table->serial_port.address, baud_rate);
> > +       } else {
> > +               snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d,%d", uart, iotype,
> > +                        table->serial_port.address, baud_rate, table->uartclk);
> >         }
> >
> >         pr_info("console: %s\n", opts);
> > diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> > index 8f775e3a08fdfb..afe45a2379866a 100644
> > --- a/include/acpi/actbl3.h
> > +++ b/include/acpi/actbl3.h
>
> The part of the patch below is outdated - SPCR v4 is supported already.
>
> Please rebase on the current mainline kernel source.

Oh awesome. Should I send out all three patches again? Or just this
one? I think patches 1 and 2 can be merged.

>
> > @@ -92,10 +92,10 @@ struct acpi_table_slit {
> >  /*******************************************************************************
> >   *
> >   * SPCR - Serial Port Console Redirection table
> > - *        Version 2
> > + *        Version 3
> >   *
> >   * Conforms to "Serial Port Console Redirection Table",
> > - * Version 1.03, August 10, 2015
> > + * Version 1.08, October 7, 2021
> >   *
> >   ******************************************************************************/
> >
> > @@ -120,7 +120,7 @@ struct acpi_table_spcr {
> >         u8 pci_function;
> >         u32 pci_flags;
> >         u8 pci_segment;
> > -       u32 reserved2;
> > +       u32 uartclk;
> >  };
> >
> >  /* Masks for pci_flags field above */
> > --
> > 2.46.0.662.g92d0881bb0-goog
> >

Re: [PATCH 3/3] ACPI: SPCR: Add support for rev 3

[Rafael J. Wysocki]

On Fri, Oct 4, 2024 at 7:45 PM Raul Rangel <rrangel@chromium.org> wrote:
>
> On Wed, Oct 2, 2024 at 12:13 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Sep 12, 2024 at 7:39 PM Raul E Rangel <rrangel@chromium.org> wrote:
> > >
> > > Revision 3 supports specifying the UART input clock. This allows for
> > > proper computation of the UART divisor when the baud rate is specified.
> > >
> > > The earlycon code can accept the following format (See `parse_options`
> > > in `earlycon.c`.):
> > > * <name>,io|mmio|mmio32|mmio32be,<addr>,<baud>,<uartclk>,<options>
> > >
> > > This change makes it so the uartclk is passed along if it's defined in
> > > the SPCR table.
> > >
> > > Booting with `earlycon` and a SPCR v3 table that has the uartclk and
> > > baud defined:
> > > [    0.028251] ACPI: SPCR: console: uart,mmio32,0xfedc9000,115200,48000000
> > > [    0.028267] earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200,48000000')
> > > [    0.028272] printk: legacy bootconsole [uart0] enabled
> > >
> > > Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table
> > >
> > > Signed-off-by: Raul E Rangel <rrangel@chromium.org>
> > >
> > > ---
> > >
> > >  drivers/acpi/spcr.c   | 5 ++++-
> > >  include/acpi/actbl3.h | 6 +++---
> > >  2 files changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > > index cd36a97b0ea2c7..67ae42afcc59ef 100644
> > > --- a/drivers/acpi/spcr.c
> > > +++ b/drivers/acpi/spcr.c
> > > @@ -209,9 +209,12 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
> > >         if (!baud_rate) {
> > >                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> > >                          table->serial_port.address);
> > > -       } else {
> > > +       } else if (table->header.revision <= 2 || !table->uartclk) {
> > >                 snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> > >                          table->serial_port.address, baud_rate);
> > > +       } else {
> > > +               snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d,%d", uart, iotype,
> > > +                        table->serial_port.address, baud_rate, table->uartclk);
> > >         }
> > >
> > >         pr_info("console: %s\n", opts);
> > > diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> > > index 8f775e3a08fdfb..afe45a2379866a 100644
> > > --- a/include/acpi/actbl3.h
> > > +++ b/include/acpi/actbl3.h
> >
> > The part of the patch below is outdated - SPCR v4 is supported already.
> >
> > Please rebase on the current mainline kernel source.
>
> Oh awesome. Should I send out all three patches again? Or just this
> one? I think patches 1 and 2 can be merged.

I have only received patch [3/3] and this one needs to be resent as
far as I'm concerned.