[PATCH v4 0/4] thermal: add driver for R-Car Gen3

[PATCH v4 0/4] thermal: add driver for R-Car Gen3

[Wolfram Sang]

Here is the next series for adding thermal support to Renesas R-Car Gen3 SoCs.
All changes from last revision went into patch 2/4, so please check the
changelog there. This series has again been tested on Salvator-X boards with
H3 and M3-W SoCs. The updated branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/thermal-on-4.10-next

I know we are ultra close, but I still dare to hope the new-driver-rule might
apply for this driver?

Please review, comment, apply. Thanks,

   Wolfram


Wolfram Sang (4):
  thermal: rcar_gen3_thermal: Document the R-Car Gen3
  thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver
  arm64: dts: r8a7795: Add R-Car Gen3 thermal support
  arm64: dts: r8a7796: Add R-Car Gen3 thermal support

 .../bindings/thermal/rcar-gen3-thermal.txt         |  56 ++++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  58 ++++
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           |  58 ++++
 drivers/thermal/Kconfig                            |   9 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/rcar_gen3_thermal.c                | 324 +++++++++++++++++++++
 6 files changed, 506 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
 create mode 100644 drivers/thermal/rcar_gen3_thermal.c

-- 
2.10.2

[PATCH v4 1/4] thermal: rcar_gen3_thermal: Document the R-Car Gen3

[Wolfram Sang]

Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../bindings/thermal/rcar-gen3-thermal.txt         | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt b/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
new file mode 100644
index 00000000000000..07a9713ae6a757
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
@@ -0,0 +1,56 @@
+* DT bindings for Renesas R-Car Gen3 Thermal Sensor driver
+
+On R-Car Gen3 SoCs, the thermal sensor controllers (TSC) control the thermal
+sensors (THS) which are the analog circuits for measuring temperature (Tj)
+inside the LSI.
+
+Required properties:
+- compatible		: "renesas,<soctype>-thermal",
+			  Examples with soctypes are:
+			    - "renesas,r8a7795-thermal" (R-Car H3)
+			    - "renesas,r8a7796-thermal" (R-Car M3-W)
+- reg			: Address ranges of the thermal registers. Each sensor
+			  needs one address range. Sorting must be done in
+			  increasing order according to datasheet, i.e.
+			  TSC1, TSC2, ...
+- clocks		: Must contain a reference to the functional clock.
+- #thermal-sensor-cells : must be <1>.
+
+Optional properties:
+
+- interrupts           : interrupts routed to the TSC (3 for H3 and M3-W)
+- power-domain		: Must contain a reference to the power domain. This
+			  property is mandatory if the thermal sensor instance
+			  is part of a controllable power domain.
+
+Example:
+
+	tsc: thermal@e6198000 {
+		compatible = "renesas,r8a7795-thermal";
+		reg = <0 0xe6198000 0 0x68>,
+		      <0 0xe61a0000 0 0x5c>,
+		      <0 0xe61a8000 0 0x5c>;
+		interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 522>;
+		power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+		#thermal-sensor-cells = <1>;
+		status = "okay";
+	};
+
+	thermal-zones {
+		sensor_thermal1: sensor-thermal1 {
+			polling-delay-passive = <250>;
+			polling-delay = <1000>;
+			thermal-sensors = <&tsc 0>;
+
+			trips {
+				sensor1_crit: sensor1-crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
-- 
2.10.2

[PATCH v4 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

[Wolfram Sang]

Add support for R-Car Gen3 thermal sensors. Polling only for now,
interrupts will be added incrementally. Same goes for reading fuses.
This is documented already, but no hardware available for now.

Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes since v3:

* call 'init' callback a tad earlier, avoids need for locking
* converted spinlock to mutex and simplified locking (only done against
  concurrent access in rcar_gen3_thermal_get_temp() now)
* use usleep_range instead of udelay
* use s64 instead of long (int didn't work)
* fixed an error path
* simplified rcar_gen3_thermal_update_temp() and its use

Thanks a lot to Eduardo and Geert for the prompt reviews!

 drivers/thermal/Kconfig             |   9 +
 drivers/thermal/Makefile            |   1 +
 drivers/thermal/rcar_gen3_thermal.c | 324 ++++++++++++++++++++++++++++++++++++
 3 files changed, 334 insertions(+)
 create mode 100644 drivers/thermal/rcar_gen3_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c2c056cc7ea52e..3912d24a07b10f 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -245,6 +245,15 @@ config RCAR_THERMAL
 	  Enable this to plug the R-Car thermal sensor driver into the Linux
 	  thermal framework.
 
+config RCAR_GEN3_THERMAL
+	tristate "Renesas R-Car Gen3 thermal driver"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on OF
+	help
+	  Enable this to plug the R-Car Gen3 thermal sensor driver into the Linux
+	  thermal framework.
+
 config KIRKWOOD_THERMAL
 	tristate "Temperature sensor on Marvell Kirkwood SoCs"
 	depends on MACH_KIRKWOOD || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index c92eb22a41ff89..1216fb31ed4036 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
+obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
 obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
 obj-y				+= samsung/
 obj-$(CONFIG_DOVE_THERMAL)  	+= dove_thermal.o
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
new file mode 100644
index 00000000000000..ef30d84b0f56a2
--- /dev/null
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -0,0 +1,324 @@
+/*
+ *  R-Car Gen3 THS thermal sensor driver
+ *  Based on rcar_thermal.c and work from Hien Dang and Khiem Nguyen.
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation.
+ * Copyright (C) 2016 Sang Engineering
+ *
+ *  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; version 2 of the License.
+ *
+ *  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/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/thermal.h>
+
+/* Register offsets */
+#define REG_GEN3_IRQSTR		0x04
+#define REG_GEN3_IRQMSK		0x08
+#define REG_GEN3_IRQCTL		0x0C
+#define REG_GEN3_IRQEN		0x10
+#define REG_GEN3_IRQTEMP1	0x14
+#define REG_GEN3_IRQTEMP2	0x18
+#define REG_GEN3_IRQTEMP3	0x1C
+#define REG_GEN3_CTSR		0x20
+#define REG_GEN3_THCTR		0x20
+#define REG_GEN3_TEMP		0x28
+#define REG_GEN3_THCODE1	0x50
+#define REG_GEN3_THCODE2	0x54
+#define REG_GEN3_THCODE3	0x58
+
+/* CTSR bits */
+#define CTSR_PONM	BIT(8)
+#define CTSR_AOUT	BIT(7)
+#define CTSR_THBGR	BIT(5)
+#define CTSR_VMEN	BIT(4)
+#define CTSR_VMST	BIT(1)
+#define CTSR_THSST	BIT(0)
+
+/* THCTR bits */
+#define THCTR_PONM	BIT(6)
+#define THCTR_THSST	BIT(0)
+
+#define CTEMP_MASK	0xFFF
+
+#define MCELSIUS(temp)	((temp) * 1000)
+#define GEN3_FUSE_MASK	0xFFF
+
+#define TSC_MAX_NUM	3
+
+/* Structure for thermal temperature calculation */
+struct equation_coefs {
+	s64 a1;
+	s64 b1;
+	s64 a2;
+	s64 b2;
+};
+
+struct rcar_gen3_thermal_tsc {
+	void __iomem *base;
+	struct thermal_zone_device *zone;
+	struct equation_coefs coef;
+	struct mutex lock;
+	u32 ctemp;
+};
+
+struct rcar_gen3_thermal_priv {
+	struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
+};
+
+struct rcar_gen3_thermal_data {
+	void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
+};
+
+/* Temperature calculation  */
+#define RCAR3_THERMAL_GRAN 500
+#define CODETSD(x) ((x) * 1000)
+#define TJ_1 96000L
+#define TJ_3 (-41000L)
+
+static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg)
+{
+	return ioread32(tsc->base + reg);
+}
+
+static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
+				     u32 reg, u32 data)
+{
+	iowrite32(data, tsc->base + reg);
+}
+
+static int _round_temp(int temp)
+{
+	int result, round_offs;
+
+	round_offs = temp >= 0 ? RCAR3_THERMAL_GRAN / 2 : -RCAR3_THERMAL_GRAN / 2;
+	result = (temp + round_offs) / RCAR3_THERMAL_GRAN;
+	return result * RCAR3_THERMAL_GRAN;
+}
+
+static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc *tsc,
+					    int *ptat, int *thcode)
+{
+	int tj_2;
+	s64 a1, b1;
+	s64 a2, b2;
+	s64 a1_num, a1_den;
+	s64 a2_num, a2_den;
+
+	tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
+		/ (ptat[0] - ptat[2])) - CODETSD(41);
+
+	/* calculate coefficients for linear equation */
+	a1_num = CODETSD(thcode[1] - thcode[2]);
+	a1_den = tj_2 - TJ_3;
+	a1 = (10000 * a1_num) / a1_den;
+	b1 = (10000 * thcode[2]) - ((a1 * TJ_3) / 1000);
+
+	a2_num = CODETSD(thcode[1] - thcode[0]);
+	a2_den = tj_2 - TJ_1;
+	a2 = (10000 * a2_num) / a2_den;
+	b2 = (10000 * thcode[0]) - ((a2 * TJ_1) / 1000);
+
+	tsc->coef.a1 = DIV_ROUND_CLOSEST(a1, 10);
+	tsc->coef.b1 = DIV_ROUND_CLOSEST(b1, 10);
+	tsc->coef.a2 = DIV_ROUND_CLOSEST(a2, 10);
+	tsc->coef.b2 = DIV_ROUND_CLOSEST(b2, 10);
+}
+
+static int _linear_temp_converter(struct equation_coefs *coef,
+					int temp_code)
+{
+	int temp, temp1, temp2;
+
+	temp1 = MCELSIUS((CODETSD(temp_code) - coef->b1)) / coef->a1;
+	temp2 = MCELSIUS((CODETSD(temp_code) - coef->b2)) / coef->a2;
+	temp = (temp1 + temp2) / 2;
+
+	return _round_temp(temp);
+}
+
+static void rcar_gen3_thermal_update_temp(struct rcar_gen3_thermal_tsc *tsc)
+{
+	tsc->ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
+}
+
+static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
+{
+	struct rcar_gen3_thermal_tsc *tsc = devdata;
+	int ctemp;
+
+	mutex_lock(&tsc->lock);
+
+	rcar_gen3_thermal_update_temp(tsc);
+	ctemp = _linear_temp_converter(&tsc->coef, tsc->ctemp);
+
+	mutex_unlock(&tsc->lock);
+
+	if ((ctemp < MCELSIUS(-40)) || (ctemp > MCELSIUS(125)))
+		return -EIO;
+
+	*temp = ctemp;
+
+	return 0;
+}
+
+static struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
+	.get_temp	= rcar_gen3_thermal_get_temp,
+};
+
+static void r8a7795_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
+{
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  0x0);
+
+	usleep_range(1000, 2000);
+
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR, CTSR_PONM);
+	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
+			   CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN);
+
+	usleep_range(100, 200);
+
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
+			  CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN |
+			  CTSR_VMST | CTSR_THSST);
+}
+
+static void r8a7796_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
+{
+	u32 reg_val;
+
+	reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
+	reg_val &= ~THCTR_PONM;
+	rcar_gen3_thermal_write(tsc, REG_GEN3_THCTR, reg_val);
+
+	usleep_range(1000, 2000);
+
+	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
+	reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
+	reg_val |= THCTR_THSST;
+	rcar_gen3_thermal_write(tsc, REG_GEN3_THCTR, reg_val);
+}
+
+static const struct rcar_gen3_thermal_data r8a7795_data = {
+	.thermal_init = r8a7795_thermal_init,
+};
+
+static const struct rcar_gen3_thermal_data r8a7796_data = {
+	.thermal_init = r8a7796_thermal_init,
+};
+
+static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
+	{ .compatible = "renesas,r8a7795-thermal", .data = &r8a7795_data},
+	{ .compatible = "renesas,r8a7796-thermal", .data = &r8a7796_data},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
+
+static int rcar_gen3_thermal_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
+static int rcar_gen3_thermal_probe(struct platform_device *pdev)
+{
+	struct rcar_gen3_thermal_priv *priv;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct thermal_zone_device *zone;
+	int ret, i;
+	const struct rcar_gen3_thermal_data *match_data = of_device_get_match_data(dev);
+
+	/* default values if FUSEs are missing */
+	int ptat[3] = { 2351, 1509, 435 };
+	int thcode[TSC_MAX_NUM][3] = {
+		{ 3248, 2800, 2221 },
+		{ 3245, 2795, 2216 },
+		{ 3250, 2805, 2237 },
+	};
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
+	for (i = 0; i < TSC_MAX_NUM; i++) {
+		struct rcar_gen3_thermal_tsc *tsc;
+
+		tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
+		if (!tsc) {
+			ret = -ENOMEM;
+			goto error_unregister;
+		}
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res)
+			break;
+
+		tsc->base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(tsc->base)) {
+			ret = PTR_ERR(tsc->base);
+			goto error_unregister;
+		}
+
+		priv->tscs[i] = tsc;
+		mutex_init(&tsc->lock);
+
+		match_data->thermal_init(tsc);
+		_linear_coefficient_calculation(tsc, ptat, thcode[i]);
+
+		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
+					&rcar_gen3_tz_of_ops);
+		if (IS_ERR(zone)) {
+			dev_err(dev, "Can't register thermal zone\n");
+			ret = PTR_ERR(zone);
+			goto error_unregister;
+		}
+		tsc->zone = zone;
+	}
+
+	return 0;
+
+error_unregister:
+	rcar_gen3_thermal_remove(pdev);
+
+	return ret;
+}
+
+static struct platform_driver rcar_gen3_thermal_driver = {
+	.driver	= {
+		.name	= "rcar_gen3_thermal",
+		.of_match_table = rcar_gen3_thermal_dt_ids,
+	},
+	.probe		= rcar_gen3_thermal_probe,
+	.remove		= rcar_gen3_thermal_remove,
+};
+module_platform_driver(rcar_gen3_thermal_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("R-Car Gen3 THS thermal sensor driver");
+MODULE_AUTHOR("Wolfram Sang <wsa+renesas@sang-engineering.com>");
-- 
2.10.2

Re: [PATCH v4 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

[Eduardo Valentin]

On Thu, Dec 01, 2016 at 11:04:40PM +0100, Wolfram Sang wrote:
> Add support for R-Car Gen3 thermal sensors. Polling only for now,
> interrupts will be added incrementally. Same goes for reading fuses.
> This is documented already, but no hardware available for now.
> 
> Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
> Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Would you please check the following from checkpatch too?
That is how the patch would apply. Enter to continue

WARNING: please write a paragraph that describes the config symbol fully
#82: FILE: drivers/thermal/Kconfig:248:
+config RCAR_GEN3_THERMAL

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#107: 
new file mode 100644

CHECK: struct mutex definition without comment
#186: FILE: drivers/thermal/rcar_gen3_thermal.c:75:
+	struct mutex lock;

WARNING: line over 80 characters
#204: FILE: drivers/thermal/rcar_gen3_thermal.c:93:
+static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg)

CHECK: Alignment should match open parenthesis
#210: FILE: drivers/thermal/rcar_gen3_thermal.c:99:
+static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
+				     u32 reg, u32 data)

WARNING: line over 80 characters
#219: FILE: drivers/thermal/rcar_gen3_thermal.c:108:
+	round_offs = temp >= 0 ? RCAR3_THERMAL_GRAN / 2 : -RCAR3_THERMAL_GRAN / 2;

CHECK: Alignment should match open parenthesis
#254: FILE: drivers/thermal/rcar_gen3_thermal.c:143:
+static int _linear_temp_converter(struct equation_coefs *coef,
+					int temp_code)

CHECK: Alignment should match open parenthesis
#304: FILE: drivers/thermal/rcar_gen3_thermal.c:193:
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
+			   CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN);

CHECK: Alignment should match open parenthesis
#309: FILE: drivers/thermal/rcar_gen3_thermal.c:198:
+	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
+			  CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN |

WARNING: line over 80 characters
#361: FILE: drivers/thermal/rcar_gen3_thermal.c:250:
+	const struct rcar_gen3_thermal_data *match_data = of_device_get_match_data(dev);

CHECK: Alignment should match open parenthesis
#406: FILE: drivers/thermal/rcar_gen3_thermal.c:295:
+		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
+					&rcar_gen3_tz_of_ops);

total: 0 errors, 5 warnings, 6 checks, 346 lines checked



> ---
> 
> Changes since v3:
> 
> * call 'init' callback a tad earlier, avoids need for locking
> * converted spinlock to mutex and simplified locking (only done against
>   concurrent access in rcar_gen3_thermal_get_temp() now)
> * use usleep_range instead of udelay
> * use s64 instead of long (int didn't work)
> * fixed an error path
> * simplified rcar_gen3_thermal_update_temp() and its use
> 
> Thanks a lot to Eduardo and Geert for the prompt reviews!
> 
>  drivers/thermal/Kconfig             |   9 +
>  drivers/thermal/Makefile            |   1 +
>  drivers/thermal/rcar_gen3_thermal.c | 324 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 334 insertions(+)
>  create mode 100644 drivers/thermal/rcar_gen3_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index c2c056cc7ea52e..3912d24a07b10f 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -245,6 +245,15 @@ config RCAR_THERMAL
>  	  Enable this to plug the R-Car thermal sensor driver into the Linux
>  	  thermal framework.
>  
> +config RCAR_GEN3_THERMAL
> +	tristate "Renesas R-Car Gen3 thermal driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on OF
> +	help
> +	  Enable this to plug the R-Car Gen3 thermal sensor driver into the Linux
> +	  thermal framework.
> +
>  config KIRKWOOD_THERMAL
>  	tristate "Temperature sensor on Marvell Kirkwood SoCs"
>  	depends on MACH_KIRKWOOD || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index c92eb22a41ff89..1216fb31ed4036 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>  obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> +obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
>  obj-$(CONFIG_KIRKWOOD_THERMAL)  += kirkwood_thermal.o
>  obj-y				+= samsung/
>  obj-$(CONFIG_DOVE_THERMAL)  	+= dove_thermal.o
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> new file mode 100644
> index 00000000000000..ef30d84b0f56a2
> --- /dev/null
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -0,0 +1,324 @@
> +/*
> + *  R-Car Gen3 THS thermal sensor driver
> + *  Based on rcar_thermal.c and work from Hien Dang and Khiem Nguyen.
> + *
> + * Copyright (C) 2016 Renesas Electronics Corporation.
> + * Copyright (C) 2016 Sang Engineering
> + *
> + *  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; version 2 of the License.
> + *
> + *  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/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/thermal.h>
> +
> +/* Register offsets */
> +#define REG_GEN3_IRQSTR		0x04
> +#define REG_GEN3_IRQMSK		0x08
> +#define REG_GEN3_IRQCTL		0x0C
> +#define REG_GEN3_IRQEN		0x10
> +#define REG_GEN3_IRQTEMP1	0x14
> +#define REG_GEN3_IRQTEMP2	0x18
> +#define REG_GEN3_IRQTEMP3	0x1C
> +#define REG_GEN3_CTSR		0x20
> +#define REG_GEN3_THCTR		0x20
> +#define REG_GEN3_TEMP		0x28
> +#define REG_GEN3_THCODE1	0x50
> +#define REG_GEN3_THCODE2	0x54
> +#define REG_GEN3_THCODE3	0x58
> +
> +/* CTSR bits */
> +#define CTSR_PONM	BIT(8)
> +#define CTSR_AOUT	BIT(7)
> +#define CTSR_THBGR	BIT(5)
> +#define CTSR_VMEN	BIT(4)
> +#define CTSR_VMST	BIT(1)
> +#define CTSR_THSST	BIT(0)
> +
> +/* THCTR bits */
> +#define THCTR_PONM	BIT(6)
> +#define THCTR_THSST	BIT(0)
> +
> +#define CTEMP_MASK	0xFFF
> +
> +#define MCELSIUS(temp)	((temp) * 1000)
> +#define GEN3_FUSE_MASK	0xFFF
> +
> +#define TSC_MAX_NUM	3
> +
> +/* Structure for thermal temperature calculation */
> +struct equation_coefs {
> +	s64 a1;
> +	s64 b1;
> +	s64 a2;
> +	s64 b2;

Can you please elaborate again why these could not be int?

> +};
> +
> +struct rcar_gen3_thermal_tsc {
> +	void __iomem *base;
> +	struct thermal_zone_device *zone;
> +	struct equation_coefs coef;
> +	struct mutex lock;
> +	u32 ctemp;
> +};
> +
> +struct rcar_gen3_thermal_priv {
> +	struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
> +};
> +
> +struct rcar_gen3_thermal_data {
> +	void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
> +};
> +
> +/* Temperature calculation  */
> +#define RCAR3_THERMAL_GRAN 500
> +#define CODETSD(x) ((x) * 1000)
> +#define TJ_1 96000L
> +#define TJ_3 (-41000L)
> +
> +static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg)
> +{
> +	return ioread32(tsc->base + reg);
> +}
> +
> +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> +				     u32 reg, u32 data)
> +{
> +	iowrite32(data, tsc->base + reg);
> +}
> +
> +static int _round_temp(int temp)
> +{
> +	int result, round_offs;
> +
> +	round_offs = temp >= 0 ? RCAR3_THERMAL_GRAN / 2 : -RCAR3_THERMAL_GRAN / 2;
> +	result = (temp + round_offs) / RCAR3_THERMAL_GRAN;
> +	return result * RCAR3_THERMAL_GRAN;
> +}
> +
> +static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc *tsc,
> +					    int *ptat, int *thcode)
> +{
> +	int tj_2;
> +	s64 a1, b1;
> +	s64 a2, b2;
> +	s64 a1_num, a1_den;
> +	s64 a2_num, a2_den;
> +
> +	tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
> +		/ (ptat[0] - ptat[2])) - CODETSD(41);
> +
> +	/* calculate coefficients for linear equation */
> +	a1_num = CODETSD(thcode[1] - thcode[2]);
> +	a1_den = tj_2 - TJ_3;
> +	a1 = (10000 * a1_num) / a1_den;
> +	b1 = (10000 * thcode[2]) - ((a1 * TJ_3) / 1000);
> +
> +	a2_num = CODETSD(thcode[1] - thcode[0]);
> +	a2_den = tj_2 - TJ_1;
> +	a2 = (10000 * a2_num) / a2_den;
> +	b2 = (10000 * thcode[0]) - ((a2 * TJ_1) / 1000);
> +
> +	tsc->coef.a1 = DIV_ROUND_CLOSEST(a1, 10);
> +	tsc->coef.b1 = DIV_ROUND_CLOSEST(b1, 10);
> +	tsc->coef.a2 = DIV_ROUND_CLOSEST(a2, 10);
> +	tsc->coef.b2 = DIV_ROUND_CLOSEST(b2, 10);

What is a a1, b1, a2, b2 typical values?

are you sure they do not fit into int? Looks like you start from pretty small values,
but multiply by 10^3 on num and den to get better precision?

> +}
> +
> +static int _linear_temp_converter(struct equation_coefs *coef,
> +					int temp_code)
> +{
> +	int temp, temp1, temp2;
> +
> +	temp1 = MCELSIUS((CODETSD(temp_code) - coef->b1)) / coef->a1;
> +	temp2 = MCELSIUS((CODETSD(temp_code) - coef->b2)) / coef->a2;

aren't we overflowing the result of this 64 bit math assigned into an int?

besides, again, temp_code fits into an int, why coef->[ab][12] do not ?

even if you do have a valid reason to use 64bit, that probably needs to be doc into the driver
code.

> +	temp = (temp1 + temp2) / 2;
> +
> +	return _round_temp(temp);
> +}
> +
> +static void rcar_gen3_thermal_update_temp(struct rcar_gen3_thermal_tsc *tsc)
> +{
> +	tsc->ctemp = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> +}
> +
> +static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> +{
> +	struct rcar_gen3_thermal_tsc *tsc = devdata;
> +	int ctemp;
> +
> +	mutex_lock(&tsc->lock);

much better now.

> +
> +	rcar_gen3_thermal_update_temp(tsc);
> +	ctemp = _linear_temp_converter(&tsc->coef, tsc->ctemp);
> +
> +	mutex_unlock(&tsc->lock);
> +
> +	if ((ctemp < MCELSIUS(-40)) || (ctemp > MCELSIUS(125)))
> +		return -EIO;


> +
> +	*temp = ctemp;
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
> +	.get_temp	= rcar_gen3_thermal_get_temp,
> +};
> +
> +static void r8a7795_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> +{
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  0x0);
> +
> +	usleep_range(1000, 2000);

good

> +
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR, CTSR_PONM);
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
> +			   CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN);
> +
> +	usleep_range(100, 200);
> +
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
> +			  CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN |
> +			  CTSR_VMST | CTSR_THSST);
> +}
> +
> +static void r8a7796_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> +{
> +	u32 reg_val;
> +
> +	reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
> +	reg_val &= ~THCTR_PONM;
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_THCTR, reg_val);
> +
> +	usleep_range(1000, 2000);
> +
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> +	reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
> +	reg_val |= THCTR_THSST;
> +	rcar_gen3_thermal_write(tsc, REG_GEN3_THCTR, reg_val);
> +}
> +
> +static const struct rcar_gen3_thermal_data r8a7795_data = {
> +	.thermal_init = r8a7795_thermal_init,
> +};
> +
> +static const struct rcar_gen3_thermal_data r8a7796_data = {
> +	.thermal_init = r8a7796_thermal_init,
> +};
> +
> +static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> +	{ .compatible = "renesas,r8a7795-thermal", .data = &r8a7795_data},
> +	{ .compatible = "renesas,r8a7796-thermal", .data = &r8a7796_data},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> +
> +static int rcar_gen3_thermal_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	pm_runtime_put(dev);
> +	pm_runtime_disable(dev);
> +
> +	return 0;
> +}
> +
> +static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> +{
> +	struct rcar_gen3_thermal_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct thermal_zone_device *zone;
> +	int ret, i;
> +	const struct rcar_gen3_thermal_data *match_data = of_device_get_match_data(dev);
> +
> +	/* default values if FUSEs are missing */
> +	int ptat[3] = { 2351, 1509, 435 };
> +	int thcode[TSC_MAX_NUM][3] = {
> +		{ 3248, 2800, 2221 },
> +		{ 3245, 2795, 2216 },
> +		{ 3250, 2805, 2237 },
> +	};

see, these seam to be small numbers, that apparently, after some 10^3 math, should still
fit into an int, no?

> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +
> +	for (i = 0; i < TSC_MAX_NUM; i++) {
> +		struct rcar_gen3_thermal_tsc *tsc;
> +
> +		tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
> +		if (!tsc) {
> +			ret = -ENOMEM;
> +			goto error_unregister;
> +		}
> +
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		if (!res)
> +			break;
> +
> +		tsc->base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(tsc->base)) {
> +			ret = PTR_ERR(tsc->base);
> +			goto error_unregister;
> +		}
> +
> +		priv->tscs[i] = tsc;
> +		mutex_init(&tsc->lock);
> +
> +		match_data->thermal_init(tsc);
> +		_linear_coefficient_calculation(tsc, ptat, thcode[i]);
> +
> +		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> +					&rcar_gen3_tz_of_ops);
> +		if (IS_ERR(zone)) {
> +			dev_err(dev, "Can't register thermal zone\n");
> +			ret = PTR_ERR(zone);
> +			goto error_unregister;
> +		}
> +		tsc->zone = zone;
> +	}
> +
> +	return 0;
> +
> +error_unregister:
> +	rcar_gen3_thermal_remove(pdev);
> +
> +	return ret;
> +}
> +
> +static struct platform_driver rcar_gen3_thermal_driver = {
> +	.driver	= {
> +		.name	= "rcar_gen3_thermal",
> +		.of_match_table = rcar_gen3_thermal_dt_ids,
> +	},
> +	.probe		= rcar_gen3_thermal_probe,
> +	.remove		= rcar_gen3_thermal_remove,
> +};
> +module_platform_driver(rcar_gen3_thermal_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("R-Car Gen3 THS thermal sensor driver");
> +MODULE_AUTHOR("Wolfram Sang <wsa+renesas@sang-engineering.com>");
> -- 
> 2.10.2
> 

Re: [PATCH v4 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 3368 bytes --]


> Would you please check the following from checkpatch too?

I saw them and chose to ignore them. I am not strict with those warnings
within the i2c subsystem as well. I can change the series if your mileage
varies, of course.

> WARNING: please write a paragraph that describes the config symbol fully
> #82: FILE: drivers/thermal/Kconfig:248:
> +config RCAR_GEN3_THERMAL

I can make up something.

> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

Is this mandatory?

> CHECK: struct mutex definition without comment
> #186: FILE: drivers/thermal/rcar_gen3_thermal.c:75:
> +	struct mutex lock;

Can change, but if there is only one lock, I don't really see much
benefit from this check.

> WARNING: line over 80 characters
> #204: FILE: drivers/thermal/rcar_gen3_thermal.c:93:
> +static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg)
> 
> CHECK: Alignment should match open parenthesis
> #210: FILE: drivers/thermal/rcar_gen3_thermal.c:99:
> +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> +				     u32 reg, u32 data)

I have those warnings (80 chars and open parens) ignored by default but
if you think it makes the code more readable, I'll change it.

> > +static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc *tsc,
> > +					    int *ptat, int *thcode)
> > +{
> > +	int tj_2;
> > +	s64 a1, b1;
> > +	s64 a2, b2;
> > +	s64 a1_num, a1_den;
> > +	s64 a2_num, a2_den;
> > +
> > +	tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
> > +		/ (ptat[0] - ptat[2])) - CODETSD(41);
> > +
> > +	/* calculate coefficients for linear equation */
> > +	a1_num = CODETSD(thcode[1] - thcode[2]);
> > +	a1_den = tj_2 - TJ_3;
> > +	a1 = (10000 * a1_num) / a1_den;
> > +	b1 = (10000 * thcode[2]) - ((a1 * TJ_3) / 1000);
> > +
> > +	a2_num = CODETSD(thcode[1] - thcode[0]);
> > +	a2_den = tj_2 - TJ_1;
> > +	a2 = (10000 * a2_num) / a2_den;
> > +	b2 = (10000 * thcode[0]) - ((a2 * TJ_1) / 1000);
> > +
> > +	tsc->coef.a1 = DIV_ROUND_CLOSEST(a1, 10);
> > +	tsc->coef.b1 = DIV_ROUND_CLOSEST(b1, 10);
> > +	tsc->coef.a2 = DIV_ROUND_CLOSEST(a2, 10);
> > +	tsc->coef.b2 = DIV_ROUND_CLOSEST(b2, 10);
> 
> What is a a1, b1, a2, b2 typical values?
> 
> are you sure they do not fit into int? Looks like you start from pretty small values,
> but multiply by 10^3 on num and den to get better precision?

Typical values are a few thousand. a1_num uses CODETSD which multiplies
by 1000 and makes it a million. a1 then multiplies again by 10000 which
makes it 10 billion. No int.

I am quite sure the formulas can be rearranged to fit into an int. As
mentioned before, I hoped we could start with the already tested
formulas since documentation on them is sparse.

> > +static int _linear_temp_converter(struct equation_coefs *coef,
> > +					int temp_code)
> > +{
> > +	int temp, temp1, temp2;
> > +
> > +	temp1 = MCELSIUS((CODETSD(temp_code) - coef->b1)) / coef->a1;
> > +	temp2 = MCELSIUS((CODETSD(temp_code) - coef->b2)) / coef->a2;
> 
> aren't we overflowing the result of this 64 bit math assigned into an int?

The division ensures that we get an int. Hmmm, not very pretty, I agree.

Sigh, maybe it is better to refactor the formulas before submitting
upstream :/

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

[Eduardo Valentin]

On Wed, Dec 07, 2016 at 10:06:27AM +0100, Wolfram Sang wrote:
> 
> > Would you please check the following from checkpatch too?
> 
> I saw them and chose to ignore them. I am not strict with those warnings
> within the i2c subsystem as well. I can change the series if your mileage
> varies, of course.

at least those that make sense.

> 
> > WARNING: please write a paragraph that describes the config symbol fully
> > #82: FILE: drivers/thermal/Kconfig:248:
> > +config RCAR_GEN3_THERMAL
> 
> I can make up something.

cool

> 
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> 
> Is this mandatory?

not really.

> 
> > CHECK: struct mutex definition without comment
> > #186: FILE: drivers/thermal/rcar_gen3_thermal.c:75:
> > +	struct mutex lock;
> 
> Can change, but if there is only one lock, I don't really see much
> benefit from this check.
> 

still good to explicitly write it down to prevent future changes to
abuse the lock.

> > WARNING: line over 80 characters
> > #204: FILE: drivers/thermal/rcar_gen3_thermal.c:93:
> > +static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg)
> > 
> > CHECK: Alignment should match open parenthesis
> > #210: FILE: drivers/thermal/rcar_gen3_thermal.c:99:
> > +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> > +				     u32 reg, u32 data)
> 
> I have those warnings (80 chars and open parens) ignored by default but
> if you think it makes the code more readable, I'll change it.
> 

I typically ask to keep checkpatch as clean as possible. The above can
easily be avoided by returning right after static inline.

> > > +static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc *tsc,
> > > +					    int *ptat, int *thcode)
> > > +{
> > > +	int tj_2;
> > > +	s64 a1, b1;
> > > +	s64 a2, b2;
> > > +	s64 a1_num, a1_den;
> > > +	s64 a2_num, a2_den;
> > > +
> > > +	tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137)
> > > +		/ (ptat[0] - ptat[2])) - CODETSD(41);
> > > +
> > > +	/* calculate coefficients for linear equation */
> > > +	a1_num = CODETSD(thcode[1] - thcode[2]);
> > > +	a1_den = tj_2 - TJ_3;
> > > +	a1 = (10000 * a1_num) / a1_den;
> > > +	b1 = (10000 * thcode[2]) - ((a1 * TJ_3) / 1000);
> > > +
> > > +	a2_num = CODETSD(thcode[1] - thcode[0]);
> > > +	a2_den = tj_2 - TJ_1;
> > > +	a2 = (10000 * a2_num) / a2_den;
> > > +	b2 = (10000 * thcode[0]) - ((a2 * TJ_1) / 1000);
> > > +
> > > +	tsc->coef.a1 = DIV_ROUND_CLOSEST(a1, 10);
> > > +	tsc->coef.b1 = DIV_ROUND_CLOSEST(b1, 10);
> > > +	tsc->coef.a2 = DIV_ROUND_CLOSEST(a2, 10);
> > > +	tsc->coef.b2 = DIV_ROUND_CLOSEST(b2, 10);
> > 
> > What is a a1, b1, a2, b2 typical values?
> > 
> > are you sure they do not fit into int? Looks like you start from pretty small values,
> > but multiply by 10^3 on num and den to get better precision?
> 
> Typical values are a few thousand. a1_num uses CODETSD which multiplies
> by 1000 and makes it a million. a1 then multiplies again by 10000 which
> makes it 10 billion. No int.
> 
> I am quite sure the formulas can be rearranged to fit into an int. As
> mentioned before, I hoped we could start with the already tested
> formulas since documentation on them is sparse.
> 
> > > +static int _linear_temp_converter(struct equation_coefs *coef,
> > > +					int temp_code)
> > > +{
> > > +	int temp, temp1, temp2;
> > > +
> > > +	temp1 = MCELSIUS((CODETSD(temp_code) - coef->b1)) / coef->a1;
> > > +	temp2 = MCELSIUS((CODETSD(temp_code) - coef->b2)) / coef->a2;
> > 
> > aren't we overflowing the result of this 64 bit math assigned into an int?
> 
> The division ensures that we get an int. Hmmm, not very pretty, I agree.
> 
> Sigh, maybe it is better to refactor the formulas before submitting
> upstream :/
> 

cool,  I am checking on the next version.

> Regards,
> 
>    Wolfram
> 


BR,

Eduardo Valentin

Re: [PATCH v4 2/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

[Eduardo Valentin]

Hello Wolfram,

On Thu, Dec 01, 2016 at 11:04:40PM +0100, Wolfram Sang wrote:
> Add support for R-Car Gen3 thermal sensors. Polling only for now,
> interrupts will be added incrementally. Same goes for reading fuses.
> This is documented already, but no hardware available for now.
> 
> Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
> Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Changes since v3:
> 
> * call 'init' callback a tad earlier, avoids need for locking
> * converted spinlock to mutex and simplified locking (only done against
>   concurrent access in rcar_gen3_thermal_get_temp() now)
> * use usleep_range instead of udelay
> * use s64 instead of long (int didn't work)
> * fixed an error path
> * simplified rcar_gen3_thermal_update_temp() and its use
> 

When I try compiling this driver (from this commit only), using
multi_v7_defconfig, I get this error when set as module:
ERROR: "__aeabi_ldivmod" [drivers/thermal/rcar_gen3_thermal.ko]
undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed

And these errors when builtin:
  LD      init/built-in.o
  drivers/built-in.o: In function `rcar_gen3_thermal_get_temp':
  :(.text+0x461778): undefined reference to `__aeabi_ldivmod'
  :(.text+0x461798): undefined reference to `__aeabi_ldivmod'
  drivers/built-in.o: In function `rcar_gen3_thermal_probe':
  :(.text+0x461948): undefined reference to `__aeabi_ldivmod'
  :(.text+0x461998): undefined reference to `__aeabi_ldivmod'
  :(.text+0x4619d0): undefined reference to `__aeabi_ldivmod'
  drivers/built-in.o::(.text+0x4619fc): more undefined references to
  `__aeabi_ldivmod' follow
  Makefile:962: recipe for target 'vmlinux' failed
  make: *** [vmlinux] Error 1

So, definitely would make sense to squash patch 5 here. BUT,

looking closer to it, I am getting this in the menuconfig on the flag
you added:
  │ Symbol: 64BIT [=64BIT]
  │  
  │ Type  : unknown   


  Am I missing something?


  BR,

Eduardo

[PATCH v4 3/4] arm64: dts: r8a7795: Add R-Car Gen3 thermal support

[Wolfram Sang]

Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 58 ++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 8c15040f2540d6..0699e04fbdf264 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1541,5 +1541,63 @@
 				};
 			};
 		};
