[PATCH V11 0/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[PATCH V11 0/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

This is an attempt to finish Martin's great work on the bcm2835
thermal driver. It includes now all Eduardo's suggestions and the
explanations from the Raspberry Pi forum [1].

ChangeLog:
V1 -> V2: added specific settings depending on compatiblity
added trip point based on register
setting up ctrl-register if HW is not enabled by firmware
as per recommendation of Eric (untested)
check that clock frequency is in range
(1.9 - 5MHz - as per comment in clk-bcm2835.c)
V2 -> V4: moved back to thermal (not using bcm sub-directory)
set polling interval to 1second (was 0ms, so interrupt driven)
V5 -> V6: added correct depends in KConfig
removed defined default for RESET_DELAY
removed obvious comments
clarify HW setup comments if not set up by FW already
move clk_prepare_enable to an earlier stage and add error handling
clarify warning when TS-clock runs out of recommended range
clk_disable_unprepare added in bcm2835_thermal_remove
added comment on recommended temperature ranges for SOC
V6 -> V7: removed depends on ARCH_BCM2836 || ARCH_BCM2837 in Kconfig
V7 -> V8: rebased
V8 -> V9: moved to use the thermal framework offset and slope in
thermal_zone_parameters as per request
V9 -> V10: implement support for thermal zone descriptor, define offset and
slope within DT, apply forum explanations, replace symbolic with octal
permissions
V10 -> V11: fix parse issue in bcm2837.dtsi, fix probing and remove unused tzp
(sorry about this crap)

[1] - https://www.raspberrypi.org/forums/viewtopic.php?f=72&t=160289&p=1040448

Stefan Wahren (6):
  of: base: Implement read function for s32 array
  thermal: of-thermal: Implement signed coefficient support
  dt-bindings: Add thermal zone to bcm2835-thermal example
  ARM: dts: bcm283x: Add CPU thermal zone with 1 trip point
  ARM64: dts: bcm2837: Define CPU thermal coefficients
  thermal: bcm2835: add thermal driver for bcm2835 SoC

 .../bindings/thermal/brcm,bcm2835-thermal.txt      |   32 +-
 arch/arm/boot/dts/bcm2835.dtsi                     |    4 +
 arch/arm/boot/dts/bcm2836.dtsi                     |    4 +
 arch/arm/boot/dts/bcm283x.dtsi                     |   21 ++
 arch/arm64/boot/dts/broadcom/bcm2837.dtsi          |    4 +
 drivers/thermal/Kconfig                            |    8 +
 drivers/thermal/Makefile                           |    1 +
 drivers/thermal/bcm2835_thermal.c                  |  315 ++++++++++++++++++++
 drivers/thermal/of-thermal.c                       |    5 +-
 include/linux/of.h                                 |   35 +++
 10 files changed, 423 insertions(+), 6 deletions(-)
 create mode 100644 drivers/thermal/bcm2835_thermal.c

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 1/6] of: base: Implement read function for s32 array

[Stefan Wahren]

In order to read signed thermal coefficients from DT we need a proper
function.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 include/linux/of.h |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323..98a046a 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -474,6 +474,34 @@ static inline int of_property_read_u32_array(const struct device_node *np,
 }
 
 /**
+ * of_property_read_s32_array - Find and read an array of 32 bit signed integers
+ * from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_values:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
+ *
+ * Search for a property in a device node and read 32-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_values is modified only if a valid s32 value can be decoded.
+ */
+static inline int of_property_read_s32_array(const struct device_node *np,
+					     const char *propname,
+					     s32 *out_values, size_t sz)
+{
+	int ret = of_property_read_variable_u32_array(np, propname, out_values,
+						      sz, 0);
+	if (ret >= 0)
+		return 0;
+	else
+		return ret;
+}
+
+/**
  * of_property_read_u64_array - Find and read an array of 64 bit integers
  * from a property.
  *
@@ -676,6 +704,13 @@ static inline int of_property_read_u32_array(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_property_read_s32_array(const struct device_node *np,
+					     const char *propname,
+					     s32 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u64_array(const struct device_node *np,
 					     const char *propname,
 					     u64 *out_values, size_t sz)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Stefan Wahren]

Use the new function of_property_read_s32_array() to prepare
of-thermal for negative coefficients. These are used by
the upcoming bcm2835_thermal driver.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 drivers/thermal/of-thermal.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index d04ec3b..491d58a 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	struct device_node *child = NULL, *gchild;
 	struct __thermal_zone *tz;
 	int ret, i;
-	u32 prop, coef[2];
+	u32 prop;
+	s32 coef[2];
 
 	if (!np) {
 		pr_err("no thermal zone np\n");
@@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
 	 * one sensor per thermal zone. Thus, we are considering
 	 * only the first two values as slope and offset.
 	 */
-	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
+	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
 	if (ret == 0) {
 		tz->slope = coef[0];
 		tz->offset = coef[1];
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 3/6] dt-bindings: Add thermal zone to bcm2835-thermal example

[Stefan Wahren]

Add a thermal zone in order to make the example complete.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../bindings/thermal/brcm,bcm2835-thermal.txt      |   32 +++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
index 474531d..da8c5b7 100644
--- a/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/brcm,bcm2835-thermal.txt
@@ -3,15 +3,39 @@ Binding for Thermal Sensor driver for BCM2835 SoCs.
 Required parameters:
 -------------------
 
-compatible: 	should be one of: "brcm,bcm2835-thermal",
-		"brcm,bcm2836-thermal" or "brcm,bcm2837-thermal"
-reg:		Address range of the thermal registers.
-clocks: 	Phandle of the clock used by the thermal sensor.
+compatible: 		should be one of: "brcm,bcm2835-thermal",
+			"brcm,bcm2836-thermal" or "brcm,bcm2837-thermal"
+reg:			Address range of the thermal registers.
+clocks: 		Phandle of the clock used by the thermal sensor.
+#thermal-sensor-cells:	should be 0 (see thermal.txt)
 
 Example:
 
+thermal-zones {
+	cpu_thermal: cpu-thermal {
+		polling-delay-passive = <0>;
+		polling-delay = <1000>;
+
+		thermal-sensors = <&thermal>;
+
+		trips {
+			cpu-crit {
+				temperature	= <80000>;
+				hysteresis	= <0>;
+				type		= "critical";
+			};
+		};
+
+		coefficients = <(-538)	407000>;
+
+		cooling-maps {
+		};
+	};
+};
+
 thermal: thermal@7e212000 {
 	compatible = "brcm,bcm2835-thermal";
 	reg = <0x7e212000 0x8>;
 	clocks = <&clocks BCM2835_CLOCK_TSENS>;
+	#thermal-sensor-cells = <0>;
 };
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 4/6] ARM: dts: bcm283x: Add CPU thermal zone with 1 trip point

[Stefan Wahren]

As suggested by Eduardo Valentin this adds the thermal zone for
the bcm2835 SoC with its single thermal sensor. We start with
the criticial trip point and leave the cooling devices empty
since we don't have any at the moment. Since the coefficients
could vary depending on the SoC we need to define them separate.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 arch/arm/boot/dts/bcm2835.dtsi |    4 ++++
 arch/arm/boot/dts/bcm2836.dtsi |    4 ++++
 arch/arm/boot/dts/bcm283x.dtsi |   21 +++++++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 0890d97..659b6e9 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -24,6 +24,10 @@
 	};
 };
 
+&cpu_thermal {
+	coefficients = <(-538)	407000>;
+};
+
 /* enable thermal sensor with the correct compatible property set */
 &thermal {
 	compatible = "brcm,bcm2835-thermal";
diff --git a/arch/arm/boot/dts/bcm2836.dtsi b/arch/arm/boot/dts/bcm2836.dtsi
index 519a44f..da3deeb 100644
--- a/arch/arm/boot/dts/bcm2836.dtsi
+++ b/arch/arm/boot/dts/bcm2836.dtsi
@@ -77,6 +77,10 @@
 	interrupts = <8>;
 };
 
+&cpu_thermal {
+	coefficients = <(-538)	407000>;
+};
+
 /* enable thermal sensor with the correct compatible property set */
 &thermal {
 	compatible = "brcm,bcm2836-thermal";
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index a3106aa..9bc0a1c9 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -19,6 +19,26 @@
 		bootargs = "earlyprintk console=ttyAMA0";
 	};
 
+	thermal-zones {
+		cpu_thermal: cpu-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <1000>;
+
+			thermal-sensors = <&thermal>;
+
+			trips {
+				cpu-crit {
+					temperature	= <80000>;
+					hysteresis	= <0>;
+					type		= "critical";
+				};
+			};
+
+			cooling-maps {
+			};
+		};
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -394,6 +414,7 @@
 			compatible = "brcm,bcm2835-thermal";
 			reg = <0x7e212000 0x8>;
 			clocks = <&clocks BCM2835_CLOCK_TSENS>;
+			#thermal-sensor-cells = <0>;
 			status = "disabled";
 		};
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 5/6] ARM64: dts: bcm2837: Define CPU thermal coefficients

[Stefan Wahren]

This defines the bcm2837 SoC specific thermal coefficients in
order to initialize the thermal driver correctly.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 arch/arm64/boot/dts/broadcom/bcm2837.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2837.dtsi b/arch/arm64/boot/dts/broadcom/bcm2837.dtsi
index 19f2fe6..2d5de6f0 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2837.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcm2837.dtsi
@@ -75,6 +75,10 @@
 	interrupts = <8>;
 };
 
