| Message ID | 1433727349-23330-1-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
Hello Krzysztof, On Mon, Jun 8, 2015 at 3:35 AM, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > During probe the regulator (if present) was enabled but not disabled in > case of failure. So an unsuccessful probe lead to enabling the > regulator which was actually not needed because the device was not > enabled. > > Additionally each deferred probe lead to increase of regulator enable > count so it would not be effectively disabled during removal of the > device. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator defined at device tree") > Cc: <stable@vger.kernel.org> > Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > --- > > I am not entirely convinced that this should go to stable. Leaving a > regulator enabled in case of probe failure (no exynos TMU device) or > after deferred probe (regulator won't be disabled during device removal) > is not a critical issue, just leaks power. Yes, as you said leaving the regulator enabled is not critical but OTOH is a very small patch and is fixing a very evident bug so I think it's OK. Best regards, Javier -- 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
Hi Krzysztof, > During probe the regulator (if present) was enabled but not disabled > in case of failure. So an unsuccessful probe lead to enabling the > regulator which was actually not needed because the device was not > enabled. > > Additionally each deferred probe lead to increase of regulator enable > count so it would not be effectively disabled during removal of the > device. Thanks for catching this. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator > defined at device tree") Cc: <stable@vger.kernel.org> > > --- > > I am not entirely convinced that this should go to stable. Leaving a > regulator enabled in case of probe failure (no exynos TMU device) or > after deferred probe (regulator won't be disabled during device > removal) is not a critical issue, just leaks power. > --- > drivers/thermal/samsung/exynos_tmu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/thermal/samsung/exynos_tmu.c > b/drivers/thermal/samsung/exynos_tmu.c index > 531f4b179871..13c3aceed19d 100644 --- > a/drivers/thermal/samsung/exynos_tmu.c +++ > b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ > err_clk_sec: if (!IS_ERR(data->clk_sec)) > clk_unprepare(data->clk_sec); > err_sensor: > + if (!IS_ERR_OR_NULL(data->regulator)) > + regulator_disable(data->regulator); > thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); > > return ret; Acked-by: Lukasz Majewski <l.majewski@samsung.com> I will test it and afterwards add to samsung-thermal tree.
2015-06-09 1:14 GMT+09:00 Lukasz Majewski <l.majewski@samsung.com>: > Hi Krzysztof, > >> During probe the regulator (if present) was enabled but not disabled >> in case of failure. So an unsuccessful probe lead to enabling the >> regulator which was actually not needed because the device was not >> enabled. >> >> Additionally each deferred probe lead to increase of regulator enable >> count so it would not be effectively disabled during removal of the >> device. > > Thanks for catching this. > >> >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator >> defined at device tree") Cc: <stable@vger.kernel.org> >> >> --- >> >> I am not entirely convinced that this should go to stable. Leaving a >> regulator enabled in case of probe failure (no exynos TMU device) or >> after deferred probe (regulator won't be disabled during device >> removal) is not a critical issue, just leaks power. >> --- >> drivers/thermal/samsung/exynos_tmu.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/thermal/samsung/exynos_tmu.c >> b/drivers/thermal/samsung/exynos_tmu.c index >> 531f4b179871..13c3aceed19d 100644 --- >> a/drivers/thermal/samsung/exynos_tmu.c +++ >> b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ >> err_clk_sec: if (!IS_ERR(data->clk_sec)) >> clk_unprepare(data->clk_sec); >> err_sensor: >> + if (!IS_ERR_OR_NULL(data->regulator)) >> + regulator_disable(data->regulator); >> thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); >> >> return ret; > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > I will test it and afterwards add to samsung-thermal tree. Hi ?ukasz, I can't find this patch in v4.2-rc1 or your tree. What happened? Best regards, Krzysztof -- 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
Hi Krzysztof, > 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <l.majewski@samsung.com>: > > Hi Krzysztof, > > > >> During probe the regulator (if present) was enabled but not > >> disabled in case of failure. So an unsuccessful probe lead to > >> enabling the regulator which was actually not needed because the > >> device was not enabled. > >> > >> Additionally each deferred probe lead to increase of regulator > >> enable count so it would not be effectively disabled during > >> removal of the device. > > > > Thanks for catching this. > > > >> > >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > >> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator > >> defined at device tree") Cc: <stable@vger.kernel.org> > >> > >> --- > >> > >> I am not entirely convinced that this should go to stable. Leaving > >> a regulator enabled in case of probe failure (no exynos TMU > >> device) or after deferred probe (regulator won't be disabled > >> during device removal) is not a critical issue, just leaks power. > >> --- > >> drivers/thermal/samsung/exynos_tmu.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/thermal/samsung/exynos_tmu.c > >> b/drivers/thermal/samsung/exynos_tmu.c index > >> 531f4b179871..13c3aceed19d 100644 --- > >> a/drivers/thermal/samsung/exynos_tmu.c +++ > >> b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ > >> err_clk_sec: if (!IS_ERR(data->clk_sec)) > >> clk_unprepare(data->clk_sec); > >> err_sensor: > >> + if (!IS_ERR_OR_NULL(data->regulator)) > >> + regulator_disable(data->regulator); > >> thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); > >> > >> return ret; > > > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > > > I will test it and afterwards add to samsung-thermal tree. > > Hi ?ukasz, > > I can't find this patch in v4.2-rc1 or your tree. What happened? I will got together with Chanowoo patches. I will send PR today to Eduardo. > > Best regards, > Krzysztof
On 06.07.2015 16:01, Lukasz Majewski wrote: > Hi Krzysztof, > >> 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <l.majewski@samsung.com>: >>> Hi Krzysztof, >>> >>>> During probe the regulator (if present) was enabled but not >>>> disabled in case of failure. So an unsuccessful probe lead to >>>> enabling the regulator which was actually not needed because the >>>> device was not enabled. >>>> >>>> Additionally each deferred probe lead to increase of regulator >>>> enable count so it would not be effectively disabled during >>>> removal of the device. >>> >>> Thanks for catching this. >>> >>>> >>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >>>> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator >>>> defined at device tree") Cc: <stable@vger.kernel.org> >>>> >>>> --- >>>> >>>> I am not entirely convinced that this should go to stable. Leaving >>>> a regulator enabled in case of probe failure (no exynos TMU >>>> device) or after deferred probe (regulator won't be disabled >>>> during device removal) is not a critical issue, just leaks power. >>>> --- >>>> drivers/thermal/samsung/exynos_tmu.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c >>>> b/drivers/thermal/samsung/exynos_tmu.c index >>>> 531f4b179871..13c3aceed19d 100644 --- >>>> a/drivers/thermal/samsung/exynos_tmu.c +++ >>>> b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ >>>> err_clk_sec: if (!IS_ERR(data->clk_sec)) >>>> clk_unprepare(data->clk_sec); >>>> err_sensor: >>>> + if (!IS_ERR_OR_NULL(data->regulator)) >>>> + regulator_disable(data->regulator); >>>> thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); >>>> >>>> return ret; >>> >>> Acked-by: Lukasz Majewski <l.majewski@samsung.com> >>> >>> I will test it and afterwards add to samsung-thermal tree. >> >> Hi ?ukasz, >> >> I can't find this patch in v4.2-rc1 or your tree. What happened? > > I will got together with Chanowoo patches. I will send PR today to > Eduardo. Thanks! Best regards, Krzysztof -- 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
Hi Krzysztof, > 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <l.majewski@samsung.com>: > > Hi Krzysztof, > > > >> During probe the regulator (if present) was enabled but not > >> disabled in case of failure. So an unsuccessful probe lead to > >> enabling the regulator which was actually not needed because the > >> device was not enabled. > >> > >> Additionally each deferred probe lead to increase of regulator > >> enable count so it would not be effectively disabled during > >> removal of the device. > > > > Thanks for catching this. > > > >> > >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > >> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator > >> defined at device tree") Cc: <stable@vger.kernel.org> > >> > >> --- > >> > >> I am not entirely convinced that this should go to stable. Leaving > >> a regulator enabled in case of probe failure (no exynos TMU > >> device) or after deferred probe (regulator won't be disabled > >> during device removal) is not a critical issue, just leaks power. > >> --- > >> drivers/thermal/samsung/exynos_tmu.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/thermal/samsung/exynos_tmu.c > >> b/drivers/thermal/samsung/exynos_tmu.c index > >> 531f4b179871..13c3aceed19d 100644 --- > >> a/drivers/thermal/samsung/exynos_tmu.c +++ > >> b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ > >> err_clk_sec: if (!IS_ERR(data->clk_sec)) > >> clk_unprepare(data->clk_sec); > >> err_sensor: > >> + if (!IS_ERR_OR_NULL(data->regulator)) > >> + regulator_disable(data->regulator); > >> thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); > >> > >> return ret; > > > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > > > I will test it and afterwards add to samsung-thermal tree. > > Hi ?ukasz, > > I can't find this patch in v4.2-rc1 or your tree. What happened? > > Best regards, > Krzysztof Applied to linux-samsung-thermal.git Thanks for fix and sorry for the delay.
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 531f4b179871..13c3aceed19d 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -1392,6 +1392,8 @@ err_clk_sec: if (!IS_ERR(data->clk_sec)) clk_unprepare(data->clk_sec); err_sensor: + if (!IS_ERR_OR_NULL(data->regulator)) + regulator_disable(data->regulator); thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); return ret;
During probe the regulator (if present) was enabled but not disabled in case of failure. So an unsuccessful probe lead to enabling the regulator which was actually not needed because the device was not enabled. Additionally each deferred probe lead to increase of regulator enable count so it would not be effectively disabled during removal of the device. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator defined at device tree") Cc: <stable@vger.kernel.org> --- I am not entirely convinced that this should go to stable. Leaving a regulator enabled in case of probe failure (no exynos TMU device) or after deferred probe (regulator won't be disabled during device removal) is not a critical issue, just leaks power. --- drivers/thermal/samsung/exynos_tmu.c | 2 ++ 1 file changed, 2 insertions(+)