Re: [PATCH 3/3] i2c: qcom-geni: Avoid unnecessary transfer cancel on address NACK

[Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>]

On 7/8/2026 11:45 AM, Praveen Talari wrote:
> When a target does not acknowledge its address phase, the GENI hardware
> raises a NACK interrupt. In this case, SE_GENI_M_GP_LENGTH remains zero,
> indicating that no data phase has started and the transfer was aborted at
> the address stage.
> 
> The driver currently treats all transfer errors similarly and always
> issues geni_i2c_cancel_xfer() followed by TX/RX FSM resets for DMA
> transfers. For address NACKs, the transfer has already terminated in
> hardware and issuing an additional cancel/reset sequence is unnecessary.
> 
> Track address NACKs separately by checking the transfer progress length
> when a NACK interrupt is reported. Skip the transfer cancel operation and
> DMA FSM reset paths when the NACK occurred during the address phase.
> 
> This avoids redundant cancel/reset operations for a normal address NACK
> condition while preserving the existing error recovery flow for all other
> transfer failures.
> 
When a target does not acknowledge its address phase
The driver currently treats all transfer errors similarly
Track address NACKs separately
avoids redundant cancel/reset operations for a normal address NACK

Looks repeated , in general you are handling address NACK and avoiding 
cancel/reset operation. Differentiate this NACK response against real 
errors.
Try to simplify and shorten it.

> Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
>   drivers/i2c/busses/i2c-qcom-geni.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 9490aee4928c..0448654f2678 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -130,6 +130,7 @@ struct geni_i2c_dev {
>   	struct dma_chan *rx_c;
>   	bool no_dma;
>   	bool gpi_mode;
> +	bool addr_nack;
>   	bool is_tx_multi_desc_xfer;
>   	u32 num_msgs;
>   	struct geni_i2c_gpi_multi_desc_xfer i2c_multi_desc_config;
> @@ -293,8 +294,12 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev)
>   	if (!cur ||
>   	    m_stat & (M_CMD_FAILURE_EN | M_CMD_ABORT_EN) ||
>   	    dm_rx_st & (DM_I2C_CB_ERR)) {
> -		if (m_stat & M_GP_IRQ_1_EN)
> +		if (m_stat & M_GP_IRQ_1_EN) {
>   			geni_i2c_err(gi2c, NACK);
So we are handling NACK first and then deciding if its ADDR NACK ?
You can pass ADDR or DATA nack to this function right ? and print the 
error log accordingly inside.

> +			val = readl_relaxed(base + SE_GENI_M_GP_LENGTH);
> +			if (!val)
> +				gi2c->addr_nack = true;
> +		}
>   		if (m_stat & M_GP_IRQ_3_EN)
>   			geni_i2c_err(gi2c, BUS_PROTO);
>   		if (m_stat & M_GP_IRQ_4_EN)
> @@ -443,7 +448,7 @@ static void geni_i2c_rx_msg_cleanup(struct geni_i2c_dev *gi2c,
[...]