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

Re: [Xen-devel] [PATCH] ns16550: Add ACPI support



Hi Jan, Julien,

On 2020/1/20 16:38, Jan Beulich wrote:
On 18.01.2020 13:32, Julien Grall wrote:
On 17/01/2020 08:33, Jan Beulich wrote:
On 17.01.2020 04:40, Wei Xu wrote:
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1620,6 +1620,61 @@ 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 acpi_table_spcr *spcr = NULL;
+    acpi_status status;
+    struct ns16550 *uart;
+
+    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];
You want to justify the choice of what (on x86 at least= we'd call
com1 in the patch description. Also this could be the initializer
of the variable.
This is the same choice as we made for the DT binding (see
ns16550_uart()). We only support one UART on Arm which happen to be
ns16550_com[0] (but named diferrently).

The code below is actually quite similar to the DT parsing, so maybe we
want to provide a common helper here.
That's all fine, but doesn't eliminate the need to say so in the
description.

+    /*  Register with generic serial driver. */
+    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
+
+    return 0;
+}
+
+ACPI_DEVICE_START(ans16550, "NS16550 UART", DEVICE_SERIAL)
+    .class_type = ACPI_DBG2_16550_COMPATIBLE,
+    .init = ns16550_acpi_uart_init,
+ACPI_DEVICE_END
I don't expect this to build on x86.
This is only meant to target Arm. So maybe we want to protect the whole
code with defined(CONFIG_ACPI) && defined(CONFIG_ARM).
Indeed, that's what the remark was aiming at.

Jan

.


Thanks!
I will address the comments in V2.

Best Regards,
Wei



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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