[PATCH net-next 1/3] net: phy: realtek: read duplex and gbit master from PHYSR register

[PATCH net-next 1/3] net: phy: realtek: read duplex and gbit master from PHYSR register

[Daniel Golle]

The PHYSR MMD register is present and defined equally for all RTL82xx
Ethernet PHYs.
Read duplex and Gbit master bits from rtlgen_decode_speed() and rename
it to rtlgen_decode_physr().

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/realtek.c | 48 ++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index c15d2f66ef0d..717284a71667 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -80,15 +80,19 @@
 
 #define RTL822X_VND2_GANLPAR				0xa414
 
-#define RTL822X_VND2_PHYSR				0xa434
-
 #define RTL8366RB_POWER_SAVE			0x15
 #define RTL8366RB_POWER_SAVE_ON			BIT(12)
 
 #define RTL9000A_GINMR				0x14
 #define RTL9000A_GINMR_LINK_STATUS		BIT(4)
 
-#define RTLGEN_SPEED_MASK			0x0630
+#define RTL_VND2_PHYSR				0xa434
+#define RTL_VND2_PHYSR_LINK			BIT(2)
+#define RTL_VND2_PHYSR_DUPLEX			BIT(3)
+#define RTL_VND2_PHYSR_SPEEDL			GENMASK(5, 4)
+#define RTL_VND2_PHYSR_SPEEDH			GENMASK(10, 9)
+#define RTL_VND2_PHYSR_MASTER			BIT(11)
+#define RTL_VND2_PHYSR_SPEED_MASK		(RTL_VND2_PHYSR_SPEEDL | RTL_VND2_PHYSR_SPEEDH)
 
 #define RTL_GENERIC_PHYID			0x001cc800
 #define RTL_8211FVD_PHYID			0x001cc878
@@ -660,9 +664,24 @@ static int rtl8366rb_config_init(struct phy_device *phydev)
 }
 
 /* get actual speed to cover the downshift case */
-static void rtlgen_decode_speed(struct phy_device *phydev, int val)
+static void rtlgen_decode_physr(struct phy_device *phydev, int val)
 {
-	switch (val & RTLGEN_SPEED_MASK) {
+	/* bit 2
+	 * 0: Link not OK
+	 * 1: Link OK
+	 */
+	phydev->link = !!(val & RTL_VND2_PHYSR_LINK);
+
+	/* bit 3
+	 * 0: Half Duplex
+	 * 1: Full Duplex
+	 */
+	if (val & RTL_VND2_PHYSR_DUPLEX)
+		phydev->duplex = DUPLEX_FULL;
+	else
+		phydev->duplex = DUPLEX_HALF;
+
+	switch (val & RTL_VND2_PHYSR_SPEED_MASK) {
 	case 0x0000:
 		phydev->speed = SPEED_10;
 		break;
@@ -684,6 +703,19 @@ static void rtlgen_decode_speed(struct phy_device *phydev, int val)
 	default:
 		break;
 	}
+
+	/* bit 11
+	 * 0: Slave Mode
+	 * 1: Master Mode
+	 */
+	if (phydev->speed >= 1000) {
+		if (val & RTL_VND2_PHYSR_MASTER)
+			phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
+		else
+			phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
+	} else {
+		phydev->master_slave_state = MASTER_SLAVE_STATE_UNSUPPORTED;
+	}
 }
 
 static int rtlgen_read_status(struct phy_device *phydev)
@@ -701,7 +733,7 @@ static int rtlgen_read_status(struct phy_device *phydev)
 	if (val < 0)
 		return val;
 
-	rtlgen_decode_speed(phydev, val);
+	rtlgen_decode_physr(phydev, val);
 
 	return 0;
 }
@@ -1007,11 +1039,11 @@ static int rtl822x_c45_read_status(struct phy_device *phydev)
 		return 0;
 
 	/* Read actual speed from vendor register. */
-	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL822X_VND2_PHYSR);
+	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL_VND2_PHYSR);
 	if (val < 0)
 		return val;
 
-	rtlgen_decode_speed(phydev, val);
+	rtlgen_decode_physr(phydev, val);
 
 	return 0;
 }
-- 
2.47.0

