| Message ID | 1387477045-19037-2-git-send-email-b20788@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
Hi Anson, On Thu, Dec 19, 2013 at 01:17:24PM -0500, Anson Huang wrote: > + if (IS_ERR(data->thermal_clk)) { > + ret = IS_ERR(data->thermal_clk); You probably what to return PTR_ERR here. baruch > + dev_err(&pdev->dev, "failed to get thermal clk!\n"); > + return ret; > + }
Best Regards. Anson huang ??? Freescale Semiconductor Shanghai ?????????192?A?2? 201203 Tel:021-28937058 >-----Original Message----- >From: Baruch Siach [mailto:baruch@tkos.co.il] >Sent: Thursday, December 19, 2013 2:25 PM >To: Huang Yongcai-B20788 >Cc: shawn.guo@linaro.org; kernel@pengutronix.de; rui.zhang@intel.com; >eduardo.valentin@ti.com; devicetree@vger.kernel.org; linux-pm@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org >Subject: Re: [PATCH 2/3] thermal: imx: add necessary clk operation > >Hi Anson, > >On Thu, Dec 19, 2013 at 01:17:24PM -0500, Anson Huang wrote: >> + if (IS_ERR(data->thermal_clk)) { >> + ret = IS_ERR(data->thermal_clk); > >You probably what to return PTR_ERR here. > Yes, thanks, will fix it in V2. [Anson] >baruch > >> + dev_err(&pdev->dev, "failed to get thermal clk!\n"); >> + return ret; >> + } > >-- > http://baruch.siach.name/blog/ ~. .~ Tk Open Systems >=}------------------------------------------------ooO--U--Ooo------------{= > - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - >
On Thu, Dec 19, 2013 at 01:17:24PM -0500, Anson Huang wrote: > @@ -427,6 +429,13 @@ static int imx_thermal_probe(struct platform_device *pdev) > return ret; > } > > + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(data->thermal_clk)) { > + ret = IS_ERR(data->thermal_clk); > + dev_err(&pdev->dev, "failed to get thermal clk!\n"); > + return ret; > + } > + So when the new kernel runs on a board with an old DTB installed, thermal driver will be broken. Shawn -- 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
DQoNCkJlc3QgUmVnYXJkcy4NCkFuc29uIGh1YW5nIOm7hOWLh+aJjQ0KwqANCkZyZWVzY2FsZSBT ZW1pY29uZHVjdG9yIFNoYW5naGFpDQrkuIrmtbfmtabkuJzmlrDljLrkuq7mma/ot68xOTLlj7dB 5bqnMualvA0KMjAxMjAzDQpUZWw6MDIxLTI4OTM3MDU4DQoNCg0KPi0tLS0tT3JpZ2luYWwgTWVz c2FnZS0tLS0tDQo+RnJvbTogU2hhd24gR3VvIFttYWlsdG86c2hhd24uZ3VvQGxpbmFyby5vcmdd DQo+U2VudDogVGh1cnNkYXksIERlY2VtYmVyIDE5LCAyMDEzIDI6NTkgUE0NCj5UbzogSHVhbmcg WW9uZ2NhaS1CMjA3ODgNCj5DYzoga2VybmVsQHBlbmd1dHJvbml4LmRlOyBydWkuemhhbmdAaW50 ZWwuY29tOyBlZHVhcmRvLnZhbGVudGluQHRpLmNvbTsNCj5kZXZpY2V0cmVlQHZnZXIua2VybmVs Lm9yZzsgbGludXgtZG9jQHZnZXIua2VybmVsLm9yZzsgbGludXgtYXJtLQ0KPmtlcm5lbEBsaXN0 cy5pbmZyYWRlYWQub3JnOyBsaW51eC1wbUB2Z2VyLmtlcm5lbC5vcmcNCj5TdWJqZWN0OiBSZTog W1BBVENIIDIvM10gdGhlcm1hbDogaW14OiBhZGQgbmVjZXNzYXJ5IGNsayBvcGVyYXRpb24NCj4N Cj5PbiBUaHUsIERlYyAxOSwgMjAxMyBhdCAwMToxNzoyNFBNIC0wNTAwLCBBbnNvbiBIdWFuZyB3 cm90ZToNCj4+IEBAIC00MjcsNiArNDI5LDEzIEBAIHN0YXRpYyBpbnQgaW14X3RoZXJtYWxfcHJv YmUoc3RydWN0IHBsYXRmb3JtX2RldmljZQ0KPipwZGV2KQ0KPj4gIAkJcmV0dXJuIHJldDsNCj4+ ICAJfQ0KPj4NCj4+ICsJZGF0YS0+dGhlcm1hbF9jbGsgPSBkZXZtX2Nsa19nZXQoJnBkZXYtPmRl diwgTlVMTCk7DQo+PiArCWlmIChJU19FUlIoZGF0YS0+dGhlcm1hbF9jbGspKSB7DQo+PiArCQly ZXQgPSBJU19FUlIoZGF0YS0+dGhlcm1hbF9jbGspOw0KPj4gKwkJZGV2X2VycigmcGRldi0+ZGV2 LCAiZmFpbGVkIHRvIGdldCB0aGVybWFsIGNsayFcbiIpOw0KPj4gKwkJcmV0dXJuIHJldDsNCj4+ ICsJfQ0KPj4gKw0KPg0KPlNvIHdoZW4gdGhlIG5ldyBrZXJuZWwgcnVucyBvbiBhIGJvYXJkIHdp dGggYW4gb2xkIERUQiBpbnN0YWxsZWQsIHRoZXJtYWwNCj5kcml2ZXIgd2lsbCBiZSBicm9rZW4u DQo+DQpZZXMsIEkgdGhvdWdodCBhYm91dCB0aGlzIGNhc2UsIGJ1dCB0aGUgcHJldmlvdXMgaW1w bGVtZW50IGlzIGluY29ycmVjdCwgaWYNCnRoZSBQTEwzIGlzIG5vdCBlbmFibGVkIGJ5IG90aGVy IGRyaXZlcnMsIHRoZXJtYWwgZHJpdmVyIHdpbGwgbm90IHdvcmssIHNvDQp0aGlzIHBhdGNoIGlz IGEgYnVnIGZpeCwgbm90IGVuaGFuY2VtZW50LiBTbyB3ZSBzdGlsbCBuZWVkIHRvIGNvbnNpZGVy IG9sZA0KZHRzIGNhc2U/IA0KDQpBbnNvbg0KDQo+U2hhd24NCg0K -- 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
On Thu, Dec 19, 2013 at 07:08:16AM +0000, Anson.Huang@freescale.com wrote: > >On Thu, Dec 19, 2013 at 01:17:24PM -0500, Anson Huang wrote: > >> @@ -427,6 +429,13 @@ static int imx_thermal_probe(struct platform_device > >*pdev) > >> return ret; > >> } > >> > >> + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > >> + if (IS_ERR(data->thermal_clk)) { > >> + ret = IS_ERR(data->thermal_clk); > >> + dev_err(&pdev->dev, "failed to get thermal clk!\n"); > >> + return ret; > >> + } > >> + > > > >So when the new kernel runs on a board with an old DTB installed, thermal > >driver will be broken. > > > Yes, I thought about this case, but the previous implement is incorrect, if > the PLL3 is not enabled by other drivers, thermal driver will not work, so > this patch is a bug fix, not enhancement. So we still need to consider old > dts case? The thing is mainline kernel runs on many board with thermal driver being functional today. That said, PLL3 is already enabled on these platforms when thermal driver is running. You cannot fix a bug but meanwhile break these users who use old DTB. Shawn -- 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
Best Regards. Anson huang ??? Freescale Semiconductor Shanghai ?????????192?A?2? 201203 Tel:021-28937058 >-----Original Message----- >From: Shawn Guo [mailto:shawn.guo@linaro.org] >Sent: Thursday, December 19, 2013 3:29 PM >To: Huang Yongcai-B20788 >Cc: kernel@pengutronix.de; rui.zhang@intel.com; eduardo.valentin@ti.com; >devicetree@vger.kernel.org; linux-doc@vger.kernel.org; linux-arm- >kernel@lists.infradead.org; linux-pm@vger.kernel.org >Subject: Re: [PATCH 2/3] thermal: imx: add necessary clk operation > >On Thu, Dec 19, 2013 at 07:08:16AM +0000, Anson.Huang@freescale.com wrote: >> >On Thu, Dec 19, 2013 at 01:17:24PM -0500, Anson Huang wrote: >> >> @@ -427,6 +429,13 @@ static int imx_thermal_probe(struct >> >> platform_device >> >*pdev) >> >> return ret; >> >> } >> >> >> >> + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); >> >> + if (IS_ERR(data->thermal_clk)) { >> >> + ret = IS_ERR(data->thermal_clk); >> >> + dev_err(&pdev->dev, "failed to get thermal clk!\n"); >> >> + return ret; >> >> + } >> >> + >> > >> >So when the new kernel runs on a board with an old DTB installed, >> >thermal driver will be broken. >> > >> Yes, I thought about this case, but the previous implement is >> incorrect, if the PLL3 is not enabled by other drivers, thermal driver >> will not work, so this patch is a bug fix, not enhancement. So we >> still need to consider old dts case? > >The thing is mainline kernel runs on many board with thermal driver being >functional today. That said, PLL3 is already enabled on these platforms when >thermal driver is running. You cannot fix a bug but meanwhile break these >users who use old DTB. Okay, got it, I will replace the return with one warning message in V2. Anson > >Shawn
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c index 1d6c801..7abbc04 100644 --- a/drivers/thermal/imx_thermal.c +++ b/drivers/thermal/imx_thermal.c @@ -7,6 +7,7 @@ * */ +#include <linux/clk.h> #include <linux/cpu_cooling.h> #include <linux/cpufreq.h> #include <linux/delay.h> @@ -73,6 +74,7 @@ struct imx_thermal_data { unsigned long last_temp; bool irq_enabled; int irq; + struct clk *thermal_clk; }; static void imx_set_alarm_temp(struct imx_thermal_data *data, @@ -427,6 +429,13 @@ static int imx_thermal_probe(struct platform_device *pdev) return ret; } + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(data->thermal_clk)) { + ret = IS_ERR(data->thermal_clk); + dev_err(&pdev->dev, "failed to get thermal clk!\n"); + return ret; + } + /* Make sure sensor is in known good state for measurements */ regmap_write(map, TEMPSENSE0 + REG_CLR, TEMPSENSE0_POWER_DOWN); regmap_write(map, TEMPSENSE0 + REG_CLR, TEMPSENSE0_MEASURE_TEMP); @@ -457,6 +466,19 @@ static int imx_thermal_probe(struct platform_device *pdev) return ret; } + /* + * Thermal sensor needs clk on to get correct value, normally + * we should enable its clk before taking measurement and disable + * clk after measurement is done, but if alarm function is enabled, + * hardware will auto measure the temperature periodically, so we + * need to keep the clk always on for alarm function. + */ + ret = clk_prepare_enable(data->thermal_clk); + if (ret) { + dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret); + return ret; + } + /* Enable measurements at ~ 10 Hz */ regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ); measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
Thermal sensor needs pll3_usb_otg when measuring temperature, otherwise the temperature read will be incorrect, so need to enable this clk before sensor working, for alarm function, as hardware will take measurement periodically, so we should keep this clk always on once alarm function is enabled. Signed-off-by: Anson Huang <b20788@freescale.com> --- drivers/thermal/imx_thermal.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)