On 10.02.2017 08:39, Andrzej Hajda wrote:
> 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.
I have just found in spec of newer version of the chip configuration bit
NO_ARB_FASTSDA. Citation:
> This bit deactivates the modified logic of the
> abnormal operation when the SCL is falling. Then,
> other I2C device quickly lowers the SDA line, and
> SDA has fallen before the first PCLK period
> elapses.
> 0 = Skip the master arbitration checking during the
> first PCLK period after SCL falls
> 1 = Restore original logic
I have no hardware with newer chip to really test if this solves the
issue, but I suppose it could, the only problem is that this bit is not
present in current chip version, Exynos5433.
Regards
Andrzej
>
> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
