Thread (20 messages) 20 messages, 5 authors, 2021-03-09

Re: [PATCH v2 3/4] arm64: dts: ti: Add support for Siemens IOT2050 boards

From: Jan Kiszka <jan.kiszka@siemens.com>
Date: 2021-03-09 07:25:08
Also in: linux-devicetree, lkml

On 04.03.21 07:58, Vignesh Raghavendra wrote:
Hi,

On 2/12/21 1:02 AM, Jan Kiszka wrote:
quoted
From: Jan Kiszka <jan.kiszka@siemens.com>

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@siemens.com>
Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>
Thanks!
Few minor comments below:

[...]
quoted
+
+&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 { ... };
Done.
quoted
+		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";
+	};
+};
[...]
quoted
+&dwc3_0 {
+	status = "okay";
+};
+
+&usb0_phy {
+	status = "okay";
+};
+
These nodes are enabled by default right? Above is redundant.
Seems like historic left-overs here - fixed.
quoted
+&usb0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_pins_default>;
+	dr_mode = "host";
+};
+
+&dwc3_1 {
+	status = "okay";
+};
+
+&usb1_phy {
+	status = "okay";
+};
+
Same here...
Also fixed.
quoted
+&usb1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb1_pins_default>;
+	dr_mode = "host";
+};
+
[...]
quoted
+&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?
At least to my understanding, "rohm,dh2228fv" is commonly used for
declaring spidev, and this is what we need for userland here.
quoted
+};
+
+&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.
Done.
quoted
+	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
Thanks again, v3 will come after some testing.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help