Re: [PATCH 4/4] dts: iot2050: Support IOT2050-SM variant

[Jan Kiszka]

On 19.12.23 08:54, Krzysztof Kozlowski wrote:
> On 18/12/2023 17:36, Jan Kiszka wrote:
>> From: Baocheng Su <baocheng.su@xxxxxxxxxxx>
>>
>> Main differences between the new variant and Advanced PG2:
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
> 
>>
>> 1. Arduino interface is removed. Instead, an new ASIC is added for
>>    communicating with PLC 1200 signal modules.
>> 2. USB 3.0 type A connector is removed, only USB 2.0 type A connector is
>>    avaiable.
>> 3. DP interface is tailored down. Instead, to communicate with the
>>    PLC 1200 signal modules, a USB 3.0 type B connector is added but the
>>    signal is not USB.
>> 4. DDR size is increased to 4 GB.
>> 5. Two sensors are added, one tilt sensor and one light sensor.
>>
>> The light sensor it not yet added to the DT at this stage as it depends
>> on to-be-added bindings.
>>
>> Co-developed-by: Chao Zeng <chao.zeng@xxxxxxxxxxx>
>> Signed-off-by: Chao Zeng <chao.zeng@xxxxxxxxxxx>
>> Co-developed-by: Li Hua Qian <huaqian.li@xxxxxxxxxxx>
>> Signed-off-by: Li Hua Qian <huaqian.li@xxxxxxxxxxx>
>> Signed-off-by: Baocheng Su <baocheng.su@xxxxxxxxxxx>
>> [Jan: rebase over arduino refactoring, split-out light sensor]
>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>> ---
>>  arch/arm64/boot/dts/ti/Makefile               |   1 +
>>  .../dts/ti/k3-am6548-iot2050-advanced-sm.dts  | 210 ++++++++++++++++++
>>  2 files changed, 211 insertions(+)
>>  create mode 100644 arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-sm.dts
>>
>> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
>> index 77a347f9f47d..9b15eaad284c 100644
>> --- a/arch/arm64/boot/dts/ti/Makefile
>> +++ b/arch/arm64/boot/dts/ti/Makefile
>> @@ -53,6 +53,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-am6528-iot2050-basic-pg2.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-m2.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-pg2.dtb
>> +dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-sm.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am654-base-board.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am654-gp-evm.dtb
>>  dtb-$(CONFIG_ARCH_K3) += k3-am654-evm.dtb
>> diff --git a/arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-sm.dts b/arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-sm.dts
>> new file mode 100644
>> index 000000000000..ab3eef683890
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-sm.dts
>> @@ -0,0 +1,210 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Siemens AG, 2023
>> + *
>> + * Authors:
>> + *   Baocheng Su <baocheng.su@xxxxxxxxxxx>
>> + *   Chao Zeng <chao.zeng@xxxxxxxxxxx>
>> + *   Huaqian Li <huaqian.li@xxxxxxxxxxx>
>> + *
>> + * AM6548-based (quad-core) IOT2050 SM variant, Product Generation 2
>> + * 4 GB RAM, 16 GB eMMC, USB-serial converter on connector X30
>> + *
>> + * Product homepage:
>> + * https://new.siemens.com/global/en/products/automation/pc-based/iot-gateways/simatic-iot2050.html
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "k3-am6548-iot2050-advanced-common.dtsi"
>> +#include "k3-am65-iot2050-common-pg2.dtsi"
>> +
>> +/ {
>> +	compatible = "siemens,iot2050-advanced-sm", "ti,am654";
>> +	model = "SIMATIC IOT2050 Advanced SM";
>> +
>> +	memory@80000000 {
>> +		device_type = "memory";
>> +		/* 4G RAM */
>> +		reg = <0x00000000 0x80000000 0x00000000 0x80000000>,
>> +		      <0x00000008 0x80000000 0x00000000 0x80000000>;
>> +	};
>> +
>> +	aliases {
>> +		spi1 = &main_spi0;
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&leds_pins_default>, <&user1_led_pins>;
>> +
>> +		user-led1-red {
> 
> led-0
> 
>> +			gpios = <&wkup_gpio0 52 GPIO_ACTIVE_HIGH>;
> 
> Missing function, missing color. Color goes as property, not as node name.
> 
>> +		};
>> +
>> +		user-led1-green {
> 
> led-1
> 
>> +			gpios = <&wkup_gpio0 53 GPIO_ACTIVE_HIGH>;
> 
> Ditto
> 

This is adjusting the existing LED nodes in k3-am65-iot2050-common.dtsi,
not introducing new ones. We can add the color properties in a separate
patch, but the node names are now part of the kernel ABI. Changing them
would break existing userland.

> 
>> +
>> +&dwc3_0 {
>> +	assigned-clock-parents = <&k3_clks 151 4>,  /* set REF_CLK to 20MHz i.e. PER0_PLL/48 */
>> +				 <&k3_clks 151 9>;  /* set PIPE3_TXB_CLK to CLK_12M_RC/256 (for HS only) */
>> +	/delete-property/ phys;
>> +	/delete-property/ phy-names;
> 
> If your board need to remove phys from the SoC node, something is wrong.
> Either your board or SoC.
> 
> Any removal of properties in DTS is weird and unexpected. It deserves
> comments.

This goes along disabling USB3 which is by default enabled via
k3-am65-iot2050-common-pg2.dtsi

> 
>> +};
>> +
>> +&usb0 {
>> +	maximum-speed = "high-speed";
>> +	/delete-property/ snps,dis-u1-entry-quirk;
>> +	/delete-property/ snps,dis-u2-entry-quirk;
> 
> Same questions. If SoC has quirks, how can your board be fixed? It's SoC
> property... or you are using different SoC.
> 

Same story.

Baocheng, Zeng Chao, correct me if there is more behind that.

Jan

-- 
Siemens AG, Technology
Linux Expert Center