MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Arınç ÜNAL]

[-- Attachment #1: Type: text/plain, Size: 2200 bytes --]

Hi all,

The company I currently work for has got an Ralink mt7621a board with an 
external phy connected. It's a Realtek rtl8367s switch.
I've been running gregkh/staging staging-next & netdev/net-next master 
branches with Sergio's "clk: ralink: make system controller a reset 
provider" v8 patch series.

We don't have traffic flow on the RGMII2 bus which is shared by the 2nd 
GMAC of the SoC, MT7530's GMAC5 and an external phy (rtl switch in our 
case).

According to Documentation/devicetree/bindings/net/dsa/mt7530.txt, I can 
either configure the external phy to connect to the second GMAC of the 
mt7621 SoC or to MT7530's GMAC5 to create a cascade.

None of the documented configurations work:
External phy <-> 2nd GMAC
External phy <-> MT7530's GMAC5

The external switch works with Mediatek SDK ethernet driver on External 
phy <-> 2nd GMAC mode.

I suspect there is a problem with the mtk_eth_soc driver on upstream.
Same issue on 5.10 (OpenWrt Master) and 4.14 (OpenWrt 19.07)

The board's RTL8367S schematics is in the attachments.

Dumbed down wiring scheme:
                             CPU
                      ┌───────────────┐
                      │ GMAC0 | GMAC1 │
                      └───┼───────┼───┘
                          │       │
             ┌────────────┼┐      │
      MT7530 │0 1 2 3 4 5 6│      │
             └─────────────┘      │
                          ┌───────┘
             ┌────────────┼┐
    RTL8367S │0 1 2 3 4 6 7│
             └┼─┼─┼─┼─┼────┘
      ┌───────┘ │ │ │ └───────┐
      │     ┌───┘ │ └───┐     │
      │     │     │     │     │
      │     │     │     │     │
  ┌───┼─────┼─────┼─────┼─────┼───┐
  │ sw1p0 sw1p1 sw1p2 sw1p3 sw1p4 │
  └───────────────────────────────┘

Cheers.
Arınç

[-- Attachment #2: Xeront_7531_8367.pdf --]
[-- Type: application/pdf, Size: 361521 bytes --]

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[DENG Qingfang]

Hi,

Do you set the ethernet pinmux correctly?

&ethernet {
    pinctrl-names = "default";
    pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
};

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Arınç ÜNAL]

Hey Deng,

On 23/01/2022 09:51, DENG Qingfang wrote:
> Hi,
> 
> Do you set the ethernet pinmux correctly?
> 
> &ethernet {
>      pinctrl-names = "default";
>      pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
> };

This fixed it! We did have &rgmii2_pins on the gmac1 node (it was 
originally on external_phy) so we never thought to investigate the 
pinctrl configuration further! Turns out &rgmii2_pins needs to be 
defined on the ethernet node instead.

I'll send a patch to address this on mt7621.dtsi.

You saved us a bunch of time, thank you very much!

Cheers.
Arınç

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Andrew Lunn]

On Sun, Jan 23, 2022 at 11:33:04AM +0300, Arınç ÜNAL wrote:
> Hey Deng,
> 
> On 23/01/2022 09:51, DENG Qingfang wrote:
> > Hi,
> > 
> > Do you set the ethernet pinmux correctly?
> > 
> > &ethernet {
> >      pinctrl-names = "default";
> >      pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
> > };
> 
> This fixed it! We did have &rgmii2_pins on the gmac1 node (it was originally
> on external_phy) so we never thought to investigate the pinctrl
> configuration further! Turns out &rgmii2_pins needs to be defined on the
> ethernet node instead.

PHYs are generally external, so pinmux on them makes no sense. PHYs in
DT are not devices in the usual sense, so i don't think the driver
core will handle pinmux for them, even if you did list them.

This could be interesting for the DT compliance checker. Ideally we
want it to warn if it finds a pinmux configuration in a PHY node.

It also sounds like you had them somewhere else wrong?

     Andrew

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Arınç ÜNAL]

On 23/01/2022 18:26, Andrew Lunn wrote:
> On Sun, Jan 23, 2022 at 11:33:04AM +0300, Arınç ÜNAL wrote:
>> Hey Deng,
>>
>> On 23/01/2022 09:51, DENG Qingfang wrote:
>>> Hi,
>>>
>>> Do you set the ethernet pinmux correctly?
>>>
>>> &ethernet {
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
>>> };
>>
>> This fixed it! We did have &rgmii2_pins on the gmac1 node (it was originally
>> on external_phy) so we never thought to investigate the pinctrl
>> configuration further! Turns out &rgmii2_pins needs to be defined on the
>> ethernet node instead.
> 
> PHYs are generally external, so pinmux on them makes no sense. PHYs in
> DT are not devices in the usual sense, so i don't think the driver
> core will handle pinmux for them, even if you did list them.
> 
> This could be interesting for the DT compliance checker. Ideally we
> want it to warn if it finds a pinmux configuration in a PHY node.

I don't see any warnings about it:

$ cpp -nostdinc -I include -I arch -undef -x assembler-with-cpp 
drivers/staging/mt7621-dts/mt7621.dtsi mt7621.dtsi.preprocessed
$ dtc -I dts -O dtb -p 0x1000 mt7621.dtsi.preprocessed -o mt7621.dtb
drivers/staging/mt7621-dts/mt7621.dtsi:28.21-33.4: Warning 
(unit_address_vs_reg): /cpuintc@0: node has a unit name, but no reg property
drivers/staging/mt7621-dts/mt7621.dtsi:40.34-47.6: Warning 
(unit_address_vs_reg): /fixedregulator@0: node has a unit name, but no 
reg property
drivers/staging/mt7621-dts/mt7621.dtsi:49.39-56.4: Warning 
(unit_address_vs_reg): /fixedregulator@1: node has a unit name, but no 
reg property
drivers/staging/mt7621-dts/mt7621.dtsi:410.11-449.7: Warning 
(unit_address_vs_reg): /ethernet@1e100000/mdio-bus/switch0@0/ports: node 
has a reg or ranges property, but no unit name
drivers/staging/mt7621-dts/mt7621.dtsi:28.21-33.4: Warning 
(unique_unit_address): /cpuintc@0: duplicate unit-address (also used in 
node /fixedregulator@0)

> 
> It also sounds like you had them somewhere else wrong?

Yes, it was under the phy_external node:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/mt7621-dts/mt7621.dtsi#n350

I had put it under gmac1 node instead since we didn't enable the 
phy_external node.

Arınç

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Florian Fainelli]

On 1/23/2022 7:26 AM, Andrew Lunn wrote:
> On Sun, Jan 23, 2022 at 11:33:04AM +0300, Arınç ÜNAL wrote:
>> Hey Deng,
>>
>> On 23/01/2022 09:51, DENG Qingfang wrote:
>>> Hi,
>>>
>>> Do you set the ethernet pinmux correctly?
>>>
>>> &ethernet {
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
>>> };
>>
>> This fixed it! We did have &rgmii2_pins on the gmac1 node (it was originally
>> on external_phy) so we never thought to investigate the pinctrl
>> configuration further! Turns out &rgmii2_pins needs to be defined on the
>> ethernet node instead.
> 
> PHYs are generally external, so pinmux on them makes no sense. PHYs in
> DT are not devices in the usual sense, so i don't think the driver
> core will handle pinmux for them, even if you did list them.

Not sure I understand your comment here, this is configuring the pinmux 
on the SoC side in order for the second RGMII interface's data path to work.

It is not uncommon for the same set of I/O pads to be used by different 
functions within the chip. For instance the chips I work with happily 
offer RGMII, MTSIF, PDM (I2S), TSIO on the same pads via different 
pinmuxing options.

Also, this is declaring a pinmuxing function for the Ethernet MAC, which 
is a perfectly valid use case and typically how pinmuxing is declared 
for a given SoC.
-- 
Florian

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Russell King (Oracle)]

On Mon, Jan 24, 2022 at 09:13:38AM -0800, Florian Fainelli wrote:
> On 1/23/2022 7:26 AM, Andrew Lunn wrote:
> > On Sun, Jan 23, 2022 at 11:33:04AM +0300, Arınç ÜNAL wrote:
> > > Hey Deng,
> > > 
> > > On 23/01/2022 09:51, DENG Qingfang wrote:
> > > > Hi,
> > > > 
> > > > Do you set the ethernet pinmux correctly?
> > > > 
> > > > &ethernet {
> > > >       pinctrl-names = "default";
> > > >       pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
> > > > };
> > > 
> > > This fixed it! We did have &rgmii2_pins on the gmac1 node (it was originally
> > > on external_phy) so we never thought to investigate the pinctrl
> > > configuration further! Turns out &rgmii2_pins needs to be defined on the
> > > ethernet node instead.
> > 
> > PHYs are generally external, so pinmux on them makes no sense. PHYs in
> > DT are not devices in the usual sense, so i don't think the driver
> > core will handle pinmux for them, even if you did list them.
> 
> Not sure I understand your comment here, this is configuring the pinmux on
> the SoC side in order for the second RGMII interface's data path to work.

The pinmux configuration was listed under the external PHY node, which
is qutie unusual. In the case of phylib and external ethernet PHYs,
this can be a problem.

The pinmux configuration is normally handled at device probe time by
the device model, but remember phylib bypasses that when it attaches
the generic PHY driver - meaning you don't get the pinmux configured.

What this means is that pinmux configuration in ethernet PHY nodes is
unreliable. It will only happen if we have a specific driver for the
PHY and the driver model binds that driver.

Of course, if we killed the generic driver, that would get around this
issue by requiring every PHY to have its own specific driver, but there
would be many complaints because likely lots would stop working.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: MT7621 SoC Traffic Won't Flow on RGMII2 Bus/2nd GMAC

[Florian Fainelli]

On 1/24/2022 9:26 AM, Russell King (Oracle) wrote:
> On Mon, Jan 24, 2022 at 09:13:38AM -0800, Florian Fainelli wrote:
>> On 1/23/2022 7:26 AM, Andrew Lunn wrote:
>>> On Sun, Jan 23, 2022 at 11:33:04AM +0300, Arınç ÜNAL wrote:
>>>> Hey Deng,
>>>>
>>>> On 23/01/2022 09:51, DENG Qingfang wrote:
>>>>> Hi,
>>>>>
>>>>> Do you set the ethernet pinmux correctly?
>>>>>
>>>>> &ethernet {
>>>>>        pinctrl-names = "default";
>>>>>        pinctrl-0 = <&rgmii1_pins &rgmii2_pins &mdio_pins>;
>>>>> };
>>>>
>>>> This fixed it! We did have &rgmii2_pins on the gmac1 node (it was originally
>>>> on external_phy) so we never thought to investigate the pinctrl
>>>> configuration further! Turns out &rgmii2_pins needs to be defined on the
>>>> ethernet node instead.
>>>
>>> PHYs are generally external, so pinmux on them makes no sense. PHYs in
>>> DT are not devices in the usual sense, so i don't think the driver
>>> core will handle pinmux for them, even if you did list them.
>>
>> Not sure I understand your comment here, this is configuring the pinmux on
>> the SoC side in order for the second RGMII interface's data path to work.
> 
> The pinmux configuration was listed under the external PHY node, which
> is qutie unusual. In the case of phylib and external ethernet PHYs,
> this can be a problem.
> 
> The pinmux configuration is normally handled at device probe time by
> the device model, but remember phylib bypasses that when it attaches
> the generic PHY driver - meaning you don't get the pinmux configured.
> 
> What this means is that pinmux configuration in ethernet PHY nodes is
> unreliable. It will only happen if we have a specific driver for the
> PHY and the driver model binds that driver.
> 
> Of course, if we killed the generic driver, that would get around this
> issue by requiring every PHY to have its own specific driver, but there
> would be many complaints because likely lots would stop working.

I suppose that explains why this is still in staging then :) Andrew's 
answer makes more sense now, thanks.
-- 
Florian