On 30.01.2024 22:58, Andi Shyti wrote:
> Hi Heiner,
>
> On Tue, Jan 30, 2024 at 11:25:33AM +0100, Heiner Kallweit wrote:
>> On 30.01.2024 11:00, Andi Shyti wrote:
>>>>> On Fri, Sep 22, 2023 at 09:35:55PM +0200, Heiner Kallweit wrote:
>>>>>> I see no need for a driver-specific timeout value, therefore let's go
>>>>>> with the i2c core default timeout of 1s set by i2c_register_adapter().
>>>>>
>>>>> Why is the timeout value not needed in your opinion? Is the
>>>>> datasheet specifying anything here?
>>>>>
>>>> I2C core sets a timeout of 1s if driver doesn't set a timeout value.
>>>> So for me the question is: Is there an actual need or benefit of
>>>> setting a lower timeout value? And that's something I don't see.
>>>
>>> yes, that's why I am asking and I would like to have an opinion
>>> from Jean. I will try to get hold of the datasheets, as well and
>>> see if there is any constraint on the timeout.
>>>
>> The datasheet for the 7-series (doc# 326776-003) states:
>>
>> 5.21.3.2 Bus Time Out (The PCH as SMBus Master)
>> If there is an error in the transaction, such that an SMBus device does not signal an
>> acknowledge, or holds the clock lower than the allowed time-out time, the transaction
>> will time out. The PCH will discard the cycle and set the DEV_ERR bit. The time out
>> minimum is 25 ms (800 RTC clocks). The time-out counter inside the PCH will start
>> after the last bit of data is transferred by the PCH and it is waiting for a response.
>> The 25-ms time-out counter will not count under the following conditions:
>> 1. BYTE_DONE_STATUS bit (SMBus I/O Offset 00h, Bit 7) is set
>> 2. The SECOND_TO_STS bit (TCO I/O Offset 06h, Bit 1) is not set (this indicates that
>> the system has not locked up).
>>
>> It's my understanding that the chip will signal timeout after 25ms. So we should never
>> have the case that the host timeout kicks in (as long as it's >25ms).
>> So the host timeout value doesn't really matter.
>
> This driver is used by many platforms. I scrolled through the
> datasheets of few of them and they differ from each other.
>
> This was set by Jean[*] that's why I need to hear from him from
> where this 200 ms comes from.
>
> As this change doesn't change much to the economy of the driver,
> I would take it out from this series or place it at the end.
>
> As of now, I'm going to take patch 1 and 2 in and you can
> resubmit a v2 without the first two patches.
>
Sounds good. Thanks!
> Thanks,
> Andi
>
> [*] b3b8df97723d ("i2c: i801: Use wait_event_timeout to wait for interrupts")
Heiner
