From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>,
Philippe Schenker <philippe.schenker@toradex.com>,
netdev@vger.kernel.org, Joakim Zhang <qiangqing.zhang@nxp.com>,
"David S . Miller" <davem@davemloft.net>,
Heiner Kallweit <hkallweit1@gmail.com>,
Jakub Kicinski <kuba@kernel.org>,
Fabio Estevam <festevam@gmail.com>,
Fugang Duan <fugang.duan@nxp.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 3/3] net: fec: reset phy on resume after power-up
Date: Wed, 15 Dec 2021 10:29:07 +0000 [thread overview]
Message-ID: <YbnDc/snmb1WYVCt@shell.armlinux.org.uk> (raw)
In-Reply-To: <Ybm3NDeq96TSjh+k@lunn.ch>
On Wed, Dec 15, 2021 at 10:36:52AM +0100, Andrew Lunn wrote:
> On Tue, Dec 14, 2021 at 11:35:48PM +0100, Francesco Dolcini wrote:
> > Hello Andrew,
> >
> > On Tue, Dec 14, 2021 at 07:54:54PM +0100, Andrew Lunn wrote:
> > > What i don't particularly like about this is that the MAC driver is
> > > doing it. Meaning if this PHY is used with any other MAC, the same
> > > code needs adding there.
> > This is exactly the same case as phy_reset_after_clk_enable() [1][2], to
> > me it does not look that bad.
> >
> > > So maybe in the phy driver, add a suspend handler, which asserts the
> > > reset. This call here will take it out of reset, so applying the reset
> > > you need?
> > Asserting the reset in the phylib in suspend path is a bad idea, in the
> > general case in which the PHY is powered in suspend the
> > power-consumption is likely to be higher if the device is in reset
> > compared to software power-down using the BMCR register (at least for
> > the PHY datasheet I checked).
>
> Maybe i don't understand your hardware.
>
> You have a regulator providing power of the PHY.
>
> You have a reset, i guess a GPIO, connected to the reset pin of the
> PHY.
>
> What you could do is:
>
> PHY driver suspend handler does a phy_device_reset(ndev->phydev, 1)
> to put the PHY into reset.
>
> MAC driver disables the regulator.
>
> Power consumption should now be 0, since it does not have any power.
>
> On resume, the MAC enables the regulator. At this point, the PHY gets
> power, but is still held in reset. It is now consuming power, but not
> doing anything. The MAC calls phy_hw_init(), which calls
> phy_device_reset(ndev->phydev, 0), taking the PHY out of reset.
>
> Hopefully, this release from reset is enough to make the PHY work.
>
> Doing it like this also addresses Russell point. phy_hw_init() is not
> putting the device into reset, it is only taking it out of reset, if
> it happens to be already in reset. So we are not slowing down link up
> for everybody.
Here's another question which no one seems to have considered. If the
PHY power source can be controlled, why doesn't the firmware describe
the power supply for the PHY, and why doesn't the PHY driver control
the PHY power source? Why is that in the SoC network driver?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2021-12-15 10:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-14 12:16 [PATCH net-next 0/3] Add Possiblity to Reset PHY After Power-up Philippe Schenker
2021-12-14 12:16 ` [PATCH net-next 1/3] net: phy: add phy_reset_after_power_on() function Philippe Schenker
2021-12-14 12:16 ` [PATCH net-next 2/3] net: phy: micrel: add reset-after-power-on flag to ksz9x31 phys Philippe Schenker
2021-12-14 12:16 ` [PATCH net-next 3/3] net: fec: reset phy on resume after power-up Philippe Schenker
2021-12-14 18:54 ` Andrew Lunn
2021-12-14 19:09 ` Russell King (Oracle)
2021-12-14 22:35 ` Francesco Dolcini
2021-12-15 9:36 ` Andrew Lunn
2021-12-15 10:29 ` Russell King (Oracle) [this message]
2021-12-15 11:01 ` Francesco Dolcini
2021-12-15 11:10 ` Francesco Dolcini
2021-12-15 19:34 ` Andrew Lunn
2021-12-15 19:48 ` Francesco Dolcini
2021-12-16 4:52 ` Joakim Zhang
2021-12-16 7:52 ` Francesco Dolcini
2021-12-16 10:24 ` Andrew Lunn
2021-12-16 11:24 ` Francesco Dolcini
2021-12-16 11:28 ` Andrew Lunn
2021-12-16 11:31 ` Francesco Dolcini
2021-12-16 11:32 ` Andrew Lunn
2021-12-15 10:25 ` Joakim Zhang
2021-12-14 12:23 ` [PATCH net-next 0/3] Add Possiblity to Reset PHY After Power-up Francesco Dolcini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YbnDc/snmb1WYVCt@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=festevam@gmail.com \
--cc=francesco.dolcini@toradex.com \
--cc=fugang.duan@nxp.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=philippe.schenker@toradex.com \
--cc=qiangqing.zhang@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).