Skip to content

Commit

Permalink
Merge branch 'net-add-negotiation-of-in-band-capabilities'
Browse files Browse the repository at this point in the history
Russell King says:

====================
net: add negotiation of in-band capabilities

This is a repost without RFC for this series, shrunk down to 13 patches
by removing the non-Marvell PCS.

Phylink's handling of in-band has been deficient for a long time, and
people keep hitting problems with it. Notably, situations with the way-
to-late standardized 2500Base-X and whether that should or should not
have in-band enabled. We have also been carrying a hack in the form of
phylink_phy_no_inband() for a PHY that has been used on a SFP module,
but has no in-band capabilities, not even for SGMII.

When phylink is trying to operate in in-band mode, this series will look
at the capabilities of the MAC-side PCS and PHY, and work out whether
in-band can or should be used, programming the PHY as appropriate. This
includes in-band bypass mode at the PHY.

We don't... yet... support bypass on the MAC side PCS, because that
requires yet more complexity.

Patch 1 passes struct phylink and struct phylink_pcs into
phylink_pcs_neg_mode() so we can look at more state in this function in
a future patch.

Patch 2 splits "cur_link_an_mode" (the MLO_AN_* mode) into two separate
purposes - a requested and an active mode. The active mode is the one
we will be using for the MAC, which becomes dependent on the result of
in-band negotiation.

Patch 3 adds debug to phylink_major_config() so we can see what is going
on with the requested and active AN modes.

Patch 4 adds to phylib a method to get the in-band capabilities of the
PHY from phylib. Patches 5 and 6 add implementations for BCM84881 and
some Marvell PHYs found on SFPs.

Patch 7 adds to phylib a method to configure the PHY in-band signalling,
and patch 8 implements it for those Marvell PHYs that support the method
in patch 4.

Patch 9 does the same as patch 4 but for the MAC-side PCS, with patches
10 and 11 adding support to Marvell NETA and PP2.

Patch 12 adds the code to phylink_pcs_neg_mode() which looks at the
capabilities, and works out whether to use in-band or out-band mode for
driving the link between the MAC PCS and PHY.

Patch 13 removes the phylink_phy_no_inband() hack now that we are
publishing the in-band capabilities from the BCM84881 PHY driver.

Three more PCS, omitted from this series due to the limit of 15 patches,
will be sent once this has been merged.
====================

Link: https://patch.msgid.link/Z08kCwxdkU4n2V6x@shell.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Dec 5, 2024
2 parents 4485043 + 77ac9a8 commit f029c40
Show file tree
Hide file tree
Showing 8 changed files with 474 additions and 92 deletions.
27 changes: 17 additions & 10 deletions drivers/net/ethernet/marvell/mvneta.c
Original file line number Diff line number Diff line change
Expand Up @@ -3960,20 +3960,27 @@ static struct mvneta_port *mvneta_pcs_to_port(struct phylink_pcs *pcs)
return container_of(pcs, struct mvneta_port, phylink_pcs);
}

static int mvneta_pcs_validate(struct phylink_pcs *pcs,
unsigned long *supported,
const struct phylink_link_state *state)
static unsigned int mvneta_pcs_inband_caps(struct phylink_pcs *pcs,
phy_interface_t interface)
{
/* We only support QSGMII, SGMII, 802.3z and RGMII modes.
* When in 802.3z mode, we must have AN enabled:
/* When operating in an 802.3z mode, we must have AN enabled:
* "Bit 2 Field InBandAnEn In-band Auto-Negotiation enable. ...
* When <PortType> = 1 (1000BASE-X) this field must be set to 1."
* Therefore, inband is "required".
*/
if (phy_interface_mode_is_8023z(state->interface) &&
!phylink_test(state->advertising, Autoneg))
return -EINVAL;
if (phy_interface_mode_is_8023z(interface))
return LINK_INBAND_ENABLE;

return 0;
/* QSGMII, SGMII and RGMII can be configured to use inband
* signalling of the AN result. Indicate these as "possible".
*/
if (interface == PHY_INTERFACE_MODE_SGMII ||
interface == PHY_INTERFACE_MODE_QSGMII ||
phy_interface_mode_is_rgmii(interface))
return LINK_INBAND_DISABLE | LINK_INBAND_ENABLE;

/* For any other modes, indicate that inband is not supported. */
return LINK_INBAND_DISABLE;
}

static void mvneta_pcs_get_state(struct phylink_pcs *pcs,
Expand Down Expand Up @@ -4071,7 +4078,7 @@ static void mvneta_pcs_an_restart(struct phylink_pcs *pcs)
}

static const struct phylink_pcs_ops mvneta_phylink_pcs_ops = {
.pcs_validate = mvneta_pcs_validate,
.pcs_inband_caps = mvneta_pcs_inband_caps,
.pcs_get_state = mvneta_pcs_get_state,
.pcs_config = mvneta_pcs_config,
.pcs_an_restart = mvneta_pcs_an_restart,
Expand Down
25 changes: 16 additions & 9 deletions drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -6224,19 +6224,26 @@ static const struct phylink_pcs_ops mvpp2_phylink_xlg_pcs_ops = {
.pcs_config = mvpp2_xlg_pcs_config,
};

static int mvpp2_gmac_pcs_validate(struct phylink_pcs *pcs,
unsigned long *supported,
const struct phylink_link_state *state)
static unsigned int mvpp2_gmac_pcs_inband_caps(struct phylink_pcs *pcs,
phy_interface_t interface)
{
/* When in 802.3z mode, we must have AN enabled:
/* When operating in an 802.3z mode, we must have AN enabled:
* Bit 2 Field InBandAnEn In-band Auto-Negotiation enable. ...
* When <PortType> = 1 (1000BASE-X) this field must be set to 1.
* Therefore, inband is "required".
*/
if (phy_interface_mode_is_8023z(state->interface) &&
!phylink_test(state->advertising, Autoneg))
return -EINVAL;
if (phy_interface_mode_is_8023z(interface))
return LINK_INBAND_ENABLE;

return 0;
/* SGMII and RGMII can be configured to use inband signalling of the
* AN result. Indicate these as "possible".
*/
if (interface == PHY_INTERFACE_MODE_SGMII ||
phy_interface_mode_is_rgmii(interface))
return LINK_INBAND_DISABLE | LINK_INBAND_ENABLE;

/* For any other modes, indicate that inband is not supported. */
return LINK_INBAND_DISABLE;
}

static void mvpp2_gmac_pcs_get_state(struct phylink_pcs *pcs,
Expand Down Expand Up @@ -6343,7 +6350,7 @@ static void mvpp2_gmac_pcs_an_restart(struct phylink_pcs *pcs)
}

static const struct phylink_pcs_ops mvpp2_phylink_gmac_pcs_ops = {
.pcs_validate = mvpp2_gmac_pcs_validate,
.pcs_inband_caps = mvpp2_gmac_pcs_inband_caps,
.pcs_get_state = mvpp2_gmac_pcs_get_state,
.pcs_config = mvpp2_gmac_pcs_config,
.pcs_an_restart = mvpp2_gmac_pcs_an_restart,
Expand Down
10 changes: 10 additions & 0 deletions drivers/net/phy/bcm84881.c
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,21 @@ static int bcm84881_read_status(struct phy_device *phydev)
return genphy_c45_read_mdix(phydev);
}

/* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
* or 802.3z control word, so inband will not work.
*/
static unsigned int bcm84881_inband_caps(struct phy_device *phydev,
phy_interface_t interface)
{
return LINK_INBAND_DISABLE;
}

static struct phy_driver bcm84881_drivers[] = {
{
.phy_id = 0xae025150,
.phy_id_mask = 0xfffffff0,
.name = "Broadcom BCM84881",
.inband_caps = bcm84881_inband_caps,
.config_init = bcm84881_config_init,
.probe = bcm84881_probe,
.get_features = bcm84881_get_features,
Expand Down
48 changes: 48 additions & 0 deletions drivers/net/phy/marvell.c
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,48 @@ static int marvell_config_aneg_fiber(struct phy_device *phydev)
return genphy_check_and_restart_aneg(phydev, changed);
}

static unsigned int m88e1111_inband_caps(struct phy_device *phydev,
phy_interface_t interface)
{
/* In 1000base-X and SGMII modes, the inband mode can be changed
* through the Fibre page BMCR ANENABLE bit.
*/
if (interface == PHY_INTERFACE_MODE_1000BASEX ||
interface == PHY_INTERFACE_MODE_SGMII)
return LINK_INBAND_DISABLE | LINK_INBAND_ENABLE |
LINK_INBAND_BYPASS;

return 0;
}

static int m88e1111_config_inband(struct phy_device *phydev, unsigned int modes)
{
u16 extsr, bmcr;
int err;

if (phydev->interface != PHY_INTERFACE_MODE_1000BASEX &&
phydev->interface != PHY_INTERFACE_MODE_SGMII)
return -EINVAL;

if (modes == LINK_INBAND_BYPASS)
extsr = MII_M1111_HWCFG_SERIAL_AN_BYPASS;
else
extsr = 0;

if (modes == LINK_INBAND_DISABLE)
bmcr = 0;
else
bmcr = BMCR_ANENABLE;

err = phy_modify(phydev, MII_M1111_PHY_EXT_SR,
MII_M1111_HWCFG_SERIAL_AN_BYPASS, extsr);
if (err < 0)
return extsr;

return phy_modify_paged(phydev, MII_MARVELL_FIBER_PAGE, MII_BMCR,
BMCR_ANENABLE, bmcr);
}

static int m88e1111_config_aneg(struct phy_device *phydev)
{
int extsr = phy_read(phydev, MII_M1111_PHY_EXT_SR);
Expand Down Expand Up @@ -3677,6 +3719,8 @@ static struct phy_driver marvell_drivers[] = {
.name = "Marvell 88E1112",
/* PHY_GBIT_FEATURES */
.probe = marvell_probe,
.inband_caps = m88e1111_inband_caps,
.config_inband = m88e1111_config_inband,
.config_init = m88e1112_config_init,
.config_aneg = marvell_config_aneg,
.config_intr = marvell_config_intr,
Expand All @@ -3698,6 +3742,8 @@ static struct phy_driver marvell_drivers[] = {
/* PHY_GBIT_FEATURES */
.flags = PHY_POLL_CABLE_TEST,
.probe = marvell_probe,
.inband_caps = m88e1111_inband_caps,
.config_inband = m88e1111_config_inband,
.config_init = m88e1111gbe_config_init,
.config_aneg = m88e1111_config_aneg,
.read_status = marvell_read_status,
Expand All @@ -3721,6 +3767,8 @@ static struct phy_driver marvell_drivers[] = {
.name = "Marvell 88E1111 (Finisar)",
/* PHY_GBIT_FEATURES */
.probe = marvell_probe,
.inband_caps = m88e1111_inband_caps,
.config_inband = m88e1111_config_inband,
.config_init = m88e1111gbe_config_init,
.config_aneg = m88e1111_config_aneg,
.read_status = marvell_read_status,
Expand Down
53 changes: 53 additions & 0 deletions drivers/net/phy/phy.c
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,59 @@ static int phy_check_link_status(struct phy_device *phydev)
return 0;
}

/**
* phy_inband_caps - query which in-band signalling modes are supported
* @phydev: a pointer to a &struct phy_device
* @interface: the interface mode for the PHY
*
* Returns zero if it is unknown what in-band signalling is supported by the
* PHY (e.g. because the PHY driver doesn't implement the method.) Otherwise,
* returns a bit mask of the LINK_INBAND_* values from
* &enum link_inband_signalling to describe which inband modes are supported
* by the PHY for this interface mode.
*/
unsigned int phy_inband_caps(struct phy_device *phydev,
phy_interface_t interface)
{
if (phydev->drv && phydev->drv->inband_caps)
return phydev->drv->inband_caps(phydev, interface);

return 0;
}
EXPORT_SYMBOL_GPL(phy_inband_caps);

/**
* phy_config_inband - configure the desired PHY in-band mode
* @phydev: the phy_device struct
* @modes: in-band modes to configure
*
* Description: disables, enables or enables-with-bypass in-band signalling
* between the PHY and host system.
*
* Returns: zero on success, or negative errno value.
*/
int phy_config_inband(struct phy_device *phydev, unsigned int modes)
{
int err;

if (!!(modes & LINK_INBAND_DISABLE) +
!!(modes & LINK_INBAND_ENABLE) +
!!(modes & LINK_INBAND_BYPASS) != 1)
return -EINVAL;

mutex_lock(&phydev->lock);
if (!phydev->drv)
err = -EIO;
else if (!phydev->drv->config_inband)
err = -EOPNOTSUPP;
else
err = phydev->drv->config_inband(phydev, modes);
mutex_unlock(&phydev->lock);

return err;
}
EXPORT_SYMBOL(phy_config_inband);

/**
* _phy_start_aneg - start auto-negotiation for this PHY device
* @phydev: the phy_device struct
Expand Down
Loading

0 comments on commit f029c40

Please sign in to comment.