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

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

[Daniel Kurtz]

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 <djkurtz@chromium.org>
---
 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

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Andy Shevchenko]

On Wed, Mar 14, 2018 at 2:36 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> 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
>

Thanks, this is what I meant.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Suggested-by: ?

> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  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
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Ricardo Ribalda Delgado]

Hi Daniel


On Wed, Mar 14, 2018 at 1:36 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>
> 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


If the port and the mode (mmio32) is always fixed, couldn't we just
add those two into
early_amdcz_setup?


Thanks!

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Daniel Kurtz]

Hi Ricardo,

On Wed, Mar 14, 2018 at 4:54 AM Ricardo Ribalda Delgado <
ricardo.ribalda@gmail.com> wrote:

> Hi Daniel


> On Wed, Mar 14, 2018 at 1:36 AM, Daniel Kurtz <djkurtz@chromium.org>
wrote:
> >
> > 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


> If the port and the mode (mmio32) is always fixed, couldn't we just
> add those two into
> early_amdcz_setup?

There are multiple memory mapped UARTs on at least the one chip I am aware
of, specifying the address here chooses which port to use as the early
console.

In fact, the recommended way is to have firmware specify an ACPI SPCR table
with OEMID="AMDCZ " (see https://patchwork.kernel.org/patch/10281307/) to
configure proper access and address.  With an SPCR table in place,  the
kernel command line just becomes "earlycon", with no parameters.

-Dan

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Andy Shevchenko]

On Wed, Mar 14, 2018 at 6:29 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> On Wed, Mar 14, 2018 at 4:54 AM Ricardo Ribalda Delgado <
> ricardo.ribalda@gmail.com> wrote:
>> On Wed, Mar 14, 2018 at 1:36 AM, Daniel Kurtz <djkurtz@chromium.org>
> wrote:

> In fact, the recommended way is to have firmware specify an ACPI SPCR table
> with OEMID="AMDCZ " (see https://patchwork.kernel.org/patch/10281307/) to
> configure proper access and address.

Hmm... I was thinking it's already there. And thus, this is just a
quirk for *existing* firmware that doesn't correctly configured
hardware.
(Yes, I'm aware about one nuance in SPCR specification I'm trying to
address via official ways)

>  With an SPCR table in place,  the
> kernel command line just becomes "earlycon", with no parameters.

SPCR *provides* an address of UART (required by specification).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Aaron Durbin]

On Wed, Mar 14, 2018 at 10:38 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 14, 2018 at 6:29 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>> On Wed, Mar 14, 2018 at 4:54 AM Ricardo Ribalda Delgado <
>> ricardo.ribalda@gmail.com> wrote:
>>> On Wed, Mar 14, 2018 at 1:36 AM, Daniel Kurtz <djkurtz@chromium.org>
>> wrote:
>
>> In fact, the recommended way is to have firmware specify an ACPI SPCR table
>> with OEMID="AMDCZ " (see https://patchwork.kernel.org/patch/10281307/) to
>> configure proper access and address.
>
> Hmm... I was thinking it's already there. And thus, this is just a
> quirk for *existing* firmware that doesn't correctly configured
> hardware.
> (Yes, I'm aware about one nuance in SPCR specification I'm trying to
> address via official ways)
>
>>  With an SPCR table in place,  the
>> kernel command line just becomes "earlycon", with no parameters.
>
> SPCR *provides* an address of UART (required by specification).

What is "it's" in your first sentence? The access method? Baud rate
can't be configured ever in the kernel w/o knowing the input clock to
the uart block. That's already been brought up, and it is inherently a
requirement to know that to recalculate the divisor. These patches are
doing early OOB binding to set the proper input clock because: 1. SPCR
doesn't have that information 2. you nak'd the generic way of
specifying the input clock on the command line. Sadly, this situation
is not unique to this hardware. There is hardware all over that does
not meet the current assumptions being made in the early uart drivers
within the kernel.

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Andy Shevchenko]

On Wed, Mar 14, 2018 at 7:12 PM, Aaron Durbin <adurbin@chromium.org> wrote:
> On Wed, Mar 14, 2018 at 10:38 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Wed, Mar 14, 2018 at 6:29 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>> On Wed, Mar 14, 2018 at 4:54 AM Ricardo Ribalda Delgado <
>>> ricardo.ribalda@gmail.com> wrote:
>>>> On Wed, Mar 14, 2018 at 1:36 AM, Daniel Kurtz <djkurtz@chromium.org>
>>> wrote:
>>
>>> In fact, the recommended way is to have firmware specify an ACPI SPCR table
>>> with OEMID="AMDCZ " (see https://patchwork.kernel.org/patch/10281307/) to
>>> configure proper access and address.
>>
>> Hmm... I was thinking it's already there. And thus, this is just a
>> quirk for *existing* firmware that doesn't correctly configured
>> hardware.
>> (Yes, I'm aware about one nuance in SPCR specification I'm trying to
>> address via official ways)
>>
>>>  With an SPCR table in place,  the
>>> kernel command line just becomes "earlycon", with no parameters.
>>
>> SPCR *provides* an address of UART (required by specification).
>
> What is "it's" in your first sentence? The access method?

*SPCR table* itself on the platforms in question.

>  There is hardware all over that does
> not meet the current assumptions being made in the early uart drivers
> within the kernel.

I know, that's why I'm working now on a proposal to SPCR specification
to make our life slightly easier in that sense.

P.S. In case you are interested, I have crafted SPCR in U-Boot for
Intel Edison (ACPI case) where UART clock is also non-standard and did
some tests. It works quite nicely when firmware, **iff written
properly**, configures UART beforehand. In this case no clock
information is needed, no code needs to be added into the kernel.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Alan Cox]

