netdev - Re: [net PATCH 1/1] net: phy: fix phy link up when limiting speed via device tree

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGVrzcZcs5n7A+VF3uextDgONS_UNLYDianw=NwH62Bx-6ZEyw@mail.gmail.com>
Date:	Thu, 25 Jun 2015 10:31:53 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Mugunthan V N <mugunthanvnm@...com>
Cc:	netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>
Subject: Re: [net PATCH 1/1] net: phy: fix phy link up when limiting speed via
 device tree

2015-06-25 9:51 GMT-07:00 Mugunthan V N <mugunthanvnm@...com>:
> When limiting phy link speed using "max-speed" to 100mbps or less on a
> giga bit phy, phy never completes auto negotiation and phy state
> machine is held in PHY_AN. Fixing this issue by comparing the giga
> bit advertise though phydev->supported doesn't have it but phy has
> BMSR_ESTATEN set. So that auto negotiation is restarted as old and
> new advertise are different and link comes up fine.

This looks valid, however, I am curious why this part of the code:

        oldadv = adv;
        adv &= ~(ADVERTISE_ALL | ADVERTISE_100BASE4 | ADVERTISE_PAUSE_CAP |
                 ADVERTISE_PAUSE_ASYM);
        adv |= ethtool_adv_to_mii_adv_t(advertise);

        if (adv != oldadv) {
                err = phy_write(phydev, MII_ADVERTISE, adv);

                if (err < 0)
                        return err;
                changed = 1;
        }

Is not flagging this as a change already?

>
> Signed-off-by: Mugunthan V N <mugunthanvnm@...com>
> ---
>  drivers/net/phy/phy_device.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index bdfe51f..d551df6 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -796,10 +796,11 @@ static int genphy_config_advert(struct phy_device *phydev)
>         if (phydev->supported & (SUPPORTED_1000baseT_Half |
>                                  SUPPORTED_1000baseT_Full)) {
>                 adv |= ethtool_adv_to_mii_ctrl1000_t(advertise);
> -               if (adv != oldadv)
> -                       changed = 1;
>         }
>
> +       if (adv != oldadv)
> +               changed = 1;
> +
>         err = phy_write(phydev, MII_CTRL1000, adv);
>         if (err < 0)
>                 return err;
> --
> 2.4.4.409.g5b1d901
>



-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html