[PATCH v1 0/2] net: phy: at803x: support qca8081 1G version chip

[PATCH v1 0/2] net: phy: at803x: support qca8081 1G version chip

[Luo Jie]

This patch series add supporting qca8081 1G version chip, the 1G version
chip can be identified by the register mmd7.0x901d bit0.

In addition, qca8081 does not support 1000BaseX mode and the sgmii fifo
reset is added on the link changed, which assert the fifo on the link
down, deassert the fifo on the link up.

Changes in v1:
	* switch to use genphy_c45_pma_read_abilities.
	* Remove the patch [remove 1000BaseX mode of qca8081].
	* move the sgmii fifo reset to link_change_notify.

Luo Jie (2):
  net: phy: at803x: support qca8081 1G chip type
  net: phy: at803x: add qca8081 fifo reset on the link changed

 drivers/net/phy/at803x.c | 95 ++++++++++++++++++++++++++++------------
 1 file changed, 68 insertions(+), 27 deletions(-)


base-commit: 3a8a670eeeaa40d87bd38a587438952741980c18
-- 
2.17.1

[PATCH v1 1/2] net: phy: at803x: support qca8081 1G chip type

[Luo Jie]

The qca8081 1G chip version does not support 2.5 capability, which
is distinguished from qca8081 2.5G chip according to the bit0 of
register mmd7.0x901d.

The fast retrain and master slave seed configs are only needed when
the 2.5G capability is supported.

Switch to use genphy_c45_pma_read_abilities for .get_features API.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 81 ++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1f307d90518..86cb030e5ebf 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -272,6 +272,10 @@
 #define QCA808X_CDT_STATUS_STAT_OPEN		2
 #define QCA808X_CDT_STATUS_STAT_SHORT		3
 
+/* QCA808X 1G chip type */
+#define QCA808X_PHY_MMD7_CHIP_TYPE		0x901d
+#define QCA808X_PHY_CHIP_TYPE_1G		BIT(0)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -897,15 +901,6 @@ static int at803x_get_features(struct phy_device *phydev)
 	if (err)
 		return err;
 
-	if (phydev->drv->phy_id == QCA8081_PHY_ID) {
-		err = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE);
-		if (err < 0)
-			return err;
-
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported,
-				err & MDIO_PMA_NG_EXTABLE_2_5GBT);
-	}
-
 	if (phydev->drv->phy_id != ATH8031_PHY_ID)
 		return 0;
 
@@ -1770,20 +1765,22 @@ static int qca808x_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	/* Config the fast retrain for the link 2500M */
-	ret = qca808x_phy_fast_retrain_config(phydev);
-	if (ret)
-		return ret;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported)) {
+		/* Config the fast retrain for the link 2500M */
+		ret = qca808x_phy_fast_retrain_config(phydev);
+		if (ret)
+			return ret;
 
-	/* Configure lower ramdom seed to make phy linked as slave mode */
-	ret = qca808x_phy_ms_random_seed_set(phydev);
-	if (ret)
-		return ret;
+		/* Configure lower ramdom seed to make phy linked as slave mode */
+		ret = qca808x_phy_ms_random_seed_set(phydev);
+		if (ret)
+			return ret;
 
-	/* Enable seed */
-	ret = qca808x_phy_ms_seed_enable(phydev, true);
-	if (ret)
-		return ret;
+		/* Enable seed */
+		ret = qca808x_phy_ms_seed_enable(phydev, true);
+		if (ret)
+			return ret;
+	}
 
 	/* Configure adc threshold as 100mv for the link 10M */
 	return at803x_debug_reg_mask(phydev, QCA808X_PHY_DEBUG_ADC_THRESHOLD,
@@ -1822,11 +1819,13 @@ static int qca808x_read_status(struct phy_device *phydev)
 		 * value is configured as the same value, the link can't be up and no link change
 		 * occurs.
 		 */
-		if (phydev->master_slave_state == MASTER_SLAVE_STATE_ERR) {
-			qca808x_phy_ms_seed_enable(phydev, false);
-		} else {
-			qca808x_phy_ms_random_seed_set(phydev);
-			qca808x_phy_ms_seed_enable(phydev, true);
+		if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported)) {
+			if (phydev->master_slave_state == MASTER_SLAVE_STATE_ERR) {
+				qca808x_phy_ms_seed_enable(phydev, false);
+			} else {
+				qca808x_phy_ms_random_seed_set(phydev);
+				qca808x_phy_ms_seed_enable(phydev, true);
+			}
 		}
 	}
 
@@ -1991,6 +1990,34 @@ static int qca808x_cable_test_get_status(struct phy_device *phydev, bool *finish
 	return 0;
 }
 
+static int qca808x_get_features(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_pma_read_abilities(phydev);
+	if (ret)
+		return ret;
+
+	/* The autoneg ability is not existed in bit3 of MMD7.1,
+	 * but it is supported by qca808x PHY, so we add it here
+	 * manually.
+	 */
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+
+	/* As for the qca8081 1G version chip, the 2500baseT ability is also
+	 * existed in the bit0 of MMD1.21, we need to remove it manually if
+	 * it is the qca8081 1G chip according to the bit0 of MMD7.0x901d.
+	 */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_PHY_MMD7_CHIP_TYPE);
+	if (ret < 0)
+		return ret;
+
+	if (QCA808X_PHY_CHIP_TYPE_1G & ret)
+		linkmode_clear_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported);
+
+	return 0;
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -2160,7 +2187,7 @@ static struct phy_driver at803x_driver[] = {
 	.set_tunable		= at803x_set_tunable,
 	.set_wol		= at803x_set_wol,
 	.get_wol		= at803x_get_wol,
-	.get_features		= at803x_get_features,
+	.get_features		= qca808x_get_features,
 	.config_aneg		= at803x_config_aneg,
 	.suspend		= genphy_suspend,
 	.resume			= genphy_resume,
-- 
2.17.1

Re: [PATCH v1 1/2] net: phy: at803x: support qca8081 1G chip type

[Andrew Lunn]

On Tue, Jul 04, 2023 at 05:00:15PM +0800, Luo Jie wrote:
> The qca8081 1G chip version does not support 2.5 capability, which
> is distinguished from qca8081 2.5G chip according to the bit0 of
> register mmd7.0x901d.
> 
> The fast retrain and master slave seed configs are only needed when
> the 2.5G capability is supported.
> 
> Switch to use genphy_c45_pma_read_abilities for .get_features API.

It is better to have lots of small patches, each doing one thing. If
something regresses, a git bisect gives a much finer idea where the
problem is. It is also easier to review small patches with good commit
messages.

So please break this patch up.

> -	/* Configure lower ramdom seed to make phy linked as slave mode */
> -	ret = qca808x_phy_ms_random_seed_set(phydev);
> -	if (ret)
> -		return ret;
> +		/* Configure lower ramdom seed to make phy linked as slave mode */
> +		ret = qca808x_phy_ms_random_seed_set(phydev);
> +		if (ret)
> +			return ret;

Shouldn't this depend on how MDIO_MMD_AN, MDIO_AN_T1_ADV_L bit
MDIO_AN_T1_ADV_M_MST is set? Maybe the user wants it to prefer master
rather than slave?

I know you are just trying to move code around, but it does seem like
a good time to also improve the code.

FYI: net-next is closed at the moment. Officially you should post as
RFC, or wait until it opens again.

  Andrew

Re: [PATCH v1 1/2] net: phy: at803x: support qca8081 1G chip type

[Jie Luo]

On 7/5/2023 7:24 AM, Andrew Lunn wrote:
> On Tue, Jul 04, 2023 at 05:00:15PM +0800, Luo Jie wrote:
>> The qca8081 1G chip version does not support 2.5 capability, which
>> is distinguished from qca8081 2.5G chip according to the bit0 of
>> register mmd7.0x901d.
>>
>> The fast retrain and master slave seed configs are only needed when
>> the 2.5G capability is supported.
>>
>> Switch to use genphy_c45_pma_read_abilities for .get_features API.
> 
> It is better to have lots of small patches, each doing one thing. If
> something regresses, a git bisect gives a much finer idea where the
> problem is. It is also easier to review small patches with good commit
> messages.
> 
> So please break this patch up.

Ok, i will split this patch to small patches, thanks for the suggestion.

> 
>> -	/* Configure lower ramdom seed to make phy linked as slave mode */
>> -	ret = qca808x_phy_ms_random_seed_set(phydev);
>> -	if (ret)
>> -		return ret;
>> +		/* Configure lower ramdom seed to make phy linked as slave mode */
>> +		ret = qca808x_phy_ms_random_seed_set(phydev);
>> +		if (ret)
>> +			return ret;
> 
> Shouldn't this depend on how MDIO_MMD_AN, MDIO_AN_T1_ADV_L bit
> MDIO_AN_T1_ADV_M_MST is set? Maybe the user wants it to prefer master
> rather than slave?
> 
> I know you are just trying to move code around, but it does seem like
> a good time to also improve the code.
> 
> FYI: net-next is closed at the moment. Officially you should post as
> RFC, or wait until it opens again.
> 
>    Andrew

Hi Andrew,
The master/slave configuration/status is only existed in MII reg9, 10 on 
qca8081 PHY, which is not existed in MDIO_MMD_AN or MDIO_MMD_PMAPMD, i 
will improve this code according to the user's prefer master/slave 
configuration in the next patch series.

Thanks for the reminder of close window, i will upload the patches after 
the open of next window.

[PATCH v1 2/2] net: phy: at803x: add qca8081 fifo reset on the link changed

[Luo Jie]

The qca8081 sgmii fifo needs to be reset on link down and
released on the link up in case of any abnormal issue
such as the packet blocked on the PHY.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/at803x.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 86cb030e5ebf..c26dec1763f3 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -276,6 +276,9 @@
 #define QCA808X_PHY_MMD7_CHIP_TYPE		0x901d
 #define QCA808X_PHY_CHIP_TYPE_1G		BIT(0)
 
+#define QCA8081_PHY_SERDES_MMD1_FIFO_CTRL	0x9072
+#define QCA8081_PHY_FIFO_RSTN			BIT(11)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -2018,6 +2021,16 @@ static int qca808x_get_features(struct phy_device *phydev)
 	return 0;
 }
 
+static void qca808x_link_change_notify(struct phy_device *phydev)
+{
+	/* Assert interface sgmii fifo on link down, deassert it on link up,
+	 * the interface device address is always phy address added by 1.
+	 */
+	mdiobus_c45_modify_changed(phydev->mdio.bus, phydev->mdio.addr + 1,
+			MDIO_MMD_PMAPMD, QCA8081_PHY_SERDES_MMD1_FIFO_CTRL,
+			QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -2196,6 +2209,7 @@ static struct phy_driver at803x_driver[] = {
 	.soft_reset		= qca808x_soft_reset,
 	.cable_test_start	= qca808x_cable_test_start,
 	.cable_test_get_status	= qca808x_cable_test_get_status,
+	.link_change_notify	= qca808x_link_change_notify,
 }, };
 
 module_phy_driver(at803x_driver);
-- 
2.17.1

Re: [PATCH v1 2/2] net: phy: at803x: add qca8081 fifo reset on the link changed

[Andrew Lunn]

On Tue, Jul 04, 2023 at 05:00:16PM +0800, Luo Jie wrote:
> The qca8081 sgmii fifo needs to be reset on link down and
> released on the link up in case of any abnormal issue
> such as the packet blocked on the PHY.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew