2012-06-20 07:06:13

by Rob Lee

[permalink] [raw]
Subject: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

Based on v3.5-rc3 plus recently submmited cpu_cooling functionality here:

http://www.spinics.net/lists/kernel/msg1340221.html

I am told this cpu cooling patch has been accepted and exists in len brown's
next branch.

link to previous submissions:
v4: http://comments.gmane.org/gmane.linux.acpi.devel/51779
v3: http://www.spinics.net/lists/arm-kernel/msg155955.html
v2: http://www.spinics.net/lists/arm-kernel/msg155790.html
v1: http://www.spinics.net/lists/arm-kernel/msg155111.html

Changes in v5:
1. Modified to use anatop mfd driver for accessing anatop registers
2. Made necessary changes to work with latest generic CPU cooling code.
3. Added Config changes and functionality to allow testing on parts without
without programmed temperature sensor calibration values.
4. General cleanup and addition of comments.

Changes in v4:
1. Removed bad suspend/resume assignment into thermal class. After further
examination and discussion with SoC designers, a sequence is now used
for making measurements that is is unaffected by system suspendresumes.
Temp Sensor automatically powers off in hardware during the low power mode
caused by a system suspend.
2. Moved some structures from static to dynamic allocation.
3. Added some noise handling to temperatuer sensor readings.

Changes in v3:
1. Fixed the various issues pointed out in v2
2. Made other code cleanup and a bit of re-organizing
3. Removed unecessary platform driver and device.

Changes in v2:
1. Cleaned up some style issues pointed out in v1
2. Made various other code cleanup and re-organizing
3. Added temperature sensor calibration
4. Created platform driver and device to hook into pm suspend.


Performed some basic testing to ensure proper cooling operating. If
you want to test this, full testing requires imx6q cpufreq
implementation (not yet in v3.5).

Robert Lee (1):
ARM: imx: Add basic imx6q cpu thermal management

arch/arm/boot/dts/imx6q.dtsi | 5 +
drivers/thermal/Kconfig | 28 ++
drivers/thermal/Makefile | 1 +
drivers/thermal/imx6q_thermal.c | 593 +++++++++++++++++++++++++++++++++++++++
4 files changed, 627 insertions(+)
create mode 100644 drivers/thermal/imx6q_thermal.c

--
1.7.10


2012-06-20 07:06:18

by Rob Lee

[permalink] [raw]
Subject: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

Add imx6q cpu thermal management driver using the new cpu cooling
interface which limits system performance via cpufreq to reduce
the cpu temperature. Temperature readings are taken using
the imx6q temperature sensor and this functionality was added
as part of this cpu thermal management driver.

Signed-off-by: Robert Lee <[email protected]>
---
arch/arm/boot/dts/imx6q.dtsi | 5 +
drivers/thermal/Kconfig | 28 ++
drivers/thermal/Makefile | 1 +
drivers/thermal/imx6q_thermal.c | 593 +++++++++++++++++++++++++++++++++++++++
4 files changed, 627 insertions(+)
create mode 100644 drivers/thermal/imx6q_thermal.c

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 8c90cba..2650f65 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -442,6 +442,10 @@
anatop-min-voltage = <725000>;
anatop-max-voltage = <1450000>;
};
+
+ thermal {
+ compatible ="fsl,anatop-thermal";
+ };
};

usbphy@020c9000 { /* USBPHY1 */
@@ -659,6 +663,7 @@
};

ocotp@021bc000 {
+ compatible = "fsl,imx6q-ocotp";
reg = <0x021bc000 0x4000>;
};

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 04c6796..b80c408 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -30,6 +30,34 @@ config CPU_THERMAL
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 MFD_ANATOP && CPU_THERMAL
+ help
+ Adds thermal management for IMX6Q.
+
+config IMX6Q_THERMAL_FAKE_CALIBRATION
+ bool "IMX6Q fake temperature sensor calibration (FOR TESTING ONLY)"
+ depends on IMX6Q_THERMAL
+ help
+ This enables a fake temp sensor calibration value for parts without
+ the correction calibration values burned into OCOTP. If the part
+ already has the calibrated values burned into OCOTP, enabling this
+ does nothing.
+ WARNING: Use of this feature is for testing only as it will cause
+ incorrect temperature readings which will result in incorrect system
+ thermal limiting behavior such as premature system performance
+ limiting or lack of proper performance reduction to reduce cpu
+ temperature
+
+config IMX6Q_THERMAL_FAKE_CAL_VAL
+ hex "IMX6Q fake temperature sensor calibration value"
+ depends on IMX6Q_THERMAL_FAKE_CALIBRATION
+ default 0x5704c67d
+ help
+ Refer to the temperature sensor section of the imx6q reference manual
+ for more inforation on how this value is used.
+
config SPEAR_THERMAL
bool "SPEAr thermal sensor driver"
depends on THERMAL
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 4636e35..fc4004e 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_THERMAL) += thermal_sys.o
obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
obj-$(CONFIG_EXYNOS_THERMAL) += exynos_thermal.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..255d646
--- /dev/null
+++ b/drivers/thermal/imx6q_thermal.c
@@ -0,0 +1,593 @@
+/*
+ * 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/device.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>
+#include <linux/platform_device.h>
+#include <linux/mfd/anatop.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 BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF 0x00000008
+
+#define HW_ANADIG_TEMPSENSE0 0x00000180
+#define HW_ANADIG_TEMPSENSE0_SET 0x00000184
+#define HW_ANADIG_TEMPSENSE0_CLR 0x00000188
+#define HW_ANADIG_TEMPSENSE0_TOG 0x0000018c
+
+#define BP_ANADIG_TEMPSENSE0_TEMP_VALUE 8
+#define BM_ANADIG_TEMPSENSE0_TEMP_VALUE 0x000FFF00
+#define BM_ANADIG_TEMPSENSE0_FINISHED 0x00000004
+#define BM_ANADIG_TEMPSENSE0_MEASURE_TEMP 0x00000002
+#define BM_ANADIG_TEMPSENSE0_POWER_DOWN 0x00000001
+
+#define HW_ANADIG_TEMPSENSE1 0x00000190
+#define HW_ANADIG_TEMPSENSE1_SET 0x00000194
+#define HW_ANADIG_TEMPSENSE1_CLR 0x00000198
+#define BP_ANADIG_TEMPSENSE1_MEASURE_FREQ 0
+#define BM_ANADIG_TEMPSENSE1_MEASURE_FREQ 0x0000FFFF
+
+#define HW_OCOTP_ANA1 0x000004E0
+
+#define IMX6Q_THERMAL_POLLING_FREQUENCY_MS 1000
+
+#define IMX6Q_THERMAL_ACTIVE_TRP_PTS 3
+/* assumption: always one critical trip point */
+#define IMX6Q_THERMAL_TOTAL_TRP_PTS (IMX6Q_THERMAL_ACTIVE_TRP_PTS + 1)
+
+struct imx6q_sensor_data {
+ int c1, c2;
+ char name[16];
+ u8 meas_delay; /* in milliseconds */
+ u32 noise_margin; /* in millicelcius */
+ int last_temp; /* in millicelcius */
+ /*
+ * When noise filtering, if consecutive measurements are each higher
+ * up to consec_high_limit times, assume changing temperature readings
+ * to be valid and not noise.
+ */
+ u32 consec_high_limit;
+ struct anatop *anatopmfd;
+};
+
+struct imx6q_thermal_data {
+ enum thermal_trip_type type[IMX6Q_THERMAL_TOTAL_TRP_PTS];
+ struct freq_clip_table freq_tab[IMX6Q_THERMAL_ACTIVE_TRP_PTS];
+ unsigned int crit_temp_level;
+};
+
+struct imx6q_thermal_zone {
+ struct thermal_zone_device *therm_dev;
+ struct thermal_cooling_device *cool_dev;
+ struct imx6q_sensor_data *sensor_data;
+ struct imx6q_thermal_data *thermal_data;
+ struct thermal_zone_device_ops dev_ops;
+};
+
+static struct imx6q_sensor_data g_sensor_data __devinitdata = {
+ .name = "imx6q-temp_sens",
+ .meas_delay = 1, /* in milliseconds */
+ .noise_margin = 3000,
+ .consec_high_limit = 3,
+};
+
+/*
+ * This data defines the various temperature trip points that will trigger
+ * cooling action when crossed.
+ */
+static struct imx6q_thermal_data g_thermal_data __devinitdata = {
+ .type[0] = THERMAL_TRIP_ACTIVE,
+ .freq_tab[0] = {
+ .freq_clip_max = 800 * 1000,
+ .temp_level = 85000,
+ },
+ .type[1] = THERMAL_TRIP_ACTIVE,
+ .freq_tab[1] = {
+ .freq_clip_max = 400 * 1000,
+ .temp_level = 90000,
+ },
+ .type[2] = THERMAL_TRIP_ACTIVE,
+ .freq_tab[2] = {
+ .freq_clip_max = 200 * 1000,
+ .temp_level = 95000,
+ },
+ .type[3] = THERMAL_TRIP_CRITICAL,
+ .crit_temp_level = 100000,
+};
+
+static int th_sys_get_temp(struct thermal_zone_device *thermal,
+ unsigned long *temp);
+
+static struct imx6q_thermal_zone *th_zone;
+static void __iomem *ocotp_base;
+
+static inline int imx6q_get_temp(int *temp, struct imx6q_sensor_data *sd)
+{
+ unsigned int n_meas;
+ unsigned int reg;
+
+ do {
+ /*
+ * Every time we measure the temperature, we will power on the
+ * temperature sensor, enable measurements, take a reading,
+ * disable measurements, power off the temperature sensor.
+ */
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN);
+
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
+ BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
+ BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
+
+ /*
+ * According to imx6q temp sensor designers,
+ * it may require up to ~17us to complete
+ * a measurement. But this timing isn't checked on every part
+ * nor is it specified in the datasheet, so we check the
+ * 'finished' status bit to be sure of completion of a valid
+ * measurement.
+ */
+ msleep(sd->meas_delay);
+
+ reg = anatop_read_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0);
+
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
+ BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
+
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN);
+
+ } while (!(reg & BM_ANADIG_TEMPSENSE0_FINISHED) &&
+ (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE));
+
+ n_meas = (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
+ >> BP_ANADIG_TEMPSENSE0_TEMP_VALUE;
+
+ /* See imx6q_thermal_process_fuse_data() for forumla derivation. */
+ *temp = sd->c2 + (sd->c1 * n_meas);
+
+ pr_debug("Temperature: %d\n", *temp / 1000);
+
+ return 0;
+}
+
+static int th_sys_get_temp(struct thermal_zone_device *thermal,
+ unsigned long *temp)
+{
+ int i, total = 0, tmp = 0;
+ const u8 loop = 5;
+ u32 consec_high = 0;
+
+ struct imx6q_sensor_data *sd;
+
+ sd = ((struct imx6q_thermal_zone *)thermal->devdata)->sensor_data;
+
+ /*
+ * Measure temperature and handle noise
+ *
+ * While the imx6q temperature sensor is designed to minimize being
+ * affected by system noise, it's safest to run sanity checks and
+ * perform any necessary filitering.
+ */
+ for (i = 0; (sd->noise_margin) && (i < loop); i++) {
+ imx6q_get_temp(&tmp, sd);
+
+ if ((abs(tmp - sd->last_temp) <= sd->noise_margin) ||
+ (consec_high >= sd->consec_high_limit)) {
+ sd->last_temp = tmp;
+ i = 0;
+ break;
+ }
+ if (tmp > sd->last_temp)
+ consec_high++;
+
+ /*
+ * ignore first measurement as the previous measurement was
+ * a long time ago.
+ */
+ if (i)
+ total += tmp;
+
+ sd->last_temp = tmp;
+ }
+
+ if (sd->noise_margin && i)
+ tmp = total / (loop - 1);
+
+ /*
+ * The thermal framework code stores temperature in unsigned long. Also,
+ * it has references to "millicelcius" which limits the lowest
+ * temperature possible (compared to Kelvin).
+ */
+ if (tmp > 0)
+ *temp = tmp;
+ else
+ *temp = 0;
+
+ return 0;
+}
+
+static int th_sys_get_mode(struct thermal_zone_device *thermal,
+ enum thermal_device_mode *mode)
+{
+ *mode = THERMAL_DEVICE_ENABLED;
+ return 0;
+}
+
+static int th_sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
+ enum thermal_trip_type *type)
+{
+ struct imx6q_thermal_data *p;
+
+ if (trip >= thermal->trips)
+ return -EINVAL;
+
+ p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
+
+ *type = p->type[trip];
+
+ return 0;
+}
+
+static int th_sys_get_trip_temp(struct thermal_zone_device *thermal, int trip,
+ unsigned long *temp)
+{
+ struct imx6q_thermal_data *p;
+ enum thermal_trip_type type;
+
+ if (trip >= thermal->trips)
+ return -EINVAL;
+
+ p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
+
+ thermal->ops->get_trip_type(thermal, trip, &type);
+
+ if (type == THERMAL_TRIP_CRITICAL)
+ *temp = p->crit_temp_level;
+ else
+ *temp = p->freq_tab[trip].temp_level;
+ return 0;
+}
+
+static int th_sys_get_crit_temp(struct thermal_zone_device *thermal,
+ unsigned long *temp)
+{
+ struct imx6q_thermal_data *p;
+
+ p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
+ *temp = p->crit_temp_level;
+ return 0;
+}
+
+static int th_sys_bind(struct thermal_zone_device *thermal,
+ struct thermal_cooling_device *cdev)
+{
+ int i;
+ struct thermal_cooling_device *cd;
+
+ cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
+
+ /* if the cooling device is the one from imx6 bind it */
+ if (cdev != cd)
+ return 0;
+
+ for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
+ if (thermal_zone_bind_cooling_device(thermal, i, cdev)) {
+ pr_err("error binding cooling dev\n");
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int th_sys_unbind(struct thermal_zone_device *thermal,
+ struct thermal_cooling_device *cdev)
+{
+ int i;
+
+ struct thermal_cooling_device *cd;
+
+ cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
+
+ if (cdev != cd)
+ return 0;
+
+ for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
+ if (thermal_zone_unbind_cooling_device(thermal, i, cdev)) {
+ pr_err("error unbinding cooling dev\n");
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static struct thermal_zone_device_ops g_dev_ops __devinitdata = {
+ .bind = th_sys_bind,
+ .unbind = th_sys_unbind,
+ .get_temp = th_sys_get_temp,
+ .get_mode = th_sys_get_mode,
+ .get_trip_type = th_sys_get_trip_type,
+ .get_trip_temp = th_sys_get_trip_temp,
+ .get_crit_temp = th_sys_get_crit_temp,
+};
+
+static int __devinit imx6q_thermal_process_fuse_data(unsigned int fuse_data,
+ struct imx6q_sensor_data *sd)
+{
+ int t1, t2, n1, n2;
+
+ if (fuse_data == 0 || fuse_data == 0xffffffff) {
+ pr_warn("WARNING: Incorrect temperature sensor value of %x "
+ "detected\n", fuse_data);
+
+#ifdef CONFIG_IMX6Q_THERMAL_FAKE_CALIBRATION
+ pr_warn(
+ "WARNING: Using fake calibration value of %x value. "
+ "This will cause your system performance to be limited "
+ "prematurely (compared to a using properly calibrated "
+ "value) OR will cause the system to allow the "
+ "temperature to exceed the maximum specified "
+ "temperature without the proper performance "
+ "limiting.\n", CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL);
+
+ fuse_data = CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL;
+#else
+ pr_warn("WARNING: Disabling CPU thermal protection\n");
+ return -EINVAL;
+#endif
+ }
+
+ /*
+ * Fuse data layout:
+ * [31:20] sensor value @ 25C
+ * [19:8] sensor value of hot
+ * [7:0] hot temperature value
+ */
+ n1 = fuse_data >> 20;
+ n2 = (fuse_data & 0xfff00) >> 8;
+ t2 = fuse_data & 0xff;
+ t1 = 25; /* t1 always 25C */
+
+ pr_debug(" -- temperature sensor calibration data --\n");
+ pr_debug("HW_OCOTP_ANA1: %x\n", fuse_data);
+ pr_debug("n1: %d\nn2: %d\nt1: %d\nt2: %d\n", n1, n2, t1, t2);
+
+ /*
+ * Derived from linear interpolation,
+ * Tmeas = T2 + (Nmeas - N2) * (T1 - T2) / (N1 - N2)
+ * We want to reduce this down to the minimum computation necessary
+ * for each temperature read. Also, we want Tmeas in millicelcius
+ * and we don't want to lose precision from integer division. So...
+ * milli_Tmeas = 1000 * T2 + 1000 * (Nmeas - N2) * (T1 - T2) / (N1 - N2)
+ * Let constant c1 = 1000 * (T1 - T2) / (N1 - N2)
+ * milli_Tmeas = (1000 * T2) + c1 * (Nmeas - N2)
+ * milli_Tmeas = (1000 * T2) + (c1 * Nmeas) - (c1 * N2)
+ * Let constant c2 = (1000 * T2) - (c1 * N2)
+ * milli_Tmeas = c2 + (c1 * Nmeas)
+ */
+ sd->c1 = (1000 * (t1 - t2)) / (n1 - n2);
+ sd->c2 = (1000 * t2) - (sd->c1 * n2);
+
+ pr_debug("c1: %i\n", sd->c1);
+ pr_debug("c2: %i\n", sd->c2);
+
+ return 0;
+}
+
+static int anatop_thermal_suspend(struct platform_device *pdev,
+ pm_message_t state)
+{
+ struct imx6q_sensor_data *sd = pdev->dev.platform_data;
+
+ /* power off the sensor during suspend */
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
+ BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
+
+ anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN);
+
+ return 0;
+}
+
+static int __devexit anatop_thermal_remove(struct platform_device *pdev)
+{
+ if (th_zone && th_zone->therm_dev)
+ thermal_zone_device_unregister(th_zone->therm_dev);
+
+ if (th_zone && th_zone->cool_dev)
+ cpufreq_cooling_unregister(th_zone->cool_dev);
+
+ kfree(th_zone->sensor_data);
+ kfree(th_zone->thermal_data);
+ kfree(th_zone);
+
+ if (ocotp_base)
+ iounmap(ocotp_base);
+
+ pr_info("i.MX6Q: Kernel Thermal management unregistered\n");
+
+ return 0;
+}
+
+static int __devinit anatop_thermal_probe(struct platform_device *pdev)
+{
+ struct device_node *np_ocotp, *np_thermal;
+ unsigned int fuse_data;
+ struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
+ int ret, i;
+
+ np_ocotp = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
+ np_thermal = of_find_compatible_node(NULL, NULL, "fsl,anatop-thermal");
+
+ if (!(np_ocotp && np_thermal && anatopmfd))
+ return -ENXIO; /* not a compatible platform */
+
+ ocotp_base = of_iomap(np_ocotp, 0);
+
+ if (!ocotp_base) {
+ pr_err("Could not retrieve ocotp-base\n");
+ ret = -ENXIO;
+ goto err_unregister;
+ }
+
+ fuse_data = readl_relaxed(ocotp_base + HW_OCOTP_ANA1);
+
+ th_zone = kzalloc(sizeof(struct imx6q_thermal_zone), GFP_KERNEL);
+ if (!th_zone) {
+ ret = -ENOMEM;
+ goto err_unregister;
+ }
+
+ th_zone->dev_ops = g_dev_ops;
+
+ th_zone->thermal_data =
+ kzalloc(sizeof(struct imx6q_thermal_data), GFP_KERNEL);
+
+ if (!th_zone->thermal_data) {
+ ret = -ENOMEM;
+ goto err_unregister;
+ }
+
+ memcpy(th_zone->thermal_data, &g_thermal_data,
+ sizeof(struct imx6q_thermal_data));
+
+ for (i = 0; i < ARRAY_SIZE(g_thermal_data.freq_tab); i++)
+ th_zone->thermal_data->freq_tab[i].mask_val = cpumask_of(0);
+
+
+ th_zone->sensor_data =
+ kzalloc(sizeof(struct imx6q_sensor_data), GFP_KERNEL);
+
+ if (!th_zone->sensor_data) {
+ ret = -ENOMEM;
+ goto err_unregister;
+ }
+
+ memcpy(th_zone->sensor_data, &g_sensor_data,
+ sizeof(struct imx6q_sensor_data));
+
+ th_zone->sensor_data->anatopmfd = anatopmfd;
+
+ ret = imx6q_thermal_process_fuse_data(fuse_data, th_zone->sensor_data);
+
+ if (ret) {
+ pr_err("Invalid temperature calibration data.\n");
+ goto err_unregister;
+ }
+
+ if (!th_zone->sensor_data->meas_delay)
+ th_zone->sensor_data->meas_delay = 1;
+
+ /* Make sure sensor is in known good state for measurements */
+ anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN);
+
+ anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
+ BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
+
+ anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE1, 0,
+ BM_ANADIG_TEMPSENSE1_MEASURE_FREQ);
+
+ anatop_write_reg(anatopmfd, HW_ANADIG_ANA_MISC0_SET,
+ BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF,
+ BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF);
+
+ anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN,
+ BM_ANADIG_TEMPSENSE0_POWER_DOWN);
+
+ th_zone->cool_dev = cpufreq_cooling_register(
+ (struct freq_clip_table *)th_zone->thermal_data->freq_tab,
+ IMX6Q_THERMAL_ACTIVE_TRP_PTS);
+
+ 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_data->name, IMX6Q_THERMAL_TOTAL_TRP_PTS,
+ th_zone, &th_zone->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;
+ }
+
+ pdev->dev.platform_data = th_zone->sensor_data;
+
+ return 0;
+
+err_unregister:
+ anatop_thermal_remove(pdev);
+ return ret;
+}
+
+static struct of_device_id __devinitdata of_anatop_thermal_match_tbl[] = {
+ { .compatible = "fsl,anatop-thermal", },
+ { /* end */ }
+};
+
+static struct platform_driver anatop_thermal = {
+ .driver = {
+ .name = "anatop_thermal",
+ .owner = THIS_MODULE,
+ .of_match_table = of_anatop_thermal_match_tbl,
+ },
+ .probe = anatop_thermal_probe,
+ .remove = anatop_thermal_remove,
+ .suspend = anatop_thermal_suspend,
+ /* due to implentation of imx6q_get_temp , resume is unnecessary */
+};
+
+static int __devinit anatop_thermal_init(void)
+{
+ return platform_driver_register(&anatop_thermal);
+}
+device_initcall(anatop_thermal_init);
+
+static void __exit anatop_thermal_exit(void)
+{
+ platform_driver_unregister(&anatop_thermal);
+}
+module_exit(anatop_thermal_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor");
+MODULE_DESCRIPTION("i.MX6Q SoC thermal driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:imx6q-thermal");
--
1.7.10

2012-06-20 08:26:43

by Rob Lee

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

On Wed, Jun 20, 2012 at 2:06 AM, Robert Lee <[email protected]> wrote:
> Add imx6q cpu thermal management driver using the new cpu cooling
> interface which limits system performance via cpufreq to reduce
> the cpu temperature. ?Temperature readings are taken using
> the imx6q temperature sensor and this functionality was added
> as part of this cpu thermal management driver.
>
> Signed-off-by: Robert Lee <[email protected]>
> ---
> ?arch/arm/boot/dts/imx6q.dtsi ? ?| ? ?5 +
> ?drivers/thermal/Kconfig ? ? ? ? | ? 28 ++
> ?drivers/thermal/Makefile ? ? ? ?| ? ?1 +
> ?drivers/thermal/imx6q_thermal.c | ?593 +++++++++++++++++++++++++++++++++++++++
> ?4 files changed, 627 insertions(+)
> ?create mode 100644 drivers/thermal/imx6q_thermal.c
>
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 8c90cba..2650f65 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -442,6 +442,10 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?anatop-min-voltage = <725000>;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?anatop-max-voltage = <1450000>;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?};
> +
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? thermal {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible ="fsl,anatop-thermal";
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? };
> ? ? ? ? ? ? ? ? ? ? ? ?};
>
> ? ? ? ? ? ? ? ? ? ? ? ?usbphy@020c9000 { /* USBPHY1 */
> @@ -659,6 +663,7 @@
> ? ? ? ? ? ? ? ? ? ? ? ?};
>
> ? ? ? ? ? ? ? ? ? ? ? ?ocotp@021bc000 {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "fsl,imx6q-ocotp";
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x021bc000 0x4000>;
> ? ? ? ? ? ? ? ? ? ? ? ?};
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 04c6796..b80c408 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -30,6 +30,34 @@ config CPU_THERMAL
> ? ? ? ? ?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 MFD_ANATOP && CPU_THERMAL
> + ? ? ? help
> + ? ? ? ? Adds thermal management for IMX6Q.
> +
> +config IMX6Q_THERMAL_FAKE_CALIBRATION
> + ? ? ? bool "IMX6Q fake temperature sensor calibration (FOR TESTING ONLY)"
> + ? ? ? depends on IMX6Q_THERMAL
> + ? ? ? help
> + ? ? ? ? This enables a fake temp sensor calibration value for parts without
> + ? ? ? ? the correction calibration values burned into OCOTP. If the part
> + ? ? ? ? already has the calibrated values burned into OCOTP, enabling this
> + ? ? ? ? does nothing.
> + ? ? ? ? WARNING: Use of this feature is for testing only as it will cause
> + ? ? ? ? incorrect temperature readings which will result in incorrect system
> + ? ? ? ? thermal limiting behavior such as premature system performance
> + ? ? ? ? limiting or lack of proper performance reduction to reduce cpu
> + ? ? ? ? temperature
> +
> +config IMX6Q_THERMAL_FAKE_CAL_VAL
> + ? ? ? hex "IMX6Q fake temperature sensor calibration value"
> + ? ? ? depends on IMX6Q_THERMAL_FAKE_CALIBRATION
> + ? ? ? default 0x5704c67d
> + ? ? ? help
> + ? ? ? ? Refer to the temperature sensor section of the imx6q reference manual
> + ? ? ? ? for more inforation on how this value is used.
> +
> ?config SPEAR_THERMAL
> ? ? ? ?bool "SPEAr thermal sensor driver"
> ? ? ? ?depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 4636e35..fc4004e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_THERMAL) ? ? ? ? ? += thermal_sys.o
> ?obj-$(CONFIG_CPU_THERMAL) ? ? ? += cpu_cooling.o
> ?obj-$(CONFIG_SPEAR_THERMAL) ? ? ? ? ? ?+= spear_thermal.o
> ?obj-$(CONFIG_EXYNOS_THERMAL) ? ? ? ? ? += exynos_thermal.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..255d646
> --- /dev/null
> +++ b/drivers/thermal/imx6q_thermal.c
> @@ -0,0 +1,593 @@
> +/*
> + * 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/device.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>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/anatop.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 BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF ?0x00000008
> +
> +#define HW_ANADIG_TEMPSENSE0 ? ? ? ? ? ? ? ? ? 0x00000180
> +#define HW_ANADIG_TEMPSENSE0_SET ? ? ? ? ? ? ? 0x00000184
> +#define HW_ANADIG_TEMPSENSE0_CLR ? ? ? ? ? ? ? 0x00000188
> +#define HW_ANADIG_TEMPSENSE0_TOG ? ? ? ? ? ? ? 0x0000018c
> +
> +#define BP_ANADIG_TEMPSENSE0_TEMP_VALUE ? ? ? ? ? ? ? ?8
> +#define BM_ANADIG_TEMPSENSE0_TEMP_VALUE ? ? ? ? ? ? ? ?0x000FFF00
> +#define BM_ANADIG_TEMPSENSE0_FINISHED ? ? ? ? ?0x00000004
> +#define BM_ANADIG_TEMPSENSE0_MEASURE_TEMP ? ? ?0x00000002
> +#define BM_ANADIG_TEMPSENSE0_POWER_DOWN ? ? ? ? ? ? ? ?0x00000001
> +
> +#define HW_ANADIG_TEMPSENSE1 ? ? ? ? ? ? ? ? ? 0x00000190
> +#define HW_ANADIG_TEMPSENSE1_SET ? ? ? ? ? ? ? 0x00000194
> +#define HW_ANADIG_TEMPSENSE1_CLR ? ? ? ? ? ? ? 0x00000198
> +#define BP_ANADIG_TEMPSENSE1_MEASURE_FREQ ? ? ?0
> +#define BM_ANADIG_TEMPSENSE1_MEASURE_FREQ ? ? ?0x0000FFFF
> +
> +#define HW_OCOTP_ANA1 ? ? ? ? ? ? ? ? ? ? ? ? ?0x000004E0
> +
> +#define IMX6Q_THERMAL_POLLING_FREQUENCY_MS 1000
> +
> +#define IMX6Q_THERMAL_ACTIVE_TRP_PTS 3
> +/* assumption: always one critical trip point */
> +#define IMX6Q_THERMAL_TOTAL_TRP_PTS (IMX6Q_THERMAL_ACTIVE_TRP_PTS + 1)
> +
> +struct imx6q_sensor_data {
> + ? ? ? int ? ? c1, c2;
> + ? ? ? char ? ?name[16];
> + ? ? ? u8 ? ? ?meas_delay; ? ? /* in milliseconds */
> + ? ? ? u32 ? ? noise_margin; ? /* in millicelcius */
> + ? ? ? int ? ? last_temp; ? ? ?/* in millicelcius */
> + ? ? ? /*
> + ? ? ? ?* When noise filtering, if consecutive measurements are each higher
> + ? ? ? ?* up to consec_high_limit times, assume changing temperature readings
> + ? ? ? ?* to be valid and not noise.
> + ? ? ? ?*/
> + ? ? ? u32 ? ? consec_high_limit;
> + ? ? ? struct anatop *anatopmfd;
> +};
> +
> +struct imx6q_thermal_data {
> + ? ? ? enum thermal_trip_type type[IMX6Q_THERMAL_TOTAL_TRP_PTS];
> + ? ? ? struct freq_clip_table freq_tab[IMX6Q_THERMAL_ACTIVE_TRP_PTS];
> + ? ? ? unsigned int crit_temp_level;
> +};
> +
> +struct imx6q_thermal_zone {
> + ? ? ? struct thermal_zone_device ? ? ?*therm_dev;
> + ? ? ? struct thermal_cooling_device ? *cool_dev;
> + ? ? ? struct imx6q_sensor_data ? ? ? ?*sensor_data;
> + ? ? ? struct imx6q_thermal_data ? ? ? *thermal_data;
> + ? ? ? struct thermal_zone_device_ops ?dev_ops;
> +};
> +
> +static struct imx6q_sensor_data g_sensor_data __devinitdata = {
> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "imx6q-temp_sens",
> + ? ? ? .meas_delay ? ? ? ? ? ? = 1, ? ?/* in milliseconds */
> + ? ? ? .noise_margin ? ? ? ? ? = 3000,
> + ? ? ? .consec_high_limit ? ? ?= 3,
> +};
> +
> +/*
> + * This data defines the various temperature trip points that will trigger
> + * cooling action when crossed.
> + */
> +static struct imx6q_thermal_data g_thermal_data __devinitdata = {
> + ? ? ? .type[0] = THERMAL_TRIP_ACTIVE,
> + ? ? ? .freq_tab[0] = {
> + ? ? ? ? ? ? ? .freq_clip_max = 800 * 1000,
> + ? ? ? ? ? ? ? .temp_level = 85000,
> + ? ? ? },
> + ? ? ? .type[1] = THERMAL_TRIP_ACTIVE,
> + ? ? ? .freq_tab[1] = {
> + ? ? ? ? ? ? ? .freq_clip_max = 400 * 1000,
> + ? ? ? ? ? ? ? .temp_level = 90000,
> + ? ? ? },
> + ? ? ? .type[2] = THERMAL_TRIP_ACTIVE,
> + ? ? ? .freq_tab[2] = {
> + ? ? ? ? ? ? ? .freq_clip_max = 200 * 1000,
> + ? ? ? ? ? ? ? .temp_level = 95000,
> + ? ? ? },
> + ? ? ? .type[3] = THERMAL_TRIP_CRITICAL,
> + ? ? ? .crit_temp_level = 100000,
> +};
> +
> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *temp);
> +
> +static struct imx6q_thermal_zone ? ? ? *th_zone;
> +static void __iomem ? ? ? ? ? ? ? ? ? ?*ocotp_base;
> +
> +static inline int imx6q_get_temp(int *temp, struct imx6q_sensor_data *sd)
> +{
> + ? ? ? unsigned int n_meas;
> + ? ? ? unsigned int reg;
> +
> + ? ? ? do {
> + ? ? ? ? ? ? ? /*
> + ? ? ? ? ? ? ? ?* Every time we measure the temperature, we will power on the
> + ? ? ? ? ? ? ? ?* temperature sensor, enable measurements, take a reading,
> + ? ? ? ? ? ? ? ?* disable measurements, power off the temperature sensor.
> + ? ? ? ? ? ? ? ?*/
> + ? ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + ? ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + ? ? ? ? ? ? ? /*
> + ? ? ? ? ? ? ? ?* According to imx6q temp sensor designers,
> + ? ? ? ? ? ? ? ?* it may require up to ~17us to complete
> + ? ? ? ? ? ? ? ?* a measurement. ?But this timing isn't checked on every part
> + ? ? ? ? ? ? ? ?* nor is it specified in the datasheet, so we check the
> + ? ? ? ? ? ? ? ?* 'finished' status bit to be sure of completion of a valid
> + ? ? ? ? ? ? ? ?* measurement.
> + ? ? ? ? ? ? ? ?*/
> + ? ? ? ? ? ? ? msleep(sd->meas_delay);
> +
> + ? ? ? ? ? ? ? reg = anatop_read_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0);
> +
> + ? ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + ? ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + ? ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + ? ? ? } while (!(reg & BM_ANADIG_TEMPSENSE0_FINISHED) &&
> + ? ? ? ? ? ? ? ?(reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE));
> +
> + ? ? ? n_meas = (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
> + ? ? ? ? ? ? ? ? ? ? ? >> BP_ANADIG_TEMPSENSE0_TEMP_VALUE;
> +
> + ? ? ? /* See imx6q_thermal_process_fuse_data() for forumla derivation. */
> + ? ? ? *temp = sd->c2 + (sd->c1 * n_meas);
> +
> + ? ? ? pr_debug("Temperature: %d\n", *temp / 1000);
> +
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *temp)
> +{
> + ? ? ? int i, total = 0, tmp = 0;
> + ? ? ? const u8 loop = 5;
> + ? ? ? u32 consec_high = 0;
> +
> + ? ? ? struct imx6q_sensor_data *sd;
> +
> + ? ? ? sd = ((struct imx6q_thermal_zone *)thermal->devdata)->sensor_data;
> +
> + ? ? ? /*
> + ? ? ? ?* Measure temperature and handle noise
> + ? ? ? ?*
> + ? ? ? ?* While the imx6q temperature sensor is designed to minimize being
> + ? ? ? ?* affected by system noise, it's safest to run sanity checks and
> + ? ? ? ?* perform any necessary filitering.
> + ? ? ? ?*/
> + ? ? ? for (i = 0; (sd->noise_margin) && (i < loop); i++) {
> + ? ? ? ? ? ? ? imx6q_get_temp(&tmp, sd);
> +
> + ? ? ? ? ? ? ? if ((abs(tmp - sd->last_temp) <= sd->noise_margin) ||
> + ? ? ? ? ? ? ? ? ? ? ? (consec_high >= sd->consec_high_limit)) {
> + ? ? ? ? ? ? ? ? ? ? ? sd->last_temp = tmp;
> + ? ? ? ? ? ? ? ? ? ? ? i = 0;
> + ? ? ? ? ? ? ? ? ? ? ? break;
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? if (tmp > sd->last_temp)
> + ? ? ? ? ? ? ? ? ? ? ? consec_high++;
> +
> + ? ? ? ? ? ? ? /*
> + ? ? ? ? ? ? ? ?* ignore first measurement as the previous measurement was
> + ? ? ? ? ? ? ? ?* a long time ago.
> + ? ? ? ? ? ? ? ?*/
> + ? ? ? ? ? ? ? if (i)
> + ? ? ? ? ? ? ? ? ? ? ? total += tmp;
> +
> + ? ? ? ? ? ? ? sd->last_temp = tmp;
> + ? ? ? }
> +
> + ? ? ? if (sd->noise_margin && i)
> + ? ? ? ? ? ? ? tmp = total / (loop - 1);
> +
> + ? ? ? /*
> + ? ? ? ?* The thermal framework code stores temperature in unsigned long. Also,
> + ? ? ? ?* it has references to "millicelcius" which limits the lowest
> + ? ? ? ?* temperature possible (compared to Kelvin).
> + ? ? ? ?*/
> + ? ? ? if (tmp > 0)
> + ? ? ? ? ? ? ? *temp = tmp;
> + ? ? ? else
> + ? ? ? ? ? ? ? *temp = 0;
> +
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_get_mode(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? enum thermal_device_mode *mode)
> +{
> + ? ? ? *mode = THERMAL_DEVICE_ENABLED;
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?enum thermal_trip_type *type)
> +{
> + ? ? ? struct imx6q_thermal_data *p;
> +
> + ? ? ? if (trip >= thermal->trips)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> +
> + ? ? ? *type = p->type[trip];
> +
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_get_trip_temp(struct thermal_zone_device *thermal, int trip,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *temp)
> +{
> + ? ? ? struct imx6q_thermal_data *p;
> + ? ? ? enum thermal_trip_type type;
> +
> + ? ? ? if (trip >= thermal->trips)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> +
> + ? ? ? thermal->ops->get_trip_type(thermal, trip, &type);
> +
> + ? ? ? if (type == THERMAL_TRIP_CRITICAL)
> + ? ? ? ? ? ? ? *temp = p->crit_temp_level;
> + ? ? ? else
> + ? ? ? ? ? ? ? *temp = p->freq_tab[trip].temp_level;
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_get_crit_temp(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *temp)
> +{
> + ? ? ? struct imx6q_thermal_data *p;
> +
> + ? ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> + ? ? ? *temp = p->crit_temp_level;
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_bind(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? struct thermal_cooling_device *cdev)
> +{
> + ? ? ? int i;
> + ? ? ? struct thermal_cooling_device *cd;
> +
> + ? ? ? cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
> +
> + ? ? ? /* if the cooling device is the one from imx6 bind it */
> + ? ? ? if (cdev != cd)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
> + ? ? ? ? ? ? ? if (thermal_zone_bind_cooling_device(thermal, i, cdev)) {
> + ? ? ? ? ? ? ? ? ? ? ? pr_err("error binding cooling dev\n");
> + ? ? ? ? ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static int th_sys_unbind(struct thermal_zone_device *thermal,
> + ? ? ? ? ? ? ? ? ? ? ? ? struct thermal_cooling_device *cdev)
> +{
> + ? ? ? int i;
> +
> + ? ? ? struct thermal_cooling_device *cd;
> +
> + ? ? ? cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
> +
> + ? ? ? if (cdev != cd)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
> + ? ? ? ? ? ? ? if (thermal_zone_unbind_cooling_device(thermal, i, cdev)) {
> + ? ? ? ? ? ? ? ? ? ? ? pr_err("error unbinding cooling dev\n");
> + ? ? ? ? ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static struct thermal_zone_device_ops g_dev_ops __devinitdata = {
> + ? ? ? .bind = th_sys_bind,
> + ? ? ? .unbind = th_sys_unbind,
> + ? ? ? .get_temp = th_sys_get_temp,
> + ? ? ? .get_mode = th_sys_get_mode,
> + ? ? ? .get_trip_type = th_sys_get_trip_type,
> + ? ? ? .get_trip_temp = th_sys_get_trip_temp,
> + ? ? ? .get_crit_temp = th_sys_get_crit_temp,
> +};
> +
> +static int __devinit imx6q_thermal_process_fuse_data(unsigned int fuse_data,
> + ? ? ? ? ? ? ? ?struct imx6q_sensor_data *sd)
> +{
> + ? ? ? int t1, t2, n1, n2;
> +
> + ? ? ? if (fuse_data == 0 || fuse_data == 0xffffffff) {
> + ? ? ? ? ? ? ? pr_warn("WARNING: Incorrect temperature sensor value of %x "
> + ? ? ? ? ? ? ? ? ? ? ? "detected\n", fuse_data);
> +
> +#ifdef CONFIG_IMX6Q_THERMAL_FAKE_CALIBRATION
> + ? ? ? ? ? ? ? pr_warn(
> + ? ? ? ? ? ? ? ? ? ? ? "WARNING: Using fake calibration value of %x value. "
> + ? ? ? ? ? ? ? ? ? ? ? "This will cause your system performance to be limited "
> + ? ? ? ? ? ? ? ? ? ? ? "prematurely (compared to a using properly calibrated "
> + ? ? ? ? ? ? ? ? ? ? ? "value) OR will cause the system to allow the "
> + ? ? ? ? ? ? ? ? ? ? ? "temperature to exceed the maximum specified "
> + ? ? ? ? ? ? ? ? ? ? ? "temperature without the proper performance "
> + ? ? ? ? ? ? ? ? ? ? ? "limiting.\n", CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL);
> +
> + ? ? ? ? ? ? ? fuse_data = CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL;
> +#else
> + ? ? ? ? ? ? ? pr_warn("WARNING: Disabling CPU thermal protection\n");
> + ? ? ? ? ? ? ? return -EINVAL;
> +#endif
> + ? ? ? }
> +
> + ? ? ? /*
> + ? ? ? ?* Fuse data layout:
> + ? ? ? ?* [31:20] sensor value @ 25C
> + ? ? ? ?* [19:8] sensor value of hot
> + ? ? ? ?* [7:0] hot temperature value
> + ? ? ? ?*/
> + ? ? ? n1 = fuse_data >> 20;
> + ? ? ? n2 = (fuse_data & 0xfff00) >> 8;
> + ? ? ? t2 = fuse_data & 0xff;
> + ? ? ? t1 = 25; /* t1 always 25C */
> +
> + ? ? ? pr_debug(" -- temperature sensor calibration data --\n");
> + ? ? ? pr_debug("HW_OCOTP_ANA1: %x\n", fuse_data);
> + ? ? ? pr_debug("n1: %d\nn2: %d\nt1: %d\nt2: %d\n", n1, n2, t1, t2);
> +
> + ? ? ? /*
> + ? ? ? ?* Derived from linear interpolation,
> + ? ? ? ?* Tmeas = T2 + (Nmeas - N2) * (T1 - T2) / (N1 - N2)
> + ? ? ? ?* We want to reduce this down to the minimum computation necessary
> + ? ? ? ?* for each temperature read. ?Also, we want Tmeas in millicelcius
> + ? ? ? ?* and we don't want to lose precision from integer division. So...
> + ? ? ? ?* milli_Tmeas = 1000 * T2 + 1000 * (Nmeas - N2) * (T1 - T2) / (N1 - N2)
> + ? ? ? ?* Let constant c1 = 1000 * (T1 - T2) / (N1 - N2)
> + ? ? ? ?* milli_Tmeas = (1000 * T2) + c1 * (Nmeas - N2)
> + ? ? ? ?* milli_Tmeas = (1000 * T2) + (c1 * Nmeas) - (c1 * N2)
> + ? ? ? ?* Let constant c2 = (1000 * T2) - (c1 * N2)
> + ? ? ? ?* milli_Tmeas = c2 + (c1 * Nmeas)
> + ? ? ? ?*/
> + ? ? ? sd->c1 = (1000 * (t1 - t2)) / (n1 - n2);
> + ? ? ? sd->c2 = (1000 * t2) - (sd->c1 * n2);
> +
> + ? ? ? pr_debug("c1: %i\n", sd->c1);
> + ? ? ? pr_debug("c2: %i\n", sd->c2);
> +
> + ? ? ? return 0;
> +}
> +
> +static int anatop_thermal_suspend(struct platform_device *pdev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pm_message_t state)
> +{
> + ? ? ? struct imx6q_sensor_data *sd = pdev->dev.platform_data;
> +
> + ? ? ? /* power off the sensor during suspend */
> + ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + ? ? ? return 0;
> +}
> +
> +static int __devexit anatop_thermal_remove(struct platform_device *pdev)
> +{
> + ? ? ? if (th_zone && th_zone->therm_dev)
> + ? ? ? ? ? ? ? thermal_zone_device_unregister(th_zone->therm_dev);
> +
> + ? ? ? if (th_zone && th_zone->cool_dev)
> + ? ? ? ? ? ? ? cpufreq_cooling_unregister(th_zone->cool_dev);
> +
> + ? ? ? kfree(th_zone->sensor_data);
> + ? ? ? kfree(th_zone->thermal_data);
> + ? ? ? kfree(th_zone);
> +
> + ? ? ? if (ocotp_base)
> + ? ? ? ? ? ? ? iounmap(ocotp_base);
> +
> + ? ? ? pr_info("i.MX6Q: Kernel Thermal management unregistered\n");
> +
> + ? ? ? return 0;
> +}
> +
> +static int __devinit anatop_thermal_probe(struct platform_device *pdev)
> +{
> + ? ? ? struct device_node *np_ocotp, *np_thermal;
> + ? ? ? unsigned int fuse_data;
> + ? ? ? struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
> + ? ? ? int ret, i;
> +
> + ? ? ? np_ocotp = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
> + ? ? ? np_thermal = of_find_compatible_node(NULL, NULL, "fsl,anatop-thermal");
> +
> + ? ? ? if (!(np_ocotp && np_thermal && anatopmfd))
> + ? ? ? ? ? ? ? return -ENXIO; /* not a compatible platform */

Hmmm, I need to change this to return 0 (for multi platform builds).
Or change the intialization to be imx6q only.

> +
> + ? ? ? ocotp_base = of_iomap(np_ocotp, 0);
> +
> + ? ? ? if (!ocotp_base) {
> + ? ? ? ? ? ? ? pr_err("Could not retrieve ocotp-base\n");
> + ? ? ? ? ? ? ? ret = -ENXIO;
> + ? ? ? ? ? ? ? goto err_unregister;
> + ? ? ? }
> +
> + ? ? ? fuse_data = readl_relaxed(ocotp_base + HW_OCOTP_ANA1);
> +
> + ? ? ? th_zone = kzalloc(sizeof(struct imx6q_thermal_zone), GFP_KERNEL);
> + ? ? ? if (!th_zone) {
> + ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? goto err_unregister;
> + ? ? ? }
> +
> + ? ? ? th_zone->dev_ops = g_dev_ops;
> +
> + ? ? ? th_zone->thermal_data =
> + ? ? ? ? ? ? ? kzalloc(sizeof(struct imx6q_thermal_data), GFP_KERNEL);
> +
> + ? ? ? if (!th_zone->thermal_data) {
> + ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? goto err_unregister;
> + ? ? ? }
> +
> + ? ? ? memcpy(th_zone->thermal_data, &g_thermal_data,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct imx6q_thermal_data));
> +
> + ? ? ? for (i = 0; i < ARRAY_SIZE(g_thermal_data.freq_tab); i++)
> + ? ? ? ? ? ? ? th_zone->thermal_data->freq_tab[i].mask_val = cpumask_of(0);
> +
> +
> + ? ? ? th_zone->sensor_data =
> + ? ? ? ? ? ? ? kzalloc(sizeof(struct imx6q_sensor_data), GFP_KERNEL);
> +
> + ? ? ? if (!th_zone->sensor_data) {
> + ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? goto err_unregister;
> + ? ? ? }
> +
> + ? ? ? memcpy(th_zone->sensor_data, &g_sensor_data,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct imx6q_sensor_data));
> +
> + ? ? ? th_zone->sensor_data->anatopmfd = anatopmfd;
> +
> + ? ? ? ret = imx6q_thermal_process_fuse_data(fuse_data, th_zone->sensor_data);
> +
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? pr_err("Invalid temperature calibration data.\n");
> + ? ? ? ? ? ? ? goto err_unregister;
> + ? ? ? }
> +
> + ? ? ? if (!th_zone->sensor_data->meas_delay)
> + ? ? ? ? ? ? ? th_zone->sensor_data->meas_delay = 1;
> +
> + ? ? ? /* Make sure sensor is in known good state for measurements */
> + ? ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + ? ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + ? ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE1, 0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE1_MEASURE_FREQ);
> +
> + ? ? ? anatop_write_reg(anatopmfd, HW_ANADIG_ANA_MISC0_SET,
> + ? ? ? ? ? ? ? BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF,
> + ? ? ? ? ? ? ? BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF);
> +
> + ? ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + ? ? ? th_zone->cool_dev = cpufreq_cooling_register(
> + ? ? ? ? ? ? ? (struct freq_clip_table *)th_zone->thermal_data->freq_tab,
> + ? ? ? ? ? ? ? IMX6Q_THERMAL_ACTIVE_TRP_PTS);
> +
> + ? ? ? 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_data->name, IMX6Q_THERMAL_TOTAL_TRP_PTS,
> + ? ? ? ? ? ? ? th_zone, &th_zone->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;
> + ? ? ? }
> +
> + ? ? ? pdev->dev.platform_data = th_zone->sensor_data;
> +
> + ? ? ? return 0;
> +
> +err_unregister:
> + ? ? ? anatop_thermal_remove(pdev);
> + ? ? ? return ret;
> +}
> +
> +static struct of_device_id __devinitdata of_anatop_thermal_match_tbl[] = {
> + ? ? ? { .compatible = "fsl,anatop-thermal", },
> + ? ? ? { /* end */ }
> +};
> +
> +static struct platform_driver anatop_thermal = {
> + ? ? ? .driver = {
> + ? ? ? ? ? ? ? .name ? = "anatop_thermal",
> + ? ? ? ? ? ? ? .owner ?= THIS_MODULE,
> + ? ? ? ? ? ? ? .of_match_table = of_anatop_thermal_match_tbl,
> + ? ? ? },
> + ? ? ? .probe ?= anatop_thermal_probe,
> + ? ? ? .remove = anatop_thermal_remove,
> + ? ? ? .suspend = anatop_thermal_suspend,
> + ? ? ? /* due to implentation of imx6q_get_temp , resume is unnecessary */
> +};
> +
> +static int __devinit anatop_thermal_init(void)
> +{
> + ? ? ? return platform_driver_register(&anatop_thermal);
> +}
> +device_initcall(anatop_thermal_init);
> +
> +static void __exit anatop_thermal_exit(void)
> +{
> + ? ? ? platform_driver_unregister(&anatop_thermal);
> +}
> +module_exit(anatop_thermal_exit);
> +
> +MODULE_AUTHOR("Freescale Semiconductor");
> +MODULE_DESCRIPTION("i.MX6Q SoC thermal driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx6q-thermal");
> --
> 1.7.10
>

2012-06-20 10:11:50

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

On Wed, Jun 20, 2012 at 02:06:04AM -0500, Robert Lee wrote:
> Add imx6q cpu thermal management driver using the new cpu cooling
> interface which limits system performance via cpufreq to reduce
> the cpu temperature. Temperature readings are taken using
> the imx6q temperature sensor and this functionality was added
> as part of this cpu thermal management driver.
>
> Signed-off-by: Robert Lee <[email protected]>
> ---
> arch/arm/boot/dts/imx6q.dtsi | 5 +
> drivers/thermal/Kconfig | 28 ++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/imx6q_thermal.c | 593 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 627 insertions(+)
> create mode 100644 drivers/thermal/imx6q_thermal.c
>
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 8c90cba..2650f65 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -442,6 +442,10 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <1450000>;
> };
> +
> + thermal {
> + compatible ="fsl,anatop-thermal";
> + };
> };
>
> usbphy@020c9000 { /* USBPHY1 */
> @@ -659,6 +663,7 @@
> };
>
> ocotp@021bc000 {
> + compatible = "fsl,imx6q-ocotp";
> reg = <0x021bc000 0x4000>;
> };
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 04c6796..b80c408 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -30,6 +30,34 @@ config CPU_THERMAL
> 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 MFD_ANATOP && CPU_THERMAL
> + help
> + Adds thermal management for IMX6Q.
> +
> +config IMX6Q_THERMAL_FAKE_CALIBRATION
> + bool "IMX6Q fake temperature sensor calibration (FOR TESTING ONLY)"
> + depends on IMX6Q_THERMAL
> + help
> + This enables a fake temp sensor calibration value for parts without
> + the correction calibration values burned into OCOTP. If the part
> + already has the calibrated values burned into OCOTP, enabling this
> + does nothing.
> + WARNING: Use of this feature is for testing only as it will cause
> + incorrect temperature readings which will result in incorrect system
> + thermal limiting behavior such as premature system performance
> + limiting or lack of proper performance reduction to reduce cpu
> + temperature
> +
> +config IMX6Q_THERMAL_FAKE_CAL_VAL
> + hex "IMX6Q fake temperature sensor calibration value"
> + depends on IMX6Q_THERMAL_FAKE_CALIBRATION
> + default 0x5704c67d
> + help
> + Refer to the temperature sensor section of the imx6q reference manual
> + for more inforation on how this value is used.

Don't add such stuff to Kconfig. If during runtime you detect that there
is no calibration data, then issue a warning and fall back to a safe
value. If you really think this should be configurable, add a sysfs
entry for it. "FOR TESTING ONLY" seems to imply though that it shouldn't
be configurable.


> +
> config SPEAR_THERMAL
> bool "SPEAr thermal sensor driver"
> depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 4636e35..fc4004e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_THERMAL) += thermal_sys.o
> obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
> obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> obj-$(CONFIG_EXYNOS_THERMAL) += exynos_thermal.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..255d646
> --- /dev/null
> +++ b/drivers/thermal/imx6q_thermal.c
> @@ -0,0 +1,593 @@
> +/*
> + * 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/device.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>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/anatop.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 BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF 0x00000008
> +
> +#define HW_ANADIG_TEMPSENSE0 0x00000180
> +#define HW_ANADIG_TEMPSENSE0_SET 0x00000184
> +#define HW_ANADIG_TEMPSENSE0_CLR 0x00000188
> +#define HW_ANADIG_TEMPSENSE0_TOG 0x0000018c
> +
> +#define BP_ANADIG_TEMPSENSE0_TEMP_VALUE 8
> +#define BM_ANADIG_TEMPSENSE0_TEMP_VALUE 0x000FFF00
> +#define BM_ANADIG_TEMPSENSE0_FINISHED 0x00000004
> +#define BM_ANADIG_TEMPSENSE0_MEASURE_TEMP 0x00000002
> +#define BM_ANADIG_TEMPSENSE0_POWER_DOWN 0x00000001
> +
> +#define HW_ANADIG_TEMPSENSE1 0x00000190
> +#define HW_ANADIG_TEMPSENSE1_SET 0x00000194
> +#define HW_ANADIG_TEMPSENSE1_CLR 0x00000198
> +#define BP_ANADIG_TEMPSENSE1_MEASURE_FREQ 0
> +#define BM_ANADIG_TEMPSENSE1_MEASURE_FREQ 0x0000FFFF
> +
> +#define HW_OCOTP_ANA1 0x000004E0
> +
> +#define IMX6Q_THERMAL_POLLING_FREQUENCY_MS 1000
> +
> +#define IMX6Q_THERMAL_ACTIVE_TRP_PTS 3
> +/* assumption: always one critical trip point */
> +#define IMX6Q_THERMAL_TOTAL_TRP_PTS (IMX6Q_THERMAL_ACTIVE_TRP_PTS + 1)
> +
> +struct imx6q_sensor_data {
> + int c1, c2;
> + char name[16];
> + u8 meas_delay; /* in milliseconds */
> + u32 noise_margin; /* in millicelcius */
> + int last_temp; /* in millicelcius */

s/celcius/celsius/

> + /*
> + * When noise filtering, if consecutive measurements are each higher
> + * up to consec_high_limit times, assume changing temperature readings
> + * to be valid and not noise.
> + */
> + u32 consec_high_limit;
> + struct anatop *anatopmfd;
> +};
> +
> +struct imx6q_thermal_data {
> + enum thermal_trip_type type[IMX6Q_THERMAL_TOTAL_TRP_PTS];
> + struct freq_clip_table freq_tab[IMX6Q_THERMAL_ACTIVE_TRP_PTS];
> + unsigned int crit_temp_level;
> +};
> +
> +struct imx6q_thermal_zone {
> + struct thermal_zone_device *therm_dev;
> + struct thermal_cooling_device *cool_dev;
> + struct imx6q_sensor_data *sensor_data;
> + struct imx6q_thermal_data *thermal_data;
> + struct thermal_zone_device_ops dev_ops;
> +};
> +
> +static struct imx6q_sensor_data g_sensor_data __devinitdata = {
> + .name = "imx6q-temp_sens",
> + .meas_delay = 1, /* in milliseconds */

This is initialized here, once again in probe and never changed to any
other value.

> + .noise_margin = 3000,
> + .consec_high_limit = 3,
> +};

Also constant values. What's the purpose of collecting these in this
struct?

> +
> +/*
> + * This data defines the various temperature trip points that will trigger
> + * cooling action when crossed.
> + */
> +static struct imx6q_thermal_data g_thermal_data __devinitdata = {
> + .type[0] = THERMAL_TRIP_ACTIVE,
> + .freq_tab[0] = {
> + .freq_clip_max = 800 * 1000,
> + .temp_level = 85000,
> + },
> + .type[1] = THERMAL_TRIP_ACTIVE,
> + .freq_tab[1] = {
> + .freq_clip_max = 400 * 1000,
> + .temp_level = 90000,
> + },
> + .type[2] = THERMAL_TRIP_ACTIVE,
> + .freq_tab[2] = {
> + .freq_clip_max = 200 * 1000,
> + .temp_level = 95000,
> + },
> + .type[3] = THERMAL_TRIP_CRITICAL,
> + .crit_temp_level = 100000,
> +};
> +
> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
> + unsigned long *temp);
> +
> +static struct imx6q_thermal_zone *th_zone;
> +static void __iomem *ocotp_base;

This is a driver and drivers should generally be multi instance safe.

> +
> +static inline int imx6q_get_temp(int *temp, struct imx6q_sensor_data *sd)
> +{
> + unsigned int n_meas;
> + unsigned int reg;
> +
> + do {
> + /*
> + * Every time we measure the temperature, we will power on the
> + * temperature sensor, enable measurements, take a reading,
> + * disable measurements, power off the temperature sensor.
> + */
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
> + BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + /*
> + * According to imx6q temp sensor designers,
> + * it may require up to ~17us to complete
> + * a measurement. But this timing isn't checked on every part
> + * nor is it specified in the datasheet, so we check the
> + * 'finished' status bit to be sure of completion of a valid
> + * measurement.
> + */
> + msleep(sd->meas_delay);

The comment seems to have nothing to do with calling msleep, and it's
not clear to me why you have to put the argument to msleep into a
struct.

> +
> + reg = anatop_read_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0);
> +
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + } while (!(reg & BM_ANADIG_TEMPSENSE0_FINISHED) &&
> + (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE));

You should never solely depend on hardware to break out of loops.

> +
> + n_meas = (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
> + >> BP_ANADIG_TEMPSENSE0_TEMP_VALUE;
> +
> + /* See imx6q_thermal_process_fuse_data() for forumla derivation. */
> + *temp = sd->c2 + (sd->c1 * n_meas);
> +
> + pr_debug("Temperature: %d\n", *temp / 1000);
> +
> + return 0;
> +}
> +
> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
> + unsigned long *temp)
> +{
> + int i, total = 0, tmp = 0;
> + const u8 loop = 5;
> + u32 consec_high = 0;
> +
> + struct imx6q_sensor_data *sd;
> +
> + sd = ((struct imx6q_thermal_zone *)thermal->devdata)->sensor_data;
> +
> + /*
> + * Measure temperature and handle noise
> + *
> + * While the imx6q temperature sensor is designed to minimize being
> + * affected by system noise, it's safest to run sanity checks and
> + * perform any necessary filitering.

s/filitering/filtering/

> + */
> + for (i = 0; (sd->noise_margin) && (i < loop); i++) {
> + imx6q_get_temp(&tmp, sd);
> +
> + if ((abs(tmp - sd->last_temp) <= sd->noise_margin) ||
> + (consec_high >= sd->consec_high_limit)) {
> + sd->last_temp = tmp;
> + i = 0;
> + break;
> + }
> + if (tmp > sd->last_temp)
> + consec_high++;
> +
> + /*
> + * ignore first measurement as the previous measurement was
> + * a long time ago.
> + */
> + if (i)
> + total += tmp;
> +
> + sd->last_temp = tmp;
> + }
> +
> + if (sd->noise_margin && i)
> + tmp = total / (loop - 1);
> +
> + /*
> + * The thermal framework code stores temperature in unsigned long. Also,
> + * it has references to "millicelcius" which limits the lowest

s/celcius/celsius/

> + * temperature possible (compared to Kelvin).
> + */
> + if (tmp > 0)
> + *temp = tmp;
> + else
> + *temp = 0;
> +
> + return 0;
> +}
> +
> +static int th_sys_get_mode(struct thermal_zone_device *thermal,
> + enum thermal_device_mode *mode)
> +{
> + *mode = THERMAL_DEVICE_ENABLED;
> + return 0;
> +}
> +
> +static int th_sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
> + enum thermal_trip_type *type)
> +{
> + struct imx6q_thermal_data *p;
> +
> + if (trip >= thermal->trips)
> + return -EINVAL;
> +
> + p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> +
> + *type = p->type[trip];
> +
> + return 0;
> +}
> +
> +static int th_sys_get_trip_temp(struct thermal_zone_device *thermal, int trip,
> + unsigned long *temp)
> +{
> + struct imx6q_thermal_data *p;
> + enum thermal_trip_type type;
> +
> + if (trip >= thermal->trips)
> + return -EINVAL;
> +
> + p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> +
> + thermal->ops->get_trip_type(thermal, trip, &type);
> +
> + if (type == THERMAL_TRIP_CRITICAL)
> + *temp = p->crit_temp_level;
> + else
> + *temp = p->freq_tab[trip].temp_level;
> + return 0;
> +}
> +
> +static int th_sys_get_crit_temp(struct thermal_zone_device *thermal,
> + unsigned long *temp)
> +{
> + struct imx6q_thermal_data *p;
> +
> + p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
> + *temp = p->crit_temp_level;
> + return 0;
> +}
> +
> +static int th_sys_bind(struct thermal_zone_device *thermal,
> + struct thermal_cooling_device *cdev)
> +{
> + int i;
> + struct thermal_cooling_device *cd;
> +
> + cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
> +
> + /* if the cooling device is the one from imx6 bind it */
> + if (cdev != cd)
> + return 0;
> +
> + for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
> + if (thermal_zone_bind_cooling_device(thermal, i, cdev)) {
> + pr_err("error binding cooling dev\n");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int th_sys_unbind(struct thermal_zone_device *thermal,
> + struct thermal_cooling_device *cdev)
> +{
> + int i;
> +
> + struct thermal_cooling_device *cd;
> +
> + cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
> +
> + if (cdev != cd)
> + return 0;
> +
> + for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
> + if (thermal_zone_unbind_cooling_device(thermal, i, cdev)) {
> + pr_err("error unbinding cooling dev\n");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct thermal_zone_device_ops g_dev_ops __devinitdata = {
> + .bind = th_sys_bind,
> + .unbind = th_sys_unbind,
> + .get_temp = th_sys_get_temp,
> + .get_mode = th_sys_get_mode,
> + .get_trip_type = th_sys_get_trip_type,
> + .get_trip_temp = th_sys_get_trip_temp,
> + .get_crit_temp = th_sys_get_crit_temp,
> +};
> +
> +static int __devinit imx6q_thermal_process_fuse_data(unsigned int fuse_data,
> + struct imx6q_sensor_data *sd)
> +{
> + int t1, t2, n1, n2;
> +
> + if (fuse_data == 0 || fuse_data == 0xffffffff) {
> + pr_warn("WARNING: Incorrect temperature sensor value of %x "
> + "detected\n", fuse_data);
> +
> +#ifdef CONFIG_IMX6Q_THERMAL_FAKE_CALIBRATION
> + pr_warn(
> + "WARNING: Using fake calibration value of %x value. "
> + "This will cause your system performance to be limited "
> + "prematurely (compared to a using properly calibrated "
> + "value) OR will cause the system to allow the "
> + "temperature to exceed the maximum specified "
> + "temperature without the proper performance "
> + "limiting.\n", CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL);
> +
> + fuse_data = CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL;
> +#else
> + pr_warn("WARNING: Disabling CPU thermal protection\n");
> + return -EINVAL;
> +#endif
> + }
> +
> + /*
> + * Fuse data layout:
> + * [31:20] sensor value @ 25C
> + * [19:8] sensor value of hot
> + * [7:0] hot temperature value
> + */
> + n1 = fuse_data >> 20;
> + n2 = (fuse_data & 0xfff00) >> 8;
> + t2 = fuse_data & 0xff;
> + t1 = 25; /* t1 always 25C */
> +
> + pr_debug(" -- temperature sensor calibration data --\n");
> + pr_debug("HW_OCOTP_ANA1: %x\n", fuse_data);
> + pr_debug("n1: %d\nn2: %d\nt1: %d\nt2: %d\n", n1, n2, t1, t2);
> +
> + /*
> + * Derived from linear interpolation,
> + * Tmeas = T2 + (Nmeas - N2) * (T1 - T2) / (N1 - N2)
> + * We want to reduce this down to the minimum computation necessary
> + * for each temperature read. Also, we want Tmeas in millicelcius
> + * and we don't want to lose precision from integer division. So...
> + * milli_Tmeas = 1000 * T2 + 1000 * (Nmeas - N2) * (T1 - T2) / (N1 - N2)
> + * Let constant c1 = 1000 * (T1 - T2) / (N1 - N2)
> + * milli_Tmeas = (1000 * T2) + c1 * (Nmeas - N2)
> + * milli_Tmeas = (1000 * T2) + (c1 * Nmeas) - (c1 * N2)
> + * Let constant c2 = (1000 * T2) - (c1 * N2)
> + * milli_Tmeas = c2 + (c1 * Nmeas)
> + */
> + sd->c1 = (1000 * (t1 - t2)) / (n1 - n2);
> + sd->c2 = (1000 * t2) - (sd->c1 * n2);
> +
> + pr_debug("c1: %i\n", sd->c1);
> + pr_debug("c2: %i\n", sd->c2);
> +
> + return 0;
> +}
> +
> +static int anatop_thermal_suspend(struct platform_device *pdev,
> + pm_message_t state)
> +{
> + struct imx6q_sensor_data *sd = pdev->dev.platform_data;
> +
> + /* power off the sensor during suspend */
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + return 0;
> +}
> +
> +static int __devexit anatop_thermal_remove(struct platform_device *pdev)
> +{
> + if (th_zone && th_zone->therm_dev)
> + thermal_zone_device_unregister(th_zone->therm_dev);
> +
> + if (th_zone && th_zone->cool_dev)
> + cpufreq_cooling_unregister(th_zone->cool_dev);
> +
> + kfree(th_zone->sensor_data);
> + kfree(th_zone->thermal_data);
> + kfree(th_zone);
> +
> + if (ocotp_base)
> + iounmap(ocotp_base);
> +
> + pr_info("i.MX6Q: Kernel Thermal management unregistered\n");
> +
> + return 0;
> +}
> +
> +static int __devinit anatop_thermal_probe(struct platform_device *pdev)
> +{
> + struct device_node *np_ocotp, *np_thermal;
> + unsigned int fuse_data;
> + struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
> + int ret, i;
> +
> + np_ocotp = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
> + np_thermal = of_find_compatible_node(NULL, NULL, "fsl,anatop-thermal");

np_thermal = pdev->dev.of_node;

> +
> + if (!(np_ocotp && np_thermal && anatopmfd))
> + return -ENXIO; /* not a compatible platform */
> +
> + ocotp_base = of_iomap(np_ocotp, 0);
> +
> + if (!ocotp_base) {
> + pr_err("Could not retrieve ocotp-base\n");
> + ret = -ENXIO;
> + goto err_unregister;
> + }
> +
> + fuse_data = readl_relaxed(ocotp_base + HW_OCOTP_ANA1);
> +
> + th_zone = kzalloc(sizeof(struct imx6q_thermal_zone), GFP_KERNEL);

consider using devm_*

> + if (!th_zone) {
> + ret = -ENOMEM;
> + goto err_unregister;
> + }
> +
> + th_zone->dev_ops = g_dev_ops;
> +
> + th_zone->thermal_data =
> + kzalloc(sizeof(struct imx6q_thermal_data), GFP_KERNEL);

Why do you allocate this seperately? You could embed a struct
imx6q_thermal_data in struct imx6q_thermal_zone.

> +
> + if (!th_zone->thermal_data) {
> + ret = -ENOMEM;
> + goto err_unregister;
> + }
> +
> + memcpy(th_zone->thermal_data, &g_thermal_data,
> + sizeof(struct imx6q_thermal_data));
> +
> + for (i = 0; i < ARRAY_SIZE(g_thermal_data.freq_tab); i++)
> + th_zone->thermal_data->freq_tab[i].mask_val = cpumask_of(0);
> +
> +
> + th_zone->sensor_data =
> + kzalloc(sizeof(struct imx6q_sensor_data), GFP_KERNEL);

This one also could be embedded in struct imx6q_thermal_zone.

> +
> + if (!th_zone->sensor_data) {
> + ret = -ENOMEM;
> + goto err_unregister;
> + }
> +
> + memcpy(th_zone->sensor_data, &g_sensor_data,
> + sizeof(struct imx6q_sensor_data));
> +
> + th_zone->sensor_data->anatopmfd = anatopmfd;
> +
> + ret = imx6q_thermal_process_fuse_data(fuse_data, th_zone->sensor_data);
> +
> + if (ret) {
> + pr_err("Invalid temperature calibration data.\n");

use dev_* functions for logging throughout the driver.

> + goto err_unregister;
> + }
> +
> + if (!th_zone->sensor_data->meas_delay)
> + th_zone->sensor_data->meas_delay = 1;
> +
> + /* Make sure sensor is in known good state for measurements */
> + anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
> + BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
> +
> + anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE1, 0,
> + BM_ANADIG_TEMPSENSE1_MEASURE_FREQ);
> +
> + anatop_write_reg(anatopmfd, HW_ANADIG_ANA_MISC0_SET,
> + BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF,
> + BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF);
> +
> + anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN,
> + BM_ANADIG_TEMPSENSE0_POWER_DOWN);
> +
> + th_zone->cool_dev = cpufreq_cooling_register(
> + (struct freq_clip_table *)th_zone->thermal_data->freq_tab,
> + IMX6Q_THERMAL_ACTIVE_TRP_PTS);
> +
> + 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_data->name, IMX6Q_THERMAL_TOTAL_TRP_PTS,
> + th_zone, &th_zone->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;
> + }
> +
> + pdev->dev.platform_data = th_zone->sensor_data;

platform_data is for passing information from the one who registers the
platform_device to the driver. What you have to use here is
platform_set_drvdata(). Also you have to initialize this before
registering the thermal zone, not afterwards.

> +
> + return 0;
> +
> +err_unregister:
> + anatop_thermal_remove(pdev);
> + return ret;
> +}
> +
> +static struct of_device_id __devinitdata of_anatop_thermal_match_tbl[] = {
> + { .compatible = "fsl,anatop-thermal", },
> + { /* end */ }
> +};
> +
> +static struct platform_driver anatop_thermal = {
> + .driver = {
> + .name = "anatop_thermal",
> + .owner = THIS_MODULE,
> + .of_match_table = of_anatop_thermal_match_tbl,
> + },
> + .probe = anatop_thermal_probe,
> + .remove = anatop_thermal_remove,
> + .suspend = anatop_thermal_suspend,
> + /* due to implentation of imx6q_get_temp , resume is unnecessary */
> +};
> +
> +static int __devinit anatop_thermal_init(void)
> +{
> + return platform_driver_register(&anatop_thermal);
> +}
> +device_initcall(anatop_thermal_init);
> +
> +static void __exit anatop_thermal_exit(void)
> +{
> + platform_driver_unregister(&anatop_thermal);
> +}
> +module_exit(anatop_thermal_exit);

use module_platform_driver

> +
> +MODULE_AUTHOR("Freescale Semiconductor");
> +MODULE_DESCRIPTION("i.MX6Q SoC thermal driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx6q-thermal");
> --
> 1.7.10
>
>

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

2012-06-20 14:13:36

by Rob Lee

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

Sascha, thanks for the review.

On Wed, Jun 20, 2012 at 5:11 AM, Sascha Hauer <[email protected]> wrote:
> On Wed, Jun 20, 2012 at 02:06:04AM -0500, Robert Lee wrote:
>> Add imx6q cpu thermal management driver using the new cpu cooling
>> interface which limits system performance via cpufreq to reduce
>> the cpu temperature. ?Temperature readings are taken using
>> the imx6q temperature sensor and this functionality was added
>> as part of this cpu thermal management driver.
>>
>> Signed-off-by: Robert Lee <[email protected]>
>> ---
>> ?arch/arm/boot/dts/imx6q.dtsi ? ?| ? ?5 +
>> ?drivers/thermal/Kconfig ? ? ? ? | ? 28 ++
>> ?drivers/thermal/Makefile ? ? ? ?| ? ?1 +
>> ?drivers/thermal/imx6q_thermal.c | ?593 +++++++++++++++++++++++++++++++++++++++
>> ?4 files changed, 627 insertions(+)
>> ?create mode 100644 drivers/thermal/imx6q_thermal.c
>>
>> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
>> index 8c90cba..2650f65 100644
>> --- a/arch/arm/boot/dts/imx6q.dtsi
>> +++ b/arch/arm/boot/dts/imx6q.dtsi
>> @@ -442,6 +442,10 @@
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? anatop-min-voltage = <725000>;
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? anatop-max-voltage = <1450000>;
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? };
>> +
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? thermal {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible ="fsl,anatop-thermal";
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? };
>> ? ? ? ? ? ? ? ? ? ? ? };
>>
>> ? ? ? ? ? ? ? ? ? ? ? usbphy@020c9000 { /* USBPHY1 */
>> @@ -659,6 +663,7 @@
>> ? ? ? ? ? ? ? ? ? ? ? };
>>
>> ? ? ? ? ? ? ? ? ? ? ? ocotp@021bc000 {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? compatible = "fsl,imx6q-ocotp";
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg = <0x021bc000 0x4000>;
>> ? ? ? ? ? ? ? ? ? ? ? };
>>
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 04c6796..b80c408 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -30,6 +30,34 @@ config CPU_THERMAL
>> ? ? ? ? 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 MFD_ANATOP && CPU_THERMAL
>> + ? ? help
>> + ? ? ? Adds thermal management for IMX6Q.
>> +
>> +config IMX6Q_THERMAL_FAKE_CALIBRATION
>> + ? ? bool "IMX6Q fake temperature sensor calibration (FOR TESTING ONLY)"
>> + ? ? depends on IMX6Q_THERMAL
>> + ? ? help
>> + ? ? ? This enables a fake temp sensor calibration value for parts without
>> + ? ? ? the correction calibration values burned into OCOTP. If the part
>> + ? ? ? already has the calibrated values burned into OCOTP, enabling this
>> + ? ? ? does nothing.
>> + ? ? ? WARNING: Use of this feature is for testing only as it will cause
>> + ? ? ? incorrect temperature readings which will result in incorrect system
>> + ? ? ? thermal limiting behavior such as premature system performance
>> + ? ? ? limiting or lack of proper performance reduction to reduce cpu
>> + ? ? ? temperature
>> +
>> +config IMX6Q_THERMAL_FAKE_CAL_VAL
>> + ? ? hex "IMX6Q fake temperature sensor calibration value"
>> + ? ? depends on IMX6Q_THERMAL_FAKE_CALIBRATION
>> + ? ? default 0x5704c67d
>> + ? ? help
>> + ? ? ? Refer to the temperature sensor section of the imx6q reference manual
>> + ? ? ? for more inforation on how this value is used.
>
> Don't add such stuff to Kconfig. If during runtime you detect that there
> is no calibration data, then issue a warning and fall back to a safe
> value. If you really think this should be configurable, add a sysfs
> entry for it. "FOR TESTING ONLY" seems to imply though that it shouldn't
> be configurable.
>

