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 <[email protected]>
---
Changes since v2:
- Refactored the code to prepare for later patches.

CC: Andrew Cooper <[email protected]>
CC: George Dunlap <[email protected]>
CC: Ian Jackson <[email protected]>
CC: Jan Beulich <[email protected]>
CC: Konrad Rzeszutek Wilk <[email protected]>
CC: Stefano Stabellini <[email protected]>
CC: Tim Deegan <[email protected]>
CC: Wei Liu <[email protected]>
CC: Julien Grall <[email protected]>

  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,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to