[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] ns16550: Add ACPI support
- To: Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>
- From: Wei Xu <xuwei5@xxxxxxxxxxxxx>
- Date: Tue, 21 Jan 2020 11:28:49 +0800
- Cc: sstabellini@xxxxxxxxxx, Wei Liu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, linuxarm@xxxxxxxxxx, shameerali.kolothum.thodi@xxxxxxxxxx, prime.zeng@xxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Tue, 21 Jan 2020 03:29:09 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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
|