All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI
@ 2017-11-02 10:13 Bhupinder Thakur
  2017-11-02 10:13 ` [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform Bhupinder Thakur
  2017-11-02 12:15 ` [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Julien Grall
  0 siblings, 2 replies; 5+ messages in thread
From: Bhupinder Thakur @ 2017-11-02 10:13 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich

Currently, Xen supports only DT based initialization of 16550 UART.
This patch adds support for initializing 16550 UART using ACPI SPCR table.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Julien Grall <julien.grall@arm.com>

 xen/drivers/char/ns16550.c  | 57 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/8250-uart.h |  1 +
 2 files changed, 58 insertions(+)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index e0f8199..b3f6d85 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1538,6 +1538,63 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
 DT_DEVICE_END
 
 #endif /* HAS_DEVICE_TREE */
+
+#ifdef CONFIG_ACPI
+#include <xen/acpi.h>
+
+static int __init ns16550_acpi_uart_init(const void *data)
+{
+    struct ns16550 *uart;
+    acpi_status status;
+    struct acpi_table_spcr *spcr = NULL;
+
+    status = acpi_get_table(ACPI_SIG_SPCR, 0,
+                            (struct acpi_table_header **)&spcr);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("ns16550: Failed to get SPCR table\n");
+        return -EINVAL;
+    }
+
+    uart = &ns16550_com[0];
+
+    ns16550_init_common(uart);
+
+    uart->baud      = BAUD_AUTO;
+    uart->data_bits = 8;
+    uart->parity    = spcr->parity;
+    uart->stop_bits = spcr->stop_bits;
+    uart->io_base = spcr->serial_port.address;
+    uart->irq = spcr->interrupt;
+    uart->reg_width = spcr->serial_port.bit_width/8;
+    uart->reg_shift = 0;
+    uart->io_size = UART_MAX_REG<<uart->reg_shift;
+
+    irq_set_type(spcr->interrupt, spcr->interrupt_type);
+
+    uart->vuart.base_addr = uart->io_base;
+    uart->vuart.size = uart->io_size;
+    uart->vuart.data_off = UART_THR <<uart->reg_shift;
+    uart->vuart.status_off = UART_LSR<<uart->reg_shift;
+    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;
+
+    /* Register with generic serial driver. */
+    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
+
+    return 0;
+}
+
+ACPI_DEVICE_START(ns16550c, "16550 COMPAT UART", DEVICE_SERIAL)
+        .class_type = ACPI_DBG2_16550_COMPATIBLE,
+        .init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
+ACPI_DEVICE_START(ns16550s, "16550 SUBSET UART", DEVICE_SERIAL)
+        .class_type = ACPI_DBG2_16550_SUBSET,
+        .init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
+
+#endif
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index 5c3bac3..1b3e137 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -35,6 +35,7 @@
 #define UART_USR          0x1f    /* Status register (DW) */
 #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
 #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
+#define UART_MAX_REG      (UART_USR+1)
 
 /* Interrupt Enable Register */
 #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform
  2017-11-02 10:13 [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Bhupinder Thakur
@ 2017-11-02 10:13 ` Bhupinder Thakur
  2017-11-02 12:20   ` Julien Grall
  2017-11-02 12:15 ` [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Julien Grall
  1 sibling, 1 reply; 5+ messages in thread
From: Bhupinder Thakur @ 2017-11-02 10:13 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich

    The console was not working on HP Moonshot (HPE Proliant Aarch64) because
    the UART registers were accessed as 8-bit aligned addresses. However,
    registers are 32-bit aligned for HP Moonshot.

    Since ACPI/SPCR table does not specify the register shift to be applied to the
    register offset, this patch implements an erratum to correctly set the register
    shift for HP Moonshot.

    Similar erratum was implemented in linux:

    commit 79a648328d2a604524a30523ca763fbeca0f70e3
    Author: Loc Ho <lho@apm.com>
    Date:   Mon Jul 3 14:33:09 2017 -0700

        ACPI: SPCR: Workaround for APM X-Gene 8250 UART 32-alignment errata

        APM X-Gene verion 1 and 2 have an 8250 UART with its register
        aligned to 32-bit. In addition, the latest released BIOS
        encodes the access field as 8-bit access instead 32-bit access.
        This causes no console with ACPI boot as the console
        will not match X-Gene UART port due to the lack of mmio32
        option.

        Signed-off-by: Loc Ho <lho@apm.com>
        Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
        Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Julien Grall <julien.grall@arm.com>

 xen/drivers/char/ns16550.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index b3f6d85..e716aba 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1542,6 +1542,33 @@ DT_DEVICE_END
 #ifdef CONFIG_ACPI
 #include <xen/acpi.h>
 
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+    bool xgene_8250 = false;
+
+    if ( tb->interface_type != ACPI_DBG2_16550_COMPATIBLE )
+        return false;
+
+    if ( memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+         memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE) )
+        return false;
+
+    if ( !memcmp(tb->header.oem_table_id, "XGENESPC",
+         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0 )
+        xgene_8250 = true;
+
+    if ( !memcmp(tb->header.oem_table_id, "ProLiant",
+         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1 )
+        xgene_8250 = true;
+
+    return xgene_8250;
+}
+
 static int __init ns16550_acpi_uart_init(const void *data)
 {
     struct ns16550 *uart;
@@ -1568,9 +1595,20 @@ static int __init ns16550_acpi_uart_init(const void *data)
     uart->io_base = spcr->serial_port.address;
     uart->irq = spcr->interrupt;
     uart->reg_width = spcr->serial_port.bit_width/8;
-    uart->reg_shift = 0;
-    uart->io_size = UART_MAX_REG<<uart->reg_shift;
 
+    if ( xgene_8250_erratum_present(spcr) )
+    {
+        /*
+         * for xgene v1 and v2 the registers are 32-bit and so a
+         * register shift of 2 has to be applied to get the
+         * correct register offset.
+         */
+        uart->reg_shift = 2;
+    }
+    else
+        uart->reg_shift = 0;
+
+    uart->io_size = UART_MAX_REG<<uart->reg_shift;
     irq_set_type(spcr->interrupt, spcr->interrupt_type);
 
     uart->vuart.base_addr = uart->io_base;
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI
  2017-11-02 10:13 [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Bhupinder Thakur
  2017-11-02 10:13 ` [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform Bhupinder Thakur
@ 2017-11-02 12:15 ` Julien Grall
  2017-11-08  6:16   ` Bhupinder Thakur
  1 sibling, 1 reply; 5+ messages in thread
From: Julien Grall @ 2017-11-02 12:15 UTC (permalink / raw)
  To: Bhupinder Thakur, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich

Hi Bhupinder,

Please write a cover letter even if it is small when your send a series 
with multiple patches.

On 02/11/17 10:13, Bhupinder Thakur wrote:
> Currently, Xen supports only DT based initialization of 16550 UART.
> This patch adds support for initializing 16550 UART using ACPI SPCR table.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> 
>   xen/drivers/char/ns16550.c  | 57 +++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/xen/8250-uart.h |  1 +
>   2 files changed, 58 insertions(+)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0f8199..b3f6d85 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1538,6 +1538,63 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
>   DT_DEVICE_END
>   
>   #endif /* HAS_DEVICE_TREE */
> +
> +#ifdef CONFIG_ACPI

The code below is going to break x86 build. You need to do #if 
defined(CONFIG_ACPI) && defined(CONFIG_ARM)

> +#include <xen/acpi.h>
> +
> +static int __init ns16550_acpi_uart_init(const void *data)
> +{
> +    struct ns16550 *uart;
> +    acpi_status status;
> +    struct acpi_table_spcr *spcr = NULL;
> +
> +    status = acpi_get_table(ACPI_SIG_SPCR, 0,
> +                            (struct acpi_table_header **)&spcr);
> +
> +    if ( ACPI_FAILURE(status) )
> +    {
> +        printk("ns16550: Failed to get SPCR table\n");
> +        return -EINVAL;
> +    }
> +
> +    uart = &ns16550_com[0];
> +
> +    ns16550_init_common(uart);
> +
> +    uart->baud      = BAUD_AUTO;
> +    uart->data_bits = 8;
> +    uart->parity    = spcr->parity;
> +    uart->stop_bits = spcr->stop_bits;
> +    uart->io_base = spcr->serial_port.address;
> +    uart->irq = spcr->interrupt;
> +    uart->reg_width = spcr->serial_port.bit_width/8;

width / 8;

> +    uart->reg_shift = 0;
> +    uart->io_size = UART_MAX_REG<<uart->reg_shift;

space before and after <<.

Also, io_size seems to be computed differently in pci_uart_config. I am 
not sure why the difference here?

> +
> +    irq_set_type(spcr->interrupt, spcr->interrupt_type);
> +
> +    uart->vuart.base_addr = uart->io_base;
> +    uart->vuart.size = uart->io_size;
> +    uart->vuart.data_off = UART_THR <<uart->reg_shift;

Ditto for the space.

> +    uart->vuart.status_off = UART_LSR<<uart->reg_shift;

Ditto.

> +    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;

Ditto.

Also, the code looks very similar to the DT version. Is there any way to 
share it?

> +
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> +
> +    return 0;
> +}
> +
> +ACPI_DEVICE_START(ns16550c, "16550 COMPAT UART", DEVICE_SERIAL)
> +        .class_type = ACPI_DBG2_16550_COMPATIBLE,
> +        .init = ns16550_acpi_uart_init,
> +ACPI_DEVICE_END
> +ACPI_DEVICE_START(ns16550s, "16550 SUBSET UART", DEVICE_SERIAL)
> +        .class_type = ACPI_DBG2_16550_SUBSET,
> +        .init = ns16550_acpi_uart_init,
> +ACPI_DEVICE_END
> +
> +#endif
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> index 5c3bac3..1b3e137 100644
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -35,6 +35,7 @@
>   #define UART_USR          0x1f    /* Status register (DW) */
>   #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
>   #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
> +#define UART_MAX_REG      (UART_USR+1)
>   
>   /* Interrupt Enable Register */
>   #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform
  2017-11-02 10:13 ` [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform Bhupinder Thakur
@ 2017-11-02 12:20   ` Julien Grall
  0 siblings, 0 replies; 5+ messages in thread
From: Julien Grall @ 2017-11-02 12:20 UTC (permalink / raw)
  To: Bhupinder Thakur, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich

Hi Bhupinder,

On 02/11/17 10:13, Bhupinder Thakur wrote:
>      The console was not working on HP Moonshot (HPE Proliant Aarch64) because
>      the UART registers were accessed as 8-bit aligned addresses. However,
>      registers are 32-bit aligned for HP Moonshot.
> 
>      Since ACPI/SPCR table does not specify the register shift to be applied to the
>      register offset, this patch implements an erratum to correctly set the register
>      shift for HP Moonshot.
> 
>      Similar erratum was implemented in linux:

Can you remove the tab at the beginning of each of the lines above?

> 
>      commit 79a648328d2a604524a30523ca763fbeca0f70e3
>      Author: Loc Ho <lho@apm.com>
>      Date:   Mon Jul 3 14:33:09 2017 -0700
> 
>          ACPI: SPCR: Workaround for APM X-Gene 8250 UART 32-alignment errata
> 
>          APM X-Gene verion 1 and 2 have an 8250 UART with its register
>          aligned to 32-bit. In addition, the latest released BIOS
>          encodes the access field as 8-bit access instead 32-bit access.
>          This causes no console with ACPI boot as the console
>          will not match X-Gene UART port due to the lack of mmio32
>          option.
> 
>          Signed-off-by: Loc Ho <lho@apm.com>
>          Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>          Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> 
>   xen/drivers/char/ns16550.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index b3f6d85..e716aba 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1542,6 +1542,33 @@ DT_DEVICE_END
>   #ifdef CONFIG_ACPI
>   #include <xen/acpi.h>
>   
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +    bool xgene_8250 = false;
> +
> +    if ( tb->interface_type != ACPI_DBG2_16550_COMPATIBLE )
> +        return false;
> +
> +    if ( memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +         memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE) )
> +        return false;
> +
> +    if ( !memcmp(tb->header.oem_table_id, "XGENESPC",
> +         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0 )
Please align ACPI_OEM_TABLE_ID_SIZE with ( after memcmp.

> +        xgene_8250 = true;
> +
> +    if ( !memcmp(tb->header.oem_table_id, "ProLiant",
> +         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1 )

Ditto.

> +        xgene_8250 = true;
> +
> +    return xgene_8250;
> +}
> +
>   static int __init ns16550_acpi_uart_init(const void *data)
>   {
>       struct ns16550 *uart;
> @@ -1568,9 +1595,20 @@ static int __init ns16550_acpi_uart_init(const void *data)
>       uart->io_base = spcr->serial_port.address;
>       uart->irq = spcr->interrupt;
>       uart->reg_width = spcr->serial_port.bit_width/8;
> -    uart->reg_shift = 0;
> -    uart->io_size = UART_MAX_REG<<uart->reg_shift;
>   
> +    if ( xgene_8250_erratum_present(spcr) )
> +    {
> +        /*
> +         * for xgene v1 and v2 the registers are 32-bit and so a
> +         * register shift of 2 has to be applied to get the
> +         * correct register offset.
> +         */
> +        uart->reg_shift = 2;
> +    }
> +    else
> +        uart->reg_shift = 0;
> +
> +    uart->io_size = UART_MAX_REG<<uart->reg_shift;

I am not why this change here. It might because of diff is confused?

>       irq_set_type(spcr->interrupt, spcr->interrupt_type);
>   
>       uart->vuart.base_addr = uart->io_base;
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI
  2017-11-02 12:15 ` [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Julien Grall
@ 2017-11-08  6:16   ` Bhupinder Thakur
  0 siblings, 0 replies; 5+ messages in thread
From: Bhupinder Thakur @ 2017-11-08  6:16 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, Xen-devel

Hi Julien,

On 2 November 2017 at 17:45, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Bhupinder,
>
> Please write a cover letter even if it is small when your send a series with
> multiple patches.
>
>
> On 02/11/17 10:13, Bhupinder Thakur wrote:
>>
>> Currently, Xen supports only DT based initialization of 16550 UART.
>> This patch adds support for initializing 16550 UART using ACPI SPCR table.
>>
>> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
>> ---
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: George Dunlap <George.Dunlap@eu.citrix.com>
>> CC: Ian Jackson <ian.jackson@eu.citrix.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Julien Grall <julien.grall@arm.com>
>>
>>   xen/drivers/char/ns16550.c  | 57
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   xen/include/xen/8250-uart.h |  1 +
>>   2 files changed, 58 insertions(+)
>>
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index e0f8199..b3f6d85 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -1538,6 +1538,63 @@ DT_DEVICE_START(ns16550, "NS16550 UART",
>> DEVICE_SERIAL)
>>   DT_DEVICE_END
>>     #endif /* HAS_DEVICE_TREE */
>> +
>> +#ifdef CONFIG_ACPI
>
>
> The code below is going to break x86 build. You need to do #if
> defined(CONFIG_ACPI) && defined(CONFIG_ARM)
>
>
>> +#include <xen/acpi.h>
>> +
>> +static int __init ns16550_acpi_uart_init(const void *data)
>> +{
>> +    struct ns16550 *uart;
>> +    acpi_status status;
>> +    struct acpi_table_spcr *spcr = NULL;
>> +
>> +    status = acpi_get_table(ACPI_SIG_SPCR, 0,
>> +                            (struct acpi_table_header **)&spcr);
>> +
>> +    if ( ACPI_FAILURE(status) )
>> +    {
>> +        printk("ns16550: Failed to get SPCR table\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    uart = &ns16550_com[0];
>> +
>> +    ns16550_init_common(uart);
>> +
>> +    uart->baud      = BAUD_AUTO;
>> +    uart->data_bits = 8;
>> +    uart->parity    = spcr->parity;
>> +    uart->stop_bits = spcr->stop_bits;
>> +    uart->io_base = spcr->serial_port.address;
>> +    uart->irq = spcr->interrupt;
>> +    uart->reg_width = spcr->serial_port.bit_width/8;
>
>
> width / 8;
>
>> +    uart->reg_shift = 0;
>> +    uart->io_size = UART_MAX_REG<<uart->reg_shift;
>
>
> space before and after <<.
>
> Also, io_size seems to be computed differently in pci_uart_config. I am not
> sure why the difference here?

In pci_uart_config:

uart->io_size = max(8U << param->reg_shift,
                                 param->uart_offset);

I was not sure which param to consider to get the uart_offset. Since
the max register that ns16550 uses is UART_USR, I
calculated the io_size based on that.
>
>> +
>> +    irq_set_type(spcr->interrupt, spcr->interrupt_type);
>> +
>> +    uart->vuart.base_addr = uart->io_base;
>> +    uart->vuart.size = uart->io_size;
>> +    uart->vuart.data_off = UART_THR <<uart->reg_shift;
>
>
> Ditto for the space.
>
>> +    uart->vuart.status_off = UART_LSR<<uart->reg_shift;
>
>
> Ditto.
>
>> +    uart->vuart.status = UART_LSR_THRE|UART_LSR_TEMT;
>
>
> Ditto.
>
> Also, the code looks very similar to the DT version. Is there any way to
> share it?
>
>
>> +
>> +    /* Register with generic serial driver. */
>> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
>> +
>> +    return 0;
>> +}
>> +
>> +ACPI_DEVICE_START(ns16550c, "16550 COMPAT UART", DEVICE_SERIAL)
>> +        .class_type = ACPI_DBG2_16550_COMPATIBLE,
>> +        .init = ns16550_acpi_uart_init,
>> +ACPI_DEVICE_END
>> +ACPI_DEVICE_START(ns16550s, "16550 SUBSET UART", DEVICE_SERIAL)
>> +        .class_type = ACPI_DBG2_16550_SUBSET,
>> +        .init = ns16550_acpi_uart_init,
>> +ACPI_DEVICE_END
>> +
>> +#endif
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
>> index 5c3bac3..1b3e137 100644
>> --- a/xen/include/xen/8250-uart.h
>> +++ b/xen/include/xen/8250-uart.h
>> @@ -35,6 +35,7 @@
>>   #define UART_USR          0x1f    /* Status register (DW) */
>>   #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
>>   #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
>> +#define UART_MAX_REG      (UART_USR+1)
>>     /* Interrupt Enable Register */
>>   #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
>>
>
> Cheers,
>
> --
> Julien Grall

Regards,
Bhupinder

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-08  6:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 10:13 [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Bhupinder Thakur
2017-11-02 10:13 ` [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform Bhupinder Thakur
2017-11-02 12:20   ` Julien Grall
2017-11-02 12:15 ` [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI Julien Grall
2017-11-08  6:16   ` Bhupinder Thakur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.