2014-12-26 10:29:39

by Pavel Machek

[permalink] [raw]
Subject: [PATCH] add omap34xx temperature monitoring support


Signed-off-by: Pavel Machek <[email protected]>

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6529c09..9007ca9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -28,6 +28,10 @@ config HWMON_VID
tristate
default n

+config SENSORS_OMAP34XX
+ bool "TI OMAP34xx internal temperature sensor"
+ depends on ARCH_OMAP3 && HIGH_RES_TIMERS
+
config HWMON_DEBUG_CHIP
bool "Hardware Monitoring Chip debugging messages"
default n
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 6728064..5c3b5d1 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_SENSORS_NCT6683) += nct6683.o
obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o
obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_OMAP34XX) += omap34xx_temp.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/omap34xx_temp.c b/drivers/hwmon/omap34xx_temp.c
new file mode 100644
index 0000000..bc7a72f
--- /dev/null
+++ b/drivers/hwmon/omap34xx_temp.c
@@ -0,0 +1,263 @@
+/*
+ * omap34xx_temp.c - Linux kernel module for OMAP34xx hardware monitoring
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Written by Peter De Schrijver <[email protected]>
+ *
+ * Inspired by k8temp.c
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/hrtimer.h>
+#include <linux/module.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+#include "../../arch/arm/mach-omap2/control.h"
+
+#define TEMP_SENSOR_SOC BIT(8)
+#define TEMP_SENSOR_EOCZ BIT(7)
+
+/* minimum delay for EOCZ rise after SOC rise is
+ * 11 cycles of the 32.768Khz clock */
+#define EOCZ_MIN_RISING_DELAY (11 * 30518)
+
+/* maximum delay for EOCZ rise after SOC rise is
+ * 14 cycles of the 32.768Khz clock */
+#define EOCZ_MAX_RISING_DELAY (14 * 30518)
+
+/* minimum delay for EOCZ falling is
+ * 36 cycles of the 32.768Khz clock */
+#define EOCZ_MIN_FALLING_DELAY (36 * 30518)
+
+/* maximum delay for EOCZ falling is
+ * 40 cycles of the 32.768Khz clock */
+#define EOCZ_MAX_FALLING_DELAY (40 * 30518)
+
+struct omap34xx_data {
+ struct device *hwmon_dev;
+ struct clk *clk_32k;
+ struct mutex update_lock;
+ const char *name;
+ char valid;
+ unsigned long last_updated;
+ u32 temp;
+};
+
+static struct platform_device omap34xx_temp_device = {
+ .name = "omap34xx_temp",
+ .id = -1,
+};
+
+static int adc_to_temp[] = {
+ -40, -40, -40, -40, -40, -39, -38, -36, -34, -32, -31, -29, -28, -26,
+ -25, -24, -22, -21, -19, -18, -17, -15, -14, -12, -11, -9, -8, -7, -5,
+ -4, -2, -1, 0, 1, 3, 4, 5, 7, 8, 10, 11, 13, 14, 15, 17, 18, 20, 21,
+ 22, 24, 25, 27, 28, 30, 31, 32, 34, 35, 37, 38, 39, 41, 42, 44, 45,
+ 47, 48, 49, 51, 52, 53, 55, 56, 58, 59, 60, 62, 63, 65, 66, 67, 69,
+ 70, 72, 73, 74, 76, 77, 79, 80, 81, 83, 84, 85, 87, 88, 89, 91, 92,
+ 94, 95, 96, 98, 99, 100, 102, 103, 105, 106, 107, 109, 110, 111, 113,
+ 114, 116, 117, 118, 120, 121, 122, 124, 124, 125, 125, 125, 125, 125};
+
+static inline u32 wait_for_eocz(int min_delay, int max_delay, u32 level)
+{
+ struct timespec timeout;
+ ktime_t expire;
+ u32 temp_sensor_reg;
+
+ level &= 1;
+ level *= TEMP_SENSOR_EOCZ;
+
+ expire = ktime_add_ns(ktime_get(), max_delay);
+ timeout = ns_to_timespec(min_delay);
+ hrtimer_nanosleep(&timeout, NULL, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
+ do {
+ temp_sensor_reg = omap_ctrl_readl(OMAP343X_CONTROL_TEMP_SENSOR);
+ if ((temp_sensor_reg & TEMP_SENSOR_EOCZ) == level)
+ break;
+ } while (ktime_us_delta(expire, ktime_get()) > 0);
+
+ return (temp_sensor_reg & TEMP_SENSOR_EOCZ) == level;
+}
+
+static void omap34xx_update(struct omap34xx_data *data)
+{
+ u32 temp_sensor_reg;
+
+ mutex_lock(&data->update_lock);
+
+ if (!data->valid
+ || time_after(jiffies, data->last_updated + HZ)) {
+ clk_prepare_enable(data->clk_32k);
+
+ temp_sensor_reg = omap_ctrl_readl(OMAP343X_CONTROL_TEMP_SENSOR);
+ temp_sensor_reg |= TEMP_SENSOR_SOC;
+ omap_ctrl_writel(temp_sensor_reg, OMAP343X_CONTROL_TEMP_SENSOR);
+
+ if (!wait_for_eocz(EOCZ_MIN_RISING_DELAY,
+ EOCZ_MAX_RISING_DELAY, 1))
+ goto err;
+
+ temp_sensor_reg = omap_ctrl_readl(OMAP343X_CONTROL_TEMP_SENSOR);
+ temp_sensor_reg &= ~TEMP_SENSOR_SOC;
+ omap_ctrl_writel(temp_sensor_reg, OMAP343X_CONTROL_TEMP_SENSOR);
+
+ if (!wait_for_eocz(EOCZ_MIN_FALLING_DELAY,
+ EOCZ_MAX_FALLING_DELAY, 0))
+ goto err;
+
+ data->temp = omap_ctrl_readl(OMAP343X_CONTROL_TEMP_SENSOR) &
+ ((1<<7) - 1);
+ data->last_updated = jiffies;
+ data->valid = 1;
+
+err:
+ clk_disable_unprepare(data->clk_32k);
+ }
+
+ mutex_unlock(&data->update_lock);
+}
+
+static ssize_t show_name(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct omap34xx_data *data = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%s\n", data->name);
+}
+
+static ssize_t show_temp_raw(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct omap34xx_data *data = dev_get_drvdata(dev);
+
+ omap34xx_update(data);
+
+ return sprintf(buf, "%d\n", data->temp);
+}
+
+static ssize_t show_temp(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct omap34xx_data *data = dev_get_drvdata(dev);
+
+ omap34xx_update(data);
+
+ return sprintf(buf, "%d\n", adc_to_temp[data->temp]);
+}
+
+static SENSOR_DEVICE_ATTR_2(temp1_input, S_IRUGO, show_temp, NULL, 0, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_input_raw, S_IRUGO, show_temp_raw,
+ NULL, 0, 0);
+static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+
+static int omap34xx_temp_probe(void)
+{
+ int err;
+ struct omap34xx_data *data;
+
+ err = platform_device_register(&omap34xx_temp_device);
+ if (err) {
+ pr_err("Unable to register omap34xx temperature device\n");
+ goto exit;
+ }
+
+ data = kzalloc(sizeof(struct omap34xx_data), GFP_KERNEL);
+ if (!data) {
+ err = -ENOMEM;
+ goto exit_platform;
+ }
+
+ dev_set_drvdata(&omap34xx_temp_device.dev, data);
+ mutex_init(&data->update_lock);
+ data->name = "omap34xx_temp";
+
+ data->clk_32k = clk_get(&omap34xx_temp_device.dev, "ts_fck");
+ if (IS_ERR(data->clk_32k)) {
+ err = PTR_ERR(data->clk_32k);
+ goto exit_free;
+ }
+
+ err = device_create_file(&omap34xx_temp_device.dev,
+ &sensor_dev_attr_temp1_input.dev_attr);
+ if (err)
+ goto clock_free;
+
+ err = device_create_file(&omap34xx_temp_device.dev,
+ &sensor_dev_attr_temp1_input_raw.dev_attr);
+ if (err)
+ goto exit_remove;
+
+ err = device_create_file(&omap34xx_temp_device.dev, &dev_attr_name);
+ if (err)
+ goto exit_remove_raw;
+
+ data->hwmon_dev = hwmon_device_register(&omap34xx_temp_device.dev);
+
+ if (IS_ERR(data->hwmon_dev)) {
+ err = PTR_ERR(data->hwmon_dev);
+ goto exit_remove_all;
+ }
+
+ return 0;
+
+exit_remove_all:
+ device_remove_file(&omap34xx_temp_device.dev,
+ &dev_attr_name);
+exit_remove_raw:
+ device_remove_file(&omap34xx_temp_device.dev,
+ &sensor_dev_attr_temp1_input_raw.dev_attr);
+exit_remove:
+ device_remove_file(&omap34xx_temp_device.dev,
+ &sensor_dev_attr_temp1_input.dev_attr);
+clock_free:
+ clk_put(data->clk_32k);
+
+exit_free:
+ kfree(data);
+exit_platform:
+ platform_device_unregister(&omap34xx_temp_device);
+exit:
+ return err;
+}
+
+static int __init omap34xx_temp_init(void)
+{
+ return omap34xx_temp_probe();
+}
+
+static void __exit omap34xx_temp_exit(void)
+{
+ struct omap34xx_data *data =
+ dev_get_drvdata(&omap34xx_temp_device.dev);
+
+ clk_put(data->clk_32k);
+ hwmon_device_unregister(data->hwmon_dev);
+ device_remove_file(&omap34xx_temp_device.dev,
+ &sensor_dev_attr_temp1_input.dev_attr);
+ device_remove_file(&omap34xx_temp_device.dev, &dev_attr_name);
+ kfree(data);
+ platform_device_unregister(&omap34xx_temp_device);
+}
+
+MODULE_AUTHOR("Peter De Schrijver");
+MODULE_DESCRIPTION("Omap34xx temperature sensor");
+MODULE_LICENSE("GPL");
+
+module_init(omap34xx_temp_init)
+module_exit(omap34xx_temp_exit)
+

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


2014-12-26 12:35:22

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH 0/3] OMAP3 temperature sensor

Hi,

I've prepared an updated variant of the omap34xx temperature monitor driver.
It's based on the N9 driver instead of the N900 driver (and thus has omap36xx
support).

The differences compared to the original driver are:

* DT based
* No includes from arch, instead uses syscon DT node + regmap
* Removed support for raw temperature reading
- I assume this was used for debugging and regmap can be used
instead with newer kernels.
* Introduction of managed resources where possible
* Usage of module_platform_driver() macro
* Added some comments referencing the TRM

So far the patchset is _compile-tested only_. I will test it on my N900 in
the next days.

-- Sebastian

Sebastian Reichel (3):
DT Binding for omap3 temperature sensor
hwmon: Driver for OMAP3 temperature sensor
ARM: dts: OMAP34xx/36xx: Add temperature sensor

.../bindings/hwmon/omap3-temperature.txt | 25 ++
arch/arm/boot/dts/omap34xx.dtsi | 7 +
arch/arm/boot/dts/omap36xx.dtsi | 7 +
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++
6 files changed, 355 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
create mode 100644 drivers/hwmon/omap3-temp.c

--
2.1.3

2014-12-26 12:35:28

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH 1/3] DT Binding for omap3 temperature sensor

OMAP34xx and OMAP36xx processors contain a register in the syscon area,
which can be used to determine the SoCs temperature. This provides a
DT binding specification for the temperature monitor.

Signed-off-by: Sebastian Reichel <[email protected]>
---
.../bindings/hwmon/omap3-temperature.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/omap3-temperature.txt

diff --git a/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
new file mode 100644
index 0000000..99631ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
@@ -0,0 +1,25 @@
+* OMAP3 temperature sensor
+
+The OMAP34xx and OMAP36xx processors contain a register in the syscon area,
+which can be used to determine the SoCs temperature.
+
+Requires node properties:
+- compatible : should contain one of
+ - "ti,omap34xx-temperature-sensor" for OMAP34xx
+ - "ti,omap36xx-temperature-sensor" for OMAP36xx
+- syscon : Should be a phandle to system configuration node which
+ encompases the temperature register
+- clocks : Should contain 32KHz fclk clock specifier
+- clock-names : Should contain clock names
+ - "fck" for the 32KHz fclk clock specifier
+
+Example for omap34xx:
+
+/ {
+ temperature-sensor {
+ compatible = "ti,omap34xx-temperature-sensor";
+ syscon = <&omap3_scm_general>;
+ clocks = <&ts_fck>;
+ clock-names = "fck";
+ };
+};
--
2.1.3

2014-12-26 12:35:45

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH 3/3] ARM: dts: OMAP34xx/36xx: Add temperature sensor

OMAP34xx and OMAP36xx processors contain a register in the
syscon area, which can be used to determine the SoCs temperature.

Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm/boot/dts/omap34xx.dtsi | 7 +++++++
arch/arm/boot/dts/omap36xx.dtsi | 7 +++++++
2 files changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi
index 3819c1e..64679ee 100644
--- a/arch/arm/boot/dts/omap34xx.dtsi
+++ b/arch/arm/boot/dts/omap34xx.dtsi
@@ -38,6 +38,13 @@
pinctrl-single,function-mask = <0xff1f>;
};
};
+
+ omap3_temperature_sensor: temperature-sensor {
+ compatible = "ti,omap34xx-temperature-sensor";
+ syscon = <&omap3_scm_general>;
+ clocks = <&ts_fck>;
+ clock-names = "fck";
+ };
};

&ssi {
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 541704a..be9ee1d 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -70,6 +70,13 @@
pinctrl-single,function-mask = <0xff1f>;
};
};
+
+ omap3_temperature_sensor: temperature-sensor {
+ compatible = "ti,omap36xx-temperature-sensor";
+ syscon = <&omap3_scm_general>;
+ clocks = <&ts_fck>;
+ clock-names = "fck";
+ };
};

/* OMAP3630 needs dss_96m_fck for VENC */
--
2.1.3

2014-12-26 12:35:44

by Sebastian Reichel

[permalink] [raw]
Subject: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

OMAP34xx and OMAP36xx processors contain a register in the syscon area,
which can be used to determine the SoCs temperature. This patch provides
a DT based driver for the temperature sensor based on an older driver
written by Peter De Schrijver for the Nokia N900 and N9.

Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/hwmon/Kconfig | 8 ++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 316 insertions(+)
create mode 100644 drivers/hwmon/omap3-temp.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 6529c09..749748d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1135,6 +1135,14 @@ config SENSORS_NCT7802
This driver can also be built as a module. If so, the module
will be called nct7802.

+config SENSORS_OMAP3_TEMP
+ tristate "OMAP3 Temperature Sensor"
+ depends on OF && (ARCH_OMAP3 || COMPILE_TEST)
+ select MFD_SYSCON
+ help
+ If you say yes here you get support for the temperature sensor
+ built into OMAP3 processors.
+
config SENSORS_PCF8591
tristate "Philips PCF8591 ADC/DAC"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 6728064..5a69773 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_SENSORS_NCT6683) += nct6683.o
obj-$(CONFIG_SENSORS_NCT6775) += nct6775.o
obj-$(CONFIG_SENSORS_NCT7802) += nct7802.o
obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
+obj-$(CONFIG_SENSORS_OMAP3_TEMP) += omap3-temp.o
obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c
new file mode 100644
index 0000000..5c331c5
--- /dev/null
+++ b/drivers/hwmon/omap3-temp.c
@@ -0,0 +1,307 @@
+/*
+ * omap3-temp.c - driver for OMAP34xx and OMAP36xx temperature sensor
+ *
+ * Copyright (c) 2014 Sebastian Reichel <[email protected]>
+ * Copyright (C) 2008, 2009, 2010 Nokia Corporation
+ *
+ * based on Peter De Schrijver's driver for N9
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/clk.h>
+#include <linux/hrtimer.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/sched.h>
+#include <linux/stat.h>
+
+/* 32.768Khz clock speed in nano seconds */
+#define CLOCK_32K_SPEED_NS 30518
+
+/* minimum delay for EOCZ rise after SOC rise is
+ * 11 cycles of the 32.768Khz clock */
+#define EOCZ_MIN_RISING_DELAY (11 * CLOCK_32K_SPEED_NS)
+
+/* From docs, maximum delay for EOCZ rise after SOC rise is
+ * 14 cycles of the 32.768Khz clock. But after some experiments,
+ * 24 cycles as maximum is safer. */
+#define EOCZ_MAX_RISING_DELAY (24 * CLOCK_32K_SPEED_NS)
+
+/* minimum delay for EOCZ falling is
+ * 36 cycles of the 32.768Khz clock */
+#define EOCZ_MIN_FALLING_DELAY (36 * CLOCK_32K_SPEED_NS)
+
+/* maximum delay for EOCZ falling is
+ * 40 cycles of the 32.768Khz clock */
+#define EOCZ_MAX_FALLING_DELAY (40 * CLOCK_32K_SPEED_NS)
+
+/* temperature register offset in the syscon register area */
+#define SYSCON_TEMP_REG 0x02B4
+
+/* TRM: Table 7-11. ADC Codes Versus Temperature */
+static const int adc_to_temp_3430[] = {
+ -400, -400, -400, -400, -400, -390, -380, -360, -340, -320, -310,
+ -290, -280, -260, -250, -240, -220, -210, -190, -180, -170, -150,
+ -140, -120, -110, -90, -80, -70, -50, -40, -20, -10, 00, 10, 30,
+ 40, 50, 70, 80, 100, 110, 130, 140, 150, 170, 180, 200, 210, 220,
+ 240, 250, 270, 280, 300, 310, 320, 340, 350, 370, 380, 390, 410, 420,
+ 440, 450, 470, 480, 490, 510, 520, 530, 550, 560, 580, 590, 600, 620,
+ 630, 650, 660, 670, 690, 700, 720, 730, 740, 760, 770, 790, 800, 810,
+ 830, 840, 850, 870, 880, 890, 910, 920, 940, 950, 960, 980, 990, 1000,
+ 1020, 1030, 1050, 1060, 1070, 1090, 1100, 1110, 1130, 1140, 1160,
+ 1170, 1180, 1200, 1210, 1220, 1240, 1240, 1250, 1250, 1250, 1250,
+ 1250};
+
+/* TRM: Table 13-11. ADC Code Versus Temperature */
+static const int adc_to_temp_3630[] = {
+ -400, -400, -400, -400, -400, -400, -400, -400, -400, -400, -400,
+ -400, -400, -400, -380, -350, -340, -320, -300, -280, -260, -240,
+ -220, -200, -185, -170, -150, -135, -120, -100, -80, -65, -50, -35,
+ -15, 0, 20, 35, 50, 65, 85, 100, 120, 135, 150, 170, 190, 210, 230,
+ 250, 270, 285, 300, 320, 335, 350, 370, 385, 400, 420, 435, 450, 470,
+ 485, 500, 520, 535, 550, 570, 585, 600, 620, 640, 660, 680, 700, 715,
+ 735, 750, 770, 785, 800, 820, 835, 850, 870, 885, 900, 920, 935, 950,
+ 970, 985, 1000, 1020, 1035, 1050, 1070, 1090, 1110, 1130, 1150, 1170,
+ 1185, 1200, 1220, 1235, 1250, 1250, 1250, 1250, 1250, 1250, 1250,
+ 1250, 1250, 1250, 1250, 1250, 1250, 1250, 1250, 1250, 1250, 1250,
+ 1250, 1250, 1250};
+
+struct omap3_temp_type {
+ const int *adc_to_temp;
+ u8 soc_bit;
+ u8 eocz_bit;
+};
+
+/* TRM: Table 7-228. CONTROL_TEMP_SENSOR */
+static const struct omap3_temp_type omap34xx_temp_type = {
+ .eocz_bit = 7,
+ .soc_bit = 8,
+ .adc_to_temp = adc_to_temp_3430,
+};
+
+/* TRM: Table 13-239. CONTROL_TEMP_SENSOR */
+static const struct omap3_temp_type omap36xx_temp_type = {
+ .eocz_bit = 8,
+ .soc_bit = 9,
+ .adc_to_temp = adc_to_temp_3630,
+};
+
+struct omap3_temp_data {
+ struct device *hwmon_dev;
+ struct regmap *syscon;
+ struct clk *clk_32k;
+ struct omap3_temp_type *hwdata;
+ /* mutex to protect the update procedure while reading from sensor */
+ struct mutex update_lock;
+ const char *name;
+ unsigned long last_updated;
+ u32 temperature;
+ bool valid;
+};
+
+static inline u32 wait_for_eocz(int min_delay, int max_delay, u32 level,
+ struct omap3_temp_data *data)
+{
+ ktime_t timeout, expire;
+ u32 temp_sensor_reg, eocz_mask;
+
+ eocz_mask = BIT(data->hwdata->eocz_bit);
+ level &= 1;
+ level *= eocz_mask;
+
+ expire = ktime_add_ns(ktime_get(), max_delay);
+ timeout = ktime_set(0, min_delay);
+ __set_current_state(TASK_INTERRUPTIBLE);
+ schedule_hrtimeout(&timeout, HRTIMER_MODE_REL);
+ do {
+ regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
+ if ((temp_sensor_reg & eocz_mask) == level)
+ break;
+ } while (ktime_us_delta(expire, ktime_get()) > 0);
+
+ return (temp_sensor_reg & eocz_mask) == level;
+}
+
+static int omap3_temp_update(struct omap3_temp_data *data)
+{
+ int e = 0;
+ u32 temp_sensor_reg;
+ u32 soc_mask = BIT(data->hwdata->soc_bit);
+
+ mutex_lock(&data->update_lock);
+
+ if (!data->valid || time_after(jiffies, data->last_updated + HZ)) {
+ clk_enable(data->clk_32k);
+
+ regmap_update_bits(data->syscon, SYSCON_TEMP_REG,
+ soc_mask, soc_mask);
+
+ if (!wait_for_eocz(EOCZ_MIN_RISING_DELAY,
+ EOCZ_MAX_RISING_DELAY, 1, data)) {
+ e = -EIO;
+ goto err;
+ }
+
+ regmap_update_bits(data->syscon, SYSCON_TEMP_REG, soc_mask, 0);
+
+ if (!wait_for_eocz(EOCZ_MIN_FALLING_DELAY,
+ EOCZ_MAX_FALLING_DELAY, 0, data)) {
+ e = -EIO;
+ goto err;
+ }
+
+ regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
+ data->temperature = temp_sensor_reg & ((1<<7) - 1);
+ data->last_updated = jiffies;
+ data->valid = true;
+
+err:
+ clk_disable(data->clk_32k);
+ }
+
+ mutex_unlock(&data->update_lock);
+ return e;
+}
+
+static ssize_t show_temp(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct omap3_temp_data *data = dev_get_drvdata(dev);
+ int temp;
+ int ret;
+
+ ret = omap3_temp_update(data);
+ if (ret < 0)
+ return ret;
+
+ temp = data->hwdata->adc_to_temp[data->temperature];
+
+ return sprintf(buf, "%d.%d\n", temp / 10, temp % 10);
+}
+
+static ssize_t show_name(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ struct omap3_temp_data *data = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%s\n", data->name);
+}
+
+static SENSOR_DEVICE_ATTR_2(temp_input, S_IRUGO, show_temp, NULL, 0, 0);
+static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+
+static const struct of_device_id omap3_temp_dt_ids[] = {
+ {
+ .compatible = "ti,omap34xx-temperature-sensor",
+ .data = &omap34xx_temp_type,
+ },
+ {
+ .compatible = "ti,omap36xx-temperature-sensor",
+ .data = &omap36xx_temp_type,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, omap3_temp_dt_ids);
+
+static int omap3_temp_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct omap3_temp_data *data;
+ const struct of_device_id *of_id;
+ int err;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ of_id = of_match_device(omap3_temp_dt_ids, &pdev->dev);
+ if (!of_id) {
+ dev_warn(&pdev->dev, "unsupported device!");
+ return -ENODEV;
+ }
+
+ mutex_init(&data->update_lock);
+ data->name = "omap3-temperature";
+
+ data->clk_32k = devm_clk_get(&pdev->dev, "fck");
+ if (IS_ERR(data->clk_32k))
+ return PTR_ERR(data->clk_32k);
+
+ data->hwdata = (struct omap3_temp_type *)of_id->data;
+
+ data->syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
+ if (IS_ERR(data->syscon))
+ return PTR_ERR(data->syscon);
+
+ platform_set_drvdata(pdev, data);
+
+ err = device_create_file(&pdev->dev,
+ &sensor_dev_attr_temp_input.dev_attr);
+ if (err)
+ goto fail_temp_file;
+
+ err = device_create_file(&pdev->dev, &dev_attr_name);
+ if (err)
+ goto fail_name_file;
+
+ data->hwmon_dev = hwmon_device_register(&pdev->dev);
+ if (IS_ERR(data->hwmon_dev)) {
+ err = PTR_ERR(data->hwmon_dev);
+ goto fail_hwmon_reg;
+ }
+
+ return 0;
+
+fail_hwmon_reg:
+ device_remove_file(&pdev->dev, &dev_attr_name);
+fail_name_file:
+ device_remove_file(&pdev->dev, &sensor_dev_attr_temp_input.dev_attr);
+fail_temp_file:
+ return err;
+}
+
+static int omap3_temp_remove(struct platform_device *pdev)
+{
+ struct omap3_temp_data *data = platform_get_drvdata(pdev);
+
+ if (!data)
+ return 0;
+
+ hwmon_device_unregister(data->hwmon_dev);
+ device_remove_file(&pdev->dev, &dev_attr_name);
+ device_remove_file(&pdev->dev, &sensor_dev_attr_temp_input.dev_attr);
+
+ return 0;
+}
+
+static struct platform_driver omap3_temp_driver = {
+ .probe = omap3_temp_probe,
+ .remove = omap3_temp_remove,
+ .driver = {
+ .name = "omap3-temperature",
+ .of_match_table = omap3_temp_dt_ids,
+ },
+};
+
+module_platform_driver(omap3_temp_driver);
+
+MODULE_AUTHOR("Sebastian Reichel");
+MODULE_DESCRIPTION("OMAP34xx/OMAP36xx temperature sensor");
+MODULE_LICENSE("GPL");
--
2.1.3

