Re: [PATCH 01/20] net: phy: realtek: Fix events detection failure in LPI mode

On 09.02.2021 11:15, Serge Semin wrote:

On Mon, Feb 08, 2021 at 09:14:02PM +0100, Heiner Kallweit wrote:

quoted

On 08.02.2021 15:03, Serge Semin wrote:

quoted

It has been noticed that RTL8211E PHY stops detecting and reporting events
when EEE is successfully advertised and RXC stopping in LPI is enabled.
The freeze happens right after 3.0.10 bit (PC1R "Clock Stop Enable"
register) is set. At the same time LED2 stops blinking as if EEE mode has
been disabled. Notably the network traffic still flows through the PHY
with no obvious problem. Anyway if any MDIO read procedure is performed
after the "RXC stop in LPI" mode is enabled PHY gets to be unfrozen, LED2
starts blinking and PHY interrupts happens again. The problem has been
noticed on RTL8211E PHY working together with DW GMAC 3.73a MAC and
reporting its event via a dedicated IRQ signal. (Obviously the problem has
been unnoticed in the polling mode, since it gets naturally fixed by the
periodic MDIO read procedure from the PHY status register - BMSR.)

In order to fix that problem we suggest to locally re-implement the MMD
write method for RTL8211E PHY and perform a dummy read right after the
PC1R register is accessed to enable the RXC stopping in LPI mode.

Signed-off-by: Serge Semin <redacted>
---
 drivers/net/phy/realtek.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 99ecd6c4c15a..cbb86c257aae 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c

@@ -559,6 +559,42 @@ static int rtl822x_write_mmd(struct phy_device *phydev, int devnum, u16 regnum,
 	return ret;
 }
 
+static int rtl8211e_write_mmd(struct phy_device *phydev, int devnum, u16 regnum,
+			      u16 val)
+{
+	int ret;
+
+	/* Write to the MMD registers by using the standard control/data pair.
+	 * The only difference is that we need to perform a dummy read after
+	 * the PC1R.CLKSTOP_EN bit is set. It's required to workaround an issue
+	 * of a partial core freeze so LED2 stops blinking in EEE mode, PHY
+	 * stops detecting the link change and raising IRQs until any read from
+	 * its registers performed. That happens only if and right after the PHY
+	 * is enabled to stop RXC in LPI mode.
+	 */
+	ret = __phy_write(phydev, MII_MMD_CTRL, devnum);
+	if (ret)
+		return ret;
+
+	ret = __phy_write(phydev, MII_MMD_DATA, regnum);
+	if (ret)
+		return ret;
+
+	ret = __phy_write(phydev, MII_MMD_CTRL, devnum | MII_MMD_CTRL_NOINCR);
+	if (ret)
+		return ret;
+

quoted

Nice analysis. Alternatively to duplicating this code piece we could
export mmd_phy_indirect(). But up to you.

I also considered creating a generic method to access the MMD
registers of a generic PHY, something like phy_read()/phy_write(), but
for MMD (alas just exporting mmd_phy_indirect() would not be enough).
But as I see it such methods need to be created only after we get to
have at least several places with duplicating direct MMD-read/write
patterns. Doing that just for a single place seems redundant. Anyway it's
up to maintainers to decide whether they want to see a generic part
of the phy_read_mmd()/phy_write_mmd() methods being detached and
exported as something like genphy_{read,write}_mmd() methods. I can do
that in v2 if you ask me to.

Right, adding something like a genphy_{read,write}_mmd() doesn't make
too much sense for now. What I meant is just exporting mmd_phy_indirect().
Then you don't have to open-code the first three steps of a mmd read/write.
And it requires no additional code in phylib.
But that's not at all a showstopper here.

-Sergey

quoted

quoted

+	ret = __phy_write(phydev, MII_MMD_DATA, val);
+	if (ret)
+		return ret;
+
+	if (devnum == MDIO_MMD_PCS && regnum == MDIO_CTRL1 &&
+	    val & MDIO_PCS_CTRL1_CLKSTOP_EN)
+		ret =  __phy_read(phydev, MII_MMD_DATA);
+
+	return ret < 0 ? ret : 0;
+}
+
 static int rtl822x_get_features(struct phy_device *phydev)
 {
 	int val;

@@ -725,6 +761,7 @@ static struct phy_driver realtek_drvs[] = {
 		.resume		= genphy_resume,
 		.read_page	= rtl821x_read_page,
 		.write_page	= rtl821x_write_page,
+		.write_mmd	= rtl8211e_write_mmd,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc916),
 		.name		= "RTL8211F Gigabit Ethernet",

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel