[PATCH 01/13] imx: imx8mp_evk: enable eth support

ZHIZHIKIN Andrey andrey.zhizhikin at leica-geosystems.com
Mon Dec 28 22:34:38 CET 2020


Hello Peng,

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Peng Fan (OSS)
> Sent: Monday, December 28, 2020 1:17 PM
> To: sbabic at denx.de; festevam at gmail.com
> Cc: uboot-imx at nxp.com; u-boot at lists.denx.de; Peng Fan <peng.fan at nxp.com>
> Subject: [PATCH 01/13] imx: imx8mp_evk: enable eth support
> 
> 
> From: Peng Fan <peng.fan at nxp.com>
> 
> Add board code to configure the network interface
> Add net defconfig
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> ---
>  arch/arm/include/asm/arch-imx8m/imx-regs.h |  2 +
>  board/freescale/imx8mp_evk/imx8mp_evk.c    | 81 ++++++++++++++++++++++
>  configs/imx8mp_evk_defconfig               | 11 +++
>  include/configs/imx8mp_evk.h               | 16 +++++
>  4 files changed, 110 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> index f1c410ec78..f5711155b7 100644
> --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h
> +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h
> @@ -62,6 +62,8 @@
>  #define DDRC_IPS_BASE_ADDR(X)  (0x3d400000 + ((X) * 0x2000000))
>  #define DDR_CSD1_BASE_ADDR     0x40000000
> 
> +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK 0x70000
> +
>  #if !defined(__ASSEMBLY__)
>  #include <asm/types.h>
>  #include <linux/bitops.h>
> diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c
> b/board/freescale/imx8mp_evk/imx8mp_evk.c
> index 034a349236..d6b863df84 100644
> --- a/board/freescale/imx8mp_evk/imx8mp_evk.c
> +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c
> @@ -7,9 +7,13 @@
>  #include <env.h>
>  #include <errno.h>
>  #include <init.h>
> +#include <miiphy.h>
> +#include <netdev.h>
> +#include <linux/delay.h>
>  #include <asm/mach-imx/iomux-v3.h>
>  #include <asm-generic/gpio.h>
>  #include <asm/arch/imx8mp_pins.h>
> +#include <asm/arch/clock.h>
>  #include <asm/arch/sys_proto.h>
>  #include <asm/mach-imx/gpio.h>
> 
> @@ -40,8 +44,85 @@ int board_early_init_f(void)
>         return 0;
>  }
> 
> +#define FEC_RST_PAD IMX_GPIO_NR(4, 2)
> +static iomux_v3_cfg_t const fec1_rst_pads[] = {
> +       MX8MP_PAD_SAI1_RXD0__GPIO4_IO02 |
> MUX_PAD_CTRL(NO_PAD_CTRL),
> +};
> +
> +static void setup_iomux_fec(void)
> +{
> +       imx_iomux_v3_setup_multiple_pads(fec1_rst_pads,
> +                                        ARRAY_SIZE(fec1_rst_pads));
> +
> +       gpio_request(FEC_RST_PAD, "fec1_rst");
> +       gpio_direction_output(FEC_RST_PAD, 0);
> +       mdelay(15);
> +       gpio_direction_output(FEC_RST_PAD, 1);
> +       mdelay(100);
> +}
> +
> +static int setup_fec(void)
> +{
> +       struct iomuxc_gpr_base_regs *gpr =
> +               (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
> +
> +       setup_iomux_fec();
> +
> +       /* Enable RGMII TX clk output */
> +       setbits_le32(&gpr->gpr[1], BIT(22));
> +
> +       return 0;

There is only one return code here (and it is 0), and it is not checked anywhere below.

Can this function be made "static void setup_fec(void)"?

Besides, return code is not checked in board_init below...

> +}
> +
> +#define EQOS_RST_PAD IMX_GPIO_NR(4, 22)
> +static iomux_v3_cfg_t const eqos_rst_pads[] = {
> +       MX8MP_PAD_SAI2_RXC__GPIO4_IO22 | MUX_PAD_CTRL(NO_PAD_CTRL),
> +};
> +
> +static void setup_iomux_eqos(void)
> +{
> +       imx_iomux_v3_setup_multiple_pads(eqos_rst_pads,
> +                                        ARRAY_SIZE(eqos_rst_pads));
> +
> +       gpio_request(EQOS_RST_PAD, "eqos_rst");
> +       gpio_direction_output(EQOS_RST_PAD, 0);
> +       mdelay(15);
> +       gpio_direction_output(EQOS_RST_PAD, 1);
> +       mdelay(100);
> +}
> +
> +static int setup_eqos(void)
> +{
> +       struct iomuxc_gpr_base_regs *gpr =
> +               (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
> +
> +       setup_iomux_eqos();
> +
> +       /* set INTF as RGMII, enable RGMII TXC clock */
> +       clrsetbits_le32(&gpr->gpr[1],
> +                       IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK, BIT(16));
> +       setbits_le32(&gpr->gpr[1], BIT(19) | BIT(21));
> +
> +       return set_clk_eqos(ENET_125MHZ);
> +}

Function does return a value, but return code is never checked in board_init below.

Perhaps, either make it as void, or check the return code?

> +
> +#if CONFIG_IS_ENABLED(NET)
> +int board_phy_config(struct phy_device *phydev)
> +{
> +       if (phydev->drv->config)
> +               phydev->drv->config(phydev);
> +       return 0;
> +}
> +#endif
> +
>  int board_init(void)
>  {
> +       if (CONFIG_IS_ENABLED(FEC_MXC))
> +               setup_fec();
> +
> +       if (CONFIG_IS_ENABLED(DWC_ETH_QOS))
> +               setup_eqos();
> +

Does it make sense to setup ETH QOS if FEC is not enabled?

>         return 0;
>  }
> 
> diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
> index cd5724e811..de9db697e8 100644
> --- a/configs/imx8mp_evk_defconfig
> +++ b/configs/imx8mp_evk_defconfig
> @@ -73,6 +73,14 @@ CONFIG_MMC_IO_VOLTAGE=y
>  CONFIG_FSL_ESDHC_IMX=y
>  CONFIG_PHYLIB=y
>  CONFIG_DM_ETH=y
> +CONFIG_DWC_ETH_QOS=y
> +

Are those blanks needed in defconfig (here and one below)? 

> +CONFIG_PHY_GIGE=y
> +CONFIG_FEC_MXC=y
> +CONFIG_MII=y
> +CONFIG_PHYLIB=y
> +CONFIG_PHY_REALTEK=y
> +
>  CONFIG_PINCTRL=y
>  CONFIG_SPL_PINCTRL=y
>  CONFIG_PINCTRL_IMX8M=y
> @@ -85,3 +93,6 @@ CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_PSCI=y
>  CONFIG_SYSRESET_WATCHDOG=y
>  CONFIG_IMX_WATCHDOG=y
> +CONFIG_CMD_DHCP=y
> +CONFIG_CMD_MII=y
> +CONFIG_CMD_PING=y
> diff --git a/include/configs/imx8mp_evk.h b/include/configs/imx8mp_evk.h
> index 8253c6aa2f..7abaf5ff84 100644
> --- a/include/configs/imx8mp_evk.h
> +++ b/include/configs/imx8mp_evk.h
> @@ -44,6 +44,22 @@
> 
>  #endif
> 
> +#if defined(CONFIG_CMD_NET)
> +#define CONFIG_ETHPRIME                 "eth1" /* Set eqos to primary since we use
> its MDIO */
> +
> +#define CONFIG_FEC_XCV_TYPE             RGMII
> +#define CONFIG_FEC_MXC_PHYADDR          1
> +#define FEC_QUIRK_ENET_MAC
> +
> +#define DWC_NET_PHYADDR                        1
> +#ifdef CONFIG_DWC_ETH_QOS
> +#define CONFIG_SYS_NONCACHED_MEMORY     (1 * SZ_1M)     /* 1M */
> +#endif
> +
> +#define PHY_ANEG_TIMEOUT 20000
> +
> +#endif
> +
>  /* Initial environment variables */
>  #define CONFIG_EXTRA_ENV_SETTINGS              \
>         "script=boot.scr\0" \
> --
> 2.28.0


-- andrey


More information about the U-Boot mailing list