[PATCH] board: add InnoComm i.MX8MM WB15EVK

Matt Porter
2020-04-20T21:09:48Z

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@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