Re: [Xen-devel] [PATCH 1/3 v3] xen: Refactor 16550 UART code

[Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>]

On Fri, Nov 24, 2017 at 05:09:10PM +0530, Bhupinder Thakur wrote:
> This patch refactors the 8250 UART code so that code can be reused
> by later patches, which add support for ACPI based UART
> initialization.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
> Changes since v2:
> - Refactored the code to prepare for later patches.
> 
> 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 | 53 
> ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index e0f8199..c5dfc1e 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1462,16 +1462,32 @@ void __init ns16550_init(int index, struct 
> ns16550_defaults *defaults)
>      ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
>  }
>  
> +#ifdef CONFIG_ARM
> +static void ns16550_vuart_init(struct ns16550 *uart)
> +{
> +    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;
> +}
> +#endif
> +
> +static void ns16550_register_uart(struct ns16550 *uart)
> +{
> +    /* Register with generic serial driver. */
> +    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> +}
> +
>  #ifdef CONFIG_HAS_DEVICE_TREE
> -static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
> -                                       const void *data)
> +
> +static int ns16550_init_dt(struct ns16550 **puart,
> +                           const struct dt_device_node *dev)
>  {
> -    struct ns16550 *uart;
>      int res;
>      u32 reg_shift, reg_width;
>      u64 io_size;
> -
> -    uart = &ns16550_com[0];
> +    struct ns16550 *uart = &ns16550_com[0];

I am sure I understand this change. In the #2 patch you are doing
this the old way:

 +static int ns16550_init_acpi(struct ns16550 **puart)
+{
+    struct acpi_table_spcr *spcr;
+    int status;
+    struct ns16550 *uart = &ns16550_com[0];

Any particular resaon for this change?

If you can restore this to the original way in this function you
can put:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>  
>      ns16550_init_common(uart);
>  
> @@ -1510,18 +1526,29 @@ static int __init ns16550_uart_dt_init(struct 
> dt_device_node *dev,
>  
>      uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
>  
> -    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;
> +    *puart = uart;
>  
> -    /* Register with generic serial driver. */
> -    serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
> +    return 0;
> +}
> +
> +static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
> +                                       const void *data)
> +{
> +    struct ns16550 *uart;
> +    int ret;
> +
> +    ret = ns16550_init_dt(&uart, data);
> +
> +    if ( ret )
> +        return ret;
> +
> +    ns16550_vuart_init(uart);
> +
> +    ns16550_register_uart(uart);
>  
>      dt_device_set_used_by(dev, DOMID_XEN);
>  
> -    return 0;
> +    return ret;
>  }
>  
>  static const struct dt_device_match ns16550_dt_match[] __initconst =
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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