> @@ -264,8 +265,10 @@ static int mv3310_config_init(struct phy_device *phydev)
> if (ret)
> return ret;
>
> - linkmode_and(phydev->advertising, phydev->advertising,
> - phydev->supported);
> + /* Make sure we advertise all the supported modes, and not just the
> + * default one specified in the driver's .features.
> + */
> + linkmode_copy(phydev->advertising, phydev->supported);
Hi Maxime
What Russell is referring to is:
static int phy_probe(struct device *dev)
{
..
/* Start out supporting everything. Eventually,
* a controller will attach, and may modify one
* or both of these values
*/
linkmode_copy(phydev->supported, phydrv->features);
of_set_phy_supported(phydev);
linkmode_copy(phydev->advertising, phydev->supported);
/* Get the EEE modes we want to prohibit. We will ask
* the PHY stop advertising these mode later on
*/
of_set_phy_eee_broken(phydev);
/* The Pause Frame bits indicate that the PHY can support passing
* pause frames. During autonegotiation, the PHYs will determine if
* they should allow pause frames to pass. The MAC driver should then
* use that result to determine whether to enable flow control via
* pause frames.
*
* Normally, PHY drivers should not set the Pause bits, and instead
* allow phylib to do that. However, there may be some situations
* (e.g. hardware erratum) where the driver wants to set only one
* of these bits.
*/
if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydrv->features) ||
test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydrv->features)) {
linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
phydev->supported);
linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->supported);
if (test_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydrv->features))
linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
phydev->supported);
if (test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydrv->features))
linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->supported);
} else {
linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
phydev->supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
phydev->supported);
}
So by doing a copy of supported into advertising, you can stomping
over any restrictions applied via of_set_phy_supported(),
of_set_phy_eee_broken(phydev), and any pause control settings which
might of happened.
What might make sense here is that a PHY driver can replace its
.features member at run time, in its config_init() call. The core then
needs to perform these evaluations. So i'm guessing we need to split
this code out of probe() and move it into phy_init_hw()?
Heiner, you know this code better than anybody. What do you think?
Andrew
