linux-kernel - Re: [PATCH] thermal: imx: correct suspend/resume flow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <C95204EE-D28A-4AB4-B60B-E7CAB8AC3B8E@freescale.com>
Date:	Mon, 23 Dec 2013 14:33:43 +0000
From:	"Anson.Huang@...escale.com" <Anson.Huang@...escale.com>
To:	Shawn Guo <shawn.guo@...aro.org>
CC:	"rui.zhang@...el.com" <rui.zhang@...el.com>,
	"eduardo.valentin@...com" <eduardo.valentin@...com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] thermal: imx: correct suspend/resume flow



Sent from Anson's iPhone

> 在 2013年12月23日，22:28，"Shawn Guo" <shawn.guo@...aro.org> 写道：
> 
>> On Mon, Dec 23, 2013 at 03:51:58PM -0500, Anson Huang wrote:
>> Current imx thermal sensor's suspend function will return fail if
>> thermal sensor is always enabled, but because alarm function is enabled,
>> thermal sensor will be always enabled as well, hence break system's suspend,
>> this patch will disable thermal sensor before suspend and re-enable it
>> after resume, it fixes the failure of suspend/resume caused by thermal driver.
> 
> We should probably make it clear in the commit log that it's a
> regression caused by commit 37713a1 (thermal: imx: implement thermal
> alarm interrupt handling).

agreed, will improve the commit log in V2.

> 
>> 
>> Signed-off-by: Anson Huang <b20788@...escale.com>
>> ---
>> drivers/thermal/imx_thermal.c |   31 +++++++++++++++++--------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
>> index cbb16f3..9a9a6c2 100644
>> --- a/drivers/thermal/imx_thermal.c
>> +++ b/drivers/thermal/imx_thermal.c
>> @@ -510,27 +510,30 @@ static int imx_thermal_suspend(struct device *dev)
>> {
>>    struct imx_thermal_data *data = dev_get_drvdata(dev);
>>    struct regmap *map = data->tempmon;
>> -    u32 val;
>> 
>> -    regmap_read(map, TEMPSENSE0, &val);
>> -    if ((val & TEMPSENSE0_POWER_DOWN) == 0) {
>> -        /*
>> -         * If a measurement is taking place, wait for a long enough
>> -         * time for it to finish, and then check again.  If it still
>> -         * does not finish, something must go wrong.
>> -         */
>> -        udelay(50);
>> -        regmap_read(map, TEMPSENSE0, &val);
>> -        if ((val & TEMPSENSE0_POWER_DOWN) == 0)
>> -            return -ETIMEDOUT;
>> -    }
>> +    /*
>> +     * Need to disable thermal sensor, otherwise, when thermal core
>> +     * try to get temperature before thermal sensor resume, a wrong
>> +     * temperature will be read as the thermal sensor is powered
>> +     * down.
>> +     */
>> +    regmap_write(map, TEMPSENSE0 + REG_CLR, TEMPSENSE0_MEASURE_TEMP);
>> +    regmap_write(map, TEMPSENSE0 + REG_SET, TEMPSENSE0_POWER_DOWN);
>> +    data->mode = THERMAL_DEVICE_DISABLED;
> 
> Is it really necessary to update data->mode through suspend/resume
> cycle?

Yes, as the thermal core may try to get temperature before imx thermal sensor resume, if not setting its mode to disabled, thermal core will get a wrong temp and caused system power off by mistake, as the get_temp function will decide whether to power up thermal core according to its mode.

> 
> Shawn
> 
>> 
>>    return 0;
>> }
>> 
>> static int imx_thermal_resume(struct device *dev)
>> {
>> -    /* Nothing to do for now */
>> +    struct imx_thermal_data *data = dev_get_drvdata(dev);
>> +    struct regmap *map = data->tempmon;
>> +
>> +    /* Enabled thermal sensor after resume */
>> +    regmap_write(map, TEMPSENSE0 + REG_CLR, TEMPSENSE0_POWER_DOWN);
>> +    regmap_write(map, TEMPSENSE0 + REG_SET, TEMPSENSE0_MEASURE_TEMP);
>> +    data->mode = THERMAL_DEVICE_ENABLED;
>> +
>>    return 0;
>> }
>> #endif
>> -- 
>> 1.7.9.5
>