2021-10-18 03:49:45

by Luo Jie

[permalink] [raw]
Subject: [PATCH v3 06/13] net: phy: add qca8081 read_status

1. Separate the function at803x_read_specific_status from
the at803x_read_status, since it can be reused by the
read_status of qca8081 phy driver excepting adding the
2500M speed.

2. Add the qca8081 read_status function qca808x_read_status.

Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/phy/at803x.c | 95 ++++++++++++++++++++++++++++++----------
1 file changed, 73 insertions(+), 22 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 0df474628461..42d3f8ccca94 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -41,6 +41,9 @@
#define AT803X_SS_SPEED_DUPLEX_RESOLVED BIT(11)
#define AT803X_SS_MDIX BIT(6)

+#define QCA808X_SS_SPEED_MASK GENMASK(9, 7)
+#define QCA808X_SS_SPEED_2500 4
+
#define AT803X_INTR_ENABLE 0x12
#define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15)
#define AT803X_INTR_ENABLE_SPEED_CHANGED BIT(14)
@@ -934,27 +937,9 @@ static void at803x_link_change_notify(struct phy_device *phydev)
}
}

-static int at803x_read_status(struct phy_device *phydev)
+static int at803x_read_specific_status(struct phy_device *phydev)
{
- int ss, err, old_link = phydev->link;
-
- /* Update the link, but return if there was an error */
- err = genphy_update_link(phydev);
- if (err)
- return err;
-
- /* why bother the PHY if nothing can have changed */
- if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
- return 0;
-
- phydev->speed = SPEED_UNKNOWN;
- phydev->duplex = DUPLEX_UNKNOWN;
- phydev->pause = 0;
- phydev->asym_pause = 0;
-
- err = genphy_read_lpa(phydev);
- if (err < 0)
- return err;
+ int ss;

/* Read the AT8035 PHY-Specific Status register, which indicates the
* speed and duplex that the PHY is actually using, irrespective of
@@ -965,13 +950,19 @@ static int at803x_read_status(struct phy_device *phydev)
return ss;

if (ss & AT803X_SS_SPEED_DUPLEX_RESOLVED) {
- int sfc;
+ int sfc, speed;

sfc = phy_read(phydev, AT803X_SPECIFIC_FUNCTION_CONTROL);
if (sfc < 0)
return sfc;

- switch (FIELD_GET(AT803X_SS_SPEED_MASK, ss)) {
+ /* qca8081 takes the different bits for speed value from at803x */
+ if (phydev->drv->phy_id == QCA8081_PHY_ID)
+ speed = FIELD_GET(QCA808X_SS_SPEED_MASK, ss);
+ else
+ speed = FIELD_GET(AT803X_SS_SPEED_MASK, ss);
+
+ switch (speed) {
case AT803X_SS_SPEED_10:
phydev->speed = SPEED_10;
break;
@@ -981,6 +972,9 @@ static int at803x_read_status(struct phy_device *phydev)
case AT803X_SS_SPEED_1000:
phydev->speed = SPEED_1000;
break;
+ case QCA808X_SS_SPEED_2500:
+ phydev->speed = SPEED_2500;
+ break;
}
if (ss & AT803X_SS_DUPLEX)
phydev->duplex = DUPLEX_FULL;
@@ -1005,6 +999,35 @@ static int at803x_read_status(struct phy_device *phydev)
}
}

+ return 0;
+}
+
+static int at803x_read_status(struct phy_device *phydev)
+{
+ int err, old_link = phydev->link;
+
+ /* Update the link, but return if there was an error */
+ err = genphy_update_link(phydev);
+ if (err)
+ return err;
+
+ /* why bother the PHY if nothing can have changed */
+ if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
+ return 0;
+
+ phydev->speed = SPEED_UNKNOWN;
+ phydev->duplex = DUPLEX_UNKNOWN;
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+
+ err = genphy_read_lpa(phydev);
+ if (err < 0)
+ return err;
+
+ err = at803x_read_specific_status(phydev);
+ if (err < 0)
+ return err;
+
if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete)
phy_resolve_aneg_pause(phydev);

