On Thu, Dec 3, 2020 at 9:46 AM Gedare Bloom <ged...@rtems.org> wrote:
>
>
> On Thu, Dec 3, 2020 at 8:25 AM Kinsey Moore <kinsey.mo...@oarcorp.com>
> wrote:
>
>> The zynq-uart set_attributes implementation was configured to always
>> return false which causes spconsole01 to fail. This restores the
>> disabled implementation which sets the baud rate registers
>> appropriately and allows spconsole01 to pass. This also expands the
>> set_attributes functionality to allow setting of the stop bits,
>> character width, and parity.
>> ---
>> bsps/include/dev/serial/zynq-uart.h | 7 +++
>> bsps/shared/dev/serial/zynq-uart-polled.c | 2 +-
>> bsps/shared/dev/serial/zynq-uart.c | 64 +++++++++++++++++++++--
>> 3 files changed, 67 insertions(+), 6 deletions(-)
>>
>> diff --git a/bsps/include/dev/serial/zynq-uart.h
>> b/bsps/include/dev/serial/zynq-uart.h
>> index 2c0f250a3a..0eb1dd5f29 100644
>> --- a/bsps/include/dev/serial/zynq-uart.h
>> +++ b/bsps/include/dev/serial/zynq-uart.h
>> @@ -78,6 +78,13 @@ void zynq_uart_write_polled(
>> */
>> void zynq_uart_reset_tx_flush(zynq_uart_context *ctx);
>>
>> +int zynq_cal_baud_rate(
>> + uint32_t baudrate,
>> + uint32_t* brgr,
>> + uint32_t* bauddiv,
>> + uint32_t modereg
>> +);
>> +
>> #ifdef __cplusplus
>> }
>> #endif /* __cplusplus */
>> diff --git a/bsps/shared/dev/serial/zynq-uart-polled.c
>> b/bsps/shared/dev/serial/zynq-uart-polled.c
>> index a1b51ea521..442431d502 100644
>> --- a/bsps/shared/dev/serial/zynq-uart-polled.c
>> +++ b/bsps/shared/dev/serial/zynq-uart-polled.c
>> @@ -40,7 +40,7 @@ uint32_t zynq_uart_input_clock(void)
>> return ZYNQ_CLOCK_UART;
>> }
>>
>> -static int zynq_cal_baud_rate(uint32_t baudrate,
>> +int zynq_cal_baud_rate(uint32_t baudrate,
>> uint32_t* brgr,
>> uint32_t* bauddiv,
>> uint32_t modereg)
>> diff --git a/bsps/shared/dev/serial/zynq-uart.c
>> b/bsps/shared/dev/serial/zynq-uart.c
>> index 41adb196ab..39e2e65924 100644
>> --- a/bsps/shared/dev/serial/zynq-uart.c
>> +++ b/bsps/shared/dev/serial/zynq-uart.c
>> @@ -142,25 +142,79 @@ static bool zynq_uart_set_attributes(
>> const struct termios *term
>> )
>> {
>> -#if 0
>> - volatile zynq_uart *regs = zynq_uart_get_regs(minor);
>> + zynq_uart_context *ctx = (zynq_uart_context *) context;
>> + volatile zynq_uart *regs = ctx->regs;
>> uint32_t brgr = 0;
>> uint32_t bauddiv = 0;
>> + uint32_t mode = 0;
>> int rc;
>>
>> rc = zynq_cal_baud_rate(115200, &brgr, &bauddiv, regs->mode);
>> if (rc != 0)
>> return rc;
>>
>> + /*
>> + * Configure the mode register
>> + */
>> + mode |= ZYNQ_UART_MODE_CHMODE(ZYNQ_UART_MODE_CHMODE_NORMAL);
>> +
>> + /*
>> + * Parity
>> + */
>> +
>>
> 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.)
>
>> + mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_NONE);
>> + if (term->c_cflag & PARENB) {
>> + if (!(term->c_cflag & PARODD)) {
>> + mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_ODD);
>> + } else {
>> + mode |= ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_EVEN);
>> + }
>> + }
>> +
>> + /*
>> + * Character Size
>> + */
>> +
>> + if (term->c_cflag & CSIZE) {
>> + switch (term->c_cflag & CSIZE)
>>
> Duplicating the conditional isn't needed. Let the switch handle the
> default case also, like:
>
>
>> + {
>> + case CS6:
>> + mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_6);
>> + break;
>> + case CS7:
>> + mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_7);
>> + break;
>> + case CS8:
>>
> case 0:
>
>> + mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8);
>> + break;
>
> + default:
>> + return false;
>>
> One more thing, I think this default case is dead code and can be removed.
> + }
>> + } else {
>> + /* default to 9600,8,N,1 */
>> + mode |= ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8);
>> + }
>> +
>> + /*
>> + * Stop Bits
>> + */
>> +
>> + if (term->c_cflag & CSTOPB) {
>> + /* 2 stop bits */
>> + mode |= ZYNQ_UART_MODE_NBSTOP(ZYNQ_UART_MODE_NBSTOP_STOP_2);
>> + } else {
>> + /* 1 stop bit */
>> + mode |= ZYNQ_UART_MODE_NBSTOP(ZYNQ_UART_MODE_NBSTOP_STOP_1);
>> + }
>> +
>> +
>>
> avoid 2+ blank lines in a row
>
>
>> regs->control &= ~(ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN);
>> + regs->mode = mode;
>> regs->baud_rate_gen = ZYNQ_UART_BAUD_RATE_GEN_CD(brgr);
>> regs->baud_rate_div = ZYNQ_UART_BAUD_RATE_DIV_BDIV(bauddiv);
>> regs->control |= ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN;
>>
>> return true;
>> -#else
>> - return false;
>> -#endif
>> }
>>
>> const rtems_termios_device_handler zynq_uart_handler = {
>> --
>> 2.20.1
>>
>> _______________________________________________
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>>
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
