Hi Nitin,
On 14/11/2014 21:13, nitin.g...@freescale.com wrote:
> From: Nitin Garg <nitin.g...@freescale.com>
>
> i.MX6 SoC has onchip temperature sensor. Add driver
> for this sensor.
>
> Signed-off-by: Nitin Garg <nitin.g...@freescale.com>
> ---
> drivers/Makefile | 1 +
> drivers/thermal/Makefile | 8 +++
> drivers/thermal/imx_thermal.c | 144
> +++++++++++++++++++++++++++++++++++++++++
> include/imx_thermal.h | 15 +++++
> 4 files changed, 168 insertions(+)
> create mode 100644 drivers/thermal/Makefile
> create mode 100644 drivers/thermal/imx_thermal.c
> create mode 100644 include/imx_thermal.h
>
I have some conceptional question here. This introduces a new set or,
better, "class" of drivers, as we are used to see in kernel as
"thermal". As we discussed in last u-boot mini summit, the preferred way
to introduce new drivers into current u-boot is to use DM (Driver
Model), because this is the way u-boot will follow in the future. It
will take some time porting the current drivers, but it is easier for
drivers introducing new features.
This is exactly this case: there is at the moment no "thermal" drivers,
and it would be nice to introduce it using DM, else we will have the
problem in the near future to port this drivers, too, and maybe some
further drivers adding thermal monitoring.
This means you have to add a "thermal" class (you can take a look at the
spi driver, it was already ported to dm) and define the functions your
class exports. I think at the moment you define only a "monitor", but
feel free to add further functions, preferably in the same way as the
linux kernel does (see include/linux/thermal.h in kernel tree).
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 33227c8..7683c61 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -21,3 +21,4 @@ obj-y += pwm/
> obj-y += input/
> # SOC specific infrastructure drivers.
> obj-y += soc/
> +obj-y += thermal/
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> new file mode 100644
> index 0000000..04ae395
> --- /dev/null
> +++ b/drivers/thermal/Makefile
> @@ -0,0 +1,8 @@
> +#
> +# (C) Copyright 2014 Freescale Semiconductor, Inc.
> +# Author: Nitin Garg <nitin.g...@freescale.com>
> +#
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +obj-$(CONFIG_IMX6_THERMAL) += imx_thermal.o
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> new file mode 100644
> index 0000000..fe9dd85
> --- /dev/null
> +++ b/drivers/thermal/imx_thermal.c
> @@ -0,0 +1,144 @@
> +/*
> + * (C) Copyright 2014 Freescale Semiconductor, Inc.
> + * Author: Nitin Garg <nitin.g...@freescale.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <div64.h>
> +#include <fuse.h>
> +#include <asm/io.h>
> +#include <asm/arch/clock.h>
> +
> +#define TEMPERATURE_MIN -40
> +#define TEMPERATURE_HOT 80
> +#define TEMPERATURE_MAX 125
> +#define FACTOR0 10000000
> +#define FACTOR1 15976
> +#define FACTOR2 4297157
> +#define MEASURE_FREQ 327
> +
> +#define TEMPSENSE0_TEMP_CNT_SHIFT 8
> +#define TEMPSENSE0_TEMP_CNT_MASK (0xfff << TEMPSENSE0_TEMP_CNT_SHIFT)
> +#define TEMPSENSE0_FINISHED (1 << 2)
> +#define TEMPSENSE0_MEASURE_TEMP (1 << 1)
> +#define TEMPSENSE0_POWER_DOWN (1 << 0)
> +#define MISC0_REFTOP_SELBIASOFF (1 << 3)
> +#define TEMPSENSE1_MEASURE_FREQ 0xffff
> +
> +static int read_cpu_temperature(u32 fuse)
> +{
> + int temperature;
> + unsigned int reg, n_meas;
> + struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR;
> + int t1, n1;
> + u32 c1, c2;
> + u64 temp64;
> +
> + /* make sure pll3 is enabled for thermal sensor */
> + enable_pll3();
> +
> + /*
> + * Sensor data layout:
> + * [31:20] - sensor value @ 25C
> + * We use universal formula now and only need sensor value @ 25C
> + * slope = 0.4297157 - (0.0015976 * 25C fuse)
> + */
> + n1 = fuse >> 20;
> + t1 = 25; /* t1 always 25C */
> +
> + /*
> + * Derived from linear interpolation:
> + * slope = 0.4297157 - (0.0015976 * 25C fuse)
> + * slope = (FACTOR2 - FACTOR1 * n1) / FACTOR0
> + * (Nmeas - n1) / (Tmeas - t1) = slope
> + * We want to reduce this down to the minimum computation necessary
> + * for each temperature read. Also, we want Tmeas in millicelsius
> + * and we don't want to lose precision from integer division. So...
> + * Tmeas = (Nmeas - n1) / slope + t1
> + * milli_Tmeas = 1000 * (Nmeas - n1) / slope + 1000 * t1
> + * milli_Tmeas = -1000 * (n1 - Nmeas) / slope + 1000 * t1
> + * Let constant c1 = (-1000 / slope)
> + * milli_Tmeas = (n1 - Nmeas) * c1 + 1000 * t1
> + * Let constant c2 = n1 *c1 + 1000 * t1
> + * milli_Tmeas = c2 - Nmeas * c1
> + */
> + temp64 = FACTOR0;
> + temp64 *= 1000;
> + do_div(temp64, FACTOR1 * n1 - FACTOR2);
> + c1 = temp64;
> + c2 = n1 * c1 + 1000 * t1;
> +
> + /*
> + * now we only use single measure, every time we read
> + * the temperature, we will power on/down anadig thermal
> + * module
> + */
> + writel(TEMPSENSE0_POWER_DOWN, &anatop->tempsense0_clr);
> + writel(MISC0_REFTOP_SELBIASOFF, &anatop->ana_misc0_set);
> +
> + /* setup measure freq */
> + reg = readl(&anatop->tempsense1);
> + reg &= ~TEMPSENSE1_MEASURE_FREQ;
> + reg |= MEASURE_FREQ;
> + writel(reg, &anatop->tempsense1);
> +
> + /* start the measurement process */
> + writel(TEMPSENSE0_MEASURE_TEMP, &anatop->tempsense0_clr);
> + writel(TEMPSENSE0_FINISHED, &anatop->tempsense0_clr);
> + writel(TEMPSENSE0_MEASURE_TEMP, &anatop->tempsense0_set);
> +
> + /* make sure that the latest temp is valid */
> + while ((readl(&anatop->tempsense0) &
> + TEMPSENSE0_FINISHED) == 0)
> + udelay(10000);
> +
> + /* read temperature count */
> + reg = readl(&anatop->tempsense0);
> + n_meas = (reg & TEMPSENSE0_TEMP_CNT_MASK)
> + >> TEMPSENSE0_TEMP_CNT_SHIFT;
> + writel(TEMPSENSE0_FINISHED, &anatop->tempsense0_clr);
> +
> + /* milli_Tmeas = c2 - Nmeas * c1 */
> + temperature = (c2 - n_meas * c1)/1000;
> +
> + /* power down anatop thermal sensor */
> + writel(TEMPSENSE0_POWER_DOWN, &anatop->tempsense0_set);
> + writel(MISC0_REFTOP_SELBIASOFF, &anatop->ana_misc0_clr);
> +
> + return temperature;
> +}
> +
> +void check_cpu_temperature(void)
> +{
> + int cpu_tmp = 0;
> + u32 fuse = ~0;
> +
> + /* Read Temperature calibration data fuse */
> + fuse_read(1, 6, &fuse);
> +
> + /* Check for valid fuse */
> + if (fuse == 0 || fuse == ~0) {
> + printf("CPU: Temperature: invalid data, fuse: 0x%x\n", fuse);
> + return;
> + }
> +
> + cpu_tmp = read_cpu_temperature(fuse);
> + while (cpu_tmp > TEMPERATURE_MIN && cpu_tmp < TEMPERATURE_MAX) {
> + if (cpu_tmp >= TEMPERATURE_HOT) {
> + printf("CPU is %d C, too hot to boot, waiting...\n",
> + cpu_tmp);
> + udelay(5000000);
> + cpu_tmp = read_cpu_temperature(fuse);
> + } else {
> + break;
> + }
> + }
I would split here the interface. We have a driver, and this should
return back its result - but without taking an action. This decision
should be done by the user of the driver (the cpu code).
> +
> + if (cpu_tmp > TEMPERATURE_MIN && cpu_tmp < TEMPERATURE_MAX)
> + printf("CPU: Temperature %d C\n", cpu_tmp);
> + else
> + printf("CPU: Temperature: invalid sensor data\n");
> +}
> diff --git a/include/imx_thermal.h b/include/imx_thermal.h
> new file mode 100644
> index 0000000..979664a
> --- /dev/null
> +++ b/include/imx_thermal.h
> @@ -0,0 +1,15 @@
> +/*
> + * IMX Thermal Defines
> + *-------------------------------------------------------------------
> + *
> + * Copyright 2014 Freescale Semiconductor, Inc
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef __IMX6_THERMAL_H__
> +#define __IMX6_THERMAL_H__
> +
> +void check_cpu_temperature(void);
> +
> +#endif /* __IMX6_THERMAL_H__ */
>
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
