[PATCH] board: add InnoComm i.MX8MM WB15EVK

Matt Porter mporter at konsulko.com
Mon Apr 20 23:09:48 CEST 2020


On Thu, Apr 16, 2020 at 02:33:17PM -0300, Fabio Estevam wrote:
> Hi Matt,

Hi Fabio, thanks for the review!

> On Thu, Apr 9, 2020 at 6:51 PM Matt Porter <mporter at konsulko.com> wrote:
> 
> Hopefully this dts will be sent upstream.

Yes, this just made it out first. Updating both accordingly below.

> 
> > +/ {
> > +       model = "InnoComm i.MX8MM WB15EVK";
> > +       compatible = "fsl,imx8mm-wb15evk", "fsl,imx8mm";
> 
> Should be "innocomm,imx8mm-wb15evk", "fsl,imx8mm" instead, since the
> board manufacturer is InnoComm.

Oops, yeah, will fix that.

> 
> > +&gpio1 {
> > +       phy_en {
> > +               gpio-hog;
> > +               gpios = <10 GPIO_ACTIVE_HIGH>;
> > +               output-high;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pinctrl_phy_en>;
> > +       };
> 
> Shouldn't this be modelled as a phy-supply GPIO controlled regulator instead?

Yes, converting it to a regulator will be better.

> > +CONFIG_SPL_SYS_ICACHE_OFF=y
> > +CONFIG_SPL_SYS_DCACHE_OFF=y

> Can't we work with caches enabled by now?

Yes, I verified we're fine with cache on so I'll fix that.

> > +/* USDHC */
> > +#define CONFIG_FSL_USDHC
> 
> Better put it in the defconfig instead.

Ok.
> 
> > +#define CONFIG_FEC_XCV_TYPE             RGMII
> > +#define FEC_QUIRK_ENET_MAC
> 
> Shouldn't this be moved to a SoC header instead of each board file?

It does look like it should be moved to imx-regs.h like imx6/7 do.

> > +
> > +#define IMX_FEC_BASE                    0x30BE0000
> 
> Not needed as you are using FEC DM.

Ok.

-Matt


More information about the U-Boot mailing list