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

2021-03-08 Thread Jan Kiszka
On 04.03.21 07:58, Vignesh Raghavendra wrote:
> Hi,
> 
> On 2/12/21 1:02 AM, Jan Kiszka wrote:
>> From: Jan Kiszka 
>>
>> 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 
> 
> Reviewed-by: Vignesh Raghavendra 
> 

Thanks!

> Few minor comments below:
> 
> [...]
> 
>> +
>> +_i2c0 {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_i2c0_pins_default>;
>> +clock-frequency = <40>;
>> +
>> +psu: tps62363@60 {
> 
> Please use generic node names:
> 
>   psu: regulator@60 { ... };
> 

Done.

>> +compatible = "ti,tps62363";
>> +reg =  <0x60>;
>> +regulator-name = "tps62363-vout";
>> +regulator-min-microvolt = <50>;
>> +regulator-max-microvolt = <150>;
>> +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";
>> +};
>> +};
> 
> [...]
> 
>> +_0 {
>> +status = "okay";
>> +};
>> +
>> +_phy {
>> +status = "okay";
>> +};
>> +
> 
> These nodes are enabled by default right? Above is redundant.

Seems like historic left-overs here - fixed.

> 
>> + {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_pins_default>;
>> +dr_mode = "host";
>> +};
>> +
>> +_1 {
>> +status = "okay";
>> +};
>> +
>> +_phy {
>> +status = "okay";
>> +};
>> +
> 
> Same here...

Also fixed.

> 
>> + {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_pins_default>;
>> +dr_mode = "host";
>> +};
>> +
> 
> [...]
> 
>> +_spi0 {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_spi0_pins_default>;
>> +
>> +#address-cells = <1>;
>> +#size-cells= <0>;
>> +ti,pindir-d0-out-d1-in = <1>;
>> +
>> +spidev@0 {
>> +compatible = "rohm,dh2228fv";
>> +spi-max-frequency = <2000>;
>> +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.

> 
>> +};
>> +
>> + {
>> +status = "disabled";
>> +};
>> +
>> + {
>> +adc {
>> +ti,adc-channels = <0 1 2 3 4 5>;
>> +};
>> +};
>> +
>> + {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_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 = <5000>;
>> +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>;
>> +};
>> +};
>> +
>> + {
>> +status = "okay";
>> +
> 
> Node is enabled 

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

2021-03-03 Thread Vignesh Raghavendra
Hi,

On 2/12/21 1:02 AM, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> 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 

Reviewed-by: Vignesh Raghavendra 

Few minor comments below:

[...]

> +
> +_i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_i2c0_pins_default>;
> + clock-frequency = <40>;
> +
> + psu: tps62363@60 {

Please use generic node names:

psu: regulator@60 { ... };

> + compatible = "ti,tps62363";
> + reg =  <0x60>;
> + regulator-name = "tps62363-vout";
> + regulator-min-microvolt = <50>;
> + regulator-max-microvolt = <150>;
> + 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";
> + };
> +};

[...]

> +_0 {
> + status = "okay";
> +};
> +
> +_phy {
> + status = "okay";
> +};
> +

These nodes are enabled by default right? Above is redundant.

> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins_default>;
> + dr_mode = "host";
> +};
> +
> +_1 {
> + status = "okay";
> +};
> +
> +_phy {
> + status = "okay";
> +};
> +

Same here...

> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins_default>;
> + dr_mode = "host";
> +};
> +

[...]

> +_spi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_spi0_pins_default>;
> +
> + #address-cells = <1>;
> + #size-cells= <0>;
> + ti,pindir-d0-out-d1-in = <1>;
> +
> + spidev@0 {
> + compatible = "rohm,dh2228fv";
> + spi-max-frequency = <2000>;
> + reg = <0>;
> + };

Is the device really dh2228fv?

> +};
> +
> + {
> + status = "disabled";
> +};
> +
> + {
> + adc {
> + ti,adc-channels = <0 1 2 3 4 5>;
> + };
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_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 = <5000>;
> + 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>;
> + };
> +};
> +
> + {
> + status = "okay";
> +

Node is enabled by default. Please drop above line for consistency.

> + pinctrl-names = "default";
> + pinctrl-0 = <_vout1_pins_default>;
> +
> + assigned-clocks = <_clks 67 2>;
> + assigned-clock-parents = <_clks 67 5>;
> +};
> +
> +_ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@1 {
> + reg = <1>;
> +
> + 

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

2021-02-11 Thread Jan Kiszka
From: Jan Kiszka 

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 
---
 arch/arm64/boot/dts/ti/Makefile   |   2 +
 .../boot/dts/ti/k3-am65-iot2050-common.dtsi   | 679 ++
 .../boot/dts/ti/k3-am6528-iot2050-basic.dts   |  61 ++
 .../dts/ti/k3-am6548-iot2050-advanced.dts |  60 ++
 4 files changed, 802 insertions(+)
 create mode 100644 arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
 create mode 100644 arch/arm64/boot/dts/ti/k3-am6528-iot2050-basic.dts
 create mode 100644 arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced.dts

diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 65506f21ba30..22108491f16e 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -7,6 +7,8 @@
 #
 
 dtb-$(CONFIG_ARCH_K3) += k3-am654-base-board.dtb
+dtb-$(CONFIG_ARCH_K3) += k3-am6528-iot2050-basic.dtb
+dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced.dtb
 
 dtb-$(CONFIG_ARCH_K3) += k3-j721e-common-proc-board.dtb
 
diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi 
b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
new file mode 100644
index ..28e31c219a33
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
@@ -0,0 +1,679 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ *   Le Jin 
+ *   Jan Kiszka 
+ *
+ * Common bits of the IOT2050 Basic and Advanced variants
+ */
+
+/dts-v1/;
+
+#include "k3-am654.dtsi"
+#include 
+
+/ {
+   aliases {
+   spi0 = _spi0;
+   };
+
+   chosen {
+   stdout-path = "serial3:115200n8";
+   bootargs = "earlycon=ns16550a,mmio32,0x0281";
+   };
+
+   reserved-memory {
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   secure_ddr: secure-ddr@9e80 {
+   reg = <0 0x9e80 0 0x0180>; /* for OP-TEE */
+   alignment = <0x1000>;
+   no-map;
+   };
+
+   mcu_r5fss0_core0_dma_memory_region: r5f-dma-memory@a000 {
+   compatible = "shared-dma-pool";
+   reg = <0 0xa000 0 0x10>;
+   no-map;
+   };
+
+   mcu_r5fss0_core0_memory_region: r5f-memory@a010 {
+   compatible = "shared-dma-pool";
+   reg = <0 0xa010 0 0xf0>;
+   no-map;
+   };
+
+   mcu_r5fss0_core1_dma_memory_region: r5f-dma-memory@a100 {
+   compatible = "shared-dma-pool";
+   reg = <0 0xa100 0 0x10>;
+   no-map;
+   };
+
+   mcu_r5fss0_core1_memory_region: r5f-memory@a110 {
+   compatible = "shared-dma-pool";
+   reg = <0 0xa110 0 0xf0>;
+   no-map;
+   };
+
+   rtos_ipc_memory_region: ipc-memories@a200 {
+   reg = <0x00 0xa200 0x00 0x0020>;
+   alignment = <0x1000>;
+   no-map;
+   };
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_default>;
+
+   status-led-red {
+   gpios = <_gpio0 32 GPIO_ACTIVE_HIGH>;
+   panic-indicator;
+   };
+
+   status-led-green {
+   gpios = <_gpio0 24 GPIO_ACTIVE_HIGH>;
+   };
+
+   user-led1-red {
+   gpios = <_3 14 GPIO_ACTIVE_HIGH>;
+   };
+
+   user-led1-green {
+   gpios = <_2 15 GPIO_ACTIVE_HIGH>;
+   };
+
+   user-led2-red {
+   gpios = <_gpio0 17 GPIO_ACTIVE_HIGH>;
+   };
+
+   user-led2-green {
+   gpios = <_gpio0 22 GPIO_ACTIVE_HIGH>;
+   };
+   };
+
+   dp_refclk: clock {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1920>;
+   };
+};
+
+_pmx0 {
+   wkup_i2c0_pins_default: wkup-i2c0-pins-default {
+   pinctrl-single,pins = <
+   /* (AC7) WKUP_I2C0_SCL */
+