Skip to content

[FIX] Fix bcm pl011 UART #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[FIX] Fix bcm pl011 UART #168

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09a20f9
libplatsupport: Fix bcm pl011 UART
FelixSchladt Aug 29, 2023
e456c24
libplatsupport: fix typo
FelixSchladt Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions libplatsupport/src/mach/bcm/pl011_uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pl011_regs_t;
/* IMSC register */
#define IMSC_RXIM BIT(4)
#define IMSC_TXIM BIT(5)
#define IMSC_RTIM BIT(6)
#define IMSC_OEIM BIT(10)

/* ICR register */
#define ICR_RXIC BIT(4)
Expand Down Expand Up @@ -96,16 +98,18 @@ static inline void pl011_uart_disable_fifo(ps_chardevice_t *dev)
r->lcrh &= ~LCRH_FEN;
}

static inline void pl011_uart_enable_rx_irq(ps_chardevice_t *dev)
static inline void pl011_uart_enable_irqs(ps_chardevice_t *dev)
{
pl011_regs_t *r = pl011_uart_get_priv(dev);
r->imsc |= IMSC_RXIM;
r->imsc |= IMSC_RXIM; // Receive interrupt mask
r->imsc |= IMSC_RTIM; // Receive timeout interrupt mask
r->imsc |= IMSC_OEIM; // Overrun error interrupt mask
}

static inline void pl011_uart_disable_rx_irq(ps_chardevice_t *dev)
static inline void pl011_uart_disable_irqs(ps_chardevice_t *dev)
{
pl011_regs_t *r = pl011_uart_get_priv(dev);
r->imsc &= ~IMSC_RXIM;
r->imsc = 0;
}

static inline void pl011_uart_wait_busy(ps_chardevice_t *dev)
Expand All @@ -128,7 +132,7 @@ static int pl011_uart_cr_configure(ps_chardevice_t *dev)
uint32_t val = r->cr;

val |= CR_TXE; // Transmit enable
val |= CR_RXE; // Teceive enable
val |= CR_RXE; // Receive enable

r->cr = val;

Expand Down Expand Up @@ -193,8 +197,7 @@ static int pl011_uart_configure(ps_chardevice_t *dev)
pl011_uart_disable(dev);

// Disable RX/all interrupts
//pl011_uart_disable_rx_irq(dev);
r->imsc = 0x7f1;
pl011_uart_disable_irqs(dev);

// Wait till UART is not busy anymore
pl011_uart_wait_busy(dev);
Expand Down Expand Up @@ -227,14 +230,14 @@ static int pl011_uart_configure(ps_chardevice_t *dev)
*
*/
// Enable FIFO
//pl011_uart_enable_fifo(dev);
pl011_uart_enable_fifo(dev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that the problem mentioned in the comment is properly addressed. I am afraid that enabling FIFOs is just a work-around for an unfixed bug, probably caused by the original's code assumptions of the current state of the UART or initialisation.

Have you tried fully configuring the UART and setting all registers, and clearing any pending IRQs and errors at init?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the current state again today and it is working fine.

Sending single bytes is not an issue.
For testing I did setup a simple uart echo server which is interrupt triggered.

2024-03-15_13-43

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please print something out at every receive interrupt and check that you don't get any when no input is given.

Or keep a counter and print out the counter value too.


// Enable interrupts
pl011_uart_enable_irqs(dev);

// Enable UART
pl011_uart_enable(dev);

// Enable RX interrupt
pl011_uart_enable_rx_irq(dev);

return 0;
}

Expand Down