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

[Anson.Huang@xxxxxxxxxxxxx]

Sent from Anson's iPhone

> 在 2013年12月23日，22:28，"Shawn Guo" <shawn.guo@xxxxxxxxxx> 写道：
> 
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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
> 
㈤旃??????+-遍荻?w??笔???dz罐??骅w*jg??????/??罐????璀??摺?囤??????:+v???佶>W?贽i?xPj???-?+?d?