| Message ID | 20231114073209.40756-1-tony@atomide.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/1] arm64: dts: ti: k3-am62-wakeup: Configure ti-sysc for wkup_uart0 | expand |
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
* 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
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
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
* 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
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
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 {