On Sun, Apr 26, 2020 at 2:12 PM Marek Vasut <[email protected]> wrote: > > On 4/26/20 6:45 PM, Adam Ford wrote: > > Beacon EmbeddedWorks, formerly known as Logic PD, is releasing > > a devkit based on the i.MX8M Mini SoC consisting of baseboard + > > SOM. > > > > It supports eMMC on the SOM, microSD on the baseboard, various > > GPIO, the PINCTRL, and UART. > > > > Signed-off-by: Adam Ford <[email protected]> > > --- > > The device tree files are pulled from Shawn Guo's imx branch > > destined for Linux 5.8 and have already been removed, but > > the ddrc node was deleted. > >
Marek, Thanks for the review. > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > index 59a2713cb2..249d446f69 100644 > > --- a/arch/arm/dts/Makefile > > +++ b/arch/arm/dts/Makefile > > @@ -730,7 +730,8 @@ dtb-$(CONFIG_ARCH_IMX8M) += \ > > imx8mm-verdin.dtb \ > > imx8mn-ddr4-evk.dtb \ > > imx8mq-evk.dtb \ > > - imx8mp-evk.dtb > > + imx8mp-evk.dtb \ > > + imx8mm-beacon-kit.dtb > > Keep the list sorted. Before verdin or before ddr4-evk? It's already out of order. > > > dtb-$(CONFIG_ARCH_IMXRT) += imxrt1050-evk.dtb \ > > imxrt1020-evk.dtb > > [...] > > > diff --git a/arch/arm/mach-imx/imx8m/Kconfig > > b/arch/arm/mach-imx/imx8m/Kconfig > > index 58f1758ab6..6f6e63472f 100644 > > --- a/arch/arm/mach-imx/imx8m/Kconfig > > +++ b/arch/arm/mach-imx/imx8m/Kconfig > > @@ -56,6 +56,12 @@ config TARGET_VERDIN_IMX8MM > > select SUPPORT_SPL > > select IMX8M_LPDDR4 > > > > +config TARGET_IMX8MM_BEACON > > + bool "imx8mm Beacon Embedded devkit" > > + select IMX8MM > > + select SUPPORT_SPL > > + select IMX8M_LPDDR4 > > + > > endchoice > > > > source "board/freescale/imx8mq_evk/Kconfig" > > @@ -63,5 +69,6 @@ source "board/freescale/imx8mm_evk/Kconfig" > > source "board/freescale/imx8mn_evk/Kconfig" > > source "board/freescale/imx8mp_evk/Kconfig" > > source "board/toradex/verdin-imx8mm/Kconfig" > > +source "board/beacon/imx8mm/Kconfig" > > Keep the list sorted. I can make this alphabetic. > > [...] > > > diff --git a/board/beacon/imx8mm/Makefile b/board/beacon/imx8mm/Makefile > > new file mode 100644 > > index 0000000000..7d83895de8 > > --- /dev/null > > +++ b/board/beacon/imx8mm/Makefile > > @@ -0,0 +1,19 @@ > > +# > > +# Copyright 2020 Compass Electronics Group, LLC > > +# > > +# SPDX-License-Identifier: GPL-2.0+ > > +# > > + > > +obj-y += imx8mm_beacon.o > > +obj-y += ../../freescale/common/ > > + > > +ifdef CONFIG_SPL_BUILD > > +obj-y += spl.o > > +ifdef CONFIG_IMX8M_4G_LPDDR4 > > +obj-y += lpddr4_timing_4g.o > > +else > > +obj-$(CONFIG_IMX8M_LPDDR4) += lpddr4_timing.o > > +obj-$(CONFIG_IMX8M_DDR4) += ddr4_timing.o > > Some of these files are missing in this patch . I will remove it. It's only ever going to be LPDDR4, we won't be supporting DDR4. > > > +endif > > + > > +endif > > diff --git a/board/beacon/imx8mm/README b/board/beacon/imx8mm/README > > new file mode 100644 > > index 0000000000..6dc916fc10 > > --- /dev/null > > +++ b/board/beacon/imx8mm/README > > @@ -0,0 +1,37 @@ > > +U-Boot for the Beacon EmbeddedWorks Devkit > > + > > +Quick Start > > +=========== > > +- Build the ARM Trusted firmware binary > > +- Get ddr fimware > > +- Build U-Boot > > +- Boot > > + > > +Get and Build the ARM Trusted firmware > > +====================================== > > +Note: srctree is U-Boot source directory > > + > > +$ git clone https://source.codeaurora.org/external/imx/imx-atf > > +$ git checkout imx_4.19.35_1.0.0 > > +$ make PLAT=imx8mm bl31 ARCH=arm CROSS_COMPILE=aarch64-linux-gnu- > > +$ cp build/imx8mm/release/bl31.bin $(srctree) > > Why not use mainline TFA instead of this NXP one ? The testing of upstream kernel code appears to require the NXP ATF instead of upstream ATF. Some of the DDRC stuff doesnt' appear to work with upstream ATF. > > [...] > > > diff --git a/board/beacon/imx8mm/imx8mm_beacon.c > > b/board/beacon/imx8mm/imx8mm_beacon.c > > new file mode 100644 > > index 0000000000..e82e8b78d8 > > --- /dev/null > > +++ b/board/beacon/imx8mm/imx8mm_beacon.c > > [...] > > > +#if IS_ENABLED(CONFIG_FEC_MXC) > > +static int setup_fec(void) > > +{ > > + struct iomuxc_gpr_base_regs *gpr = > > + (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; > > + > > + /* Use 125M anatop REF_CLK1 for ENET1, not from external */ > > + clrsetbits_le32(&gpr->gpr[1], 0x2000, 0); > > + > > + return 0; > > +} > > + > > +int board_phy_config(struct phy_device *phydev) > > +{ > > + /* enable rgmii rxc skew and phy mode select to RGMII copper */ > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x1f); > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x8); > > + > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x00); > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x82ee); > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); > > + phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x100); > > Which kind of ethernet PHY does this board use ? AR8031 > > > + if (phydev->drv->config) > > + phydev->drv->config(phydev); > > + return 0; > > +} > > +#endif > > + > > +int board_init(void) > > +{ > > + if (IS_ENABLED(CONFIG_FEC_MXC)) > > + setup_fec(); > > + > > + return 0; > > +} > > + > > +int board_mmc_get_env_dev(int devno) > > +{ > > + return devno; > > +} > > [...] > > > diff --git a/board/beacon/imx8mm/spl.c b/board/beacon/imx8mm/spl.c > > new file mode 100644 > > [...] > > > +static int power_init_board(void) > > +{ > > + struct udevice *dev; > > + int ret; > > + > > + ret = pmic_get("pmic@4b", &dev); > > + if (ret == -ENODEV) { > > + puts("No pmic\n"); > > + return 0; > > + } > > + if (ret != 0) > > + return ret; > > + > > + /* decrease RESET key long push time from the default 10s to 10ms */ > > + pmic_reg_write(dev, BD718XX_PWRONCONFIG1, 0x0); > > + > > + /* unlock the PMIC regs */ > > + pmic_reg_write(dev, BD718XX_REGLOCK, 0x1); > > + > > + /* increase VDD_SOC to typical value 0.85v before first DRAM access */ > > + pmic_reg_write(dev, BD718XX_BUCK1_VOLT_RUN, 0x0f); > > + > > + /* increase VDD_DRAM to 0.975v for 3Ghz DDR */ > > + pmic_reg_write(dev, BD718XX_1ST_NODVS_BUCK_VOLT, 0x83); > > + > > +#ifndef CONFIG_IMX8M_LPDDR4 > > Is this ever an option? You only have LPDDR4 config files in this patch, > so this looks like a remnant from the EVK. You're right, I can remove this. It will always be LPDDR4. > > > + /* increase NVCC_DRAM_1V2 to 1.2v for DDR4 */ > > + pmic_reg_write(dev, BD718XX_4TH_NODVS_BUCK_VOLT, 0x28); > > +#endif > > + > > + /* lock the PMIC regs */ > > + pmic_reg_write(dev, BD718XX_REGLOCK, 0x11); > > + > > + return 0; > > +} > > [...] > > > +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > +{ > > + puts("resetting ...\n"); > > + > > + reset_cpu(WDOG1_BASE_ADDR); > > + > > + return 0; > > +} > > Please do not reimplement reset in SPL, just use some reset driver. > This must also be fixed on the NXP devkits, because they are broken and > wrong. What should they be doing instead? I am just modeling it after the NXP dev kit and it appears to work. Since I don't know what the 'reight' was is, I don't know what I need to do. Ideally, it would be nice if NXP had platform-common code that would do it instead of a series of board files with nearly identical code. adam

