| Message ID | 1511523552-23628-2-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | xen: ACPI/SPCR based initialization of 8250 UART | expand |
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@linaro.org> > --- > Changes since v2: > - Refactored the code to prepare for later patches. > > 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 | 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@oracle.com> > > 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@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Bhupinder, On 24/11/17 11:39, 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@linaro.org> > --- > Changes since v2: > - Refactored the code to prepare for later patches. > > 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 | 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) __init. > +{ > + 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) __init > +{ > + /* Register with generic serial driver. */ > + serial_register_uart(uart - ns16550_com, &ns16550_driver, uart); > +} I don't think this function should exist. Arm is using only one port, but this is not true for x86. It would be better to fold this into each implementation (e.g ACPI and DT). > + > #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, __init > + 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]; > > 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); Why do you need to create ns16550_init_dt? It is only used here, > + > + 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 = > Cheers,
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]; 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 =
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@linaro.org> --- Changes since v2: - Refactored the code to prepare for later patches. 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 | 53 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 13 deletions(-)