Re: [PATCH] spmi: mtk-pmif: Add workaround for FSM lockup/error in read operation

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 7/1/26 14:19, AngeloGioacchino Del Regno wrote:
> The SPMI PMIF in some SoCs like MT8196 is affected by a hardware
> issue that makes the first read operation on each SID to fail as
> the FSM locks up after sending the read command, and this happens
> at least in the following conditions:
>   - At boot, after bootloader handoff; and
>   - At every suspend->resume cycle.
> 
> This critical bug may produce unwanted issues in regulator drivers
> which may fail to set voltages, making the platform to end up in a
> undervoltage or, a bit more critically, an overvoltage condition,
> producing instability or ... worse.
> 
> In order to work around this issue, add a retry mechanism into the
> pmif_spmi_read_cmd() callback which will resend the read command
> up to 3 times in the following conditions:
>   - Software Interface returns an error; or
>   - Software Interface never gets in WFVLDCLR state.
> 
> As to avoid uselessly blocking for too much time, only during the
> read-retry loop, the maximum SWINF polling time for each trial is
> reduced to PMIF_TIMEOUT_US / 2: this was tested on multiple SoCs
> and seems to always be enough, as when any read succeeds it will
> always take less than 40ms.
> In any case, this is still allowing 1.5 times the previous maximum
> polling time, reaching a total maximum (for 3 retries) of 150ms.
> 
> Worst case testing that I performed saw a third retry only 3 times
> out of ~100 reboots.
> 
> Fixes: 1f5be2d7f743 ("spmi: mtk-pmif: Add support for MT8196 SPMI Controller")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

This is a *fix*, and has been waiting for 2.5 months.

Can anyone please apply it?

Thanks,
Angelo

> ---
>   drivers/spmi/spmi-mtk-pmif.c | 38 +++++++++++++++++++++++++++---------
>   1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/spmi/spmi-mtk-pmif.c b/drivers/spmi/spmi-mtk-pmif.c
> index 1048420b5afb..61c916edaec4 100644
> --- a/drivers/spmi/spmi-mtk-pmif.c
> +++ b/drivers/spmi/spmi-mtk-pmif.c
> @@ -21,6 +21,7 @@
>   #define SWINF_WFVLDCLR	0x06
>   
>   #define GET_SWINF(x)	(((x) >> 1) & 0x7)
> +#define GET_SWINFERR(x) (((x) >> 18) & 0x1)
>   
>   #define PMIF_CMD_REG_0		0
>   #define PMIF_CMD_REG		1
> @@ -349,6 +350,7 @@ static int pmif_spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>   	struct pmif *arb = to_mtk_pmif(ctrl);
>   	struct ch_reg *inf_reg;
>   	int ret;
> +	u8 retry = 0;
>   	u32 data, cmd;
>   	unsigned long flags;
>   
> @@ -385,17 +387,35 @@ static int pmif_spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>   		return ret;
>   	}
>   
> -	/* Send the command. */
>   	cmd = (opc << 30) | (sid << 24) | ((len - 1) << 16) | addr;
> -	pmif_writel(arb, pbus, cmd, inf_reg->ch_send);
> +	do {
> +		/* Send the command. */
> +		pmif_writel(arb, pbus, cmd, inf_reg->ch_send);
> +
> +		/*
> +		 * Wait for Software Interface FSM state to be WFVLDCLR or to
> +		 * return an error.
> +		 *
> +		 * If this is WFVLDCLR, read the data and clear the valid flag;
> +		 * If error or timeout, retry for a maximum of 3 times as a
> +		 * workaround for an hardware issue.
> +		 */
> +		ret = readl_poll_timeout_atomic(pbus->base + arb->data->regs[inf_reg->ch_sta],
> +						data,
> +						GET_SWINF(data) == SWINF_WFVLDCLR ||
> +						GET_SWINFERR(data),
> +						PMIF_DELAY_US, PMIF_TIMEOUT_US / 2);
> +		if (ret < 0)
> +			continue;
> +
> +		if (GET_SWINFERR(data)) {
> +			ret = -EIO;
> +			continue;
> +		}
> +
> +		break;
> +	} while (++retry < 3);
>   
> -	/*
> -	 * Wait for Software Interface FSM state to be WFVLDCLR,
> -	 * read the data and clear the valid flag.
> -	 */
> -	ret = readl_poll_timeout_atomic(pbus->base + arb->data->regs[inf_reg->ch_sta],
> -					data, GET_SWINF(data) == SWINF_WFVLDCLR,
> -					PMIF_DELAY_US, PMIF_TIMEOUT_US);
>   	if (ret < 0) {
>   		raw_spin_unlock_irqrestore(&pbus->lock, flags);
>   		dev_err(&ctrl->dev, "failed to wait for SWINF_WFVLDCLR\n");