Re: [PATCH 2/3 v3] xen: Add support for initializing 16550 UART using ACPI

[Julien Grall <julien.grall@linaro.org>]

Hi Bhupinder,

On 24/11/17 11:39, 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>
> ---
> Changes since v2:
> - renamed UART_MAX_REG to UART_NUM_REGS
> - aligned some assignment statements
> - some coding style changes
> 
> 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  | 67 +++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/xen/8250-uart.h |  1 +
>   2 files changed, 68 insertions(+)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index c5dfc1e..af4712f 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -29,6 +29,10 @@
>   #ifdef CONFIG_X86
>   #include <asm/fixmap.h>
>   #endif
> +#ifdef CONFIG_ACPI
> +#include <xen/acpi.h>
> +#endif
> +
>   
>   /*
>    * Configure serial port with a string:
> @@ -1565,6 +1569,69 @@ DT_DEVICE_START(ns16550, "NS16550 UART", DEVICE_SERIAL)
>   DT_DEVICE_END
>   
>   #endif /* HAS_DEVICE_TREE */
> +
> +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM)
> +
> +static int ns16550_init_acpi(struct ns16550 **puart)

This should be __init. But why do you need to create 2 separate 
functions? I don't think this bring any enhancement to the code.

> +{
> +    struct acpi_table_spcr *spcr;
> +    int status;
> +    struct ns16550 *uart = &ns16550_com[0];
> +
> +    ns16550_init_common(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->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_NUM_REGS << uart->reg_shift;
> +
> +    irq_set_type(spcr->interrupt, spcr->interrupt_type);
> +
> +    *puart = uart;
> +
> +    return 0;
> +}
> +
> +static int __init ns16550_acpi_uart_init(const void *data)
> +{
> +    int ret;
> +    struct ns16550 *uart;
> +
> +    ret = ns16550_init_acpi(&uart);
> +    if ( ret )
> +        return ret;
> +
> +    ns16550_vuart_init(uart);
> +
> +    ns16550_register_uart(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..849a5c0 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_NUM_REGS     (UART_USR + 1)
>   
>   /* Interrupt Enable Register */
>   #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel