linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure
@ 2015-10-08  5:34 Krzysztof Kozlowski
  2015-10-08  5:34 ` [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor Krzysztof Kozlowski
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-08  5:34 UTC (permalink / raw)
  To: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	Krzysztof Kozlowski, linux-pm, linux-samsung-soc,
	linux-arm-kernel, linux-kernel
  Cc: stable

During probe if the regulator could not be enabled, the error exit path
would still disable it. This could lead to unbalanced counter of
regulator enable/disable.

The patch moves code for getting and enabling the regulator from
exynos_map_dt_data() to probe function because it is really not a part
of getting Device Tree properties.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on probe failure")
Cc: <stable@vger.kernel.org>
---
 drivers/thermal/samsung/exynos_tmu.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 0bae8cc6c23a..23f4320f8ef7 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1168,27 +1168,10 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct exynos_tmu_platform_data *pdata;
 	struct resource res;
-	int ret;
 
 	if (!data || !pdev->dev.of_node)
 		return -ENODEV;
 
-	/*
-	 * Try enabling the regulator if found
-	 * TODO: Add regulator as an SOC feature, so that regulator enable
-	 * is a compulsory call.
-	 */
-	data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
-	if (!IS_ERR(data->regulator)) {
-		ret = regulator_enable(data->regulator);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to enable vtmu\n");
-			return ret;
-		}
-	} else {
-		dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
-	}
-
 	data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
 	if (data->id < 0)
 		data->id = 0;
@@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		pr_err("thermal: tz: %p ERROR\n", data->tzd);
 		return PTR_ERR(data->tzd);
 	}
+
+	/*
+	 * Try enabling the regulator if found
+	 * TODO: Add regulator as an SOC feature, so that regulator enable
+	 * is a compulsory call.
+	 */
+	data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
+	if (!IS_ERR(data->regulator)) {
+		ret = regulator_enable(data->regulator);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to enable vtmu\n");
+			return ret;
+		}
+	} else {
+		dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
+	}
+
 	ret = exynos_map_dt_data(pdev);
 	if (ret)
 		goto err_sensor;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor
  2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
@ 2015-10-08  5:34 ` Krzysztof Kozlowski
  2015-10-08 16:43   ` Alim Akhtar
  2015-10-08  5:34 ` [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-08  5:34 UTC (permalink / raw)
  To: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	Krzysztof Kozlowski, linux-pm, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

Thermal core could not read the temperature after registering the
thermal sensor with thermal_zone_of_sensor_register() because the driver
was not yet initialized.

The call trace looked like:
    exynos_tmu_probe()
        thermal_zone_of_sensor_register()
            of_thermal_set_mode()
                thermal_zone_device_update()
                    exynos_get_temp()
                        if (!data->tmu_read) return -EINVAL;
        exynos_map_dt_data()
            data->tmu_read = ...

This produced an error in dmesg:
thermal thermal_zone0: failed to read out thermal zone (-22)

Register the thermal_zone_device later, after parsing Device Tree and
enabling necessary clocks, but before calling exynos_tmu_initialize()
which uses the registered thermal_zone_device.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: 3b6a1a805f34 ("thermal: samsung: core: Exynos TMU rework to use device tree for configuration")
---
 drivers/thermal/samsung/exynos_tmu.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 23f4320f8ef7..bc71a61f0c4a 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1289,13 +1289,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, data);
 	mutex_init(&data->lock);
 
-	data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
-						    &exynos_sensor_ops);
-	if (IS_ERR(data->tzd)) {
-		pr_err("thermal: tz: %p ERROR\n", data->tzd);
-		return PTR_ERR(data->tzd);
-	}
-
 	/*
 	 * Try enabling the regulator if found
 	 * TODO: Add regulator as an SOC feature, so that regulator enable
@@ -1365,21 +1358,36 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		break;
 	};
 
+	/*
+	 * data->tzd must be registered before calling exynos_tmu_initialize(),
+	 * requesting irq and calling exynos_tmu_control().
+	 */
+	data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
+						    &exynos_sensor_ops);
+	if (IS_ERR(data->tzd)) {
+		ret = PTR_ERR(data->tzd);
+		dev_err(&pdev->dev, "Failed to register sensor: %d\n", ret);
+		goto err_sclk;
+	}
+
 	ret = exynos_tmu_initialize(pdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize TMU\n");
-		goto err_sclk;
+		goto err_thermal;
 	}
 
 	ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq,
 		IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
-		goto err_sclk;
+		goto err_thermal;
 	}
 
 	exynos_tmu_control(pdev, true);
 	return 0;
+
+err_thermal:
+	thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
 err_sclk:
 	clk_disable_unprepare(data->sclk);
 err_clk:
