| Message ID | 1509617589-22760-1-git-send-email-bhupinder.thakur@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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
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 */
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(+)