+
+		tsc: thermal@e6198000 {
+			compatible = "renesas,r8a7795-thermal";
+			reg = <0 0xe6198000 0 0x68>,
+			      <0 0xe61a0000 0 0x5c>,
+			      <0 0xe61a8000 0 0x5c>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 522>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#thermal-sensor-cells = <1>;
+			status = "okay";
+		};
+
+		thermal-zones {
+			sensor_thermal1: sensor-thermal1 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 0>;
+
+				trips {
+					sensor1_crit: sensor1-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal2: sensor-thermal2 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 1>;
+
+				trips {
+					sensor2_crit: sensor2-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal3: sensor-thermal3 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 2>;
+
+				trips {
+					sensor3_crit: sensor3-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+		};
 	};
 };
-- 
2.10.2

[PATCH v4 4/4] arm64: dts: r8a7796: Add R-Car Gen3 thermal support

[Wolfram Sang]

Signed-off-by: Hien Dang <hien.dang.eb@renesas.com>
Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 58 ++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 9217da98352565..5261f0f77ce2e9 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -251,5 +251,63 @@
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 			status = "disabled";
 		};
+
+		tsc: thermal@e6198000 {
+			compatible = "renesas,r8a7796-thermal";
+			reg = <0 0xe6198000 0 0x68>,
+			      <0 0xe61a0000 0 0x5c>,
+			      <0 0xe61a8000 0 0x5c>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 522>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			#thermal-sensor-cells = <1>;
+			status = "okay";
+		};
+
+		thermal-zones {
+			sensor_thermal1: sensor-thermal1 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 0>;
+
+				trips {
+					sensor1_crit: sensor1-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal2: sensor-thermal2 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 1>;
+
+				trips {
+					sensor2_crit: sensor2-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal3: sensor-thermal3 {
+				polling-delay-passive = <250>;
+				polling-delay = <1000>;
+				thermal-sensors = <&tsc 2>;
+
+				trips {
+					sensor3_crit: sensor3-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+		};
 	};
 };
-- 
2.10.2

[PATCH v4 5/4] thermal: rcar_gen3_thermal: restrict to 64BIT

[Wolfram Sang]

COMPILE_TEST triggers problems on 32 bit machines, so limit this driver
to 64BIT for now. This is no loss because the hardware is only available
on 64 bit SoCs anyhow. After we obtained more data from the hardware
engineers, we will refactor the code to be usable with COMPILE_TEST.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

buildbot reported the problem and seems to be happy now.

 drivers/thermal/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 3912d24a07b10f..edaed1282791bf 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -247,7 +247,7 @@ config RCAR_THERMAL
 
 config RCAR_GEN3_THERMAL
 	tristate "Renesas R-Car Gen3 thermal driver"
-	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on ARCH_RENESAS && 64BIT
 	depends on HAS_IOMEM
 	depends on OF
 	help
-- 
2.10.2

Re: [PATCH v4 5/4] thermal: rcar_gen3_thermal: restrict to 64BIT

[Eduardo Valentin]

On Fri, Dec 02, 2016 at 09:44:22PM +0100, Wolfram Sang wrote:
> COMPILE_TEST triggers problems on 32 bit machines, so limit this driver
> to 64BIT for now. This is no loss because the hardware is only available
> on 64 bit SoCs anyhow. After we obtained more data from the hardware
> engineers, we will refactor the code to be usable with COMPILE_TEST.
> 

Don't you want to fold this one into patch 2 and mention the limitation
on that patch's commit log?

> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> buildbot reported the problem and seems to be happy now.
> 
>  drivers/thermal/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 3912d24a07b10f..edaed1282791bf 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -247,7 +247,7 @@ config RCAR_THERMAL
>  
>  config RCAR_GEN3_THERMAL
>  	tristate "Renesas R-Car Gen3 thermal driver"
> -	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on ARCH_RENESAS && 64BIT
>  	depends on HAS_IOMEM
>  	depends on OF
>  	help
> -- 
> 2.10.2
> 

Re: [PATCH v4 5/4] thermal: rcar_gen3_thermal: restrict to 64BIT

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On Tue, Dec 06, 2016 at 04:39:57PM -0800, Eduardo Valentin wrote:
> On Fri, Dec 02, 2016 at 09:44:22PM +0100, Wolfram Sang wrote:
> > COMPILE_TEST triggers problems on 32 bit machines, so limit this driver
> > to 64BIT for now. This is no loss because the hardware is only available
> > on 64 bit SoCs anyhow. After we obtained more data from the hardware
> > engineers, we will refactor the code to be usable with COMPILE_TEST.
> > 
> 
> Don't you want to fold this one into patch 2 and mention the limitation
> on that patch's commit log?

Yeah, will do with V3. Didn't want to resend before the next review.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 0/4] thermal: add driver for R-Car Gen3

[Geert Uytterhoeven]

Hi Wolfram,

On Thu, Dec 1, 2016 at 11:04 PM, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Here is the next series for adding thermal support to Renesas R-Car Gen3 SoCs.
> All changes from last revision went into patch 2/4, so please check the
> changelog there. This series has again been tested on Salvator-X boards with
> H3 and M3-W SoCs. The updated branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/thermal-on-4.10-next

Dropped from renesas-drivers, as Niklas provided a branch with the
reworked version.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds