Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY support

[Gatien CHEVALLIER]

On 9/18/25 17:36, Russell King (Oracle) wrote:
> On Thu, Sep 18, 2025 at 05:07:00PM +0200, Gatien CHEVALLIER wrote:
> > On 9/18/25 15:59, Russell King (Oracle) wrote:
> > >   > So no. In a situation like this, either we want to be in interrupt
> > > mode (in which case we have an interrupt), or the pin is wired to
> > > a power management controller and needs to be in PME mode, or it isn't
> > > wired.
> >
> > If you are in interrupt mode, plugging a cable would trigger a
> > system wakeup in low-power mode if the INTB/PMEB line is wired to a
> > power management controller and the WoL is enabled because we're no
> > longer in polling mode, wouldn't it?
>
> What Andrew suggested, which is what I implemented for Realtek, other
> interrupts get disabled when we enter suspend:
>
> static int rtl8211f_suspend(struct phy_device *phydev)
> {
> ...
>          /* If a PME event is enabled, then configure the interrupt for
>           * PME events only, disabling link interrupt. We avoid switching
>           * to PMEB mode as we don't have a status bit for that.
>           */
>          if (device_may_wakeup(&phydev->mdio.dev)) {
>                  ret = phy_write_paged(phydev, 0xa42, RTL821x_INER,
>                                        RTL8211F_INER_PME);
>
> This disables all other interrupts when entering suspend _if_ WoL
> is enabled and only if WoL is enabled.
>
> If you're getting woken up when you unplug/replug the ethernet cable
> when WoL is disabled, that suggests you have something wrong in your
> interrupt controller - the wake-up state of the interrupt is managed
> by core driver-model code. I tested this on nVidia Jetson Xavier NX
> and if WoL wasn't enabled at the PHY, no wakeup occurred.

I'll replicate what you've done for the masking of interrupt when
going to low-power modes to the Microchip driver and see where it takes
me. The wakeup occurred because I didn't mask the other interrupt
sources in nINT mode... :)

> > You can argue that as per the Realtek 8211F datasheet:
> > "The interrupts can be individually enabled or disabled by setting or
> > clearing bits in the interrupt enable register INER". That requires
> > PHY registers handling when going to low-power mode.
>
> ... which is what my patch does.
>
> > There are PHYs like the LAN8742 on which 3 pins can be configured
> > as nINT(equivalent to INTB), and 2 as nPME(equivalent to PMEB). The
> > smsc driver, as is, contains hardcoded nPME mode on the
> > LED2/nINT/nPME/nINTSEL pin. What if a manufacturer wired the power
> > management controller to the LED1/nINT/nPME/nINTSEL?
> > This is where the pinctrl would help even if I do agree it might be a
> > bit tedious at first. The pinctrl would be optional though.
>
> I'm not opposing the idea of pinctrl on PHYs. I'm opposing the idea
> of tying it into the WoL code in a way that makes it mandatory.
> Of course, if it makes sense for a PHY driver to do pinctrl stuff
> then go ahead - and if from that, the driver can work out that
> the PHY is wake-up capable, even better.
>
> What I was trying to say is that in such a case as the Realtek
> driver, I don't want to see pinctrl forced upon it unless there is
> a real reason and benefit, especially when there are simpler ways
> to do this.

Yes, sure, I think there's a proper use-case for it. If it's going
to happen, I think it would need a dedicated P-R. I'll send a V3 in
the near future addressing what we discussed here. Thank you for
the feedback.

> I also think that it would be helpful to add the wakeup-source
> property where PHYs are so capable even if the PHY driver doesn't
> need it for two reasons. 1. OS independence. 2. it's useful docs.
> 3. just because our driver as it stands at whatever moment in time
> doesn't make use of it doesn't mean that will always be the case.
> (e.g., we may want to have e.g. phylib looking at that property.)