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

[Gilles Talis]

Hi Fabio,
thank you for the second review!

Le ven. 25 oct. 2024 à 07:20, Fabio Estevam <festevam at gmail.com> a écrit :
>
> Hi Gilles,
>
> On Thu, Oct 24, 2024 at 9:44 PM Gilles Talis <gilles.talis at gmail.com> wrote:
> >
> > The Emcraft Systems NavQ+ kit is a mobile robotics platform
> > based on NXP i.MX8 MPlus SoC.
> >
> > The following interfaces and devices are enabled:
> > - eMMC
> > - Gigabit Ethernet (through eQOS interface)
> > - SD-Card
> > - UART console
> >
> > The device tree file is taken from upstream Linux Kernel
> > through OF_UPSTREAM
> >
> > Signed-off-by: Gilles Talis <gilles.talis at gmail.com>
> > ---
> > Changes v1 --> v2:
> > - Removed unused usdhc properties from dtsi
> > - Moved to correct ethernet PHY driver (REALTEK -> ATHEROS)
> > - Removed empty board_late_init() function
> > - Moved env variables to an env file
>
> This version looks better, thanks.
>
> CI reports an error.
>
> Warning, treated as error:
> 142/builds/u-boot/custodians/u-boot-imx/doc/board/emcraft/index.rst:6:toctree
> contains reference to nonexisting document
> 'board/emcraft/imx8mp-navqp'
> 143make[1]: *** [doc/Makefile:70: htmldocs] Error 2
> 144make: *** [Makefile:2445: htmldocs] Error 2
>
> Please fix and resend.
Good catch! I will fix this.

>
> > +#include <miiphy.h>
> > +
> > +#if CONFIG_IS_ENABLED(NET) || CONFIG_IS_ENABLED(NET_LWIP)
> > +int board_phy_config(struct phy_device *phydev)
> > +{
> > +       if (phydev->drv->config)
> > +               phydev->drv->config(phydev);
> > +
> > +       return 0;
>
> Can't board_phy_config() be removed now?
Indeed. Will do.

>
> > diff --git a/board/emcraft/imx8mp_navqp/imx8mp_navqp.env b/board/emcraft/imx8mp_navqp/imx8mp_navqp.env
> > new file mode 100644
> > index 0000000000..a8b76574b4
> > --- /dev/null
> > +++ b/board/emcraft/imx8mp_navqp/imx8mp_navqp.env
> > @@ -0,0 +1,13 @@
>
> SPDX header is missing for this file.
OK. Will do.

>
> > +int board_fit_config_name_match(const char *name)
> > +{
> > +       if (is_imx8mp() &&
> > +           !strcmp(name, "imx8mp-navqp"))
> > +               return 0;
> > +
> > +       return -1;
>
> return -EINVAL;
OK.

>
> > +#if defined(CONFIG_CMD_NET)
> > +#define CFG_FEC_MXC_PHYADDR          1
>
> With DM, this is no longer used, right?
Indeed. Will remove.