Message-ID: <aNFUG3uCx1Xe25d3@shell.armlinux.org.uk>
Date: Mon, 22 Sep 2025 14:50:19 +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 02:16:03PM +0100, Russell King (Oracle) wrote:
> 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.

Having now been through i2c_transfer_rollball(), the additional
transactions on the bus will take between 97 and 98 cycles on top
of what I mentioned, or 970us - 980us at 100kHz.

In total, that works out at a minimum of 1.81ms for a successful
access to the hardware, assuming every I2C transaction occurs
back-to-back to the previous transaction.

Thanks.

-- 
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