[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 24/25 v6] xen/arm: vpl011: Add a pl011 uart DT node in the guest device tree



Hi Bhupinder,

On 17/07/17 14:06, Bhupinder Thakur wrote:
The SBSA UART node format is as specified in
Documentation/devicetree/bindings/serial/arm_sbsa_uart.txt and given below:

ARM SBSA defined generic UART
------------------------------
This UART uses a subset of the PL011 registers and consequently lives
in the PL011 driver. It's baudrate and other communication parameters
cannot be adjusted at runtime, so it lacks a clock specifier here.

Required properties:
- compatible: must be "arm,sbsa-uart"
- reg: exactly one register range
- interrupts: exactly one interrupt specifier
- current-speed: the (fixed) baud rate set by the firmware

Currently the baud rate of 115200 has been selected as a default value,
which is one of the valid baud rate setttings. Higher baud rate was

s/setttings/settings/

selected since an emulated pl011 can support any valid baud rate without
any limitation of the hardware.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

 tools/libxl/libxl_arm.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index e3e5791..9eee50c 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -44,10 +44,22 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
     uint32_t nr_spis = 0;
     unsigned int i;

+    /*
+     * If pl011 vuart is enabled then increment the nr_spis to allow allocation
+     * of SPI VIRQ for pl011.
+     */
+    if (d_config->b_info.arch_arm.vuart)

vuart is an enum. Please follow what we did for the gic_version, i.e using a switch or at least checking the value of vuart.

+        nr_spis += (GUEST_VPL011_SPI - 32) + 1;
+
     for (i = 0; i < d_config->b_info.num_irqs; i++) {
         uint32_t irq = d_config->b_info.irqs[i];
         uint32_t spi;

+        if (d_config->b_info.arch_arm.vuart && (irq == GUEST_VPL011_SPI)) {
+            LOG(ERROR, "Physical IRQ %u conflicting with pl011 SPI\n", irq);
+            return ERROR_FAIL;
+        }

This limitation looks a bit random. Can we have a TODO in the code and the commit message to explain the reason of this limitation?

+
         if (irq < 32)
             continue;

@@ -130,9 +142,10 @@ static struct arch_info {
     const char *guest_type;
     const char *timer_compat;
     const char *cpu_compat;
+    const char *uart_compat;
 } arch_info[] = {
-    {"xen-3.0-armv7l",  "arm,armv7-timer", "arm,cortex-a15" },
-    {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8" },
+    {"xen-3.0-armv7l",  "arm,armv7-timer", "arm,cortex-a15", "arm,sbsa-uart" },
+    {"xen-3.0-aarch64", "arm,armv8-timer", "arm,armv8", "arm,sbsa-uart" },
 };

 /*
@@ -590,6 +603,38 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
     return 0;
 }

+static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
+                                 const struct arch_info *ainfo,
+                                 struct xc_dom_image *dom)
+{
+    int res;
+    gic_interrupt intr;
+
+    res = fdt_begin_node(fdt, "sbsa-pl011");
+    if (res) return res;
+
+    res = fdt_property_compat(gc, fdt, 1, ainfo->uart_compat);

NIT: uart_compat is exactly the same for AArch64 and AArch32. So you can directly use "arm,sbsa-uart" here.

+    if (res) return res;
+
+    res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                            1,
+                            GUEST_PL011_BASE, GUEST_PL011_SIZE);
+    if (res) return res;
+
+    set_interrupt(intr, GUEST_VPL011_SPI, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+
+    res = fdt_property_interrupts(gc, fdt, &intr, 1);
+    if (res) return res;
+
+    /* Use a default baud rate of 115200. */
+    fdt_property_u32(fdt, "current-speed", 115200);
+
+    res = fdt_end_node(fdt);
+    if (res) return res;
+
+    return 0;
+}
+
 static const struct arch_info *get_arch_info(libxl__gc *gc,
                                              const struct xc_dom_image *dom)
 {
@@ -889,6 +934,9 @@ next_resize:
         FDT( make_timer_node(gc, fdt, ainfo, xc_config->clock_frequency) );
         FDT( make_hypervisor_node(gc, fdt, vers) );

+        if (info->arch_arm.vuart)
+            FDT( make_vpl011_uart_node(gc, fdt, ainfo, dom) );
+
         if (pfdt)
             FDT( copy_partial_fdt(gc, fdt, pfdt) );



Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.