Re: [PATCH] net: mdio: mdio-bcm-unimac: Delay before first poll

From: Florian Fainelli
Date: Wed Dec 13 2023 - 11:21:05 EST



On 12/13/2023 7:01 AM, Russell King (Oracle) wrote:
On Wed, Dec 13, 2023 at 11:57:52AM +0100, Andrew Lunn wrote:
On Tue, Dec 12, 2023 at 04:02:49PM -0800, Justin Chen wrote:
With a clock interval of 400 nsec and a 64 bit transactions (32 bit
preamble & 16 bit control & 16 bit data), it is reasonable to assume
the mdio transaction will take 25.6 usec. Add a 30 usec delay before
the first poll to reduce the chance of a 1000-2000 usec sleep.

#define MDIO_C45 0

suggests the hardware can do C45? The timing works out different then.
Maybe add a comment by the udelay() that is assumes C22, to give a
clue to somebody who is adding C45 support the delay needs to be
re-evaluated.

Yes the hardware supports C45 though as Russell points out, the driver intentionally does not support it.


Note, however, that the driver only supports C22 operations (it only
populates the read|write functions, not the c45 variants).

However, it doesn't explicitly set the MDIO_C22 bit in the configuration
register, so what ends up being spat out on the bus would be dependent
on the boot loader configuration.

The hardware is guaranteed to come up with the MDIO_C22 bit being set by default though it would not hurt setting it explicitly, that would be more future proof.


However, I'm wondering why unimac_mdio_poll() isn't written as
(based on current code):

return read_poll_timeout(unimac_mdio_readl(priv, MDIO_CMD), val,
!(val & MDIO_START_BUSY), 2000,
2000000);

rather than open-coding the io polling.

The driver long predates the introduction of the iopoll.h header and its routines. That sounds like another change that could be made.
--
Florian

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature