[RFC v2 0/4] ACPI: SPCR: 32-bit access and non-standard baud rate

[RFC v2 0/4] ACPI: SPCR: 32-bit access and non-standard baud rate

[Aleksey Makarov]

ACPI [SPCR] (Serial Port Console Redirection Table) specifies which console
should be used by system.  'ARM Server Base Boot Requirements' [SBBR] mentions
SPCR as a mandatory ACPI table.  Support for this table for Linux kernel
([SPCR v10]) was merged for 4.9 (patches 1/4-3/4) and 4.10 (patch 4/4).

It turns out that this approach does not work for all the existing hardware.
There are two problems with AppliedMicro X-Gene based boards
([discussion], [v1]):

1. Their console is a 16550 port that requires 32-bit access.  Now SPCR does not
   have any method to specify this.

2. Some of the boards don't use the "standard" 16550 clock rate so supplying
   a baud rate makes it change to a random baud rate.

Patch 1/4 uses 'Register Bit Width' field of the ACPI Generic Address
Structure that specifies the address of the UART registers to
decide if the driver should use "mmio32" access instead of "mmio".
This fixes problem 1 for existing hardware/firmware.

To fix problem 2, I suggest to introduce a new value '0' for the "Baud Rate"
field of SPCR (now this value is reserved).  I would like to discuss if this
could be added to SPCR spec and will fix the problem.

Patch 2/4 introduces a check for this value.  In this case the code does not
emit baud rate specification for initialization of the console.  This fixes
problem 2.

It was also suggested ([v1]) to add a new Microsoft Debug Port Table 2 ([DBG2])
(the table used to enumerate the various subtypes of serial port covered by the
SPCR) 16550 UART subtype that may be needed for some additional platforms,
such as those based upon AppliedMicro X-Gene ARMv8 SoCs.  This new subtype would
be 16550-compatible with 32-bit access.  There already exists 32-bit variant
ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.

Patch 3/4 introduces this value for DBG2 table.

Patch 4/4 uses it.

RFC v2:
  - Add a fix for consoles with non-standard baud rate.
  - Introduce a new type of console that is 16550 with 32-bit access
  - Check for that type of console.

v1: [v1]

[SPCR]       https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
[DBG2]       http://go.microsoft.com/fwlink/p/?LinkId=234837
[SBBR]       http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[SPCR v10]   https://lkml.kernel.org/r/20160905123617.18775-1-aleksey.makarov@linaro.org
[discussion] https://lkml.kernel.org/r/7fa523de-3fbb-1566-f521-927143f73d1e@redhat.com
[v1]         https://lkml.kernel.org/r/20161205130534.11080-1-aleksey.makarov@linaro.org

Aleksey Makarov (4):
  ACPI: SPCR: check bit width for the 16550 UART
  ACPI: SPCR: don't initialize baud rate
  ACPI: DBG2: add 16550 UART with 32-bit access
  ACPI: SPCR: support 16550 UART with 32-bit access

 drivers/acpi/spcr.c   | 29 ++++++++++++++++++++++-------
 include/acpi/actbl2.h |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

-- 
2.10.2

[RFC v2 1/4] ACPI: SPCR: check bit width for the 16550 UART

[Aleksey Makarov]

Check the 'Register Bit Width' field of the ACPI Generic Address
Structure that specifies the address of the UART registers to
decide if the driver should use "mmio32" access instead of "mmio".

If the driver is other than 16550 the access width is defined
by the Interface Type field of the SPCR table.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org>
Reported-by: Heyi Guo <heyi.guo@linaro.org>
---
 drivers/acpi/spcr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index e8d7bc7..6c6710b 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -70,6 +70,10 @@ int __init parse_spcr(bool earlycon)
 		break;
 	case ACPI_DBG2_16550_COMPATIBLE:
 	case ACPI_DBG2_16550_SUBSET:
+		if (table->serial_port.space_id ==
+			ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+		    table->serial_port.bit_width == 32)
+			iotype = "mmio32";
 		uart = "uart";
 		break;
 	default:
-- 
2.10.2

[RFC v2 2/4] ACPI: SPCR: don't initialize baud rate

[Aleksey Makarov]

AppliedMicro X-Gene based boards don't use the "standard"
16550 clock rate so supplying a baud rate makes it change
to a random baud rate.

I suggest to introduce a new value '0' for the "Baud
Rate" field of SPCR (now this value is reserved).

This patch introduces a check for this value.  In this case
the code does not emit baud rate specification for initialization
of the console.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 drivers/acpi/spcr.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 6c6710b..2bf338c 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -37,7 +37,7 @@ int __init parse_spcr(bool earlycon)
 	acpi_status status;
 	char *uart;
 	char *iotype;
-	int baud_rate;
+	char *baud_rate_string;
 	int err;
 
 	if (acpi_disabled)
