Received-SPF: pass (google.com: domain of linux-kernel+bounces-4879-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <4c31adc5-3fc5-47bc-9766-6d3d1eeddb65@siemens.com>
Date: Tue, 19 Dec 2023 09:22:21 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 4/4] dts: iot2050: Support IOT2050-SM variant
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Nishanth Menon <nm@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>,
 Tero Kristo <kristo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Bao Cheng Su <baocheng.su@siemens.com>,
 Chao Zeng <chao.zeng@siemens.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
 devicetree@vger.kernel.org, Li Hua Qian <huaqian.li@siemens.com>
References: <cover.1702917360.git.jan.kiszka@siemens.com>
 <11e0b0c8b828254567a8ff89820c067cacad2150.1702917360.git.jan.kiszka@siemens.com>
 <8b3daa3c-dbf8-4286-b04e-011cd9b0efa5@linaro.org>
From: Jan Kiszka <jan.kiszka@siemens.com>
In-Reply-To: <8b3daa3c-dbf8-4286-b04e-011cd9b0efa5@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?eDBpa1paZ3Iyc2ZLc2NtaTIxNmhVYVpobzROUDBIVDN5NU1Iejd3Nisyb0xH?=
 =?utf-8?B?OWFlb092Lzh3QWJXQmRoMXp4WEduZ2F6VjFlTTNiQWcvRCtWbEJLWmtOQ1BG?=
 =?utf-8?B?a0kxcFQ5aEp2WU5JRWN4YjNKbTh4QnpybUpLUTB2YlU3UE44RnpIczVaOGg1?=
 =?utf-8?B?cXlXbE92RXZUWGRQTUtld0sxeXlkWXVjT2RPQ2VVMkNjS2RYbS9tZEFiSm1I?=
 =?utf-8?B?M3JtdnFBL3JvaURhdGZLR3B1TE1ISEdxaHUrQUw5cDVmMVo1MGIwclFpY2Fp?=
 =?utf-8?B?QmNCS2hnNFoxRy9LaHhMcXJoRU1mbFJPc3ExZWd1REhrbjA2NU9IU2tXeXRQ?=
 =?utf-8?B?NEE2WXU3WnZNTy8zZFlGa2R5VDZyNWd1NUVxamFRdThTcjU2QmZvS0lnSXNO?=
 =?utf-8?B?dUlVU29sVGtvcityTk9xejdlZXJoOVdZcHZEVE1XRFdxS3F5SGFiY3lpVEJL?=
 =?utf-8?B?N1Byb1M4UHVxM3pLdjR3OFRXaGIyd2M0cTV3NUxxS0tHdlYwYTNBMVdhWmhN?=
 =?utf-8?B?VG9UOGlHNEpHNFFlNTZjQmcxTm5lbHN4c3VOeisrVGcwTFQxb0dMSkpOdEg0?=
 =?utf-8?B?L2E1Q2JNNzUxdzduaDZKUmZNTVkwV1d2MTVXYWwreEJnNVI5L052STVyWjVH?=
 =?utf-8?B?Y0Nadlk4Tmo5SWJ4QlZ3UmxFTE1hcWh0eGhFaExOdTJvcC9rU1NEbjRHb0NJ?=
 =?utf-8?B?NHFydnRVU2hUS0xiaGxtRnpkUjc4QnBlV0pPSFNCd2JnUDl2d3hLSXhFVy91?=
 =?utf-8?B?dExMTmVzOXZVckVpY3RRcjB2UjBzUUt5ZkFnVzBZSW1TWXUrNWw4ekh5SUtD?=
 =?utf-8?B?a2llTFNEN3RRbGUrZ21LakxRM0c3b1JIczNjdUhQaUtVV0FYbzY2d1Z4T01S?=
 =?utf-8?B?UCtwUDJJR0QyNHorTjhkZTYvbEhjM3dsZHZkQWRzcWgySlMyRXNOSi92bVRt?=
 =?utf-8?B?V0FnRmFwUWpFU0VYRXlVZkFQYUlWd3NCdHdXdXRPUjVmVVRGR2NkZTluMmda?=
 =?utf-8?B?U1JsamtmMDhLa1p0VjZac3Q5ZWNHbCs1cEpKNjd4d0dlbDdqcG9Ed0p3dnpa?=
 =?utf-8?B?ci8rcnBPSkZSWGVoVGtaWnRvVkIzUml2M2NsNkR3bFNYNUROY1dZSGVwTkxv?=
 =?utf-8?B?c0VsY0paZlVnM2JoTGdDYzFCUHFxRmdBUmJnVlF4a09MYXlucndjZkJCcm04?=
 =?utf-8?B?NlArcU9nQzdGRlNsUnUzS1ZmeUlHNko5dUZudisyck1Kd2pDWEdvSDFVUzFh?=
 =?utf-8?B?VjdZcEVBNXpVUE1CYm5KTnkxenZreDlrVUZXck9aYlI5dXIwVm4xc3VmMG81?=
 =?utf-8?B?WUZlR3hDU1RqN08zSCtVdUYzQjVpNFlLQkhkZ2QzWEtweE10T3RSaHZBQlhF?=
 =?utf-8?B?WjNMZG8ralJCSDNjMGlPWFpsTlBPQWQyY0lWUFRFZGs4aUpISjZHcjBaOXdn?=
 =?utf-8?B?WUpuc2JRZmltV1Y1RzhySjRKYmlzRTJpSzU4QWZwdU9WNWo2bzdOMlhFVmZm?=
 =?utf-8?B?M0dldGttOW1zbHJjOFR4ZmhsdWtBRmVXMUM2YnZDUXJWeCt0RXZoVkoyR2Rx?=
 =?utf-8?B?QlFsV2VlUWF1YlhjSWNmUDQ1VXM3OXIzd0dIdjZSNENVbnExQXZST3hMWTJa?=
 =?utf-8?B?QURpR3IxRk5oc1lVcUtrTkpMYzFtSUxvb09JVms5cjJaaUwwOGxhS2RYL2Fq?=
 =?utf-8?B?RDl5djZPS3JoS1dtK0Vkd3h4c3NvQXMva0hkME84WWxSazhNY2Vsb3NSZWhv?=
 =?utf-8?B?cExZQkxjT0lmS2NJYnBVb2o3ZVZhWmJhNmVoVFFCZEdsMUNkSkg1eTdydEFt?=
 =?utf-8?B?bERDRWtLcGlZL0JVb0xnc1d4NHNtbE44T0VDeTdkQitZUi8vZDB4RzliYlNw?=
 =?utf-8?B?R3JzQjZiN1hYY1FuMlJIL2RoaEk2WjF4MkNhWmNYZ3RxYS9jNExobDJHWFda?=
 =?utf-8?B?bFZ3dzNWQ3VsL0xvblk2M0MxQUw5cDdPUEIzd1l3ZU90cWVtRmJsUzdUTVRt?=
 =?utf-8?B?N2d6bVYrcTBzWHJOb2J3am1DbytTZjFwS0dLVjVxcEZhSUlyTGp6aW0xSEt6?=
 =?utf-8?B?dVVkSEkyVzJrbnFQSjl5Sis0SXFDWTNBcE9iR2ZzTUowWW01SFBzY04xQ3RC?=
 =?utf-8?B?a3MxRnA5RlhZTVM2ejBMUnFveGpwTkx3cFdRME0rR25ReDhtbDJpZ2kzNWMw?=
 =?utf-8?B?eEE9PQ==?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cc43fce8-6082-46bd-d8cd-08dc006ba0c5
X-MS-Exchange-CrossTenant-AuthSource: AS4PR10MB6181.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2023 08:22:24.0725
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +jDpJu+K6+ToyvER4EncQUY6FBa0q6FQV559GDeFUXk6wR3PE24r3ysWX0wlM+xk99TazCxKfc+nyeIFHsYcrg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR10MB3529

On 19.12.23 08:54, Krzysztof Kozlowski wrote:
> On 18/12/2023 17:36, Jan Kiszka wrote:
>> From: Baocheng Su <baocheng.su@siemens.com>
>>
>> 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@siemens.com>
>> Signed-off-by: Chao Zeng <chao.zeng@siemens.com>
>> Co-developed-by: Li Hua Qian <huaqian.li@siemens.com>
>> Signed-off-by: Li Hua Qian <huaqian.li@siemens.com>
>> Signed-off-by: Baocheng Su <baocheng.su@siemens.com>
>> [Jan: rebase over arduino refactoring, split-out light sensor]
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  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@siemens.com>
>> + *   Chao Zeng <chao.zeng@siemens.com>
>> + *   Huaqian Li <huaqian.li@siemens.com>
>> + *
>> + * 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