net: stmmac: Refactor FPE as a separate module

PATCH Superseded Furong Xu 3 participants 18 messages Oct 15, 2024 – Oct 18, 2024

Subsystems (4)

Networking Network drivers Arm STM32 stmmac Ethernet driver

lore

Share & tools

lore

Furong Xu

author

2024-10-15 09:09 UTC #0 lore raw

cover net: stmmac: Refactor FPE as a separate module

Refactor FPE implementation by moving common code for DWMAC4 and DWXGMAC into a separate FPE module.

FPE implementation for DWMAC4 and DWXGMAC differs only for:
1) Offset address of MAC_FPE_CTRL_STS and MTL_FPE_CTRL_STS
2) FPRQ(Frame Preemption Residue Queue) field in MAC_RxQ_Ctrl1

Tested on DWMAC CORE 5.20a and DWXGMAC CORE 3.20a

Furong Xu (5):
net: stmmac: Introduce separate files for FPE implementation
net: stmmac: Introduce stmmac_fpe_ops for gmac4 and xgmac
net: stmmac: Rework marco definitions for gmac4 and xgmac
net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ
net: stmmac: xgmac: Complete FPE support

 drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
 drivers/net/ethernet/stmicro/stmmac/common.h  |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c |  12 -
 drivers/net/ethernet/stmicro/stmmac/dwmac5.c  | 150 ------
 drivers/net/ethernet/stmicro/stmmac/dwmac5.h  |  26 --
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |   7 +-
 .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  28 --
 drivers/net/ethernet/stmicro/stmmac/hwif.c    |   7 +
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  54 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  10 -
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 442 ++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.h  |  38 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 149 +-----
 .../net/ethernet/stmicro/stmmac/stmmac_tc.c   |   4 +-
 15 files changed, 527 insertions(+), 405 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h

Signature · 1 line

2.34.1

Furong Xu

author

2024-10-15 09:10 UTC #1 lore raw

PATCH 1/5 net: stmmac: Introduce separate files for FPE implementation

By moving FPE related code info separate files, FPE implementation becomes a separate module initially.
No functional change intended.

Signed-off-by Furong Xu <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac5.c  | 150 --------
 drivers/net/ethernet/stmicro/stmmac/dwmac5.h  |  26 --
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |   2 -
 .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  27 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  10 -
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 336 ++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.h  |  54 +++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 149 +-------
 11 files changed, 396 insertions(+), 363 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h

drivers/net/ethernet/stmicro/stmmac/Makefile +1 -1

index c2f0e91f6bf8..7e46dca90628 100644
@@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o	\
 	      mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o dwmac4_descs.o	\
 	      dwmac4_dma.o dwmac4_lib.o dwmac4_core.o dwmac5.o hwif.o \
 	      stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
-	      stmmac_xdp.o stmmac_est.o \
+	      stmmac_xdp.o stmmac_est.o stmmac_fpe.o \
 	      $(stmmac-y)
 
 stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +1 -0

index e65a65666cc1..4d217926820a 100644
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include "stmmac.h"
+#include "stmmac_fpe.h"
 #include "stmmac_pcs.h"
 #include "dwmac4.h"
 #include "dwmac5.h"

drivers/net/ethernet/stmicro/stmmac/dwmac5.c +0 ~~-150~~

153 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/dwmac5.h +0 ~~-26~~

index 6c6eb6790e83..00b151b3b688 100644
@@ -11,15 +11,6 @@
 #define PRTYEN				BIT(1)
 #define TMOUTEN				BIT(0)
 
-#define MAC_FPE_CTRL_STS		0x00000234
-#define TRSP				BIT(19)
-#define TVER				BIT(18)
-#define RRSP				BIT(17)
-#define RVER				BIT(16)
-#define SRSP				BIT(2)
-#define SVER				BIT(1)
-#define EFPE				BIT(0)
-
 #define MAC_PPS_CONTROL			0x00000b70
 #define PPS_MAXIDX(x)			((((x) + 1) * 8) - 1)
 #define PPS_MINIDX(x)			((x) * 8)

@@ -39,12 +30,6 @@
 #define MAC_PPSx_INTERVAL(x)		(0x00000b88 + ((x) * 0x10))
 #define MAC_PPSx_WIDTH(x)		(0x00000b8c + ((x) * 0x10))
 
-#define MTL_FPE_CTRL_STS		0x00000c90
-/* Preemption Classification */
-#define DWMAC5_PREEMPTION_CLASS		GENMASK(15, 8)
-/* Additional Fragment Size of preempted frames */
-#define DWMAC5_ADD_FRAG_SZ		GENMASK(1, 0)
-
 #define MTL_RXP_CONTROL_STATUS		0x00000ca0
 #define RXPI				BIT(31)
 #define NPE				GENMASK(23, 16)

@@ -108,16 +93,5 @@ int dwmac5_rxp_config(void __iomem *ioaddr, struct stmmac_tc_entry *entries,
 int dwmac5_flex_pps_config(void __iomem *ioaddr, int index,
 			   struct stmmac_pps_cfg *cfg, bool enable,
 			   u32 sub_second_inc, u32 systime_flags);
-void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
-			  u32 num_txq, u32 num_rxq,
-			  bool tx_enable, bool pmac_enable);
-void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,
-			     struct stmmac_fpe_cfg *cfg,
-			     enum stmmac_mpacket_type type);
-int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev);
-int dwmac5_fpe_get_add_frag_size(const void __iomem *ioaddr);
-void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size);
-int dwmac5_fpe_map_preemption_class(struct net_device *ndev,
-				    struct netlink_ext_ack *extack, u32 pclass);
 
 #endif /* __DWMAC5_H__ */

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +0 -2

