2019-08-11 23:44:49

by Tao Ren

[permalink] [raw]
Subject: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation

From: Heiner Kallweit <[email protected]>

This patch adds support for clause 37 1000Base-X auto-negotiation.

Signed-off-by: Heiner Kallweit <[email protected]>
Signed-off-by: Tao Ren <[email protected]>
---
Changes in v7:
- Update "if (AUTONEG_ENABLE != phydev->autoneg)" to
"if (phydev->autoneg != AUTONEG_ENABLE)" so checkpatch.pl is happy.
Changes in v6:
- add "Signed-off-by: Tao Ren <[email protected]>"
Changes in v1-v5:
- nothing changed. It's given v5 just to align with the version of
patch series.

drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
include/linux/phy.h | 5 ++
2 files changed, 144 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 252a712d1b2b..301a794b2963 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct phy_device *phydev)
return changed;
}

+/**
+ * genphy_c37_config_advert - sanitize and advertise auto-negotiation parameters
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MII_ADVERTISE with the appropriate values,
+ * after sanitizing the values to make sure we only advertise
+ * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
+ * hasn't changed, and > 0 if it has changed. This function is intended
+ * for Clause 37 1000Base-X mode.
+ */
+static int genphy_c37_config_advert(struct phy_device *phydev)
+{
+ u16 adv = 0;
+
+ /* Only allow advertising what this PHY supports */
+ linkmode_and(phydev->advertising, phydev->advertising,
+ phydev->supported);
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+ phydev->advertising))
+ adv |= ADVERTISE_1000XFULL;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ phydev->advertising))
+ adv |= ADVERTISE_1000XPAUSE;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ phydev->advertising))
+ adv |= ADVERTISE_1000XPSE_ASYM;
+
+ return phy_modify_changed(phydev, MII_ADVERTISE,
+ ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
+ ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
+ adv);
+}
+
/**
* genphy_config_eee_advert - disable unwanted eee mode advertisement
* @phydev: target phy_device struct
@@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_config_aneg);

+/**
+ * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
+ * @phydev: target phy_device struct
+ *
+ * Description: If auto-negotiation is enabled, we configure the
+ * advertising, and then restart auto-negotiation. If it is not
+ * enabled, then we write the BMCR. This function is intended
+ * for use with Clause 37 1000Base-X mode.
+ */
+int genphy_c37_config_aneg(struct phy_device *phydev)
+{
+ int err, changed;
+
+ if (phydev->autoneg != AUTONEG_ENABLE)
+ return genphy_setup_forced(phydev);
+
+ err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
+ BMCR_SPEED1000);
+ if (err)
+ return err;
+
+ changed = genphy_c37_config_advert(phydev);
+ if (changed < 0) /* error */
+ return changed;
+
+ if (!changed) {
+ /* Advertisement hasn't changed, but maybe aneg was never on to
+ * begin with? Or maybe phy was isolated?
+ */
+ int ctl = phy_read(phydev, MII_BMCR);
+
+ if (ctl < 0)
+ return ctl;
+
+ if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
+ changed = 1; /* do restart aneg */
+ }
+
+ /* Only restart aneg if we are advertising something different
+ * than we were before.
+ */
+ if (changed > 0)
+ return genphy_restart_aneg(phydev);
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c37_config_aneg);
+
/**
* genphy_aneg_done - return auto-negotiation status
* @phydev: target phy_device struct
@@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_read_status);

+/**
+ * genphy_c37_read_status - check the link status and update current link state
+ * @phydev: target phy_device struct
+ *
+ * Description: Check the link, then figure out the current state
+ * by comparing what we advertise with what the link partner
+ * advertises. This function is for Clause 37 1000Base-X mode.
+ */
+int genphy_c37_read_status(struct phy_device *phydev)
+{
+ int lpa, 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->duplex = DUPLEX_UNKNOWN;
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+
+ if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+ lpa = phy_read(phydev, MII_LPA);
+ if (lpa < 0)
+ return lpa;
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ phydev->lp_advertising, lpa & LPA_LPACK);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+ phydev->lp_advertising, lpa & LPA_1000XFULL);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+ phydev->lp_advertising, lpa & LPA_1000XPAUSE);
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ phydev->lp_advertising,
+ lpa & LPA_1000XPAUSE_ASYM);
+
+ phy_resolve_aneg_linkmode(phydev);
+ } else if (phydev->autoneg == AUTONEG_DISABLE) {
+ int bmcr = phy_read(phydev, MII_BMCR);
+
+ if (bmcr < 0)
+ return bmcr;
+
+ if (bmcr & BMCR_FULLDPLX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(genphy_c37_read_status);
+
/**
* genphy_soft_reset - software reset the PHY via BMCR_RESET bit
* @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 462b90b73f93..81a2921512ee 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
int genphy_resume(struct phy_device *phydev);
int genphy_loopback(struct phy_device *phydev, bool enable);
int genphy_soft_reset(struct phy_device *phydev);
+
+/* Clause 37 */
+int genphy_c37_config_aneg(struct phy_device *phydev);
+int genphy_c37_read_status(struct phy_device *phydev);
+
static inline int genphy_no_soft_reset(struct phy_device *phydev)
{
return 0;
--
2.17.1


2019-08-19 22:19:31

by Tao Ren

[permalink] [raw]
Subject: Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation

On 8/11/19 4:40 PM, Tao Ren wrote:
> From: Heiner Kallweit <[email protected]>
>
> This patch adds support for clause 37 1000Base-X auto-negotiation.
>
> Signed-off-by: Heiner Kallweit <[email protected]>
> Signed-off-by: Tao Ren <[email protected]>

A kind reminder: could someone help to review the patch when you have bandwidth?


Cheers,

Tao

> ---
> Changes in v7:
> - Update "if (AUTONEG_ENABLE != phydev->autoneg)" to
> "if (phydev->autoneg != AUTONEG_ENABLE)" so checkpatch.pl is happy.
> Changes in v6:
> - add "Signed-off-by: Tao Ren <[email protected]>"
> Changes in v1-v5:
> - nothing changed. It's given v5 just to align with the version of
> patch series.
>
> drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 5 ++
> 2 files changed, 144 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 252a712d1b2b..301a794b2963 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct phy_device *phydev)
> return changed;
> }
>
> +/**
> + * genphy_c37_config_advert - sanitize and advertise auto-negotiation parameters
> + * @phydev: target phy_device struct
> + *
> + * Description: Writes MII_ADVERTISE with the appropriate values,
> + * after sanitizing the values to make sure we only advertise
> + * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
> + * hasn't changed, and > 0 if it has changed. This function is intended
> + * for Clause 37 1000Base-X mode.
> + */
> +static int genphy_c37_config_advert(struct phy_device *phydev)
> +{
> + u16 adv = 0;
> +
> + /* Only allow advertising what this PHY supports */
> + linkmode_and(phydev->advertising, phydev->advertising,
> + phydev->supported);
> +
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XFULL;
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XPAUSE;
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XPSE_ASYM;
> +
> + return phy_modify_changed(phydev, MII_ADVERTISE,
> + ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
> + ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
> + adv);
> +}
> +
> /**
> * genphy_config_eee_advert - disable unwanted eee mode advertisement
> * @phydev: target phy_device struct
> @@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> +/**
> + * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
> + * @phydev: target phy_device struct
> + *
> + * Description: If auto-negotiation is enabled, we configure the
> + * advertising, and then restart auto-negotiation. If it is not
> + * enabled, then we write the BMCR. This function is intended
> + * for use with Clause 37 1000Base-X mode.
> + */
> +int genphy_c37_config_aneg(struct phy_device *phydev)
> +{
> + int err, changed;
> +
> + if (phydev->autoneg != AUTONEG_ENABLE)
> + return genphy_setup_forced(phydev);
> +
> + err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
> + BMCR_SPEED1000);
> + if (err)
> + return err;
> +
> + changed = genphy_c37_config_advert(phydev);
> + if (changed < 0) /* error */
> + return changed;
> +
> + if (!changed) {
> + /* Advertisement hasn't changed, but maybe aneg was never on to
> + * begin with? Or maybe phy was isolated?
> + */
> + int ctl = phy_read(phydev, MII_BMCR);
> +
> + if (ctl < 0)
> + return ctl;
> +
> + if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
> + changed = 1; /* do restart aneg */
> + }
> +
> + /* Only restart aneg if we are advertising something different
> + * than we were before.
> + */
> + if (changed > 0)
> + return genphy_restart_aneg(phydev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(genphy_c37_config_aneg);
> +
> /**
> * genphy_aneg_done - return auto-negotiation status
> * @phydev: target phy_device struct
> @@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_read_status);
>
> +/**
> + * genphy_c37_read_status - check the link status and update current link state
> + * @phydev: target phy_device struct
> + *
> + * Description: Check the link, then figure out the current state
> + * by comparing what we advertise with what the link partner
> + * advertises. This function is for Clause 37 1000Base-X mode.
> + */
> +int genphy_c37_read_status(struct phy_device *phydev)
> +{
> + int lpa, 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->duplex = DUPLEX_UNKNOWN;
> + phydev->pause = 0;
> + phydev->asym_pause = 0;
> +
> + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
> + lpa = phy_read(phydev, MII_LPA);
> + if (lpa < 0)
> + return lpa;
> +
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + phydev->lp_advertising, lpa & LPA_LPACK);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> + phydev->lp_advertising, lpa & LPA_1000XFULL);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> + phydev->lp_advertising, lpa & LPA_1000XPAUSE);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> + phydev->lp_advertising,
> + lpa & LPA_1000XPAUSE_ASYM);
> +
> + phy_resolve_aneg_linkmode(phydev);
> + } else if (phydev->autoneg == AUTONEG_DISABLE) {
> + int bmcr = phy_read(phydev, MII_BMCR);
> +
> + if (bmcr < 0)
> + return bmcr;
> +
> + if (bmcr & BMCR_FULLDPLX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(genphy_c37_read_status);
> +
> /**
> * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
> * @phydev: target phy_device struct
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 462b90b73f93..81a2921512ee 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
> int genphy_resume(struct phy_device *phydev);
> int genphy_loopback(struct phy_device *phydev, bool enable);
> int genphy_soft_reset(struct phy_device *phydev);
> +
> +/* Clause 37 */
> +int genphy_c37_config_aneg(struct phy_device *phydev);
> +int genphy_c37_read_status(struct phy_device *phydev);
> +
> static inline int genphy_no_soft_reset(struct phy_device *phydev)
> {
> return 0;
>

2019-08-19 23:17:55

by René van Dorst

[permalink] [raw]
Subject: Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation

Hi Tao,

Quoting Tao Ren <[email protected]>:

> On 8/11/19 4:40 PM, Tao Ren wrote:
>> From: Heiner Kallweit <[email protected]>
>>
>> This patch adds support for clause 37 1000Base-X auto-negotiation.
>>
>> Signed-off-by: Heiner Kallweit <[email protected]>
>> Signed-off-by: Tao Ren <[email protected]>
>
> A kind reminder: could someone help to review the patch when you
> have bandwidth?
>

FWIW: I have a similar setup with my device. MAC -> PHY -> SFP cage.
PHY is a Qualcomm at8031 and is used as a RGMII-to-SerDes converter.
SerDes only support 100Base-FX and 1000Base-X in this converter mode.
PHY also supports a RJ45 port but that is not wired on my device.

I converted [0] at803x driver to make use of the PHYLINK API for SFP cage and
also of these new c37 functions.

In autoneg on and off, it detects the link and can ping a host on the network.
Tested with 1gbit BiDi optical(1000Base-X) and RJ45 module(SGMII).
Both work and both devices detects unplug and plug-in of the cable.

output of ethtool:

Autoneg on
Settings for lan5:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 1000baseX/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 7
Transceiver: internal
Auto-negotiation: on
Supports Wake-on: g
Wake-on: d
Link detected: yes

Autoneg off
Settings for lan5:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 1000baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: No
Advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 7
Transceiver: internal
Auto-negotiation: off
Supports Wake-on: g
Wake-on: d
Link detected: yes

Tested-by: René van Dorst <[email protected]>

Greats,

René

[0]
https://github.com/vDorst/linux-1/blob/1d8cb01bc8047bda94c076676e47b09d2f31069d/drivers/net/phy/at803x.c

>
> Cheers,
>
> Tao
>
>> ---
>> Changes in v7:
>> - Update "if (AUTONEG_ENABLE != phydev->autoneg)" to
>> "if (phydev->autoneg != AUTONEG_ENABLE)" so checkpatch.pl is happy.
>> Changes in v6:
>> - add "Signed-off-by: Tao Ren <[email protected]>"
>> Changes in v1-v5:
>> - nothing changed. It's given v5 just to align with the version of
>> patch series.
>>
>> drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
>> include/linux/phy.h | 5 ++
>> 2 files changed, 144 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 252a712d1b2b..301a794b2963 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct
>> phy_device *phydev)
>> return changed;
>> }
>>
>> +/**
>> + * genphy_c37_config_advert - sanitize and advertise
>> auto-negotiation parameters
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Writes MII_ADVERTISE with the appropriate values,
>> + * after sanitizing the values to make sure we only advertise
>> + * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
>> + * hasn't changed, and > 0 if it has changed. This function is intended
>> + * for Clause 37 1000Base-X mode.
>> + */
>> +static int genphy_c37_config_advert(struct phy_device *phydev)
>> +{
>> + u16 adv = 0;
>> +
>> + /* Only allow advertising what this PHY supports */
>> + linkmode_and(phydev->advertising, phydev->advertising,
>> + phydev->supported);
>> +
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XFULL;
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XPAUSE;
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XPSE_ASYM;
>> +
>> + return phy_modify_changed(phydev, MII_ADVERTISE,
>> + ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
>> + ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
>> + adv);
>> +}
>> +
>> /**
>> * genphy_config_eee_advert - disable unwanted eee mode advertisement
>> * @phydev: target phy_device struct
>> @@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_config_aneg);
>>
>> +/**
>> + * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: If auto-negotiation is enabled, we configure the
>> + * advertising, and then restart auto-negotiation. If it is not
>> + * enabled, then we write the BMCR. This function is intended
>> + * for use with Clause 37 1000Base-X mode.
>> + */
>> +int genphy_c37_config_aneg(struct phy_device *phydev)
>> +{
>> + int err, changed;
>> +
>> + if (phydev->autoneg != AUTONEG_ENABLE)
>> + return genphy_setup_forced(phydev);
>> +
>> + err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
>> + BMCR_SPEED1000);
>> + if (err)
>> + return err;
>> +
>> + changed = genphy_c37_config_advert(phydev);
>> + if (changed < 0) /* error */
>> + return changed;
>> +
>> + if (!changed) {
>> + /* Advertisement hasn't changed, but maybe aneg was never on to
>> + * begin with? Or maybe phy was isolated?
>> + */
>> + int ctl = phy_read(phydev, MII_BMCR);
>> +
>> + if (ctl < 0)
>> + return ctl;
>> +
>> + if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
>> + changed = 1; /* do restart aneg */
>> + }
>> +
>> + /* Only restart aneg if we are advertising something different
>> + * than we were before.
>> + */
>> + if (changed > 0)
>> + return genphy_restart_aneg(phydev);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(genphy_c37_config_aneg);
>> +
>> /**
>> * genphy_aneg_done - return auto-negotiation status
>> * @phydev: target phy_device struct
>> @@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_read_status);
>>
>> +/**
>> + * genphy_c37_read_status - check the link status and update
>> current link state
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Check the link, then figure out the current state
>> + * by comparing what we advertise with what the link partner
>> + * advertises. This function is for Clause 37 1000Base-X mode.
>> + */
>> +int genphy_c37_read_status(struct phy_device *phydev)
>> +{
>> + int lpa, 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->duplex = DUPLEX_UNKNOWN;
>> + phydev->pause = 0;
>> + phydev->asym_pause = 0;
>> +
>> + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
>> + lpa = phy_read(phydev, MII_LPA);
>> + if (lpa < 0)
>> + return lpa;
>> +
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>> + phydev->lp_advertising, lpa & LPA_LPACK);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
>> + phydev->lp_advertising, lpa & LPA_1000XFULL);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>> + phydev->lp_advertising, lpa & LPA_1000XPAUSE);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> + phydev->lp_advertising,
>> + lpa & LPA_1000XPAUSE_ASYM);
>> +
>> + phy_resolve_aneg_linkmode(phydev);
>> + } else if (phydev->autoneg == AUTONEG_DISABLE) {
>> + int bmcr = phy_read(phydev, MII_BMCR);
>> +
>> + if (bmcr < 0)
>> + return bmcr;
>> +
>> + if (bmcr & BMCR_FULLDPLX)
>> + phydev->duplex = DUPLEX_FULL;
>> + else
>> + phydev->duplex = DUPLEX_HALF;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(genphy_c37_read_status);
>> +
>> /**
>> * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
>> * @phydev: target phy_device struct
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 462b90b73f93..81a2921512ee 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
>> int genphy_resume(struct phy_device *phydev);
>> int genphy_loopback(struct phy_device *phydev, bool enable);
>> int genphy_soft_reset(struct phy_device *phydev);
>> +
>> +/* Clause 37 */
>> +int genphy_c37_config_aneg(struct phy_device *phydev);
>> +int genphy_c37_read_status(struct phy_device *phydev);
>> +
>> static inline int genphy_no_soft_reset(struct phy_device *phydev)
>> {
>> return 0;
>>



2019-08-19 23:25:07

by Tao Ren

[permalink] [raw]
Subject: Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation

On 8/19/19 4:15 PM, René van Dorst wrote:
> Hi Tao,
>
> Quoting Tao Ren <[email protected]>:
>
>> On 8/11/19 4:40 PM, Tao Ren wrote:
>>> From: Heiner Kallweit <[email protected]>
>>>
>>> This patch adds support for clause 37 1000Base-X auto-negotiation.
>>>
>>> Signed-off-by: Heiner Kallweit <[email protected]>
>>> Signed-off-by: Tao Ren <[email protected]>
>>
>> A kind reminder: could someone help to review the patch when you have bandwidth?
>>
>
> FWIW: I have a similar setup with my device. MAC -> PHY -> SFP cage.
> PHY is a Qualcomm at8031 and is used as a RGMII-to-SerDes converter.
> SerDes only support 100Base-FX and 1000Base-X in this converter mode.
> PHY also supports a RJ45 port but that is not wired on my device.
>
> I converted [0] at803x driver to make use of the PHYLINK API for SFP cage and
> also of these new c37 functions.
>
> In autoneg on and off, it detects the link and can ping a host on the network.
> Tested with 1gbit BiDi optical(1000Base-X) and RJ45 module(SGMII).
> Both work and both devices detects unplug and plug-in of the cable.
>
> output of ethtool:
>
> Autoneg on
> Settings for lan5:
>         Supported ports: [ TP MII ]
>         Supported link modes:   100baseT/Half 100baseT/Full
>                                 1000baseT/Full
>                                 1000baseX/Full
>         Supported pause frame use: Symmetric Receive-only
>         Supports auto-negotiation: Yes
>         Supported FEC modes: Not reported
>         Advertised link modes:  100baseT/Half 100baseT/Full
>                                 1000baseT/Full
>                                 1000baseX/Full
>         Advertised pause frame use: Symmetric Receive-only
>         Advertised auto-negotiation: Yes
>         Advertised FEC modes: Not reported
>         Link partner advertised link modes:  1000baseX/Full
>         Link partner advertised pause frame use: Symmetric Receive-only
>         Link partner advertised auto-negotiation: Yes
>         Link partner advertised FEC modes: Not reported
>         Speed: 1000Mb/s
>         Duplex: Full
>         Port: MII
>         PHYAD: 7
>         Transceiver: internal
>         Auto-negotiation: on
>         Supports Wake-on: g
>         Wake-on: d
>         Link detected: yes
>
> Autoneg off
> Settings for lan5:
>         Supported ports: [ TP MII ]
>         Supported link modes:   100baseT/Half 100baseT/Full
>                                 1000baseT/Full
>                                 1000baseX/Full
>         Supported pause frame use: Symmetric Receive-only
>         Supports auto-negotiation: Yes
>         Supported FEC modes: Not reported
>         Advertised link modes:  1000baseT/Full
>         Advertised pause frame use: Symmetric Receive-only
>         Advertised auto-negotiation: No
>         Advertised FEC modes: Not reported
>         Speed: 1000Mb/s
>         Duplex: Full
>         Port: MII
>         PHYAD: 7
>         Transceiver: internal
>         Auto-negotiation: off
>         Supports Wake-on: g
>         Wake-on: d
>         Link detected: yes
>
> Tested-by: René van Dorst <[email protected]>
>
> Greats,
>
> René

Great. Thank you for the testing, René.


Cheers,

Tao