This is an attempt to get rid of a nasty, out-of-tree hack to use PHY
wake on LAN via named gpio + irq (wakeup-souce) request. Instead, the
STMMAC_FLAG_USE_PHY_WOL flag is set if the rockchip,phy-wol property is
defined in a similar fashion as the mediatek dwmac does with the
mediatek,mac-wol property.
Unfortunately my current approach does not suffice and therefore some
advice would be more than welcome, so this use case gets support
upstream.
The goal is to use WOL in suspend-to-RAM, where we disable the MAC power
supply to increase power savings, leaving the PHY running as the wakeup
source. The system is based on a Rockchip SoC and the PHY is a
TI DP83826E, which can generate interrupts upon reception of magic
packets.
Setting the USE_PHY_WOL flag configures the PHY as expected (its driver
writes the MAC address and the interrupt configuration into the PHY
registers) and an interrupt is generated with every magic packet,
but only during normal operation i.e. there is no interrupt generation
in suspend-to-RAM.
Moreover, WOL stops working in freeze mode (where the MAC is still
powered) and it only works again if the rockchip,phy-wol property is
removed.
A (probably naive) wakeup-source property in the dt node does not help.
So now I am trying to find out why the PHY does not react in suspend and
why its interrupt is ignored in freeze mode, but I might be overlooking
some other important point to consider.
My biggest concern is that I might be overlooking the fact that there is
no way to use WOL even with the USE_PHY_WOL flag if the MAC is not
powered and the hack must stay forever. I still have hope because then WOL
should still work in freeze mode, so there must be something else to
consider.
In case it might help, this is the current device tree snippet with
the new property:
&gmac0 {
assigned-clocks = <&cru SCLK_GMAC0_RX_TX>,
<&cru SCLK_GMAC0>;
assigned-clock-parents = <&cru SCLK_GMAC0_RMII_SPEED>,
<&gmac0_clkin>;
clock_in_out = "input";
phy-handle = <&dp83826>;
phy-mode = "rmii";
phy-supply = <&vcc3v3_eth>; /* always-on regulator */
pinctrl-names = "default";
pinctrl-0 = <&gmac0_miim
&gmac0_clkinout
&gmac0_rx_er
&gmac0_rx_bus2
&gmac0_tx_bus2>;
rockchip,phy-wol; /* NEW PROPERTY */
status = "okay";
};
&mdio0 {
#address-cells = <1>;
#size-cells = <0>;
dp83826: ethernet-phy@0 {
compatible = "ethernet-phy-ieee802.3-c22";
reg = <0x0>;
interrupt-parent = <&gpio0>;
interrupts = <RK_PD3 IRQ_TYPE_EDGE_FALLING>;
pinctrl-names = "default";
pinctrl-0 = <ð_wake_intn ð_phy_rstn>;
reset-assert-us = <1000>;
reset-deassert-us = <2000>;
reset-gpios = <&gpio0 RK_PD4 GPIO_ACTIVE_LOW>;
// wakeup-source; <--- no effect
};
};
Signed-off-by: Javier Carrasco <[email protected]>
---
Javier Carrasco (2):
dt-bindings: net: rockchip-dwmac: add rockchip,phy-wol property
net: stmmac: dwmac-rk: add support for PHY wake on LAN
Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 5 +++++
2 files changed, 11 insertions(+)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231123-dwmac-rk_phy_wol-0f18fc6c2ec5
Best regards,
--
Javier Carrasco <[email protected]>
This property defines if PHY WOL is preferred. If it is not defined, MAC
WOL will be preferred instead.
Signed-off-by: Javier Carrasco <[email protected]>
---
Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
index 70bbc4220e2a..fc4b02a5a375 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
@@ -91,6 +91,12 @@ properties:
The phandle of the syscon node for the peripheral general register file.
$ref: /schemas/types.yaml#/definitions/phandle
+ rockchip,phy-wol:
+ type: boolean
+ description:
+ If present, indicates that PHY WOL is preferred. MAC WOL is preferred
+ otherwise.
+
tx_delay:
description: Delay value for TXD timing.
$ref: /schemas/types.yaml#/definitions/uint32
--
2.39.2
On Thu, Nov 23, 2023 at 01:14:13PM +0100, Javier Carrasco wrote:
> This property defines if PHY WOL is preferred. If it is not defined, MAC
> WOL will be preferred instead.
>
> Signed-off-by: Javier Carrasco <[email protected]>
> ---
> Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> index 70bbc4220e2a..fc4b02a5a375 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> @@ -91,6 +91,12 @@ properties:
> The phandle of the syscon node for the peripheral general register file.
> $ref: /schemas/types.yaml#/definitions/phandle
>
> + rockchip,phy-wol:
> + type: boolean
> + description:
> + If present, indicates that PHY WOL is preferred. MAC WOL is preferred
> + otherwise.
Although I suspect this isn't, it sounds like software policy. What
attribute of the hardware determines which is preferred?
> Setting the USE_PHY_WOL flag configures the PHY as expected (its driver
> writes the MAC address and the interrupt configuration into the PHY
> registers) and an interrupt is generated with every magic packet,
> but only during normal operation i.e. there is no interrupt generation
> in suspend-to-RAM.
Do you have a logic analyser connected? Can you see if the PHY is
toggling its output pin? We then know if its a PHY problem, or a SoC
problem.
> A (probably naive) wakeup-source property in the dt node does not help.
> So now I am trying to find out why the PHY does not react in suspend and
> why its interrupt is ignored in freeze mode, but I might be overlooking
> some other important point to consider.
What is the clock setup? Sometimes the MAC gives a clock to the
PHY. Sometimes the PHY gives a lock to the MAC. If its MAC->PHY, and
this clock is getting turned off, that might cause a problem.
Andrew
Hi Andrew,
On 23.11.23 20:04, Andrew Lunn wrote:
>> Setting the USE_PHY_WOL flag configures the PHY as expected (its driver
>> writes the MAC address and the interrupt configuration into the PHY
>> registers) and an interrupt is generated with every magic packet,
>> but only during normal operation i.e. there is no interrupt generation
>> in suspend-to-RAM.
>
> Do you have a logic analyser connected? Can you see if the PHY is
> toggling its output pin? We then know if its a PHY problem, or a SoC
> problem.
>
what I meant by no interrupt generation was not even physical interrupt
generation i.e. the signal does not toggle in suspend-to-RAM, but it
does during normal operation.
>> A (probably naive) wakeup-source property in the dt node does not help.
>> So now I am trying to find out why the PHY does not react in suspend and
>> why its interrupt is ignored in freeze mode, but I might be overlooking
>> some other important point to consider.
>
> What is the clock setup? Sometimes the MAC gives a clock to the
> PHY. Sometimes the PHY gives a lock to the MAC. If its MAC->PHY, and
> this clock is getting turned off, that might cause a problem.
>
> Andrew
Thank you for your feedback, I will analyze the clocks carefully.
Primarily I would like to know if my approach makes sense in the first
place even if there is no clock dependency. Two dwmacs (realtek and
intel) make use of the USE_PHY_WOL flag, but I could not find any dts
where the mac-wol property is used to check if the MAC power source is
off in suspend.
Best regards,
Javier Carrasco
On 23.11.23 18:20, Conor Dooley wrote:
> On Thu, Nov 23, 2023 at 01:14:13PM +0100, Javier Carrasco wrote:
>> This property defines if PHY WOL is preferred. If it is not defined, MAC
>> WOL will be preferred instead.
>>
>> Signed-off-by: Javier Carrasco <[email protected]>
>> ---
>> Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> index 70bbc4220e2a..fc4b02a5a375 100644
>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> @@ -91,6 +91,12 @@ properties:
>> The phandle of the syscon node for the peripheral general register file.
>> $ref: /schemas/types.yaml#/definitions/phandle
>>
>> + rockchip,phy-wol:
>> + type: boolean
>> + description:
>> + If present, indicates that PHY WOL is preferred. MAC WOL is preferred
>> + otherwise.
>
> Although I suspect this isn't, it sounds like software policy. What
> attribute of the hardware determines which is preferred?
Maybe the word "preferred" set off a red flag. The description is taken
from the mediatek,mac-wol, which is used to set the same flag with
inverted logic (I could invert my logic to call mine rockchip,mac-wol
and use a description without "preferences").
This property is used to enable the PHY WOL in case the MAC is powered
off in suspend mode, so it cannot provide WOL. This is done by a PMIC as
defined in the device tree and that should not be something the software
could tweak.
Best regards,
Javier Carrasco
On 23/11/2023 20:36, Javier Carrasco wrote:
> On 23.11.23 18:20, Conor Dooley wrote:
>> On Thu, Nov 23, 2023 at 01:14:13PM +0100, Javier Carrasco wrote:
>>> This property defines if PHY WOL is preferred. If it is not defined, MAC
>>> WOL will be preferred instead.
>>>
>>> Signed-off-by: Javier Carrasco <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>>> index 70bbc4220e2a..fc4b02a5a375 100644
>>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>>> @@ -91,6 +91,12 @@ properties:
>>> The phandle of the syscon node for the peripheral general register file.
>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>
>>> + rockchip,phy-wol:
>>> + type: boolean
>>> + description:
>>> + If present, indicates that PHY WOL is preferred. MAC WOL is preferred
>>> + otherwise.
>>
>> Although I suspect this isn't, it sounds like software policy. What
>> attribute of the hardware determines which is preferred?
>
> Maybe the word "preferred" set off a red flag. The description is taken
> from the mediatek,mac-wol, which is used to set the same flag with
> inverted logic (I could invert my logic to call mine rockchip,mac-wol
> and use a description without "preferences").
>
> This property is used to enable the PHY WOL in case the MAC is powered
> off in suspend mode, so it cannot provide WOL. This is done by a PMIC as
> defined in the device tree and that should not be something the software
> could tweak.
I wonder if generic wakeup-source property could not be used. WOL is a
bit different because it allows to actually turn on the computer, but
otherwise it is also a wake-up.
Best regards,
Krzysztof
On Thu, Nov 23, 2023 at 05:20:48PM +0000, Conor Dooley wrote:
> On Thu, Nov 23, 2023 at 01:14:13PM +0100, Javier Carrasco wrote:
> > This property defines if PHY WOL is preferred. If it is not defined, MAC
> > WOL will be preferred instead.
> >
> > Signed-off-by: Javier Carrasco <[email protected]>
> > ---
> > Documentation/devicetree/bindings/net/rockchip-dwmac.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> > index 70bbc4220e2a..fc4b02a5a375 100644
> > --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> > @@ -91,6 +91,12 @@ properties:
> > The phandle of the syscon node for the peripheral general register file.
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > + rockchip,phy-wol:
> > + type: boolean
> > + description:
> > + If present, indicates that PHY WOL is preferred. MAC WOL is preferred
> > + otherwise.
>
> Although I suspect this isn't, it sounds like software policy. What
> attribute of the hardware determines which is preferred?
I tend to agree, its a software policy. Doing WoL in the PHY should be
the preferred solution, because it allows the MAC to be powered off,
saving more power. If the PHY does not implement it, then the MAC
should be used.
It should be possible for the MAC driver to pass the WoL settings to
the PHY, and if it returns EOPNOTSUPP, or maybe EINVAL, implement the
WoL in the MAC.
This might be a behaviour change, depending on the MAC driver. So i
could imaging a less risk tolerant developers wanting a knob to enable
this. However, if done correctly, using the PHY instead of the MAC
should not be visible from the users perspective.
Andrew