[PATCH net-next v2 0/2] net: phy: at803x: paging support

[PATCH net-next v2 0/2] net: phy: at803x: paging support

[Michael Walle]

Add paging support to the QCA AR8031/33 PHY. This will be needed if we
add support for the .config_inband_aneg callback, see series [1].

The driver itself already accesses the fiber page (without proper locking).
The former version of this patchset converted the access to
phy_read_paged(), but Vladimir Oltean mentioned that it is dead code.
Therefore, the second patch will just remove it.

changes since v1:
 - second patch will remove at803x_aneg_done() altogether

Michael Walle (2):
  net: phy: at803x: add pages support to AR8031/33
  net: phy: at803x: remove at803x_aneg_done()

 drivers/net/phy/at803x.c | 66 +++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 31 deletions(-)

-- 
2.20.1

[PATCH net-next v2 1/2] net: phy: at803x: add pages support to AR8031/33

[Michael Walle]

The AR8031 has two register sets: Copper and Fiber. The fiber page is
used in case of 100Base-FX and 1000Base-X. But more importantly it is
also used for the SGMII link. Add support to switch between these two.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/at803x.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c2aa4c92edde..194b414207d3 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -144,6 +144,9 @@
 #define ATH8035_PHY_ID 0x004dd072
 #define AT8030_PHY_ID_MASK			0xffffffef
 
+#define AT803X_FIBER_PAGE 0
+#define AT803X_COPPER_PAGE 1
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -1143,6 +1146,36 @@ static int at803x_cable_test_start(struct phy_device *phydev)
 	return 0;
 }
 
+static int at803x_read_page(struct phy_device *phydev)
+{
+	int val;
+
+	val = __phy_read(phydev, AT803X_REG_CHIP_CONFIG);
+	if (val < 0)
+		return val;
+
+	return (val & AT803X_BT_BX_REG_SEL) ? AT803X_COPPER_PAGE : AT803X_FIBER_PAGE;
+}
+
+static int at803x_write_page(struct phy_device *phydev, int page)
+{
+	u16 sel;
+
+	switch (page) {
+	case AT803X_FIBER_PAGE:
+		sel = 0;
+		break;
+	case AT803X_COPPER_PAGE:
+		sel = AT803X_BT_BX_REG_SEL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return __phy_modify(phydev, AT803X_REG_CHIP_CONFIG,
+			    AT803X_BT_BX_REG_SEL, sel);
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -1189,6 +1222,8 @@ static struct phy_driver at803x_driver[] = {
 	.flags			= PHY_POLL_CABLE_TEST,
 	.probe			= at803x_probe,
 	.remove			= at803x_remove,
+	.read_page		= at803x_read_page,
+	.write_page		= at803x_write_page,
 	.config_init		= at803x_config_init,
 	.config_aneg		= at803x_config_aneg,
 	.soft_reset		= genphy_soft_reset,
-- 
2.20.1

[PATCH net-next v2 2/2] net: phy: at803x: remove at803x_aneg_done()

[Michael Walle]

at803x_aneg_done() is pretty much dead code since the patch series
"net: phy: improve and simplify phylib state machine" [1]. Just remove
it.

[1] https://lore.kernel.org/netdev/922c223b-7bc0-e0ec-345d-2034b796af91@gmail.com/

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/at803x.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 194b414207d3..479dc2590236 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -754,36 +754,6 @@ static void at803x_link_change_notify(struct phy_device *phydev)
 	}
 }
 
-static int at803x_aneg_done(struct phy_device *phydev)
-{
-	int ccr;
-
-	int aneg_done = genphy_aneg_done(phydev);
-	if (aneg_done != BMSR_ANEGCOMPLETE)
-		return aneg_done;
-
-	/*
-	 * in SGMII mode, if copper side autoneg is successful,
-	 * also check SGMII side autoneg result
-	 */
-	ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
-	if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
-		return aneg_done;
-
-	/* switch to SGMII/fiber page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
-
-	/* check if the SGMII link is OK. */
-	if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
-		phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
-		aneg_done = 0;
-	}
-	/* switch back to copper page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
-
-	return aneg_done;
-}
-
 static int at803x_read_status(struct phy_device *phydev)
 {
 	int ss, err, old_link = phydev->link;
@@ -1233,7 +1203,6 @@ static struct phy_driver at803x_driver[] = {
 	.resume			= at803x_resume,
 	/* PHY_GBIT_FEATURES */
 	.read_status		= at803x_read_status,
-	.aneg_done		= at803x_aneg_done,
 	.config_intr		= &at803x_config_intr,
 	.handle_interrupt	= at803x_handle_interrupt,
 	.get_tunable		= at803x_get_tunable,
-- 
2.20.1

Re: [PATCH net-next v2 0/2] net: phy: at803x: paging support

[Vladimir Oltean]

On Thu, Feb 18, 2021 at 07:52:38PM +0100, Michael Walle wrote:
> Add paging support to the QCA AR8031/33 PHY. This will be needed if we
> add support for the .config_inband_aneg callback, see series [1].
>
> The driver itself already accesses the fiber page (without proper locking).
> The former version of this patchset converted the access to
> phy_read_paged(), but Vladimir Oltean mentioned that it is dead code.
> Therefore, the second patch will just remove it.
>
> changes since v1:
>  - second patch will remove at803x_aneg_done() altogether

I'm pretty sure net-next is closed now, since David sent the pull
request, and I didn't come to a conclusion yet regarding the final
form of the phy_config_inband_aneg method either.

Re: [PATCH net-next v2 0/2] net: phy: at803x: paging support

[Michael Walle]

Am 2021-02-18 20:26, schrieb Vladimir Oltean:
> On Thu, Feb 18, 2021 at 07:52:38PM +0100, Michael Walle wrote:
>> Add paging support to the QCA AR8031/33 PHY. This will be needed if we
>> add support for the .config_inband_aneg callback, see series [1].
>> 
>> The driver itself already accesses the fiber page (without proper 
>> locking).
>> The former version of this patchset converted the access to
>> phy_read_paged(), but Vladimir Oltean mentioned that it is dead code.
>> Therefore, the second patch will just remove it.
>> 
>> changes since v1:
>>  - second patch will remove at803x_aneg_done() altogether
> 
> I'm pretty sure net-next is closed now, since David sent the pull
> request, and I didn't come to a conclusion yet regarding the final
> form of the phy_config_inband_aneg method either.

Yeah I wasn't sure. http://vger.kernel.org/~davem/net-next.html says
it is still open. But anyway, if not, I'll resend the patch after
the merge window. I've also thought about splitting it into two
individual patches, because they aren't dependent on each other
anymore.

We'll need the page support anyway, even if phy_config_inband_aneg
will change. Ok granted, the cover letter might be misleading then.

-michael

Re: [PATCH net-next v2 0/2] net: phy: at803x: paging support

[David Miller]

From: Michael Walle <michael@walle.cc>
Date: Thu, 18 Feb 2021 20:46:10 +0100

> Am 2021-02-18 20:26, schrieb Vladimir Oltean:
>> On Thu, Feb 18, 2021 at 07:52:38PM +0100, Michael Walle wrote:
>>> Add paging support to the QCA AR8031/33 PHY. This will be needed if we
>>> add support for the .config_inband_aneg callback, see series [1].
>>> The driver itself already accesses the fiber page (without proper
>>> locking).
>>> The former version of this patchset converted the access to
>>> phy_read_paged(), but Vladimir Oltean mentioned that it is dead code.
>>> Therefore, the second patch will just remove it.
>>> changes since v1:
>>>  - second patch will remove at803x_aneg_done() altogether
>> I'm pretty sure net-next is closed now, since David sent the pull
>> request, and I didn't come to a conclusion yet regarding the final
>> form of the phy_config_inband_aneg method either.
> 
> Yeah I wasn't sure. http://vger.kernel.org/~davem/net-next.html says
> it is still open. But anyway, if not, I'll resend the patch after
> the merge window. I've also thought about splitting it into two
> individual patches, because they aren't dependent on each other
> anymore.

It is closed now, so this will have to be deferred.

Thanks.