Re: [PATCH 3/3] [OMAP:I2C]OMAP3430 Silicon Errata 1.153

[Nishanth Menon <nm@ti.com>]

Sonasath, Moiz had written, on 07/15/2009 10:40 AM, the following:
>> When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
>> Otherwise some data bytes can be lost while transferring them from the
>> memory to the I2C interface.
>>
>> Do a Busy-wait for XUDF, before writing data to DATA_REG. While waiting
>> if there is NACK | AL, set the appropriate error flags, ack the pending
>> +							}
>> +							cpu_relax();
>> +							stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
>> +						}
> this is an infinite while loop + it tries to handle error cases - 
> essentially do another isr routine inside itself.
> [Moiz]
> Yes, it is a busy wait loop. But AFAIK while we come to this part of
 > the code the only thing that can go wrong in the transfer is either
 > the device would go off suddenly (creating a NACK) or there is an
 > arbitration lost(AL). This loop takes care of these two error conditions.
 > Apart from these, if the hardware is functional, the XUDF bit should
 > be set as soon as the FIFO gets empty.
> 
> Correct me if I am missing something.
That is exactly what I was complaining about -> why not use the existing 
logic above in the code to take care of it as I indicated below? do you 
see a problem with the logic I send? it is lesser code and should IMHO 
take care of the same issue without the additional 3rd nested while loop
> 
> How about:
> Apply [1] and the following?
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index ad8d201..e3f21af 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -728,6 +728,12 @@ omap_i2c_isr(int this_irq, void *dev_id)
>                  }
>                  if (stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)) {
>                          u8 num_bytes = 1;
> +                       if (cpu_is_omap34xx() &&
> +                               !(stat & OMAP_I2C_STAT_XUDF)){
> +                               cpu_relax();
> +                               continue;
> +                       }
> +
>                          if (dev->fifo_size) {
>                                  if (stat & OMAP_I2C_STAT_XRDY)
>                                          num_bytes = dev->fifo_size;
> 
> 
> Regards,
> Nishanth Menon
> Ref:
> [1] http://patchwork.kernel.org/patch/32332/

-- 
Regards,
Nishanth Menon