Messages in this thread | ![/](/images/icornerl.gif) | | From | Joakim Zhang <> | Subject | RE: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered down | Date | Mon, 13 Dec 2021 04:39:15 +0000 |
| |
Hi Philippe,
> -----Original Message----- > From: Philippe Schenker <philippe.schenker@toradex.com> > Sent: 2021年12月10日 21:51 > To: festevam@gmail.com; linux@armlinux.org.uk; kuba@kernel.org; > netdev@vger.kernel.org; davem@davemloft.net; Joakim Zhang > <qiangqing.zhang@nxp.com>; andrew@lunn.ch > Cc: linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered > down > > On Tue, 2021-12-07 at 01:58 +0000, Joakim Zhang wrote: > > > > Hi Philippe, > > > > > -----Original Message----- > > > From: Philippe Schenker <philippe.schenker@toradex.com> > > > Sent: 2021年12月6日 18:13 > > > To: netdev@vger.kernel.org; Joakim Zhang > <qiangqing.zhang@nxp.com>; > > > Fabio Estevam <festevam@gmail.com>; Fugang Duan > > > <fugang.duan@nxp.com>; David S . Miller <davem@davemloft.net>; > > > Russell King <linux@armlinux.org.uk>; Andrew Lunn <andrew@lunn.ch>; > > > Jakub Kicinski <kuba@kernel.org> > > > Cc: Philippe Schenker <philippe.schenker@toradex.com>; > > > linux-kernel@vger.kernel.org > > > Subject: [RFC PATCH 0/2] Reset PHY in fec_resume if it got powered > > > down > > > > > > > > > If a hardware-design is able to control power to the Ethernet PHY > > > and relying on software to do a reset, the PHY does no longer work > > > after resuming from suspend, given the PHY does need a > > > hardware-reset. > > > The Freescale fec driver does currently control the reset-signal of > > > a phy but does not issue a reset on resume. > > > > > > On Toradex Apalis iMX8 board we do have such a design where we also > > > don't place the RC circuit to delay the reset-line by hardware. > > > Hence we fully rely on software to do so. > > > Since I didn't manage to get the needed parts of Apalis iMX8 working > > > with mainline this patchset was only tested on the downstream kernel > > > toradex_5.4-2.3.x-imx. [1] This kernel is based on NXP's release > > > imx_5.4.70_2.3.0. [2] The affected code is still the same on > > > mainline kernel, which would actually make me comfortable merging > > > this patch, but due to this fact I'm sending this as RFC maybe > > > someone else is able to test this code. > > > > > > This patchset aims to change the behavior by resetting the ethernet > > > PHY in fec_resume. A short description of the patches can be found > > > below, please find a detailed description in the commit-messages of > > > the respective patches. > > > > > > [PATCH 2/2] net: fec: reset phy in resume if it was powered down > > > > > > This patch calls fec_reset_phy just after regulator enable in > > > fec_resume, when the phy is resumed > > > > > > [PATCH 1/2] net: fec: make fec_reset_phy not only usable once > > > > > > This patch prepares the function fec_reset_phy to be called multiple > > > times. It stores the data around the reset-gpio in fec_enet_private. > > > This patch aims to do no functional changes. > > > > > > [1] > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit > > > .tor%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Ca43416 > 6e62464 > > > > c4b69c408d9bbe420ef%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C637 > > > > 747410747358279%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi > LCJQIjoi > > > > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8u1EudB0 > aDkC > > > K9tzrqVznAWY6mDgBIHqIQCyDWrsH4g%3D&reserved=0 > > > > adex.com%2Fcgit%2Flinux-toradex.git%2Flog%2F%3Fh%3Dtoradex_5.4-2.3.x > > > > -imx&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c138ed9232 > > > > 4a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > > > > 7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw > > > > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sda > > > > ta=Bw%2BZdqhAjPXqKJFZCXp0mtId1x9mkX6f6MW2ky6U1ww%3D&res > > > erved=0 > > > [2] > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fso > > > urc > > > > e.codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2Flog%2F%3Fh%3Dimx_ > > > > 5.4.70_2.3.0&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf3c13 > > > > 8ed92324a8d75e708d9b8a11b9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > > > > 0%7C0%7C637743824364193423%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > > > > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000 > > > > &sdata=of9z9hfVhHakVScLxCdEo%2BXmd2B9Ad9X8Rry6GjEZE4%3D&a > > > mp;reserved=0 > > > > > > > In fec driver, it has supported hardware reset for PHY when MAC resume > > back, > > > > fec_resume() -> phy_init_hw() -> phy_device_reset() de-assert the > > reset signal, you only need implement the properties which PHY core > > provided. > > > > I think you should not use deprecated reset properties provided by fec > > driver, instead the common reset properties provided by PHY core. > > > > Please check the dt-bindings for more details: > > Documentation/devicetree/bindings/net/fsl,fec.yaml > > Documentation/devicetree/bindings/net/ethernet-phy.yaml > > > > Best Regards, > > Joakim Zhang > > Hi Joakim and many thanks for that hint! I tried that out but unfortunately it > still does not work due to phy_init_hw() only deasserting the reset. For that > to work, for example a call of phy_device_reset(ndev->phydev, 1); in > fec_suspend or in fec_resume before enabling the supply would work, in > order to assert that reset.
Yes, you are right. It only de-assert the reset in phy_init_hw(), should not enough for you.
It seems that phy reset would not be performed during suspend/resume in the phylib. AFAIK, a hardware reset automatically generated when power on for most PHYs, the PHY you used may be special, and not sure why phylib has not taken reset into suspend/resume scenario, only perform PHY suspend/resume.
> I see now two ways to go to fix our issue: > > 1. Assert the phy-reset gpio in fec_suspend() or fec_resume() > > 2. Add support for regulators in drivers/net/phy/phy-core.c and handle the > phy-reset properly in there with assert-us and deassert-us delays. > > As you probably have a much better overview: Do you see another possibility > to handle phy-reset after resuming? Or which way shall I choose to go > forward?
This should be a common issue for specific PHY, and I prefer to performing reset in phylib.
Could you please take a look at phy_reset_after_clk_enable()? Is it possible to add a function like phy_reset_after_power_on() (adding extra macro, e.g. PHY_RST_AFTER_POWER_ON) for these PHYs?
Best Regards, Joakim Zhang > Thanks in advance for any advice > Philippe > > > > Philippe Schenker (2): > > > net: fec: make fec_reset_phy not only usable once > > > net: fec: reset phy in resume if it was powered down > > > > > > drivers/net/ethernet/freescale/fec.h | 6 ++ > > > drivers/net/ethernet/freescale/fec_main.c | 98 > ++++++++++++++++---- > > > --- > > > 2 files changed, 73 insertions(+), 31 deletions(-) > > > > > > -- > > > 2.34.0 > >
| ![\](/images/icornerr.gif) |