[PATCH] ARM: imx: pico-imx8mq: Add support for Technexion Pico-iMX8MQ

[Fabio Estevam]

Hi Marek,

On Wed, Apr 1, 2020 at 9:42 PM Marek Vasut <marek.vasut at gmail.com> wrote:

> @@ -0,0 +1,416 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 Wandboard, Org.
> + * Copyright 2017 NXP
> + *
> + * Author: Richard Hu <hakahu at gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +/* First 128KB is for PSCI ATF. */
> +/memreserve/ 0x40000000 0x00020000;

Please remove this as it is not part of the official device tree.

If you really need it, then add it to the u-boot.dtsi version.

I guess this is not needed and I plan to remove it from imx8mq-evk as well.0x0},

> +#include <common.h>
> +#include <env.h>
> +#include <init.h>
> +#include <malloc.h>
> +#include <errno.h>
> +#include <asm/io.h>
> +#include <miiphy.h>
> +#include <netdev.h>
> +#include <asm/mach-imx/iomux-v3.h>
> +#include <asm-generic/gpio.h>
> +#include <fsl_esdhc_imx.h>
> +#include <mmc.h>
> +#include <asm/arch/imx8mq_pins.h>
> +#include <asm/arch/sys_proto.h>
> +#include <asm/mach-imx/gpio.h>
> +#include <asm/mach-imx/mxc_i2c.h>
> +#include <asm/arch/clock.h>
> +#include <spl.h>
> +#include <power/pmic.h>
> +#include <power/pfuze100_pmic.h>

This include file is not needed as this PMIC is not used on this board.

> +static void setup_iomux_fec(void)
> +{
> +       imx_iomux_v3_setup_multiple_pads(fec1_pads, ARRAY_SIZE(fec1_pads));
> +
> +       gpio_request(IMX_GPIO_NR(1, 0), "fec1_pwr");
> +       gpio_direction_output(IMX_GPIO_NR(1, 0), 1);
> +       udelay(500);
> +
> +       gpio_request(IMX_GPIO_NR(1, 9), "fec1_rst");
> +       gpio_direction_output(IMX_GPIO_NR(1, 9), 0);
> +       udelay(500);
> +       gpio_direction_output(IMX_GPIO_NR(1, 9), 1);


This should be done via device tree instead of board code.

> +int board_late_init(void)
> +{
> +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> +       env_set("board_name", "EVK");

This board is not an EVK ;-)

> +       printf("%s: LPDDR4 %i GiB\n", __func__, size);
> +       ddr_init(dram_timing);
> +       writel(size, M4_BOOTROM_BASE_ADDR);

Why is this needed?

> +       /* Adjust pmic voltage to 1.0V for 800M */
> +       setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1);

The comment does not match the code.

> index 0000000000..9193e1f406
> --- /dev/null
> +++ b/configs/pico-imx8mq_defconfig
> @@ -0,0 +1,49 @@
> +CONFIG_ARM=y
> +CONFIG_SPL_SYS_ICACHE_OFF=y
> +CONFIG_SPL_SYS_DCACHE_OFF=y

Can't we have caches enabled now? I don't recall why we disabled it
for i.MX8MQ EVK, but I thought this is no longer an issue.

> diff --git a/include/configs/pico-imx8mq.h b/include/configs/pico-imx8mq.h
> new file mode 100644
> index 0000000000..0f1c7831e6
> --- /dev/null
> +++ b/include/configs/pico-imx8mq.h
> @@ -0,0 +1,216 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2018 NXP
> + */
> +
> +#ifndef __IMX8M_EVK_H
> +#define __IMX8M_EVK_H

This should be adjusted for the pico board name instead of evk.

> +#define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> +
> +#define CONFIG_POWER
> +#define CONFIG_POWER_I2C
> +#define CONFIG_POWER_PFUZE100
> +#define CONFIG_POWER_PFUZE100_I2C_ADDR 0x08

This PMIC is not used on this board.


> +
> +/* ENET Config */
> +/* ENET1 */
> +#if defined(CONFIG_CMD_NET)
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_MII
> +#define CONFIG_MII
> +#define CONFIG_ETHPRIME                        "FEC"
> +
> +#define CONFIG_FEC_MXC

This could be moved to defconfig.

> +#define CONFIG_FEC_XCV_TYPE            RGMII
> +#define CONFIG_FEC_MXC_PHYADDR         1
> +#define FEC_QUIRK_ENET_MAC
> +
> +#define CONFIG_PHY_GIGE
> +#define IMX_FEC_BASE                   0x30BE0000

Not needed if you use DM.

> +
> +#define CONFIG_PHYLIB
> +#define CONFIG_PHY_ATHEROS
> +#endif
> +
> +/* Initial environment variables */
> +#define CONFIG_EXTRA_ENV_SETTINGS                                      \
> +       "script=boot.scr\0"                                             \
> +       "image=Image\0"                                                 \
> +       "console=ttymxc0,115200\0"                                      \
> +       "fdt_addr=0x43000000\0"                                         \
> +       "fdt_high=0xffffffffffffffff\0"                                 \
> +       "boot_fdt=try\0"                                                \

Please remove "boot_fdt".

This only made sense when we wanted to supported both dt and non-dt
platforms in the beginning of i.MX6 times.