[PATCH net-next 1/2] dt-bindings: net: dwmac-imx: add nxp,phy-wol

[PATCH net-next 1/2] dt-bindings: net: dwmac-imx: add nxp,phy-wol

[Catalin Popescu]

Add support for PHY WOL capability to dwmac-imx MAC driver.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
index 4c01cae7c93a..6cf373772eb1 100644
--- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
+++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
@@ -71,6 +71,12 @@ properties:
     description:
       To select RMII reference clock from external.
 
+  nxp,phy-wol:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If present, indicates that PHY supports WOL(Wake-On-LAN), and PHY WOL will be enabled.
+      Otherwise, MAC WOL is preferred.
+
 required:
   - compatible
   - clocks

base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.34.1

[PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[Catalin Popescu]

Add support for PHY WOL capability into dwmac-imx MAC driver.
This is required to enable WOL feature on a platform where MAC
WOL capability is not sufficient and WOL capability built into
the PHY is actually needed.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 6b65420e11b5..9d7278c083bf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -59,6 +59,7 @@ struct imx_priv_data {
 	struct regmap *intf_regmap;
 	u32 intf_reg_off;
 	bool rmii_refclk_ext;
+	bool phy_wol;
 	void __iomem *base_addr;
 
 	const struct imx_dwmac_ops *ops;
@@ -312,6 +313,8 @@ imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
 		}
 	}
 
+	dwmac->phy_wol = of_property_read_bool(np, "nxp,phy-wol");
+
 	return err;
 }
 