2014-12-26 15:55:09

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] add omap34xx temperature monitoring support

* Pavel Machek <[email protected]> [141226 02:32]:
> --- /dev/null
> +++ b/drivers/hwmon/omap34xx_temp.c
> @@ -0,0 +1,263 @@
> +/*
> + * omap34xx_temp.c - Linux kernel module for OMAP34xx hardware monitoring
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + *
> + * Written by Peter De Schrijver <[email protected]>
> + *
> + * Inspired by k8temp.c
> + *
> + * This file is subject to the terms and conditions of the GNU General
> + * Public License. See the file "COPYING" in the main directory of this
> + * archive for more details.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/hrtimer.h>
> +#include <linux/module.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +
> +#include "../../arch/arm/mach-omap2/control.h"

No need to do this, you can use syscon here like pbias-regulator.c
is doing.

Regards,

Tony

2014-12-26 16:18:11

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] add omap34xx temperature monitoring support

* Tony Lindgren <[email protected]> [141226 07:57]:
> * Pavel Machek <[email protected]> [141226 02:32]:
> > --- /dev/null
> > +++ b/drivers/hwmon/omap34xx_temp.c
> > @@ -0,0 +1,263 @@
> > +/*
> > + * omap34xx_temp.c - Linux kernel module for OMAP34xx hardware monitoring
> > + *
> > + * Copyright (C) 2008 Nokia Corporation
> > + *
> > + * Written by Peter De Schrijver <[email protected]>
> > + *
> > + * Inspired by k8temp.c
> > + *
> > + * This file is subject to the terms and conditions of the GNU General
> > + * Public License. See the file "COPYING" in the main directory of this
> > + * archive for more details.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/hrtimer.h>
> > +#include <linux/module.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/slab.h>
> > +
> > +#include "../../arch/arm/mach-omap2/control.h"
>
> No need to do this, you can use syscon here like pbias-regulator.c
> is doing.

Oh looks like you're already using syscon, nice. What defines do you
need from control.h?

Those should be in the driver if private to the driver, or else we
should have some minimal header in include/linux somewhere if some
control.h defines really need to be exposed.

Regards,

Tony

2014-12-26 16:26:43

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH] add omap34xx temperature monitoring support

On Friday 26 December 2014 17:17:57 Tony Lindgren wrote:
> * Tony Lindgren <[email protected]> [141226 07:57]:
> > * Pavel Machek <[email protected]> [141226 02:32]:
> > > --- /dev/null
> > > +++ b/drivers/hwmon/omap34xx_temp.c
> > > @@ -0,0 +1,263 @@
> > > +/*
> > > + * omap34xx_temp.c - Linux kernel module for OMAP34xx
> > > hardware monitoring + *
> > > + * Copyright (C) 2008 Nokia Corporation
> > > + *
> > > + * Written by Peter De Schrijver
> > > <[email protected]> + *
> > > + * Inspired by k8temp.c
> > > + *
> > > + * This file is subject to the terms and conditions of
> > > the GNU General + * Public License. See the file
> > > "COPYING" in the main directory of this + * archive for
> > > more details.
> > > + *
> > > + * This program is distributed in the hope that it will
> > > be useful, + * but WITHOUT ANY WARRANTY; without even the
> > > implied warranty of + * MERCHANTABILITY or FITNESS FOR A
> > > PARTICULAR PURPOSE. See the + * GNU General Public
> > > License for more details.
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/hrtimer.h>
> > > +#include <linux/module.h>
> > > +#include <linux/hwmon.h>
> > > +#include <linux/hwmon-sysfs.h>
> > > +#include <linux/err.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/io.h>
> > > +#include <linux/slab.h>
> > > +
> > > +#include "../../arch/arm/mach-omap2/control.h"
> >
> > No need to do this, you can use syscon here like
> > pbias-regulator.c is doing.
>
> Oh looks like you're already using syscon, nice. What defines
> do you need from control.h?
>
> Those should be in the driver if private to the driver, or
> else we should have some minimal header in include/linux
> somewhere if some control.h defines really need to be
> exposed.
>
> Regards,
>
> Tony

Hi Tony,

Sebastian Reichel has already sent new version of temperature
driver. See email thread with subject:

[PATCH 0/3] OMAP3 temperature sensor

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2014-12-26 16:35:04

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] add omap34xx temperature monitoring support

* Pali Rohár <[email protected]> [141226 08:29]:
> On Friday 26 December 2014 17:17:57 Tony Lindgren wrote:
> > > > +
> > > > +#include "../../arch/arm/mach-omap2/control.h"
> > >
> > > No need to do this, you can use syscon here like
> > > pbias-regulator.c is doing.
> >
> > Oh looks like you're already using syscon, nice. What defines
> > do you need from control.h?
> >
> > Those should be in the driver if private to the driver, or
> > else we should have some minimal header in include/linux
> > somewhere if some control.h defines really need to be
> > exposed.
> >
> > Regards,
> >
> > Tony
>
> Hi Tony,
>
> Sebastian Reichel has already sent new version of temperature
> driver. See email thread with subject:
>
> [PATCH 0/3] OMAP3 temperature sensor

Yeah great, looks like I was also reading Sebastian's patch
with the syscon comments above :)

Regards,

Tony

2014-12-26 17:19:49

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/3] DT Binding for omap3 temperature sensor

On Fri 2014-12-26 13:34:52, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This provides a
> DT binding specification for the temperature monitor.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> .../bindings/hwmon/omap3-temperature.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
> new file mode 100644
> index 0000000..99631ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
> @@ -0,0 +1,25 @@
> +* OMAP3 temperature sensor
> +
> +The OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> +which can be used to determine the SoCs temperature.
> +
> +Requires node properties:
> +- compatible : should contain one of
> + - "ti,omap34xx-temperature-sensor" for OMAP34xx
> + - "ti,omap36xx-temperature-sensor" for OMAP36xx
> +- syscon : Should be a phandle to system configuration node which
> + encompases the temperature register
> +- clocks : Should contain 32KHz fclk clock specifier
> +- clock-names : Should contain clock names
> + - "fck" for the 32KHz fclk clock specifier

I don't quite get it. The temperature sensor is internal on the CPU,
right? Why do we need device tree to describe it? As soon as we have
CPU that is compatible to ti,omap3430, we know everything we need to
know, no?

> +Example for omap34xx:
> +
> +/ {
> + temperature-sensor {
> + compatible = "ti,omap34xx-temperature-sensor";
> + syscon = <&omap3_scm_general>;
> + clocks = <&ts_fck>;
> + clock-names = "fck";
> + };
> +};

Or is there something that depends on the board there? Or do we want
to do it like this to be consistent with existing bindings?

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-26 17:26:33

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

> + if (!data->valid || time_after(jiffies, data->last_updated + HZ)) {
> + clk_enable(data->clk_32k);

This needs to be clk_prepare_enable()

> + regmap_update_bits(data->syscon, SYSCON_TEMP_REG,
> + soc_mask, soc_mask);
> +
> + if (!wait_for_eocz(EOCZ_MIN_RISING_DELAY,
> + EOCZ_MAX_RISING_DELAY, 1, data)) {
> + e = -EIO;
> + goto err;
> + }
> +
> + regmap_update_bits(data->syscon, SYSCON_TEMP_REG, soc_mask, 0);
> +
> + if (!wait_for_eocz(EOCZ_MIN_FALLING_DELAY,
> + EOCZ_MAX_FALLING_DELAY, 0, data)) {
> + e = -EIO;
> + goto err;
> + }
> +
> + regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
> + data->temperature = temp_sensor_reg & ((1<<7) - 1);
> + data->last_updated = jiffies;
> + data->valid = true;
> +
> +err:
> + clk_disable(data->clk_32k);

And this clk_disable_unprepare().

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-26 23:50:48

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 1/3] DT Binding for omap3 temperature sensor

Hi Pavel,

On Fri, Dec 26, 2014 at 06:19:44PM +0100, Pavel Machek wrote:
> On Fri 2014-12-26 13:34:52, Sebastian Reichel wrote:
> > OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > which can be used to determine the SoCs temperature. This provides a
> > DT binding specification for the temperature monitor.
> >
> > Signed-off-by: Sebastian Reichel <[email protected]>
> > ---
> > .../bindings/hwmon/omap3-temperature.txt | 25 ++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
> > new file mode 100644
> > index 0000000..99631ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/omap3-temperature.txt
> > @@ -0,0 +1,25 @@
> > +* OMAP3 temperature sensor
> > +
> > +The OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > +which can be used to determine the SoCs temperature.
> > +
> > +Requires node properties:
> > +- compatible : should contain one of
> > + - "ti,omap34xx-temperature-sensor" for OMAP34xx
> > + - "ti,omap36xx-temperature-sensor" for OMAP36xx
> > +- syscon : Should be a phandle to system configuration node which
> > + encompases the temperature register
> > +- clocks : Should contain 32KHz fclk clock specifier
> > +- clock-names : Should contain clock names
> > + - "fck" for the 32KHz fclk clock specifier
>
> I don't quite get it. The temperature sensor is internal on the CPU,
> right? Why do we need device tree to describe it? As soon as we have
> CPU that is compatible to ti,omap3430, we know everything we need to
> know, no?

Lots of stuff is SoC internal and described in the DT (e.g. serial
controllers). Just have a look in omap3.dtsi or omap34xx.dtsi.

I put the temperature sensor into its own node for the following
reasons:

* syscon reference
* clock reference

I first thought about loading the driver from the syscon driver,
but omap uses a generic one, so that's not an option. Apart from
that one would still need the clock reference.

> > +Example for omap34xx:
> > +
> > +/ {
> > + temperature-sensor {
> > + compatible = "ti,omap34xx-temperature-sensor";
> > + syscon = <&omap3_scm_general>;
> > + clocks = <&ts_fck>;
> > + clock-names = "fck";
> > + };
> > +};
>
> Or is there something that depends on the board there? Or do we want
> to do it like this to be consistent with existing bindings?

This is SoC specific and should go into the omap34xx.dtsi and
omap36xx.dtsi files. See also PATCH 3/3.

-- Sebastian


Attachments:
(No filename) (2.63 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-27 19:09:34

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: dts: OMAP34xx/36xx: Add temperature sensor

On Fri 2014-12-26 13:34:54, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the
> syscon area, which can be used to determine the SoCs temperature.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

Acked-by: Pavel Machek <[email protected]>
Tested-by: Pavel Machek <[email protected]>

Driver binds and does something, so binding works ok.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 19:09:51

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 1/3] DT Binding for omap3 temperature sensor

On Fri 2014-12-26 13:34:52, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This provides a
> DT binding specification for the temperature monitor.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

Acked-by: Pavel Machek <[email protected]>
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 19:24:36

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

Hi!


> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

I did clk_enable -> clk_prepare_enable conversion, as described in
another email.

I got occasional error reading:

root@n900:~# cat /sys/class/hwmon/hwmon0/device/temp_input
20.0
root@n900:~# cat /sys/class/hwmon/hwmon0/device/temp_input
cat: /sys/class/hwmon/hwmon0/device/temp_input: Input/output error
root@n900:~# cat /sys/class/hwmon/hwmon0/device/temp_input
20.0

Nothing in dmesg. It takes few tries to reproduce..

On other attempt, it hung hard:

root@n900:~# cat /sys/class/hwmon/hwmon0/device/temp_input ; cat
/sys/class/hwmon/hwmon0/device/temp_input ;cat
/sys/class/hwmon/hwmon0/device/temp_input ; cat
/sys/class/hwmon/hwmon0/device/temp_input
17.0
17.0
^[[A



^Z


^C^C^C

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 19:40:53

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

I'd suggest these cleanups... But I don't see why it would fail. (Aha,
and sorry for trailing whitespace, you'll probably need to delete it.)

Signed-off-by: Pavel Machek <[email protected]>

diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c
index afe1b5a..8a69604 100644
--- a/drivers/hwmon/omap3-temp.c
+++ b/drivers/hwmon/omap3-temp.c
@@ -35,21 +35,29 @@
/* 32.768Khz clock speed in nano seconds */
#define CLOCK_32K_SPEED_NS 30518

