[PATCH] board: emcraft: Add support for Emcraft Systems NavQ+

[Gilles Talis]

Hi Fabio,
thanks for the review.

Le lun. 21 oct. 2024 à 08:21, Fabio Estevam <festevam at gmail.com> a écrit :
>
> Hi Gilles,
>
> On Sun, Oct 20, 2024 at 5:10 PM Gilles Talis <gilles.talis at gmail.com> wrote:
>
> > +/ {
> > +       wdt-reboot {
> > +               compatible = "wdt-reboot";
> > +               wdt = <&wdog1>;
> > +               bootph-pre-ram;
> > +       };
>
> Please add a blank line here.
Noted.

>
> > +       firmware {
> > +               optee {
> > +                       compatible = "linaro,optee-tz";
> > +                       method = "smc";
> > +               };
> > +       };
> > +};
>
> > +&usdhc3 {
> > +       bootph-pre-ram;
> > +       mmc-hs400-1_8v;
> > +       mmc-hs400-enhanced-strobe;
>
> Why are the 2 properties above only present in the U-Boot dtsi and not
> in the kernel devicetree?
I think it is copy/paste without further verification :-)

>
> > +#if CONFIG_IS_ENABLED(NET) || CONFIG_IS_ENABLED(NET_LWIP)
> > +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);
>
> Are these board code settings needed? Can't this be derived in the
> devicetree instead?
Yes, they are needed. I don't know how they can be derived in the
devicetree, but I will check.

>
> > +int board_late_init(void)
> > +{
> > +       return 0;
> > +}
>
> This can be removed, along with CONFIG_BOARD_LATE_INIT=y.
OK. Will do.

>
> > diff --git a/board/emcraft/imx8mp_navqp/imximage-8mp-lpddr4.cfg b/board/emcraft/imx8mp_navqp/imximage-8mp-lpddr4.cfg
> > new file mode 100644
> > index 0000000000..999cb630fe
> > --- /dev/null
> > +++ b/board/emcraft/imx8mp_navqp/imximage-8mp-lpddr4.cfg
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2021, 2024 NXP
> > + */
> > +
> > +
>
> Remove the double-blank line.
OK.

>
> > +Get and Build the ARM Trusted Firmware (Trusted Firmware A)
> > +-----------------------------------------------------------
> > +
> > +.. code-block:: bash
> > +
> > +    $ echo "Downloading and building TF-A..."
> > +    $ git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
> > +    $ cd trusted-firmware-a
>
> Please specify a specific branch of the trusted-firmware-a repo.
OK.

>
> > +/* Initial environment variables */
> > +#define CFG_EXTRA_ENV_SETTINGS         \
> > +       BOOTENV \
> > +       "scriptaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> > +       "kernel_addr_r=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> > +       "image=Image\0" \
> > +       "console=ttymxc1,115200 earlycon=ec_imx6q,0x30890000,115200\0" \
> > +       "fdt_addr_r=0x43000000\0"                       \
> > +       "boot_fdt=try\0" \
> > +       "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
> > +       "initrd_addr=0x43800000\0"              \
> > +       "bootm_size=0x10000000\0" \
> > +       "mmcpart=1\0" \
> > +       "mmcroot=/dev/mmcblk1p2 rootwait rw\0" \
>
> Please move the above variables to
> board/emcraft/imx8mp_navqp/imx8mp_navqp.env instead.
OK. Will do.

thanks
Gilles.