On Wed, Oct 23, 2019 at 9:36 AM Tony Lindgren <tony@xxxxxxxxxxx> wrote: > > * H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> [191023 04:42]: > > > > > Am 23.10.2019 um 00:19 schrieb Tony Lindgren <tony@xxxxxxxxxxx>: > > > > > > * Adam Ford <aford173@xxxxxxxxx> [191022 19:01]: > > >> On Tue, Oct 22, 2019 at 11:22 AM Tony Lindgren <tony@xxxxxxxxxxx> wrote: > > >>> > > >>> Hi, > > >>> > > >>> * Adam Ford <aford173@xxxxxxxxx> [191007 15:06]: > > >>>> The some in the OMAP3 family have a bandgap thermal sensor, but > > >>>> omap2plus has it disabled. > > >>>> > > >>>> This patch enables the OMAP3_THERMAL by default like the rest of > > >>>> the OMAP family. > > >>> > > >>> Looks like this breaks off mode during idle for omap3, and that's > > >>> probably why it never got enabled. The difference in power > > >>> consumption during idle is about 7mW vs 32mW for the SoC as > > >>> measured from torpedo shunt for main_battery_som. > > >>> > > >>> I think the right fix might be simply to add handling for > > >>> CPU_CLUSTER_PM_ENTER to the related thermal driver to disable > > >>> it during idle like we have for gpio-omap.c for example. > > >> > > >> I am not sure I know where to start on fixing that issue. Would you > > >> entertain enabling the driver if we set the device tree to 'disabled' > > >> by default? This way if people want to to use it, it can be enabled > > >> on a per-device option. Once the power stuff gets resolved, we might > > >> be able to enable it by default. For people who are planning on using > > >> the DM3730 @ 1GHz in high temp environments, I am not sure they'll > > >> care about low power. > > > > > > They should both work fine together though. They are not mutually > > > exclusive features. > > > > > >> I'll try to look into it when I have time, but I was hoping a > > >> compromise might be a reasonable work-around. > > > > > > It should be hopefully a trivial fix.. I have not looked at the > > > driver code though. > > > > If I am taken right, it is the drivers/thermal/ti-soc-thermal/ti-*.c > > which is a common driver for omap3, omap4, omap5. They only differ > > in the thermal data and which registers and bits are used to access > > the ADC. > > Yes so it seems. Enabling OMAP3_THERMAL adds support to > of_ti_bandgap_match[] for omap3 and causes the issue. > > > So is this problem with off mode also known for omap4 and omap5? > > Probably. But we don't have off mode working for omap4, and > it cannot be used for omap5 AFAIK. > > My guess is we need to call clk_disable() and call > ti_bandgap_save_ctxt() on CPU_CLUSTER_PM_ENTER similar to > what ti_bandgap_suspend does. And then restore it on > CPU_CLUSTER_PM_EXIT. > > There's a similar example already in gpio_omap_cpu_notifier(). > Not sure if there is some related errata to deal with too, > probably the old Nokia n900 or n9 would provide some hints > on what exactly needs to be done. I 'think' I have a patch ready that does what you're asking, but I will fully admit that I don't completely grasp what's going on. I'll submit it as an RFC, but I am not even sure I understand what to put into the description, so if you're OK with reviewing the RFC, feel free to mark up the actual commit message as well. >From what I can see, the changes haven't negatively impact stuff. I didn't see the power consumption go up before, so I am not sure I can replicate your findings. It'll be posted shortly. adam > > Regards, > > Tony
