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: <2954e50a44a14ab48b3345fa471f4e60@huawei.com>
Date:   Fri, 1 Mar 2019 20:22:01 +0000
From:   Salil Mehta <salil.mehta@...wei.com>
To:     Michal Vokáč <michal.vokac@...ft.com>,
        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>,
        "lipeng (Y)" <lipeng321@...wei.com>,
        "shenjian (K)" <shenjian15@...wei.com>,
        Linuxarm <linuxarm@...wei.com>
Subject: RE: Question about setting speed and duplex failed after
 auto-negotiation disabled on marvell phy

(+) LinuxArm

> From: Michal Vokáč [mailto:michal.vokac@...ft.com]
> Sent: Friday, March 1, 2019 12:58 PM
> To: liweihang <liweihang@...ilicon.com>
> Cc: netdev@...r.kernel.org; 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/
> >