Skip to content

drivers: serial: ns16550: Fix FIFO detection on Xilinx 16550 #88468

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.

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 1 commit into
base: main
Choose a base branch
from

Conversation

robhancocksed
Copy link
Contributor

The Xilinx AXI UART16550 FPGA IP core seems to have a quirk where the first read of the combined FCR/IIR register, after FCR is written when configuring the port, just reads back FCR (which is only supposed to happen when the DLAB bit in LCR is set) rather than reading the IIR register. This causes the code using the IIR register for detecting FIFO support to misbehave and not detect that a FIFO is present.

Add a dummy read of the IIR register before the real one to avoid this problem. This is expected to be harmless on other implementations.

The Xilinx AXI UART16550 FPGA IP core seems to have a quirk where the
first read of the combined FCR/IIR register, after FCR is written when
configuring the port, just reads back FCR (which is only supposed to
happen when the DLAB bit in LCR is set) rather than reading the IIR
register. This causes the code using the IIR register for detecting FIFO
support to misbehave and not detect that a FIFO is present.

Add a dummy read of the IIR register before the real one to avoid this
problem. This is expected to be harmless on other implementations.

Signed-off-by: Robert Hancock <[email protected]>
@github-actions github-actions bot added the area: UART Universal Asynchronous Receiver-Transmitter label Apr 10, 2025
@github-actions github-actions bot requested a review from dcpleung April 10, 2025 22:51
@dcpleung
Copy link
Member

Shouldn't this be fixed in the IP since it's soft IP on FPGA?

@robhancocksed
Copy link
Contributor Author

Shouldn't this be fixed in the IP since it's soft IP on FPGA?

Ideally, but this IP is probably burned into a lot of logic builds by now, and Xilinx seems reluctant to make changes to some of these long-standing IP cores, (presumably) especially when it can be worked around in software.

I'm not 100% sure why Linux (for example) doesn't hit this, but presumably the initialization sequence is slightly different such that it doesn't run into this problem.

@robhancocksed
Copy link
Contributor Author

Ping.. any feedback?

@dcpleung
Copy link
Member

Might be a good idea to wrap it with a kconfig. Or even better with devicetree where, if any UART instance is from Xilinx, do the extra read. For non-Xilinx UART, this is an unnecessary read so it is better to limit it to where it is actually needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UART Universal Asynchronous Receiver-Transmitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants