[PATCH V3 0/6] thermal: Align devm_thermal_zone_{device,of_sensor}_register

[PATCH V3 0/6] thermal: Align devm_thermal_zone_{device,of_sensor}_register

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

This patchset attempts to align devm_thermal_zone_{device,of_sensor}_register()
functions and allow passing tz params to devm_thermal_zone_of_sensor_register()
to allow settings no_hwmon params without digging into the tz structures.

The approach is as follows:
1) Add {devm_,}thermal_zone_of_sensor_register_params(), which behaves just
   like the old variant without _params() suffix, except it takes the struct
   thermal_zone_params as an argument. The old _params()-less version of the
   function is converted to use the new function and pass params = NULL.
   The old _params()-less versions of the functions are retained to avoid
   breaking the ABI.
2) Add the no_hwmon handling into thermal_zone_of_sensor_register_params(),
   that's patch 3.
3) Convert drivers from devm_thermal_zone_of_sensor_register() with structure
   diggings to devm_thermal_zone_of_sensor_register_params() without any
   extra structure digging, that's the rest of the patches.

Marek Vasut (6):
  thermal: split thermal_zone_of_sensor_register{,_param}()
  thermal: split devm_thermal_zone_of_sensor_register{,_param}()
  thermal: Register hwmon in thermal_zone_of_sensor_register_param()
  thermal: stm32: Convert to
    devm_thermal_zone_of_sensor_register_params()
  thermal: rcar_thermal: Convert to
    devm_thermal_zone_of_sensor_register_params()
  thermal: rcar_gen3_thermal: Register hwmon sysfs interface

 drivers/thermal/of-thermal.c        | 106 +++++++++++++++++++++++++---
 drivers/thermal/rcar_gen3_thermal.c |   8 ++-
 drivers/thermal/rcar_thermal.c      |  33 ++++-----
 drivers/thermal/st/stm_thermal.c    |  20 ++----
 include/linux/thermal.h             |  25 +++++++
 5 files changed, 146 insertions(+), 46 deletions(-)

V2: Minor fixes to address feedback from the bot
V3: - Work around the From line and SoB line checkpatch warning
    - Reorder the SoB line at the end

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

-- 
2.18.0

[PATCH V3 1/6] thermal: split thermal_zone_of_sensor_register{,_param}()

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

Introduce new thermal_zone_of_sensor_register_params() function, which
allows passing struct thermal_zone_params into it and convert original
thermal_zone_of_sensor_register() to call it with params set to NULL.

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: Add EXPORT_SYMBOL_GPL() to fix build as a module
V3: - Work around the From line and SoB line checkpatch warning
    - Reorder the SoB line at the end
---
 drivers/thermal/of-thermal.c | 51 +++++++++++++++++++++++++++++++++---
 include/linux/thermal.h      | 12 +++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 4bfdb4a1e47d..1fb450d02ab5 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -446,7 +446,8 @@ thermal_zone_of_add_sensor(struct device_node *zone,
 }
 
 /**
- * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone
+ * thermal_zone_of_sensor_register_params - registers a sensor to a DT thermal
+ *					zone with thermal zone parameters
  * @dev: a valid struct device pointer of a sensor device. Must contain
  *       a valid .of_node, for the sensor node.
  * @sensor_id: a sensor identifier, in case the sensor IP has more
@@ -454,6 +455,7 @@ thermal_zone_of_add_sensor(struct device_node *zone,
  * @data: a private pointer (owned by the caller) that will be passed
  *        back, when a temperature reading is needed.
  * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ * @tzp: thermal zone platform parameters
  *
  * This function will search the list of thermal zones described in device
  * tree and look for the zone that refer to the sensor device pointed by
@@ -478,8 +480,9 @@ thermal_zone_of_add_sensor(struct device_node *zone,
  * check the return value with help of IS_ERR() helper.
  */
 struct thermal_zone_device *
-thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
-				const struct thermal_zone_of_device_ops *ops)
+thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
+	void *data, const struct thermal_zone_of_device_ops *ops,
+	struct thermal_zone_params *tzp)
 {
 	struct device_node *np, *child, *sensor_np;
 	struct thermal_zone_device *tzd = ERR_PTR(-ENODEV);
@@ -533,6 +536,48 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
 
 	return tzd;
 }
+EXPORT_SYMBOL_GPL(thermal_zone_of_sensor_register_params);
+
+/**
+ * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone
+ * @dev: a valid struct device pointer of a sensor device. Must contain
+ *       a valid .of_node, for the sensor node.
+ * @sensor_id: a sensor identifier, in case the sensor IP has more
+ *             than one sensors
+ * @data: a private pointer (owned by the caller) that will be passed
+ *        back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * This function will search the list of thermal zones described in device
+ * tree and look for the zone that refer to the sensor device pointed by
+ * @dev->of_node as temperature providers. For the zone pointing to the
+ * sensor node, the sensor will be added to the DT thermal zone device.
+ *
+ * The thermal zone temperature is provided by the @get_temp function
+ * pointer. When called, it will have the private pointer @data back.
+ *
+ * The thermal zone temperature trend is provided by the @get_trend function
+ * pointer. When called, it will have the private pointer @data back.
+ *
+ * TODO:
+ * 01 - This function must enqueue the new sensor instead of using
+ * it as the only source of temperature values.
+ *
+ * 02 - There must be a way to match the sensor with all thermal zones
+ * that refer to it.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ */
+
+struct thermal_zone_device *
+thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
+				const struct thermal_zone_of_device_ops *ops)
+{
+	return thermal_zone_of_sensor_register_params(dev, sensor_id, data,
+						      ops, NULL);
+}
 EXPORT_SYMBOL_GPL(thermal_zone_of_sensor_register);
 
 /**
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f4705f46c2f..922034eae74b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -378,6 +378,10 @@ struct thermal_trip {
 struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops);
+struct thermal_zone_device *
+thermal_zone_of_sensor_register_params(struct device *dev, int id, void *data,
+			const struct thermal_zone_of_device_ops *ops,
+			struct thermal_zone_params *tzp);
 void thermal_zone_of_sensor_unregister(struct device *dev,
 				       struct thermal_zone_device *tz);
 struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
@@ -393,6 +397,14 @@ thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct thermal_zone_device *
+thermal_zone_of_sensor_register_params(struct device *dev, int id, void *data,
+			const struct thermal_zone_of_device_ops *ops,
+			struct thermal_zone_params *tzp)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline
 void thermal_zone_of_sensor_unregister(struct device *dev,
 				       struct thermal_zone_device *tz)
-- 
2.18.0

Re: [PATCH V3 1/6] thermal: split thermal_zone_of_sensor_register{,_param}()

[Simon Horman]

On Mon, Dec 17, 2018 at 04:56:39PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Introduce new thermal_zone_of_sensor_register_params() function, which
> allows passing struct thermal_zone_params into it and convert original
> thermal_zone_of_sensor_register() to call it with params set to NULL.
> 
> 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>

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

[PATCH V3 2/6] thermal: split devm_thermal_zone_of_sensor_register{,_param}()

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

Introduce new split:devm_thermal_zone_of_sensor_register_params()
function, which allows passing struct thermal_zone_params into it
and convert original devm_thermal_zone_of_sensor_register() to call
it with params set to NULL.

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: Add EXPORT_SYMBOL_GPL() to fix build as a module
V3: - Work around the From line and SoB line checkpatch warning
    - Reorder the SoB line at the end
---
 drivers/thermal/of-thermal.c | 43 +++++++++++++++++++++++++++++++-----
 include/linux/thermal.h      | 13 +++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 1fb450d02ab5..e1a303a5698c 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -638,8 +638,8 @@ static int devm_thermal_zone_of_sensor_match(struct device *dev, void *res,
 }
 
 /**
- * devm_thermal_zone_of_sensor_register - Resource managed version of
- *				thermal_zone_of_sensor_register()
+ * devm_thermal_zone_of_sensor_register_params - Resource managed version of
+ *				thermal_zone_of_sensor_register_params()
  * @dev: a valid struct device pointer of a sensor device. Must contain
  *       a valid .of_node, for the sensor node.
  * @sensor_id: a sensor identifier, in case the sensor IP has more
@@ -647,8 +647,9 @@ static int devm_thermal_zone_of_sensor_match(struct device *dev, void *res,
  * @data: a private pointer (owned by the caller) that will be passed
  *	  back, when a temperature reading is needed.
  * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ * @tzp: thermal zone platform parameters
  *
- * Refer thermal_zone_of_sensor_register() for more details.
+ * Refer thermal_zone_of_sensor_register_params() for more details.
  *
  * Return: On success returns a valid struct thermal_zone_device,
  * otherwise, it returns a corresponding ERR_PTR(). Caller must
@@ -656,9 +657,10 @@ static int devm_thermal_zone_of_sensor_match(struct device *dev, void *res,
  * Registered thermal_zone_device device will automatically be
  * released when device is unbounded.
  */
-struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
+struct thermal_zone_device *devm_thermal_zone_of_sensor_register_params(
 	struct device *dev, int sensor_id,
-	void *data, const struct thermal_zone_of_device_ops *ops)
+	void *data, const struct thermal_zone_of_device_ops *ops,
+	struct thermal_zone_params *tzp)
 {
 	struct thermal_zone_device **ptr, *tzd;
 
@@ -667,7 +669,8 @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	tzd = thermal_zone_of_sensor_register(dev, sensor_id, data, ops);
+	tzd = thermal_zone_of_sensor_register_params(dev, sensor_id, data,
+						     ops, tzp);
 	if (IS_ERR(tzd)) {
 		devres_free(ptr);
 		return tzd;
@@ -678,6 +681,34 @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
 
 	return tzd;
 }
+EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_register_params);
+
+/**
+ * devm_thermal_zone_of_sensor_register - Resource managed version of
+ *				thermal_zone_of_sensor_register()
+ * @dev: a valid struct device pointer of a sensor device. Must contain
+ *       a valid .of_node, for the sensor node.
+ * @sensor_id: a sensor identifier, in case the sensor IP has more
+ *	       than one sensors
+ * @data: a private pointer (owned by the caller) that will be passed
+ *	  back, when a temperature reading is needed.
+ * @ops: struct thermal_zone_of_device_ops *. Must contain at least .get_temp.
+ *
+ * Refer thermal_zone_of_sensor_register() for more details.
+ *
+ * Return: On success returns a valid struct thermal_zone_device,
+ * otherwise, it returns a corresponding ERR_PTR(). Caller must
+ * check the return value with help of IS_ERR() helper.
+ * Registered thermal_zone_device device will automatically be
+ * released when device is unbounded.
+ */
+struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
+	struct device *dev, int sensor_id,
+	void *data, const struct thermal_zone_of_device_ops *ops)
+{
+	return devm_thermal_zone_of_sensor_register_params(dev, sensor_id,
+							   data, ops, NULL);
+}
 EXPORT_SYMBOL_GPL(devm_thermal_zone_of_sensor_register);
 
 /**
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 922034eae74b..a9e7bdeea1bb 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -387,6 +387,10 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
 		struct device *dev, int id, void *data,
 		const struct thermal_zone_of_device_ops *ops);
+struct thermal_zone_device *devm_thermal_zone_of_sensor_register_params(
+		struct device *dev, int id, void *data,
+		const struct thermal_zone_of_device_ops *ops,
+		struct thermal_zone_params *tzp);
 void devm_thermal_zone_of_sensor_unregister(struct device *dev,
 					    struct thermal_zone_device *tz);
 #else
@@ -418,6 +422,15 @@ static inline struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
 	return ERR_PTR(-ENODEV);
 }
 
+static inline struct thermal_zone_device *
+	devm_thermal_zone_of_sensor_register_params(
+		struct device *dev, int id, void *data,
+		const struct thermal_zone_of_device_ops *ops,
+		struct thermal_zone_params *tzp)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline
 void devm_thermal_zone_of_sensor_unregister(struct device *dev,
 					    struct thermal_zone_device *tz)
-- 
2.18.0

Re: [PATCH V3 2/6] thermal: split devm_thermal_zone_of_sensor_register{,_param}()

[Simon Horman]

On Mon, Dec 17, 2018 at 04:56:40PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Introduce new split:devm_thermal_zone_of_sensor_register_params()
> function, which allows passing struct thermal_zone_params into it
> and convert original devm_thermal_zone_of_sensor_register() to call
> it with params set to NULL.
> 
> 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>

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

[PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
in case thermal_zone_params->no_hwmon is set to false. This behavior is
the same as thermal_zone_device_register().

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/of-thermal.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index e1a303a5698c..5ccff7b678de 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -15,6 +15,7 @@
 #include <linux/string.h>
 
 #include "thermal_core.h"
+#include "thermal_hwmon.h"
 
 /***   Private data structures to represent thermal device tree data ***/
 
@@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
 		if (sensor_specs.np == sensor_np && id == sensor_id) {
 			tzd = thermal_zone_of_add_sensor(child, sensor_np,
 							 data, ops);
-			if (!IS_ERR(tzd))
+			if (!IS_ERR(tzd)) {
+				tzd->tzp = tzp;
 				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
+				if (!tzp || !tzp->no_hwmon) {
+					ret = thermal_add_hwmon_sysfs(tzd);
+					if (ret)
+						tzd = ERR_PTR(ret);
+				}
+			}
 
 			of_node_put(sensor_specs.np);
 			of_node_put(child);
@@ -605,6 +613,8 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
 
 	tz = tzd->devdata;
 
+	thermal_remove_hwmon_sysfs(tzd);
+
 	/* no __thermal_zone, nothing to be done */
 	if (!tz)
 		return;
-- 
2.18.0

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]


> +			if (!IS_ERR(tzd)) {
> +				tzd->tzp = tzp;

I tried to read the thermal core but couldn't really figure it. We are
not leaking/overwriting the tzp pointer acquired in of-thermal.c here?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut]

On 12/17/2018 07:12 PM, Wolfram Sang wrote:
> 
>> +			if (!IS_ERR(tzd)) {
>> +				tzd->tzp = tzp;
> 
> I tried to read the thermal core but couldn't really figure it. We are
> not leaking/overwriting the tzp pointer acquired in of-thermal.c here?

I think so, and Eduardo seem to be confirming it, albeit it's rather
inobvious.

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Simon Horman]

On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
> in case thermal_zone_params->no_hwmon is set to false. This behavior is
> the same as thermal_zone_device_register().

As per v2:

If it is the same then is there any value in a helper function?

Regardless, this patch looks good to me.

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

> 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/of-thermal.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index e1a303a5698c..5ccff7b678de 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -15,6 +15,7 @@
>  #include <linux/string.h>
>  
>  #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>  
>  /***   Private data structures to represent thermal device tree data ***/
>  
> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
> -			if (!IS_ERR(tzd))
> +			if (!IS_ERR(tzd)) {
> +				tzd->tzp = tzp;
>  				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +				if (!tzp || !tzp->no_hwmon) {
> +					ret = thermal_add_hwmon_sysfs(tzd);
> +					if (ret)
> +						tzd = ERR_PTR(ret);
> +				}
> +			}
>  
>  			of_node_put(sensor_specs.np);
>  			of_node_put(child);
> @@ -605,6 +613,8 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
>  
>  	tz = tzd->devdata;
>  
> +	thermal_remove_hwmon_sysfs(tzd);
> +
>  	/* no __thermal_zone, nothing to be done */
>  	if (!tz)
>  		return;
> -- 
> 2.18.0
> 

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Eduardo Valentin]

On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
> in case thermal_zone_params->no_hwmon is set to false. This behavior is
> the same as thermal_zone_device_register().
> 
> 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/of-thermal.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index e1a303a5698c..5ccff7b678de 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -15,6 +15,7 @@
>  #include <linux/string.h>
>  
>  #include "thermal_core.h"
> +#include "thermal_hwmon.h"
>  
>  /***   Private data structures to represent thermal device tree data ***/
>  
> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
> -			if (!IS_ERR(tzd))
> +			if (!IS_ERR(tzd)) {
> +				tzd->tzp = tzp;

So, here you will overwrite what was done in of_parse_thermal_zones().
That means, after this point, property like sustainable power, slope and
offset are gone.

>  				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +				if (!tzp || !tzp->no_hwmon) {
> +					ret = thermal_add_hwmon_sysfs(tzd);
> +					if (ret)
> +						tzd = ERR_PTR(ret);
> +				}
> +			}
>  
>  			of_node_put(sensor_specs.np);
>  			of_node_put(child);
> @@ -605,6 +613,8 @@ void thermal_zone_of_sensor_unregister(struct device *dev,
>  
>  	tz = tzd->devdata;
>  
> +	thermal_remove_hwmon_sysfs(tzd);
> +
>  	/* no __thermal_zone, nothing to be done */
>  	if (!tz)
>  		return;
> -- 
> 2.18.0
> 

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut]

On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>> From: Marek Vasut <marek.vasut@gmail.com>
>>
>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>> the same as thermal_zone_device_register().
>>
>> 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/of-thermal.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>> index e1a303a5698c..5ccff7b678de 100644
>> --- a/drivers/thermal/of-thermal.c
>> +++ b/drivers/thermal/of-thermal.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/string.h>
>>  
>>  #include "thermal_core.h"
>> +#include "thermal_hwmon.h"
>>  
>>  /***   Private data structures to represent thermal device tree data ***/
>>  
>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>  							 data, ops);
>> -			if (!IS_ERR(tzd))
>> +			if (!IS_ERR(tzd)) {
>> +				tzd->tzp = tzp;
> 
> So, here you will overwrite what was done in of_parse_thermal_zones().
> That means, after this point, property like sustainable power, slope and
> offset are gone.

Hmmmmm, that was rather inobvious, indeed.

Do you have some suggestion how to pass in the no_hwmon = false then ?
Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
to clean that up.

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut]

On 12/22/18 3:19 AM, Marek Vasut wrote:
> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>
>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>>> the same as thermal_zone_device_register().
>>>
>>> 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/of-thermal.c | 12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>> index e1a303a5698c..5ccff7b678de 100644
>>> --- a/drivers/thermal/of-thermal.c
>>> +++ b/drivers/thermal/of-thermal.c
>>> @@ -15,6 +15,7 @@
>>>  #include <linux/string.h>
>>>  
>>>  #include "thermal_core.h"
>>> +#include "thermal_hwmon.h"
>>>  
>>>  /***   Private data structures to represent thermal device tree data ***/
>>>  
>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>>  							 data, ops);
>>> -			if (!IS_ERR(tzd))
>>> +			if (!IS_ERR(tzd)) {
>>> +				tzd->tzp = tzp;
>>
>> So, here you will overwrite what was done in of_parse_thermal_zones().
>> That means, after this point, property like sustainable power, slope and
>> offset are gone.
> 
> Hmmmmm, that was rather inobvious, indeed.
> 
> Do you have some suggestion how to pass in the no_hwmon = false then ?
> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
> to clean that up.

Bump ?

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut]

On 1/15/19 1:35 AM, Marek Vasut wrote:
> On 12/22/18 3:19 AM, Marek Vasut wrote:
>> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
>>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>>
>>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>>>> the same as thermal_zone_device_register().
>>>>
>>>> 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/of-thermal.c | 12 +++++++++++-
>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>>> index e1a303a5698c..5ccff7b678de 100644
>>>> --- a/drivers/thermal/of-thermal.c
>>>> +++ b/drivers/thermal/of-thermal.c
>>>> @@ -15,6 +15,7 @@
>>>>  #include <linux/string.h>
>>>>  
>>>>  #include "thermal_core.h"
>>>> +#include "thermal_hwmon.h"
>>>>  
>>>>  /***   Private data structures to represent thermal device tree data ***/
>>>>  
>>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>>>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>>>  							 data, ops);
>>>> -			if (!IS_ERR(tzd))
>>>> +			if (!IS_ERR(tzd)) {
>>>> +				tzd->tzp = tzp;
>>>
>>> So, here you will overwrite what was done in of_parse_thermal_zones().
>>> That means, after this point, property like sustainable power, slope and
>>> offset are gone.
>>
>> Hmmmmm, that was rather inobvious, indeed.
>>
>> Do you have some suggestion how to pass in the no_hwmon = false then ?
>> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
>> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
>> to clean that up.
> 
> Bump ?

Bump again, any suggestions ?

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Eduardo Valentin]

On Mon, Jan 28, 2019 at 01:10:11PM +0100, Marek Vasut wrote:
> On 1/15/19 1:35 AM, Marek Vasut wrote:
> > On 12/22/18 3:19 AM, Marek Vasut wrote:
> >> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
> >>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
> >>>> From: Marek Vasut <marek.vasut@gmail.com>
> >>>>
> >>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
> >>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
> >>>> the same as thermal_zone_device_register().
> >>>>
> >>>> 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/of-thermal.c | 12 +++++++++++-
> >>>>  1 file changed, 11 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> >>>> index e1a303a5698c..5ccff7b678de 100644
> >>>> --- a/drivers/thermal/of-thermal.c
> >>>> +++ b/drivers/thermal/of-thermal.c
> >>>> @@ -15,6 +15,7 @@
> >>>>  #include <linux/string.h>
> >>>>  
> >>>>  #include "thermal_core.h"
> >>>> +#include "thermal_hwmon.h"
> >>>>  
> >>>>  /***   Private data structures to represent thermal device tree data ***/
> >>>>  
> >>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
> >>>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
> >>>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
> >>>>  							 data, ops);
> >>>> -			if (!IS_ERR(tzd))
> >>>> +			if (!IS_ERR(tzd)) {
> >>>> +				tzd->tzp = tzp;
> >>>
> >>> So, here you will overwrite what was done in of_parse_thermal_zones().
> >>> That means, after this point, property like sustainable power, slope and
> >>> offset are gone.
> >>
> >> Hmmmmm, that was rather inobvious, indeed.
> >>
> >> Do you have some suggestion how to pass in the no_hwmon = false then ?
> >> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
> >> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
> >> to clean that up.
> > 

Yeah, that is an issue.

> > Bump ?
> 
> Bump again, any suggestions ?

Yeah, a couple of ideas have been proposed for this issue.

First most tempting one is to have a DT property per thermal zone.
Making it linux specific, something prefixed by linux,<property>. I
recall Amit Kutcheria trying something similar to this, but dont
remember where that went. Frankly, this is a Linux thing, I am not
convinced DT is really the right place to fix this.

Another hack that could be written is a module parameter for of-thermal
that would reflect the no_hwmon value, globally. The down side here is
you have to make sure all drivers match that no_hwmon value, right?
> 
> -- 
> Best regards,
> Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut]

On 2/6/19 12:24 AM, Eduardo Valentin wrote:
> On Mon, Jan 28, 2019 at 01:10:11PM +0100, Marek Vasut wrote:
>> On 1/15/19 1:35 AM, Marek Vasut wrote:
>>> On 12/22/18 3:19 AM, Marek Vasut wrote:
>>>> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
>>>>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>>>>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>>>>
>>>>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>>>>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>>>>>> the same as thermal_zone_device_register().
>>>>>>
>>>>>> 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/of-thermal.c | 12 +++++++++++-
>>>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>>>>> index e1a303a5698c..5ccff7b678de 100644
>>>>>> --- a/drivers/thermal/of-thermal.c
>>>>>> +++ b/drivers/thermal/of-thermal.c
>>>>>> @@ -15,6 +15,7 @@
>>>>>>  #include <linux/string.h>
>>>>>>  
>>>>>>  #include "thermal_core.h"
>>>>>> +#include "thermal_hwmon.h"
>>>>>>  
>>>>>>  /***   Private data structures to represent thermal device tree data ***/
>>>>>>  
>>>>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>>>>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>>>>>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>>>>>  							 data, ops);
>>>>>> -			if (!IS_ERR(tzd))
>>>>>> +			if (!IS_ERR(tzd)) {
>>>>>> +				tzd->tzp = tzp;
>>>>>
>>>>> So, here you will overwrite what was done in of_parse_thermal_zones().
>>>>> That means, after this point, property like sustainable power, slope and
>>>>> offset are gone.
>>>>
>>>> Hmmmmm, that was rather inobvious, indeed.
>>>>
>>>> Do you have some suggestion how to pass in the no_hwmon = false then ?
>>>> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
>>>> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
>>>> to clean that up.
>>>
> 
> Yeah, that is an issue.
> 
>>> Bump ?
>>
>> Bump again, any suggestions ?
> 
> Yeah, a couple of ideas have been proposed for this issue.
> 
> First most tempting one is to have a DT property per thermal zone.
> Making it linux specific, something prefixed by linux,<property>. I
> recall Amit Kutcheria trying something similar to this, but dont
> remember where that went. Frankly, this is a Linux thing, I am not
> convinced DT is really the right place to fix this.

DT is not the right place for this, DT describes hardware and this is
policy, so it shouldn't be in DT.

> Another hack that could be written is a module parameter for of-thermal
> that would reflect the no_hwmon value, globally. The down side here is
> you have to make sure all drivers match that no_hwmon value, right?

Indeed, it should be the driver which decides whether or not to register
a HWMON interface for the thermal zone.

I guess for now, I'll just discard this entire series and hack the data
structure like other drivers do, since I don't see any reasonable solution.

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Geert Uytterhoeven]

Hi all,

On Mon, Feb 11, 2019 at 9:43 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 2/6/19 12:24 AM, Eduardo Valentin wrote:
> > On Mon, Jan 28, 2019 at 01:10:11PM +0100, Marek Vasut wrote:
> >> On 1/15/19 1:35 AM, Marek Vasut wrote:
> >>> On 12/22/18 3:19 AM, Marek Vasut wrote:
> >>>> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
> >>>>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
> >>>>>> From: Marek Vasut <marek.vasut@gmail.com>
> >>>>>>
> >>>>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
> >>>>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
> >>>>>> the same as thermal_zone_device_register().
> >>>>>>
> >>>>>> 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/of-thermal.c | 12 +++++++++++-
> >>>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> >>>>>> index e1a303a5698c..5ccff7b678de 100644
> >>>>>> --- a/drivers/thermal/of-thermal.c
> >>>>>> +++ b/drivers/thermal/of-thermal.c
> >>>>>> @@ -15,6 +15,7 @@
> >>>>>>  #include <linux/string.h>
> >>>>>>
> >>>>>>  #include "thermal_core.h"
> >>>>>> +#include "thermal_hwmon.h"
> >>>>>>
> >>>>>>  /***   Private data structures to represent thermal device tree data ***/
> >>>>>>
> >>>>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
> >>>>>>                  if (sensor_specs.np == sensor_np && id == sensor_id) {
> >>>>>>                          tzd = thermal_zone_of_add_sensor(child, sensor_np,
> >>>>>>                                                           data, ops);
> >>>>>> -                        if (!IS_ERR(tzd))
> >>>>>> +                        if (!IS_ERR(tzd)) {
> >>>>>> +                                tzd->tzp = tzp;
> >>>>>
> >>>>> So, here you will overwrite what was done in of_parse_thermal_zones().
> >>>>> That means, after this point, property like sustainable power, slope and
> >>>>> offset are gone.
> >>>>
> >>>> Hmmmmm, that was rather inobvious, indeed.
> >>>>
> >>>> Do you have some suggestion how to pass in the no_hwmon = false then ?
> >>>> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
> >>>> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
> >>>> to clean that up.
> >>>
> >
> > Yeah, that is an issue.
> >
> >>> Bump ?
> >>
> >> Bump again, any suggestions ?
> >
> > Yeah, a couple of ideas have been proposed for this issue.
> >
> > First most tempting one is to have a DT property per thermal zone.
> > Making it linux specific, something prefixed by linux,<property>. I
> > recall Amit Kutcheria trying something similar to this, but dont
> > remember where that went. Frankly, this is a Linux thing, I am not
> > convinced DT is really the right place to fix this.
>
> DT is not the right place for this, DT describes hardware and this is
> policy, so it shouldn't be in DT.

Indeed.

> > Another hack that could be written is a module parameter for of-thermal
> > that would reflect the no_hwmon value, globally. The down side here is
> > you have to make sure all drivers match that no_hwmon value, right?
>
> Indeed, it should be the driver which decides whether or not to register
> a HWMON interface for the thermal zone.
>
> I guess for now, I'll just discard this entire series and hack the data
> structure like other drivers do, since I don't see any reasonable solution.

Pardon my ignorance, but when would a thermal driver (not) want to register
a hwmon interface for the thermal zone?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Guenter Roeck]

On 2/12/19 12:52 AM, Geert Uytterhoeven wrote:
> Hi all,
> 
> On Mon, Feb 11, 2019 at 9:43 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 2/6/19 12:24 AM, Eduardo Valentin wrote:
>>> On Mon, Jan 28, 2019 at 01:10:11PM +0100, Marek Vasut wrote:
>>>> On 1/15/19 1:35 AM, Marek Vasut wrote:
>>>>> On 12/22/18 3:19 AM, Marek Vasut wrote:
>>>>>> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
>>>>>>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>>>>>>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>>>>>>
>>>>>>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>>>>>>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>>>>>>>> the same as thermal_zone_device_register().
>>>>>>>>
>>>>>>>> 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/of-thermal.c | 12 +++++++++++-
>>>>>>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>>>>>>> index e1a303a5698c..5ccff7b678de 100644
>>>>>>>> --- a/drivers/thermal/of-thermal.c
>>>>>>>> +++ b/drivers/thermal/of-thermal.c
>>>>>>>> @@ -15,6 +15,7 @@
>>>>>>>>   #include <linux/string.h>
>>>>>>>>
>>>>>>>>   #include "thermal_core.h"
>>>>>>>> +#include "thermal_hwmon.h"
>>>>>>>>
>>>>>>>>   /***   Private data structures to represent thermal device tree data ***/
>>>>>>>>
>>>>>>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>>>>>>>                   if (sensor_specs.np == sensor_np && id == sensor_id) {
>>>>>>>>                           tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>>>>>>>                                                            data, ops);
>>>>>>>> -                        if (!IS_ERR(tzd))
>>>>>>>> +                        if (!IS_ERR(tzd)) {
>>>>>>>> +                                tzd->tzp = tzp;
>>>>>>>
>>>>>>> So, here you will overwrite what was done in of_parse_thermal_zones().
>>>>>>> That means, after this point, property like sustainable power, slope and
>>>>>>> offset are gone.
>>>>>>
>>>>>> Hmmmmm, that was rather inobvious, indeed.
>>>>>>
>>>>>> Do you have some suggestion how to pass in the no_hwmon = false then ?
>>>>>> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
>>>>>> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
>>>>>> to clean that up.
>>>>>
>>>
>>> Yeah, that is an issue.
>>>
>>>>> Bump ?
>>>>
>>>> Bump again, any suggestions ?
>>>
>>> Yeah, a couple of ideas have been proposed for this issue.
>>>
>>> First most tempting one is to have a DT property per thermal zone.
>>> Making it linux specific, something prefixed by linux,<property>. I
>>> recall Amit Kutcheria trying something similar to this, but dont
>>> remember where that went. Frankly, this is a Linux thing, I am not
>>> convinced DT is really the right place to fix this.
>>
>> DT is not the right place for this, DT describes hardware and this is
>> policy, so it shouldn't be in DT.
> 
> Indeed.
> 
>>> Another hack that could be written is a module parameter for of-thermal
>>> that would reflect the no_hwmon value, globally. The down side here is
>>> you have to make sure all drivers match that no_hwmon value, right?
>>
>> Indeed, it should be the driver which decides whether or not to register
>> a HWMON interface for the thermal zone.
>>
>> I guess for now, I'll just discard this entire series and hack the data
>> structure like other drivers do, since I don't see any reasonable solution.
> 
> Pardon my ignorance, but when would a thermal driver (not) want to register
> a hwmon interface for the thermal zone?
> 

Thermal zones can also be registered from hwmon drivers, so we have a
chicken-and-egg problem.

Guenter

[PATCH V3 4/6] thermal: stm32: Convert to devm_thermal_zone_of_sensor_register_params()

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

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

From: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: David HERNANDEZ SANCHEZ <david.hernandezsanchez@st.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
To: linux-pm@vger.kernel.org
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
---
V2: Rename stm_tz_ops to stm_tz_parms to avoid duplicate symbol name
V3: - Work around the From line and SoB line checkpatch warning
    - Reorder the SoB line at the end
---
 drivers/thermal/st/stm_thermal.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
index 47623da0f91b..827565d94c20 100644
--- a/drivers/thermal/st/stm_thermal.c
+++ b/drivers/thermal/st/stm_thermal.c
@@ -20,7 +20,6 @@
 #include <linux/thermal.h>
 
 #include "../thermal_core.h"
-#include "../thermal_hwmon.h"
 
 /* DTS register offsets */
 #define DTS_CFGR1_OFFSET	0x0
@@ -103,6 +102,10 @@ struct stm_thermal_sensor {
 	int t0, fmt0, ramp_coeff;
 };
 
+static struct thermal_zone_params stm_tz_parms = {
+	.no_hwmon	= false,
+};
+
 static irqreturn_t stm_thermal_alarm_irq(int irq, void *sdata)
 {
 	struct stm_thermal_sensor *sensor = sdata;
@@ -652,9 +655,8 @@ static int stm_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	sensor->th_dev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
-							      sensor,
-							      &stm_tz_ops);
+	sensor->th_dev = devm_thermal_zone_of_sensor_register_params(&pdev->dev,
+				0, sensor, &stm_tz_ops, &stm_tz_parms);
 
 	if (IS_ERR(sensor->th_dev)) {
 		dev_err(&pdev->dev, "%s: thermal zone sensor registering KO\n",
@@ -711,15 +713,6 @@ static int stm_thermal_probe(struct platform_device *pdev)
 		goto err_tz;
 	}
 
-	/*
-	 * Thermal_zone doesn't enable hwmon as default,
-	 * enable it here
-	 */
-	sensor->th_dev->tzp->no_hwmon = false;
-	ret = thermal_add_hwmon_sysfs(sensor->th_dev);
-	if (ret)
-		goto err_tz;
-
 	sensor->mode = THERMAL_DEVICE_ENABLED;
 
 	dev_info(&pdev->dev, "%s: Driver initialized successfully\n",
@@ -737,7 +730,6 @@ static int stm_thermal_remove(struct platform_device *pdev)
 	struct stm_thermal_sensor *sensor = platform_get_drvdata(pdev);
 
 	stm_thermal_sensor_off(sensor);
-	thermal_remove_hwmon_sysfs(sensor->th_dev);
 	thermal_zone_of_sensor_unregister(&pdev->dev, sensor->th_dev);
 
 	return 0;
-- 
2.18.0

Re: [PATCH V3 4/6] thermal: stm32: Convert to devm_thermal_zone_of_sensor_register_params()

[Simon Horman]

On Mon, Dec 17, 2018 at 04:56:42PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Convert the stm32 code to devm_thermal_zone_of_sensor_register_params(),
> no functional change.
> 
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: David HERNANDEZ SANCHEZ <david.hernandezsanchez@st.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> To: linux-pm@vger.kernel.org
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

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

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

[marek.vasut]

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(-)

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

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

[Simon Horman]

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
> 

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

[Marek Vasut]

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);
[...]

-- 
Best regards,
Marek Vasut

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

[Simon Horman]

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
> 

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

[Marek Vasut]

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
>>


-- 
Best regards,
Marek Vasut

[PATCH V3 6/6] thermal: rcar_gen3_thermal: Register hwmon sysfs interface

[marek.vasut]

From: Marek Vasut <marek.vasut@gmail.com>

Use devm_thermal_zone_of_sensor_register_params() with thermal zone
params to enable the hwmon sysfs interface.

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_gen3_thermal.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 75786cc8e2f9..d86b8e746313 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -85,6 +85,10 @@ struct rcar_gen3_thermal_priv {
 	void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
 };
 
+static struct thermal_zone_params rcar_gen3_tz_of_params = {
+	.no_hwmon	= false,
+};
+
 static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc,
 					 u32 reg)
 {
@@ -416,8 +420,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		priv->thermal_init(tsc);
 		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
 
-		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
-							    &rcar_gen3_tz_of_ops);
+		zone = devm_thermal_zone_of_sensor_register_params(dev, i, tsc,
+				&rcar_gen3_tz_of_ops, &rcar_gen3_tz_of_params);
 		if (IS_ERR(zone)) {
 			dev_err(dev, "Can't register thermal zone\n");
 			ret = PTR_ERR(zone);
-- 
2.18.0

Re: [PATCH V3 6/6] thermal: rcar_gen3_thermal: Register hwmon sysfs interface

[Simon Horman]

On Mon, Dec 17, 2018 at 04:56:44PM +0100, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> Use devm_thermal_zone_of_sensor_register_params() with thermal zone
> params to enable the hwmon sysfs interface.
> 
> 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>

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