Hi, Eduardo Best Regards! Anson Huang > -----Original Message----- > From: Eduardo Valentin [mailto:edubezval@xxxxxxxxx] > Sent: 2019年2月24日 5:39 > To: Anson Huang <anson.huang@xxxxxxx> > Cc: robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; shawnguo@xxxxxxxxxx; > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; > catalin.marinas@xxxxxxx; will.deacon@xxxxxxx; rui.zhang@xxxxxxxxx; > daniel.lezcano@xxxxxxxxxx; Aisheng Dong <aisheng.dong@xxxxxxx>; > ulf.hansson@xxxxxxxxxx; sboyd@xxxxxxxxxx; Daniel Baluta > <daniel.baluta@xxxxxxx>; Andy Gross <andy.gross@xxxxxxxxxx>; > horms+renesas@xxxxxxxxxxxx; heiko@xxxxxxxxx; arnd@xxxxxxxx; > maxime.ripard@xxxxxxxxxxx; bjorn.andersson@xxxxxxxxxx; > jagan@xxxxxxxxxxxxxxxxxxxx; enric.balletbo@xxxxxxxxxxxxx; > marc.w.gonzalez@xxxxxxx; olof@xxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > pm@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > 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 <Anson.Huang@xxxxxxx> > > --- > > 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 <Anson.Huang@xxxxxxx>"); > > +MODULE_DESCRIPTION("Thermal driver for NXP i.MX SoCs with system > > +controller"); MODULE_LICENSE("GPL v2"); > > -- > > 2.7.4 > >