[PATCH 00/18] thermal: exynos: further fixes and cleanups

[PATCH 00/18] thermal: exynos: further fixes and cleanups

[Bartlomiej Zolnierkiewicz]

Hi,

This patchset contains Exynos thermal driver fixes and cleanups.

Highlights:
* Move code valid for all SoCs from ->tmu_initialize method
  to exynos_tmu_initialize().
* Add ->set_trip_[temp,hyst] methods and convert ->tmu_initialize
  method implementations to use them.
* Remove bogus trip reporting to user-space.

The patchset is based on top of:
- "[PATCH 00/14] thermal: exynos: pending fixes and cleanups"
  (https://lkml.org/lkml/2018/4/16/256))
- "[PATCH] thermal: samsung: Remove support for Exynos5440"
  (https://lkml.org/lkml/2018/4/26/405)

Tested on Exynos4210 based Trats board and Exynos5422 based Odroid-XU3
Lite board.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Bartlomiej Zolnierkiewicz (18):
  thermal: exynos: fix setting rising_threshold for Exynos5433
  thermal: exynos: always check for trips points existence
  thermal: exynos: always check for critical trip points existence
  thermal: exynos: check STATUS register in exynos_tmu_initialize()
  thermal: exynos: use sanitize_temp_error() in exynos7_tmu_initialize()
  thermal: exynos: fix trips limit checking in get_th_reg()
  thermal: exynos: remove threshold_code checking from
    exynos4210_tmu_initialize()
  thermal: exynos: make ->tmu_initialize method void
  thermal: exynos: clear IRQs later in exynos4412_tmu_initialize()
  thermal: exynos: move IRQs clearing to exynos_tmu_initialize()
  thermal: exynos: add exynos*_tmu_set_[trip,hyst]() helpers
  thermal: exynos: do not use trips structure directly in
    ->tmu_initialize
  thermal: exynos: set trips in ascending order in
    exynos7_tmu_initialize()
  thermal: exynos: move trips setting to exynos_tmu_initialize()
  thermal: exynos: check return values of ->get_trip_[temp,hyst] methods
  thermal: exynos: cleanup code for enabling threshold interrupts
  thermal: exynos: remove unused defines for Exynos5433
  thermal: exynos: remove trip reporting to user-space

 drivers/thermal/samsung/exynos_tmu.c | 548 ++++++++++++++---------------------
 1 file changed, 215 insertions(+), 333 deletions(-)

-- 
1.9.1

[PATCH 01/18] thermal: exynos: fix setting rising_threshold for Exynos5433

[Bartlomiej Zolnierkiewicz]

Add missing clearing of the previous value when setting rising
temperature threshold.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index cda716c..523d26e 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -577,6 +577,7 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
 		threshold_code = temp_to_code(data, temp);
 
 		rising_threshold = readl(data->base + rising_reg_offset);
+		rising_threshold &= ~(0xff << j * 8);
 		rising_threshold |= (threshold_code << j * 8);
 		writel(rising_threshold, data->base + rising_reg_offset);
 
-- 
1.9.1

Re: [PATCH 01/18] thermal: exynos: fix setting rising_threshold for Exynos5433

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Add missing clearing of the previous value when setting rising
> temperature threshold.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index cda716c..523d26e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -577,6 +577,7 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
>  		threshold_code = temp_to_code(data, temp);
>  
>  		rising_threshold = readl(data->base + rising_reg_offset);
> +		rising_threshold &= ~(0xff << j * 8);
>  		rising_threshold |= (threshold_code << j * 8);

Bartlomiej,

I see this code is duplicated all around the driver, so I can't blame you to
fix it in the same way it is written today but this is not how to deal with
fields in a register mapping. Can you fix it by adding correct macros with
masks?

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 01/18] thermal: exynos: fix setting rising_threshold for Exynos5433

[Bartlomiej Zolnierkiewicz]

On Monday, April 30, 2018 04:36:56 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Add missing clearing of the previous value when setting rising
> > temperature threshold.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index cda716c..523d26e 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -577,6 +577,7 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
> >  		threshold_code = temp_to_code(data, temp);
> >  
> >  		rising_threshold = readl(data->base + rising_reg_offset);
> > +		rising_threshold &= ~(0xff << j * 8);
> >  		rising_threshold |= (threshold_code << j * 8);
> 
> Bartlomiej,
> 
> I see this code is duplicated all around the driver, so I can't blame you to
> fix it in the same way it is written today but this is not how to deal with

This patch is a bugfix (which may be backported later) and it shouldn't be
mixed with cleanups.

> fields in a register mapping. Can you fix it by adding correct macros with
> masks?

After my patch series we still have following occurrences of "RMW" code:

        th = readl(data->base + EXYNOS_THD_TEMP_RISE);
        th &= ~(0xff << 8 * trip);
        th |= temp_to_code(data, temp) << 8 * trip;
        writel(th, data->base + EXYNOS_THD_TEMP_RISE);

        th = readl(data->base + EXYNOS_THD_TEMP_FALL);
        th &= ~(0xff << 8 * trip);
        if (hyst)
                th |= temp_to_code(data, temp - hyst) << 8 * trip;
        writel(th, data->base + EXYNOS_THD_TEMP_FALL);

        th = readl(data->base + reg_off);
        th &= ~(0xff << j * 8);
        th |= (temp_to_code(data, temp) << j * 8);
        writel(th, data->base + reg_off);

        th = readl(data->base + reg_off);
        th &= ~(0xff << j * 8);
        th |= (temp_to_code(data, temp - hyst) << j * 8);
        writel(th, data->base + reg_off);

        th = readl(data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
        th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
        th |= temp_to_code(data, temp) << (16 * bit_off);
        writel(th, data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);

        th = readl(data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
        th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
        th |= temp_to_code(data, temp - hyst) << (16 * bit_off);
        writel(th, data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);

My personal opinion is that adding new macro for the code above will
only obfuscate it and make it harder to understand,

Anyway how would you like this new macro to look like (how generic or
specific it should be etc.)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 02/18] thermal: exynos: always check for trips points existence

[Bartlomiej Zolnierkiewicz]

* Check for trip points existence in exynos_tmu_initialize() so it is
  checked on all SoCs.

* Use dev_err() instead of pr_err().

* Fix dev_err() to reference "device tree" not "of-thermal.c".

* Remove no longer needed checks from exynos4210_tmu_initialize() and
  get_th_reg().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 523d26e..9e040eb 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -309,12 +309,6 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
 	unsigned long temp;
 	int i;
 
-	if (!trips) {
-		pr_err("%s: Cannot get trip points from of-thermal.c!\n",
-		       __func__);
-		return 0;
-	}
-
 	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
 		if (trips[i].type == THERMAL_TRIP_CRITICAL)
 			continue;
@@ -334,14 +328,23 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
 static int exynos_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+	struct thermal_zone_device *tzd = data->tzd;
+	const struct thermal_trip * const trips =
+		of_thermal_get_trip_points(tzd);
 	int ret;
 
-	if (of_thermal_get_ntrips(data->tzd) > data->ntrip) {
+	if (!trips) {
+		dev_err(&pdev->dev,
+			"Cannot get trip points from device tree!\n");
+		return -ENODEV;
+	}
+
+	if (of_thermal_get_ntrips(tzd) > data->ntrip) {
 		dev_info(&pdev->dev,
 			 "More trip points than supported by this TMU.\n");
 		dev_info(&pdev->dev,
 			 "%d trip points should be configured in polling mode.\n",
-			 (of_thermal_get_ntrips(data->tzd) - data->ntrip));
+			 (of_thermal_get_ntrips(tzd) - data->ntrip));
 	}
 
 	mutex_lock(&data->lock);
@@ -397,13 +400,6 @@ static int exynos4210_tmu_initialize(struct platform_device *pdev)
 	unsigned long reference, temp;
 	unsigned int status;
 
-	if (!trips) {
-		pr_err("%s: Cannot get trip points from of-thermal.c!\n",
-		       __func__);
-		ret = -ENODEV;
-		goto out;
-	}
-
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
 	if (!status) {
 		ret = -EBUSY;
-- 
1.9.1

[PATCH 03/18] thermal: exynos: always check for critical trip points existence

[Bartlomiej Zolnierkiewicz]

* Check for critical trip point existence in exynos_tmu_initialize()
  so it is checked on all SoCs (except Exynos5433 for now).

* Use dev_err() instead of pr_err().

* Fix dev_err() to reference "device tree" not "of-thermal.c".

* Remove no longer needed check from exynos4412_tmu_initialize().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 9e040eb..a0c1604 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -331,7 +331,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	struct thermal_zone_device *tzd = data->tzd;
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tzd);
-	int ret;
+	int ret = 0, temp;
 
 	if (!trips) {
 		dev_err(&pdev->dev,
@@ -339,6 +339,14 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	if (data->soc != SOC_ARCH_EXYNOS5433) /* FIXME */
+		ret = tzd->ops->get_crit_temp(tzd, &temp);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"No CRITICAL trip point defined in device tree!\n");
+		goto out;
+	}
+
 	if (of_thermal_get_ntrips(tzd) > data->ntrip) {
 		dev_info(&pdev->dev,
 			 "More trip points than supported by this TMU.\n");
@@ -356,7 +364,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	mutex_unlock(&data->lock);
 	if (!IS_ERR(data->clk_sec))
 		clk_disable(data->clk_sec);
-
+out:
 	return ret;
 }
 
@@ -480,13 +488,6 @@ static int exynos4412_tmu_initialize(struct platform_device *pdev)
 		}
 	}
 
-	if (i == of_thermal_get_ntrips(data->tzd)) {
-		pr_err("%s: No CRITICAL trip point defined at of-thermal.c!\n",
-		       __func__);
-		ret = -EINVAL;
-		goto out;
-	}
-
 	threshold_code = temp_to_code(data, crit_temp / MCELSIUS);
 	/* 1-4 level to be assigned in th0 reg */
 	rising_threshold &= ~(0xff << 8 * i);
-- 
1.9.1

Re: [PATCH 03/18] thermal: exynos: always check for critical trip points existence

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:18PM +0200, Bartlomiej Zolnierkiewicz wrote:
> * Check for critical trip point existence in exynos_tmu_initialize()
>   so it is checked on all SoCs (except Exynos5433 for now).

Why "except Exynos5433" ?
 
> * Use dev_err() instead of pr_err().
> 
> * Fix dev_err() to reference "device tree" not "of-thermal.c".
> 
> * Remove no longer needed check from exynos4412_tmu_initialize().
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Re: [PATCH 03/18] thermal: exynos: always check for critical trip points existence

[Bartlomiej Zolnierkiewicz]

On Monday, April 30, 2018 04:44:50 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:18PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > * Check for critical trip point existence in exynos_tmu_initialize()
> >   so it is checked on all SoCs (except Exynos5433 for now).
> 
> Why "except Exynos5433" ?

I was a bit vague here - Exynos5433 DTS needs to be fixed first to define
critical trip points.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 03/18] thermal: exynos: always check for critical trip points existence

[Daniel Lezcano]

On Mon, Apr 30, 2018 at 05:24:15PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Monday, April 30, 2018 04:44:50 PM Daniel Lezcano wrote:
> > On Thu, Apr 26, 2018 at 01:51:18PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > * Check for critical trip point existence in exynos_tmu_initialize()
> > >   so it is checked on all SoCs (except Exynos5433 for now).
> > 
> > Why "except Exynos5433" ?
> 
> I was a bit vague here - Exynos5433 DTS needs to be fixed first to define
> critical trip points.

Is it possible to fix the DT in the series, so we get rid of the "FIXME" ?

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 03/18] thermal: exynos: always check for critical trip points existence

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 11:00:18 AM Daniel Lezcano wrote:
> On Mon, Apr 30, 2018 at 05:24:15PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Monday, April 30, 2018 04:44:50 PM Daniel Lezcano wrote:
> > > On Thu, Apr 26, 2018 at 01:51:18PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > * Check for critical trip point existence in exynos_tmu_initialize()
> > > >   so it is checked on all SoCs (except Exynos5433 for now).
> > > 
> > > Why "except Exynos5433" ?
> > 
> > I was a bit vague here - Exynos5433 DTS needs to be fixed first to define
> > critical trip points.
> 
> Is it possible to fix the DT in the series, so we get rid of the "FIXME" ?

This DT change needs more effort so it is planned for some later time
(also doing it in this series would create dependency on arm-soc tree).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 04/18] thermal: exynos: check STATUS register in exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

STATUS register is present on all SoCs so move its checking into
exynos_tmu_initialize().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 45 +++++++++++-------------------------
 1 file changed, 13 insertions(+), 32 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index a0c1604..3b41666 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -331,6 +331,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	struct thermal_zone_device *tzd = data->tzd;
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tzd);
+	unsigned int status;
 	int ret = 0, temp;
 
 	if (!trips) {
@@ -359,7 +360,13 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	clk_enable(data->clk);
 	if (!IS_ERR(data->clk_sec))
 		clk_enable(data->clk_sec);
-	ret = data->tmu_initialize(pdev);
+
+	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
+	if (!status)
+		ret = -EBUSY;
+	else
+		ret = data->tmu_initialize(pdev);
+
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 	if (!IS_ERR(data->clk_sec))
@@ -406,13 +413,6 @@ static int exynos4210_tmu_initialize(struct platform_device *pdev)
 		of_thermal_get_trip_points(tz);
 	int ret = 0, threshold_code, i;
 	unsigned long reference, temp;
-	unsigned int status;
-
-	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
-	if (!status) {
-		ret = -EBUSY;
-		goto out;
-	}
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
 
@@ -441,16 +441,10 @@ static int exynos4412_tmu_initialize(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(data->tzd);
-	unsigned int status, trim_info, con, ctrl, rising_threshold;
+	unsigned int trim_info, con, ctrl, rising_threshold;
 	int ret = 0, threshold_code, i;
 	unsigned long crit_temp = 0;
 
-	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
-	if (!status) {
-		ret = -EBUSY;
-		goto out;
-	}
-
 	if (data->soc == SOC_ARCH_EXYNOS3250 ||
 	    data->soc == SOC_ARCH_EXYNOS4412 ||
 	    data->soc == SOC_ARCH_EXYNOS5250) {
@@ -497,7 +491,6 @@ static int exynos4412_tmu_initialize(struct platform_device *pdev)
 	con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 
-out:
 	return ret;
 }
 
@@ -505,17 +498,11 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int status, trim_info;
+	unsigned int trim_info;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
 	int temp, temp_hist;
 	int ret = 0, threshold_code, i, sensor_id, cal_type;
 
-	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
-	if (!status) {
-		ret = -EBUSY;
-		goto out;
-	}
-
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
 
@@ -590,7 +577,7 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-out:
+
 	return ret;
 }
 
@@ -598,18 +585,12 @@ static int exynos7_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int status, trim_info;
+	unsigned int trim_info;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
 	int ret = 0, threshold_code, i;
 	int temp, temp_hist;
 	unsigned int reg_off, bit_off;
 
-	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
-	if (!status) {
-		ret = -EBUSY;
-		goto out;
-	}
-
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 
 	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
@@ -667,7 +648,7 @@ static int exynos7_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-out:
+
 	return ret;
 }
 
-- 
1.9.1

Re: [PATCH 04/18] thermal: exynos: check STATUS register in exynos_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:19PM +0200, Bartlomiej Zolnierkiewicz wrote:
> STATUS register is present on all SoCs so move its checking into
> exynos_tmu_initialize().
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 05/18] thermal: exynos: use sanitize_temp_error() in exynos7_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Fix sanitize_temp_error() to handle Exynos7 SoCs and then use it in
exynos7_tmu_initialize().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 3b41666..5a64879 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -286,7 +286,11 @@ static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
 
 static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
 {
-	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
+	u16 tmu_temp_mask =
+		(data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK
+						: EXYNOS_TMU_TEMP_MASK;
+
+	data->temp_error1 = trim_info & tmu_temp_mask;
 	data->temp_error2 = ((trim_info >> EXYNOS_TRIMINFO_85_SHIFT) &
 				EXYNOS_TMU_TEMP_MASK);
 
@@ -592,12 +596,7 @@ static int exynos7_tmu_initialize(struct platform_device *pdev)
 	unsigned int reg_off, bit_off;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
-
-	data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK;
-	if (!data->temp_error1 ||
-	    (data->min_efuse_value > data->temp_error1) ||
-	    (data->temp_error1 > data->max_efuse_value))
-		data->temp_error1 = data->efuse_value & EXYNOS_TMU_TEMP_MASK;
+	sanitize_temp_error(data, trim_info);
 
 	/* Write temperature code for rising and falling threshold */
 	for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) {
-- 
1.9.1

Re: [PATCH 05/18] thermal: exynos: use sanitize_temp_error() in exynos7_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:20PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Fix sanitize_temp_error() to handle Exynos7 SoCs and then use it in
> exynos7_tmu_initialize().
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---


Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 06/18] thermal: exynos: fix trips limit checking in get_th_reg()

[Bartlomiej Zolnierkiewicz]

of_thermal_get_ntrips() may return value bigger than supported
by a given SoC (i.e. on Exynos5422/5800) so fix the code to not
iterate the loop for i values >= data->ntrip.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 5a64879..58cd68e 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tz);
 	unsigned long temp;
-	int i;
+	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
 
-	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
+	for (i = 0; i < ntrips; i++) {
 		if (trips[i].type == THERMAL_TRIP_CRITICAL)
 			continue;
 
-- 
1.9.1

Re: [PATCH 06/18] thermal: exynos: fix trips limit checking in get_th_reg()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> of_thermal_get_ntrips() may return value bigger than supported
> by a given SoC (i.e. on Exynos5422/5800) 

Can you elaborate a bit ?

> so fix the code to not
> iterate the loop for i values >= data->ntrip.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 5a64879..58cd68e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
>  	const struct thermal_trip * const trips =
>  		of_thermal_get_trip_points(tz);
>  	unsigned long temp;
> -	int i;
> +	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
>  
> -	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
> +	for (i = 0; i < ntrips; i++) {
>  		if (trips[i].type == THERMAL_TRIP_CRITICAL)
>  			continue;
>  
> -- 
> 1.9.1
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 06/18] thermal: exynos: fix trips limit checking in get_th_reg()

[Bartlomiej Zolnierkiewicz]

On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > of_thermal_get_ntrips() may return value bigger than supported
> > by a given SoC (i.e. on Exynos5422/5800) 
> 
> Can you elaborate a bit ?

Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones)
while data->ntrip is 4, the current code works fine by accident as
the threshold values for trip points 5 & 6 don't fit into 32-bits
threshold value (however since they are passive ones this is okay).

Of course the code for handling passive trip points still needs to
be fixed to properly handle all odd cases (which are not present in
current DTS files).

[1] arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi

> > so fix the code to not
> > iterate the loop for i values >= data->ntrip.
> > 
> > There should be no functional changes caused by this patch.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 5a64879..58cd68e 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
> >  	const struct thermal_trip * const trips =
> >  		of_thermal_get_trip_points(tz);
> >  	unsigned long temp;
> > -	int i;
> > +	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
> >  
> > -	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
> > +	for (i = 0; i < ntrips; i++) {
> >  		if (trips[i].type == THERMAL_TRIP_CRITICAL)
> >  			continue;

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 06/18] thermal: exynos: fix trips limit checking in get_th_reg()

[Daniel Lezcano]

On Mon, Apr 30, 2018 at 05:48:29PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote:
> > On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > of_thermal_get_ntrips() may return value bigger than supported
> > > by a given SoC (i.e. on Exynos5422/5800) 
> > 
> > Can you elaborate a bit ?
> 
> Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones)
> while data->ntrip is 4, the current code works fine by accident as
> the threshold values for trip points 5 & 6 don't fit into 32-bits
> threshold value (however since they are passive ones this is okay).

Ah, I see. data->ntrip is the SoC specific tmu max value capping what is
defined in the DT, right ?

> Of course the code for handling passive trip points still needs to
> be fixed to properly handle all odd cases (which are not present in
> current DTS files).
> 
> [1] arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi

Yeah, it is definitively a good idea of cleaning up this driver :/


Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 06/18] thermal: exynos: fix trips limit checking in get_th_reg()

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 11:39:14 AM Daniel Lezcano wrote:
> On Mon, Apr 30, 2018 at 05:48:29PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote:
> > > On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > of_thermal_get_ntrips() may return value bigger than supported
> > > > by a given SoC (i.e. on Exynos5422/5800) 
> > > 
> > > Can you elaborate a bit ?
> > 
> > Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones)
> > while data->ntrip is 4, the current code works fine by accident as
> > the threshold values for trip points 5 & 6 don't fit into 32-bits
> > threshold value (however since they are passive ones this is okay).
> 
> Ah, I see. data->ntrip is the SoC specific tmu max value capping what is
> defined in the DT, right ?

It is SoC specific max value for active + critical trips points.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 07/18] thermal: exynos: remove threshold_code checking from exynos4210_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

On Exynos4210 one-point trimming is always used and data->temp_error1
is equal to 75. Therefore temp_to_code() will never return negative
value for the reference temperature conversion.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 58cd68e..26a0cb9 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -423,10 +423,6 @@ static int exynos4210_tmu_initialize(struct platform_device *pdev)
 	/* Write temperature code for threshold */
 	reference = trips[0].temperature / MCELSIUS;
 	threshold_code = temp_to_code(data, reference);
-	if (threshold_code < 0) {
-		ret = threshold_code;
-		goto out;
-	}
 	writeb(threshold_code, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
 
 	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
@@ -436,7 +432,7 @@ static int exynos4210_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-out:
+
 	return ret;
 }
 
-- 
1.9.1

Re: [PATCH 07/18] thermal: exynos: remove threshold_code checking from exynos4210_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:22PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Exynos4210 one-point trimming is always used and data->temp_error1
> is equal to 75. Therefore temp_to_code() will never return negative
> value for the reference temperature conversion.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 6 +-----
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>



-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 08/18] thermal: exynos: make ->tmu_initialize method void

[Bartlomiej Zolnierkiewicz]

All implementations of ->tmu_initialize always return 0 so make
the method void and convert all implementations accordingly.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 26a0cb9..44a426a 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -220,7 +220,7 @@ struct exynos_tmu_data {
 	unsigned int ntrip;
 	bool enabled;
 
-	int (*tmu_initialize)(struct platform_device *pdev);
+	void (*tmu_initialize)(struct platform_device *pdev);
 	void (*tmu_control)(struct platform_device *pdev, bool on);
 	int (*tmu_read)(struct exynos_tmu_data *data);
 	void (*tmu_set_emulation)(struct exynos_tmu_data *data, int temp);
@@ -369,7 +369,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	if (!status)
 		ret = -EBUSY;
 	else
-		ret = data->tmu_initialize(pdev);
+		data->tmu_initialize(pdev);
 
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
@@ -409,13 +409,13 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	mutex_unlock(&data->lock);
 }
 
-static int exynos4210_tmu_initialize(struct platform_device *pdev)
+static void exynos4210_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tz);
-	int ret = 0, threshold_code, i;
+	int threshold_code, i;
 	unsigned long reference, temp;
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
@@ -432,17 +432,15 @@ static int exynos4210_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-
-	return ret;
 }
 
-static int exynos4412_tmu_initialize(struct platform_device *pdev)
+static void exynos4412_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(data->tzd);
 	unsigned int trim_info, con, ctrl, rising_threshold;
-	int ret = 0, threshold_code, i;
+	int threshold_code, i;
 	unsigned long crit_temp = 0;
 
 	if (data->soc == SOC_ARCH_EXYNOS3250 ||
@@ -490,18 +488,16 @@ static int exynos4412_tmu_initialize(struct platform_device *pdev)
 	con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
 	con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
-
-	return ret;
 }
 
-static int exynos5433_tmu_initialize(struct platform_device *pdev)
+static void exynos5433_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
 	int temp, temp_hist;
-	int ret = 0, threshold_code, i, sensor_id, cal_type;
+	int threshold_code, i, sensor_id, cal_type;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
@@ -577,17 +573,15 @@ static int exynos5433_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-
-	return ret;
 }
 
-static int exynos7_tmu_initialize(struct platform_device *pdev)
+static void exynos7_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
-	int ret = 0, threshold_code, i;
+	int threshold_code, i;
 	int temp, temp_hist;
 	unsigned int reg_off, bit_off;
 
@@ -643,8 +637,6 @@ static int exynos7_tmu_initialize(struct platform_device *pdev)
 	}
 
 	data->tmu_clear_irqs(data);
-
-	return ret;
 }
 
 static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
-- 
1.9.1

Re: [PATCH 08/18] thermal: exynos: make ->tmu_initialize method void

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:23PM +0200, Bartlomiej Zolnierkiewicz wrote:
> All implementations of ->tmu_initialize always return 0 so make
> the method void and convert all implementations accordingly.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 09/18] thermal: exynos: clear IRQs later in exynos4412_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Clear IRQs after enabling thermal tripping (it should make no
difference in driver operation). This prepares the driver code
to moving IRQs clearing call from ->tmu_initialize method to
exynos_tmu_initialize().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 44a426a..1664d37 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -470,8 +470,6 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	writel(rising_threshold, data->base + EXYNOS_THD_TEMP_RISE);
 	writel(get_th_reg(data, 0, true), data->base + EXYNOS_THD_TEMP_FALL);
 
-	data->tmu_clear_irqs(data);
-
 	/* if last threshold limit is also present */
 	for (i = 0; i < of_thermal_get_ntrips(data->tzd); i++) {
 		if (trips[i].type == THERMAL_TRIP_CRITICAL) {
@@ -488,6 +486,8 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
 	con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
+
+	data->tmu_clear_irqs(data);
 }
 
 static void exynos5433_tmu_initialize(struct platform_device *pdev)
-- 
1.9.1

Re: [PATCH 09/18] thermal: exynos: clear IRQs later in exynos4412_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:24PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Clear IRQs after enabling thermal tripping (it should make no
> difference in driver operation). This prepares the driver code
> to moving IRQs clearing call from ->tmu_initialize method to
> exynos_tmu_initialize().
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 10/18] thermal: exynos: move IRQs clearing to exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Move ->tmu_clear_irqs call from ->tmu_initialize method to
exynos_tmu_initialize().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 1664d37..82484c5 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -368,8 +368,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
 	if (!status)
 		ret = -EBUSY;
-	else
+	else {
 		data->tmu_initialize(pdev);
+		data->tmu_clear_irqs(data);
+	}
 
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
@@ -430,8 +432,6 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev)
 		writeb(temp - reference, data->base +
 		       EXYNOS4210_TMU_REG_TRIG_LEVEL0 + i * 4);
 	}
-
-	data->tmu_clear_irqs(data);
 }
 
 static void exynos4412_tmu_initialize(struct platform_device *pdev)
@@ -486,8 +486,6 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
 	con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
-
-	data->tmu_clear_irqs(data);
 }
 
 static void exynos5433_tmu_initialize(struct platform_device *pdev)
@@ -571,8 +569,6 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 		falling_threshold |= (threshold_code << j * 8);
 		writel(falling_threshold, data->base + falling_reg_offset);
 	}
-
-	data->tmu_clear_irqs(data);
 }
 
 static void exynos7_tmu_initialize(struct platform_device *pdev)
@@ -635,8 +631,6 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
 		writel(falling_threshold,
 		       data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
 	}
-
-	data->tmu_clear_irqs(data);
 }
 
 static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
-- 
1.9.1

Re: [PATCH 10/18] thermal: exynos: move IRQs clearing to exynos_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:25PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Move ->tmu_clear_irqs call from ->tmu_initialize method to
> exynos_tmu_initialize().
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>



-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 10/18] thermal: exynos: move IRQs clearing to exynos_tmu_initialize()

[Eduardo Valentin]

Hey,

On Thu, Apr 26, 2018 at 01:51:25PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Move ->tmu_clear_irqs call from ->tmu_initialize method to
> exynos_tmu_initialize().
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 1664d37..82484c5 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -368,8 +368,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	status = readb(data->base + EXYNOS_TMU_REG_STATUS);
>  	if (!status)
>  		ret = -EBUSY;
> -	else
> +	else {
>  		data->tmu_initialize(pdev);
> +		data->tmu_clear_irqs(data);
> +	}

You should actually enclose the if statement also in curls here:
	if (!status) {
		ret = -EBUSY;
	} else {
		data->tmu_initialize(pdev);
		data->tmu_clear_irqs(data);
	}


Fixing myself, but next time pay attention to kernel coding style
CHECK: Unbalanced braces around else statement
#35: FILE: drivers/thermal/samsung/exynos_tmu.c:371:
+	else {

total: 0 errors, 0 warnings, 1 checks, 43 lines checked

> 

[PATCH 11/18] thermal: exynos: add exynos*_tmu_set_[trip,hyst]() helpers

[Bartlomiej Zolnierkiewicz]

Add exynos*_tmu_set_[trip,hyst]() helpers and convert
all ->tmu_initialize implementations accordingly.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 282 +++++++++++++++++------------------
 1 file changed, 140 insertions(+), 142 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 82484c5..80265d2 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -305,30 +305,6 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
 			EXYNOS_TMU_TEMP_MASK;
 }
 
-static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling)
-{
-	struct thermal_zone_device *tz = data->tzd;
-	const struct thermal_trip * const trips =
-		of_thermal_get_trip_points(tz);
-	unsigned long temp;
-	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
-
-	for (i = 0; i < ntrips; i++) {
-		if (trips[i].type == THERMAL_TRIP_CRITICAL)
-			continue;
-
-		temp = trips[i].temperature / MCELSIUS;
-		if (falling)
-			temp -= (trips[i].hysteresis / MCELSIUS);
-		else
-			threshold &= ~(0xff << 8 * i);
-
-		threshold |= temp_to_code(data, temp) << 8 * i;
-	}
-
-	return threshold;
-}
-
 static int exynos_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
@@ -411,37 +387,79 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	mutex_unlock(&data->lock);
 }
 
+static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
+					 int trip, u8 temp)
+{
+	const struct thermal_trip * const trips =
+		of_thermal_get_trip_points(data->tzd);
+	u8 ref, th_code;
+
+	ref = trips[0].temperature / MCELSIUS;
+
+	if (trip == 0) {
+		th_code = temp_to_code(data, ref);
+		writeb(th_code, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
+	}
+
+	temp -= ref;
+	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip * 4);
+}
+
 static void exynos4210_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tz);
-	int threshold_code, i;
-	unsigned long reference, temp;
+	unsigned long temp;
+	int i;
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
 
-	/* Write temperature code for threshold */
-	reference = trips[0].temperature / MCELSIUS;
-	threshold_code = temp_to_code(data, reference);
-	writeb(threshold_code, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
-
 	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
 		temp = trips[i].temperature / MCELSIUS;
-		writeb(temp - reference, data->base +
-		       EXYNOS4210_TMU_REG_TRIG_LEVEL0 + i * 4);
+		exynos4210_tmu_set_trip_temp(data, i, temp);
+	}
+}
+
+static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data,
+					 int trip, u8 temp)
+{
+	u32 th, con;
+
+	th = readl(data->base + EXYNOS_THD_TEMP_RISE);
+	th &= ~(0xff << 8 * trip);
+	th |= temp_to_code(data, temp) << 8 * trip;
+	writel(th, data->base + EXYNOS_THD_TEMP_RISE);
+
+	if (trip == 3) {
+		con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
+		con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
+		writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 	}
 }
 
+static void exynos4412_tmu_set_trip_hyst(struct exynos_tmu_data *data,
+					 int trip, u8 temp, u8 hyst)
+{
+	u32 th;
+
+	th = readl(data->base + EXYNOS_THD_TEMP_FALL);
+	th &= ~(0xff << 8 * trip);
+	if (hyst)
+		th |= temp_to_code(data, temp - hyst) << 8 * trip;
+	writel(th, data->base + EXYNOS_THD_TEMP_FALL);
+}
+
 static void exynos4412_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+	struct thermal_zone_device *tz = data->tzd;
 	const struct thermal_trip * const trips =
-		of_thermal_get_trip_points(data->tzd);
-	unsigned int trim_info, con, ctrl, rising_threshold;
-	int threshold_code, i;
-	unsigned long crit_temp = 0;
+		of_thermal_get_trip_points(tz);
+	unsigned long temp, hyst;
+	unsigned int trim_info, ctrl;
+	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
 
 	if (data->soc == SOC_ARCH_EXYNOS3250 ||
 	    data->soc == SOC_ARCH_EXYNOS4412 ||
@@ -465,27 +483,53 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 	sanitize_temp_error(data, trim_info);
 
 	/* Write temperature code for rising and falling threshold */
-	rising_threshold = readl(data->base + EXYNOS_THD_TEMP_RISE);
-	rising_threshold = get_th_reg(data, rising_threshold, false);
-	writel(rising_threshold, data->base + EXYNOS_THD_TEMP_RISE);
-	writel(get_th_reg(data, 0, true), data->base + EXYNOS_THD_TEMP_FALL);
-
-	/* if last threshold limit is also present */
-	for (i = 0; i < of_thermal_get_ntrips(data->tzd); i++) {
-		if (trips[i].type == THERMAL_TRIP_CRITICAL) {
-			crit_temp = trips[i].temperature;
-			break;
-		}
+	for (i = 0; i < ntrips; i++) {
+		temp = trips[i].temperature / MCELSIUS;
+		exynos4412_tmu_set_trip_temp(data, i, temp);
+
+		hyst = trips[i].hysteresis / MCELSIUS;
+		exynos4412_tmu_set_trip_hyst(data, i, temp, hyst);
 	}
+}
 
-	threshold_code = temp_to_code(data, crit_temp / MCELSIUS);
-	/* 1-4 level to be assigned in th0 reg */
-	rising_threshold &= ~(0xff << 8 * i);
-	rising_threshold |= threshold_code << 8 * i;
-	writel(rising_threshold, data->base + EXYNOS_THD_TEMP_RISE);
-	con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
-	con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
-	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
+static void exynos5433_tmu_set_trip_temp(struct exynos_tmu_data *data,
+					 int trip, u8 temp)
+{
+	unsigned int reg_off, j;
+	u32 th;
+
+	if (trip > 3) {
+		reg_off = EXYNOS5433_THD_TEMP_RISE7_4;
+		j = trip - 4;
+	} else {
+		reg_off = EXYNOS5433_THD_TEMP_RISE3_0;
+		j = trip;
+	}
+
+	th = readl(data->base + reg_off);
+	th &= ~(0xff << j * 8);
+	th |= (temp_to_code(data, temp) << j * 8);
+	writel(th, data->base + reg_off);
+}
+
+static void exynos5433_tmu_set_trip_hyst(struct exynos_tmu_data *data,
+					 int trip, u8 temp, u8 hyst)
+{
+	unsigned int reg_off, j;
+	u32 th;
+
+	if (trip > 3) {
+		reg_off = EXYNOS5433_THD_TEMP_FALL7_4;
+		j = trip - 4;
+	} else {
+		reg_off = EXYNOS5433_THD_TEMP_FALL3_0;
+		j = trip;
+	}
+
+	th = readl(data->base + reg_off);
+	th &= ~(0xff << j * 8);
+	th |= (temp_to_code(data, temp - hyst) << j * 8);
+	writel(th, data->base + reg_off);
 }
 
 static void exynos5433_tmu_initialize(struct platform_device *pdev)
@@ -493,9 +537,7 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
-	unsigned int rising_threshold = 0, falling_threshold = 0;
-	int temp, temp_hist;
-	int threshold_code, i, sensor_id, cal_type;
+	int sensor_id, cal_type, i, temp, hyst;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
@@ -525,111 +567,67 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 
 	/* Write temperature code for rising and falling threshold */
 	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		int rising_reg_offset, falling_reg_offset;
-		int j = 0;
-
-		switch (i) {
-		case 0:
-		case 1:
-		case 2:
-		case 3:
-			rising_reg_offset = EXYNOS5433_THD_TEMP_RISE3_0;
-			falling_reg_offset = EXYNOS5433_THD_TEMP_FALL3_0;
-			j = i;
-			break;
-		case 4:
-		case 5:
-		case 6:
-		case 7:
-			rising_reg_offset = EXYNOS5433_THD_TEMP_RISE7_4;
-			falling_reg_offset = EXYNOS5433_THD_TEMP_FALL7_4;
-			j = i - 4;
-			break;
-		default:
-			continue;
-		}
-
 		/* Write temperature code for rising threshold */
 		tz->ops->get_trip_temp(tz, i, &temp);
 		temp /= MCELSIUS;
-		threshold_code = temp_to_code(data, temp);
-
-		rising_threshold = readl(data->base + rising_reg_offset);
-		rising_threshold &= ~(0xff << j * 8);
-		rising_threshold |= (threshold_code << j * 8);
-		writel(rising_threshold, data->base + rising_reg_offset);
+		exynos5433_tmu_set_trip_temp(data, i, temp);
 
 		/* Write temperature code for falling threshold */
-		tz->ops->get_trip_hyst(tz, i, &temp_hist);
-		temp_hist = temp - (temp_hist / MCELSIUS);
-		threshold_code = temp_to_code(data, temp_hist);
-
-		falling_threshold = readl(data->base + falling_reg_offset);
-		falling_threshold &= ~(0xff << j * 8);
-		falling_threshold |= (threshold_code << j * 8);
-		writel(falling_threshold, data->base + falling_reg_offset);
+		tz->ops->get_trip_hyst(tz, i, &hyst);
+		hyst /= MCELSIUS;
+		exynos5433_tmu_set_trip_hyst(data, i, temp, hyst);
 	}
 }
 
+static void exynos7_tmu_set_trip_temp(struct exynos_tmu_data *data,
+				      int trip, u8 temp)
+{
+	unsigned int reg_off, bit_off;
+	u32 th;
+
+	reg_off = ((7 - trip) / 2) * 4;
+	bit_off = ((8 - trip) % 2);
+
+	th = readl(data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
+	th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
+	th |= temp_to_code(data, temp) << (16 * bit_off);
+	writel(th, data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
+}
+
+static void exynos7_tmu_set_trip_hyst(struct exynos_tmu_data *data,
+				      int trip, u8 temp, u8 hyst)
+{
+	unsigned int reg_off, bit_off;
+	u32 th;
+
+	reg_off = ((7 - trip) / 2) * 4;
+	bit_off = ((8 - trip) % 2);
+
+	th = readl(data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
+	th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
+	th |= temp_to_code(data, temp - hyst) << (16 * bit_off);
+	writel(th, data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
+}
+
 static void exynos7_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
-	unsigned int rising_threshold = 0, falling_threshold = 0;
-	int threshold_code, i;
-	int temp, temp_hist;
-	unsigned int reg_off, bit_off;
+	int i, temp, hyst;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
 
 	/* Write temperature code for rising and falling threshold */
 	for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) {
-		/*
-		 * On exynos7 there are 4 rising and 4 falling threshold
-		 * registers (0x50-0x5c and 0x60-0x6c respectively). Each
-		 * register holds the value of two threshold levels (at bit
-		 * offsets 0 and 16). Based on the fact that there are atmost
-		 * eight possible trigger levels, calculate the register and
-		 * bit offsets where the threshold levels are to be written.
-		 *
-		 * e.g. EXYNOS7_THD_TEMP_RISE7_6 (0x50)
-		 * [24:16] - Threshold level 7
-		 * [8:0] - Threshold level 6
-		 * e.g. EXYNOS7_THD_TEMP_RISE5_4 (0x54)
-		 * [24:16] - Threshold level 5
-		 * [8:0] - Threshold level 4
-		 *
-		 * and similarly for falling thresholds.
-		 *
-		 * Based on the above, calculate the register and bit offsets
-		 * for rising/falling threshold levels and populate them.
-		 */
-		reg_off = ((7 - i) / 2) * 4;
-		bit_off = ((8 - i) % 2);
-
 		tz->ops->get_trip_temp(tz, i, &temp);
 		temp /= MCELSIUS;
+		exynos7_tmu_set_trip_temp(data, i, temp);
 
-		tz->ops->get_trip_hyst(tz, i, &temp_hist);
-		temp_hist = temp - (temp_hist / MCELSIUS);
-
-		/* Set 9-bit temperature code for rising threshold levels */
-		threshold_code = temp_to_code(data, temp);
-		rising_threshold = readl(data->base +
-			EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
-		rising_threshold &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
-		rising_threshold |= threshold_code << (16 * bit_off);
-		writel(rising_threshold,
-		       data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
-
-		/* Set 9-bit temperature code for falling threshold levels */
-		threshold_code = temp_to_code(data, temp_hist);
-		falling_threshold &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off));
-		falling_threshold |= threshold_code << (16 * bit_off);
-		writel(falling_threshold,
-		       data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off);
+		tz->ops->get_trip_hyst(tz, i, &hyst);
+		hyst /= MCELSIUS;
+		exynos7_tmu_set_trip_hyst(data, i, temp, hyst);
 	}
 }
 
-- 
1.9.1

Re: [PATCH 11/18] thermal: exynos: add exynos*_tmu_set_[trip,hyst]() helpers

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:26PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Add exynos*_tmu_set_[trip,hyst]() helpers and convert
> all ->tmu_initialize implementations accordingly.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 282 +++++++++++++++++------------------
>  1 file changed, 140 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 82484c5..80265d2 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -305,30 +305,6 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
>  			EXYNOS_TMU_TEMP_MASK;
>  }

[ ... ]

> +static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
> +					 int trip, u8 temp)
> +{
> +	const struct thermal_trip * const trips =
> +		of_thermal_get_trip_points(data->tzd);
> +	u8 ref, th_code;

I would not do this kind of change (unsigned long => u8) with this patch.
Just code reorg and another patch to tweak the variable types if needed.



-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 11/18] thermal: exynos: add exynos*_tmu_set_[trip,hyst]() helpers

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 11:55:40 AM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:26PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Add exynos*_tmu_set_[trip,hyst]() helpers and convert
> > all ->tmu_initialize implementations accordingly.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 282 +++++++++++++++++------------------
> >  1 file changed, 140 insertions(+), 142 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 82484c5..80265d2 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -305,30 +305,6 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
> >  			EXYNOS_TMU_TEMP_MASK;
> >  }
> 
> [ ... ]
> 
> > +static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
> > +					 int trip, u8 temp)
> > +{
> > +	const struct thermal_trip * const trips =
> > +		of_thermal_get_trip_points(data->tzd);
> > +	u8 ref, th_code;
> 
> I would not do this kind of change (unsigned long => u8) with this patch.
> Just code reorg and another patch to tweak the variable types if needed.

This is so minor change and it is has been already tested so I prefer to
not split it from this patch now.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 12/18] thermal: exynos: do not use trips structure directly in ->tmu_initialize

[Bartlomiej Zolnierkiewicz]

Use ->get_trip_[temp,hyst] methods instead of using trips structure
directly in all ->tmu_initialize method implementations.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 80265d2..12b60e2 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -409,15 +409,13 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	const struct thermal_trip * const trips =
-		of_thermal_get_trip_points(tz);
-	unsigned long temp;
-	int i;
+	int i, temp;
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
 
 	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		temp = trips[i].temperature / MCELSIUS;
+		tz->ops->get_trip_temp(tz, i, &temp);
+		temp /= MCELSIUS;
 		exynos4210_tmu_set_trip_temp(data, i, temp);
 	}
 }
@@ -455,11 +453,9 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	const struct thermal_trip * const trips =
-		of_thermal_get_trip_points(tz);
-	unsigned long temp, hyst;
 	unsigned int trim_info, ctrl;
 	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
+	int temp, hyst;
 
 	if (data->soc == SOC_ARCH_EXYNOS3250 ||
 	    data->soc == SOC_ARCH_EXYNOS4412 ||
@@ -484,10 +480,12 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 
 	/* Write temperature code for rising and falling threshold */
 	for (i = 0; i < ntrips; i++) {
-		temp = trips[i].temperature / MCELSIUS;
+		tz->ops->get_trip_temp(tz, i, &temp);
+		temp /= MCELSIUS;
 		exynos4412_tmu_set_trip_temp(data, i, temp);
 
-		hyst = trips[i].hysteresis / MCELSIUS;
+		tz->ops->get_trip_hyst(tz, i, &hyst);
+		hyst /= MCELSIUS;
 		exynos4412_tmu_set_trip_hyst(data, i, temp, hyst);
 	}
 }
-- 
1.9.1

Re: [PATCH 12/18] thermal: exynos: do not use trips structure directly in ->tmu_initialize

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:27PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Use ->get_trip_[temp,hyst] methods instead of using trips structure
> directly in all ->tmu_initialize method implementations.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 13/18] thermal: exynos: set trips in ascending order in exynos7_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Set trips in ascending order in exynos7_tmu_initialize() (it should
make no difference in driver operation). This prepares the driver
code to moving trips setting from ->tmu_initialize method to
exynos_tmu_initialize().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 12b60e2..571511f 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -618,7 +618,7 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
 	sanitize_temp_error(data, trim_info);
 
 	/* Write temperature code for rising and falling threshold */
-	for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) {
+	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
 		tz->ops->get_trip_temp(tz, i, &temp);
 		temp /= MCELSIUS;
 		exynos7_tmu_set_trip_temp(data, i, temp);
-- 
1.9.1

Re: [PATCH 13/18] thermal: exynos: set trips in ascending order in exynos7_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:28PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Set trips in ascending order in exynos7_tmu_initialize() (it should
> make no difference in driver operation). This prepares the driver
> code to moving trips setting from ->tmu_initialize method to
> exynos_tmu_initialize().
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 12b60e2..571511f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -618,7 +618,7 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
>  	sanitize_temp_error(data, trim_info);
>  
>  	/* Write temperature code for rising and falling threshold */
> -	for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) {
> +	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {

Capped with data->ntrip ?

>  		tz->ops->get_trip_temp(tz, i, &temp);
>  		temp /= MCELSIUS;
>  		exynos7_tmu_set_trip_temp(data, i, temp);
> -- 
> 1.9.1
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 13/18] thermal: exynos: set trips in ascending order in exynos7_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 12:02:42 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:28PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Set trips in ascending order in exynos7_tmu_initialize() (it should
> > make no difference in driver operation). This prepares the driver
> > code to moving trips setting from ->tmu_initialize method to
> > exynos_tmu_initialize().
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 12b60e2..571511f 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -618,7 +618,7 @@ static void exynos7_tmu_initialize(struct platform_device *pdev)
> >  	sanitize_temp_error(data, trim_info);
> >  
> >  	/* Write temperature code for rising and falling threshold */
> > -	for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) {
> > +	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
> 
> Capped with data->ntrip ?

No need to, the code in question is removed in patch #14 (also Exynos7
DTS doesn't define more trip points than data->ntrip).

> >  		tz->ops->get_trip_temp(tz, i, &temp);
> >  		temp /= MCELSIUS;
> >  		exynos7_tmu_set_trip_temp(data, i, temp);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 14/18] thermal: exynos: move trips setting to exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

* Add dummy exynos4210_tmu_set_trip_hyst() helper.

* Add ->tmu_set_trip_temp and ->tmu_set_trip_hyst methods to struct
  exynos_tmu_data and set them in exynos_map_dt_data().

* Move trips setting to exynos_tmu_initialize().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 88 +++++++++++++++---------------------
 1 file changed, 37 insertions(+), 51 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 571511f..244aaf6 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -220,6 +220,10 @@ struct exynos_tmu_data {
 	unsigned int ntrip;
 	bool enabled;
 
+	void (*tmu_set_trip_temp)(struct exynos_tmu_data *data, int trip,
+				 u8 temp);
+	void (*tmu_set_trip_hyst)(struct exynos_tmu_data *data, int trip,
+				 u8 temp, u8 hyst);
 	void (*tmu_initialize)(struct platform_device *pdev);
 	void (*tmu_control)(struct platform_device *pdev, bool on);
 	int (*tmu_read)(struct exynos_tmu_data *data);
@@ -312,7 +316,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	const struct thermal_trip * const trips =
 		of_thermal_get_trip_points(tzd);
 	unsigned int status;
-	int ret = 0, temp;
+	int ret = 0, temp, hyst;
 
 	if (!trips) {
 		dev_err(&pdev->dev,
@@ -345,7 +349,24 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	if (!status)
 		ret = -EBUSY;
 	else {
+		int i, ntrips =
+			min_t(int, of_thermal_get_ntrips(tzd), data->ntrip);
+
 		data->tmu_initialize(pdev);
+
+		/* Write temperature code for rising and falling threshold */
+		for (i = 0; i < ntrips; i++) {
+			/* Write temperature code for rising threshold */
+			tzd->ops->get_trip_temp(tzd, i, &temp);
+			temp /= MCELSIUS;
+			data->tmu_set_trip_temp(data, i, temp);
+
+			/* Write temperature code for falling threshold */
+			tzd->ops->get_trip_hyst(tzd, i, &hyst);
+			hyst /= MCELSIUS;
+			data->tmu_set_trip_hyst(data, i, temp, hyst);
+		}
+
 		data->tmu_clear_irqs(data);
 	}
 
@@ -405,19 +426,17 @@ static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data,
 	writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip * 4);
 }
 
+/* failing thresholds are not supported on Exynos4210 */
+static void exynos4210_tmu_set_trip_hyst(struct exynos_tmu_data *data,
+					 int trip, u8 temp, u8 hyst)
+{
+}
+
 static void exynos4210_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
-	int i, temp;
 
 	sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO));
-
-	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		tz->ops->get_trip_temp(tz, i, &temp);
-		temp /= MCELSIUS;
-		exynos4210_tmu_set_trip_temp(data, i, temp);
-	}
 }
 
 static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data,
@@ -452,10 +471,7 @@ static void exynos4412_tmu_set_trip_hyst(struct exynos_tmu_data *data,
 static void exynos4412_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info, ctrl;
-	int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip);
-	int temp, hyst;
 
 	if (data->soc == SOC_ARCH_EXYNOS3250 ||
 	    data->soc == SOC_ARCH_EXYNOS4412 ||
@@ -477,17 +493,6 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev)
 		trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 
 	sanitize_temp_error(data, trim_info);
-
-	/* Write temperature code for rising and falling threshold */
-	for (i = 0; i < ntrips; i++) {
-		tz->ops->get_trip_temp(tz, i, &temp);
-		temp /= MCELSIUS;
-		exynos4412_tmu_set_trip_temp(data, i, temp);
-
-		tz->ops->get_trip_hyst(tz, i, &hyst);
-		hyst /= MCELSIUS;
-		exynos4412_tmu_set_trip_hyst(data, i, temp, hyst);
-	}
 }
 
 static void exynos5433_tmu_set_trip_temp(struct exynos_tmu_data *data,
@@ -533,9 +538,8 @@ static void exynos5433_tmu_set_trip_hyst(struct exynos_tmu_data *data,
 static void exynos5433_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
-	int sensor_id, cal_type, i, temp, hyst;
+	int sensor_id, cal_type;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
@@ -562,19 +566,6 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "Calibration type is %d-point calibration\n",
 			cal_type ?  2 : 1);
-
-	/* Write temperature code for rising and falling threshold */
-	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		/* Write temperature code for rising threshold */
-		tz->ops->get_trip_temp(tz, i, &temp);
-		temp /= MCELSIUS;
-		exynos5433_tmu_set_trip_temp(data, i, temp);
-
-		/* Write temperature code for falling threshold */
-		tz->ops->get_trip_hyst(tz, i, &hyst);
-		hyst /= MCELSIUS;
-		exynos5433_tmu_set_trip_hyst(data, i, temp, hyst);
-	}
 }
 
 static void exynos7_tmu_set_trip_temp(struct exynos_tmu_data *data,
@@ -610,23 +601,10 @@ static void exynos7_tmu_set_trip_hyst(struct exynos_tmu_data *data,
 static void exynos7_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
-	struct thermal_zone_device *tz = data->tzd;
 	unsigned int trim_info;
-	int i, temp, hyst;
 
 	trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO);
 	sanitize_temp_error(data, trim_info);
-
-	/* Write temperature code for rising and falling threshold */
-	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		tz->ops->get_trip_temp(tz, i, &temp);
-		temp /= MCELSIUS;
-		exynos7_tmu_set_trip_temp(data, i, temp);
-
-		tz->ops->get_trip_hyst(tz, i, &hyst);
-		hyst /= MCELSIUS;
-		exynos7_tmu_set_trip_hyst(data, i, temp, hyst);
-	}
 }
 
 static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
@@ -989,6 +967,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 
 	switch (data->soc) {
 	case SOC_ARCH_EXYNOS4210:
+		data->tmu_set_trip_temp = exynos4210_tmu_set_trip_temp;
+		data->tmu_set_trip_hyst = exynos4210_tmu_set_trip_hyst;
 		data->tmu_initialize = exynos4210_tmu_initialize;
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4210_tmu_read;
@@ -1006,6 +986,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 	case SOC_ARCH_EXYNOS5260:
 	case SOC_ARCH_EXYNOS5420:
 	case SOC_ARCH_EXYNOS5420_TRIMINFO:
+		data->tmu_set_trip_temp = exynos4412_tmu_set_trip_temp;
+		data->tmu_set_trip_hyst = exynos4412_tmu_set_trip_hyst;
 		data->tmu_initialize = exynos4412_tmu_initialize;
 		data->tmu_control = exynos4210_tmu_control;
 		data->tmu_read = exynos4412_tmu_read;
@@ -1023,6 +1005,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->max_efuse_value = 100;
 		break;
 	case SOC_ARCH_EXYNOS5433:
+		data->tmu_set_trip_temp = exynos5433_tmu_set_trip_temp;
+		data->tmu_set_trip_hyst = exynos5433_tmu_set_trip_hyst;
 		data->tmu_initialize = exynos5433_tmu_initialize;
 		data->tmu_control = exynos5433_tmu_control;
 		data->tmu_read = exynos4412_tmu_read;
@@ -1039,6 +1023,8 @@ static int exynos_map_dt_data(struct platform_device *pdev)
 		data->max_efuse_value = 150;
 		break;
 	case SOC_ARCH_EXYNOS7:
+		data->tmu_set_trip_temp = exynos7_tmu_set_trip_temp;
+		data->tmu_set_trip_hyst = exynos7_tmu_set_trip_hyst;
 		data->tmu_initialize = exynos7_tmu_initialize;
 		data->tmu_control = exynos7_tmu_control;
 		data->tmu_read = exynos7_tmu_read;
-- 
1.9.1

Re: [PATCH 14/18] thermal: exynos: move trips setting to exynos_tmu_initialize()

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:29PM +0200, Bartlomiej Zolnierkiewicz wrote:
> * Add dummy exynos4210_tmu_set_trip_hyst() helper.
> 
> * Add ->tmu_set_trip_temp and ->tmu_set_trip_hyst methods to struct
>   exynos_tmu_data and set them in exynos_map_dt_data().
> 
> * Move trips setting to exynos_tmu_initialize().
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

[ ... ]

> +/* failing thresholds are not supported on Exynos4210 */
> +static void exynos4210_tmu_set_trip_hyst(struct exynos_tmu_data *data,
> +					 int trip, u8 temp, u8 hyst)
> +{
> +}
> +

May be you can get rid of this empty function and check against a NULL pointer
in exynos_tmu_initialize ?

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 14/18] thermal: exynos: move trips setting to exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 12:31:26 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:29PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > * Add dummy exynos4210_tmu_set_trip_hyst() helper.
> > 
> > * Add ->tmu_set_trip_temp and ->tmu_set_trip_hyst methods to struct
> >   exynos_tmu_data and set them in exynos_map_dt_data().
> > 
> > * Move trips setting to exynos_tmu_initialize().
> > 
> > There should be no functional changes caused by this patch.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> 
> [ ... ]
> 
> > +/* failing thresholds are not supported on Exynos4210 */
> > +static void exynos4210_tmu_set_trip_hyst(struct exynos_tmu_data *data,
> > +					 int trip, u8 temp, u8 hyst)
> > +{
> > +}
> > +
> 
> May be you can get rid of this empty function and check against a NULL pointer
> in exynos_tmu_initialize ?

This is more a question of taste and I prefer adding this one dummy function
instead of adding NULL pointer check for all SoCs.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 15/18] thermal: exynos: check return values of ->get_trip_[temp,hyst] methods

[Bartlomiej Zolnierkiewicz]

Check return values of ->get_trip_[temp,hyst] methods in
exynos_tmu_initialize().

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 244aaf6..abe0737 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -357,19 +357,23 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		/* Write temperature code for rising and falling threshold */
 		for (i = 0; i < ntrips; i++) {
 			/* Write temperature code for rising threshold */
-			tzd->ops->get_trip_temp(tzd, i, &temp);
+			ret = tzd->ops->get_trip_temp(tzd, i, &temp);
+			if (ret)
+				goto err;
 			temp /= MCELSIUS;
 			data->tmu_set_trip_temp(data, i, temp);
 
 			/* Write temperature code for falling threshold */
-			tzd->ops->get_trip_hyst(tzd, i, &hyst);
+			ret = tzd->ops->get_trip_hyst(tzd, i, &hyst);
+			if (ret)
+				goto err;
 			hyst /= MCELSIUS;
 			data->tmu_set_trip_hyst(data, i, temp, hyst);
 		}
 
 		data->tmu_clear_irqs(data);
 	}
-
+err:
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 	if (!IS_ERR(data->clk_sec))
-- 
1.9.1

Re: [PATCH 15/18] thermal: exynos: check return values of ->get_trip_[temp,hyst] methods

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:30PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Check return values of ->get_trip_[temp,hyst] methods in
> exynos_tmu_initialize().
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 244aaf6..abe0737 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -357,19 +357,23 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  		/* Write temperature code for rising and falling threshold */
>  		for (i = 0; i < ntrips; i++) {
>  			/* Write temperature code for rising threshold */
> -			tzd->ops->get_trip_temp(tzd, i, &temp);
> +			ret = tzd->ops->get_trip_temp(tzd, i, &temp);
> +			if (ret)
> +				goto err;
>  			temp /= MCELSIUS;
>  			data->tmu_set_trip_temp(data, i, temp);
>  
>  			/* Write temperature code for falling threshold */
> -			tzd->ops->get_trip_hyst(tzd, i, &hyst);
> +			ret = tzd->ops->get_trip_hyst(tzd, i, &hyst);
> +			if (ret)
> +				goto err;

Could this fail for 4210 ?

>  			hyst /= MCELSIUS;
>  			data->tmu_set_trip_hyst(data, i, temp, hyst);
>  		}
>  
>  		data->tmu_clear_irqs(data);
>  	}
> -
> +err:
>  	clk_disable(data->clk);
>  	mutex_unlock(&data->lock);
>  	if (!IS_ERR(data->clk_sec))
> -- 
> 1.9.1
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 15/18] thermal: exynos: check return values of ->get_trip_[temp,hyst] methods

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 12:43:04 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:30PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Check return values of ->get_trip_[temp,hyst] methods in
> > exynos_tmu_initialize().
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 244aaf6..abe0737 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -357,19 +357,23 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  		/* Write temperature code for rising and falling threshold */
> >  		for (i = 0; i < ntrips; i++) {
> >  			/* Write temperature code for rising threshold */
> > -			tzd->ops->get_trip_temp(tzd, i, &temp);
> > +			ret = tzd->ops->get_trip_temp(tzd, i, &temp);
> > +			if (ret)
> > +				goto err;
> >  			temp /= MCELSIUS;
> >  			data->tmu_set_trip_temp(data, i, temp);
> >  
> >  			/* Write temperature code for falling threshold */
> > -			tzd->ops->get_trip_hyst(tzd, i, &hyst);
> > +			ret = tzd->ops->get_trip_hyst(tzd, i, &hyst);
> > +			if (ret)
> > +				goto err;
> 
> Could this fail for 4210 ?

It can't, please see the method implementation in of-thermal.c:

static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz, int trip,
				    int *hyst)
{
	struct __thermal_zone *data = tz->devdata;

	if (trip >= data->ntrips || trip < 0)
		return -EDOM;

	*hyst = data->trips[trip].hysteresis;

	return 0;
}

> >  			hyst /= MCELSIUS;
> >  			data->tmu_set_trip_hyst(data, i, temp, hyst);
> >  		}
> >  
> >  		data->tmu_clear_irqs(data);
> >  	}
> > -
> > +err:
> >  	clk_disable(data->clk);
> >  	mutex_unlock(&data->lock);
> >  	if (!IS_ERR(data->clk_sec))

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 16/18] thermal: exynos: cleanup code for enabling threshold interrupts

[Bartlomiej Zolnierkiewicz]

Cleanup code for enabling threshold interrupts in ->tmu_control
method implementations.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 101 ++++++++++++-----------------------
 1 file changed, 34 insertions(+), 67 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index abe0737..9639acf 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -76,9 +76,6 @@
 #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
 
 #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
-#define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
-#define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
-#define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
 #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
 
 #define EXYNOS_EMUL_TIME	0x57F0
@@ -136,13 +133,6 @@
 #define EXYNOS7_TMU_TEMP_MASK			0x1ff
 #define EXYNOS7_PD_DET_EN_SHIFT			23
 #define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
-#define EXYNOS7_TMU_INTEN_RISE1_SHIFT		1
-#define EXYNOS7_TMU_INTEN_RISE2_SHIFT		2
-#define EXYNOS7_TMU_INTEN_RISE3_SHIFT		3
-#define EXYNOS7_TMU_INTEN_RISE4_SHIFT		4
-#define EXYNOS7_TMU_INTEN_RISE5_SHIFT		5
-#define EXYNOS7_TMU_INTEN_RISE6_SHIFT		6
-#define EXYNOS7_TMU_INTEN_RISE7_SHIFT		7
 #define EXYNOS7_EMUL_DATA_SHIFT			7
 #define EXYNOS7_EMUL_DATA_MASK			0x1ff
 
@@ -615,29 +605,27 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en;
+	unsigned int con, interrupt_en = 0, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 3)
-			 << EXYNOS_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			 << EXYNOS_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			 << EXYNOS_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			 << EXYNOS_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |=
+				(1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4));
+		}
 
 		if (data->soc != SOC_ARCH_EXYNOS4210)
 			interrupt_en |=
 				interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-	} else {
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+	} else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
-	}
+
 	writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
@@ -646,36 +634,25 @@ static void exynos5433_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en, pd_det_en;
+	unsigned int con, interrupt_en = 0, pd_det_en, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 7)
-			<< EXYNOS7_TMU_INTEN_RISE7_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 6)
-			<< EXYNOS7_TMU_INTEN_RISE6_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 5)
-			<< EXYNOS7_TMU_INTEN_RISE5_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 4)
-			<< EXYNOS7_TMU_INTEN_RISE4_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 3)
-			<< EXYNOS7_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			<< EXYNOS7_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			<< EXYNOS7_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			<< EXYNOS7_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |=
+				(1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i));
+		}
 
 		interrupt_en |=
 			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-	} else {
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+	} else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
-	}
 
 	pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0;
 
@@ -688,37 +665,27 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en;
+	unsigned int con, interrupt_en = 0, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		con |= (1 << EXYNOS7_PD_DET_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 7)
-			<< EXYNOS7_TMU_INTEN_RISE7_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 6)
-			<< EXYNOS7_TMU_INTEN_RISE6_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 5)
-			<< EXYNOS7_TMU_INTEN_RISE5_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 4)
-			<< EXYNOS7_TMU_INTEN_RISE4_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 3)
-			<< EXYNOS7_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			<< EXYNOS7_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			<< EXYNOS7_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			<< EXYNOS7_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |=
+				(1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i));
+		}
 
 		interrupt_en |=
 			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+		con |= (1 << EXYNOS7_PD_DET_EN_SHIFT);
 	} else {
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
 		con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
 	}
 
 	writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN);
-- 
1.9.1

Re: [PATCH 16/18] thermal: exynos: cleanup code for enabling threshold interrupts

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:31PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Cleanup code for enabling threshold interrupts in ->tmu_control
> method implementations.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 101 ++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index abe0737..9639acf 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -76,9 +76,6 @@
>  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
>  
>  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
> -#define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
> -#define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
> -#define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
>  #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
>  
>  #define EXYNOS_EMUL_TIME	0x57F0
> @@ -136,13 +133,6 @@
>  #define EXYNOS7_TMU_TEMP_MASK			0x1ff
>  #define EXYNOS7_PD_DET_EN_SHIFT			23
>  #define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
> -#define EXYNOS7_TMU_INTEN_RISE1_SHIFT		1
> -#define EXYNOS7_TMU_INTEN_RISE2_SHIFT		2
> -#define EXYNOS7_TMU_INTEN_RISE3_SHIFT		3
> -#define EXYNOS7_TMU_INTEN_RISE4_SHIFT		4
> -#define EXYNOS7_TMU_INTEN_RISE5_SHIFT		5
> -#define EXYNOS7_TMU_INTEN_RISE6_SHIFT		6
> -#define EXYNOS7_TMU_INTEN_RISE7_SHIFT		7
>  #define EXYNOS7_EMUL_DATA_SHIFT			7
>  #define EXYNOS7_EMUL_DATA_MASK			0x1ff
>  
> @@ -615,29 +605,27 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
>  {
>  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>  	struct thermal_zone_device *tz = data->tzd;
> -	unsigned int con, interrupt_en;
> +	unsigned int con, interrupt_en = 0, i;
>  
>  	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
>  
>  	if (on) {
> -		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
> -		interrupt_en =
> -			(of_thermal_is_trip_valid(tz, 3)
> -			 << EXYNOS_TMU_INTEN_RISE3_SHIFT) |
> -			(of_thermal_is_trip_valid(tz, 2)
> -			 << EXYNOS_TMU_INTEN_RISE2_SHIFT) |
> -			(of_thermal_is_trip_valid(tz, 1)
> -			 << EXYNOS_TMU_INTEN_RISE1_SHIFT) |
> -			(of_thermal_is_trip_valid(tz, 0)
> -			 << EXYNOS_TMU_INTEN_RISE0_SHIFT);
> +		for (i = 0; i < data->ntrip; i++) {
> +			if (!of_thermal_is_trip_valid(tz, i))
> +				continue;
> +
> +			interrupt_en |=
> +				(1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4));
> +		}

As EXYNOS_TMU_INTEN_RISE0_SHIFT is equal to zero, may be you can replace this
by BITS(i * 4) ?

Same comments for exynos5433 and exynos7 below.

I don't know which one was intended : 
 ((EXYNOS_TMU_INTEN_RISE0_SHIFT + i) * 4) or
  (EXYNOS_TMU_INTEN_RISE0_SHIFT + (i * 4))

but if it is the former it is lucky it works because the macro is zero.

Is it possible to have the registers layout, that would facilitate the review.


-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 16/18] thermal: exynos: cleanup code for enabling threshold interrupts

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 01:02:39 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:31PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Cleanup code for enabling threshold interrupts in ->tmu_control
> > method implementations.
> > 
> > There should be no functional changes caused by this patch.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 101 ++++++++++++-----------------------
> >  1 file changed, 34 insertions(+), 67 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index abe0737..9639acf 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -76,9 +76,6 @@
> >  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> >  
> >  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
> > -#define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
> > -#define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
> > -#define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
> >  #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
> >  
> >  #define EXYNOS_EMUL_TIME	0x57F0
> > @@ -136,13 +133,6 @@
> >  #define EXYNOS7_TMU_TEMP_MASK			0x1ff
> >  #define EXYNOS7_PD_DET_EN_SHIFT			23
> >  #define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
> > -#define EXYNOS7_TMU_INTEN_RISE1_SHIFT		1
> > -#define EXYNOS7_TMU_INTEN_RISE2_SHIFT		2
> > -#define EXYNOS7_TMU_INTEN_RISE3_SHIFT		3
> > -#define EXYNOS7_TMU_INTEN_RISE4_SHIFT		4
> > -#define EXYNOS7_TMU_INTEN_RISE5_SHIFT		5
> > -#define EXYNOS7_TMU_INTEN_RISE6_SHIFT		6
> > -#define EXYNOS7_TMU_INTEN_RISE7_SHIFT		7
> >  #define EXYNOS7_EMUL_DATA_SHIFT			7
> >  #define EXYNOS7_EMUL_DATA_MASK			0x1ff
> >  
> > @@ -615,29 +605,27 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
> >  {
> >  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> >  	struct thermal_zone_device *tz = data->tzd;
> > -	unsigned int con, interrupt_en;
> > +	unsigned int con, interrupt_en = 0, i;
> >  
> >  	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
> >  
> >  	if (on) {
> > -		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
> > -		interrupt_en =
> > -			(of_thermal_is_trip_valid(tz, 3)
> > -			 << EXYNOS_TMU_INTEN_RISE3_SHIFT) |
> > -			(of_thermal_is_trip_valid(tz, 2)
> > -			 << EXYNOS_TMU_INTEN_RISE2_SHIFT) |
> > -			(of_thermal_is_trip_valid(tz, 1)
> > -			 << EXYNOS_TMU_INTEN_RISE1_SHIFT) |
> > -			(of_thermal_is_trip_valid(tz, 0)
> > -			 << EXYNOS_TMU_INTEN_RISE0_SHIFT);
> > +		for (i = 0; i < data->ntrip; i++) {
> > +			if (!of_thermal_is_trip_valid(tz, i))
> > +				continue;
> > +
> > +			interrupt_en |=
> > +				(1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4));
> > +		}
> 
> As EXYNOS_TMU_INTEN_RISE0_SHIFT is equal to zero, may be you can replace this
> by BITS(i * 4) ?
> 
> Same comments for exynos5433 and exynos7 below.

Good point, I will replace it by using BIT() macro.

> I don't know which one was intended : 

The one that doesn't change the driver behavior as stated in the patch
description.

>  ((EXYNOS_TMU_INTEN_RISE0_SHIFT + i) * 4) or
>   (EXYNOS_TMU_INTEN_RISE0_SHIFT + (i * 4))
> 
> but if it is the former it is lucky it works because the macro is zero.
> 
> Is it possible to have the registers layout, that would facilitate the review.

I'm sorry but Exynos TMU documentation is not publicly available.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 16/18] thermal: exynos: cleanup code for enabling threshold interrupts

[Bartlomiej Zolnierkiewicz]

On Wednesday, May 02, 2018 12:03:44 PM Bartlomiej Zolnierkiewicz wrote:
> On Tuesday, May 01, 2018 01:02:39 PM Daniel Lezcano wrote:
> > On Thu, Apr 26, 2018 at 01:51:31PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > Cleanup code for enabling threshold interrupts in ->tmu_control
> > > method implementations.
> > > 
> > > There should be no functional changes caused by this patch.
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > ---
> > >  drivers/thermal/samsung/exynos_tmu.c | 101 ++++++++++++-----------------------
> > >  1 file changed, 34 insertions(+), 67 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > > index abe0737..9639acf 100644
> > > --- a/drivers/thermal/samsung/exynos_tmu.c
> > > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > > @@ -76,9 +76,6 @@
> > >  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> > >  
> > >  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
> > > -#define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
> > > -#define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
> > > -#define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
> > >  #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
> > >  
> > >  #define EXYNOS_EMUL_TIME	0x57F0
> > > @@ -136,13 +133,6 @@
> > >  #define EXYNOS7_TMU_TEMP_MASK			0x1ff
> > >  #define EXYNOS7_PD_DET_EN_SHIFT			23
> > >  #define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
> > > -#define EXYNOS7_TMU_INTEN_RISE1_SHIFT		1
> > > -#define EXYNOS7_TMU_INTEN_RISE2_SHIFT		2
> > > -#define EXYNOS7_TMU_INTEN_RISE3_SHIFT		3
> > > -#define EXYNOS7_TMU_INTEN_RISE4_SHIFT		4
> > > -#define EXYNOS7_TMU_INTEN_RISE5_SHIFT		5
> > > -#define EXYNOS7_TMU_INTEN_RISE6_SHIFT		6
> > > -#define EXYNOS7_TMU_INTEN_RISE7_SHIFT		7
> > >  #define EXYNOS7_EMUL_DATA_SHIFT			7
> > >  #define EXYNOS7_EMUL_DATA_MASK			0x1ff
> > >  
> > > @@ -615,29 +605,27 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on)
> > >  {
> > >  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> > >  	struct thermal_zone_device *tz = data->tzd;
> > > -	unsigned int con, interrupt_en;
> > > +	unsigned int con, interrupt_en = 0, i;
> > >  
> > >  	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
> > >  
> > >  	if (on) {
> > > -		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
> > > -		interrupt_en =
> > > -			(of_thermal_is_trip_valid(tz, 3)
> > > -			 << EXYNOS_TMU_INTEN_RISE3_SHIFT) |
> > > -			(of_thermal_is_trip_valid(tz, 2)
> > > -			 << EXYNOS_TMU_INTEN_RISE2_SHIFT) |
> > > -			(of_thermal_is_trip_valid(tz, 1)
> > > -			 << EXYNOS_TMU_INTEN_RISE1_SHIFT) |
> > > -			(of_thermal_is_trip_valid(tz, 0)
> > > -			 << EXYNOS_TMU_INTEN_RISE0_SHIFT);
> > > +		for (i = 0; i < data->ntrip; i++) {
> > > +			if (!of_thermal_is_trip_valid(tz, i))
> > > +				continue;
> > > +
> > > +			interrupt_en |=
> > > +				(1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4));
> > > +		}
> > 
> > As EXYNOS_TMU_INTEN_RISE0_SHIFT is equal to zero, may be you can replace this
> > by BITS(i * 4) ?
> > 
> > Same comments for exynos5433 and exynos7 below.
> 
> Good point, I will replace it by using BIT() macro.

From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: [PATCH] thermal: exynos: cleanup code for enabling threshold interrupts

Cleanup code for enabling threshold interrupts in ->tmu_control
method implementations.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c |  100 ++++++++++-------------------------
 1 file changed, 31 insertions(+), 69 deletions(-)

Index: b/drivers/thermal/samsung/exynos_tmu.c
===================================================================
--- a/drivers/thermal/samsung/exynos_tmu.c	2018-05-02 12:25:14.393266604 +0200
+++ b/drivers/thermal/samsung/exynos_tmu.c	2018-05-02 12:28:23.545271367 +0200
@@ -75,10 +75,6 @@
 #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
 #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
 
-#define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
-#define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
-#define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
-#define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
 #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
 
 #define EXYNOS_EMUL_TIME	0x57F0
@@ -135,14 +131,6 @@
 
 #define EXYNOS7_TMU_TEMP_MASK			0x1ff
 #define EXYNOS7_PD_DET_EN_SHIFT			23
-#define EXYNOS7_TMU_INTEN_RISE0_SHIFT		0
-#define EXYNOS7_TMU_INTEN_RISE1_SHIFT		1
-#define EXYNOS7_TMU_INTEN_RISE2_SHIFT		2
-#define EXYNOS7_TMU_INTEN_RISE3_SHIFT		3
-#define EXYNOS7_TMU_INTEN_RISE4_SHIFT		4
-#define EXYNOS7_TMU_INTEN_RISE5_SHIFT		5
-#define EXYNOS7_TMU_INTEN_RISE6_SHIFT		6
-#define EXYNOS7_TMU_INTEN_RISE7_SHIFT		7
 #define EXYNOS7_EMUL_DATA_SHIFT			7
 #define EXYNOS7_EMUL_DATA_MASK			0x1ff
 
@@ -615,29 +603,26 @@ static void exynos4210_tmu_control(struc
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en;
+	unsigned int con, interrupt_en = 0, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 3)
-			 << EXYNOS_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			 << EXYNOS_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			 << EXYNOS_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			 << EXYNOS_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |= BIT(i * 4);
+		}
 
 		if (data->soc != SOC_ARCH_EXYNOS4210)
 			interrupt_en |=
 				interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-	} else {
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+	} else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
-	}
+
 	writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
 	writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
 }
@@ -646,36 +631,24 @@ static void exynos5433_tmu_control(struc
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en, pd_det_en;
+	unsigned int con, interrupt_en = 0, pd_det_en, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 7)
-			<< EXYNOS7_TMU_INTEN_RISE7_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 6)
-			<< EXYNOS7_TMU_INTEN_RISE6_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 5)
-			<< EXYNOS7_TMU_INTEN_RISE5_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 4)
-			<< EXYNOS7_TMU_INTEN_RISE4_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 3)
-			<< EXYNOS7_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			<< EXYNOS7_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			<< EXYNOS7_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			<< EXYNOS7_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |= BIT(i);
+		}
 
 		interrupt_en |=
 			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
-	} else {
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+	} else
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
-	}
 
 	pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0;
 
@@ -688,37 +661,26 @@ static void exynos7_tmu_control(struct p
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct thermal_zone_device *tz = data->tzd;
-	unsigned int con, interrupt_en;
+	unsigned int con, interrupt_en = 0, i;
 
 	con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL));
 
 	if (on) {
-		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
-		con |= (1 << EXYNOS7_PD_DET_EN_SHIFT);
-		interrupt_en =
-			(of_thermal_is_trip_valid(tz, 7)
-			<< EXYNOS7_TMU_INTEN_RISE7_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 6)
-			<< EXYNOS7_TMU_INTEN_RISE6_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 5)
-			<< EXYNOS7_TMU_INTEN_RISE5_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 4)
-			<< EXYNOS7_TMU_INTEN_RISE4_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 3)
-			<< EXYNOS7_TMU_INTEN_RISE3_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 2)
-			<< EXYNOS7_TMU_INTEN_RISE2_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 1)
-			<< EXYNOS7_TMU_INTEN_RISE1_SHIFT) |
-			(of_thermal_is_trip_valid(tz, 0)
-			<< EXYNOS7_TMU_INTEN_RISE0_SHIFT);
+		for (i = 0; i < data->ntrip; i++) {
+			if (!of_thermal_is_trip_valid(tz, i))
+				continue;
+
+			interrupt_en |= BIT(i);
+		}
 
 		interrupt_en |=
 			interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
+
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
+		con |= (1 << EXYNOS7_PD_DET_EN_SHIFT);
 	} else {
 		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
 		con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT);
-		interrupt_en = 0; /* Disable all interrupts */
 	}
 
 	writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN);

[PATCH 17/18] thermal: exynos: remove unused defines for Exynos5433

[Bartlomiej Zolnierkiewicz]

Remove unused defines for Exynos5433.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 9639acf..ed02b40 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -96,11 +96,6 @@
 #define EXYNOS4412_MUX_ADDR_SHIFT          20
 
 /* Exynos5433 specific registers */
-#define EXYNOS5433_TMU_REG_CONTROL1		0x024
-#define EXYNOS5433_TMU_SAMPLING_INTERVAL	0x02c
-#define EXYNOS5433_TMU_COUNTER_VALUE0		0x030
-#define EXYNOS5433_TMU_COUNTER_VALUE1		0x034
-#define EXYNOS5433_TMU_REG_CURRENT_TEMP1	0x044
 #define EXYNOS5433_THD_TEMP_RISE3_0		0x050
 #define EXYNOS5433_THD_TEMP_RISE7_4		0x054
 #define EXYNOS5433_THD_TEMP_FALL3_0		0x060
-- 
1.9.1

Re: [PATCH 17/18] thermal: exynos: remove unused defines for Exynos5433

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:32PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Remove unused defines for Exynos5433.

I agree to remove these macros but is there a place with the documentation for
those values if we need to put them back ?
 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 9639acf..ed02b40 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -96,11 +96,6 @@
>  #define EXYNOS4412_MUX_ADDR_SHIFT          20
>  
>  /* Exynos5433 specific registers */
> -#define EXYNOS5433_TMU_REG_CONTROL1		0x024
> -#define EXYNOS5433_TMU_SAMPLING_INTERVAL	0x02c
> -#define EXYNOS5433_TMU_COUNTER_VALUE0		0x030
> -#define EXYNOS5433_TMU_COUNTER_VALUE1		0x034
> -#define EXYNOS5433_TMU_REG_CURRENT_TEMP1	0x044
>  #define EXYNOS5433_THD_TEMP_RISE3_0		0x050
>  #define EXYNOS5433_THD_TEMP_RISE7_4		0x054
>  #define EXYNOS5433_THD_TEMP_FALL3_0		0x060
> -- 
> 1.9.1
> 

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 17/18] thermal: exynos: remove unused defines for Exynos5433

[Bartlomiej Zolnierkiewicz]

On Tuesday, May 01, 2018 01:11:20 PM Daniel Lezcano wrote:
> On Thu, Apr 26, 2018 at 01:51:32PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Remove unused defines for Exynos5433.
> 
> I agree to remove these macros but is there a place with the documentation for
> those values if we need to put them back ?

They can be always resurrected from git repo if needed.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 18/18] thermal: exynos: remove trip reporting to user-space

[Bartlomiej Zolnierkiewicz]

Remove trip reporting to user-space - I'm not aware of any user-space
program which relies on it and there is a thermal user-space governor
which does it in proper way nowadays.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index ed02b40..f785e6a 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -216,33 +216,6 @@ struct exynos_tmu_data {
 	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
 };
 
-static void exynos_report_trigger(struct exynos_tmu_data *p)
-{
-	char data[10], *envp[] = { data, NULL };
-	struct thermal_zone_device *tz = p->tzd;
-	int temp;
-	unsigned int i;
-
-	if (!tz) {
-		pr_err("No thermal zone device defined\n");
-		return;
-	}
-
-	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
-	mutex_lock(&tz->lock);
-	/* Find the level for which trip happened */
-	for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
-		tz->ops->get_trip_temp(tz, i, &temp);
-		if (tz->last_temperature < temp)
-			break;
-	}
-
-	snprintf(data, sizeof(data), "%u", i);
-	kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, envp);
-	mutex_unlock(&tz->lock);
-}
-
 /*
  * TMU treats temperature as a mapped temperature code.
  * The temperature is converted differently depending on the calibration type.
@@ -814,7 +787,8 @@ static void exynos_tmu_work(struct work_struct *work)
 	if (!IS_ERR(data->clk_sec))
 		clk_disable(data->clk_sec);
 
-	exynos_report_trigger(data);
+	thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
+
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
 
-- 
1.9.1

Re: [PATCH 18/18] thermal: exynos: remove trip reporting to user-space

[Daniel Lezcano]

On Thu, Apr 26, 2018 at 01:51:33PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Remove trip reporting to user-space - I'm not aware of any user-space
> program which relies on it and there is a thermal user-space governor
> which does it in proper way nowadays.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog