Re: [PATCH 4/7] spmi: apple: Implement remaining commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent SPMI commands from different contexts will interleave I/O register accesses, corrupting the hardware FIFO.
--

--- Patch [4]: [PATCH 4/7] spmi: apple: Implement remaining commands ---
commit 98224e3bba3190837586da097a367e28d4210468
Author: Alba Mendez <me@alba.sh>

spmi: apple: Implement remaining commands

Add support for zero write and power management commands to the Apple SPMI
controller.

> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index 4678a9e5270aa..d69d16665ce85 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c

[ ... ]

> @@ -63,18 +63,27 @@ static int apple_spmi_wait_rx_not_empty(struct spmi_controller *ctrl)
>  	return 0;
>  }
>  
> -static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> -			 u16 saddr, u8 *buf, size_t len)
> +static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> +			 u16 param, const u8 *buf, size_t len, u8 *ibuf, size_t ilen)
>  {
>  	struct apple_spmi *spmi = spmi_controller_get_drvdata(ctrl);
> -	u32 spmi_cmd = apple_spmi_pack_cmd(opc, sid, saddr, len);
> +	u32 spmi_cmd = apple_spmi_pack_cmd(opc, sid, param);
>  	u32 reply, rsp;
>  	size_t len_read = 0;
> -	u8 i;
> +	size_t i = 0, j;
>  	int ret;
>  
>  	writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
>  
> +	while (i < len) {
> +		j = 0;
> +		spmi_cmd = 0;
> +		while ((j < 4) & (i < len))
> +			spmi_cmd |= buf[i++] << (j++ * 8);
> +
> +		writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
> +	}

[Severity: High]
This is a pre-existing issue, but can concurrent SPMI commands from different
contexts interleave I/O register accesses here and corrupt the hardware FIFO?

Multiple client drivers registered on the same SPMI bus might perform
concurrent reads and writes using their own regmap instances.

Because spmi_raw_cmd() is entered concurrently by multiple threads, it issues
multiple sequential writel() and readl() calls to the shared hardware FIFOs 
without holding a bus-level spinlock or mutex.

Could this lead to malformed SPMI bus transactions if thread writes get
interleaved in this loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-t603x-spmi-v1-0-e1a29fcd2d38@chaosmail.tech?part=4