Hi Sebastian,
(I believe I forgot to include devel@rtems.org in my previous answer, sorry for
the duplicate email)
Sorry for the delayed answer, we were quite busy at work.
You will find attached the Git patch file for the SCI bug. I've taken into
account your suggestions.
While testing the counterpart of my protocol on a Zynq, I found a similar issue
where the UART is assumed to be used for printable text only. The second patch
fixes it.
I've used `git format-patch -2 HEAD` to generate the patch files. If something
is not quite right for you to integrate my changes, let me know.
Best regards,
Adrien Chardon, on behalf of Reflex Aerospace
________________________________
From: Sebastian Huber <sebastian.hu...@embedded-brains.de>
Sent: 25 January 2024 20:29
To: Adrien Chardon <adr...@reflexaerospace.com>; devel@rtems.org
<devel@rtems.org>
Subject: Re: [Patch] bsp/tms570/sci: fix bug in tms570_sci_read_received_chars()
[You don't often get email from sebastian.hu...@embedded-brains.de. Learn why
this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hello Adrien,
the change looks good. I would remove the
tms570_sci_read_received_chars() and TMS570_SCI_BUFFER_SIZE:
static void tms570_sci_interrupt_handler(void * arg)
{
rtems_termios_tty *tty = arg;
tms570_sci_context *ctx = rtems_termios_get_device_context(tty);
/*
* Check if we have received something.
*/
if ( (ctx->regs->FLR & TMS570_SCI_FLR_RXRDY ) == TMS570_SCI_FLR_RXRDY ) {
char buf[1];
buf[0] = ctx->regs->RD;
rtems_termios_enqueue_raw_characters(tty, buf, 1);
}
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
From bcf82a1217d9bdf3ca5aca0beb713fb8abf24ee0 Mon Sep 17 00:00:00 2001
From: Adrien Chardon <adr...@reflexaerospace.com>
Date: Tue, 30 Jan 2024 18:02:04 +0100
Subject: [PATCH 2/2] bsps/shared/zynq-uart-polled: fix bug in
zynq_uart_initialize()
Similar to the recent commit in tms570-sci.c, the assumption that a UART will
only see printable ASCII characters, instead of any value in the range
0x00-0xFF, is wrong.
A non forgiving binary protocol will be thrown off by this driver sending
"\r\r\r\r" when initializing.
If a user wants to flush the interface, they should explicitely use the
dedicated function `tcflush(fd, TCIOFLUSH);`.
---
bsps/shared/dev/serial/zynq-uart-polled.c | 6 ------
1 file changed, 6 deletions(-)
diff --git bsps/shared/dev/serial/zynq-uart-polled.c bsps/shared/dev/serial/zynq-uart-polled.c
index 6865fa8d6f..222758c0d8 100644
--- bsps/shared/dev/serial/zynq-uart-polled.c
+++ bsps/shared/dev/serial/zynq-uart-polled.c
@@ -144,12 +144,6 @@ void zynq_uart_initialize(rtems_termios_device_context *base)
| ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_NONE)
| ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8)
| mode_clks;
-
- while (zynq_uart_read_polled(base) >= 0) {
- /* Drop */
- }
-
- zynq_uart_reset_tx_flush(ctx);
}
int zynq_uart_read_polled(rtems_termios_device_context *base)
--
2.34.1
From 6130a1f06150d5194d6af34ff722567cd72f5de6 Mon Sep 17 00:00:00 2001
From: Adrien Chardon <adr...@reflexaerospace.com>
Date: Wed, 24 Jan 2024 18:01:52 +0100
Subject: [PATCH 1/2] bsp/tms570/sci: fix bug in
tms570_sci_read_received_chars()
`tms570_sci_interrupt_handler()` is called when an RX interrupt fires. It checks
in the register `FLR`, the `RXRDY` bit (Receiver ready flag - indicate that the
SCIRD contains new data). If it is set, it calls
`tms570_sci_read_received_chars()`.
`tms570_sci_read_received_chars()` checks the register `RD` against 0. If it is
non zero, it returns 1 to indicate that one byte was read.
In the old behavior, if it is zero, the function returns 0 to indicate that no
data was read.
The new behavior is to not silently drop 0x00 bytes. Ignoring 0x00 bytes is fine
when working with printable text (which, I assume, is how this driver was
tested), but as soon as the UART is used in non canonical (raw) mode, with
potentially 0x00 bytes, these bytes will be silently dropped, causing issues in
the data/protocol layer above.
---
bsps/arm/tms570/console/tms570-sci.c | 45 ++++++----------------------
1 file changed, 9 insertions(+), 36 deletions(-)
diff --git bsps/arm/tms570/console/tms570-sci.c bsps/arm/tms570/console/tms570-sci.c
index d5668034ef..0d1e10f108 100644
--- bsps/arm/tms570/console/tms570-sci.c
+++ bsps/arm/tms570/console/tms570-sci.c
@@ -33,8 +33,6 @@
#include <bsp/fatal.h>
#include <bsp/irq.h>
-#define TMS570_SCI_BUFFER_SIZE 1
-
// This symbol can overwriten in user space. Not that making it const prevented
// the weak/strong mechanism to work and the linker always preferred this symbol
// over the strong one in user space.
@@ -176,33 +174,6 @@ rtems_device_driver console_initialize(
return RTEMS_SUCCESSFUL;
}
-/**
- * @brief Reads chars from HW
- *
- * Reads chars from HW peripheral specified in driver context.
- * TMS570 does not have HW buffer for serial line so this function can
- * return only 0 or 1 char
- *
- * @param[in] ctx context of the driver
- * @param[out] buf read data buffer
- * @param[in] N size of buffer
- * @retval x Number of read chars from peripherals
- */
-static int tms570_sci_read_received_chars(
- tms570_sci_context * ctx,
- char * buf,
- int N)
-{
- if ( N < 1 ) {
- return 0;
- }
- if ( ctx->regs->RD != 0 ) {
- buf[0] = ctx->regs->RD;
- return 1;
- }
- return 0;
-}
-
/**
* @brief Enables RX interrupt
*
@@ -356,23 +327,25 @@ static void tms570_sci_interrupt_handler(void * arg)
{
rtems_termios_tty *tty = arg;
tms570_sci_context *ctx = rtems_termios_get_device_context(tty);
- char buf[TMS570_SCI_BUFFER_SIZE];
- size_t n;
/*
* Check if we have received something.
*/
if ( (ctx->regs->FLR & TMS570_SCI_FLR_RXRDY ) == TMS570_SCI_FLR_RXRDY ) {
- n = tms570_sci_read_received_chars(ctx, buf, TMS570_SCI_BUFFER_SIZE);
- if ( n > 0 ) {
- /* Hand the data over to the Termios infrastructure */
- rtems_termios_enqueue_raw_characters(tty, buf, n);
- }
+ char buf[1];
+
+ /* Read the received byte */
+ buf[0] = ctx->regs->RD & 0x000000FF;
+
+ /* Hand the data over to the Termios infrastructure */
+ rtems_termios_enqueue_raw_characters(tty, buf, 1);
}
/*
* Check if we have something transmitted.
*/
if ( (ctx->regs->FLR & TMS570_SCI_FLR_TXRDY ) == TMS570_SCI_FLR_TXRDY ) {
+ size_t n;
+
n = tms570_sci_transmitted_chars(ctx);
if ( n > 0 ) {
/*
--
2.34.1
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
