diff mbox series

[V3,5/6] thermal: rcar_thermal: Convert to devm_thermal_zone_of_sensor_register_params()

Message ID 20181217155644.29278-6-marek.vasut@gmail.com
State Under Review
Delegated to: Geert Uytterhoeven
Headers show
Series thermal: Align devm_thermal_zone_{device,of_sensor}_register | expand

Commit Message

Marek Vasut Dec. 17, 2018, 3:56 p.m. UTC
From: Marek Vasut <marek.vasut@gmail.com>

Convert the rcar code to devm_thermal_zone_of_sensor_register_params(),
no functional change.

From: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-renesas-soc@vger.kernel.org
To: linux-pm@vger.kernel.org
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
---
V2: No change
V3: - Work around the From line and SoB line checkpatch warning
    - Reorder the SoB line at the end
---
 drivers/thermal/rcar_thermal.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

Comments

Simon Horman Dec. 18, 2018, 11:05 a.m. UTC | #1
On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(),
> no functional change.
> 
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-renesas-soc@vger.kernel.org
> To: linux-pm@vger.kernel.org
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> ---
> V2: No change
> V3: - Work around the From line and SoB line checkpatch warning
>     - Reorder the SoB line at the end

As per v2:

This patch looks good to me, though I'm not sure why { } need
to be introduced into the 4th hunk.

That notwithstanding:

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/thermal/rcar_thermal.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 8014a207d8d9..ec70c2119c77 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -19,8 +19,6 @@
>  #include <linux/spinlock.h>
>  #include <linux/thermal.h>
>  
> -#include "thermal_hwmon.h"
> -
>  #define IDLE_INTERVAL	5000
>  
>  #define COMMON_STR	0x00
> @@ -54,6 +52,10 @@ struct rcar_thermal_chip {
>  	unsigned int nirqs;
>  };
>  
> +static struct thermal_zone_params rcar_thermal_params = {
> +	.no_hwmon	= false,
> +};
> +
>  static const struct rcar_thermal_chip rcar_thermal = {
>  	.use_of_thermal = 0,
>  	.has_filonoff = 1,
> @@ -458,9 +460,7 @@ static int rcar_thermal_remove(struct platform_device *pdev)
>  	rcar_thermal_for_each_priv(priv, common) {
>  		rcar_thermal_irq_disable(priv);
>  		cancel_delayed_work_sync(&priv->work);
> -		if (priv->chip->use_of_thermal)
> -			thermal_remove_hwmon_sysfs(priv->zone);
> -		else
> +		if (!priv->chip->use_of_thermal)
>  			thermal_zone_device_unregister(priv->zone);
>  	}
>  
> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  		if (ret < 0)
>  			goto error_unregister;
>  
> -		if (chip->use_of_thermal)
> -			priv->zone = devm_thermal_zone_of_sensor_register(
> +		if (chip->use_of_thermal) {
> +			priv->zone =
> +				devm_thermal_zone_of_sensor_register_params(
>  						dev, i, priv,
> -						&rcar_thermal_zone_of_ops);
> -		else
> +						&rcar_thermal_zone_of_ops,
> +						&rcar_thermal_params);
> +		} else {
>  			priv->zone = thermal_zone_device_register(
>  						"rcar_thermal",
>  						1, 0, priv,
>  						&rcar_thermal_zone_ops, NULL, 0,
>  						idle);
> +		}
> +
>  		if (IS_ERR(priv->zone)) {
>  			dev_err(dev, "can't register thermal zone\n");
>  			ret = PTR_ERR(priv->zone);
> @@ -571,17 +575,6 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  			goto error_unregister;
>  		}
>  
> -		if (chip->use_of_thermal) {
> -			/*
> -			 * thermal_zone doesn't enable hwmon as default,
> -			 * but, enable it here to keep compatible
> -			 */
> -			priv->zone->tzp->no_hwmon = false;
> -			ret = thermal_add_hwmon_sysfs(priv->zone);
> -			if (ret)
> -				goto error_unregister;
> -		}
> -
>  		rcar_thermal_irq_enable(priv);
>  
>  		list_move_tail(&priv->list, &common->head);
> -- 
> 2.18.0
>
Marek Vasut Dec. 19, 2018, 11:25 p.m. UTC | #2
On 12/18/2018 12:05 PM, Simon Horman wrote:
> On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote:
>> From: Marek Vasut <marek.vasut@gmail.com>
>>
>> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(),
>> no functional change.
>>
>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Eduardo Valentin <edubezval@gmail.com>
>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> To: linux-pm@vger.kernel.org
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>> ---
>> V2: No change
>> V3: - Work around the From line and SoB line checkpatch warning
>>     - Reorder the SoB line at the end
> 
> As per v2:
> 
> This patch looks good to me, though I'm not sure why { } need
> to be introduced into the 4th hunk.

Because it's a multi-line code , even though it's just a single
line-wrapped function call. I can drop that part, but I think it makes
it visually far more obvious where the conditional block starts/ends and
I recall seeing something about this in kernel coding style too.

[...]

>> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>>  		if (ret < 0)
>>  			goto error_unregister;
>>  
>> -		if (chip->use_of_thermal)
>> -			priv->zone = devm_thermal_zone_of_sensor_register(
>> +		if (chip->use_of_thermal) {
>> +			priv->zone =
>> +				devm_thermal_zone_of_sensor_register_params(
>>  						dev, i, priv,
>> -						&rcar_thermal_zone_of_ops);
>> -		else
>> +						&rcar_thermal_zone_of_ops,
>> +						&rcar_thermal_params);
>> +		} else {
>>  			priv->zone = thermal_zone_device_register(
>>  						"rcar_thermal",
>>  						1, 0, priv,
>>  						&rcar_thermal_zone_ops, NULL, 0,
>>  						idle);
>> +		}
>> +
>>  		if (IS_ERR(priv->zone)) {
>>  			dev_err(dev, "can't register thermal zone\n");
>>  			ret = PTR_ERR(priv->zone);
[...]
Simon Horman Dec. 20, 2018, 7:46 a.m. UTC | #3
On Thu, Dec 20, 2018 at 12:25:17AM +0100, Marek Vasut wrote:
> On 12/18/2018 12:05 PM, Simon Horman wrote:
> > On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote:
> >> From: Marek Vasut <marek.vasut@gmail.com>
> >>
> >> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(),
> >> no functional change.
> >>
> >> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Cc: Eduardo Valentin <edubezval@gmail.com>
> >> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Cc: Zhang Rui <rui.zhang@intel.com>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> To: linux-pm@vger.kernel.org
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> ---
> >> V2: No change
> >> V3: - Work around the From line and SoB line checkpatch warning
> >>     - Reorder the SoB line at the end
> > 
> > As per v2:
> > 
> > This patch looks good to me, though I'm not sure why { } need
> > to be introduced into the 4th hunk.
> 
> Because it's a multi-line code , even though it's just a single
> line-wrapped function call. I can drop that part, but I think it makes
> it visually far more obvious where the conditional block starts/ends and
> I recall seeing something about this in kernel coding style too.

I lean towards removing {} but I do not feel at all strongly about this.

> 
> [...]
> 
> >> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev)
> >>  		if (ret < 0)
> >>  			goto error_unregister;
> >>  
> >> -		if (chip->use_of_thermal)
> >> -			priv->zone = devm_thermal_zone_of_sensor_register(
> >> +		if (chip->use_of_thermal) {
> >> +			priv->zone =
> >> +				devm_thermal_zone_of_sensor_register_params(
> >>  						dev, i, priv,
> >> -						&rcar_thermal_zone_of_ops);
> >> -		else
> >> +						&rcar_thermal_zone_of_ops,
> >> +						&rcar_thermal_params);
> >> +		} else {
> >>  			priv->zone = thermal_zone_device_register(
> >>  						"rcar_thermal",
> >>  						1, 0, priv,
> >>  						&rcar_thermal_zone_ops, NULL, 0,
> >>  						idle);
> >> +		}
> >> +
> >>  		if (IS_ERR(priv->zone)) {
> >>  			dev_err(dev, "can't register thermal zone\n");
> >>  			ret = PTR_ERR(priv->zone);
> [...]
> 
> -- 
> Best regards,
> Marek Vasut
>
Marek Vasut Dec. 20, 2018, 11:13 p.m. UTC | #4
On 12/20/2018 08:46 AM, Simon Horman wrote:
> On Thu, Dec 20, 2018 at 12:25:17AM +0100, Marek Vasut wrote:
>> On 12/18/2018 12:05 PM, Simon Horman wrote:
>>> On Mon, Dec 17, 2018 at 04:56:43PM +0100, marek.vasut@gmail.com wrote:
>>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>>
>>>> Convert the rcar code to devm_thermal_zone_of_sensor_register_params(),
>>>> no functional change.
>>>>
>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> Cc: Eduardo Valentin <edubezval@gmail.com>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: Zhang Rui <rui.zhang@intel.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> To: linux-pm@vger.kernel.org
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> ---
>>>> V2: No change
>>>> V3: - Work around the From line and SoB line checkpatch warning
>>>>     - Reorder the SoB line at the end
>>>
>>> As per v2:
>>>
>>> This patch looks good to me, though I'm not sure why { } need
>>> to be introduced into the 4th hunk.
>>
>> Because it's a multi-line code , even though it's just a single
>> line-wrapped function call. I can drop that part, but I think it makes
>> it visually far more obvious where the conditional block starts/ends and
>> I recall seeing something about this in kernel coding style too.
> 
> I lean towards removing {} but I do not feel at all strongly about this.

Well does it improve the readability if they are removed ?

>> [...]
>>
>>>> @@ -554,16 +554,20 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>>>>  		if (ret < 0)
>>>>  			goto error_unregister;
>>>>  
>>>> -		if (chip->use_of_thermal)
>>>> -			priv->zone = devm_thermal_zone_of_sensor_register(
>>>> +		if (chip->use_of_thermal) {
>>>> +			priv->zone =
>>>> +				devm_thermal_zone_of_sensor_register_params(
>>>>  						dev, i, priv,
>>>> -						&rcar_thermal_zone_of_ops);
>>>> -		else
>>>> +						&rcar_thermal_zone_of_ops,
>>>> +						&rcar_thermal_params);
>>>> +		} else {
>>>>  			priv->zone = thermal_zone_device_register(
>>>>  						"rcar_thermal",
>>>>  						1, 0, priv,
>>>>  						&rcar_thermal_zone_ops, NULL, 0,
>>>>  						idle);
>>>> +		}
>>>> +
>>>>  		if (IS_ERR(priv->zone)) {
>>>>  			dev_err(dev, "can't register thermal zone\n");
>>>>  			ret = PTR_ERR(priv->zone);
>> [...]
>>
>> -- 
>> Best regards,
>> Marek Vasut
>>
diff mbox series

Patch

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 8014a207d8d9..ec70c2119c77 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -19,8 +19,6 @@ 
 #include <linux/spinlock.h>
 #include <linux/thermal.h>
 
-#include "thermal_hwmon.h"
-
 #define IDLE_INTERVAL	5000
 
 #define COMMON_STR	0x00
@@ -54,6 +52,10 @@  struct rcar_thermal_chip {
 	unsigned int nirqs;
 };
 
+static struct thermal_zone_params rcar_thermal_params = {
+	.no_hwmon	= false,
+};
+
 static const struct rcar_thermal_chip rcar_thermal = {
 	.use_of_thermal = 0,
 	.has_filonoff = 1,
@@ -458,9 +460,7 @@  static int rcar_thermal_remove(struct platform_device *pdev)
 	rcar_thermal_for_each_priv(priv, common) {
 		rcar_thermal_irq_disable(priv);
 		cancel_delayed_work_sync(&priv->work);
-		if (priv->chip->use_of_thermal)
-			thermal_remove_hwmon_sysfs(priv->zone);
-		else
+		if (!priv->chip->use_of_thermal)
 			thermal_zone_device_unregister(priv->zone);
 	}
 
@@ -554,16 +554,20 @@  static int rcar_thermal_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto error_unregister;
 
-		if (chip->use_of_thermal)
-			priv->zone = devm_thermal_zone_of_sensor_register(
+		if (chip->use_of_thermal) {
+			priv->zone =
+				devm_thermal_zone_of_sensor_register_params(
 						dev, i, priv,
-						&rcar_thermal_zone_of_ops);
-		else
+						&rcar_thermal_zone_of_ops,
+						&rcar_thermal_params);
+		} else {
 			priv->zone = thermal_zone_device_register(
 						"rcar_thermal",
 						1, 0, priv,
 						&rcar_thermal_zone_ops, NULL, 0,
 						idle);
+		}
+
 		if (IS_ERR(priv->zone)) {
 			dev_err(dev, "can't register thermal zone\n");
 			ret = PTR_ERR(priv->zone);
@@ -571,17 +575,6 @@  static int rcar_thermal_probe(struct platform_device *pdev)
 			goto error_unregister;
 		}
 
-		if (chip->use_of_thermal) {
-			/*
-			 * thermal_zone doesn't enable hwmon as default,
-			 * but, enable it here to keep compatible
-			 */
-			priv->zone->tzp->no_hwmon = false;
-			ret = thermal_add_hwmon_sysfs(priv->zone);
-			if (ret)
-				goto error_unregister;
-		}
-
 		rcar_thermal_irq_enable(priv);
 
 		list_move_tail(&priv->list, &common->head);