+&cpu_thermal {
+	coefficients = <(-538)	412000>;
+};
+
 /* enable thermal sensor with the correct compatible property set */
 &thermal {
 	compatible = "brcm,bcm2837-thermal";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

Add basic thermal driver for bcm2835 SoC.

This driver currently make sure that tsense HW block is set up
correctly.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
 drivers/thermal/Kconfig           |    8 +
 drivers/thermal/Makefile          |    1 +
 drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
 3 files changed, 324 insertions(+)
 create mode 100644 drivers/thermal/bcm2835_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b343..3bd2406 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -453,4 +453,12 @@ config ZX2967_THERMAL
 	  the primitive temperature sensor embedded in zx2967 SoCs.
 	  This sensor generates the real time die temperature.
 
+config BCM2835_THERMAL
+	tristate "Thermal sensors on bcm2835 SoC"
+	depends on ARCH_BCM2835 || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on THERMAL_OF
+	help
+	  Support for thermal sensors on Broadcom bcm2835 SoCs.
+
 endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae20..f23cde0 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
 obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
 obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
+obj-$(CONFIG_BCM2835_THERMAL)	+= bcm2835_thermal.o
diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
new file mode 100644
index 0000000..a9768f1
--- /dev/null
+++ b/drivers/thermal/bcm2835_thermal.c
@@ -0,0 +1,315 @@
+/*
+ * Driver for Broadcom BCM2835 SoC temperature sensor
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define BCM2835_TS_TSENSCTL			0x00
+#define BCM2835_TS_TSENSSTAT			0x04
+
+#define BCM2835_TS_TSENSCTL_PRWDW		BIT(0)
+#define BCM2835_TS_TSENSCTL_RSTB		BIT(1)
+
+/*
+ * bandgap reference voltage in 6 mV increments
+ * 000b = 1178 mV, 001b = 1184 mV, ... 111b = 1220 mV
+ */
+#define BCM2835_TS_TSENSCTL_CTRL_BITS		3
+#define BCM2835_TS_TSENSCTL_CTRL_SHIFT		2
+#define BCM2835_TS_TSENSCTL_CTRL_MASK		    \
+	GENMASK(BCM2835_TS_TSENSCTL_CTRL_BITS +     \
+		BCM2835_TS_TSENSCTL_CTRL_SHIFT - 1, \
+		BCM2835_TS_TSENSCTL_CTRL_SHIFT)
+#define BCM2835_TS_TSENSCTL_CTRL_DEFAULT	1
+#define BCM2835_TS_TSENSCTL_EN_INT		BIT(5)
+#define BCM2835_TS_TSENSCTL_DIRECT		BIT(6)
+#define BCM2835_TS_TSENSCTL_CLR_INT		BIT(7)
+#define BCM2835_TS_TSENSCTL_THOLD_SHIFT		8
+#define BCM2835_TS_TSENSCTL_THOLD_BITS		10
+#define BCM2835_TS_TSENSCTL_THOLD_MASK		     \
+	GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
+		BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
+		BCM2835_TS_TSENSCTL_THOLD_SHIFT)
+/*
+ * time how long the block to be asserted in reset
+ * which based on a clock counter (TSENS clock assumed)
+ */
+#define BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT	18
+#define BCM2835_TS_TSENSCTL_RSTDELAY_BITS	8
+#define BCM2835_TS_TSENSCTL_REGULEN		BIT(26)
+
+#define BCM2835_TS_TSENSSTAT_DATA_BITS		10
+#define BCM2835_TS_TSENSSTAT_DATA_SHIFT		0
+#define BCM2835_TS_TSENSSTAT_DATA_MASK		     \
+	GENMASK(BCM2835_TS_TSENSSTAT_DATA_BITS +     \
+		BCM2835_TS_TSENSSTAT_DATA_SHIFT - 1, \
+		BCM2835_TS_TSENSSTAT_DATA_SHIFT)
+#define BCM2835_TS_TSENSSTAT_VALID		BIT(10)
+#define BCM2835_TS_TSENSSTAT_INTERRUPT		BIT(11)
+
+struct bcm2835_thermal_data {
+	struct thermal_zone_device *tz;
+	void __iomem *regs;
+	struct clk *clk;
+	struct dentry *debugfsdir;
+};
+
+static int bcm2835_thermal_adc2temp(u32 adc, int offset, int slope)
+{
+	return offset + slope * adc;
+}
+
+static int bcm2835_thermal_temp2adc(int temp, int offset, int slope)
+{
+	temp -= offset;
+	temp /= slope;
+
+	if (temp < 0)
+		temp = 0;
+	if (temp >= BIT(BCM2835_TS_TSENSSTAT_DATA_BITS))
+		temp = BIT(BCM2835_TS_TSENSSTAT_DATA_BITS) - 1;
+
+	return temp;
+}
+
+static int bcm2835_thermal_get_temp(void *d, int *temp)
+{
+	struct bcm2835_thermal_data *data = d;
+	u32 val = readl(data->regs + BCM2835_TS_TSENSSTAT);
+
+	if (!(val & BCM2835_TS_TSENSSTAT_VALID))
+		return -EIO;
+
+	val &= BCM2835_TS_TSENSSTAT_DATA_MASK;
+
+	*temp = bcm2835_thermal_adc2temp(
+		val,
+		thermal_zone_get_offset(data->tz),
+		thermal_zone_get_slope(data->tz));
+
+	return 0;
+}
+
+static const struct debugfs_reg32 bcm2835_thermal_regs[] = {
+	{
+		.name = "ctl",
+		.offset = 0
+	},
+	{
+		.name = "stat",
+		.offset = 4
+	}
+};
+
+static void bcm2835_thermal_debugfs(struct platform_device *pdev)
+{
+	struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+	struct bcm2835_thermal_data *data = tz->devdata;
+	struct debugfs_regset32 *regset;
+
+	data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL);
+	if (!data->debugfsdir)
+		return;
+
+	regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->regs = bcm2835_thermal_regs;
+	regset->nregs = ARRAY_SIZE(bcm2835_thermal_regs);
+	regset->base = data->regs;
+
+	debugfs_create_regset32("regset", 0444, data->debugfsdir, regset);
+}
+
+static struct thermal_zone_of_device_ops bcm2835_thermal_ops = {
+	.get_temp = bcm2835_thermal_get_temp,
+};
+
+static const struct of_device_id bcm2835_thermal_of_match_table[];
+static int bcm2835_thermal_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct thermal_zone_device *tz;
+	struct bcm2835_thermal_data *data;
+	struct resource *res;
+	int err = 0;
+	u32 val;
+	unsigned long rate;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	match = of_match_device(bcm2835_thermal_of_match_table,
+				&pdev->dev);
+	if (!match)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->regs)) {
+		err = PTR_ERR(data->regs);
+		dev_err(&pdev->dev, "Could not get registers: %d\n", err);
+		return err;
+	}
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		err = PTR_ERR(data->clk);
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Could not get clk: %d\n", err);
+		return err;
+	}
+
+	err = clk_prepare_enable(data->clk);
+	if (err)
+		return err;
+
+	rate = clk_get_rate(data->clk);
+	if ((rate < 1920000) || (rate > 5000000))
+		dev_warn(&pdev->dev,
+			 "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92 to 5MHz\n",
+			 data->clk, data->clk);
+
+	/* register of thermal sensor and get info from DT */
+	tz = thermal_zone_of_sensor_register(&pdev->dev, 0, data,
+					     &bcm2835_thermal_ops);
+	if (IS_ERR(tz)) {
+		err = PTR_ERR(tz);
+		dev_err(&pdev->dev,
+			"Failed to register the thermal device: %d\n",
+			err);
+		goto err_clk;
+	}
+
+	/*
+	 * right now the FW does set up the HW-block, so we are not
+	 * touching the configuration registers.
+	 * But if the HW is not enabled, then set it up
+	 * using "sane" values used by the firmware right now.
+	 */
+	val = readl(data->regs + BCM2835_TS_TSENSCTL);
+	if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
+		int trip_temp, offset, slope;
+
+		slope = thermal_zone_get_slope(tz);
+		offset = thermal_zone_get_offset(tz);
+		/*
+		 * For now we deal only with critical, otherwise
+		 * would need to iterate
+		 */
+		err = tz->ops->get_trip_temp(tz, 0, &trip_temp);
+		if (err < 0) {
+			err = PTR_ERR(tz);
+			dev_err(&pdev->dev,
+				"Not able to read trip_temp: %d\n",
+				err);
+			goto err_tz;
+		}
+
+		/* set bandgap reference voltage and enable voltage regulator */
+		val = (BCM2835_TS_TSENSCTL_CTRL_DEFAULT <<
+		       BCM2835_TS_TSENSCTL_CTRL_SHIFT) |
+		      BCM2835_TS_TSENSCTL_REGULEN;
+
+		/* use the recommended reset duration */
+		val |= (0xFE << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
+
+		/*  trip_adc value from info */
+		val |= bcm2835_thermal_temp2adc(trip_temp,
+						offset,
+						slope)
+			<< BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+		/* write the value back to the register as 2 steps */
+		writel(val, data->regs + BCM2835_TS_TSENSCTL);
+		val |= BCM2835_TS_TSENSCTL_RSTB;
+		writel(val, data->regs + BCM2835_TS_TSENSCTL);
+	}
+
+	data->tz = tz;
+
+	platform_set_drvdata(pdev, tz);
+
+	bcm2835_thermal_debugfs(pdev);
+
+	return 0;
+err_tz:
+	thermal_zone_of_sensor_unregister(&pdev->dev, tz);
+err_clk:
+	clk_disable_unprepare(data->clk);
+
+	return err;
+}
+
+static int bcm2835_thermal_remove(struct platform_device *pdev)
+{
+	struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+	struct bcm2835_thermal_data *data = tz->devdata;
+
+	debugfs_remove_recursive(data->debugfsdir);
+	thermal_zone_of_sensor_unregister(&pdev->dev, tz);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+/*
+ * Note: as per Raspberry Foundation FAQ
+ * (https://www.raspberrypi.org/help/faqs/#performanceOperatingTemperature)
+ * the recommended temperature range for the SoC -40C to +85C
+ * so the trip limit is set to 80C.
+ * this applies to all the BCM283X SoC
+ */
+
+static const struct of_device_id bcm2835_thermal_of_match_table[] = {
+	{
+		.compatible = "brcm,bcm2835-thermal",
+	},
+	{
+		.compatible = "brcm,bcm2836-thermal",
+	},
+	{
+		.compatible = "brcm,bcm2837-thermal",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
+
+static struct platform_driver bcm2835_thermal_driver = {
+	.probe = bcm2835_thermal_probe,
+	.remove = bcm2835_thermal_remove,
+	.driver = {
+		.name = "bcm2835_thermal",
+		.of_match_table = bcm2835_thermal_of_match_table,
+	},
+};
+module_platform_driver(bcm2835_thermal_driver);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 0/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

> Stefan Wahren <stefan.wahren@i2se.com> hat am 12. März 2017 um 23:10 geschrieben:
> 
> 
> This is an attempt to finish Martin's great work on the bcm2835
> thermal driver. It includes now all Eduardo's suggestions and the
> explanations from the Raspberry Pi forum [1].
> 
> ChangeLog:
> V1 -> V2: added specific settings depending on compatiblity
> added trip point based on register
> setting up ctrl-register if HW is not enabled by firmware
> as per recommendation of Eric (untested)
> check that clock frequency is in range
> (1.9 - 5MHz - as per comment in clk-bcm2835.c)
> V2 -> V4: moved back to thermal (not using bcm sub-directory)
> set polling interval to 1second (was 0ms, so interrupt driven)
> V5 -> V6: added correct depends in KConfig
> removed defined default for RESET_DELAY
> removed obvious comments
> clarify HW setup comments if not set up by FW already
> move clk_prepare_enable to an earlier stage and add error handling
> clarify warning when TS-clock runs out of recommended range
> clk_disable_unprepare added in bcm2835_thermal_remove
> added comment on recommended temperature ranges for SOC
> V6 -> V7: removed depends on ARCH_BCM2836 || ARCH_BCM2837 in Kconfig
> V7 -> V8: rebased
> V8 -> V9: moved to use the thermal framework offset and slope in
> thermal_zone_parameters as per request
> V9 -> V10: implement support for thermal zone descriptor, define offset and
> slope within DT, apply forum explanations, replace symbolic with octal
> permissions
> V10 -> V11: fix parse issue in bcm2837.dtsi, fix probing and remove unused tzp
> (sorry about this crap)
> 
> [1] - https://www.raspberrypi.org/forums/viewtopic.php?f=72&t=160289&p=1040448
> 

gentle ping ...

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Nobuhiro Iwamatsu]

Hi, Stefan.

2017-03-13 7:11 GMT+09:00 Stefan Wahren <stefan.wahren@i2se.com>:
> Add basic thermal driver for bcm2835 SoC.
>
> This driver currently make sure that tsense HW block is set up
> correctly.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/thermal/Kconfig           |    8 +
>  drivers/thermal/Makefile          |    1 +
>  drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 324 insertions(+)
>  create mode 100644 drivers/thermal/bcm2835_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b343..3bd2406 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>           the primitive temperature sensor embedded in zx2967 SoCs.
>           This sensor generates the real time die temperature.
>
> +config BCM2835_THERMAL
> +       tristate "Thermal sensors on bcm2835 SoC"
> +       depends on ARCH_BCM2835 || COMPILE_TEST
> +       depends on HAS_IOMEM
> +       depends on THERMAL_OF
> +       help
> +         Support for thermal sensors on Broadcom bcm2835 SoCs.
> +
>  endif
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 7adae20..f23cde0 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
>  obj-$(CONFIG_MTK_THERMAL)      += mtk_thermal.o
>  obj-$(CONFIG_GENERIC_ADC_THERMAL)      += thermal-generic-adc.o
>  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
> +obj-$(CONFIG_BCM2835_THERMAL)  += bcm2835_thermal.o
> diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
> new file mode 100644
> index 0000000..a9768f1
> --- /dev/null
> +++ b/drivers/thermal/bcm2835_thermal.c

<snip>

> +static struct thermal_zone_of_device_ops bcm2835_thermal_ops = {
> +       .get_temp = bcm2835_thermal_get_temp,
> +};
> +
> +static const struct of_device_id bcm2835_thermal_of_match_table[];

bcm2835_thermal_of_match_table has been defined twice.
I think that it is better to define bcm2835_thermal_of_match_table below
or move bcm2835_thermal_probe to bottom.

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

Hi Nobuhiro,

> Nobuhiro Iwamatsu <iwamatsu-+mkmVskJBflAfugRpC6u6w@public.gmane.org> hat am 23. März 2017 um 17:53 geschrieben:
> 
> 
> Hi, Stefan.
> 
> 2017-03-13 7:11 GMT+09:00 Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>:
> > Add basic thermal driver for bcm2835 SoC.
> >
> > This driver currently make sure that tsense HW block is set up
> > correctly.
> >
> > Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> > ---
> >  drivers/thermal/Kconfig           |    8 +
> >  drivers/thermal/Makefile          |    1 +
> >  drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 324 insertions(+)
> >  create mode 100644 drivers/thermal/bcm2835_thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 776b343..3bd2406 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -453,4 +453,12 @@ config ZX2967_THERMAL
> >           the primitive temperature sensor embedded in zx2967 SoCs.
> >           This sensor generates the real time die temperature.
> >
> > +config BCM2835_THERMAL
> > +       tristate "Thermal sensors on bcm2835 SoC"
> > +       depends on ARCH_BCM2835 || COMPILE_TEST
> > +       depends on HAS_IOMEM
> > +       depends on THERMAL_OF
> > +       help
> > +         Support for thermal sensors on Broadcom bcm2835 SoCs.
> > +
> >  endif
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 7adae20..f23cde0 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
> >  obj-$(CONFIG_MTK_THERMAL)      += mtk_thermal.o
> >  obj-$(CONFIG_GENERIC_ADC_THERMAL)      += thermal-generic-adc.o
> >  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
> > +obj-$(CONFIG_BCM2835_THERMAL)  += bcm2835_thermal.o
> > diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
> > new file mode 100644
> > index 0000000..a9768f1
> > --- /dev/null
> > +++ b/drivers/thermal/bcm2835_thermal.c
> 
> <snip>
> 
> > +static struct thermal_zone_of_device_ops bcm2835_thermal_ops = {
> > +       .get_temp = bcm2835_thermal_get_temp,
> > +};
> > +
> > +static const struct of_device_id bcm2835_thermal_of_match_table[];
> 
> bcm2835_thermal_of_match_table has been defined twice.
> I think that it is better to define bcm2835_thermal_of_match_table below
> or move bcm2835_thermal_probe to bottom.

thanks for catching. I'm currently waiting for feedback from Eduardo and Rob before sending a new version.

Stefan

> 
> Best regards,
>   Nobuhiro
> 
> -- 
> Nobuhiro Iwamatsu
>    iwamatsu at {nigauri.org / debian.org}
>    GPG ID: 40AD1FA6
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 1/6] of: base: Implement read function for s32 array

[Frank Rowand]

On 03/12/17 15:11, Stefan Wahren wrote:
> In order to read signed thermal coefficients from DT we need a proper
> function.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  include/linux/of.h |   35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 21e6323..98a046a 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -474,6 +474,34 @@ static inline int of_property_read_u32_array(const struct device_node *np,
>  }
>  
>  /**
> + * of_property_read_s32_array - Find and read an array of 32 bit signed integers
> + * from a property.
> + *
> + * @np:		device node from which the property value is to be read.
> + * @propname:	name of the property to be searched.
> + * @out_values:	pointer to return value, modified only if return value is 0.
> + * @sz:		number of array elements to read
> + *
> + * Search for a property in a device node and read 32-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_values is modified only if a valid s32 value can be decoded.
> + */
> +static inline int of_property_read_s32_array(const struct device_node *np,
> +					     const char *propname,
> +					     s32 *out_values, size_t sz)
> +{
> +	int ret = of_property_read_variable_u32_array(np, propname, out_values,
> +						      sz, 0);

Add the type coercion of out_values to make it obvious:

+       int ret = of_property_read_variable_u32_array(np, propname,
+                                                     (u32*) out_values, sz, 0);

> +	if (ret >= 0)
> +		return 0;
> +	else
> +		return ret;
> +}
> +
> +/**
>   * of_property_read_u64_array - Find and read an array of 64 bit integers
>   * from a property.
>   *
> @@ -676,6 +704,13 @@ static inline int of_property_read_u32_array(const struct device_node *np,
>  	return -ENOSYS;
>  }
>  
> +static inline int of_property_read_s32_array(const struct device_node *np,
> +					     const char *propname,
> +					     s32 *out_values, size_t sz)
> +{
> +	return -ENOSYS;
> +}
> +
>  static inline int of_property_read_u64_array(const struct device_node *np,
>  					     const char *propname,
>  					     u64 *out_values, size_t sz)
> 

Reviewed-by: Frank Rowand <frowand.list@gmail.com>

Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Frank Rowand]

On 03/12/17 15:11, Stefan Wahren wrote:
> Use the new function of_property_read_s32_array() to prepare
> of-thermal for negative coefficients. These are used by
> the upcoming bcm2835_thermal driver.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  drivers/thermal/of-thermal.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d04ec3b..491d58a 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	struct device_node *child = NULL, *gchild;
>  	struct __thermal_zone *tz;
>  	int ret, i;
> -	u32 prop, coef[2];
> +	u32 prop;
> +	s32 coef[2];
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	 * one sensor per thermal zone. Thus, we are considering
>  	 * only the first two values as slope and offset.
>  	 */
> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
>  	if (ret == 0) {
>  		tz->slope = coef[0];
>  		tz->offset = coef[1];
> 

Since you are doing so much work to fix reading the array of s32 property, you
might also want to do the same for the s32 properties, "polling-delay-passive"
and "polling-delay".  Just change of_property_read_u32() to of_property_read_s32()
and change the type of prop to match.


drivers/thermal/of-thermal.c: In function 'thermal_of_build_thermal_zone':
drivers/thermal/of-thermal.c:841:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
drivers/thermal/of-thermal.c:848:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]


 836         ret = of_property_read_u32(np, "polling-delay-passive", &prop);
 837         if (ret < 0) {
 838                 pr_err("missing polling-delay-passive property\n");
 839                 goto free_tz;
 840         }
 841         tz->passive_delay = prop;
 842
 843         ret = of_property_read_u32(np, "polling-delay", &prop);
 844         if (ret < 0) {
 845                 pr_err("missing polling-delay property\n");
 846                 goto free_tz;
 847         }
 848         tz->polling_delay = prop;


-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Stefan Wahren]

> Frank Rowand <frowand.list@gmail.com> hat am 24. März 2017 um 00:32 geschrieben:
> 
> 
> On 03/12/17 15:11, Stefan Wahren wrote:
> > Use the new function of_property_read_s32_array() to prepare
> > of-thermal for negative coefficients. These are used by
> > the upcoming bcm2835_thermal driver.
> > 
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> >  drivers/thermal/of-thermal.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> > index d04ec3b..491d58a 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
> >  	struct device_node *child = NULL, *gchild;
> >  	struct __thermal_zone *tz;
> >  	int ret, i;
> > -	u32 prop, coef[2];
> > +	u32 prop;
> > +	s32 coef[2];
> >  
> >  	if (!np) {
> >  		pr_err("no thermal zone np\n");
> > @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
> >  	 * one sensor per thermal zone. Thus, we are considering
> >  	 * only the first two values as slope and offset.
> >  	 */
> > -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> > +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
> >  	if (ret == 0) {
> >  		tz->slope = coef[0];
> >  		tz->offset = coef[1];
> > 
> 
> Since you are doing so much work to fix reading the array of s32 property, you
> might also want to do the same for the s32 properties, "polling-delay-passive"
> and "polling-delay".  Just change of_property_read_u32() to of_property_read_s32()
> and change the type of prop to match.
> 

The intension behind this patch series is adding a new thermal driver not fixing of-thermal. Since the initial version of this series was posted by Martin in May 2016 i do not want to wait much longer.

Btw changing polling-delay-passive and polling-delay into a signed doesn't make any sense to me. Why do we need negative delays?

I suggest to send a separate patch for this issue.

Thanks for the review.

> 
> drivers/thermal/of-thermal.c: In function 'thermal_of_build_thermal_zone':
> drivers/thermal/of-thermal.c:841:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
> drivers/thermal/of-thermal.c:848:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
> 
> 
>  836         ret = of_property_read_u32(np, "polling-delay-passive", &prop);
>  837         if (ret < 0) {
>  838                 pr_err("missing polling-delay-passive property\n");
>  839                 goto free_tz;
>  840         }
>  841         tz->passive_delay = prop;
>  842
>  843         ret = of_property_read_u32(np, "polling-delay", &prop);
>  844         if (ret < 0) {
>  845                 pr_err("missing polling-delay property\n");
>  846                 goto free_tz;
>  847         }
>  848         tz->polling_delay = prop;
> 
> 
> -Frank

Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Frank Rowand]

On 03/24/17 00:27, Stefan Wahren wrote:
> 
>> Frank Rowand <frowand.list@gmail.com> hat am 24. März 2017 um 00:32 geschrieben:
>>
>>
>> On 03/12/17 15:11, Stefan Wahren wrote:
>>> Use the new function of_property_read_s32_array() to prepare
>>> of-thermal for negative coefficients. These are used by
>>> the upcoming bcm2835_thermal driver.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>>  drivers/thermal/of-thermal.c |    5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>> index d04ec3b..491d58a 100644
>>> --- a/drivers/thermal/of-thermal.c
>>> +++ b/drivers/thermal/of-thermal.c
>>> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>>>  	struct device_node *child = NULL, *gchild;
>>>  	struct __thermal_zone *tz;
>>>  	int ret, i;
>>> -	u32 prop, coef[2];
>>> +	u32 prop;
>>> +	s32 coef[2];
>>>  
>>>  	if (!np) {
>>>  		pr_err("no thermal zone np\n");
>>> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>>>  	 * one sensor per thermal zone. Thus, we are considering
>>>  	 * only the first two values as slope and offset.
>>>  	 */
>>> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
>>> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
>>>  	if (ret == 0) {
>>>  		tz->slope = coef[0];
>>>  		tz->offset = coef[1];
>>>
>>
>> Since you are doing so much work to fix reading the array of s32 property, you
>> might also want to do the same for the s32 properties, "polling-delay-passive"
>> and "polling-delay".  Just change of_property_read_u32() to of_property_read_s32()
>> and change the type of prop to match.
>>
> 
> The intension behind this patch series is adding a new thermal driver
> not fixing of-thermal. Since the initial version of this series was
> posted by Martin in May 2016 i do not want to wait much longer.

Not my driver, your choice, I was just pointing out some compile warnings
(without thinking enough about the context of the warnings - see below).

> 
> Btw changing polling-delay-passive and polling-delay into a signed
> doesn't make any sense to me. Why do we need negative delays?

Good point.  I did not actually look at what the meaning of the two
properties is, and whether it makes sense for them to have a negative
value.  I also did not check the binding description (doing so now,
it clearly states that these property values are unsigned), I just
noted that there is a mismatch between the type of the properties
(u32) and the variables they are assigned to.  As the compile warnings
below indicate, if the property value is large enough, it will be a
negative value after assignment to the int variable.  The proper
answer is probably to change the variables to unsigned.

> 
> I suggest to send a separate patch for this issue.

It would be good if someone did.

Not a critical issue, just a trap waiting to catch someone who puts
a very large value for one of those properties in a device tree source
file and does not realize it will be a negative number in the driver.

> 
> Thanks for the review.
> 
>>
>> drivers/thermal/of-thermal.c: In function 'thermal_of_build_thermal_zone':
>> drivers/thermal/of-thermal.c:841:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
>> drivers/thermal/of-thermal.c:848:2: warning: conversion to 'int' from 'u32' may change the sign of the result [-Wsign-conversion]
>>
>>
>>  836         ret = of_property_read_u32(np, "polling-delay-passive", &prop);
>>  837         if (ret < 0) {
>>  838                 pr_err("missing polling-delay-passive property\n");
>>  839                 goto free_tz;
>>  840         }
>>  841         tz->passive_delay = prop;
>>  842
>>  843         ret = of_property_read_u32(np, "polling-delay", &prop);
>>  844         if (ret < 0) {
>>  845                 pr_err("missing polling-delay property\n");
>>  846                 goto free_tz;
>>  847         }
>>  848         tz->polling_delay = prop;
>>
>>
>> -Frank
> 

Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Eduardo Valentin]

Stefen,

On Sun, Mar 12, 2017 at 10:11:01PM +0000, Stefan Wahren wrote:
> Use the new function of_property_read_s32_array() to prepare
> of-thermal for negative coefficients. These are used by
> the upcoming bcm2835_thermal driver.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>

Once done with the comment pointed by Frank, you can add my
Reviewed-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/thermal/of-thermal.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d04ec3b..491d58a 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	struct device_node *child = NULL, *gchild;
>  	struct __thermal_zone *tz;
>  	int ret, i;
> -	u32 prop, coef[2];
> +	u32 prop;
> +	s32 coef[2];
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	 * one sensor per thermal zone. Thus, we are considering
>  	 * only the first two values as slope and offset.
>  	 */
> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);
>  	if (ret == 0) {
>  		tz->slope = coef[0];
>  		tz->offset = coef[1];
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 2/6] thermal: of-thermal: Implement signed coefficient support

[Eduardo Valentin]

On Sun, Mar 12, 2017 at 10:11:01PM +0000, Stefan Wahren wrote:
> Use the new function of_property_read_s32_array() to prepare
> of-thermal for negative coefficients. These are used by
> the upcoming bcm2835_thermal driver.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  drivers/thermal/of-thermal.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d04ec3b..491d58a 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -821,7 +821,8 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	struct device_node *child = NULL, *gchild;
>  	struct __thermal_zone *tz;
>  	int ret, i;
> -	u32 prop, coef[2];
> +	u32 prop;
> +	s32 coef[2];
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -851,7 +852,7 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>  	 * one sensor per thermal zone. Thus, we are considering
>  	 * only the first two values as slope and offset.
>  	 */
> -	ret = of_property_read_u32_array(np, "coefficients", coef, 2);
> +	ret = of_property_read_s32_array(np, "coefficients", coef, 2);

For the scope of the proposed change I see not problem with this patch.

However, this needs to go in after the of core change gets accepted.
Alternatively, I could take the full series if you get an Ack from OF
maintainers.

>  	if (ret == 0) {
>  		tz->slope = coef[0];
>  		tz->offset = coef[1];
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Eduardo Valentin]

On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> Add basic thermal driver for bcm2835 SoC.
> 
> This driver currently make sure that tsense HW block is set up
> correctly.
> 
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

I see no issue with this driver at this point.

As I mentioned, the full series needs to go together. You either get an
Ack from OF maintainers, and I take the series, or you can add my

Acked-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

after fixing the small comment from Nobuhiro.

> ---
>  drivers/thermal/Kconfig           |    8 +
>  drivers/thermal/Makefile          |    1 +
>  drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 324 insertions(+)
>  create mode 100644 drivers/thermal/bcm2835_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b343..3bd2406 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>  	  the primitive temperature sensor embedded in zx2967 SoCs.
>  	  This sensor generates the real time die temperature.
>  
> +config BCM2835_THERMAL
> +	tristate "Thermal sensors on bcm2835 SoC"
> +	depends on ARCH_BCM2835 || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on THERMAL_OF
> +	help
> +	  Support for thermal sensors on Broadcom bcm2835 SoCs.

small suggestion here:

-If you are sending a new version fixing Nobuhiro comment, you could take
the opportunity to improve the driver Kconfig help description.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Eduardo Valentin]

On Tue, Mar 28, 2017 at 09:58:09PM -0700, Eduardo Valentin wrote:
> On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> > Add basic thermal driver for bcm2835 SoC.
> > 
> > This driver currently make sure that tsense HW block is set up
> > correctly.
> > 
> > Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > Acked-by: Eric Anholt <eric@anholt.net>
> 
> I see no issue with this driver at this point.
> 
> As I mentioned, the full series needs to go together. You either get an
> Ack from OF maintainers, and I take the series, or you can add my
> 
> Acked-by: Eduardo Valentin <edubezval@gmail.com>
> 
> after fixing the small comment from Nobuhiro.
> 

Based on other drivers that require negative values, I assume patches 1
and 2 of these series are needed, but more for enforcing the API, given
that the current of-thermal driver still retrieves the negative values
from DTB [1].

A third option on top of what I suggested above is to split the series.
Given that you could still get the proper coefficients
with current of-thermal, I would suggest you to split this series into
two (patches 1-2, 3-6) in your next version, so we avoid futher delays
on the driver, while patches 1-2 waits for acks from OF side.

[1] - https://patchwork.kernel.org/patch/9612613/

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

> Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> hat am 30. März 2017 um 06:57 geschrieben:
> 
> 
> On Tue, Mar 28, 2017 at 09:58:09PM -0700, Eduardo Valentin wrote:
> > On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> > > Add basic thermal driver for bcm2835 SoC.
> > > 
> > > This driver currently make sure that tsense HW block is set up
> > > correctly.
> > > 
> > > Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > > Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> > 
> > I see no issue with this driver at this point.
> > 
> > As I mentioned, the full series needs to go together. You either get an
> > Ack from OF maintainers, and I take the series, or you can add my
> > 
> > Acked-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > 
> > after fixing the small comment from Nobuhiro.
> > 
> 
> Based on other drivers that require negative values, I assume patches 1
> and 2 of these series are needed, but more for enforcing the API, given
> that the current of-thermal driver still retrieves the negative values
> from DTB [1].
> 
> [1] - https://patchwork.kernel.org/patch/9612613/

I can't see that the OMAP5 thermal driver is using thermal_zone_get_slope() [2]. I got the wrong values without patch 1 and 2. So i wouldn't rely on that statement. But i will doublecheck it.

[2] - http://elixir.free-electrons.com/ident?v=4.11-rc4&i=thermal_zone_get_slope
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Stefan Wahren]

Hi Eduardo,

> Eduardo Valentin <edubezval@gmail.com> hat am 30. März 2017 um 06:57 geschrieben:
> 
> 
> On Tue, Mar 28, 2017 at 09:58:09PM -0700, Eduardo Valentin wrote:
> > On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> > > Add basic thermal driver for bcm2835 SoC.
> > > 
> > > This driver currently make sure that tsense HW block is set up
> > > correctly.
> > > 
> > > Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > > Acked-by: Eric Anholt <eric@anholt.net>
> > 
> > I see no issue with this driver at this point.
> > 
> > As I mentioned, the full series needs to go together. You either get an
> > Ack from OF maintainers, and I take the series, or you can add my
> > 
> > Acked-by: Eduardo Valentin <edubezval@gmail.com>
> > 
> > after fixing the small comment from Nobuhiro.
> > 
> 
> Based on other drivers that require negative values, I assume patches 1
> and 2 of these series are needed, but more for enforcing the API, given
> that the current of-thermal driver still retrieves the negative values
> from DTB [1].
> 
> A third option on top of what I suggested above is to split the series.
> Given that you could still get the proper coefficients
> with current of-thermal, I would suggest you to split this series into
> two (patches 1-2, 3-6) in your next version, so we avoid futher delays
> on the driver, while patches 1-2 waits for acks from OF side.

