Hi Oliver On Wed, 2022-10-05 at 16:22 +0200, oliver.gra...@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@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 = <ðphy1>; > > > + 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