| Message ID | 20201228121710.17235-1-peng.fan@oss.nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/13] imx: imx8mp_evk: enable eth support | expand |
On Mon, Dec 28, 2020 at 7:27 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > From: Ye Li <ye.li@nxp.com> > > Enable print to show the DRAM rate of current setting and training > result. > > Signed-off-by: Ye Li <ye.li@nxp.com> > Reviewed-by: Peng Fan <peng.fan@nxp.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/ddr/imx/imx8m/ddr_init.c | 7 ++++--- > drivers/ddr/imx/imx8m/ddrphy_utils.c | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c > index 99a67edfb0..65739dbaa7 100644 > --- a/drivers/ddr/imx/imx8m/ddr_init.c > +++ b/drivers/ddr/imx/imx8m/ddr_init.c > @@ -96,7 +96,7 @@ int ddr_init(struct dram_timing_info *dram_timing) > unsigned int tmp, initial_drate, target_freq; > int ret; > > - debug("DDRINFO: start DRAM init\n"); > + printf("DDRINFO: start DRAM init\n"); Why not make this optional? With debug enabled, it will print. This makes extra chatter for everyone. It also undoes: 0d3bc813 ("imx8m: ddr_init: Move ddr_init() messages to debug level") > > /* Step1: Follow the power up procedure */ > if (is_imx8mq()) { > @@ -119,6 +119,7 @@ int ddr_init(struct dram_timing_info *dram_timing) > > initial_drate = dram_timing->fsp_msg[0].drate; > /* default to the frequency point 0 clock */ > + printf("DDRINFO: DRAM rate %dMTS\n", initial_drate); > ddrphy_init_set_dfi_clk(initial_drate); > > /* D-aasert the presetn */ > @@ -185,7 +186,7 @@ int ddr_init(struct dram_timing_info *dram_timing) > tmp = reg32_read(DDRPHY_CalBusy(0)); > } while ((tmp & 0x1)); > > - debug("DDRINFO:ddrphy calibration done\n"); > + printf("DDRINFO:ddrphy calibration done\n"); > > /* Step15: Set SWCTL.sw_done to 0 */ > reg32_write(DDRC_SWCTL(0), 0x00000000); > @@ -240,7 +241,7 @@ int ddr_init(struct dram_timing_info *dram_timing) > > /* enable port 0 */ > reg32_write(DDRC_PCTRL_0(0), 0x00000001); > - debug("DDRINFO: ddrmix config done\n"); > + printf("DDRINFO: ddrmix config done\n"); > > board_dram_ecc_scrub(); > > diff --git a/drivers/ddr/imx/imx8m/ddrphy_utils.c b/drivers/ddr/imx/imx8m/ddrphy_utils.c > index 0f8baefb1f..326b92d784 100644 > --- a/drivers/ddr/imx/imx8m/ddrphy_utils.c > +++ b/drivers/ddr/imx/imx8m/ddrphy_utils.c > @@ -104,7 +104,7 @@ int wait_ddrphy_training_complete(void) > debug("Training PASS\n"); > return 0; > } else if (mail == 0xff) { > - debug("Training FAILED\n"); > + printf("Training FAILED\n"); > return -1; > } > } > -- > 2.28.0 >
On Mon, Dec 28, 2020 at 10:53 AM Adam Ford <aford173@gmail.com> wrote: > Why not make this optional? With debug enabled, it will print. This > makes extra chatter for everyone. > It also undoes: 0d3bc813 ("imx8m: ddr_init: Move ddr_init() messages > to debug level") Exactly. This type of information could be of interest during development, but for the final version, it is noise.
Hello Peng, > -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Peng Fan (OSS) > Sent: Monday, December 28, 2020 1:17 PM > To: sbabic@denx.de; festevam@gmail.com > Cc: uboot-imx@nxp.com; u-boot@lists.denx.de; Peng Fan <peng.fan@nxp.com> > Subject: [PATCH 01/13] imx: imx8mp_evk: enable eth support > > > From: Peng Fan <peng.fan@nxp.com> > > Add board code to configure the network interface > Add net defconfig > > Signed-off-by: Peng Fan <peng.fan@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
Hi Peng, On Mon, Dec 28, 2020 at 8:48 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > +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); The Ethernet PHY reset can be handled by the device tree.
> Subject: Re: [PATCH 01/13] imx: imx8mp_evk: enable eth support > > Hi Peng, > > On Mon, Dec 28, 2020 at 8:48 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> > wrote: > > > +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); > > The Ethernet PHY reset can be handled by the device tree. I could use phy-reset-gpios for it. But latest device tree I think prefer reset-gpios under the phy node which is not supported by U-Boot. I'll use phy-reset-gpios. Thanks, Peng.
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; +} + +#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); +} + +#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(); + 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 + +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" \