Re: [PATCH v9 6/6] spmi: apple: Add interrupt functionality

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race between command timeout recovery and delayed interrupt handler causes spurious command failures (-EIO).
--

commit b173ce6913a5026fc32908944fd6149c07832b2c
Author: Alba Mendez <me@alba.sh>

spmi: apple: Add interrupt functionality

Add support for interrupts sent by slave devices and use IRQ for
RX FIFO if possible, reducing latency compared to polling.

> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index fabccd25aa0da..2390174452ab4 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c

[ ... ]

> @@ -84,8 +165,10 @@ static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid, u16 param,
>  
>  	if (spmi->prev_fail) {
>  		writel(SPMI_ACT_FIFO_FLUSH, spmi->regs + SPMI_ACT_REG);
> +		apple_spmi_irq_ack_raw(spmi, SPMI_IRQ_NOTIFY);
>  		spmi->prev_fail = false;
>  	}
> +	reinit_completion(&spmi->fifo_rx);
>  
>  	writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);

[Severity: High]
Could this sequence lead to a race condition with a delayed interrupt?

If a previous command times out, setting spmi->prev_fail = true, but the
hardware responds shortly after, the NOTIFY interrupt might be raised.

If spmi_raw_cmd() is then called for the next command, it runs concurrently
with apple_spmi_irq_handler() on another CPU. The handler reads the NOTIFY
bit and commits to calling complete():

apple_spmi_irq_handler() {
    ...
    if (val & BIT(SPMI_IRQ_NOTIFY % 32)) {
        apple_spmi_irq_ack_raw(spmi, SPMI_IRQ_NOTIFY);
        complete(&spmi->fifo_rx);
        handled = true;
    }
}

Concurrently, the thread clears the hardware IRQ state and calls
reinit_completion(&spmi->fifo_rx) here without synchronizing against the
IRQ handler (as the handler does not take fifo_lock).

If complete() executes after or concurrently with reinit_completion(), it
leaves fifo_rx signaled. The new command's wait_for_completion_timeout()
will return immediately, see an empty FIFO, and fail with -EIO. 

Additionally, reinit_completion() performs a non-atomic update which races
with the spinlock operations inside complete().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-t603x-spmi-v9-0-09e702e3b153@chaosmail.tech?part=6