On 20.03.2019 13:22, liweihang wrote: > > >> -----Original Message----- >> From: Florian Fainelli [mailto:f.faine...@gmail.com] >> Sent: Wednesday, March 20, 2019 11:37 AM >> To: liweihang <liweih...@hisilicon.com>; Phil Reid >> <pr...@electromag.com.au>; netdev@vger.kernel.org >> Cc: Andrew Lunn <and...@lunn.ch>; David S. Miller >> <da...@davemloft.net>; dongsheng.w...@hxt-semitech.com; >> cphe...@gmail.com; clemens.gru...@pqgruber.com; hkallwe...@gmail.com; >> n...@nbd.name; harini.kata...@xilinx.com >> Subject: Re: regression from: net: phy: marvell: Avoid unnecessary soft reset >> >> >> >> On 3/19/2019 7:34 PM, liweihang wrote: >>> Hi all, >>> >>> I've met a similar issue and sent an email to discuss about it before: >>> Question about setting speed and duplex failed after auto-negotiation >>> disabled on marvell phy >>> >>> d6ab93364734 net: phy: marvell: Avoid unnecessary soft reset I >>> reverted this patch and the auto-negotiation works ok. >>> >>> Florian, could you please read my previous email and give me some advice? >> >> If you can copy the patch author on that email the next time that will help >> expedite things. >> >> So the problem seems to come from the fact that unless the BCMR_RESET bit >> is written, then m88e1121_config_aneg_rgmii_delays() has no effect, does >> that sound like what you are observing? >> >> Does the following work for you (Phil and yourself)? > > Thank you, Florian. But that didn't work for me either. I think the key > question is > as what Heiner said, some bits need to be preserved. > > The MII_BMCR contained information of speed and duplex mode, but when we > call genphy_soft_reset(), these bits will be cleared. > I think instead of
ret = phy_write(phydev, MII_BMCR, BMCR_RESET); we should use ret = phy_set_bits(phydev, MII_BMCR, BMCR_RESET); This is still in line with Clause 22 but covers more PHY's. A lot of PHY's won't be affected because they reset all BMCR bits to a default anyway. Could you please test this? If it's ok for you I'd submit a patch. >> >> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index >> 3ccba37bd6dd..6a1ea4c2042a 100644 >> --- a/drivers/net/phy/marvell.c >> +++ b/drivers/net/phy/marvell.c >> @@ -448,6 +448,10 @@ static int m88e1121_config_aneg(struct phy_device >> *phydev) >> err = m88e1121_config_aneg_rgmii_delays(phydev); >> if (err < 0) >> return err; >> + >> + err = genphy_soft_reset(phydev); >> + if (err < 0) >> + return err; >> } >> >> err = marvell_set_polarity(phydev, phydev->mdix_ctrl); >> >
