[PATCH RESEND V13 1/5] dt-bindings: fsl: scu: add thermal binding

[PATCH RESEND V13 1/5] dt-bindings: fsl: scu: add thermal binding

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as
system controller, the system controller is in charge of system
power, clock and thermal sensors etc. management, Linux kernel
has to communicate with system controller via MU (message unit)
IPC to get temperature from thermal sensors, this patch adds
binding doc for i.MX system controller thermal driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No change, just rebase the patch to top of linux-next and based on my watchdog patch:
https://patchwork.kernel.org/patch/10962183/
---
 .../devicetree/bindings/arm/freescale/fsl,scu.txt        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
index a575e42..fc3844e 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -155,6 +155,17 @@ Required properties:
 Optional properties:
 - timeout-sec: contains the watchdog timeout in seconds.
 
+Thermal bindings based on SCU Message Protocol
+------------------------------------------------------------
+
+Required properties:
+- compatible:			Should be :
+				  "fsl,imx8qxp-sc-thermal"
+				followed by "fsl,imx-sc-thermal";
+
+- #thermal-sensor-cells:	See Documentation/devicetree/bindings/thermal/thermal.txt
+				for a description.
+
 Example (imx8qxp):
 -------------
 aliases {
@@ -222,6 +233,11 @@ firmware {
 			compatible = "fsl,imx8qxp-sc-wdt", "fsl,imx-sc-wdt";
 			timeout-sec = <60>;
 		};
+
+		tsens: thermal-sensor {
+			compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+			#thermal-sensor-cells = <1>;
+		};
 	};
 };
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting sensor ID from DT

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

On some platforms like i.MX8QXP, the thermal driver needs a
real HW sensor ID from DT thermal zone, the HW sensor ID is
used to get temperature from SCU firmware, and the virtual
sensor ID starting from 0 to N is NOT used at all, this patch
adds new API thermal_zone_of_get_sensor_id() to provide the
feature of getting sensor ID from DT thermal zone's node.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V12:
	- adjust the second parameter of thermal_zone_of_get_sensor_id() API, then caller no need
	  to pass the of_phandle_args structure and put the sensor_specs.np manually, also putting
	  the sensor node device check inside this API to make it easy for usage;
---
 drivers/thermal/of-thermal.c | 66 +++++++++++++++++++++++++++++++++-----------
 include/linux/thermal.h      | 10 +++++++
 2 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index dc5093b..a53792b 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node *zone,
 }
 
 /**
+ * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal zone
+ * @tz_np: a valid thermal zone device node.
+ * @sensor_np: a sensor node of a valid sensor device.
+ * @id: a sensor ID pointer will be passed back.
+ *
+ * This function will get sensor ID from a given thermal zone node, use
+ * "thermal-sensors" as list name, and get sensor ID from first phandle's
+ * argument.
+ *
+ * Return: 0 on success, proper error code otherwise.
+ */
+
+int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
+				  struct device_node *sensor_np,
+				  u32 *id)
+{
+	struct of_phandle_args sensor_specs;
+	int ret;
+
+	ret = of_parse_phandle_with_args(tz_np,
+					 "thermal-sensors",
+					 "#thermal-sensor-cells",
+					 0,
+					 &sensor_specs);
+	if (ret)
+		return ret;
+
+	if (sensor_specs.np != sensor_np) {
+		of_node_put(sensor_specs.np);
+		return -ENODEV;
+	}
+
+	if (sensor_specs.args_count >= 1) {
+		*id = sensor_specs.args[0];
+		WARN(sensor_specs.args_count > 1,
+		     "%pOFn: too many cells in sensor specifier %d\n",
+		     sensor_specs.np, sensor_specs.args_count);
+	} else {
+		*id = 0;
+	}
+
+	of_node_put(sensor_specs.np);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
+
+/**
  * 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.
@@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
 	sensor_np = of_node_get(dev->of_node);
 
 	for_each_available_child_of_node(np, child) {
-		struct of_phandle_args sensor_specs;
 		int ret, id;
 
 		/* For now, thermal framework supports only 1 sensor per zone */
-		ret = of_parse_phandle_with_args(child, "thermal-sensors",
-						 "#thermal-sensor-cells",
-						 0, &sensor_specs);
+		ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
 		if (ret)
 			continue;
 
-		if (sensor_specs.args_count >= 1) {
-			id = sensor_specs.args[0];
-			WARN(sensor_specs.args_count > 1,
-			     "%pOFn: too many cells in sensor specifier %d\n",
-			     sensor_specs.np, sensor_specs.args_count);
-		} else {
-			id = 0;
-		}
-
-		if (sensor_specs.np == sensor_np && id == sensor_id) {
+		if (id == sensor_id) {
 			tzd = thermal_zone_of_add_sensor(child, sensor_np,
 							 data, ops);
 			if (!IS_ERR(tzd))
 				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
 
-			of_node_put(sensor_specs.np);
 			of_node_put(child);
 			goto exit;
 		}
-		of_node_put(sensor_specs.np);
 	}
 exit:
 	of_node_put(sensor_np);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 15a4ca5..5edffe6 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -375,6 +375,9 @@ struct thermal_trip {
 
 /* Function declarations */
 #ifdef CONFIG_THERMAL_OF
+int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
+				  struct device_node *sensor_np,
+				  u32 *id);
 struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops);
@@ -386,6 +389,13 @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
 void devm_thermal_zone_of_sensor_unregister(struct device *dev,
 					    struct thermal_zone_device *tz);
 #else
+
+static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
+					 struct device_node *sensor_np,
+					 u32 *id)
+{
+	return -ENOENT;
+}
 static inline struct thermal_zone_device *
 thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
 				const struct thermal_zone_of_device_ops *ops)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND V13 3/5] thermal: imx_sc: add i.MX system controller thermal support

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
inside, the system controller is in charge of controlling power,
clock and thermal sensors etc..

This patch adds i.MX system controller thermal driver support,
Linux kernel has to communicate with system controller via MU
(message unit) IPC to get each thermal sensor's temperature,
it supports multiple sensors which are passed from device tree,
please see the binding doc for details.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
	- remove the COMPILE_TEST dependency as it depends on SCU firmware;
	- use new argument according to API thermal_zone_of_get_sensor_id change.
---
 drivers/thermal/Kconfig          |  11 +++
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/imx_sc_thermal.c | 142 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 drivers/thermal/imx_sc_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 9966364..454cbe5 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -233,6 +233,17 @@ config IMX_THERMAL
 	  cpufreq is used as the cooling device to throttle CPUs when the
 	  passive trip is crossed.
 
+config IMX_SC_THERMAL
+	tristate "Temperature sensor driver for NXP i.MX SoCs with System Controller"
+	depends on ARCH_MXC && IMX_SCU
+	depends on OF
+	help
+	  Support for Temperature Monitor (TEMPMON) found on NXP i.MX SoCs with
+	  system controller inside, Linux kernel has to communicate with system
+	  controller via MU (message unit) IPC to get temperature from thermal
+	  sensor. It supports one critical trip point and one
+	  passive trip point for each thermal sensor.
+
 config MAX77620_THERMAL
 	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
 	depends on MFD_MAX77620
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 74a37c7..717a1ba 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_DB8500_THERMAL)	+= db8500_thermal.o
 obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
 obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
 obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
+obj-$(CONFIG_IMX_SC_THERMAL)	+= imx_sc_thermal.o
 obj-$(CONFIG_MAX77620_THERMAL)	+= max77620_thermal.o
 obj-$(CONFIG_QORIQ_THERMAL)	+= qoriq_thermal.o
 obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
diff --git a/drivers/thermal/imx_sc_thermal.c b/drivers/thermal/imx_sc_thermal.c
new file mode 100644
index 0000000..d406ecb
--- /dev/null
+++ b/drivers/thermal/imx_sc_thermal.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018-2019 NXP.
+ */
+
+#include <linux/err.h>
+#include <linux/firmware/imx/sci.h>
+#include <linux/firmware/imx/types.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define IMX_SC_MISC_FUNC_GET_TEMP	13
+
+static struct imx_sc_ipc *thermal_ipc_handle;
+
+struct imx_sc_sensor {
+	struct thermal_zone_device *tzd;
+	u32 resource_id;
+};
+
+struct req_get_temp {
+	u16 resource_id;
+	u8 type;
+} __packed;
+
+struct resp_get_temp {
+	u16 celsius;
+	u8 tenths;
+} __packed;
+
+struct imx_sc_msg_misc_get_temp {
+	struct imx_sc_rpc_msg hdr;
+	union {
+		struct req_get_temp req;
+		struct resp_get_temp resp;
+	} data;
+};
+
+static int imx_sc_thermal_get_temp(void *data, int *temp)
+{
+	struct imx_sc_msg_misc_get_temp msg;
+	struct imx_sc_rpc_msg *hdr = &msg.hdr;
+	struct imx_sc_sensor *sensor = data;
+	int ret;
+
+	msg.data.req.resource_id = sensor->resource_id;
+	msg.data.req.type = IMX_SC_C_TEMP;
+
+	hdr->ver = IMX_SC_RPC_VERSION;
+	hdr->svc = IMX_SC_RPC_SVC_MISC;
+	hdr->func = IMX_SC_MISC_FUNC_GET_TEMP;
+	hdr->size = 2;
+
+	ret = imx_scu_call_rpc(thermal_ipc_handle, &msg, true);
+	if (ret) {
+		dev_err(&sensor->tzd->device, "read temp sensor %d failed, ret %d\n",
+			sensor->resource_id, ret);
+		return ret;
+	}
+
+	*temp = msg.data.resp.celsius * 1000 + msg.data.resp.tenths * 100;
+
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops imx_sc_thermal_ops = {
+	.get_temp = imx_sc_thermal_get_temp,
+};
+
+static int imx_sc_thermal_probe(struct platform_device *pdev)
+{
+	struct device_node *np, *child, *sensor_np;
+	struct imx_sc_sensor *sensor;
+	int ret;
+
+	ret = imx_scu_get_handle(&thermal_ipc_handle);
+	if (ret)
+		return ret;
+
+	np = of_find_node_by_name(NULL, "thermal-zones");
+	if (!np)
+		return -ENODEV;
+
+	sensor_np = of_node_get(pdev->dev.of_node);
+
+	for_each_available_child_of_node(np, child) {
+		sensor = devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
+		if (!sensor) {
+			of_node_put(sensor_np);
+			return -ENOMEM;
+		}
+
+		ret = thermal_zone_of_get_sensor_id(child,
+						    sensor_np,
+						    &sensor->resource_id);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"failed to get valid sensor resource id: %d\n",
+				ret);
+			break;
+		}
+
+		sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
+								   sensor->resource_id,
+								   sensor,
+								   &imx_sc_thermal_ops);
+		if (IS_ERR(sensor->tzd)) {
+			dev_err(&pdev->dev, "failed to register thermal zone\n");
+			ret = PTR_ERR(sensor->tzd);
+			break;
+		}
+	}
+
+	of_node_put(sensor_np);
+
+	return ret;
+}
+
+static const struct of_device_id imx_sc_thermal_table[] = {
+	{ .compatible = "fsl,imx-sc-thermal", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
+
+static struct platform_driver imx_sc_thermal_driver = {
+		.probe = imx_sc_thermal_probe,
+		.driver = {
+			.name = "imx-sc-thermal",
+			.of_match_table = imx_sc_thermal_table,
+		},
+};
+module_platform_driver(imx_sc_thermal_driver);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND V13 4/5] defconfig: arm64: add i.MX system controller thermal support

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

This patch enables CONFIG_IMX_SC_THERMAL as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change, just rebase the patch to top of linux-next and based on below my patch:
https://patchwork.kernel.org/patch/10959025/
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index fa1ea8f..c33bf882 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -422,6 +422,7 @@ CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
 CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_QORIQ_THERMAL=m
+CONFIG_IMX_SC_THERMAL=m
 CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_RCAR_THERMAL=y
 CONFIG_RCAR_GEN3_THERMAL=y
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RESEND V13 5/5] arm64: dts: imx: add i.MX8QXP thermal support

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Add i.MX8QXP CPU thermal zone support.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change, just rebase the patch to top of linux-next and below my patch:
https://patchwork.kernel.org/patch/10962185/
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 37 ++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 9f52da6..4448c65 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/pinctrl/pads-imx8qxp.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -162,6 +163,11 @@
 			compatible = "fsl,imx8qxp-sc-wdt", "fsl,imx-sc-wdt";
 			timeout-sec = <60>;
 		};
