also it's suspicions that in m88e1116r_config_init() delay is present after first soft_reset() and missed after second.
Maxim. ср, 20 мар. 2019 г. в 21:17, Heiner Kallweit <hkallwe...@gmail.com>: > > 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); > >> > > > -- Best regards, Maxim Uvarov
