add omap34xx temperature monitoring support

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

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt new file +4 -0

new file mode 100644
index 0000000..99631ad
@@ -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.

Quoted 27 lines · Sebastian Reichel

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

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt new file +11 -0

new file mode 100644
index 0000000..99631ad
@@ -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

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt line 14

Quoted 3 lines

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt new file +3 -0

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

Quoted 5 lines

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt new file +5 -0

+Example for omap34xx:
+
+/ {
+	temperature-sensor {
+		compatible = "ti,omap34xx-temperature-sensor";

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt line 25

Quoted 5 lines

Documentation/devicetree/bindings/hwmon/omap3-temperature.txt new file +5 -0

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

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

drivers/hwmon/omap3-temp.c new file +14 -0

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

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

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

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

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

Quoted 5 lines · Sebastian Reichel

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.

Quoted 7 lines

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

Quoted 16 lines · Pavel Machek

Quoted 5 lines · Sebastian Reichel

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.

Quoted 7 lines

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:

Quoted 7 lines

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

drivers/hwmon/omap3-temp.c +0 -2

index 8a69604..1b8c768 100644
@@ -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);

Quoted 47 lines · Pavel Machek

Quoted 16 lines · Pavel Machek

Quoted 5 lines · Sebastian Reichel

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.

Quoted 7 lines

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:

Quoted 7 lines

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

drivers/hwmon/omap3-temp.c +0 -2

index 8a69604..1b8c768 100644
@@ -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);

drivers/hwmon/omap3-temp.c static inline bool wait_for_eocz(struct omap3_temp_data *data, · line 147

Quoted 9 lines

drivers/hwmon/omap3-temp.c +2 -2

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

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

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.

Quoted 4 lines · Sebastian Reichel

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?

Quoted 6 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 4 lines

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.

Quoted 12 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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?

Quoted 2 lines

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.

Quoted 16 lines · Pavel Machek

Quoted 14 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 3 lines

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 ?

Quoted 16 lines · Pavel Machek

Quoted 14 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 3 lines

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 ?

Quoted 16 lines · Pavel Machek

Quoted 14 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 3 lines

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 ?

Quoted 17 lines · Guenter Roeck

Quoted 16 lines · Pavel Machek

Quoted 14 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 6 lines

Quoted 3 lines

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 ?

After some fixes... yes, it seems to be same hardware.

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

drivers/thermal/ti-soc-thermal/Kconfig +9 -0

index bd4c7be..a49495f 100644
@@ -21,6 +21,15 @@ config TI_THERMAL
 	  This includes trip points definitions, extrapolation rules and
 	  CPU cooling device bindings.
 
+config OMAP3_THERMAL
+	bool "Texas Instruments OMAP3 thermal support"
+	depends on TI_SOC_THERMAL
+	depends on ARCH_OMAP3
+	help
+	  If you say yes here you get thermal support for the Texas Instruments
+	  OMAP3 SoC family. The current chip supported are:
+	   - OMAP3430
+
 config OMAP4_THERMAL
 	bool "Texas Instruments OMAP4 thermal support"
 	depends on TI_SOC_THERMAL

drivers/thermal/ti-soc-thermal/Makefile +1 -0

index 1226b24..2b5b220 100644
@@ -2,5 +2,6 @@ obj-$(CONFIG_TI_SOC_THERMAL)		+= ti-soc-thermal.o
 ti-soc-thermal-y			:= ti-bandgap.o
 ti-soc-thermal-$(CONFIG_TI_THERMAL)	+= ti-thermal-common.o
 ti-soc-thermal-$(CONFIG_DRA752_THERMAL)	+= dra752-thermal-data.o
+ti-soc-thermal-$(CONFIG_OMAP3_THERMAL)  += omap3-thermal-data.o
 ti-soc-thermal-$(CONFIG_OMAP4_THERMAL)	+= omap4-thermal-data.o
 ti-soc-thermal-$(CONFIG_OMAP5_THERMAL)	+= omap5-thermal-data.o

drivers/thermal/ti-soc-thermal/omap3-thermal-data.c new file +99 -0

new file mode 100644
index 0000000..a79ebf2
@@ -0,0 +1,99 @@
+/*
+ * OMAP3 thermal driver.
+ *
+ * Copyright (C) 2011-2012 Texas Instruments Inc.
+ * Copyright (C) 2014 Pavel Machek <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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 "ti-thermal.h"
+#include "ti-bandgap.h"
+
+/*
+ * OMAP34XX has one instance of thermal sensor for MPU
+ * need to describe the individual bit fields
+ */
+static struct temp_sensor_registers
+omap34xx_mpu_temp_sensor_registers = {
+	.temp_sensor_ctrl = 0,
+	.bgap_tempsoff_mask = 0, /* Unused, we don't have POWER_SWITCH */
+	.bgap_soc_mask = BIT(8),
+	.bgap_eocz_mask = BIT(7),
+	.bgap_dtemp_mask = 0x7f,
+
+	.bgap_mode_ctrl = 0,
+	.mode_ctrl_mask = 0,	/* Unused, no MODE_CONFIG */
+
+	.bgap_efuse = 0,
+};
+
+/* Thresholds and limits for OMAP34XX MPU temperature sensor */
+static struct temp_sensor_data omap34xx_mpu_temp_sensor_data = {
+	.min_freq = 32768,
+	.max_freq = 32768,
+	.max_temp = -99000,
+	.min_temp = 99000,
+	.hyst_val = 5000,
+};
+
+/*
+ * Temperature values in milli degree celsius
+ * ADC code values from 530 to 923
+ */
+static const int
+omap34xx_adc_to_temp[] = {
+	-40000, -40000, -40000, -40000, -40000, -39000, -38000, -36000,
+	-34000, -32000, -31000,	-29000, -28000, -26000, -25000, -24000,
+	-22000, -21000, -19000, -18000, -17000, -15000,	-14000, -12000,
+	-11000, -9000, -8000, -7000, -5000, -4000, -2000, -1000, 0000,
+	1000, 3000, 4000, 5000, 7000, 8000, 10000, 11000, 13000, 14000,
+	15000, 17000, 18000, 20000, 21000, 22000, 24000, 25000, 27000,
+	28000, 30000, 31000, 32000, 34000, 35000, 37000, 38000, 39000,
+	41000, 42000, 44000, 45000, 47000, 48000, 49000, 51000, 52000,
+	53000, 55000, 56000, 58000, 59000, 60000, 62000, 63000, 65000,
+	66000, 67000, 69000, 70000, 72000, 73000, 74000, 76000, 77000,
+	79000, 80000, 81000, 83000, 84000, 85000, 87000, 88000, 89000,
+	91000, 92000, 94000, 95000, 96000, 98000, 99000, 100000,
+	102000, 103000, 105000, 106000, 107000, 109000, 110000, 111000,
+	113000, 114000, 116000, 117000, 118000, 120000, 121000, 122000,
+	124000, 124000, 125000, 125000, 125000, 125000,	125000
+};
+
+/* OMAP34XX data */
+const struct ti_bandgap_data omap34xx_data = {
+	.features = TI_BANDGAP_FEATURE_CLK_CTRL,
+	.fclock_name = "ts_fck",
+	.div_ck_name = "ts_fck",
+	.conv_table = omap34xx_adc_to_temp,
+	.adc_start_val = 0,
+	.adc_end_val = 127,
+	.expose_sensor = ti_thermal_expose_sensor,
+	.remove_sensor = ti_thermal_remove_sensor,
+
+	.sensors = {
+		{
+		.registers = &omap34xx_mpu_temp_sensor_registers,
+		.ts_data = &omap34xx_mpu_temp_sensor_data,
+		.domain = "cpu",
+		.slope = 0,
+		.constant = 20000,
+		.slope_pcb = 0,
+		.constant_pcb = 20000,
+		.register_cooling = NULL,
+		.unregister_cooling = NULL,
+		},
+	},
+	.sensor_count = 1,
+
+//	.sensor_count = 0,
+};
+

drivers/thermal/ti-soc-thermal/ti-bandgap.c +73 -54

index 634b6ce..a4180a5 100644
@@ -40,6 +40,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_gpio.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include "ti-bandgap.h"
 

@@ -103,19 +104,15 @@ do {								\
  */
 static int ti_bandgap_power(struct ti_bandgap *bgp, bool on)
 {
-	int i, ret = 0;
+	int i;
 
-	if (!TI_BANDGAP_HAS(bgp, POWER_SWITCH)) {
-		ret = -ENOTSUPP;
-		goto exit;
-	}
+	if (!TI_BANDGAP_HAS(bgp, POWER_SWITCH))
+		return -ENOTSUPP;
 
 	for (i = 0; i < bgp->conf->sensor_count; i++)
 		/* active on 0 */
 		RMW_BITS(bgp, i, temp_sensor_ctrl, bgap_tempsoff_mask, !on);
-
-exit:
-	return ret;
+	return 0;
 }
 
 /**

@@ -154,6 +151,7 @@ static u32 ti_bandgap_read_temp(struct ti_bandgap *bgp, int id)
 	if (TI_BANDGAP_HAS(bgp, FREEZE_BIT))
 		RMW_BITS(bgp, id, bgap_mask_ctrl, mask_freeze_mask, 0);
 
+	printk("Bandgap: ADC is %d\n", temp);
 	return temp;
 }
 

@@ -263,18 +261,13 @@ static
 int ti_bandgap_adc_to_mcelsius(struct ti_bandgap *bgp, int adc_val, int *t)
 {
 	const struct ti_bandgap_data *conf = bgp->conf;
-	int ret = 0;
 
 	/* look up for temperature in the table and return the temperature */
-	if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val) {
-		ret = -ERANGE;
-		goto exit;
-	}
+	if (adc_val < conf->adc_start_val || adc_val > conf->adc_end_val)
+		return -ERANGE;
 
 	*t = bgp->conf->conv_table[adc_val - conf->adc_start_val];
-
-exit:
-	return ret;
+	return 0;
 }
 
 /**

@@ -295,16 +288,14 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc)
 {
 	const struct ti_bandgap_data *conf = bgp->conf;
 	const int *conv_table = bgp->conf->conv_table;
-	int high, low, mid, ret = 0;
+	int high, low, mid;
 
 	low = 0;
 	high = conf->adc_end_val - conf->adc_start_val;
 	mid = (high + low) / 2;
 
-	if (temp < conv_table[low] || temp > conv_table[high]) {
-		ret = -ERANGE;
-		goto exit;
-	}
+	if (temp < conv_table[low] || temp > conv_table[high])
+		return -ERANGE;
 
 	while (low < high) {
 		if (temp < conv_table[mid])

@@ -315,9 +306,7 @@ int ti_bandgap_mcelsius_to_adc(struct ti_bandgap *bgp, long temp, int *adc)
 	}
 
 	*adc = conf->adc_start_val + low;
-
-exit:
-	return ret;
+	return 0;
 }
 
 /**

@@ -343,13 +332,11 @@ int ti_bandgap_add_hyst(struct ti_bandgap *bgp, int adc_val, int hyst_val,
 	 */
 	ret = ti_bandgap_adc_to_mcelsius(bgp, adc_val, &temp);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	temp += hyst_val;
 
 	ret = ti_bandgap_mcelsius_to_adc(bgp, temp, sum);
