Re: [PATCH v2 2/7] spmi: apple: Validate FIFO state

[Janne Grunau <j@jannau.net>]

On Tue, Jul 28, 2026 at 11:28:08AM +0200, Sasha Finkelstein wrote:
> From: Alba Mendez <me@alba.sh>
> 
> Check for data before reading the body of a reply, and check for
> end of data afterwards.
> 
> Signed-off-by: Alba Mendez <me@alba.sh>
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---
>  drivers/spmi/spmi-apple-controller.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index 376cf682c43e..b4f442d0b4ad 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c
> @@ -78,6 +78,10 @@ static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  
>  	/* Read SPMI data reply */
>  	while (len_read < len) {
> +		if (readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY) {
> +			dev_err(&ctrl->dev, "FIFO lacks reply data, controller stuck?\n");

this looks like it maybe should use dev_err_ratelimited()

> +			return -EIO;
> +		}
>  		rsp = readl(spmi->regs + SPMI_RSP_REG);
>  		i = 0;
>  		while ((len_read < len) && (i < 4)) {
> @@ -86,6 +90,9 @@ static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  		}
>  	}
>  
> +	if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY))
> +		dev_warn(&ctrl->dev, "FIFO has extra data\n");

would it make sense to flush the fifo here to avoid issues on the next
command? I would assume if we don't flush this condition will trigger
for every following command.

> +
>  	return 0;
>  }
>  
> @@ -115,6 +122,9 @@ static int spmi_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  	/* Discard */
>  	readl(spmi->regs + SPMI_RSP_REG);
>  
> +	if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY))
> +		dev_warn(&ctrl->dev, "FIFO has extra data\n");

same here

> +
>  	return 0;
>  }

Janne