[v2] net: macb: WOL enhancements

[PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Vineeth Karumanchi 1 year, 11 months ago

- Enable WOL for ZynqMP devices.
- Enable queue disable and WOL for Versal devices.

Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 62d796ef4035..55d8c1f3ee80 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4903,7 +4903,7 @@ static const struct macb_config np4_config = {
 
 static const struct macb_config zynqmp_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
-		MACB_CAPS_JUMBO |
+		MACB_CAPS_JUMBO | MACB_CAPS_WOL |
 		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
 	.dma_burst_length = 16,
 	.clk_init = macb_clk_init,
@@ -4954,7 +4954,9 @@ static const struct macb_config sama7g5_emac_config = {
 
 static const struct macb_config versal_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
-		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
+		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH |
+		MACB_CAPS_QUEUE_DISABLE | MACB_CAPS_NEED_TSUCLK |
+		MACB_CAPS_WOL,
 	.dma_burst_length = 16,
 	.clk_init = macb_clk_init,
 	.init = init_reset_optional,
-- 
2.34.1

Re: [PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Andrew Lunn 1 year, 11 months ago

On Thu, Feb 22, 2024 at 09:08:47PM +0530, Vineeth Karumanchi wrote:
> - Enable WOL for ZynqMP devices.
> - Enable queue disable and WOL for Versal devices.
> 
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 62d796ef4035..55d8c1f3ee80 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4903,7 +4903,7 @@ static const struct macb_config np4_config = {
>  
>  static const struct macb_config zynqmp_config = {
>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
> -		MACB_CAPS_JUMBO |
> +		MACB_CAPS_JUMBO | MACB_CAPS_WOL |
>  		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>  	.dma_burst_length = 16,
>  	.clk_init = macb_clk_init,
> @@ -4954,7 +4954,9 @@ static const struct macb_config sama7g5_emac_config = {
>  
>  static const struct macb_config versal_config = {
>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
> -		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
> +		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH |
> +		MACB_CAPS_QUEUE_DISABLE | MACB_CAPS_NEED_TSUCLK |
> +		MACB_CAPS_WOL,

So WoL is a proprietary extension only available in AMD versions of
the MACB?

    Andrew

Re: [PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Vineeth Karumanchi 1 year, 11 months ago

Hi Andrew,

On 23/02/24 1:04 am, Andrew Lunn wrote:
> On Thu, Feb 22, 2024 at 09:08:47PM +0530, Vineeth Karumanchi wrote:
>> - Enable WOL for ZynqMP devices.
>> - Enable queue disable and WOL for Versal devices.
>>
>> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
>> ---
>>   drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 62d796ef4035..55d8c1f3ee80 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -4903,7 +4903,7 @@ static const struct macb_config np4_config = {
>>   
>>   static const struct macb_config zynqmp_config = {
>>   	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>> -		MACB_CAPS_JUMBO |
>> +		MACB_CAPS_JUMBO | MACB_CAPS_WOL |
>>   		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>>   	.dma_burst_length = 16,
>>   	.clk_init = macb_clk_init,
>> @@ -4954,7 +4954,9 @@ static const struct macb_config sama7g5_emac_config = {
>>   
>>   static const struct macb_config versal_config = {
>>   	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
>> -		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
>> +		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH |
>> +		MACB_CAPS_QUEUE_DISABLE | MACB_CAPS_NEED_TSUCLK |
>> +		MACB_CAPS_WOL,
> 
> So WoL is a proprietary extension only available in AMD versions of
> the MACB?
> 

It is not specific to AMD versions. All Cadence GEM IP versions have the 
capability, but specific vendors might enable or disable it as per their 
requirements.

WOL was previously enabled via the device-tree attribute. Some users 
might not leverage it.

However, we can add MACB_CAPS_WOL to "default_gem_config", but we don't 
know which devices have WOL support.

Please let me know your suggestions.

Vineeth
>      Andrew

Re: [PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Andrew Lunn 1 year, 11 months ago

> It is not specific to AMD versions. All Cadence GEM IP versions have the
> capability, but specific vendors might enable or disable it as per their
> requirements.

Do you mean it is an option to synthesizer it or not? So although the
basic IP licensed from Cadence has it, a silicon vendor could remove
it?

> WOL was previously enabled via the device-tree attribute. Some users might
> not leverage it.

This is not typical. If the hardware supports it, we let the end user
decided if they want to use it or not.

So if all silicon should have it, enable it everywhere. If there is an
option to save some gates and leave it out of the silicon, then we do
need some per device knowledge, or a register which tells us what the
synthesis options where.

	Andrew

Re: [PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Karumanchi, Vineeth 1 year, 11 months ago

Hi Andrew,

On 2/23/2024 6:56 PM, Andrew Lunn wrote:
>> It is not specific to AMD versions. All Cadence GEM IP versions have the
>> capability, but specific vendors might enable or disable it as per their
>> requirements.
> 
> Do you mean it is an option to synthesizer it or not? So although the
> basic IP licensed from Cadence has it, a silicon vendor could remove
> it?
> 

Regarding that, we are unsure. However, based on observation from all 
previous cadence IP's in AMD Soc's, this feature was available.

>> WOL was previously enabled via the device-tree attribute. Some users might
>> not leverage it.
> 
> This is not typical. If the hardware supports it, we let the end user
> decided if they want to use it or not.
> 
> So if all silicon should have it, enable it everywhere. If there is an
> option to save some gates and leave it out of the silicon, then we do
> need some per device knowledge, or a register which tells us what the
> synthesis options where.
> 

I have looked into all config_debug* registers of multiple versions 
available with us and there is no mention of WOL. I think we can add 
MACB_WOL_CAPS to default_config and advertise in ethtool by default.

Please let me know your suggestions/thoughts.
-- 
🙏 vineeth

Re: [PATCH net-next v2 3/4] net: macb: Enable queue disable and WOL

Posted by Karumanchi, Vineeth 1 year, 11 months ago

Hi Andrew,

On 2/27/2024 10:34 AM, Karumanchi, Vineeth wrote:
> Hi Andrew,
> 
> On 2/23/2024 6:56 PM, Andrew Lunn wrote:
>>> It is not specific to AMD versions. All Cadence GEM IP versions have the
>>> capability, but specific vendors might enable or disable it as per their
>>> requirements.
>>
>> Do you mean it is an option to synthesizer it or not? So although the
>> basic IP licensed from Cadence has it, a silicon vendor could remove
>> it?
>>
> 
> Regarding that, we are unsure. However, based on observation from all 
> previous cadence IP's in AMD Soc's, this feature was available.
> 
>>> WOL was previously enabled via the device-tree attribute. Some users 
>>> might
>>> not leverage it.
>>
>> This is not typical. If the hardware supports it, we let the end user
>> decided if they want to use it or not.
>>
>> So if all silicon should have it, enable it everywhere. If there is an
>> option to save some gates and leave it out of the silicon, then we do
>> need some per device knowledge, or a register which tells us what the
>> synthesis options where.
>>
> 
> I have looked into all config_debug* registers of multiple versions 
> available with us and there is no mention of WOL. I think we can add 
> MACB_WOL_CAPS to default_config and advertise in ethtool by default.
> 
> Please let me know your suggestions/thoughts.


Adding WOL caps to default_gem_config is not effective.
The individual config is overriding default_config.

I only see enabling WOL by default,
- By default, Advertise magic-packet and ARP in ethtool.
- Always set device_set_wakeup_capable to true.
- "magic-packet" property is no longer useful.

Please let me know your suggestions.

-- 
🙏 vineeth