Re: [PATCH v3 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode

[Aniket RANDIVE]

On 10/27/2026 8:01 PM, Mukesh Savaliya wrote:
> On 10/27/2026 10:43 AM, Aniket Randive wrote:
> [...]
>
> > +static int geni_i2c_gpi_bus_cmd(struct geni_i2c_dev *gi2c, enum 
> > i2c_op cmd)
> > +{
> > +    const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
> > +    struct dma_async_tx_descriptor *desc;
> > +    struct gpi_i2c_config peripheral = {};
> > +    struct dma_slave_config config = {};
> > +    unsigned long time_left;
> > +    unsigned long flags;
> > +    dma_cookie_t cookie;
> > +    int ret;
> > +
> > +    config.peripheral_config = &peripheral;
> > +    config.peripheral_size = sizeof(peripheral);
> > +
> > +    peripheral.set_config  = 1;
> > +    peripheral.pack_enable = I2C_PACK_TX | I2C_PACK_RX;
> > +    peripheral.cycle_count = itr->t_cycle_cnt;
> > +    peripheral.high_count  = itr->t_high_cnt;
> > +    peripheral.low_count   = itr->t_low_cnt;
> > +    peripheral.clk_div     = itr->clk_div;
> > +    peripheral.op          = cmd;
> > +
> > +    ret = dmaengine_slave_config(gi2c->tx_c, &config);
> > +    if (ret) {
> > +        dev_err(gi2c->se.dev, "dma config error %d for bus cmd %u\n", 
> > ret, cmd);
> > +        return ret;
> > +    }
> > +
> > +    desc = dmaengine_prep_slave_single(gi2c->tx_c, 0, 0, DMA_MEM_TO_DEV,
> > +                       DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> > +    if (!desc)
> > +        return -EIO;
> > +
> > +    desc->callback_result = i2c_gpi_cb_result;
> > +    desc->callback_param = gi2c;
> > +
> > +    cookie = dmaengine_submit(desc);
> > +    if (dma_submit_error(cookie))
> > +        return -EIO;
>
> Rather capture ret and return it if it's an error ?

sure. I will make the change in next patch.
> > +
> > +    /*
> > +     * cur and err are shared with geni_i2c_irq() and the GPI callback
> > +     * i2c_gpi_cb_result(); both write err (the IRQ handler under
> > +     * gi2c->lock). Reset them under the lock before issuing the 
> > transfer.
> > +     */
> > +    spin_lock_irqsave(&gi2c->lock, flags);
> > +    gi2c->cur = NULL;
> > +    gi2c->err = 0;
> > +    spin_unlock_irqrestore(&gi2c->lock, flags);
> > +    reinit_completion(&gi2c->done);
> Safe to init before dmaengine_submit().> + dma_async_issue_pending(gi2c- 
>  >tx_c);

Agree. I will make the change in next patch.

> > +
> > +    time_left = wait_for_completion_timeout(&gi2c->done, gi2c- 
> > >adap.timeout);
> > +    if (!time_left) {
> > +        dev_err(gi2c->se.dev, "timeout waiting for GPI bus cmd %u\n", 
> > cmd);
> > +        ret = dmaengine_terminate_sync(gi2c->tx_c);
> > +        if (ret)
> > +            dev_err(gi2c->se.dev, "DMA termination failed for bus cmd 
> > %u: %d\n",
> > +                cmd, ret);
> > +        return -ETIMEDOUT;
> > +    }
> > +
> > +    return gi2c->err;
> > +}
> > +[...]