Re: [PATCH v5 2/2] spi: qcom-geni: Use GPIO to notify master of SPI target activity

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] spi: qcom-geni: missing ready_gpio deassertion in GENI_SE_FIFO mode
- [High] spi: qcom-geni: ready_gpio completely unhandled in GENI_GPI_DMA mode
--

commit b777efb262a38b2b19686188b7453a1af9e45b0c
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

spi: qcom-geni: Use GPIO to notify master of SPI target activity

This commit adds an optional ready GPIO to notify the SPI master of the
target's active state when operating in SPI target mode. The GPIO is
asserted when a transfer begins and deasserted upon completion, abort,
or timeout.

> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index bced6279a0624..91e75e213f929 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c

[ ... ]

> @@ -936,8 +941,12 @@ static int spi_geni_transfer_one(struct spi_controller *spi,
>  	if (mas->cur_xfer_mode == GENI_SE_FIFO || mas->cur_xfer_mode == GENI_SE_DMA) {
>  		ret = setup_se_xfer(xfer, mas, slv->mode, spi);
>  		/* SPI framework expects +ve ret code to wait for transfer complete */
> -		if (!ret)
> +		if (!ret) {
> +			if (spi->target)
> +				gpiod_set_value(mas->ready_gpio, 1);
>  			ret = 1;
> +		}
> +
>  		return ret;
>  	}
>  	return setup_gsi_xfer(xfer, mas, slv, spi);

[Severity: High]
Should ready_gpio be asserted and deasserted for GENI_GPI_DMA mode as well?

If the hardware configuration supports GPI DMA, spi_geni_init() can select
GENI_GPI_DMA mode for the target. When spi_geni_transfer_one() delegates
directly to setup_gsi_xfer() here, the ready_gpio assertion is skipped.

Similarly, the DMA completion in spi_gsi_callback_result() does not seem to
deassert the GPIO.

Could this cause the SPI master to wait indefinitely for the ready signal on
platforms with GPI DMA enabled, leading to bus stalls and total communication
failure?

[ ... ]

> @@ -1023,6 +1032,8 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
>  		if (dma_rx_status & RX_RESET_DONE)
>  			complete(&mas->rx_reset_done);
>  		if (!mas->tx_rem_bytes && !mas->rx_rem_bytes && xfer) {
> +			if (spi->target)
> +				gpiod_set_value(mas->ready_gpio, 0);
>  			spi_finalize_current_transfer(spi);
>  			mas->cur_xfer = NULL;
>  		}

[Severity: High]
Does the GENI_SE_FIFO mode completion path in geni_spi_isr() also need to
deassert ready_gpio?

When a target SPI transfer falls back to GENI_SE_FIFO mode and the transfer
completes, geni_spi_isr() processes M_CMD_DONE_EN, calls
spi_finalize_current_transfer(), and clears mas->cur_xfer:

drivers/spi/spi-geni-qcom.c:geni_spi_isr() {
    ...
    if (mas->cur_xfer_mode == GENI_SE_FIFO) {
        ...
        if (m_irq & M_CMD_DONE_EN) {
            if (mas->cur_xfer) {
                spi_finalize_current_transfer(spi);
                mas->cur_xfer = NULL;
    ...
}

However, the GPIO doesn't appear to be deasserted in this path.

If the ready_gpio remains permanently asserted after the first GENI_SE_FIFO
transfer completes successfully, could this break the target-ready handshake
protocol and cause the SPI master to incorrectly perceive the target as always
ready?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927-use_gpio_to_notify_master_of_spi_target_activity-v5-0-9b973623080f@oss.qualcomm.com?part=2