[PATCH 0/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal support

[PATCH 0/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal support

[Khiem Nguyen]

This patchset adds new thermal sensor driver to support 3 sensors
found in R-Car Gen3 series.

It has been decided to create new driver, instead of using the existing
thermal driver due to many differences in HW setting flows, the method
to calculate temperatures value and some newly supported features.

The new driver can support both polling mode and interrupt mode.
It has been tested with R-Car H3.

This driver is developed based on early work of Hien Dang and Thao Nguyen.
All comments are welcome.

Khiem Nguyen (4):
  thermal: rcar_gen3_thermal: Document the R-Car Gen3 thermal bindings
  thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver support
  arm64: dts: r8a7795: Add R-Car Gen3 thermal support
  arm64: defconfig: Enable R-Car Gen3 thermal support

 .../bindings/thermal/rcar-gen3-thermal.txt         |  79 ++++
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  86 ++++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/thermal/Kconfig                            |   9 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/rcar_gen3_thermal.c                | 524 +++++++++++++++++++++
 6 files changed, 701 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
 create mode 100644 drivers/thermal/rcar_gen3_thermal.c

-- 
1.9.1

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

[Khiem Nguyen]

Signed-off-by: Hien Dang <hien.dang.eb@rvc.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 .../bindings/thermal/rcar-gen3-thermal.txt         | 79 ++++++++++++++++++++++
 1 file changed, 79 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 0000000..ed6ce45
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
@@ -0,0 +1,79 @@
+* DT bindings for Renesas R-Car Gen3 Thermal Sensor driver
+
+Required properties:
+- compatible		: "renesas,thermal-<soctype>",
+			  Examples with soctypes are:
+			    - "renesas,thermal-r8a7795" (R-Car H3)
+			    - "renesas,thermal-r8a7796" (R-Car M3)
+			    - "renesas,rcar-gen3-thermal" as fallback
+- reg			: Address range of the thermal registers.
+- clocks		: Must contain a reference to the functional clock.
+- #thermal-sensor-cells : Please see ./thermal.txt
+
+Option properties:
+
+- interrupts		: Use interrupt
+- 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 (non interrupt support):
+
+	tsc1: thermal@e6198000 {
+		compatible = "renesas,thermal-r8a7795",
+			"renesas,rcar-gen3-thermal";
+		reg = <0 0xe6198000 0 0x5c>;
+		clocks = <&cpg CPG_MOD 522>;
+		power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+		#thermal-sensor-cells = <0>;
+		status = "okay";
+	};
+
+	thermal-zones {
+		sensor_thermal1: sensor-thermal1 {
+			polling-delay-passive = <250>;
+			polling-delay = <1000>;
+
+			/* sensor ID */
+			thermal-sensors = <&tsc1>;
+
+			trips {
+				sensor1_crit: sensor1-crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
+Example (interrupt support):
+
+	tsc1: thermal@e6198000 {
+		compatible = "renesas,thermal-r8a7795",
+			"renesas,rcar-gen3-thermal";
+		reg = <0 0xe6198000 0 0x5c>;
+		interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 522>;
+		power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+		#thermal-sensor-cells = <0>;
+		status = "okay";
+	};
+
+	thermal-zones {
+		sensor_thermal1: sensor-thermal1 {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+
+			/* sensor ID */
+			thermal-sensors = <&tsc1>;
+
+			trips {
+				sensor1_crit: sensor1-crit {
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
-- 
1.9.1

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

[Khiem Nguyen]

Signed-off-by: Hien Dang <hien.dang.eb@rvc.renesas.com>
Signed-off-by: Thao Nguyen <thao.nguyen.yb@rvc.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 drivers/thermal/Kconfig             |   9 +
 drivers/thermal/Makefile            |   1 +
 drivers/thermal/rcar_gen3_thermal.c | 524 ++++++++++++++++++++++++++++++++++++
 3 files changed, 534 insertions(+)
 create mode 100644 drivers/thermal/rcar_gen3_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 2d702ca..151feb7 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -223,6 +223,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 cded802..3ac9186 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -31,6 +31,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 0000000..a9a372b
--- /dev/null
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -0,0 +1,524 @@
+/*
+ *  R-Car Gen3 THS/CIVM thermal sensor driver
+ *  Based on drivers/thermal/rcar_thermal.c
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation.
+ *
+ *  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/irq.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spinlock.h>
+#include <linux/thermal.h>
+
+/* Register offset */
+#define REG_GEN3_CTSR		0x20
+#define REG_GEN3_THCTR		0x20
+#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_TEMP		0x28
+#define REG_GEN3_THCODE1	0x50
+#define REG_GEN3_THCODE2	0x54
+#define REG_GEN3_THCODE3	0x58
+
+#define PTAT_BASE		0xE6198000
+#define REG_GEN3_PTAT1		0x5C
+#define REG_GEN3_PTAT2		0x60
+#define REG_GEN3_PTAT3		0x64
+#define PTAT_SIZE		REG_GEN3_PTAT3
+
+/* CTSR bit */
+#define PONM            (0x1 << 8)
+#define AOUT            (0x1 << 7)
+#define THBGR           (0x1 << 5)
+#define VMEN            (0x1 << 4)
+#define VMST            (0x1 << 1)
+#define THSST           (0x1 << 0)
+
+/* THCTR bit */
+#define CTCTL		(0x1 << 24)
+#define THCNTSEN(x)	(x << 16)
+
+#define BIT_LEN_12	0x1
+
+#define CTEMP_MASK	0xFFF
+
+#define MCELSIUS(temp)			((temp) * 1000)
+#define TEMP_IRQ_SHIFT(tsc_id)	(0x1 << tsc_id)
+#define TEMPD_IRQ_SHIFT(tsc_id)	(0x1 << (tsc_id + 3))
+#define GEN3_FUSE_MASK	0xFFF
+
+/* Structure for thermal temperature calculation */
+struct equation_coefs {
+	long a1;
+	long b1;
+	long a2;
+	long b2;
+};
+
+struct fuse_factors {
+	int thcode_1;
+	int thcode_2;
+	int thcode_3;
+	int ptat_1;
+	int ptat_2;
+	int ptat_3;
+};
+
+struct rcar_gen3_thermal_priv {
+	void __iomem *base;
+	struct device *dev;
+	struct thermal_zone_device *zone;
+	struct delayed_work work;
+	struct fuse_factors factor;
+	struct equation_coefs coef;
+	spinlock_t lock;
+	int id;
+	int irq;
+	u32 ctemp;
+	const struct rcar_gen3_thermal_data *data;
+};
+
+struct rcar_gen3_thermal_data {
+	int (*thermal_init)(struct rcar_gen3_thermal_priv *priv);
+};
+
+#define rcar_priv_to_dev(priv)		((priv)->dev)
+#define rcar_has_irq_support(priv)	((priv)->irq)
+
+/* Temperature calculation  */
+#define CODETSD(x)		((x) * 1000)
+#define TJ_1 96000L
+#define TJ_3 (-41000L)
+#define PW2(x) ((x)*(x))
+
+static u32 thermal_reg_read(struct rcar_gen3_thermal_priv *priv, u32 reg)
+{
+	return ioread32(priv->base + reg);
+}
+
+static void thermal_reg_write(struct rcar_gen3_thermal_priv *priv,
+				u32 reg, u32 data)
+{
+	iowrite32(data, priv->base + reg);
+}
+
+static int _round_temp(int temp)
+{
+	int tmp1, tmp2;
+	int result = 0;
+
+	tmp1 = abs(temp) % 1000;
+	tmp2 = abs(temp) / 1000;
+
+	if (tmp1 < 250)
+		result = CODETSD(tmp2);
+	else if (tmp1 < 750 && tmp1 >= 250)
+		result = CODETSD(tmp2) + 500;
+	else
+		result = CODETSD(tmp2) + 1000;
+
+	return ((temp < 0) ? (result * (-1)) : result);
+}
+
+static int _read_fuse_factor(struct rcar_gen3_thermal_priv *priv)
+{
+	/*
+	 * FIXME: The value should be read from some FUSE registers.
+	 * For available SoC, these registers have not been supported yet.
+	 * The pre-defined value will be applied for now.
+	 */
+	priv->factor.ptat_1 = 2351;
+	priv->factor.ptat_2 = 1509;
+	priv->factor.ptat_3 = 435;
+	switch (priv->id) {
+	case 0:
+		priv->factor.thcode_1 = 3248;
+		priv->factor.thcode_2 = 2800;
+		priv->factor.thcode_3 = 2221;
+		break;
+	case 1:
+		priv->factor.thcode_1 = 3245;
+		priv->factor.thcode_2 = 2795;
+		priv->factor.thcode_3 = 2216;
+		break;
+	case 2:
+		priv->factor.thcode_1 = 3250;
+		priv->factor.thcode_2 = 2805;
+		priv->factor.thcode_3 = 2237;
+		break;
+	}
+
+	return 0;
+}
+
+static void _linear_coefficient_calculation(struct rcar_gen3_thermal_priv *priv)
+{
+	int tj_2 = 0;
+	long a1, b1;
+	long a2, b2;
+	long a1_num, a1_den;
+	long a2_num, a2_den;
+
+	tj_2 = (CODETSD((priv->factor.ptat_2 - priv->factor.ptat_3) * 137)
+		/ (priv->factor.ptat_1 - priv->factor.ptat_3)) - CODETSD(41);
+
+	/*
+	 * The following code is to calculate coefficients for linear equation.
+	 */
+	/* Coefficient a1 and b1 */
+	a1_num = CODETSD(priv->factor.thcode_2 - priv->factor.thcode_3);
+	a1_den = tj_2 - TJ_3;
+	a1 = (10000 * a1_num) / a1_den;
+	b1 = (10000 * priv->factor.thcode_3) - ((a1 * TJ_3) / 1000);
+
+	/* Coefficient a2 and b2 */
+	a2_num = CODETSD(priv->factor.thcode_2 - priv->factor.thcode_1);
+	a2_den = tj_2 - TJ_1;
+	a2 = (10000 * a2_num) / a2_den;
+	b2 = (10000 * priv->factor.thcode_1) - ((a2 * TJ_1) / 1000);
+
+	priv->coef.a1 = DIV_ROUND_CLOSEST(a1, 10);
+	priv->coef.b1 = DIV_ROUND_CLOSEST(b1, 10);
+	priv->coef.a2 = DIV_ROUND_CLOSEST(a2, 10);
+	priv->coef.b2 = DIV_ROUND_CLOSEST(b2, 10);
+}
+
+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);
+}
+
+/*
+ *		Zone device functions
+ */
+static int rcar_gen3_thermal_update_temp(struct rcar_gen3_thermal_priv *priv)
+{
+	u32 ctemp;
+	int i;
+	unsigned long flags;
+	u32 reg = REG_GEN3_IRQTEMP1 + (priv->id * 4);
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	for (i = 0; i < 256; i++) {
+		ctemp = thermal_reg_read(priv, REG_GEN3_TEMP) & CTEMP_MASK;
+		if (rcar_has_irq_support(priv)) {
+			thermal_reg_write(priv, reg, ctemp);
+			if (thermal_reg_read(priv, REG_GEN3_IRQSTR) != 0)
+				break;
+		} else
+			break;
+
+		udelay(150);
+	}
+
+	priv->ctemp = ctemp;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
+{
+	struct rcar_gen3_thermal_priv *priv = devdata;
+	int ctemp;
+	unsigned long flags;
+
+	rcar_gen3_thermal_update_temp(priv);
+
+	spin_lock_irqsave(&priv->lock, flags);
+	ctemp = _linear_temp_converter(priv->coef, priv->ctemp);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	if ((ctemp < MCELSIUS(-40)) || (ctemp > MCELSIUS(125))) {
+		struct device *dev = rcar_priv_to_dev(priv);
+
+		dev_dbg(dev, "Temperature is not measured correctly!\n");
+		return -EIO;
+	}
+
+	*temp = ctemp;
+
+	return 0;
+}
+
+static int r8a7795_thermal_init(struct rcar_gen3_thermal_priv *priv)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	thermal_reg_write(priv, REG_GEN3_CTSR,  THBGR);
+	thermal_reg_write(priv, REG_GEN3_CTSR,  0x0);
+
+	udelay(1000);
+
+	thermal_reg_write(priv, REG_GEN3_CTSR, PONM);
+	thermal_reg_write(priv, REG_GEN3_IRQCTL, 0x3F);
+	thermal_reg_write(priv, REG_GEN3_IRQEN, TEMP_IRQ_SHIFT(priv->id) |
+						TEMPD_IRQ_SHIFT(priv->id));
+	thermal_reg_write(priv, REG_GEN3_CTSR,
+			PONM | AOUT | THBGR | VMEN);
+	udelay(100);
+
+	thermal_reg_write(priv, REG_GEN3_CTSR,
+			PONM | AOUT | THBGR | VMEN | VMST | THSST);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int r8a7796_thermal_init(struct rcar_gen3_thermal_priv *priv)
+{
+	unsigned long flags;
+	unsigned long reg_val;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	thermal_reg_write(priv, REG_GEN3_THCTR,  0x0);
+	udelay(1000);
+	thermal_reg_write(priv, REG_GEN3_IRQCTL, 0x3F);
+	thermal_reg_write(priv, REG_GEN3_IRQEN, TEMP_IRQ_SHIFT(priv->id) |
+						TEMPD_IRQ_SHIFT(priv->id));
+	thermal_reg_write(priv, REG_GEN3_THCTR,
+						CTCTL | THCNTSEN(BIT_LEN_12));
+	reg_val = thermal_reg_read(priv, REG_GEN3_THCTR);
+	reg_val &= ~CTCTL;
+	reg_val |= THSST;
+	thermal_reg_write(priv, REG_GEN3_THCTR, reg_val);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+/*
+ *		Interrupt
+ */
+#define rcar_gen3_thermal_irq_enable(p)		_thermal_irq_ctrl(p, 1)
+#define rcar_gen3_thermal_irq_disable(p)	_thermal_irq_ctrl(p, 0)
+static void _thermal_irq_ctrl(struct rcar_gen3_thermal_priv *priv, int enable)
+{
+	unsigned long flags;
+
+	if (!rcar_has_irq_support(priv))
+		return;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	thermal_reg_write(priv, REG_GEN3_IRQMSK,
+		enable ? (TEMP_IRQ_SHIFT(priv->id) |
+			TEMPD_IRQ_SHIFT(priv->id)) : 0);
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static void rcar_gen3_thermal_work(struct work_struct *work)
+{
+	struct rcar_gen3_thermal_priv *priv;
+
+	priv = container_of(work, struct rcar_gen3_thermal_priv, work.work);
+
+	thermal_zone_device_update(priv->zone);
+
+	rcar_gen3_thermal_irq_enable(priv);
+}
+
+static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
+{
+	struct rcar_gen3_thermal_priv *priv = data;
+	unsigned long flags;
+	int status;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	status = thermal_reg_read(priv, REG_GEN3_IRQSTR);
+	thermal_reg_write(priv, REG_GEN3_IRQSTR, 0);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	if ((status & TEMP_IRQ_SHIFT(priv->id)) ||
+		(status & TEMPD_IRQ_SHIFT(priv->id))) {
+		rcar_gen3_thermal_irq_disable(priv);
+		schedule_delayed_work(&priv->work,
+				      msecs_to_jiffies(300));
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
+	.get_temp	= rcar_gen3_thermal_get_temp,
+};
+
+/*
+ *		Platform functions
+ */
+static int rcar_gen3_thermal_remove(struct platform_device *pdev)
+{
+	struct rcar_gen3_thermal_priv *priv = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	rcar_gen3_thermal_irq_disable(priv);
+	thermal_zone_of_sensor_unregister(dev, priv->zone);
+
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
+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,thermal-r8a7795", .data = &r8a7795_data},
+	{ .compatible = "renesas,thermal-r8a7796", .data = &r8a7796_data},
+	{ .compatible = "renesas,rcar-gen3-thermal", .data = &r8a7796_data},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
+
+static int rcar_gen3_thermal_probe(struct platform_device *pdev)
+{
+	struct rcar_gen3_thermal_priv *priv;
+	struct device *dev = &pdev->dev;
+	struct resource *res, *irq;
+	int ret = -ENODEV;
+	int idle;
+	struct device_node *tz_nd, *tmp_nd;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->dev = dev;
+
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
+	priv->data = of_device_get_match_data(dev);
+	if (!priv->data)
+		goto error_unregister;
+
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	priv->irq = 0;
+	if (irq) {
+		priv->irq = 1;
+		for_each_node_with_property(tz_nd, "polling-delay") {
+			tmp_nd = of_parse_phandle(tz_nd,
+					"thermal-sensors", 0);
+			if (tmp_nd && !strcmp(tmp_nd->full_name,
+					dev->of_node->full_name)) {
+				of_property_read_u32(tz_nd, "polling-delay",
+					&idle);
+				(idle > 0) ? (priv->irq = 0) :
+						(priv->irq = 1);
+				break;
+			}
+		}
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		goto error_unregister;
+
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base)) {
+		ret = PTR_ERR(priv->base);
+		goto error_unregister;
+	}
+
+	spin_lock_init(&priv->lock);
+	INIT_DELAYED_WORK(&priv->work, rcar_gen3_thermal_work);
+
+	priv->id = of_alias_get_id(dev->of_node, "tsc");
+
+	priv->zone = devm_thermal_zone_of_sensor_register(dev, 0, priv,
+				&rcar_gen3_tz_of_ops);
+
+	if (IS_ERR(priv->zone)) {
+		dev_err(dev, "Can't register thermal zone\n");
+		ret = PTR_ERR(priv->zone);
+		priv->zone = NULL;
+		goto error_unregister;
+	}
+
+	priv->data->thermal_init(priv);
+	ret = _read_fuse_factor(priv);
+	if (ret)
+		goto error_unregister;
+	_linear_coefficient_calculation(priv);
+	ret = rcar_gen3_thermal_update_temp(priv);
+
+	if (ret < 0)
+		goto error_unregister;
+
+
+	rcar_gen3_thermal_irq_enable(priv);
+
+	/* Interrupt */
+	if (irq) {
+		ret = devm_request_irq(dev, irq->start,
+					rcar_gen3_thermal_irq, 0,
+				       dev_name(dev), priv);
+		if (ret) {
+			dev_err(dev, "IRQ request failed\n ");
+			goto error_unregister;
+		}
+	}
+
+	dev_info(dev, "probed\n");
+
+	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/CIVM thermal sensor driver");
+MODULE_AUTHOR("Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>");
-- 
1.9.1

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

[Khiem Nguyen]

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

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index c14e89a..db203db 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -25,6 +25,9 @@
 		i2c4 = &i2c4;
 		i2c5 = &i2c5;
 		i2c6 = &i2c6;
+		tsc0 = &tsc1;
+		tsc1 = &tsc2;
+		tsc2 = &tsc3;
 	};
 
 	psci {
@@ -1620,5 +1623,88 @@
 			clocks = <&cpg CPG_MOD 613>;
 			power-domains = <&sysc R8A7795_PD_A3VP>;
 		};
+
+		tsc1: thermal@e6198000 {
+			compatible = "renesas,thermal-r8a7795",
+				"renesas,rcar-gen3-thermal";
+			reg = <0 0xe6198000 0 0x5c>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 522>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#thermal-sensor-cells = <0>;
+			status = "okay";
+		};
+
+		tsc2: thermal@e61a0000 {
+			compatible = "renesas,thermal-r8a7795",
+				"renesas,rcar-gen3-thermal";
+			reg = <0 0xe61a0000 0 0x5c>;
+			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 522>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#thermal-sensor-cells = <0>;
+			status = "okay";
+		};
+
+		tsc3: thermal@e61a8000 {
+			compatible = "renesas,thermal-r8a7795",
+				"renesas,rcar-gen3-thermal";
+			reg = <0 0xe61a8000 0 0x5c>;
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 522>;
+			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+			#thermal-sensor-cells = <0>;
+			status = "okay";
+		};
+
+		thermal-zones {
+			sensor_thermal1: sensor-thermal1 {
+				polling-delay-passive = <250>;
+				polling-delay = <0>;
+
+				/* sensor ID */
+				thermal-sensors = <&tsc1>;
+
+				trips {
+					sensor1_crit: sensor1-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal2: sensor-thermal2 {
+				polling-delay-passive = <250>;
+				polling-delay = <0>;
+
+				/* sensor ID */
+				thermal-sensors = <&tsc2>;
+
+				trips {
+					sensor2_crit: sensor2-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+
+			sensor_thermal3: sensor-thermal3 {
+				polling-delay-passive = <250>;
+				polling-delay = <0>;
+
+				/* sensor ID */
+				thermal-sensors = <&tsc3>;
+
+				trips {
+					sensor3_crit: sensor3-crit {
+						temperature = <90000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+			};
+		};
 	};
 };
-- 
1.9.1

[PATCH 4/4] arm64: defconfig: Enable R-Car Gen3 thermal support

[Khiem Nguyen]

Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 4f3e1f6..b5551d9 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -200,8 +200,10 @@ CONFIG_POWER_RESET_SYSCON=y
 CONFIG_SENSORS_LM90=m
 CONFIG_SENSORS_INA2XX=m
 CONFIG_THERMAL=y
+CONFIG_THERMAL_OF=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_EXYNOS_THERMAL=y
+CONFIG_RCAR_GEN3_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_RENESAS_WDT=y
 CONFIG_MFD_SPMI_PMIC=y
-- 
1.9.1

[PATCH/RFC 0/3] thermal: rcar_gen3_thermal: Apply shared interrupts for thermal sensors

[Khiem Nguyen]

This patchset intents to improve thermal driver operation in interrupt mode,
which has been introduced in [1].

The original idea is using 1 interrupt for each thermal sensor, to detect both up
and down temperature. It caused issue when the temperature is changing rapidly.

The new idea is about using shared interrupt I/F for all three sensors.
Two interrupts will be set to detect temperature up and down. When interrupt occurs,
the temperature will be updated in all 3 sensors, if the temperature value is changed. 

All comments are welcome.

[1]
[PATCH 0/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal support
https://www.spinics.net/lists/kernel/msg2282663.html

--
Gaku Inami (3):
  thermal: rcar_gen3_thermal: Modify the shared irq with initialization
  thermal: rcar_gen3_thermal: Modify the way to detect the interrupts
  arm64: dts: r8a7795: Support shared irq for thermal sensors

 arch/arm64/boot/dts/renesas/r8a7795.dtsi |  9 ++--
 drivers/thermal/rcar_gen3_thermal.c      | 89 ++++++++++++++++++++------------
 2 files changed, 63 insertions(+), 35 deletions(-)

-- 
1.9.1

[PATCH/RFC 1/3] thermal: rcar_gen3_thermal: Modify the shared irq with initialization

[Khiem Nguyen]

In R-CAR Gen3 series, it has some thermal sensors. The interrupt I/F
that can be used in thermal sensors is three. So it should be used
the interrupt I/F as shared.

This patch changes the shared settings for the thermal interrupts.

Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index a9a372b..e640a14 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -416,6 +416,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	int ret = -ENODEV;
 	int idle;
 	struct device_node *tz_nd, *tmp_nd;
+	int i, irq_cnt;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -489,13 +490,18 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	rcar_gen3_thermal_irq_enable(priv);
 
 	/* Interrupt */
-	if (irq) {
-		ret = devm_request_irq(dev, irq->start,
-					rcar_gen3_thermal_irq, 0,
-				       dev_name(dev), priv);
-		if (ret) {
-			dev_err(dev, "IRQ request failed\n ");
-			goto error_unregister;
+	if (rcar_has_irq_support(priv)) {
+		irq_cnt = platform_irq_count(pdev);
+		for (i = 0; i < irq_cnt; i++) {
+			irq = platform_get_resource(pdev, IORESOURCE_IRQ, i);
+			ret = devm_request_irq(dev, irq->start,
+					       rcar_gen3_thermal_irq,
+					       IRQF_SHARED,
+					       dev_name(dev), priv);
+			if (ret) {
+				dev_err(dev, "IRQ request failed\n ");
+				goto error_unregister;
+			}
 		}
 	}
 
-- 
1.9.1

[PATCH/RFC 2/3] thermal: rcar_gen3_thermal: Modify the way to detect the interrupts

[Khiem Nguyen]

The current implementation is that the interrupt I/F and thermal
sensor are assigned with one-to-one. So it can't be set the more
interrupt trigger to one thermal sensor. Also, the interrupt is
detected by a little bit change in temperature.

In order to solve the above problems, the interrupt of thermal
sensor is changed as below.
  - Change the shared interrupt in each thermal sensors.
  - Detect the interrupt when the temperature is changed
    one degree up and down.

Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 69 +++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index e640a14..dc5f231 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -63,6 +63,11 @@
 
 #define CTEMP_MASK	0xFFF
 
+#define IRQ_TEMP1_BIT	(0x1 << 0)
+#define IRQ_TEMP2_BIT	(0x1 << 1)
+#define IRQ_TEMPD1_BIT	(0x1 << 3)
+#define IRQ_TEMPD2_BIT	(0x1 << 4)
+
 #define MCELSIUS(temp)			((temp) * 1000)
 #define TEMP_IRQ_SHIFT(tsc_id)	(0x1 << tsc_id)
 #define TEMPD_IRQ_SHIFT(tsc_id)	(0x1 << (tsc_id + 3))
@@ -216,28 +221,42 @@ int _linear_temp_converter(struct equation_coefs coef,
 	return _round_temp(temp);
 }
 
+int _linear_celsius_to_temp(struct equation_coefs coef,
+					int ctemp)
+{
+	int temp_code, temp1, temp2;
+
+	temp1 = (ctemp * coef.a1 / 1000 + coef.b1) / 1000;
+	temp2 = (ctemp * coef.a2 / 1000 + coef.b2) / 1000;
+	temp_code = (temp1 + temp2) / 2;
+
+	return temp_code;
+}
+
 /*
  *		Zone device functions
  */
 static int rcar_gen3_thermal_update_temp(struct rcar_gen3_thermal_priv *priv)
 {
 	u32 ctemp;
-	int i;
 	unsigned long flags;
-	u32 reg = REG_GEN3_IRQTEMP1 + (priv->id * 4);
+	int temp_cel, temp_code;
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	for (i = 0; i < 256; i++) {
-		ctemp = thermal_reg_read(priv, REG_GEN3_TEMP) & CTEMP_MASK;
-		if (rcar_has_irq_support(priv)) {
-			thermal_reg_write(priv, reg, ctemp);
-			if (thermal_reg_read(priv, REG_GEN3_IRQSTR) != 0)
-				break;
-		} else
-			break;
+	ctemp = thermal_reg_read(priv, REG_GEN3_TEMP) & CTEMP_MASK;
+	if (rcar_has_irq_support(priv)) {
+		temp_cel = _linear_temp_converter(priv->coef, ctemp);
+
+		/* set the interrupts to exceed the temperature */
+		temp_code = _linear_celsius_to_temp(priv->coef,
+						    temp_cel + MCELSIUS(1));
+		thermal_reg_write(priv, REG_GEN3_IRQTEMP1, temp_code);
 
-		udelay(150);
+		/* set the interrupts to fall below the temperature */
+		temp_code = _linear_celsius_to_temp(priv->coef,
+						    temp_cel - MCELSIUS(1));
+		thermal_reg_write(priv, REG_GEN3_IRQTEMP2, temp_code);
 	}
 
 	priv->ctemp = ctemp;
@@ -283,14 +302,14 @@ static int r8a7795_thermal_init(struct rcar_gen3_thermal_priv *priv)
 
 	thermal_reg_write(priv, REG_GEN3_CTSR, PONM);
 	thermal_reg_write(priv, REG_GEN3_IRQCTL, 0x3F);
-	thermal_reg_write(priv, REG_GEN3_IRQEN, TEMP_IRQ_SHIFT(priv->id) |
-						TEMPD_IRQ_SHIFT(priv->id));
+	thermal_reg_write(priv, REG_GEN3_IRQEN,
+			   IRQ_TEMP1_BIT | IRQ_TEMPD2_BIT);
 	thermal_reg_write(priv, REG_GEN3_CTSR,
-			PONM | AOUT | THBGR | VMEN);
+			   PONM | AOUT | THBGR | VMEN);
 	udelay(100);
 
 	thermal_reg_write(priv, REG_GEN3_CTSR,
-			PONM | AOUT | THBGR | VMEN | VMST | THSST);
+			   PONM | AOUT | THBGR | VMEN | VMST | THSST);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 
@@ -305,11 +324,11 @@ static int r8a7796_thermal_init(struct rcar_gen3_thermal_priv *priv)
 	spin_lock_irqsave(&priv->lock, flags);
 	thermal_reg_write(priv, REG_GEN3_THCTR,  0x0);
 	udelay(1000);
+
 	thermal_reg_write(priv, REG_GEN3_IRQCTL, 0x3F);
-	thermal_reg_write(priv, REG_GEN3_IRQEN, TEMP_IRQ_SHIFT(priv->id) |
-						TEMPD_IRQ_SHIFT(priv->id));
-	thermal_reg_write(priv, REG_GEN3_THCTR,
-						CTCTL | THCNTSEN(BIT_LEN_12));
+	thermal_reg_write(priv, REG_GEN3_IRQEN,
+			   IRQ_TEMP1_BIT | IRQ_TEMPD2_BIT);
+	thermal_reg_write(priv, REG_GEN3_THCTR, CTCTL | THCNTSEN(BIT_LEN_12));
 	reg_val = thermal_reg_read(priv, REG_GEN3_THCTR);
 	reg_val &= ~CTCTL;
 	reg_val |= THSST;
@@ -334,8 +353,7 @@ static void _thermal_irq_ctrl(struct rcar_gen3_thermal_priv *priv, int enable)
 
 	spin_lock_irqsave(&priv->lock, flags);
 	thermal_reg_write(priv, REG_GEN3_IRQMSK,
-		enable ? (TEMP_IRQ_SHIFT(priv->id) |
-			TEMPD_IRQ_SHIFT(priv->id)) : 0);
+		enable ? (IRQ_TEMP1_BIT | IRQ_TEMPD2_BIT) : 0);
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
@@ -361,11 +379,12 @@ static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
 	thermal_reg_write(priv, REG_GEN3_IRQSTR, 0);
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	if ((status & TEMP_IRQ_SHIFT(priv->id)) ||
-		(status & TEMPD_IRQ_SHIFT(priv->id))) {
+	if (status == 0)
+		return IRQ_NONE;
+
+	if (status & (IRQ_TEMP1_BIT | IRQ_TEMPD2_BIT)) {
 		rcar_gen3_thermal_irq_disable(priv);
-		schedule_delayed_work(&priv->work,
-				      msecs_to_jiffies(300));
+		schedule_delayed_work(&priv->work, 0);
 	}
 
 	return IRQ_HANDLED;
-- 
1.9.1

[PATCH/RFC 3/3] arm64: dts: r8a7795: Support shared irq for thermal sensors

[Khiem Nguyen]

This patch adds the shared interrupts for thermal sensors
TSC1/TSC2/TSC3.

Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index db203db..761df2b 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1628,7 +1628,8 @@
 			compatible = "renesas,thermal-r8a7795",
 				"renesas,rcar-gen3-thermal";
 			reg = <0 0xe6198000 0 0x5c>;
-			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 522>;
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			#thermal-sensor-cells = <0>;
@@ -1639,7 +1640,8 @@
 			compatible = "renesas,thermal-r8a7795",
 				"renesas,rcar-gen3-thermal";
 			reg = <0 0xe61a0000 0 0x5c>;
-			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 522>;
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			#thermal-sensor-cells = <0>;
@@ -1650,7 +1652,8 @@
 			compatible = "renesas,thermal-r8a7795",
 				"renesas,rcar-gen3-thermal";
 			reg = <0 0xe61a8000 0 0x5c>;
-			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&cpg CPG_MOD 522>;
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			#thermal-sensor-cells = <0>;
-- 
1.9.1

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

[Kuninori Morimoto]

Hi Khiem-san

Thank you for your patch

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

You want to have "static" function here ?

> +static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> +{
> +	struct rcar_gen3_thermal_priv *priv = devdata;
> +	int ctemp;
> +	unsigned long flags;
> +
> +	rcar_gen3_thermal_update_temp(priv);
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	ctemp = _linear_temp_converter(priv->coef, priv->ctemp);
> +	spin_unlock_irqrestore(&priv->lock, flags);

using pointer on _linear_temp_converter() is reasonable ?
especially for struct equation_coefs coef

> +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,thermal-r8a7795", .data = &r8a7795_data},
> +	{ .compatible = "renesas,thermal-r8a7796", .data = &r8a7796_data},
> +	{ .compatible = "renesas,rcar-gen3-thermal", .data = &r8a7796_data},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);

We can't have general case in this case ?
"renesas,rcar-gen3-thermal" is not needed IMO.
Especially this driver doesn't need to care about back compatibility yet.

> +static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> +{
> +	struct rcar_gen3_thermal_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res, *irq;
> +	int ret = -ENODEV;
> +	int idle;
> +	struct device_node *tz_nd, *tmp_nd;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->dev = dev;
> +
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +
> +	priv->data = of_device_get_match_data(dev);
> +	if (!priv->data)
> +		goto error_unregister;
> +
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	priv->irq = 0;
> +	if (irq) {
> +		priv->irq = 1;
> +		for_each_node_with_property(tz_nd, "polling-delay") {
> +			tmp_nd = of_parse_phandle(tz_nd,
> +					"thermal-sensors", 0);
> +			if (tmp_nd && !strcmp(tmp_nd->full_name,
> +					dev->of_node->full_name)) {
> +				of_property_read_u32(tz_nd, "polling-delay",
> +					&idle);
> +				(idle > 0) ? (priv->irq = 0) :
> +						(priv->irq = 1);
> +				break;
> +			}

it is not readable for me.

	if (idle > 0)
		priv->irq = 0;
	break;

is enough ?

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		goto error_unregister;
> +
> +	priv->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->base)) {
> +		ret = PTR_ERR(priv->base);
> +		goto error_unregister;
> +	}
> +
> +	spin_lock_init(&priv->lock);
> +	INIT_DELAYED_WORK(&priv->work, rcar_gen3_thermal_work);
> +
> +	priv->id = of_alias_get_id(dev->of_node, "tsc");

Do we really need alias ?
is "tsc" good naming ?
Having this explanation on [1/4] patch document is useful.
of_alias_get_id() can return -ENODEV, but no error check ?

> +	priv->zone = devm_thermal_zone_of_sensor_register(dev, 0, priv,
> +				&rcar_gen3_tz_of_ops);
> +
> +	if (IS_ERR(priv->zone)) {
> +		dev_err(dev, "Can't register thermal zone\n");
> +		ret = PTR_ERR(priv->zone);
> +		priv->zone = NULL;
> +		goto error_unregister;
> +	}

It is not bad operation, but not readable.
How about to have local struct thermal_zone_device *zone, like this ?

	zone = devm_thermal_zone_of_sensor_register(xxxx);
	if (IS_ERR(zone)) {
		...
		ret = PTR_ERR(zone);
		goto error_unregister;
	}
	priv->zone = zone;

> +	priv->data->thermal_init(priv);

thermal_init() has return value;

> +	ret = _read_fuse_factor(priv);
> +	if (ret)
> +		goto error_unregister;
> +	_linear_coefficient_calculation(priv);
> +	ret = rcar_gen3_thermal_update_temp(priv);
> +
> +	if (ret < 0)
> +		goto error_unregister;

This is very picky comment about empty line,
but this is readable for me

	ret = _read_fuse_factor(priv);
	if (ret)
		goto error_unregister;

	_linear_coefficient_calculation(priv);

	ret = rcar_gen3_thermal_update_temp(priv);
	if (ret < 0)
		goto error_unregister;

Re: [PATCH 0/4] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal support

[Geert Uytterhoeven]

Hi Khiem,

On Sun, Jun 19, 2016 at 5:31 AM, Khiem Nguyen
<khiem.nguyen.xt@rvc.renesas.com> wrote:
> This patchset adds new thermal sensor driver to support 3 sensors
> found in R-Car Gen3 series.
>
> It has been decided to create new driver, instead of using the existing
> thermal driver due to many differences in HW setting flows, the method
> to calculate temperatures value and some newly supported features.
>
> The new driver can support both polling mode and interrupt mode.
> It has been tested with R-Car H3.
>
> This driver is developed based on early work of Hien Dang and Thao Nguyen.
> All comments are welcome.

Thanks for your series!

Please CC linux-renesas-soc for drivers related to Renesas SoCs.
Please CC Simon for dts and defconfig updates that should go through his tree.

> Khiem Nguyen (4):
>   thermal: rcar_gen3_thermal: Document the R-Car Gen3 thermal bindings
>   thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver support
>   arm64: dts: r8a7795: Add R-Car Gen3 thermal support
>   arm64: defconfig: Enable R-Car Gen3 thermal support
>
>  .../bindings/thermal/rcar-gen3-thermal.txt         |  79 ++++
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  86 ++++
>  arch/arm64/configs/defconfig                       |   2 +
>  drivers/thermal/Kconfig                            |   9 +
>  drivers/thermal/Makefile                           |   1 +
>  drivers/thermal/rcar_gen3_thermal.c                | 524 +++++++++++++++++++++
>  6 files changed, 701 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
>  create mode 100644 drivers/thermal/rcar_gen3_thermal.c

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

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

[Geert Uytterhoeven]

Hi Khiem,

On Sun, Jun 19, 2016 at 5:33 AM, Khiem Nguyen
<khiem.nguyen.xt@rvc.renesas.com> wrote:
>
> Signed-off-by: Hien Dang <hien.dang.eb@rvc.renesas.com>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>

Thanks for your patch!

> ---
>  .../bindings/thermal/rcar-gen3-thermal.txt         | 79 ++++++++++++++++++++++
>  1 file changed, 79 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 0000000..ed6ce45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
> @@ -0,0 +1,79 @@
> +* DT bindings for Renesas R-Car Gen3 Thermal Sensor driver
> +
> +Required properties:
> +- compatible           : "renesas,thermal-<soctype>",

Please use "renesas,<soctype>-thermal", as this is the preferred order for
new bindings.

> +                         Examples with soctypes are:
> +                           - "renesas,thermal-r8a7795" (R-Car H3)

renesas,r8a7795-thermal

> +                           - "renesas,thermal-r8a7796" (R-Car M3)

renesas,r8a7796-thermal, M3-W

> +                           - "renesas,rcar-gen3-thermal" as fallback
> +- reg                  : Address range of the thermal registers.
> +- clocks               : Must contain a reference to the functional clock.

According to the datasheet, the USB_EXTAL clock is also an input for the
thermal block. Should it be added?

> +- #thermal-sensor-cells : Please see ./thermal.txt
> +
> +Option properties:

Optional

> +
> +- interrupts           : Use interrupt
> +- 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 (non interrupt support):

Is there a good reason why you wouldn't want to use interrupts?
If not, please make it mandatory.

> +
> +       tsc1: thermal@e6198000 {
> +               compatible = "renesas,thermal-r8a7795",
> +                       "renesas,rcar-gen3-thermal";
> +               reg = <0 0xe6198000 0 0x5c>;

According to the datasheet, there are more registers beyond this range.
Shouldn't it be "reg = <0 0xe6198000 0 0x68>;"?

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

Re: [PATCH/RFC 3/3] arm64: dts: r8a7795: Support shared irq for thermal sensors

[Geert Uytterhoeven]

Hi Khiem,

On Sun, Jun 19, 2016 at 6:16 AM, Khiem Nguyen
<khiem.nguyen.xt@rvc.renesas.com> wrote:
> This patch adds the shared interrupts for thermal sensors
> TSC1/TSC2/TSC3.
>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@rvc.renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index db203db..761df2b 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -1628,7 +1628,8 @@
>                         compatible = "renesas,thermal-r8a7795",
>                                 "renesas,rcar-gen3-thermal";
>                         reg = <0 0xe6198000 0 0x5c>;
> -                       interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
> +                       interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;

If you make this change, the DT bindings should be updated first.

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

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

[Khiem Nguyen]

Hi Morimoto-san,
 
Thanks for your comments.

> > +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);
> > +}
> 
> You want to have "static" function here ?

Sound good. Will update in v2.

> > +static int rcar_gen3_thermal_get_temp(void *devdata, int *temp) {
> > +	struct rcar_gen3_thermal_priv *priv = devdata;
> > +	int ctemp;
> > +	unsigned long flags;
> > +
> > +	rcar_gen3_thermal_update_temp(priv);
> > +
> > +	spin_lock_irqsave(&priv->lock, flags);
> > +	ctemp = _linear_temp_converter(priv->coef, priv->ctemp);
> > +	spin_unlock_irqrestore(&priv->lock, flags);
> 
> using pointer on _linear_temp_converter() is reasonable ?
> especially for struct equation_coefs coef
 
I failed to see the benefit of the change.
Could you elaborate the points ?
e.g better memory protection, faster byte-code execution, readability, etc


> > +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,thermal-r8a7795", .data = &r8a7795_data},
> > +	{ .compatible = "renesas,thermal-r8a7796", .data = &r8a7796_data},
> > +	{ .compatible = "renesas,rcar-gen3-thermal", .data = &r8a7796_data},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> 
> We can't have general case in this case ?
> "renesas,rcar-gen3-thermal" is not needed IMO.
> Especially this driver doesn't need to care about back compatibility yet.

OK. I see your point. Will update in V2.

> > +static int rcar_gen3_thermal_probe(struct platform_device *pdev) {
> > +	struct rcar_gen3_thermal_priv *priv;
> > +	struct device *dev = &pdev->dev;
> > +	struct resource *res, *irq;
> > +	int ret = -ENODEV;
> > +	int idle;
> > +	struct device_node *tz_nd, *tmp_nd;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	priv->dev = dev;
> > +
> > +	pm_runtime_enable(dev);
> > +	pm_runtime_get_sync(dev);
> > +
> > +	priv->data = of_device_get_match_data(dev);
> > +	if (!priv->data)
> > +		goto error_unregister;
> > +
> > +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +	priv->irq = 0;
> > +	if (irq) {
> > +		priv->irq = 1;
> > +		for_each_node_with_property(tz_nd, "polling-delay") {
> > +			tmp_nd = of_parse_phandle(tz_nd,
> > +					"thermal-sensors", 0);
> > +			if (tmp_nd && !strcmp(tmp_nd->full_name,
> > +					dev->of_node->full_name)) {
> > +				of_property_read_u32(tz_nd, "polling-delay",
> > +					&idle);
> > +				(idle > 0) ? (priv->irq = 0) :
> > +						(priv->irq = 1);
> > +				break;
> > +			}
> 
> it is not readable for me.
> 
> 	if (idle > 0)
> 		priv->irq = 0;
> 	break;
> 
> is enough ?

Unfortunately, it's not.
The code tries to check "polling-delay" in order to select polling mode and get polling duration from DT.
So, your proposal just do 1st part.

> 
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		goto error_unregister;
> > +
> > +	priv->base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(priv->base)) {
> > +		ret = PTR_ERR(priv->base);
> > +		goto error_unregister;
> > +	}
> > +
> > +	spin_lock_init(&priv->lock);
> > +	INIT_DELAYED_WORK(&priv->work, rcar_gen3_thermal_work);
> > +
> > +	priv->id = of_alias_get_id(dev->of_node, "tsc");
> 
> Do we really need alias ?
> is "tsc" good naming ?

It's the abbreviation of Thermal sensor controller.
The term has been described in HW manual. Therefore, I think it's 'reasonable' name.

> Having this explanation on [1/4] patch document is useful.
> of_alias_get_id() can return -ENODEV, but no error check ?

Good point. Will fix in v2.

> > +	priv->zone = devm_thermal_zone_of_sensor_register(dev, 0, priv,
> > +				&rcar_gen3_tz_of_ops);
> > +
> > +	if (IS_ERR(priv->zone)) {
> > +		dev_err(dev, "Can't register thermal zone\n");
> > +		ret = PTR_ERR(priv->zone);
> > +		priv->zone = NULL;
> > +		goto error_unregister;
> > +	}
> 
> It is not bad operation, but not readable.
> How about to have local struct thermal_zone_device *zone, like this ?

It's good point.
I saw that other thermal drivers also did that way.

The original source code follows same code flow as rcar-thermal driver.
Perhaps, we can change the code flow for both drivers, as your idea.

> 	zone = devm_thermal_zone_of_sensor_register(xxxx);
> 	if (IS_ERR(zone)) {
> 		...
> 		ret = PTR_ERR(zone);
> 		goto error_unregister;
> 	}
> 	priv->zone = zone;
> 
> > +	priv->data->thermal_init(priv);
> 
> thermal_init() has return value;

OK. Will fix in v2.

> 
> > +	ret = _read_fuse_factor(priv);
> > +	if (ret)
> > +		goto error_unregister;
> > +	_linear_coefficient_calculation(priv);
> > +	ret = rcar_gen3_thermal_update_temp(priv);
> > +
> > +	if (ret < 0)
> > +		goto error_unregister;
> 
> This is very picky comment about empty line, but this is readable for me
> 
> 	ret = _read_fuse_factor(priv);
> 	if (ret)
> 		goto error_unregister;
> 
> 	_linear_coefficient_calculation(priv);
> 
> 	ret = rcar_gen3_thermal_update_temp(priv);
> 	if (ret < 0)
> 		goto error_unregister;

OK. Newline does not harm anything.
Will add in v2.

Thanks.

Best regards,
KHIEM Nguyen