[v1] hwmon: (cros_ec) register thermal sensors to thermal framework

[PATCH] hwmon: (cros_ec) register thermal sensors to thermal framework

Posted by Sung-Chi, Li 1 year, 1 month ago

cros_ec hwmon driver probes available thermal sensors when probing the
driver.  Register these thermal sensors to the thermal framework, such
that thermal framework can adopt these sensors as well.

Signed-off-by: Sung-Chi, Li <lschyi@chromium.org>
---
 drivers/hwmon/cros_ec_hwmon.c | 69 +++++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 5514cf780b8b..4b1ea431e3d2 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -7,20 +7,31 @@
 
 #include <linux/device.h>
 #include <linux/hwmon.h>
+#include <linux/list.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
+#include <linux/thermal.h>
 #include <linux/types.h>
 #include <linux/units.h>
 
-#define DRV_NAME	"cros-ec-hwmon"
+#define DRV_NAME		"cros-ec-hwmon"
+#define THERMAL_VAL_OFFSET	200
 
 struct cros_ec_hwmon_priv {
 	struct cros_ec_device *cros_ec;
 	const char *temp_sensor_names[EC_TEMP_SENSOR_ENTRIES + EC_TEMP_SENSOR_B_ENTRIES];
 	u8 usable_fans;
+	struct list_head sensors;
+};
+
+struct cros_ec_sensor_data {
+	struct cros_ec_device *cros_ec;
+	struct thermal_zone_device *tz_dev;
+	int addr;
+	struct list_head node;
 };
 
 static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index, u16 *speed)
@@ -185,11 +196,32 @@ static const struct hwmon_chip_info cros_ec_hwmon_chip_info = {
 	.info = cros_ec_hwmon_info,
 };
 
-static void cros_ec_hwmon_probe_temp_sensors(struct device *dev, struct cros_ec_hwmon_priv *priv,
+static int cros_ec_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct cros_ec_sensor_data *data = thermal_zone_device_priv(tz);
+	int ret;
+	u8 val;
+
+	ret = cros_ec_hwmon_read_temp(data->cros_ec, data->addr, &val);
+	if (ret)
+		return -ENODATA;
+	*temp = (val + THERMAL_VAL_OFFSET - 273) * 1000;
+	return 0;
+}
+
+static const struct thermal_zone_device_ops thermal_ops = {
+	.get_temp = cros_ec_thermal_get_temp,
+};
+
+static void cros_ec_hwmon_probe_temp_sensors(struct cros_ec_device *cros_ec,
+					     struct device *dev,
+					     struct cros_ec_hwmon_priv *priv,
+					     struct list_head *head,
 					     u8 thermal_version)
 {
 	struct ec_params_temp_sensor_get_info req = {};
 	struct ec_response_temp_sensor_get_info resp;
+	struct cros_ec_sensor_data *data;
 	size_t candidates, i, sensor_name_size;
 	int ret;
 	u8 temp;
@@ -216,6 +248,23 @@ static void cros_ec_hwmon_probe_temp_sensors(struct device *dev, struct cros_ec_
 		priv->temp_sensor_names[i] = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
 							    (int)sensor_name_size,
 							    resp.sensor_name);
+
+		data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+		if (!data)
+			continue;
+
+		data->addr = i;
+		data->cros_ec = cros_ec;
+		data->tz_dev = devm_thermal_of_zone_register(
+			cros_ec->dev, i, data, &thermal_ops);
+		if (IS_ERR_VALUE(data->tz_dev)) {
+			dev_err(dev,
+				"failed to register %zu thermal sensor, err = %ld",
+				i, PTR_ERR(data->tz_dev));
+			continue;
+		}
+
+		list_add(&data->node, head);
 	}
 }
 
@@ -255,8 +304,10 @@ static int cros_ec_hwmon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->cros_ec = cros_ec;
+	INIT_LIST_HEAD(&priv->sensors);
 
-	cros_ec_hwmon_probe_temp_sensors(dev, priv, thermal_version);
+	cros_ec_hwmon_probe_temp_sensors(cros_ec, dev, priv, &priv->sensors,
+					 thermal_version);
 	cros_ec_hwmon_probe_fans(priv);
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, "cros_ec", priv,
@@ -265,6 +316,17 @@ static int cros_ec_hwmon_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
+static void cros_ec_hwmon_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
+	struct cros_ec_sensor_data *iter;
+
+	list_for_each_entry(iter, &priv->sensors, node) {
+		devm_thermal_of_zone_unregister(dev, iter->tz_dev);
+	}
+}
+
 static const struct platform_device_id cros_ec_hwmon_id[] = {
 	{ DRV_NAME, 0 },
 	{}
@@ -273,6 +335,7 @@ static const struct platform_device_id cros_ec_hwmon_id[] = {
 static struct platform_driver cros_ec_hwmon_driver = {
 	.driver.name	= DRV_NAME,
 	.probe		= cros_ec_hwmon_probe,
+	.remove		= cros_ec_hwmon_remove,
 	.id_table	= cros_ec_hwmon_id,
 };
 module_platform_driver(cros_ec_hwmon_driver);
-- 
2.47.0.277.g8800431eea-goog

Re: [PATCH] hwmon: (cros_ec) register thermal sensors to thermal framework

Posted by Thomas Weißschuh 1 year, 1 month ago

Hi!

On 2024-11-11 15:49:04+0800, Sung-Chi, Li wrote:
> cros_ec hwmon driver probes available thermal sensors when probing the
> driver.  Register these thermal sensors to the thermal framework, such
> that thermal framework can adopt these sensors as well.
> 
> Signed-off-by: Sung-Chi, Li <lschyi@chromium.org>
> ---
>  drivers/hwmon/cros_ec_hwmon.c | 69 +++++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index 5514cf780b8b..4b1ea431e3d2 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -7,20 +7,31 @@
>  
>  #include <linux/device.h>
>  #include <linux/hwmon.h>
> +#include <linux/list.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/platform_data/cros_ec_commands.h>
>  #include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/thermal.h>
>  #include <linux/types.h>
>  #include <linux/units.h>
>  
> -#define DRV_NAME	"cros-ec-hwmon"
> +#define DRV_NAME		"cros-ec-hwmon"
> +#define THERMAL_VAL_OFFSET	200
>  
>  struct cros_ec_hwmon_priv {
>  	struct cros_ec_device *cros_ec;
>  	const char *temp_sensor_names[EC_TEMP_SENSOR_ENTRIES + EC_TEMP_SENSOR_B_ENTRIES];
>  	u8 usable_fans;
> +	struct list_head sensors;
> +};
> +
> +struct cros_ec_sensor_data {

cros_ec_hwmon_thermal_zone_data.

> +	struct cros_ec_device *cros_ec;
> +	struct thermal_zone_device *tz_dev;
> +	int addr;

This is not an address, but an index.

> +	struct list_head node;
>  };
>  
>  static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index, u16 *speed)
> @@ -185,11 +196,32 @@ static const struct hwmon_chip_info cros_ec_hwmon_chip_info = {
>  	.info = cros_ec_hwmon_info,
>  };
>  
> -static void cros_ec_hwmon_probe_temp_sensors(struct device *dev, struct cros_ec_hwmon_priv *priv,
> +static int cros_ec_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct cros_ec_sensor_data *data = thermal_zone_device_priv(tz);
> +	int ret;
> +	u8 val;
> +
> +	ret = cros_ec_hwmon_read_temp(data->cros_ec, data->addr, &val);
> +	if (ret)
> +		return -ENODATA;
> +	*temp = (val + THERMAL_VAL_OFFSET - 273) * 1000;

Use cros_ec_hwmon_temp_to_millicelsius() and cros_ec_hwmon_is_error_temp().

> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops thermal_ops = {
> +	.get_temp = cros_ec_thermal_get_temp,
> +};

Use the cros_ec_hwmon symbol prefix.

> +
> +static void cros_ec_hwmon_probe_temp_sensors(struct cros_ec_device *cros_ec,
> +					     struct device *dev,
> +					     struct cros_ec_hwmon_priv *priv,
> +					     struct list_head *head,
>  					     u8 thermal_version)
>  {
>  	struct ec_params_temp_sensor_get_info req = {};
>  	struct ec_response_temp_sensor_get_info resp;
> +	struct cros_ec_sensor_data *data;
>  	size_t candidates, i, sensor_name_size;
>  	int ret;
>  	u8 temp;
> @@ -216,6 +248,23 @@ static void cros_ec_hwmon_probe_temp_sensors(struct device *dev, struct cros_ec_
>  		priv->temp_sensor_names[i] = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
>  							    (int)sensor_name_size,
>  							    resp.sensor_name);
> +
> +		data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +		if (!data)
> +			continue;
> +
> +		data->addr = i;
> +		data->cros_ec = cros_ec;
> +		data->tz_dev = devm_thermal_of_zone_register(
> +			cros_ec->dev, i, data, &thermal_ops);

I'm a bit confused. The uses thermal configuration from OF,
but this driver has no specific OF support.
Can you provide a usage example in the commit message?

Does it not need a binding documentation update?

> +		if (IS_ERR_VALUE(data->tz_dev)) {
> +			dev_err(dev,
> +				"failed to register %zu thermal sensor, err = %ld",
> +				i, PTR_ERR(data->tz_dev));

Use %pe.

> +			continue;
> +		}
> +
> +		list_add(&data->node, head);
>  	}
>  }
>  
> @@ -255,8 +304,10 @@ static int cros_ec_hwmon_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	priv->cros_ec = cros_ec;
> +	INIT_LIST_HEAD(&priv->sensors);
>  
> -	cros_ec_hwmon_probe_temp_sensors(dev, priv, thermal_version);
> +	cros_ec_hwmon_probe_temp_sensors(cros_ec, dev, priv, &priv->sensors,
> +					 thermal_version);

cros_ec is already passed as priv->cros_ec.

>  	cros_ec_hwmon_probe_fans(priv);
>  
>  	hwmon_dev = devm_hwmon_device_register_with_info(dev, "cros_ec", priv,
> @@ -265,6 +316,17 @@ static int cros_ec_hwmon_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(hwmon_dev);
>  }
>  
> +static void cros_ec_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
> +	struct cros_ec_sensor_data *iter;
> +
> +	list_for_each_entry(iter, &priv->sensors, node) {
> +		devm_thermal_of_zone_unregister(dev, iter->tz_dev);

This shouldn't be needed, the zone will be unregistered automatically by
the devm framework.
This also means that .remove and priv->sensors can go away.

> +	}
> +}
> +
>  static const struct platform_device_id cros_ec_hwmon_id[] = {
>  	{ DRV_NAME, 0 },
>  	{}
> @@ -273,6 +335,7 @@ static const struct platform_device_id cros_ec_hwmon_id[] = {
>  static struct platform_driver cros_ec_hwmon_driver = {
>  	.driver.name	= DRV_NAME,
>  	.probe		= cros_ec_hwmon_probe,
> +	.remove		= cros_ec_hwmon_remove,
>  	.id_table	= cros_ec_hwmon_id,
>  };
>  module_platform_driver(cros_ec_hwmon_driver);
> -- 
> 2.47.0.277.g8800431eea-goog
>

[PATCH v3 1/2] hwmon: (cros_ec) register thermal sensors to thermal framework

Posted by Sung-Chi, Li 1 year, 1 month ago

cros_ec hwmon driver probes available thermal sensors when probing the
driver.  Register these thermal sensors to the thermal framework as well
via setting HWMON_C_REGISTER_TZ as a chip info, such that thermal
framework can adopt these sensors as well.

To make cros_ec registrable to thermal framework, the cros_ec dts need
the corresponding changes:

&cros_ec {
	#thermal-sensor-cells = <1>;
};

Signed-off-by: Sung-Chi, Li <lschyi@chromium.org>
---
 Changes in v2:
   - Rename `cros_ec_sensor_data` to `cros_ec_hwmon_thermal_zone_data`.
   - Rename `addr` in struct `cros_ec_hwmon_thermal_zone_data` to `idx`.
   - Use `cros_ec_hwmon_temp_to_millicelsius` to do value conversion in
     `cros_ec_thermal_get_temp` function.
   - Rename `cros_ec_thermal_get_temp` to `cros_ec_hwmon_thermal_get_temp` to
     make `cros_ec_hwmon` a prefix.
   - Use `%pe` in `cros_ec_hwmon_probe_temp_sensors` when printing out
     `data->tz_dev` if failed register thermal device.
   - Remove `cros_ec_hwmon_remove`, and the `.remove` value in
     `cros_ec_hwmon_driver` since there is no need to call
     `devm_thermal_of_zone_unregister` for clean up.
   - Revert function signature of `cros_ec_hwmon_probe_temp_sensors` since all
     needed parameters are presented.
   - Revert include of `linux/list.h` because no list data structure is used.
---
 Changes in v3:
   - Revert all changes and just as add HWMON_C_REGISTER_TZ as a chip info.
   - Remove unneeded Change-Id tag in commit message.
---
 drivers/hwmon/cros_ec_hwmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 5514cf780b8b..9991c3fa020a 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -141,6 +141,7 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
 }
 
 static const struct hwmon_channel_info * const cros_ec_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
 	HWMON_CHANNEL_INFO(fan,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
 			   HWMON_F_INPUT | HWMON_F_FAULT,
-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH v3 1/2] hwmon: (cros_ec) register thermal sensors to thermal framework

Posted by Guenter Roeck 1 year, 1 month ago

On Wed, Nov 13, 2024 at 10:39:51AM +0800, Sung-Chi, Li wrote:
> cros_ec hwmon driver probes available thermal sensors when probing the
> driver.  Register these thermal sensors to the thermal framework as well
> via setting HWMON_C_REGISTER_TZ as a chip info, such that thermal
> framework can adopt these sensors as well.
> 
> To make cros_ec registrable to thermal framework, the cros_ec dts need
> the corresponding changes:
> 
> &cros_ec {
> 	#thermal-sensor-cells = <1>;
> };
> 
> Signed-off-by: Sung-Chi, Li <lschyi@chromium.org>

Applied.

Thanks,
Guenter

[PATCH v3 2/2] dt-bindings: mfd: Add properties for thermal sensor cells

Posted by Sung-Chi, Li 1 year, 1 month ago

The cros_ec supports reading thermal values from thermal sensors
connect to it. Add the property '#thermal-sensor-cells' bindings, such
that thermal framework can recognize cros_ec as a valid thermal device.

Signed-off-by: Sung-Chi, Li <lschyi@chromium.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Changes in v2:
   - Add changes for DTS binding.
 Changes in v3:
   - Remove unneeded Change-Id tag in commit message.
---
 Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index aac8819bd00b..c7d63e3aacd2 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -96,6 +96,9 @@ properties:
   '#gpio-cells':
     const: 2
 
+  '#thermal-sensor-cells':
+    const: 1
+
   gpio-controller: true
 
   typec:
-- 
2.47.0.338.g60cca15819-goog