Ok, I'll remove this in v6.

>
>> +
>> ?config SPEAR_THERMAL
>> ? ? ? bool "SPEAr thermal sensor driver"
>> ? ? ? depends on THERMAL
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index 4636e35..fc4004e 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -6,3 +6,4 @@ obj-$(CONFIG_THERMAL) ? ? ? ? += thermal_sys.o
>> ?obj-$(CONFIG_CPU_THERMAL) ? ? ? += cpu_cooling.o
>> ?obj-$(CONFIG_SPEAR_THERMAL) ? ? ? ? ?+= spear_thermal.o
>> ?obj-$(CONFIG_EXYNOS_THERMAL) ? ? ? ? += exynos_thermal.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..255d646
>> --- /dev/null
>> +++ b/drivers/thermal/imx6q_thermal.c
>> @@ -0,0 +1,593 @@
>> +/*
>> + * 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/device.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>
>> +#include <linux/platform_device.h>
>> +#include <linux/mfd/anatop.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 BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF ? ? ? ?0x00000008
>> +
>> +#define HW_ANADIG_TEMPSENSE0 ? ? ? ? ? ? ? ? 0x00000180
>> +#define HW_ANADIG_TEMPSENSE0_SET ? ? ? ? ? ? 0x00000184
>> +#define HW_ANADIG_TEMPSENSE0_CLR ? ? ? ? ? ? 0x00000188
>> +#define HW_ANADIG_TEMPSENSE0_TOG ? ? ? ? ? ? 0x0000018c
>> +
>> +#define BP_ANADIG_TEMPSENSE0_TEMP_VALUE ? ? ? ? ? ? ?8
>> +#define BM_ANADIG_TEMPSENSE0_TEMP_VALUE ? ? ? ? ? ? ?0x000FFF00
>> +#define BM_ANADIG_TEMPSENSE0_FINISHED ? ? ? ? ? ? ? ?0x00000004
>> +#define BM_ANADIG_TEMPSENSE0_MEASURE_TEMP ? ?0x00000002
>> +#define BM_ANADIG_TEMPSENSE0_POWER_DOWN ? ? ? ? ? ? ?0x00000001
>> +
>> +#define HW_ANADIG_TEMPSENSE1 ? ? ? ? ? ? ? ? 0x00000190
>> +#define HW_ANADIG_TEMPSENSE1_SET ? ? ? ? ? ? 0x00000194
>> +#define HW_ANADIG_TEMPSENSE1_CLR ? ? ? ? ? ? 0x00000198
>> +#define BP_ANADIG_TEMPSENSE1_MEASURE_FREQ ? ?0
>> +#define BM_ANADIG_TEMPSENSE1_MEASURE_FREQ ? ?0x0000FFFF
>> +
>> +#define HW_OCOTP_ANA1 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0x000004E0
>> +
>> +#define IMX6Q_THERMAL_POLLING_FREQUENCY_MS 1000
>> +
>> +#define IMX6Q_THERMAL_ACTIVE_TRP_PTS 3
>> +/* assumption: always one critical trip point */
>> +#define IMX6Q_THERMAL_TOTAL_TRP_PTS (IMX6Q_THERMAL_ACTIVE_TRP_PTS + 1)
>> +
>> +struct imx6q_sensor_data {
>> + ? ? int ? ? c1, c2;
>> + ? ? char ? ?name[16];
>> + ? ? u8 ? ? ?meas_delay; ? ? /* in milliseconds */
>> + ? ? u32 ? ? noise_margin; ? /* in millicelcius */
>> + ? ? int ? ? last_temp; ? ? ?/* in millicelcius */
>
> s/celcius/celsius/
>

Oops, will fix this in v6.

>> + ? ? /*
>> + ? ? ?* When noise filtering, if consecutive measurements are each higher
>> + ? ? ?* up to consec_high_limit times, assume changing temperature readings
>> + ? ? ?* to be valid and not noise.
>> + ? ? ?*/
>> + ? ? u32 ? ? consec_high_limit;
>> + ? ? struct anatop *anatopmfd;
>> +};
>> +
>> +struct imx6q_thermal_data {
>> + ? ? enum thermal_trip_type type[IMX6Q_THERMAL_TOTAL_TRP_PTS];
>> + ? ? struct freq_clip_table freq_tab[IMX6Q_THERMAL_ACTIVE_TRP_PTS];
>> + ? ? unsigned int crit_temp_level;
>> +};
>> +
>> +struct imx6q_thermal_zone {
>> + ? ? struct thermal_zone_device ? ? ?*therm_dev;
>> + ? ? struct thermal_cooling_device ? *cool_dev;
>> + ? ? struct imx6q_sensor_data ? ? ? ?*sensor_data;
>> + ? ? struct imx6q_thermal_data ? ? ? *thermal_data;
>> + ? ? struct thermal_zone_device_ops ?dev_ops;
>> +};
>> +
>> +static struct imx6q_sensor_data g_sensor_data __devinitdata = {
>> + ? ? .name ? ? ? ? ? ? ? ? ? = "imx6q-temp_sens",
>> + ? ? .meas_delay ? ? ? ? ? ? = 1, ? ?/* in milliseconds */
>
> This is initialized here, once again in probe and never changed to any
> other value.
>
>> + ? ? .noise_margin ? ? ? ? ? = 3000,
>> + ? ? .consec_high_limit ? ? ?= 3,
>> +};
>
> Also constant values. What's the purpose of collecting these in this
> struct?
>

There's not a good purpose for doing it this way now. It came from a
very early implementation that ended up changing before submitting.
I'll fix this in v6.

>> +
>> +/*
>> + * This data defines the various temperature trip points that will trigger
>> + * cooling action when crossed.
>> + */
>> +static struct imx6q_thermal_data g_thermal_data __devinitdata = {
>> + ? ? .type[0] = THERMAL_TRIP_ACTIVE,
>> + ? ? .freq_tab[0] = {
>> + ? ? ? ? ? ? .freq_clip_max = 800 * 1000,
>> + ? ? ? ? ? ? .temp_level = 85000,
>> + ? ? },
>> + ? ? .type[1] = THERMAL_TRIP_ACTIVE,
>> + ? ? .freq_tab[1] = {
>> + ? ? ? ? ? ? .freq_clip_max = 400 * 1000,
>> + ? ? ? ? ? ? .temp_level = 90000,
>> + ? ? },
>> + ? ? .type[2] = THERMAL_TRIP_ACTIVE,
>> + ? ? .freq_tab[2] = {
>> + ? ? ? ? ? ? .freq_clip_max = 200 * 1000,
>> + ? ? ? ? ? ? .temp_level = 95000,
>> + ? ? },
>> + ? ? .type[3] = THERMAL_TRIP_CRITICAL,
>> + ? ? .crit_temp_level = 100000,
>> +};
>> +
>> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *temp);
>> +
>> +static struct imx6q_thermal_zone ? ? *th_zone;
>> +static void __iomem ? ? ? ? ? ? ? ? ?*ocotp_base;
>
> This is a driver and drivers should generally be multi instance safe.
>

I don't understand what this comment is referring to. Could you elaborate?

>> +
>> +static inline int imx6q_get_temp(int *temp, struct imx6q_sensor_data *sd)
>> +{
>> + ? ? unsigned int n_meas;
>> + ? ? unsigned int reg;
>> +
>> + ? ? do {
>> + ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ?* Every time we measure the temperature, we will power on the
>> + ? ? ? ? ? ? ?* temperature sensor, enable measurements, take a reading,
>> + ? ? ? ? ? ? ?* disable measurements, power off the temperature sensor.
>> + ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
>> +
>> + ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
>> +
>> + ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ?* According to imx6q temp sensor designers,
>> + ? ? ? ? ? ? ?* it may require up to ~17us to complete
>> + ? ? ? ? ? ? ?* a measurement. ?But this timing isn't checked on every part
>> + ? ? ? ? ? ? ?* nor is it specified in the datasheet, so we check the
>> + ? ? ? ? ? ? ?* 'finished' status bit to be sure of completion of a valid
>> + ? ? ? ? ? ? ?* measurement.
>> + ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? msleep(sd->meas_delay);
>
> The comment seems to have nothing to do with calling msleep, and it's
> not clear to me why you have to put the argument to msleep into a
> struct.
>

Agree, it should move below to where we actually check the status bit.
There isn't a good reason why the argument must be in the sensor data
struct in the current implementation. I will fix these issues in v6.

>> +
>> + ? ? ? ? ? ? reg = anatop_read_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0);
>> +
>> + ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
>> +
>> + ? ? ? ? ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
>> + ? ? ? ? ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
>> +
>> + ? ? } while (!(reg & BM_ANADIG_TEMPSENSE0_FINISHED) &&
>> + ? ? ? ? ? ? ?(reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE));
>
> You should never solely depend on hardware to break out of loops.
>

Ok, I will re-work the implementation to not require this.

>> +
>> + ? ? n_meas = (reg & BM_ANADIG_TEMPSENSE0_TEMP_VALUE)
>> + ? ? ? ? ? ? ? ? ? ? >> BP_ANADIG_TEMPSENSE0_TEMP_VALUE;
>> +
>> + ? ? /* See imx6q_thermal_process_fuse_data() for forumla derivation. */
>> + ? ? *temp = sd->c2 + (sd->c1 * n_meas);
>> +
>> + ? ? pr_debug("Temperature: %d\n", *temp / 1000);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_get_temp(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned long *temp)
>> +{
>> + ? ? int i, total = 0, tmp = 0;
>> + ? ? const u8 loop = 5;
>> + ? ? u32 consec_high = 0;
>> +
>> + ? ? struct imx6q_sensor_data *sd;
>> +
>> + ? ? sd = ((struct imx6q_thermal_zone *)thermal->devdata)->sensor_data;
>> +
>> + ? ? /*
>> + ? ? ?* Measure temperature and handle noise
>> + ? ? ?*
>> + ? ? ?* While the imx6q temperature sensor is designed to minimize being
>> + ? ? ?* affected by system noise, it's safest to run sanity checks and
>> + ? ? ?* perform any necessary filitering.
>
> s/filitering/filtering/
>

Oops, will fix in v6.

>> + ? ? ?*/
>> + ? ? for (i = 0; (sd->noise_margin) && (i < loop); i++) {
>> + ? ? ? ? ? ? imx6q_get_temp(&tmp, sd);
>> +
>> + ? ? ? ? ? ? if ((abs(tmp - sd->last_temp) <= sd->noise_margin) ||
>> + ? ? ? ? ? ? ? ? ? ? (consec_high >= sd->consec_high_limit)) {
>> + ? ? ? ? ? ? ? ? ? ? sd->last_temp = tmp;
>> + ? ? ? ? ? ? ? ? ? ? i = 0;
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? if (tmp > sd->last_temp)
>> + ? ? ? ? ? ? ? ? ? ? consec_high++;
>> +
>> + ? ? ? ? ? ? /*
>> + ? ? ? ? ? ? ?* ignore first measurement as the previous measurement was
>> + ? ? ? ? ? ? ?* a long time ago.
>> + ? ? ? ? ? ? ?*/
>> + ? ? ? ? ? ? if (i)
>> + ? ? ? ? ? ? ? ? ? ? total += tmp;
>> +
>> + ? ? ? ? ? ? sd->last_temp = tmp;
>> + ? ? }
>> +
>> + ? ? if (sd->noise_margin && i)
>> + ? ? ? ? ? ? tmp = total / (loop - 1);
>> +
>> + ? ? /*
>> + ? ? ?* The thermal framework code stores temperature in unsigned long. Also,
>> + ? ? ?* it has references to "millicelcius" which limits the lowest
>
> s/celcius/celsius/
>

Oops, will fix in v6.

>> + ? ? ?* temperature possible (compared to Kelvin).
>> + ? ? ?*/
>> + ? ? if (tmp > 0)
>> + ? ? ? ? ? ? *temp = tmp;
>> + ? ? else
>> + ? ? ? ? ? ? *temp = 0;
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_get_mode(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? ? ? enum thermal_device_mode *mode)
>> +{
>> + ? ? *mode = THERMAL_DEVICE_ENABLED;
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?enum thermal_trip_type *type)
>> +{
>> + ? ? struct imx6q_thermal_data *p;
>> +
>> + ? ? if (trip >= thermal->trips)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
>> +
>> + ? ? *type = p->type[trip];
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *temp)
>> +{
>> + ? ? struct imx6q_thermal_data *p;
>> + ? ? enum thermal_trip_type type;
>> +
>> + ? ? if (trip >= thermal->trips)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
>> +
>> + ? ? thermal->ops->get_trip_type(thermal, trip, &type);
>> +
>> + ? ? if (type == THERMAL_TRIP_CRITICAL)
>> + ? ? ? ? ? ? *temp = p->crit_temp_level;
>> + ? ? else
>> + ? ? ? ? ? ? *temp = p->freq_tab[trip].temp_level;
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_get_crit_temp(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long *temp)
>> +{
>> + ? ? struct imx6q_thermal_data *p;
>> +
>> + ? ? p = ((struct imx6q_thermal_zone *)thermal->devdata)->thermal_data;
>> + ? ? *temp = p->crit_temp_level;
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_bind(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? struct thermal_cooling_device *cdev)
>> +{
>> + ? ? int i;
>> + ? ? struct thermal_cooling_device *cd;
>> +
>> + ? ? cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
>> +
>> + ? ? /* if the cooling device is the one from imx6 bind it */
>> + ? ? if (cdev != cd)
>> + ? ? ? ? ? ? return 0;
>> +
>> + ? ? for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
>> + ? ? ? ? ? ? if (thermal_zone_bind_cooling_device(thermal, i, cdev)) {
>> + ? ? ? ? ? ? ? ? ? ? pr_err("error binding cooling dev\n");
>> + ? ? ? ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int th_sys_unbind(struct thermal_zone_device *thermal,
>> + ? ? ? ? ? ? ? ? ? ? ? struct thermal_cooling_device *cdev)
>> +{
>> + ? ? int i;
>> +
>> + ? ? struct thermal_cooling_device *cd;
>> +
>> + ? ? cd = ((struct imx6q_thermal_zone *)thermal->devdata)->cool_dev;
>> +
>> + ? ? if (cdev != cd)
>> + ? ? ? ? ? ? return 0;
>> +
>> + ? ? for (i = 0; i < IMX6Q_THERMAL_ACTIVE_TRP_PTS; i++) {
>> + ? ? ? ? ? ? if (thermal_zone_unbind_cooling_device(thermal, i, cdev)) {
>> + ? ? ? ? ? ? ? ? ? ? pr_err("error unbinding cooling dev\n");
>> + ? ? ? ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static struct thermal_zone_device_ops g_dev_ops __devinitdata = {
>> + ? ? .bind = th_sys_bind,
>> + ? ? .unbind = th_sys_unbind,
>> + ? ? .get_temp = th_sys_get_temp,
>> + ? ? .get_mode = th_sys_get_mode,
>> + ? ? .get_trip_type = th_sys_get_trip_type,
>> + ? ? .get_trip_temp = th_sys_get_trip_temp,
>> + ? ? .get_crit_temp = th_sys_get_crit_temp,
>> +};
>> +
>> +static int __devinit imx6q_thermal_process_fuse_data(unsigned int fuse_data,
>> + ? ? ? ? ? ? ?struct imx6q_sensor_data *sd)
>> +{
>> + ? ? int t1, t2, n1, n2;
>> +
>> + ? ? if (fuse_data == 0 || fuse_data == 0xffffffff) {
>> + ? ? ? ? ? ? pr_warn("WARNING: Incorrect temperature sensor value of %x "
>> + ? ? ? ? ? ? ? ? ? ? "detected\n", fuse_data);
>> +
>> +#ifdef CONFIG_IMX6Q_THERMAL_FAKE_CALIBRATION
>> + ? ? ? ? ? ? pr_warn(
>> + ? ? ? ? ? ? ? ? ? ? "WARNING: Using fake calibration value of %x value. "
>> + ? ? ? ? ? ? ? ? ? ? "This will cause your system performance to be limited "
>> + ? ? ? ? ? ? ? ? ? ? "prematurely (compared to a using properly calibrated "
>> + ? ? ? ? ? ? ? ? ? ? "value) OR will cause the system to allow the "
>> + ? ? ? ? ? ? ? ? ? ? "temperature to exceed the maximum specified "
>> + ? ? ? ? ? ? ? ? ? ? "temperature without the proper performance "
>> + ? ? ? ? ? ? ? ? ? ? "limiting.\n", CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL);
>> +
>> + ? ? ? ? ? ? fuse_data = CONFIG_IMX6Q_THERMAL_FAKE_CAL_VAL;
>> +#else
>> + ? ? ? ? ? ? pr_warn("WARNING: Disabling CPU thermal protection\n");
>> + ? ? ? ? ? ? return -EINVAL;
>> +#endif
>> + ? ? }
>> +
>> + ? ? /*
>> + ? ? ?* Fuse data layout:
>> + ? ? ?* [31:20] sensor value @ 25C
>> + ? ? ?* [19:8] sensor value of hot
>> + ? ? ?* [7:0] hot temperature value
>> + ? ? ?*/
>> + ? ? n1 = fuse_data >> 20;
>> + ? ? n2 = (fuse_data & 0xfff00) >> 8;
>> + ? ? t2 = fuse_data & 0xff;
>> + ? ? t1 = 25; /* t1 always 25C */
>> +
>> + ? ? pr_debug(" -- temperature sensor calibration data --\n");
>> + ? ? pr_debug("HW_OCOTP_ANA1: %x\n", fuse_data);
>> + ? ? pr_debug("n1: %d\nn2: %d\nt1: %d\nt2: %d\n", n1, n2, t1, t2);
>> +
>> + ? ? /*
>> + ? ? ?* Derived from linear interpolation,
>> + ? ? ?* Tmeas = T2 + (Nmeas - N2) * (T1 - T2) / (N1 - N2)
>> + ? ? ?* We want to reduce this down to the minimum computation necessary
>> + ? ? ?* for each temperature read. ?Also, we want Tmeas in millicelcius
>> + ? ? ?* and we don't want to lose precision from integer division. So...
>> + ? ? ?* milli_Tmeas = 1000 * T2 + 1000 * (Nmeas - N2) * (T1 - T2) / (N1 - N2)
>> + ? ? ?* Let constant c1 = 1000 * (T1 - T2) / (N1 - N2)
>> + ? ? ?* milli_Tmeas = (1000 * T2) + c1 * (Nmeas - N2)
>> + ? ? ?* milli_Tmeas = (1000 * T2) + (c1 * Nmeas) - (c1 * N2)
>> + ? ? ?* Let constant c2 = (1000 * T2) - (c1 * N2)
>> + ? ? ?* milli_Tmeas = c2 + (c1 * Nmeas)
>> + ? ? ?*/
>> + ? ? sd->c1 = (1000 * (t1 - t2)) / (n1 - n2);
>> + ? ? sd->c2 = (1000 * t2) - (sd->c1 * n2);
>> +
>> + ? ? pr_debug("c1: %i\n", sd->c1);
>> + ? ? pr_debug("c2: %i\n", sd->c2);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int anatop_thermal_suspend(struct platform_device *pdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pm_message_t state)
>> +{
>> + ? ? struct imx6q_sensor_data *sd = pdev->dev.platform_data;
>> +
>> + ? ? /* power off the sensor during suspend */
>> + ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
>> +
>> + ? ? anatop_write_reg(sd->anatopmfd, HW_ANADIG_TEMPSENSE0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int __devexit anatop_thermal_remove(struct platform_device *pdev)
>> +{
>> + ? ? if (th_zone && th_zone->therm_dev)
>> + ? ? ? ? ? ? thermal_zone_device_unregister(th_zone->therm_dev);
>> +
>> + ? ? if (th_zone && th_zone->cool_dev)
>> + ? ? ? ? ? ? cpufreq_cooling_unregister(th_zone->cool_dev);
>> +
>> + ? ? kfree(th_zone->sensor_data);
>> + ? ? kfree(th_zone->thermal_data);
>> + ? ? kfree(th_zone);
>> +
>> + ? ? if (ocotp_base)
>> + ? ? ? ? ? ? iounmap(ocotp_base);
>> +
>> + ? ? pr_info("i.MX6Q: Kernel Thermal management unregistered\n");
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static int __devinit anatop_thermal_probe(struct platform_device *pdev)
>> +{
>> + ? ? struct device_node *np_ocotp, *np_thermal;
>> + ? ? unsigned int fuse_data;
>> + ? ? struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
>> + ? ? int ret, i;
>> +
>> + ? ? np_ocotp = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp");
>> + ? ? np_thermal = of_find_compatible_node(NULL, NULL, "fsl,anatop-thermal");
>
> np_thermal = pdev->devI don't quite understand this..of_node;
>

I see, thanks. In this case I was just using the np_thermal as a
check to be sure we are on the right platform, but it seems that the
platform driver has already done the check for "fsl,anatop-thermal",
so I can remove this line completely and remove the np_thermal check
below.

>> +
>> + ? ? if (!(np_ocotp && np_thermal && anatopmfd))
>> + ? ? ? ? ? ? return -ENXIO; /* not a compatible platform */
>> +
>> + ? ? ocotp_base = of_iomap(np_ocotp, 0);
>> +
>> + ? ? if (!ocotp_base) {
>> + ? ? ? ? ? ? pr_err("Could not retrieve ocotp-base\n");
>> + ? ? ? ? ? ? ret = -ENXIO;
>> + ? ? ? ? ? ? goto err_unregister;
>> + ? ? }
>> +
>> + ? ? fuse_data = readl_relaxed(ocotp_base + HW_OCOTP_ANA1);
>> +
>> + ? ? th_zone = kzalloc(sizeof(struct imx6q_thermal_zone), GFP_KERNEL);
>
> consider using devm_*
>

Ok, will look at converting to devm_kzalloc and removing the
respective kfree calls in v6.

>> + ? ? if (!th_zone) {
>> + ? ? ? ? ? ? ret = -ENOMEM;
>> + ? ? ? ? ? ? goto err_unregister;
>> + ? ? }
>> +
>> + ? ? th_zone->dev_ops = g_dev_ops;
>> +
>> + ? ? th_zone->thermal_data =
>> + ? ? ? ? ? ? kzalloc(sizeof(struct imx6q_thermal_data), GFP_KERNEL);
>
> Why do you allocate this seperately? You could embed a struct
> imx6q_thermal_data in struct imx6q_thermal_zone.
>

In one implementation I did during development this made more sense,
but now I should embed it as you say. I'll change this in v6.

>> +
>> + ? ? if (!th_zone->thermal_data) {
>> + ? ? ? ? ? ? ret = -ENOMEM;
>> + ? ? ? ? ? ? goto err_unregister;
>> + ? ? }
>> +
>> + ? ? memcpy(th_zone->thermal_data, &g_thermal_data,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct imx6q_thermal_data));
>> +
>> + ? ? for (i = 0; i < ARRAY_SIZE(g_thermal_data.freq_tab); i++)
>> + ? ? ? ? ? ? th_zone->thermal_data->freq_tab[i].mask_val = cpumask_of(0);
>> +
>> +
>> + ? ? th_zone->sensor_data =
>> + ? ? ? ? ? ? kzalloc(sizeof(struct imx6q_sensor_data), GFP_KERNEL);
>
> This one also could be embedded in struct imx6q_thermal_zone.
>

Same. Will change in v6.

>> +
>> + ? ? if (!th_zone->sensor_data) {
>> + ? ? ? ? ? ? ret = -ENOMEM;
>> + ? ? ? ? ? ? goto err_unregister;
>> + ? ? }
>> +
>> + ? ? memcpy(th_zone->sensor_data, &g_sensor_data,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(struct imx6q_sensor_data));
>> +
>> + ? ? th_zone->sensor_data->anatopmfd = anatopmfd;
>> +
>> + ? ? ret = imx6q_thermal_process_fuse_data(fuse_data, th_zone->sensor_data);
>> +
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? pr_err("Invalid temperature calibration data.\n");
>
> use dev_* functions for logging throughout the driver.
>

Will change in v6.

>> + ? ? ? ? ? ? goto err_unregister;
>> + ? ? }
>> +
>> + ? ? if (!th_zone->sensor_data->meas_delay)
>> + ? ? ? ? ? ? th_zone->sensor_data->meas_delay = 1;
>> +
>> + ? ? /* Make sure sensor is in known good state for measurements */
>> + ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
>> +
>> + ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0, 0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_MEASURE_TEMP);
>> +
>> + ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE1, 0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE1_MEASURE_FREQ);
>> +
>> + ? ? anatop_write_reg(anatopmfd, HW_ANADIG_ANA_MISC0_SET,
>> + ? ? ? ? ? ? BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF,
>> + ? ? ? ? ? ? BM_ANADIG_ANA_MISC0_REFTOP_SELBIASOFF);
>> +
>> + ? ? anatop_write_reg(anatopmfd, HW_ANADIG_TEMPSENSE0,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN,
>> + ? ? ? ? ? ? BM_ANADIG_TEMPSENSE0_POWER_DOWN);
>> +
>> + ? ? th_zone->cool_dev = cpufreq_cooling_register(
>> + ? ? ? ? ? ? (struct freq_clip_table *)th_zone->thermal_data->freq_tab,
>> + ? ? ? ? ? ? IMX6Q_THERMAL_ACTIVE_TRP_PTS);
>> +
>> + ? ? 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_data->name, IMX6Q_THERMAL_TOTAL_TRP_PTS,
>> + ? ? ? ? ? ? th_zone, &th_zone->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;
>> + ? ? }
>> +
>> + ? ? pdev->dev.platform_data = th_zone->sensor_data;
>
> platform_data is for passing information from the one who registers the
> platform_device to the driver. What you have to use here is
> platform_set_drvdata(). Also you have to initialize this before
> registering the thermal zone, not afterwards.
>

Will fix this in v6.

>> +
>> + ? ? return 0;
>> +
>> +err_unregister:
>> + ? ? anatop_thermal_remove(pdev);
>> + ? ? return ret;
>> +}
>> +
>> +static struct of_device_id __devinitdata of_anatop_thermal_match_tbl[] = {
>> + ? ? { .compatible = "fsl,anatop-thermal", },
>> + ? ? { /* end */ }
>> +};
>> +
>> +static struct platform_driver anatop_thermal = {
>> + ? ? .driver = {
>> + ? ? ? ? ? ? .name ? = "anatop_thermal",
>> + ? ? ? ? ? ? .owner ?= THIS_MODULE,
>> + ? ? ? ? ? ? .of_match_table = of_anatop_thermal_match_tbl,
>> + ? ? },
>> + ? ? .probe ?= anatop_thermal_probe,
>> + ? ? .remove = anatop_thermal_remove,
>> + ? ? .suspend = anatop_thermal_suspend,
>> + ? ? /* due to implentation of imx6q_get_temp , resume is unnecessary */
>> +};
>> +
>> +static int __devinit anatop_thermal_init(void)
>> +{
>> + ? ? return platform_driver_register(&anatop_thermal);
>> +}
>> +device_initcall(anatop_thermal_init);
>> +
>> +static void __exit anatop_thermal_exit(void)
>> +{
>> + ? ? platform_driver_unregister(&anatop_thermal);
>> +}
>> +module_exit(anatop_thermal_exit);
>
> use module_platform_driver
>

Will fix this in v6.

Thanks,
Rob

>> +
>> +MODULE_AUTHOR("Freescale Semiconductor");
>> +MODULE_DESCRIPTION("i.MX6Q SoC thermal driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:imx6q-thermal");
>> --
>> 1.7.10
>>
>>
>
> --
> 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 |

2012-06-20 14:21:22

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

On Wed, Jun 20, 2012 at 09:12:51AM -0500, Rob Lee wrote:
> Sascha, thanks for the review.
>
> >> +
> >> +static struct imx6q_thermal_zone ? ? *th_zone;
> >> +static void __iomem ? ? ? ? ? ? ? ? ?*ocotp_base;
> >
> > This is a driver and drivers should generally be multi instance safe.
> >
>
> I don't understand what this comment is referring to. Could you elaborate?

Drivers can only be multi instance safe when all variables are inside a
instance specific struct and you pass a pointer to this struct around.
What if the i.MX7 has two different ocotp_base and you want to use this
driver on both ocotp?

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 |

2012-06-20 14:33:48

by Rob Lee

[permalink] [raw]
Subject: Re: [PATCH v5] ARM: imx: Add basic imx6q cpu thermal management

On Wed, Jun 20, 2012 at 9:20 AM, Sascha Hauer <[email protected]> wrote:
> On Wed, Jun 20, 2012 at 09:12:51AM -0500, Rob Lee wrote:
>> Sascha, thanks for the review.
>>
>> >> +
>> >> +static struct imx6q_thermal_zone ? ? *th_zone;
>> >> +static void __iomem ? ? ? ? ? ? ? ? ?*ocotp_base;
>> >
>> > This is a driver and drivers should generally be multi instance safe.
>> >
>>
>> I don't understand what this comment is referring to. ?Could you elaborate?
>
> Drivers can only be multi instance safe when all variables are inside a
> instance specific struct and you pass a pointer to this struct around.
> What if the i.MX7 has two different ocotp_base and you want to use this
> driver on both ocotp?
>

Understood, thanks. I'll fix this in v6.

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