Hi, On 2/12/21 1:02 AM, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > > Add support for two Siemens SIMATIC IOT2050 variants, Basic and > Advanced. They are based on the TI AM6528 GP and AM6548 SOCs HS, thus > differ in their number of cores and availability of security features. > Furthermore the Advanced version comes with more RAM, an eMMC and a few > internal differences. > > Based on original version by Le Jin. > > Link: https://new.siemens.com/global/en/products/automation/pc-based/iot-gateways/simatic-iot2050.html > Link: https://github.com/siemens/meta-iot2050 > Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> Reviewed-by: Vignesh Raghavendra <vigneshr@xxxxxx> Few minor comments below: [...] > + > +&mcu_i2c0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mcu_i2c0_pins_default>; > + clock-frequency = <400000>; > + > + psu: tps62363@60 { Please use generic node names: psu: regulator@60 { ... }; > + compatible = "ti,tps62363"; > + reg = <0x60>; > + regulator-name = "tps62363-vout"; > + regulator-min-microvolt = <500000>; > + regulator-max-microvolt = <1500000>; > + regulator-boot-on; > + ti,vsel0-state-high; > + ti,vsel1-state-high; > + ti,enable-vout-discharge; > + }; > + > + /* D4200 */ > + pcal9535_1: gpio@20 { > + compatible = "nxp,pcal9535"; > + reg = <0x20>; > + #gpio-cells = <2>; > + gpio-controller; > + gpio-line-names = > + "A0-pull", "A1-pull", "A2-pull", "A3-pull", "A4-pull", > + "A5-pull", "", "", > + "IO14-enable", "IO15-enable", "IO16-enable", > + "IO17-enable", "IO18-enable", "IO19-enable"; > + }; > + > + /* D4201 */ > + pcal9535_2: gpio@21 { > + compatible = "nxp,pcal9535"; > + reg = <0x21>; > + #gpio-cells = <2>; > + gpio-controller; > + gpio-line-names = > + "IO0-direction", "IO1-direction", "IO2-direction", > + "IO3-direction", "IO4-direction", "IO5-direction", > + "IO6-direction", "IO7-direction", > + "IO8-direction", "IO9-direction", "IO10-direction", > + "IO11-direction", "IO12-direction", "IO13-direction", > + "IO19-direction"; > + }; > + > + /* D4202 */ > + pcal9535_3: gpio@25 { > + compatible = "nxp,pcal9535"; > + reg = <0x25>; > + #gpio-cells = <2>; > + gpio-controller; > + gpio-line-names = > + "IO0-pull", "IO1-pull", "IO2-pull", "IO3-pull", > + "IO4-pull", "IO5-pull", "IO6-pull", "IO7-pull", > + "IO8-pull", "IO9-pull", "IO10-pull", "IO11-pull", > + "IO12-pull", "IO13-pull"; > + }; > +}; [...] > +&dwc3_0 { > + status = "okay"; > +}; > + > +&usb0_phy { > + status = "okay"; > +}; > + These nodes are enabled by default right? Above is redundant. > +&usb0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&usb0_pins_default>; > + dr_mode = "host"; > +}; > + > +&dwc3_1 { > + status = "okay"; > +}; > + > +&usb1_phy { > + status = "okay"; > +}; > + Same here... > +&usb1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&usb1_pins_default>; > + dr_mode = "host"; > +}; > + [...] > +&mcu_spi0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mcu_spi0_pins_default>; > + > + #address-cells = <1>; > + #size-cells= <0>; > + ti,pindir-d0-out-d1-in = <1>; > + > + spidev@0 { > + compatible = "rohm,dh2228fv"; > + spi-max-frequency = <20000000>; > + reg = <0>; > + }; Is the device really dh2228fv? > +}; > + > +&tscadc0 { > + status = "disabled"; > +}; > + > +&tscadc1 { > + adc { > + ti,adc-channels = <0 1 2 3 4 5>; > + }; > +}; > + > +&ospi0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mcu_fss0_ospi0_pins_default>; > + > + flash@0 { > + compatible = "jedec,spi-nor"; > + reg = <0x0>; > + spi-tx-bus-width = <1>; > + spi-rx-bus-width = <1>; > + spi-max-frequency = <50000000>; > + cdns,tshsl-ns = <60>; > + cdns,tsd2d-ns = <60>; > + cdns,tchsh-ns = <60>; > + cdns,tslch-ns = <60>; > + cdns,read-delay = <2>; > + #address-cells = <1>; > + #size-cells = <1>; > + }; > +}; > + > +&dss { > + status = "okay"; > + Node is enabled by default. Please drop above line for consistency. > + pinctrl-names = "default"; > + pinctrl-0 = <&dss_vout1_pins_default>; > + > + assigned-clocks = <&k3_clks 67 2>; > + assigned-clock-parents = <&k3_clks 67 5>; > +}; > + > +&dss_ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@1 { > + reg = <1>; > + > + dpi_out: endpoint { > + remote-endpoint = <&bridge_in>; > + }; > + }; > +}; > + [...] Regards Vignesh