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]>
---
Changes since V7:
- remove unused property "tsens-num";
- improve the compatible description;
- update examples according to latest dts file.
---
.../devicetree/bindings/arm/freescale/fsl,scu.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
index 72d481c..d89147e 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -122,6 +122,19 @@ 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.
+
+- imx,sensor-resource-id: Property array to specify each thermal zone's sensor resource ID.
+
Example (imx8qxp):
-------------
lsio_mu1: mailbox@5d1c0000 {
@@ -168,6 +181,12 @@ firmware {
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
+
+ tsens: thermal-sensor {
+ compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+ #thermal-sensor-cells = <0>;
+ imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
+ };
};
};
--
2.7.4
This patch enables CONFIG_IMX_SC_THERMAL as module.
Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2d9c390..52d503e 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -413,6 +413,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
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]>
---
Changes since V7:
- remove unused structure imx_sc_thermal_data to simply the driver;
- move the "imx,sensor-resource-id" property from thermal zone node to scu tsens node, and
get this property using phandle;
- remove unused sensor number got from dts, now it is not needed.
---
drivers/thermal/Kconfig | 11 +++
drivers/thermal/Makefile | 1 +
drivers/thermal/imx_sc_thermal.c | 144 +++++++++++++++++++++++++++++++++++++++
3 files changed, 156 insertions(+)
create mode 100644 drivers/thermal/imx_sc_thermal.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 58bb7d7..fec0ef5 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..145e73b
--- /dev/null
+++ b/drivers/thermal/imx_sc_thermal.c
@@ -0,0 +1,144 @@
+// 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_probe(struct platform_device *pdev)
+{
+ struct device_node *np, *sensor_np;
+ int ret, i = 0;
+
+ 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, sensor_np) {
+ struct of_phandle_args tsens_args;
+ struct imx_sc_sensor *sensor =
+ devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
+ if (!sensor)
+ return -ENOMEM;
+
+ ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
+ "#thermal-sensor-cells", i, &tsens_args);
+ if (ret < 0)
+ return ret;
+
+ ret = of_property_read_u32_index(tsens_args.np,
+ "imx,sensor-resource-id", i, &sensor->resource_id);
+ if (ret) {
+ 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,
+ i++,
+ sensor,
+ &imx_sc_thermal_ops);
+ if (IS_ERR(sensor->tzd)) {
+ dev_err(&pdev->dev, "failed to register thermal zone\n");
+ ret = PTR_ERR(sensor->tzd);
+ break;
+ }
+ }
+
+ of_node_put(np);
+ of_node_put(sensor_np);
+
+ return ret;
+}
+
+static const struct of_device_id imx_sc_thermal_table[] = {
+ { .compatible = "fsl,imx-sc-thermal", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
+
+static struct platform_driver imx_sc_thermal_driver = {
+ .probe = imx_sc_thermal_probe,
+ .driver = {
+ .name = "imx-sc-thermal",
+ .of_match_table = imx_sc_thermal_table,
+ },
+};
+module_platform_driver(imx_sc_thermal_driver);
+
+MODULE_AUTHOR("Anson Huang <[email protected]>");
+MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
+MODULE_LICENSE("GPL v2");
--
2.7.4
Add i.MX8QXP CPU thermal zone support.
Signed-off-by: Anson Huang <[email protected]>
---
Changes since V7:
- move the "imx,sensor-resource-id" to scu tsens node;
- correct #thermal-sensor-cells value to be 0 as there is ONLY one thermal zone now;
- add cooling map for passive mode.
---
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 35 ++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 4c3dd95..eccdf28 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>;
@@ -34,6 +35,7 @@
reg = <0x0 0x0>;
enable-method = "psci";
next-level-cache = <&A35_L2>;
+ #cooling-cells = <2>;
};
A35_1: cpu@1 {
@@ -116,6 +118,12 @@
rtc: rtc {
compatible = "fsl,imx8qxp-sc-rtc";
};
+
+ tsens: thermal-sensor {
+ compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
+ #thermal-sensor-cells = <0>;
+ imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
+ };
};
timer {
@@ -443,4 +451,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 0>;
+ 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
On Thu, Feb 21, 2019 at 06:38:30AM +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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> Changes since V7:
> - remove unused property "tsens-num";
> - improve the compatible description;
> - update examples according to latest dts file.
> ---
> .../devicetree/bindings/arm/freescale/fsl,scu.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> index 72d481c..d89147e 100644
> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -122,6 +122,19 @@ 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.
> +
> +- imx,sensor-resource-id: Property array to specify each thermal zone's sensor resource ID.
If this is an array...
> +
> Example (imx8qxp):
> -------------
> lsio_mu1: mailbox@5d1c0000 {
> @@ -168,6 +181,12 @@ firmware {
> rtc: rtc {
> compatible = "fsl,imx8qxp-sc-rtc";
> };
> +
> + tsens: thermal-sensor {
> + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
> + #thermal-sensor-cells = <0>;
> + imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
then this example should have an array, to be better express how this is
supposed to work. Can you please resend this with an array instead?
> + };
> };
> };
>
> --
> 2.7.4
>
Hey Anson,
On Thu, Feb 21, 2019 at 06:38:39AM +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]>
> ---
> Changes since V7:
> - remove unused structure imx_sc_thermal_data to simply the driver;
> - move the "imx,sensor-resource-id" property from thermal zone node to scu tsens node, and
> get this property using phandle;
> - remove unused sensor number got from dts, now it is not needed.
> ---
> drivers/thermal/Kconfig | 11 +++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/imx_sc_thermal.c | 144 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 156 insertions(+)
> create mode 100644 drivers/thermal/imx_sc_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 58bb7d7..fec0ef5 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..145e73b
> --- /dev/null
> +++ b/drivers/thermal/imx_sc_thermal.c
> @@ -0,0 +1,144 @@
> +// 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_probe(struct platform_device *pdev)
> +{
> + struct device_node *np, *sensor_np;
> + int ret, i = 0;
> +
> + 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;
I am still failing to understand why you need to do the search above
and ..
> +
> + for_each_available_child_of_node(np, sensor_np) {
> + struct of_phandle_args tsens_args;
> + struct imx_sc_sensor *sensor =
> + devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL);
> + if (!sensor)
> + return -ENOMEM;
> +
> + ret = of_parse_phandle_with_args(sensor_np, "thermal-sensors",
> + "#thermal-sensor-cells", i, &tsens_args);
the above shouldnt be needed too.
> + if (ret < 0)
> + return ret;
> +
> + ret = of_property_read_u32_index(tsens_args.np,
> + "imx,sensor-resource-id", i, &sensor->resource_id);
If you put "imx,sensor-resource-id" inside tsens node, like described in
your patch 01, you should be able to get np from pdev->dev.of_node,
right?
Then you should be able to just read the "imx,sensor-resource-id"
directly from pdev->dev.of_node, no?
> + if (ret) {
> + 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,
> + i++,
> + sensor,
> + &imx_sc_thermal_ops);
> + if (IS_ERR(sensor->tzd)) {
> + dev_err(&pdev->dev, "failed to register thermal zone\n");
> + ret = PTR_ERR(sensor->tzd);
> + break;
> + }
> + }
> +
> + of_node_put(np);
> + of_node_put(sensor_np);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id imx_sc_thermal_table[] = {
> + { .compatible = "fsl,imx-sc-thermal", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
> +
> +static struct platform_driver imx_sc_thermal_driver = {
> + .probe = imx_sc_thermal_probe,
> + .driver = {
> + .name = "imx-sc-thermal",
> + .of_match_table = imx_sc_thermal_table,
> + },
> +};
> +module_platform_driver(imx_sc_thermal_driver);
> +
> +MODULE_AUTHOR("Anson Huang <[email protected]>");
> +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system controller");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
On Sat, Feb 23, 2019 at 01:38:00PM -0800, Eduardo Valentin wrote:
> On Thu, Feb 21, 2019 at 06:38:30AM +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 <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> > Changes since V7:
> > - remove unused property "tsens-num";
> > - improve the compatible description;
> > - update examples according to latest dts file.
> > ---
> > .../devicetree/bindings/arm/freescale/fsl,scu.txt | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > index 72d481c..d89147e 100644
> > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > @@ -122,6 +122,19 @@ 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.
> > +
> > +- imx,sensor-resource-id: Property array to specify each thermal zone's sensor resource ID.
>
> If this is an array...
>
> > +
> > Example (imx8qxp):
> > -------------
> > lsio_mu1: mailbox@5d1c0000 {
> > @@ -168,6 +181,12 @@ firmware {
> > rtc: rtc {
> > compatible = "fsl,imx8qxp-sc-rtc";
> > };
> > +
> > + tsens: thermal-sensor {
> > + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-thermal";
> > + #thermal-sensor-cells = <0>;
> > + imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
>
> then this example should have an array, to be better express how this is
> supposed to work. Can you please resend this with an array instead?
I mean this could be one example of an array with more than one element.
>
> > + };
> > };
> > };
> >
> > --
> > 2.7.4
> >
Hi, Eduardo
Best Regards!
Anson Huang
> -----Original Message-----
> From: Eduardo Valentin [mailto:[email protected]]
> Sent: 2019??2??24?? 5:38
> To: Anson Huang <[email protected]>
> Cc: [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]; Daniel Baluta
> <[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]; linux-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH V8 1/4] dt-bindings: fsl: scu: add thermal binding
>
> On Thu, Feb 21, 2019 at 06:38:30AM +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 <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> > Changes since V7:
> > - remove unused property "tsens-num";
> > - improve the compatible description;
> > - update examples according to latest dts file.
> > ---
> > .../devicetree/bindings/arm/freescale/fsl,scu.txt | 19
> +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > index 72d481c..d89147e 100644
> > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > @@ -122,6 +122,19 @@ 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.
> > +
> > +- imx,sensor-resource-id: Property array to specify each thermal zone's
> sensor resource ID.
>
> If this is an array...
>
> > +
> > Example (imx8qxp):
> > -------------
> > lsio_mu1: mailbox@5d1c0000 {
> > @@ -168,6 +181,12 @@ firmware {
> > rtc: rtc {
> > compatible = "fsl,imx8qxp-sc-rtc";
> > };
> > +
> > + tsens: thermal-sensor {
> > + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-
> thermal";
> > + #thermal-sensor-cells = <0>;
> > + imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
>
> then this example should have an array, to be better express how this is
> supposed to work. Can you please resend this with an array instead?
On i.MX8QXP, there is ONLY 1 thermal zone available currently, but the thermal driver
is supposed to support multi thermal zones as well for i.MX8QM, in V9 patch set I just
sent, I use the " imx,sensor-resource-id " element count to determine how many thermal
sensors will be supported, so that we can also get rid of searching the thermal-zones' child
node, I improved the description as below, is it OK?
41 +- imx,sensor-resource-id: A single integer for single thermal zone's resource ID or
42 + an array of integers to specify each thermal zone's sensor
43 + resource ID.
Thanks,
Anson.
>
> > + };
> > };
> > };
> >
> > --
> > 2.7.4
> >
Hi, Eduardo
Best Regards!
Anson Huang
> -----Original Message-----
> From: Eduardo Valentin [mailto:[email protected]]
> Sent: 2019??2??24?? 5:39
> To: Anson Huang <[email protected]>
> Cc: [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]; Daniel Baluta
> <[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]; linux-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH V8 2/4] thermal: imx_sc: add i.MX system controller
> thermal support
>
> Hey Anson,
>
> On Thu, Feb 21, 2019 at 06:38:39AM +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]>
> > ---
> > Changes since V7:
> > - remove unused structure imx_sc_thermal_data to simply the driver;
> > - move the "imx,sensor-resource-id" property from thermal zone
> node to scu tsens node, and
> > get this property using phandle;
> > - remove unused sensor number got from dts, now it is not needed.
> > ---
> > drivers/thermal/Kconfig | 11 +++
> > drivers/thermal/Makefile | 1 +
> > drivers/thermal/imx_sc_thermal.c | 144
> > +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 156 insertions(+)
> > create mode 100644 drivers/thermal/imx_sc_thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > 58bb7d7..fec0ef5 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..145e73b
> > --- /dev/null
> > +++ b/drivers/thermal/imx_sc_thermal.c
> > @@ -0,0 +1,144 @@
> > +// 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_probe(struct platform_device *pdev) {
> > + struct device_node *np, *sensor_np;
> > + int ret, i = 0;
> > +
> > + 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;
>
> I am still failing to understand why you need to do the search above and ..
I used this to determine how many thermal zones will be supported from dts, and
get each thermal zone's resource ID for communicating with system controller, but
now I just found I can use the "imx,sensor-resource-id" property's element count
to determine the thermal zone number, so in V9 patch series, I remove this search.
>
> > +
> > + for_each_available_child_of_node(np, sensor_np) {
> > + struct of_phandle_args tsens_args;
> > + struct imx_sc_sensor *sensor =
> > + devm_kzalloc(&pdev->dev, sizeof(*sensor),
> GFP_KERNEL);
> > + if (!sensor)
> > + return -ENOMEM;
> > +
> > + ret = of_parse_phandle_with_args(sensor_np, "thermal-
> sensors",
> > + "#thermal-sensor-cells", i, &tsens_args);
>
> the above shouldnt be needed too.
Already removed in V9.
>
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = of_property_read_u32_index(tsens_args.np,
> > + "imx,sensor-resource-id", i, &sensor->resource_id);
>
> If you put "imx,sensor-resource-id" inside tsens node, like described in your
> patch 01, you should be able to get np from pdev->dev.of_node, right?
>
> Then you should be able to just read the "imx,sensor-resource-id"
> directly from pdev->dev.of_node, no?
Ah, yes, I just don't remembered why I need to get the device node from phandle,
so stupid in last patch series... Thanks for the comment, please help review V9 patch series.
Anson.
>
> > + if (ret) {
> > + 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,
> > + i++,
> > + sensor,
> > +
> &imx_sc_thermal_ops);
> > + if (IS_ERR(sensor->tzd)) {
> > + dev_err(&pdev->dev, "failed to register thermal
> zone\n");
> > + ret = PTR_ERR(sensor->tzd);
> > + break;
> > + }
> > + }
> > +
> > + of_node_put(np);
> > + of_node_put(sensor_np);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct of_device_id imx_sc_thermal_table[] = {
> > + { .compatible = "fsl,imx-sc-thermal", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_sc_thermal_table);
> > +
> > +static struct platform_driver imx_sc_thermal_driver = {
> > + .probe = imx_sc_thermal_probe,
> > + .driver = {
> > + .name = "imx-sc-thermal",
> > + .of_match_table = imx_sc_thermal_table,
> > + },
> > +};
> > +module_platform_driver(imx_sc_thermal_driver);
> > +
> > +MODULE_AUTHOR("Anson Huang <[email protected]>");
> > +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system
> > +controller"); MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> >
[...]
> > On Thu, Feb 21, 2019 at 06:38:30AM +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 <[email protected]>
> > > Reviewed-by: Rob Herring <[email protected]>
You need drop the former Reviewed-by once there's significant changes.
> > > +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.
> > > +
> > > +- imx,sensor-resource-id: Property array to specify each thermal
> zone's
> > sensor resource ID.
> >
> > If this is an array...
> >
> > > +
> > > Example (imx8qxp):
> > > -------------
> > > lsio_mu1: mailbox@5d1c0000 {
> > > @@ -168,6 +181,12 @@ firmware {
> > > rtc: rtc {
> > > compatible = "fsl,imx8qxp-sc-rtc";
> > > };
> > > +
> > > + tsens: thermal-sensor {
> > > + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-
> > thermal";
> > > + #thermal-sensor-cells = <0>;
> > > + imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
> >
> > then this example should have an array, to be better express how this
> > is supposed to work. Can you please resend this with an array instead?
>
> On i.MX8QXP, there is ONLY 1 thermal zone available currently, but the thermal
> driver is supposed to support multi thermal zones as well for i.MX8QM, in V9
> patch set I just sent, I use the " imx,sensor-resource-id " element count to
> determine how many thermal sensors will be supported, so that we can also
> get rid of searching the thermal-zones' child node, I improved the description
> as below, is it OK?
>
Device tree is used to describe HW.
#thermal-sensor-cells is less to be an optional parameter to me.
If HW supports, then it should be 1 and update the example accordingly.
Regards
Dong Aisheng
> 41 +- imx,sensor-resource-id: A single integer for single thermal zone's
> resource ID or
> 42 + an array of integers to specify each
> thermal zone's sensor
> 43 + resource ID.
Best Regards!
Anson Huang
> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019??2??26?? 20:01
> To: Anson Huang <[email protected]>; 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];
> Daniel Baluta <[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]; linux-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: RE: [PATCH V8 1/4] dt-bindings: fsl: scu: add thermal binding
>
> [...]
> > > On Thu, Feb 21, 2019 at 06:38:30AM +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 <[email protected]>
> > > > Reviewed-by: Rob Herring <[email protected]>
>
> You need drop the former Reviewed-by once there's significant changes.
>
> > > > +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.
> > > > +
> > > > +- imx,sensor-resource-id: Property array to specify each
> thermal
> > zone's
> > > sensor resource ID.
> > >
> > > If this is an array...
> > >
> > > > +
> > > > Example (imx8qxp):
> > > > -------------
> > > > lsio_mu1: mailbox@5d1c0000 {
> > > > @@ -168,6 +181,12 @@ firmware {
> > > > rtc: rtc {
> > > > compatible = "fsl,imx8qxp-sc-rtc";
> > > > };
> > > > +
> > > > + tsens: thermal-sensor {
> > > > + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-
> > > thermal";
> > > > + #thermal-sensor-cells = <0>;
> > > > + imx,sensor-resource-id = <IMX_SC_R_SYSTEM>;
> > >
> > > then this example should have an array, to be better express how
> > > this is supposed to work. Can you please resend this with an array
> instead?
> >
> > On i.MX8QXP, there is ONLY 1 thermal zone available currently, but the
> > thermal driver is supposed to support multi thermal zones as well for
> > i.MX8QM, in V9 patch set I just sent, I use the "
> > imx,sensor-resource-id " element count to determine how many thermal
> > sensors will be supported, so that we can also get rid of searching
> > the thermal-zones' child node, I improved the description as below, is it OK?
> >
>
> Device tree is used to describe HW.
> #thermal-sensor-cells is less to be an optional parameter to me.
> If HW supports, then it should be 1 and update the example accordingly.
OK, I changed it to "1" in V10 patch series.
Thanks,
Anson.
>
> Regards
> Dong Aisheng
>
> > 41 +- imx,sensor-resource-id: A single integer for single thermal zone's
> > resource ID or
> > 42 + an array of integers to specify each
> > thermal zone's sensor
> > 43 + resource ID.