-/* minimum delay for EOCZ rise after SOC rise is
- * 11 cycles of the 32.768Khz clock */
+/*
+ * minimum delay for EOCZ rise after SOC rise is
+ * 11 cycles of the 32.768Khz clock
+ */
#define EOCZ_MIN_RISING_DELAY (11 * CLOCK_32K_SPEED_NS)

-/* From docs, maximum delay for EOCZ rise after SOC rise is
+/*
+ * From docs, maximum delay for EOCZ rise after SOC rise is
* 14 cycles of the 32.768Khz clock. But after some experiments,
- * 24 cycles as maximum is safer. */
+ * 24 cycles as maximum is safer.
+ */
#define EOCZ_MAX_RISING_DELAY (24 * CLOCK_32K_SPEED_NS)

-/* minimum delay for EOCZ falling is
- * 36 cycles of the 32.768Khz clock */
+/*
+ * minimum delay for EOCZ falling is
+ * 36 cycles of the 32.768Khz clock
+ */
#define EOCZ_MIN_FALLING_DELAY (36 * CLOCK_32K_SPEED_NS)

-/* maximum delay for EOCZ falling is
- * 40 cycles of the 32.768Khz clock */
+/*
+ * maximum delay for EOCZ falling is
+ * 40 cycles of the 32.768Khz clock
+ */
#define EOCZ_MAX_FALLING_DELAY (40 * CLOCK_32K_SPEED_NS)

/* temperature register offset in the syscon register area */
@@ -116,8 +124,8 @@ struct omap3_temp_data {
bool valid;
};

-static inline u32 wait_for_eocz(int min_delay, int max_delay, u32 level,
- struct omap3_temp_data *data)
+static inline bool wait_for_eocz(struct omap3_temp_data *data,
+ int min_delay, int max_delay, u32 level)
{
ktime_t timeout, expire;
u32 temp_sensor_reg, eocz_mask;
@@ -133,10 +141,10 @@ static inline u32 wait_for_eocz(int min_delay, int max_delay, u32 level,
do {
regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
if ((temp_sensor_reg & eocz_mask) == level)
- break;
+ return true;
} while (ktime_us_delta(expire, ktime_get()) > 0);

- return (temp_sensor_reg & eocz_mask) == level;
+ return false;
}

static int omap3_temp_update(struct omap3_temp_data *data)
@@ -153,16 +161,16 @@ static int omap3_temp_update(struct omap3_temp_data *data)
regmap_update_bits(data->syscon, SYSCON_TEMP_REG,
soc_mask, soc_mask);

- if (!wait_for_eocz(EOCZ_MIN_RISING_DELAY,
- EOCZ_MAX_RISING_DELAY, 1, data)) {
+ if (!wait_for_eocz(data, EOCZ_MIN_RISING_DELAY,
+ EOCZ_MAX_RISING_DELAY, 1)) {
e = -EIO;
goto err;
}

regmap_update_bits(data->syscon, SYSCON_TEMP_REG, soc_mask, 0);

- if (!wait_for_eocz(EOCZ_MIN_FALLING_DELAY,
- EOCZ_MAX_FALLING_DELAY, 0, data)) {
+ if (!wait_for_eocz(data, EOCZ_MIN_FALLING_DELAY,
+ EOCZ_MAX_FALLING_DELAY, 0)) {
e = -EIO;
goto err;
}


--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 19:48:55

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

When it hangs, it seems to hang here:

if (!wait_for_eocz(data, EOCZ_MIN_RISING_DELAY,
EOCZ_MAX_RISING_DELAY, 1)) {
e = -EIO;
goto
err;
}

...so wait does not seem to reliably wait.
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 19:58:30

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> drivers/hwmon/Kconfig | 8 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 316 insertions(+)
> create mode 100644 drivers/hwmon/omap3-temp.c

When it hangs, it loops here:

do {
regmap_read(data->syscon, SYSCON_TEMP_REG,&temp_sensor_reg);
if ((temp_sensor_reg & eocz_mask) == level)
return true;
printk("=");
}
while (ktime_us_delta(expire, ktime_get()) > 0);

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 22:35:21

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Sat 2014-12-27 20:58:25, Pavel Machek wrote:
> On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> > OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > which can be used to determine the SoCs temperature. This patch provides
> > a DT based driver for the temperature sensor based on an older driver
> > written by Peter De Schrijver for the Nokia N900 and N9.
> >
> > Signed-off-by: Sebastian Reichel <[email protected]>
> > ---
> > drivers/hwmon/Kconfig | 8 ++
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 316 insertions(+)
> > create mode 100644 drivers/hwmon/omap3-temp.c
>
> When it hangs, it loops here:
>
> do {
> regmap_read(data->syscon, SYSCON_TEMP_REG,&temp_sensor_reg);
> if ((temp_sensor_reg & eocz_mask) == level)
> return true;
> printk("=");
> }
> while (ktime_us_delta(expire, ktime_get()) > 0);

And this fixes the hang, and makes level handling more readable.

Fix the timeout code, now it actually works. Driver still fails after
a while.

Signed-off-by: Pavel Machek <[email protected]>

diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c
index 8a69604..1b8c768 100644
--- a/drivers/hwmon/omap3-temp.c
+++ b/drivers/hwmon/omap3-temp.c
@@ -130,9 +130,7 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
ktime_t timeout, expire;
u32 temp_sensor_reg, eocz_mask;

eocz_mask = BIT(data->hwdata->eocz_bit);
- level &= 1;
- level *= eocz_mask;

expire = ktime_add_ns(ktime_get(), max_delay);
timeout = ktime_set(0, min_delay);
@@ -140,9 +141,9 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
schedule_hrtimeout(&timeout, HRTIMER_MODE_REL);
do {
regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
- if ((temp_sensor_reg & eocz_mask) == level)
+ if (!!(temp_sensor_reg & eocz_mask) == level)
return true;
- } while (ktime_us_delta(expire, ktime_get()) > 0);
+ } while (ktime_after(expire, ktime_get()));

return false;
}

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-27 23:26:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.
>
> Signed-off-by: Sebastian Reichel <[email protected]>

As I mentioned before, this version stops working after a
while. Version I originally posted seems to work ok under same load.

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-28 08:25:11

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Sat, Dec 27, 2014 at 11:35:16PM +0100, Pavel Machek wrote:
> On Sat 2014-12-27 20:58:25, Pavel Machek wrote:
> > On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> > > OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > > which can be used to determine the SoCs temperature. This patch provides
> > > a DT based driver for the temperature sensor based on an older driver
> > > written by Peter De Schrijver for the Nokia N900 and N9.
> > >
> > > Signed-off-by: Sebastian Reichel <[email protected]>
> > > ---
> > > drivers/hwmon/Kconfig | 8 ++
> > > drivers/hwmon/Makefile | 1 +
> > > drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 316 insertions(+)
> > > create mode 100644 drivers/hwmon/omap3-temp.c
> >
> > When it hangs, it loops here:
> >
> > do {
> > regmap_read(data->syscon, SYSCON_TEMP_REG,&temp_sensor_reg);
> > if ((temp_sensor_reg & eocz_mask) == level)
> > return true;
> > printk("=");
> > }
> > while (ktime_us_delta(expire, ktime_get()) > 0);
>
> And this fixes the hang, and makes level handling more readable.
>
> Fix the timeout code, now it actually works. Driver still fails after
> a while.
>
> Signed-off-by: Pavel Machek <[email protected]>
>
> diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c
> index 8a69604..1b8c768 100644
> --- a/drivers/hwmon/omap3-temp.c
> +++ b/drivers/hwmon/omap3-temp.c
> @@ -130,9 +130,7 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
> ktime_t timeout, expire;
> u32 temp_sensor_reg, eocz_mask;
>
> eocz_mask = BIT(data->hwdata->eocz_bit);
> - level &= 1;
> - level *= eocz_mask;
>
> expire = ktime_add_ns(ktime_get(), max_delay);
> timeout = ktime_set(0, min_delay);
> @@ -140,9 +141,9 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
> schedule_hrtimeout(&timeout, HRTIMER_MODE_REL);
> do {
> regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
> - if ((temp_sensor_reg & eocz_mask) == level)
> + if (!!(temp_sensor_reg & eocz_mask) == level)
> return true;
> - } while (ktime_us_delta(expire, ktime_get()) > 0);
> + } while (ktime_after(expire, ktime_get()));
>
Does this have to be a hard loop, without sleep ? I am a bit concerned that it
may loop for more than a ms.

