<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 3, 2020 at 9:46 AM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 3, 2020 at 8:25 AM Kinsey Moore <<a href="mailto:kinsey.moore@oarcorp.com" target="_blank">kinsey.moore@oarcorp.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The zynq-uart set_attributes implementation was configured to always<br>
return false which causes spconsole01 to fail. This restores the<br>
disabled implementation which sets the baud rate registers<br>
appropriately and allows spconsole01 to pass. This also expands the<br>
set_attributes functionality to allow setting of the stop bits,<br>
character width, and parity.<br>
---<br>
 bsps/include/dev/serial/zynq-uart.h       |  7 +++<br>
 bsps/shared/dev/serial/zynq-uart-polled.c |  2 +-<br>
 bsps/shared/dev/serial/zynq-uart.c        | 64 +++++++++++++++++++++--<br>
 3 files changed, 67 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/bsps/include/dev/serial/zynq-uart.h b/bsps/include/dev/serial/zynq-uart.h<br>
index 2c0f250a3a..0eb1dd5f29 100644<br>
--- a/bsps/include/dev/serial/zynq-uart.h<br>
+++ b/bsps/include/dev/serial/zynq-uart.h<br>
@@ -78,6 +78,13 @@ void zynq_uart_write_polled(<br>
   */<br>
 void zynq_uart_reset_tx_flush(zynq_uart_context *ctx);<br>
<br>
+int zynq_cal_baud_rate(<br>
+  uint32_t  baudrate,<br>
+  uint32_t* brgr,<br>
+  uint32_t* bauddiv,<br>
+  uint32_t  modereg<br>
+);<br>
+<br>
 #ifdef __cplusplus<br>
 }<br>
 #endif /* __cplusplus */<br>
diff --git a/bsps/shared/dev/serial/zynq-uart-polled.c b/bsps/shared/dev/serial/zynq-uart-polled.c<br>
index a1b51ea521..442431d502 100644<br>
--- a/bsps/shared/dev/serial/zynq-uart-polled.c<br>
+++ b/bsps/shared/dev/serial/zynq-uart-polled.c<br>
@@ -40,7 +40,7 @@ uint32_t zynq_uart_input_clock(void)<br>
   return ZYNQ_CLOCK_UART;<br>
 }<br>
<br>
-static int zynq_cal_baud_rate(uint32_t  baudrate,<br>
+int zynq_cal_baud_rate(uint32_t  baudrate,<br>
                               uint32_t* brgr,<br>
                               uint32_t* bauddiv,<br>
                               uint32_t  modereg)<br>
diff --git a/bsps/shared/dev/serial/zynq-uart.c b/bsps/shared/dev/serial/zynq-uart.c<br>
index 41adb196ab..39e2e65924 100644<br>
--- a/bsps/shared/dev/serial/zynq-uart.c<br>
+++ b/bsps/shared/dev/serial/zynq-uart.c<br>
@@ -142,25 +142,79 @@ static bool zynq_uart_set_attributes(<br>
   const struct termios *term<br>
 )<br>
 {<br>
-#if 0<br>
-  volatile zynq_uart *regs = zynq_uart_get_regs(minor);<br>
+  zynq_uart_context *ctx = (zynq_uart_context *) context;<br>
+  volatile zynq_uart *regs = ctx->regs;<br>
   uint32_t brgr = 0;<br>
   uint32_t bauddiv = 0;<br>
+  uint32_t mode = 0;<br>
   int rc;<br>
<br>
   rc = zynq_cal_baud_rate(115200, &brgr, &bauddiv, regs->mode);<br>
   if (rc != 0)<br>
     return rc;<br>
<br>
+  /*<br>
+   * Configure the mode register<br>
+   */<br>
+  mode |= ZYNQ_UART_MODE_CHMODE(ZYNQ_UART_MODE_CHMODE_NORMAL);<br>
+<br>
+  /*<br>
+   * Parity<br>
+   */<br>
+<br></blockquote><div>I'd avoid extra vertical space between comments and relevant code. The Parity comment applies to the following block of code, so it should be kept "closer" to it for readability. (This may not be an official style rule, but seems good practice.) </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+  mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_NONE);<br>
+  if (term->c_cflag & PARENB) {<br>
+    if (!(term->c_cflag & PARODD)) {<br>
+      mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_ODD);<br>
+    } else {<br>
+      mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_EVEN);<br>
+    }<br>
+  }<br>
+<br>
+  /*<br>
+   * Character Size<br>
+   */<br>
+<br>
+  if (term->c_cflag & CSIZE) {<br>
+    switch (term->c_cflag & CSIZE)<br></blockquote><div>Duplicating the conditional isn't needed. Let the switch handle the default case also, like:</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    {<br>
+    case CS6:<br>
+      mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_6);<br>
+      break;<br>
+    case CS7:<br>
+      mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_7);<br>
+      break;<br>
+    case CS8:<br></blockquote><div>          case 0: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+      mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8);<br>
+      break;</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    default:<br>
+      return false;<br></blockquote></div></div></blockquote><div>One more thing, I think this default case is dead code and can be removed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    }<br>
+  } else {<br>
+    /* default to 9600,8,N,1 */<br>
+    mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8);<br>
+  }<br>
+<br>
+  /*<br>
+   * Stop Bits<br>
+   */<br>
+<br>
+  if (term->c_cflag & CSTOPB) {<br>
+    /* 2 stop bits */<br>
+    mode |= ZYNQ_UART_MODE_NBSTOP(ZYNQ_UART_MODE_NBSTOP_STOP_2);<br>
+  } else {<br>
+    /* 1 stop bit */<br>
+    mode |= ZYNQ_UART_MODE_NBSTOP(ZYNQ_UART_MODE_NBSTOP_STOP_1);<br>
+  }<br>
+<br>
+<br></blockquote><div>avoid 2+ blank lines in a row</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
   regs->control &= ~(ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN);<br>
+  regs->mode = mode;<br>
   regs->baud_rate_gen = ZYNQ_UART_BAUD_RATE_GEN_CD(brgr);<br>
   regs->baud_rate_div = ZYNQ_UART_BAUD_RATE_DIV_BDIV(bauddiv);<br>
   regs->control |= ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN;<br>
<br>
   return true;<br>
-#else<br>
-  return false;<br>
-#endif<br>
 }<br>
<br>
 const rtems_termios_device_handler zynq_uart_handler = {<br>
-- <br>
2.20.1<br>
<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div>
</blockquote></div></div>