@@ -353,6 +356,11 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 	if (data->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
 		plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY;
 
+	if (dwmac->phy_wol)
+		plat_dat->flags |= STMMAC_FLAG_USE_PHY_WOL;
+	else
+		plat_dat->flags &= ~STMMAC_FLAG_USE_PHY_WOL;
+
 	/* Default TX Q0 to use TSO and rest TXQ for TBS */
 	for (int i = 1; i < plat_dat->tx_queues_to_use; i++)
 		plat_dat->tx_queues_cfg[i].tbs_en = 1;
-- 
2.34.1

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[Florian Fainelli]

On 3/6/24 09:24, Catalin Popescu wrote:
> Add support for PHY WOL capability into dwmac-imx MAC driver.
> This is required to enable WOL feature on a platform where MAC
> WOL capability is not sufficient and WOL capability built into
> the PHY is actually needed.
> 
> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>

Nope, this is not about how to do this. You use a Device Tree property 
as a policy rather than properly describe your systems capabilities.

What sort of Wake-on-LAN do you want to be done by the PHY exactly? Does 
the PHY have packet matching capabilities, or do you want to wake-up 
from a PHY event like link up/down/any interrupt?

If the former, then you would need to interrogate the PHY driver via 
phy_ethtool_get_wol() to figure out what Wake-on-LAN modes it is capable 
of supporting and then make a decision whether to prioritize Wake-on-LAN 
from the PHY or the MAC, or maybe only the PHY can actually wake-up the 
system in your case.

If the latter, then you need to add support for WAKE_PHY to the stmmac
driver.

pw-bot: cr
-- 
Florian

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[POPESCU Catalin]

On 06.03.24 18:41, Florian Fainelli wrote:
> [Some people who received this message don't often get email from 
> f.fainelli@gmail.com. Learn why this is important at 
> https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be 
> careful while clicking links, opening attachments, or replying to this 
> email.
>
>
> On 3/6/24 09:24, Catalin Popescu wrote:
>> Add support for PHY WOL capability into dwmac-imx MAC driver.
>> This is required to enable WOL feature on a platform where MAC
>> WOL capability is not sufficient and WOL capability built into
>> the PHY is actually needed.
>>
>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>
> Nope, this is not about how to do this. You use a Device Tree property
> as a policy rather than properly describe your systems capabilities.
I'm not sure what policy means in that context.
BTW, dwmac-mediatek does the same with binding "mediatek,mac-wol" which 
is a commit from 03/2022.
I understand this way of doing became "unacceptable" since then ??
>
> What sort of Wake-on-LAN do you want to be done by the PHY exactly? Does
> the PHY have packet matching capabilities, or do you want to wake-up
> from a PHY event like link up/down/any interrupt?

PHY is TI dp83826 and has secure magic packet capability. For the wakeup 
we rely on a external MCU which is signaled through a PHY's GPIO which 
toggles only on magic packet reception.
We want to wakeup _only_ on magic packet reception.

>
> If the former, then you would need to interrogate the PHY driver via
> phy_ethtool_get_wol() to figure out what Wake-on-LAN modes it is capable
> of supporting and then make a decision whether to prioritize Wake-on-LAN
> from the PHY or the MAC, or maybe only the PHY can actually wake-up the
> system in your case.
>
stmmac already calls phy_ethtool_get_wol/phy_ethtool_set_wol through 
phylink_ethtool_get_wol/phylink_ethtool_set_wol. But needs flag 
STMMAC_FLAG_USE_PHY_WOL to be set. Otherwise, it will only work with MAC 
WOL which we don't want. With the new binding we just allow the MAC 
driver to call the PHY for the WOL capability. This doesn't force WOL to 
enabled or disabled, as it is still up to ethtool to configure it.
> If the latter, then you need to add support for WAKE_PHY to the stmmac
> driver.
No, we don't want WAKE_PHY, we want WAKE_MAGIC/WAKE_MAGICSECURE which 
stmmac driver already supports.
>
> pw-bot: cr
> -- 
> Florian
>

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[POPESCU Catalin]

On 07.03.24 10:13, POPESCU Catalin wrote:
> On 06.03.24 18:41, Florian Fainelli wrote:
>> [Some people who received this message don't often get email from
>> f.fainelli@gmail.com. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> This email is not from Hexagon’s Office 365 instance. Please be
>> careful while clicking links, opening attachments, or replying to this
>> email.
>>
>>
>> On 3/6/24 09:24, Catalin Popescu wrote:
>>> Add support for PHY WOL capability into dwmac-imx MAC driver.
>>> This is required to enable WOL feature on a platform where MAC
>>> WOL capability is not sufficient and WOL capability built into
>>> the PHY is actually needed.
>>>
>>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>> Nope, this is not about how to do this. You use a Device Tree property
>> as a policy rather than properly describe your systems capabilities.
> I'm not sure what policy means in that context.
> BTW, dwmac-mediatek does the same with binding "mediatek,mac-wol" which
> is a commit from 03/2022.
> I understand this way of doing became "unacceptable" since then ??
>> What sort of Wake-on-LAN do you want to be done by the PHY exactly? Does
>> the PHY have packet matching capabilities, or do you want to wake-up
>> from a PHY event like link up/down/any interrupt?
> PHY is TI dp83826 and has secure magic packet capability. For the wakeup
> we rely on a external MCU which is signaled through a PHY's GPIO which
> toggles only on magic packet reception.
> We want to wakeup _only_ on magic packet reception.
>
>> If the former, then you would need to interrogate the PHY driver via
>> phy_ethtool_get_wol() to figure out what Wake-on-LAN modes it is capable
>> of supporting and then make a decision whether to prioritize Wake-on-LAN
>> from the PHY or the MAC, or maybe only the PHY can actually wake-up the
>> system in your case.
>>
> stmmac already calls phy_ethtool_get_wol/phy_ethtool_set_wol through
> phylink_ethtool_get_wol/phylink_ethtool_set_wol. But needs flag
> STMMAC_FLAG_USE_PHY_WOL to be set. Otherwise, it will only work with MAC
> WOL which we don't want. With the new binding we just allow the MAC
> driver to call the PHY for the WOL capability. This doesn't force WOL to
> enabled or disabled, as it is still up to ethtool to configure it.
>> If the latter, then you need to add support for WAKE_PHY to the stmmac
>> driver.
> No, we don't want WAKE_PHY, we want WAKE_MAGIC/WAKE_MAGICSECURE which
> stmmac driver already supports.
Actually, sttmac doesn't support WAKE_MAGICSECURE but we don't really 
care as PHY driver supports it.
>> pw-bot: cr
>> -- 
>> Florian
>>

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[Florian Fainelli]

On 3/7/2024 1:13 AM, POPESCU Catalin wrote:
> On 06.03.24 18:41, Florian Fainelli wrote:
>> [Some people who received this message don't often get email from
>> f.fainelli@gmail.com. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> This email is not from Hexagon’s Office 365 instance. Please be
>> careful while clicking links, opening attachments, or replying to this
>> email.
>>
>>
>> On 3/6/24 09:24, Catalin Popescu wrote:
>>> Add support for PHY WOL capability into dwmac-imx MAC driver.
>>> This is required to enable WOL feature on a platform where MAC
>>> WOL capability is not sufficient and WOL capability built into
>>> the PHY is actually needed.
>>>
>>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>>
>> Nope, this is not about how to do this. You use a Device Tree property
>> as a policy rather than properly describe your systems capabilities.
> I'm not sure what policy means in that context.
> BTW, dwmac-mediatek does the same with binding "mediatek,mac-wol" which
> is a commit from 03/2022.

Policy here means you want a certain behavior from the OS that is 
consuming the Device Tree, and that behavior is encoded via a Device 
Tree property. This is different from describing how the hardware works 
which does not make any provisions for getting a behavior out of the OS.

> I understand this way of doing became "unacceptable" since then ??

It was not acceptable then, but there is only a limited reviewer time, 
and it is easy unfortunately to sneak through reviewers.

>>
>> What sort of Wake-on-LAN do you want to be done by the PHY exactly? Does
>> the PHY have packet matching capabilities, or do you want to wake-up
>> from a PHY event like link up/down/any interrupt?
> 
> PHY is TI dp83826 and has secure magic packet capability. For the wakeup
> we rely on a external MCU which is signaled through a PHY's GPIO which
> toggles only on magic packet reception.
> We want to wakeup _only_ on magic packet reception.

Then you need to represent that wake-up GPIO line in the Device Tree, 
associate it with the PHY's Device Tree node for starters and add in a 
'wakeup-source' property in the Device Tree.

Now the PHY driver can know about the existence of a GPIO and it can 
know the PHY is a system wake-up source, so the driver can call 
device_set_wakeup_capable().

In user-space you have to configure the network interface with 
WAKE_MAGICSECURE which needs to propagate to the PHY driver for adequate 
configuration. Still in user-space you need to make the PHY device 
wake-up *enabled* by doing:

echo "enable" > /sys/class/net/ethX/attached_phydev/power/wakeup

If both WAKE_MAGICSECURE is enabled and the PHY device in sysfs reports 
that it is wake-up enabled would you wake-up from the PHY's GPIO. Your 
PHY driver ought to be modified to check for both

device_wakeup_enabled() and wolopts being non-zero to call 
enable_irq_wake() on the GPIO interrupt line.

That's how I would go about doing this, yes it's a tad more complicated 
than adding an ad-hoc Device Tree property, but it's more flexible and 
it's transposable to other configurations, too. Whether that sort of 
encoding needs to be in the individual PHY drivers or somewhere in the 
PHY library can be decided if we have more than one similar 
configuration to support.

> 
>>
>> If the former, then you would need to interrogate the PHY driver via
>> phy_ethtool_get_wol() to figure out what Wake-on-LAN modes it is capable
>> of supporting and then make a decision whether to prioritize Wake-on-LAN
>> from the PHY or the MAC, or maybe only the PHY can actually wake-up the
>> system in your case.
>>
> stmmac already calls phy_ethtool_get_wol/phy_ethtool_set_wol through
> phylink_ethtool_get_wol/phylink_ethtool_set_wol. But needs flag
> STMMAC_FLAG_USE_PHY_WOL to be set. Otherwise, it will only work with MAC
> WOL which we don't want. With the new binding we just allow the MAC
> driver to call the PHY for the WOL capability. This doesn't force WOL to
> enabled or disabled, as it is still up to ethtool to configure it.
>> If the latter, then you need to add support for WAKE_PHY to the stmmac
>> driver.
> No, we don't want WAKE_PHY, we want WAKE_MAGIC/WAKE_MAGICSECURE which
> stmmac driver already supports.

Does not matter, it should be up to user-space to intersect between what 
the PHY is capable of waking you from, and what the intent is.
-- 
Florian

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[POPESCU Catalin]

On 07.03.24 19:52, Florian Fainelli wrote:
> [Some people who received this message don't often get email from 
> f.fainelli@gmail.com. Learn why this is important at 
> https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be 
> careful while clicking links, opening attachments, or replying to this 
> email.
>
>
> On 3/7/2024 1:13 AM, POPESCU Catalin wrote:
>> On 06.03.24 18:41, Florian Fainelli wrote:
>>> [Some people who received this message don't often get email from
>>> f.fainelli@gmail.com. Learn why this is important at
>>> https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> This email is not from Hexagon’s Office 365 instance. Please be
>>> careful while clicking links, opening attachments, or replying to this
>>> email.
>>>
>>>
>>> On 3/6/24 09:24, Catalin Popescu wrote:
>>>> Add support for PHY WOL capability into dwmac-imx MAC driver.
>>>> This is required to enable WOL feature on a platform where MAC
>>>> WOL capability is not sufficient and WOL capability built into
>>>> the PHY is actually needed.
>>>>
>>>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>>>
>>> Nope, this is not about how to do this. You use a Device Tree property
>>> as a policy rather than properly describe your systems capabilities.
>> I'm not sure what policy means in that context.
>> BTW, dwmac-mediatek does the same with binding "mediatek,mac-wol" which
>> is a commit from 03/2022.
>
> Policy here means you want a certain behavior from the OS that is
> consuming the Device Tree, and that behavior is encoded via a Device
> Tree property. This is different from describing how the hardware works
> which does not make any provisions for getting a behavior out of the OS.
>
>> I understand this way of doing became "unacceptable" since then ??
>
> It was not acceptable then, but there is only a limited reviewer time,
> and it is easy unfortunately to sneak through reviewers.
>
>>>
>>> What sort of Wake-on-LAN do you want to be done by the PHY exactly? 
>>> Does
>>> the PHY have packet matching capabilities, or do you want to wake-up
>>> from a PHY event like link up/down/any interrupt?
>>
>> PHY is TI dp83826 and has secure magic packet capability. For the wakeup
>> we rely on a external MCU which is signaled through a PHY's GPIO which
>> toggles only on magic packet reception.
>> We want to wakeup _only_ on magic packet reception.
>
> Then you need to represent that wake-up GPIO line in the Device Tree,
> associate it with the PHY's Device Tree node for starters and add in a
> 'wakeup-source' property in the Device Tree.
The GPIO I was referring to is a PHY GPIO not a SOC GPIO, so there's no 
way to describe it into the DT.
The PHY is connected on the SOC MDIO bus, so the SOC programs the PHY, 
but the PHY wakes up the external MCU which in turn wakes up the SOC.

>
> Now the PHY driver can know about the existence of a GPIO and it can
> know the PHY is a system wake-up source, so the driver can call
> device_set_wakeup_capable().
>
> In user-space you have to configure the network interface with
> WAKE_MAGICSECURE which needs to propagate to the PHY driver for adequate
> configuration. Still in user-space you need to make the PHY device
> wake-up *enabled* by doing:
>
> echo "enable" > /sys/class/net/ethX/attached_phydev/power/wakeup
>
> If both WAKE_MAGICSECURE is enabled and the PHY device in sysfs reports
> that it is wake-up enabled would you wake-up from the PHY's GPIO. Your
> PHY driver ought to be modified to check for both
>
> device_wakeup_enabled() and wolopts being non-zero to call
> enable_irq_wake() on the GPIO interrupt line.
>
> That's how I would go about doing this, yes it's a tad more complicated
> than adding an ad-hoc Device Tree property, but it's more flexible and
> it's transposable to other configurations, too. Whether that sort of
> encoding needs to be in the individual PHY drivers or somewhere in the
> PHY library can be decided if we have more than one similar
> configuration to support.
>
Yes, it's more complicated and it doesn't apply to our platform.
But, this doesn't really matter in the end, the problem I'm trying to 
address here is to allow stmac for IMX to support PHY WOL.
Since the binding is not acceptable, I guess the only option here is to 
remove flag STMMAC_FLAG_USE_PHY_WOL from stmac driver and replace it 
with a call to phylink_ethtool_get_wol to identify whether the PHY is 
WOL capable or not.
Then, how should we allow the platform to choose b/w MAC WOL and PHY WOL 
if both are supported ?
AFAIK ethtool only knows about MAC WOL capability since it interrogates 
the MAC interface. ethtool doesn't know anything about the PHY, or does 
it ??

>>
>>>
>>> If the former, then you would need to interrogate the PHY driver via
>>> phy_ethtool_get_wol() to figure out what Wake-on-LAN modes it is 
>>> capable
>>> of supporting and then make a decision whether to prioritize 
>>> Wake-on-LAN
>>> from the PHY or the MAC, or maybe only the PHY can actually wake-up the
>>> system in your case.
>>>
>> stmmac already calls phy_ethtool_get_wol/phy_ethtool_set_wol through
>> phylink_ethtool_get_wol/phylink_ethtool_set_wol. But needs flag
>> STMMAC_FLAG_USE_PHY_WOL to be set. Otherwise, it will only work with MAC
>> WOL which we don't want. With the new binding we just allow the MAC
>> driver to call the PHY for the WOL capability. This doesn't force WOL to
>> enabled or disabled, as it is still up to ethtool to configure it.
>>> If the latter, then you need to add support for WAKE_PHY to the stmmac
>>> driver.
>> No, we don't want WAKE_PHY, we want WAKE_MAGIC/WAKE_MAGICSECURE which
>> stmmac driver already supports.
>
> Does not matter, it should be up to user-space to intersect between what
> the PHY is capable of waking you from, and what the intent is.
> -- 
> Florian

Re: [PATCH net-next 2/2] net: stmmac: dwmac-imx: add support for PHY WOL

[Florian Fainelli]

On 3/8/24 09:48, POPESCU Catalin wrote:
> On 07.03.24 19:52, Florian Fainelli wrote:
>> [Some people who received this message don't often get email from
>> f.fainelli@gmail.com. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> This email is not from Hexagon’s Office 365 instance. Please be
>> careful while clicking links, opening attachments, or replying to this
>> email.
>>
>>
>> On 3/7/2024 1:13 AM, POPESCU Catalin wrote:
>>> On 06.03.24 18:41, Florian Fainelli wrote:
>>>> [Some people who received this message don't often get email from
>>>> f.fainelli@gmail.com. Learn why this is important at
>>>> https://aka.ms/LearnAboutSenderIdentification ]
>>>>
>>>> This email is not from Hexagon’s Office 365 instance. Please be
>>>> careful while clicking links, opening attachments, or replying to this
>>>> email.
>>>>
>>>>
>>>> On 3/6/24 09:24, Catalin Popescu wrote:
>>>>> Add support for PHY WOL capability into dwmac-imx MAC driver.
>>>>> This is required to enable WOL feature on a platform where MAC
>>>>> WOL capability is not sufficient and WOL capability built into
>>>>> the PHY is actually needed.
>>>>>
>>>>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>>>>
>>>> Nope, this is not about how to do this. You use a Device Tree property
>>>> as a policy rather than properly describe your systems capabilities.
>>> I'm not sure what policy means in that context.
>>> BTW, dwmac-mediatek does the same with binding "mediatek,mac-wol" which
>>> is a commit from 03/2022.
>>
>> Policy here means you want a certain behavior from the OS that is
>> consuming the Device Tree, and that behavior is encoded via a Device
>> Tree property. This is different from describing how the hardware works
>> which does not make any provisions for getting a behavior out of the OS.
>>
>>> I understand this way of doing became "unacceptable" since then ??
>>
>> It was not acceptable then, but there is only a limited reviewer time,
>> and it is easy unfortunately to sneak through reviewers.
>>
>>>>
>>>> What sort of Wake-on-LAN do you want to be done by the PHY exactly?
>>>> Does
>>>> the PHY have packet matching capabilities, or do you want to wake-up
>>>> from a PHY event like link up/down/any interrupt?
>>>
>>> PHY is TI dp83826 and has secure magic packet capability. For the wakeup
>>> we rely on a external MCU which is signaled through a PHY's GPIO which
>>> toggles only on magic packet reception.
>>> We want to wakeup _only_ on magic packet reception.
>>
>> Then you need to represent that wake-up GPIO line in the Device Tree,
>> associate it with the PHY's Device Tree node for starters and add in a
>> 'wakeup-source' property in the Device Tree.
> The GPIO I was referring to is a PHY GPIO not a SOC GPIO, so there's no
> way to describe it into the DT.

Well, technically there is, it's just that the PHY is not registered 
with Linux as a GPIO controller/provider, nothing prevents you from 
doing that, but it starts raising the bar.

> The PHY is connected on the SOC MDIO bus, so the SOC programs the PHY,
> but the PHY wakes up the external MCU which in turn wakes up the SOC.

OK, but that still needs to be described somehow, otherwise you are just 
cutting corners and assuming that the pin from the PHY to the external 
MCU is only driven when the PHY drives it, how about other wake-up 
sources to the MCU?

> 
>>
>> Now the PHY driver can know about the existence of a GPIO and it can
>> know the PHY is a system wake-up source, so the driver can call
>> device_set_wakeup_capable().
>>
>> In user-space you have to configure the network interface with
>> WAKE_MAGICSECURE which needs to propagate to the PHY driver for adequate
>> configuration. Still in user-space you need to make the PHY device
>> wake-up *enabled* by doing:
>>
>> echo "enable" > /sys/class/net/ethX/attached_phydev/power/wakeup
>>
>> If both WAKE_MAGICSECURE is enabled and the PHY device in sysfs reports
>> that it is wake-up enabled would you wake-up from the PHY's GPIO. Your
>> PHY driver ought to be modified to check for both
>>
>> device_wakeup_enabled() and wolopts being non-zero to call
>> enable_irq_wake() on the GPIO interrupt line.
>>
>> That's how I would go about doing this, yes it's a tad more complicated
>> than adding an ad-hoc Device Tree property, but it's more flexible and
>> it's transposable to other configurations, too. Whether that sort of
>> encoding needs to be in the individual PHY drivers or somewhere in the
>> PHY library can be decided if we have more than one similar
>> configuration to support.
>>
> Yes, it's more complicated and it doesn't apply to our platform.
> But, this doesn't really matter in the end, the problem I'm trying to
> address here is to allow stmac for IMX to support PHY WOL.
> Since the binding is not acceptable, I guess the only option here is to
> remove flag STMMAC_FLAG_USE_PHY_WOL from stmac driver and replace it
> with a call to phylink_ethtool_get_wol to identify whether the PHY is
> WOL capable or not.
> Then, how should we allow the platform to choose b/w MAC WOL and PHY WOL
> if both are supported ?

We don't have a good way to configure that decision consistently and 
across *all* device drivers currently, what I can think of as the least 
bad solution is to intersect between the PHY supported WOL modes, the 
MAC supported WOL modes, and checking which of those is a wake-up enable 
device via device_wakeup_enabled() and use that one with a preference 
for the PHY since that is the closest to the wire. But this might be 
good for me and you, maybe not for others.

> AFAIK ethtool only knows about MAC WOL capability since it interrogates
> the MAC interface. ethtool doesn't know anything about the PHY, or does
> it ??

No we don't, and until Maxime's patches about PHY topology land upstream:

https://lwn.net/Articles/961959/

we do not want to invent many different ways of specifying which of the 
MAC or the PHY should be used for WOL. FWIW, I have a similar need:

https://www.spinics.net/lists/netdev/msg751196.html

https://lore.kernel.org/all/20231026224509.112353-1-florian.fainelli@broadcom.com/
-- 
Florian

Re: [PATCH net-next 1/2] dt-bindings: net: dwmac-imx: add nxp,phy-wol

[Florian Fainelli]

On 3/6/24 09:24, Catalin Popescu wrote:
> Add support for PHY WOL capability to dwmac-imx MAC driver.
> 
> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
> ---
>   Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> index 4c01cae7c93a..6cf373772eb1 100644
> --- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> @@ -71,6 +71,12 @@ properties:
>       description:
>         To select RMII reference clock from external.
>   
> +  nxp,phy-wol:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If present, indicates that PHY supports WOL(Wake-On-LAN), and PHY WOL will be enabled.
> +      Otherwise, MAC WOL is preferred.

This is encoding a policy in Device Tree, which is not acceptable, see 
my comment about your second patch.

pw-bot: cr
-- 
Florian