Adding Grygorii and Tero, On 07/25/2017 04:17 AM, Stefan Eichenberger wrote: > On 25 July 2017 at 11:10, Tony Lindgren <tony@xxxxxxxxxxx> 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): >> ... >> >>> This patch will immediately set the disabled module to >>> _HWMOD_STATE_DISABLED. >> >> Thanks I noticed the same issue with rng on hs omap4. I think this is a good >> solution. And then the status = "incomplete" patch I posted a while back >> can be used to idle devices that are safe to touch and are not wired up on >> a device. Will do some testing on your patch. > > Perfect, thanks for testing! The patch should apply on top of > linux-omap/for-next. Sorry but I have to say hard NAK for this patch. This patch will introduce all sorts of chaos with PM low power modes in addition to leaving many IPs in undefined power states. What happens on a system when RTC is present and powered but is marked disabled in the DT? Nothing, which means it is just left powered for no reason at all. With this patch in place the only way to ensure suspend will work is to have every single device marked "okay" regardless of whether or not it's in use or present on the system. The original intent of having omap_hwmod touch hwmods regardless of their dt state is to put the system into a defined state at boot time for PM. HWMODs do not have a fixed default state, every hwmod has a default state which can be on or off at boot depending on the platform, and without this in place, some modules can easily be left on by accident which will entirely prevent suspend on platforms like am335x or am437x if the IP sits in the peripheral power domain. I understand there are situations like RTC where it may not be clocked and attempting to idle it will cause a panic which is an issue we have on am43x-epos-evm, I tried to address this long back here [1]. That thread is a bit long but contains a lot of information about why we can't apply this patch without trashing existing PM on OMAP based platforms. Basically we need some sort of method of marking specific IPs as 'not-present' which is different than disabled. 'disabled' implies that the device is off, not necessarily that it is missing entirely. A solution as suggested by Grygorii deep in the thread I linked may be to entirely drop the problematic node from the specific board/platform where it causes an issue, and include it only when appropriate, or what Tony had suggested before that marks a module as entirely missing so we don't touch it all. Regards, Dave [1] https://www.spinics.net/lists/linux-omap/msg116382.html > > Regards, > Stefan > -- > 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 > -- 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
