Re: [PATCH v2] net: fec: Refactor: #define magic constants

[Denis Kirjanov <dkirjanov@suse.de>]

On 2/9/24 12:11, Csókás Bence wrote:
> Add defines for bits of ECR, RCR control registers, TX watermark etc.
> 
> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>

Please add net-next prefix

> ---
>  drivers/net/ethernet/freescale/fec_main.c | 50 +++++++++++++++--------
>  1 file changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 63707e065141..a16220eff9b3 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -85,8 +85,6 @@ static int fec_enet_xdp_tx_xmit(struct fec_enet_private *fep,
>  
>  static const u16 fec_enet_vlan_pri_to_queue[8] = {0, 0, 1, 1, 1, 2, 2, 2};
>  
> -/* Pause frame feild and FIFO threshold */
> -#define FEC_ENET_FCE	(1 << 5)
>  #define FEC_ENET_RSEM_V	0x84
>  #define FEC_ENET_RSFL_V	16
>  #define FEC_ENET_RAEM_V	0x8
> @@ -240,8 +238,8 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>  #define PKT_MINBUF_SIZE		64
>  
>  /* FEC receive acceleration */
> -#define FEC_RACC_IPDIS		(1 << 1)
> -#define FEC_RACC_PRODIS		(1 << 2)
> +#define FEC_RACC_IPDIS		BIT(1)
> +#define FEC_RACC_PRODIS		BIT(2)
>  #define FEC_RACC_SHIFT16	BIT(7)
>  #define FEC_RACC_OPTIONS	(FEC_RACC_IPDIS | FEC_RACC_PRODIS)
>  
> @@ -273,8 +271,23 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>  #define FEC_MMFR_TA		(2 << 16)
>  #define FEC_MMFR_DATA(v)	(v & 0xffff)
>  /* FEC ECR bits definition */
> -#define FEC_ECR_MAGICEN		(1 << 2)
> -#define FEC_ECR_SLEEP		(1 << 3)
> +#define FEC_ECR_RESET           BIT(0)
> +#define FEC_ECR_ETHEREN         BIT(1)
> +#define FEC_ECR_MAGICEN         BIT(2)
> +#define FEC_ECR_SLEEP           BIT(3)
> +#define FEC_ECR_EN1588          BIT(4)
> +#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_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_RMII            BIT(8)
> +#define FEC_RCR_10BASET         BIT(9)
> +/* TX WMARK bits */
> +#define FEC_TXWMRK_STRFWD       BIT(8)
>  
>  #define FEC_MII_TIMEOUT		30000 /* us */
>  
> @@ -1137,18 +1150,18 @@ fec_restart(struct net_device *ndev)
>  		    fep->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
>  			rcntl |= (1 << 6);
>  		else if (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
> -			rcntl |= (1 << 8);
> +			rcntl |= FEC_RCR_RMII;
>  		else
> -			rcntl &= ~(1 << 8);
> +			rcntl &= ~FEC_RCR_RMII;
>  
>  		/* 1G, 100M or 10M */
>  		if (ndev->phydev) {
>  			if (ndev->phydev->speed == SPEED_1000)
>  				ecntl |= (1 << 5);
>  			else if (ndev->phydev->speed == SPEED_100)
> -				rcntl &= ~(1 << 9);
> +				rcntl &= ~FEC_RCR_10BASET;
>  			else
> -				rcntl |= (1 << 9);
> +				rcntl |= FEC_RCR_10BASET;
>  		}
>  	} else {
>  #ifdef FEC_MIIGSK_ENR
> @@ -1181,7 +1194,7 @@ fec_restart(struct net_device *ndev)
>  	if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
>  	    ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
>  	     ndev->phydev && ndev->phydev->pause)) {
> -		rcntl |= FEC_ENET_FCE;
> +		rcntl |= FEC_RCR_FLOWCTL;
>  
>  		/* set FIFO threshold parameter to reduce overrun */
>  		writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
> @@ -1192,7 +1205,7 @@ fec_restart(struct net_device *ndev)
>  		/* OPD */
>  		writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
>  	} else {
> -		rcntl &= ~FEC_ENET_FCE;
> +		rcntl &= ~FEC_RCR_FLOWCTL;
>  	}
>  #endif /* !defined(CONFIG_M5272) */
>  
> @@ -1207,13 +1220,13 @@ fec_restart(struct net_device *ndev)
>  
>  	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
>  		/* enable ENET endian swap */
> -		ecntl |= (1 << 8);
> +		ecntl |= FEC_ECR_BYTESWP;
>  		/* enable ENET store and forward mode */
> -		writel(1 << 8, fep->hwp + FEC_X_WMRK);
> +		writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
>  	}
>  
>  	if (fep->bufdesc_ex)
> -		ecntl |= (1 << 4);
> +		ecntl |= FEC_ECR_EN1588;
>  
>  	if (fep->quirks & FEC_QUIRK_DELAYED_CLKS_SUPPORT &&
>  	    fep->rgmii_txc_dly)
> @@ -1312,7 +1325,8 @@ static void
>  fec_stop(struct net_device *ndev)
>  {
>  	struct fec_enet_private *fep = netdev_priv(ndev);
> -	u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
> +	u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & FEC_RCR_RMII;
> +	u32 ecntl = 0;
>  	u32 val;
>  
>  	/* We cannot expect a graceful transmit stop without link !!! */
> @@ -1345,9 +1359,11 @@ fec_stop(struct net_device *ndev)
>  	/* We have to keep ENET enabled to have MII interrupt stay working */
>  	if (fep->quirks & FEC_QUIRK_ENET_MAC &&
>  		!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
> -		writel(2, fep->hwp + FEC_ECNTRL);
> +		ecntl |= FEC_ECR_ETHEREN;
>  		writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
>  	}
> +
> +	writel(ecntl, fep->hwp + FEC_ECNTRL);
>  }
>  
>