> Sadly, this situation
> is not unique to this hardware. There is hardware all over that does
> not meet the current assumptions being made in the early uart drivers
> within the kernel.

Is there any fundamental reason you can't just embed dt entries in the
ACPI table to describe the other features you need. I appreciate it
doesn't solve the generic PC case but it ought to help for anything where
the firmware cares about Linux ?

Alan

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Aaron Durbin]

On Mon, Mar 26, 2018 at 12:24 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> Sadly, this situation
>> is not unique to this hardware. There is hardware all over that does
>> not meet the current assumptions being made in the early uart drivers
>> within the kernel.
>
> Is there any fundamental reason you can't just embed dt entries in the
> ACPI table to describe the other features you need. I appreciate it
> doesn't solve the generic PC case but it ought to help for anything where
> the firmware cares about Linux ?

What's the method for doing that? Using _DSD methods? Or have a
pointer to examples? Sorry, I haven't spelunked into the current state
of bridging ACPI and devicetree in a while.

-Aaron

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Alan Cox]

On Mon, 26 Mar 2018 20:56:45 -0600
Aaron Durbin <adurbin@chromium.org> wrote:

> On Mon, Mar 26, 2018 at 12:24 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> >> Sadly, this situation
> >> is not unique to this hardware. There is hardware all over that does
> >> not meet the current assumptions being made in the early uart drivers
> >> within the kernel.  
> >
> > Is there any fundamental reason you can't just embed dt entries in the
> > ACPI table to describe the other features you need. I appreciate it
> > doesn't solve the generic PC case but it ought to help for anything where
> > the firmware cares about Linux ?  
> 
> What's the method for doing that? Using _DSD methods? Or have a
> pointer to examples? Sorry, I haven't spelunked into the current state
> of bridging ACPI and devicetree in a while.

ACPI 5.1 adds an _DSD method UUID for device properties.

The kernel device_property_* interface will pick them up just as if they
came from DT tables etc.

Alan

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Aaron Durbin]

On Thu, Mar 29, 2018 at 6:34 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> On Mon, 26 Mar 2018 20:56:45 -0600
> Aaron Durbin <adurbin@chromium.org> wrote:
>
>> On Mon, Mar 26, 2018 at 12:24 PM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> >> Sadly, this situation
>> >> is not unique to this hardware. There is hardware all over that does
>> >> not meet the current assumptions being made in the early uart drivers
>> >> within the kernel.
>> >
>> > Is there any fundamental reason you can't just embed dt entries in the
>> > ACPI table to describe the other features you need. I appreciate it
>> > doesn't solve the generic PC case but it ought to help for anything where
>> > the firmware cares about Linux ?
>>
>> What's the method for doing that? Using _DSD methods? Or have a
>> pointer to examples? Sorry, I haven't spelunked into the current state
>> of bridging ACPI and devicetree in a while.
>
> ACPI 5.1 adds an _DSD method UUID for device properties.
>
> The kernel device_property_* interface will pick them up just as if they
> came from DT tables etc.

But we don't have the full ACPI interpreter up in the early part of
the kernel. All these 'early' devices have their own setup/config
which is the source of the issue. Or maybe I am wrong about the full
interpreter and the early drivers are just not taking advantage of the
ACPI device binding?

-Aaron

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Alan Cox]

> But we don't have the full ACPI interpreter up in the early part of
> the kernel. All these 'early' devices have their own setup/config
> which is the source of the issue. Or maybe I am wrong about the full
> interpreter and the early drivers are just not taking advantage of the
> ACPI device binding?

In very early boot with serial console you just have to pray. No change
there. Once ACPI comes up you however have the information to populate
everything and configure correctly.

So it should work fine except for kernel hackers trying to do early boot
debug, which is a small (but important) cornercase ?

Alan

Re: [PATCH 1/3] serial: 8250_early: Add earlycon support for AMD Carrizo / Stoneyridge

[Aaron Durbin]

On Fri, Apr 6, 2018 at 7:47 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>> But we don't have the full ACPI interpreter up in the early part of
>> the kernel. All these 'early' devices have their own setup/config
>> which is the source of the issue. Or maybe I am wrong about the full
>> interpreter and the early drivers are just not taking advantage of the
>> ACPI device binding?
>
> In very early boot with serial console you just have to pray. No change
> there. Once ACPI comes up you however have the information to populate
> everything and configure correctly.
>
> So it should work fine except for kernel hackers trying to do early boot
> debug, which is a small (but important) cornercase ?

Yes, I think we're on the same page. This patch series is trying to
get the early console working correctly given the current code
assumptions.

-Aaron

[PATCH 3/3] serial: core: Allow skipping old serial port initialization

[Daniel Kurtz]

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 most 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).

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

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/tty/serial/8250/8250_core.c  | 6 ++++++
 drivers/tty/serial/8250/8250_early.c | 1 +
 include/linux/serial_8250.h          | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..91ee206096f1 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 skip_old_serial_ports;
+EXPORT_SYMBOL(skip_old_serial_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 (skip_old_serial_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..8511b8e25b3f 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;
 
+	skip_old_serial_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..cb6fd144529c 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -146,6 +146,8 @@ void serial8250_unregister_port(int line);
 void serial8250_suspend_port(int line);
 void serial8250_resume_port(int line);
 
+extern bool skip_old_serial_ports;
+
 extern int early_serial_setup(struct uart_port *port);
 
 extern int early_serial8250_setup(struct earlycon_device *device,
-- 
2.16.2.804.g6dcf76e118-goog