[PATCH v4] imx: support i.MX8QM DMSSE20 a1 board

[Marcel Ziswiler]

Hi Oliver

On Wed, 2022-10-05 at 16:22 +0200, oliver.graute at kococonnector.com wrote:
> On 13/07/22, Marcel Ziswiler wrote:

[snip]

> 
> > > + */
> > > +
> > > +/dts-v1/;
> > > +
> > > +/* First 128KB is for PSCI ATF. */
> > > +/memreserve/ 0x80000000 0x00020000;
> > > +
> > > +#include "fsl-imx8qm.dtsi"
> > > +
> > > +/ {
> > > +       model = "Advantech iMX8QM DMSSE20";
> > > +       compatible = "fsl,imx8qm-mek", "fsl,imx8qm";
> > > +
> > > +       aliases {
> > > +          mmc0 = &usdhc1;
> > > +          mmc2 = &usdhc3;
> > > +       };
> > > +
> > > +       chosen {
> > > +               bootargs = "console=ttyLP0,115200 earlycon=lpuart32,0x5a060000,115200";
> > 
> > This stuff is completely downstream bogus.
> 
> I'am confused here because I see such statements in a lot of device
> trees.

What I meant is the part after earlycon. It is recommend to not have anything there 

https://www.kernel.org/doc/html/v4.14/admin-guide/kernel-parameters.html

> When used with no options, the early console is
> determined by the stdout-path property in device
> tree's chosen node.

> > > +               stdout-path = &lpuart0;
> > > +       };
> > > +
> > > +       regulators {
> > 
> > That grouping is also bogus.
> 
> What do you mean exactly?

What I meant is that this grouping does not make any sense at all and needs to go. Just leave "[tab]regulators
{" and its closing "};" away and remove one "[tab]" of indentation for all them regulators. For reference e.g.
see here

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8qm-mek.dts?h=v6.0#n30

> > > +               compatible = "simple-bus";
> > > +               #address-cells = <1>;
> > > +               #size-cells = <0>;
> > > +
> > > +               reg_usb_otg1_vbus: regulator at 0 {
> > > +                       compatible = "regulator-fixed";
> > > +                       reg = <0>;
> > 
> > Using any such is also bogus (same with above @0 notation, of course).
> 
> I'am confused here too. What is the right notation now?

There simply is no such register zero or anything. See also above linked example.

> > > +                       regulator-name = "usb_otg1_vbus";
> > > +                       regulator-min-microvolt = <5000000>;
> > > +                       regulator-max-microvolt = <5000000>;
> > > +                       gpio = <&gpio4 3 GPIO_ACTIVE_HIGH>;
> > > +                       enable-active-high;
> > > +               };
> > > +
> > > +               reg_usdhc2_vmmc: usdhc2_vmmc {
> > > +                       compatible = "regulator-fixed";
> > > +                       regulator-name = "sw-3p3-sd1";
> > > +                       regulator-min-microvolt = <3300000>;
> > > +                       regulator-max-microvolt = <3300000>;
> > > +                       gpio = <&gpio4 7 GPIO_ACTIVE_HIGH>;
> > > +                       enable-active-high;
> > > +               };
> > > +
> > > +               busfreq {
> > > +                       status = "disabled";
> > > +               };
> > > +       };
> > > +};

[snip]

> > > +&fec2 {
> > > +       pinctrl-names = "default";
> > > +       pinctrl-0 = <&pinctrl_fec2>;
> > > +       phy-mode = "rgmii-id";
> > > +       phy-handle = <&ethphy1>;
> > > +       fsl,ar8031-phy-fixup;
> > > +       fsl,magic-packet;
> > > +       status = "okay";
> > > +       fsl,mii-exclusive;
> > 
> > That actually is just downstream only NXP bogus.
> 
> you mean the `fsl,mii-exclusive`? so I should just drop it?

Yes, you may grep the entire kernel sources for even "mii-exclusive". It does not exist anywhere so nothing
will ever happen except confusing people.

[snip]

> > 
> > Otherwise looks great. Keep up the good work!
> > 
> 
> thx for the review

You are very welcome.

> > Cheers
> > 
> > Marcel
> 
> Best Regards,
> 
> Oliver

Cheers

Marcel