Messages in this thread | | | Subject | Re: [PATCH net-next v3] net: phylink: Add helpers for c22 registers without MDIO | From | Sean Anderson <> | Date | Thu, 9 Dec 2021 17:26:16 -0500 |
| |
ping?
On 11/19/21 10:58 AM, Sean Anderson wrote: > Some devices expose memory-mapped c22-compliant PHYs. Because these > devices do not have an MDIO bus, we cannot use the existing helpers. > Refactor the existing helpers to allow supplying the values for c22 > registers directly, instead of using MDIO to access them. Only get_state > and set_advertisement are converted, since they contain the most complex > logic. Because set_advertisement is never actually used outside > phylink_mii_c22_pcs_config, move the MDIO-writing part into that > function. Because some modes do not need the advertisement register set > at all, we use -EINVAL for this purpose. > > Additionally, a new function phylink_pcs_enable_an is provided to > determine whether to enable autonegotiation. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > This series was originally submitted as [1]. Although does it not > include its intended user (macb), I have submitted it separately at the > behest of Russell. > > [1] https://lore.kernel.org/netdev/YVtypfZJfivfDnu7@lunn.ch/T/#m50877e4daf344ac0b5efced38c79246ad2b9cb6e > > Changes in v3: > - Change adv type from u16 to int > > Changes in v2: > - Add phylink_pcs_enable_an > - Also remove set_advertisement > - Use mdiobus_modify_changed > > drivers/net/phy/phylink.c | 120 +++++++++++++++++++++----------------- > include/linux/phylink.h | 7 ++- > 2 files changed, 72 insertions(+), 55 deletions(-) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index 33462fdc7add..428f9dc02d0e 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -2813,6 +2813,52 @@ void phylink_decode_usxgmii_word(struct phylink_link_state *state, > } > EXPORT_SYMBOL_GPL(phylink_decode_usxgmii_word); > > +/** > + * phylink_mii_c22_pcs_decode_state() - Decode MAC PCS state from MII registers > + * @state: a pointer to a &struct phylink_link_state. > + * @bmsr: The value of the %MII_BMSR register > + * @lpa: The value of the %MII_LPA register > + * > + * Helper for MAC PCS supporting the 802.3 clause 22 register set for > + * clause 37 negotiation and/or SGMII control. > + * > + * Parse the Clause 37 or Cisco SGMII link partner negotiation word into > + * the phylink @state structure. This is suitable to be used for implementing > + * the mac_pcs_get_state() member of the struct phylink_mac_ops structure if > + * accessing @bmsr and @lpa cannot be done with MDIO directly. > + */ > +void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state, > + u16 bmsr, u16 lpa) > +{ > + state->link = !!(bmsr & BMSR_LSTATUS); > + state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE); > + /* If there is no link or autonegotiation is disabled, the LP advertisement > + * data is not meaningful, so don't go any further. > + */ > + if (!state->link || !state->an_enabled) > + return; > + > + switch (state->interface) { > + case PHY_INTERFACE_MODE_1000BASEX: > + phylink_decode_c37_word(state, lpa, SPEED_1000); > + break; > + > + case PHY_INTERFACE_MODE_2500BASEX: > + phylink_decode_c37_word(state, lpa, SPEED_2500); > + break; > + > + case PHY_INTERFACE_MODE_SGMII: > + case PHY_INTERFACE_MODE_QSGMII: > + phylink_decode_sgmii_word(state, lpa); > + break; > + > + default: > + state->link = false; > + break; > + } > +} > +EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_decode_state); > + > /** > * phylink_mii_c22_pcs_get_state() - read the MAC PCS state > * @pcs: a pointer to a &struct mdio_device. > @@ -2839,55 +2885,26 @@ void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs, > return; > } > > - state->link = !!(bmsr & BMSR_LSTATUS); > - state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE); > - /* If there is no link or autonegotiation is disabled, the LP advertisement > - * data is not meaningful, so don't go any further. > - */ > - if (!state->link || !state->an_enabled) > - return; > - > - switch (state->interface) { > - case PHY_INTERFACE_MODE_1000BASEX: > - phylink_decode_c37_word(state, lpa, SPEED_1000); > - break; > - > - case PHY_INTERFACE_MODE_2500BASEX: > - phylink_decode_c37_word(state, lpa, SPEED_2500); > - break; > - > - case PHY_INTERFACE_MODE_SGMII: > - case PHY_INTERFACE_MODE_QSGMII: > - phylink_decode_sgmii_word(state, lpa); > - break; > - > - default: > - state->link = false; > - break; > - } > + phylink_mii_c22_pcs_decode_state(state, bmsr, lpa); > } > EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_get_state); > > /** > - * phylink_mii_c22_pcs_set_advertisement() - configure the clause 37 PCS > + * phylink_mii_c22_pcs_encode_advertisement() - configure the clause 37 PCS > * advertisement > - * @pcs: a pointer to a &struct mdio_device. > * @interface: the PHY interface mode being configured > * @advertising: the ethtool advertisement mask > * > * Helper for MAC PCS supporting the 802.3 clause 22 register set for > * clause 37 negotiation and/or SGMII control. > * > - * Configure the clause 37 PCS advertisement as specified by @state. This > - * does not trigger a renegotiation; phylink will do that via the > - * mac_an_restart() method of the struct phylink_mac_ops structure. > + * Encode the clause 37 PCS advertisement as specified by @interface and > + * @advertising. > * > - * Returns negative error code on failure to configure the advertisement, > - * zero if no change has been made, or one if the advertisement has changed. > + * Return: The new value for @adv, or ``-EINVAL`` if it should not be changed. > */ > -int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs, > - phy_interface_t interface, > - const unsigned long *advertising) > +int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface, > + const unsigned long *advertising) > { > u16 adv; > > @@ -2901,18 +2918,15 @@ int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs, > if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, > advertising)) > adv |= ADVERTISE_1000XPSE_ASYM; > - > - return mdiodev_modify_changed(pcs, MII_ADVERTISE, 0xffff, adv); > - > + return adv; > case PHY_INTERFACE_MODE_SGMII: > - return mdiodev_modify_changed(pcs, MII_ADVERTISE, 0xffff, 0x0001); > - > + return 0x0001; > default: > /* Nothing to do for other modes */ > - return 0; > + return -EINVAL; > } > } > -EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_set_advertisement); > +EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_encode_advertisement); > > /** > * phylink_mii_c22_pcs_config() - configure clause 22 PCS > @@ -2930,16 +2944,18 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode, > phy_interface_t interface, > const unsigned long *advertising) > { > - bool changed; > + bool changed = 0; > u16 bmcr; > - int ret; > + int ret, adv; > > - ret = phylink_mii_c22_pcs_set_advertisement(pcs, interface, > - advertising); > - if (ret < 0) > - return ret; > - > - changed = ret > 0; > + adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising); > + if (adv >= 0) { > + ret = mdiobus_modify_changed(pcs->bus, pcs->addr, > + MII_ADVERTISE, 0xffff, adv); > + if (ret < 0) > + return ret; > + changed = ret; > + } > > /* Ensure ISOLATE bit is disabled */ > if (mode == MLO_AN_INBAND && > @@ -2952,7 +2968,7 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode, > if (ret < 0) > return ret; > > - return changed ? 1 : 0; > + return changed; > } > EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config); > > diff --git a/include/linux/phylink.h b/include/linux/phylink.h > index 3563820a1765..01224235df0f 100644 > --- a/include/linux/phylink.h > +++ b/include/linux/phylink.h > @@ -527,11 +527,12 @@ void phylink_set_port_modes(unsigned long *bits); > void phylink_set_10g_modes(unsigned long *mask); > void phylink_helper_basex_speed(struct phylink_link_state *state); > > +void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state, > + u16 bmsr, u16 lpa); > void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs, > struct phylink_link_state *state); > -int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs, > - phy_interface_t interface, > - const unsigned long *advertising); > +int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface, > + const unsigned long *advertising); > int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode, > phy_interface_t interface, > const unsigned long *advertising); >
| |