On 09.02.2017 17:27, Wolfram Sang wrote:
> On Thu, Jan 05, 2017 at 01:06:53PM +0100, Andrzej Hajda wrote:
>> In case of arbitration lost adequate interrupt sometimes is not signaled. As
>> a result transfer timeouts and is not retried, as it should. To avoid such
>> cases code is added to check transaction status in case of every interrupt.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
>> ---
>> +
>> + switch (trans_status & HSI2C_MASTER_ST_MASK) {
>> + case HSI2C_MASTER_ST_LOSE:
>> + i2c->state = -EAGAIN;
>> + goto stop;
>> + }
> Why not using if() instead of switch() as in the rest of the driver?
Following reasons (not serious ones):
1. With if() the line should be split anyway to keep 80 chars per line
limit, and the result looks less readable, so no gain:
if ((trans_status & HSI2C_MASTER_ST_MASK) ==
HSI2C_MASTER_ST_LOSE) {
i2c->state = -EAGAIN;
goto stop;
}
2. It is ready to handle other status values as well, without repeating
long if() clause, in fact during tests I have added more values, but
finally they went out.
3. In the rest of the functions if() is used because code tests if some
bits are set, here we test if some bit field have specific value,
slightly different thing.
Of course I can change to if() if you prefer it.
> And there is arbitration lost checking already with int_status &
> HSI2C_INT_TRANS_ABORT. Any guess why it doesn't trigger?
>
Nope. This patch is just a result of comparing register values during
good and bad transfer.
I have looked for similar issues over the net, but without ultimate
answer, some hints are that master can check status of SDA line too early.
Anyway it works correctly with gpio bit-banging driver, it suggests
there could be something wrong in hsi2c logic.
Regards
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
