Uwe Kleine-König <[email protected]> writes:

> On Mon, Jul 09, 2018 at 11:43:03AM +0200, Esben Haabendal wrote:
>> From: Esben Haabendal <[email protected]>
>> 
>> Always update the stopped state when busy status have been checked.
>> This is identical to what was done before, with the exception of error
>> handling.
>> Without this change, some errors cause the stopped state to be left in
>> incorrect state in i2c_imx_stop(), i2c_imx_dma_read(), i2c_imx_read() and
>> i2c_imx_xfer().
>> 
>> Signed-off-by: Esben Haabendal <[email protected]>
>> ---
>>  drivers/i2c/busses/i2c-imx.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
>> index d86f152176a4..1db8e6790afc 100644
>> --- a/drivers/i2c/busses/i2c-imx.c
>> +++ b/drivers/i2c/busses/i2c-imx.c
>> @@ -421,10 +421,14 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct 
>> *i2c_imx, int for_busy)
>>                      return -EAGAIN;
>>              }
>>  
>> -            if (for_busy && (temp & I2SR_IBB))
>> +            if (for_busy && (temp & I2SR_IBB)) {
>> +                    i2c_imx->stopped = 0;
>>                      break;
>> -            if (!for_busy && !(temp & I2SR_IBB))
>> +            }
>> +            if (!for_busy && !(temp & I2SR_IBB)) {
>> +                    i2c_imx->stopped = 1;
>>                      break;
>> +            }
>
> Would it make sense to assign to ->stopped independent of for_busy?

What do you mean?

Assigning to ->stopped on each check for I2SR_IBB in loop, independent
of the for_busy argument?  I don't think so.  The additional assignments
would be to the same value as it is set to already.

>>              if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
>>                      dev_dbg(&i2c_imx->adapter.dev,
>>                              "<%s> I2C bus is busy\n", __func__);
>> @@ -538,7 +542,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>>      result = i2c_imx_bus_busy(i2c_imx, 1);
>>      if (result)
>>              return result;
>> -    i2c_imx->stopped = 0;
>>  
>>      temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>>      temp &= ~I2CR_DMAEN;
>> @@ -569,7 +572,6 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
>>  
>>      if (!i2c_imx->stopped) {
>>              i2c_imx_bus_busy(i2c_imx, 0);
>> -            i2c_imx->stopped = 1;
>>      }
>
> The braces can go away here.

Will do.

/Esben

Reply via email to