Despite CM_IDLEST1_CORE and CM_FCLKEN1_CORE behaving normal, disabling SPI leads to messages like: Powerdomain (core_pwrdm) didn't enter target state 0 and according to /sys/kernel/debug/pm_debug/count off state is not entered. That was not connected to SPI during the discussion of disabling SPI. See: https://lore.kernel.org/linux-omap/20230122100852.32ae082c@aktux/
Fix excess DMA channel usage by disabling DMA only instead of disabling the SPI modules, so powermanagement can da all its work.
Fixes: a622310f7f01 ("ARM: dts: gta04: fix excess dma channel usage") CC: stable@vger.kernel.org Signed-off-by: Andreas Kemnade andreas@kemnade.info --- arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi index 3661340009e7a..11f8af34498b1 100644 --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi @@ -612,19 +612,23 @@ &i2c3 { };
&mcspi1 { - status = "disabled"; + /delete-property/ dmas; + /delete-property/ dma-names; };
&mcspi2 { - status = "disabled"; + /delete-property/ dmas; + /delete-property/ dma-names; };
&mcspi3 { - status = "disabled"; + /delete-property/ dmas; + /delete-property/ dma-names; };
&mcspi4 { - status = "disabled"; + /delete-property/ dmas; + /delete-property/ dma-names; };
&usb_otg_hs {
Hi,
On 08/11/2024 00:51, Andreas Kemnade wrote:
Despite CM_IDLEST1_CORE and CM_FCLKEN1_CORE behaving normal, disabling SPI leads to messages like: Powerdomain (core_pwrdm) didn't enter target state 0 and according to /sys/kernel/debug/pm_debug/count off state is not entered. That was not connected to SPI during the discussion of disabling SPI. See: https://lore.kernel.org/linux-omap/20230122100852.32ae082c@aktux/
Fix excess DMA channel usage by disabling DMA only instead of disabling the SPI modules, so powermanagement can da all its work.
s/powermanagement/power management s/da/do
Fixes: a622310f7f01 ("ARM: dts: gta04: fix excess dma channel usage") CC: stable@vger.kernel.org Signed-off-by: Andreas Kemnade andreas@kemnade.info
arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi index 3661340009e7a..11f8af34498b1 100644 --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi @@ -612,19 +612,23 @@ &i2c3 { }; &mcspi1 {
- status = "disabled";
But according to commit a622310f7f01 ("ARM: dts: gta04: fix excess dma channel usage"), these mcspi modules are not used. So it doesn't make sense to enable them even if it seems to solve the power management issue?
Does bootloader leave the mcspi modules in a unwanted state? Would it make sense for the bus driver to explicitly turn off all modules?
- /delete-property/ dmas;
- /delete-property/ dma-names;
}; &mcspi2 {
- status = "disabled";
- /delete-property/ dmas;
- /delete-property/ dma-names;
}; &mcspi3 {
- status = "disabled";
- /delete-property/ dmas;
- /delete-property/ dma-names;
}; &mcspi4 {
- status = "disabled";
- /delete-property/ dmas;
- /delete-property/ dma-names;
}; &usb_otg_hs {
Am Fri, 8 Nov 2024 14:42:14 +0200 schrieb Roger Quadros rogerq@kernel.org:
diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi index 3661340009e7a..11f8af34498b1 100644 --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi @@ -612,19 +612,23 @@ &i2c3 { }; &mcspi1 {
- status = "disabled";
But according to commit a622310f7f01 ("ARM: dts: gta04: fix excess dma channel usage"), these mcspi modules are not used. So it doesn't make sense to enable them even if it seems to solve the power management issue?
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
I can try a ti-sysc based fix in parallel.
Does bootloader leave the mcspi modules in a unwanted state?
Or at least something related to them. As said, for the blamed patch I checked only for CM_IDLEST1_CORE and CM_FCLKEN1_CORE.
Would it make sense for the bus driver to explicitly turn off all modules?
Hmm, not very clear what you mean. AFAIK everything below ti-sysc gets turned off if a disable is in the child node. Explicitly disabling such stuff in the dtsi and enable it in the board dts sound sane to me at first glance. I think it is a common pattern. The question is whether that causes confusion with not ti-sysc stuff. Well, having status=okay everywhere in the dts should not harm. But as said for a regression fix some overhaul affecting every device is out of scope.
Regards, Andreas
On 08/11/2024 19:41, Andreas Kemnade wrote:
Am Fri, 8 Nov 2024 14:42:14 +0200 schrieb Roger Quadros rogerq@kernel.org:
diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi index 3661340009e7a..11f8af34498b1 100644 --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi @@ -612,19 +612,23 @@ &i2c3 { }; &mcspi1 {
- status = "disabled";
But according to commit a622310f7f01 ("ARM: dts: gta04: fix excess dma channel usage"), these mcspi modules are not used. So it doesn't make sense to enable them even if it seems to solve the power management issue?
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
I can try a ti-sysc based fix in parallel.
Does bootloader leave the mcspi modules in a unwanted state?
Or at least something related to them. As said, for the blamed patch I checked only for CM_IDLEST1_CORE and CM_FCLKEN1_CORE.
Would it make sense for the bus driver to explicitly turn off all modules?
Hmm, not very clear what you mean. AFAIK everything below ti-sysc gets turned off if a disable is in the child node. Explicitly disabling such stuff in the dtsi and enable it in the board dts sound sane to me at first glance. I think it is a common pattern. The question is whether that causes confusion with not ti-sysc stuff. Well, having status=okay everywhere in the dts should not harm. But as said for a regression fix some overhaul affecting every device is out of scope.
McSPI modules have Revision, Syconfig and Sysstatus registers. Is it because we are missing the ti-sysc representation for it that the module power is not being correctly handled in Linux if module is kept disabled?
* Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
Regards,
Tony
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
Regards, Andreas
Am Mon, 11 Nov 2024 19:31:17 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
well, I tried a bit further: I build the omap spi driver as module. and booted With mcspi3 not disabled and no module autoload.
without module loaded: pm bad, same as with mcspi3 disabled with module loaded: core pm ok with module loaded and unloaded: core pm ok.
so at least a trace.
Regards, Andreas
Am Mon, 11 Nov 2024 23:46:04 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 19:31:17 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
well, I tried a bit further: I build the omap spi driver as module. and booted With mcspi3 not disabled and no module autoload.
without module loaded: pm bad, same as with mcspi3 disabled with module loaded: core pm ok with module loaded and unloaded: core pm ok.
so at least a trace.
ok, I am a bit further. mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master. Having the pins muxed as mode 7 also helps or selecting a pulldown for cs. (cs is active high per default!) switching to pullup does not harm once the spi module is off, but having active cs seems to prevent idling despite CM_IDLEST1_CORE not showing it.
History: u-boot muxes McSPI3, because it can be available on an optionally fitted pin header. But there is no user known (would need a dtb overlay anyways). So I will rather mux to mode 7.
Regards, Andreas
* Andreas Kemnade andreas@kemnade.info [241116 20:27]:
mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master.
OK interesting. Maybe set up a quirk function for it in ti-sysc.c. That way the mcspi will get idled also when set to status disabled and no mcspi driver is loaded.
Regards,
Tony
Hi Tony,
Am Sun, 17 Nov 2024 13:19:03 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241116 20:27]:
mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master.
OK interesting. Maybe set up a quirk function for it in ti-sysc.c. That way the mcspi will get idled also when set to status disabled and no mcspi driver is loaded.
First of all I think if status = "disabled" then no pins should be muxed to mcspi. That prevents all mess.
So the only case left is spi enabled but no driver and CS input is active. If we configure things as master via a quirk if the setup is slave then switch something to output which should not be. We would have some output againt output situation at least for a moment.
Maybe pinmuxing stuff in ti-sysc? Hmm, I am really unsure if it is worth it.
Regarding the GTA04 case:
mcspi1,2 and 4 are not muxed, so that issue occurs only with mcspi3, and therefore the most simple solution is to mux it to mode7 as sent in the v2 which should also be suitable for -stable.
Regards, Andreas
On 16/11/2024 22:27, Andreas Kemnade wrote:
Am Mon, 11 Nov 2024 23:46:04 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 19:31:17 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
well, I tried a bit further: I build the omap spi driver as module. and booted With mcspi3 not disabled and no module autoload.
without module loaded: pm bad, same as with mcspi3 disabled with module loaded: core pm ok with module loaded and unloaded: core pm ok.
so at least a trace.
ok, I am a bit further. mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master. Having the pins muxed as mode 7 also helps or selecting a pulldown for cs. (cs is active high per default!) switching to pullup does not harm once the spi module is off, but having active cs seems to prevent idling despite CM_IDLEST1_CORE not showing it.
History: u-boot muxes McSPI3, because it can be available on an optionally fitted pin header. But there is no user known (would need a dtb overlay anyways). So I will rather mux to mode 7.
I'm sorry I didn't fully understand the problem.
So, u-boot configures pinmux for McSPI3 and enables McSPI3 as well but fails to disable it properly? And because McSPI3 is in slave mode and CS is active it fails to transition to idle in Linux?
So isn't this a u-boot issue?
Am Mon, 18 Nov 2024 15:08:48 +0200 schrieb Roger Quadros rogerq@kernel.org:
On 16/11/2024 22:27, Andreas Kemnade wrote:
Am Mon, 11 Nov 2024 23:46:04 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 19:31:17 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
They are not used, if they are just disabled, kernel does not touch them, so if it is there, the kernel can handle pm. At least as long as it is not under ti,sysc.
There are probably cleaner solutions for this, but for a CC: stable I would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
well, I tried a bit further: I build the omap spi driver as module. and booted With mcspi3 not disabled and no module autoload.
without module loaded: pm bad, same as with mcspi3 disabled with module loaded: core pm ok with module loaded and unloaded: core pm ok.
so at least a trace.
ok, I am a bit further. mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master. Having the pins muxed as mode 7 also helps or selecting a pulldown for cs. (cs is active high per default!) switching to pullup does not harm once the spi module is off, but having active cs seems to prevent idling despite CM_IDLEST1_CORE not showing it.
History: u-boot muxes McSPI3, because it can be available on an optionally fitted pin header. But there is no user known (would need a dtb overlay anyways). So I will rather mux to mode 7.
I'm sorry I didn't fully understand the problem.
So, u-boot configures pinmux for McSPI3 and enables McSPI3 as well but fails to disable it properly?
At least it sets Pinmux.
And because McSPI3 is in slave mode and CS is active it fails to transition to idle in Linux?
yes, slave mode is default.
So isn't this a u-boot issue?
Just telling u-boot to not mux McSPI3 helps. So, yes u-boot should not set it. But. I have no clear idea how bitrot the u-boot update process is. I would prefer setting the pinmux also in kernel back to the default.
Regards, Andreas
On 28/11/2024 00:58, Andreas Kemnade wrote:
Am Mon, 18 Nov 2024 15:08:48 +0200 schrieb Roger Quadros rogerq@kernel.org:
On 16/11/2024 22:27, Andreas Kemnade wrote:
Am Mon, 11 Nov 2024 23:46:04 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 19:31:17 +0100 schrieb Andreas Kemnade andreas@kemnade.info:
Am Mon, 11 Nov 2024 17:09:53 +0200 schrieb Tony Lindgren tony@atomide.com:
- Andreas Kemnade andreas@kemnade.info [241108 17:41]:
> They are not used, if they are just disabled, kernel does not touch > them, so if it is there, the kernel can handle > pm. At least as long as it is not under ti,sysc. > > There are probably cleaner solutions for this, but for a CC: stable I > would prefer something less invasive.
For unused devices, it's best to configure things to use ti-sysc, and then set status disabled (or reserved) for the child devices only. This way the parent interconnect target module is PM runtime managed by Linux, and it's power domain gets properly idled for the unused devices too.
Hmm, we also have omap_hwmod_setup_all() which is still called if without device nodes being available.
Converting mcspi to ti-sysc is more than 100 lines. So it does not qualify for stable.
> I can try a ti-sysc based fix in parallel.
Yeah that should be trivial hopefully :)
I played around, got pm issues too, tried to force-enable things (via power/control), watched CM_IDLEST1_CORE and CM_FCLKEN1_CORE, they behave. Bits are set or reset.
but not CM_IDLEST_CKGEN, it is 0x209 instead of 0x1.
I test from initramfs, so no mmc activity involved
removing status = "disabled" from mcspi3 solves things. With and without ti-sysc conversion. removing status = "disabled" from mcspi4 seems not to help.
That all cannot be... I will retry tomorrow.
well, I tried a bit further: I build the omap spi driver as module. and booted With mcspi3 not disabled and no module autoload.
without module loaded: pm bad, same as with mcspi3 disabled with module loaded: core pm ok with module loaded and unloaded: core pm ok.
so at least a trace.
ok, I am a bit further. mcspi is per default in slave mode, setting it to master solves issues. And that happens when the driver is probed because its default is master. Having the pins muxed as mode 7 also helps or selecting a pulldown for cs. (cs is active high per default!) switching to pullup does not harm once the spi module is off, but having active cs seems to prevent idling despite CM_IDLEST1_CORE not showing it.
History: u-boot muxes McSPI3, because it can be available on an optionally fitted pin header. But there is no user known (would need a dtb overlay anyways). So I will rather mux to mode 7.
I'm sorry I didn't fully understand the problem.
So, u-boot configures pinmux for McSPI3 and enables McSPI3 as well but fails to disable it properly?
At least it sets Pinmux.
And because McSPI3 is in slave mode and CS is active it fails to transition to idle in Linux?
yes, slave mode is default.
So isn't this a u-boot issue?
Just telling u-boot to not mux McSPI3 helps. So, yes u-boot should not set it. But. I have no clear idea how bitrot the u-boot update process is. I would prefer setting the pinmux also in kernel back to the default.
That would be fine.
linux-stable-mirror@lists.linaro.org
-
Andreas Kemnade -
Roger Quadros -
Tony Lindgren