[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform



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@xxxxxxx>
     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@xxxxxxx>
         Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
         Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
---
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

  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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.