[PATCH] serial: msm_geni: validate RX data with IRQ status

[Casey Connolly]

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.

> 
> Signed-off-by: Balaji Selvanathan <balaji.selvanathan at oss.qualcomm.com>
> ---
>   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,