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 <[email protected]>
Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator defined at device tree")
Cc: <[email protected]>
---
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;
--
1.9.1
Hello Krzysztof,
On Mon, Jun 8, 2015 at 3:35 AM, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator defined at device tree")
> Cc: <[email protected]>
>
Reviewed-by: Javier Martinez Canillas <[email protected]>
> ---
>
> 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
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 <[email protected]>
> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator
> defined at device tree") Cc: <[email protected]>
>
> ---
>
> 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 <[email protected]>
I will test it and afterwards add to samsung-thermal tree.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
2015-06-09 1:14 GMT+09:00 Lukasz Majewski <[email protected]>:
> 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 <[email protected]>
>> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator
>> defined at device tree") Cc: <[email protected]>
>>
>> ---
>>
>> 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 <[email protected]>
>
> 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
Hi Krzysztof,
> 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <[email protected]>:
> > 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 <[email protected]>
> >> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator
> >> defined at device tree") Cc: <[email protected]>
> >>
> >> ---
> >>
> >> 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 <[email protected]>
> >
> > 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
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
On 06.07.2015 16:01, Lukasz Majewski wrote:
> Hi Krzysztof,
>
>> 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <[email protected]>:
>>> 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 <[email protected]>
>>>> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator
>>>> defined at device tree") Cc: <[email protected]>
>>>>
>>>> ---
>>>>
>>>> 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 <[email protected]>
>>>
>>> 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
Hi Krzysztof,
> 2015-06-09 1:14 GMT+09:00 Lukasz Majewski <[email protected]>:
> > 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 <[email protected]>
> >> Fixes: 498d22f616f6 ("thermal: exynos: Support for TMU regulator
> >> defined at device tree") Cc: <[email protected]>
> >>
> >> ---
> >>
> >> 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 <[email protected]>
> >
> > 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.
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group