diff mbox

[RFC] ARM: imx: Add imx6q thermal

Message ID 1326266285-11656-1-git-send-email-rob.lee@linaro.org
State New
Headers show

Commit Message

Rob Jan. 11, 2012, 7:18 a.m. UTC
Add thermal support for i.MX6Q.  Uses recently submitted common
cpu_cooling functionality shown here:

http://www.spinics.net/lists/linux-pm/msg26500.html

Have some todo items but basic implementation is done and I'd like to
get any helpful feedback on it.

Todo:
- Add sensor calibration.
- Re-organize code/files if deemed necessary by community.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 drivers/thermal/Kconfig         |    6 +
 drivers/thermal/Makefile        |    1 +
 drivers/thermal/imx6q_thermal.c |  370 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 377 insertions(+), 0 deletions(-)
 create mode 100644 drivers/thermal/imx6q_thermal.c

Comments

Dirk Behme Jan. 12, 2012, 7:15 a.m. UTC | #1
Hi Rob,

On 11.01.2012 08:18, Robert Lee wrote:
> Add thermal support for i.MX6Q.  Uses recently submitted common
> cpu_cooling functionality shown here:
> 
> http://www.spinics.net/lists/linux-pm/msg26500.html
> 
> Have some todo items but basic implementation is done and I'd like to
> get any helpful feedback on it.
> 
> Todo:
> - Add sensor calibration.
> - Re-organize code/files if deemed necessary by community.

Just fyi, I had a short discussion with Eric Miao about this. The 
conclusion was:

The patch itself is OK. It's just lacking some features, but these can 
be added later. The driver supports thermal reading only, but not 
accurate as the thermal calibration data is burned into the FUSE, and 
differ from chip to chip. At the moment only incorrect thermal 
information will be exposed to user space. The patch is lacking 
connections to cpufreq and cpu hotplug, but those could be added later, too.

Best regards

Dirk
Sascha Hauer Jan. 12, 2012, 9:36 a.m. UTC | #2
On Wed, Jan 11, 2012 at 01:18:05AM -0600, Robert Lee wrote:
> Add thermal support for i.MX6Q.  Uses recently submitted common
> cpu_cooling functionality shown here:
> 
> http://www.spinics.net/lists/linux-pm/msg26500.html
> 
> Have some todo items but basic implementation is done and I'd like to
> get any helpful feedback on it.

Generally shouldn't this be a regular driver?


> +
> +/* register define of anatop */
> +#define HW_ANADIG_ANA_MISC0	(0x00000150)
> +#define HW_ANADIG_ANA_MISC0_SET	(0x00000154)
> +#define HW_ANADIG_ANA_MISC0_CLR	(0x00000158)
> +#define HW_ANADIG_ANA_MISC0_TOG	(0x0000015c)

unnecessary braces. Please remove.

> +
> +#if IMX6Q_THERMAL_DEBUG
> +	pr_info("Temperature is %lu C\n", *temp);
> +#endif

Please don't add your own debug defines which then issue
messages with pr_info. Use pr_debug in the first place.

Sascha
Rob Jan. 12, 2012, 8:47 p.m. UTC | #3
Hello Dirk, thanks for the review.  Comments below.

On Thu, Jan 12, 2012 at 1:15 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> Hi Rob,
>
>
> On 11.01.2012 08:18, Robert Lee wrote:
>>
>> Add thermal support for i.MX6Q.  Uses recently submitted common
>> cpu_cooling functionality shown here:
>>
>> http://www.spinics.net/lists/linux-pm/msg26500.html
>>
>> Have some todo items but basic implementation is done and I'd like to
>> get any helpful feedback on it.
>>
>> Todo:
>> - Add sensor calibration.

I should have added: "(coming in version 2 of this patch)".  The
reading of the fuse value and the calibration will be in the next
version of this patch.

>> - Re-organize code/files if deemed necessary by community.
>
>
> Just fyi, I had a short discussion with Eric Miao about this. The conclusion
> was:
>
> The patch itself is OK. It's just lacking some features, but these can be
> added later. The driver supports thermal reading only, but not accurate as
> the thermal calibration data is burned into the FUSE, and differ from chip
> to chip. At the moment only incorrect thermal information will be exposed to
> user space. The patch is lacking connections to cpufreq and cpu hotplug, but
> those could be added later, too.
>

This patch connects to cpufreq using cpu_cooling functionality
recently submitted in Amit Katchhap's patch linked above.  This
registration is done here:

+
+       th_zone->cool_dev = cpufreq_cooling_register(
+               (struct freq_pctg_table *)th_zone->thermal_data->freq_tab,
+               IMX6Q_THERMAL_ACT_TRP_PTS, cpumask_of(0));
+

I tested it successfully with CPUFREQ during development, but I did
not re-test in this final patch and noticed a couple of bugs I
introduced later that will prevent it from working properly.  v2 will
resolve these issues.

> Best regards
>
> Dirk
Rob Jan. 13, 2012, 12:11 a.m. UTC | #4
Hello Sascha, comments below.  Thanks.

On Thu, Jan 12, 2012 at 3:36 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Jan 11, 2012 at 01:18:05AM -0600, Robert Lee wrote:
>> Add thermal support for i.MX6Q.  Uses recently submitted common
>> cpu_cooling functionality shown here:
>>
>> http://www.spinics.net/lists/linux-pm/msg26500.html
>>
>> Have some todo items but basic implementation is done and I'd like to
>> get any helpful feedback on it.
>
> Generally shouldn't this be a regular driver?
>

After considering your comment, there is a small window of up to 17
microsecond immediately following the msleep(1) call where if the
system suspended completely in 17 microseconds, an internal oscillator
needed by the sensor could get powered off in a very low power suspend
mode, and I'd guess this could distort the temperature, although the
designers say it won't cause any problem in the system.  While hitting
that race condition that seems highly unlikely, it's still better not
to have that race condition so creating and registering a
platfrom_driver structure would give a suspend and resume hook to
allow this to be handled correctly.  It seems wasteful to create this
structure just for the suspend and resume callbacks but I'm not
knowledgeable on a lightweight method to use instead.  The suspend
notifier PM_POST_SUSPEND doesn't seem to guarantee it will get called
before the work queue thread continues (at least on SMP systems).  If
there was one default registered device that in turn had its own list
of suspend and resume callbacks from those who registered to it, this
might work.  But I'm sure there is some reason why that is a bad idea
to set up such a mechanism.

Besides handling of suspend and resume, are there other reasons why
this should be a driver?

>
>> +
>> +/* register define of anatop */
>> +#define HW_ANADIG_ANA_MISC0  (0x00000150)
>> +#define HW_ANADIG_ANA_MISC0_SET      (0x00000154)
>> +#define HW_ANADIG_ANA_MISC0_CLR      (0x00000158)
>> +#define HW_ANADIG_ANA_MISC0_TOG      (0x0000015c)
>
> unnecessary braces. Please remove.

Ok.

>
>> +
>> +#if IMX6Q_THERMAL_DEBUG
>> +     pr_info("Temperature is %lu C\n", *temp);
>> +#endif
>
> Please don't add your own debug defines which then issue
> messages with pr_info. Use pr_debug in the first place.

Ok.

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
diff mbox

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 298c1cd..dd8cede 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -29,3 +29,9 @@  config CPU_THERMAL
 	  This will be useful for platforms using the generic thermal interface
 	  and not the ACPI interface.
 	  If you want this support, you should say Y or M here.
+
+config IMX6Q_THERMAL
+	bool "IMX6Q Thermal interface support"
+	depends on THERMAL && CPU_THERMAL
+	help
+	  Adds thermal management for IMX6Q.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 655cbc4..e2bcffe 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -4,3 +4,4 @@ 
 
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 obj-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
+obj-$(CONFIG_IMX6Q_THERMAL)	+= imx6q_thermal.o
diff --git a/drivers/thermal/imx6q_thermal.c b/drivers/thermal/imx6q_thermal.c
new file mode 100644
index 0000000..161a1a9
--- /dev/null
+++ b/drivers/thermal/imx6q_thermal.c
@@ -0,0 +1,370 @@ 
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/* i.MX6Q Thermal Implementation */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/types.h>
+#include <linux/thermal.h>
+#include <linux/io.h>
+#include <linux/syscalls.h>
+#include <linux/cpufreq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+#include <linux/cpu_cooling.h>
+
+/* register define of anatop */
+#define HW_ANADIG_ANA_MISC0	(0x00000150)
+#define HW_ANADIG_ANA_MISC0_SET	(0x00000154)
+#define HW_ANADIG_ANA_MISC0_CLR	(0x00000158)
+#define HW_ANADIG_ANA_MISC0_TOG	(0x0000015c)
+
+#define HW_ANADIG_TEMPSENSE0	(0x00000180)
+#define HW_ANADIG_TEMPSENSE0_SET	(0x00000184)
+#define HW_ANADIG_TEMPSENSE0_CLR	(0x00000188)
+#define HW_ANADIG_TEMPSENSE0_TOG	(0x0000018c)
+
+#define HW_ANADIG_TEMPSENSE1	(0x00000190)
+#define HW_ANADIG_TEMPSENSE1_SET	(0x00000194)
+#define HW_ANADIG_TEMPSENSE1_CLR	(0x00000198)
+
+#define BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF 0x00000008
+
+#define BP_ANADIG_TEMPSENSE0_TEMP_VALUE      8
+#define BM_ANADIG_TEMPSENSE0_TEMP_VALUE 0x000FFF00
+#define BF_ANADIG_TEMPSENSE0_TEMP_VALUE(v)  \
+	(((v) << 8) & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
+#define BM_ANADIG_TEMPSENSE0_FINISHED 0x00000004
+#define BM_ANADIG_TEMPSENSE0_MEASURE_TEMP 0x00000002
+#define BM_ANADIG_TEMPSENSE0_POWER_DOWN 0x00000001
+
+#define BP_ANADIG_TEMPSENSE1_MEASURE_FREQ      0
+#define BM_ANADIG_TEMPSENSE1_MEASURE_FREQ 0x0000FFFF
+#define BF_ANADIG_TEMPSENSE1_MEASURE_FREQ(v)  \
+	(((v) << 0) & BM_ANADIG_TEMPSENSE1_MEASURE_FREQ)
+
+#define CONVER_CONST			14113  /* need to add calibration */
+#define CONVER_DIV				17259
+#define REG_VALUE_TO_CEL(val) (((CONVER_CONST - val * 10)\
+			* 1000) / CONVER_DIV);
+
+#define IMX6Q_THERMAL_POLLING_FREQUENCY_MS 1000
+#define IMX6Q_THERMAL_ACT_TRP_PTS 3
+/* assumption: always one critical trip point */
+#define IMX6Q_THERMAL_TOTAL_TRP_PTS (IMX6Q_THERMAL_ACT_TRP_PTS + 1)
+#define IMX6Q_THERMAL_DEBUG 1
+
+struct trip_point {
+	u8 temp; /* in celcius */
+	u8 type;
+};
+
+struct imx6q_thermal_data {
+	struct trip_point trp_pts[IMX6Q_THERMAL_TOTAL_TRP_PTS];
+	struct freq_pctg_table freq_tab[IMX6Q_THERMAL_ACT_TRP_PTS];
+};
+
+struct thermal_sensor_conf {
+	char	*name;
+	int	(*read_temperature)(void *data);
+};
+
+static int imx6q_get_temp(struct thermal_zone_device *thermal,
+				  unsigned long *temp);
+
+static struct thermal_sensor_conf imx6q_sensor_conf = {
+	.name			= "imx6q-temp_sens",
+	.read_temperature	= (int (*)(void *))imx6q_get_temp,
+
+};
+
+/*
+ * This data defines the various trip points that will trigger action
+ * when crossed.
+ */
+static struct imx6q_thermal_data thermal_data = {
+	.trp_pts[0] = {
+		.temp	 = 85,
+		.type	 = THERMAL_TRIP_STATE_ACTIVE,
+	},
+	.freq_tab[0] = {
+		.freq_clip_pctg[0] = 25,
+	},
+	.trp_pts[1] = {
+		.temp	 = 90,
+		.type	 = THERMAL_TRIP_STATE_ACTIVE,
+	},
+	.freq_tab[1] = {
+		.freq_clip_pctg[0] = 65,
+	},
+	.trp_pts[2] = {
+		.temp	 = 95,
+		.type	 = THERMAL_TRIP_STATE_ACTIVE,
+	},
+	.freq_tab[2] = {
+		.freq_clip_pctg[0] = 99,
+	},
+	.trp_pts[3] = {
+		.temp	 = 100,
+		.type	 = THERMAL_TRIP_CRITICAL,
+	},
+};
+
+struct imx6q_thermal_zone {
+	struct thermal_zone_device *therm_dev;
+	struct thermal_cooling_device *cool_dev;
+	struct thermal_sensor_conf *sensor_conf;
+	struct imx6q_thermal_data *thermal_data;
+};
+
+static struct imx6q_thermal_zone *th_zone;
+
+static void __iomem *anatop_base;
+
+static int imx6q_get_mode(struct thermal_zone_device *thermal,
+			    enum thermal_device_mode *mode)
+{
+	*mode = THERMAL_DEVICE_ENABLED;
+	return 0;
+}
+
+static int imx6q_get_trip_type(struct thermal_zone_device *thermal, int trip,
+				 enum thermal_trip_type *type)
+{
+	if (trip >= IMX6Q_THERMAL_TOTAL_TRP_PTS)
+		return -EINVAL;
+
+	*type = th_zone->thermal_data->trp_pts[trip].type;
+
+	return 0;
+}
+
+static int imx6q_get_trip_temp(struct thermal_zone_device *thermal, int trip,
+				 unsigned long *temp)
+{
+	if (trip >= IMX6Q_THERMAL_TOTAL_TRP_PTS)
+		return -EINVAL;
+
+	*temp = th_zone->thermal_data->trp_pts[trip].temp;
+
+	/*convert the temperature into millicelsius*/
+	*temp = *temp * 1000;
+	return 0;
+}
+
+static int imx6q_get_crit_temp(struct thermal_zone_device *thermal,
+				 unsigned long *temp)
+{
+
+	*temp = th_zone->thermal_data->trp_pts[
+		IMX6Q_THERMAL_TOTAL_TRP_PTS - 1].temp;
+	/*convert the temperature into millicelsius*/
+	*temp = *temp * 1000;
+	return 0;
+}
+
+static int imx6q_bind(struct thermal_zone_device *thermal,
+			struct thermal_cooling_device *cdev)
+{
+	/* if the cooling device is the one from imx6 bind it */
+	if (cdev != th_zone->cool_dev)
+		return 0;
+
+	if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
+		pr_err("error binding cooling dev\n");
+		return -EINVAL;
+	}
+	if (thermal_zone_bind_cooling_device(thermal, 1, cdev)) {
+		pr_err("error binding cooling dev\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int imx6q_unbind(struct thermal_zone_device *thermal,
+			  struct thermal_cooling_device *cdev)
+{
+	if (cdev != th_zone->cool_dev)
+		return 0;
+	if (thermal_zone_unbind_cooling_device(thermal, 0, cdev)) {
+		pr_err("error unbinding cooling dev\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int imx6q_temp_sens_reg_dump(void)
+{
+	if (!anatop_base) {
+		pr_info("anatop_base is not initialized!!!\n");
+		return -EINVAL;
+	}
+	pr_info("HW_ANADIG_TEMPSENSE0 = 0x%x\n",
+			readl_relaxed(anatop_base + HW_ANADIG_TEMPSENSE0));
+	pr_info("HW_ANADIG_TEMPSENSE1 = 0x%x\n",
+			readl_relaxed(anatop_base + HW_ANADIG_TEMPSENSE1));
+	return 0;
+}
+
+static int imx6q_get_temp(struct thermal_zone_device *thermal,
+				  unsigned long *temp)
+{
+	unsigned int tmp;
+	unsigned int reg;
+
+	/*
+	 * For now we only using single measure.  Every time we measure
+	 * the temperature, we will power on/down the anadig module
+	 */
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+		anatop_base + HW_ANADIG_TEMPSENSE0_CLR);
+
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_FINISHED,
+		anatop_base + HW_ANADIG_TEMPSENSE0_CLR);
+
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
+		anatop_base + HW_ANADIG_TEMPSENSE0_SET);
+	/*
+	 * According to designers, may take up to ~17us for hardware to make
+	 * a measurement.  But because we have a 'finished' status bit, so we
+	 * check it just in case the designers are liars.
+	 */
+	do  {
+		msleep(1);
+	} while (!(readl_relaxed(anatop_base + HW_ANADIG_TEMPSENSE0)
+		& BM_ANADIG_TEMPSENSE0_FINISHED));
+
+	reg = readl_relaxed(anatop_base + HW_ANADIG_TEMPSENSE0);
+
+	tmp = (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
+			>> BP_ANADIG_TEMPSENSE0_TEMP_VALUE;
+
+#if IMX6Q_THERMAL_DEBUG
+	imx6q_temp_sens_reg_dump();
+#endif
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
+		anatop_base + HW_ANADIG_TEMPSENSE0_CLR);
+
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+			anatop_base + HW_ANADIG_TEMPSENSE0_SET);
+
+	*temp = REG_VALUE_TO_CEL(tmp);
+
+#if IMX6Q_THERMAL_DEBUG
+	pr_info("Temperature is %lu C\n", *temp);
+#endif
+	return 0;
+}
+
+/* bind callback functions to thermalzone */
+static struct thermal_zone_device_ops imx6q_dev_ops = {
+	.bind = imx6q_bind,
+	.unbind = imx6q_unbind,
+	.get_temp = imx6q_get_temp,
+	.get_mode = imx6q_get_mode,
+	.get_trip_type = imx6q_get_trip_type,
+	.get_trip_temp = imx6q_get_trip_temp,
+	.get_crit_temp = imx6q_get_crit_temp,
+};
+
+void imx6q_unregister_thermal(void)
+{
+	if (th_zone && th_zone->cool_dev)
+		cpufreq_cooling_unregister();
+
+	if (th_zone && th_zone->therm_dev)
+		thermal_zone_device_unregister(th_zone->therm_dev);
+
+	kfree(th_zone);
+
+	pr_info("i.MX6Q: Kernel Thermal management unregistered\n");
+}
+EXPORT_SYMBOL(imx6q_unregister_thermal);
+
+int __init imx6q_register_thermal(void)
+{
+	struct device_node *np;
+	int ret;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
+	anatop_base = of_iomap(np, 0);
+
+	if (!anatop_base) {
+		pr_err("Could not retrieve anantop-base\n");
+		return -EINVAL;
+	}
+
+	/* Make sure sensor is in known good state for measurements */
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+			anatop_base + HW_ANADIG_TEMPSENSE0_CLR);
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
+		anatop_base + HW_ANADIG_TEMPSENSE0_CLR);
+	writel_relaxed(BM_ANADIG_TEMPSENSE1_MEASURE_FREQ,
+		anatop_base + HW_ANADIG_TEMPSENSE1_CLR);
+	writel_relaxed(BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF,
+			anatop_base + HW_ANADIG_ANA_MISC0_SET);
+	writel_relaxed(BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+			anatop_base + HW_ANADIG_TEMPSENSE0_SET);
+
+	th_zone = kzalloc(sizeof(struct imx6q_thermal_zone), GFP_KERNEL);
+	if (!th_zone) {
+		ret = -ENOMEM;
+		goto err_unregister;
+	}
+
+	th_zone->sensor_conf = &imx6q_sensor_conf;
+
+	th_zone->thermal_data = &thermal_data;
+	if (!th_zone->thermal_data) {
+		pr_err("Temperature sensor data not initialised\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	th_zone->cool_dev = cpufreq_cooling_register(
+		(struct freq_pctg_table *)th_zone->thermal_data->freq_tab,
+		IMX6Q_THERMAL_ACT_TRP_PTS, cpumask_of(0));
+
+	if (IS_ERR(th_zone->cool_dev)) {
+		pr_err("Failed to register cpufreq cooling device\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	th_zone->therm_dev = thermal_zone_device_register(
+		th_zone->sensor_conf->name, 3, NULL, &imx6q_dev_ops,
+		0, 0, 0, IMX6Q_THERMAL_POLLING_FREQUENCY_MS);
+
+	if (IS_ERR(th_zone->therm_dev)) {
+		pr_err("Failed to register thermal zone device\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	pr_info("i.MX6: Kernel Thermal management registered\n");
+
+	return 0;
+
+err_unregister:
+	imx6q_unregister_thermal();
+	return ret;
+}
+EXPORT_SYMBOL(imx6q_register_thermal);
+
+module_init(imx6q_register_thermal);