Re: [PATCH net-next 2/3] dt-bindings: net: cdns,macb: Add wol-arp-packet property

[Vineeth Karumanchi <vineeth.karumanchi@amd.com>]

Hi Conor,

On 30/01/24 11:00 pm, Conor Dooley wrote:
> On Tue, Jan 30, 2024 at 04:18:44PM +0530, Vineeth Karumanchi wrote:
>> "wol-arp-packet" property enables WOL with ARP packet.
>> It is an extension to "magic-packet for WOL.
> 
> If it is an extension to "magic-packet" why does it not depend on
> "magic-packet"? Are there systems that would only support the magic arp
> packet but a regular magic packet?
> 

The IP version on ZU+ and Versal supports the below combinations for WOL 
event:

1. Magic packet (Wake-on magic packet only)
2. ARP (Wake-on ARP packet only)
3. Magic packet or ARP (Wake-on magic or ARP packets)

The existing DT binding already has one entry for
wol via magic packet. We are adding ARP packet support to the existing 
implementation.

I will change the commit message in v2.

>>
>> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
>> ---
>> 7c4a1d0cfdc1 net: macb: make magic-packet property generic
>> which added magic-property support and wol-arp-packet addition
>> is similar extension.
>> ---
>>   Documentation/devicetree/bindings/net/cdns,macb.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
>> index bf8894a0257e..4bea177e85bc 100644
>> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
>> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
>> @@ -144,6 +144,11 @@ patternProperties:
>>           description:
>>             Indicates that the hardware supports waking up via magic packet.
>>   
>> +      wol-arp-packet:
> 
> Bikeshedding perhaps, but why not call it "magic-arp-packet" if it has
> the same function as the other property here?
> 

Magic packet and ARP packets are two different wol events.
IP supports configuring in the above-mentioned ways.
Hence, I think it would be good to not mix with magic packet.

Please let me know your suggestions/comments.

Thanks,
Vineeth 🙏

> Thanks,
> Conor.
> 
>> +        type: boolean
>> +        description:
>> +          Indicates that the hardware supports waking up via ARP packet.
>> +
>>       unevaluatedProperties: false
>>   
>>   required:
>> -- 
>> 2.34.1
>>