Re: [PATCH net-next v2 1/8] net: mdio: realtek-rtl9300: Add polling documentation

[Andrew Lunn <andrew@lunn.ch>]

> + * Each device has a SMI_POLL_CTRL register. A per-port bitmask decides if the hardware polling of
> + * the associated bus/address is active or not. The hardware runs a tight loop over this and for
> + * each set polling bit it issues a status check for the PHY. Attaching a logic analyzer to the
> + * MDIO bus of an RTL8380 and RTL8393 gives the following commands (in kernel notation):
> + *
> + *	RTL8380				RTL8393
> + *	---------------------------	---------------------------
> + *	phy_write(phy, 31, 0x0);	phy_read(phy, 0);
> + *	phy_write(phy, 13, 0x7);	phy_read(phy, 1);
> + *	phy_write(phy, 14, 0x3c);	phy_read(phy, 4);
> + *	phy_write(phy, 13, 0x8007);	phy_read(phy, 5);
> + *	phy_read(phy, 14);		phy_read(phy, 6);
> + *	phy_write(phy, 13, 0x7);	phy_read(phy, 9);
> + *	phy_write(phy, 14, 0x3d);	phy_read(phy, 10);
> + *	phy_write(phy, 13, 0x8007);	phy_read(phy, 15);
> + *	phy_read(phy, 14);		phy_write(phy, 13, 0x7);
> + *	phy_read(phy, 9);		phy_write(phy, 14, 0x3c);
> + *	phy_read(phy, 10);		phy_write(phy, 13, 0x4007);
> + *	phy_read(phy, 15);		phy_read(phy, 14);
> + *	phy_read(phy, 0);		phy_write(phy, 13, 0x7);
> + *	phy_read(phy, 1);		phy_write(phy, 14, 0x3d);
> + *	phy_read(phy, 4);		phy_write(phy, 13, 0x4007);
> + *	phy_read(phy, 5);		phy_read(phy, 14);
> + *	phy_read(phy, 6);

Great to see this reverse engineering.

> + *
> + * The c45 over c22 register 13/14 sequences read MDIO_AN_EEE_ADV and MDIO_AN_EEE_LPABLE.

How do you tell it that C45 over C22 is actually supported by the PHY?
Not all PHYs do. Some PHYs use those registers for other things.

> + * How does MDIO access from kernel work?
> + *
> + * When issuing MDIO accesses via an MMIO based interface the final write to the command register
> + * sets a "run command now" bit. Between two polling sequences for different PHYs the hardware
> + * checks if a user command needs to run and sends it onto the bus. Afterwards it simply continues
> + * its polling work. Inspecting the command sequence for a paged read on the logic analyzer gives:
> + *
> + *	RTL8380				RTL8393
> + *	---------------------------	---------------------------
> + *	phy_write(phy, 31, page);	phy_write(phy, 31, page);
> + *	phy_write(phy, reg, value);	phy_write(phy, reg, value);
> + *					phy_write(phy, 31, 0);
> + *
> + * What does this mean?
> + *
> + * There are slight differences in polling and PHY access between the models but the challenge
> + * stays the same. On the one hand that greatly simplifies the MAC layer, on the other hand it
> + * has some implications for the kernel PHY subsystem.
> + *
> + * - Without the polling and a proper MAC status, some of the link handling features do not work.
> + *   Especially an unpopulated MAC_LINK_STS register cancels operations to other MAC registers.
> + * - The Realtek page register 31 is magically modified in the background. On the RTL838x it is
> + *   simply reset. Other devices have hardware mitigations for this in place.
> + * - A c45 over c22 kernel access sequence is most likely to fail because chances are high that
> + *   the polling engine overwrites registers 13/14 in between.
> + * - PHY firmware loading can have issues. Especially if a PHY is designed to expect a clean
> + *   sequence of registers and values without deviation.
> + * - An access to one PHY will need to wait for the next free slot of the polling engine.

* - PHYs which make use of pages will break the hardware polling,
*   because it is not aware a different page is currently selected, and
*   the values it reads from the PHY do not mean what it expects.

> + *
> + * Conclusion: Kernel access to the PHYs must know and handle any interference that arises from
> + * the above described hardware polling.

This is not the best of wording. We need to narrow it down from
'kernel', to Realtek MDIO bus driver. What we cannot do is have PHY
drivers need to know anything about this. Working around this needs to
be limited to the Realtek MDIO driver, and probably the MDIO bus
locking operations.

When the PHY driver does a paged access, it takes the MDIO bus
lock. We need that to disable the HW polling. Once the paged access is
complete and the MDIO bus lock is released, we can re-enable HW
polling. I'm pretty sure C45 over C22 already takes the MDIO lock, so
that also solves the issue you pointed out above.

We also need an understanding of how the hardware uses the values it
reads during poll. One obvious issue i see is that it is not reading
register 26, so how does it know what speed the realtek PHYs are
using, in order to correctly configure the MAC?

    Andrew

---
pw-bot: cr