Re: [PATCH v3 6/7] spi: cadence-xspi: Add support for panic writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] spi: cadence-xspi: Unbounded wait for hardware completion in STIG commands
--

commit 84bca01aaddf0cb9dc5516a5faf39530c5682fae
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: cadence-xspi: Add support for panic writes

This commit advertises support for panic writes in the Cadence XSPI driver.
When a memory operation is received with the panic flag set, the driver
operates without relying on interrupts and without sleeping.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c

[ ... ]

> @@ -595,23 +592,23 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  
>  		cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
>  
> -		if (cdns_xspi->irq >= 0) {
> +		if (cdns_xspi->irq >= 0 && !op->panic_write) {
>  			wait_for_completion(&cdns_xspi->sdma_complete);

[Severity: Medium]
Could this unbounded wait lead to a hang if the hardware DMA completion
interrupt is lost during a non-panic SPI memory operation?

The fallback polling path in cdns_xspi_is_sdma_ready() correctly enforces
a timeout, but this interrupt path in cdns_xspi_send_stig_command()
blocks indefinitely in an uninterruptible state.

Does it make sense to use wait_for_completion_timeout() here to prevent
potential kernel lockups from unresponsive hardware?

>  			if (cdns_xspi->sdma_error) {
>  				cdns_xspi->set_interrupts_handler(cdns_xspi, false);
>  				return -EIO;
>  			}
> -		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, true)) {
> +		} else if (!cdns_xspi_is_sdma_ready(cdns_xspi, !op->panic_write)) {
>  			return -EIO;
>  		}
>  
>  		cdns_xspi->sdma_handler(cdns_xspi);
>  	}
>  
> -	if (cdns_xspi->irq >= 0) {
> +	if (cdns_xspi->irq >= 0 && !op->panic_write) {
>  		wait_for_completion(&cdns_xspi->cmd_complete);

[Severity: Medium]
Similarly, could this wait for the STIG command completion expose the
system to a hardware-induced hang if the interrupt is dropped?

>  		cdns_xspi->set_interrupts_handler(cdns_xspi, false);
> -	} else if (!cdns_xspi_is_stig_ready(cdns_xspi, true)) {
> +	} else if (!cdns_xspi_is_stig_ready(cdns_xspi, !op->panic_write)) {
>  		return -EIO;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014154301.157755-1-paul.cercueil@bootlin.com?part=6

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/