diff mbox series

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

Message ID 11e0b0c8b828254567a8ff89820c067cacad2150.1702917360.git.jan.kiszka@siemens.com
State New
Headers
Series arm64: dts: iot2050: Add support for new SM variant |

Commit Message

Jan Kiszka Dec. 18, 2023, 4:36 p.m. UTC 
  From: Baocheng Su <baocheng.su@siemens.com>

Main differences between the new variant and Advanced PG2:

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

Comments

Krzysztof Kozlowski Dec. 19, 2023, 7:54 a.m. UTC | #1 
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


> +
> +&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.

> +};
> +
> +&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.

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 8:22 a.m. UTC | #2 
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
Krzysztof Kozlowski Dec. 19, 2023, 8:48 a.m. UTC | #3 
On 19/12/2023 09:22, Jan Kiszka wrote:
>>
>>> +			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


Then why aren't you overriding by phandle/label?

> patch, but the node names are now part of the kernel ABI. Changing them
> would break existing userland.

You mean label. Why node names became the ABI? Which interface exposes them?

> 
>>
>>> +
>>> +&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

Isn't this mistake? Common part enables only these pieces which are
working in common hardware SoM. If your common part of hardware, which
DTSI should represent, has USB3 then why is it being disabled here? If
common hardware design does not have USB3, then why is it being enabled
in DTSI?
> 

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 9:03 a.m. UTC | #4 
On 19.12.23 09:48, Krzysztof Kozlowski wrote:
> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>
>>>> +			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
> 
> 
> Then why aren't you overriding by phandle/label?
> 

We could do that as well if we added labels first (they don't exist so 
far). Not seeing any difference, though.

>> patch, but the node names are now part of the kernel ABI. Changing them
>> would break existing userland.
> 
> You mean label. Why node names became the ABI? Which interface exposes them?

root@iot2050-debian:~# ls -l /sys/class/leds/
total 0
lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red

>>>> +
>>>> +&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
> 
> Isn't this mistake? Common part enables only these pieces which are
> working in common hardware SoM. If your common part of hardware, which
> DTSI should represent, has USB3 then why is it being disabled here? If
> common hardware design does not have USB3, then why is it being enabled
> in DTSI?

It's a trade-off between adding yet another dtsi for those widely 
common bits vs. adjusting the differences of only one variant from 
that. We do the same for the Display Port so far.

Jan
Krzysztof Kozlowski Dec. 19, 2023, 9:45 a.m. UTC | #5 
On 19/12/2023 10:03, Jan Kiszka wrote:
> On 19.12.23 09:48, Krzysztof Kozlowski wrote:
>> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>>
>>>>> +			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
>>
>>
>> Then why aren't you overriding by phandle/label?
>>
> 
> We could do that as well if we added labels first (they don't exist so 
> far). Not seeing any difference, though.
> 
>>> patch, but the node names are now part of the kernel ABI. Changing them
>>> would break existing userland.
>>
>> You mean label. Why node names became the ABI? Which interface exposes them?
> 
> root@iot2050-debian:~# ls -l /sys/class/leds/


> total 0
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red
> 
>>>>> +
>>>>> +&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
>>
>> Isn't this mistake? Common part enables only these pieces which are
>> working in common hardware SoM. If your common part of hardware, which
>> DTSI should represent, has USB3 then why is it being disabled here? If
>> common hardware design does not have USB3, then why is it being enabled
>> in DTSI?
> 
> It's a trade-off between adding yet another dtsi for those widely 
> common bits vs. adjusting the differences of only one variant from 

You don't need to add one more DTSI to achieve proper architecture of
DTS/DTSI split.

> that. We do the same for the Display Port so far.

DTSI represents common piece of hardware, like SoM or re-usable blocks,
not trade-off.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 19, 2023, 9:50 a.m. UTC | #6 
On 19/12/2023 10:03, Jan Kiszka wrote:
> On 19.12.23 09:48, Krzysztof Kozlowski wrote:
>> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>>
>>>>> +			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
>>
>>
>> Then why aren't you overriding by phandle/label?
>>
> 
> We could do that as well if we added labels first (they don't exist so 
> far). Not seeing any difference, though.

