Re: [PATCH 2/4] spi: bcmbca-hsspi: return error from failed controller suspend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Propagating the error from `spi_controller_suspend()` leaves the SPI controller permanently wedged in a suspended state.
--

commit 4a5c38a8fabc327922f122498eb97084160abb1e
Author: Jiawen Liu <1298662399@qq.com>

spi: bcmbca-hsspi: return error from failed controller suspend

This commit updates bcmbca_hsspi_suspend to check the return value of
spi_controller_suspend. If stopping the queue fails, it propagates the error
back to the PM core to prevent disabling the clocks while transfers are active.

> diff --git a/drivers/spi/spi-bcmbca-hsspi.c b/drivers/spi/spi-bcmbca-hsspi.c
> index 09c1472ae4fa1..af88ce04948b7 100644
> --- a/drivers/spi/spi-bcmbca-hsspi.c
> +++ b/drivers/spi/spi-bcmbca-hsspi.c
> @@ -568,8 +568,12 @@ static int bcmbca_hsspi_suspend(struct device *dev)
>  {
>  	struct spi_controller *host = dev_get_drvdata(dev);
>  	struct bcmbca_hsspi *bs = spi_controller_get_devdata(host);
> +	int ret;
> +
> +	ret = spi_controller_suspend(host);
> +	if (ret)
> +		return ret;

[Severity: High]
Does returning the error directly here leave the SPI controller permanently
stuck in a suspended state?

Looking at how spi_controller_suspend() handles errors, it unconditionally
marks the controller as suspended before returning:

drivers/spi/spi.c:spi_controller_suspend() {
	if (ctlr->queued) {
		ret = spi_stop_queue(ctlr);
		if (ret)
			dev_err(&ctlr->dev, "queue stop failed\n");
	}

	__spi_mark_suspended(ctlr);
	return ret;
}

If bcmbca_hsspi_suspend() returns this error to the PM core, the system
suspend is aborted. Because the device's suspend failed, the PM core considers
the device to be active and will not invoke its resume callback during the
PM abort recovery phase.

Since spi_controller_resume() is never called, the controller remains
flagged as suspended, meaning all future SPI transfers will fail with
-ESHUTDOWN until a subsequent successful suspend/resume cycle.

Should we explicitly call spi_controller_resume(host) to unwind the
internal suspend state before returning the error?

>  
> -	spi_controller_suspend(host);
>  	clk_disable_unprepare(bs->pll_clk);
>  	clk_disable_unprepare(bs->clk);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620083931.1120616-1-1298662399@qq.com?part=2