Skip to content

Commit

Permalink
libplatsupport: fix CR/LF handling
Browse files Browse the repository at this point in the history 
Fix the bug that LF CR is printed instead of CR LF.

Signed-off-by: Axel Heider <[email protected]>
  • Loading branch information
Axel Heider committed Jan 11, 2024
1 parent 83afb88 commit 970c79d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
8 changes: 5 additions & 3 deletions libplatsupport/src/plat/fvp/serial.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ int uart_getchar(ps_chardevice_t *d)

int uart_putchar(ps_chardevice_t *d, int c)
{
while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0);

*REG_PTR(d->vaddr, UARTDR) = c;
if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) {
uart_putchar(d, '\r');
}

while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0) {
/* busy loop */
}
*REG_PTR(d->vaddr, UARTDR) = c;

return c;
}

Expand Down
8 changes: 5 additions & 3 deletions libplatsupport/src/plat/hikey/serial.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ int uart_getchar(ps_chardevice_t *d)

int uart_putchar(ps_chardevice_t *d, int c)
{
while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0);

*REG_PTR(d->vaddr, UARTDR) = c;
if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) {
uart_putchar(d, '\r');
}

while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0) {
/* busy loop */
}
*REG_PTR(d->vaddr, UARTDR) = c;

return c;
}

Expand Down
8 changes: 5 additions & 3 deletions libplatsupport/src/plat/qemu-arm-virt/serial.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ int uart_getchar(ps_chardevice_t *d)

int uart_putchar(ps_chardevice_t *d, int c)
{
while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0);

This comment has been minimized.

Sign in to view

Copy link
@axel-h

axel-h Jan 12, 2024

Member

I don't understand where the race will happen. We call uart_putchar() (recursively) with \r first if we see a \n. In this call it will do a blocking wait, then send \r. Then we are back here again, do another blocking wait and send the \n.

This comment has been minimized.

Sign in to view

Copy link
@heshamelmatary

heshamelmatary Jan 12, 2024

Contributor

Yes, you beat me before I deleted the comment not to add noise :) I guess leaving it at the top will also be fine?

This comment has been minimized.

Sign in to view

Copy link
@axel-h

axel-h Jan 12, 2024 •
edited
Loading

Member

I don't think so. If we return from outputting \r, the FIFO could be full again. We just check if the FIFO has space - which I think means on char or more? I'm not sure how the hardware can be configured concerning the PL011_UARTFR_TXFF bit, but it does not guarantee the FIFO is completely empty, So to be safe, we have to run another check that there is space in the FIFO, before we can output \n. Usually, UART is horribly slow compared to the core speed, so when printing a lot, I'd expect to run into a full FIFO quickly and then looping ages for just one char to get out.

This comment has been minimized.

Sign in to view

Copy link
@heshamelmatary

heshamelmatary Jan 12, 2024

Contributor

Yes, you're right, I followed this commit.

*REG_PTR(d->vaddr, UARTDR) = c;
if (c == '\n' && (d->flags & SERIAL_AUTO_CR)) {
uart_putchar(d, '\r');
}

while ((*REG_PTR(d->vaddr, UARTFR) & PL011_UARTFR_TXFF) != 0)
/* busy loop */
}
*REG_PTR(d->vaddr, UARTDR) = c;

return c;
}

Expand Down

0 comments on commit 970c79d

Please sign in to comment.