2023-05-01 12:16:45

by Arınç ÜNAL

[permalink] [raw]
Subject: [PATCH net 1/2] net: dsa: mt7530: fix corrupt frames using trgmii on 40 MHz XTAL MT7621

From: Arınç ÜNAL <[email protected]>

The multi-chip module MT7530 switch with a 40 MHz oscillator on the
MT7621AT, MT7621DAT, and MT7621ST SoCs forwards corrupt frames using
trgmii.

This is caused by the assumption that MT7621 SoCs have got 150 MHz PLL,
hence using the ncpo1 value, 0x0780.

My testing shows this value works on Unielec U7621-06, Bartel's testing
shows it won't work on Hi-Link HLK-MT7621A and Netgear WAC104. All devices
tested have got 40 MHz oscillators.

Using the value for 125 MHz PLL, 0x0640, works on all boards at hand. The
definitions for 125 MHz PLL exist on the Banana Pi BPI-R2 BSP source code
whilst 150 MHz PLL don't.

Forwarding frames using trgmii on the MCM MT7530 switch with a 25 MHz
oscillator on the said MT7621 SoCs works fine because the ncpo1 value
defined for it is for 125 MHz PLL.

Change the 150 MHz PLL comment to 125 MHz PLL, and use the 125 MHz PLL
ncpo1 values for both oscillator frequencies.

Link: https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/81d24bbce7d99524d0771a8bdb2d6663e4eb4faa/u-boot-mt/drivers/net/rt2880_eth.c#L2195
Fixes: 7ef6f6f8d237 ("net: dsa: mt7530: Add MT7621 TRGMII mode support")
Tested-by: Bartel Eerdekens <[email protected]>
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c680873819b0..7d9f9563dbda 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -426,9 +426,9 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
else
ssc_delta = 0x87;
if (priv->id == ID_MT7621) {
- /* PLL frequency: 150MHz: 1.2GBit */
+ /* PLL frequency: 125MHz: 1.0GBit */
if (xtal == HWTRAP_XTAL_40MHZ)
- ncpo1 = 0x0780;
+ ncpo1 = 0x0640;
if (xtal == HWTRAP_XTAL_25MHZ)
ncpo1 = 0x0a00;
} else { /* PLL frequency: 250MHz: 2.0Gbit */
--
2.39.2


2023-05-01 12:17:07

by Arınç ÜNAL

[permalink] [raw]
Subject: [PATCH net 2/2] net: dsa: mt7530: fix network connectivity with multiple CPU ports

From: Arınç ÜNAL <[email protected]>

On mt753x_cpu_port_enable() there's code that enables flooding for the CPU
port only. Since mt753x_cpu_port_enable() runs twice when both CPU ports
are enabled, port 6 becomes the only port to forward the frames to. But
port 5 is the active port, so no frames received from the user ports will
be forwarded to port 5 which breaks network connectivity.

Every bit of the BC_FFP, UNM_FFP, and UNU_FFP bits represents a port. Fix
this issue by setting the bit that corresponds to the CPU port without
overwriting the other bits.

Clear the bits beforehand only for the MT7531 switch. According to the
documents MT7621 Giga Switch Programming Guide v0.3 and MT7531 Reference
Manual for Development Board v1.0, after reset, the BC_FFP, UNM_FFP, and
UNU_FFP bits are set to 1 for MT7531, 0 for MT7530.

Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 7d9f9563dbda..9bc54e1348cb 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1002,9 +1002,9 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
mt7530_write(priv, MT7530_PVC_P(port),
PORT_SPEC_TAG);

- /* Disable flooding by default */
- mt7530_rmw(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK,
- BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port)));
+ /* Enable flooding on the CPU port */
+ mt7530_set(priv, MT7530_MFC, BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) |
+ UNU_FFP(BIT(port)));

/* Set CPU port number */
if (priv->id == ID_MT7621)
@@ -2367,6 +2367,10 @@ mt7531_setup_common(struct dsa_switch *ds)
/* Enable and reset MIB counters */
mt7530_mib_reset(ds);

+ /* Disable flooding on all ports */
+ mt7530_clear(priv, MT7530_MFC, BC_FFP_MASK | UNM_FFP_MASK |
+ UNU_FFP_MASK);
+
for (i = 0; i < MT7530_NUM_PORTS; i++) {
/* Disable forwarding by default on all ports */
mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
--
2.39.2

2023-05-02 18:27:43

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net 1/2] net: dsa: mt7530: fix corrupt frames using trgmii on 40 MHz XTAL MT7621



On 5/1/2023 5:15 AM, [email protected] wrote:
> From: Arınç ÜNAL <[email protected]>
>
> The multi-chip module MT7530 switch with a 40 MHz oscillator on the
> MT7621AT, MT7621DAT, and MT7621ST SoCs forwards corrupt frames using
> trgmii.
>
> This is caused by the assumption that MT7621 SoCs have got 150 MHz PLL,
> hence using the ncpo1 value, 0x0780.
>
> My testing shows this value works on Unielec U7621-06, Bartel's testing
> shows it won't work on Hi-Link HLK-MT7621A and Netgear WAC104. All devices
> tested have got 40 MHz oscillators.
>
> Using the value for 125 MHz PLL, 0x0640, works on all boards at hand. The
> definitions for 125 MHz PLL exist on the Banana Pi BPI-R2 BSP source code
> whilst 150 MHz PLL don't.
>
> Forwarding frames using trgmii on the MCM MT7530 switch with a 25 MHz
> oscillator on the said MT7621 SoCs works fine because the ncpo1 value
> defined for it is for 125 MHz PLL.
>
> Change the 150 MHz PLL comment to 125 MHz PLL, and use the 125 MHz PLL
> ncpo1 values for both oscillator frequencies.
>
> Link: https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/81d24bbce7d99524d0771a8bdb2d6663e4eb4faa/u-boot-mt/drivers/net/rt2880_eth.c#L2195
> Fixes: 7ef6f6f8d237 ("net: dsa: mt7530: Add MT7621 TRGMII mode support")
> Tested-by: Bartel Eerdekens <[email protected]>
> Tested-by: Arınç ÜNAL <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2023-05-02 18:36:30

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net 2/2] net: dsa: mt7530: fix network connectivity with multiple CPU ports



On 5/1/2023 5:15 AM, [email protected] wrote:
> From: Arınç ÜNAL <[email protected]>
>
> On mt753x_cpu_port_enable() there's code that enables flooding for the CPU
> port only. Since mt753x_cpu_port_enable() runs twice when both CPU ports
> are enabled, port 6 becomes the only port to forward the frames to. But
> port 5 is the active port, so no frames received from the user ports will
> be forwarded to port 5 which breaks network connectivity.
>
> Every bit of the BC_FFP, UNM_FFP, and UNU_FFP bits represents a port. Fix
> this issue by setting the bit that corresponds to the CPU port without
> overwriting the other bits.
>
> Clear the bits beforehand only for the MT7531 switch. According to the
> documents MT7621 Giga Switch Programming Guide v0.3 and MT7531 Reference
> Manual for Development Board v1.0, after reset, the BC_FFP, UNM_FFP, and
> UNU_FFP bits are set to 1 for MT7531, 0 for MT7530.
>
> Tested-by: Arınç ÜNAL <[email protected]>

This tag is implied by your Signed-off-by tag. No Fixes tag for this one?
--
Florian

2023-05-02 20:08:34

by Arınç ÜNAL

[permalink] [raw]
Subject: Re: [PATCH net 2/2] net: dsa: mt7530: fix network connectivity with multiple CPU ports

On 02/05/2023 21:30, Florian Fainelli wrote:
>
>
> On 5/1/2023 5:15 AM, [email protected] wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> On mt753x_cpu_port_enable() there's code that enables flooding for the
>> CPU
>> port only. Since mt753x_cpu_port_enable() runs twice when both CPU ports
>> are enabled, port 6 becomes the only port to forward the frames to. But
>> port 5 is the active port, so no frames received from the user ports will
>> be forwarded to port 5 which breaks network connectivity.
>>
>> Every bit of the BC_FFP, UNM_FFP, and UNU_FFP bits represents a port. Fix
>> this issue by setting the bit that corresponds to the CPU port without
>> overwriting the other bits.
>>
>> Clear the bits beforehand only for the MT7531 switch. According to the
>> documents MT7621 Giga Switch Programming Guide v0.3 and MT7531 Reference
>> Manual for Development Board v1.0, after reset, the BC_FFP, UNM_FFP, and
>> UNU_FFP bits are set to 1 for MT7531, 0 for MT7530.
>>
>> Tested-by: Arınç ÜNAL <[email protected]>
>
> This tag is implied by your Signed-off-by tag. No Fixes tag for this one?

I've put it with v2. Let me send v3 to remove the tested-by and add your
reviewed-by to the other patch.

Arınç