On 24. 09. 24 9:16, Marco Felsch wrote:
Hi Michael, thanks for the patch, please see below some notes but nothing crucial.+ +&i2c1 { + clock-frequency = <400000>; + pinctrl-0 = <&pinctrl_i2c1>; + pinctrl-names = "default"; + status = "okay"; + + pmic@25 { + compatible = "nxp,pca9450c"; + reg = <0x25>; + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; + interrupt-parent = <&gpio1>; + pinctrl-0 = <&pinctrl_pmic>; + pinctrl-names = "default"; + + regulators { + BUCK1 { + regulator-always-on;All your PMIC rails are marked as regulator-always-on. I didn't expect this from a (battery backed) IOT platform.
These are correct. I is not a battery backed device. It operates on 12V from wall power adapter. It is more like an Ethernet router/edge device for a B2B solution.
+ regulator-boot-on; + regulator-max-microvolt = <1000000>; + regulator-min-microvolt = <720000>; + regulator-name = "BUCK1"; + regulator-ramp-delay = <3125>; + }; +
+ +&iomuxc {Albeit we do sort the node names alphabetical we put the iomuxc at the very end at least for the i.MX platforms. Maybe this is something Frank's tool should be aware of in the future :)
I am used to do it the way you say. I just accepted Frank's comment and that it is maybe a new standard to sort all nodes strictly alphabetically. OK, I will move the iomuxc node back to the end as usual.
+&usdhc3 { + assigned-clocks = <&clk IMX8MP_CLK_USDHC3>; + assigned-clock-rates = <400000000>; + bus-width = <8>; + non-removable;Please move the bus-width and non-removeable properties below the pinctrl-* properties.
OK.
+ pinctrl-0 = <&pinctrl_usdhc3>; + pinctrl-1 = <&pinctrl_usdhc3_100mhz>; + pinctrl-2 = <&pinctrl_usdhc3_200mhz>; + pinctrl-names = "default", "state_100mhz", "state_200mhz"; + status = "okay"; +}; + +&wdog1 { + fsl,ext-reset-output;This property should be put below the pinctrl-* properties.
OK. Thank you for the review, Michal
+ pinctrl-0 = <&pinctrl_wdog>; + pinctrl-names = "default"; + status = "okay"; +};
