Message-ID: <2c3644ae-960d-42c7-816d-180acba0e289@kabelmail.de>
Date: Mon, 22 Sep 2025 16:46:31 +0200
From: Janpieter Sollie <janpieter.sollie@...elmail.de>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
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)

(rewriting as plain text, mail client accidentally converted to html)
Op 22/09/2025 om 14:38 schreef Russell King (Oracle):
> 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.
No, but I've read many kernel code dealing with it, desperately trying to figure out the error.
But yes, this is only the software part.
> 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_.
and here's my first error:
 > 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
I wrote usecs, but was obviously in a wrong universe, it should have been msecs.  Sorry
>
> 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.
Yes, I'm afraid of the last one as well ...
Honestly, even after re-reading it (twice),
I'm still not sure if / when I made an error, so my calculations are below
>
> 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're welcome, It's always useful to get corrected no mather what.
I'm simply trying to find a solution for hardware I built myself,
which is more or less "problem I created myself".
And I _WILL_ make dumb errors. I'm sorry for that.
but hey, at least I'm not crying like a little kid 'please help me to fix my internet' ....
Yes, I know the definition of ns -> us -> ms -> sec,
the commands used (example for i = 4 here, a total msleep of 245):

$ echo $(($(sed -n 's/.*\ \([0-9]\+\)\ ns\ in\ iteration\ 4/\1/p' < tempoutput.txt | sort -n | 
head -n 1) - 245000000)) -> for min
$ echo $(($(sed -n 's/.*\ \([0-9]\+\)\ ns\ in\ iteration\ 4/\1/p' < tempoutput.txt | sort -n | 
tail -n 1) - 245000000)) -> for max
$ echo diff at iteration 4: $((123074939 - 82868351)) -> for diff
$ echo $(($(sed -n 's/.*\ \([0-9]\+\)\ ns\ in\ iteration\ 4/\1/p' < tempoutput.txt | sort -n | 
awk '{x+=$0}END{print int(x/NR)}' -) - 245000000)) -> for average
avg: 86375811
so that's 86375811ns, 86375us, and 86ms, or ~12ms for each iteration
>
> 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.
Ouch ... that's a _LOT_ more than I thought.That explains the difference.  Sorry.
>
> 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.
you are giving a lot of reasons why it's unreliable ...
and I certainly won't dare to calculate useless statistics anymore.
>
> However, it seems you're very keen to blame the I2C bus hardware...
>
Based on my mails, I can certainly see why you're thinking this way.
I have no idea what goes wrong anywhere between me making a modification in the mdio.c file -> 
i2c code -> ... -> SFP phy.
I'm curious what goes wrong, notice the 3 dots in between,
I know there's a pca9545 muxer in in there further complicating it, but that's about it.

Long story short: should I somehow try to test the reliability of something else?

Thanks,

Janpieter Sollie

Powered by blists - more mailing lists