| Message ID | 20230113211151.2314874-1-andreas@kemnade.info |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: dts: gta04: fix excess dma channel usage | expand |
On Fri, Jan 13, 2023 at 3:12 PM Andreas Kemnade <andreas@kemnade.info> wrote: > > From: "H. Nikolaus Schaller" <hns@goldelico.com> > > OMAP processors support 32 channels but there is no check or > inspect this except booting a device and looking at dmesg reports > of not available channels. > > Recently some more subsystems with DMA (aes1+2) were added filling > the list of dma channels beyond the limit of 32 (even if other > parameters indicate 96 or 128 channels). This leads to random > subsystem failures i(e.g. mcbsp for audio) after boot or boot > messages that DMA can not be initialized. > > Another symptom is that > > /sys/kernel/debug/dmaengine/summary > > has 32 entries and does not show all required channels. > > Fix by disabling unused (on the GTA04 hardware) mcspi1...4. > Each SPI channel allocates 4 DMA channels rapidly filling > the available ones. > > Disabling unused SPI modules on the OMAP3 SoC may also save > some energy (has not been checked). Tony, Would it make sense to make this default in the omap3.dtsi file and enable them in the individual boards that need it?
Hi, * Adam Ford <aford173@gmail.com> [230116 14:16]: > Would it make sense to make this default in the omap3.dtsi file and > enable them in the individual boards that need it? In general disabling the unused devices by default for omaps will break the power management. The disabled devices are completely ignored by the kernel, and the devices are left to whatever the bootloader state might be. For SoCs using firmware to manage devices it's a bit different story however. The firmware can still idle disabled devices based on a late_initcall for example, even if the kernel knows nothing about the disabled devices. Regards, Tony
* Andreas Kemnade <andreas@kemnade.info> [230116 16:39]: > On Mon, 16 Jan 2023 16:51:57 +0200 > Tony Lindgren <tony@atomide.com> wrote: > > > Hi, > > > > * Adam Ford <aford173@gmail.com> [230116 14:16]: > > > Would it make sense to make this default in the omap3.dtsi file and > > > enable them in the individual boards that need it? > > > > In general disabling the unused devices by default for omaps will break > > the power management. The disabled devices are completely ignored by the > > kernel, and the devices are left to whatever the bootloader state might > > be. > > > hmm, shouldn't ti-sysc keep things disabled in most cases? It is still a bit > known because there is no status = "disabled" in the target-module@xxx node. Oh right, if the child device is tagged disabled (instead of the the parent ti-sysc tagged disabled) the module should get idled just fine as long as the module related quirks are implemented for ti-sysc.c. But still, I'd rather not start tagging devices disabled by default and then re-enabling everywhere since we never needed it before. It just adds a lot of pointless status tinkering, see commit 12afc0cf8121 ("ARM: dts: Drop pointless status changing for am3 musb"). So considering things, IMO it's best to set only the child device with status disabled, and set it at the board specific dts file in this case. Also note that the dma channels could be freed with /delete-property/ at the board specific dts file even for devices that are usable if not really needed. Regards, Tony
On Mon, Jan 16, 2023 at 10:56 AM Tony Lindgren <tony@atomide.com> wrote: > > * Andreas Kemnade <andreas@kemnade.info> [230116 16:39]: > > On Mon, 16 Jan 2023 16:51:57 +0200 > > Tony Lindgren <tony@atomide.com> wrote: > > > > > Hi, > > > > > > * Adam Ford <aford173@gmail.com> [230116 14:16]: > > > > Would it make sense to make this default in the omap3.dtsi file and > > > > enable them in the individual boards that need it? > > > > > > In general disabling the unused devices by default for omaps will break > > > the power management. The disabled devices are completely ignored by the > > > kernel, and the devices are left to whatever the bootloader state might > > > be. > > > > > hmm, shouldn't ti-sysc keep things disabled in most cases? It is still a bit > > known because there is no status = "disabled" in the target-module@xxx node. > > Oh right, if the child device is tagged disabled (instead of the the parent > ti-sysc tagged disabled) the module should get idled just fine as long as the > module related quirks are implemented for ti-sysc.c. > > But still, I'd rather not start tagging devices disabled by default and then > re-enabling everywhere since we never needed it before. It just adds a lot > of pointless status tinkering, see commit 12afc0cf8121 ("ARM: dts: Drop > pointless status changing for am3 musb"). > > So considering things, IMO it's best to set only the child device with > status disabled, and set it at the board specific dts file in this case. Doesn't this imply the target-module stuff needs to be implemented for the drivers? It looks like a lot of the omap3 drivers are still using hwmods although some have target-modules. In this case, the mcspi drivers that Andreas is disabling don't appear to have target-module stuff configured. adam > > Also note that the dma channels could be freed with /delete-property/ at the > board specific dts file even for devices that are usable if not really > needed. > > Regards, > > Tony >
* Adam Ford <aford173@gmail.com> [230116 17:00]: > Doesn't this imply the target-module stuff needs to be implemented for > the drivers? It looks like a lot of the omap3 drivers are still using > hwmods although some have target-modules. In this case, the mcspi > drivers that Andreas is disabling don't appear to have target-module > stuff configured. Sorry I don't remember if omap_device.c ignores status disabled or not. But in any case, it should be trivial to update omap3.dtsi to configure some of the devices like mcspi to probe with device tree data and ti-sysc as needed. Regards, Tony
* Tony Lindgren <tony@atomide.com> [230116 17:33]: > * Adam Ford <aford173@gmail.com> [230116 17:00]: > > Doesn't this imply the target-module stuff needs to be implemented for > > the drivers? It looks like a lot of the omap3 drivers are still using > > hwmods although some have target-modules. In this case, the mcspi > > drivers that Andreas is disabling don't appear to have target-module > > stuff configured. > > Sorry I don't remember if omap_device.c ignores status disabled or not. > But in any case, it should be trivial to update omap3.dtsi to configure > some of the devices like mcspi to probe with device tree data and ti-sysc > as needed. So as long as gta04 power management still behaves with this patch it should good to go. Just to note, if mcspi is at some point is updated to probe with ti-sysc, the disabled flag needs to be kept at the mcspi child node, not at the sysc target module level to idle the unused devices. Regards, Tony
On Thu, 19 Jan 2023 09:30:20 +0200 Tony Lindgren <tony@atomide.com> wrote: > * Tony Lindgren <tony@atomide.com> [230116 17:33]: > > * Adam Ford <aford173@gmail.com> [230116 17:00]: > > > Doesn't this imply the target-module stuff needs to be implemented for > > > the drivers? It looks like a lot of the omap3 drivers are still using > > > hwmods although some have target-modules. In this case, the mcspi > > > drivers that Andreas is disabling don't appear to have target-module > > > stuff configured. > > > > Sorry I don't remember if omap_device.c ignores status disabled or not. > > But in any case, it should be trivial to update omap3.dtsi to configure > > some of the devices like mcspi to probe with device tree data and ti-sysc > > as needed. > > So as long as gta04 power management still behaves with this patch it > should good to go. > # sleep 10 ; /usr/local/bin/idledump CM_IDLEST1_CORE 00000042 CM_IDLEST3_CORE 00000000 CM_FCLKEN1_CORE 00000000 CM_FCLKEN3_CORE 00000002 CM_CLKSTST_CORE 00000003 CM_IDLEST_CKGEN 00000209 CM_IDLEST2_CKGEN 00000000 CM_FCLKEN_DSS 00000000 CM_IDLEST_DSS 00000000 CM_FCLKEN_CAM 00000000 CM_IDLEST_CAM 00000000 CM_FCLKEN_PER 00000000 CM_IDLEST_PER 00000000 FCLKEN3_CORE becomes 0 after unbinding the bandgap sensor. but... # cat /sys/kernel/debug/pm_debug/time usbhost_pwrdm (ON),OFF:830267486567,RET:0,INA:0,ON:12202880865 sgx_pwrdm (INA),OFF:0,RET:0,INA:841224365234,ON:1245971680 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:842470336914 per_pwrdm (ON),OFF:520406799328,RET:30043365464,INA:0,ON:292020111087 hmmm.... but does not look like anything related to mcspi*. Regards, Andreas
diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi index 87e0ab1bbe95..e0be0fb23f80 100644 --- a/arch/arm/boot/dts/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/omap3-gta04.dtsi @@ -612,6 +612,22 @@ &i2c3 { clock-frequency = <100000>; }; +&mcspi1 { + status = "disabled"; +}; + +&mcspi2 { + status = "disabled"; +}; + +&mcspi3 { + status = "disabled"; +}; + +&mcspi4 { + status = "disabled"; +}; + &usb_otg_hs { interface-type = <0>; usb-phy = <&usb2_phy>;