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

[Gatien CHEVALLIER <gatien.chevallier@foss.st.com>]

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.)
>