serial: Wrong baud rate applied on zynq7000

[ThreeFx]

trafficstars

I have a CAmkES project for which I want to use SerialServer in order to multiplex access to a serial console. The hardware in use is a Zynq7000.

When running the default serial_server applications (I've tried both polling and interrupt-based), the console output is garbled after the system boots:

## Starting application at 0x006f9000 ...

ELF-loader started on CPU: ARM Ltd. Cortex-A9 r3p0
  paddr=[6f9000..aa4ab3]
No DTB passed in from boot loader.
Looking for DTB in CPIO archive...found at 7fb1b4.
Loaded DTB from 7fb1b4.
   paddr=[3c000..3efff]
ELF-loading image 'kernel'
  paddr=[0..3bfff]
  vaddr=[e0000000..e003bfff]
  virt_entry=e0000000
ELF-loading image 'capdl-loader'
  paddr=[3f000..307fff]
  vaddr=[10000..2d8fff]
  virt_entry=18794
Enabling MMU and paging
Bootstrapping kernel
Booting all finished, dropped to user space
LJdd�䄄E��Ƈ�D�$Db

Typing leads to more garbage displayed on screen. This makes me suspect it's that an error occurs when setting the baud rate in libplatsupport/src/mach/zynq/serial.c. Indeed, applying this patch fixes the issue:

diff --git a/libplatsupport/src/mach/zynq/serial.c b/libplatsupport/src/mach/zynq/serial.c
index 15e47b0..7780b6d 100644
--- a/libplatsupport/src/mach/zynq/serial.c
+++ b/libplatsupport/src/mach/zynq/serial.c
@@ -321,31 +321,31 @@ static void zynq_uart_set_baud(
 
     zynq_uart_calc_baud_divs(UART_REF_CLK, bps, &div8, &cd, &bdiv);
 
-    /* Disable the Rx path */
-    regs->cr &= ~UART_CR_RXEN;
+    ///* Disable the Rx path */
+    //regs->cr &= ~UART_CR_RXEN;
 
-    /* Disable the Tx path */
-    regs->cr &= ~UART_CR_TXEN;
+    ///* Disable the Tx path */
+    //regs->cr &= ~UART_CR_TXEN;
 
-    /* Apply the calculated values */
-    if (div8) {
-        regs->mr |= UART_MR_CLKS;
-    } else {
-        regs->mr &= ~UART_MR_CLKS;
-    }
+    ///* Apply the calculated values */
+    //if (div8) {
+    //    regs->mr |= UART_MR_CLKS;
+    //} else {
+    //    regs->mr &= ~UART_MR_CLKS;
+    //}
 
-    regs->baudgen = cd;
-    regs->bauddiv = bdiv;
+    //regs->baudgen = cd;
+    //regs->bauddiv = bdiv;
 
-    /* Reset the Tx and Rx paths */
-    regs->cr |= UART_CR_TXRES | UART_CR_RXRES;
-    while (regs->cr & (UART_CR_TXRES | UART_CR_RXRES));
+    ///* Reset the Tx and Rx paths */
+    //regs->cr |= UART_CR_TXRES | UART_CR_RXRES;
+    //while (regs->cr & (UART_CR_TXRES | UART_CR_RXRES));
 
-    /* Enable the Rx path */
-    zynq_uart_enable_rx(regs);
+    ///* Enable the Rx path */
+    //zynq_uart_enable_rx(regs);
 
-    /* Enable the Tx path */
-    zynq_uart_enable_tx(regs);
+    ///* Enable the Tx path */
+    //zynq_uart_enable_tx(regs);
 }
 
 int serial_configure(

Recompiling the application makes it work as expected (u-boot initializes the console before seL4 runs):

## Starting application at 0x006f9000 ...

ELF-loader started on CPU: ARM Ltd. Cortex-A9 r3p0
  paddr=[6f9000..aa4ab3]
No DTB passed in from boot loader.
Looking for DTB in CPIO archive...found at 7fb1b4.
Loaded DTB from 7fb1b4.
   paddr=[3c000..3efff]
ELF-loading image 'kernel'
  paddr=[0..3bfff]
  vaddr=[e0000000..e003bfff]
  virt_entry=e0000000
ELF-loading image 'capdl-loader'
  paddr=[3f000..307fff]
  vaddr=[10000..2d8fff]
  virt_entry=18794
Enabling MMU and paging
Bootstrapping kernel
Booting all finished, dropped to user space
interruptcli: Hello Serial Server!
interruptcli: hello
interruptcli: test
interruptcli: this is a test

The baud rate calculated by zynq_uart_calc_baud_divs is 115207. I don't know a lot about UART consoles, but I think that 115207 is close enough that it should still work and display sensible output.

As far as I know we don't set any non-standard clock values anywhere.

[ThreeFx]

Hmm, it seems that the Zynq7000 has the same issues as the ZynqMP platforms (70c2666), so maybe we should also exclude the baud init on Zynq7000 platforms?

[axel-h]

Hmm, it seems that the Zynq7000 has the same issues as the ZynqMP platforms (70c2666), so maybe we should also exclude the baud init on Zynq7000 platforms?

Did this work for you? Then a PR would be appreciated. I don't have a board available to test this - just QEMU where I assume this does not matter as this detail is not emulated.