-
-exit:
 	return ret;
 }
 

@@ -468,22 +455,18 @@ exit:
  */
 static inline int ti_bandgap_validate(struct ti_bandgap *bgp, int id)
 {
-	int ret = 0;
-
 	if (!bgp || IS_ERR(bgp)) {
 		pr_err("%s: invalid bandgap pointer\n", __func__);
-		ret = -EINVAL;
-		goto exit;
+		return -EINVAL;
 	}
 
 	if ((id < 0) || (id >= bgp->conf->sensor_count)) {
 		dev_err(bgp->dev, "%s: sensor id out of range (%d)\n",
 			__func__, id);
-		ret = -ERANGE;
+		return -ERANGE;
 	}
 
-exit:
-	return ret;
+	return 0;
 }
 
 /**

@@ -511,12 +494,10 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val,
 
 	ret = ti_bandgap_validate(bgp, id);
 	if (ret)
-		goto exit;
+		return ret;
 
-	if (!TI_BANDGAP_HAS(bgp, TALERT)) {
-		ret = -ENOTSUPP;
-		goto exit;
-	}
+	if (!TI_BANDGAP_HAS(bgp, TALERT))
+		return -ENOTSUPP;
 
 	ts_data = bgp->conf->sensors[id].ts_data;
 	tsr = bgp->conf->sensors[id].registers;

@@ -529,17 +510,15 @@ static int _ti_bandgap_write_threshold(struct ti_bandgap *bgp, int id, int val,
 	}
 
 	if (ret)
-		goto exit;
-
+		return ret;
+	
 	ret = ti_bandgap_mcelsius_to_adc(bgp, val, &adc_val);
 	if (ret < 0)
-		goto exit;
+		return ret;		
 
 	spin_lock(&bgp->lock);
 	ret = ti_bandgap_update_alert_threshold(bgp, id, adc_val, hot);
 	spin_unlock(&bgp->lock);
-
-exit:
 	return ret;
 }
 

@@ -582,7 +561,7 @@ static int _ti_bandgap_read_threshold(struct ti_bandgap *bgp, int id,
 
 	temp = ti_bandgap_readl(bgp, tsr->bgap_threshold);
 	temp = (temp & mask) >> __ffs(mask);
-	ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp);
+	ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp);
 	if (ret) {
 		dev_err(bgp->dev, "failed to read thot\n");
 		ret = -EIO;

@@ -856,10 +835,11 @@ int ti_bandgap_read_temperature(struct ti_bandgap *bgp, int id,
 	temp = ti_bandgap_read_temp(bgp, id);
 	spin_unlock(&bgp->lock);
 
-	ret |= ti_bandgap_adc_to_mcelsius(bgp, temp, &temp);
+	ret = ti_bandgap_adc_to_mcelsius(bgp, temp, &temp);
 	if (ret)
 		return -EIO;
 
+	printk("Bandgap: temperature is %d\n", temp);
 	*temperature = temp;
 
 	return 0;

@@ -918,6 +898,7 @@ static int
 ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
 {
 	u32 temp = 0, counter = 1000;
+	struct temp_sensor_registers *tsr;
 
 	/* Select single conversion mode */
 	if (TI_BANDGAP_HAS(bgp, MODE_CONFIG))

@@ -925,16 +906,26 @@ ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id)
 
 	/* Start of Conversion = 1 */
 	RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 1);
-	/* Wait until DTEMP is updated */
-	temp = ti_bandgap_read_temp(bgp, id);
 
-	while ((temp == 0) && --counter)
-		temp = ti_bandgap_read_temp(bgp, id);
-	/* REVISIT: Check correct condition for end of conversion */
+	/* Wait for EOCZ going up */
+	tsr = bgp->conf->sensors[id].registers;
+	
+	while (--counter) {
+		if (ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) & tsr->bgap_eocz_mask)
+			break;
+		printk("@");
+	}
 
 	/* Start of Conversion = 0 */
 	RMW_BITS(bgp, id, temp_sensor_ctrl, bgap_soc_mask, 0);
 
+	counter = 1000;
+	while (--counter) {
+		if (!(ti_bandgap_readl(bgp, tsr->temp_sensor_ctrl) & tsr->bgap_eocz_mask))
+			break;
+		printk("#");
+	}
+
 	return 0;
 }
 

@@ -1196,6 +1187,8 @@ int ti_bandgap_probe(struct platform_device *pdev)
 	struct ti_bandgap *bgp;
 	int clk_rate, ret = 0, i;
 
+	printk("ti_bandgap: probe\n");
+
 	bgp = ti_bandgap_build(pdev);
 	if (IS_ERR(bgp)) {
 		dev_err(&pdev->dev, "failed to fetch platform data\n");

@@ -1220,11 +1213,10 @@ int ti_bandgap_probe(struct platform_device *pdev)
 		goto free_irqs;
 	}
 
-	bgp->div_clk = clk_get(NULL,  bgp->conf->div_ck_name);
+	bgp->div_clk = clk_get(NULL, bgp->conf->div_ck_name);
 	ret = IS_ERR(bgp->div_clk);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"failed to request div_ts_ck clock ref\n");
+		dev_err(&pdev->dev, "failed to request div_ts_ck clock ref\n");
 		ret = PTR_ERR(bgp->div_clk);
 		goto free_irqs;
 	}

@@ -1312,6 +1304,8 @@ int ti_bandgap_probe(struct platform_device *pdev)
 	/* Every thing is good? Then expose the sensors */
 	for (i = 0; i < bgp->conf->sensor_count; i++) {
 		char *domain;
+		extern int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
+					     char *domain);
 
 		if (bgp->conf->sensors[i].register_cooling) {
 			ret = bgp->conf->sensors[i].register_cooling(bgp, i);

@@ -1319,12 +1313,29 @@ int ti_bandgap_probe(struct platform_device *pdev)
 				goto remove_sensors;
 		}
 
+		printk("bandgap: exposing sensor: %p\n", bgp->conf->expose_sensor);
+		
 		if (bgp->conf->expose_sensor) {
+			printk("bandgap: calling %p %p\n", bgp->conf->expose_sensor, ti_thermal_expose_sensor);
 			domain = bgp->conf->sensors[i].domain;
 			ret = bgp->conf->expose_sensor(bgp, i, domain);
+			printk("bandgap: done\n");
 			if (ret)
 				goto remove_last_cooling;
 		}
+
+		printk("bandgap: exposing sensor\n");
+		{
+			int t;
+
+			ti_bandgap_force_single_read(bgp, 0);
+
+			t = ti_bandgap_read_temp(bgp, 0);
+			printk("Temperature ADC: %d\n", t);			
+			ti_bandgap_read_temperature(bgp, 0, &t);
+			printk("Temperature: %d\n", t);
+			mdelay(1000);
+		}
 	}
 
 	/*

@@ -1365,6 +1376,8 @@ free_irqs:
 		free_irq(gpio_to_irq(bgp->tshut_gpio), NULL);
 		gpio_free(bgp->tshut_gpio);
 	}
+	printk("ti_bandgap: probe: something failed\n");
+	mdelay(10000);
 
 	return ret;
 }

@@ -1509,6 +1522,12 @@ static SIMPLE_DEV_PM_OPS(ti_bandgap_dev_pm_ops, ti_bandgap_suspend,
 #endif
 
 static const struct of_device_id of_ti_bandgap_match[] = {
+#ifdef CONFIG_OMAP3_THERMAL
+	{
+		.compatible = "ti,omap34xx-bandgap",
+		.data = (void *)&omap34xx_data,
+	},
+#endif
 #ifdef CONFIG_OMAP4_THERMAL
 	{
 		.compatible = "ti,omap4430-bandgap",

drivers/thermal/ti-soc-thermal/ti-bandgap.h +2 -0

index b3adf72..3f05386 100644
@@ -384,6 +384,8 @@ int ti_bandgap_set_sensor_data(struct ti_bandgap *bgp, int id, void *data);
 void *ti_bandgap_get_sensor_data(struct ti_bandgap *bgp, int id);
 int ti_bandgap_get_trend(struct ti_bandgap *bgp, int id, int *trend);
 
+extern const struct ti_bandgap_data omap34xx_data;
+
 #ifdef CONFIG_OMAP4_THERMAL
 extern const struct ti_bandgap_data omap4430_data;
 extern const struct ti_bandgap_data omap4460_data;

drivers/thermal/ti-soc-thermal/ti-thermal-common.c +6 -1

index 5fd0386..f91655f 100644
@@ -328,18 +328,23 @@ int ti_thermal_expose_sensor(struct ti_bandgap *bgp, int id,
 {
 	struct ti_thermal_data *data;
 
+	printk("expose_sensor\n");
+
 	data = ti_bandgap_get_sensor_data(bgp, id);
 
 	if (!data || IS_ERR(data))
 		data = ti_thermal_build_data(bgp, id);
 
-	if (!data)
+	if (!data) {
+		printk("no data\n");	
 		return -EINVAL;
+	}
 
 	/* in case this is specified by DT */
 	data->ti_thermal = thermal_zone_of_sensor_register(bgp->dev, id,
 					data, &ti_of_thermal_ops);
 	if (IS_ERR(data->ti_thermal)) {
+		printk("of_sensor_register failed\n");
 		/* Create thermal zone */
 		data->ti_thermal = thermal_zone_device_register(domain,
 				OMAP_TRIP_NUMBER, 0, data, &ti_thermal_ops,

Signature · 3 lines

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

Quoted 18 lines · Pavel Machek

Quoted 17 lines · Guenter Roeck

Quoted 16 lines · Pavel Machek

Quoted 14 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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.

Quoted 9 lines

Quoted 6 lines

Quoted 3 lines

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 ?

After some fixes... yes, it seems to be same hardware.

So this should be the way to go, but then we have others claim that it should not be done because the OMAP3 sensors are too unreliable to use for thermal decisions. Not really sure where that leaves us. I am kind of opposed to have similar drivers for similar chips in two different subsystems.

Is it possible to add the patch below to the omap thermal driver and not use it for thermal decisions ?

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

Pavel,

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

After some fixes... yes, it seems to be same hardware.

Quoted 8 lines

So this should be the way to go, but then we have others claim that it should not be done because the OMAP3 sensors are too unreliable to use for thermal decisions. Not really sure where that leaves us. I am kind of opposed to have similar drivers for similar chips in two different subsystems.

Is it possible to add the patch below to the omap thermal driver and not use it for thermal decisions ?

Well... noone forces you to enable the driver, and I don't think it will do any thermal decisions on N900 as it is ... so we should be ok.

Plus, it seems to work reasonably well (say +- 5 C), so situation does not seem to be as bad as TI claims.

Nokia was actually using it in production.

Quoted 12 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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?

Quoted 2 lines

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.

Quoted 13 lines · Pavel Machek

Quoted 12 lines · Nishanth Menon

Quoted 10 lines · Grazvydas Ignotas

Quoted 4 lines · Sebastian Reichel

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?

Quoted 5 lines

Quoted 2 lines

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

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.

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

drivers/hwmon/omap34xx_temp.c new file +20 -0

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

Quoted 28 lines · Pavel Machek

drivers/hwmon/omap34xx_temp.c new file +25 -0

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

drivers/hwmon/omap34xx_temp.c line 30

Quoted 5 lines

drivers/hwmon/omap34xx_temp.c new file +5 -0

+#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.

Quoted 31 lines · Tony Lindgren

Quoted 30 lines · Pavel Machek

drivers/hwmon/omap34xx_temp.c new file +2 -0

@@ -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.
+ */

drivers/hwmon/omap34xx_temp.c new file +7 -0

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

Quoted 8 lines

drivers/hwmon/omap34xx_temp.c line 30

Quoted 5 lines

drivers/hwmon/omap34xx_temp.c new file +5 -0

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

Quoted 16 lines · On Friday 26 December 2014 17:17:57 Tony Lindgren wrote:

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+#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,

Quoted 1 line

Tony

Hi Tony,

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

[PATCH 0/3] OMAP3 temperature sensor