From: Jan Kiszka <jan.kiszka@siemens.com>
To: Vignesh Raghavendra <vigneshr@ti.com>, Nishanth Menon <nm@ti.com>,
Tero Kristo <kristo@kernel.org>, Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org,
Bao Cheng Su <baocheng.su@siemens.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Le Jin <le.jin@siemens.com>
Subject: Re: [PATCH v2 3/4] arm64: dts: ti: Add support for Siemens IOT2050 boards
Date: Tue, 9 Mar 2021 08:18:52 +0100 [thread overview]
Message-ID: <0561ad0d-7297-35ad-a3a9-49dc9a6bacd3@siemens.com> (raw)
In-Reply-To: <95e4231c-6bee-ba64-412f-87d257df61c4@ti.com>
On 04.03.21 07:58, Vignesh Raghavendra wrote:
> Hi,
>
> On 2/12/21 1:02 AM, Jan Kiszka wrote:
>> 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:
>
> [...]
>
>> +
>> +&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.
>> + 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.
Seems like historic left-overs here - fixed.
>
>> +&usb0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&usb0_pins_default>;
>> + dr_mode = "host";
>> +};
>> +
>> +&dwc3_1 {
>> + status = "okay";
>> +};
>> +
>> +&usb1_phy {
>> + status = "okay";
>> +};
>> +
>
> Same here...
Also fixed.
>
>> +&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?
At least to my understanding, "rohm,dh2228fv" is commonly used for
declaring spidev, and this is what we need for userland here.
>
>> +};
>> +
>> +&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.
>
>> + 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
next prev parent reply other threads:[~2021-03-09 7:25 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-11 19:32 [PATCH v2 0/4] arm64: Add TI AM65x-based IOT2050 boards Jan Kiszka
2021-02-11 19:32 ` [PATCH v2 1/4] dt-bindings: Add Siemens vendor prefix Jan Kiszka
2021-03-05 19:33 ` Rob Herring
2021-03-05 20:18 ` Nishanth Menon
2021-02-11 19:32 ` [PATCH v2 2/4] dt-bindings: arm: ti: Add bindings for Siemens IOT2050 boards Jan Kiszka
2021-03-04 6:14 ` Vignesh Raghavendra
2021-03-05 19:33 ` Rob Herring
2021-02-11 19:32 ` [PATCH v2 3/4] arm64: dts: ti: Add support " Jan Kiszka
2021-03-04 6:58 ` Vignesh Raghavendra
2021-03-09 7:18 ` Jan Kiszka [this message]
2021-03-09 8:38 ` [PATCH v3 " Jan Kiszka
2021-03-09 14:41 ` Nishanth Menon
2021-03-09 15:15 ` Jan Kiszka
2021-03-09 15:10 ` Nishanth Menon
2021-03-09 15:22 ` Jan Kiszka
2021-03-09 15:38 ` Nishanth Menon
2021-02-11 19:32 ` [PATCH v2 4/4] arm64: dts: ti: k3-am65-main: Add device_type to pcie*_rc nodes Jan Kiszka
2021-02-12 14:55 ` Kishon Vijay Abraham I
2021-02-12 19:31 ` [PATCH v2 0/4] arm64: Add TI AM65x-based IOT2050 boards Jan Kiszka
2021-03-03 14:05 ` Nishanth Menon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0561ad0d-7297-35ad-a3a9-49dc9a6bacd3@siemens.com \
--to=jan.kiszka@siemens.com \
--cc=baocheng.su@siemens.com \
--cc=devicetree@vger.kernel.org \
--cc=kristo@kernel.org \
--cc=le.jin@siemens.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nm@ti.com \
--cc=robh+dt@kernel.org \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).