LinuxLists.cc - [PATCH V5 0/4] Add i.MX System Controller thermal driver support

2018-12-20 09:23:09

Subject: [PATCH V5 0/4] Add i.MX System Controller thermal driver support

2018-12-20 09:23:29

Subject: [PATCH V5 4/4] ARM64: dts: imx: add i.MX8QXP thermal support

Add i.MX8QXP CPU thermal zone support.

Signed-off-by: Anson Huang <[email protected]>
---
No change since V4.
---
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 9b08688..48f622c 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -9,6 +9,7 @@
#include <dt-bindings/firmware/imx/rsrc.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/pinctrl/pads-imx8qxp.h>
+#include <dt-bindings/thermal/thermal.h>

#include "imx8-ca35.dtsi"

@@ -87,6 +88,12 @@
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
+
+ tsens: thermal-sensor {
+ compatible = "fsl,imx8qxp-sc-thermal";
+ tsens-num = <1>;
+ #thermal-sensor-cells = <1>;
+ };
};

adma_subsys: bus@59000000 {
@@ -410,4 +417,24 @@
#size-cells = <1>;
ranges = <0x5f000000 0x0 0x5f000000 0x1000000>;
};
+
+ thermal_zones: thermal-zones {
+ cpu-thermal0 {
+ polling-delay-passive = <250>;
+ polling-delay = <2000>;
+ thermal-sensors = <&tsens 355>;
+ trips {
+ cpu_alert0: trip0 {
+ temperature = <107000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_crit0: trip1 {
+ temperature = <127000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+ };
};
--
2.7.4

2018-12-20 10:31:19

by Anson Huang

[permalink] [raw]

Subject: [PATCH V5 1/4] dt-bindings: fsl: scu: add thermal binding

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 since V4.
---
.../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 3f3d697..b77ec29 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -112,6 +112,16 @@ RTC bindings based on SCU Message Protocol
Required properties:
- compatible: should be "fsl,imx8qxp-sc-rtc";

+Thermal bindings based on SCU Message Protocol
+------------------------------------------------------------
+
+Required properties:
+- compatible : Must be "fsl,imx8qxp-sc-thermal";
+- tsens-num : Total number of thermal sensors supported;
+- #thermal-sensor-cells : Should be 1. See
+ Documentation/devicetree/bindings/thermal/thermal.txt
+ for a description.
+
Example (imx8qxp):
-------------
lsio_mu1: mailbox@5d1c0000 {
@@ -158,6 +168,12 @@ firmware {
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
+
+ tsens: thermal-sensor {
+ compatible = "fsl,imx8qxp-sc-thermal";
+ tsens-num = <1>;
+ #thermal-sensor-cells = <1>;
+ };
};
};

--
2.7.4

2018-12-20 10:55:19

by Anson Huang

[permalink] [raw]

Subject: [PATCH V5 3/4] defconfig: arm64: add i.MX system controller thermal support

This patch enables CONFIG_IMX_SC_THERMAL as module.

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

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index abd80c5..9f12324 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -376,6 +376,7 @@ CONFIG_SENSORS_RASPBERRYPI_HWMON=m
CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
CONFIG_CPU_THERMAL=y
CONFIG_THERMAL_EMULATION=y
+CONFIG_IMX_SC_THERMAL=m
CONFIG_ROCKCHIP_THERMAL=m
CONFIG_RCAR_GEN3_THERMAL=y
CONFIG_ARMADA_THERMAL=y
--
2.7.4

2018-12-20 10:55:32

by Anson Huang

[permalink] [raw]

Subject: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

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]>
---
ChangeLog since V4:
- Add driver dependency on IMX_SCU to avoid compile error when IMX_SCU is NOT selected;
- Remove unnecessary of different errors handle for imx_scu_get_handle, already included in API.
---
drivers/thermal/Kconfig | 11 +++
drivers/thermal/Makefile | 1 +
drivers/thermal/imx_sc_thermal.c | 200 +++++++++++++++++++++++++++++++++++++++
3 files changed, 212 insertions(+)
create mode 100644 drivers/thermal/imx_sc_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 0e69edc..8d4601d 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -222,6 +222,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) || COMPILE_TEST
+ 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 610344e..1b13f6a 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..1030a31
--- /dev/null
+++ b/drivers/thermal/imx_sc_thermal.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018 NXP.
+ */
+
+#include <linux/err.h>
+#include <linux/firmware/imx/sci.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
+#define IMX_SC_C_TEMP 0
+
+static struct imx_sc_ipc *thermal_ipc_handle;
+
+struct imx_sc_sensor {
+ struct thermal_zone_device *tzd;
+ u32 resource_id;
+};
+
+struct imx_sc_thermal_data {
+ struct imx_sc_sensor *sensor;
+};
+
+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;
+} __packed;
+
+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) {
+ pr_err("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_register_sensor(struct platform_device *pdev,
+ struct imx_sc_sensor *sensor)
+{
+ struct thermal_zone_device *tzd;
+
+ tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
+ sensor->resource_id,
+ sensor,
+ &imx_sc_thermal_ops);
+ if (IS_ERR(tzd)) {
+ dev_err(&pdev->dev, "failed to register sensor: %d\n",
+ sensor->resource_id);
+ return PTR_ERR(tzd);
+ }
+
+ sensor->tzd = tzd;
+
+ return 0;
+}
+
+static int imx_sc_thermal_get_sensor_id(struct device_node *sensor_np, u32 *id)
+{
+ struct of_phandle_args sensor_specs;
+ int ret;
+
+ ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
+ "#thermal-sensor-cells",
+ 0, &sensor_specs);
+ if (ret)
+ return ret;
+
+ 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 {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int imx_sc_thermal_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *sensor_np = NULL;
+ struct imx_sc_thermal_data *data;
+ struct imx_sc_sensor *sensors;
+ u32 sensor_num;
+ int ret, i;
+
+ ret = imx_scu_get_handle(&thermal_ipc_handle);
+ if (ret)
+ return ret;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "tsens-num", &sensor_num);
+ if (ret || !sensor_num) {
+ dev_err(&pdev->dev, "failed to get valid temp sensor number!\n");
+ return -EINVAL;
+ }
+
+ sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) * sensor_num,
+ GFP_KERNEL);
+ if (!sensors)
+ return -ENOMEM;
+
+ data->sensor = sensors;
+
+ np = of_find_node_by_name(NULL, "thermal-zones");
+ if (!np)
+ return -ENODEV;
+
+ for (i = 0; i < sensor_num; i++) {
+ struct imx_sc_sensor *sensor = &data->sensor[i];
+
+ sensor_np = of_get_next_child(np, sensor_np);
+ ret = imx_sc_thermal_get_sensor_id(sensor_np, &sensor->resource_id);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "failed to get valid sensor resource id: %d\n",
+ ret);
+ break;
+ }
+
+ ret = imx_sc_thermal_register_sensor(pdev, sensor);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
+ ret);
+ break;
+ }
+ }
+
+ of_node_put(np);
+ of_node_put(sensor_np);
+
+ return ret;
+}
+
+static const struct of_device_id imx_sc_thermal_table[] = {
+ { .compatible = "fsl,imx8qxp-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

2018-12-25 22:48:47

by Eduardo Valentin

[permalink] [raw]

Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> 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]>
> ---
> ChangeLog since V4:
> - Add driver dependency on IMX_SCU to avoid compile error when IMX_SCU is NOT selected;
> - Remove unnecessary of different errors handle for imx_scu_get_handle, already included in API.
> ---
> drivers/thermal/Kconfig | 11 +++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/imx_sc_thermal.c | 200 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 212 insertions(+)
> create mode 100644 drivers/thermal/imx_sc_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 0e69edc..8d4601d 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -222,6 +222,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) || COMPILE_TEST
> + 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 610344e..1b13f6a 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..1030a31
> --- /dev/null
> +++ b/drivers/thermal/imx_sc_thermal.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 NXP.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/firmware/imx/sci.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
> +#define IMX_SC_C_TEMP 0
> +
> +static struct imx_sc_ipc *thermal_ipc_handle;
> +
> +struct imx_sc_sensor {
> + struct thermal_zone_device *tzd;
> + u32 resource_id;
> +};
> +
> +struct imx_sc_thermal_data {
> + struct imx_sc_sensor *sensor;
> +};
> +
> +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;
> +} __packed;
> +
> +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) {
> + pr_err("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_register_sensor(struct platform_device *pdev,
> + struct imx_sc_sensor *sensor)
> +{
> + struct thermal_zone_device *tzd;
> +
> + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> + sensor->resource_id,
> + sensor,
> + &imx_sc_thermal_ops);
> + if (IS_ERR(tzd)) {
> + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> + sensor->resource_id);
> + return PTR_ERR(tzd);
> + }
> +
> + sensor->tzd = tzd;
> +
> + return 0;
> +}
> +
> +static int imx_sc_thermal_get_sensor_id(struct device_node *sensor_np, u32 *id)
> +{
> + struct of_phandle_args sensor_specs;
> + int ret;
> +
> + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> + "#thermal-sensor-cells",
> + 0, &sensor_specs);
> + if (ret)
> + return ret;
> +
> + 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 {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +

So, there should be no need for doing a search of sensor ids. In fact,
if the amount of sensors or the type of sensors supported changes, i.e.
the sensor ids are do not match, you should have a different compatible, IMO.

> +static int imx_sc_thermal_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *sensor_np = NULL;
> + struct imx_sc_thermal_data *data;
> + struct imx_sc_sensor *sensors;
> + u32 sensor_num;
> + int ret, i;
> +
> + ret = imx_scu_get_handle(&thermal_ipc_handle);
> + if (ret)
> + return ret;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32(np, "tsens-num", &sensor_num);
> + if (ret || !sensor_num) {
> + dev_err(&pdev->dev, "failed to get valid temp sensor number!\n");
> + return -EINVAL;
> + }
> +
> + sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) * sensor_num,
> + GFP_KERNEL);
> + if (!sensors)
> + return -ENOMEM;
> +
> + data->sensor = sensors;
> +
> + np = of_find_node_by_name(NULL, "thermal-zones");
> + if (!np)
> + return -ENODEV;
> +
> + for (i = 0; i < sensor_num; i++) {
> + struct imx_sc_sensor *sensor = &data->sensor[i];
> +
> + sensor_np = of_get_next_child(np, sensor_np);
> + ret = imx_sc_thermal_get_sensor_id(sensor_np, &sensor->resource_id);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "failed to get valid sensor resource id: %d\n",
> + ret);
> + break;
> + }
> +
> + ret = imx_sc_thermal_register_sensor(pdev, sensor);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> + ret);
> + break;
> + }
> + }
> +
> + of_node_put(np);
> + of_node_put(sensor_np);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id imx_sc_thermal_table[] = {
> + { .compatible = "fsl,imx8qxp-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
>

2018-12-26 04:01:57

by Anson Huang

[permalink] [raw]

Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

Hi, Eduardo

Best Regards!
Anson Huang

> -----Original Message-----
> From: Eduardo Valentin [mailto:[email protected]]
> Sent: 2018??12??26?? 6:44
> To: Anson Huang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Aisheng Dong <[email protected]>; [email protected];
> [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller
> thermal support
>
> On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> > 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]>
> > ---
> > ChangeLog since V4:
> > - Add driver dependency on IMX_SCU to avoid compile error when
> IMX_SCU is NOT selected;
> > - Remove unnecessary of different errors handle for
> imx_scu_get_handle, already included in API.
> > ---
> > drivers/thermal/Kconfig | 11 +++
> > drivers/thermal/Makefile | 1 +
> > drivers/thermal/imx_sc_thermal.c | 200
> > +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 212 insertions(+)
> > create mode 100644 drivers/thermal/imx_sc_thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > 0e69edc..8d4601d 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -222,6 +222,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) || COMPILE_TEST
> > + 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
> > 610344e..1b13f6a 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..1030a31
> > --- /dev/null
> > +++ b/drivers/thermal/imx_sc_thermal.c
> > @@ -0,0 +1,200 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2018 NXP.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/firmware/imx/sci.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
> > +#define IMX_SC_C_TEMP 0
> > +
> > +static struct imx_sc_ipc *thermal_ipc_handle;
> > +
> > +struct imx_sc_sensor {
> > + struct thermal_zone_device *tzd;
> > + u32 resource_id;
> > +};
> > +
> > +struct imx_sc_thermal_data {
> > + struct imx_sc_sensor *sensor;
> > +};
> > +
> > +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;
> > +} __packed;
> > +
> > +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) {
> > + pr_err("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_register_sensor(struct platform_device *pdev,
> > + struct imx_sc_sensor *sensor)
> > +{
> > + struct thermal_zone_device *tzd;
> > +
> > + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > + sensor->resource_id,
> > + sensor,
> > + &imx_sc_thermal_ops);
> > + if (IS_ERR(tzd)) {
> > + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> > + sensor->resource_id);
> > + return PTR_ERR(tzd);
> > + }
> > +
> > + sensor->tzd = tzd;
> > +
> > + return 0;
> > +}
> > +
> > +static int imx_sc_thermal_get_sensor_id(struct device_node
> > +*sensor_np, u32 *id) {
> > + struct of_phandle_args sensor_specs;
> > + int ret;
> > +
> > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > + "#thermal-sensor-cells",
> > + 0, &sensor_specs);
> > + if (ret)
> > + return ret;
> > +
> > + 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 {
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> So, there should be no need for doing a search of sensor ids. In fact, if the
> amount of sensors or the type of sensors supported changes, i.e.
> the sensor ids are do not match, you should have a different compatible, IMO.

So do you mean I can just read the sensor id (actually it is our SoC's resource id) from the
DTB file's each child node of thermal_zones? Then I can skip the search of sensor ids, just
add a sensor id property and read it out during thermal driver probe? Yes, this looks like
easier, is my understanding correct? If yes, I can do it in next version. Thanks.

Anson

>
> > +static int imx_sc_thermal_probe(struct platform_device *pdev) {
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device_node *sensor_np = NULL;
> > + struct imx_sc_thermal_data *data;
> > + struct imx_sc_sensor *sensors;
> > + u32 sensor_num;
> > + int ret, i;
> > +
> > + ret = imx_scu_get_handle(&thermal_ipc_handle);
> > + if (ret)
> > + return ret;
> > +
> > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + ret = of_property_read_u32(np, "tsens-num", &sensor_num);
> > + if (ret || !sensor_num) {
> > + dev_err(&pdev->dev, "failed to get valid temp sensor number!\n");
> > + return -EINVAL;
> > + }
> > +
> > + sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) * sensor_num,
> > + GFP_KERNEL);
> > + if (!sensors)
> > + return -ENOMEM;
> > +
> > + data->sensor = sensors;
> > +
> > + np = of_find_node_by_name(NULL, "thermal-zones");
> > + if (!np)
> > + return -ENODEV;
> > +
> > + for (i = 0; i < sensor_num; i++) {
> > + struct imx_sc_sensor *sensor = &data->sensor[i];
> > +
> > + sensor_np = of_get_next_child(np, sensor_np);
> > + ret = imx_sc_thermal_get_sensor_id(sensor_np,
> &sensor->resource_id);
> > + if (ret < 0) {
> > + dev_err(&pdev->dev,
> > + "failed to get valid sensor resource id: %d\n",
> > + ret);
> > + break;
> > + }
> > +
> > + ret = imx_sc_thermal_register_sensor(pdev, sensor);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> > + ret);
> > + break;
> > + }
> > + }
> > +
> > + of_node_put(np);
> > + of_node_put(sensor_np);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct of_device_id imx_sc_thermal_table[] = {
> > + { .compatible = "fsl,imx8qxp-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-01-10 09:30:50

by Anson Huang

[permalink] [raw]

Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

Hi, Eduardo

Best Regards!
Anson Huang

> -----Original Message-----
> From: Anson Huang
> Sent: 2018??12??26?? 11:36
> To: 'Eduardo Valentin' <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Aisheng Dong <[email protected]>; [email protected];
> [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller
> thermal support
>
> Hi, Eduardo
>
> Best Regards!
> Anson Huang
>
> > -----Original Message-----
> > From: Eduardo Valentin [mailto:[email protected]]
> > Sent: 2018??12??26?? 6:44
> > To: Anson Huang <[email protected]>
> > Cc: [email protected]; [email protected];
> [email protected];
> > [email protected]; [email protected]; [email protected];
> > Aisheng Dong <[email protected]>; [email protected];
> > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > controller thermal support
> >
> > On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> > > 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]>
> > > ---
> > > ChangeLog since V4:
> > > - Add driver dependency on IMX_SCU to avoid compile error when
> > IMX_SCU is NOT selected;
> > > - Remove unnecessary of different errors handle for
> > imx_scu_get_handle, already included in API.
> > > ---
> > > drivers/thermal/Kconfig | 11 +++
> > > drivers/thermal/Makefile | 1 +
> > > drivers/thermal/imx_sc_thermal.c | 200
> > > +++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 212 insertions(+)
> > > create mode 100644 drivers/thermal/imx_sc_thermal.c
> > >
> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > > 0e69edc..8d4601d 100644
> > > --- a/drivers/thermal/Kconfig
> > > +++ b/drivers/thermal/Kconfig
> > > @@ -222,6 +222,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) || COMPILE_TEST
> > > + 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 610344e..1b13f6a 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..1030a31
> > > --- /dev/null
> > > +++ b/drivers/thermal/imx_sc_thermal.c
> > > @@ -0,0 +1,200 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2018 NXP.
> > > + */
> > > +
> > > +#include <linux/err.h>
> > > +#include <linux/firmware/imx/sci.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
> > > +#define IMX_SC_C_TEMP 0
> > > +
> > > +static struct imx_sc_ipc *thermal_ipc_handle;
> > > +
> > > +struct imx_sc_sensor {
> > > + struct thermal_zone_device *tzd;
> > > + u32 resource_id;
> > > +};
> > > +
> > > +struct imx_sc_thermal_data {
> > > + struct imx_sc_sensor *sensor;
> > > +};
> > > +
> > > +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;
> > > +} __packed;
> > > +
> > > +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) {
> > > + pr_err("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_register_sensor(struct platform_device *pdev,
> > > + struct imx_sc_sensor *sensor) {
> > > + struct thermal_zone_device *tzd;
> > > +
> > > + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > > + sensor->resource_id,
> > > + sensor,
> > > + &imx_sc_thermal_ops);
> > > + if (IS_ERR(tzd)) {
> > > + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> > > + sensor->resource_id);
> > > + return PTR_ERR(tzd);
> > > + }
> > > +
> > > + sensor->tzd = tzd;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int imx_sc_thermal_get_sensor_id(struct device_node
> > > +*sensor_np, u32 *id) {
> > > + struct of_phandle_args sensor_specs;
> > > + int ret;
> > > +
> > > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > > + "#thermal-sensor-cells",
> > > + 0, &sensor_specs);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + 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 {
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > So, there should be no need for doing a search of sensor ids. In fact,
> > if the amount of sensors or the type of sensors supported changes, i.e.
> > the sensor ids are do not match, you should have a different compatible,
> IMO.
>
> So do you mean I can just read the sensor id (actually it is our SoC's resource id)
> from the DTB file's each child node of thermal_zones? Then I can skip the
> search of sensor ids, just add a sensor id property and read it out during
> thermal driver probe? Yes, this looks like easier, is my understanding correct?
> If yes, I can do it in next version. Thanks.
>
> Anson

Can you be more detail about the search of sensor ids? For this SoC, there will be more
sensors(thermal-zone) added later, like DDR controller thermal sensor and pmic sensor etc., they will use
same compatible, and in the imx_sc_thermal_get_sensor_id(), it just read out the sensor id
from the dtb's each thermal zone's " thermal-sensors " property, I did NOT see any search here, thanks!

Anson.

>
> >
> > > +static int imx_sc_thermal_probe(struct platform_device *pdev) {
> > > + struct device_node *np = pdev->dev.of_node;
> > > + struct device_node *sensor_np = NULL;
> > > + struct imx_sc_thermal_data *data;
> > > + struct imx_sc_sensor *sensors;
> > > + u32 sensor_num;
> > > + int ret, i;
> > > +
> > > + ret = imx_scu_get_handle(&thermal_ipc_handle);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > > + if (!data)
> > > + return -ENOMEM;
> > > +
> > > + ret = of_property_read_u32(np, "tsens-num", &sensor_num);
> > > + if (ret || !sensor_num) {
> > > + dev_err(&pdev->dev, "failed to get valid temp sensor number!\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) * sensor_num,
> > > + GFP_KERNEL);
> > > + if (!sensors)
> > > + return -ENOMEM;
> > > +
> > > + data->sensor = sensors;
> > > +
> > > + np = of_find_node_by_name(NULL, "thermal-zones");
> > > + if (!np)
> > > + return -ENODEV;
> > > +
> > > + for (i = 0; i < sensor_num; i++) {
> > > + struct imx_sc_sensor *sensor = &data->sensor[i];
> > > +
> > > + sensor_np = of_get_next_child(np, sensor_np);
> > > + ret = imx_sc_thermal_get_sensor_id(sensor_np,
> > &sensor->resource_id);
> > > + if (ret < 0) {
> > > + dev_err(&pdev->dev,
> > > + "failed to get valid sensor resource id: %d\n",
> > > + ret);
> > > + break;
> > > + }
> > > +
> > > + ret = imx_sc_thermal_register_sensor(pdev, sensor);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> > > + ret);
> > > + break;
> > > + }
> > > + }
> > > +
> > > + of_node_put(np);
> > > + of_node_put(sensor_np);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static const struct of_device_id imx_sc_thermal_table[] = {
> > > + { .compatible = "fsl,imx8qxp-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-01-22 06:56:49

by Anson Huang

[permalink] [raw]

Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

Hi, Eduardo

Best Regards!
Anson Huang

> -----Original Message-----
> From: Anson Huang
> Sent: 2019??1??10?? 17:28
> To: 'Eduardo Valentin' <[email protected]>
> Cc: '[email protected]' <[email protected]>; '[email protected]'
> <[email protected]>; '[email protected]'
> <[email protected]>; '[email protected]'
> <[email protected]>; '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>; Aisheng Dong
> <[email protected]>; '[email protected]'
> <[email protected]>; '[email protected]' <[email protected]>;
> Andy Gross <[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]'
> <[email protected]>; '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>; dl-linux-imx
> <[email protected]>
> Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller
> thermal support
>
> Hi, Eduardo
>
> Best Regards!
> Anson Huang
>
> > -----Original Message-----
> > From: Anson Huang
> > Sent: 2018??12??26?? 11:36
> > To: 'Eduardo Valentin' <[email protected]>
> > Cc: [email protected]; [email protected];
> [email protected];
> > [email protected]; [email protected]; [email protected];
> > Aisheng Dong <[email protected]>; [email protected];
> > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > controller thermal support
> >
> > Hi, Eduardo
> >
> > Best Regards!
> > Anson Huang
> >
> > > -----Original Message-----
> > > From: Eduardo Valentin [mailto:[email protected]]
> > > Sent: 2018??12??26?? 6:44
> > > To: Anson Huang <[email protected]>
> > > Cc: [email protected]; [email protected];
> > [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > Aisheng Dong <[email protected]>; [email protected];
> > > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > > Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > > controller thermal support
> > >
> > > On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> > > > 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]>
> > > > ---
> > > > ChangeLog since V4:
> > > > - Add driver dependency on IMX_SCU to avoid compile error when
> > > IMX_SCU is NOT selected;
> > > > - Remove unnecessary of different errors handle for
> > > imx_scu_get_handle, already included in API.
> > > > ---
> > > > drivers/thermal/Kconfig | 11 +++
> > > > drivers/thermal/Makefile | 1 +
> > > > drivers/thermal/imx_sc_thermal.c | 200
> > > > +++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 212 insertions(+) create mode 100644
> > > > drivers/thermal/imx_sc_thermal.c
> > > >
> > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > index 0e69edc..8d4601d 100644
> > > > --- a/drivers/thermal/Kconfig
> > > > +++ b/drivers/thermal/Kconfig
> > > > @@ -222,6 +222,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) || COMPILE_TEST
> > > > + 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 610344e..1b13f6a 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..1030a31
> > > > --- /dev/null
> > > > +++ b/drivers/thermal/imx_sc_thermal.c
> > > > @@ -0,0 +1,200 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright 2018 NXP.
> > > > + */
> > > > +
> > > > +#include <linux/err.h>
> > > > +#include <linux/firmware/imx/sci.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
> > > > +#define IMX_SC_C_TEMP 0
> > > > +
> > > > +static struct imx_sc_ipc *thermal_ipc_handle;
> > > > +
> > > > +struct imx_sc_sensor {
> > > > + struct thermal_zone_device *tzd;
> > > > + u32 resource_id;
> > > > +};
> > > > +
> > > > +struct imx_sc_thermal_data {
> > > > + struct imx_sc_sensor *sensor;
> > > > +};
> > > > +
> > > > +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;
> > > > +} __packed;
> > > > +
> > > > +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) {
> > > > + pr_err("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_register_sensor(struct platform_device *pdev,
> > > > + struct imx_sc_sensor *sensor) {
> > > > + struct thermal_zone_device *tzd;
> > > > +
> > > > + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > > > + sensor->resource_id,
> > > > + sensor,
> > > > + &imx_sc_thermal_ops);
> > > > + if (IS_ERR(tzd)) {
> > > > + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> > > > + sensor->resource_id);
> > > > + return PTR_ERR(tzd);
> > > > + }
> > > > +
> > > > + sensor->tzd = tzd;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int imx_sc_thermal_get_sensor_id(struct device_node
> > > > +*sensor_np, u32 *id) {
> > > > + struct of_phandle_args sensor_specs;
> > > > + int ret;
> > > > +
> > > > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > > > + "#thermal-sensor-cells",
> > > > + 0, &sensor_specs);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + 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 {
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > So, there should be no need for doing a search of sensor ids. In
> > > fact, if the amount of sensors or the type of sensors supported changes, i.e.
> > > the sensor ids are do not match, you should have a different
> > > compatible,
> > IMO.
> >
> > So do you mean I can just read the sensor id (actually it is our SoC's
> > resource id) from the DTB file's each child node of thermal_zones?
> > Then I can skip the search of sensor ids, just add a sensor id
> > property and read it out during thermal driver probe? Yes, this looks like
> easier, is my understanding correct?
> > If yes, I can do it in next version. Thanks.
> >
> > Anson
>
> Can you be more detail about the search of sensor ids? For this SoC, there will
> be more
> sensors(thermal-zone) added later, like DDR controller thermal sensor and
> pmic sensor etc., they will use same compatible, and in the
> imx_sc_thermal_get_sensor_id(), it just read out the sensor id from the dtb's
> each thermal zone's " thermal-sensors " property, I did NOT see any search
> here, thanks!
>
> Anson.

I can remove the imx_sc_thermal_get_sensor_id() in the patch, and add below property in dts, then
just read out the resource id using of_property_read_u32(), then we can save the sensor id search function
in the driver, but need to add a property in dts, is this what you mean?

.probe:
- ret = imx_sc_thermal_get_sensor_id(sensor_np, &sensor->resource_id);
- if (ret < 0) {
+ if (of_property_read_u32(sensor_np, "sensor-resource-id",
+ &sensor->resource_id)) {

dts:
454 thermal_zones: thermal-zones {
455 cpu-thermal0 {
456 polling-delay-passive = <250>;
457 polling-delay = <2000>;
458 + sensor-resource-id = <355>;
459 thermal-sensors = <&tsens 355>;

Anson.

>
> >
> > >
> > > > +static int imx_sc_thermal_probe(struct platform_device *pdev) {
> > > > + struct device_node *np = pdev->dev.of_node;
> > > > + struct device_node *sensor_np = NULL;
> > > > + struct imx_sc_thermal_data *data;
> > > > + struct imx_sc_sensor *sensors;
> > > > + u32 sensor_num;
> > > > + int ret, i;
> > > > +
> > > > + ret = imx_scu_get_handle(&thermal_ipc_handle);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > > > + if (!data)
> > > > + return -ENOMEM;
> > > > +
> > > > + ret = of_property_read_u32(np, "tsens-num", &sensor_num);
> > > > + if (ret || !sensor_num) {
> > > > + dev_err(&pdev->dev, "failed to get valid temp sensor
> number!\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) *
> sensor_num,
> > > > + GFP_KERNEL);
> > > > + if (!sensors)
> > > > + return -ENOMEM;
> > > > +
> > > > + data->sensor = sensors;
> > > > +
> > > > + np = of_find_node_by_name(NULL, "thermal-zones");
> > > > + if (!np)
> > > > + return -ENODEV;
> > > > +
> > > > + for (i = 0; i < sensor_num; i++) {
> > > > + struct imx_sc_sensor *sensor = &data->sensor[i];
> > > > +
> > > > + sensor_np = of_get_next_child(np, sensor_np);
> > > > + ret = imx_sc_thermal_get_sensor_id(sensor_np,
> > > &sensor->resource_id);
> > > > + if (ret < 0) {
> > > > + dev_err(&pdev->dev,
> > > > + "failed to get valid sensor resource id: %d\n",
> > > > + ret);
> > > > + break;
> > > > + }
> > > > +
> > > > + ret = imx_sc_thermal_register_sensor(pdev, sensor);
> > > > + if (ret) {
> > > > + dev_err(&pdev->dev, "failed to register thermal
> sensor: %d\n",
> > > > + ret);
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + of_node_put(np);
> > > > + of_node_put(sensor_np);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static const struct of_device_id imx_sc_thermal_table[] = {
> > > > + { .compatible = "fsl,imx8qxp-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-02-05 23:54:08

by Eduardo Valentin

[permalink] [raw]

Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

On Tue, Jan 22, 2019 at 06:54:30AM +0000, Anson Huang wrote:
> Hi, Eduardo
>
> Best Regards!
> Anson Huang

Taking a look

>
> > -----Original Message-----
> > From: Anson Huang
> > Sent: 2019年1月10日 17:28
> > To: 'Eduardo Valentin' <[email protected]>
> > Cc: '[email protected]' <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>; Aisheng Dong
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > Andy Gross <[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]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>; dl-linux-imx
> > <[email protected]>
> > Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller
> > thermal support
> >
> > Hi, Eduardo
> >
> > Best Regards!
> > Anson Huang
> >
> > > -----Original Message-----
> > > From: Anson Huang
> > > Sent: 2018年12月26日 11:36
> > > To: 'Eduardo Valentin' <[email protected]>
> > > Cc: [email protected]; [email protected];
> > [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > Aisheng Dong <[email protected]>; [email protected];
> > > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > > Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > > controller thermal support
> > >
> > > Hi, Eduardo
> > >
> > > Best Regards!
> > > Anson Huang
> > >
> > > > -----Original Message-----
> > > > From: Eduardo Valentin [mailto:[email protected]]
> > > > Sent: 2018年12月26日 6:44
> > > > To: Anson Huang <[email protected]>
> > > > Cc: [email protected]; [email protected];
> > > [email protected];
> > > > [email protected]; [email protected]; [email protected];
> > > > Aisheng Dong <[email protected]>; [email protected];
> > > > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > > > Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > > > controller thermal support
> > > >
> > > > On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> > > > > 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]>
> > > > > ---
> > > > > ChangeLog since V4:
> > > > > - Add driver dependency on IMX_SCU to avoid compile error when
> > > > IMX_SCU is NOT selected;
> > > > > - Remove unnecessary of different errors handle for
> > > > imx_scu_get_handle, already included in API.
> > > > > ---
> > > > > drivers/thermal/Kconfig | 11 +++
> > > > > drivers/thermal/Makefile | 1 +
> > > > > drivers/thermal/imx_sc_thermal.c | 200
> > > > > +++++++++++++++++++++++++++++++++++++++
> > > > > 3 files changed, 212 insertions(+) create mode 100644
> > > > > drivers/thermal/imx_sc_thermal.c
> > > > >
> > > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > > index 0e69edc..8d4601d 100644
> > > > > --- a/drivers/thermal/Kconfig
> > > > > +++ b/drivers/thermal/Kconfig
> > > > > @@ -222,6 +222,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) || COMPILE_TEST
> > > > > + 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 610344e..1b13f6a 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..1030a31
> > > > > --- /dev/null
> > > > > +++ b/drivers/thermal/imx_sc_thermal.c
> > > > > @@ -0,0 +1,200 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright 2018 NXP.
> > > > > + */
> > > > > +
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/firmware/imx/sci.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
> > > > > +#define IMX_SC_C_TEMP 0
> > > > > +
> > > > > +static struct imx_sc_ipc *thermal_ipc_handle;
> > > > > +
> > > > > +struct imx_sc_sensor {
> > > > > + struct thermal_zone_device *tzd;
> > > > > + u32 resource_id;
> > > > > +};
> > > > > +
> > > > > +struct imx_sc_thermal_data {
> > > > > + struct imx_sc_sensor *sensor;
> > > > > +};
> > > > > +
> > > > > +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;
> > > > > +} __packed;
> > > > > +
> > > > > +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) {
> > > > > + pr_err("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_register_sensor(struct platform_device *pdev,
> > > > > + struct imx_sc_sensor *sensor) {
> > > > > + struct thermal_zone_device *tzd;
> > > > > +
> > > > > + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > > > > + sensor->resource_id,
> > > > > + sensor,
> > > > > + &imx_sc_thermal_ops);
> > > > > + if (IS_ERR(tzd)) {
> > > > > + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> > > > > + sensor->resource_id);
> > > > > + return PTR_ERR(tzd);
> > > > > + }
> > > > > +
> > > > > + sensor->tzd = tzd;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int imx_sc_thermal_get_sensor_id(struct device_node
> > > > > +*sensor_np, u32 *id) {
> > > > > + struct of_phandle_args sensor_specs;
> > > > > + int ret;
> > > > > +
> > > > > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > > > > + "#thermal-sensor-cells",
> > > > > + 0, &sensor_specs);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + 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 {
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > >
> > > > So, there should be no need for doing a search of sensor ids. In
> > > > fact, if the amount of sensors or the type of sensors supported changes, i.e.
> > > > the sensor ids are do not match, you should have a different
> > > > compatible,
> > > IMO.
> > >
> > > So do you mean I can just read the sensor id (actually it is our SoC's
> > > resource id) from the DTB file's each child node of thermal_zones?
> > > Then I can skip the search of sensor ids, just add a sensor id
> > > property and read it out during thermal driver probe? Yes, this looks like
> > easier, is my understanding correct?
> > > If yes, I can do it in next version. Thanks.
> > >
> > > Anson
> >
> > Can you be more detail about the search of sensor ids? For this SoC, there will
> > be more
> > sensors(thermal-zone) added later, like DDR controller thermal sensor and
> > pmic sensor etc., they will use same compatible, and in the
> > imx_sc_thermal_get_sensor_id(), it just read out the sensor id from the dtb's
> > each thermal zone's " thermal-sensors " property, I did NOT see any search
> > here, thanks!
> >
> > Anson.
>
> I can remove the imx_sc_thermal_get_sensor_id() in the patch, and add below property in dts, then
> just read out the resource id using of_property_read_u32(), then we can save the sensor id search function
> in the driver, but need to add a property in dts, is this what you mean?
>
> .probe:
> - ret = imx_sc_thermal_get_sensor_id(sensor_np, &sensor->resource_id);
> - if (ret < 0) {
> + if (of_property_read_u32(sensor_np, "sensor-resource-id",
> + &sensor->resource_id)) {
>
> dts:
> 454 thermal_zones: thermal-zones {
> 455 cpu-thermal0 {
> 456 polling-delay-passive = <250>;
> 457 polling-delay = <2000>;
> 458 + sensor-resource-id = <355>;
> 459 thermal-sensors = <&tsens 355>;
>
> Anson.
>
> >
> > >
> > > >
> > > > > +static int imx_sc_thermal_probe(struct platform_device *pdev) {
> > > > > + struct device_node *np = pdev->dev.of_node;
> > > > > + struct device_node *sensor_np = NULL;
> > > > > + struct imx_sc_thermal_data *data;
> > > > > + struct imx_sc_sensor *sensors;
> > > > > + u32 sensor_num;
> > > > > + int ret, i;
> > > > > +
> > > > > + ret = imx_scu_get_handle(&thermal_ipc_handle);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > > > > + if (!data)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + ret = of_property_read_u32(np, "tsens-num", &sensor_num);
> > > > > + if (ret || !sensor_num) {
> > > > > + dev_err(&pdev->dev, "failed to get valid temp sensor
> > number!\n");
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + sensors = devm_kzalloc(&pdev->dev, sizeof(*data->sensor) *
> > sensor_num,
> > > > > + GFP_KERNEL);
> > > > > + if (!sensors)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + data->sensor = sensors;
> > > > > +
> > > > > + np = of_find_node_by_name(NULL, "thermal-zones");
> > > > > + if (!np)
> > > > > + return -ENODEV;
> > > > > +
> > > > > + for (i = 0; i < sensor_num; i++) {
> > > > > + struct imx_sc_sensor *sensor = &data->sensor[i];
> > > > > +
> > > > > + sensor_np = of_get_next_child(np, sensor_np);
> > > > > + ret = imx_sc_thermal_get_sensor_id(sensor_np,
> > > > &sensor->resource_id);
> > > > > + if (ret < 0) {
> > > > > + dev_err(&pdev->dev,
> > > > > + "failed to get valid sensor resource id: %d\n",
> > > > > + ret);
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + ret = imx_sc_thermal_register_sensor(pdev, sensor);
> > > > > + if (ret) {
> > > > > + dev_err(&pdev->dev, "failed to register thermal
> > sensor: %d\n",
> > > > > + ret);
> > > > > + break;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + of_node_put(np);
> > > > > + of_node_put(sensor_np);
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +static const struct of_device_id imx_sc_thermal_table[] = {
> > > > > + { .compatible = "fsl,imx8qxp-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-02-05 23:56:15

by Eduardo Valentin

[permalink] [raw]

Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller thermal support

On Tue, Jan 22, 2019 at 06:54:30AM +0000, Anson Huang wrote:
> Hi, Eduardo
>
> Best Regards!
> Anson Huang
>
> > -----Original Message-----
> > From: Anson Huang
> > Sent: 2019年1月10日 17:28
> > To: 'Eduardo Valentin' <[email protected]>
> > Cc: '[email protected]' <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>; Aisheng Dong
> > <[email protected]>; '[email protected]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > Andy Gross <[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]'
> > <[email protected]>; '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>;
> > '[email protected]' <[email protected]>; dl-linux-imx
> > <[email protected]>
> > Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system controller
> > thermal support
> >
> > Hi, Eduardo
> >
> > Best Regards!
> > Anson Huang
> >
> > > -----Original Message-----
> > > From: Anson Huang
> > > Sent: 2018年12月26日 11:36
> > > To: 'Eduardo Valentin' <[email protected]>
> > > Cc: [email protected]; [email protected];
> > [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > Aisheng Dong <[email protected]>; [email protected];
> > > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > > Subject: RE: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > > controller thermal support
> > >
> > > Hi, Eduardo
> > >
> > > Best Regards!
> > > Anson Huang
> > >
> > > > -----Original Message-----
> > > > From: Eduardo Valentin [mailto:[email protected]]
> > > > Sent: 2018年12月26日 6:44
> > > > To: Anson Huang <[email protected]>
> > > > Cc: [email protected]; [email protected];
> > > [email protected];
> > > > [email protected]; [email protected]; [email protected];
> > > > Aisheng Dong <[email protected]>; [email protected];
> > > > [email protected]; Andy Gross <[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]; dl-linux-imx <[email protected]>
> > > > Subject: Re: [PATCH V5 2/4] thermal: imx_sc: add i.MX system
> > > > controller thermal support
> > > >
> > > > On Thu, Dec 20, 2018 at 09:08:32AM +0000, Anson Huang wrote:
> > > > > 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]>
> > > > > ---
> > > > > ChangeLog since V4:
> > > > > - Add driver dependency on IMX_SCU to avoid compile error when
> > > > IMX_SCU is NOT selected;
> > > > > - Remove unnecessary of different errors handle for
> > > > imx_scu_get_handle, already included in API.
> > > > > ---
> > > > > drivers/thermal/Kconfig | 11 +++
> > > > > drivers/thermal/Makefile | 1 +
> > > > > drivers/thermal/imx_sc_thermal.c | 200
> > > > > +++++++++++++++++++++++++++++++++++++++
> > > > > 3 files changed, 212 insertions(+) create mode 100644
> > > > > drivers/thermal/imx_sc_thermal.c
> > > > >
> > > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > > index 0e69edc..8d4601d 100644
> > > > > --- a/drivers/thermal/Kconfig
> > > > > +++ b/drivers/thermal/Kconfig
> > > > > @@ -222,6 +222,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) || COMPILE_TEST
> > > > > + 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 610344e..1b13f6a 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..1030a31
> > > > > --- /dev/null
> > > > > +++ b/drivers/thermal/imx_sc_thermal.c
> > > > > @@ -0,0 +1,200 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright 2018 NXP.
> > > > > + */
> > > > > +
> > > > > +#include <linux/err.h>
> > > > > +#include <linux/firmware/imx/sci.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
> > > > > +#define IMX_SC_C_TEMP 0
> > > > > +
> > > > > +static struct imx_sc_ipc *thermal_ipc_handle;
> > > > > +
> > > > > +struct imx_sc_sensor {
> > > > > + struct thermal_zone_device *tzd;
> > > > > + u32 resource_id;
> > > > > +};
> > > > > +
> > > > > +struct imx_sc_thermal_data {
> > > > > + struct imx_sc_sensor *sensor;
> > > > > +};
> > > > > +
> > > > > +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;
> > > > > +} __packed;
> > > > > +
> > > > > +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) {
> > > > > + pr_err("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_register_sensor(struct platform_device *pdev,
> > > > > + struct imx_sc_sensor *sensor) {
> > > > > + struct thermal_zone_device *tzd;
> > > > > +
> > > > > + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> > > > > + sensor->resource_id,
> > > > > + sensor,
> > > > > + &imx_sc_thermal_ops);
> > > > > + if (IS_ERR(tzd)) {
> > > > > + dev_err(&pdev->dev, "failed to register sensor: %d\n",
> > > > > + sensor->resource_id);
> > > > > + return PTR_ERR(tzd);
> > > > > + }
> > > > > +
> > > > > + sensor->tzd = tzd;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int imx_sc_thermal_get_sensor_id(struct device_node
> > > > > +*sensor_np, u32 *id) {
> > > > > + struct of_phandle_args sensor_specs;
> > > > > + int ret;
> > > > > +
> > > > > + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> > > > > + "#thermal-sensor-cells",
> > > > > + 0, &sensor_specs);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + 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 {
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > >
> > > > So, there should be no need for doing a search of sensor ids. In
> > > > fact, if the amount of sensors or the type of sensors supported changes, i.e.
> > > > the sensor ids are do not match, you should have a different
> > > > compatible,
> > > IMO.
> > >
> > > So do you mean I can just read the sensor id (actually it is our SoC's
> > > resource id) from the DTB file's each child node of thermal_zones?
> > > Then I can skip the search of sensor ids, just add a sensor id
> > > property and read it out during thermal driver probe? Yes, this looks like
> > easier, is my understanding correct?
> > > If yes, I can do it in next version. Thanks.
> > >
> > > Anson
> >
> > Can you be more detail about the search of sensor ids? For this SoC, there will
> > be more
> > sensors(thermal-zone) added later, like DDR controller thermal sensor and
> > pmic sensor etc., they will use same compatible, and in the
> > imx_sc_thermal_get_sensor_id(), it just read out the sensor id from the dtb's
> > each thermal zone's " thermal-sensors " property, I did NOT see any search
> > here, thanks!
> >
> > Anson.
>
> I can remove the imx_sc_thermal_get_sensor_id() in the patch, and add below property in dts, then
> just read out the resource id using of_property_read_u32(), then we can save the sensor id search function
> in the driver, but need to add a property in dts, is this what you mean?
>
> .probe:
> - ret = imx_sc_thermal_get_sensor_id(sensor_np, &sensor->resource_id);
> - if (ret < 0) {
> + if (of_property_read_u32(sensor_np, "sensor-resource-id",
> + &sensor->resource_id)) {
>
> dts:
> 454 thermal_zones: thermal-zones {
> 455 cpu-thermal0 {
> 456 polling-delay-passive = <250>;
> 457 polling-delay = <2000>;
> 458 + sensor-resource-id = <355>;

Yeah, I believe this would be best description for your case. In your
current patch you are abusing a bit of the thermal descriptor. That
sensor cell is to actually help identify the sensor within the device
tree description...

> 459 thermal-sensors = <&tsens 355>;
>

when you add 355 that would mean this is sensor 355 of all sensors
handled by your tsens. I would actually even assume that there are at
least 355 sensors in that tsens.

I mean, there is nothing really wrong with that syntactically, but it is
a slightly different usage of that #thermal-sensors-cell.

If you I got your patch right, you need a resource-id to be sent in a
command. For that I would recommend adding a specific property, with the
prefix imx, in it. Something like imx,sensor-resource-id, or something
fits better for you.

> Anson.
>