Message-ID: <aNFMEzweb1RLMuoF@shell.armlinux.org.uk>
Date: Mon, 22 Sep 2025 14:16:03 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Janpieter Sollie <janpieter.sollie@...elmail.de>
Cc: Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
	Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [RFC] increase MDIO i2c poll timeout gradually (including patch)

On Mon, Sep 22, 2025 at 01:38:01PM +0100, Russell King (Oracle) wrote:
> On Mon, Sep 22, 2025 at 12:54:20PM +0200, Janpieter Sollie wrote:
> > this is a diff of 10usecs (i=10), 40usecs (i=4) and 30usecs (i=3) my device
> > is running the i2c_transfer_rollball().
> > seems a lot to me when an i2c call takes 11-12 usecs avg per call
> > are you sure these numbers point to a stable i2c bus?
> 
> I guess you've never dealt with I2C buses before. As has already been
> stated, the clock rate for I2C used with SFP modules (which is, if you
> like, I2C v1) is 100kHz. that's 10us per bit.
> 
> An I2C transaction consists of one start bit, 8 bits for the address
> and r/w bit, one bit for the ack, 8 bits for the each byte of data
> and their individual ACK bits, and finally a stop bit. If a restart
> condition is used, the stop and start between the messages can be
> combined into a restart condition, saving one bit.
> 
> That works out at 1 + 8 + 1 + N*(8 + 1) + 1 bits, or 11 + 9 * N bits
> or clocks.
> 
> The polling consists of two transactions on the bus:
> 
> - a write of one byte - giving 20 clock cycles.
> - a read of six bytes - giving 65 clock cycles.
> 
> So that's 85 clock cycles, or 84 if using restart. At 10us per cycle,
> that's 840us _minimum_.
> 
> If i2c_transfer() for that write and read are taking on the order of
> 12us, that suggest the bus is being clocked at around 7MHz, which is
> certainly way too fast, a bug in the I2C driver, an issue with the
> I2C hardware, or maybe an error in calculating how long a call takes.
> 
> And... it's your interpretation of your results.
> 
> Remember, these are nanoseconds (ns), nanoseconds are 1000 microseconds
> (us) and there are 1000000 nanoseconds in a millisecond (ms). Sorry
> to teach you to suck eggs, but based on your reply it seems necessary
> to point this out.
> 
> You quoted an average of 99901858ns - 99.9ms for the i=3 case.
> You quoted an average of 86375811ns - 86.4ms for the i=4 case.
> 
> Given that the difference in msleep() delay is 5ms, and we're
> talking about 50 or 55ms here, for the i=3 case that's 45ms
> for i2c_transfer(). For the i=4 case, that's 36.4ms.
> 
> However, msleep() is not accurate - and may even be bucket-based so I
> wouldn't rely on the requested msleep() interval being what you end
> up with - which seems to be suggested by the difference of almost 10ms
> in the apparent time that i2c_transfer() takes. 10ms, not 10us. So,
> unless you actually obtain timestamps before and after the
> i2c_transfer() call and calculate their difference, I would not read
> too much into that.
> 
> In any case, figures in the realms of milliseconds are certainly in the
> realm of possibility for a 100kHz bus - as I say, one instance of a
> transaction _should_ be no less than 840 microseconds, so if your
> calculations come out less than that, you should not be claiming
> "bad bus" or something like that, but at first revalidating your
> analysis or interpretation of the figures.
> 
> Also, because of scheduling delays, and some I2C drivers will sleep
> waiting for the transaction to finish, even that measurement I
> suggest can not be said to relate to the actual time it takes for
> the transactions on the bus, unless you're running a hard-realtime OS.
> 
> However, it seems you're very keen to blame the I2C bus hardware...

I'll also point out that if an error occurs on the I2C bus, the earliest
that could be detected is after the address byte (lack of ACK to the
address - non-responsive slave) which is 10 clocks, or 100us.

If an error occurs, then i2c_transfer() should either return an error,
or indicate that the transaction has not been completed.

What I have missed in my analysis is that it's not calling
i2c_transfer() but i2c_transfer_rollball() which does many more bus
transactions. So, my figures of 840us minimum is likely a grossly
under the expected time.

However, my fundamental point still stands - that being 10us for
i2c_transfer_rollball() is an insanely short period for the work that
needs to be done no matter _what_ happens on the hardware bus.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists