Hi Julien,
On 2 November 2017 at 17:45, Julien Grall 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
>> ---
>> CC: Andrew Cooper
>> CC: George Dunlap
>> CC: Ian Jackson
>> CC: Jan Beulich
>> CC: Konrad Rzeszutek Wilk
>> CC: Stefano Stabellini
>> CC: Tim Deegan
>> CC: Wei Liu
>> CC: Julien Grall
>>
>> 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
>> +
>> +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
>
> 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
>
> Ditto for the space.
>
>> +uart->vuart.status_off = UART_LSR
>
> 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_ERDAI0x01/* rx data recv'd */
>>
>
> Cheers,
>
> --
> Julien Grall
Regards,
Bhupinder
___
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