index 6a2c7d22df1e..917796293c26 100644
@@ -193,8 +193,6 @@
 #define XGMAC_MDIO_ADDR			0x00000200
 #define XGMAC_MDIO_DATA			0x00000204
 #define XGMAC_MDIO_C22P			0x00000220
-#define XGMAC_FPE_CTRL_STS		0x00000280
-#define XGMAC_EFPE			BIT(0)
 #define XGMAC_ADDRx_HIGH(x)		(0x00000300 + (x) * 0x8)
 #define XGMAC_ADDR_MAX			32
 #define XGMAC_AE			BIT(31)

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +1 ~~-26~~

index f519d43738b0..111ba5a524ed 100644
@@ -8,6 +8,7 @@
 #include <linux/crc32.h>
 #include <linux/iopoll.h>
 #include "stmmac.h"
+#include "stmmac_fpe.h"
 #include "stmmac_ptp.h"
 #include "dwxlgmac2.h"
 #include "dwxgmac2.h"

@@ -1504,32 +1505,6 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,
 	writel(value, ioaddr + XGMAC_RX_CONFIG);
 }
 
-static void dwxgmac3_fpe_configure(void __iomem *ioaddr,
-				   struct stmmac_fpe_cfg *cfg,
-				   u32 num_txq, u32 num_rxq,
-				   bool tx_enable, bool pmac_enable)
-{
-	u32 value;
-
-	if (!tx_enable) {
-		value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
-
-		value &= ~XGMAC_EFPE;
-
-		writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
-		return;
-	}
-
-	value = readl(ioaddr + XGMAC_RXQ_CTRL1);
-	value &= ~XGMAC_RQ;
-	value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
-	writel(value, ioaddr + XGMAC_RXQ_CTRL1);
-
-	value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
-	value |= XGMAC_EFPE;
-	writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
-}
-
 const struct stmmac_ops dwxgmac210_ops = {
 	.core_init = dwxgmac2_core_init,
 	.set_mac = dwxgmac2_set_mac,

drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 ~~-10~~

index ea135203ff2e..816b979e72cc 100644
@@ -146,15 +146,6 @@ struct stmmac_channel {
 	u32 index;
 };
 
-/* FPE link-partner hand-shaking mPacket type */
-enum stmmac_mpacket_type {
-	MPACKET_VERIFY = 0,
-	MPACKET_RESPONSE = 1,
-};
-
-#define STMMAC_FPE_MM_MAX_VERIFY_RETRIES	3
-#define STMMAC_FPE_MM_MAX_VERIFY_TIME_MS	128
-
 struct stmmac_fpe_cfg {
 	/* Serialize access to MAC Merge state between ethtool requests
 	 * and link state updates.

@@ -420,7 +411,6 @@ bool stmmac_eee_init(struct stmmac_priv *priv);
 int stmmac_reinit_queues(struct net_device *dev, u32 rx_cnt, u32 tx_cnt);
 int stmmac_reinit_ringparam(struct net_device *dev, u32 rx_size, u32 tx_size);
 int stmmac_bus_clks_config(struct stmmac_priv *priv, bool enabled);
-void stmmac_fpe_apply(struct stmmac_priv *priv);
 
 static inline bool stmmac_xdp_is_enabled(struct stmmac_priv *priv)
 {

drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +1 -1

index 2a37592a6281..2792a4c6cbcd 100644
@@ -17,9 +17,9 @@
 #include <linux/net_tstamp.h>
 
 #include "stmmac.h"
+#include "stmmac_fpe.h"
 #include "dwmac_dma.h"
 #include "dwxgmac2.h"
-#include "dwmac5.h"
 
 #define REG_SPACE_SIZE	0x1060
 #define GMAC4_REG_SPACE_SIZE	0x116C

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c new file +336 -0

336 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h new file +54 -0

54 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 ~~-147~~

179 lines not on this page · raw message

Signature · 1 line

2.34.1

Furong Xu

author

2024-10-15 09:10 UTC #3 lore raw

PATCH 2/5 net: stmmac: Introduce stmmac_fpe_ops for gmac4 and xgmac

In reply to Furong Xu

By moving FPE function callbacks from stmmac_ops to stmmac_fpe_ops, FPE implementation becomes a separate module completely, like the EST implementation.

Signed-off-by Furong Xu <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  1 +
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 13 -----
 .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  3 --
 drivers/net/ethernet/stmicro/stmmac/hwif.c    |  7 +++
 drivers/net/ethernet/stmicro/stmmac/hwif.h    | 54 ++++++++++---------
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 44 ++++++++++-----
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.h  | 16 ------
 7 files changed, 69 insertions(+), 69 deletions(-)

drivers/net/ethernet/stmicro/stmmac/common.h +1 -0

index 684489156dce..a6e6ef687f36 100644
@@ -592,6 +592,7 @@ struct mac_device_info {
 	const struct stmmac_tc_ops *tc;
 	const struct stmmac_mmc_ops *mmc;
 	const struct stmmac_est_ops *est;
+	const struct stmmac_fpe_ops *fpe;
 	struct dw_xpcs *xpcs;
 	struct phylink_pcs *phylink_pcs;
 	struct mii_regs mii;	/* MII register Addresses */

drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c +0 ~~-13~~

index 4d217926820a..1c45b7c1660f 100644
@@ -16,7 +16,6 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include "stmmac.h"
-#include "stmmac_fpe.h"
 #include "stmmac_pcs.h"
 #include "dwmac4.h"
 #include "dwmac5.h"

@@ -1262,12 +1261,6 @@ const struct stmmac_ops dwmac410_ops = {
 	.set_arp_offload = dwmac4_set_arp_offload,
 	.config_l3_filter = dwmac4_config_l3_filter,
 	.config_l4_filter = dwmac4_config_l4_filter,
-	.fpe_configure = dwmac5_fpe_configure,
-	.fpe_send_mpacket = dwmac5_fpe_send_mpacket,
-	.fpe_irq_status = dwmac5_fpe_irq_status,
-	.fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size,
-	.fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size,
-	.fpe_map_preemption_class = dwmac5_fpe_map_preemption_class,
 	.add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr,
 	.del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr,
 	.restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,

@@ -1317,12 +1310,6 @@ const struct stmmac_ops dwmac510_ops = {
 	.set_arp_offload = dwmac4_set_arp_offload,
 	.config_l3_filter = dwmac4_config_l3_filter,
 	.config_l4_filter = dwmac4_config_l4_filter,
-	.fpe_configure = dwmac5_fpe_configure,
-	.fpe_send_mpacket = dwmac5_fpe_send_mpacket,
-	.fpe_irq_status = dwmac5_fpe_irq_status,
-	.fpe_get_add_frag_size = dwmac5_fpe_get_add_frag_size,
-	.fpe_set_add_frag_size = dwmac5_fpe_set_add_frag_size,
-	.fpe_map_preemption_class = dwmac5_fpe_map_preemption_class,
 	.add_hw_vlan_rx_fltr = dwmac4_add_hw_vlan_rx_fltr,
 	.del_hw_vlan_rx_fltr = dwmac4_del_hw_vlan_rx_fltr,
 	.restore_hw_vlan_rx_fltr = dwmac4_restore_hw_vlan_rx_fltr,

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +0 -3

index 111ba5a524ed..ce2cbae15973 100644
@@ -8,7 +8,6 @@
 #include <linux/crc32.h>
 #include <linux/iopoll.h>
 #include "stmmac.h"
-#include "stmmac_fpe.h"
 #include "stmmac_ptp.h"
 #include "dwxlgmac2.h"
 #include "dwxgmac2.h"

@@ -1545,7 +1544,6 @@ const struct stmmac_ops dwxgmac210_ops = {
 	.config_l3_filter = dwxgmac2_config_l3_filter,
 	.config_l4_filter = dwxgmac2_config_l4_filter,
 	.set_arp_offload = dwxgmac2_set_arp_offload,
-	.fpe_configure = dwxgmac3_fpe_configure,
 };
 
 static void dwxlgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode,

@@ -1602,7 +1600,6 @@ const struct stmmac_ops dwxlgmac2_ops = {
 	.config_l3_filter = dwxgmac2_config_l3_filter,
 	.config_l4_filter = dwxgmac2_config_l4_filter,
 	.set_arp_offload = dwxgmac2_set_arp_offload,
-	.fpe_configure = dwxgmac3_fpe_configure,
 };
 
 int dwxgmac2_setup(struct stmmac_priv *priv)

drivers/net/ethernet/stmicro/stmmac/hwif.c +7 -0

index 88cce28b2f98..5969963d132b 100644
@@ -116,6 +116,7 @@ static const struct stmmac_hwif_entry {
 	const void *tc;
 	const void *mmc;
 	const void *est;
+	const void *fpe;
 	int (*setup)(struct stmmac_priv *priv);
 	int (*quirks)(struct stmmac_priv *priv);
 } stmmac_hw[] = {

@@ -194,6 +195,7 @@ static const struct stmmac_hwif_entry {
 		.tc = &dwmac510_tc_ops,
 		.mmc = &dwmac_mmc_ops,
 		.est = &dwmac510_est_ops,
+		.fpe = &dwmac5_fpe_ops,
 		.setup = dwmac4_setup,
 		.quirks = NULL,
 	}, {

@@ -214,6 +216,7 @@ static const struct stmmac_hwif_entry {
 		.tc = &dwmac510_tc_ops,
 		.mmc = &dwmac_mmc_ops,
 		.est = &dwmac510_est_ops,
+		.fpe = &dwmac5_fpe_ops,
 		.setup = dwmac4_setup,
 		.quirks = NULL,
 	}, {

@@ -234,6 +237,7 @@ static const struct stmmac_hwif_entry {
 		.tc = &dwmac510_tc_ops,
 		.mmc = &dwmac_mmc_ops,
 		.est = &dwmac510_est_ops,
+		.fpe = &dwmac5_fpe_ops,
 		.setup = dwmac4_setup,
 		.quirks = NULL,
 	}, {

@@ -255,6 +259,7 @@ static const struct stmmac_hwif_entry {
 		.tc = &dwxgmac_tc_ops,
 		.mmc = &dwxgmac_mmc_ops,
 		.est = &dwmac510_est_ops,
+		.fpe = &dwxgmac_fpe_ops,
 		.setup = dwxgmac2_setup,
 		.quirks = NULL,
 	}, {

@@ -276,6 +281,7 @@ static const struct stmmac_hwif_entry {
 		.tc = &dwxgmac_tc_ops,
 		.mmc = &dwxgmac_mmc_ops,
 		.est = &dwmac510_est_ops,
+		.fpe = &dwxgmac_fpe_ops,
 		.setup = dwxlgmac2_setup,
 		.quirks = stmmac_dwxlgmac_quirks,
 	},

@@ -351,6 +357,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
 		mac->tc = mac->tc ? : entry->tc;
 		mac->mmc = mac->mmc ? : entry->mmc;
 		mac->est = mac->est ? : entry->est;
+		mac->fpe = mac->fpe ? : entry->fpe;
 
 		priv->hw = mac;
 		priv->ptpaddr = priv->ioaddr + entry->regs.ptp_off;

drivers/net/ethernet/stmicro/stmmac/hwif.h +30 ~~-24~~

78 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +31 ~~-13~~

87 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h +0 ~~-16~~

index d4d46a07d6a7..a113b5c57de9 100644
@@ -36,19 +36,3 @@ void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up);
 void stmmac_fpe_event_status(struct stmmac_priv *priv, int status);
 void stmmac_fpe_init(struct stmmac_priv *priv);
 void stmmac_fpe_apply(struct stmmac_priv *priv);
-
-void dwmac5_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
-			  u32 num_txq, u32 num_rxq,
-			  bool tx_enable, bool pmac_enable);
-void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,
-			     struct stmmac_fpe_cfg *cfg,
-			     enum stmmac_mpacket_type type);
-int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev);
-int dwmac5_fpe_get_add_frag_size(const void __iomem *ioaddr);
-void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size);
-int dwmac5_fpe_map_preemption_class(struct net_device *ndev,
-				    struct netlink_ext_ack *extack, u32 pclass);
-
-void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
-			    u32 num_txq, u32 num_rxq,
-			    bool tx_enable, bool pmac_enable);

Signature · 1 line

2.34.1

Furong Xu

author

2024-10-15 09:10 UTC #5 lore raw

PATCH 3/5 net: stmmac: Rework marco definitions for gmac4 and xgmac

In reply to Furong Xu

Rename and add marco definitions to better reuse them in common code.

Signed-off-by Furong Xu <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 49 ++++++++++---------
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.h  | 26 +++++-----
 2 files changed, 38 insertions(+), 37 deletions(-)

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +25 ~~-24~~

index c01eb7243d56..0c13d5aee3d2 100644
@@ -168,7 +168,7 @@ static void dwmac5_fpe_configure(void __iomem *ioaddr,
 	u32 value;
 
 	if (tx_enable) {
-		cfg->fpe_csr = EFPE;
+		cfg->fpe_csr = STMMAC_MAC_FPE_CTRL_STS_EFPE;
 		value = readl(ioaddr + GMAC_RXQ_CTRL1);
 		value &= ~GMAC_RXQCTRL_FPRQ;
 		value |= (num_rxq - 1) << GMAC_RXQCTRL_FPRQ_SHIFT;

@@ -176,14 +176,14 @@ static void dwmac5_fpe_configure(void __iomem *ioaddr,
 	} else {
 		cfg->fpe_csr = 0;
 	}
-	writel(cfg->fpe_csr, ioaddr + MAC_FPE_CTRL_STS);
+	writel(cfg->fpe_csr, ioaddr + GMAC5_MAC_FPE_CTRL_STS);
 
 	value = readl(ioaddr + GMAC_INT_EN);
 
 	if (pmac_enable) {
 		if (!(value & GMAC_INT_FPE_EN)) {
 			/* Dummy read to clear any pending masked interrupts */
-			readl(ioaddr + MAC_FPE_CTRL_STS);
+			readl(ioaddr + GMAC5_MAC_FPE_CTRL_STS);
 
 			value |= GMAC_INT_FPE_EN;
 		}

@@ -204,24 +204,24 @@ static int dwmac5_fpe_irq_status(void __iomem *ioaddr, struct net_device *dev)
 	/* Reads from the MAC_FPE_CTRL_STS register should only be performed
 	 * here, since the status flags of MAC_FPE_CTRL_STS are "clear on read"
 	 */
-	value = readl(ioaddr + MAC_FPE_CTRL_STS);
+	value = readl(ioaddr + GMAC5_MAC_FPE_CTRL_STS);
 
-	if (value & TRSP) {
+	if (value & STMMAC_MAC_FPE_CTRL_STS_TRSP) {
 		status |= FPE_EVENT_TRSP;
 		netdev_dbg(dev, "FPE: Respond mPacket is transmitted\n");
 	}
 
-	if (value & TVER) {
+	if (value & STMMAC_MAC_FPE_CTRL_STS_TVER) {
 		status |= FPE_EVENT_TVER;
 		netdev_dbg(dev, "FPE: Verify mPacket is transmitted\n");
 	}
 
-	if (value & RRSP) {
+	if (value & STMMAC_MAC_FPE_CTRL_STS_RRSP) {
 		status |= FPE_EVENT_RRSP;
 		netdev_dbg(dev, "FPE: Respond mPacket is received\n");
 	}
 
-	if (value & RVER) {
+	if (value & STMMAC_MAC_FPE_CTRL_STS_RVER) {
 		status |= FPE_EVENT_RVER;
 		netdev_dbg(dev, "FPE: Verify mPacket is received\n");
 	}

@@ -236,16 +236,17 @@ static void dwmac5_fpe_send_mpacket(void __iomem *ioaddr,
 	u32 value = cfg->fpe_csr;
 
 	if (type == MPACKET_VERIFY)
-		value |= SVER;
+		value |= STMMAC_MAC_FPE_CTRL_STS_SVER;
 	else if (type == MPACKET_RESPONSE)
-		value |= SRSP;
+		value |= STMMAC_MAC_FPE_CTRL_STS_SRSP;
 
-	writel(value, ioaddr + MAC_FPE_CTRL_STS);
+	writel(value, ioaddr + GMAC5_MAC_FPE_CTRL_STS);
 }
 
 static int dwmac5_fpe_get_add_frag_size(const void __iomem *ioaddr)
 {
-	return FIELD_GET(DWMAC5_ADD_FRAG_SZ, readl(ioaddr + MTL_FPE_CTRL_STS));
+	return FIELD_GET(FPE_MTL_ADD_FRAG_SZ,
+			 readl(ioaddr + GMAC5_MTL_FPE_CTRL_STS));
 }
 
 static void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr,

@@ -253,9 +254,9 @@ static void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr,
 {
 	u32 value;
 
-	value = readl(ioaddr + MTL_FPE_CTRL_STS);
-	writel(u32_replace_bits(value, add_frag_size, DWMAC5_ADD_FRAG_SZ),
-	       ioaddr + MTL_FPE_CTRL_STS);
+	value = readl(ioaddr + GMAC5_MTL_FPE_CTRL_STS);
+	writel(u32_replace_bits(value, add_frag_size, FPE_MTL_ADD_FRAG_SZ),
+	       ioaddr + GMAC5_MTL_FPE_CTRL_STS);
 }
 
 #define ALG_ERR_MSG "TX algorithm SP is not suitable for one-to-many mapping"

@@ -307,9 +308,9 @@ static int dwmac5_fpe_map_preemption_class(struct net_device *ndev,
 	}
 
 update_mapping:
-	val = readl(priv->ioaddr + MTL_FPE_CTRL_STS);
-	writel(u32_replace_bits(val, preemptible_txqs, DWMAC5_PREEMPTION_CLASS),
-	       priv->ioaddr + MTL_FPE_CTRL_STS);
+	val = readl(priv->ioaddr + GMAC5_MTL_FPE_CTRL_STS);
+	writel(u32_replace_bits(val, preemptible_txqs, FPE_MTL_PREEMPTION_CLASS),
+	       priv->ioaddr + GMAC5_MTL_FPE_CTRL_STS);
 
 	return 0;
 }

@@ -322,11 +323,11 @@ static void dwxgmac3_fpe_configure(void __iomem *ioaddr,
 	u32 value;
 
 	if (!tx_enable) {
-		value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
+		value = readl(ioaddr + XGMAC_MAC_FPE_CTRL_STS);
 
-		value &= ~XGMAC_EFPE;
+		value &= ~STMMAC_MAC_FPE_CTRL_STS_EFPE;
 
-		writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
+		writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS);
 		return;
 	}

@@ -335,9 +336,9 @@ static void dwxgmac3_fpe_configure(void __iomem *ioaddr,
 	value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
 	writel(value, ioaddr + XGMAC_RXQ_CTRL1);
 
-	value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
-	value |= XGMAC_EFPE;
-	writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
+	value = readl(ioaddr + XGMAC_MAC_FPE_CTRL_STS);
+	value |= STMMAC_MAC_FPE_CTRL_STS_EFPE;
+	writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS);
 }
 
 const struct stmmac_fpe_ops dwmac5_fpe_ops = {

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h +13 ~~-13~~

index a113b5c57de9..c0305f11575b 100644
@@ -8,23 +8,23 @@
 #define STMMAC_FPE_MM_MAX_VERIFY_RETRIES	3
 #define STMMAC_FPE_MM_MAX_VERIFY_TIME_MS	128
 
-#define MAC_FPE_CTRL_STS		0x00000234
-#define TRSP				BIT(19)
-#define TVER				BIT(18)
-#define RRSP				BIT(17)
-#define RVER				BIT(16)
-#define SRSP				BIT(2)
-#define SVER				BIT(1)
-#define EFPE				BIT(0)
+#define GMAC5_MAC_FPE_CTRL_STS		0x00000234
+#define XGMAC_MAC_FPE_CTRL_STS		0x00000280
 
-#define MTL_FPE_CTRL_STS		0x00000c90
+#define GMAC5_MTL_FPE_CTRL_STS		0x00000c90
+#define XGMAC_MTL_FPE_CTRL_STS		0x00001090
 /* Preemption Classification */
-#define DWMAC5_PREEMPTION_CLASS		GENMASK(15, 8)
+#define FPE_MTL_PREEMPTION_CLASS	GENMASK(15, 8)
 /* Additional Fragment Size of preempted frames */
-#define DWMAC5_ADD_FRAG_SZ		GENMASK(1, 0)
+#define FPE_MTL_ADD_FRAG_SZ		GENMASK(1, 0)
 
-#define XGMAC_FPE_CTRL_STS		0x00000280
-#define XGMAC_EFPE			BIT(0)
+#define STMMAC_MAC_FPE_CTRL_STS_TRSP	BIT(19)
+#define STMMAC_MAC_FPE_CTRL_STS_TVER	BIT(18)
+#define STMMAC_MAC_FPE_CTRL_STS_RRSP	BIT(17)
+#define STMMAC_MAC_FPE_CTRL_STS_RVER	BIT(16)
+#define STMMAC_MAC_FPE_CTRL_STS_SRSP	BIT(2)
+#define STMMAC_MAC_FPE_CTRL_STS_SVER	BIT(1)
+#define STMMAC_MAC_FPE_CTRL_STS_EFPE	BIT(0)
 
 /* FPE link-partner hand-shaking mPacket type */
 enum stmmac_mpacket_type {

Signature · 1 line

2.34.1

Furong Xu

author

2024-10-15 09:10 UTC #8 lore raw

PATCH 4/5 net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ

In reply to Furong Xu

Synopsys XGMAC Databook defines MAC_RxQ_Ctrl1 register:
RQ: Frame Preemption Residue Queue

XGMAC_FPRQ is more readable and more consistent with GMAC4.

Signed-off-by Furong Xu <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h   | 4 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +2 -2

index 917796293c26..c66fa6040672 100644
@@ -84,8 +84,8 @@
 #define XGMAC_MCBCQEN			BIT(15)
 #define XGMAC_MCBCQ			GENMASK(11, 8)
 #define XGMAC_MCBCQ_SHIFT		8
-#define XGMAC_RQ			GENMASK(7, 4)
-#define XGMAC_RQ_SHIFT			4
+#define XGMAC_FPRQ			GENMASK(7, 4)
+#define XGMAC_FPRQ_SHIFT		4
 #define XGMAC_UPQ			GENMASK(3, 0)
 #define XGMAC_UPQ_SHIFT			0
 #define XGMAC_RXQ_CTRL2			0x000000a8

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +2 -2

index 0c13d5aee3d2..6060a1d702c6 100644
@@ -332,8 +332,8 @@ static void dwxgmac3_fpe_configure(void __iomem *ioaddr,
 	}
 
 	value = readl(ioaddr + XGMAC_RXQ_CTRL1);
-	value &= ~XGMAC_RQ;
-	value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
+	value &= ~XGMAC_FPRQ;
+	value |= (num_rxq - 1) << XGMAC_FPRQ_SHIFT;
 	writel(value, ioaddr + XGMAC_RXQ_CTRL1);
 
 	value = readl(ioaddr + XGMAC_MAC_FPE_CTRL_STS);

Signature · 1 line

2.34.1

Furong Xu

author

2024-10-15 09:10 UTC #13 lore raw

PATCH 5/5 net: stmmac: xgmac: Complete FPE support

In reply to Furong Xu

FPE implementation for DWMAC4 and DWXGMAC differs only for:
1) Offset address of MAC_FPE_CTRL_STS and MTL_FPE_CTRL_STS
2) FPRQ(Frame Preemption Residue Queue) field in MAC_RxQ_Ctrl1

Refactor stmmac_fpe_ops callback functions to avoid code duplication between gmac4 and xgmac.

Signed-off-by Furong Xu <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |   1 +
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 149 ++++++++++++++----
 .../net/ethernet/stmicro/stmmac/stmmac_tc.c   |   4 +-
 3 files changed, 121 insertions(+), 33 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +1 -0

index c66fa6040672..e1c54f3a8ee7 100644
@@ -96,6 +96,7 @@
 #define XGMAC_LPIIS			BIT(5)
 #define XGMAC_PMTIS			BIT(4)
 #define XGMAC_INT_EN			0x000000b4
+#define XGMAC_FPEIE			BIT(15)
 #define XGMAC_TSIE			BIT(12)
 #define XGMAC_LPIIE			BIT(5)
 #define XGMAC_PMTIE			BIT(4)

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +118 ~~-31~~

210 lines not on this page · raw message

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +2 -2

index 75ad2da1a37f..6a79e6a111ed 100644
@@ -1290,8 +1290,8 @@ const struct stmmac_tc_ops dwxgmac_tc_ops = {
 	.setup_cls_u32 = tc_setup_cls_u32,
 	.setup_cbs = tc_setup_cbs,
 	.setup_cls = tc_setup_cls,
-	.setup_taprio = tc_setup_taprio_without_fpe,
+	.setup_taprio = tc_setup_taprio,
 	.setup_etf = tc_setup_etf,
 	.query_caps = tc_query_caps,
-	.setup_mqprio = tc_setup_mqprio_unimplemented,
+	.setup_mqprio = tc_setup_dwmac510_mqprio,
 };

Signature · 1 line

2.34.1

Simon Horman

reviewer

2024-10-17 12:29 UTC #14 lore raw

Re: [PATCH net-next v1 5/5] net: stmmac: xgmac: Complete FPE support

Quoted 8 lines · Furong Xu

FPE implementation for DWMAC4 and DWXGMAC differs only for:
1) Offset address of MAC_FPE_CTRL_STS and MTL_FPE_CTRL_STS
2) FPRQ(Frame Preemption Residue Queue) field in MAC_RxQ_Ctrl1
Refactor stmmac_fpe_ops callback functions to avoid code duplication between gmac4 and xgmac.
Signed-off-by Furong Xu <[email protected]>

Hi Furong Xu,

I think it would be best to split this patch so that the refactor of dwmac4 code is in one patch, and adding xgmac code is in another.

...

Quoted 8 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +0 -0

index 6060a1d702c6..80f12b6e84e6 100644
@@ -160,41 +160,54 @@ void stmmac_fpe_apply(struct stmmac_priv *priv)
 	}
 }

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c void stmmac_fpe_apply(struct stmmac_priv *priv) · line 168

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +6 -4

-static void dwmac5_fpe_configure(void __iomem *ioaddr,
-				 struct stmmac_fpe_cfg *cfg,
-				 u32 num_txq, u32 num_rxq,
-				 bool tx_enable, bool pmac_enable)
+static void common_fpe_configure(void __iomem *ioaddr,
+				 struct stmmac_fpe_cfg *cfg, u32 rxq,
+				 bool tx_enable, bool pmac_enable,
+				 u32 rxq_addr, u32 fprq_mask, u32 fprq_shift,
+				 u32 mac_fpe_addr, u32 int_en_addr,
+				 u32 int_en_bit)

This function now has a lot of parameters. Could we consider another way? One idea I had was that describes the addresses for different chips.

...

Quoted 5 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +0 -0
index 75ad2da1a37f..6a79e6a111ed 100644

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c const struct stmmac_tc_ops dwxgmac_tc_ops = { · line 1297

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c +2 -2

@@ -1290,8 +1290,8 @@ const struct stmmac_tc_ops dwxgmac_tc_ops = {
 	.setup_cls_u32 = tc_setup_cls_u32,
 	.setup_cbs = tc_setup_cbs,
 	.setup_cls = tc_setup_cls,
-	.setup_taprio = tc_setup_taprio_without_fpe,
+	.setup_taprio = tc_setup_taprio,
 	.setup_etf = tc_setup_etf,
 	.query_caps = tc_query_caps,
-	.setup_mqprio = tc_setup_mqprio_unimplemented,
+	.setup_mqprio = tc_setup_dwmac510_mqprio,
 };

It is not clear to me how this hunk relates to the rest of the patch.

Simon Horman

reviewer Reviewed-by on v1 1/5

2024-10-17 12:39 UTC #2 lore raw

Re: [PATCH net-next v1 1/5] net: stmmac: Introduce separate files for FPE implementation

In reply to Furong Xu

Quoted 5 lines · Furong Xu

By moving FPE related code info separate files, FPE implementation becomes a separate module initially.
No functional change intended.
Signed-off-by Furong Xu <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Simon Horman

reviewer Reviewed-by on v1 2/5

2024-10-17 12:39 UTC #4 lore raw

Re: [PATCH net-next v1 2/5] net: stmmac: Introduce stmmac_fpe_ops for gmac4 and xgmac

In reply to Furong Xu

Quoted 5 lines · Furong Xu

By moving FPE function callbacks from stmmac_ops to stmmac_fpe_ops, FPE implementation becomes a separate module completely, like the EST implementation.
Signed-off-by Furong Xu <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Simon Horman

reviewer Reviewed-by on v1 3/5

2024-10-17 12:40 UTC #6 lore raw

Re: [PATCH net-next v1 3/5] net: stmmac: Rework marco definitions for gmac4 and xgmac

In reply to Furong Xu

Quoted 3 lines · Furong Xu

Rename and add marco definitions to better reuse them in common code.
Signed-off-by Furong Xu <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Simon Horman

reviewer Reviewed-by on v1 4/5

2024-10-17 12:40 UTC #9 lore raw

Re: [PATCH net-next v1 4/5] net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ

In reply to Furong Xu

Quoted 6 lines · Furong Xu

Synopsys XGMAC Databook defines MAC_RxQ_Ctrl1 register:
RQ: Frame Preemption Residue Queue
XGMAC_FPRQ is more readable and more consistent with GMAC4.
Signed-off-by Furong Xu <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Simon Horman

reviewer Reviewed-by on v1 4/5

2024-10-17 12:41 UTC #10 lore raw

Re: [PATCH net-next v1 4/5] net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ

In reply to Furong Xu

Quoted 6 lines · Furong Xu

Synopsys XGMAC Databook defines MAC_RxQ_Ctrl1 register:
RQ: Frame Preemption Residue Queue
XGMAC_FPRQ is more readable and more consistent with GMAC4.
Signed-off-by Furong Xu <[email protected]>

Reviewed-by Simon Horman <[email protected]>

Vladimir Oltean

2024-10-17 17:06 UTC #16 lore raw

In reply to Furong Xu

Quoted 26 lines · Furong Xu

Refactor FPE implementation by moving common code for DWMAC4 and DWXGMAC into a separate FPE module.
FPE implementation for DWMAC4 and DWXGMAC differs only for:
1) Offset address of MAC_FPE_CTRL_STS and MTL_FPE_CTRL_STS
2) FPRQ(Frame Preemption Residue Queue) field in MAC_RxQ_Ctrl1
Tested on DWMAC CORE 5.20a and DWXGMAC CORE 3.20a
Furong Xu (5):
net: stmmac: Introduce separate files for FPE implementation
net: stmmac: Introduce stmmac_fpe_ops for gmac4 and xgmac
net: stmmac: Rework marco definitions for gmac4 and xgmac
net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ
net: stmmac: xgmac: Complete FPE support
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
 drivers/net/ethernet/stmicro/stmmac/common.h  |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac4_core.c |  12 -
 drivers/net/ethernet/stmicro/stmmac/dwmac5.c  | 150 ------
 drivers/net/ethernet/stmicro/stmmac/dwmac5.h  |  26 --
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |   7 +-
 .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  28 --
 drivers/net/ethernet/stmicro/stmmac/hwif.c    |   7 +
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  54 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  10 -

Quoted 12 lines

 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 442 ++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_fpe.h  |  38 ++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 149 +-----
 .../net/ethernet/stmicro/stmmac/stmmac_tc.c   |   4 +-
 15 files changed, 527 insertions(+), 405 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.h

Signature · 1 line

2.34.1

Sergey Syomin is the one who originally requested the splitting of FPE into a separate module.
https://lore.kernel.org/netdev/max7qd6eafatuse22ymmbfhumrctvf2lenwzhn6sxsm5ugebh6@udblqrtlblbf/

I guess you could CC him on next patch revisions, maybe he can take a look and see if it is what he had in mind. I don't care so much about internal stmmac organization stuff.

Vladimir Oltean

2024-10-17 17:16 UTC #7 lore raw

Re: [PATCH net-next v1 3/5] net: stmmac: Rework marco definitions for gmac4 and xgmac

In reply to Furong Xu

Quoted 1 line · Furong Xu

Rename and add marco definitions to better reuse them in common code.

s/marco/macro/ (twice)

Vladimir Oltean

2024-10-17 17:18 UTC #11 lore raw

Re: [PATCH net-next v1 4/5] net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ

In reply to Furong Xu

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h line 88

Quoted 12 lines · Furong Xu

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +2 -2

index 917796293c26..c66fa6040672 100644
@@ -84,8 +84,8 @@
 #define XGMAC_MCBCQEN			BIT(15)
 #define XGMAC_MCBCQ			GENMASK(11, 8)
 #define XGMAC_MCBCQ_SHIFT		8
-#define XGMAC_RQ			GENMASK(7, 4)
-#define XGMAC_RQ_SHIFT			4
+#define XGMAC_FPRQ			GENMASK(7, 4)
+#define XGMAC_FPRQ_SHIFT		4

If you made use of FIELD_PREP(), you would not need the _SHIFT variant at all (though that would be a separate logical change).

Quoted 3 lines

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +0 -0

 #define XGMAC_UPQ			GENMASK(3, 0)
 #define XGMAC_UPQ_SHIFT			0
 #define XGMAC_RXQ_CTRL2			0x000000a8

Vladimir Oltean

2024-10-17 17:31 UTC #15 lore raw

Re: [PATCH net-next v1 5/5] net: stmmac: xgmac: Complete FPE support

In reply to Furong Xu

Quoted 8 lines · Furong Xu

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +0 -0

index 6060a1d702c6..80f12b6e84e6 100644
@@ -160,41 +160,54 @@ void stmmac_fpe_apply(struct stmmac_priv *priv)
 	}
 }

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c void stmmac_fpe_apply(struct stmmac_priv *priv) · line 168

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +6 -4

-static void dwmac5_fpe_configure(void __iomem *ioaddr,
-				 struct stmmac_fpe_cfg *cfg,
-				 u32 num_txq, u32 num_rxq,
-				 bool tx_enable, bool pmac_enable)
+static void common_fpe_configure(void __iomem *ioaddr,
+				 struct stmmac_fpe_cfg *cfg, u32 rxq,
+				 bool tx_enable, bool pmac_enable,
+				 u32 rxq_addr, u32 fprq_mask, u32 fprq_shift,
+				 u32 mac_fpe_addr, u32 int_en_addr,
+				 u32 int_en_bit)

11 arguments to a function is a bit too much. Could you introduce a structure with FPE constants per hardware IP, and just pass a pointer to that?

Quoted 30 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +19 -6

 {
 	u32 value;
 
-		writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS);
-		return;
+	if (!num_tc) {
+		/* Restore default TC:Queue mapping */
+		for (u32 i = 0; i < priv->plat->tx_queues_to_use; i++) {
+			val = readl(priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(i));
+			writel(u32_replace_bits(val, i, XGMAC_Q2TCMAP),
+			       priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(i));
+		}
 	}
 
-	value = readl(ioaddr + XGMAC_RXQ_CTRL1);
-	value &= ~XGMAC_FPRQ;
-	value |= (num_rxq - 1) << XGMAC_FPRQ_SHIFT;
-	writel(value, ioaddr + XGMAC_RXQ_CTRL1);
+	/* Synopsys Databook:
+	 * "All Queues within a traffic class are selected in a round robin
+	 * fashion (when packets are available) when the traffic class is
+	 * selected by the scheduler for packet transmission. This is true for
+	 * any of the scheduling algorithms."
+	 */
+	for (u32 tc = 0; tc < num_tc; tc++) {
+		count = ndev->tc_to_txq[tc].count;
+		offset = ndev->tc_to_txq[tc].offset;
+
+		if (pclass & BIT(tc))
+			preemptible_txqs |= GENMASK(offset + count - 1, offset);

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c static void dwxgmac3_fpe_configure(void __iomem *ioaddr, · line 417

Quoted 10 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +6 -3

 
-	value = readl(ioaddr + XGMAC_MAC_FPE_CTRL_STS);
-	value |= STMMAC_MAC_FPE_CTRL_STS_EFPE;
-	writel(value, ioaddr + XGMAC_MAC_FPE_CTRL_STS);
+		for (u32 i = 0; i < count; i++) {
+			val = readl(priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(offset + i));
+			writel(u32_replace_bits(val, tc, XGMAC_Q2TCMAP),
+			       priv->ioaddr + XGMAC_MTL_TXQ_OPMODE(offset + i));
+		}
+	}

I agree with Simon that this patch is hard to review. The diff looks like a jungle here, the portion with - has nothing to do with the portion with +. Please try to do as suggested, first refactor existing code into the common stuff, then call common stuff from new places. Also try to keep an eye on how things look in git diff, and splitting even further if it gets messy.

Furong Xu

author

2024-10-18 01:33 UTC #17 lore raw

In reply to Vladimir Oltean

Hi Vladimir,

Quoted 7 lines · Vladimir Oltean

Sergey Syomin is the one who originally requested the splitting of FPE into a separate module.
https://lore.kernel.org/netdev/max7qd6eafatuse22ymmbfhumrctvf2lenwzhn6sxsm5ugebh6@udblqrtlblbf/
I guess you could CC him on next patch revisions, maybe he can take a look and see if it is what he had in mind. I don't care so much about internal stmmac organization stuff.

Actually, I sent a patchset to move common code for FPE into a separate FPE module before he suggested to do that, and you gave me valueable comments in that patchset ;)
https://lore.kernel.org/netdev/[email protected]/

It will be nice to CC him and listen from him.

Simon Horman

reviewer

2024-10-18 19:16 UTC #12 lore raw

Re: [PATCH net-next v1 4/5] net: stmmac: xgmac: Rename XGMAC_RQ to XGMAC_FPRQ

In reply to Vladimir Oltean

Quoted 16 lines · Vladimir Oltean

drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h line 88
Quoted 12 lines · Furong Xu
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +2 -2
index 917796293c26..c66fa6040672 100644
@@ -84,8 +84,8 @@
 #define XGMAC_MCBCQEN			BIT(15)
 #define XGMAC_MCBCQ			GENMASK(11, 8)
 #define XGMAC_MCBCQ_SHIFT		8
-#define XGMAC_RQ			GENMASK(7, 4)
-#define XGMAC_RQ_SHIFT			4
+#define XGMAC_FPRQ			GENMASK(7, 4)
+#define XGMAC_FPRQ_SHIFT		4
If you made use of FIELD_PREP(), you would not need the _SHIFT variant at all (though that would be a separate logical change).

Quoted 4 lines

Quoted 3 lines
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +0 -0
 #define XGMAC_UPQ			GENMASK(3, 0)
 #define XGMAC_UPQ_SHIFT			0
 #define XGMAC_RXQ_CTRL2			0x000000a8