Hi Casey,
On 6/30/2026 1:27 PM, Casey Connolly wrote:
Hi Balaji,
On 6/29/26 13:31, Balaji Selvanathan wrote:
The GENI UART hardware can retain stale data in the RX FIFO.
The msm_serial_pending() function was reporting this stale data as
pending input, causing U-Boot to incorrectly assume a key press and
stop at the CLI prompt instead of continuing the boot process.
Add IRQ status validation to distinguish between legitimate received
data and stale FIFO contents. Check the Secondary IRQ status register
for RX watermark or last flags before reporting pending data. If the
FIFO contains data but no valid IRQ flags are set, ignore it as stale.
Taking a look here compared to the Linux polling mode driver I don't
think this is the right way to solve this.
In qcom_geni_serial_stop_rx_fifo() in Linux after clearing the RX IRQs
the cancel command is sent (and polled on) and then if
S_RX_FIFO_LAST_EN is set the FIFO is drained.
So the right fix here for U-Boot would be to do something similar in
msm_geni_serial_setup_rx(), send a cancel rather than an abort, wait
for it to complete (or timeout), drain the fifo, and then send an
abort if S_GENI_CMD_ACTIVE is still set to fully reset it.
so NAK, please solve this properly by fixing the init sequence.
Fixed the init sequence in v2:
https://lore.kernel.org/u-boot/[email protected]/
But even after fixing init sequence, still the issue is observed, so
seems SE_GENI_S_IRQ_STATUS register has to be checked to make sure data
is valid
(https://lore.kernel.org/u-boot/[email protected]/).
Thanks,
Balaji
Signed-off-by: Balaji Selvanathan <[email protected]>
---
drivers/serial/serial_msm_geni.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/serial/serial_msm_geni.c
b/drivers/serial/serial_msm_geni.c
index 3dca581f68f..6d30f150966 100644
--- a/drivers/serial/serial_msm_geni.c
+++ b/drivers/serial/serial_msm_geni.c
@@ -424,13 +424,30 @@ static int msm_serial_getc(struct udevice *dev)
static int msm_serial_pending(struct udevice *dev, bool input)
{
struct msm_serial_data *priv = dev_get_priv(dev);
-
- if (input)
- return readl(priv->base + SE_GENI_RX_FIFO_STATUS) &
- RX_FIFO_WC_MSK;
- else
+ u32 rx_fifo_status, s_irq_status;
+ int result;
+
+ if (input) {
+ rx_fifo_status = readl(priv->base + SE_GENI_RX_FIFO_STATUS);
+ result = rx_fifo_status & RX_FIFO_WC_MSK;
+
+ /*
+ * Validate RX data with IRQ status
+ * Only report pending data if valid RX IRQ indicators are set.
What does this mean "valid RX IRQ indicators"??? This comment is
fluff, use better variable names (result -> word_count)
+ */
+ if (result) {
if (word_count > 0)> + s_irq_status = readl(priv->base +
SE_GENI_S_IRQ_STATUS);
+
+ if (!(s_irq_status & (S_RX_FIFO_WATERMARK_EN |
S_RX_FIFO_LAST_EN))) {
+ /* No valid RX IRQ, ignore stale data */
+ return 0;
+ }
+ }
+ return result;
+ } else {
return readl(priv->base + SE_GENI_TX_FIFO_STATUS) &
TX_FIFO_WC_MSK;
+ }
return 0;
}
---
base-commit: 6902fb4c17faa375003124c451c2550deab5463d
change-id: 20260629-uart-e07d893e4508
Best regards,
