2019-06-10 02:53:04

by Anson Huang

[permalink] [raw]
Subject: [PATCH V14 1/5] dt-bindings: fsl: scu: add thermal binding

From: Anson Huang <[email protected]>

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 <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
No change.
---
.../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


2019-06-10 02:53:09

by Anson Huang

[permalink] [raw]
Subject: [PATCH V14 3/5] thermal: imx_sc: add i.MX system controller thermal support

From: Anson Huang <[email protected]>

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 <[email protected]>
---
No 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 <[email protected]>");
+MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2019-06-10 02:53:13

by Anson Huang

[permalink] [raw]
Subject: [PATCH V14 4/5] defconfig: arm64: add i.MX system controller thermal support

From: Anson Huang <[email protected]>

This patch enables CONFIG_IMX_SC_THERMAL as module.

Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b3d6eb8..323e772 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -414,6 +414,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

2019-06-10 02:53:17

by Anson Huang

[permalink] [raw]
Subject: [PATCH V14 5/5] arm64: dts: imx: add i.MX8QXP thermal support

From: Anson Huang <[email protected]>

Add i.MX8QXP CPU thermal zone support.

Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
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 b2cb818..12044be 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>;
@@ -168,6 +169,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 {
@@ -536,4 +542,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

2019-06-10 02:53:57

by Anson Huang

[permalink] [raw]
Subject: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT

From: Anson Huang <[email protected]>

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 <[email protected]>
---
Changes since V13:
- add new API into the thermal API doc.
---
Documentation/thermal/sysfs-api.txt | 8 +++++
drivers/thermal/of-thermal.c | 66 ++++++++++++++++++++++++++++---------
include/linux/thermal.h | 10 ++++++
3 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index c3fa500..8d7f1b1 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
for the thermal zone device, which might be useful for platform
drivers for temperature calculations.

+1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
+ struct device_node *sensor_np,
+ u32 *id)
+
+ This interface is used to get the sensor id from thermal sensor's
+ phandle argument, it might be necessary for some platforms which
+ have specific sensor ID rather than virtual ID from 0 - N.
+
1.2 thermal cooling device interface
1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
void *devdata, struct thermal_cooling_device_ops *)
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

2019-06-11 12:18:51

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V14 1/5] dt-bindings: fsl: scu: add thermal binding

> From: [email protected] [mailto:[email protected]]
> Sent: Monday, June 10, 2019 10:53 AM
>
> 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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Dong Aisheng

2019-06-11 12:24:43

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT

Hi Anson,

The implementation looks good to me.
A few minor comments on the doc:

> From: [email protected] [mailto:[email protected]]
> Sent: Monday, June 10, 2019 10:53 AM
>
> 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 API is unware of HW ID or Virtual ID. So we probably no need to mention
It here.

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

"It's useful for thermal driver to register the specific thermal zone devices
from DT in a common way."

> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V13:
> - add new API into the thermal API doc.
> ---
> Documentation/thermal/sysfs-api.txt | 8 +++++
> drivers/thermal/of-thermal.c | 66
> ++++++++++++++++++++++++++++---------
> include/linux/thermal.h | 10 ++++++
> 3 files changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index c3fa500..8d7f1b1 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> for the thermal zone device, which might be useful for platform
> drivers for temperature calculations.
>
> +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> + struct device_node *sensor_np,
> + u32 *id)
> +
> + This interface is used to get the sensor id from thermal sensor's
> + phandle argument, it might be necessary for some platforms which
> + have specific sensor ID rather than virtual ID from 0 - N.

Does below one look better?

"This interface is used to get the sensor id from the given thermal zone in
DT, which might be useful for thermal drivers to register specific thermal
zone device in a common way."

> +
> 1.2 thermal cooling device interface
> 1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char
> *name,
> void *devdata, struct thermal_cooling_device_ops *) 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.

the sensor ID returned if success

> + *
> + * 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.

"This function will get sensor ID from a given thermal zone node and the sensor
get must match the temperature providers @sensor_np."

Regards
Dong Aisheng

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

2019-06-11 12:37:37

by Anson Huang

[permalink] [raw]
Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT

Hi, Aisheng

> -----Original Message-----
> From: Aisheng Dong
> Sent: Tuesday, June 11, 2019 7:47 PM
> To: Anson Huang <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Peng Fan <[email protected]>; [email protected];
> [email protected]; Daniel Baluta <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; Leonard Crestez <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]
> Cc: dl-linux-imx <[email protected]>
> Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor
> ID from DT
>
> Hi Anson,
>
> The implementation looks good to me.
> A few minor comments on the doc:

This patch is pending for so long and even till now I am NOT sure if everyone is OK for
this change, so I will wait for some comments if there is any, and will improve the doc
together in next version.

Thanks,
Anson

>
> > From: [email protected] [mailto:[email protected]]
> > Sent: Monday, June 10, 2019 10:53 AM
> >
> > 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 API is unware of HW ID or Virtual ID. So we probably no need to
> mention It here.
>
> > 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.
> >
>
> "It's useful for thermal driver to register the specific thermal zone devices
> from DT in a common way."
>
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V13:
> > - add new API into the thermal API doc.
> > ---
> > Documentation/thermal/sysfs-api.txt | 8 +++++
> > drivers/thermal/of-thermal.c | 66
> > ++++++++++++++++++++++++++++---------
> > include/linux/thermal.h | 10 ++++++
> > 3 files changed, 68 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/thermal/sysfs-api.txt
> > b/Documentation/thermal/sysfs-api.txt
> > index c3fa500..8d7f1b1 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> > for the thermal zone device, which might be useful for platform
> > drivers for temperature calculations.
> >
> > +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > + struct device_node *sensor_np,
> > + u32 *id)
> > +
> > + This interface is used to get the sensor id from thermal sensor's
> > + phandle argument, it might be necessary for some platforms which
> > + have specific sensor ID rather than virtual ID from 0 - N.
>
> Does below one look better?
>
> "This interface is used to get the sensor id from the given thermal zone in DT,
> which might be useful for thermal drivers to register specific thermal zone
> device in a common way."
>
> > +
> > 1.2 thermal cooling device interface
> > 1.2.1 struct thermal_cooling_device
> > *thermal_cooling_device_register(char
> > *name,
> > void *devdata, struct thermal_cooling_device_ops *) 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.
>
> the sensor ID returned if success
>
> > + *
> > + * 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.
>
> "This function will get sensor ID from a given thermal zone node and the
> sensor get must match the temperature providers @sensor_np."
>
> Regards
> Dong Aisheng
>
> > + *
> > + * 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

2019-06-12 07:44:48

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting sensor ID from DT

Hi Eduardo & Rui,

> From: Anson Huang
> Sent: Tuesday, June 11, 2019 8:37 PM
>
> > From: Aisheng Dong
> > Sent: Tuesday, June 11, 2019 7:47 PM
> > Subject: RE: [PATCH V14 2/5] thermal: of-thermal: add API for getting
> > sensor ID from DT
> >
> > Hi Anson,
> >
> > The implementation looks good to me.
> > A few minor comments on the doc:
>
> This patch is pending for so long and even till now I am NOT sure if everyone is
> OK for this change, so I will wait for some comments if there is any, and will
> improve the doc together in next version.
>

The API change seems good to me.
But since this is a core changes, we need your inputs to confirm whether this approach
is ok to you.

So please tell us whether you're fine with this approach.

Regards
Dong Aisheng

> Thanks,
> Anson
>
> >
> > > From: [email protected] [mailto:[email protected]]
> > > Sent: Monday, June 10, 2019 10:53 AM
> > >
> > > 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 API is unware of HW ID or Virtual ID. So we probably no need to
> > mention It here.
> >
> > > 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.
> > >
> >
> > "It's useful for thermal driver to register the specific thermal zone
> > devices from DT in a common way."
> >
> > > Signed-off-by: Anson Huang <[email protected]>
> > > ---
> > > Changes since V13:
> > > - add new API into the thermal API doc.
> > > ---
> > > Documentation/thermal/sysfs-api.txt | 8 +++++
> > > drivers/thermal/of-thermal.c | 66
> > > ++++++++++++++++++++++++++++---------
> > > include/linux/thermal.h | 10 ++++++
> > > 3 files changed, 68 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/Documentation/thermal/sysfs-api.txt
> > > b/Documentation/thermal/sysfs-api.txt
> > > index c3fa500..8d7f1b1 100644
> > > --- a/Documentation/thermal/sysfs-api.txt
> > > +++ b/Documentation/thermal/sysfs-api.txt
> > > @@ -159,6 +159,14 @@ temperature) and throttle appropriate devices.
> > > for the thermal zone device, which might be useful for platform
> > > drivers for temperature calculations.
> > >
> > > +1.1.9 int thermal_zone_of_get_sensor_id(struct device_node *tz_np,
> > > + struct device_node *sensor_np,
> > > + u32 *id)
> > > +
> > > + This interface is used to get the sensor id from thermal sensor's
> > > + phandle argument, it might be necessary for some platforms which
> > > + have specific sensor ID rather than virtual ID from 0 - N.
> >
> > Does below one look better?
> >
> > "This interface is used to get the sensor id from the given thermal
> > zone in DT, which might be useful for thermal drivers to register
> > specific thermal zone device in a common way."
> >
> > > +
> > > 1.2 thermal cooling device interface
> > > 1.2.1 struct thermal_cooling_device
> > > *thermal_cooling_device_register(char
> > > *name,
> > > void *devdata, struct thermal_cooling_device_ops *) 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.
> >
> > the sensor ID returned if success
> >
> > > + *
> > > + * 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.
> >
> > "This function will get sensor ID from a given thermal zone node and
> > the sensor get must match the temperature providers @sensor_np."
> >
> > Regards
> > Dong Aisheng
> >
> > > + *
> > > + * 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