@@ -82,25 +82,33 @@ int __init parse_spcr(bool earlycon)
 	}
 
 	switch (table->baud_rate) {
+	case 0:
+		/*
+		 * This value is not standaritzed by ACPI SPCR for now.
+		 * It means that hardware should not be re-initialized with
+		 * new speed.
+		 */
+		baud_rate_string = "";
+		break;
 	case 3:
-		baud_rate = 9600;
+		baud_rate_string = ",9600";
 		break;
 	case 4:
-		baud_rate = 19200;
+		baud_rate_string = ",19200";
 		break;
 	case 6:
-		baud_rate = 57600;
+		baud_rate_string = ",57600";
 		break;
 	case 7:
-		baud_rate = 115200;
+		baud_rate_string = ",115200";
 		break;
 	default:
 		err = -ENOENT;
 		goto done;
 	}
 
-	snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
-		 table->serial_port.address, baud_rate);
+	snprintf(opts, sizeof(opts), "%s,%s,0x%llx%s", uart, iotype,
+		 table->serial_port.address, baud_rate_string);
 
 	pr_info("console: %s\n", opts);
 
-- 
2.10.2

[RFC v2 3/4] ACPI: DBG2: add 16550 UART with 32-bit access

[Aleksey Makarov]

It was suggested to add a new Microsoft Debug Port Table 2
(DBG2) (the table used to enumerate the various subtypes of serial
port covered by the SPCR) 16550 UART subtype that may be needed for
some additional platforms, such as those based upon AppliedMicro
X-Gene ARMv8 SoCs.  This new subtype would be 16550-compatible
with 32-bit access.  There already exists 32-bit variant
ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.

This patch introduces it to Linux kernel.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 include/acpi/actbl2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index c93dbad..f219b04 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -371,6 +371,7 @@ struct acpi_dbg2_device {
 
 #define ACPI_DBG2_16550_COMPATIBLE  0x0000
 #define ACPI_DBG2_16550_SUBSET      0x0001
+#define ACPI_DBG2_16550_32BIT       0x0002
 #define ACPI_DBG2_ARM_PL011         0x0003
 #define ACPI_DBG2_ARM_SBSA_32BIT    0x000D
 #define ACPI_DBG2_ARM_SBSA_GENERIC  0x000E
-- 
2.10.2

[RFC v2 4/4] ACPI: SPCR: support 16550 UART with 32-bit access

[Aleksey Makarov]

It was suggested to add a new Microsoft Debug Port Table 2
(DBG2) (the table used to enumerate the various subtypes of serial
port covered by the SPCR) 16550 UART subtype that may be needed for
some additional platforms, such as those based upon AppliedMicro
X-Gene ARMv8 SoCs.  This new subtype would be 16550-compatible
with 32-bit access.  There already exists 32-bit variant
ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.

This patch supports this type of console.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 drivers/acpi/spcr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 2bf338c..bc17e77 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -68,6 +68,9 @@ int __init parse_spcr(bool earlycon)
 	case ACPI_DBG2_BCM2835:
 		uart = "pl011";
 		break;
+	case ACPI_DBG2_16550_32BIT:
+		iotype = "mmio32";
+		/* fall through */
 	case ACPI_DBG2_16550_COMPATIBLE:
 	case ACPI_DBG2_16550_SUBSET:
 		if (table->serial_port.space_id ==
-- 
2.10.2

Re: [RFC v2 0/4] ACPI: SPCR: 32-bit access and non-standard baud rate

[Jon Masters]

Hi Aleksey,

On 12/06/2016 12:58 PM, Aleksey Makarov wrote:

> It turns out that this approach does not work for all the existing hardware.
> There are two problems with AppliedMicro X-Gene based boards
> ([discussion], [v1]):
> 
> 1. Their console is a 16550 port that requires 32-bit access.  Now SPCR does not
>    have any method to specify this.
> 
> 2. Some of the boards don't use the "standard" 16550 clock rate so supplying
>    a baud rate makes it change to a random baud rate.
> 
> Patch 1/4 uses 'Register Bit Width' field of the ACPI Generic Address
> Structure that specifies the address of the UART registers to
> decide if the driver should use "mmio32" access instead of "mmio".
> This fixes problem 1 for existing hardware/firmware.
> 
> To fix problem 2, I suggest to introduce a new value '0' for the "Baud Rate"
> field of SPCR (now this value is reserved).  I would like to discuss if this
> could be added to SPCR spec and will fix the problem.

Part 1 could well be solved in this way, provided it can be made part
of the mainstream specification. It seems fairly reasonable, however.

I'm not sure I subscribe to part 2 because there could be all manner
of havoc changing the interpretation of existing zero values. That is
why I personally favor an AppliedMicro SPCR subtype specific to them.

That all said, I did discuss exactly part 2 with the folks responsible
for the SPCR specification and they are thinking about their preferred
solution, which will likely either be a new subtype for Applied, or
perhaps allow for what you describe. I will followup when I know.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop

Re: [RFC v2 3/4] ACPI: DBG2: add 16550 UART with 32-bit access

[Rafael J. Wysocki]

On Tue, Dec 6, 2016 at 6:58 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> It was suggested to add a new Microsoft Debug Port Table 2
> (DBG2) (the table used to enumerate the various subtypes of serial
> port covered by the SPCR) 16550 UART subtype that may be needed for
> some additional platforms, such as those based upon AppliedMicro
> X-Gene ARMv8 SoCs.  This new subtype would be 16550-compatible
> with 32-bit access.  There already exists 32-bit variant
> ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.
>
> This patch introduces it to Linux kernel.
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  include/acpi/actbl2.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index c93dbad..f219b04 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -371,6 +371,7 @@ struct acpi_dbg2_device {
>
>  #define ACPI_DBG2_16550_COMPATIBLE  0x0000
>  #define ACPI_DBG2_16550_SUBSET      0x0001
> +#define ACPI_DBG2_16550_32BIT       0x0002
>  #define ACPI_DBG2_ARM_PL011         0x0003
>  #define ACPI_DBG2_ARM_SBSA_32BIT    0x000D
>  #define ACPI_DBG2_ARM_SBSA_GENERIC  0x000E
> --

This is an ACPICA change and it first needs to be made to the upstream
ACPICA code base.

Thanks,
Rafael

Re: [RFC v2 3/4] ACPI: DBG2: add 16550 UART with 32-bit access

[Jon Masters]

Quick note - specifically, Rafael, this is not currently part of the SPCR specification and should not be considered for inclusion in Linux until that is decided. Once it is determined what the fix for SPCR is, that should then be driven into the component architecture and proposed here.

-- 
Computer Architect | Sent from my 64-bit #ARM Powered phone

> On Dec 6, 2016, at 16:00, Rafael J. Wysocki <rafael@kernel.org> wrote:
> 
> On Tue, Dec 6, 2016 at 6:58 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> It was suggested to add a new Microsoft Debug Port Table 2
>> (DBG2) (the table used to enumerate the various subtypes of serial
>> port covered by the SPCR) 16550 UART subtype that may be needed for
>> some additional platforms, such as those based upon AppliedMicro
>> X-Gene ARMv8 SoCs.  This new subtype would be 16550-compatible
>> with 32-bit access.  There already exists 32-bit variant
>> ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.
>> 
>> This patch introduces it to Linux kernel.
>> 
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> ---
>> include/acpi/actbl2.h | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index c93dbad..f219b04 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -371,6 +371,7 @@ struct acpi_dbg2_device {
>> 
>> #define ACPI_DBG2_16550_COMPATIBLE  0x0000
>> #define ACPI_DBG2_16550_SUBSET      0x0001
>> +#define ACPI_DBG2_16550_32BIT       0x0002
>> #define ACPI_DBG2_ARM_PL011         0x0003
>> #define ACPI_DBG2_ARM_SBSA_32BIT    0x000D
>> #define ACPI_DBG2_ARM_SBSA_GENERIC  0x000E
>> --
> 
> This is an ACPICA change and it first needs to be made to the upstream
> ACPICA code base.
> 
> Thanks,
> Rafael

Re: [RFC v2 3/4] ACPI: DBG2: add 16550 UART with 32-bit access

[Rafael J. Wysocki]

On Tue, Dec 6, 2016 at 10:15 PM, Jon Masters <jcm@redhat.com> wrote:
> Quick note - specifically, Rafael, this is not currently part of the SPCR specification and
> should not be considered for inclusion in Linux until that is decided. Once it is determined
> what the fix for SPCR is, that should then be driven into the component architecture and
> proposed here.

OK, good to know, thanks!


> --
> Computer Architect | Sent from my 64-bit #ARM Powered phone
>
>> On Dec 6, 2016, at 16:00, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Tue, Dec 6, 2016 at 6:58 PM, Aleksey Makarov
>> <aleksey.makarov@linaro.org> wrote:
>>> It was suggested to add a new Microsoft Debug Port Table 2
>>> (DBG2) (the table used to enumerate the various subtypes of serial
>>> port covered by the SPCR) 16550 UART subtype that may be needed for
>>> some additional platforms, such as those based upon AppliedMicro
>>> X-Gene ARMv8 SoCs.  This new subtype would be 16550-compatible
>>> with 32-bit access.  There already exists 32-bit variant
>>> ACPI_DBG2_ARM_SBSA_32BIT of SBSA console ACPI_DBG2_ARM_SBSA_GENERIC.
>>>
>>> This patch introduces it to Linux kernel.
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>> ---
>>> include/acpi/actbl2.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>> index c93dbad..f219b04 100644
>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -371,6 +371,7 @@ struct acpi_dbg2_device {
>>>
>>> #define ACPI_DBG2_16550_COMPATIBLE  0x0000
>>> #define ACPI_DBG2_16550_SUBSET      0x0001
>>> +#define ACPI_DBG2_16550_32BIT       0x0002
>>> #define ACPI_DBG2_ARM_PL011         0x0003
>>> #define ACPI_DBG2_ARM_SBSA_32BIT    0x000D
>>> #define ACPI_DBG2_ARM_SBSA_GENERIC  0x000E
>>> --
>>
>> This is an ACPICA change and it first needs to be made to the upstream
>> ACPICA code base.
>>
>> Thanks,
>> Rafael