[PATCH 5/5] ARM: dts: BCM5301X: Disable unused gmac0 and gmac2 on Asus RT-AC88U

[Arınç ÜNAL]

On 06/04/2022 21:16, Florian Fainelli wrote:
> On 4/1/22 04:36, Arınç ÜNAL wrote:
>> On 01/04/2022 13:40, Rafał Miłecki wrote:
>>> On 2022-04-01 12:20, Arınç ÜNAL wrote:
>>>> Disable gmac0 and gmac2 which are currently not used. This doesn't 
>>>> seem to
>>>> be implemented yet on drivers/net/ethernet/broadcom/bgmac-bcma.c but 
>>>> this
>>>> change is harmless, nonetheless.
>>>
>>> It doesn't matter whether Linux respects that.
>>>
>>>
>>>> Signed-off-by: Arınç ÜNAL <arinc.unal at arinc9.com>
>>>> ---
>>>>  arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> index 2f944d1c0330..0f5c5d576814 100644
>>>> --- a/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> +++ b/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dts
>>>> @@ -242,11 +242,19 @@ fixed-link {
>>>>      };
>>>>  };
>>>>
>>>> +&gmac0 {
>>>> +    status = "disabled";
>>>> +};
>>>> +
>>>>  &gmac1 {
>>>>      nvmem-cells = <&et1macaddr>;
>>>>      nvmem-cell-names = "mac-address";
>>>>  };
>>>>
>>>> +&gmac2 {
>>>> +    status = "disabled";
>>>> +};
>>>
>>> I don't think that is correct. Those interfaces are still there and
>>> they are actually connected to switch ports. If you configure your
>>> switch properly you can use them.
>>>
>>> Someone may want to use e.g. gmac0 & gmac1 with two sets of ports to
>>> speed up network communication.
>>>
>>> I think gmac2 is required if you want to enable FA (flow acceleration /
>>> accelerator) - even though there isn't Linux driver for it yet.
>>>
>>> They are not disabled / unpopulated / non functional interfaces.
>>
>> I understand your point. However, while we're not supposed to care 
>> whether the kernel respects the bindings, don't we also need to make 
>> the bindings work on the version of the Linux kernel we're submitting 
>> the bindings to?
>>
>> With the current way DSA works, only one switch port can be used as a 
>> CPU port. If we were to remove the status = "disabled" property from 
>> port at 8 which connects to gmac2, it'd break the communication between 
>> the switch and the CPU on the current Linux kernel.
> 
> Are you sure? Because DSA still picks up the CPU port from lowest port 
> to highest port number. If port 6 is enabled, then it should take 
> precedence.

I just tested this on the router. You're right. We can keep port at 8 
enabled. I also made sure gmac0 is not connected to any switch GMAC. 
I'll send a patch to address these.

> 
>>
>> If a new driver or a feature is introduced, we should update the 
>> bindings accordingly afterwards.
>>
>> For this reason, I don't see an issue with explaining the driver side 
>> of it on the commit log for DT bindings.
>>
>> DT bindings are not exactly static either. Someone could want to use 
>> gmac2 instead of gmac1. In that case, I think they should change the 
>> bindings themselves as it's for their own use.
> 
> This is true, but we are not changing the binding here, the binding is 
> the contract between the DTB provider and the DTB consumer, it describes 
> properties, their shape and size etc. We are just changing the status of 
> particular nodes.

Understood, thank you.

By the way, I don't see my applied patches on the link you referred to.

Arınç