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.
Thanks,
Andi
[*] b3b8df97723d ("i2c: i801: Use wait_event_timeout to wait for interrupts")