Other than that, I assume we'll see an updated version with the coding style
issues and hang-up problems fixed.

Thanks,
Guenter

2014-12-28 10:18:05

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Sun, Dec 28, 2014 at 12:24:47AM -0800, Guenter Roeck wrote:
> On Sat, Dec 27, 2014 at 11:35:16PM +0100, Pavel Machek wrote:
> > On Sat 2014-12-27 20:58:25, Pavel Machek wrote:
> > > On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote:
> > > > OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > > > which can be used to determine the SoCs temperature. This patch provides
> > > > a DT based driver for the temperature sensor based on an older driver
> > > > written by Peter De Schrijver for the Nokia N900 and N9.
> > > >
> > > > Signed-off-by: Sebastian Reichel <[email protected]>
> > > > ---
> > > > drivers/hwmon/Kconfig | 8 ++
> > > > drivers/hwmon/Makefile | 1 +
> > > > drivers/hwmon/omap3-temp.c | 307 +++++++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 316 insertions(+)
> > > > create mode 100644 drivers/hwmon/omap3-temp.c
> > >
> > > When it hangs, it loops here:
> > >
> > > do {
> > > regmap_read(data->syscon, SYSCON_TEMP_REG,&temp_sensor_reg);
> > > if ((temp_sensor_reg & eocz_mask) == level)
> > > return true;
> > > printk("=");
> > > }
> > > while (ktime_us_delta(expire, ktime_get()) > 0);
> >
> > And this fixes the hang, and makes level handling more readable.
> >
> > Fix the timeout code, now it actually works. Driver still fails after
> > a while.
> >
> > Signed-off-by: Pavel Machek <[email protected]>
> >
> > diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c
> > index 8a69604..1b8c768 100644
> > --- a/drivers/hwmon/omap3-temp.c
> > +++ b/drivers/hwmon/omap3-temp.c
> > @@ -130,9 +130,7 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
> > ktime_t timeout, expire;
> > u32 temp_sensor_reg, eocz_mask;
> >
> > eocz_mask = BIT(data->hwdata->eocz_bit);
> > - level &= 1;
> > - level *= eocz_mask;
> >
> > expire = ktime_add_ns(ktime_get(), max_delay);
> > timeout = ktime_set(0, min_delay);
> > @@ -140,9 +141,9 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data,
> > schedule_hrtimeout(&timeout, HRTIMER_MODE_REL);
> > do {
> > regmap_read(data->syscon, SYSCON_TEMP_REG, &temp_sensor_reg);
> > - if ((temp_sensor_reg & eocz_mask) == level)
> > + if (!!(temp_sensor_reg & eocz_mask) == level)
> > return true;
> > - } while (ktime_us_delta(expire, ktime_get()) > 0);
> > + } while (ktime_after(expire, ktime_get()));
> >
> Does this have to be a hard loop, without sleep ? I am a bit concerned that it
> may loop for more than a ms.
>
> Other than that, I assume we'll see an updated version with the coding style
> issues and hang-up problems fixed.

I will send a v2 patchset with Pavels change requests. It may take a
few days though, since I'm currently at a conference (31C3).

-- Sebastian


Attachments:
(No filename) (2.71 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-29 17:53:02

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> which can be used to determine the SoCs temperature. This patch provides
> a DT based driver for the temperature sensor based on an older driver
> written by Peter De Schrijver for the Nokia N900 and N9.

The sensor looks like an earlier iteration of sensors used in newer
OMAPs, which are already supported by maybe
drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
that driver instead?

--
Grazvydas

2014-12-29 18:01:07

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
> On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
>> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
>> which can be used to determine the SoCs temperature. This patch provides
>> a DT based driver for the temperature sensor based on an older driver
>> written by Peter De Schrijver for the Nokia N900 and N9.
>
> The sensor looks like an earlier iteration of sensors used in newer
> OMAPs, which are already supported by maybe
> drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
> that driver instead?

Just to be clear - OMAP4 is the first time that the sensors were
reliable enough to be used.

---
Regards,
Nishanth Menon

2014-12-29 18:16:03

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Mon 2014-12-29 12:01:03, Nishanth Menon wrote:
> On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
> > On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
> >> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> >> which can be used to determine the SoCs temperature. This patch provides
> >> a DT based driver for the temperature sensor based on an older driver
> >> written by Peter De Schrijver for the Nokia N900 and N9.
> >
> > The sensor looks like an earlier iteration of sensors used in newer
> > OMAPs, which are already supported by maybe
> > drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
> > that driver instead?
>
> Just to be clear - OMAP4 is the first time that the sensors were
> reliable enough to be used.

When testing initial version of the patch, they seem to work very well
in the omap3 case.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-29 19:05:24

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Mon, Dec 29, 2014 at 07:15:56PM +0100, Pavel Machek wrote:
> On Mon 2014-12-29 12:01:03, Nishanth Menon wrote:
> > On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
> > > On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
> > >> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > >> which can be used to determine the SoCs temperature. This patch provides
> > >> a DT based driver for the temperature sensor based on an older driver
> > >> written by Peter De Schrijver for the Nokia N900 and N9.
> > >
> > > The sensor looks like an earlier iteration of sensors used in newer
> > > OMAPs, which are already supported by maybe
> > > drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
> > > that driver instead?
> >
> > Just to be clear - OMAP4 is the first time that the sensors were
> > reliable enough to be used.
>
> When testing initial version of the patch, they seem to work very well
> in the omap3 case.
>
Pavel,

can you look into the omap4 thermal driver to see if it can be used ?

Thanks,
Guenter

2014-12-29 20:38:20

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On 12/29/2014 12:15 PM, Pavel Machek wrote:
> On Mon 2014-12-29 12:01:03, Nishanth Menon wrote:
>> On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
>>> On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
>>>> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
>>>> which can be used to determine the SoCs temperature. This patch provides
>>>> a DT based driver for the temperature sensor based on an older driver
>>>> written by Peter De Schrijver for the Nokia N900 and N9.
>>>
>>> The sensor looks like an earlier iteration of sensors used in newer
>>> OMAPs, which are already supported by maybe
>>> drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
>>> that driver instead?
>>
>> Just to be clear - OMAP4 is the first time that the sensors were
>> reliable enough to be used.
>
> When testing initial version of the patch, they seem to work very well
> in the omap3 case.

Just be careful when you try to make thermal policy like decisions
based on this sensor. Placement of the sensor w.r.t the actual logic
generating heat has to be a factor as well. If you are just looking
for an approximation temperature (thermometerish kind), you might be
ok with this. I am not sure we'd find any TI data around this.. just a
heads up.

Also notice http://www.ti.com/lit/er/sprz278f/sprz278f.pdf "Advisory
3.1.1.186 MMC OCP Clock Not Gated When Thermal Sensor Is Used" I think
there were accuracy issues at certain values etc.. So remember to do a
off mode type PM tests as well before you consider requesting these to
be merged.

--
Regards,
Nishanth Menon

2014-12-30 18:00:08

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Mon 2014-12-29 14:35:55, Nishanth Menon wrote:
> On 12/29/2014 12:15 PM, Pavel Machek wrote:
> > On Mon 2014-12-29 12:01:03, Nishanth Menon wrote:
> >> On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
> >>> On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
> >>>> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> >>>> which can be used to determine the SoCs temperature. This patch provides
> >>>> a DT based driver for the temperature sensor based on an older driver
> >>>> written by Peter De Schrijver for the Nokia N900 and N9.
> >>>
> >>> The sensor looks like an earlier iteration of sensors used in newer
> >>> OMAPs, which are already supported by maybe
> >>> drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
> >>> that driver instead?
> >>
> >> Just to be clear - OMAP4 is the first time that the sensors were
> >> reliable enough to be used.
> >
> > When testing initial version of the patch, they seem to work very well
> > in the omap3 case.
>
> Just be careful when you try to make thermal policy like decisions
> based on this sensor. Placement of the sensor w.r.t the actual logic

I guess we won't do that, certainly not anytime soon.

> Also notice http://www.ti.com/lit/er/sprz278f/sprz278f.pdf "Advisory
> 3.1.1.186 MMC OCP Clock Not Gated When Thermal Sensor Is Used" I think
> there were accuracy issues at certain values etc.. So remember to do a
> off mode type PM tests as well before you consider requesting these to
> be merged.

Thanks a lot for a pointer. 3x 100uA power draw when you enable
temperature sensor, because it also errorneously enables MMC. I wonder
how the solution would look in the device tree...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2014-12-30 22:46:27

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor

On Mon 2014-12-29 11:04:48, Guenter Roeck wrote:
> On Mon, Dec 29, 2014 at 07:15:56PM +0100, Pavel Machek wrote:
> > On Mon 2014-12-29 12:01:03, Nishanth Menon wrote:
> > > On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas <[email protected]> wrote:
> > > > On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel <[email protected]> wrote:
> > > >> OMAP34xx and OMAP36xx processors contain a register in the syscon area,
> > > >> which can be used to determine the SoCs temperature. This patch provides
> > > >> a DT based driver for the temperature sensor based on an older driver
> > > >> written by Peter De Schrijver for the Nokia N900 and N9.
> > > >
> > > > The sensor looks like an earlier iteration of sensors used in newer
> > > > OMAPs, which are already supported by maybe
> > > > drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update
> > > > that driver instead?
> > >
> > > Just to be clear - OMAP4 is the first time that the sensors were
> > > reliable enough to be used.
> >
> > When testing initial version of the patch, they seem to work very well
> > in the omap3 case.
> >
> Pavel,
>
> can you look into the omap4 thermal driver to see if it can be used ?

The hardware registers are named the same way...

Unfortunately, TI moves registers around with each release, and OMAP4
stuff is _way_ more complex and maze of ifdefs, too.

static struct temp_sensor_data omap4430_mpu_temp_sensor_data = {
.min_freq = OMAP4430_MIN_FREQ,
.max_freq = OMAP4430_MAX_FREQ,
.max_temp = OMAP4430_MAX_TEMP,
.min_temp = OMAP4430_MIN_TEMP,
.hyst_val = OMAP4430_HYST_VAL,
};

and each define used just once. Would be easier to read and modify if
the ifdefs were removed...

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html