[PATCH] net: phy: at803x: select correct page on initialization

[PATCH] net: phy: at803x: select correct page on initialization

[David Bauer]

The Atheros AR8031 and AR8033 expose different registers for SGMII/Fiber
as well as the copper side of the PHY depending on the BT_BX_REG_SEL bit
in the chip configure register.

The driver assumes the copper side is selected on probe, but this might
not be the case depending which page was last selected by the
bootloader.

Select the copper page when initializing the configuration to circumvent
this.

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/net/phy/at803x.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 481cf48c9b9e..c027ae820a6a 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -103,6 +103,9 @@
 #define AT803X_CLK_OUT_STRENGTH_HALF		1
 #define AT803X_CLK_OUT_STRENGTH_QUARTER		2
 
+#define AT803X_PAGE_COPPER		0
+#define AT803X_PAGE_FIBER		1
+
 #define ATH9331_PHY_ID 0x004dd041
 #define ATH8030_PHY_ID 0x004dd076
 #define ATH8031_PHY_ID 0x004dd074
@@ -160,6 +163,21 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 	return phy_write(phydev, AT803X_DEBUG_DATA, val);
 }
 
+static int at803x_switch_page(struct phy_device *phydev, u8 page)
+{
+	int ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
+
+	if (ccr < 0)
+		return ccr;
+
+	if (page == AT803X_PAGE_COPPER)
+		ccr = ccr | AT803X_BT_BX_REG_SEL;
+	else
+		ccr = ccr & ~AT803X_BT_BX_REG_SEL;
+
+	return phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr);
+}
+
 static int at803x_enable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
@@ -534,6 +552,12 @@ static int at803x_config_init(struct phy_device *phydev)
 {
 	int ret;
 
+	if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
+		ret = at803x_switch_page(phydev, AT803X_PAGE_COPPER);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* The RX and TX delay default is:
 	 *   after HW reset: RX delay enabled and TX delay disabled
 	 *   after SW reset: RX delay enabled, while TX delay retains the
@@ -641,7 +665,7 @@ static int at803x_aneg_done(struct phy_device *phydev)
 		return aneg_done;
 
 	/* switch to SGMII/fiber page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
+	at803x_switch_page(phydev, AT803X_PAGE_FIBER);
 
 	/* check if the SGMII link is OK. */
 	if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
@@ -649,7 +673,7 @@ static int at803x_aneg_done(struct phy_device *phydev)
 		aneg_done = 0;
 	}
 	/* switch back to copper page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
+	at803x_switch_page(phydev, AT803X_PAGE_COPPER);
 
 	return aneg_done;
 }
-- 
2.25.2

Re: [PATCH] net: phy: at803x: select correct page on initialization

[Andrew Lunn]

On Mon, Mar 23, 2020 at 05:27:30PM +0100, David Bauer wrote:
> The Atheros AR8031 and AR8033 expose different registers for SGMII/Fiber
> as well as the copper side of the PHY depending on the BT_BX_REG_SEL bit
> in the chip configure register.
> 
> The driver assumes the copper side is selected on probe, but this might
> not be the case depending which page was last selected by the
> bootloader.
> 
> Select the copper page when initializing the configuration to circumvent
> this.

Hi David

You might want to look at phy_read_paged(), phy_write_paged(), etc,
depending on your needs. There can be locking issues, which these
functions address.

	  Andrew

Re: [PATCH] net: phy: at803x: select correct page on initialization

[David Bauer]

Hello Andrew,

On 3/23/20 5:53 PM, Andrew Lunn wrote:
> You might want to look at phy_read_paged(), phy_write_paged(), etc,
> depending on your needs. There can be locking issues, which these
> functions address.

Thanks for the hint, i was not aware of these methods. I see how they can handle
e.g. the page-switching logic in phy_read_paged.

However i still need to select the copper page on config initialization manually,
as genphy_update_link is not page aware or am i missing something here?

Best wishes
David

Re: [PATCH] net: phy: at803x: select correct page on initialization

[Andrew Lunn]

On Mon, Mar 23, 2020 at 10:08:49PM +0100, David Bauer wrote:
> Hello Andrew,
> 
> On 3/23/20 5:53 PM, Andrew Lunn wrote:
> > You might want to look at phy_read_paged(), phy_write_paged(), etc,
> > depending on your needs. There can be locking issues, which these
> > functions address.
> 
> Thanks for the hint, i was not aware of these methods. I see how they can handle
> e.g. the page-switching logic in phy_read_paged.
> 
> However i still need to select the copper page on config initialization manually,
> as genphy_update_link is not page aware or am i missing something here?

Hi David

Yes, for that initial setup, you need to directly write to the
register. But at803x_aneg_done() should use these helpers.

	  Andrew