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
| ||
|
Message-ID: <20190121201715.GA20277@lunn.ch> Date: Mon, 21 Jan 2019 21:17:15 +0100 From: Andrew Lunn <andrew@...n.ch> To: Maxime Chevallier <maxime.chevallier@...tlin.com> Cc: davem@...emloft.net, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, linux-arm-kernel@...ts.infradead.org, Antoine Tenart <antoine.tenart@...tlin.com>, thomas.petazzoni@...tlin.com, gregory.clement@...tlin.com, miquel.raynal@...tlin.com, nadavh@...vell.com, stefanc@...vell.com, mw@...ihalf.com Subject: Re: [PATCH net-next 4/7] net: phy: marvell10g: Add support for 2.5GBASET and 5GBASET > @@ -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
Powered by blists - more mailing lists