@@ -1324,6 +1347,33 @@ static int qca83xx_config_init(struct phy_device *phydev)
return 0;
}

+static int qca808x_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
+ if (ret < 0)
+ return ret;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
+ ret & MDIO_AN_10GBT_STAT_LP2_5G);
+
+ ret = genphy_read_status(phydev);
+ if (ret)
+ return ret;
+
+ ret = at803x_read_specific_status(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (phydev->link && phydev->speed == SPEED_2500)
+ phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
+ else
+ phydev->interface = PHY_INTERFACE_MODE_SMII;
+
+ return 0;
+}
+
static struct phy_driver at803x_driver[] = {
{
/* Qualcomm Atheros AR8035 */
@@ -1445,6 +1495,7 @@ static struct phy_driver at803x_driver[] = {
.get_wol = at803x_get_wol,
.suspend = genphy_suspend,
.resume = genphy_resume,
+ .read_status = qca808x_read_status,
}, };

module_phy_driver(at803x_driver);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2021-10-18 21:43:28

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 06/13] net: phy: add qca8081 read_status

> +static int qca808x_read_status(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
> + if (ret < 0)
> + return ret;
> +
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
> + ret & MDIO_AN_10GBT_STAT_LP2_5G);
> +

Could genphy_c45_read_lpa() be used here?

Andrew

2021-10-19 12:12:19

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH v3 06/13] net: phy: add qca8081 read_status


On 10/19/2021 5:42 AM, Andrew Lunn wrote:
>> +static int qca808x_read_status(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
>> + ret & MDIO_AN_10GBT_STAT_LP2_5G);
>> +
> Could genphy_c45_read_lpa() be used here?
>
> Andrew

Hi Andrew,

Thanks for the comments,  the MDIO_STAT1 of PHY does not follow the
standard, bit0~bit6 of MDIO_STAT1 are

always 0, genphy_c45_read_lpa can't be used.

2021-10-19 12:36:29

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 06/13] net: phy: add qca8081 read_status

On Tue, Oct 19, 2021 at 08:10:15PM +0800, Jie Luo wrote:
>
> On 10/19/2021 5:42 AM, Andrew Lunn wrote:
> > > +static int qca808x_read_status(struct phy_device *phydev)
> > > +{
> > > + int ret;
> > > +
> > > + ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
> > > + ret & MDIO_AN_10GBT_STAT_LP2_5G);
> > > +
> > Could genphy_c45_read_lpa() be used here?
> >
> > Andrew
>
> Hi Andrew,
>
> Thanks for the comments,? the MDIO_STAT1 of PHY does not follow the
> standard, bit0~bit6 of MDIO_STAT1 are
>
> always 0, genphy_c45_read_lpa can't be used.

O.K. It is a shame the hardware partially follow the standard, but
breaks it as well. Why go to the effort of partially following it,
when you don't gain anything from it because you need custom code
anyway?

Andrew

2021-10-19 12:49:55

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH v3 06/13] net: phy: add qca8081 read_status


On 10/19/2021 8:31 PM, Andrew Lunn wrote:
> On Tue, Oct 19, 2021 at 08:10:15PM +0800, Jie Luo wrote:
>> On 10/19/2021 5:42 AM, Andrew Lunn wrote:
>>>> +static int qca808x_read_status(struct phy_device *phydev)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_STAT);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->lp_advertising,
>>>> + ret & MDIO_AN_10GBT_STAT_LP2_5G);
>>>> +
>>> Could genphy_c45_read_lpa() be used here?
>>>
>>> Andrew
>> Hi Andrew,
>>
>> Thanks for the comments,  the MDIO_STAT1 of PHY does not follow the
>> standard, bit0~bit6 of MDIO_STAT1 are
>>
>> always 0, genphy_c45_read_lpa can't be used.
> O.K. It is a shame the hardware partially follow the standard, but
> breaks it as well. Why go to the effort of partially following it,
> when you don't gain anything from it because you need custom code
> anyway?
>
> Andrew

Hi Andrew,

Thanks for the suggestion. qca8081 PHY indeed add 2.5G capability based
on the

general 1G PHY, i will feedback this to the HW design team, thanks for
this comments.