Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()

[Russell King (Oracle)]

On Thu, Apr 09, 2026 at 11:58:04AM +0000, Biju Das wrote:
> Hi Russell King,
> 
> > -----Original Message-----
> > From: Russell King <linux@xxxxxxxxxxxxxxx>
> > Sent: 09 April 2026 12:30
> > Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
> > 
> > On Thu, Apr 09, 2026 at 11:19:43AM +0000, Biju Das wrote:
> > > Hi Russell King,
> > >
> > > > -----Original Message-----
> > > > From: Russell King <linux@xxxxxxxxxxxxxxx>
> > > > Sent: 09 April 2026 12:05
> > > > Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove
> > > > ksz9131_resume()
> > > >
> > > > On Thu, Apr 09, 2026 at 10:52:35AM +0000, Biju Das wrote:
> > > > > Hi Russell King,
> > > > >
> > > > > Thanks for the feedback.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Russell King <linux@xxxxxxxxxxxxxxx>
> > > > > > Sent: 09 April 2026 11:30
> > > > > > Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove
> > > > > > ksz9131_resume()
> > > > > >
> > > > > > phy_init_hw() will also call drv->config_intr(), so that doesn't need to be done either.
> > > > > >
> > > > > > It will also call drv->config_init(), which will call kszphy_config_reset().
> > > > > >
> > > > > > So most of kszphy_resume() becomes unnecessary. I think the only
> > > > > > thing that remains would be the call to kszphy_enable_clk() -
> > > > > > and is it fine to call that after
> > > > phy_init_hw() ?
> > > > >
> > > > > It just needs kszphy_enable_clk() and phydev->drv->config_intr()
> > > > > to enable PHY interrupts for suspend-to-RAM to work on RZ/G3E SMARC EVK.
> > > >
> > > > I think you mean WoL rather than suspend-to-RAM, although I don't
> > > > see anything in micrel.c that hints that WoL is supported, so please explain why and how the PHY
> > interrupt impacts suspend-to-RAM.
> > >
> > > This is not WoL. During Suspend-to-RAM, the DDR goes into retention
> > > mode while the CPU, SoC, and PHY power is cut off.
> > >
> > > During resume, TF-A detects WARM_RESET, brings DDR out of retention,
> > > and jumps to the PSCI resume path.
> > >
> > > >
> > > > Note that a particular interrupt should not wake the system unless
> > > > enable_irq_wake() has been called for that specific interrupt.
> > >
> > > If PHY interrupts are not configured during resume, no link interrupt is received and the message:
> > > "renesas-gbeth 11c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx"
> > > is not seen, as shown in [1].
> > 
> > ... and why does that happen? Is it because the PHY has lost its interrupt configuration and that needs
> > to be reprogrammed?
> 
> Yes, but phy_init_hw() reconfigures the PHY interrupt during resume.
> This is due to phydev->interrupts = PHY_INTERRUPT_DISABLED; in the suspend path, as you mentioned below.
> 
> > 
> > If you don't disable the PHY interrupt in the suspend path, then will the call to drv->config_intr()
> > via phy_init_hw() before
> > phy_resume() be sufficient?
> 
> Yes, I confirm that if the PHY interrupt is not disabled in the suspend path, the call to
> drv->config_intr() via phy_init_hw() before phy_resume() would be sufficient.

I think we need a simple solution for 7.0, but subject to Andrew's
agreement, I think we should consider having phy_init_hw() inside
phy_resume(), and a series of cleanup patches that result from that
change, including getting rid of unnecessary code in micrel.c for
the next kernel cycle. As I say, subject to Andrew's agreement, please
can you look into this. Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!