[PATCH net-next 2/3] net: phy: realtek: change order of calls in C22 read_status()

[Daniel Golle]

Always call rtlgen_read_status() first, so genphy_read_status() which
is called by it clears bits in case auto-negotiation has not completed.
Also clear 10GBT link-partner advertisement bits in case auto-negotiation
is disabled or has not completed.

Suggested-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/realtek.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 717284a71667..895008738ec0 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -956,17 +956,25 @@ static void rtl822xb_update_interface(struct phy_device *phydev)
 
 static int rtl822x_read_status(struct phy_device *phydev)
 {
-	if (phydev->autoneg == AUTONEG_ENABLE) {
-		int lpadv = phy_read_paged(phydev, 0xa5d, 0x13);
+	int lpadv, ret;
 
-		if (lpadv < 0)
-			return lpadv;
+	ret = rtlgen_read_status(phydev);
+	if (ret < 0)
+		return ret;
 
-		mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising,
-						  lpadv);
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    !phydev->autoneg_complete) {
+		mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising, 0);
+		return 0;
 	}
 
-	return rtlgen_read_status(phydev);
+	lpadv = phy_read_paged(phydev, 0xa5d, 0x13);
+	if (lpadv < 0)
+		return lpadv;
+
+	mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising, lpadv);
+
+	return 0;
 }
 
 static int rtl822xb_read_status(struct phy_device *phydev)
-- 
2.47.0

[PATCH net-next 3/3] net: phy: realtek: clear 1000Base-T link partner advertisement

[Daniel Golle]

Clear 1000Base-T link partner advertisement bits in Clause-45
read_status() function in case auto-negotiation is disabled or has not
been completed.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/phy/realtek.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 895008738ec0..49b47341c17e 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -1033,6 +1033,10 @@ static int rtl822x_c45_read_status(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    !genphy_c45_aneg_done(phydev))
+		mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, 0);
+
 	/* Vendor register as C45 has no standardized support for 1000BaseT */
 	if (phydev->autoneg == AUTONEG_ENABLE) {
 		val = phy_read_mmd(phydev, MDIO_MMD_VEND2,
-- 
2.47.0

Re: [PATCH net-next 1/3] net: phy: realtek: read duplex and gbit master from PHYSR register

[Russell King (Oracle)]

On Wed, Oct 09, 2024 at 02:53:03AM +0100, Daniel Golle wrote:
> -static void rtlgen_decode_speed(struct phy_device *phydev, int val)
> +static void rtlgen_decode_physr(struct phy_device *phydev, int val)
>  {
> -	switch (val & RTLGEN_SPEED_MASK) {
> +	/* bit 2
> +	 * 0: Link not OK
> +	 * 1: Link OK
> +	 */
> +	phydev->link = !!(val & RTL_VND2_PHYSR_LINK);

Be careful with this. The link status bit in the BMSR is latched-low,
meaning that it guarantees to inform the reader that the link failed at
some point between the preceding read and current read.

This is important to know, so code can react to a possibly different
negotiation result (we must see a link-fail to recognise a different
set of negotiation results.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/3] net: phy: realtek: read duplex and gbit master from PHYSR register

[Daniel Golle]

On Wed, Oct 09, 2024 at 11:01:59AM +0100, Russell King (Oracle) wrote:
> On Wed, Oct 09, 2024 at 02:53:03AM +0100, Daniel Golle wrote:
> > -static void rtlgen_decode_speed(struct phy_device *phydev, int val)
> > +static void rtlgen_decode_physr(struct phy_device *phydev, int val)
> >  {
> > -	switch (val & RTLGEN_SPEED_MASK) {
> > +	/* bit 2
> > +	 * 0: Link not OK
> > +	 * 1: Link OK
> > +	 */
> > +	phydev->link = !!(val & RTL_VND2_PHYSR_LINK);
> 
> Be careful with this. The link status bit in the BMSR is latched-low,
> meaning that it guarantees to inform the reader that the link failed at
> some point between the preceding read and current read.
> 
> This is important to know, so code can react to a possibly different
> negotiation result (we must see a link-fail to recognise a different
> set of negotiation results.)

The datasheet calls that bit "Real Time Link Status".
If you think we should not use it, I will drop it.