Confusion? Your code suggests new node, thus you got review like you got.

> 
>>> patch, but the node names are now part of the kernel ABI. Changing them
>>> would break existing userland.
>>
>> You mean label. Why node names became the ABI? Which interface exposes them?
> 
> root@iot2050-debian:~# ls -l /sys/class/leds/
> total 0
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red

I replied too fast previous and did not include answer here:

You have label for that... Somehow all these nodes are half-baked,
without all the expected properties and now you call node name as ABI.
The node name is not the ABI.

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 9:54 a.m. UTC | #7 
On 19.12.23 10:50, Krzysztof Kozlowski wrote:
> On 19/12/2023 10:03, Jan Kiszka wrote:
>> On 19.12.23 09:48, Krzysztof Kozlowski wrote:
>>> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>>>
>>>>>> +			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
>>>
>>>
>>> Then why aren't you overriding by phandle/label?
>>>
>>
>> We could do that as well if we added labels first (they don't exist so 
>> far). Not seeing any difference, though.
> 
> Confusion? Your code suggests new node, thus you got review like you got.
> 
>>
>>>> patch, but the node names are now part of the kernel ABI. Changing them
>>>> would break existing userland.
>>>
>>> You mean label. Why node names became the ABI? Which interface exposes them?
>>
>> root@iot2050-debian:~# ls -l /sys/class/leds/
>> total 0
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
>> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red
> 
> I replied too fast previous and did not include answer here:
> 
> You have label for that... Somehow all these nodes are half-baked,
> without all the expected properties and now you call node name as ABI.
> The node name is not the ABI.

Well, existing userspace uses those names, and adding the properties
would break that interface. Now, does Linux do that?

Jan
Jan Kiszka Dec. 19, 2023, 9:56 a.m. UTC | #8 
On 19.12.23 10:54, Jan Kiszka wrote:
> On 19.12.23 10:50, Krzysztof Kozlowski wrote:
>> On 19/12/2023 10:03, Jan Kiszka wrote:
>>> On 19.12.23 09:48, Krzysztof Kozlowski wrote:
>>>> On 19/12/2023 09:22, Jan Kiszka wrote:
>>>>>>
>>>>>>> +			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
>>>>
>>>>
>>>> Then why aren't you overriding by phandle/label?
>>>>
>>>
>>> We could do that as well if we added labels first (they don't exist so 
>>> far). Not seeing any difference, though.
>>
>> Confusion? Your code suggests new node, thus you got review like you got.
>>
>>>
>>>>> patch, but the node names are now part of the kernel ABI. Changing them
>>>>> would break existing userland.
>>>>
>>>> You mean label. Why node names became the ABI? Which interface exposes them?
>>>
>>> root@iot2050-debian:~# ls -l /sys/class/leds/
>>> total 0
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
>>> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red
>>
>> I replied too fast previous and did not include answer here:
>>
>> You have label for that... Somehow all these nodes are half-baked,
>> without all the expected properties and now you call node name as ABI.
>> The node name is not the ABI.
> 
> Well, existing userspace uses those names, and adding the properties
> would break that interface. Now, does Linux do that?
> 

Obviously, we could deviate from the existing naming scheme only for the
new variant, keeping it for the other 5, but that will be "fun" to maintain.

Jan
Krzysztof Kozlowski Dec. 19, 2023, 9:58 a.m. UTC | #9 
On 19/12/2023 10:54, Jan Kiszka wrote:
>>>> You mean label. Why node names became the ABI? Which interface exposes them?
>>>
>>> root@iot2050-debian:~# ls -l /sys/class/leds/
>>> total 0
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
>>> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red
>>
>> I replied too fast previous and did not include answer here:
>>
>> You have label for that... Somehow all these nodes are half-baked,
>> without all the expected properties and now you call node name as ABI.
>> The node name is not the ABI.
> 
> Well, existing userspace uses those names, and adding the properties
> would break that interface. Now, does Linux do that?

I don't think you understood the concept. There is no change for
userspace. Same interface, same names. No ABI break.

