[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] ns16550: Add ACPI support for ARM only
On 03.02.2020 12:21, Wei Xu wrote: > Parse the ACPI SPCR table and initialize the 16550 compatible serial port > for ARM only. Currently we only support one UART on ARM. Some fields > which we do not care yet on ARM are ignored. > > Signed-off-by: Wei Xu <xuwei5@xxxxxxxxxxxxx> > > --- > Changes in v3: > - address the code style comments from Jan > - use container_of to do cast > - list all fields we ignored > - check the console redirection is disabled or not before init the uart > - init the uart io_size and width via spcr->serial_port > > Changes in v2: > - improve commit message > - remove the spcr initialization > - add comments for the uart initialization and configuration > - adjust the code style issue > - limit the code only built on ACPI and ARM > --- > xen/drivers/char/ns16550.c | 75 > ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c > index aa87c57..741b510 100644 > --- a/xen/drivers/char/ns16550.c > +++ b/xen/drivers/char/ns16550.c > @@ -1620,6 +1620,81 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL) > DT_DEVICE_END > > #endif /* HAS_DEVICE_TREE */ > + > +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM) > +#include <xen/acpi.h> > + > +static int __init ns16550_acpi_uart_init(const void *data) > +{ > + struct acpi_table_header *table; > + struct acpi_table_spcr *spcr; > + acpi_status status; > + /* > + * Same as the DT part. > + * Only support one UART on ARM which happen to be ns16550_com[0]. > + */ > + struct ns16550 *uart = &ns16550_com[0]; > + > + status = acpi_get_table(ACPI_SIG_SPCR, 0, &table); > + if ( ACPI_FAILURE(status) ) > + { > + printk("ns16550: Failed to get SPCR table\n"); > + return -EINVAL; > + } > + > + spcr = container_of(table, struct acpi_table_spcr, header); > + > + /* > + * The serial port address may be 0 for example > + * if the console redirection is disabled. > + */ > + if ( unlikely(!spcr->serial_port.address) ) > + { > + printk("ns16550: the serial port address is invalid\n"); Is zero really an invalid address, or is it rather a proper indicator of there not being any device? > + return -EINVAL; > + } > + > + ns16550_init_common(uart); > + > + /* > + * The baud rate is pre-configured by the firmware. But this isn't the same as BAUD_AUTO, is it? If firmware pre-configures the baud rate, isn't it this structure which it would use to communicate the information? > + * And currently the ACPI part is only targeting ARM so the following > + * fields pc_interrupt, pci_device_id, pci_vendor_id, pci_bus, > pci_device, > + * pci_function, pci_flags, pci_segment and flow_control which we do not > + * care yet are ignored. How come flow control is of no interest? I'd also group all the pci_* fields into a simple "and all PCI related ones". > + */ > + 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->io_size = spcr->serial_port.bit_width; Once again: You should not ignore the GAS address space indicator. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |