[PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

The devices in the wkup domain are capable of waking up the system from
suspend. We can configure the wkup domain devices in a generic way using
the ti-sysc interconnect target module driver like we have done with the
earlier TI SoCs.

As ti-sysc manages the SYSCONFIG related registers independent of the
child hardware device, the wake-up configuration is also set even if
wkup_uart0 is reserved by sysfw.

The wkup_uart0 device has interconnect target module register mapping like
dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
block in the target module. The power domain and clock affects the whole
interconnect target module.

Note we change the functional clock name to follow the ti-sysc binding
and use "fck" instead of "fclk".

Tested-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Changes since v1:

- Added Tested-by from Dhruva

---
 arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
--- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
@@ -5,6 +5,8 @@
  * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
+#include <dt-bindings/bus/ti-sysc.h>
+
 &cbass_wakeup {
 	wkup_conf: syscon@43000000 {
 		bootph-all;
@@ -21,14 +23,33 @@ chipid: chipid@14 {
 		};
 	};
 
-	wkup_uart0: serial@2b300000 {
-		compatible = "ti,am64-uart", "ti,am654-uart";
-		reg = <0x00 0x2b300000 0x00 0x100>;
-		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+	target-module@2b300000 {
+		compatible = "ti,sysc-omap2", "ti,sysc";
+		reg = <0 0x2b300050 0 0x4>,
+		      <0 0x2b300054 0 0x4>,
+		      <0 0x2b300058 0 0x4>;
+		reg-names = "rev", "sysc", "syss";
+		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
+				 SYSC_OMAP2_SOFTRESET |
+				 SYSC_OMAP2_AUTOIDLE)>;
+		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+				<SYSC_IDLE_NO>,
+				<SYSC_IDLE_SMART>,
+				<SYSC_IDLE_SMART_WKUP>;
+		ti,syss-mask = <1>;
 		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 114 0>;
-		clock-names = "fclk";
-		status = "disabled";
+		clock-names = "fck";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0 0x2b300000 0x100000>;
+
+		wkup_uart0: serial@2b300000 {
+			compatible = "ti,am64-uart", "ti,am654-uart";
+			reg = <0 0x100>;
+			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
 	};
 
 	wkup_i2c0: i2c@2b200000 {
-- 
2.42.1

[PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

The devices in the wkup domain are capable of waking up the system from
suspend. We can configure the wkup domain devices in a generic way using
the ti-sysc interconnect target module driver like we have done with the
earlier TI SoCs.

As ti-sysc manages the SYSCONFIG related registers independent of the
child hardware device, the wake-up configuration is also set even if
wkup_uart0 is reserved by sysfw.

The wkup_uart0 device has interconnect target module register mapping like
dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
block in the target module. The power domain and clock affects the whole
interconnect target module.

Note we change the functional clock name to follow the ti-sysc binding
and use "fck" instead of "fclk".

Tested-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Changes since v1:

- Added Tested-by from Dhruva

---
 arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
--- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
@@ -5,6 +5,8 @@
  * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
+#include <dt-bindings/bus/ti-sysc.h>
+
 &cbass_wakeup {
 	wkup_conf: syscon@43000000 {
 		bootph-all;
@@ -21,14 +23,33 @@ chipid: chipid@14 {
 		};
 	};
 
-	wkup_uart0: serial@2b300000 {
-		compatible = "ti,am64-uart", "ti,am654-uart";
-		reg = <0x00 0x2b300000 0x00 0x100>;
-		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+	target-module@2b300000 {
+		compatible = "ti,sysc-omap2", "ti,sysc";
+		reg = <0 0x2b300050 0 0x4>,
+		      <0 0x2b300054 0 0x4>,
+		      <0 0x2b300058 0 0x4>;
+		reg-names = "rev", "sysc", "syss";
+		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
+				 SYSC_OMAP2_SOFTRESET |
+				 SYSC_OMAP2_AUTOIDLE)>;
+		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+				<SYSC_IDLE_NO>,
+				<SYSC_IDLE_SMART>,
+				<SYSC_IDLE_SMART_WKUP>;
+		ti,syss-mask = <1>;
 		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
 		clocks = <&k3_clks 114 0>;
-		clock-names = "fclk";
-		status = "disabled";
+		clock-names = "fck";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0 0x2b300000 0x100000>;
+
+		wkup_uart0: serial@2b300000 {
+			compatible = "ti,am64-uart", "ti,am654-uart";
+			reg = <0 0x100>;
+			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
 	};
 
 	wkup_i2c0: i2c@2b200000 {
-- 
2.42.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> The devices in the wkup domain are capable of waking up the system from
> suspend. We can configure the wkup domain devices in a generic way using
> the ti-sysc interconnect target module driver like we have done with the
> earlier TI SoCs.
>
> As ti-sysc manages the SYSCONFIG related registers independent of the
> child hardware device, the wake-up configuration is also set even if
> wkup_uart0 is reserved by sysfw.
>
> The wkup_uart0 device has interconnect target module register mapping like
> dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
> block in the target module. The power domain and clock affects the whole
> interconnect target module.
>
> Note we change the functional clock name to follow the ti-sysc binding
> and use "fck" instead of "fclk".
>
> Tested-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>
> Changes since v1:
>
> - Added Tested-by from Dhruva
>
> ---
>  arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> --- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> @@ -5,6 +5,8 @@
>   * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
>   */
>  
> +#include <dt-bindings/bus/ti-sysc.h>
> +
>  &cbass_wakeup {
>  	wkup_conf: syscon@43000000 {
>  		bootph-all;
> @@ -21,14 +23,33 @@ chipid: chipid@14 {
>  		};
>  	};
>  
> -	wkup_uart0: serial@2b300000 {
> -		compatible = "ti,am64-uart", "ti,am654-uart";
> -		reg = <0x00 0x2b300000 0x00 0x100>;
> -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +	target-module@2b300000 {
> +		compatible = "ti,sysc-omap2", "ti,sysc";
> +		reg = <0 0x2b300050 0 0x4>,
> +		      <0 0x2b300054 0 0x4>,
> +		      <0 0x2b300058 0 0x4>;
> +		reg-names = "rev", "sysc", "syss";
> +		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> +				 SYSC_OMAP2_SOFTRESET |
> +				 SYSC_OMAP2_AUTOIDLE)>;
> +		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +				<SYSC_IDLE_NO>,
> +				<SYSC_IDLE_SMART>,
> +				<SYSC_IDLE_SMART_WKUP>;
> +		ti,syss-mask = <1>;
>  		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 114 0>;

I'm a little confused why these power-domain and clocks stay here and
are not moved under the wkup_uart0 node... 

> -		clock-names = "fclk";
> -		status = "disabled";
> +		clock-names = "fck";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0 0x2b300000 0x100000>;
> +
> +		wkup_uart0: serial@2b300000 {
> +			compatible = "ti,am64-uart", "ti,am654-uart";
> +			reg = <0 0x100>;
> +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";

...here.

The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
me those should be in the wkup_uart0 node.

Kevin

[1] https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am62x/devices.html

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> The devices in the wkup domain are capable of waking up the system from
> suspend. We can configure the wkup domain devices in a generic way using
> the ti-sysc interconnect target module driver like we have done with the
> earlier TI SoCs.
>
> As ti-sysc manages the SYSCONFIG related registers independent of the
> child hardware device, the wake-up configuration is also set even if
> wkup_uart0 is reserved by sysfw.
>
> The wkup_uart0 device has interconnect target module register mapping like
> dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
> block in the target module. The power domain and clock affects the whole
> interconnect target module.
>
> Note we change the functional clock name to follow the ti-sysc binding
> and use "fck" instead of "fclk".
>
> Tested-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>
> Changes since v1:
>
> - Added Tested-by from Dhruva
>
> ---
>  arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> --- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> @@ -5,6 +5,8 @@
>   * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
>   */
>  
> +#include <dt-bindings/bus/ti-sysc.h>
> +
>  &cbass_wakeup {
>  	wkup_conf: syscon@43000000 {
>  		bootph-all;
> @@ -21,14 +23,33 @@ chipid: chipid@14 {
>  		};
>  	};
>  
> -	wkup_uart0: serial@2b300000 {
> -		compatible = "ti,am64-uart", "ti,am654-uart";
> -		reg = <0x00 0x2b300000 0x00 0x100>;
> -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +	target-module@2b300000 {
> +		compatible = "ti,sysc-omap2", "ti,sysc";
> +		reg = <0 0x2b300050 0 0x4>,
> +		      <0 0x2b300054 0 0x4>,
> +		      <0 0x2b300058 0 0x4>;
> +		reg-names = "rev", "sysc", "syss";
> +		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> +				 SYSC_OMAP2_SOFTRESET |
> +				 SYSC_OMAP2_AUTOIDLE)>;
> +		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +				<SYSC_IDLE_NO>,
> +				<SYSC_IDLE_SMART>,
> +				<SYSC_IDLE_SMART_WKUP>;
> +		ti,syss-mask = <1>;
>  		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 114 0>;

I'm a little confused why these power-domain and clocks stay here and
are not moved under the wkup_uart0 node... 

> -		clock-names = "fclk";
> -		status = "disabled";
> +		clock-names = "fck";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0 0x2b300000 0x100000>;
> +
> +		wkup_uart0: serial@2b300000 {
> +			compatible = "ti,am64-uart", "ti,am654-uart";
> +			reg = <0 0x100>;
> +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";

...here.

The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
me those should be in the wkup_uart0 node.

Kevin

[1] https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am62x/devices.html

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [231205 18:14]:
> I'm a little confused why these power-domain and clocks stay here and
> are not moved under the wkup_uart0 node... 

The resources are also needed by the interconnect target module. It's the
wrapper IP for the child device(s). In this case there's one chip 8250 IP
instance. In some other devices there can be multiple child IP devices
wired to one target module.

> > -		clock-names = "fclk";
> > -		status = "disabled";
> > +		clock-names = "fck";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0 0x2b300000 0x100000>;
> > +
> > +		wkup_uart0: serial@2b300000 {
> > +			compatible = "ti,am64-uart", "ti,am654-uart";
> > +			reg = <0 0x100>;
> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > +			status = "disabled";
> 
> ...here.
> 
> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
> me those should be in the wkup_uart0 node.

Those resources are also needed for the parent target module for revision
detection, quirks, reset, idle register configuration, and to probe the
child devices.

Here the 8250 IP can be set to status = "reserved" when used by the
firmware, and 8250 not touched by Linux. However, the parent interconnect
target module still needs to be configured for idle registers and wake-up
path register bit so the wake-up from deeper suspend states works.

Regards,

Tony

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [231205 18:14]:
> I'm a little confused why these power-domain and clocks stay here and
> are not moved under the wkup_uart0 node... 

The resources are also needed by the interconnect target module. It's the
wrapper IP for the child device(s). In this case there's one chip 8250 IP
instance. In some other devices there can be multiple child IP devices
wired to one target module.

> > -		clock-names = "fclk";
> > -		status = "disabled";
> > +		clock-names = "fck";
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0 0x2b300000 0x100000>;
> > +
> > +		wkup_uart0: serial@2b300000 {
> > +			compatible = "ti,am64-uart", "ti,am654-uart";
> > +			reg = <0 0x100>;
> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > +			status = "disabled";
> 
> ...here.
> 
> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
> me those should be in the wkup_uart0 node.

Those resources are also needed for the parent target module for revision
detection, quirks, reset, idle register configuration, and to probe the
child devices.

Here the 8250 IP can be set to status = "reserved" when used by the
firmware, and 8250 not touched by Linux. However, the parent interconnect
target module still needs to be configured for idle registers and wake-up
path register bit so the wake-up from deeper suspend states works.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
>> I'm a little confused why these power-domain and clocks stay here and
>> are not moved under the wkup_uart0 node... 
>
> The resources are also needed by the interconnect target module. It's the
> wrapper IP for the child device(s). In this case there's one chip 8250 IP
> instance. In some other devices there can be multiple child IP devices
> wired to one target module.
>
>> > -		clock-names = "fclk";
>> > -		status = "disabled";
>> > +		clock-names = "fck";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <1>;
>> > +		ranges = <0 0 0x2b300000 0x100000>;
>> > +
>> > +		wkup_uart0: serial@2b300000 {
>> > +			compatible = "ti,am64-uart", "ti,am654-uart";
>> > +			reg = <0 0x100>;
>> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
>> > +			status = "disabled";
>> 
>> ...here.
>> 
>> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
>> me those should be in the wkup_uart0 node.
>
> Those resources are also needed for the parent target module for revision
> detection, quirks, reset, idle register configuration, and to probe the
> child devices.
>
> Here the 8250 IP can be set to status = "reserved" when used by the
> firmware, and 8250 not touched by Linux. However, the parent interconnect
> target module still needs to be configured for idle registers and wake-up
> path register bit so the wake-up from deeper suspend states works.

OK, makes sense.  Thanks for the clarification.

In that case, shouldn't the same be done for the other wakeup sources
there (e.g. wkup_rtc0) ?

Kevin

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
>> I'm a little confused why these power-domain and clocks stay here and
>> are not moved under the wkup_uart0 node... 
>
> The resources are also needed by the interconnect target module. It's the
> wrapper IP for the child device(s). In this case there's one chip 8250 IP
> instance. In some other devices there can be multiple child IP devices
> wired to one target module.
>
>> > -		clock-names = "fclk";
>> > -		status = "disabled";
>> > +		clock-names = "fck";
>> > +		#address-cells = <1>;
>> > +		#size-cells = <1>;
>> > +		ranges = <0 0 0x2b300000 0x100000>;
>> > +
>> > +		wkup_uart0: serial@2b300000 {
>> > +			compatible = "ti,am64-uart", "ti,am654-uart";
>> > +			reg = <0 0x100>;
>> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
>> > +			status = "disabled";
>> 
>> ...here.
>> 
>> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
>> me those should be in the wkup_uart0 node.
>
> Those resources are also needed for the parent target module for revision
> detection, quirks, reset, idle register configuration, and to probe the
> child devices.
>
> Here the 8250 IP can be set to status = "reserved" when used by the
> firmware, and 8250 not touched by Linux. However, the parent interconnect
> target module still needs to be configured for idle registers and wake-up
> path register bit so the wake-up from deeper suspend states works.

OK, makes sense.  Thanks for the clarification.

In that case, shouldn't the same be done for the other wakeup sources
there (e.g. wkup_rtc0) ?

Kevin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [231208 17:13]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
> >> I'm a little confused why these power-domain and clocks stay here and
> >> are not moved under the wkup_uart0 node... 
> >
> > The resources are also needed by the interconnect target module. It's the
> > wrapper IP for the child device(s). In this case there's one chip 8250 IP
> > instance. In some other devices there can be multiple child IP devices
> > wired to one target module.
> >
> >> > -		clock-names = "fclk";
> >> > -		status = "disabled";
> >> > +		clock-names = "fck";
> >> > +		#address-cells = <1>;
> >> > +		#size-cells = <1>;
> >> > +		ranges = <0 0 0x2b300000 0x100000>;
> >> > +
> >> > +		wkup_uart0: serial@2b300000 {
> >> > +			compatible = "ti,am64-uart", "ti,am654-uart";
> >> > +			reg = <0 0x100>;
> >> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> >> > +			status = "disabled";
> >> 
> >> ...here.
> >> 
> >> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
> >> me those should be in the wkup_uart0 node.
> >
> > Those resources are also needed for the parent target module for revision
> > detection, quirks, reset, idle register configuration, and to probe the
> > child devices.
> >
> > Here the 8250 IP can be set to status = "reserved" when used by the
> > firmware, and 8250 not touched by Linux. However, the parent interconnect
> > target module still needs to be configured for idle registers and wake-up
> > path register bit so the wake-up from deeper suspend states works.
> 
> OK, makes sense.  Thanks for the clarification.

One more thing to clarify, there's nothing stopping also mapping the needed
resources for the child IP too if needed by the driver or the dts binding.
The calls for resources by the 8250 driver just won't do anything as the
resources are already enabled by the parent.

> In that case, shouldn't the same be done for the other wakeup sources
> there (e.g. wkup_rtc0) ?

Yes it should be done for devices with the wake-up path wired like
wkup_rtc0.

Regards,

Tony

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [231208 17:13]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
> >> I'm a little confused why these power-domain and clocks stay here and
> >> are not moved under the wkup_uart0 node... 
> >
> > The resources are also needed by the interconnect target module. It's the
> > wrapper IP for the child device(s). In this case there's one chip 8250 IP
> > instance. In some other devices there can be multiple child IP devices
> > wired to one target module.
> >
> >> > -		clock-names = "fclk";
> >> > -		status = "disabled";
> >> > +		clock-names = "fck";
> >> > +		#address-cells = <1>;
> >> > +		#size-cells = <1>;
> >> > +		ranges = <0 0 0x2b300000 0x100000>;
> >> > +
> >> > +		wkup_uart0: serial@2b300000 {
> >> > +			compatible = "ti,am64-uart", "ti,am654-uart";
> >> > +			reg = <0 0x100>;
> >> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> >> > +			status = "disabled";
> >> 
> >> ...here.
> >> 
> >> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
> >> me those should be in the wkup_uart0 node.
> >
> > Those resources are also needed for the parent target module for revision
> > detection, quirks, reset, idle register configuration, and to probe the
> > child devices.
> >
> > Here the 8250 IP can be set to status = "reserved" when used by the
> > firmware, and 8250 not touched by Linux. However, the parent interconnect
> > target module still needs to be configured for idle registers and wake-up
> > path register bit so the wake-up from deeper suspend states works.
> 
> OK, makes sense.  Thanks for the clarification.

One more thing to clarify, there's nothing stopping also mapping the needed
resources for the child IP too if needed by the driver or the dts binding.
The calls for resources by the 8250 driver just won't do anything as the
resources are already enabled by the parent.

> In that case, shouldn't the same be done for the other wakeup sources
> there (e.g. wkup_rtc0) ?

Yes it should be done for devices with the wake-up path wired like
wkup_rtc0.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [231208 17:13]:
>> Tony Lindgren <tony@atomide.com> writes:
>> 
>> > * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
>> >> I'm a little confused why these power-domain and clocks stay here and
>> >> are not moved under the wkup_uart0 node... 
>> >
>> > The resources are also needed by the interconnect target module. It's the
>> > wrapper IP for the child device(s). In this case there's one chip 8250 IP
>> > instance. In some other devices there can be multiple child IP devices
>> > wired to one target module.
>> >
>> >> > -		clock-names = "fclk";
>> >> > -		status = "disabled";
>> >> > +		clock-names = "fck";
>> >> > +		#address-cells = <1>;
>> >> > +		#size-cells = <1>;
>> >> > +		ranges = <0 0 0x2b300000 0x100000>;
>> >> > +
>> >> > +		wkup_uart0: serial@2b300000 {
>> >> > +			compatible = "ti,am64-uart", "ti,am654-uart";
>> >> > +			reg = <0 0x100>;
>> >> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
>> >> > +			status = "disabled";
>> >> 
>> >> ...here.
>> >> 
>> >> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
>> >> me those should be in the wkup_uart0 node.
>> >
>> > Those resources are also needed for the parent target module for revision
>> > detection, quirks, reset, idle register configuration, and to probe the
>> > child devices.
>> >
>> > Here the 8250 IP can be set to status = "reserved" when used by the
>> > firmware, and 8250 not touched by Linux. However, the parent interconnect
>> > target module still needs to be configured for idle registers and wake-up
>> > path register bit so the wake-up from deeper suspend states works.
>> 
>> OK, makes sense.  Thanks for the clarification.
>
> One more thing to clarify, there's nothing stopping also mapping the needed
> resources for the child IP too if needed by the driver or the dts binding.
> The calls for resources by the 8250 driver just won't do anything as the
> resources are already enabled by the parent.

OK, thanks.

>> In that case, shouldn't the same be done for the other wakeup sources
>> there (e.g. wkup_rtc0) ?
>
> Yes it should be done for devices with the wake-up path wired like
> wkup_rtc0.

OK, in that case, this all makes sense to me.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [231208 17:13]:
>> Tony Lindgren <tony@atomide.com> writes:
>> 
>> > * Kevin Hilman <khilman@kernel.org> [231205 18:14]:
>> >> I'm a little confused why these power-domain and clocks stay here and
>> >> are not moved under the wkup_uart0 node... 
>> >
>> > The resources are also needed by the interconnect target module. It's the
>> > wrapper IP for the child device(s). In this case there's one chip 8250 IP
>> > instance. In some other devices there can be multiple child IP devices
>> > wired to one target module.
>> >
>> >> > -		clock-names = "fclk";
>> >> > -		status = "disabled";
>> >> > +		clock-names = "fck";
>> >> > +		#address-cells = <1>;
>> >> > +		#size-cells = <1>;
>> >> > +		ranges = <0 0 0x2b300000 0x100000>;
>> >> > +
>> >> > +		wkup_uart0: serial@2b300000 {
>> >> > +			compatible = "ti,am64-uart", "ti,am654-uart";
>> >> > +			reg = <0 0x100>;
>> >> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
>> >> > +			status = "disabled";
>> >> 
>> >> ...here.
>> >> 
>> >> The SCI device ID 114 is specifically for wkup_uart0[1], so it seems to
>> >> me those should be in the wkup_uart0 node.
>> >
>> > Those resources are also needed for the parent target module for revision
>> > detection, quirks, reset, idle register configuration, and to probe the
>> > child devices.
>> >
>> > Here the 8250 IP can be set to status = "reserved" when used by the
>> > firmware, and 8250 not touched by Linux. However, the parent interconnect
>> > target module still needs to be configured for idle registers and wake-up
>> > path register bit so the wake-up from deeper suspend states works.
>> 
>> OK, makes sense.  Thanks for the clarification.
>
> One more thing to clarify, there's nothing stopping also mapping the needed
> resources for the child IP too if needed by the driver or the dts binding.
> The calls for resources by the 8250 driver just won't do anything as the
> resources are already enabled by the parent.

OK, thanks.

>> In that case, shouldn't the same be done for the other wakeup sources
>> there (e.g. wkup_rtc0) ?
>
> Yes it should be done for devices with the wake-up path wired like
> wkup_rtc0.

OK, in that case, this all makes sense to me.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>

Kevin

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Nishanth Menon]

On 09:32-20231114, Tony Lindgren wrote:
> The devices in the wkup domain are capable of waking up the system from
> suspend. We can configure the wkup domain devices in a generic way using
> the ti-sysc interconnect target module driver like we have done with the
> earlier TI SoCs.
> 
> As ti-sysc manages the SYSCONFIG related registers independent of the
> child hardware device, the wake-up configuration is also set even if
> wkup_uart0 is reserved by sysfw.
> 
> The wkup_uart0 device has interconnect target module register mapping like
> dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
> block in the target module. The power domain and clock affects the whole
> interconnect target module.
> 
> Note we change the functional clock name to follow the ti-sysc binding
> and use "fck" instead of "fclk".
> 
> Tested-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> Changes since v1:
> 
> - Added Tested-by from Dhruva
> 
> ---
>  arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> --- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> @@ -5,6 +5,8 @@
>   * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
>   */
>  
> +#include <dt-bindings/bus/ti-sysc.h>
> +
>  &cbass_wakeup {
>  	wkup_conf: syscon@43000000 {
>  		bootph-all;
> @@ -21,14 +23,33 @@ chipid: chipid@14 {
>  		};
>  	};
>  
> -	wkup_uart0: serial@2b300000 {
> -		compatible = "ti,am64-uart", "ti,am654-uart";
> -		reg = <0x00 0x2b300000 0x00 0x100>;
> -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +	target-module@2b300000 {

should be  target-module@2b300050 to match up with reg?

> +		compatible = "ti,sysc-omap2", "ti,sysc";
> +		reg = <0 0x2b300050 0 0x4>,
> +		      <0 0x2b300054 0 0x4>,
> +		      <0 0x2b300058 0 0x4>;
> +		reg-names = "rev", "sysc", "syss";
> +		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> +				 SYSC_OMAP2_SOFTRESET |
> +				 SYSC_OMAP2_AUTOIDLE)>;
> +		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +				<SYSC_IDLE_NO>,
> +				<SYSC_IDLE_SMART>,
> +				<SYSC_IDLE_SMART_WKUP>;
> +		ti,syss-mask = <1>;
>  		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 114 0>;
> -		clock-names = "fclk";
> -		status = "disabled";
> +		clock-names = "fck";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0 0x2b300000 0x100000>;
> +
> +		wkup_uart0: serial@2b300000 {

serial@0  to match up with reg?

> +			compatible = "ti,am64-uart", "ti,am654-uart";
> +			reg = <0 0x100>;
> +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
>  	};
>  
>  	wkup_i2c0: i2c@2b200000 {
> -- 
> 2.42.1

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Nishanth Menon]

On 09:32-20231114, Tony Lindgren wrote:
> The devices in the wkup domain are capable of waking up the system from
> suspend. We can configure the wkup domain devices in a generic way using
> the ti-sysc interconnect target module driver like we have done with the
> earlier TI SoCs.
> 
> As ti-sysc manages the SYSCONFIG related registers independent of the
> child hardware device, the wake-up configuration is also set even if
> wkup_uart0 is reserved by sysfw.
> 
> The wkup_uart0 device has interconnect target module register mapping like
> dra7 wkup uart. There is a 1 MB interconnect target range with one uart IP
> block in the target module. The power domain and clock affects the whole
> interconnect target module.
> 
> Note we change the functional clock name to follow the ti-sysc binding
> and use "fck" instead of "fclk".
> 
> Tested-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> Changes since v1:
> 
> - Added Tested-by from Dhruva
> 
> ---
>  arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi | 33 ++++++++++++++++++----
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> --- a/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-wakeup.dtsi
> @@ -5,6 +5,8 @@
>   * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com/
>   */
>  
> +#include <dt-bindings/bus/ti-sysc.h>
> +
>  &cbass_wakeup {
>  	wkup_conf: syscon@43000000 {
>  		bootph-all;
> @@ -21,14 +23,33 @@ chipid: chipid@14 {
>  		};
>  	};
>  
> -	wkup_uart0: serial@2b300000 {
> -		compatible = "ti,am64-uart", "ti,am654-uart";
> -		reg = <0x00 0x2b300000 0x00 0x100>;
> -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +	target-module@2b300000 {

should be  target-module@2b300050 to match up with reg?

> +		compatible = "ti,sysc-omap2", "ti,sysc";
> +		reg = <0 0x2b300050 0 0x4>,
> +		      <0 0x2b300054 0 0x4>,
> +		      <0 0x2b300058 0 0x4>;
> +		reg-names = "rev", "sysc", "syss";
> +		ti,sysc-mask = <(SYSC_OMAP2_ENAWAKEUP |
> +				 SYSC_OMAP2_SOFTRESET |
> +				 SYSC_OMAP2_AUTOIDLE)>;
> +		ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +				<SYSC_IDLE_NO>,
> +				<SYSC_IDLE_SMART>,
> +				<SYSC_IDLE_SMART_WKUP>;
> +		ti,syss-mask = <1>;
>  		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
>  		clocks = <&k3_clks 114 0>;
> -		clock-names = "fclk";
> -		status = "disabled";
> +		clock-names = "fck";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0 0x2b300000 0x100000>;
> +
> +		wkup_uart0: serial@2b300000 {

serial@0  to match up with reg?

> +			compatible = "ti,am64-uart", "ti,am654-uart";
> +			reg = <0 0x100>;
> +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
>  	};
>  
>  	wkup_i2c0: i2c@2b200000 {
> -- 
> 2.42.1

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [231215 16:00]:
> On 09:32-20231114, Tony Lindgren wrote:
> > -	wkup_uart0: serial@2b300000 {
> > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > +	target-module@2b300000 {
> 
> should be  target-module@2b300050 to match up with reg?

It's best to use the target-module IO range here, not the first reg.
The first reg may be tossed anywhere in the target module address
space depending on the device.

Ideally of course there would be just a standardized range of target
module related registers at the end of the IO space..

> > +		wkup_uart0: serial@2b300000 {
> 
> serial@0  to match up with reg?

Yes thanks for catching this. The 8250 IP is at the beginning of the
target module IO space. Will post an updated patch.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [231215 16:00]:
> On 09:32-20231114, Tony Lindgren wrote:
> > -	wkup_uart0: serial@2b300000 {
> > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > +	target-module@2b300000 {
> 
> should be  target-module@2b300050 to match up with reg?

It's best to use the target-module IO range here, not the first reg.
The first reg may be tossed anywhere in the target module address
space depending on the device.

Ideally of course there would be just a standardized range of target
module related registers at the end of the IO space..

> > +		wkup_uart0: serial@2b300000 {
> 
> serial@0  to match up with reg?

Yes thanks for catching this. The 8250 IP is at the beginning of the
target module IO space. Will post an updated patch.

Regards,

Tony

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Nishanth Menon]

On 09:28-20231218, Tony Lindgren wrote:
> * Nishanth Menon <nm@ti.com> [231215 16:00]:
> > On 09:32-20231114, Tony Lindgren wrote:
> > > -	wkup_uart0: serial@2b300000 {
> > > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > > +	target-module@2b300000 {
> > 
> > should be  target-module@2b300050 to match up with reg?
> 
> It's best to use the target-module IO range here, not the first reg.
> The first reg may be tossed anywhere in the target module address
> space depending on the device.
> 
> Ideally of course there would be just a standardized range of target
> module related registers at the end of the IO space..

Sorry Tony, but the node address must matchup with the reg description
for the node. I'd rather not sit and argue about this with automated
tools and deal with trivial patches - so either tools like dtc need to
ignore this or lets just fix the node address.
 
> > > +		wkup_uart0: serial@2b300000 {
> > 
> > serial@0  to match up with reg?
> 
> Yes thanks for catching this. The 8250 IP is at the beginning of the
> target module IO space. Will post an updated patch.
> 
> Regards,
> 
> Tony

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Nishanth Menon]

On 09:28-20231218, Tony Lindgren wrote:
> * Nishanth Menon <nm@ti.com> [231215 16:00]:
> > On 09:32-20231114, Tony Lindgren wrote:
> > > -	wkup_uart0: serial@2b300000 {
> > > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > > +	target-module@2b300000 {
> > 
> > should be  target-module@2b300050 to match up with reg?
> 
> It's best to use the target-module IO range here, not the first reg.
> The first reg may be tossed anywhere in the target module address
> space depending on the device.
> 
> Ideally of course there would be just a standardized range of target
> module related registers at the end of the IO space..

Sorry Tony, but the node address must matchup with the reg description
for the node. I'd rather not sit and argue about this with automated
tools and deal with trivial patches - so either tools like dtc need to
ignore this or lets just fix the node address.
 
> > > +		wkup_uart0: serial@2b300000 {
> > 
> > serial@0  to match up with reg?
> 
> Yes thanks for catching this. The 8250 IP is at the beginning of the
> target module IO space. Will post an updated patch.
> 
> Regards,
> 
> Tony

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [231218 16:19]:
> On 09:28-20231218, Tony Lindgren wrote:
> > * Nishanth Menon <nm@ti.com> [231215 16:00]:
> > > On 09:32-20231114, Tony Lindgren wrote:
> > > > -	wkup_uart0: serial@2b300000 {
> > > > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > > > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > > > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > > > +	target-module@2b300000 {
> > > 
> > > should be  target-module@2b300050 to match up with reg?
> > 
> > It's best to use the target-module IO range here, not the first reg.
> > The first reg may be tossed anywhere in the target module address
> > space depending on the device.
> > 
> > Ideally of course there would be just a standardized range of target
> > module related registers at the end of the IO space..
> 
> Sorry Tony, but the node address must matchup with the reg description
> for the node. I'd rather not sit and argue about this with automated
> tools and deal with trivial patches - so either tools like dtc need to
> ignore this or lets just fix the node address.

Oh sorry I did not mean to add a make W=1 dtbs warning. Yeah let's change
the node name to match the first reg to avoid the warning.

It should be also add some better checks for node names with ranges but
with just a few control registers.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [231218 16:19]:
> On 09:28-20231218, Tony Lindgren wrote:
> > * Nishanth Menon <nm@ti.com> [231215 16:00]:
> > > On 09:32-20231114, Tony Lindgren wrote:
> > > > -	wkup_uart0: serial@2b300000 {
> > > > -		compatible = "ti,am64-uart", "ti,am654-uart";
> > > > -		reg = <0x00 0x2b300000 0x00 0x100>;
> > > > -		interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > > > +	target-module@2b300000 {
> > > 
> > > should be  target-module@2b300050 to match up with reg?
> > 
> > It's best to use the target-module IO range here, not the first reg.
> > The first reg may be tossed anywhere in the target module address
> > space depending on the device.
> > 
> > Ideally of course there would be just a standardized range of target
> > module related registers at the end of the IO space..
> 
> Sorry Tony, but the node address must matchup with the reg description
> for the node. I'd rather not sit and argue about this with automated
> tools and deal with trivial patches - so either tools like dtc need to
> ignore this or lets just fix the node address.

Oh sorry I did not mean to add a make W=1 dtbs warning. Yeah let's change
the node name to match the first reg to avoid the warning.

It should be also add some better checks for node names with ranges but
with just a few control registers.

Regards,

Tony