@@ -1390,7 +1398,6 @@ err_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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL
  2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
  2015-10-08  5:34 ` [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor Krzysztof Kozlowski
@ 2015-10-08  5:34 ` Krzysztof Kozlowski
  2015-10-08 16:46   ` Alim Akhtar
  2015-10-08  5:34 ` [PATCH 4/5] thermal: exynos: Remove unneeded semicolon Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-08  5:34 UTC (permalink / raw)
  To: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	Krzysztof Kozlowski, linux-pm, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

The NULL check in probe's error path is not needed because in that time
the regulator cannot be NULL (regulator_get() returns valid pointer or
ERR_PTR).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index bc71a61f0c4a..eac6aebf82f3 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -1396,7 +1396,7 @@ err_clk_sec:
 	if (!IS_ERR(data->clk_sec))
 		clk_unprepare(data->clk_sec);
 err_sensor:
-	if (!IS_ERR_OR_NULL(data->regulator))
+	if (!IS_ERR(data->regulator))
 		regulator_disable(data->regulator);
 
 	return ret;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] thermal: exynos: Remove unneeded semicolon
  2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
  2015-10-08  5:34 ` [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor Krzysztof Kozlowski
  2015-10-08  5:34 ` [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL Krzysztof Kozlowski
@ 2015-10-08  5:34 ` Krzysztof Kozlowski
  2015-11-04 10:52   ` Lukasz Majewski
  2015-10-08  5:34 ` [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable Krzysztof Kozlowski
  2015-10-08 16:45 ` [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Alim Akhtar
  4 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-08  5:34 UTC (permalink / raw)
  To: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	Krzysztof Kozlowski, linux-pm, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

Remove semicolons after switch statement.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index eac6aebf82f3..1af7ea8dda71 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -548,7 +548,7 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
 	default:
 		pdata->cal_type = TYPE_ONE_POINT_TRIMMING;
 		break;
-	};
+	}
 
 	dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
 			cal_type ?  2 : 1);
@@ -1356,7 +1356,7 @@ static int exynos_tmu_probe(struct platform_device *pdev)
 		break;
 	default:
 		break;
-	};
+	}
 
 	/*
 	 * data->tzd must be registered before calling exynos_tmu_initialize(),
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable
  2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2015-10-08  5:34 ` [PATCH 4/5] thermal: exynos: Remove unneeded semicolon Krzysztof Kozlowski
@ 2015-10-08  5:34 ` Krzysztof Kozlowski
  2015-10-08 16:47   ` Alim Akhtar
  2015-10-08 16:45 ` [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Alim Akhtar
  4 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-08  5:34 UTC (permalink / raw)
  To: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	Krzysztof Kozlowski, linux-pm, linux-samsung-soc,
	linux-arm-kernel, linux-kernel

The 'ret' variable in exynos5440_tmu_initialize() is initialized to 0
and returned as is. Replace it with direct return statement. This also
fixes coccinelle warning:
drivers/thermal/samsung/exynos_tmu.c:611:5-8: Unneeded variable: "ret". Return "0" on line 654

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 1af7ea8dda71..f340e6edcb49 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -608,7 +608,7 @@ static int exynos5440_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	unsigned int trim_info = 0, con, rising_threshold;
-	int ret = 0, threshold_code;
+	int threshold_code;
 	int crit_temp = 0;
 
 	/*
@@ -651,7 +651,8 @@ static int exynos5440_tmu_initialize(struct platform_device *pdev)
 	/* Clear the PMIN in the common TMU register */
 	if (!data->id)
 		writel(0, data->base_second + EXYNOS5440_TMU_PMIN);
-	return ret;
+
+	return 0;
 }
 
 static int exynos7_tmu_initialize(struct platform_device *pdev)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor
  2015-10-08  5:34 ` [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor Krzysztof Kozlowski
@ 2015-10-08 16:43   ` Alim Akhtar
  2015-11-04 11:18     ` Lukasz Majewski
  0 siblings, 1 reply; 16+ messages in thread
From: Alim Akhtar @ 2015-10-08 16:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hello,

On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> Thermal core could not read the temperature after registering the
> thermal sensor with thermal_zone_of_sensor_register() because the driver
> was not yet initialized.
>
> The call trace looked like:
>     exynos_tmu_probe()
>         thermal_zone_of_sensor_register()
>             of_thermal_set_mode()
>                 thermal_zone_device_update()
>                     exynos_get_temp()
>                         if (!data->tmu_read) return -EINVAL;
>         exynos_map_dt_data()
>             data->tmu_read = ...
>
> This produced an error in dmesg:
> thermal thermal_zone0: failed to read out thermal zone (-22)
>
> Register the thermal_zone_device later, after parsing Device Tree and
> enabling necessary clocks, but before calling exynos_tmu_initialize()
> which uses the registered thermal_zone_device.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: 3b6a1a805f34 ("thermal: samsung: core: Exynos TMU rework to use device tree for configuration")
> ---
Patch looks good to me.

Before this patch I was getting below on exynos5800 board:
[    2.648629] thermal thermal_zone0: failed to read out thermal zone (-22)
[    2.653906] 10060000.tmu supply vtmu not found, using dummy regulator
[    2.660521] thermal thermal_zone1: failed to read out thermal zone (-22)
[    2.666985] 10064000.tmu supply vtmu not found, using dummy regulator
[    2.673658] thermal thermal_zone2: failed to read out thermal zone (-22)
[    2.680093] 10068000.tmu supply vtmu not found, using dummy regulator
[    2.686901] thermal thermal_zone3: failed to read out thermal zone (-22)
[    2.693187] 1006c000.tmu supply vtmu not found, using dummy regulator
[    2.699914] thermal thermal_zone4: failed to read out thermal zone (-22)
[    2.706332] 100a0000.tmu supply vtmu not found, using dummy regulator


Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

Tested on exynos5800 peach board, so
Tested-by: Alim Akhtar <alim.akhtar@samsung.com>


>  drivers/thermal/samsung/exynos_tmu.c | 27 +++++++++++++++++--------
--
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 23f4320f8ef7..bc71a61f0c4a 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -1289,13 +1289,6 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>         platform_set_drvdata(pdev, data);
>         mutex_init(&data->lock);
>
> -       data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
> -                                                   &exynos_sensor_ops);
> -       if (IS_ERR(data->tzd)) {
> -               pr_err("thermal: tz: %p ERROR\n", data->tzd);
> -               return PTR_ERR(data->tzd);
> -       }
> -
>         /*
>          * Try enabling the regulator if found
>          * TODO: Add regulator as an SOC feature, so that regulator enable
> @@ -1365,21 +1358,36 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>                 break;
>         };
>
> +       /*
> +        * data->tzd must be registered before calling exynos_tmu_initialize(),
> +        * requesting irq and calling exynos_tmu_control().
> +        */
> +       data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
> +                                                   &exynos_sensor_ops);
> +       if (IS_ERR(data->tzd)) {
> +               ret = PTR_ERR(data->tzd);
> +               dev_err(&pdev->dev, "Failed to register sensor: %d\n", ret);
> +               goto err_sclk;
> +       }
> +
>         ret = exynos_tmu_initialize(pdev);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to initialize TMU\n");
> -               goto err_sclk;
> +               goto err_thermal;
>         }
>
>         ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq,
>                 IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
> -               goto err_sclk;
> +               goto err_thermal;
>         }
>
>         exynos_tmu_control(pdev, true);
>         return 0;
> +
> +err_thermal:
> +       thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
>  err_sclk:
>         clk_disable_unprepare(data->sclk);
>  err_clk:
> @@ -1390,7 +1398,6 @@ err_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
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure
  2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2015-10-08  5:34 ` [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable Krzysztof Kozlowski
@ 2015-10-08 16:45 ` Alim Akhtar
  2015-10-09 10:58   ` Krzysztof Kozlowski
  4 siblings, 1 reply; 16+ messages in thread
From: Alim Akhtar @ 2015-10-08 16:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable

Hello,

On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> During probe if the regulator could not be enabled, the error exit path
> would still disable it. This could lead to unbalanced counter of
> regulator enable/disable.
>
Do you see a regulator unbalanced reported here during boot? You may
want to add that to commit message.

> The patch moves code for getting and enabling the regulator from
> exynos_map_dt_data() to probe function because it is really not a part
> of getting Device Tree properties.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on probe failure")
> Cc: <stable@vger.kernel.org>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 0bae8cc6c23a..23f4320f8ef7 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -1168,27 +1168,10 @@ static int exynos_map_dt_data(struct platform_device *pdev)
>         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>         struct exynos_tmu_platform_data *pdata;
>         struct resource res;
> -       int ret;
>
>         if (!data || !pdev->dev.of_node)
>                 return -ENODEV;
>
> -       /*
> -        * Try enabling the regulator if found
> -        * TODO: Add regulator as an SOC feature, so that regulator enable
> -        * is a compulsory call.
> -        */
> -       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
> -       if (!IS_ERR(data->regulator)) {
> -               ret = regulator_enable(data->regulator);
> -               if (ret) {
> -                       dev_err(&pdev->dev, "failed to enable vtmu\n");
> -                       return ret;
> -               }
> -       } else {
> -               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
> -       }
> -
>         data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
>         if (data->id < 0)
>                 data->id = 0;
> @@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>                 pr_err("thermal: tz: %p ERROR\n", data->tzd);
>                 return PTR_ERR(data->tzd);
>         }
> +
> +       /*
> +        * Try enabling the regulator if found
> +        * TODO: Add regulator as an SOC feature, so that regulator enable
> +        * is a compulsory call.
> +        */
> +       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
> +       if (!IS_ERR(data->regulator)) {
> +               ret = regulator_enable(data->regulator);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "failed to enable vtmu\n");
> +                       return ret;
> +               }
> +       } else {
> +               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
> +       }
> +
>         ret = exynos_map_dt_data(pdev);
>         if (ret)
>                 goto err_sensor;
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL
  2015-10-08  5:34 ` [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL Krzysztof Kozlowski
@ 2015-10-08 16:46   ` Alim Akhtar
  2015-11-04 11:17     ` Lukasz Majewski
  0 siblings, 1 reply; 16+ messages in thread
From: Alim Akhtar @ 2015-10-08 16:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hello,

On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> The NULL check in probe's error path is not needed because in that time
> the regulator cannot be NULL (regulator_get() returns valid pointer or
> ERR_PTR).
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>  drivers/thermal/samsung/exynos_tmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index bc71a61f0c4a..eac6aebf82f3 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -1396,7 +1396,7 @@ err_clk_sec:
>         if (!IS_ERR(data->clk_sec))
>                 clk_unprepare(data->clk_sec);
>  err_sensor:
> -       if (!IS_ERR_OR_NULL(data->regulator))
> +       if (!IS_ERR(data->regulator))
>                 regulator_disable(data->regulator);
>
>         return ret;
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable
  2015-10-08  5:34 ` [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable Krzysztof Kozlowski
@ 2015-10-08 16:47   ` Alim Akhtar
  2015-11-04 10:51     ` Lukasz Majewski
  0 siblings, 1 reply; 16+ messages in thread
From: Alim Akhtar @ 2015-10-08 16:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lukasz Majewski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hello,

On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> The 'ret' variable in exynos5440_tmu_initialize() is initialized to 0
> and returned as is. Replace it with direct return statement. This also
> fixes coccinelle warning:
> drivers/thermal/samsung/exynos_tmu.c:611:5-8: Unneeded variable: "ret". Return "0" on line 654
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>  drivers/thermal/samsung/exynos_tmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 1af7ea8dda71..f340e6edcb49 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -608,7 +608,7 @@ static int exynos5440_tmu_initialize(struct platform_device *pdev)
>  {
>         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>         unsigned int trim_info = 0, con, rising_threshold;
> -       int ret = 0, threshold_code;
> +       int threshold_code;
>         int crit_temp = 0;
>
>         /*
> @@ -651,7 +651,8 @@ static int exynos5440_tmu_initialize(struct platform_device *pdev)
>         /* Clear the PMIN in the common TMU register */
>         if (!data->id)
>                 writel(0, data->base_second + EXYNOS5440_TMU_PMIN);
> -       return ret;
> +
> +       return 0;
>  }
>
>  static int exynos7_tmu_initialize(struct platform_device *pdev)
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure
  2015-10-08 16:45 ` [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Alim Akhtar
@ 2015-10-09 10:58   ` Krzysztof Kozlowski
  2015-10-10  1:46     ` Alim Akhtar
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2015-10-09 10:58 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: k.kozlowski.k, Lukasz Majewski, Zhang Rui, Eduardo Valentin,
	Kukjin Kim, linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable

W dniu 09.10.2015 o 01:45, Alim Akhtar pisze:
> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> During probe if the regulator could not be enabled, the error exit path
>> would still disable it. This could lead to unbalanced counter of
>> regulator enable/disable.
>>
> Do you see a regulator unbalanced reported here during boot? You may
> want to add that to commit message.

I did not see the warning/error message about unbalanced disable. It
would happen in certain condition only - no other enables of regulator
and count going below 0.

I would have to simulate this error to get the warning message. I don't
think it is worth the effort.

Best regards,
Krzysztof

> 
>> The patch moves code for getting and enabling the regulator from
>> exynos_map_dt_data() to probe function because it is really not a part
>> of getting Device Tree properties.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on probe failure")
>> Cc: <stable@vger.kernel.org>
>> ---
>>  drivers/thermal/samsung/exynos_tmu.c | 34 +++++++++++++++++-----------------
>>  1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index 0bae8cc6c23a..23f4320f8ef7 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -1168,27 +1168,10 @@ static int exynos_map_dt_data(struct platform_device *pdev)
>>         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>>         struct exynos_tmu_platform_data *pdata;
>>         struct resource res;
>> -       int ret;
>>
>>         if (!data || !pdev->dev.of_node)
>>                 return -ENODEV;
>>
>> -       /*
>> -        * Try enabling the regulator if found
>> -        * TODO: Add regulator as an SOC feature, so that regulator enable
>> -        * is a compulsory call.
>> -        */
>> -       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
>> -       if (!IS_ERR(data->regulator)) {
>> -               ret = regulator_enable(data->regulator);
>> -               if (ret) {
>> -                       dev_err(&pdev->dev, "failed to enable vtmu\n");
>> -                       return ret;
>> -               }
>> -       } else {
>> -               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
>> -       }
>> -
>>         data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
>>         if (data->id < 0)
>>                 data->id = 0;
>> @@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>                 pr_err("thermal: tz: %p ERROR\n", data->tzd);
>>                 return PTR_ERR(data->tzd);
>>         }
>> +
>> +       /*
>> +        * Try enabling the regulator if found
>> +        * TODO: Add regulator as an SOC feature, so that regulator enable
>> +        * is a compulsory call.
>> +        */
>> +       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
>> +       if (!IS_ERR(data->regulator)) {
>> +               ret = regulator_enable(data->regulator);
>> +               if (ret) {
>> +                       dev_err(&pdev->dev, "failed to enable vtmu\n");
>> +                       return ret;
>> +               }
>> +       } else {
>> +               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
>> +       }
>> +
>>         ret = exynos_map_dt_data(pdev);
>>         if (ret)
>>                 goto err_sensor;
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure
  2015-10-09 10:58   ` Krzysztof Kozlowski
@ 2015-10-10  1:46     ` Alim Akhtar
  2015-11-04 10:52       ` Lukasz Majewski
  0 siblings, 1 reply; 16+ messages in thread
From: Alim Akhtar @ 2015-10-10  1:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: k.kozlowski.k, Lukasz Majewski, Zhang Rui, Eduardo Valentin,
	Kukjin Kim, linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, stable

Hello,

On Fri, Oct 9, 2015 at 4:28 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 09.10.2015 o 01:45, Alim Akhtar pisze:
>> Hello,
>>
>> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> During probe if the regulator could not be enabled, the error exit path
>>> would still disable it. This could lead to unbalanced counter of
>>> regulator enable/disable.
>>>
>> Do you see a regulator unbalanced reported here during boot? You may
>> want to add that to commit message.
>
> I did not see the warning/error message about unbalanced disable. It
> would happen in certain condition only - no other enables of regulator
> and count going below 0.
>
> I would have to simulate this error to get the warning message. I don't
> think it is worth the effort.
>
Ok, looking at code, it does looks to be calling regulator disable in
case regulator enable fails.
Feel free to add
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Thanks!!

> Best regards,
> Krzysztof
>
>>
>>> The patch moves code for getting and enabling the regulator from
>>> exynos_map_dt_data() to probe function because it is really not a part
>>> of getting Device Tree properties.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on probe failure")
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>  drivers/thermal/samsung/exynos_tmu.c | 34 +++++++++++++++++-----------------
>>>  1 file changed, 17 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>> index 0bae8cc6c23a..23f4320f8ef7 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>> @@ -1168,27 +1168,10 @@ static int exynos_map_dt_data(struct platform_device *pdev)
>>>         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>>>         struct exynos_tmu_platform_data *pdata;
>>>         struct resource res;
>>> -       int ret;
>>>
>>>         if (!data || !pdev->dev.of_node)
>>>                 return -ENODEV;
>>>
>>> -       /*
>>> -        * Try enabling the regulator if found
>>> -        * TODO: Add regulator as an SOC feature, so that regulator enable
>>> -        * is a compulsory call.
>>> -        */
>>> -       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
>>> -       if (!IS_ERR(data->regulator)) {
>>> -               ret = regulator_enable(data->regulator);
>>> -               if (ret) {
>>> -                       dev_err(&pdev->dev, "failed to enable vtmu\n");
>>> -                       return ret;
>>> -               }
>>> -       } else {
>>> -               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
>>> -       }
>>> -
>>>         data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
>>>         if (data->id < 0)
>>>                 data->id = 0;
>>> @@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>>                 pr_err("thermal: tz: %p ERROR\n", data->tzd);
>>>                 return PTR_ERR(data->tzd);
>>>         }
>>> +
>>> +       /*
>>> +        * Try enabling the regulator if found
>>> +        * TODO: Add regulator as an SOC feature, so that regulator enable
>>> +        * is a compulsory call.
>>> +        */
>>> +       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
>>> +       if (!IS_ERR(data->regulator)) {
>>> +               ret = regulator_enable(data->regulator);
>>> +               if (ret) {
>>> +                       dev_err(&pdev->dev, "failed to enable vtmu\n");
>>> +                       return ret;
>>> +               }
>>> +       } else {
>>> +               dev_info(&pdev->dev, "Regulator node (vtmu) not found\n");
>>> +       }
>>> +
>>>         ret = exynos_map_dt_data(pdev);
>>>         if (ret)
>>>                 goto err_sensor;
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>>
>



-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable
  2015-10-08 16:47   ` Alim Akhtar
@ 2015-11-04 10:51     ` Lukasz Majewski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Majewski @ 2015-11-04 10:51 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > The 'ret' variable in exynos5440_tmu_initialize() is initialized to
> > 0 and returned as is. Replace it with direct return statement. This
> > also fixes coccinelle warning:
> > drivers/thermal/samsung/exynos_tmu.c:611:5-8: Unneeded variable:
> > "ret". Return "0" on line 654
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > 1af7ea8dda71..f340e6edcb49 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -608,7 +608,7 @@ static
> > int exynos5440_tmu_initialize(struct platform_device *pdev) {
> >         struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> >         unsigned int trim_info = 0, con, rising_threshold;
> > -       int ret = 0, threshold_code;
> > +       int threshold_code;
> >         int crit_temp = 0;
> >
> >         /*
> > @@ -651,7 +651,8 @@ static int exynos5440_tmu_initialize(struct
> > platform_device *pdev) /* Clear the PMIN in the common TMU register
> > */ if (!data->id)
> >                 writel(0, data->base_second + EXYNOS5440_TMU_PMIN);
> > -       return ret;
> > +
> > +       return 0;
> >  }
> >
> >  static int exynos7_tmu_initialize(struct platform_device *pdev)
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majordomo@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

Test HW: Odroid XU3 - Exynos5433

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure
  2015-10-10  1:46     ` Alim Akhtar
@ 2015-11-04 10:52       ` Lukasz Majewski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Majewski @ 2015-11-04 10:52 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-samsung-soc@vger.kernel.org, linux-pm,
	k.kozlowski.k, linux-kernel@vger.kernel.org, stable,
	Eduardo Valentin, Kukjin Kim, Zhang Rui,
	linux-arm-kernel@lists.infradead.org

Hi Alim,

> Hello,
> 
> On Fri, Oct 9, 2015 at 4:28 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > W dniu 09.10.2015 o 01:45, Alim Akhtar pisze:
> >> Hello,
> >>
> >> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> >> <k.kozlowski@samsung.com> wrote:
> >>> During probe if the regulator could not be enabled, the error
> >>> exit path would still disable it. This could lead to unbalanced
> >>> counter of regulator enable/disable.
> >>>
> >> Do you see a regulator unbalanced reported here during boot? You
> >> may want to add that to commit message.
> >
> > I did not see the warning/error message about unbalanced disable. It
> > would happen in certain condition only - no other enables of
> > regulator and count going below 0.
> >
> > I would have to simulate this error to get the warning message. I
> > don't think it is worth the effort.
> >
> Ok, looking at code, it does looks to be calling regulator disable in
> case regulator enable fails.
> Feel free to add
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> Thanks!!
> 
> > Best regards,
> > Krzysztof
> >
> >>
> >>> The patch moves code for getting and enabling the regulator from
> >>> exynos_map_dt_data() to probe function because it is really not a
> >>> part of getting Device Tree properties.
> >>>
> >>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >>> Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on
> >>> probe failure") Cc: <stable@vger.kernel.org>
> >>> ---
> >>>  drivers/thermal/samsung/exynos_tmu.c | 34
> >>> +++++++++++++++++----------------- 1 file changed, 17
> >>> insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> >>> b/drivers/thermal/samsung/exynos_tmu.c index
> >>> 0bae8cc6c23a..23f4320f8ef7 100644 ---
> >>> a/drivers/thermal/samsung/exynos_tmu.c +++
> >>> b/drivers/thermal/samsung/exynos_tmu.c @@ -1168,27 +1168,10 @@
> >>> static int exynos_map_dt_data(struct platform_device *pdev)
> >>> struct exynos_tmu_data *data = platform_get_drvdata(pdev); struct
> >>> exynos_tmu_platform_data *pdata; struct resource res;
> >>> -       int ret;
> >>>
> >>>         if (!data || !pdev->dev.of_node)
> >>>                 return -ENODEV;
> >>>
> >>> -       /*
> >>> -        * Try enabling the regulator if found
> >>> -        * TODO: Add regulator as an SOC feature, so that
> >>> regulator enable
> >>> -        * is a compulsory call.
> >>> -        */
> >>> -       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
> >>> -       if (!IS_ERR(data->regulator)) {
> >>> -               ret = regulator_enable(data->regulator);
> >>> -               if (ret) {
> >>> -                       dev_err(&pdev->dev, "failed to enable
> >>> vtmu\n");
> >>> -                       return ret;
> >>> -               }
> >>> -       } else {
> >>> -               dev_info(&pdev->dev, "Regulator node (vtmu) not
> >>> found\n");
> >>> -       }
> >>> -
> >>>         data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
> >>>         if (data->id < 0)
> >>>                 data->id = 0;
> >>> @@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct
> >>> platform_device *pdev) pr_err("thermal: tz: %p ERROR\n",
> >>> data->tzd); return PTR_ERR(data->tzd);
> >>>         }
> >>> +
> >>> +       /*
> >>> +        * Try enabling the regulator if found
> >>> +        * TODO: Add regulator as an SOC feature, so that
> >>> regulator enable
> >>> +        * is a compulsory call.
> >>> +        */
> >>> +       data->regulator = devm_regulator_get(&pdev->dev, "vtmu");
> >>> +       if (!IS_ERR(data->regulator)) {
> >>> +               ret = regulator_enable(data->regulator);
> >>> +               if (ret) {
> >>> +                       dev_err(&pdev->dev, "failed to enable
> >>> vtmu\n");
> >>> +                       return ret;
> >>> +               }
> >>> +       } else {
> >>> +               dev_info(&pdev->dev, "Regulator node (vtmu) not
> >>> found\n");
> >>> +       }
> >>> +
> >>>         ret = exynos_map_dt_data(pdev);
> >>>         if (ret)
> >>>                 goto err_sensor;
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>> _______________________________________________
> >>> linux-arm-kernel mailing list
> >>> linux-arm-kernel@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> >>
> >>
> >
> 
> 
> 

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] thermal: exynos: Remove unneeded semicolon
  2015-10-08  5:34 ` [PATCH 4/5] thermal: exynos: Remove unneeded semicolon Krzysztof Kozlowski
@ 2015-11-04 10:52   ` Lukasz Majewski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Majewski @ 2015-11-04 10:52 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Zhang Rui, Eduardo Valentin, Kukjin Kim, linux-pm,
	linux-samsung-soc, linux-arm-kernel, linux-kernel

Hi Krzysztof,

> Remove semicolons after switch statement.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c index
> eac6aebf82f3..1af7ea8dda71 100644 ---
> a/drivers/thermal/samsung/exynos_tmu.c +++
> b/drivers/thermal/samsung/exynos_tmu.c @@ -548,7 +548,7 @@ static int
> exynos5433_tmu_initialize(struct platform_device *pdev) default:
>  		pdata->cal_type = TYPE_ONE_POINT_TRIMMING;
>  		break;
> -	};
> +	}
>  
>  	dev_info(&pdev->dev, "Calibration type is %d-point
> calibration\n", cal_type ?  2 : 1);
> @@ -1356,7 +1356,7 @@ static int exynos_tmu_probe(struct
> platform_device *pdev) break;
>  	default:
>  		break;
> -	};
> +	}
>  
>  	/*
>  	 * data->tzd must be registered before calling
> exynos_tmu_initialize(),

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL
  2015-10-08 16:46   ` Alim Akhtar
@ 2015-11-04 11:17     ` Lukasz Majewski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Majewski @ 2015-11-04 11:17 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > The NULL check in probe's error path is not needed because in that
> > time the regulator cannot be NULL (regulator_get() returns valid
> > pointer or ERR_PTR).
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > bc71a61f0c4a..eac6aebf82f3 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -1396,7 +1396,7 @@
> > err_clk_sec: if (!IS_ERR(data->clk_sec))
> >                 clk_unprepare(data->clk_sec);
> >  err_sensor:
> > -       if (!IS_ERR_OR_NULL(data->regulator))
> > +       if (!IS_ERR(data->regulator))
> >                 regulator_disable(data->regulator);
> >
> >         return ret;
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majordomo@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor
  2015-10-08 16:43   ` Alim Akhtar
@ 2015-11-04 11:18     ` Lukasz Majewski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Majewski @ 2015-11-04 11:18 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Krzysztof Kozlowski, Zhang Rui, Eduardo Valentin, Kukjin Kim,
	linux-pm, linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > Thermal core could not read the temperature after registering the
> > thermal sensor with thermal_zone_of_sensor_register() because the
> > driver was not yet initialized.
> >
> > The call trace looked like:
> >     exynos_tmu_probe()
> >         thermal_zone_of_sensor_register()
> >             of_thermal_set_mode()
> >                 thermal_zone_device_update()
> >                     exynos_get_temp()
> >                         if (!data->tmu_read) return -EINVAL;
> >         exynos_map_dt_data()
> >             data->tmu_read = ...
> >
> > This produced an error in dmesg:
> > thermal thermal_zone0: failed to read out thermal zone (-22)
> >
> > Register the thermal_zone_device later, after parsing Device Tree
> > and enabling necessary clocks, but before calling
> > exynos_tmu_initialize() which uses the registered
> > thermal_zone_device.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Fixes: 3b6a1a805f34 ("thermal: samsung: core: Exynos TMU rework to
> > use device tree for configuration") ---
> Patch looks good to me.
> 
> Before this patch I was getting below on exynos5800 board:
> [    2.648629] thermal thermal_zone0: failed to read out thermal zone
> (-22) [    2.653906] 10060000.tmu supply vtmu not found, using dummy
> regulator [    2.660521] thermal thermal_zone1: failed to read out
> thermal zone (-22) [    2.666985] 10064000.tmu supply vtmu not found,
> using dummy regulator [    2.673658] thermal thermal_zone2: failed to
> read out thermal zone (-22) [    2.680093] 10068000.tmu supply vtmu
> not found, using dummy regulator [    2.686901] thermal
> thermal_zone3: failed to read out thermal zone (-22) [    2.693187]
> 1006c000.tmu supply vtmu not found, using dummy regulator
> [    2.699914] thermal thermal_zone4: failed to read out thermal zone
> (-22) [    2.706332] 100a0000.tmu supply vtmu not found, using dummy
> regulator
> 
> 
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> 
> Tested on exynos5800 peach board, so
> Tested-by: Alim Akhtar <alim.akhtar@samsung.com>
> 
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 27 +++++++++++++++++--------
> --
> >  1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > 23f4320f8ef7..bc71a61f0c4a 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -1289,13 +1289,6 @@
> > static int exynos_tmu_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, data); mutex_init(&data->lock);
> >
> > -       data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0,
> > data,
> > -
> > &exynos_sensor_ops);
> > -       if (IS_ERR(data->tzd)) {
> > -               pr_err("thermal: tz: %p ERROR\n", data->tzd);
> > -               return PTR_ERR(data->tzd);
> > -       }
> > -
> >         /*
> >          * Try enabling the regulator if found
> >          * TODO: Add regulator as an SOC feature, so that regulator
> > enable @@ -1365,21 +1358,36 @@ static int exynos_tmu_probe(struct
> > platform_device *pdev) break;
> >         };
> >
> > +       /*
> > +        * data->tzd must be registered before calling
> > exynos_tmu_initialize(),
> > +        * requesting irq and calling exynos_tmu_control().
> > +        */
> > +       data->tzd = thermal_zone_of_sensor_register(&pdev->dev, 0,
> > data,
> > +
> > &exynos_sensor_ops);
> > +       if (IS_ERR(data->tzd)) {
> > +               ret = PTR_ERR(data->tzd);
> > +               dev_err(&pdev->dev, "Failed to register sensor:
> > %d\n", ret);
> > +               goto err_sclk;
> > +       }
> > +
> >         ret = exynos_tmu_initialize(pdev);
> >         if (ret) {
> >                 dev_err(&pdev->dev, "Failed to initialize TMU\n");
> > -               goto err_sclk;
> > +               goto err_thermal;
> >         }
> >
> >         ret = devm_request_irq(&pdev->dev, data->irq,
> > exynos_tmu_irq, IRQF_TRIGGER_RISING | IRQF_SHARED,
> > dev_name(&pdev->dev), data); if (ret) {
> >                 dev_err(&pdev->dev, "Failed to request irq: %d\n",
> > data->irq);
> > -               goto err_sclk;
> > +               goto err_thermal;
> >         }
> >
> >         exynos_tmu_control(pdev, true);
> >         return 0;
> > +
> > +err_thermal:
> > +       thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd);
> >  err_sclk:
> >         clk_disable_unprepare(data->sclk);
> >  err_clk:
> > @@ -1390,7 +1398,6 @@ err_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
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> 

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-11-04 11:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08  5:34 [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Krzysztof Kozlowski
2015-10-08  5:34 ` [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor Krzysztof Kozlowski
2015-10-08 16:43   ` Alim Akhtar
2015-11-04 11:18     ` Lukasz Majewski
2015-10-08  5:34 ` [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL Krzysztof Kozlowski
2015-10-08 16:46   ` Alim Akhtar
2015-11-04 11:17     ` Lukasz Majewski
2015-10-08  5:34 ` [PATCH 4/5] thermal: exynos: Remove unneeded semicolon Krzysztof Kozlowski
2015-11-04 10:52   ` Lukasz Majewski
2015-10-08  5:34 ` [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable Krzysztof Kozlowski
2015-10-08 16:47   ` Alim Akhtar
2015-11-04 10:51     ` Lukasz Majewski
2015-10-08 16:45 ` [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure Alim Akhtar
2015-10-09 10:58   ` Krzysztof Kozlowski
2015-10-10  1:46     ` Alim Akhtar
2015-11-04 10:52       ` Lukasz Majewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).