[PATCHv5,2/5] thermal: rcar_gen3_thermal: Add R-Car Gen3 thermal driver

Commit Message

Niklas Söderlund Dec. 12, 2016, 2:18 p.m. UTC

From: Wolfram Sang <[e-mail]>

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 <[e-mail]>
Signed-off-by: Thao Nguyen <[e-mail]>
Signed-off-by: Khiem Nguyen <[e-mail]>
Signed-off-by: Wolfram Sang <[e-mail]>
[Niklas: document and rework temperature calculation]
Signed-off-by: Niklas Söderlund <[e-mail]>
---
 drivers/thermal/Kconfig             |   9 +
 drivers/thermal/Makefile            |   1 +
 drivers/thermal/rcar_gen3_thermal.c | 328 ++++++++++++++++++++++++++++++++++++
 3 files changed, 338 insertions(+)
 create mode 100644 drivers/thermal/rcar_gen3_thermal.c

Comments

Wolfram Sang Dec. 12, 2016, 9:45 p.m. UTC | #1

Hi Niklas,

thanks for this work!

> +/*
> + * Linear approximation for temperature
> + *
> + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
> + *
> + * The constants a and b are calculated using two triplets of int values PTAT
> + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
> + * coded values from driver. The formula to calculate a and b are taken from
> + * BSP and sparsely documented and understood.
> + *
> + * Examining the linear formula and the formula used to calculate constants a
> + * and b while knowing that the span for PTAT and THCODE values are between
> + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
> + * Integer also needs to be signed so that leaves 7 bits for decimal
> + * fixed point scaling, which amounts to a decimal scaling factor of 100.
> + */
> +
> +#define SCALE_FACTOR 100
> +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
> +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
> +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
> +#define SCALE_TO_MCELSIUS(_x) ((_x) * 10)

Spaces around operators everywhere, please.

I wonder about SCALE_MUL; isn't that more like "unscaling" because _a
and _b are already scaled?

And since _b is always a constant, couldn't we simply drop this macro
and simply do _a * _b (with _a being scaled already and _b not)?

Regards,

   Wolfram

Eduardo Valentin Dec. 13, 2016, 3:50 a.m. UTC | #2

Hey,

On Mon, Dec 12, 2016 at 03:18:02PM +0100, Niklas Söderlund wrote:
> From: Wolfram Sang <[e-mail]>
> 
> 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 <[e-mail]>
> Signed-off-by: Thao Nguyen <[e-mail]>
> Signed-off-by: Khiem Nguyen <[e-mail]>
> Signed-off-by: Wolfram Sang <[e-mail]>
> [Niklas: document and rework temperature calculation]
> Signed-off-by: Niklas Söderlund <[e-mail]>
> ---
>  drivers/thermal/Kconfig             |   9 +
>  drivers/thermal/Makefile            |   1 +
>  drivers/thermal/rcar_gen3_thermal.c | 328 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 338 insertions(+)
>  create mode 100644 drivers/thermal/rcar_gen3_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index a13541b..da71f94 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -243,6 +243,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

cool that we have got COMPILE_TEST to work again!

thanks for that, I appreciate it.

> +	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 c92eb22..1216fb3 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 0000000..7d78498
> --- /dev/null
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -0,0 +1,328 @@
> +/*
> + *  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 {
> +	int a1;
> +	int b1;
> +	int a2;
> +	int b2;

a little description of each coeff would be welcome.

> +};
> +
> +struct rcar_gen3_thermal_tsc {
> +	void __iomem *base;
> +	struct thermal_zone_device *zone;
> +	struct equation_coefs coef;
> +	struct mutex lock;
> +};
> +
> +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);
> +};
> +
> +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);
> +}
> +
> +/*
> + * Linear approximation for temperature
> + *
> + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a

the type of equation described above can be described using
of-thermal/DT. Have you tried using the coefficients DT property to get
those from DT?

> + *
> + * The constants a and b are calculated using two triplets of int values PTAT
> + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
> + * coded values from driver. The formula to calculate a and b are taken from
> + * BSP and sparsely documented and understood.
> + *

hmmm.. OK. that gets a bit more interesting. 

So you can get a and b queried from hardware. cool.

but you can also get those hardcoded in the code. In that case, I would
suggest hardcode them in DT, instead, using the coefficients property.

> + * Examining the linear formula and the formula used to calculate constants a
> + * and b while knowing that the span for PTAT and THCODE values are between
> + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
> + * Integer also needs to be signed so that leaves 7 bits for decimal
> + * fixed point scaling, which amounts to a decimal scaling factor of 100.
> + */


I see.

> +
> +#define SCALE_FACTOR 100
> +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
> +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
> +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
> +#define SCALE_TO_MCELSIUS(_x) ((_x) * 10)
> +
> +#define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
> +
> +/* no idea where these constants come from */

thermal simulation, maybe?

> +#define TJ_1 SCALE_INT(96)
> +#define TJ_3 SCALE_INT(-41)
> +
> +static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> +					 int *ptat, int *thcode)
> +{
> +	int tj_2;
> +
> +	/* TODO: Find documentation and document constant calculation formula */
> +
> +	tj_2 = SCALE_DIV(SCALE_MUL(SCALE_INT(ptat[1] - ptat[2]), SCALE_INT(137)),
> +			 SCALE_INT(ptat[0] - ptat[2])) - SCALE_INT(41);
> +
> +	coef->a1 = SCALE_DIV(SCALE_INT(thcode[1] - thcode[2]), tj_2 - TJ_3);
> +	coef->b1 = SCALE_INT(thcode[2]) - SCALE_MUL(coef->a1, TJ_3);
> +
> +	coef->a2 = SCALE_DIV(SCALE_INT(thcode[1] - thcode[0]), tj_2 - TJ_1);
> +	coef->b2 = SCALE_INT(thcode[0]) - SCALE_MUL(coef->a2, TJ_1);
> +}
> +
> +static int rcar_gen3_thermal_round(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 int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> +{
> +	struct rcar_gen3_thermal_tsc *tsc = devdata;
> +	int mcelsius, val1, val2;
> +	u32 reg;
> +
> +	/* Read register and convert to mili Celsius */
> +	mutex_lock(&tsc->lock);
> +
> +	reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> +
> +	val1 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b1, tsc->coef.a1);
> +	val2 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b2, tsc->coef.a2);

I see. there are actually two sensors here, and 

> +	mcelsius = SCALE_TO_MCELSIUS((val1 + val2) / 2);

the driver always output the average of them.

based on previous driver history, people would actually expose all
sensors. Unless you want to be really strict to always average the two.

which to me seams a bit odd also, based on other drivers I have seen.
Hardware folks would typically ask for caring about all sensors. For
example, care about max, not average. But average is also fine, as long
as temperature does not move fast enough in one of the sides.

For example, you see a spike on one, and the other, still fine (taking
some thermal resistance before seen the heat), you would already take an
action. And here you will first average (filter the spike out), before
taking any action. That essentially means, you are delaying any action
further until the rise is seen by the average / filter.

Is that really what you want?

> +
> +	mutex_unlock(&tsc->lock);
> +
> +	/* Make sure we are inside specifications */
> +	if ((mcelsius < MCELSIUS(-40)) || (mcelsius > MCELSIUS(125)))
> +		return -EIO;
> +
> +	/* Round value to device granularity setting */
> +	*temp = rcar_gen3_thermal_round(mcelsius);
> +
> +	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 */
> +	/* TODO: Read values from hardware on supported platforms */
> +	int ptat[3] = { 2351, 1509, 435 };
> +	int thcode[TSC_MAX_NUM][3] = {
> +		{ 3248, 2800, 2221 },
> +		{ 3245, 2795, 2216 },
> +		{ 3250, 2805, 2237 },
> +	};

Now that I understand a bit what is going on here, I believe the above
hard coded values may be moved to DT, as long as you are willing to
describe more than one sensor and attach each one thermal zone.

> +
> +	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);
> +		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);

oh, the thcode's are currently not read then?

> +
> +		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> +							    &rcar_gen3_tz_of_ops);

and you are already doing it, therefore those coeff could really come
from the DT, no?

> +		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 <[e-mail]>");
> -- 
> 2.10.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfram Sang Dec. 13, 2016, 5:38 a.m. UTC | #3

Hi,

> > + *
> > + * The constants a and b are calculated using two triplets of int values PTAT
> > + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
> > + * coded values from driver. The formula to calculate a and b are taken from
> > + * BSP and sparsely documented and understood.
> > + *
> 
> hmmm.. OK. that gets a bit more interesting. 
> 
> So you can get a and b queried from hardware. cool.
> 
> but you can also get those hardcoded in the code. In that case, I would
> suggest hardcode them in DT, instead, using the coefficients property.

It is only the engineering samples which have the coeffs hardcoded. All
future revisions of the same SoC will have the values as fuses from
registers. Sadly, we won't have access to newer versions for a while. To
avoid having seperate DTSI per engineering sample revision, I planned
for using the new soc_device_match() mechanism which was introduced for
exactly such use cases.

> > +static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> > +{
> > +	struct rcar_gen3_thermal_tsc *tsc = devdata;
> > +	int mcelsius, val1, val2;
> > +	u32 reg;
> > +
> > +	/* Read register and convert to mili Celsius */
> > +	mutex_lock(&tsc->lock);
> > +
> > +	reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
> > +
> > +	val1 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b1, tsc->coef.a1);
> > +	val2 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b2, tsc->coef.a2);
> 
> I see. there are actually two sensors here, and 

Really? val1 and val2 are both based on 'reg'. Also, the datasheet
explicitly mentions 'three sensors in the LSI'.

Thanks for the fast response!

   Wolfram

Geert Uytterhoeven Dec. 13, 2016, 8:19 a.m. UTC | #4

Hi Niklas,

On Mon, Dec 12, 2016 at 3:18 PM, Niklas Söderlund
<[e-mail]> wrote:
> +/*
> + * Linear approximation for temperature
> + *
> + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
> + *
> + * The constants a and b are calculated using two triplets of int values PTAT
> + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
> + * coded values from driver. The formula to calculate a and b are taken from
> + * BSP and sparsely documented and understood.
> + *
> + * Examining the linear formula and the formula used to calculate constants a
> + * and b while knowing that the span for PTAT and THCODE values are between
> + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
> + * Integer also needs to be signed so that leaves 7 bits for decimal
> + * fixed point scaling, which amounts to a decimal scaling factor of 100.
> + */
> +
> +#define SCALE_FACTOR 100

What about using 128 instead?
Fixed point is much easier with shifts
(the compiler will turn multiplications in shifts where appropriate).

> +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
> +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
> +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))

DIV_ROUND_CLOSEST()

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Geert Uytterhoeven Dec. 13, 2016, 8:25 a.m. UTC | #5

On Mon, Dec 12, 2016 at 10:45 PM, Wolfram Sang <[e-mail]> wrote:
>> +/*
>> + * Linear approximation for temperature
>> + *
>> + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
>> + *
>> + * The constants a and b are calculated using two triplets of int values PTAT
>> + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
>> + * coded values from driver. The formula to calculate a and b are taken from
>> + * BSP and sparsely documented and understood.
>> + *
>> + * Examining the linear formula and the formula used to calculate constants a
>> + * and b while knowing that the span for PTAT and THCODE values are between
>> + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
>> + * Integer also needs to be signed so that leaves 7 bits for decimal
>> + * fixed point scaling, which amounts to a decimal scaling factor of 100.
>> + */
>> +
>> +#define SCALE_FACTOR 100
>> +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
>> +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
>> +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
>> +#define SCALE_TO_MCELSIUS(_x) ((_x) * 10)
>
> Spaces around operators everywhere, please.
>
> I wonder about SCALE_MUL; isn't that more like "unscaling" because _a
> and _b are already scaled?

No, it's a standard fixed point multiplication, where you have to compensate
for the double scaling due to the multiplication.

Perhaps the macros should be called e.g. FIXPT_MUL() and FIXPT_DIV()?

> And since _b is always a constant, couldn't we simply drop this macro
> and simply do _a * _b (with _a being scaled already and _b not)?

Yes, for multiplication by an integer (not a fixed point number), you can
just do the multiplication. Which also avoids having to care about rounding.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Wolfram Sang Dec. 13, 2016, 8:39 a.m. UTC | #6

> Perhaps the macros should be called e.g. FIXPT_MUL() and FIXPT_DIV()?

Yes, that naming would be more readable to me.

Niklas Söderlund Dec. 13, 2016, 9:43 a.m. UTC | #7

Hi Eduardo,

Thanks for your feedback. I will skip commenting where Wolfram already 
have.

On 2016-12-12 19:50:54 -0800, Eduardo Valentin wrote:

<snip>

> > +/* Structure for thermal temperature calculation */
> > +struct equation_coefs {
> > +	int a1;
> > +	int b1;
> > +	int a2;
> > +	int b2;
> 
> a little description of each coeff would be welcome.

Yes, I too would like to have better documentation of the formulas and 
the meaning of the coefficients.

<snip, Wolfram already covert this>

> 
> > +
> > +	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);
> > +		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> 
> oh, the thcode's are currently not read then?

No as Wolfram commented, reading THCODE and PTAT from hardware is not 
possible on the boards we have to test on so I think this needs to be 
added once we can test it. Do you agree this is the best option?

> 
> > +
> > +		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> > +							    &rcar_gen3_tz_of_ops);
> 
> and you are already doing it, therefore those coeff could really come
> from the DT, no?
> 
> > +		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 <[e-mail]>");
> > -- 
> > 2.10.2
> >

Niklas Söderlund Dec. 13, 2016, 9:53 a.m. UTC | #8

Hi Geert,

Thanks for your feedback.

On 2016-12-13 09:19:53 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, Dec 12, 2016 at 3:18 PM, Niklas Söderlund
> <[e-mail]> wrote:
> > +/*
> > + * Linear approximation for temperature
> > + *
> > + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
> > + *
> > + * The constants a and b are calculated using two triplets of int values PTAT
> > + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
> > + * coded values from driver. The formula to calculate a and b are taken from
> > + * BSP and sparsely documented and understood.
> > + *
> > + * Examining the linear formula and the formula used to calculate constants a
> > + * and b while knowing that the span for PTAT and THCODE values are between
> > + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
> > + * Integer also needs to be signed so that leaves 7 bits for decimal
> > + * fixed point scaling, which amounts to a decimal scaling factor of 100.
> > + */
> > +
> > +#define SCALE_FACTOR 100
> 
> What about using 128 instead?
> Fixed point is much easier with shifts
> (the compiler will turn multiplications in shifts where appropriate).

I tried using binary scaled instead of decimal scaled fixed point but 
noticed that the diff I got compared to the original formula which used 
decimal scaled (factor 1000) where larger so I picked decimal scaling.  

Maybe with feedback from Morimoto-san or Khiem we can switch to binary 
scaled number as it should over all increase the accuracy of the 
calculations, but then maybe some of the other hard coded constant also 
should be updated?

> 
> > +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
> > +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
> > +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
> 
> DIV_ROUND_CLOSEST()

Don't DIV_ROUND_CLOSEST() require the divisor to be a positive integer?  

  tj_2 = SCALE_DIV(SCALE_MUL(SCALE_INT(ptat[1] - ptat[2]), SCALE_INT(137)),
                   SCALE_INT(ptat[0] - ptat[2])) - SCALE_INT(41);

In this case if ptat[0] < (ptat[2] + 41) the divisor is negative so I 
think DIV_ROUND_CLOSEST() can't be used, or am I misunderstanding?

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]
> 
> 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

Niklas Söderlund Dec. 13, 2016, 10:07 a.m. UTC | #9

On 2016-12-13 09:39:04 +0100, Wolfram Sang wrote:
> 
> > Perhaps the macros should be called e.g. FIXPT_MUL() and FIXPT_DIV()?
> 
> Yes, that naming would be more readable to me.
> 

FIXPT_ is a much better prefix, will update in next version. Thanks for 
the suggestion Geert.

Geert Uytterhoeven Dec. 13, 2016, 10:09 a.m. UTC | #10

Hi Niklas,

On Tue, Dec 13, 2016 at 10:53 AM, Niklas Söderlund
<[e-mail]> wrote:
> On 2016-12-13 09:19:53 +0100, Geert Uytterhoeven wrote:
>> On Mon, Dec 12, 2016 at 3:18 PM, Niklas Söderlund
>> <[e-mail]> wrote:
>> > +/*
>> > + * Linear approximation for temperature
>> > + *
>> > + * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
>> > + *
>> > + * The constants a and b are calculated using two triplets of int values PTAT
>> > + * and THCODE. PTAT and THCODE can either be read from hardware or use hard
>> > + * coded values from driver. The formula to calculate a and b are taken from
>> > + * BSP and sparsely documented and understood.
>> > + *
>> > + * Examining the linear formula and the formula used to calculate constants a
>> > + * and b while knowing that the span for PTAT and THCODE values are between
>> > + * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
>> > + * Integer also needs to be signed so that leaves 7 bits for decimal
>> > + * fixed point scaling, which amounts to a decimal scaling factor of 100.
>> > + */
>> > +
>> > +#define SCALE_FACTOR 100
>>
>> What about using 128 instead?
>> Fixed point is much easier with shifts
>> (the compiler will turn multiplications in shifts where appropriate).
>
> I tried using binary scaled instead of decimal scaled fixed point but
> noticed that the diff I got compared to the original formula which used
> decimal scaled (factor 1000) where larger so I picked decimal scaling.

Interesting...

>> > +#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
>> > +#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
>> > +#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
>>
>> DIV_ROUND_CLOSEST()
>
> Don't DIV_ROUND_CLOSEST() require the divisor to be a positive integer?

Right.

>   tj_2 = SCALE_DIV(SCALE_MUL(SCALE_INT(ptat[1] - ptat[2]), SCALE_INT(137)),
>                    SCALE_INT(ptat[0] - ptat[2])) - SCALE_INT(41);
>
> In this case if ptat[0] < (ptat[2] + 41) the divisor is negative so I
> think DIV_ROUND_CLOSEST() can't be used, or am I misunderstanding?

Then we need our own macro that can handle that. Or enhance
DIV_ROUND_CLOSEST().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Dec. 14, 2016, 4:38 a.m. UTC | #11

hey,

On Tue, Dec 13, 2016 at 10:43:40AM +0100, Niklas Söderlund wrote:
> Hi Eduardo,
> 
> Thanks for your feedback. I will skip commenting where Wolfram already 
> have.
> 
> On 2016-12-12 19:50:54 -0800, Eduardo Valentin wrote:
> 
> <snip>
> 
> > > +/* Structure for thermal temperature calculation */
> > > +struct equation_coefs {
> > > +	int a1;
> > > +	int b1;
> > > +	int a2;
> > > +	int b2;
> > 
> > a little description of each coeff would be welcome.
> 
> Yes, I too would like to have better documentation of the formulas and 
> the meaning of the coefficients.
> 
> <snip, Wolfram already covert this>

Ok..

> 
> > 
> > > +
> > > +	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);
> > > +		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> > 
> > oh, the thcode's are currently not read then?
> 
> No as Wolfram commented, reading THCODE and PTAT from hardware is not 
> possible on the boards we have to test on so I think this needs to be 
> added once we can test it. Do you agree this is the best option?

I agree here. I was under the impression that would be both types of
chips out there, with and without thcode. But looks like, at the end,
only a few engineers will have access to those without it, right?