+
+		tsens: thermal-sensor {
+			compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+			#thermal-sensor-cells = <1>;
+		};
 	};
 
 	timer {
@@ -530,4 +536,35 @@
 			power-domains = <&pd IMX_SC_R_GPIO_7>;
 		};
 	};
+
+	thermal_zones: thermal-zones {
+		cpu-thermal0 {
+			polling-delay-passive = <250>;
+			polling-delay = <2000>;
+			thermal-sensors = <&tsens IMX_SC_R_SYSTEM>;
+			trips {
+				cpu_alert0: trip0 {
+					temperature = <107000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				cpu_crit0: trip1 {
+					temperature = <127000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device =
+						<&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A35_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A35_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A35_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+
+				};
+			};
+		};
+	};
 };
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting sensor ID from DT

[Eduardo Valentin]

On Tue, May 28, 2019 at 02:06:18PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> On some platforms like i.MX8QXP, the thermal driver needs a
> real HW sensor ID from DT thermal zone, the HW sensor ID is
> used to get temperature from SCU firmware, and the virtual
> sensor ID starting from 0 to N is NOT used at all, this patch
> adds new API thermal_zone_of_get_sensor_id() to provide the
> feature of getting sensor ID from DT thermal zone's node.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V12:
> 	- adjust the second parameter of thermal_zone_of_get_sensor_id() API, then caller no need
> 	  to pass the of_phandle_args structure and put the sensor_specs.np manually, also putting
> 	  the sensor node device check inside this API to make it easy for usage;

What happened to using nxp,resource-id property in your driver?
Why do we need this as an API in of-thermal? What other drivers may
benefit of this?

Regardless, this patch needs to document the new API under
Documentation/

> ---
>  drivers/thermal/of-thermal.c | 66 +++++++++++++++++++++++++++++++++-----------
>  include/linux/thermal.h      | 10 +++++++
>  2 files changed, 60 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index dc5093b..a53792b 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node *zone,
>  }
>  
>  /**
> + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal zone
> + * @tz_np: a valid thermal zone device node.
> + * @sensor_np: a sensor node of a valid sensor device.
> + * @id: a sensor ID pointer will be passed back.
> + *
> + * This function will get sensor ID from a given thermal zone node, use
> + * "thermal-sensors" as list name, and get sensor ID from first phandle's
> + * argument.
> + *
> + * Return: 0 on success, proper error code otherwise.
> + */
> +
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> +				  struct device_node *sensor_np,
> +				  u32 *id)
> +{
> +	struct of_phandle_args sensor_specs;
> +	int ret;
> +
> +	ret = of_parse_phandle_with_args(tz_np,
> +					 "thermal-sensors",
> +					 "#thermal-sensor-cells",
> +					 0,
> +					 &sensor_specs);
> +	if (ret)
> +		return ret;
> +
> +	if (sensor_specs.np != sensor_np) {
> +		of_node_put(sensor_specs.np);
> +		return -ENODEV;
> +	}
> +
> +	if (sensor_specs.args_count >= 1) {
> +		*id = sensor_specs.args[0];
> +		WARN(sensor_specs.args_count > 1,
> +		     "%pOFn: too many cells in sensor specifier %d\n",
> +		     sensor_specs.np, sensor_specs.args_count);
> +	} else {
> +		*id = 0;
> +	}
> +
> +	of_node_put(sensor_specs.np);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> +
> +/**
>   * 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.
> @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>  	sensor_np = of_node_get(dev->of_node);
>  
>  	for_each_available_child_of_node(np, child) {
> -		struct of_phandle_args sensor_specs;
>  		int ret, id;
>  
>  		/* For now, thermal framework supports only 1 sensor per zone */
> -		ret = of_parse_phandle_with_args(child, "thermal-sensors",
> -						 "#thermal-sensor-cells",
> -						 0, &sensor_specs);
> +		ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
>  		if (ret)
>  			continue;
>  
> -		if (sensor_specs.args_count >= 1) {
> -			id = sensor_specs.args[0];
> -			WARN(sensor_specs.args_count > 1,
> -			     "%pOFn: too many cells in sensor specifier %d\n",
> -			     sensor_specs.np, sensor_specs.args_count);
> -		} else {
> -			id = 0;
> -		}
> -
> -		if (sensor_specs.np == sensor_np && id == sensor_id) {
> +		if (id == sensor_id) {
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
>  			if (!IS_ERR(tzd))
>  				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
>  
> -			of_node_put(sensor_specs.np);
>  			of_node_put(child);
>  			goto exit;
>  		}
> -		of_node_put(sensor_specs.np);
>  	}
>  exit:
>  	of_node_put(sensor_np);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 15a4ca5..5edffe6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -375,6 +375,9 @@ struct thermal_trip {
>  
>  /* Function declarations */
>  #ifdef CONFIG_THERMAL_OF
> +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> +				  struct device_node *sensor_np,
> +				  u32 *id);
>  struct thermal_zone_device *
>  thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>  				const struct thermal_zone_of_device_ops *ops);
> @@ -386,6 +389,13 @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
>  void devm_thermal_zone_of_sensor_unregister(struct device *dev,
>  					    struct thermal_zone_device *tz);
>  #else
> +
> +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> +					 struct device_node *sensor_np,
> +					 u32 *id)
> +{
> +	return -ENOENT;
> +}
>  static inline struct thermal_zone_device *
>  thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
>  				const struct thermal_zone_of_device_ops *ops)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting sensor ID from DT

[Anson Huang]

Hi, Eduardo

> -----Original Message-----
> From: Eduardo Valentin <edubezval@gmail.com>
> Sent: Wednesday, May 29, 2019 11:02 AM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> catalin.marinas@arm.com; will.deacon@arm.com; rui.zhang@intel.com;
> daniel.lezcano@linaro.org; Aisheng Dong <aisheng.dong@nxp.com>;
> ulf.hansson@linaro.org; Peng Fan <peng.fan@nxp.com>; Daniel Baluta
> <daniel.baluta@nxp.com>; maxime.ripard@bootlin.com; olof@lixom.net;
> jagan@amarulasolutions.com; horms+renesas@verge.net.au; Leonard Crestez
> <leonard.crestez@nxp.com>; bjorn.andersson@linaro.org;
> dinguyen@kernel.org; enric.balletbo@collabora.com;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-pm@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: Re: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting
> sensor ID from DT
> 
> On Tue, May 28, 2019 at 02:06:18PM +0800, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > On some platforms like i.MX8QXP, the thermal driver needs a real HW
> > sensor ID from DT thermal zone, the HW sensor ID is used to get
> > temperature from SCU firmware, and the virtual sensor ID starting from
> > 0 to N is NOT used at all, this patch adds new API
> > thermal_zone_of_get_sensor_id() to provide the feature of getting
> > sensor ID from DT thermal zone's node.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V12:
> > 	- adjust the second parameter of thermal_zone_of_get_sensor_id() API,
> then caller no need
> > 	  to pass the of_phandle_args structure and put the sensor_specs.np
> manually, also putting
> > 	  the sensor node device check inside this API to make it easy for
> > usage;
> 
> What happened to using nxp,resource-id property in your driver?
> Why do we need this as an API in of-thermal? What other drivers may benefit
> of this?
> 
> Regardless, this patch needs to document the new API under Documentation/

As Rob has different opinion about this property, he thought it is unnecessary, see below
discussion mail, that is why I need to add API to get the resource ID from phandle argument.
I am totally confused now, which approach should we adopt?

https://patchwork.kernel.org/patch/10831397/

Thanks,
Anson

> 
> > ---
> >  drivers/thermal/of-thermal.c | 66 +++++++++++++++++++++++++++++++++---
> --------
> >  include/linux/thermal.h      | 10 +++++++
> >  2 files changed, 60 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/thermal/of-thermal.c
> > b/drivers/thermal/of-thermal.c index dc5093b..a53792b 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct device_node
> > *zone,  }
> >
> >  /**
> > + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal
> > + zone
> > + * @tz_np: a valid thermal zone device node.
> > + * @sensor_np: a sensor node of a valid sensor device.
> > + * @id: a sensor ID pointer will be passed back.
> > + *
> > + * This function will get sensor ID from a given thermal zone node,
> > + use
> > + * "thermal-sensors" as list name, and get sensor ID from first
> > + phandle's
> > + * argument.
> > + *
> > + * Return: 0 on success, proper error code otherwise.
> > + */
> > +
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > +				  struct device_node *sensor_np,
> > +				  u32 *id)
> > +{
> > +	struct of_phandle_args sensor_specs;
> > +	int ret;
> > +
> > +	ret = of_parse_phandle_with_args(tz_np,
> > +					 "thermal-sensors",
> > +					 "#thermal-sensor-cells",
> > +					 0,
> > +					 &sensor_specs);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (sensor_specs.np != sensor_np) {
> > +		of_node_put(sensor_specs.np);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (sensor_specs.args_count >= 1) {
> > +		*id = sensor_specs.args[0];
> > +		WARN(sensor_specs.args_count > 1,
> > +		     "%pOFn: too many cells in sensor specifier %d\n",
> > +		     sensor_specs.np, sensor_specs.args_count);
> > +	} else {
> > +		*id = 0;
> > +	}
> > +
> > +	of_node_put(sensor_specs.np);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> > +
> > +/**
> >   * 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.
> > @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device
> *dev, int sensor_id, void *data,
> >  	sensor_np = of_node_get(dev->of_node);
> >
> >  	for_each_available_child_of_node(np, child) {
> > -		struct of_phandle_args sensor_specs;
> >  		int ret, id;
> >
> >  		/* For now, thermal framework supports only 1 sensor per
> zone */
> > -		ret = of_parse_phandle_with_args(child, "thermal-sensors",
> > -						 "#thermal-sensor-cells",
> > -						 0, &sensor_specs);
> > +		ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> >  		if (ret)
> >  			continue;
> >
> > -		if (sensor_specs.args_count >= 1) {
> > -			id = sensor_specs.args[0];
> > -			WARN(sensor_specs.args_count > 1,
> > -			     "%pOFn: too many cells in sensor specifier %d\n",
> > -			     sensor_specs.np, sensor_specs.args_count);
> > -		} else {
> > -			id = 0;
> > -		}
> > -
> > -		if (sensor_specs.np == sensor_np && id == sensor_id) {
> > +		if (id == sensor_id) {
> >  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
> >  							 data, ops);
> >  			if (!IS_ERR(tzd))
> >  				tzd->ops->set_mode(tzd,
> THERMAL_DEVICE_ENABLED);
> >
> > -			of_node_put(sensor_specs.np);
> >  			of_node_put(child);
> >  			goto exit;
> >  		}
> > -		of_node_put(sensor_specs.np);
> >  	}
> >  exit:
> >  	of_node_put(sensor_np);
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > 15a4ca5..5edffe6 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -375,6 +375,9 @@ struct thermal_trip {
> >
> >  /* Function declarations */
> >  #ifdef CONFIG_THERMAL_OF
> > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > +				  struct device_node *sensor_np,
> > +				  u32 *id);
> >  struct thermal_zone_device *
> >  thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> >  				const struct thermal_zone_of_device_ops
> *ops); @@ -386,6 +389,13
> > @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register(
> >  void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> >  					    struct thermal_zone_device *tz);
> #else
> > +
> > +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > +					 struct device_node *sensor_np,
> > +					 u32 *id)
> > +{
> > +	return -ENOENT;
> > +}
> >  static inline struct thermal_zone_device *
> > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> >  				const struct thermal_zone_of_device_ops
> *ops)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting sensor ID from DT

[Anson Huang]

Hi, Eduardo

> -----Original Message-----
> From: Anson Huang
> Sent: Wednesday, May 29, 2019 11:37 AM
> To: Eduardo Valentin <edubezval@gmail.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> catalin.marinas@arm.com; will.deacon@arm.com; rui.zhang@intel.com;
> daniel.lezcano@linaro.org; Aisheng Dong <aisheng.dong@nxp.com>;
> ulf.hansson@linaro.org; Peng Fan <peng.fan@nxp.com>; Daniel Baluta
> <daniel.baluta@nxp.com>; maxime.ripard@bootlin.com; olof@lixom.net;
> jagan@amarulasolutions.com; horms+renesas@verge.net.au; Leonard
> Crestez <leonard.crestez@nxp.com>; bjorn.andersson@linaro.org;
> dinguyen@kernel.org; enric.balletbo@collabora.com;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-pm@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: RE: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for getting
> sensor ID from DT
> 
> Hi, Eduardo
> 
> > -----Original Message-----
> > From: Eduardo Valentin <edubezval@gmail.com>
> > Sent: Wednesday, May 29, 2019 11:02 AM
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > catalin.marinas@arm.com; will.deacon@arm.com; rui.zhang@intel.com;
> > daniel.lezcano@linaro.org; Aisheng Dong <aisheng.dong@nxp.com>;
> > ulf.hansson@linaro.org; Peng Fan <peng.fan@nxp.com>; Daniel Baluta
> > <daniel.baluta@nxp.com>; maxime.ripard@bootlin.com; olof@lixom.net;
> > jagan@amarulasolutions.com; horms+renesas@verge.net.au; Leonard
> > Crestez <leonard.crestez@nxp.com>; bjorn.andersson@linaro.org;
> > dinguyen@kernel.org; enric.balletbo@collabora.com;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-pm@vger.kernel.org; dl-linux-imx
> > <linux- imx@nxp.com>
> > Subject: Re: [PATCH RESEND V13 2/5] thermal: of-thermal: add API for
> > getting sensor ID from DT
> >
> > On Tue, May 28, 2019 at 02:06:18PM +0800, Anson.Huang@nxp.com wrote:
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > >
> > > On some platforms like i.MX8QXP, the thermal driver needs a real HW
> > > sensor ID from DT thermal zone, the HW sensor ID is used to get
> > > temperature from SCU firmware, and the virtual sensor ID starting
> > > from
> > > 0 to N is NOT used at all, this patch adds new API
> > > thermal_zone_of_get_sensor_id() to provide the feature of getting
> > > sensor ID from DT thermal zone's node.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > Changes since V12:
> > > 	- adjust the second parameter of thermal_zone_of_get_sensor_id()
> > > API,
> > then caller no need
> > > 	  to pass the of_phandle_args structure and put the sensor_specs.np
> > manually, also putting
> > > 	  the sensor node device check inside this API to make it easy for
> > > usage;
> >
> > What happened to using nxp,resource-id property in your driver?
> > Why do we need this as an API in of-thermal? What other drivers may
> > benefit of this?
> >
> > Regardless, this patch needs to document the new API under
> > Documentation/
> 
> As Rob has different opinion about this property, he thought it is
> unnecessary, see below discussion mail, that is why I need to add API to get
> the resource ID from phandle argument.
> I am totally confused now, which approach should we adopt?
> 
> https://patchwork.kernel.org/patch/10831397/

I will add the new API document in V14, I remembered that there is also other vendors
have similar sensor HW ID as i.MX8QXP, instead of adding private properties for each vendor,
adding an API to read out the sensor ID can benefit us a lot I think.

Thanks,
Anson

> 
> Thanks,
> Anson
> 
> >
> > > ---
> > >  drivers/thermal/of-thermal.c | 66
> > > +++++++++++++++++++++++++++++++++---
> > --------
> > >  include/linux/thermal.h      | 10 +++++++
> > >  2 files changed, 60 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/thermal/of-thermal.c
> > > b/drivers/thermal/of-thermal.c index dc5093b..a53792b 100644
> > > --- a/drivers/thermal/of-thermal.c
> > > +++ b/drivers/thermal/of-thermal.c
> > > @@ -449,6 +449,54 @@ thermal_zone_of_add_sensor(struct
> device_node
> > > *zone,  }
> > >
> > >  /**
> > > + * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal
> > > + zone
> > > + * @tz_np: a valid thermal zone device node.
> > > + * @sensor_np: a sensor node of a valid sensor device.
> > > + * @id: a sensor ID pointer will be passed back.
> > > + *
> > > + * This function will get sensor ID from a given thermal zone node,
> > > + use
> > > + * "thermal-sensors" as list name, and get sensor ID from first
> > > + phandle's
> > > + * argument.
> > > + *
> > > + * Return: 0 on success, proper error code otherwise.
> > > + */
> > > +
> > > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > +				  struct device_node *sensor_np,
> > > +				  u32 *id)
> > > +{
> > > +	struct of_phandle_args sensor_specs;
> > > +	int ret;
> > > +
> > > +	ret = of_parse_phandle_with_args(tz_np,
> > > +					 "thermal-sensors",
> > > +					 "#thermal-sensor-cells",
> > > +					 0,
> > > +					 &sensor_specs);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (sensor_specs.np != sensor_np) {
> > > +		of_node_put(sensor_specs.np);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	if (sensor_specs.args_count >= 1) {
> > > +		*id = sensor_specs.args[0];
> > > +		WARN(sensor_specs.args_count > 1,
> > > +		     "%pOFn: too many cells in sensor specifier %d\n",
> > > +		     sensor_specs.np, sensor_specs.args_count);
> > > +	} else {
> > > +		*id = 0;
> > > +	}
> > > +
> > > +	of_node_put(sensor_specs.np);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id);
> > > +
> > > +/**
> > >   * 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.
> > > @@ -499,36 +547,22 @@ thermal_zone_of_sensor_register(struct device
> > *dev, int sensor_id, void *data,
> > >  	sensor_np = of_node_get(dev->of_node);
> > >
> > >  	for_each_available_child_of_node(np, child) {
> > > -		struct of_phandle_args sensor_specs;
> > >  		int ret, id;
> > >
> > >  		/* For now, thermal framework supports only 1 sensor per
> > zone */
> > > -		ret = of_parse_phandle_with_args(child, "thermal-sensors",
> > > -						 "#thermal-sensor-cells",
> > > -						 0, &sensor_specs);
> > > +		ret = thermal_zone_of_get_sensor_id(child, sensor_np, &id);
> > >  		if (ret)
> > >  			continue;
> > >
> > > -		if (sensor_specs.args_count >= 1) {
> > > -			id = sensor_specs.args[0];
> > > -			WARN(sensor_specs.args_count > 1,
> > > -			     "%pOFn: too many cells in sensor specifier %d\n",
> > > -			     sensor_specs.np, sensor_specs.args_count);
> > > -		} else {
> > > -			id = 0;
> > > -		}
> > > -
> > > -		if (sensor_specs.np == sensor_np && id == sensor_id) {
> > > +		if (id == sensor_id) {
> > >  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
> > >  							 data, ops);
> > >  			if (!IS_ERR(tzd))
> > >  				tzd->ops->set_mode(tzd,
> > THERMAL_DEVICE_ENABLED);
> > >
> > > -			of_node_put(sensor_specs.np);
> > >  			of_node_put(child);
> > >  			goto exit;
> > >  		}
> > > -		of_node_put(sensor_specs.np);
> > >  	}
> > >  exit:
> > >  	of_node_put(sensor_np);
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h index
> > > 15a4ca5..5edffe6 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -375,6 +375,9 @@ struct thermal_trip {
> > >
> > >  /* Function declarations */
> > >  #ifdef CONFIG_THERMAL_OF
> > > +int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > +				  struct device_node *sensor_np,
> > > +				  u32 *id);
> > >  struct thermal_zone_device *
> > >  thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > >  				const struct thermal_zone_of_device_ops
> > *ops); @@ -386,6 +389,13
> > > @@ struct thermal_zone_device
> *devm_thermal_zone_of_sensor_register(
> > >  void devm_thermal_zone_of_sensor_unregister(struct device *dev,
> > >  					    struct thermal_zone_device *tz);
> > #else
> > > +
> > > +static int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > +					 struct device_node *sensor_np,
> > > +					 u32 *id)
> > > +{
> > > +	return -ENOENT;
> > > +}
> > >  static inline struct thermal_zone_device *
> > > thermal_zone_of_sensor_register(struct device *dev, int id, void *data,
> > >  				const struct thermal_zone_of_device_ops
> > *ops)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel