* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation [not found] <20190811234010.3673592-1-taoren@fb.com> @ 2019-08-19 22:18 ` Tao Ren 2019-08-19 23:15 ` René van Dorst 0 siblings, 1 reply; 3+ messages in thread From: Tao Ren @ 2019-08-19 22:18 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller, Vladimir Oltean, Arun Parameswaran, Justin Chen, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org On 8/11/19 4:40 PM, Tao Ren wrote: > From: Heiner Kallweit <hkallweit1@gmail.com> > > This patch adds support for clause 37 1000Base-X auto-negotiation. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > Signed-off-by: Tao Ren <taoren@fb.com> 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 <taoren@fb.com>" > 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; > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation 2019-08-19 22:18 ` [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation Tao Ren @ 2019-08-19 23:15 ` René van Dorst 2019-08-19 23:22 ` Tao Ren 0 siblings, 1 reply; 3+ messages in thread From: René van Dorst @ 2019-08-19 23:15 UTC (permalink / raw) To: Tao Ren Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller, Vladimir Oltean, Arun Parameswaran, Justin Chen, netdev, linux-kernel, openbmc Hi Tao, Quoting Tao Ren <taoren@fb.com>: > On 8/11/19 4:40 PM, Tao Ren wrote: >> From: Heiner Kallweit <hkallweit1@gmail.com> >> >> This patch adds support for clause 37 1000Base-X auto-negotiation. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> Signed-off-by: Tao Ren <taoren@fb.com> > > 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 <opensource@vdorst.com> 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 <taoren@fb.com>" >> 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; >> ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation 2019-08-19 23:15 ` René van Dorst @ 2019-08-19 23:22 ` Tao Ren 0 siblings, 0 replies; 3+ messages in thread From: Tao Ren @ 2019-08-19 23:22 UTC (permalink / raw) To: René van Dorst Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller, Vladimir Oltean, Arun Parameswaran, Justin Chen, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org On 8/19/19 4:15 PM, René van Dorst wrote: > Hi Tao, > > Quoting Tao Ren <taoren@fb.com>: > >> On 8/11/19 4:40 PM, Tao Ren wrote: >>> From: Heiner Kallweit <hkallweit1@gmail.com> >>> >>> This patch adds support for clause 37 1000Base-X auto-negotiation. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> Signed-off-by: Tao Ren <taoren@fb.com> >> >> 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 <opensource@vdorst.com> > > Greats, > > René Great. Thank you for the testing, René. Cheers, Tao ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-19 23:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190811234010.3673592-1-taoren@fb.com>
2019-08-19 22:18 ` [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation Tao Ren
2019-08-19 23:15 ` René van Dorst
2019-08-19 23:22 ` Tao Ren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox