Re: [PATCH v2 5/7] spmi: apple: lock around FIFOs

[Janne Grunau <j@jannau.net>]

On Tue, Jul 28, 2026 at 11:28:11AM +0200, Sasha Finkelstein wrote:
> From: Alba Mendez <me@alba.sh>
> 
> The driver was missing locking around register interactions
> 
> Signed-off-by: Alba Mendez <me@alba.sh>
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---
>  drivers/spmi/spmi-apple-controller.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index aed18b7df15c..2cd4ed1803d4 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c
> @@ -14,6 +14,7 @@
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/platform_device.h>
>  #include <linux/spmi.h>
>  
> @@ -35,6 +36,7 @@
>  
>  struct apple_spmi {
>  	void __iomem *regs;
> +	struct mutex fifo_lock;
>  };
>  
>  #define poll_reg(spmi, reg, val, cond) \
> @@ -73,6 +75,8 @@ static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  	size_t i = 0, j;
>  	int ret;
>  
> +	mutex_lock(&spmi->fifo_lock);

please use

	scoped_guard(mutex, &spmi->fifo_lock) {
> +
>  	writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
>  
>  	while (i < len) {
> @@ -86,7 +90,7 @@ static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  
>  	ret = apple_spmi_wait_rx_not_empty(ctrl);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	reply = readl(spmi->regs + SPMI_RSP_REG);
>  
> @@ -94,7 +98,8 @@ static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  	while (len_read < ilen) {
>  		if (readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY) {
>  			dev_err(&ctrl->dev, "FIFO lacks reply data, controller stuck?\n");
> -			return -EIO;
> +			ret = -EIO;
> +			goto out;
>  		}
>  		rsp = readl(spmi->regs + SPMI_RSP_REG);
>  		i = 0;
> @@ -107,6 +112,8 @@ static int spmi_raw_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");
>  
> +	mutex_unlock(&spmi->fifo_lock);

	}

and avoid the 'goto's

Janne