<div dir="ltr">A gentle reminder for this patch. Please have a look at this and let me know what changes are required. Once this is merged, I have a few more patches building upon this.<div><br></div><div>Regards,</div><div>Utkarsh</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 30, 2023 at 9:47 AM Utkarsh Verma <<a href="mailto:utkarsh@bitbanged.com">utkarsh@bitbanged.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">This patch series adds two drivers, PL011 and Mini UART. Both support<br>
interrupts and implement the termios API.<br>
<br>
Why add a new driver for the PL011 when we already have one?<br>
<br>
The existing driver is a very basic one and uses memory-mapped structs<br>
to access the registers. This proved to be problematic for the<br>
'aarch64/raspberrypi4b' BSP as the RPi 4B's MMU does not reserve the<br>
entirety of the space required by the PL011 register struct.<br>
<br>
Even the existing driver doesn't use all the struct members. So, in the<br>
new driver, macros were used instead. This has the benefit of minimalism<br>
and ensures that we only add tested features to the driver.<br>
<br>
This driver builds upon the PL011 driver present in the Xilinx Versal<br>
BSP and addresses the IRQ startup hack.<br>
<br>
In short, the new PL011 driver has the features provided by the<br>
existing driver, and it meshes well with the termios API.<br>
<br>
Lastly, there's one thing I need feedback on. The PL011 has a hardware<br>
limitation which requires me to invoke the IRQ handler manually, the<br>
first time. For this, I need access to the `tty` struct in the<br>
`write_buffer` function.<br>
<br>
<a href="https://github.com/UtkarshVerma/rtems/blob/uart-drivers/bsps/shared/dev/serial/pl011.c#L301" rel="noreferrer" target="_blank">https://github.com/UtkarshVerma/rtems/blob/uart-drivers/bsps/shared/dev/serial/pl011.c#L301</a><br>
<br>
For now, I store the tty in the device context and then pass the context<br>
to the IRQ handler. Is this a good approach? Are there better ways to do<br>
this?<br>
<br>
For convenience, feel free to check out my GitHub fork which has these<br>
changes:<br>
<br>
<a href="https://github.com/UtkarshVerma/rtems/tree/uart-drivers" rel="noreferrer" target="_blank">https://github.com/UtkarshVerma/rtems/tree/uart-drivers</a><br>
<br>
Utkarsh Verma (4):<br>
bsps/shared: Add new PL011 driver with IRQ support<br>
bsps/shared: Add new Mini UART driver<br>
spec: Add Mini UART and PL011 drivers to build spec<br>
bsps: Update BSPs to use the new PL011 driver<br>
<br>
bsps/aarch64/a53/console/console.c | 15 +-<br>
bsps/aarch64/a72/console/console.c | 15 +-<br>
bsps/aarch64/raspberrypi/console/console.c | 29 +-<br>
bsps/arm/raspberrypi/console/console-config.c | 27 +-<br>
.../realview-pbx-a9/console/console-polled.c | 5 +-<br>
.../arm/realview-pbx-a9/include/bsp/console.h | 4 +-<br>
bsps/arm/xen/console/console.c | 15 +-<br>
bsps/include/dev/serial/arm-pl011-regs.h | 143 ------<br>
.../dev/serial/{arm-pl011.h => mini-uart.h} | 52 +-<br>
bsps/include/dev/serial/pl011.h | 68 +++<br>
bsps/shared/dev/serial/arm-pl011.c | 104 ----<br>
bsps/shared/dev/serial/mini-uart.c | 316 ++++++++++++<br>
bsps/shared/dev/serial/pl011.c | 470 ++++++++++++++++++<br>
.../aarch64/raspberrypi/bspraspberrypi4.yml | 1 -<br>
spec/build/bsps/obj.yml | 7 +-<br>
15 files changed, 934 insertions(+), 337 deletions(-)<br>
delete mode 100644 bsps/include/dev/serial/arm-pl011-regs.h<br>
rename bsps/include/dev/serial/{arm-pl011.h => mini-uart.h} (64%)<br>
create mode 100644 bsps/include/dev/serial/pl011.h<br>
delete mode 100644 bsps/shared/dev/serial/arm-pl011.c<br>
create mode 100644 bsps/shared/dev/serial/mini-uart.c<br>
create mode 100644 bsps/shared/dev/serial/pl011.c<br>
<br>
-- <br>
2.41.0<br>
<br>
<br>
</blockquote></div>