[PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[gerg]

I have been working towards supporting the MT7530 switch as used in the
MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
a dual core MIPS CPU architecture. But underneath it is what appears to
be the same 7530 switch.

The following 3 patches are more of an RFC than anything. They allow
use of the mt7530 dsa driver on this device - though with some issues
still to resolve. The primary change required is to not use the 7623
specific clock and regulator setup - none of that applies when using
the 7621 (and maybe other devices?). The other change required is to
set the 7530 MFC register CPU port number and enable bit.

The unresolved issues I still have appear to be more related to the
MT7621 ethernet driver (drivers/staging/mt7621-eth/*). I am hoping
someone might have some ideas on these. I don't really have any good
documentation on the ethernet devices on the 7621, so I am kind of
working in the dark here.

1. TX packets are not getting an IP header checksum via the normal
   off-loaded checksumming when in DSA mode. I have to switch off
   NETIF_F_IP_CSUM, so the software stack generates the checksum.
   That checksum offloading works ok when not using the 7530 DSA driver.

2. Maximal sized RX packets get silently dropped. So receive side packets
   that are large (perfect case is the all-but-last packets in a fragemented
   larger packet) appear to be dropped at the mt7621 ethernet MAC level.
   The 7530 MIB switch register counters show receive packets at the physical
   switch port side and at the CPU switch port - but I get no packets
   received or errors in the 7621 ethernet MAC. If I set the mtu of the
   server at the other end a little smaller (a few bytes is enough) then
   I get all the packets through. It seems like the DSA/VLAN tag bytes
   are causing a too large packet to get silently dropped somewhere.

Any thoughts greatly appreciated...

 Documentation/devicetree/bindings/net/dsa/mt7530.txt |    5 +
 drivers/net/dsa/mt7530.c                             |   95 ++++++++++++-------
 drivers/net/dsa/mt7530.h                             |    5 +
 3 files changed, 70 insertions(+), 35 deletions(-)

[PATCH 1/3] net: dsa: mt7530: make clock/regulator setup optional

[gerg]

From: Greg Ungerer <gerg@kernel.org>

At least one device that contains the MediaTek MT7530 switch module
does not need the clock and regulator setup parts of the MT7530 DSA
driver. That setup looks to be very specific to the MT7623.

The MediaTek MT7621 SoC device contains a 7530 switch, but its MIPS CPU
cores and overall architecture do not have any of the same clock or
regulator setup as the MT7623.

Create a new devicetree tag, mediatek,no-clock-regulator, that controls
whether we should setup and use the clocks and regulators specific to
some uses of the 7530.

Signed-off-by: Greg Ungerer <gerg@kernel.org>
---
 drivers/net/dsa/mt7530.c | 86 ++++++++++++++++++++++++----------------
 drivers/net/dsa/mt7530.h |  1 +
 2 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a5de9bffe5be..532240c4bef9 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -625,14 +625,16 @@ static void mt7530_adjust_link(struct dsa_switch *ds, int port,
 		dev_dbg(priv->dev, "phy-mode for master device = %x\n",
 			phydev->interface);
 
-		/* Setup TX circuit incluing relevant PAD and driving */
-		mt7530_pad_clk_setup(ds, phydev->interface);
-
-		/* Setup RX circuit, relevant PAD and driving on the host
-		 * which must be placed after the setup on the device side is
-		 * all finished.
-		 */
-		mt7623_pad_clk_setup(ds);
+		if (priv->clkreg) {
+			/* Setup TX circuit incluing relevant PAD and driving */
+			mt7530_pad_clk_setup(ds, phydev->interface);
+
+			/* Setup RX circuit, relevant PAD and driving on the
+			 * host which must be placed after the setup on the
+			 * device side is all finished.
+			 */
+			mt7623_pad_clk_setup(ds);
+		}
 	} else {
 		u16 lcl_adv = 0, rmt_adv = 0;
 		u8 flowctrl;
@@ -1219,24 +1221,27 @@ mt7530_setup(struct dsa_switch *ds)
 	 * as two netdev instances.
 	 */
 	dn = ds->ports[MT7530_CPU_PORT].master->dev.of_node->parent;
-	priv->ethernet = syscon_node_to_regmap(dn);
-	if (IS_ERR(priv->ethernet))
-		return PTR_ERR(priv->ethernet);
 
-	regulator_set_voltage(priv->core_pwr, 1000000, 1000000);
-	ret = regulator_enable(priv->core_pwr);
-	if (ret < 0) {
-		dev_err(priv->dev,
-			"Failed to enable core power: %d\n", ret);
-		return ret;
-	}
+	if (priv->clkreg) {
+		priv->ethernet = syscon_node_to_regmap(dn);
+		if (IS_ERR(priv->ethernet))
+			return PTR_ERR(priv->ethernet);
+
+		regulator_set_voltage(priv->core_pwr, 1000000, 1000000);
+		ret = regulator_enable(priv->core_pwr);
+		if (ret < 0) {
+			dev_err(priv->dev,
+				"Failed to enable core power: %d\n", ret);
+			return ret;
+		}
 
-	regulator_set_voltage(priv->io_pwr, 3300000, 3300000);
-	ret = regulator_enable(priv->io_pwr);
-	if (ret < 0) {
-		dev_err(priv->dev, "Failed to enable io pwr: %d\n",
-			ret);
-		return ret;
+		regulator_set_voltage(priv->io_pwr, 3300000, 3300000);
+		ret = regulator_enable(priv->io_pwr);
+		if (ret < 0) {
+			dev_err(priv->dev, "Failed to enable io pwr: %d\n",
+				ret);
+			return ret;
+		}
 	}
 
 	/* Reset whole chip through gpio pin or memory-mapped registers for
@@ -1268,10 +1273,12 @@ mt7530_setup(struct dsa_switch *ds)
 		return -ENODEV;
 	}
 
-	/* Reset the switch through internal reset */
-	mt7530_write(priv, MT7530_SYS_CTRL,
-		     SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
-		     SYS_CTRL_REG_RST);
+	if (priv->clkreg) {
+		/* Reset the switch through internal reset */
+		mt7530_write(priv, MT7530_SYS_CTRL,
+			     SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
+			     SYS_CTRL_REG_RST);
+	}
 
 	/* Enable Port 6 only; P5 as GMAC5 which currently is not supported */
 	val = mt7530_read(priv, MT7530_MHWTRAP);
@@ -1356,13 +1363,22 @@ mt7530_probe(struct mdio_device *mdiodev)
 		}
 	}
 
-	priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
-	if (IS_ERR(priv->core_pwr))
-		return PTR_ERR(priv->core_pwr);
-
-	priv->io_pwr = devm_regulator_get(&mdiodev->dev, "io");
-	if (IS_ERR(priv->io_pwr))
-		return PTR_ERR(priv->io_pwr);
+	/* Use mediatek,no-clock-regulator to distinguish hardware that does
+	 * not require clock or regulator control setup (for example mt7621).
+	 */
+	priv->clkreg = !of_property_read_bool(dn, "mediatek,no-clock-regulator");
+	if (priv->clkreg) {
+		priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
+		if (IS_ERR(priv->core_pwr))
+			return PTR_ERR(priv->core_pwr);
+
+		priv->io_pwr = devm_regulator_get(&mdiodev->dev, "io");
+		if (IS_ERR(priv->io_pwr))
+			return PTR_ERR(priv->io_pwr);
+	} else {
+		dev_info(&mdiodev->dev,
+			 "MT7530 with no CLOCK or REGULATOR control\n");
+	}
 
 	/* Not MCM that indicates switch works as the remote standalone
 	 * integrated circuit so the GPIO pin would be used to complete
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index d9b407a22a58..ee97944c4507 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -431,6 +431,7 @@ struct mt7530_priv {
 	struct regulator	*io_pwr;
 	struct gpio_desc	*reset;
 	bool			mcm;
+	bool			clkreg;
 
 	struct mt7530_port	ports[MT7530_NUM_PORTS];
 	/* protect among processes for registers access*/
-- 
2.17.1

[PATCH 2/3] net: dsa: mt7530: optional setting CPU field in MFC register

[gerg]

From: Greg Ungerer <gerg@kernel.org>

Some versions of the MediaTek MT7530 switch have a CPU port number and
enable bit in their MFC register. The MT7530 instance on the MediaTek
MT7621 SoC is one that does for example. The switch will not work
without these fields being correctly setup on these devices.

Create a new devicetree tag, mediatek,mfc-has-cpuport, that signifies
that this device needs the CPU port field and enable bit set when the
port is enabled.

Signed-off-by: Greg Ungerer <gerg@kernel.org>
---
 drivers/net/dsa/mt7530.c | 9 +++++++++
 drivers/net/dsa/mt7530.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 532240c4bef9..e41ada47233a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -689,6 +689,10 @@ mt7530_cpu_port_enable(struct mt7530_priv *priv,
 	/* Unknown unicast frame fordwarding to the cpu port */
 	mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(port)));
 
+	/* Set CPU port number */
+	if (priv->mfccpu)
+		mt7530_rmw(priv, MT7530_MFC, CPU_MASK, CPU_EN | CPU_PORT(port));
+
 	/* CPU port gets connected to all user ports of
 	 * the switch
 	 */
@@ -1380,6 +1384,11 @@ mt7530_probe(struct mdio_device *mdiodev)
 			 "MT7530 with no CLOCK or REGULATOR control\n");
 	}
 
+	/* Use mediatek,mfc-has-cpuport to distinguish hardware where the MFC
+	 * register has a CPU port number field setting.
+	 */
+	priv->mfccpu = of_property_read_bool(dn, "mediatek,mfc-has-cpuport");
+
 	/* Not MCM that indicates switch works as the remote standalone
 	 * integrated circuit so the GPIO pin would be used to complete
 	 * the reset, otherwise memory-mapped register accessing used
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index ee97944c4507..cbc0725c4258 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -36,6 +36,9 @@
 #define  UNM_FFP(x)			(((x) & 0xff) << 16)
 #define  UNU_FFP(x)			(((x) & 0xff) << 8)
 #define  UNU_FFP_MASK			UNU_FFP(~0)
+#define  CPU_EN				BIT(7)
+#define  CPU_PORT(x)			((x) << 4)
+#define  CPU_MASK			(0xf << 4)
 
 /* Registers for address table access */
 #define MT7530_ATA1			0x74
@@ -432,6 +435,7 @@ struct mt7530_priv {
 	struct gpio_desc	*reset;
 	bool			mcm;
 	bool			clkreg;
+	bool			mfccpu;
 
 	struct mt7530_port	ports[MT7530_NUM_PORTS];
 	/* protect among processes for registers access*/
-- 
2.17.1

[PATCH 3/3] dt-bindings: net: dsa: add new bindings MT7530

[gerg]

From: Greg Ungerer <gerg@kernel.org>

Add descriptive entries for the new bindings introduced to support the
MT7530 implementation in the MediaTek MT7621 SoC.

New bindings added for:

  mediatek,no-clock-regulator
  mediatek,mfc-has-cpuport

Signed-off-by: Greg Ungerer <gerg@kernel.org>
---
 Documentation/devicetree/bindings/net/dsa/mt7530.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
index aa3527f71fdc..549ad7c338ea 100644
--- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -9,6 +9,11 @@ Required properties:
 - mediatek,mcm: Boolean; if defined, indicates that either MT7530 is the part
 	on multi-chip module belong to MT7623A has or the remotely standalone
 	chip as the function MT7623N reference board provided for.
+- mediatek,no-clock-regulator: Boolean; if defined, indicates that the MT7530
+	is in a system-on-chip that does not require clock or regulator
+	control setup (for example the MT7621).
+- mediatek,mfc-has-cpuport: Boolean; if defined, indicates that the MT7530
+	has an MFC register that has a CPU PORT field and ENABLE bit
 - core-supply: Phandle to the regulator node necessary for the core power.
 - io-supply: Phandle to the regulator node necessary for the I/O power.
 	See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
-- 
2.17.1

Re: [PATCH 3/3] dt-bindings: net: dsa: add new bindings MT7530

[Florian Fainelli]

Hi Greg,

On 11/29/2018 11:57 PM, gerg@kernel.org wrote:
> From: Greg Ungerer <gerg@kernel.org>
> 
> Add descriptive entries for the new bindings introduced to support the
> MT7530 implementation in the MediaTek MT7621 SoC.
> 
> New bindings added for:
> 
>   mediatek,no-clock-regulator
>   mediatek,mfc-has-cpuport

I don't think any of these properties are necessary, if you can either
use a compatible string, and/or infer the actual model at runtime in the
driver's probe function, then you can assess based on that chip model as
well as the properties being provided in Device Tree whether these
resources must be grabbed and used. See mv88e6xxx and b53 for how these
drivers deal with supporting several distinct models within the same
code base.

As far as the MFC programming goes, this is definitively something that
must be done once you know the chip model you are dealing with.
-- 
Florian

Re: [PATCH 3/3] dt-bindings: net: dsa: add new bindings MT7530

[Greg Ungerer]

Hi Florian,

On 1/12/18 3:41 am, Florian Fainelli wrote:
> Hi Greg,
> 
> On 11/29/2018 11:57 PM, gerg@kernel.org wrote:
>> From: Greg Ungerer <gerg@kernel.org>
>>
>> Add descriptive entries for the new bindings introduced to support the
>> MT7530 implementation in the MediaTek MT7621 SoC.
>>
>> New bindings added for:
>>
>>    mediatek,no-clock-regulator
>>    mediatek,mfc-has-cpuport
> 
> I don't think any of these properties are necessary, if you can either
> use a compatible string, and/or infer the actual model at runtime in the
> driver's probe function, then you can assess based on that chip model as

There is an ID register in the 7530 - though I don't know if the lower
16 bits of it can tell us enough information about the device. For me on
the MT7621 they return "0001", I assume it is a revsion ID of some type.
Problem is we do not read that until after the regulators and some of
the clocking is setup.

A compatible string of some description would be simple enough.
Are you thinking something like "mediatek,mt7621" before
"mediatek,mt7530"?


> well as the properties being provided in Device Tree whether these
> resources must be grabbed and used. See mv88e6xxx and b53 for how these
> drivers deal with supporting several distinct models within the same
> code base.

I will have a close look at those, thanks.


> As far as the MFC programming goes, this is definitively something that
> must be done once you know the chip model you are dealing with.

Yep, certainly.

Thanks
Greg

Re: [PATCH 3/3] dt-bindings: net: dsa: add new bindings MT7530

[Andrew Lunn]

> >I don't think any of these properties are necessary, if you can either
> >use a compatible string, and/or infer the actual model at runtime in the
> >driver's probe function, then you can assess based on that chip model as
> 
> There is an ID register in the 7530 - though I don't know if the lower
> 16 bits of it can tell us enough information about the device. For me on
> the MT7621 they return "0001", I assume it is a revsion ID of some type.
> Problem is we do not read that until after the regulators and some of
> the clocking is setup.

Hi Greg

I would suggest you refactor the code, so you know the ID early
on. Reading such an ID is very common in device drivers, to decide
what to do. The silicon is generally more reliable than device tree
when it comes to identification.

The only time you need a different device tree compatibly string is
when you cannot actually get to the ID, e.g. you need to turn some
clock on, or the ID is in a different place.

     Andrew

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[René van Dorst]

Quoting gerg@kernel.org:

> I have been working towards supporting the MT7530 switch as used in the
> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
> a dual core MIPS CPU architecture. But underneath it is what appears to
> be the same 7530 switch.
>
> The following 3 patches are more of an RFC than anything. They allow
> use of the mt7530 dsa driver on this device - though with some issues
> still to resolve. The primary change required is to not use the 7623
> specific clock and regulator setup - none of that applies when using
> the 7621 (and maybe other devices?). The other change required is to
> set the 7530 MFC register CPU port number and enable bit.


Hi Greg,

Good to see that more people are working on the MT7621 device [1].
So I added Bjorn to the CC.

I am also working on this but on the OpenWRT side.
My current code works for a Ubiquiti EdgeRouter X SFP. See kernel [2],  
openwrt [3]

Current status:

Using OpenWRT provided mainline v4.14 driver MT7530 and MT7623.
I patches so that MT7621 is supported.
This means DSA part is also working, internal and external phys are detected.
I can use all of the five RJ45 ports and also MT7520 switch port 5  
which connects to a external phy (at8033) for the SFP port.
Last added TRGMII part also seems to work but with issues, see below.
Openwrt uses port 5 as wan and gets a dhcp lease.

Issues:
- I can't get 2nd GMAC talk to external phy. I have tried many many  
knobs but without success.
   GMAC seems to work but no data is transmitted/received over the cable.
   But I think this can be done later on. Adding basic support for  
MT7621 is good start.
- Ethernet driver expects that the macs are initialized so that the  
mtk_hw_init can setup the hardware registers.
   But they are not. See [4]
   I don't know how to fix this. For the current code it is not an  
issue. It still works.
   But it should be fixed.
   Because of this I can't read the mac0 "phy-mode". I need this info  
to setup the tgrmii clock at hardware init.
- Ethernet speed is unstable ~30-100mbit. I think I broke something. I  
have seems 1gbit before.

I hope that this can help you the get a step further.

Greats,

René

[1] https://lists.openwrt.org/pipermail/openwrt-devel/2018-August/013614.html
[2] https://github.com/vDorst/linux-1/commits/mt7621-dsa-trgmii
[3] https://github.com/vDorst/openwrt/commits/mt7621-dsa-trgmii
[4]  
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c?h=v4.14.84#n1946


-- 
Met vriendelijke groet,

René van Dorst

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Rene,

On 30/11/18 9:27 pm, René van Dorst wrote:
> Quoting gerg@kernel.org:
> 
>> I have been working towards supporting the MT7530 switch as used in the
>> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
>> a dual core MIPS CPU architecture. But underneath it is what appears to
>> be the same 7530 switch.
>>
>> The following 3 patches are more of an RFC than anything. They allow
>> use of the mt7530 dsa driver on this device - though with some issues
>> still to resolve. The primary change required is to not use the 7623
>> specific clock and regulator setup - none of that applies when using
>> the 7621 (and maybe other devices?). The other change required is to
>> set the 7530 MFC register CPU port number and enable bit.
> 
> 
> Hi Greg,
> 
> Good to see that more people are working on the MT7621 device [1].
> So I added Bjorn to the CC.

Nice, thanks for the pointers.


> I am also working on this but on the OpenWRT side.
> My current code works for a Ubiquiti EdgeRouter X SFP. See kernel [2], openwrt [3]

I forgot to mention I am working from mainline kernels, so those
patches of mine are against 4.20-rc4. I am working on some new
custom hardware at the moment, but I have an Oolite v8.0 board I
can run code on too.


> Current status:
> 
> Using OpenWRT provided mainline v4.14 driver MT7530 and MT7623.
> I patches so that MT7621 is supported.
> This means DSA part is also working, internal and external phys are detected.
> I can use all of the five RJ45 ports and also MT7520 switch port 5 which connects to a external phy (at8033) for the SFP port.
> Last added TRGMII part also seems to work but with issues, see below.
> Openwrt uses port 5 as wan and gets a dhcp lease.
> 
> Issues:
> - I can't get 2nd GMAC talk to external phy. I have tried many many knobs but without success.
>    GMAC seems to work but no data is transmitted/received over the cable.
>    But I think this can be done later on. Adding basic support for MT7621 is good start.
> - Ethernet driver expects that the macs are initialized so that the mtk_hw_init can setup the hardware registers.
>    But they are not. See [4]
>    I don't know how to fix this. For the current code it is not an issue. It still works.
>    But it should be fixed.
>    Because of this I can't read the mac0 "phy-mode". I need this info to setup the tgrmii clock at hardware init.
> - Ethernet speed is unstable ~30-100mbit. I think I broke something. I have seems 1gbit before.
> 
> I hope that this can help you the get a step further.

Thanks, that is all good info.

Regards
Greg


> René
> 
> [1] https://lists.openwrt.org/pipermail/openwrt-devel/2018-August/013614.html
> [2] https://github.com/vDorst/linux-1/commits/mt7621-dsa-trgmii
> [3] https://github.com/vDorst/openwrt/commits/mt7621-dsa-trgmii
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c?h=v4.14.84#n1946
> 
> 

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[René van Dorst]

Quoting gerg@kernel.org:

> I have been working towards supporting the MT7530 switch as used in the
> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
> a dual core MIPS CPU architecture. But underneath it is what appears to
> be the same 7530 switch.
>
> The following 3 patches are more of an RFC than anything. They allow
> use of the mt7530 dsa driver on this device - though with some issues
> still to resolve. The primary change required is to not use the 7623
> specific clock and regulator setup - none of that applies when using
> the 7621 (and maybe other devices?). The other change required is to
> set the 7530 MFC register CPU port number and enable bit.

Hi Greg,

Good to see that more people are working on the MT7621 device [1].
So I added Bjorn to the CC.

I am also working on this but on the OpenWRT side.
My current code works for a Ubiquiti EdgeRouter X SFP. See kernel [2],  
openwrt [3]

Current status:

Using OpenWRT provided mainline v4.14 driver MT7530 and MT7623.
I patches so that MT7621 is supported.
This means DSA part is also working, internal and external phys are detected.
I can use all of the five RJ45 ports and also MT7520 switch port 5  
which connects to a external phy (at8033) for the SFP port.
Last added TRGMII part also seems to work but with issues, see below.
Openwrt uses port 5 as wan and gets a dhcp lease.

Issues:
- I can't get 2nd GMAC talk to external phy. I have tried many many  
knobs but without success.
   GMAC seems to work but no data is transmitted/received over the cable.
   But I think this can be done later on. Adding basic support for  
MT7621 is good start.
- Ethernet driver expects that the macs are initialized so that the  
mtk_hw_init can setup the hardware registers.
   But they are not. See [4]
   I don't know how to fix this. For the current code it is not an  
issue. It still works.
   But it should be fixed.
   Because of this I can't read the mac0 "phy-mode". I need this info  
to setup the tgrmii clock at hardware init.
- Ethernet speed is unstable ~30-100mbit. I think I broke something. I  
have seems 1gbit before.

I hope that this can help you the get a step further.

Greats,

René

[1] https://lists.openwrt.org/pipermail/openwrt-devel/2018-August/013614.html
[2] https://github.com/vDorst/linux-1/commits/mt7621-dsa-trgmii
[3] https://github.com/vDorst/openwrt/commits/mt7621-dsa-trgmii
[4]  
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c?h=v4.14.84#n1946

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Bjørn Mork]

gerg@kernel.org writes:

> I have been working towards supporting the MT7530 switch as used in the
> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
> a dual core MIPS CPU architecture. But underneath it is what appears to
> be the same 7530 switch.

Great!  Good to see someone pushing this idea forward.

> The following 3 patches are more of an RFC than anything. They allow
> use of the mt7530 dsa driver on this device - though with some issues
> still to resolve. The primary change required is to not use the 7623
> specific clock and regulator setup - none of that applies when using
> the 7621 (and maybe other devices?). The other change required is to
> set the 7530 MFC register CPU port number and enable bit.
>
> The unresolved issues I still have appear to be more related to the
> MT7621 ethernet driver (drivers/staging/mt7621-eth/*). I am hoping
> someone might have some ideas on these. I don't really have any good
> documentation on the ethernet devices on the 7621, so I am kind of
> working in the dark here.

No offense, but the mt7621-eth driver in staging is horrible.  What both
René and I have had some success with is adapting the mtk_eth_soc driver
already in drivers/net/ethernet/mediatek/.  Yes, I know this is supposed
to be for other SoCs, but the basic design is obviously the same.

I have had some success with a first hackish attemt based on OpenWrt.
You can find the early tree here, but note that my focus was basically
getting one specific MT7621 board up and running:
https://github.com/bmork/LEDE/tree/mt7621-with-mainline-eth-driver

This patch has most of the necessary changes to enable that driver for
MT7621:
https://github.com/bmork/LEDE/commit/3293bc63f5461ca1eb0bbc4ed90145335e7e3404

Not a big deal, as you can see.  There is of course a reason I didn't
submit this here yet: It is by no means finished...  But it works. And I
have both GMACs working with this driver, which was my primary goal.

> 1. TX packets are not getting an IP header checksum via the normal
>    off-loaded checksumming when in DSA mode. I have to switch off
>    NETIF_F_IP_CSUM, so the software stack generates the checksum.
>    That checksum offloading works ok when not using the 7530 DSA driver.

Hmm.  How do I test this?

> 2. Maximal sized RX packets get silently dropped. So receive side packets
>    that are large (perfect case is the all-but-last packets in a fragemented
>    larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>    The 7530 MIB switch register counters show receive packets at the physical
>    switch port side and at the CPU switch port - but I get no packets
>    received or errors in the 7621 ethernet MAC. If I set the mtu of the
>    server at the other end a little smaller (a few bytes is enough) then
>    I get all the packets through. It seems like the DSA/VLAN tag bytes
>    are causing a too large packet to get silently dropped somewhere.

Are you referring to the configured MTU size or some other maximal size?
If MTU, then I don't seem to have this issue with the driver from
drivers/net/ethernet/mediatek/.



Bjørn

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Bjorn,

On 30/11/18 10:16 pm, Bjørn Mork wrote:
> gerg@kernel.org writes:
> 
>> I have been working towards supporting the MT7530 switch as used in the
>> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
>> a dual core MIPS CPU architecture. But underneath it is what appears to
>> be the same 7530 switch.
> 
> Great!  Good to see someone pushing this idea forward.
> 
>> The following 3 patches are more of an RFC than anything. They allow
>> use of the mt7530 dsa driver on this device - though with some issues
>> still to resolve. The primary change required is to not use the 7623
>> specific clock and regulator setup - none of that applies when using
>> the 7621 (and maybe other devices?). The other change required is to
>> set the 7530 MFC register CPU port number and enable bit.
>>
>> The unresolved issues I still have appear to be more related to the
>> MT7621 ethernet driver (drivers/staging/mt7621-eth/*). I am hoping
>> someone might have some ideas on these. I don't really have any good
>> documentation on the ethernet devices on the 7621, so I am kind of
>> working in the dark here.
> 
> No offense, but the mt7621-eth driver in staging is horrible.  What both
> René and I have had some success with is adapting the mtk_eth_soc driver
> already in drivers/net/ethernet/mediatek/.  Yes, I know this is supposed
> to be for other SoCs, but the basic design is obviously the same.

Yes, that makes a lot of sense. I have only been working with the
linux mainline mt7621 staging driver so far for this mt7530 work.
(I started trying to make this work with linux 4.19).


> I have had some success with a first hackish attemt based on OpenWrt.
> You can find the early tree here, but note that my focus was basically
> getting one specific MT7621 board up and running:
> https://github.com/bmork/LEDE/tree/mt7621-with-mainline-eth-driver

Thats great, thanks for the pointer, I'll have a close look at that.


> This patch has most of the necessary changes to enable that driver for
> MT7621:
> https://github.com/bmork/LEDE/commit/3293bc63f5461ca1eb0bbc4ed90145335e7e3404
> 
> Not a big deal, as you can see.  There is of course a reason I didn't
> submit this here yet: It is by no means finished...  But it works. And I
> have both GMACs working with this driver, which was my primary goal.
> 
>> 1. TX packets are not getting an IP header checksum via the normal
>>     off-loaded checksumming when in DSA mode. I have to switch off
>>     NETIF_F_IP_CSUM, so the software stack generates the checksum.
>>     That checksum offloading works ok when not using the 7530 DSA driver.
> 
> Hmm.  How do I test this?

For me every transmitted packet had the wrong IP header checksum.
Every packet I received at the other end of the wire (dumped with
tcpdump) showed up with wrong header checksum. I am guessing you didn't
see this behavior - you can't really miss it :-)


>> 2. Maximal sized RX packets get silently dropped. So receive side packets
>>     that are large (perfect case is the all-but-last packets in a fragemented
>>     larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>>     The 7530 MIB switch register counters show receive packets at the physical
>>     switch port side and at the CPU switch port - but I get no packets
>>     received or errors in the 7621 ethernet MAC. If I set the mtu of the
>>     server at the other end a little smaller (a few bytes is enough) then
>>     I get all the packets through. It seems like the DSA/VLAN tag bytes
>>     are causing a too large packet to get silently dropped somewhere.
> 
> Are you referring to the configured MTU size or some other maximal size?
> If MTU, then I don't seem to have this issue with the driver from
> drivers/net/ethernet/mediatek/.

I was referring to the mtu on the system at the other end of the wire.
For me that was my development PC (just a xubuntu 18.04, x86 machine).
If I reduced it there from the default mtu of 1500 I could get packets
to be received on the mt7621. The behavior was obvious to see with large
sent packets (something simple like ping -s 2000), the first fragment was
silently dropped bu the second fragment would get through.

Anyway, I will try out the changes you have for the drivers/net/ethernet/mediatek
driver, they sound very promising.

Thanks
Greg

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Andrew Lunn]

> > 1. TX packets are not getting an IP header checksum via the normal
> >    off-loaded checksumming when in DSA mode. I have to switch off
> >    NETIF_F_IP_CSUM, so the software stack generates the checksum.
> >    That checksum offloading works ok when not using the 7530 DSA driver.
> 
> Hmm.  How do I test this?

If there are no IP checksums in the frame, the receiver will generally
drop the packet.

ethtool -k will show you what features the MAC has in terms of
offloading. So if you see NETIF_F_IP_CSUM, you know the MAC should be
doing it in hardware.

      Andrew

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Bjorn,

On 30/11/18 10:16 pm, Bjørn Mork wrote:
> gerg@kernel.org writes:
> 
>> I have been working towards supporting the MT7530 switch as used in the
>> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
>> a dual core MIPS CPU architecture. But underneath it is what appears to
>> be the same 7530 switch.
> 
> Great!  Good to see someone pushing this idea forward.
> 
>> The following 3 patches are more of an RFC than anything. They allow
>> use of the mt7530 dsa driver on this device - though with some issues
>> still to resolve. The primary change required is to not use the 7623
>> specific clock and regulator setup - none of that applies when using
>> the 7621 (and maybe other devices?). The other change required is to
>> set the 7530 MFC register CPU port number and enable bit.
>>
>> The unresolved issues I still have appear to be more related to the
>> MT7621 ethernet driver (drivers/staging/mt7621-eth/*). I am hoping
>> someone might have some ideas on these. I don't really have any good
>> documentation on the ethernet devices on the 7621, so I am kind of
>> working in the dark here.
> 
> No offense, but the mt7621-eth driver in staging is horrible.  What both
> René and I have had some success with is adapting the mtk_eth_soc driver
> already in drivers/net/ethernet/mediatek/.  Yes, I know this is supposed
> to be for other SoCs, but the basic design is obviously the same.
> 
> I have had some success with a first hackish attemt based on OpenWrt.
> You can find the early tree here, but note that my focus was basically
> getting one specific MT7621 board up and running:
> https://github.com/bmork/LEDE/tree/mt7621-with-mainline-eth-driver
> 
> This patch has most of the necessary changes to enable that driver for
> MT7621:
> https://github.com/bmork/LEDE/commit/3293bc63f5461ca1eb0bbc4ed90145335e7e3404

I applied this to my main debug linux-4.19 kernel. Didn't apply completely
cleanly but was easy to fix up.

Using that everything came up detected (the 7530 switch) and I could
quickly see that it does not suffer the problems I listed below. Both
RX and TX packets of any size work!

However I also quickly discovered that this driver was pretty unstable.
Put a bit of packet load on it, and it would stop responding, CPU lock up,
and occasional rcu stalled messages from the kernel.

The following change helped alot, but I still get some problems under
sustained load and some types of port setups. Still trying to figure
out what exactly is going on.

--- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
  
         if (likely(napi_schedule_prep(&eth->rx_napi))) {
                 __napi_schedule(&eth->rx_napi);
-               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
         }
+       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
  
         return IRQ_HANDLED;
  }
@@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int irq, void *_eth)
  
         if (likely(napi_schedule_prep(&eth->tx_napi))) {
                 __napi_schedule(&eth->tx_napi);
-               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
         }
+       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
  
         return IRQ_HANDLED;
  }


Anyway, this really looks like the right approach to me. This driver is
clearly capable of supporting the mt7621 ethernet ports. No need for the
staging driver.

Regards
Greg



> Not a big deal, as you can see.  There is of course a reason I didn't
> submit this here yet: It is by no means finished...  But it works. And I
> have both GMACs working with this driver, which was my primary goal.
> 
>> 1. TX packets are not getting an IP header checksum via the normal
>>     off-loaded checksumming when in DSA mode. I have to switch off
>>     NETIF_F_IP_CSUM, so the software stack generates the checksum.
>>     That checksum offloading works ok when not using the 7530 DSA driver.
> 
> Hmm.  How do I test this?
> 
>> 2. Maximal sized RX packets get silently dropped. So receive side packets
>>     that are large (perfect case is the all-but-last packets in a fragemented
>>     larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>>     The 7530 MIB switch register counters show receive packets at the physical
>>     switch port side and at the CPU switch port - but I get no packets
>>     received or errors in the 7621 ethernet MAC. If I set the mtu of the
>>     server at the other end a little smaller (a few bytes is enough) then
>>     I get all the packets through. It seems like the DSA/VLAN tag bytes
>>     are causing a too large packet to get silently dropped somewhere.
> 
> Are you referring to the configured MTU size or some other maximal size?
> If MTU, then I don't seem to have this issue with the driver from
> drivers/net/ethernet/mediatek/.
> 
> 
> 
> Bjørn
> 

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Bjørn Mork]

[ fixed Johns address - the openwrt.org email was apparently never restored? ]

Greg Ungerer <gerg@kernel.org> writes:

> The following change helped alot, but I still get some problems under
> sustained load and some types of port setups. Still trying to figure
> out what exactly is going on.
>
> --- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
>        if (likely(napi_schedule_prep(&eth->rx_napi))) {
>                 __napi_schedule(&eth->rx_napi);
> -               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>         }
> +       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>        return IRQ_HANDLED;
>  }
> @@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int irq, void *_eth)
>        if (likely(napi_schedule_prep(&eth->tx_napi))) {
>                 __napi_schedule(&eth->tx_napi);
> -               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>         }
> +       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>        return IRQ_HANDLED;
>  }

Yes, sorry I didn't point to that as well.  Just to be clear:  I have no
clue how this thing is actually wired up, or if you could use three
interrupts on the MT7621 too. I just messed with it until I got
something to work, based on Renés original idea and code.

> Anyway, this really looks like the right approach to me. This driver is
> clearly capable of supporting the mt7621 ethernet ports. No need for the
> staging driver.

Great! Thanks for doing this.

I did make a feeble attempt at testing this with current mainline
myself, but the only MT7621 board I have is using NAND flash.  So I
started trying to forward port the mtk-nand2 driver from OpenWrt. And
failed. Probably a simple mixup while trying to adjust to the many
changes in the raw NAND API between v4.14 and v.4.20.  Then I
optimistically attempted to use the mainline mtk-nand driver instead,
assuming it would be as simple as with the mtk-eth driver.  Which it
wasn't, of course. I guess there are a lot of things I do not understand
wrt flash and HW ECC etc...

Short version: I won't be able to test the mainline mtk-eth driver with
MT7621 on newer kernels before smarter people like John upgrade the
OpenWrt kernel.


Bjørn

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[René van Dorst]

Quoting Bjørn Mork <bjorn@mork.no>:
> Greg Ungerer <gerg@kernel.org> writes:
>
>> The following change helped alot, but I still get some problems under
>> sustained load and some types of port setups. Still trying to figure
>> out what exactly is going on.
>>
>> --- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq,  
>> void *_eth)
>>        if (likely(napi_schedule_prep(&eth->rx_napi))) {
>>                 __napi_schedule(&eth->rx_napi);
>> -               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>         }
>> +       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>        return IRQ_HANDLED;
>>  }
>> @@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int  
>> irq, void *_eth)
>>        if (likely(napi_schedule_prep(&eth->tx_napi))) {
>>                 __napi_schedule(&eth->tx_napi);
>> -               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>         }
>> +       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>        return IRQ_HANDLED;
>>  }
>
> Yes, sorry I didn't point to that as well.  Just to be clear:  I have no
> clue how this thing is actually wired up, or if you could use three
> interrupts on the MT7621 too. I just messed with it until I got
> something to work, based on Renés original idea and code.

My idea is a just a copy of mtk_handle_irq_{rx,tx} see [1]
You probably want to look at the staging driver or Ubiquity source  
with a 3.10.x kernel [2] or padavan with 3.4.x kernel [3].
AFAIK mt7621 only has 1 IRQ for ethernet part.

Greats,

René

[1]  
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1739
[2]  
https://www.ubnt.com/download/edgemax/edgerouter-x-sfp/default/edgerouter-er-xer-x-sfpep-r6-firmware-v1107
[3]  
https://bitbucket.org/padavan/rt-n56u/src/e6f45337528f668651e251057a1a0fec735f6df1/trunk/linux-3.4.x/drivers/net/raeth/raether.c?at=master&fileviewer=file-view-default#raether.c-658

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[John Crispin]

On 03/12/2018 15:00, René van Dorst wrote:
> Quoting Bjørn Mork <bjorn@mork.no>:
>> Greg Ungerer <gerg@kernel.org> writes:
>>
>>> The following change helped alot, but I still get some problems under
>>> sustained load and some types of port setups. Still trying to figure
>>> out what exactly is going on.
>>>
>>> --- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>> +++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>> @@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq, 
>>> void *_eth)
>>>        if (likely(napi_schedule_prep(&eth->rx_napi))) {
>>>                 __napi_schedule(&eth->rx_napi);
>>> -               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>>         }
>>> +       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>>        return IRQ_HANDLED;
>>>  }
>>> @@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int 
>>> irq, void *_eth)
>>>        if (likely(napi_schedule_prep(&eth->tx_napi))) {
>>>                 __napi_schedule(&eth->tx_napi);
>>> -               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>>         }
>>> +       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>>        return IRQ_HANDLED;
>>>  }
>>
>> Yes, sorry I didn't point to that as well.  Just to be clear:  I have no
>> clue how this thing is actually wired up, or if you could use three
>> interrupts on the MT7621 too. I just messed with it until I got
>> something to work, based on Renés original idea and code.
>
> My idea is a just a copy of mtk_handle_irq_{rx,tx} see [1]
> You probably want to look at the staging driver or Ubiquity source 
> with a 3.10.x kernel [2] or padavan with 3.4.x kernel [3].
> AFAIK mt7621 only has 1 IRQ for ethernet part.

correct there is only 1 single IRQ on mt7621

     John



>
> Greats,
>
> René
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/mediatek/mtk_eth_soc.c#n1739
> [2] 
> https://www.ubnt.com/download/edgemax/edgerouter-x-sfp/default/edgerouter-er-xer-x-sfpep-r6-firmware-v1107
> [3] 
> https://bitbucket.org/padavan/rt-n56u/src/e6f45337528f668651e251057a1a0fec735f6df1/trunk/linux-3.4.x/drivers/net/raeth/raether.c?at=master&fileviewer=file-view-default#raether.c-658
>
>

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi John,

On 4/12/18 12:02 am, John Crispin wrote:
> On 03/12/2018 15:00, René van Dorst wrote:
>> Quoting Bjørn Mork <bjorn@mork.no>:
>>> Greg Ungerer <gerg@kernel.org> writes:
>>>
>>>> The following change helped alot, but I still get some problems under
>>>> sustained load and some types of port setups. Still trying to figure
>>>> out what exactly is going on.
>>>>
>>>> --- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>> +++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>> @@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
>>>>        if (likely(napi_schedule_prep(&eth->rx_napi))) {
>>>>                 __napi_schedule(&eth->rx_napi);
>>>> -               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>>>         }
>>>> +       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>>>        return IRQ_HANDLED;
>>>>  }
>>>> @@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int irq, void *_eth)
>>>>        if (likely(napi_schedule_prep(&eth->tx_napi))) {
>>>>                 __napi_schedule(&eth->tx_napi);
>>>> -               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>>>         }
>>>> +       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>>>        return IRQ_HANDLED;
>>>>  }
>>>
>>> Yes, sorry I didn't point to that as well.  Just to be clear:  I have no
>>> clue how this thing is actually wired up, or if you could use three
>>> interrupts on the MT7621 too. I just messed with it until I got
>>> something to work, based on Renés original idea and code.
>>
>> My idea is a just a copy of mtk_handle_irq_{rx,tx} see [1]
>> You probably want to look at the staging driver or Ubiquity source with a 3.10.x kernel [2] or padavan with 3.4.x kernel [3].
>> AFAIK mt7621 only has 1 IRQ for ethernet part.
> 
> correct there is only 1 single IRQ on mt7621

One of the main differences I see between the mainline mtk_eth_soc.c
and the older mediatek/openwrt driver is that the older driver uses
the PDMA module for TX transmission, while the mainline uses the
QDMA module. I have no documentation on the what the differences
are between the 2 (or why there is even 2 DMA engines in there?).
Can you shed any light on that?

I did a quick and dirty recode of the QDMA transmission parts of
the mainline driver code to use the PDMA engine instead. The most
immediate result is that it suffers the same IP header checksum
problem on TX packets :-(  But it also still suffers from the
same occasional TX watchdog timeout I see with only the mainline
driver and basic support of MT7621.

What I see with the TX watchdog timeouts is that there is valid
TX descriptors, but the frame engine is just not processing them.
It seems to be just sitting there idle. The CTX and DTX registers
look valid and consistent with the local last_free/next_free
pointers.

Regards
Greg

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Bjorn,

On 3/12/18 9:34 pm, Bjørn Mork wrote:
> [ fixed Johns address - the openwrt.org email was apparently never restored? ]
> 
> Greg Ungerer <gerg@kernel.org> writes:
> 
>> The following change helped alot, but I still get some problems under
>> sustained load and some types of port setups. Still trying to figure
>> out what exactly is going on.
>>
>> --- a/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> +++ b/linux/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>> @@ -1750,8 +1750,8 @@ static irqreturn_t mtk_handle_irq_rx(int irq, void *_eth)
>>         if (likely(napi_schedule_prep(&eth->rx_napi))) {
>>                  __napi_schedule(&eth->rx_napi);
>> -               mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>          }
>> +       mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
>>         return IRQ_HANDLED;
>>   }
>> @@ -1762,11 +1762,53 @@ static irqreturn_t mtk_handle_irq_tx(int irq, void *_eth)
>>         if (likely(napi_schedule_prep(&eth->tx_napi))) {
>>                  __napi_schedule(&eth->tx_napi);
>> -               mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>          }
>> +       mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
>>         return IRQ_HANDLED;
>>   }
> 
> Yes, sorry I didn't point to that as well.  Just to be clear:  I have no
> clue how this thing is actually wired up, or if you could use three
> interrupts on the MT7621 too. I just messed with it until I got
> something to work, based on Renés original idea and code.

Understood.

By way of progress I have found that making the single IRQ handler
look like this is better than the last patch:

static irqreturn_t mtk_handle_irq(int irq, void *_eth)
{
         struct mtk_eth *eth = _eth;
         irqreturn_t rc = IRQ_NONE;

         if (mtk_r32(eth, MTK_PDMA_INT_MASK) & MTK_RX_DONE_INT) {
                 if (mtk_r32(eth, MTK_PDMA_INT_STATUS) & MTK_RX_DONE_INT)
                         rc = mtk_handle_irq_rx(irq, _eth);
         }

         if (mtk_r32(eth, MTK_QDMA_INT_MASK) & MTK_TX_DONE_INT) {
                 if (mtk_r32(eth, MTK_QMTK_INT_STATUS) & MTK_TX_DONE_INT)
                         rc = mtk_handle_irq_tx(irq, _eth);
         }

         return rc;
}

So not calling the RX or TX handlers if their interrupts
where not masked on. For some work loads that is quite reliable.
Flood ping through one port and out the other can get a lof of
packets through.

However I still get dev_watchdog timeouts after a while and with
more mixed packet loads. Seems like something is racing on the
TX path side.

Regards
Greg



>> Anyway, this really looks like the right approach to me. This driver is
>> clearly capable of supporting the mt7621 ethernet ports. No need for the
>> staging driver.
> 
> Great! Thanks for doing this.
> 
> I did make a feeble attempt at testing this with current mainline
> myself, but the only MT7621 board I have is using NAND flash.  So I
> started trying to forward port the mtk-nand2 driver from OpenWrt. And
> failed. Probably a simple mixup while trying to adjust to the many
> changes in the raw NAND API between v4.14 and v.4.20.  Then I
> optimistically attempted to use the mainline mtk-nand driver instead,
> assuming it would be as simple as with the mtk-eth driver.  Which it
> wasn't, of course. I guess there are a lot of things I do not understand
> wrt flash and HW ECC etc...
> 
> Short version: I won't be able to test the mainline mtk-eth driver with
> MT7621 on newer kernels before smarter people like John upgrade the
> OpenWrt kernel.
> 
> 
> Bjørn
> 

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[NeilBrown]

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

On Fri, Nov 30 2018, Bjørn Mork wrote:

> gerg@kernel.org writes:
>
>> I have been working towards supporting the MT7530 switch as used in the
>> MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621 is built around
>> a dual core MIPS CPU architecture. But underneath it is what appears to
>> be the same 7530 switch.
>
> Great!  Good to see someone pushing this idea forward.
>
>> The following 3 patches are more of an RFC than anything. They allow
>> use of the mt7530 dsa driver on this device - though with some issues
>> still to resolve. The primary change required is to not use the 7623
>> specific clock and regulator setup - none of that applies when using
>> the 7621 (and maybe other devices?). The other change required is to
>> set the 7530 MFC register CPU port number and enable bit.
>>
>> The unresolved issues I still have appear to be more related to the
>> MT7621 ethernet driver (drivers/staging/mt7621-eth/*). I am hoping
>> someone might have some ideas on these. I don't really have any good
>> documentation on the ethernet devices on the 7621, so I am kind of
>> working in the dark here.
>
> No offense, but the mt7621-eth driver in staging is horrible.  What both
> René and I have had some success with is adapting the mtk_eth_soc driver
> already in drivers/net/ethernet/mediatek/.  Yes, I know this is supposed
> to be for other SoCs, but the basic design is obviously the same.
>
> I have had some success with a first hackish attemt based on OpenWrt.
> You can find the early tree here, but note that my focus was basically
> getting one specific MT7621 board up and running:
> https://github.com/bmork/LEDE/tree/mt7621-with-mainline-eth-driver
>
> This patch has most of the necessary changes to enable that driver for
> MT7621:
> https://github.com/bmork/LEDE/commit/3293bc63f5461ca1eb0bbc4ed90145335e7e3404
>
> Not a big deal, as you can see.  There is of course a reason I didn't
> submit this here yet: It is by no means finished...  But it works. And I
> have both GMACs working with this driver, which was my primary goal.

Thanks a lot for posting this.  I'd be very happy to see the staging
driver disappear once this is ready and in mainline.

I got your patch working on 4.20-rc5 and did a performance comparison.
With the staging driver (using iperf3) I get
  220 MBit/sec in
  680 MBit/sec out

with the patched mainline driver I get
  190 MBit/sec in
   93 MBit/sec out

(numbers are a bit rubbery, but within 10%)

I haven't looked into why this might be, but thought I would mention it.

Strangely when I test with scp, I get about 10MB/sec in both directions
with both drivers.  Maybe the CPU limits encryption speed.

I have a 4.4-based kernel where I get 940MBit/sec both ways - using a
precursor of the current staging driver.

Thanks,
NeilBrown


>
>> 1. TX packets are not getting an IP header checksum via the normal
>>    off-loaded checksumming when in DSA mode. I have to switch off
>>    NETIF_F_IP_CSUM, so the software stack generates the checksum.
>>    That checksum offloading works ok when not using the 7530 DSA driver.
>
> Hmm.  How do I test this?
>
>> 2. Maximal sized RX packets get silently dropped. So receive side packets
>>    that are large (perfect case is the all-but-last packets in a fragemented
>>    larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>>    The 7530 MIB switch register counters show receive packets at the physical
>>    switch port side and at the CPU switch port - but I get no packets
>>    received or errors in the 7621 ethernet MAC. If I set the mtu of the
>>    server at the other end a little smaller (a few bytes is enough) then
>>    I get all the packets through. It seems like the DSA/VLAN tag bytes
>>    are causing a too large packet to get silently dropped somewhere.
>
> Are you referring to the configured MTU size or some other maximal size?
> If MTU, then I don't seem to have this issue with the driver from
> drivers/net/ethernet/mediatek/.
>
>
>
> Bjørn

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Bjørn Mork]

NeilBrown <neil@brown.name> writes:

> I got your patch working on 4.20-rc5 and did a performance comparison.
> With the staging driver (using iperf3) I get
>   220 MBit/sec in
>   680 MBit/sec out
>
> with the patched mainline driver I get
>   190 MBit/sec in
>    93 MBit/sec out
>
> (numbers are a bit rubbery, but within 10%)
>
> I haven't looked into why this might be, but thought I would mention it.
>
> Strangely when I test with scp, I get about 10MB/sec in both directions
> with both drivers.  Maybe the CPU limits encryption speed.
>
> I have a 4.4-based kernel where I get 940MBit/sec both ways - using a
> precursor of the current staging driver.

Yes, thanks for bringing this up. I should have mentioned that I haven't
considered performance at all yet.


Bjørn

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[NeilBrown]

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

On Tue, Dec 11 2018, NeilBrown wrote:

>
> I got your patch working on 4.20-rc5 and did a performance comparison.
> With the staging driver (using iperf3) I get
>   220 MBit/sec in
>   680 MBit/sec out
>
> with the patched mainline driver I get
>   190 MBit/sec in
>    93 MBit/sec out
>
> (numbers are a bit rubbery, but within 10%)
>
> I haven't looked into why this might be, but thought I would mention it.
>
> Strangely when I test with scp, I get about 10MB/sec in both directions
> with both drivers.  Maybe the CPU limits encryption speed.
>
> I have a 4.4-based kernel where I get 940MBit/sec both ways - using a
> precursor of the current staging driver.

Just FYI, I've been looking further into this, and I don't think the
problem is (entirely) related to the driver.

In my 4.4 kernel, the build_skb() call in (the equivalent of)
mtk_poll_rx() takes about 1.2usec and the call to napi_gro_receive()
takes about 3usec.

In my 4.20 kernel, these calls take about 30 and 24 usec respectively.
This easily explains the slowdown.
I don't yet know why, and won't have time to look for a few days.
I haven't looked into how this affects the
drivers/net/ethernet/mediatek driver in 4.20.

If anyone has ideas about why these might be so slow, I'd love to hear
them.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[David Miller]

From: NeilBrown <neil@brown.name>
Date: Mon, 17 Dec 2018 09:08:54 +1100

> In my 4.4 kernel, the build_skb() call in (the equivalent of)
> mtk_poll_rx() takes about 1.2usec and the call to napi_gro_receive()
> takes about 3usec.
> 
> In my 4.20 kernel, these calls take about 30 and 24 usec respectively.
> This easily explains the slowdown.

That's a huge difference.

Nothing jumps out as a possible cause except perhaps retpoline or
something like that.

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[NeilBrown]

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

On Sun, Dec 16 2018, David Miller wrote:

> From: NeilBrown <neil@brown.name>
> Date: Mon, 17 Dec 2018 09:08:54 +1100
>
>> In my 4.4 kernel, the build_skb() call in (the equivalent of)
>> mtk_poll_rx() takes about 1.2usec and the call to napi_gro_receive()
>> takes about 3usec.
>> 
>> In my 4.20 kernel, these calls take about 30 and 24 usec respectively.
>> This easily explains the slowdown.
>
> That's a huge difference.
>
> Nothing jumps out as a possible cause except perhaps retpoline or
> something like that.

I'll keep that in mind - thanks.

My guess was CPU-cache invalidation.
I just checked and the other CPU core (there are two - each
hyper-threaded - "other" meaning not the one that handles ethernet
interrupts) gets several thousand "IPI resched" interrupts while
running a 10 second (226MByte) iperf3 receive test.
About 17KB transferred per IPI.
I cannot see where build_skb() would do cache invalidation though.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Florian Fainelli]

On December 16, 2018 3:19:22 PM PST, NeilBrown <neil@brown.name> wrote:
>On Sun, Dec 16 2018, David Miller wrote:
>
>> From: NeilBrown <neil@brown.name>
>> Date: Mon, 17 Dec 2018 09:08:54 +1100
>>
>>> In my 4.4 kernel, the build_skb() call in (the equivalent of)
>>> mtk_poll_rx() takes about 1.2usec and the call to napi_gro_receive()
>>> takes about 3usec.
>>> 
>>> In my 4.20 kernel, these calls take about 30 and 24 usec
>respectively.
>>> This easily explains the slowdown.
>>
>> That's a huge difference.
>>
>> Nothing jumps out as a possible cause except perhaps retpoline or
>> something like that.
>
>I'll keep that in mind - thanks.
>
>My guess was CPU-cache invalidation.
>I just checked and the other CPU core (there are two - each
>hyper-threaded - "other" meaning not the one that handles ethernet
>interrupts) gets several thousand "IPI resched" interrupts while
>running a 10 second (226MByte) iperf3 receive test.
>About 17KB transferred per IPI.
>I cannot see where build_skb() would do cache invalidation though.

It doesn't the driver is responsible for that. How is coherency maintained between cores?

The IPI could be due to receive packet steering, is the MAC multi queue aware on the RX path?
-- 
Florian

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[NeilBrown]

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

On Sun, Dec 16 2018, Florian Fainelli wrote:

> On December 16, 2018 3:19:22 PM PST, NeilBrown <neil@brown.name> wrote:
>>On Sun, Dec 16 2018, David Miller wrote:
>>
>>> From: NeilBrown <neil@brown.name>
>>> Date: Mon, 17 Dec 2018 09:08:54 +1100
>>>
>>>> In my 4.4 kernel, the build_skb() call in (the equivalent of)
>>>> mtk_poll_rx() takes about 1.2usec and the call to napi_gro_receive()
>>>> takes about 3usec.
>>>> 
>>>> In my 4.20 kernel, these calls take about 30 and 24 usec
>>respectively.
>>>> This easily explains the slowdown.
>>>
>>> That's a huge difference.
>>>
>>> Nothing jumps out as a possible cause except perhaps retpoline or
>>> something like that.
>>
>>I'll keep that in mind - thanks.
>>
>>My guess was CPU-cache invalidation.
>>I just checked and the other CPU core (there are two - each
>>hyper-threaded - "other" meaning not the one that handles ethernet
>>interrupts) gets several thousand "IPI resched" interrupts while
>>running a 10 second (226MByte) iperf3 receive test.
>>About 17KB transferred per IPI.
>>I cannot see where build_skb() would do cache invalidation though.
>
> It doesn't the driver is responsible for that. How is coherency maintained between cores?

I suspect so - yes.  Coherency only needs explicit management with DMA
is used.  This wasn't the problem.

Further investigation showed that the problem was that I had
CONFIG_SLUB_DEBUG set.  That was probably useful in some earlier
debugging exercise, but it clearly isn't useful when performance-testing
the network.
I removed that and I have much nicer numbers - not quite the consistent
900+ that I saw with 4.4, but a lot closer.

Thanks for the encouragement, and sorry of the noise.

NeilBrown


>
> The IPI could be due to receive packet steering, is the MAC multi queue aware on the RX path?
> -- 
> Florian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Andrew Lunn]

> 2. Maximal sized RX packets get silently dropped. So receive side packets
>    that are large (perfect case is the all-but-last packets in a fragemented
>    larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>    The 7530 MIB switch register counters show receive packets at the physical
>    switch port side and at the CPU switch port - but I get no packets
>    received or errors in the 7621 ethernet MAC. If I set the mtu of the
>    server at the other end a little smaller (a few bytes is enough) then
>    I get all the packets through. It seems like the DSA/VLAN tag bytes
>    are causing a too large packet to get silently dropped somewhere.

Hi Gerg

Try increasing the MTU on the master device. Some hardware will reject
receiving packets bigger than the MTU. But if you increase the MTU by
the DSA header size, it will then receive the frame.

I have a patchset i will be posting soon to do this automatically.

  Andrew

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Andrew,

On 30/11/18 11:33 pm, Andrew Lunn wrote:
>> 2. Maximal sized RX packets get silently dropped. So receive side packets
>>     that are large (perfect case is the all-but-last packets in a fragemented
>>     larger packet) appear to be dropped at the mt7621 ethernet MAC level.
>>     The 7530 MIB switch register counters show receive packets at the physical
>>     switch port side and at the CPU switch port - but I get no packets
>>     received or errors in the 7621 ethernet MAC. If I set the mtu of the
>>     server at the other end a little smaller (a few bytes is enough) then
>>     I get all the packets through. It seems like the DSA/VLAN tag bytes
>>     are causing a too large packet to get silently dropped somewhere.
> 
> Hi Gerg
> 
> Try increasing the MTU on the master device. Some hardware will reject
> receiving packets bigger than the MTU. But if you increase the MTU by
> the DSA header size, it will then receive the frame.

I tried increasing it on the master, and it fails to set:

  # ifconfig eth0 mut 1500
  # ifconfig eth0 mtu 1504
  eth0: mtu greater than device maximum
  SIOCSIFMTU: Invalid argument

Looking within that staging driver it seems to generously set the size
of the RX descriptor buffers, so there is enough room in them. And glancing
around the driver I don't see it using the MTU to restrict the receive
packaet size (though I may be missing it).


> I have a patchset i will be posting soon to do this automatically.

That would be good.

Regards
Greg

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Andrew Lunn]

> 1. TX packets are not getting an IP header checksum via the normal
>    off-loaded checksumming when in DSA mode. I have to switch off
>    NETIF_F_IP_CSUM, so the software stack generates the checksum.
>    That checksum offloading works ok when not using the 7530 DSA driver.

With some vendors MAC hardware, there is a field in the descriptor to
indicate how big a VLAN tag the frame has. The hardware can then use
this information to skip over the VLAN tags to find the IP header, and
then perform checksuming. You might be able to re-use that, consider
the DSA header as part of the VLAN header.

Other vendors, there is no way i've found to get hadware offload of
checksumming working.

    Andrew

Re: [PATCH 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC

[Greg Ungerer]

Hi Andrew,

On 30/11/18 11:37 pm, Andrew Lunn wrote:
>> 1. TX packets are not getting an IP header checksum via the normal
>>     off-loaded checksumming when in DSA mode. I have to switch off
>>     NETIF_F_IP_CSUM, so the software stack generates the checksum.
>>     That checksum offloading works ok when not using the 7530 DSA driver.
> 
> With some vendors MAC hardware, there is a field in the descriptor to
> indicate how big a VLAN tag the frame has. The hardware can then use
> this information to skip over the VLAN tags to find the IP header, and
> then perform checksuming. You might be able to re-use that, consider
> the DSA header as part of the VLAN header.
> 
> Other vendors, there is no way i've found to get hadware offload of
> checksumming working.

Thanks for the tips, I will try out the master mtu change too.

Regards
Greg