Re: [PATCH 1/2] configs: ARM: omap2plus: Enable OMAP3_THERMAL

[Tony Lindgren]

* 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.

Regards,

Tony