From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Janpieter Sollie <janpieter.sollie@kabelmail.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
netdev@vger.kernel.org, Heiner Kallweit <hkallweit1@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [RFC] increase MDIO i2c poll timeout gradually (including patch)
Date: Mon, 22 Sep 2025 14:50:19 +0100 [thread overview]
Message-ID: <aNFUG3uCx1Xe25d3@shell.armlinux.org.uk> (raw)
In-Reply-To: <aNFMEzweb1RLMuoF@shell.armlinux.org.uk>
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!
next prev parent reply other threads:[~2025-09-22 13:50 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-19 13:52 [RFC] increase MDIO i2c poll timeout gradually (including patch) Janpieter Sollie
2025-09-19 17:04 ` Andrew Lunn
2025-09-20 10:00 ` Janpieter Sollie
2025-09-20 11:18 ` Russell King (Oracle)
2025-09-20 12:34 ` Janpieter Sollie
[not found] ` <6d444507-1c97-4904-8edb-e8cc1aa4399e@kabelmail.de>
2025-09-20 13:53 ` Russell King (Oracle)
2025-09-22 8:04 ` Janpieter Sollie
2025-09-22 10:54 ` Janpieter Sollie
2025-09-22 12:38 ` Russell King (Oracle)
2025-09-22 13:16 ` Russell King (Oracle)
2025-09-22 13:50 ` Russell King (Oracle) [this message]
2025-09-22 14:30 ` Janpieter Sollie
2025-09-22 15:25 ` Russell King (Oracle)
2025-09-23 7:02 ` Janpieter Sollie
2025-09-22 14:46 ` Janpieter Sollie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aNFUG3uCx1Xe25d3@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=janpieter.sollie@kabelmail.de \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).