2018-03-15 02:07:27

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH v3 0/3] Add earlycon support for AMD Carrizo / Stoneyridge

AMD Carrizo / Stoneyridge use a DesignWare 8250 UART that uses a 48 MHz
input clock. Currently, there is no way to tell earlycon to use a specific
uart input clock when configuring a baud rate.

Patch 1 adds an earlycon ->setup() callback to set up this clock. The setup is
triggered on the commandline by specifying "earlycon=amdcz,...".

Patch 2 adds an ACPI SPCR quirk to automatically trigger this hook when the
ACPI SPCR table OEMID field is "AMDCZ ".

Patch 3 adds an additional fix which allows botht the ACPI SPCR quirk and amdcz
->setup() to tell 8250_core to skip initialization of old serial ports that do
not exist on these SoCs.

Daniel Kurtz (3):
serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge
ACPI: SPCR: Add support for AMD CT/SZ
serial: core: Allow skipping old serial port initialization

drivers/acpi/spcr.c | 26 ++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_core.c | 6 ++++++
drivers/tty/serial/8250/8250_early.c | 16 ++++++++++++++++
include/linux/serial_8250.h | 2 ++
4 files changed, 50 insertions(+)

--
2.16.2.804.g6dcf76e118-goog



2018-03-15 02:06:18

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH v3 3/3] serial: core: Allow skipping old serial port initialization

The old_serial_port global array in 8250_core is supposed to hold an entry
for each serial port on the system that cannot be discovered via a
standard enumeration mechanism (aka ACPI/PCI/DTS). The array is populated
at compile-time from the value specified in the SERIAL_PORT_DFNS macro.
This macro is defined in arch/serial.h.

For x86, this macro is currently unconditionally initialized to supply
four ioport UARTs (0x3F8, 0x2F8, 0x3E8, 0x2E8).

However, not all x86 CPUs have these four ioport UARTs. For example, the
UARTs on AMD Carrizo and later are separate memory mapped Designware IP
blocks.

Fairly early in boot the console_initcall univ8250_console_init iterates
over this array and installs these old UARTs into the global array
serial8250_ports. Further, it attempts to register them for use as
the console. In other words, if, for example, the kernel commandline has
console=ttyS0, the console will be switched over to one of these
non-existent UARTs. Only later, when the real UART drivers are probed
and their devices are instantiated will the console switch back over to
the proper UART.

This is noticeable when using earlycon, since part of the serial console
log will appear to disappear (when the bogus old takes over) and then
re-appear (when the real UART finally gets registered for the console).

The problem is even more noticable when *not* using earlycon, since in
this case the entire console output is missing, having been incorrectly
played back to the non-existing serial port.

Create a global variable to allow skipping old serial port initialization
and wire it up to the AMDCZ ACPI SPCR quirk and the special amdcz earlycon
setup handler.

Signed-off-by: Daniel Kurtz <[email protected]>
---
Changes since v1:
* Rename variable to serial8250_skip_old_ports
* Also set variable in acpi/spcr to support no-earlycon case.

Changes since v2:
* Added IS_ENABLED(CONFIG_SERIAL_825) and removed the .h #ifdef as suggested
by Kees Cook.


drivers/acpi/spcr.c | 3 +++
drivers/tty/serial/8250/8250_core.c | 6 ++++++
drivers/tty/serial/8250/8250_early.c | 1 +
include/linux/serial_8250.h | 2 ++
4 files changed, 12 insertions(+)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 52d840d0e05b..d1affdf2bc37 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/console.h>
#include <linux/kernel.h>
+#include <linux/serial_8250.h>
#include <linux/serial_core.h>

/*
@@ -208,6 +209,8 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
}

if (amdcz_present(table)) {
+ if (IS_ENABLED(CONFIG_SERIAL_8250))
+ serial8250_skip_old_ports = true;
if (enable_earlycon)
uart = "amdcz";
}
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..a04da392a251 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -66,6 +66,9 @@ static unsigned int skip_txen_test; /* force skip of txen test at init time */
#define SERIAL_PORT_DFNS
#endif

+bool serial8250_skip_old_ports;
+EXPORT_SYMBOL(serial8250_skip_old_ports);
+
static const struct old_serial_port old_serial_port[] = {
SERIAL_PORT_DFNS /* defined in asm/serial.h */
};
@@ -537,6 +540,9 @@ static void __init serial8250_isa_init_ports(void)
if (share_irqs)
irqflag = IRQF_SHARED;

+ if (serial8250_skip_old_ports)
+ return;
+
for (i = 0, up = serial8250_ports;
i < ARRAY_SIZE(old_serial_port) && i < nr_uarts;
i++, up++) {
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index c6bf971a6038..288d2be82990 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -202,6 +202,7 @@ static int __init early_amdcz_setup(struct earlycon_device *dev,
{
struct uart_port *port = &dev->port;

+ serial8250_skip_old_ports = true;
port->uartclk = 48000000;

return early_serial8250_setup(dev, opt);
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index a27ef5f56431..ae739c34546f 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -136,6 +136,8 @@ struct uart_8250_port {
struct uart_8250_em485 *em485;
};

+extern bool serial8250_skip_old_ports;
+
static inline struct uart_8250_port *up_to_u8250p(struct uart_port *up)
{
return container_of(up, struct uart_8250_port, port);
--
2.16.2.804.g6dcf76e118-goog


2018-03-15 02:06:29

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH v3 2/3] ACPI: SPCR: Add support for AMD CT/SZ

AMD Carrizo / Stoneyridge use a DesignWare 8250 UART that uses a special
earlycon setup handler to configure its input clock in order to compute
baud rate divisor registers.
Detect them by examining the OEMID field in the SPCR header, and pass
then pass uart type amdcz to earlycon.

Signed-off-by: Daniel Kurtz <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
Changes since v1:
* added Reviewed-by

drivers/acpi/spcr.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 9d52743080a4..52d840d0e05b 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -73,6 +73,24 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
return xgene_8250;
}

+/*
+ * AMD Carrizo / Stoneyridge use a DesignWare 8250 UART that uses a special
+ * earlycon setup handler to configure its input clock in order to compute
+ * baud rate divisor registers.
+ * Detect them by examining the OEM fields in the SPCR header.
+ */
+static bool amdcz_present(struct acpi_table_spcr *tb)
+{
+ if (memcmp(tb->header.oem_id, "AMDCZ ", ACPI_OEM_ID_SIZE))
+ return false;
+
+ if (memcmp(tb->header.oem_table_id, "AMDCZ ",
+ ACPI_OEM_TABLE_ID_SIZE))
+ return false;
+
+ return true;
+}
+
/**
* acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
*
@@ -189,6 +207,11 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
uart = "qdf2400_e44";
}

+ if (amdcz_present(table)) {
+ if (enable_earlycon)
+ uart = "amdcz";
+ }
+
if (xgene_8250_erratum_present(table)) {
iotype = "mmio32";

--
2.16.2.804.g6dcf76e118-goog


2018-03-15 02:07:54

by Daniel Kurtz

[permalink] [raw]
Subject: [PATCH v3 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

AMD Carrizo / Stoneyridge use a DesignWare 8250 UART that uses a 48 MHz
input clock.

Allow these platforms to set up this clock by specifying a kernel command
line like:
earlycon=amdcz,mmio32,0xfedc6000,115200

Signed-off-by: Daniel Kurtz <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
Changes since v1:
* added Reviewed-by & Suggested-by

drivers/tty/serial/8250/8250_early.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index ae6a256524d8..c6bf971a6038 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -195,3 +195,18 @@ static int __init early_au_setup(struct earlycon_device *dev, const char *opt)
OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_au_setup);

#endif
+
+#ifdef CONFIG_SERIAL_8250_DW
+static int __init early_amdcz_setup(struct earlycon_device *dev,
+ const char *opt)
+{
+ struct uart_port *port = &dev->port;
+
+ port->uartclk = 48000000;
+
+ return early_serial8250_setup(dev, opt);
+}
+
+EARLYCON_DECLARE(amdcz, early_amdcz_setup);
+
+#endif
--
2.16.2.804.g6dcf76e118-goog


2018-03-15 13:06:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] serial: core: Allow skipping old serial port initialization

On Wed, 2018-03-14 at 20:04 -0600, Daniel Kurtz wrote:
> The old_serial_port global array in 8250_core is supposed to hold an
> entry
> for each serial port on the system that cannot be discovered via a
> standard enumeration mechanism (aka ACPI/PCI/DTS). The array is
> populated
> at compile-time from the value specified in the SERIAL_PORT_DFNS
> macro.
> This macro is defined in arch/serial.h.
>
> For x86, this macro is currently unconditionally initialized to supply
> four ioport UARTs (0x3F8, 0x2F8, 0x3E8, 0x2E8).
>
> However, not all x86 CPUs have these four ioport UARTs. For example,
> the
> UARTs on AMD Carrizo and later are separate memory mapped Designware
> IP
> blocks.
>
> Fairly early in boot the console_initcall univ8250_console_init
> iterates
> over this array and installs these old UARTs into the global array
> serial8250_ports. Further, it attempts to register them for use as
> the console. In other words, if, for example, the kernel commandline
> has
> console=ttyS0, the console will be switched over to one of these
> non-existent UARTs. Only later, when the real UART drivers are probed
> and their devices are instantiated will the console switch back over
> to
> the proper UART.
>
> This is noticeable when using earlycon, since part of the serial
> console
> log will appear to disappear (when the bogus old takes over) and then
> re-appear (when the real UART finally gets registered for the
> console).
>
> The problem is even more noticable when *not* using earlycon, since in
> this case the entire console output is missing, having been
> incorrectly
> played back to the non-existing serial port.
>
> Create a global variable to allow skipping old serial port
> initialization
> and wire it up to the AMDCZ ACPI SPCR quirk and the special amdcz
> earlycon
> setup handler.

I don't like this approach at all.
But unfortunately I have nothing to propose.
Just felt like I have to share my opinion on this.

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2018-03-15 16:26:36

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] serial: core: Allow skipping old serial port initialization

On Thu, Mar 15, 2018 at 7:04 AM Andy Shevchenko <
[email protected]> wrote:

> On Wed, 2018-03-14 at 20:04 -0600, Daniel Kurtz wrote:
> > The old_serial_port global array in 8250_core is supposed to hold an
> > entry
> > for each serial port on the system that cannot be discovered via a
> > standard enumeration mechanism (aka ACPI/PCI/DTS). The array is
> > populated
> > at compile-time from the value specified in the SERIAL_PORT_DFNS
> > macro.
> > This macro is defined in arch/serial.h.
> >
> > For x86, this macro is currently unconditionally initialized to supply
> > four ioport UARTs (0x3F8, 0x2F8, 0x3E8, 0x2E8).
> >
> > However, not all x86 CPUs have these four ioport UARTs. For example,
> > the
> > UARTs on AMD Carrizo and later are separate memory mapped Designware
> > IP
> > blocks.
> >
> > Fairly early in boot the console_initcall univ8250_console_init
> > iterates
> > over this array and installs these old UARTs into the global array
> > serial8250_ports. Further, it attempts to register them for use as
> > the console. In other words, if, for example, the kernel commandline
> > has
> > console=ttyS0, the console will be switched over to one of these
> > non-existent UARTs. Only later, when the real UART drivers are probed
> > and their devices are instantiated will the console switch back over
> > to
> > the proper UART.
> >
> > This is noticeable when using earlycon, since part of the serial
> > console
> > log will appear to disappear (when the bogus old takes over) and then
> > re-appear (when the real UART finally gets registered for the
> > console).
> >
> > The problem is even more noticable when *not* using earlycon, since in
> > this case the entire console output is missing, having been
> > incorrectly
> > played back to the non-existing serial port.
> >
> > Create a global variable to allow skipping old serial port
> > initialization
> > and wire it up to the AMDCZ ACPI SPCR quirk and the special amdcz
> > earlycon
> > setup handler.

> I don't like this approach at all.
> But unfortunately I have nothing to propose.
> Just felt like I have to share my opinion on this.

I'm not very proud of this either, but I don't know how else to set this
flag.
I proposed a Kconfig option earlier, but that also met resistance.
Perhaps another kernel command line option?


> --
> Andy Shevchenko <[email protected]>
> Intel Finland Oy

2018-03-18 07:31:57

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] serial: core: Allow skipping old serial port initialization

Hi Daniel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Daniel-Kurtz/Add-earlycon-support-for-AMD-Carrizo-Stoneyridge/20180318-024830
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64

All errors (new ones prefixed by >>):

drivers/acpi/spcr.o: In function `acpi_parse_spcr':
>> spcr.c:(.init.text+0x540): undefined reference to `serial8250_skip_old_ports'
spcr.c:(.init.text+0x560): undefined reference to `serial8250_skip_old_ports'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.11 kB)
.config.gz (49.02 kB)
Download all attachments