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] [day] [month] [year] [list]

Message-ID: <B82435381E3B2943AA4D2826ADEF0B3A014E243E@DGGEMM506-MBX.china.huawei.com>
Date:   Mon, 4 Mar 2019 07:31:22 +0000
From:   liweihang <liweihang@...ilicon.com>
To:     Michal Vokáč <michal.vokac@...ft.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>,
        Linuxarm <linuxarm@...wei.com>
Subject: RE: Question about setting speed and duplex failed after
 auto-negotiation disabled on marvell phy



> -----Original Message-----
> From: Michal Vokáč [mailto:michal.vokac@...ft.com]
> Sent: Friday, March 01, 2019 8: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.
> 

Thanks for your advice, I'm using v5.0-rc1, and I tested on net-next and got
an same result. 


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