i revert patches #1, #2 and the slope value was still correct. So i will choose the third option and split the series.

Thanks
Stefan

> 
> [1] - https://patchwork.kernel.org/patch/9612613/

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Eduardo Valentin]

Hello,
On Thu, Mar 30, 2017 at 09:11:41PM +0200, Stefan Wahren wrote:
> Hi Eduardo,
> 
> > Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> hat am 30. März 2017 um 06:57 geschrieben:
> > 
> > 

<cut>

> > A third option on top of what I suggested above is to split the series.
> > Given that you could still get the proper coefficients
> > with current of-thermal, I would suggest you to split this series into
> > two (patches 1-2, 3-6) in your next version, so we avoid futher delays
> > on the driver, while patches 1-2 waits for acks from OF side.
> 
> i revert patches #1, #2 and the slope value was still correct. So i will choose the third option and split the series.

Sounds good to me.

> 
> Thanks
> Stefan
> 
> > 
> > [1] - https://patchwork.kernel.org/patch/9612613/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC

[Eduardo Valentin]

Hello,

On Thu, Mar 30, 2017 at 08:30:29AM +0200, Stefan Wahren wrote:
> 
> > Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> hat am 30. März 2017 um 06:57 geschrieben:
> > 
> > 
> > On Tue, Mar 28, 2017 at 09:58:09PM -0700, Eduardo Valentin wrote:
> > > On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> > > > Add basic thermal driver for bcm2835 SoC.
> > > > 
> > > > This driver currently make sure that tsense HW block is set up
> > > > correctly.
> > > > 
> > > > Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> > > > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > > > Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> > > 
> > > I see no issue with this driver at this point.
> > > 
> > > As I mentioned, the full series needs to go together. You either get an
> > > Ack from OF maintainers, and I take the series, or you can add my
> > > 
> > > Acked-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > 
> > > after fixing the small comment from Nobuhiro.
> > > 
> > 
> > Based on other drivers that require negative values, I assume patches 1
> > and 2 of these series are needed, but more for enforcing the API, given
> > that the current of-thermal driver still retrieves the negative values
> > from DTB [1].
> > 
> > [1] - https://patchwork.kernel.org/patch/9612613/
> 
> I can't see that the OMAP5 thermal driver is using thermal_zone_get_slope() [2]. I got the wrong values without patch 1 and 2. So i wouldn't rely on that statement. But i will doublecheck it.

The TI-SoC driver is not currently using the slope and offset from DT.
But on my -linus branch I have added three patches from Keerthy that
adds that support.

Anyways, I am assuming you are also getting correct values at your end,
based on your other email.

BR,

> 
> [2] - http://elixir.free-electrons.com/ident?v=4.11-rc4&i=thermal_zone_get_slope
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html