On 15.02.2019 14:57, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote:
>> We have the settings array of modes which is sorted based on aneg
>> priority. Instead of checking each mode manually let's simply iterate
>> over the sorted settings.
>>
>> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
>> ---
>> drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
>> 1 file changed, 7 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index cdea028d1..5d43106fe 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
>> void phy_resolve_aneg_linkmode(struct phy_device *phydev)
>> {
>> __ETHTOOL_DECLARE_LINK_MODE_MASK(common);
>> + int i;
>>
>> linkmode_and(common, phydev->lp_advertising, phydev->advertising);
>>
>> - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
>> - phydev->speed = SPEED_10000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_5000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_2500;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_1000;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_1000;
>> - phydev->duplex = DUPLEX_HALF;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_100;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_100;
>> - phydev->duplex = DUPLEX_HALF;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
>> - common)) {
>> - phydev->speed = SPEED_10;
>> - phydev->duplex = DUPLEX_FULL;
>> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
>> - common)) {
>> - phydev->speed = SPEED_10;
>> - phydev->duplex = DUPLEX_HALF;
>> - }
>> + for (i = 0; i < ARRAY_SIZE(settings); i++)
>> + if (test_bit(settings[i].bit, common)) {
>> + phydev->speed = settings[i].speed;
>> + phydev->duplex = settings[i].duplex;
>> + break;
>> + }
>
> Hi Heiner
>
> Nice simplification.
>
> I just have one thought about this. The original code was limited to
> baseT. The new code could in theory return a non BaseT speed. For that
> to happen, it would require that phydev->lp_advertising and
> phydev->advertising contain a non BaseT link mode? Is that possible?
> I don't think it is.
>
Currently we set only BaseT modes because that's what the clause 45
standard registers offer. However drivers may come that use vendor
registers for e.g. backplane auto-negotiation (IIRC clause 73).
Now it's even better because this function shouldn't be (and doesn't
have to be) limited to a specific physical link type.
> Andrew
>
Heiner
