Re: [PATCH v3 6/7] i2c: qcom-cci: Add burst read support

[Konrad Dybcio]

On 10/16/26 10:00 AM, Loic Poulain wrote:
> CCI_I2C_READ transfers are currently limited by max_read_len to the
> amount of data that fits in the read FIFO, as the driver only drains
> the FIFO once after receiving RD_DONE. On current hardware this limits
> reads to at most 254 bytes.
> 
> CCI v1.7 and later support reads larger than the FIFO depth by
> generating RD_THRESHOLD interrupts as the FIFO fills, allowing software
> to drain data while the transfer is still in progress. When the FIFO
> becomes full, the hardware can also apply backpressure by stretching
> SCL and reports this condition through RD_PAUSE_IRQ.
> 
> Program CCI_I2C_Mm_RD_THRESHOLD to half the FIFO depth on hardware that
> supports RD_THRESHOLD and update the read path to drain the FIFO on
> each threshold interrupt until the final RD_DONE event is received.
> 
> Older hardware revisions do not support RD_THRESHOLD, keep their read
> size limited to the FIFO depth and continue relying solely on RD_DONE
> for completion.
> 
> Also emit a warning when RD_PAUSE_IRQ is observed, as FIFO backpressure
> is unexpected under normal conditions and may indicate that software is
> not draining the FIFO quickly enough.
> 
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> ---

[...]

> +#define CCI_IRQ_MASK_1				0xc10

This exists all the way back to 8974, nice

[...]

> -	ret = cci_run_queue(cci, master, queue);
> -	if (ret < 0)
> -		return ret;
> +	val = readl(cci->base + CCI_I2C_Mm_Qn_CUR_WORD_CNT(master, queue));
> +	writel(val, cci->base + CCI_I2C_Mm_Qn_EXEC_WORD_CNT(master, queue));
>  
> -	words_read = readl(cci->base + CCI_I2C_Mm_READ_BUF_LEVEL(master));
> -	words_exp = len / 4 + 1;

GPT says: can we keep this check for the legacy case?

> -	if (words_read != words_exp) {
> -		dev_err(cci->dev, "words read = %d, words expected = %d\n",
> -			words_read, words_exp);
> -		return -EIO;
> -	}
> +	reinit_completion(&cci->master[master].irq_complete);
> +
> +	cci_set_rd_threshold_irq(cci, master, true);
> +
> +	val = BIT(master * 2 + queue);
> +	writel(val, cci->base + CCI_QUEUE_START);
> +
> +	/*
> +	 * Hardware supporting RD_THRESHOLD can handle reads larger than the FIFO size.
> +	 * The FIFO is drained each time an RD_THRESHOLD interrupt is received, until
> +	 * the final RD_DONE interrupt completes the transfer.
> +	 * On hardware lacking RD_THRESHOLD support, reads are limited to the FIFO
> +	 * size and only one completion (RD_DONE interrupt) is expected.
> +	 */
> +	while (index < len) {

GPT says: the last transfer isn't waiting for RD_DONE

> +		if (!wait_for_completion_timeout(&cci->master[master].irq_complete,
> +						 CCI_TIMEOUT)) {
> +			dev_err(cci->dev, "master %d queue %d read timeout\n",
> +				master, queue);
> +			cci_reset(cci);
> +			return -ETIMEDOUT;

GPT says: disable the threshold irq here?

Konrad