netdev - Re: Question about setting speed and duplex failed after auto-negotiation disabled on marvell phy

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: <8b46d040-956d-c036-e547-d984c8ee937c@ysoft.com>
Date:   Fri, 1 Mar 2019 13:58:15 +0100
From:   Michal Vokáč <michal.vokac@...ft.com>
To:     liweihang <liweihang@...ilicon.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        linyunsheng <linyunsheng@...wei.com>,
        "Zhuangyuzeng (Yisen)" <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        "lipeng (Y)" <lipeng321@...wei.com>,
        "shenjian (K)" <shenjian15@...wei.com>
Subject: Re: Question about setting speed and duplex failed after
 auto-negotiation disabled on marvell phy

On 01. 03. 19 12:26, liweihang wrote:
> Hi all,
> 
> We encountered a problem that if there are two devices in kernel v5.0 with
> marvell phy 88E1510 connect with each other directly, one with autoneg on and
> the other off. The one who has disabled auto-negotiation will failed on setting
> speed and duplex mode. Their speed and duplex mode will go to 10M/half. And
> no matter what speed and duplex we set by ethtool -s ethx speed XX duplex full,
> they will go to 10M/half. If we disable auto-negotiation on both sides and set
> same speed and duplex, there speed and duplex mode will go to Unknown.
> 
> I found that m88e1121_config_aneg() has been modified by commit
> d6ab93364734 net: phy: marvell: Avoid unnecessary soft reset
> And in that patch, genphy_soft_reset() was moved below genphy_config_aneg(),
> which caused value of speed and duplex in MII_BMCR was cleared. And then they
> will go to 10M/half.
> 
> static int m88e1121_config_aneg(struct phy_device *phydev)
> {
> +       int changed = 0;
>         int err = 0;
> 
>          if (phy_interface_is_rgmii(phydev)) {
> @@ -487,15 +455,26 @@  static int m88e1121_config_aneg(struct phy_device *phydev)
>                            return err;
>         }
> 
> -        err = genphy_soft_reset(phydev);
> +       err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
>         if (err < 0)
>                  return err;
> 
> -        err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
> +       changed = err;
> +
> +       err = genphy_config_aneg(phydev);
>         if (err < 0)
>                  return err;
> 
> -        return genphy_config_aneg(phydev);
> +       if (phydev->autoneg != autoneg || changed) {
> +                /* A software reset is used to ensure a "commit" of the
> +                * changes is done.
> +                */
> +                err = genphy_soft_reset(phydev);
> +                if (err < 0)
> +                          return err;
> +       }
> +
> +       return 0;
> }
> 
> I moved genphy_soft_reset back and it tested ok. And in my opinion, if we have to do
> soft reset after genphy_config_aneg(phydev), it should be like this:
> 
>           if (phydev->autoneg != AUTONEG_ENABLE) {
>                     int bmcr;
> 
>                     bmcr = phy_read(phydev, MII_BMCR);
>                     if (bmcr < 0)
>                              return bmcr;
> 
>                     err = phy_write(phydev, MII_BMCR, bmcr | BMCR_RESET);
>                     if (err < 0)
>                            return err;
>         }
> 
> The above code has been mentioned in commit 3438634456c4
> net: phy: marvell: Use core genphy_soft_reset(), phy_write() was replaced by
> genphy_soft_reset() in that patch.
> 
> I think this issue will affect devices with Marvell PHY ID 88E1112/1111/1121/
> 1318/1240/1510/1540/1545/6390.
> 
> If there are any better info or suggestion regarding this problem, it would be very
> helpful, thanks in advance.

I am not sure what exact -rc version you are using (kernel v5.0 is not yet
released). I would recommend you to test the latest linux-next or even
net-next as there is quite a lot of new patches and fixes targeting Marvell
chips.

Best regards,
Michal
> 
> reference:
> [1] https://lore.kernel.org/patchwork/patch/991682/
> [2] https://patchwork.ozlabs.org/patch/795435/
>