[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen: Add support for initializing 16550 UART using ACPI
Hi Julien, On 2 November 2017 at 17:45, Julien Grall <julien.grall@xxxxxxxxxx> 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@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 | 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@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |