[PATCH net v2 1/5] net: ethernet: mtk_eth_soc: reapply mdc divider on reset

[PATCH net v2 1/5] net: ethernet: mtk_eth_soc: reapply mdc divider on reset

[Daniel Golle]

From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>

In the current method, the MDC divider was reset to the default setting
of 2.5MHz after the NETSYS SER. Therefore, we need to reapply the MDC
divider configuration function in mtk_hw_init() after reset.

Fixes: c0a440031d431 ("net: ethernet: mtk_eth_soc: set MDIO bus clock frequency")
Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: * only parse DT once, store divider in struct mtk_eth
    * make sure MDC is configured before calling of_mdiobus_register()
    * reapply after reset

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 39 +++++++++++++--------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  1 +
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 43197b28b3e74..1a235283b0e9b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -871,9 +871,25 @@ static const struct phylink_mac_ops mtk_phylink_ops = {
 	.mac_enable_tx_lpi = mtk_mac_enable_tx_lpi,
 };
 
+static void mtk_mdio_config(struct mtk_eth *eth)
+{
+	u32 val;
+
+	/* Configure MDC Divider */
+	val = FIELD_PREP(PPSC_MDC_CFG, eth->mdc_divider);
+
+	/* Configure MDC Turbo Mode */
+	if (mtk_is_netsys_v3_or_greater(eth))
+		mtk_m32(eth, 0, MISC_MDC_TURBO, MTK_MAC_MISC_V3);
+	else
+		val |= PPSC_MDC_TURBO;
+
+	mtk_m32(eth, PPSC_MDC_CFG, val, MTK_PPSC);
+}
+
 static int mtk_mdio_init(struct mtk_eth *eth)
 {
-	unsigned int max_clk = 2500000, divider;
+	unsigned int max_clk = 2500000;
 	struct device_node *mii_np;
 	int ret;
 	u32 val;
@@ -908,20 +924,9 @@ static int mtk_mdio_init(struct mtk_eth *eth)
 		}
 		max_clk = val;
 	}
-	divider = min_t(unsigned int, DIV_ROUND_UP(MDC_MAX_FREQ, max_clk), 63);
-
-	/* Configure MDC Turbo Mode */
-	if (mtk_is_netsys_v3_or_greater(eth))
-		mtk_m32(eth, 0, MISC_MDC_TURBO, MTK_MAC_MISC_V3);
-
-	/* Configure MDC Divider */
-	val = FIELD_PREP(PPSC_MDC_CFG, divider);
-	if (!mtk_is_netsys_v3_or_greater(eth))
-		val |= PPSC_MDC_TURBO;
-	mtk_m32(eth, PPSC_MDC_CFG, val, MTK_PPSC);
-
-	dev_dbg(eth->dev, "MDC is running on %d Hz\n", MDC_MAX_FREQ / divider);
-
+	eth->mdc_divider = min_t(unsigned int, DIV_ROUND_UP(MDC_MAX_FREQ, max_clk), 63);
+	mtk_mdio_config(eth);
+	dev_dbg(eth->dev, "MDC is running on %d Hz\n", MDC_MAX_FREQ / eth->mdc_divider);
 	ret = of_mdiobus_register(eth->mii_bus, mii_np);
 
 err_put_node:
@@ -3974,6 +3979,10 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 	else
 		mtk_hw_reset(eth);
 
+	/* No MT7628/88 support yet */
+	if (reset && !MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		mtk_mdio_config(eth);
+
 	if (mtk_is_netsys_v3_or_greater(eth)) {
 		/* Set FE to PDMAv2 if necessary */
 		val = mtk_r32(eth, MTK_FE_GLO_MISC);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 90a377ab4359e..39709649ea8d1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1271,6 +1271,7 @@ struct mtk_eth {
 	struct clk			*clks[MTK_CLK_MAX];
 
 	struct mii_bus			*mii_bus;
+	unsigned int			mdc_divider;
 	struct work_struct		pending_work;
 	unsigned long			state;
 
-- 
2.49.0

[PATCH net v2 2/5] net: ethernet: mtk_eth_soc: correct the max weight of the queue limit for 100Mbps

[Daniel Golle]

From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>

Without this patch, the maximum weight of the queue limit will be
incorrect when linked at 100Mbps due to an apparent typo.

Fixes: f63959c7eec31 ("net: ethernet: mtk_eth_soc: implement multi-queue support for per-port queues")
Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no change

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1a235283b0e9b..5a3cfb8908a17 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -734,7 +734,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
 		case SPEED_100:
 			val |= MTK_QTX_SCH_MAX_RATE_EN |
 			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 103) |
-			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 3);
+			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 3) |
 			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
 			break;
 		case SPEED_1000:
@@ -757,7 +757,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
 		case SPEED_100:
 			val |= MTK_QTX_SCH_MAX_RATE_EN |
 			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
-			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5);
+			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
 			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
 			break;
 		case SPEED_1000:
-- 
2.49.0

[PATCH net v2 3/5] net: ethernet: mtk_eth_soc: revise QDMA packet scheduler settings

[Daniel Golle]

From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>

The QDMA packet scheduler suffers from a performance issue.
Fix this by picking up changes from MediaTek's SDK which change to use
Token Bucket instead of Leaky Bucket and fix the SPEED_1000 configuration.

Fixes: 160d3a9b1929 ("net: ethernet: mtk_eth_soc: introduce MTK_NETSYS_V2 support")
Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no change
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 5a3cfb8908a17..bdb98c9d8b1c1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -762,8 +762,8 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
 			break;
 		case SPEED_1000:
 			val |= MTK_QTX_SCH_MAX_RATE_EN |
-			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 10) |
-			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
+			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
+			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 6) |
 			       FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 10);
 			break;
 		default:
@@ -3320,7 +3320,7 @@ static int mtk_start_dma(struct mtk_eth *eth)
 		if (mtk_is_netsys_v2_or_greater(eth))
 			val |= MTK_MUTLI_CNT | MTK_RESV_BUF |
 			       MTK_WCOMP_EN | MTK_DMAD_WR_WDONE |
-			       MTK_CHK_DDONE_EN | MTK_LEAKY_BUCKET_EN;
+			       MTK_CHK_DDONE_EN;
 		else
 			val |= MTK_RX_BT_32DWORDS;
 		mtk_w32(eth, val, reg_map->qdma.glo_cfg);
-- 
2.49.0

[PATCH net v2 4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration

[Daniel Golle]

From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>

Change hardware configuration for the NETSYSv3.
 - Enable PSE dummy page mechanism for the GDM1/2/3
 - Enable PSE drop mechanism when the WDMA Rx ring full
 - Enable PSE no-drop mechanism for packets from the WDMA Tx
 - Correct PSE free drop threshold
 - Correct PSE CDMA high threshold

Fixes: 1953f134a1a8b ("net: ethernet: mtk_eth_soc: add NETSYS_V3 version support")
Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no change

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 20 ++++++++++++++++----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  7 ++++++-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index bdb98c9d8b1c1..338849b488cae 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -4043,11 +4043,23 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 	mtk_w32(eth, 0x21021000, MTK_FE_INT_GRP);
 
 	if (mtk_is_netsys_v3_or_greater(eth)) {
-		/* PSE should not drop port1, port8 and port9 packets */
-		mtk_w32(eth, 0x00000302, PSE_DROP_CFG);
+		/* PSE dummy page mechanism */
+		mtk_w32(eth, PSE_DUMMY_WORK_GDM(1) | PSE_DUMMY_WORK_GDM(2) |
+			PSE_DUMMY_WORK_GDM(3) | DUMMY_PAGE_THR, PSE_DUMY_REQ);
+
+		/* PSE free buffer drop threshold */
+		mtk_w32(eth, 0x00600009, PSE_IQ_REV(8));
+
+		/* PSE should not drop port8, port9 and port13 packets from WDMA Tx */
+		mtk_w32(eth, 0x00002300, PSE_DROP_CFG);
+
+		/* PSE should drop packets to port8, port9 and port13 on WDMA Rx ring full */
+		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(0));
+		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(1));
+		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(2));
 
 		/* GDM and CDM Threshold */
-		mtk_w32(eth, 0x00000707, MTK_CDMW0_THRES);
+		mtk_w32(eth, 0x08000707, MTK_CDMW0_THRES);
 		mtk_w32(eth, 0x00000077, MTK_CDMW1_THRES);
 
 		/* Disable GDM1 RX CRC stripping */
@@ -4064,7 +4076,7 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 		mtk_w32(eth, 0x00000300, PSE_DROP_CFG);
 
 		/* PSE should drop packets to port 8/9 on WDMA Rx ring full */
-		mtk_w32(eth, 0x00000300, PSE_PPE0_DROP);
+		mtk_w32(eth, 0x00000300, PSE_PPE_DROP(0));
 
 		/* PSE Free Queue Flow Control  */
 		mtk_w32(eth, 0x01fa01f4, PSE_FQFC_CFG2);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 39709649ea8d1..eaa96c8483b70 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -151,7 +151,12 @@
 #define PSE_FQFC_CFG1		0x100
 #define PSE_FQFC_CFG2		0x104
 #define PSE_DROP_CFG		0x108
-#define PSE_PPE0_DROP		0x110
+#define PSE_PPE_DROP(x)		(0x110 + ((x) * 0x4))
+
+/* PSE Last FreeQ Page Request Control */
+#define PSE_DUMY_REQ		0x10C
+#define PSE_DUMMY_WORK_GDM(x)	BIT(16 + (x))
+#define DUMMY_PAGE_THR		0x1
 
 /* PSE Input Queue Reservation Register*/
 #define PSE_IQ_REV(x)		(0x140 + (((x) - 1) << 2))
-- 
2.49.0

[PATCH net v2 5/5] net: ethernet: mtk_eth_soc: convert cap_bit in mtk_eth_muxc struct to u64

[Daniel Golle]

From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>

The capabilities bitfield was converted to a 64-bit value, but a cap_bit
in struct mtk_eth_muxc which is used to store a full bitfield (rather
than the bit number, as the name would suggest) still holds only a
32-bit value.

Change the type of cap_bit to u64 in order to avoid truncating the
bitfield which results in path selection to not work with capabilities
above the 32-bit limit.

Fixes: 51a4df60db5c2 ("net: ethernet: mtk_eth_soc: convert caps in mtk_soc_data struct to u64")
Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: improve commit description

 drivers/net/ethernet/mediatek/mtk_eth_path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_path.c b/drivers/net/ethernet/mediatek/mtk_eth_path.c
index 7c27a19c4d8f4..6fbfb16438a51 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_path.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_path.c
@@ -14,7 +14,7 @@
 
 struct mtk_eth_muxc {
 	const char	*name;
-	int		cap_bit;
+	u64		cap_bit;
 	int		(*set_path)(struct mtk_eth *eth, u64 path);
 };
 
-- 
2.49.0

Re: [PATCH net v2 4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration

[Jakub Kicinski]

On Wed, 16 Apr 2025 01:51:42 +0100 Daniel Golle wrote:
> +		/* PSE should not drop port8, port9 and port13 packets from WDMA Tx */
> +		mtk_w32(eth, 0x00002300, PSE_DROP_CFG);
> +
> +		/* PSE should drop packets to port8, port9 and port13 on WDMA Rx ring full */

nit: please try to wrap at 80 chars. There's really no need to go over
on comments. Some of us stick to 80 char terminals. 

> +		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(0));
> +		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(1));
> +		mtk_w32(eth, 0x00002300, PSE_PPE_DROP(2));
>  
>  		/* GDM and CDM Threshold */
> -		mtk_w32(eth, 0x00000707, MTK_CDMW0_THRES);
> +		mtk_w32(eth, 0x08000707, MTK_CDMW0_THRES);
>  		mtk_w32(eth, 0x00000077, MTK_CDMW1_THRES);
>  
>  		/* Disable GDM1 RX CRC stripping */
> @@ -4064,7 +4076,7 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
>  		mtk_w32(eth, 0x00000300, PSE_DROP_CFG);
>  
>  		/* PSE should drop packets to port 8/9 on WDMA Rx ring full */
> -		mtk_w32(eth, 0x00000300, PSE_PPE0_DROP);
> +		mtk_w32(eth, 0x00000300, PSE_PPE_DROP(0));
>  
>  		/* PSE Free Queue Flow Control  */
>  		mtk_w32(eth, 0x01fa01f4, PSE_FQFC_CFG2);
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> index 39709649ea8d1..eaa96c8483b70 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> @@ -151,7 +151,12 @@
>  #define PSE_FQFC_CFG1		0x100
>  #define PSE_FQFC_CFG2		0x104
>  #define PSE_DROP_CFG		0x108
> -#define PSE_PPE0_DROP		0x110
> +#define PSE_PPE_DROP(x)		(0x110 + ((x) * 0x4))
> +
> +/* PSE Last FreeQ Page Request Control */
> +#define PSE_DUMY_REQ		0x10C

This really looks like misspelling of DUMMY, is it really supposed 
to have one 'M' ?

Re: [PATCH net v2 5/5] net: ethernet: mtk_eth_soc: convert cap_bit in mtk_eth_muxc struct to u64

[Jakub Kicinski]

On Wed, 16 Apr 2025 01:52:03 +0100 Daniel Golle wrote:
> The capabilities bitfield was converted to a 64-bit value, but a cap_bit
> in struct mtk_eth_muxc which is used to store a full bitfield (rather
> than the bit number, as the name would suggest) still holds only a
> 32-bit value.
> 
> Change the type of cap_bit to u64 in order to avoid truncating the
> bitfield which results in path selection to not work with capabilities
> above the 32-bit limit.

Could you please be more specific and name a bit or a field that goes
over 32b? Since this is a fix ideally we'd also have impact to the user
described in the commit message. But having enough info for the reviewer
to quickly validate the change is the bare minimum.

Re: [PATCH net v2 1/5] net: ethernet: mtk_eth_soc: reapply mdc divider on reset

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 16 Apr 2025 01:50:46 +0100 you wrote:
> From: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
> 
> In the current method, the MDC divider was reset to the default setting
> of 2.5MHz after the NETSYS SER. Therefore, we need to reapply the MDC
> divider configuration function in mtk_hw_init() after reset.
> 
> Fixes: c0a440031d431 ("net: ethernet: mtk_eth_soc: set MDIO bus clock frequency")
> Signed-off-by: Bo-Cun Chen <bc-bocun.chen@mediatek.com>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> 
> [...]

Here is the summary with links:
  - [net,v2,1/5] net: ethernet: mtk_eth_soc: reapply mdc divider on reset
    https://git.kernel.org/netdev/net/c/6bc2b6c6f16d
  - [net,v2,2/5] net: ethernet: mtk_eth_soc: correct the max weight of the queue limit for 100Mbps
    https://git.kernel.org/netdev/net/c/6b02eb372c67
  - [net,v2,3/5] net: ethernet: mtk_eth_soc: revise QDMA packet scheduler settings
    https://git.kernel.org/netdev/net/c/1b66124135f5
  - [net,v2,4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration
    (no matching commit)
  - [net,v2,5/5] net: ethernet: mtk_eth_soc: convert cap_bit in mtk_eth_muxc struct to u64
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2 4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration

[Daniel Golle]

On Thu, Apr 17, 2025 at 08:10:55AM -0700, Jakub Kicinski wrote:
> On Wed, 16 Apr 2025 01:51:42 +0100 Daniel Golle wrote:
> > +		/* PSE should not drop port8, port9 and port13 packets from WDMA Tx */
> > +		mtk_w32(eth, 0x00002300, PSE_DROP_CFG);
> > +
> > +		/* PSE should drop packets to port8, port9 and port13 on WDMA Rx ring full */
> 
> nit: please try to wrap at 80 chars. There's really no need to go over
> on comments. Some of us stick to 80 char terminals. 

Too late now to send another revision...

> > [...]
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > index 39709649ea8d1..eaa96c8483b70 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > @@ -151,7 +151,12 @@
> >  #define PSE_FQFC_CFG1		0x100
> >  #define PSE_FQFC_CFG2		0x104
> >  #define PSE_DROP_CFG		0x108
> > -#define PSE_PPE0_DROP		0x110
> > +#define PSE_PPE_DROP(x)		(0x110 + ((x) * 0x4))
> > +
> > +/* PSE Last FreeQ Page Request Control */
> > +#define PSE_DUMY_REQ		0x10C
> 
> This really looks like misspelling of DUMMY, is it really supposed 
> to have one 'M' ?

I also thought that when I first saw that and have told MediaTek engineers
about it, they told me that the register is called like that also in their
datasheet and hence they want the name to be consistent in the driver.

Re: [PATCH net v2 5/5] net: ethernet: mtk_eth_soc: convert cap_bit in mtk_eth_muxc struct to u64

[Daniel Golle]

On Thu, Apr 17, 2025 at 08:13:25AM -0700, Jakub Kicinski wrote:
> On Wed, 16 Apr 2025 01:52:03 +0100 Daniel Golle wrote:
> > The capabilities bitfield was converted to a 64-bit value, but a cap_bit
> > in struct mtk_eth_muxc which is used to store a full bitfield (rather
> > than the bit number, as the name would suggest) still holds only a
> > 32-bit value.
> > 
> > Change the type of cap_bit to u64 in order to avoid truncating the
> > bitfield which results in path selection to not work with capabilities
> > above the 32-bit limit.
> 
> Could you please be more specific and name a bit or a field that goes
> over 32b? Since this is a fix ideally we'd also have impact to the user
> described in the commit message. But having enough info for the reviewer
> to quickly validate the change is the bare minimum.

I reckon it's too late to include that information in the commit
description. However, let me still illustrate the problem here now:

In mtk_eth_soc.h:

enum mkt_eth_capabilities {
	MTK_RGMII_BIT = 0,
	MTK_TRGMII_BIT,
	MTK_SGMII_BIT,
	MTK_USXGMII_BIT,
	MTK_2P5GPHY_BIT,
	MTK_ESW_BIT,
	MTK_GEPHY_BIT,
	MTK_MUX_BIT,
	MTK_INFRA_BIT,
	MTK_SHARED_SGMII_BIT,
	MTK_HWLRO_BIT,
	MTK_RSS_BIT,
	MTK_SHARED_INT_BIT,
	MTK_PDMA_INT_BIT,
	MTK_TRGMII_MT7621_CLK_BIT,
	MTK_QDMA_BIT,
	MTK_SOC_MT7628_BIT,
	MTK_RSTCTRL_PPE1_BIT,
	MTK_RSTCTRL_PPE2_BIT,
	MTK_U3_COPHY_V2_BIT,
	MTK_SRAM_BIT,
	MTK_36BIT_DMA_BIT,

	/* MUX BITS*/
	MTK_ETH_MUX_GDM1_TO_GMAC1_ESW_BIT,
	MTK_ETH_MUX_GMAC2_GMAC0_TO_GEPHY_BIT,
	MTK_ETH_MUX_U3_GMAC2_TO_QPHY_BIT,
	MTK_ETH_MUX_GMAC2_TO_2P5GPHY_BIT,
	MTK_ETH_MUX_GMAC1_GMAC2_TO_SGMII_RGMII_BIT,
	MTK_ETH_MUX_GMAC12_TO_GEPHY_SGMII_BIT,
	MTK_ETH_MUX_GMAC123_TO_GEPHY_SGMII_BIT,
	MTK_ETH_MUX_GMAC123_TO_USXGMII_BIT,

	/* PATH BITS */
	MTK_ETH_PATH_GMAC1_RGMII_BIT,
	MTK_ETH_PATH_GMAC1_TRGMII_BIT,
	MTK_ETH_PATH_GMAC1_SGMII_BIT,
	MTK_ETH_PATH_GMAC2_RGMII_BIT,
	MTK_ETH_PATH_GMAC2_SGMII_BIT,
	MTK_ETH_PATH_GMAC2_2P5GPHY_BIT,
	MTK_ETH_PATH_GMAC2_GEPHY_BIT,
	MTK_ETH_PATH_GMAC3_SGMII_BIT,
	MTK_ETH_PATH_GDM1_ESW_BIT,
	MTK_ETH_PATH_GMAC1_USXGMII_BIT,
	MTK_ETH_PATH_GMAC2_USXGMII_BIT,
	MTK_ETH_PATH_GMAC3_USXGMII_BIT,
};

...

#define MTK_ETH_MUX_GDM1_TO_GMAC1_ESW		\
	BIT_ULL(MTK_ETH_MUX_GDM1_TO_GMAC1_ESW_BIT)
#define MTK_ETH_MUX_GMAC2_GMAC0_TO_GEPHY	\
	BIT_ULL(MTK_ETH_MUX_GMAC2_GMAC0_TO_GEPHY_BIT)
#define MTK_ETH_MUX_U3_GMAC2_TO_QPHY		\
	BIT_ULL(MTK_ETH_MUX_U3_GMAC2_TO_QPHY_BIT)
#define MTK_ETH_MUX_GMAC2_TO_2P5GPHY		\
	BIT_ULL(MTK_ETH_MUX_GMAC2_TO_2P5GPHY_BIT)
#define MTK_ETH_MUX_GMAC1_GMAC2_TO_SGMII_RGMII	\
	BIT_ULL(MTK_ETH_MUX_GMAC1_GMAC2_TO_SGMII_RGMII_BIT)
#define MTK_ETH_MUX_GMAC12_TO_GEPHY_SGMII	\
	BIT_ULL(MTK_ETH_MUX_GMAC12_TO_GEPHY_SGMII_BIT)
#define MTK_ETH_MUX_GMAC123_TO_GEPHY_SGMII	\
	BIT_ULL(MTK_ETH_MUX_GMAC123_TO_GEPHY_SGMII_BIT)
#define MTK_ETH_MUX_GMAC123_TO_USXGMII	\
	BIT_ULL(MTK_ETH_MUX_GMAC123_TO_USXGMII_BIT)

The above MTK_ETH_MUX_* macros are the ones used as values for cap_bit in
mtk_eth_path.c.
So technically MTK_ETH_MUX_GMAC123_TO_USXGMII == BIT_ULL(29)
which is still fine for a 32-bit value, but never the less BIT_ULL returns
a 64-bit type and hence using a 32-bit type to store the result is at
least misleading.

However, you are right that with the currently supported SoCs this doesn't
result in an actual bug (but it will with the upcoming addition of newer SoC
like MT7987).

Re: [PATCH net v2 4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration

[Jakub Kicinski]

On Thu, 17 Apr 2025 16:45:14 +0100 Daniel Golle wrote:
> On Thu, Apr 17, 2025 at 08:10:55AM -0700, Jakub Kicinski wrote:
> > On Wed, 16 Apr 2025 01:51:42 +0100 Daniel Golle wrote:  
> > > +		/* PSE should not drop port8, port9 and port13 packets from WDMA Tx */
> > > +		mtk_w32(eth, 0x00002300, PSE_DROP_CFG);
> > > +
> > > +		/* PSE should drop packets to port8, port9 and port13 on WDMA Rx ring full */  
> > 
> > nit: please try to wrap at 80 chars. There's really no need to go over
> > on comments. Some of us stick to 80 char terminals.   
> 
> Too late now to send another revision...

I only applied the first 3 :)

> > > [...]
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > index 39709649ea8d1..eaa96c8483b70 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > @@ -151,7 +151,12 @@
> > >  #define PSE_FQFC_CFG1		0x100
> > >  #define PSE_FQFC_CFG2		0x104
> > >  #define PSE_DROP_CFG		0x108
> > > -#define PSE_PPE0_DROP		0x110
> > > +#define PSE_PPE_DROP(x)		(0x110 + ((x) * 0x4))
> > > +
> > > +/* PSE Last FreeQ Page Request Control */
> > > +#define PSE_DUMY_REQ		0x10C  
> > 
> > This really looks like misspelling of DUMMY, is it really supposed 
> > to have one 'M' ?  
> 
> I also thought that when I first saw that and have told MediaTek engineers
> about it, they told me that the register is called like that also in their
> datasheet and hence they want the name to be consistent in the driver.

Hm, maybe add a comment ? It confused both of us, probably going 
to confuse most people later on

Re: [PATCH net v2 4/5] net: ethernet: mtk_eth_soc: net: revise NETSYSv3 hardware configuration

[Daniel Golle]

On Thu, Apr 17, 2025 at 08:59:48AM -0700, Jakub Kicinski wrote:
> On Thu, 17 Apr 2025 16:45:14 +0100 Daniel Golle wrote:
> > On Thu, Apr 17, 2025 at 08:10:55AM -0700, Jakub Kicinski wrote:
> > > On Wed, 16 Apr 2025 01:51:42 +0100 Daniel Golle wrote:  
> > > > +		/* PSE should not drop port8, port9 and port13 packets from WDMA Tx */
> > > > +		mtk_w32(eth, 0x00002300, PSE_DROP_CFG);
> > > > +
> > > > +		/* PSE should drop packets to port8, port9 and port13 on WDMA Rx ring full */  
> > > 
> > > nit: please try to wrap at 80 chars. There's really no need to go over
> > > on comments. Some of us stick to 80 char terminals.   
> > 
> > Too late now to send another revision...
> 
> I only applied the first 3 :)

Perfect, so I'll roll up the remaining two with the changes suggested.

> 
> > > > [...]
> > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > > index 39709649ea8d1..eaa96c8483b70 100644
> > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
> > > > @@ -151,7 +151,12 @@
> > > >  #define PSE_FQFC_CFG1		0x100
> > > >  #define PSE_FQFC_CFG2		0x104
> > > >  #define PSE_DROP_CFG		0x108
> > > > -#define PSE_PPE0_DROP		0x110
> > > > +#define PSE_PPE_DROP(x)		(0x110 + ((x) * 0x4))
> > > > +
> > > > +/* PSE Last FreeQ Page Request Control */
> > > > +#define PSE_DUMY_REQ		0x10C  
> > > 
> > > This really looks like misspelling of DUMMY, is it really supposed 
> > > to have one 'M' ?  
> > 
> > I also thought that when I first saw that and have told MediaTek engineers
> > about it, they told me that the register is called like that also in their
> > datasheet and hence they want the name to be consistent in the driver.
> 
> Hm, maybe add a comment ? It confused both of us, probably going 
> to confuse most people later on

Ok, will do and send v3.

Thank you!