[PATCH RESEND V11 1/4] dt-bindings: fsl: scu: add thermal binding

[PATCH RESEND V11 1/4] dt-bindings: fsl: scu: add thermal binding

[Anson Huang]

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

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V10:
	- remove property "imx,sensor-resource-id".
---
 .../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 72d481c..023fb5c 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -122,6 +122,17 @@ 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:			Should be :
+				  "fsl,imx8qxp-sc-thermal"
+				followed by "fsl,imx-sc-thermal";
+
+- #thermal-sensor-cells:	See Documentation/devicetree/bindings/thermal/thermal.txt
+				for a description.
+
 Example (imx8qxp):
 -------------
 lsio_mu1: mailbox@5d1c0000 {
@@ -168,6 +179,11 @@ firmware {
 		rtc: rtc {
 			compatible = "fsl,imx8qxp-sc-rtc";
 		};
+
+		tsens: thermal-sensor {
+			compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+			#thermal-sensor-cells = <1>;
+		};
 	};
 };
 
-- 
2.7.4

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

[Anson Huang]

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

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

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V10:
	- remove "imx,sensor-resource-id" property, put the HW resource ID inside thermal-zones node,
	  and driver needs to parse the phandle argument to get HW resource ID and use it to register
	  thermal zone.
---
 drivers/thermal/Kconfig          |  11 +++
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/imx_sc_thermal.c | 159 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+)
 create mode 100644 drivers/thermal/imx_sc_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 653aa27..4e4fa7e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -223,6 +223,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 486d682..4062627 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -40,6 +40,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..0af0f09
--- /dev/null
+++ b/drivers/thermal/imx_sc_thermal.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018-2019 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 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_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, *child;
+	int ret;
+
+	ret = imx_scu_get_handle(&thermal_ipc_handle);
+	if (ret)
+		return ret;
+
+	np = of_find_node_by_name(NULL, "thermal-zones");
+	if (!np)
+		return -ENODEV;
+
+	for_each_available_child_of_node(np, child) {
+		struct imx_sc_sensor *sensor =
+			devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
+		if (!sensor)
+			return -ENOMEM;
+
+		ret = imx_sc_thermal_get_sensor_id(child, &sensor->resource_id);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"failed to get valid sensor resource id: %d\n",
+				ret);
+			break;
+		}
+
+		sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
+								   sensor->resource_id,
+								   sensor,
+								   &imx_sc_thermal_ops);
+		if (IS_ERR(sensor->tzd)) {
+			dev_err(&pdev->dev, "failed to register thermal zone\n");
+			ret = PTR_ERR(sensor->tzd);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static const struct of_device_id imx_sc_thermal_table[] = {
+	{ .compatible = "fsl,imx-sc-thermal", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
+
+static struct platform_driver imx_sc_thermal_driver = {
+		.probe = imx_sc_thermal_probe,
+		.driver = {
+			.name = "imx-sc-thermal",
+			.of_match_table = imx_sc_thermal_table,
+		},
+};
+module_platform_driver(imx_sc_thermal_driver);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

[Daniel Lezcano]

On 10/04/2019 09:43, 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 <Anson.Huang@nxp.com>
> ---
> Changes since V10:
> 	- remove "imx,sensor-resource-id" property, put the HW resource ID inside thermal-zones node,
> 	  and driver needs to parse the phandle argument to get HW resource ID and use it to register
> 	  thermal zone.
> ---
>  drivers/thermal/Kconfig          |  11 +++
>  drivers/thermal/Makefile         |   1 +
>  drivers/thermal/imx_sc_thermal.c | 159 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 171 insertions(+)
>  create mode 100644 drivers/thermal/imx_sc_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 653aa27..4e4fa7e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -223,6 +223,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 486d682..4062627 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -40,6 +40,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..0af0f09
> --- /dev/null
> +++ b/drivers/thermal/imx_sc_thermal.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018-2019 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 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;

Do we really need to add the __packed for the struct imx_sc_sensor and
the struct imx_sc_msg_misc_get_temp?

> +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_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, *child;
> +	int ret;
> +
> +	ret = imx_scu_get_handle(&thermal_ipc_handle);
> +	if (ret)
> +		return ret;
> +
> +	np = of_find_node_by_name(NULL, "thermal-zones");
> +	if (!np)
> +		return -ENODEV;
> +
> +	for_each_available_child_of_node(np, child) {
> +		struct imx_sc_sensor *sensor =
> +			devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
> +		if (!sensor)
> +			return -ENOMEM;
> +
> +		ret = imx_sc_thermal_get_sensor_id(child, &sensor->resource_id);


IMO, this function should be generic and added to the of-thermal.c

> +		if (ret < 0) {
> +			dev_err(&pdev->dev,
> +				"failed to get valid sensor resource id: %d\n",
> +				ret);
> +			break;
> +		}
> +
> +		sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
> +								   sensor->resource_id,
> +								   sensor,
> +								   &imx_sc_thermal_ops);
> +		if (IS_ERR(sensor->tzd)) {
> +			dev_err(&pdev->dev, "failed to register thermal zone\n");
> +			ret = PTR_ERR(sensor->tzd);
> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
>
> +static const struct of_device_id imx_sc_thermal_table[] = {
> +	{ .compatible = "fsl,imx-sc-thermal", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
> +
> +static struct platform_driver imx_sc_thermal_driver = {
> +		.probe = imx_sc_thermal_probe,
> +		.driver = {
> +			.name = "imx-sc-thermal",
> +			.of_match_table = imx_sc_thermal_table,
> +		},
> +};
> +module_platform_driver(imx_sc_thermal_driver);
> +
> +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
> +MODULE_LICENSE("GPL v2");
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [EXT] Re: [PATCH RESEND V11 2/4] thermal: imx_sc: add i.MX system controller thermal support

[Anson Huang]

Hi, Daniel

Best Regards!
Anson Huang

> -----Original Message-----
> 
> On 10/04/2019 09:43, 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 <Anson.Huang@nxp.com>
> > ---
> > Changes since V10:
> >       - remove "imx,sensor-resource-id" property, put the HW resource ID
> inside thermal-zones node,
> >         and driver needs to parse the phandle argument to get HW resource ID
> and use it to register
> >         thermal zone.
> > ---
> >  drivers/thermal/Kconfig          |  11 +++
> >  drivers/thermal/Makefile         |   1 +
> >  drivers/thermal/imx_sc_thermal.c | 159
> > +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 171 insertions(+)
> >  create mode 100644 drivers/thermal/imx_sc_thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > 653aa27..4e4fa7e 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -223,6 +223,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
> > 486d682..4062627 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -40,6 +40,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..0af0f09
> > --- /dev/null
> > +++ b/drivers/thermal/imx_sc_thermal.c
> > @@ -0,0 +1,159 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2018-2019 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 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;
> 
> Do we really need to add the __packed for the struct imx_sc_sensor and the
> struct imx_sc_msg_misc_get_temp?

I think struct imx_sc_msg_misc_get_temp is no need to add __packed, remove it in V12.

> 
> > +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_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, *child;
> > +     int ret;
> > +
> > +     ret = imx_scu_get_handle(&thermal_ipc_handle);
> > +     if (ret)
> > +             return ret;
> > +
> > +     np = of_find_node_by_name(NULL, "thermal-zones");
> > +     if (!np)
> > +             return -ENODEV;
> > +
> > +     for_each_available_child_of_node(np, child) {
> > +             struct imx_sc_sensor *sensor =
> > +                     devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
> > +             if (!sensor)
> > +                     return -ENOMEM;
> > +
> > +             ret = imx_sc_thermal_get_sensor_id(child,
> > + &sensor->resource_id);
> 
> 
> IMO, this function should be generic and added to the of-thermal.c

Make sense, I have moved it to a new patch of adding new API for getting sensor ID in V12 patch series.

Thanks,
Anson

> 
> > +             if (ret < 0) {
> > +                     dev_err(&pdev->dev,
> > +                             "failed to get valid sensor resource id: %d\n",
> > +                             ret);
> > +                     break;
> > +             }
> > +
> > +             sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev-
> >dev,
> > +                                                                sensor->resource_id,
> > +                                                                sensor,
> > +                                                                &imx_sc_thermal_ops);
> > +             if (IS_ERR(sensor->tzd)) {
> > +                     dev_err(&pdev->dev, "failed to register thermal zone\n");
> > +                     ret = PTR_ERR(sensor->tzd);
> > +                     break;
> > +             }
> > +     }
> > +
> > +     return ret;
> > +}
> >
> > +static const struct of_device_id imx_sc_thermal_table[] = {
> > +     { .compatible = "fsl,imx-sc-thermal", },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
> > +
> > +static struct platform_driver imx_sc_thermal_driver = {
> > +             .probe = imx_sc_thermal_probe,
> > +             .driver = {
> > +                     .name = "imx-sc-thermal",
> > +                     .of_match_table = imx_sc_thermal_table,
> > +             },
> > +};
> > +module_platform_driver(imx_sc_thermal_driver);
> > +
> > +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> > +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system
> > +controller"); MODULE_LICENSE("GPL v2");
> >
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7Cc8cd5f
> 6ddab14679fa8308d6c1ddf788%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C636909556419507623&amp;sdata=ENeuLStvLI2b3qmoMaVd80PO
> Z3D5SDP4SLsMEgyCSmo%3D&amp;reserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> facebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Canson.huang%4
> 0nxp.com%7Cc8cd5f6ddab14679fa8308d6c1ddf788%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C636909556419517628&amp;sdata=VfEhJa%2
> F5VBvhNb0A43FSmbUUsfHuP7Sb3iC0MAdT1zQ%3D&amp;reserved=0>
> Facebook |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&amp;data=02%7C01%7Canson.huang%40nxp.c
> om%7Cc8cd5f6ddab14679fa8308d6c1ddf788%7C686ea1d3bc2b4c6fa92cd99c
> 5c301635%7C0%7C0%7C636909556419517628&amp;sdata=YYFrIOw1ad29foi
> EtLdpY83zehoSRJniUCfJGKNJer8%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2Flinaro-
> blog%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7Cc8cd5f6ddab
> 14679fa8308d6c1ddf788%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636909556419517628&amp;sdata=hUHkrV00%2FDOuCuYND7%2BtFFwK
> UBM7hvrgZXFSPm23g5M%3D&amp;reserved=0> Blog

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

[Anson Huang]

This patch enables CONFIG_IMX_SC_THERMAL as module.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 9e313f5..d4c404a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -415,6 +415,7 @@ CONFIG_SENSORS_INA2XX=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_THERMAL=y
 CONFIG_RCAR_GEN3_THERMAL=y
-- 
2.7.4

[PATCH RESEND V11 4/4] arm64: dts: imx: add i.MX8QXP thermal support

[Anson Huang]

Add i.MX8QXP CPU thermal zone support.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V10:
	- remove property "imx,sensor-resource-id" and put HW resource ID inside each
	  thermal-sensors' phandle argument.
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 70ef3db..cc1d877 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/pinctrl/pads-imx8qxp.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	interrupt-parent = <&gic>;
@@ -149,6 +150,11 @@
 		rtc: rtc {
 			compatible = "fsl,imx8qxp-sc-rtc";
 		};
+
+		tsens: thermal-sensor {
+			compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+			#thermal-sensor-cells = <1>;
+		};
 	};
 
 	timer {
@@ -484,4 +490,31 @@
 			power-domains = <&pd IMX_SC_R_GPIO_7>;
 		};
 	};
+
+	thermal_zones: thermal-zones {
+		cpu-thermal0 {
+			polling-delay-passive = <250>;
+			polling-delay = <2000>;
+			thermal-sensors = <&tsens IMX_SC_R_SYSTEM>;
+			trips {
+				cpu_alert0: trip0 {
+					temperature = <107000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+				cpu_crit0: trip1 {
+					temperature = <127000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device =
+						<&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+	};
 };
-- 
2.7.4

Re: [PATCH RESEND V11 4/4] arm64: dts: imx: add i.MX8QXP thermal support

[Rob Herring]

:u?w]\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0

Re: [PATCH RESEND V11 4/4] arm64: dts: imx: add i.MX8QXP thermal support

[Daniel Lezcano]

On 10/04/2019 09:43, Anson Huang wrote:
> Add i.MX8QXP CPU thermal zone support.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V10:
> 	- remove property "imx,sensor-resource-id" and put HW resource ID inside each
> 	  thermal-sensors' phandle argument.
> ---
>  arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 33 ++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 70ef3db..cc1d877 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/pinctrl/pads-imx8qxp.h>
> +#include <dt-bindings/thermal/thermal.h>
>  
>  / {
>  	interrupt-parent = <&gic>;
> @@ -149,6 +150,11 @@
>  		rtc: rtc {
>  			compatible = "fsl,imx8qxp-sc-rtc";
>  		};
> +
> +		tsens: thermal-sensor {
> +			compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
> +			#thermal-sensor-cells = <1>;
> +		};
>  	};
>  
>  	timer {
> @@ -484,4 +490,31 @@
>  			power-domains = <&pd IMX_SC_R_GPIO_7>;
>  		};
>  	};
> +
> +	thermal_zones: thermal-zones {
> +		cpu-thermal0 {
> +			polling-delay-passive = <250>;
> +			polling-delay = <2000>;
> +			thermal-sensors = <&tsens IMX_SC_R_SYSTEM>;
> +			trips {
> +				cpu_alert0: trip0 {
> +					temperature = <107000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +				cpu_crit0: trip1 {
> +					temperature = <127000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu_alert0>;
> +					cooling-device =
> +						<&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

	<&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
	<&A35_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
	<&A35_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
	<&A35_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;


> +				};
> +			};
> +		};
> +	};
>  };
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [EXT] Re: [PATCH RESEND V11 4/4] arm64: dts: imx: add i.MX8QXP thermal support

[Anson Huang]

Hi, Daniel

Best Regards!
Anson Huang

> -----Original Message-----
> 
> 
> On 10/04/2019 09:43, Anson Huang wrote:
> > Add i.MX8QXP CPU thermal zone support.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V10:
> >       - remove property "imx,sensor-resource-id" and put HW resource ID
> inside each
> >         thermal-sensors' phandle argument.
> > ---
> >  arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 33
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > index 70ef3db..cc1d877 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> > @@ -10,6 +10,7 @@
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/pinctrl/pads-imx8qxp.h>
> > +#include <dt-bindings/thermal/thermal.h>
> >
> >  / {
> >       interrupt-parent = <&gic>;
> > @@ -149,6 +150,11 @@
> >               rtc: rtc {
> >                       compatible = "fsl,imx8qxp-sc-rtc";
> >               };
> > +
> > +             tsens: thermal-sensor {
> > +                     compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
> > +                     #thermal-sensor-cells = <1>;
> > +             };
> >       };
> >
> >       timer {
> > @@ -484,4 +490,31 @@
> >                       power-domains = <&pd IMX_SC_R_GPIO_7>;
> >               };
> >       };
> > +
> > +     thermal_zones: thermal-zones {
> > +             cpu-thermal0 {
> > +                     polling-delay-passive = <250>;
> > +                     polling-delay = <2000>;
> > +                     thermal-sensors = <&tsens IMX_SC_R_SYSTEM>;
> > +                     trips {
> > +                             cpu_alert0: trip0 {
> > +                                     temperature = <107000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "passive";
> > +                             };
> > +                             cpu_crit0: trip1 {
> > +                                     temperature = <127000>;
> > +                                     hysteresis = <2000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +                     cooling-maps {
> > +                             map0 {
> > +                                     trip = <&cpu_alert0>;
> > +                                     cooling-device =
> > +                                             <&A35_0 THERMAL_NO_LIMIT
> > + THERMAL_NO_LIMIT>;
> 
>         <&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>         <&A35_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>         <&A35_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>         <&A35_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> 

Fix it in V12, thanks.

Anson.

> 
> > +                             };
> > +                     };
> > +             };
> > +     };
> >  };
> >
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7Cfca87d
> aa526949586d3608d6c1def467%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C636909560658582260&amp;sdata=zJ9pZFVN94TN3tci4sut9A%2Bz
> bnDklZjxy9dwEXCssHk%3D&amp;reserved=0> Linaro.org │ Open source
> software for ARM SoCs
> 
> Follow Linaro:
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> facebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7Canson.huang%4
> 0nxp.com%7Cfca87daa526949586d3608d6c1def467%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C636909560658582260&amp;sdata=8U9ZV3uL
> dMfzRitq1cl4tvxpFakxFNGW0TDZ42wDuEc%3D&amp;reserved=0> Facebook
> |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&amp;data=02%7C01%7Canson.huang%40nxp.c
> om%7Cfca87daa526949586d3608d6c1def467%7C686ea1d3bc2b4c6fa92cd99
> c5c301635%7C0%7C0%7C636909560658582260&amp;sdata=lJ5yngMoW3ELI
> xYQaASAtvGXdNVDxH5BWoKSLwRL0mM%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2Flinaro-
> blog%2F&amp;data=02%7C01%7Canson.huang%40nxp.com%7Cfca87daa526
> 949586d3608d6c1def467%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C636909560658582260&amp;sdata=hBTpeBSg7JGk0qHbC7DEp2JgN7sVq
> yhC2uzBAG%2BP01s%3D&amp;reserved=0> Blog

Re: [PATCH RESEND V11 1/4] dt-bindings: fsl: scu: add thermal binding

[Rob Herring]

On Wed, Apr 10, 2019 at 07:43:04AM +0000, Anson Huang wrote:
> NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as
> system controller, the system controller is in charge of system
> power, clock and thermal sensors etc. management, Linux kernel
> has to communicate with system controller via MU (message unit)
> IPC to get temperature from thermal sensors, this patch adds
> binding doc for i.MX system controller thermal driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V10:
> 	- remove property "imx,sensor-resource-id".
> ---
>  .../devicetree/bindings/arm/freescale/fsl,scu.txt        | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>