Re: [PATCH] i2c: exynos5: fix arbitration lost handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux