mbox series

[net-next,0/2] net: mediatek: Add MT7621 TRGMII mode support

Message ID 20190616182010.18778-1-opensource@vdorst.com (mailing list archive)
Headers show
Series net: mediatek: Add MT7621 TRGMII mode support | expand

Message

René van Dorst June 16, 2019, 6:20 p.m. UTC
Like many other mediatek SOCs, the MT7621 SOC and the internal MT7530 switch both
supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.

René van Dorst (2):
  net: ethernet: mediatek: Add MT7621 TRGMII mode support
  net: dsa: mt7530: Add MT7621 TRGMII mode support

 drivers/net/dsa/mt7530.c                    | 15 ++++++--
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 38 ++++++++++++++++++---
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 11 ++++++
 3 files changed, 58 insertions(+), 6 deletions(-)

Comments

Andrew Lunn June 17, 2019, 2:02 p.m. UTC | #1
On Sun, Jun 16, 2019 at 08:20:08PM +0200, René van Dorst wrote:
> Like many other mediatek SOCs, the MT7621 SOC and the internal MT7530 switch both
> supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.

Hi René

Is TRGMII used only between the SoC and the Switch? Or does external
ports of the switch also support 1200Mbit/s? If external ports support
this, what does ethtool show for Speed?

      Thanks
	Andrew
René van Dorst June 17, 2019, 9:33 p.m. UTC | #2
Quoting Andrew Lunn <andrew@lunn.ch>:

> On Sun, Jun 16, 2019 at 08:20:08PM +0200, René van Dorst wrote:
>> Like many other mediatek SOCs, the MT7621 SOC and the internal  
>> MT7530 switch both
>> supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.
>
> Hi René
>

Hi Andrew,

> Is TRGMII used only between the SoC and the Switch? Or does external
> ports of the switch also support 1200Mbit/s? If external ports support
> this, what does ethtool show for Speed?

Only the first GMAC of the SOC and port 6 of the switch supports this mode.
The switch can be internal in the SOC but also a separate chip.

PHYLINK and ethertool reports the link as 1Gbit.
The link is fixed-link with speed = 1000.

dmesg output with unposted PHYLINK patches:
[    5.236763] mt7530 mdio-bus:1f: configuring for fixed/trgmii link mode
[    5.249813] mt7530 mdio-bus:1f: phylink_mac_config:  
mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1  
an=1
[    6.389435] mtk_soc_eth 1e100000.ethernet eth0: phylink_mac_config:  
mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1  
an=1

# ethtool eth0
Settings for eth0:
          Supported ports: [ MII ]
          Supported link modes:   1000baseT/Full
          Supported pause frame use: No
          Supports auto-negotiation: No
          Supported FEC modes: Not reported
          Advertised link modes:  1000baseT/Full
          Advertised pause frame use: No
          Advertised auto-negotiation: No
          Advertised FEC modes: Not reported
          Speed: 1000Mb/s
          Duplex: Full
          Port: MII
          PHYAD: 0
          Transceiver: internal
          Auto-negotiation: on
          Current message level: 0x000000ff (255)
                                 drv probe link timer ifdown ifup rx_err tx_err
          Link detected: yes



I already have report from a MT7623 user that this patch gives issues.

I send v2 of the patch if I fixed that issue.

Also I think it is better to add a XTAL frequency check.
The PLL values are only valid with a 40MHz crystal.

Any other comments for v2?

Greats,

René


>
>      Thanks
> 	Andrew
Andrew Lunn June 17, 2019, 9:44 p.m. UTC | #3
On Mon, Jun 17, 2019 at 09:33:12PM +0000, René van Dorst wrote:
> Quoting Andrew Lunn <andrew@lunn.ch>:
> 
> >On Sun, Jun 16, 2019 at 08:20:08PM +0200, René van Dorst wrote:
> >>Like many other mediatek SOCs, the MT7621 SOC and the internal MT7530
> >>switch both
> >>supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.
> >
> >Hi René
> >
> 
> Hi Andrew,
> 
> >Is TRGMII used only between the SoC and the Switch? Or does external
> >ports of the switch also support 1200Mbit/s? If external ports support
> >this, what does ethtool show for Speed?
> 
> Only the first GMAC of the SOC and port 6 of the switch supports this mode.
> The switch can be internal in the SOC but also a separate chip.
> 
> PHYLINK and ethertool reports the link as 1Gbit.
> The link is fixed-link with speed = 1000.
> 
> dmesg output with unposted PHYLINK patches:
> [    5.236763] mt7530 mdio-bus:1f: configuring for fixed/trgmii link mode
> [    5.249813] mt7530 mdio-bus:1f: phylink_mac_config:
> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1
> [    6.389435] mtk_soc_eth 1e100000.ethernet eth0: phylink_mac_config:
> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1

With PHYLINK, you can probably set the fixed link to the true 1.2Gbps.

> # ethtool eth0
> Settings for eth0:
>          Supported ports: [ MII ]
>          Supported link modes:   1000baseT/Full
>          Supported pause frame use: No
>          Supports auto-negotiation: No
>          Supported FEC modes: Not reported
>          Advertised link modes:  1000baseT/Full
>          Advertised pause frame use: No
>          Advertised auto-negotiation: No
>          Advertised FEC modes: Not reported
>          Speed: 1000Mb/s

We could consider adding 1200BaseT/Full?

   Andrew
Florian Fainelli June 17, 2019, 10:19 p.m. UTC | #4
On 6/17/19 2:33 PM, René van Dorst wrote:
> Quoting Andrew Lunn <andrew@lunn.ch>:
> 
>> On Sun, Jun 16, 2019 at 08:20:08PM +0200, René van Dorst wrote:
>>> Like many other mediatek SOCs, the MT7621 SOC and the internal MT7530
>>> switch both
>>> supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.
>>
>> Hi René
>>
> 
> Hi Andrew,
> 
>> Is TRGMII used only between the SoC and the Switch? Or does external
>> ports of the switch also support 1200Mbit/s? If external ports support
>> this, what does ethtool show for Speed?
> 
> Only the first GMAC of the SOC and port 6 of the switch supports this mode.
> The switch can be internal in the SOC but also a separate chip.
> 
> PHYLINK and ethertool reports the link as 1Gbit.
> The link is fixed-link with speed = 1000.
> 
> dmesg output with unposted PHYLINK patches:
> [    5.236763] mt7530 mdio-bus:1f: configuring for fixed/trgmii link mode
> [    5.249813] mt7530 mdio-bus:1f: phylink_mac_config:
> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1
> [    6.389435] mtk_soc_eth 1e100000.ethernet eth0: phylink_mac_config:
> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1
> 
> # ethtool eth0
> Settings for eth0:
>          Supported ports: [ MII ]
>          Supported link modes:   1000baseT/Full
>          Supported pause frame use: No
>          Supports auto-negotiation: No
>          Supported FEC modes: Not reported
>          Advertised link modes:  1000baseT/Full
>          Advertised pause frame use: No
>          Advertised auto-negotiation: No
>          Advertised FEC modes: Not reported
>          Speed: 1000Mb/s
>          Duplex: Full
>          Port: MII
>          PHYAD: 0
>          Transceiver: internal
>          Auto-negotiation: on
>          Current message level: 0x000000ff (255)
>                                 drv probe link timer ifdown ifup rx_err
> tx_err
>          Link detected: yes
> 
> 
> 
> I already have report from a MT7623 user that this patch gives issues.
> 
> I send v2 of the patch if I fixed that issue.
> 
> Also I think it is better to add a XTAL frequency check.
> The PLL values are only valid with a 40MHz crystal.
> 
> Any other comments for v2?

Looks good to me otherwise.
René van Dorst June 17, 2019, 11:20 p.m. UTC | #5
Quoting Andrew Lunn <andrew@lunn.ch>:

Hi Andrew,

> On Mon, Jun 17, 2019 at 09:33:12PM +0000, René van Dorst wrote:
>> Quoting Andrew Lunn <andrew@lunn.ch>:
>>
>> >On Sun, Jun 16, 2019 at 08:20:08PM +0200, René van Dorst wrote:
>> >>Like many other mediatek SOCs, the MT7621 SOC and the internal MT7530
>> >>switch both
>> >>supports TRGMII mode. MT7621 TRGMII speed is 1200MBit.
>> >
>> >Hi René
>> >
>>
>> Hi Andrew,
>>
>> >Is TRGMII used only between the SoC and the Switch? Or does external
>> >ports of the switch also support 1200Mbit/s? If external ports support
>> >this, what does ethtool show for Speed?
>>
>> Only the first GMAC of the SOC and port 6 of the switch supports this mode.
>> The switch can be internal in the SOC but also a separate chip.
>>
>> PHYLINK and ethertool reports the link as 1Gbit.
>> The link is fixed-link with speed = 1000.
>>
>> dmesg output with unposted PHYLINK patches:
>> [    5.236763] mt7530 mdio-bus:1f: configuring for fixed/trgmii link mode
>> [    5.249813] mt7530 mdio-bus:1f: phylink_mac_config:
>> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1
>> [    6.389435] mtk_soc_eth 1e100000.ethernet eth0: phylink_mac_config:
>> mode=fixed/trgmii/1Gbps/Full adv=00,00000000,00000220 pause=12 link=1 an=1
>
> With PHYLINK, you can probably set the fixed link to the true 1.2Gbps.

By adding some extra speed states in the code it seems to work.

+               if (state->speed == 1200)
+                       mcr |= PMCR_FORCE_SPEED_1000;

dmesg:
[    5.261375] mt7530 mdio-bus:1f: configuring for fixed/trgmii link mode
[    5.274390] mt7530 mdio-bus:1f: phylink_mac_config:  
mode=fixed/trgmii/Unsupported (update phy-core.c)/Full  
adv=00,00000000,00000200 pause=12 link=1 an=1
[    6.296614] mtk_soc_eth 1e100000.ethernet eth0: configuring for  
fixed/trgmii link mode
[    6.313608] mtk_soc_eth 1e100000.ethernet eth0: phylink_mac_config:  
mode=fixed/trgmii/Unsupported (update phy core.c)/Full  
adv=00,00000000,00000200 pause=12 link=1 an=1

# ethtool eth0
Settings for eth0:
         Supported ports: [ MII ]
         Supported link modes:   Not reported
         Supported pause frame use: No
         Supports auto-negotiation: No
         Supported FEC modes: Not reported
         Advertised link modes:  Not reported
         Advertised pause frame use: No
         Advertised auto-negotiation: No
         Advertised FEC modes: Not reported
         Speed: 1200Mb/s
         Duplex: Full
         Port: MII
         PHYAD: 0
         Transceiver: internal
         Auto-negotiation: on
         Current message level: 0x000000ff (255)
                                drv probe link timer ifdown ifup rx_err tx_err
         Link detected: yes


>> # ethtool eth0
>> Settings for eth0:
>>          Supported ports: [ MII ]
>>          Supported link modes:   1000baseT/Full
>>          Supported pause frame use: No
>>          Supports auto-negotiation: No
>>          Supported FEC modes: Not reported
>>          Advertised link modes:  1000baseT/Full
>>          Advertised pause frame use: No
>>          Advertised auto-negotiation: No
>>          Advertised FEC modes: Not reported
>>          Speed: 1000Mb/s
>
> We could consider adding 1200BaseT/Full?

I don't have any opinion about this.
It is great that it shows nicely in ethtool but I think supporting more
speeds in phy_speed_to_str() is enough.

Also you may want to add other SOCs trgmii ranges too:
- 1200BaseT/Full for mt7621 only
- 2000BaseT/Full for mt7623 and mt7683
- 2600BaseT/Full for mt7623 only

I leave the decision to you.

Greats,

René

>
>    Andrew
Andrew Lunn June 18, 2019, 1:53 a.m. UTC | #6
> By adding some extra speed states in the code it seems to work.
> 
> +               if (state->speed == 1200)
> +                       mcr |= PMCR_FORCE_SPEED_1000;

Hi René

Is TRGMII always 1.2G? Or can you set it to 1000 or 1200? This
PMCR_FORCE_SPEED_1000 feels wrong.

> >We could consider adding 1200BaseT/Full?
> 
> I don't have any opinion about this.
> It is great that it shows nicely in ethtool but I think supporting more
> speeds in phy_speed_to_str() is enough.
> 
> Also you may want to add other SOCs trgmii ranges too:
> - 1200BaseT/Full for mt7621 only
> - 2000BaseT/Full for mt7623 and mt7683
> - 2600BaseT/Full for mt7623 only

Are these standardised in any way? Or MTK proprietary?  Also, is the T
in BaseT correct? These speeds work over copper cables? Or should we
be talking about 1200BaseKX?

   Thanks
	Andrew
Florian Fainelli June 18, 2019, 2:21 a.m. UTC | #7
On 6/17/2019 6:53 PM, Andrew Lunn wrote:
>> By adding some extra speed states in the code it seems to work.
>>
>> +               if (state->speed == 1200)
>> +                       mcr |= PMCR_FORCE_SPEED_1000;
> 
> Hi René
> 
> Is TRGMII always 1.2G? Or can you set it to 1000 or 1200? This
> PMCR_FORCE_SPEED_1000 feels wrong.

It is not uncommon to have to "force" 1G to get a higher speed, there is
something similar with B53 switches configuring the CPU ports at 2GB/sec
(proprietary too and not standardized either).

> 
>>> We could consider adding 1200BaseT/Full?
>>
>> I don't have any opinion about this.
>> It is great that it shows nicely in ethtool but I think supporting more
>> speeds in phy_speed_to_str() is enough.
>>
>> Also you may want to add other SOCs trgmii ranges too:
>> - 1200BaseT/Full for mt7621 only
>> - 2000BaseT/Full for mt7623 and mt7683
>> - 2600BaseT/Full for mt7623 only
> 
> Are these standardised in any way? Or MTK proprietary?  Also, is the T
> in BaseT correct? These speeds work over copper cables? Or should we
> be talking about 1200BaseKX?

Looks like this is MTK proprietary:

http://lists.infradead.org/pipermail/linux-mediatek/2016-September/007083.html
https://patchwork.kernel.org/patch/9341129/
René van Dorst June 18, 2019, 11:46 a.m. UTC | #8
Quoting Florian Fainelli <f.fainelli@gmail.com>:

Hi Andrew and Florian,

> On 6/17/2019 6:53 PM, Andrew Lunn wrote:
>>> By adding some extra speed states in the code it seems to work.
>>>
>>> +               if (state->speed == 1200)
>>> +                       mcr |= PMCR_FORCE_SPEED_1000;
>>
>> Hi René
>>
>> Is TRGMII always 1.2G? Or can you set it to 1000 or 1200?

In case of the MT7621 SOC yes, according to the SDKs the MT7623 has 2 options
2GBit and 2.6Gbit. The current mt7530 driver only set TRGMII speed at 2Gbit.

>> This PMCR_FORCE_SPEED_1000 feels wrong.
>
> It is not uncommon to have to "force" 1G to get a higher speed, there is
> something similar with B53 switches configuring the CPU ports at 2GB/sec
> (proprietary too and not standardized either).

On the SOC MAC side it is basicly only a MAC clock change.
MAC control registers still need to be set forced 1G.

>
>>
>>>> We could consider adding 1200BaseT/Full?
>>>
>>> I don't have any opinion about this.
>>> It is great that it shows nicely in ethtool but I think supporting more
>>> speeds in phy_speed_to_str() is enough.
>>>
>>> Also you may want to add other SOCs trgmii ranges too:
>>> - 1200BaseT/Full for mt7621 only
>>> - 2000BaseT/Full for mt7623 and mt7683
>>> - 2600BaseT/Full for mt7623 only
>>
>> Are these standardised in any way? Or MTK proprietary?  Also, is the T
>> in BaseT correct? These speeds work over copper cables? Or should we
>> be talking about 1200BaseKX?
>
> Looks like this is MTK proprietary:
>
> http://lists.infradead.org/pipermail/linux-mediatek/2016-September/007083.html
> https://patchwork.kernel.org/patch/9341129/
> --
> Florian

MTK proprietary, But I think it is equal too the RGMII but with a  
faster clock.

But do we need a "xxxxBaseT/Full" at all for these fixed-link cases?
If I am correct a "xxxxBaseT/Full" is only needed to automatically select the
best option. But with fixed-link we force it so extra "xxxxBaseT/Full" is not
needed.

Greats,

René