Anyway, changing them is not part of this patchset since these are not
new nodes.

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 3:37 p.m. UTC | #10 
On 19.12.23 10:58, Krzysztof Kozlowski wrote:
> On 19/12/2023 10:54, Jan Kiszka wrote:
>>>>> You mean label. Why node names became the ABI? Which interface exposes them?
>>>>
>>>> root@iot2050-debian:~# ls -l /sys/class/leds/
>>>> total 0
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
>>>> lrwxrwxrwx 1 root root 0 Dec 14 21:12 status-led-green -> ../../devices/platform/leds/leds/status-led-green
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 status-led-red -> ../../devices/platform/leds/leds/status-led-red
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-green -> ../../devices/platform/leds/leds/user-led1-green
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led1-red -> ../../devices/platform/leds/leds/user-led1-red
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-green -> ../../devices/platform/leds/leds/user-led2-green
>>>> lrwxrwxrwx 1 root root 0 Dec 19 08:55 user-led2-red -> ../../devices/platform/leds/leds/user-led2-red
>>>
>>> I replied too fast previous and did not include answer here:
>>>
>>> You have label for that... Somehow all these nodes are half-baked,
>>> without all the expected properties and now you call node name as ABI.
>>> The node name is not the ABI.
>>
>> Well, existing userspace uses those names, and adding the properties
>> would break that interface. Now, does Linux do that?
> 
> I don't think you understood the concept. There is no change for
> userspace. Same interface, same names. No ABI break.

I do understand the impact very well:
open("/sys/class/leds/user-led1-red") has to work for all the variants,
consistently and backward-compatible for userspace.

> 
> Anyway, changing them is not part of this patchset since these are not
> new nodes.

Fine, then we can leave the LED topic aside for now.

I will look into the other comments.

Jan
Krzysztof Kozlowski Dec. 19, 2023, 3:39 p.m. UTC | #11 
On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>
>>>> You have label for that... Somehow all these nodes are half-baked,
>>>> without all the expected properties and now you call node name as ABI.
>>>> The node name is not the ABI.
>>>
>>> Well, existing userspace uses those names, and adding the properties
>>> would break that interface. Now, does Linux do that?
>>
>> I don't think you understood the concept. There is no change for
>> userspace. Same interface, same names. No ABI break.
> 
> I do understand the impact very well:
> open("/sys/class/leds/user-led1-red") has to work for all the variants,
> consistently and backward-compatible for userspace.

And it will. The name is the same.

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 3:40 p.m. UTC | #12 
On 19.12.23 16:39, Krzysztof Kozlowski wrote:
> On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>>
>>>>> You have label for that... Somehow all these nodes are half-baked,
>>>>> without all the expected properties and now you call node name as ABI.
>>>>> The node name is not the ABI.
>>>>
>>>> Well, existing userspace uses those names, and adding the properties
>>>> would break that interface. Now, does Linux do that?
>>>
>>> I don't think you understood the concept. There is no change for
>>> userspace. Same interface, same names. No ABI break.
>>
>> I do understand the impact very well:
>> open("/sys/class/leds/user-led1-red") has to work for all the variants,
>> consistently and backward-compatible for userspace.
> 
> And it will. The name is the same.

Nope, it's not - I tried that already :)

root@iot2050-debian:~# ls -l /sys/class/leds/
total 0
lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:indicator -> ../../devices/platform/leds/leds/green:indicator
lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:status -> ../../devices/platform/leds/leds/green:status
lrwxrwxrwx 1 root root 0 Dec 19 09:49 mmc0:: -> ../../devices/platform/bus@100000/4fa0000.mmc/leds/mmc0::
lrwxrwxrwx 1 root root 0 Dec 19 09:49 mmc1:: -> ../../devices/platform/bus@100000/4f80000.mmc/leds/mmc1::
lrwxrwxrwx 1 root root 0 Dec 19 09:49 red:indicator -> ../../devices/platform/leds/leds/red:indicator
lrwxrwxrwx 1 root root 0 Dec 19 09:49 red:indicator_1 -> ../../devices/platform/leds/leds/red:indicator_1
lrwxrwxrwx 1 root root 0 Dec 19 09:49 red:indicator_2 -> ../../devices/platform/leds/leds/red:indicator_2
lrwxrwxrwx 1 root root 0 Dec 19 09:49 red:status -> ../../devices/platform/leds/leds/red:status

Jan
Krzysztof Kozlowski Dec. 19, 2023, 3:42 p.m. UTC | #13 
On 19/12/2023 16:40, Jan Kiszka wrote:
> On 19.12.23 16:39, Krzysztof Kozlowski wrote:
>> On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>>>
>>>>>> You have label for that... Somehow all these nodes are half-baked,
>>>>>> without all the expected properties and now you call node name as ABI.
>>>>>> The node name is not the ABI.
>>>>>
>>>>> Well, existing userspace uses those names, and adding the properties
>>>>> would break that interface. Now, does Linux do that?
>>>>
>>>> I don't think you understood the concept. There is no change for
>>>> userspace. Same interface, same names. No ABI break.
>>>
>>> I do understand the impact very well:
>>> open("/sys/class/leds/user-led1-red") has to work for all the variants,
>>> consistently and backward-compatible for userspace.
>>
>> And it will. The name is the same.
> 
> Nope, it's not - I tried that already :)
> 
> root@iot2050-debian:~# ls -l /sys/class/leds/
> total 0
> lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:indicator -> ../../devices/platform/leds/leds/green:indicator

And how does your DTS look like?

Because I also tried and it is exactly the same.

Best regards,
Krzysztof
Jan Kiszka Dec. 19, 2023, 3:48 p.m. UTC | #14 
On 19.12.23 16:42, Krzysztof Kozlowski wrote:
> On 19/12/2023 16:40, Jan Kiszka wrote:
>> On 19.12.23 16:39, Krzysztof Kozlowski wrote:
>>> On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>>>>
>>>>>>> You have label for that... Somehow all these nodes are half-baked,
>>>>>>> without all the expected properties and now you call node name as ABI.
>>>>>>> The node name is not the ABI.
>>>>>>
>>>>>> Well, existing userspace uses those names, and adding the properties
>>>>>> would break that interface. Now, does Linux do that?
>>>>>
>>>>> I don't think you understood the concept. There is no change for
>>>>> userspace. Same interface, same names. No ABI break.
>>>>
>>>> I do understand the impact very well:
>>>> open("/sys/class/leds/user-led1-red") has to work for all the variants,
>>>> consistently and backward-compatible for userspace.
>>>
>>> And it will. The name is the same.
>>
>> Nope, it's not - I tried that already :)
>>
>> root@iot2050-debian:~# ls -l /sys/class/leds/
>> total 0
>> lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:indicator -> ../../devices/platform/leds/leds/green:indicator
> 
> And how does your DTS look like?
> 
> Because I also tried and it is exactly the same.
> 

I played with

diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
index 402afa4bc1b6..a791444eeb93 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
@@ -10,6 +10,7 @@
  */
 
 #include "k3-am654.dtsi"
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/phy/phy.h>
 #include <dt-bindings/net/ti-dp83867.h>
 
@@ -84,27 +85,39 @@ leds {
 		pinctrl-0 = <&leds_pins_default>;
 
 		status-led-red {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_STATUS;
 			gpios = <&wkup_gpio0 32 GPIO_ACTIVE_HIGH>;
 			panic-indicator;
 		};
 
 		status-led-green {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_STATUS;
 			gpios = <&wkup_gpio0 24 GPIO_ACTIVE_HIGH>;
 		};
 
 		user-led1-red {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_INDICATOR;
 			gpios = <&pcal9535_3 14 GPIO_ACTIVE_HIGH>;
 		};
 
 		user-led1-green {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_INDICATOR;
 			gpios = <&pcal9535_2 15 GPIO_ACTIVE_HIGH>;
 		};
 
 		user-led2-red {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_INDICATOR;
 			gpios = <&wkup_gpio0 17 GPIO_ACTIVE_HIGH>;
 		};
 
 		user-led2-green {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_INDICATOR;
 			gpios = <&wkup_gpio0 22 GPIO_ACTIVE_HIGH>;
 		};
 	};

Jan
Krzysztof Kozlowski Dec. 19, 2023, 3:56 p.m. UTC | #15 
On 19/12/2023 16:48, Jan Kiszka wrote:
> On 19.12.23 16:42, Krzysztof Kozlowski wrote:
>> On 19/12/2023 16:40, Jan Kiszka wrote:
>>> On 19.12.23 16:39, Krzysztof Kozlowski wrote:
>>>> On 19/12/2023 16:37, Jan Kiszka wrote:
>>>>>>>>
>>>>>>>> You have label for that... Somehow all these nodes are half-baked,
>>>>>>>> without all the expected properties and now you call node name as ABI.
>>>>>>>> The node name is not the ABI.
>>>>>>>
>>>>>>> Well, existing userspace uses those names, and adding the properties
>>>>>>> would break that interface. Now, does Linux do that?
>>>>>>
>>>>>> I don't think you understood the concept. There is no change for
>>>>>> userspace. Same interface, same names. No ABI break.
>>>>>
>>>>> I do understand the impact very well:
>>>>> open("/sys/class/leds/user-led1-red") has to work for all the variants,
>>>>> consistently and backward-compatible for userspace.
>>>>
>>>> And it will. The name is the same.
>>>
>>> Nope, it's not - I tried that already :)
>>>
>>> root@iot2050-debian:~# ls -l /sys/class/leds/
>>> total 0
>>> lrwxrwxrwx 1 root root 0 Dec 19 09:49 green:indicator -> ../../devices/platform/leds/leds/green:indicator
>>
>> And how does your DTS look like?
>>
>> Because I also tried and it is exactly the same.
>>
> 
> I played with
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> index 402afa4bc1b6..a791444eeb93 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "k3-am654.dtsi"
> +#include <dt-bindings/leds/common.h>
>  #include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/net/ti-dp83867.h>
>  
> @@ -84,27 +85,39 @@ leds {
>  		pinctrl-0 = <&leds_pins_default>;
>  
>  		status-led-red {
> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_STATUS;
>  			gpios = <&wkup_gpio0 32 GPIO_ACTIVE_HIGH>;
>  			panic-indicator;

And where is the label property?

Please read my message again:

>> You:
>> patch, but the node names are now part of the kernel ABI. Changing
them would break existing userland.
> Me:
> You mean label. Why node names became the ABI? Which interface exposes
them?


>> You:
>> root@iot2050-debian:~# ls -l /sys/class/leds/
> Me:
> I replied too fast previous and did not include answer here:
> You have label for that...

So again: The stable ABI is fulfilled by using label property. Not the
Devicetree "label" phandle in front of the node, but the dedicated property.

Best regards,
Krzysztof
diff mbox series

Patch

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 {
+			gpios = <&wkup_gpio0 52 GPIO_ACTIVE_HIGH>;
+		};
+
+		user-led1-green {
+			gpios = <&wkup_gpio0 53 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+&main_pmx0 {
+	main_pcie_enable_pins_default: main-pcie-enable-default-pins {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x01d8, PIN_OUTPUT, 7)  /* (AH12) GPIO1_22 */
+		>;
+	};
+
+	main_spi0_pins: main-spi0-default-pins  {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x01c4, PIN_INPUT, 0) /* (AH13) SPI0_CLK */
+			AM65X_IOPAD(0x01c8, PIN_INPUT, 0) /* (AE13) SPI0_D0 */
+			AM65X_IOPAD(0x01cc, PIN_INPUT, 0) /* (AD13) SPI0_D1 */
+			AM65X_IOPAD(0x01bc, PIN_OUTPUT, 0) /* (AG13) SPI0_CS0 */
+		>;
+	};
+};
+
+&main_pmx1 {
+	asic_spi_mux_ctrl_pin: asic-spi-mux-ctrl-default-pins {
+		pinctrl-single,pins = <
+			AM65X_IOPAD(0x0010, PIN_OUTPUT, 7)  /* (D21) GPIO1_86 */
+		>;
+	};
+};
+
+&wkup_pmx0 {
+	user1_led_pins: user1-led-default-pins {
+		pinctrl-single,pins = <
+			/* (AB1) WKUP_UART0_RXD:WKUP_GPIO0_52, as USER 1 led red */
+			AM65X_WKUP_IOPAD(0x00a0, PIN_OUTPUT, 7)
+			/* (AB5) WKUP_UART0_TXD:WKUP_GPIO0_53, as USER 1 led green */
+			AM65X_WKUP_IOPAD(0x00a4, PIN_OUTPUT, 7)
+		>;
+	};
+
+	soc_asic_pins: soc-asic-default-pins {
+		pinctrl-single,pins = <
+			AM65X_WKUP_IOPAD(0x0044, PIN_INPUT, 7)  /* (P4) WKUP_GPIO0_29 */
+			AM65X_WKUP_IOPAD(0x0048, PIN_INPUT, 7)  /* (P5) WKUP_GPIO0_30 */
+			AM65X_WKUP_IOPAD(0x004c, PIN_INPUT, 7)  /* (P1) WKUP_GPIO0_31 */
+		>;
+	};
+};
+
+&main_gpio0 {
+	gpio-line-names = "main_gpio0-base";
+};
+
+&main_gpio1 {
+	pinctrl-names = "default";
+	pinctrl-0 =
+		<&cp2102n_reset_pin_default>,
+		<&main_pcie_enable_pins_default>,
+		<&asic_spi_mux_ctrl_pin>;
+	gpio-line-names =
+		/* 0..9 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 10..19 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 20..29 */
+		"", "", "", "", "CP2102N-RESET", "", "", "", "", "",
+		/* 30..39 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 40..49 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 50..59 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 60..69 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 70..79 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 80..86 */
+		"", "", "", "", "", "", "ASIC-spi-mux-ctrl";
+};
+
+&wkup_gpio0 {
+	pinctrl-names = "default";
+	pinctrl-0 =
+		<&push_button_pins_default>,
+		<&db9_com_mode_pins_default>,
+		<&soc_asic_pins>;
+	gpio-line-names =
+		/* 0..9 */
+		"wkup_gpio0-base", "", "", "", "UART0-mode1", "UART0-mode0",
+		"UART0-enable", "UART0-terminate", "", "WIFI-disable",
+		/* 10..19 */
+		"", "", "", "", "", "", "", "", "", "",
+		/* 20..29 */
+		"", "", "", "", "", "USER-button", "", "", "","ASIC-gpio-0",
+		/* 30..31 */
+		"ASIC-gpio-1", "ASIC-gpio-2";
+};
+
+&main_spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&main_spi0_pins>;
+
+	#address-cells = <1>;
+	#size-cells= <0>;
+};
+
+&mcu_spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcu_spi0_pins_default>;
+};
+
+&main_i2c3 {
+	accelerometer: lsm6dso@6a {
+		compatible = "st,lsm6dso";
+		reg = <0x6a>;
+	};
+
+	/delete-node/ edp-bridge@f;
+};
+
+&dss {
+	status = "disabled";
+};
+
+&dss_ports {
+	/delete-node/ port@1;
+};
+
+&serdes0 {
+	assigned-clocks = <&k3_clks 153 4>, <&serdes0 AM654_SERDES_CMU_REFCLK>;
+	assigned-clock-parents = <&k3_clks 153 8>, <&k3_clks 153 4>;
+};
+
+&serdes1 {
+	status = "disabled";
+};
+
+&pcie0_rc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&minipcie_pins_default>;
+
+	num-lanes = <1>;
+	phys = <&serdes0 PHY_TYPE_PCIE 1>;
+	phy-names = "pcie-phy0";
+	reset-gpios = <&wkup_gpio0 27 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+};
+
+&pcie1_rc {
+	status = "disabled";
+};
+
+&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;
+};
+
+&usb0 {
+	maximum-speed = "high-speed";
+	/delete-property/ snps,dis-u1-entry-quirk;
+	/delete-property/ snps,dis-u2-entry-quirk;
+};