On 08/14/2017 02:08 PM, Tony Lindgren wrote: > * Dave Gerlach <d-gerlach@xxxxxx> [170814 11:26]: >> +Nishanth and Tero, >> On 08/14/2017 12:47 PM, Tony Lindgren wrote: >>> * Stefan Eichenberger <eichest@xxxxxxxxx> [170725 02:04]: >>>> I already did an RFC [1] that tried to do the same but I think this one is >>>> cleaner. If a hardware module is disabled in devicetree the address range >>>> will still be touched by omap_hwmod.c. If the corresponding module isn't >>>> powered this can lead to a crash of the kernel (for example rtc): >>> >>> Just to recap on this, we need to first fix up any related issues >>> and then applying this should be trivial. Mostly I'm worried about >>> what all hidden dependencies we have, mosty I want to see what >>> examples for problems Dave comes up for PM on am335x :) >>> >>> Meanwhile, the workaround is to set the problem devices with >>> status = "disabled" in the board specific dts file. >>> >>> In the long run, let's plan on making status = "disabled" mean >>> that Linux does not touch the device at all until it's state >>> changes to "okay" and a struct device is created for it. >>> >> >> And to recap the issue: The main problem to solve is that any IP should be idled >> at the end of boot if no driver was bound. That is what we will lose right now >> if we stop touching nodes with status="disabled". > > That is not correct. We do have hwmod idle modules just fine > even if there's no driver and there're not marked with > status = "disabled". > I agree with this. Modules are idled fine if they are not marked with disabled. But currently they *also* idle fine if marked disabled because omap_hwmod ignores the status field and grabs the base address so it can program the sysconfig register and idle the IP. This is what will no longer be the case if we apply the original patch and that is the issue I am worried about. >> Without proper sysconfig programming certain modules will not >> idle, like USB, CPSW, or DMA on am335x which are some examples off the top of my >> head and possibly the only guilty modules. So if we do not touch these modules >> because they are marked disabled then the kernel will not be able to program the >> SYSCONFIG and the modules cannot idle, so suspend fails on am335x. > > Can you please just check what happens on am335x with USB, CPSW > and DMA with mainline kernel if you just remove status = "disabled" > for them? If they are not idled properly without "disabled" that's > where the bug is. They are idled properly without status disabled. They are also idled properly with status disabled with mainline: With status = "okay": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 With status = "disabled": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 But if I apply this patch: With status = "okay": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00070000 This is idled, but what if we do not intend to use USB? Do we just want to mark it OK and make sure no code is present so no driver gets loaded? With status = "disabled": CM_PER_USB0_CLKCTRL | 0x44E0001C | 0x00000002 This is active which we DO not want, because we did not look at the node at all, meaning omap_hwmod could not get the IP base address needed to write to the sysconfig register and did not attempt to idle it. The module is still active because it could not program the sysconfig register because omap_hwmod doesn't even know that it exists. Again, I am not saying touching status disabled nodes is the right way to do it, just that currently we do for the reasons described above and if we stop touching the nodes marked disabled we need a new solution. Regards, Dave > > Thanks, > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
