| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 9 Dec 2021 17:26:16 -0500
From: Sean Anderson <sean.anderson@...o.com>
To: netdev@...r.kernel.org, Russell King <linux@...linux.org.uk>
Cc: linux-kernel@...r.kernel.org,
Horatiu Vultur <horatiu.vultur@...rochip.com>,
Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
"David S . Miller" <davem@...emloft.net>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: [PATCH net-next v3] net: phylink: Add helpers for c22 registers
without MDIO
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@...o.com>
> Reviewed-by: Russell King (Oracle) <rmk+kernel@...linux.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);
>
Powered by blists - more mailing lists