dt-uart: cleanups, bugfixes and /chosen/stdout-path support

Signed-off-by Ian Campbell <[email protected]>

---
 xen/drivers/char/dt-uart.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

xen/drivers/char/dt-uart.c +10 -0

index fa92b5c..45a87a6 100644
@@ -70,3 +70,13 @@ void __init dt_uart_init(void)
     if ( ret )
         printk("Unable to initialize serial: %d\n", ret);
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

- Don't log at all if console=dtuart (the default) was not present, in that case the user has asked for something else, no need for every other driver to tell them this.
- Use "dtuart" in all other messages, rather than just "console" or "uart".
- Be more explicit if we are exiting because dtuart= wasn't given.
- Log the options which we've parsed.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/drivers/char/dt-uart.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

xen/drivers/char/dt-uart.c +7 -4

index 45a87a6..04dbb97 100644
@@ -41,9 +41,12 @@ void __init dt_uart_init(void)
     const char *devpath = opt_dtuart;
     char *options;
 
-    if ( !console_has("dtuart") || !strcmp(opt_dtuart, "") )
+    if ( !console_has("dtuart") )
+        return; /* Not for us */
+
+    if ( !strcmp(opt_dtuart, "") )
     {
-        printk("No console\n");
+        printk("No dtuart path configured\n");
         return;
     }
 

@@ -53,7 +56,7 @@ void __init dt_uart_init(void)
     else
         options = "";
 
-    printk("Looking for UART console %s\n", devpath);
+    printk("Looking for dtuart at \"%s\", options \"%s\"\n", devpath, options);
     if ( *devpath == '/' )
         dev = dt_find_node_by_path(devpath);
     else

@@ -68,7 +71,7 @@ void __init dt_uart_init(void)
     ret = device_init(dev, DEVICE_SERIAL, options);
 
     if ( ret )
-        printk("Unable to initialize serial: %d\n", ret);
+        printk("Unable to initialize dtuart: %d\n", ret);
 }
 
 /*

I've retained the handling of ',' for compatibility, but I'm almost inclined to just drop it, if not now then in a release or two.

Quoted 2 lines · Ian Campbell

I've retained the handling of ',' for compatibility, but I'm almost inclined to just drop it, if not now then in a release or two.

None of the current drivers support options. So I think we could drop the support of ",".

Backport: I don't think #2 is required to backport this patch.

ePAPR v1.1 section 3.5 defines the /chosen/stdout-path property to refer to the device to be used for boot console output, so if no dtuart property is given try to use that instead. This will make Xen find a suitable console by default on DT platforms which include this property.

As it happens the dtuart option has the exact same syntax as stdout-path, so we can just copy the value into that buffer if it is empty.

FWIW support for this was added to Linux in v3.19-rc1 (7914a7c5651a "of: support passing console options with stdout-path") and a fairly large number of the dts files shipped with Linux have already included a stdout-path property for quite a while now.

Since there is a base of existing device trees with the property, we do not support the legacy ',' options separator so we remain compatible.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/domain_build.c |    2 ++
 xen/drivers/char/dt-uart.c  |   29 +++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

xen/arch/arm/domain_build.c +2 -0

index de180d8..c33a73c 100644
@@ -424,6 +424,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
          *   bootargs (from module #1, above).
          * * remove bootargs,  xen,dom0-bootargs, xen,xen-bootargs,
          *   linux,initrd-start and linux,initrd-end.
+         * * remove stdout-path.
          * * remove bootargs, linux,uefi-system-table,
          *   linux,uefi-mmap-start, linux,uefi-mmap-size,
          *   linux,uefi-mmap-desc-size, and linux,uefi-mmap-desc-ver

@@ -434,6 +435,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
             if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
                  dt_property_name_is_equal(prop, "linux,initrd-start") ||
                  dt_property_name_is_equal(prop, "linux,initrd-end") ||
+                 dt_property_name_is_equal(prop, "stdout-path") ||
                  dt_property_name_is_equal(prop, "linux,uefi-system-table") ||
                  dt_property_name_is_equal(prop, "linux,uefi-mmap-start") ||
                  dt_property_name_is_equal(prop, "linux,uefi-mmap-size") ||

xen/drivers/char/dt-uart.c +2 -1

index 54e65fc..08b0d76 100644
@@ -22,6 +22,7 @@
 #include <xen/console.h>
 #include <xen/device_tree.h>
 #include <xen/serial.h>
+#include <xen/errno.h>
 
 /*
  * Configure UART port with a string:

@@ -38,7 +39,7 @@ void __init dt_uart_init(void)
 {
     struct dt_device_node *dev;
     int ret;
-    const char *devpath = opt_dtuart;
+    const char *devpath = opt_dtuart, *stdout = NULL;
     char *options;
 
     if ( !console_has("dtuart") )

xen/drivers/char/dt-uart.c void __init dt_uart_init(void) · line 58

Quoted 8 lines

xen/drivers/char/dt-uart.c +8 -0

+
+        if ( chosen )
+        {
+            ret = dt_property_read_string(chosen, "stdout-path", &stdout);
+            if ( ret >= 0 )
+            {
+                printk("Taking dtuart configuration from /chosen/stdout-path\n");
+                strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart));

The final string in opt_dtuart may be truncated if stdout is bigger than 255 characters.

I would add a check to avoid hours of debugging later.

Quoted 9 lines · Julien Grall

Quoted 8 lines

xen/drivers/char/dt-uart.c +8 -0

+
+        if ( chosen )
+        {
+            ret = dt_property_read_string(chosen, "stdout-path", &stdout);
+            if ( ret >= 0 )
+            {
+                printk("Taking dtuart configuration from /chosen/stdout-path\n");
+                strlcpy(opt_dtuart, stdout, sizeof(opt_dtuart));

Quoted 4 lines

The final string in opt_dtuart may be truncated if stdout is bigger than 255 characters.

I would add a check to avoid hours of debugging later.

Good point. I suppose it may as well warn and continue: hypothetically the truncation might only affect some non-critical options so the console might actually work, so we might as well try.