On 07/25/2017 10:38 AM, Tony Lindgren wrote: > * Dave Gerlach <d-gerlach@xxxxxx> [170725 08:28]: >> On 07/25/2017 10:03 AM, Tony Lindgren wrote: >>> * Dave Gerlach <d-gerlach@xxxxxx> [170725 07:18]: >>>> >>>> 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. >>> >>> What status = "disabled" means is that Linux does not know about the >>> device at all. We are not following the standard right now, and that's why I >>> introduced status = "incomplete" which means the device driver can probe and >>> bail out after idling the device. >> >> I don't disagree that we are not following standard, but this is the way it's >> been done and it's what makes things work. > > I've commented on numerous occasions that there should be no reason to > set any SoC internal devices with status = "disabled". And I've just checked > that the mainline kernel works just fine for PM with no "disabled" set for any > device on omap3. This non-standard PM dependency to "disabled" is something > that TI seems to have started with the TI kernel trees, and I don't agree that > "it's what makes things work". So we want every single IP loaded with a driver even if we aren't using it? What if we don't intend to use an IP? UART is a perfect example of this, there are six UARTs on am335x. If we are only hoping to use uart0 for the console and none of the others, why should we mark all six as "Okay" and have drivers loaded and the IPs ready to go? > >>>> 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. >>> >>> Default is status = "okay" and there really should be no reason to mark SoC >>> devices as "disabled" unless they really are not accessible as on HS devices. >>> We do have PM working for many omap3 devices without having to use status = >>> "disabled" at all except for some HS devices. Of course this needs to be >>> verified though :) >>> >>> But really, "disabled" is not the problem for PM, the limitations we have >>> for PM issues are missing runtime PM implementations in device drivers, >>> such as for OHCI/EHCI. >> >> What about when no driver is present and runtime PM does not apply at all? hwmod >> handles that case and shuts the IP off at boot regardless of what state it's in. > > Then just the usual, just add a driver :) Please let me know of any where > the mainline kernel relies on "disabled". > >> Example I can see already, on am335x, uart5 is left active after u-boot runs. We >> don't use uart5 by default in the kernel on am335x-evm so it's marked disabled >> in the DT, with this patch applied suspends fails with "PM: Could not transition >> all powerdomains to target state" message because uart5 (and other IPs for that >> matter) block transition of the PER domain and you don't enter suspend. How >> would we handle this and other IPs with similar issues? > > So why are you marking it with "disabled"? Just strip out all the bogus > "disabled" and see what happens. We describe all modules in the top level SoC dtsi and mark many as disabled and then mark them enabled in the board files as appropriate, so that's just the default state of most of the modules we use unless explicitly changed in the board file. > >> Again, I'm not saying we are doing things right by touching nodes with disabled >> set, but that's how it works today, and taking this patch alone will break a lot >> of things, there's a lot of other things that will need to be addressed first. > > Well then we need to first fix the issues naturally. But I already proved > that things work for my PM test case. And I suspect the bogus "disabled" > can be stripped out for all the omap variants. So please do let me know of > issues with drivers in the mainline kernel that break so we can fix them > properly. So again, my concern isn't with driver issues, it's if we don't want to use an IP and don't want a driver. Or is having a driver loaded for every IP regardless of whether or not you plan to use it what we want? > > Regards, > > 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