If you still think those without the support need right support, I would
suggest moving the hardcoded values to DT. Specially if those hardcoded
values change across chip version (so you can better describe using DT).
But, in case you think this population of chips wont grow, then might be
ok to keep the way it is, even though looks not so nice in the code :-).

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index a13541b..da71f94 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -243,6 +243,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 c92eb22..1216fb3 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 0000000..7d78498
--- /dev/null
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -0,0 +1,328 @@ 
+/*
+ *  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 {
+	int a1;
+	int b1;
+	int a2;
+	int b2;
+};
+
+struct rcar_gen3_thermal_tsc {
+	void __iomem *base;
+	struct thermal_zone_device *zone;
+	struct equation_coefs coef;
+	struct mutex lock;
+};
+
+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);
+};
+
+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);
+}
+
+/*
+ * Linear approximation for temperature
+ *
+ * [reg] = [temp] * a + b => [temp] = ([reg] - b) / a
+ *
+ * The constants a and b are calculated using two triplets of int values PTAT
+ * and THCODE. PTAT and THCODE can either be read from hardware or use hard
+ * coded values from driver. The formula to calculate a and b are taken from
+ * BSP and sparsely documented and understood.
+ *
+ * Examining the linear formula and the formula used to calculate constants a
+ * and b while knowing that the span for PTAT and THCODE values are between
+ * 0x000 and 0xfff the largest integer possible is 0xfff * 0xfff == 0xffe001.
+ * Integer also needs to be signed so that leaves 7 bits for decimal
+ * fixed point scaling, which amounts to a decimal scaling factor of 100.
+ */
+
+#define SCALE_FACTOR 100
+#define SCALE_INT(_x) ((_x) * SCALE_FACTOR)
+#define SCALE_MUL(_a, _b) (((_a)*(_b)) / SCALE_FACTOR)
+#define SCALE_DIV(_a, _b) (((_a)*SCALE_FACTOR)/(_b))
+#define SCALE_TO_MCELSIUS(_x) ((_x) * 10)
+
+#define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
+
+/* no idea where these constants come from */
+#define TJ_1 SCALE_INT(96)
+#define TJ_3 SCALE_INT(-41)
+
+static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
+					 int *ptat, int *thcode)
+{
+	int tj_2;
+
+	/* TODO: Find documentation and document constant calculation formula */
+
+	tj_2 = SCALE_DIV(SCALE_MUL(SCALE_INT(ptat[1] - ptat[2]), SCALE_INT(137)),
+			 SCALE_INT(ptat[0] - ptat[2])) - SCALE_INT(41);
+
+	coef->a1 = SCALE_DIV(SCALE_INT(thcode[1] - thcode[2]), tj_2 - TJ_3);
+	coef->b1 = SCALE_INT(thcode[2]) - SCALE_MUL(coef->a1, TJ_3);
+
+	coef->a2 = SCALE_DIV(SCALE_INT(thcode[1] - thcode[0]), tj_2 - TJ_1);
+	coef->b2 = SCALE_INT(thcode[0]) - SCALE_MUL(coef->a2, TJ_1);
+}
+
+static int rcar_gen3_thermal_round(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 int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
+{
+	struct rcar_gen3_thermal_tsc *tsc = devdata;
+	int mcelsius, val1, val2;
+	u32 reg;
+
+	/* Read register and convert to mili Celsius */
+	mutex_lock(&tsc->lock);
+
+	reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
+
+	val1 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b1, tsc->coef.a1);
+	val2 = SCALE_DIV(SCALE_INT(reg) - tsc->coef.b2, tsc->coef.a2);
+	mcelsius = SCALE_TO_MCELSIUS((val1 + val2) / 2);
+
+	mutex_unlock(&tsc->lock);
+
+	/* Make sure we are inside specifications */
+	if ((mcelsius < MCELSIUS(-40)) || (mcelsius > MCELSIUS(125)))
+		return -EIO;
+
+	/* Round value to device granularity setting */
+	*temp = rcar_gen3_thermal_round(mcelsius);
+
+	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 */
+	/* TODO: Read values from hardware on supported platforms */
+	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);
+		rcar_gen3_thermal_calc_coefs(&tsc->coef, 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 <[e-mail]>");