[PATCH RFC/RFT 2/3] i2c: omap24xx_i2c: support CONFIG_I2C_REPEATED_START in DM_I2C xfer

[Aniket Limaye]

On 06/03/25 11:24, Heiko Schocher wrote:
> Hello Aniket,
> 
> On 04.03.25 23:04, Aniket Limaye wrote:
>> Repeated Start Condition (Sr) can be used to transfer multiple i2c msgs
>> without sending a Stop condition (P). So far, the driver default was to
>> always send a Stop condition after every i2c msg.
>>
>> Add support for a config option (CONFIG_I2C_REPEATED_START) to disable
>> sending the Stop condition by default. If this config is enabled, Stop
>> condition will be sent only if explicitly requested in the msg flags OR
>> if it is the last msg in the transfer.
>>
>> Consequently, handle the Repeated Start condition (Sr) in the next msg
>> by not calling the wait_for_bb() check since it will simply timeout in
>> the absence of a stop condition (BB will be 1 until Stop is programmed)
>>
>> Signed-off-by: Aniket Limaye <a-limaye at ti.com>
>> ---
>>   drivers/i2c/omap24xx_i2c.c | 22 ++++++++++++++++------
>>   1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
>> index 0d044121d27..d3256ab666d 100644
>> --- a/drivers/i2c/omap24xx_i2c.c
>> +++ b/drivers/i2c/omap24xx_i2c.c
>> @@ -1074,21 +1074,31 @@ static int omap_i2c_xfer(struct udevice *bus, 
>> struct i2c_msg *msg, int nmsgs)
>>       u16 i2c_con_reg = 0;
>>       debug("i2c_xfer: %d messages\n", nmsgs);
>> -    for (; nmsgs > 0; nmsgs--, msg++) {
>> +    for (int i = 0; i < nmsgs; i++, msg++) {
>>           debug("i2c_xfer: chip=0x%x, len=0x%x, read=0x%x\n",
>>                 msg->addr, msg->len, (msg->flags & I2C_M_RD));
>> -        /* Wait until bus not busy */
>> -        if (wait_for_bb(priv->regs, priv->ip_rev, priv->waitdelay))
>> -            return -EREMOTEIO;
>> +        /* If previous msg sent a Stop or if this is the first msg
>> +         * Wait until bus not busy
>> +         */
> 
> Nitpick: I would like more
> 
>          /*
>           * If previous msg sent a Stop or if this is the first msg
> 
> but unsure if we have here a strict rule...
> 

Sure, will do.

>> +        if ((i2c_con_reg & I2C_CON_STP) || (i == 0)) {
>> +            if (wait_for_bb(priv->regs, priv->ip_rev, priv->waitdelay))
>> +                return -EREMOTEIO;
>> +        }
> 
> You do not need the {} brackets here
> 

Right... didn't catch that
Will update in the patch series

>> -        /* Set Controller mode with Start and Stop bit */
>> -        i2c_con_reg = I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | 
>> I2C_CON_STP;
>> +        /* Set Controller mode with Start bit */
>> +        i2c_con_reg = I2C_CON_EN | I2C_CON_MST | I2C_CON_STT;
>>           /* Set Transmitter/Receiver mode if it is a write/read msg */
>>           if (msg->flags & I2C_M_RD)
>>               i2c_con_reg &= ~I2C_CON_TRX;
>>           else
>>               i2c_con_reg |= I2C_CON_TRX;
>> +        /* Send Stop condition (P) by default */
>> +        if (!IS_ENABLED(CONFIG_I2C_REPEATED_START))
>> +            i2c_con_reg |= I2C_CON_STP;
>> +        /* Send Stop if explicitly requested or if this is the last 
>> msg */
>> +        if ((msg->flags & I2C_M_STOP) || (i == nmsgs - 1))
>> +            i2c_con_reg |= I2C_CON_STP;
>>           ret = __omap24_i2c_xfer_msg(priv->regs, priv->ip_rev, 
>> priv->waitdelay,
>>                           msg->addr, msg->buf, msg->len,
>>
> 
> Thanks!
> 
> bye,
> Heiko