[PATCH net-next 0/3] net: fec: add some optimizations

[PATCH net-next 0/3] net: fec: add some optimizations

[Wei Fang]

Add some optimizations to the fec driver, see each patch for details.

Wei Fang (3):
  net: fec: use phy_interface_mode_is_rgmii() to check RGMII mode
  net: fec: add more macros for bits of FEC_ECR
  net: fec: add fec_set_hw_mac_addr() helper function

 drivers/net/ethernet/freescale/fec_main.c | 44 ++++++++++++-----------
 1 file changed, 23 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH net-next 1/3] net: fec: use phy_interface_mode_is_rgmii() to check RGMII mode

[Wei Fang]

Use the generic helper function phy_interface_mode_is_rgmii() to check
RGMII mode.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d4eed252ad40..f4f1f38d94eb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1194,10 +1194,7 @@ fec_restart(struct net_device *ndev)
 		rcntl |= 0x40000000 | 0x00000020;
 
 		/* RGMII, RMII or MII */
-		if (fep->phy_interface == PHY_INTERFACE_MODE_RGMII ||
-		    fep->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
-		    fep->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-		    fep->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
+		if (phy_interface_mode_is_rgmii(fep->phy_interface))
 			rcntl |= (1 << 6);
 		else if (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
 			rcntl |= FEC_RCR_RMII;
-- 
2.34.1

[PATCH net-next 2/3] net: fec: add more macros for bits of FEC_ECR

[Wei Fang]

There are also some RCR bits that are not defined but are used by the
driver, so add macro definitions for these bits to improve readability
and maintainability.

In addition, although FEC_RCR_HALFDPX has been defined, it is not used
in the driver. According to the description of FEC_RCR[1] in RM, it is
used to disable receive on transmit. Therefore, it is more appropriate
to redefine FEC_RCR[1] as FEC_RCR_DRT.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f4f1f38d94eb..00f8be4119ed 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -279,13 +279,15 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #define FEC_ECR_BYTESWP         BIT(8)
 /* FEC RCR bits definition */
 #define FEC_RCR_LOOP            BIT(0)
-#define FEC_RCR_HALFDPX         BIT(1)
+#define FEC_RCR_DRT		BIT(1)
 #define FEC_RCR_MII             BIT(2)
 #define FEC_RCR_PROMISC         BIT(3)
 #define FEC_RCR_BC_REJ          BIT(4)
 #define FEC_RCR_FLOWCTL         BIT(5)
+#define FEC_RCR_RGMII		BIT(6)
 #define FEC_RCR_RMII            BIT(8)
 #define FEC_RCR_10BASET         BIT(9)
+#define FEC_RCR_NLC		BIT(30)
 /* TX WMARK bits */
 #define FEC_TXWMRK_STRFWD       BIT(8)
 
@@ -1131,7 +1133,7 @@ fec_restart(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	u32 temp_mac[2];
-	u32 rcntl = OPT_FRAME_SIZE | 0x04;
+	u32 rcntl = OPT_FRAME_SIZE | FEC_RCR_MII;
 	u32 ecntl = FEC_ECR_ETHEREN;
 
 	if (fep->bufdesc_ex)
@@ -1162,7 +1164,7 @@ fec_restart(struct net_device *ndev)
 		writel(0x04, fep->hwp + FEC_X_CNTRL);
 	} else {
 		/* No Rcv on Xmit */
-		rcntl |= 0x02;
+		rcntl |= FEC_RCR_DRT;
 		writel(0x0, fep->hwp + FEC_X_CNTRL);
 	}
 
@@ -1191,11 +1193,11 @@ fec_restart(struct net_device *ndev)
 	 */
 	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
 		/* Enable flow control and length check */
-		rcntl |= 0x40000000 | 0x00000020;
+		rcntl |= FEC_RCR_NLC | FEC_RCR_FLOWCTL;
 
 		/* RGMII, RMII or MII */
 		if (phy_interface_mode_is_rgmii(fep->phy_interface))
-			rcntl |= (1 << 6);
+			rcntl |= FEC_RCR_RGMII;
 		else if (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
 			rcntl |= FEC_RCR_RMII;
 		else
-- 
2.34.1

[PATCH net-next 3/3] net: fec: add fec_set_hw_mac_addr() helper function

[Wei Fang]

In the current driver, the MAC address is set in both fec_restart() and
fec_set_mac_address(), so a generic helper function fec_set_hw_mac_addr()
is added to set the hardware MAC address to make the code more compact.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 27 +++++++++++++----------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 00f8be4119ed..883b28e59a3c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1123,6 +1123,18 @@ static void fec_ctrl_reset(struct fec_enet_private *fep, bool allow_wol)
 	}
 }
 
+static void fec_set_hw_mac_addr(struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	u32 temp_mac[2];
+
+	memcpy(temp_mac, ndev->dev_addr, ETH_ALEN);
+	writel((__force u32)cpu_to_be32(temp_mac[0]),
+	       fep->hwp + FEC_ADDR_LOW);
+	writel((__force u32)cpu_to_be32(temp_mac[1]),
+	       fep->hwp + FEC_ADDR_HIGH);
+}
+
 /*
  * This function is called to start or restart the FEC during a link
  * change, transmit timeout, or to reconfigure the FEC.  The network
@@ -1132,7 +1144,6 @@ static void
 fec_restart(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	u32 temp_mac[2];
 	u32 rcntl = OPT_FRAME_SIZE | FEC_RCR_MII;
 	u32 ecntl = FEC_ECR_ETHEREN;
 
@@ -1145,11 +1156,7 @@ fec_restart(struct net_device *ndev)
 	 * enet-mac reset will reset mac address registers too,
 	 * so need to reconfigure it.
 	 */
-	memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
-	writel((__force u32)cpu_to_be32(temp_mac[0]),
-	       fep->hwp + FEC_ADDR_LOW);
-	writel((__force u32)cpu_to_be32(temp_mac[1]),
-	       fep->hwp + FEC_ADDR_HIGH);
+	fec_set_hw_mac_addr(ndev);
 
 	/* Clear any outstanding interrupt, except MDIO. */
 	writel((0xffffffff & ~FEC_ENET_MII), fep->hwp + FEC_IEVENT);
@@ -3693,7 +3700,6 @@ static void set_multicast_list(struct net_device *ndev)
 static int
 fec_set_mac_address(struct net_device *ndev, void *p)
 {
-	struct fec_enet_private *fep = netdev_priv(ndev);
 	struct sockaddr *addr = p;
 
 	if (addr) {
@@ -3710,11 +3716,8 @@ fec_set_mac_address(struct net_device *ndev, void *p)
 	if (!netif_running(ndev))
 		return 0;
 
-	writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
-		(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
-		fep->hwp + FEC_ADDR_LOW);
-	writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
-		fep->hwp + FEC_ADDR_HIGH);
+	fec_set_hw_mac_addr(ndev);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH net-next 1/3] net: fec: use phy_interface_mode_is_rgmii() to check RGMII mode

[Maxime Chevallier]

On Thu, 10 Jul 2025 17:09:00 +0800
Wei Fang <wei.fang@nxp.com> wrote:

> Use the generic helper function phy_interface_mode_is_rgmii() to check
> RGMII mode.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

Re: [PATCH net-next 2/3] net: fec: add more macros for bits of FEC_ECR

[Maxime Chevallier]

On Thu, 10 Jul 2025 17:09:01 +0800
Wei Fang <wei.fang@nxp.com> wrote:

> There are also some RCR bits that are not defined but are used by the
> driver, so add macro definitions for these bits to improve readability
> and maintainability.
> 
> In addition, although FEC_RCR_HALFDPX has been defined, it is not used
> in the driver. According to the description of FEC_RCR[1] in RM, it is
> used to disable receive on transmit. Therefore, it is more appropriate
> to redefine FEC_RCR[1] as FEC_RCR_DRT.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

Re: [PATCH net-next 3/3] net: fec: add fec_set_hw_mac_addr() helper function

[Maxime Chevallier]

Hi,

On Thu, 10 Jul 2025 17:09:02 +0800
Wei Fang <wei.fang@nxp.com> wrote:

> In the current driver, the MAC address is set in both fec_restart() and
> fec_set_mac_address(), so a generic helper function fec_set_hw_mac_addr()
> is added to set the hardware MAC address to make the code more compact.
> 
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 27 +++++++++++++----------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 00f8be4119ed..883b28e59a3c 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1123,6 +1123,18 @@ static void fec_ctrl_reset(struct fec_enet_private *fep, bool allow_wol)
>  	}
>  }
>  
> +static void fec_set_hw_mac_addr(struct net_device *ndev)
> +{
> +	struct fec_enet_private *fep = netdev_priv(ndev);
> +	u32 temp_mac[2];
> +
> +	memcpy(temp_mac, ndev->dev_addr, ETH_ALEN);
> +	writel((__force u32)cpu_to_be32(temp_mac[0]),
> +	       fep->hwp + FEC_ADDR_LOW);
> +	writel((__force u32)cpu_to_be32(temp_mac[1]),
> +	       fep->hwp + FEC_ADDR_HIGH);
> +}

[ ... ]

> -	writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> -		(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> -		fep->hwp + FEC_ADDR_LOW);
> -	writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
> -		fep->hwp + FEC_ADDR_HIGH);
> +	fec_set_hw_mac_addr(ndev);

It's more of a personal preference, but I find this implementation to
be much more readable than the one based on

  writel((__force u32)cpu_to_be32(temp_mac[...]), ...);

Maxime

Re: [PATCH net-next 3/3] net: fec: add fec_set_hw_mac_addr() helper function

[Andrew Lunn]

> > -	writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> > -		(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> > -		fep->hwp + FEC_ADDR_LOW);
> > -	writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
> > -		fep->hwp + FEC_ADDR_HIGH);
> > +	fec_set_hw_mac_addr(ndev);
> 
> It's more of a personal preference, but I find this implementation to
> be much more readable than the one based on
> 
>   writel((__force u32)cpu_to_be32(temp_mac[...]), ...);

It also avoids the __force, which is good.

	Andrew

RE: [PATCH net-next 3/3] net: fec: add fec_set_hw_mac_addr() helper function

[Wei Fang]

> > > -	writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> > > -		(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> > > -		fep->hwp + FEC_ADDR_LOW);
> > > -	writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
> > > -		fep->hwp + FEC_ADDR_HIGH);
> > > +	fec_set_hw_mac_addr(ndev);
> >
> > It's more of a personal preference, but I find this implementation to
> > be much more readable than the one based on
> >
> >   writel((__force u32)cpu_to_be32(temp_mac[...]), ...);
> 
> It also avoids the __force, which is good.
> 

Thanks, I will improve it.