Re: [PATCH net-next 2/5] net: fec: simplify the conditional preprocessor directives

[Frank Li <Frank.li@nxp.com>]

On Tue, Nov 11, 2025 at 06:00:54PM +0800, Wei Fang wrote:
> From the Kconfig file, we can see CONFIG_FEC depends on the following
> platform-related options.
>
> ColdFire: M523x, M527x, M5272, M528x, M520x and M532x
> S32: ARCH_S32 (ARM64)
> i.MX: SOC_IMX28 and ARCH_MXC (ARM and ARM64)
>
> Based on the code of fec driver, only some macro definitions on the
> M5272 platform are different from those on other platforms. Therefore,
> we can simplify the following complex preprocessor directives to
> "if !defined(CONFIG_M5272)".
>
> "#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || \
>      defined(CONFIG_M528x) || defined(CONFIG_M520x) || \
>      defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
>      defined(CONFIG_ARM64)"
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec.h      |  4 +---
>  drivers/net/ethernet/freescale/fec_main.c | 27 ++++++-----------------
>  2 files changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 41e0d85d15da..8e438f6e7ec4 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -24,9 +24,7 @@
>  #include <linux/timecounter.h>
>  #include <net/xdp.h>
>
> -#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
> -    defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
> -    defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
> +#if !defined(CONFIG_M5272) || defined(CONFIG_COMPILE_TEST)
>  /*
>   *	Just figures, Motorola would have to change the offsets for
>   *	registers in the same peripheral device on different models
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index e0e84f2979c8..9d0e5abe5f66 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -253,9 +253,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>   * size bits. Other FEC hardware does not, so we need to take that into
>   * account when setting it.
>   */
> -#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
> -    defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
> -    defined(CONFIG_ARM64)
> +#ifndef CONFIG_M5272
>  #define	OPT_ARCH_HAS_MAX_FL	1
>  #else
>  #define	OPT_ARCH_HAS_MAX_FL	0
> @@ -2704,9 +2702,7 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
>  }
>
>  /* List of registers that can be safety be read to dump them with ethtool */
> -#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
> -	defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
> -	defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
> +#if !defined(CONFIG_M5272) || defined(CONFIG_COMPILE_TEST)
>  static __u32 fec_enet_register_version = 2;
>  static u32 fec_enet_register_offset[] = {
>  	FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
> @@ -2780,29 +2776,20 @@ static u32 fec_enet_register_offset[] = {
>  static void fec_enet_get_regs(struct net_device *ndev,
>  			      struct ethtool_regs *regs, void *regbuf)
>  {
> +	u32 reg_cnt = ARRAY_SIZE(fec_enet_register_offset);
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>  	u32 __iomem *theregs = (u32 __iomem *)fep->hwp;
> +	u32 *reg_list = fec_enet_register_offset;
>  	struct device *dev = &fep->pdev->dev;
>  	u32 *buf = (u32 *)regbuf;
>  	u32 i, off;
>  	int ret;
> -#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
> -	defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
> -	defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
> -	u32 *reg_list;
> -	u32 reg_cnt;
> -
> -	if (!of_machine_is_compatible("fsl,imx6ul")) {
> -		reg_list = fec_enet_register_offset;
> -		reg_cnt = ARRAY_SIZE(fec_enet_register_offset);
> -	} else {
> +
> +#if !defined(CONFIG_M5272) || defined(CONFIG_COMPILE_TEST)
> +	if (of_machine_is_compatible("fsl,imx6ul")) {

There are stub of_machine_is_compatible(), so needn't #ifdef here.

Frank
>  		reg_list = fec_enet_register_offset_6ul;
>  		reg_cnt = ARRAY_SIZE(fec_enet_register_offset_6ul);
>  	}
> -#else
> -	/* coldfire */
> -	static u32 *reg_list = fec_enet_register_offset;
> -	static const u32 reg_cnt = ARRAY_SIZE(fec_enet_register_offset);
>  #endif
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
> --
> 2.34.1
>