Hi, Eduardo

Best Regards!
Anson Huang

-----Original Message-----
From: Eduardo Valentin [mailto:edubezval@gmail.com]
Sent: 2018年12月26日 6:44
To: Anson Huang <redacted>
Cc: robh+dt@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com;
will.deacon@arm.com; rui.zhang@intel.com; daniel.lezcano@linaro.org;
Aisheng Dong [off-list ref]; s.hauer@pengutronix.de;
shawnguo@kernel.org; Andy Gross [off-list ref]; arnd@arndb.de;
horms+renesas@verge.net.au; heiko@sntech.de; bjorn.andersson@linaro.org;
enric.balletbo@collabora.com; amit.kucheria@linaro.org;
ezequiel@collabora.com; olof@lixom.net; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux-pm@vger.kernel.org; dl-linux-imx [off-list ref]
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:

quoted

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 <redacted>
---
ChangeLog since V4:
    - Add driver dependency on IMX_SCU to avoid compile error when

IMX_SCU is NOT selected;

quoted

    - Remove unnecessary of different errors handle for

imx_scu_get_handle, already included in API.

quoted

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

quoted

+	depends on (ARCH_MXC && IMX_SCU) || COMPILE_TEST
+	depends on OF
+	help
+	  Support for Temperature Monitor (TEMPMON) found on NXP i.MX SoCs

with

quoted

+	  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

quoted

 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

quoted

+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);

quoted

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

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