Re: [PATCH] ARM: dts: BCM5301X: Add DT for Asus RT-AC88U

[Florian Fainelli <f.fainelli@gmail.com>]

On 9/22/2021 3:26 PM, Arınç ÜNAL wrote:
> On 22/09/2021 21:15, Florian Fainelli wrote:
>> On 9/21/21 5:19 AM, Arınç ÜNAL wrote:
>>> Hardware Info
>>> -------------
>>>
>>> Processor    - Broadcom BCM4709C0KFEBG dual-core @ 1.4 GHz
>>> Switch        - BCM53012 in BCM4709C0KFEBG & external RTL8365MB
>>
>> There is no Device Tree description of the RTL8365MB switch, can it be
>> driven/controlled via MDIO, SPI or GPIOs by any chance? This is not a
>> show stopper for accepting the patch, just wondering if you are somehow
>> trying to get that switch controlled by the rtl8366 DSA driver as well?
>>
> There's a v1 patch on net-next adding DSA support for RTL8365MB by Alvin 
> Šipraga, CC'ing them. There's also a v2 patch coming.
> https://lore.kernel.org/netdev/20210822193145.1312668-1-alvin@pqrs.dk/
> 
> I've been mailing Alvin to figure out how to define it on the device 
> tree. They have provided very useful information. Quoting a few:
> 
>  >> I'm trying to write the device tree to support this switch. I'm not 
> sure
>  >> whether the default GPIO IDs of mdc-gpios, mdio-gpios, reset-gpios &
>  >> interrupts on realtek-smi.txt kernel documentation are correct.
>  >> 
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/dsa/realtek-smi.txt 
> 
>  >
>  > These gpios are just an example. It really depends how your board is
>  > wired up. You have to figure out which SoC pad is wired to the MDC,
>  > MDIO, and RESET pins on the RTL8365MB. Then you have to make sure the
>  > pinmux is set up correctly so that these pads correspond to some GPIO
>  > with a given ID, and then pick the right GPIO controller (&chipcommon?)
>  > and put the ID after that. It will not necessarily be 21, 22, 14.
> 
>  > In summary:
>  >
>  > - figure out which pads are wired to MDC, MDIO, RESET
>  > - figure out pinmux to make them into gpios
>  > - figure out gpio ID and describe that in the device tree
>  >
> 
> I have backported the v1 patch to kernel 5.10 and tried an example 
> definition on the device tree to test it out on RT-AC88U. It's on this 
> branch:
> https://github.com/arinc9/openwrt/commits/realtek-work-asus_rt-ac88u

Your dsa,member proper looks reversed, you would want it to be:

dsa,member = <1 0>;

to indicate that these are indeed disjoint DSA trees with the tree being 
1 and the switch being member 0 (the one and only). This part of the 
driver/binding looks a bit weird too:

	switch@1 {
+		compatible = "realtek,rtl8365mb";
+		/* 22 = MDIO (has input reads), 21 = MDC (clock, output only) */
+		mdc-gpios = <&chipcommon 6 GPIO_ACTIVE_HIGH>;
+		mdio-gpios = <&chipcommon 7 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&chipcommon 14 GPIO_ACTIVE_LOW>;

this is clearly a MDIO-attached switch, so it should be a children of 
the GPIO controller node. There is a hardware MDIO controller on the 
BCM5301X so you should be able to avoid using bit-banging here and 
instead using the BCM5301X's MDIO controller proper.

> 
> It doesn't work as is, likely missing further configuration, which I'm 
> clueless to figure out myself. I'd very appreciate it if you could weigh 
> in.
> 
> [    1.598858] realtek-smi switch@1: failed to get RESET GPIO
> ---
> [    3.015528] realtek-smi switch@1: deasserted RESET
> [    3.021171] realtek-smi switch@1: found an RTL8365MB-VC switch 
> (ver=0x0040)
> [    3.028193] realtek-smi switch@1: unable to register switch ret = -517
> ---
> [    3.405527] realtek-smi switch@1: deasserted RESET
> [    3.411165] realtek-smi switch@1: found an RTL8365MB-VC switch 
> (ver=0x0040)
> [    3.418449] DSA: tree 0 already setup! Disjoint trees?
> [    3.423607] realtek-smi switch@1: unable to register switch ret = -17
> [    3.430137] realtek-smi: probe of switch@1 failed with error -17
> 
> ---
> 
> I was thinking, we figure out how to define it properly on the device 
> tree and make the driver work whilst the v2 patch is applied to 
> net-next. Then we could send another patch defining the switch on the 
> device tree.
> There's the "compatible = "realtek,rtl8365mb";" property, which would be 
> undefined until the driver is added.

That works for me, which is why I already applied your patch.
-- 
Florian