| Message ID | 1387831918-17028-1-git-send-email-b20788@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
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). > > Signed-off-by: Anson Huang <b20788@freescale.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? 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sent from Anson's iPhone > ? 2013?12?23??22:28?"Shawn Guo" <shawn.guo@linaro.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@freescale.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 >
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; 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
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. Signed-off-by: Anson Huang <b20788@freescale.com> --- drivers/thermal/imx_thermal.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)