All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] thermal: exynos: various cleanups
@ 2014-05-05 11:15 Bartlomiej Zolnierkiewicz
  2014-05-05 11:15 ` [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries Bartlomiej Zolnierkiewicz
                   ` (10 more replies)
  0 siblings, 11 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

Hi,

This patch series contains various cleanups for EXYNOS thermal
driver.  Overall it decreases driver's LOC by 13%.  It is based
on next-20140428 kernel.  It should not cause any functionality
changes.

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


Bartlomiej Zolnierkiewicz (10):
  thermal: exynos: remove unused struct exynos_tmu_registers entries
  thermal: exynos: remove unused defines
  thermal: exynos: remove dead code for HW_MODE calibration
  thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING
    calibration
  thermal: exynos: remove redundant pdata checks from
    exynos_tmu_initialize()
  thermal: exynos: remove redundant threshold_code checks from
    exynos_tmu_initialize()
  thermal: exynos: simplify temp_to_code() and code_to_temp()
  thermal: exynos: cache non_hw_trigger_levels in pdata
  thermal: exynos: remove redundant pdata checks from
    exynos_tmu_control()
  thermal: exynos: remove identical values from exynos*_tmu_registers
    structures

 drivers/thermal/samsung/exynos_thermal_common.h |   1 -
 drivers/thermal/samsung/exynos_tmu.c            | 181 ++++--------------------
 drivers/thermal/samsung/exynos_tmu.h            |  86 +----------
 drivers/thermal/samsung/exynos_tmu_data.c       |  40 +-----
 drivers/thermal/samsung/exynos_tmu_data.h       |  32 +----
 5 files changed, 37 insertions(+), 303 deletions(-)

-- 
1.8.2.3


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:12   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 02/10] thermal: exynos: remove unused defines Bartlomiej Zolnierkiewicz
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.h      | 40 -------------------------------
 drivers/thermal/samsung/exynos_tmu_data.c |  2 --
 drivers/thermal/samsung/exynos_tmu_data.h |  1 -
 3 files changed, 43 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
index 3fb6554..80dc899 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -82,8 +82,6 @@ enum soc_type {
  * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data reg.
  * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data reg.
  * @triminfo_ctrl: trim info controller register.
- * @triminfo_reload_shift: shift of triminfo reload enable bit in triminfo_ctrl
-	reg.
  * @tmu_ctrl: TMU main controller register.
  * @test_mux_addr_shift: shift bits of test mux address.
  * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl register.
@@ -98,27 +96,13 @@ enum soc_type {
 	register.
  * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
 	register.
- * @therm_trip_tq_en_shift: shift bits of thermal trip enable by TQ pin in
-	tmu_ctrl register.
  * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
  * @tmu_status: register drescribing the TMU status.
  * @tmu_cur_temp: register containing the current temperature of the TMU.
- * @tmu_cur_temp_shift: shift bits of current temp value in tmu_cur_temp
-	register.
  * @threshold_temp: register containing the base threshold level.
  * @threshold_th0: Register containing first set of rising levels.
- * @threshold_th0_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th0_l1_shift: shift bits of level1 threshold temperature.
- * @threshold_th0_l2_shift: shift bits of level2 threshold temperature.
- * @threshold_th0_l3_shift: shift bits of level3 threshold temperature.
  * @threshold_th1: Register containing second set of rising levels.
- * @threshold_th1_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th1_l1_shift: shift bits of level1 threshold temperature.
- * @threshold_th1_l2_shift: shift bits of level2 threshold temperature.
- * @threshold_th1_l3_shift: shift bits of level3 threshold temperature.
  * @threshold_th2: Register containing third set of rising levels.
- * @threshold_th2_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th3: Register containing fourth set of rising levels.
  * @threshold_th3_l0_shift: shift bits of level0 threshold temperature.
  * @tmu_inten: register containing the different threshold interrupt
 	enable bits.
@@ -131,15 +115,11 @@ enum soc_type {
  * @inten_rise2_shift: shift bits of rising 2 interrupt bits.
  * @inten_rise3_shift: shift bits of rising 3 interrupt bits.
  * @inten_fall0_shift: shift bits of falling 0 interrupt bits.
- * @inten_fall1_shift: shift bits of falling 1 interrupt bits.
- * @inten_fall2_shift: shift bits of falling 2 interrupt bits.
- * @inten_fall3_shift: shift bits of falling 3 interrupt bits.
  * @tmu_intstat: Register containing the interrupt status values.
  * @tmu_intclear: Register for clearing the raised interrupt status.
  * @emul_con: TMU emulation controller register.
  * @emul_temp_shift: shift bits of emulation temperature.
  * @emul_time_shift: shift bits of emulation time.
- * @emul_time_mask: mask bits of emulation time.
  * @tmu_irqstatus: register to find which TMU generated interrupts.
  * @tmu_pmin: register to get/set the Pmin value.
  */
@@ -149,7 +129,6 @@ struct exynos_tmu_registers {
 	u32	triminfo_85_shift;
 
 	u32	triminfo_ctrl;
-	u32	triminfo_reload_shift;
 
 	u32	tmu_ctrl;
 	u32     test_mux_addr_shift;
@@ -162,32 +141,17 @@ struct exynos_tmu_registers {
 	u32	buf_slope_sel_mask;
 	u32	calib_mode_shift;
 	u32	calib_mode_mask;
-	u32	therm_trip_tq_en_shift;
 	u32	core_en_shift;
 
 	u32	tmu_status;
 
 	u32	tmu_cur_temp;
-	u32	tmu_cur_temp_shift;
 
 	u32	threshold_temp;
 
 	u32	threshold_th0;
-	u32	threshold_th0_l0_shift;
-	u32	threshold_th0_l1_shift;
-	u32	threshold_th0_l2_shift;
-	u32	threshold_th0_l3_shift;
-
 	u32	threshold_th1;
-	u32	threshold_th1_l0_shift;
-	u32	threshold_th1_l1_shift;
-	u32	threshold_th1_l2_shift;
-	u32	threshold_th1_l3_shift;
-
 	u32	threshold_th2;
-	u32	threshold_th2_l0_shift;
-
-	u32	threshold_th3;
 	u32	threshold_th3_l0_shift;
 
 	u32	tmu_inten;
@@ -200,9 +164,6 @@ struct exynos_tmu_registers {
 	u32	inten_rise2_shift;
 	u32	inten_rise3_shift;
 	u32	inten_fall0_shift;
-	u32	inten_fall1_shift;
-	u32	inten_fall2_shift;
-	u32	inten_fall3_shift;
 
 	u32	tmu_intstat;
 
@@ -211,7 +172,6 @@ struct exynos_tmu_registers {
 	u32	emul_con;
 	u32	emul_temp_shift;
 	u32	emul_time_shift;
-	u32	emul_time_mask;
 
 	u32	tmu_irqstatus;
 	u32	tmu_pmin;
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index 476b768..36d64d6 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -96,7 +96,6 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
 	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
 	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
-	.triminfo_reload_shift = EXYNOS_TRIMINFO_RELOAD_SHIFT,
 	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
 	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
 	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
@@ -126,7 +125,6 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.emul_con = EXYNOS_EMUL_CON,
 	.emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
 	.emul_time_shift = EXYNOS_EMUL_TIME_SHIFT,
-	.emul_time_mask = EXYNOS_EMUL_TIME_MASK,
 };
 
 #define EXYNOS4412_TMU_DATA \
diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
index a1ea19d..06c4345 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.h
+++ b/drivers/thermal/samsung/exynos_tmu_data.h
@@ -63,7 +63,6 @@
 #define EXYNOS_THD_TEMP_FALL		0x54
 #define EXYNOS_EMUL_CON		0x80
 
-#define EXYNOS_TRIMINFO_RELOAD_SHIFT	1
 #define EXYNOS_TRIMINFO_25_SHIFT	0
 #define EXYNOS_TRIMINFO_85_SHIFT	8
 #define EXYNOS_TMU_RISE_INT_MASK	0x111
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 02/10] thermal: exynos: remove unused defines
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
  2014-05-05 11:15 ` [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 14:07   ` Eduardo Valentin
  2014-05-19  5:17   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration Bartlomiej Zolnierkiewicz
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu_data.h | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
index 06c4345..d4eeddb 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.h
+++ b/drivers/thermal/samsung/exynos_tmu_data.h
@@ -42,20 +42,8 @@
 /* Exynos4210 specific registers */
 #define EXYNOS4210_TMU_REG_THRESHOLD_TEMP	0x44
 #define EXYNOS4210_TMU_REG_TRIG_LEVEL0	0x50
-#define EXYNOS4210_TMU_REG_TRIG_LEVEL1	0x54
-#define EXYNOS4210_TMU_REG_TRIG_LEVEL2	0x58
-#define EXYNOS4210_TMU_REG_TRIG_LEVEL3	0x5C
-#define EXYNOS4210_TMU_REG_PAST_TEMP0	0x60
-#define EXYNOS4210_TMU_REG_PAST_TEMP1	0x64
-#define EXYNOS4210_TMU_REG_PAST_TEMP2	0x68
-#define EXYNOS4210_TMU_REG_PAST_TEMP3	0x6C
-
-#define EXYNOS4210_TMU_TRIG_LEVEL0_MASK	0x1
-#define EXYNOS4210_TMU_TRIG_LEVEL1_MASK	0x10
-#define EXYNOS4210_TMU_TRIG_LEVEL2_MASK	0x100
-#define EXYNOS4210_TMU_TRIG_LEVEL3_MASK	0x1000
+
 #define EXYNOS4210_TMU_TRIG_LEVEL_MASK	0x1111
-#define EXYNOS4210_TMU_INTCLEAR_VAL	0x1111
 
 /* Exynos5250 and Exynos4412 specific registers */
 #define EXYNOS_TMU_TRIMINFO_CON	0x14
@@ -69,8 +57,6 @@
 #define EXYNOS_TMU_RISE_INT_SHIFT	0
 #define EXYNOS_TMU_FALL_INT_MASK	0x111
 #define EXYNOS_TMU_FALL_INT_SHIFT	12
-#define EXYNOS_TMU_CLEAR_RISE_INT	0x111
-#define EXYNOS_TMU_CLEAR_FALL_INT	(0x111 << 12)
 #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
 #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
 #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
@@ -82,8 +68,6 @@
 #define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
 #define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
 #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
-#define EXYNOS_TMU_INTEN_FALL1_SHIFT	20
-#define EXYNOS_TMU_INTEN_FALL2_SHIFT	24
 
 #define EXYNOS_EMUL_TIME	0x57F0
 #define EXYNOS_EMUL_TIME_MASK	0xffff
@@ -107,13 +91,11 @@
 #define EXYNOS5440_TMU_S0_7_TH0			0x110
 #define EXYNOS5440_TMU_S0_7_TH1			0x130
 #define EXYNOS5440_TMU_S0_7_TH2			0x150
-#define EXYNOS5440_TMU_S0_7_EVTEN		0x1F0
 #define EXYNOS5440_TMU_S0_7_IRQEN		0x210
 #define EXYNOS5440_TMU_S0_7_IRQ			0x230
 /* exynos5440 common registers */
 #define EXYNOS5440_TMU_IRQ_STATUS		0x000
 #define EXYNOS5440_TMU_PMIN			0x004
-#define EXYNOS5440_TMU_TEMP			0x008
 
 #define EXYNOS5440_TMU_RISE_INT_MASK		0xf
 #define EXYNOS5440_TMU_RISE_INT_SHIFT		0
@@ -124,13 +106,6 @@
 #define EXYNOS5440_TMU_INTEN_RISE2_SHIFT	2
 #define EXYNOS5440_TMU_INTEN_RISE3_SHIFT	3
 #define EXYNOS5440_TMU_INTEN_FALL0_SHIFT	4
-#define EXYNOS5440_TMU_INTEN_FALL1_SHIFT	5
-#define EXYNOS5440_TMU_INTEN_FALL2_SHIFT	6
-#define EXYNOS5440_TMU_INTEN_FALL3_SHIFT	7
-#define EXYNOS5440_TMU_TH_RISE0_SHIFT		0
-#define EXYNOS5440_TMU_TH_RISE1_SHIFT		8
-#define EXYNOS5440_TMU_TH_RISE2_SHIFT		16
-#define EXYNOS5440_TMU_TH_RISE3_SHIFT		24
 #define EXYNOS5440_TMU_TH_RISE4_SHIFT		24
 #define EXYNOS5440_EFUSE_SWAP_OFFSET		8
 
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
  2014-05-05 11:15 ` [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries Bartlomiej Zolnierkiewicz
  2014-05-05 11:15 ` [PATCH 02/10] thermal: exynos: remove unused defines Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 14:14   ` Eduardo Valentin
  2014-05-19  5:27   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration Bartlomiej Zolnierkiewicz
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

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      | 33 +------------------------------
 drivers/thermal/samsung/exynos_tmu.h      | 13 ------------
 drivers/thermal/samsung/exynos_tmu_data.c |  3 ---
 drivers/thermal/samsung/exynos_tmu_data.h |  2 --
 4 files changed, 1 insertion(+), 50 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 0d96a51..9f2a5ee 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -76,9 +76,6 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 	struct exynos_tmu_platform_data *pdata = data->pdata;
 	int temp_code;
 
-	if (pdata->cal_mode == HW_MODE)
-		return temp;
-
 	if (data->soc == SOC_ARCH_EXYNOS4210)
 		/* temp should range between 25 and 125 */
 		if (temp < 25 || temp > 125) {
@@ -113,9 +110,6 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
 	struct exynos_tmu_platform_data *pdata = data->pdata;
 	int temp;
 
-	if (pdata->cal_mode == HW_MODE)
-		return temp_code;
-
 	if (data->soc == SOC_ARCH_EXYNOS4210)
 		/* temp_code should range between 75 and 175 */
 		if (temp_code < 75 || temp_code > 175) {
@@ -164,9 +158,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	if (TMU_SUPPORTS(pdata, TRIM_RELOAD))
 		__raw_writel(1, data->base + reg->triminfo_ctrl);
 
-	if (pdata->cal_mode == HW_MODE)
-		goto skip_calib_data;
-
 	/* Save trimming info in order to perform calibration */
 	if (data->soc == SOC_ARCH_EXYNOS5440) {
 		/*
@@ -202,7 +193,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 			(pdata->efuse_value >> reg->triminfo_85_shift) &
 			EXYNOS_TMU_TEMP_MASK;
 
-skip_calib_data:
 	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
 		dev_err(&pdev->dev, "Invalid max trigger level\n");
 		ret = -EINVAL;
@@ -311,7 +301,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct exynos_tmu_platform_data *pdata = data->pdata;
 	const struct exynos_tmu_registers *reg = pdata->registers;
-	unsigned int con, interrupt_en, cal_val;
+	unsigned int con, interrupt_en;
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
@@ -337,27 +327,6 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 		con |= (pdata->noise_cancel_mode << reg->therm_trip_mode_shift);
 	}
 
-	if (pdata->cal_mode == HW_MODE) {
-		con &= ~(reg->calib_mode_mask << reg->calib_mode_shift);
-		cal_val = 0;
-		switch (pdata->cal_type) {
-		case TYPE_TWO_POINT_TRIMMING:
-			cal_val = 3;
-			break;
-		case TYPE_ONE_POINT_TRIMMING_85:
-			cal_val = 2;
-			break;
-		case TYPE_ONE_POINT_TRIMMING_25:
-			cal_val = 1;
-			break;
-		case TYPE_NONE:
-			break;
-		default:
-			dev_err(&pdev->dev, "Invalid calibration type, using none\n");
-		}
-		con |= cal_val << reg->calib_mode_shift;
-	}
-
 	if (on) {
 		con |= (1 << reg->core_en_shift);
 		interrupt_en =
diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
index 80dc899..e417af8 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -34,11 +34,6 @@ enum calibration_type {
 	TYPE_NONE,
 };
 
-enum calibration_mode {
-	SW_MODE,
-	HW_MODE,
-};
-
 enum soc_type {
 	SOC_ARCH_EXYNOS4210 = 1,
 	SOC_ARCH_EXYNOS4412,
@@ -92,10 +87,6 @@ enum soc_type {
  * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
 	register.
  * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl register.
- * @calib_mode_shift: shift bits of calibration mode value in tmu_ctrl
-	register.
- * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
-	register.
  * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
  * @tmu_status: register drescribing the TMU status.
  * @tmu_cur_temp: register containing the current temperature of the TMU.
@@ -139,8 +130,6 @@ struct exynos_tmu_registers {
 	u32	therm_trip_en_shift;
 	u32	buf_slope_sel_shift;
 	u32	buf_slope_sel_mask;
-	u32	calib_mode_shift;
-	u32	calib_mode_mask;
 	u32	core_en_shift;
 
 	u32	tmu_status;
@@ -222,7 +211,6 @@ struct exynos_tmu_registers {
  * @default_temp_offset: default temperature offset in case of no trimming
  * @test_mux; information if SoC supports test MUX
  * @cal_type: calibration type for temperature
- * @cal_mode: calibration mode for temperature
  * @freq_clip_table: Table representing frequency reduction percentage.
  * @freq_tab_count: Count of the above table as frequency reduction may
  *	applicable to only some of the trigger levels.
@@ -253,7 +241,6 @@ struct exynos_tmu_platform_data {
 	u8 test_mux;
 
 	enum calibration_type cal_type;
-	enum calibration_mode cal_mode;
 	enum soc_type type;
 	struct freq_clip_table freq_tab[4];
 	unsigned int freq_tab_count;
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index 36d64d6..4b992d9 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -205,8 +205,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
 	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
 	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
-	.calib_mode_shift = EXYNOS_TMU_CALIB_MODE_SHIFT,
-	.calib_mode_mask = EXYNOS_TMU_CALIB_MODE_MASK,
 	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
 	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
 	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
@@ -243,7 +241,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.reference_voltage = 16, \
 	.noise_cancel_mode = 4, \
 	.cal_type = TYPE_ONE_POINT_TRIMMING, \
-	.cal_mode = 0, \
 	.efuse_value = 0x5b2d, \
 	.min_efuse_value = 16, \
 	.max_efuse_value = 76, \
diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
index d4eeddb..1fed00d 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.h
+++ b/drivers/thermal/samsung/exynos_tmu_data.h
@@ -60,8 +60,6 @@
 #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
 #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
 #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
-#define EXYNOS_TMU_CALIB_MODE_SHIFT	4
-#define EXYNOS_TMU_CALIB_MODE_MASK	0x3
 
 #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
 #define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (2 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 14:31   ` Eduardo Valentin
  2014-05-19  5:40   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize() Bartlomiej Zolnierkiewicz
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

Only TYPE_ONE_POINT_TRIMMING calibration is used so remove
the dead code for TYPE_TWO_POINT_TRIMMING calibration.

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      | 55 ++++++-------------------------
 drivers/thermal/samsung/exynos_tmu.h      | 20 +----------
 drivers/thermal/samsung/exynos_tmu_data.c | 17 +---------
 drivers/thermal/samsung/exynos_tmu_data.h |  2 --
 4 files changed, 12 insertions(+), 82 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 9f2a5ee..903566f 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -47,8 +47,7 @@
  * @irq_work: pointer to the irq work structure.
  * @lock: lock to implement synchronization.
  * @clk: pointer to the clock structure.
- * @temp_error1: fused value of the first point trim.
- * @temp_error2: fused value of the second point trim.
+ * @temp_error: fused value of the first point trim.
  * @regulator: pointer to the TMU regulator structure.
  * @reg_conf: pointer to structure to register with core thermal.
  */
@@ -62,14 +61,13 @@ struct exynos_tmu_data {
 	struct work_struct irq_work;
 	struct mutex lock;
 	struct clk *clk;
-	u8 temp_error1, temp_error2;
+	u8 temp_error;
 	struct regulator *regulator;
 	struct thermal_sensor_conf *reg_conf;
 };
 
 /*
  * TMU treats temperature as a mapped temperature code.
- * The temperature is converted differently depending on the calibration type.
  */
 static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 {
@@ -83,20 +81,7 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 			goto out;
 		}
 
-	switch (pdata->cal_type) {
-	case TYPE_TWO_POINT_TRIMMING:
-		temp_code = (temp - pdata->first_point_trim) *
-			(data->temp_error2 - data->temp_error1) /
-			(pdata->second_point_trim - pdata->first_point_trim) +
-			data->temp_error1;
-		break;
-	case TYPE_ONE_POINT_TRIMMING:
-		temp_code = temp + data->temp_error1 - pdata->first_point_trim;
-		break;
-	default:
-		temp_code = temp + pdata->default_temp_offset;
-		break;
-	}
+	temp_code = temp + data->temp_error - pdata->first_point_trim;
 out:
 	return temp_code;
 }
@@ -117,20 +102,7 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
 			goto out;
 		}
 
-	switch (pdata->cal_type) {
-	case TYPE_TWO_POINT_TRIMMING:
-		temp = (temp_code - data->temp_error1) *
-			(pdata->second_point_trim - pdata->first_point_trim) /
-			(data->temp_error2 - data->temp_error1) +
-			pdata->first_point_trim;
-		break;
-	case TYPE_ONE_POINT_TRIMMING:
-		temp = temp_code - data->temp_error1 + pdata->first_point_trim;
-		break;
-	default:
-		temp = temp_code - pdata->default_temp_offset;
-		break;
-	}
+	temp = temp_code - data->temp_error + pdata->first_point_trim;
 out:
 	return temp;
 }
@@ -179,19 +151,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	} else {
 		trim_info = readl(data->base + reg->triminfo_data);
 	}
-	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
-	data->temp_error2 = ((trim_info >> reg->triminfo_85_shift) &
-				EXYNOS_TMU_TEMP_MASK);
-
-	if (!data->temp_error1 ||
-		(pdata->min_efuse_value > data->temp_error1) ||
-		(data->temp_error1 > pdata->max_efuse_value))
-		data->temp_error1 = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
-
-	if (!data->temp_error2)
-		data->temp_error2 =
-			(pdata->efuse_value >> reg->triminfo_85_shift) &
-			EXYNOS_TMU_TEMP_MASK;
+	data->temp_error = trim_info & EXYNOS_TMU_TEMP_MASK;
+
+	if (!data->temp_error ||
+	    pdata->min_efuse_value > data->temp_error ||
+	    data->temp_error > pdata->max_efuse_value)
+		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
 
 	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
 		dev_err(&pdev->dev, "Invalid max trigger level\n");
diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
index e417af8..186e39e 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -26,14 +26,6 @@
 
 #include "exynos_thermal_common.h"
 
-enum calibration_type {
-	TYPE_ONE_POINT_TRIMMING,
-	TYPE_ONE_POINT_TRIMMING_25,
-	TYPE_ONE_POINT_TRIMMING_85,
-	TYPE_TWO_POINT_TRIMMING,
-	TYPE_NONE,
-};
-
 enum soc_type {
 	SOC_ARCH_EXYNOS4210 = 1,
 	SOC_ARCH_EXYNOS4412,
@@ -74,8 +66,6 @@ enum soc_type {
  * bitfields. The register validity, offsets and bitfield values may vary
  * slightly across different exynos SOC's.
  * @triminfo_data: register containing 2 pont trimming data
- * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data reg.
- * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data reg.
  * @triminfo_ctrl: trim info controller register.
  * @tmu_ctrl: TMU main controller register.
  * @test_mux_addr_shift: shift bits of test mux address.
@@ -116,8 +106,6 @@ enum soc_type {
  */
 struct exynos_tmu_registers {
 	u32	triminfo_data;
-	u32	triminfo_25_shift;
-	u32	triminfo_85_shift;
 
 	u32	triminfo_ctrl;
 
@@ -207,10 +195,7 @@ struct exynos_tmu_registers {
  * @min_efuse_value: minimum valid trimming data
  * @max_efuse_value: maximum valid trimming data
  * @first_point_trim: temp value of the first point trimming
- * @second_point_trim: temp value of the second point trimming
- * @default_temp_offset: default temperature offset in case of no trimming
  * @test_mux; information if SoC supports test MUX
- * @cal_type: calibration type for temperature
  * @freq_clip_table: Table representing frequency reduction percentage.
  * @freq_tab_count: Count of the above table as frequency reduction may
  *	applicable to only some of the trigger levels.
@@ -232,15 +217,12 @@ struct exynos_tmu_platform_data {
 	u8 reference_voltage;
 	u8 noise_cancel_mode;
 
-	u32 efuse_value;
+	u8 efuse_value;
 	u32 min_efuse_value;
 	u32 max_efuse_value;
 	u8 first_point_trim;
-	u8 second_point_trim;
-	u8 default_temp_offset;
 	u8 test_mux;
 
-	enum calibration_type cal_type;
 	enum soc_type type;
 	struct freq_clip_table freq_tab[4];
 	unsigned int freq_tab_count;
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index 4b992d9..c32d186 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -27,8 +27,6 @@
 #if defined(CONFIG_CPU_EXYNOS4210)
 static const struct exynos_tmu_registers exynos4210_tmu_registers = {
 	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
-	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
-	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
 	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
 	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
 	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
@@ -66,12 +64,9 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
 		.max_trigger_level = 4,
 		.gain = 15,
 		.reference_voltage = 7,
-		.cal_type = TYPE_ONE_POINT_TRIMMING,
 		.min_efuse_value = 40,
 		.max_efuse_value = 100,
 		.first_point_trim = 25,
-		.second_point_trim = 85,
-		.default_temp_offset = 50,
 		.freq_tab[0] = {
 			.freq_clip_max = 800 * 1000,
 			.temp_level = 85,
@@ -93,8 +88,6 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
 #if defined(CONFIG_SOC_EXYNOS4412) || defined(CONFIG_SOC_EXYNOS5250)
 static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
-	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
-	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
 	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
 	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
 	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
@@ -145,13 +138,10 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.gain = 8, \
 	.reference_voltage = 16, \
 	.noise_cancel_mode = 4, \
-	.cal_type = TYPE_ONE_POINT_TRIMMING, \
 	.efuse_value = 55, \
 	.min_efuse_value = 40, \
 	.max_efuse_value = 100, \
 	.first_point_trim = 25, \
-	.second_point_trim = 85, \
-	.default_temp_offset = 50, \
 	.freq_tab[0] = { \
 		.freq_clip_max = 1400 * 1000, \
 		.temp_level = 70, \
@@ -195,8 +185,6 @@ struct exynos_tmu_init_data const exynos5250_default_tmu_data = {
 #if defined(CONFIG_SOC_EXYNOS5440)
 static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
-	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
-	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
 	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
 	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
 	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
@@ -240,13 +228,10 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.gain = 5, \
 	.reference_voltage = 16, \
 	.noise_cancel_mode = 4, \
-	.cal_type = TYPE_ONE_POINT_TRIMMING, \
-	.efuse_value = 0x5b2d, \
+	.efuse_value = 45, \
 	.min_efuse_value = 16, \
 	.max_efuse_value = 76, \
 	.first_point_trim = 25, \
-	.second_point_trim = 70, \
-	.default_temp_offset = 25, \
 	.type = SOC_ARCH_EXYNOS5440, \
 	.registers = &exynos5440_tmu_registers, \
 	.features = (TMU_SUPPORT_EMULATION | TMU_SUPPORT_FALLING_TRIP | \
diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
index 1fed00d..734d1f9 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.h
+++ b/drivers/thermal/samsung/exynos_tmu_data.h
@@ -51,8 +51,6 @@
 #define EXYNOS_THD_TEMP_FALL		0x54
 #define EXYNOS_EMUL_CON		0x80
 
-#define EXYNOS_TRIMINFO_25_SHIFT	0
-#define EXYNOS_TRIMINFO_85_SHIFT	8
 #define EXYNOS_TMU_RISE_INT_MASK	0x111
 #define EXYNOS_TMU_RISE_INT_SHIFT	0
 #define EXYNOS_TMU_FALL_INT_MASK	0x111
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize()
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (3 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 14:47   ` Eduardo Valentin
  2014-05-05 11:15 ` [PATCH 06/10] thermal: exynos: remove redundant threshold_code " Bartlomiej Zolnierkiewicz
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

Remove runtime checks for pdata sanity from exynos_tmu_initialize().
The current values hardcoded in pdata will never trigger the checks
and for the new code potential mistakes should be caught during
development/review phases.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_thermal_common.h |  1 -
 drivers/thermal/samsung/exynos_tmu.c            | 13 -------------
 2 files changed, 14 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
index 3eb2ed9..cd44719 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.h
+++ b/drivers/thermal/samsung/exynos_thermal_common.h
@@ -27,7 +27,6 @@
 #define SENSOR_NAME_LEN	16
 #define MAX_TRIP_COUNT	8
 #define MAX_COOLING_DEVICE 4
-#define MAX_THRESHOLD_LEVS 5
 
 #define ACTIVE_INTERVAL 500
 #define IDLE_INTERVAL 10000
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 903566f..789d745 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -158,23 +158,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	    data->temp_error > pdata->max_efuse_value)
 		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
 
-	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
-		dev_err(&pdev->dev, "Invalid max trigger level\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
 	for (i = 0; i < pdata->max_trigger_level; i++) {
 		if (!pdata->trigger_levels[i])
 			continue;
 
-		if ((pdata->trigger_type[i] == HW_TRIP) &&
-		(!pdata->trigger_levels[pdata->max_trigger_level - 1])) {
-			dev_err(&pdev->dev, "Invalid hw trigger level\n");
-			ret = -EINVAL;
-			goto out;
-		}
-
 		/* Count trigger levels except the HW trip*/
 		if (!(pdata->trigger_type[i] == HW_TRIP))
 			trigger_levs++;
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 06/10] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (4 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize() Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 14:55   ` Eduardo Valentin
  2014-05-19  5:50   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp() Bartlomiej Zolnierkiewicz
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

Remove runtime checks for negative return values of temp_to_code()
from exynos_tmu_initialize().  The current level temperature data
hardcoded in pdata will never cause a negative temp_to_code()
return values and for the new code potential mistakes should be
caught during development/review phases.

Theres should be no functional changes caused by this patch.

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

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 789d745..a415829 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	if (data->soc == SOC_ARCH_EXYNOS4210) {
 		/* Write temperature code for threshold */
 		threshold_code = temp_to_code(data, pdata->threshold);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
 		writeb(threshold_code,
 			data->base + reg->threshold_temp);
 		for (i = 0; i < trigger_levs; i++)
@@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
 			threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i]);
-			if (threshold_code < 0) {
-				ret = threshold_code;
-				goto out;
-			}
 			rising_threshold |= threshold_code << 8 * i;
 			if (pdata->threshold_falling) {
 				threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i] -
 						pdata->threshold_falling);
-				if (threshold_code > 0)
-					falling_threshold |=
-						threshold_code << 8 * i;
+				falling_threshold |= threshold_code << 8 * i;
 			}
 		}
 
@@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 				(pdata->trigger_type[i] == HW_TRIP)) {
 			threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i]);
-			if (threshold_code < 0) {
-				ret = threshold_code;
-				goto out;
-			}
 			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
 				/* 1-4 level to be assigned in th0 reg */
 				rising_threshold |= threshold_code << 8 * i;
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp()
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (5 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 06/10] thermal: exynos: remove redundant threshold_code " Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:54   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata Bartlomiej Zolnierkiewicz
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

* Remove dead temp check from temp_to_code() (this function users
  in exynos_tmu_initialize() always pass correct temperatures and
  exynos_tmu_set_emulation() returns early for EXYNOS4210 because
  TMU_SUPPORT_EMULATION flag is not set on this SoC).

* Move temp_code check from code_to_temp() to exynos_tmu_read()
  (code_to_temp() only user).

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 | 38 +++++++++++-------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index a415829..20379eb 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -71,19 +71,7 @@ struct exynos_tmu_data {
  */
 static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 {
-	struct exynos_tmu_platform_data *pdata = data->pdata;
-	int temp_code;
-
-	if (data->soc == SOC_ARCH_EXYNOS4210)
-		/* temp should range between 25 and 125 */
-		if (temp < 25 || temp > 125) {
-			temp_code = -EINVAL;
-			goto out;
-		}
-
-	temp_code = temp + data->temp_error - pdata->first_point_trim;
-out:
-	return temp_code;
+	return temp + data->temp_error - data->pdata->first_point_trim;
 }
 
 /*
@@ -92,19 +80,7 @@ out:
  */
 static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
 {
-	struct exynos_tmu_platform_data *pdata = data->pdata;
-	int temp;
-
-	if (data->soc == SOC_ARCH_EXYNOS4210)
-		/* temp_code should range between 75 and 175 */
-		if (temp_code < 75 || temp_code > 175) {
-			temp = -ENODATA;
-			goto out;
-		}
-
-	temp = temp_code - data->temp_error + pdata->first_point_trim;
-out:
-	return temp;
+	return temp_code - data->temp_error + data->pdata->first_point_trim;
 }
 
 static int exynos_tmu_initialize(struct platform_device *pdev)
@@ -297,8 +273,16 @@ static int exynos_tmu_read(struct exynos_tmu_data *data)
 	clk_enable(data->clk);
 
 	temp_code = readb(data->base + reg->tmu_cur_temp);
-	temp = code_to_temp(data, temp_code);
 
+	if (data->soc == SOC_ARCH_EXYNOS4210)
+		/* temp_code should range between 75 and 175 */
+		if (temp_code < 75 || temp_code > 175) {
+			temp = -ENODATA;
+			goto out;
+		}
+
+	temp = code_to_temp(data, temp_code);
+out:
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
 
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (6 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp() Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:56   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control() Bartlomiej Zolnierkiewicz
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

Cache number of non-hardware trigger levels in a new pdata field
(non_hw_trigger_levels) and convert code in exynos_tmu_initialize()
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      | 16 +++-------------
 drivers/thermal/samsung/exynos_tmu.h      |  2 ++
 drivers/thermal/samsung/exynos_tmu_data.c |  3 +++
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 20379eb..a8d9524 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -90,7 +90,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	const struct exynos_tmu_registers *reg = pdata->registers;
 	unsigned int status, trim_info = 0, con;
 	unsigned int rising_threshold = 0, falling_threshold = 0;
-	int ret = 0, threshold_code, i, trigger_levs = 0;
+	int ret = 0, threshold_code, i;
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
@@ -134,29 +134,19 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 	    data->temp_error > pdata->max_efuse_value)
 		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
 
-	for (i = 0; i < pdata->max_trigger_level; i++) {
-		if (!pdata->trigger_levels[i])
-			continue;
-
-		/* Count trigger levels except the HW trip*/
-		if (!(pdata->trigger_type[i] == HW_TRIP))
-			trigger_levs++;
-	}
-
 	if (data->soc == SOC_ARCH_EXYNOS4210) {
 		/* Write temperature code for threshold */
 		threshold_code = temp_to_code(data, pdata->threshold);
 		writeb(threshold_code,
 			data->base + reg->threshold_temp);
-		for (i = 0; i < trigger_levs; i++)
+		for (i = 0; i < pdata->non_hw_trigger_levels; i++)
 			writeb(pdata->trigger_levels[i], data->base +
 			reg->threshold_th0 + i * sizeof(reg->threshold_th0));
 
 		writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
 	} else {
 		/* Write temperature code for rising and falling threshold */
-		for (i = 0;
-		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
+		for (i = 0; i < pdata->non_hw_trigger_levels; i++) {
 			threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i]);
 			rising_threshold |= threshold_code << 8 * i;
diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
index 186e39e..4845171 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -183,6 +183,7 @@ struct exynos_tmu_registers {
  *	1 = enable trigger_level[] interrupt,
  *	0 = disable trigger_level[] interrupt
  * @max_trigger_level: max trigger level supported by the TMU
+ * @non_hw_trigger_levels: number of defined non-hardware trigger levels
  * @gain: gain of amplifier in the positive-TC generator block
  *	0 <= gain <= 15
  * @reference_voltage: reference voltage of amplifier
@@ -213,6 +214,7 @@ struct exynos_tmu_platform_data {
 	enum trigger_type trigger_type[MAX_TRIP_COUNT];
 	bool trigger_enable[MAX_TRIP_COUNT];
 	u8 max_trigger_level;
+	u8 non_hw_trigger_levels;
 	u8 gain;
 	u8 reference_voltage;
 	u8 noise_cancel_mode;
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index c32d186..ef7f186 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -62,6 +62,7 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
 		.trigger_type[1] = THROTTLE_ACTIVE,
 		.trigger_type[2] = SW_TRIP,
 		.max_trigger_level = 4,
+		.non_hw_trigger_levels = 3,
 		.gain = 15,
 		.reference_voltage = 7,
 		.min_efuse_value = 40,
@@ -135,6 +136,7 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.trigger_type[2] = SW_TRIP, \
 	.trigger_type[3] = HW_TRIP, \
 	.max_trigger_level = 4, \
+	.non_hw_trigger_levels = 3, \
 	.gain = 8, \
 	.reference_voltage = 16, \
 	.noise_cancel_mode = 4, \
@@ -225,6 +227,7 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.trigger_type[0] = SW_TRIP, \
 	.trigger_type[4] = HW_TRIP, \
 	.max_trigger_level = 5, \
+	.non_hw_trigger_levels = 1, \
 	.gain = 5, \
 	.reference_voltage = 16, \
 	.noise_cancel_mode = 4, \
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control()
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (7 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-15 15:03   ` Eduardo Valentin
  2014-05-19  6:05   ` Amit Kachhap
  2014-05-05 11:15 ` [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures Bartlomiej Zolnierkiewicz
  2014-05-15  9:06 ` [PATCH 00/10] thermal: exynos: various cleanups Zhang Rui
  10 siblings, 2 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

pdata->reference_voltage and pdata->gain are always defined
to non-zero values so remove the redundant checks from
exynos_tmu_control().

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, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index a8d9524..45d7c6f 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -215,15 +215,11 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	if (pdata->test_mux)
 		con |= (pdata->test_mux << reg->test_mux_addr_shift);
 
-	if (pdata->reference_voltage) {
-		con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
-		con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
-	}
+	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
+	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
 
-	if (pdata->gain) {
-		con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
-		con |= (pdata->gain << reg->buf_slope_sel_shift);
-	}
+	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
+	con |= (pdata->gain << reg->buf_slope_sel_shift);
 
 	if (pdata->noise_cancel_mode) {
 		con &= ~(reg->therm_trip_mode_mask <<
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (8 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control() Bartlomiej Zolnierkiewicz
@ 2014-05-05 11:15 ` Bartlomiej Zolnierkiewicz
  2014-05-19  6:11   ` Amit Kachhap
  2014-05-15  9:06 ` [PATCH 00/10] thermal: exynos: various cleanups Zhang Rui
  10 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-05 11:15 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel,
	b.zolnierkie

There is no need for abstracting configuration for registers that
are identical on all SoC types.

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 ++++++------
 drivers/thermal/samsung/exynos_tmu.h      | 11 -----------
 drivers/thermal/samsung/exynos_tmu_data.c | 15 ---------------
 3 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 45d7c6f..d37e755 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -215,11 +215,11 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	if (pdata->test_mux)
 		con |= (pdata->test_mux << reg->test_mux_addr_shift);
 
-	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
-	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
+	con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK << EXYNOS_TMU_REF_VOLTAGE_SHIFT);
+	con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT;
 
-	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
-	con |= (pdata->gain << reg->buf_slope_sel_shift);
+	con &= ~(EXYNOS_TMU_BUF_SLOPE_SEL_MASK << EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
+	con |= (pdata->gain << EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
 
 	if (pdata->noise_cancel_mode) {
 		con &= ~(reg->therm_trip_mode_mask <<
@@ -228,7 +228,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 	}
 
 	if (on) {
-		con |= (1 << reg->core_en_shift);
+		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
 		interrupt_en =
 			pdata->trigger_enable[3] << reg->inten_rise3_shift |
 			pdata->trigger_enable[2] << reg->inten_rise2_shift |
@@ -238,7 +238,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 			interrupt_en |=
 				interrupt_en << reg->inten_fall0_shift;
 	} else {
-		con &= ~(1 << reg->core_en_shift);
+		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
 		interrupt_en = 0; /* Disable all interrupts */
 	}
 	writel(interrupt_en, data->base + reg->tmu_inten);
diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
index 4845171..5c25a4b 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -69,15 +69,9 @@ enum soc_type {
  * @triminfo_ctrl: trim info controller register.
  * @tmu_ctrl: TMU main controller register.
  * @test_mux_addr_shift: shift bits of test mux address.
- * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl register.
- * @buf_vref_sel_mask: mask bits of reference voltage in tmu_ctrl register.
  * @therm_trip_mode_shift: shift bits of tripping mode in tmu_ctrl register.
  * @therm_trip_mode_mask: mask bits of tripping mode in tmu_ctrl register.
  * @therm_trip_en_shift: shift bits of tripping enable in tmu_ctrl register.
- * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
-	register.
- * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl register.
- * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
  * @tmu_status: register drescribing the TMU status.
  * @tmu_cur_temp: register containing the current temperature of the TMU.
  * @threshold_temp: register containing the base threshold level.
@@ -111,14 +105,9 @@ struct exynos_tmu_registers {
 
 	u32	tmu_ctrl;
 	u32     test_mux_addr_shift;
-	u32	buf_vref_sel_shift;
-	u32	buf_vref_sel_mask;
 	u32	therm_trip_mode_shift;
 	u32	therm_trip_mode_mask;
 	u32	therm_trip_en_shift;
-	u32	buf_slope_sel_shift;
-	u32	buf_slope_sel_mask;
-	u32	core_en_shift;
 
 	u32	tmu_status;
 
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index ef7f186..32530dc 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -28,11 +28,6 @@
 static const struct exynos_tmu_registers exynos4210_tmu_registers = {
 	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
 	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
-	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
-	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
-	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
-	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
-	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
 	.tmu_status = EXYNOS_TMU_REG_STATUS,
 	.tmu_cur_temp = EXYNOS_TMU_REG_CURRENT_TEMP,
 	.threshold_temp = EXYNOS4210_TMU_REG_THRESHOLD_TEMP,
@@ -92,14 +87,9 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
 	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
 	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
 	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
-	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
-	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
 	.therm_trip_mode_shift = EXYNOS_TMU_TRIP_MODE_SHIFT,
 	.therm_trip_mode_mask = EXYNOS_TMU_TRIP_MODE_MASK,
 	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
-	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
-	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
-	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
 	.tmu_status = EXYNOS_TMU_REG_STATUS,
 	.tmu_cur_temp = EXYNOS_TMU_REG_CURRENT_TEMP,
 	.threshold_th0 = EXYNOS_THD_TEMP_RISE,
@@ -188,14 +178,9 @@ struct exynos_tmu_init_data const exynos5250_default_tmu_data = {
 static const struct exynos_tmu_registers exynos5440_tmu_registers = {
 	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
 	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
-	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
-	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
 	.therm_trip_mode_shift = EXYNOS_TMU_TRIP_MODE_SHIFT,
 	.therm_trip_mode_mask = EXYNOS_TMU_TRIP_MODE_MASK,
 	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
-	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
-	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
-	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
 	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
 	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
 	.threshold_th0 = EXYNOS5440_TMU_S0_7_TH0,
-- 
1.8.2.3


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/10] thermal: exynos: various cleanups
  2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
                   ` (9 preceding siblings ...)
  2014-05-05 11:15 ` [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures Bartlomiej Zolnierkiewicz
@ 2014-05-15  9:06 ` Zhang Rui
  2014-05-19  6:16   ` Amit Kachhap
  10 siblings, 1 reply; 36+ messages in thread
From: Zhang Rui @ 2014-05-15  9:06 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On 一, 2014-05-05 at 13:15 +0200, Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> This patch series contains various cleanups for EXYNOS thermal
> driver.  Overall it decreases driver's LOC by 13%.  It is based
> on next-20140428 kernel.  It should not cause any functionality
> changes.
> 
Amit,

what do you think of this patch set?

thanks,
rui
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> 
> Bartlomiej Zolnierkiewicz (10):
>   thermal: exynos: remove unused struct exynos_tmu_registers entries
>   thermal: exynos: remove unused defines
>   thermal: exynos: remove dead code for HW_MODE calibration
>   thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING
>     calibration
>   thermal: exynos: remove redundant pdata checks from
>     exynos_tmu_initialize()
>   thermal: exynos: remove redundant threshold_code checks from
>     exynos_tmu_initialize()
>   thermal: exynos: simplify temp_to_code() and code_to_temp()
>   thermal: exynos: cache non_hw_trigger_levels in pdata
>   thermal: exynos: remove redundant pdata checks from
>     exynos_tmu_control()
>   thermal: exynos: remove identical values from exynos*_tmu_registers
>     structures
> 
>  drivers/thermal/samsung/exynos_thermal_common.h |   1 -
>  drivers/thermal/samsung/exynos_tmu.c            | 181 ++++--------------------
>  drivers/thermal/samsung/exynos_tmu.h            |  86 +----------
>  drivers/thermal/samsung/exynos_tmu_data.c       |  40 +-----
>  drivers/thermal/samsung/exynos_tmu_data.h       |  32 +----
>  5 files changed, 37 insertions(+), 303 deletions(-)
> 



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/10] thermal: exynos: remove unused defines
  2014-05-05 11:15 ` [PATCH 02/10] thermal: exynos: remove unused defines Bartlomiej Zolnierkiewicz
@ 2014-05-15 14:07   ` Eduardo Valentin
  2014-05-19  5:17   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 14:07 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On Mon, May 05, 2014 at 01:15:31PM +0200, Bartlomiej Zolnierkiewicz wrote:
> There should be no functional changes caused by this patch.

This patch can be merged to patch 01.


> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu_data.h | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> index 06c4345..d4eeddb 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -42,20 +42,8 @@
>  /* Exynos4210 specific registers */
>  #define EXYNOS4210_TMU_REG_THRESHOLD_TEMP	0x44
>  #define EXYNOS4210_TMU_REG_TRIG_LEVEL0	0x50
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL1	0x54
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL2	0x58
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL3	0x5C
> -#define EXYNOS4210_TMU_REG_PAST_TEMP0	0x60
> -#define EXYNOS4210_TMU_REG_PAST_TEMP1	0x64
> -#define EXYNOS4210_TMU_REG_PAST_TEMP2	0x68
> -#define EXYNOS4210_TMU_REG_PAST_TEMP3	0x6C
> -
> -#define EXYNOS4210_TMU_TRIG_LEVEL0_MASK	0x1
> -#define EXYNOS4210_TMU_TRIG_LEVEL1_MASK	0x10
> -#define EXYNOS4210_TMU_TRIG_LEVEL2_MASK	0x100
> -#define EXYNOS4210_TMU_TRIG_LEVEL3_MASK	0x1000
> +
>  #define EXYNOS4210_TMU_TRIG_LEVEL_MASK	0x1111
> -#define EXYNOS4210_TMU_INTCLEAR_VAL	0x1111
>  
>  /* Exynos5250 and Exynos4412 specific registers */
>  #define EXYNOS_TMU_TRIMINFO_CON	0x14
> @@ -69,8 +57,6 @@
>  #define EXYNOS_TMU_RISE_INT_SHIFT	0
>  #define EXYNOS_TMU_FALL_INT_MASK	0x111
>  #define EXYNOS_TMU_FALL_INT_SHIFT	12
> -#define EXYNOS_TMU_CLEAR_RISE_INT	0x111
> -#define EXYNOS_TMU_CLEAR_FALL_INT	(0x111 << 12)
>  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
>  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
>  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> @@ -82,8 +68,6 @@
>  #define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
>  #define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
>  #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
> -#define EXYNOS_TMU_INTEN_FALL1_SHIFT	20
> -#define EXYNOS_TMU_INTEN_FALL2_SHIFT	24
>  
>  #define EXYNOS_EMUL_TIME	0x57F0
>  #define EXYNOS_EMUL_TIME_MASK	0xffff
> @@ -107,13 +91,11 @@
>  #define EXYNOS5440_TMU_S0_7_TH0			0x110
>  #define EXYNOS5440_TMU_S0_7_TH1			0x130
>  #define EXYNOS5440_TMU_S0_7_TH2			0x150
> -#define EXYNOS5440_TMU_S0_7_EVTEN		0x1F0
>  #define EXYNOS5440_TMU_S0_7_IRQEN		0x210
>  #define EXYNOS5440_TMU_S0_7_IRQ			0x230
>  /* exynos5440 common registers */
>  #define EXYNOS5440_TMU_IRQ_STATUS		0x000
>  #define EXYNOS5440_TMU_PMIN			0x004
> -#define EXYNOS5440_TMU_TEMP			0x008
>  
>  #define EXYNOS5440_TMU_RISE_INT_MASK		0xf
>  #define EXYNOS5440_TMU_RISE_INT_SHIFT		0
> @@ -124,13 +106,6 @@
>  #define EXYNOS5440_TMU_INTEN_RISE2_SHIFT	2
>  #define EXYNOS5440_TMU_INTEN_RISE3_SHIFT	3
>  #define EXYNOS5440_TMU_INTEN_FALL0_SHIFT	4
> -#define EXYNOS5440_TMU_INTEN_FALL1_SHIFT	5
> -#define EXYNOS5440_TMU_INTEN_FALL2_SHIFT	6
> -#define EXYNOS5440_TMU_INTEN_FALL3_SHIFT	7
> -#define EXYNOS5440_TMU_TH_RISE0_SHIFT		0
> -#define EXYNOS5440_TMU_TH_RISE1_SHIFT		8
> -#define EXYNOS5440_TMU_TH_RISE2_SHIFT		16
> -#define EXYNOS5440_TMU_TH_RISE3_SHIFT		24
>  #define EXYNOS5440_TMU_TH_RISE4_SHIFT		24
>  #define EXYNOS5440_EFUSE_SWAP_OFFSET		8
>  
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration
  2014-05-05 11:15 ` [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration Bartlomiej Zolnierkiewicz
@ 2014-05-15 14:14   ` Eduardo Valentin
  2014-05-15 15:06     ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:27   ` Amit Kachhap
  1 sibling, 1 reply; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 14:14 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

Hello Bartlomiej

On Mon, May 05, 2014 at 01:15:32PM +0200, Bartlomiej Zolnierkiewicz wrote:
> There should be no functional changes caused by this patch.
> 

Please provide better explanation when removing features. For instance,
based on the comment of commit introducing it, HW_MODE is a feature of
5540 samsung's SoC. Question is why now it is being removed?

commit 1928457ea6337043a06ca2acd9b4d01e75810a3f
Author: Amit Daniel Kachhap <amit.daniel@samsung.com>
Date:   Mon Jun 24 16:20:46 2013 +0530

    thermal: exynos: Add hardware mode thermal calibration support
        
    This patch adds support for h/w mode calibration in the TMU controller.
    Soc's like 5440 support this features. The h/w bits needed for calibration
    setting are same as that of enum calibration_type.

  


Cheers,


> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c      | 33 +------------------------------
>  drivers/thermal/samsung/exynos_tmu.h      | 13 ------------
>  drivers/thermal/samsung/exynos_tmu_data.c |  3 ---
>  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
>  4 files changed, 1 insertion(+), 50 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 0d96a51..9f2a5ee 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -76,9 +76,6 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	int temp_code;
>  
> -	if (pdata->cal_mode == HW_MODE)
> -		return temp;
> -
>  	if (data->soc == SOC_ARCH_EXYNOS4210)
>  		/* temp should range between 25 and 125 */
>  		if (temp < 25 || temp > 125) {
> @@ -113,9 +110,6 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	int temp;
>  
> -	if (pdata->cal_mode == HW_MODE)
> -		return temp_code;
> -
>  	if (data->soc == SOC_ARCH_EXYNOS4210)
>  		/* temp_code should range between 75 and 175 */
>  		if (temp_code < 75 || temp_code > 175) {
> @@ -164,9 +158,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	if (TMU_SUPPORTS(pdata, TRIM_RELOAD))
>  		__raw_writel(1, data->base + reg->triminfo_ctrl);
>  
> -	if (pdata->cal_mode == HW_MODE)
> -		goto skip_calib_data;
> -
>  	/* Save trimming info in order to perform calibration */
>  	if (data->soc == SOC_ARCH_EXYNOS5440) {
>  		/*
> @@ -202,7 +193,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  			(pdata->efuse_value >> reg->triminfo_85_shift) &
>  			EXYNOS_TMU_TEMP_MASK;
>  
> -skip_calib_data:
>  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
>  		dev_err(&pdev->dev, "Invalid max trigger level\n");
>  		ret = -EINVAL;
> @@ -311,7 +301,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	const struct exynos_tmu_registers *reg = pdata->registers;
> -	unsigned int con, interrupt_en, cal_val;
> +	unsigned int con, interrupt_en;
>  
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
> @@ -337,27 +327,6 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>  		con |= (pdata->noise_cancel_mode << reg->therm_trip_mode_shift);
>  	}
>  
> -	if (pdata->cal_mode == HW_MODE) {
> -		con &= ~(reg->calib_mode_mask << reg->calib_mode_shift);
> -		cal_val = 0;
> -		switch (pdata->cal_type) {
> -		case TYPE_TWO_POINT_TRIMMING:
> -			cal_val = 3;
> -			break;
> -		case TYPE_ONE_POINT_TRIMMING_85:
> -			cal_val = 2;
> -			break;
> -		case TYPE_ONE_POINT_TRIMMING_25:
> -			cal_val = 1;
> -			break;
> -		case TYPE_NONE:
> -			break;
> -		default:
> -			dev_err(&pdev->dev, "Invalid calibration type, using none\n");
> -		}
> -		con |= cal_val << reg->calib_mode_shift;
> -	}
> -
>  	if (on) {
>  		con |= (1 << reg->core_en_shift);
>  		interrupt_en =
> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> index 80dc899..e417af8 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -34,11 +34,6 @@ enum calibration_type {
>  	TYPE_NONE,
>  };
>  
> -enum calibration_mode {
> -	SW_MODE,
> -	HW_MODE,
> -};
> -
>  enum soc_type {
>  	SOC_ARCH_EXYNOS4210 = 1,
>  	SOC_ARCH_EXYNOS4412,
> @@ -92,10 +87,6 @@ enum soc_type {
>   * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
>  	register.
>   * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl register.
> - * @calib_mode_shift: shift bits of calibration mode value in tmu_ctrl
> -	register.
> - * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
> -	register.
>   * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
>   * @tmu_status: register drescribing the TMU status.
>   * @tmu_cur_temp: register containing the current temperature of the TMU.
> @@ -139,8 +130,6 @@ struct exynos_tmu_registers {
>  	u32	therm_trip_en_shift;
>  	u32	buf_slope_sel_shift;
>  	u32	buf_slope_sel_mask;
> -	u32	calib_mode_shift;
> -	u32	calib_mode_mask;
>  	u32	core_en_shift;
>  
>  	u32	tmu_status;
> @@ -222,7 +211,6 @@ struct exynos_tmu_registers {
>   * @default_temp_offset: default temperature offset in case of no trimming
>   * @test_mux; information if SoC supports test MUX
>   * @cal_type: calibration type for temperature
> - * @cal_mode: calibration mode for temperature
>   * @freq_clip_table: Table representing frequency reduction percentage.
>   * @freq_tab_count: Count of the above table as frequency reduction may
>   *	applicable to only some of the trigger levels.
> @@ -253,7 +241,6 @@ struct exynos_tmu_platform_data {
>  	u8 test_mux;
>  
>  	enum calibration_type cal_type;
> -	enum calibration_mode cal_mode;
>  	enum soc_type type;
>  	struct freq_clip_table freq_tab[4];
>  	unsigned int freq_tab_count;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> index 36d64d6..4b992d9 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -205,8 +205,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
>  	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
>  	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> -	.calib_mode_shift = EXYNOS_TMU_CALIB_MODE_SHIFT,
> -	.calib_mode_mask = EXYNOS_TMU_CALIB_MODE_MASK,
>  	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
>  	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
>  	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
> @@ -243,7 +241,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
>  	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> -	.cal_mode = 0, \
>  	.efuse_value = 0x5b2d, \
>  	.min_efuse_value = 16, \
>  	.max_efuse_value = 76, \
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> index d4eeddb..1fed00d 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -60,8 +60,6 @@
>  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
>  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
>  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> -#define EXYNOS_TMU_CALIB_MODE_SHIFT	4
> -#define EXYNOS_TMU_CALIB_MODE_MASK	0x3
>  
>  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
>  #define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration
  2014-05-05 11:15 ` [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration Bartlomiej Zolnierkiewicz
@ 2014-05-15 14:31   ` Eduardo Valentin
  2014-05-15 15:35     ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:40   ` Amit Kachhap
  1 sibling, 1 reply; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 14:31 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

Hello Bartlomiej,

On Mon, May 05, 2014 at 01:15:33PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Only TYPE_ONE_POINT_TRIMMING calibration is used so remove
> the dead code for TYPE_TWO_POINT_TRIMMING calibration.
> 

Only TYPE_ONE_POINT_TRIMMING is used by which SoC? This patch removes
all four types of calibrations, as present in the current code. Is this
the expected outcome?

According to commit 9d97e5c8, which introduced this feature,
TYPE_TWO_POINT_TRIMMING is supported by exynos4 tmu, as per code
history, for instance.

> There should be no functional changes caused by this patch.
> 

Well, the patch seams to be doing more than removing type two trimming.

> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c      | 55 ++++++-------------------------
>  drivers/thermal/samsung/exynos_tmu.h      | 20 +----------
>  drivers/thermal/samsung/exynos_tmu_data.c | 17 +---------
>  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
>  4 files changed, 12 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 9f2a5ee..903566f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -47,8 +47,7 @@
>   * @irq_work: pointer to the irq work structure.
>   * @lock: lock to implement synchronization.
>   * @clk: pointer to the clock structure.
> - * @temp_error1: fused value of the first point trim.
> - * @temp_error2: fused value of the second point trim.
> + * @temp_error: fused value of the first point trim.
>   * @regulator: pointer to the TMU regulator structure.
>   * @reg_conf: pointer to structure to register with core thermal.
>   */
> @@ -62,14 +61,13 @@ struct exynos_tmu_data {
>  	struct work_struct irq_work;
>  	struct mutex lock;
>  	struct clk *clk;
> -	u8 temp_error1, temp_error2;
> +	u8 temp_error;
>  	struct regulator *regulator;
>  	struct thermal_sensor_conf *reg_conf;
>  };
>  
>  /*
>   * TMU treats temperature as a mapped temperature code.
> - * The temperature is converted differently depending on the calibration type.
>   */
>  static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>  {
> @@ -83,20 +81,7 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>  			goto out;
>  		}
>  
> -	switch (pdata->cal_type) {
> -	case TYPE_TWO_POINT_TRIMMING:
> -		temp_code = (temp - pdata->first_point_trim) *
> -			(data->temp_error2 - data->temp_error1) /
> -			(pdata->second_point_trim - pdata->first_point_trim) +
> -			data->temp_error1;
> -		break;
> -	case TYPE_ONE_POINT_TRIMMING:
> -		temp_code = temp + data->temp_error1 - pdata->first_point_trim;
> -		break;
> -	default:
> -		temp_code = temp + pdata->default_temp_offset;
> -		break;
> -	}
> +	temp_code = temp + data->temp_error - pdata->first_point_trim;
>  out:
>  	return temp_code;
>  }
> @@ -117,20 +102,7 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
>  			goto out;
>  		}
>  
> -	switch (pdata->cal_type) {
> -	case TYPE_TWO_POINT_TRIMMING:
> -		temp = (temp_code - data->temp_error1) *
> -			(pdata->second_point_trim - pdata->first_point_trim) /
> -			(data->temp_error2 - data->temp_error1) +
> -			pdata->first_point_trim;
> -		break;
> -	case TYPE_ONE_POINT_TRIMMING:
> -		temp = temp_code - data->temp_error1 + pdata->first_point_trim;
> -		break;
> -	default:
> -		temp = temp_code - pdata->default_temp_offset;
> -		break;
> -	}
> +	temp = temp_code - data->temp_error + pdata->first_point_trim;
>  out:
>  	return temp;
>  }
> @@ -179,19 +151,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	} else {
>  		trim_info = readl(data->base + reg->triminfo_data);
>  	}
> -	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
> -	data->temp_error2 = ((trim_info >> reg->triminfo_85_shift) &
> -				EXYNOS_TMU_TEMP_MASK);
> -
> -	if (!data->temp_error1 ||
> -		(pdata->min_efuse_value > data->temp_error1) ||
> -		(data->temp_error1 > pdata->max_efuse_value))
> -		data->temp_error1 = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
> -
> -	if (!data->temp_error2)
> -		data->temp_error2 =
> -			(pdata->efuse_value >> reg->triminfo_85_shift) &
> -			EXYNOS_TMU_TEMP_MASK;
> +	data->temp_error = trim_info & EXYNOS_TMU_TEMP_MASK;
> +
> +	if (!data->temp_error ||
> +	    pdata->min_efuse_value > data->temp_error ||
> +	    data->temp_error > pdata->max_efuse_value)
> +		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
>  
>  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
>  		dev_err(&pdev->dev, "Invalid max trigger level\n");
> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> index e417af8..186e39e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -26,14 +26,6 @@
>  
>  #include "exynos_thermal_common.h"
>  
> -enum calibration_type {
> -	TYPE_ONE_POINT_TRIMMING,
> -	TYPE_ONE_POINT_TRIMMING_25,
> -	TYPE_ONE_POINT_TRIMMING_85,
> -	TYPE_TWO_POINT_TRIMMING,
> -	TYPE_NONE,
> -};


There are more than two types of calibrations. tmu_control covers
all types. This patch misses tmu_control.

> -
>  enum soc_type {
>  	SOC_ARCH_EXYNOS4210 = 1,
>  	SOC_ARCH_EXYNOS4412,
> @@ -74,8 +66,6 @@ enum soc_type {
>   * bitfields. The register validity, offsets and bitfield values may vary
>   * slightly across different exynos SOC's.
>   * @triminfo_data: register containing 2 pont trimming data
> - * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data reg.
> - * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data reg.
>   * @triminfo_ctrl: trim info controller register.
>   * @tmu_ctrl: TMU main controller register.
>   * @test_mux_addr_shift: shift bits of test mux address.
> @@ -116,8 +106,6 @@ enum soc_type {
>   */
>  struct exynos_tmu_registers {
>  	u32	triminfo_data;
> -	u32	triminfo_25_shift;
> -	u32	triminfo_85_shift;
>  
>  	u32	triminfo_ctrl;
>  
> @@ -207,10 +195,7 @@ struct exynos_tmu_registers {
>   * @min_efuse_value: minimum valid trimming data
>   * @max_efuse_value: maximum valid trimming data
>   * @first_point_trim: temp value of the first point trimming
> - * @second_point_trim: temp value of the second point trimming
> - * @default_temp_offset: default temperature offset in case of no trimming
>   * @test_mux; information if SoC supports test MUX
> - * @cal_type: calibration type for temperature
>   * @freq_clip_table: Table representing frequency reduction percentage.
>   * @freq_tab_count: Count of the above table as frequency reduction may
>   *	applicable to only some of the trigger levels.
> @@ -232,15 +217,12 @@ struct exynos_tmu_platform_data {
>  	u8 reference_voltage;
>  	u8 noise_cancel_mode;
>  
> -	u32 efuse_value;
> +	u8 efuse_value;

Why?

>  	u32 min_efuse_value;
>  	u32 max_efuse_value;
>  	u8 first_point_trim;
> -	u8 second_point_trim;
> -	u8 default_temp_offset;
>  	u8 test_mux;
>  
> -	enum calibration_type cal_type;
>  	enum soc_type type;
>  	struct freq_clip_table freq_tab[4];
>  	unsigned int freq_tab_count;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> index 4b992d9..c32d186 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -27,8 +27,6 @@
>  #if defined(CONFIG_CPU_EXYNOS4210)
>  static const struct exynos_tmu_registers exynos4210_tmu_registers = {
>  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
>  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> @@ -66,12 +64,9 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
>  		.max_trigger_level = 4,
>  		.gain = 15,
>  		.reference_voltage = 7,
> -		.cal_type = TYPE_ONE_POINT_TRIMMING,
>  		.min_efuse_value = 40,
>  		.max_efuse_value = 100,
>  		.first_point_trim = 25,
> -		.second_point_trim = 85,
> -		.default_temp_offset = 50,
>  		.freq_tab[0] = {
>  			.freq_clip_max = 800 * 1000,
>  			.temp_level = 85,
> @@ -93,8 +88,6 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
>  #if defined(CONFIG_SOC_EXYNOS4412) || defined(CONFIG_SOC_EXYNOS5250)
>  static const struct exynos_tmu_registers exynos4412_tmu_registers = {
>  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
> @@ -145,13 +138,10 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
>  	.gain = 8, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
>  	.efuse_value = 55, \
>  	.min_efuse_value = 40, \
>  	.max_efuse_value = 100, \
>  	.first_point_trim = 25, \
> -	.second_point_trim = 85, \
> -	.default_temp_offset = 50, \
>  	.freq_tab[0] = { \
>  		.freq_clip_max = 1400 * 1000, \
>  		.temp_level = 70, \
> @@ -195,8 +185,6 @@ struct exynos_tmu_init_data const exynos5250_default_tmu_data = {
>  #if defined(CONFIG_SOC_EXYNOS5440)
>  static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
>  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
>  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> @@ -240,13 +228,10 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.gain = 5, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> -	.efuse_value = 0x5b2d, \
> +	.efuse_value = 45, \

What efuse value has to do with removing type two calibration type?

>  	.min_efuse_value = 16, \
>  	.max_efuse_value = 76, \
>  	.first_point_trim = 25, \
> -	.second_point_trim = 70, \
> -	.default_temp_offset = 25, \
>  	.type = SOC_ARCH_EXYNOS5440, \
>  	.registers = &exynos5440_tmu_registers, \
>  	.features = (TMU_SUPPORT_EMULATION | TMU_SUPPORT_FALLING_TRIP | \
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> index 1fed00d..734d1f9 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -51,8 +51,6 @@
>  #define EXYNOS_THD_TEMP_FALL		0x54
>  #define EXYNOS_EMUL_CON		0x80
>  
> -#define EXYNOS_TRIMINFO_25_SHIFT	0
> -#define EXYNOS_TRIMINFO_85_SHIFT	8
>  #define EXYNOS_TMU_RISE_INT_MASK	0x111
>  #define EXYNOS_TMU_RISE_INT_SHIFT	0
>  #define EXYNOS_TMU_FALL_INT_MASK	0x111
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize()
  2014-05-05 11:15 ` [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize() Bartlomiej Zolnierkiewicz
@ 2014-05-15 14:47   ` Eduardo Valentin
  2014-05-15 16:24     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 14:47 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

Hello Bartlomiej,

On Mon, May 05, 2014 at 01:15:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Remove runtime checks for pdata sanity from exynos_tmu_initialize().
> The current values hardcoded in pdata will never trigger the checks
> and for the new code potential mistakes should be caught during
> development/review phases.
> 
> There should be no functional changes caused by this patch.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_thermal_common.h |  1 -
>  drivers/thermal/samsung/exynos_tmu.c            | 13 -------------
>  2 files changed, 14 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
> index 3eb2ed9..cd44719 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.h
> +++ b/drivers/thermal/samsung/exynos_thermal_common.h
> @@ -27,7 +27,6 @@
>  #define SENSOR_NAME_LEN	16
>  #define MAX_TRIP_COUNT	8
>  #define MAX_COOLING_DEVICE 4
> -#define MAX_THRESHOLD_LEVS 5
>  
>  #define ACTIVE_INTERVAL 500
>  #define IDLE_INTERVAL 10000
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 903566f..789d745 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -158,23 +158,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	    data->temp_error > pdata->max_efuse_value)
>  		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
>  
> -	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
> -		dev_err(&pdev->dev, "Invalid max trigger level\n");
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
>  	for (i = 0; i < pdata->max_trigger_level; i++) {
>  		if (!pdata->trigger_levels[i])
>  			continue;
>  
> -		if ((pdata->trigger_type[i] == HW_TRIP) &&
> -		(!pdata->trigger_levels[pdata->max_trigger_level - 1])) {
> -			dev_err(&pdev->dev, "Invalid hw trigger level\n");
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -

Does it mean no new pdata are going to be written? i.e., no new soc is
going to be supported by this driver that needs proper pdata checking?

>  		/* Count trigger levels except the HW trip*/
>  		if (!(pdata->trigger_type[i] == HW_TRIP))
>  			trigger_levs++;
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()
  2014-05-05 11:15 ` [PATCH 06/10] thermal: exynos: remove redundant threshold_code " Bartlomiej Zolnierkiewicz
@ 2014-05-15 14:55   ` Eduardo Valentin
  2014-05-15 16:56     ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:50   ` Amit Kachhap
  1 sibling, 1 reply; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 14:55 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

Hello Bartlomiej,

On Mon, May 05, 2014 at 01:15:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Remove runtime checks for negative return values of temp_to_code()
> from exynos_tmu_initialize().  The current level temperature data
> hardcoded in pdata will never cause a negative temp_to_code()
> return values and for the new code potential mistakes should be
> caught during development/review phases.
> 
> Theres should be no functional changes caused by this patch.
> 

Same question as in previous patch. Removing defensive code must
be done carefully. 

> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 789d745..a415829 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	if (data->soc == SOC_ARCH_EXYNOS4210) {
>  		/* Write temperature code for threshold */
>  		threshold_code = temp_to_code(data, pdata->threshold);
> -		if (threshold_code < 0) {
> -			ret = threshold_code;
> -			goto out;
> -		}
>  		writeb(threshold_code,
>  			data->base + reg->threshold_temp);
>  		for (i = 0; i < trigger_levs; i++)
> @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			rising_threshold |= threshold_code << 8 * i;
>  			if (pdata->threshold_falling) {
>  				threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i] -
>  						pdata->threshold_falling);
> -				if (threshold_code > 0)
> -					falling_threshold |=
> -						threshold_code << 8 * i;
> +				falling_threshold |= threshold_code << 8 * i;
>  			}
>  		}
>  
> @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  				(pdata->trigger_type[i] == HW_TRIP)) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
>  				/* 1-4 level to be assigned in th0 reg */
>  				rising_threshold |= threshold_code << 8 * i;
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control()
  2014-05-05 11:15 ` [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control() Bartlomiej Zolnierkiewicz
@ 2014-05-15 15:03   ` Eduardo Valentin
  2014-05-15 17:06     ` Bartlomiej Zolnierkiewicz
  2014-05-19  6:05   ` Amit Kachhap
  1 sibling, 1 reply; 36+ messages in thread
From: Eduardo Valentin @ 2014-05-15 15:03 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

Hello Bartlomiej,

On Mon, May 05, 2014 at 01:15:38PM +0200, Bartlomiej Zolnierkiewicz wrote:
> pdata->reference_voltage and pdata->gain are always defined
> to non-zero values so remove the redundant checks from
> exynos_tmu_control().
> 

In all existing SoCs? 
Should this be considered to any upcoming SoCs?

> 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, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index a8d9524..45d7c6f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -215,15 +215,11 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>  	if (pdata->test_mux)
>  		con |= (pdata->test_mux << reg->test_mux_addr_shift);
>  
> -	if (pdata->reference_voltage) {
> -		con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> -		con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
> -	}
> +	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> +	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
>  
> -	if (pdata->gain) {
> -		con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> -		con |= (pdata->gain << reg->buf_slope_sel_shift);
> -	}
> +	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> +	con |= (pdata->gain << reg->buf_slope_sel_shift);

The way it is put, looks like gain and reference_voltage are mandatory
pdata fields. Should these checks be moved to initialization phase?

>  
>  	if (pdata->noise_cancel_mode) {
>  		con &= ~(reg->therm_trip_mode_mask <<
> -- 
> 1.8.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration
  2014-05-15 14:14   ` Eduardo Valentin
@ 2014-05-15 15:06     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-15 15:06 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Zhang Rui, Amit Daniel Kachhap, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On Thursday, May 15, 2014 10:14:41 AM Eduardo Valentin wrote:
> Hello Bartlomiej

Hi,

> On Mon, May 05, 2014 at 01:15:32PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > There should be no functional changes caused by this patch.
> > 
> 
> Please provide better explanation when removing features. For instance,
> based on the comment of commit introducing it, HW_MODE is a feature of
> 5540 samsung's SoC. Question is why now it is being removed?
> 
> commit 1928457ea6337043a06ca2acd9b4d01e75810a3f
> Author: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Date:   Mon Jun 24 16:20:46 2013 +0530
> 
>     thermal: exynos: Add hardware mode thermal calibration support
>         
>     This patch adds support for h/w mode calibration in the TMU controller.
>     Soc's like 5440 support this features. The h/w bits needed for calibration
>     setting are same as that of enum calibration_type.

The commit above has added HW_MODE feature but it has never been enabled.
As such it has been a dead code for almost a year now and should be removed
from the kernel.  Morevoer if somebody would like to re-introduce it for
Exynos5440 (or other SoC) she/he should explain why it is needed and why it
is better than the software calibration that is used for all SoCs currently.

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

> Cheers,
> 
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c      | 33 +------------------------------
> >  drivers/thermal/samsung/exynos_tmu.h      | 13 ------------
> >  drivers/thermal/samsung/exynos_tmu_data.c |  3 ---
> >  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
> >  4 files changed, 1 insertion(+), 50 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 0d96a51..9f2a5ee 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -76,9 +76,6 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> >  	struct exynos_tmu_platform_data *pdata = data->pdata;
> >  	int temp_code;
> >  
> > -	if (pdata->cal_mode == HW_MODE)
> > -		return temp;
> > -
> >  	if (data->soc == SOC_ARCH_EXYNOS4210)
> >  		/* temp should range between 25 and 125 */
> >  		if (temp < 25 || temp > 125) {
> > @@ -113,9 +110,6 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
> >  	struct exynos_tmu_platform_data *pdata = data->pdata;
> >  	int temp;
> >  
> > -	if (pdata->cal_mode == HW_MODE)
> > -		return temp_code;
> > -
> >  	if (data->soc == SOC_ARCH_EXYNOS4210)
> >  		/* temp_code should range between 75 and 175 */
> >  		if (temp_code < 75 || temp_code > 175) {
> > @@ -164,9 +158,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  	if (TMU_SUPPORTS(pdata, TRIM_RELOAD))
> >  		__raw_writel(1, data->base + reg->triminfo_ctrl);
> >  
> > -	if (pdata->cal_mode == HW_MODE)
> > -		goto skip_calib_data;
> > -
> >  	/* Save trimming info in order to perform calibration */
> >  	if (data->soc == SOC_ARCH_EXYNOS5440) {
> >  		/*
> > @@ -202,7 +193,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  			(pdata->efuse_value >> reg->triminfo_85_shift) &
> >  			EXYNOS_TMU_TEMP_MASK;
> >  
> > -skip_calib_data:
> >  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
> >  		dev_err(&pdev->dev, "Invalid max trigger level\n");
> >  		ret = -EINVAL;
> > @@ -311,7 +301,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
> >  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> >  	struct exynos_tmu_platform_data *pdata = data->pdata;
> >  	const struct exynos_tmu_registers *reg = pdata->registers;
> > -	unsigned int con, interrupt_en, cal_val;
> > +	unsigned int con, interrupt_en;
> >  
> >  	mutex_lock(&data->lock);
> >  	clk_enable(data->clk);
> > @@ -337,27 +327,6 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
> >  		con |= (pdata->noise_cancel_mode << reg->therm_trip_mode_shift);
> >  	}
> >  
> > -	if (pdata->cal_mode == HW_MODE) {
> > -		con &= ~(reg->calib_mode_mask << reg->calib_mode_shift);
> > -		cal_val = 0;
> > -		switch (pdata->cal_type) {
> > -		case TYPE_TWO_POINT_TRIMMING:
> > -			cal_val = 3;
> > -			break;
> > -		case TYPE_ONE_POINT_TRIMMING_85:
> > -			cal_val = 2;
> > -			break;
> > -		case TYPE_ONE_POINT_TRIMMING_25:
> > -			cal_val = 1;
> > -			break;
> > -		case TYPE_NONE:
> > -			break;
> > -		default:
> > -			dev_err(&pdev->dev, "Invalid calibration type, using none\n");
> > -		}
> > -		con |= cal_val << reg->calib_mode_shift;
> > -	}
> > -
> >  	if (on) {
> >  		con |= (1 << reg->core_en_shift);
> >  		interrupt_en =
> > diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> > index 80dc899..e417af8 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.h
> > +++ b/drivers/thermal/samsung/exynos_tmu.h
> > @@ -34,11 +34,6 @@ enum calibration_type {
> >  	TYPE_NONE,
> >  };
> >  
> > -enum calibration_mode {
> > -	SW_MODE,
> > -	HW_MODE,
> > -};
> > -
> >  enum soc_type {
> >  	SOC_ARCH_EXYNOS4210 = 1,
> >  	SOC_ARCH_EXYNOS4412,
> > @@ -92,10 +87,6 @@ enum soc_type {
> >   * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
> >  	register.
> >   * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl register.
> > - * @calib_mode_shift: shift bits of calibration mode value in tmu_ctrl
> > -	register.
> > - * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
> > -	register.
> >   * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
> >   * @tmu_status: register drescribing the TMU status.
> >   * @tmu_cur_temp: register containing the current temperature of the TMU.
> > @@ -139,8 +130,6 @@ struct exynos_tmu_registers {
> >  	u32	therm_trip_en_shift;
> >  	u32	buf_slope_sel_shift;
> >  	u32	buf_slope_sel_mask;
> > -	u32	calib_mode_shift;
> > -	u32	calib_mode_mask;
> >  	u32	core_en_shift;
> >  
> >  	u32	tmu_status;
> > @@ -222,7 +211,6 @@ struct exynos_tmu_registers {
> >   * @default_temp_offset: default temperature offset in case of no trimming
> >   * @test_mux; information if SoC supports test MUX
> >   * @cal_type: calibration type for temperature
> > - * @cal_mode: calibration mode for temperature
> >   * @freq_clip_table: Table representing frequency reduction percentage.
> >   * @freq_tab_count: Count of the above table as frequency reduction may
> >   *	applicable to only some of the trigger levels.
> > @@ -253,7 +241,6 @@ struct exynos_tmu_platform_data {
> >  	u8 test_mux;
> >  
> >  	enum calibration_type cal_type;
> > -	enum calibration_mode cal_mode;
> >  	enum soc_type type;
> >  	struct freq_clip_table freq_tab[4];
> >  	unsigned int freq_tab_count;
> > diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> > index 36d64d6..4b992d9 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.c
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> > @@ -205,8 +205,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
> >  	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
> >  	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
> >  	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> > -	.calib_mode_shift = EXYNOS_TMU_CALIB_MODE_SHIFT,
> > -	.calib_mode_mask = EXYNOS_TMU_CALIB_MODE_MASK,
> >  	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
> >  	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
> >  	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
> > @@ -243,7 +241,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
> >  	.reference_voltage = 16, \
> >  	.noise_cancel_mode = 4, \
> >  	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> > -	.cal_mode = 0, \
> >  	.efuse_value = 0x5b2d, \
> >  	.min_efuse_value = 16, \
> >  	.max_efuse_value = 76, \
> > diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> > index d4eeddb..1fed00d 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.h
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> > @@ -60,8 +60,6 @@
> >  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
> >  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
> >  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> > -#define EXYNOS_TMU_CALIB_MODE_SHIFT	4
> > -#define EXYNOS_TMU_CALIB_MODE_MASK	0x3
> >  
> >  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
> >  #define EXYNOS_TMU_INTEN_RISE1_SHIFT	4


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration
  2014-05-15 14:31   ` Eduardo Valentin
@ 2014-05-15 15:35     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-15 15:35 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel


On Thursday, May 15, 2014 10:31:33 AM Eduardo Valentin wrote:
> Hello Bartlomiej,

Hi,

> On Mon, May 05, 2014 at 01:15:33PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Only TYPE_ONE_POINT_TRIMMING calibration is used so remove
> > the dead code for TYPE_TWO_POINT_TRIMMING calibration.
> > 
> 
> Only TYPE_ONE_POINT_TRIMMING is used by which SoC? This patch removes

TYPE_ONE_POINT_TRIMMING is used by all Exynos SoCs currently.

> all four types of calibrations, as present in the current code. Is this
> the expected outcome?

There are only two types of calibration (TYPE_ONE_POINT_TRIMMING and
TYPE_TWO_POINT_TRIMMING) implemented in the driver currently, the other
ones (TYPE_ONE_POINT_TRIMMING_25 and TYPE_ONE_POINT_TRIMMING_85) are
defined in calibration_type enum but are not implemented.

> According to commit 9d97e5c8, which introduced this feature,
> TYPE_TWO_POINT_TRIMMING is supported by exynos4 tmu, as per code
> history, for instance.

The commit 9d97e5c8 (which is from Wed Sep 7 18:49:08 2011) introduced
TYPE_TWO_POINT_TRIMMING implementation but no users of it have ever been
added.  IOW it has been a dead code for over 2.5 years now.

> > There should be no functional changes caused by this patch.
> > 
> 
> Well, the patch seams to be doing more than removing type two trimming.

It does related dead code removals.  I can improve the patch description
if needed to be more verbose but it doesn't change the fact that the patch
doesn't contain any functional changes.

> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c      | 55 ++++++-------------------------
> >  drivers/thermal/samsung/exynos_tmu.h      | 20 +----------
> >  drivers/thermal/samsung/exynos_tmu_data.c | 17 +---------
> >  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
> >  4 files changed, 12 insertions(+), 82 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 9f2a5ee..903566f 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -47,8 +47,7 @@
> >   * @irq_work: pointer to the irq work structure.
> >   * @lock: lock to implement synchronization.
> >   * @clk: pointer to the clock structure.
> > - * @temp_error1: fused value of the first point trim.
> > - * @temp_error2: fused value of the second point trim.
> > + * @temp_error: fused value of the first point trim.
> >   * @regulator: pointer to the TMU regulator structure.
> >   * @reg_conf: pointer to structure to register with core thermal.
> >   */
> > @@ -62,14 +61,13 @@ struct exynos_tmu_data {
> >  	struct work_struct irq_work;
> >  	struct mutex lock;
> >  	struct clk *clk;
> > -	u8 temp_error1, temp_error2;
> > +	u8 temp_error;
> >  	struct regulator *regulator;
> >  	struct thermal_sensor_conf *reg_conf;
> >  };
> >  
> >  /*
> >   * TMU treats temperature as a mapped temperature code.
> > - * The temperature is converted differently depending on the calibration type.
> >   */
> >  static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> >  {
> > @@ -83,20 +81,7 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> >  			goto out;
> >  		}
> >  
> > -	switch (pdata->cal_type) {
> > -	case TYPE_TWO_POINT_TRIMMING:
> > -		temp_code = (temp - pdata->first_point_trim) *
> > -			(data->temp_error2 - data->temp_error1) /
> > -			(pdata->second_point_trim - pdata->first_point_trim) +
> > -			data->temp_error1;
> > -		break;
> > -	case TYPE_ONE_POINT_TRIMMING:
> > -		temp_code = temp + data->temp_error1 - pdata->first_point_trim;
> > -		break;
> > -	default:
> > -		temp_code = temp + pdata->default_temp_offset;
> > -		break;
> > -	}
> > +	temp_code = temp + data->temp_error - pdata->first_point_trim;
> >  out:
> >  	return temp_code;
> >  }
> > @@ -117,20 +102,7 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
> >  			goto out;
> >  		}
> >  
> > -	switch (pdata->cal_type) {
> > -	case TYPE_TWO_POINT_TRIMMING:
> > -		temp = (temp_code - data->temp_error1) *
> > -			(pdata->second_point_trim - pdata->first_point_trim) /
> > -			(data->temp_error2 - data->temp_error1) +
> > -			pdata->first_point_trim;
> > -		break;
> > -	case TYPE_ONE_POINT_TRIMMING:
> > -		temp = temp_code - data->temp_error1 + pdata->first_point_trim;
> > -		break;
> > -	default:
> > -		temp = temp_code - pdata->default_temp_offset;
> > -		break;
> > -	}
> > +	temp = temp_code - data->temp_error + pdata->first_point_trim;
> >  out:
> >  	return temp;
> >  }
> > @@ -179,19 +151,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  	} else {
> >  		trim_info = readl(data->base + reg->triminfo_data);
> >  	}
> > -	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
> > -	data->temp_error2 = ((trim_info >> reg->triminfo_85_shift) &
> > -				EXYNOS_TMU_TEMP_MASK);
> > -
> > -	if (!data->temp_error1 ||
> > -		(pdata->min_efuse_value > data->temp_error1) ||
> > -		(data->temp_error1 > pdata->max_efuse_value))
> > -		data->temp_error1 = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
> > -
> > -	if (!data->temp_error2)
> > -		data->temp_error2 =
> > -			(pdata->efuse_value >> reg->triminfo_85_shift) &
> > -			EXYNOS_TMU_TEMP_MASK;
> > +	data->temp_error = trim_info & EXYNOS_TMU_TEMP_MASK;
> > +
> > +	if (!data->temp_error ||
> > +	    pdata->min_efuse_value > data->temp_error ||
> > +	    data->temp_error > pdata->max_efuse_value)
> > +		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
> >  
> >  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
> >  		dev_err(&pdev->dev, "Invalid max trigger level\n");
> > diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> > index e417af8..186e39e 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.h
> > +++ b/drivers/thermal/samsung/exynos_tmu.h
> > @@ -26,14 +26,6 @@
> >  
> >  #include "exynos_thermal_common.h"
> >  
> > -enum calibration_type {
> > -	TYPE_ONE_POINT_TRIMMING,
> > -	TYPE_ONE_POINT_TRIMMING_25,
> > -	TYPE_ONE_POINT_TRIMMING_85,
> > -	TYPE_TWO_POINT_TRIMMING,
> > -	TYPE_NONE,
> > -};
> 
> 
> There are more than two types of calibrations. tmu_control covers
> all types. This patch misses tmu_control.

Please take a look at patch #3 (the code in question has been removed by
it altogether).

> > -
> >  enum soc_type {
> >  	SOC_ARCH_EXYNOS4210 = 1,
> >  	SOC_ARCH_EXYNOS4412,
> > @@ -74,8 +66,6 @@ enum soc_type {
> >   * bitfields. The register validity, offsets and bitfield values may vary
> >   * slightly across different exynos SOC's.
> >   * @triminfo_data: register containing 2 pont trimming data
> > - * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data reg.
> > - * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data reg.
> >   * @triminfo_ctrl: trim info controller register.
> >   * @tmu_ctrl: TMU main controller register.
> >   * @test_mux_addr_shift: shift bits of test mux address.
> > @@ -116,8 +106,6 @@ enum soc_type {
> >   */
> >  struct exynos_tmu_registers {
> >  	u32	triminfo_data;
> > -	u32	triminfo_25_shift;
> > -	u32	triminfo_85_shift;
> >  
> >  	u32	triminfo_ctrl;
> >  
> > @@ -207,10 +195,7 @@ struct exynos_tmu_registers {
> >   * @min_efuse_value: minimum valid trimming data
> >   * @max_efuse_value: maximum valid trimming data
> >   * @first_point_trim: temp value of the first point trimming
> > - * @second_point_trim: temp value of the second point trimming
> > - * @default_temp_offset: default temperature offset in case of no trimming
> >   * @test_mux; information if SoC supports test MUX
> > - * @cal_type: calibration type for temperature
> >   * @freq_clip_table: Table representing frequency reduction percentage.
> >   * @freq_tab_count: Count of the above table as frequency reduction may
> >   *	applicable to only some of the trigger levels.
> > @@ -232,15 +217,12 @@ struct exynos_tmu_platform_data {
> >  	u8 reference_voltage;
> >  	u8 noise_cancel_mode;
> >  
> > -	u32 efuse_value;
> > +	u8 efuse_value;
> 
> Why?

In exynos_tmu_initialize() if data->temp_error2 was zero then its value was
obtained from bits 8-15 of efuse_value (bits 16-31 are always zero).  After
TYPE_TWO_POINT_TRIMMING code removal data->temp_error2 was no longer needed
and was also removed.  Thus only bits 0-7 of efuse_value are ever used and
its type can be changed from u32 to u8.

> >  	u32 min_efuse_value;
> >  	u32 max_efuse_value;
> >  	u8 first_point_trim;
> > -	u8 second_point_trim;
> > -	u8 default_temp_offset;
> >  	u8 test_mux;
> >  
> > -	enum calibration_type cal_type;
> >  	enum soc_type type;
> >  	struct freq_clip_table freq_tab[4];
> >  	unsigned int freq_tab_count;
> > diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> > index 4b992d9..c32d186 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.c
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> > @@ -27,8 +27,6 @@
> >  #if defined(CONFIG_CPU_EXYNOS4210)
> >  static const struct exynos_tmu_registers exynos4210_tmu_registers = {
> >  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> > -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> > -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
> >  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
> >  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> >  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> > @@ -66,12 +64,9 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
> >  		.max_trigger_level = 4,
> >  		.gain = 15,
> >  		.reference_voltage = 7,
> > -		.cal_type = TYPE_ONE_POINT_TRIMMING,
> >  		.min_efuse_value = 40,
> >  		.max_efuse_value = 100,
> >  		.first_point_trim = 25,
> > -		.second_point_trim = 85,
> > -		.default_temp_offset = 50,
> >  		.freq_tab[0] = {
> >  			.freq_clip_max = 800 * 1000,
> >  			.temp_level = 85,
> > @@ -93,8 +88,6 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
> >  #if defined(CONFIG_SOC_EXYNOS4412) || defined(CONFIG_SOC_EXYNOS5250)
> >  static const struct exynos_tmu_registers exynos4412_tmu_registers = {
> >  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> > -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> > -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
> >  	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
> >  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
> >  	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
> > @@ -145,13 +138,10 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
> >  	.gain = 8, \
> >  	.reference_voltage = 16, \
> >  	.noise_cancel_mode = 4, \
> > -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> >  	.efuse_value = 55, \
> >  	.min_efuse_value = 40, \
> >  	.max_efuse_value = 100, \
> >  	.first_point_trim = 25, \
> > -	.second_point_trim = 85, \
> > -	.default_temp_offset = 50, \
> >  	.freq_tab[0] = { \
> >  		.freq_clip_max = 1400 * 1000, \
> >  		.temp_level = 70, \
> > @@ -195,8 +185,6 @@ struct exynos_tmu_init_data const exynos5250_default_tmu_data = {
> >  #if defined(CONFIG_SOC_EXYNOS5440)
> >  static const struct exynos_tmu_registers exynos5440_tmu_registers = {
> >  	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
> > -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> > -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
> >  	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
> >  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> >  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> > @@ -240,13 +228,10 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
> >  	.gain = 5, \
> >  	.reference_voltage = 16, \
> >  	.noise_cancel_mode = 4, \
> > -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> > -	.efuse_value = 0x5b2d, \
> > +	.efuse_value = 45, \
> 
> What efuse value has to do with removing type two calibration type?

Bits 8-15 of efuse_value are only used for TYPE_TWO_POINT_TRIMMING code
(please take a look at changes inside exynos_tmu_initialize() for details).

> >  	.min_efuse_value = 16, \
> >  	.max_efuse_value = 76, \
> >  	.first_point_trim = 25, \
> > -	.second_point_trim = 70, \
> > -	.default_temp_offset = 25, \
> >  	.type = SOC_ARCH_EXYNOS5440, \
> >  	.registers = &exynos5440_tmu_registers, \
> >  	.features = (TMU_SUPPORT_EMULATION | TMU_SUPPORT_FALLING_TRIP | \
> > diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
> > index 1fed00d..734d1f9 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.h
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> > @@ -51,8 +51,6 @@
> >  #define EXYNOS_THD_TEMP_FALL		0x54
> >  #define EXYNOS_EMUL_CON		0x80
> >  
> > -#define EXYNOS_TRIMINFO_25_SHIFT	0
> > -#define EXYNOS_TRIMINFO_85_SHIFT	8
> >  #define EXYNOS_TMU_RISE_INT_MASK	0x111
> >  #define EXYNOS_TMU_RISE_INT_SHIFT	0
> >  #define EXYNOS_TMU_FALL_INT_MASK	0x111

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize()
  2014-05-15 14:47   ` Eduardo Valentin
@ 2014-05-15 16:24     ` Bartlomiej Zolnierkiewicz
  2014-05-19  5:47       ` Amit Kachhap
  0 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-15 16:24 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On Thursday, May 15, 2014 10:47:40 AM Eduardo Valentin wrote:
> Hello Bartlomiej,

Hi,

> On Mon, May 05, 2014 at 01:15:34PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Remove runtime checks for pdata sanity from exynos_tmu_initialize().
> > The current values hardcoded in pdata will never trigger the checks
> > and for the new code potential mistakes should be caught during
> > development/review phases.
> > 
> > There should be no functional changes caused by this patch.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_thermal_common.h |  1 -
> >  drivers/thermal/samsung/exynos_tmu.c            | 13 -------------
> >  2 files changed, 14 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
> > index 3eb2ed9..cd44719 100644
> > --- a/drivers/thermal/samsung/exynos_thermal_common.h
> > +++ b/drivers/thermal/samsung/exynos_thermal_common.h
> > @@ -27,7 +27,6 @@
> >  #define SENSOR_NAME_LEN	16
> >  #define MAX_TRIP_COUNT	8
> >  #define MAX_COOLING_DEVICE 4
> > -#define MAX_THRESHOLD_LEVS 5
> >  
> >  #define ACTIVE_INTERVAL 500
> >  #define IDLE_INTERVAL 10000
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 903566f..789d745 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -158,23 +158,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  	    data->temp_error > pdata->max_efuse_value)
> >  		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
> >  
> > -	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
> > -		dev_err(&pdev->dev, "Invalid max trigger level\n");
> > -		ret = -EINVAL;
> > -		goto out;
> > -	}
> > -
> >  	for (i = 0; i < pdata->max_trigger_level; i++) {
> >  		if (!pdata->trigger_levels[i])
> >  			continue;
> >  
> > -		if ((pdata->trigger_type[i] == HW_TRIP) &&
> > -		(!pdata->trigger_levels[pdata->max_trigger_level - 1])) {
> > -			dev_err(&pdev->dev, "Invalid hw trigger level\n");
> > -			ret = -EINVAL;
> > -			goto out;
> > -		}
> > -
> 
> Does it mean no new pdata are going to be written? i.e., no new soc is
> going to be supported by this driver that needs proper pdata checking?

This is not a proper checking.  The checks in question are done at runtime
in a production code for data that is hardcoded inside driver during
development time and later it doesn't change.  Such data should be verified
during development and review time (i.e. by a script parsing relevant data
from exynos_tmu_data.c, one can also argue that verification to be done is
so simple that the review by a maintainer should be enough).

> >  		/* Count trigger levels except the HW trip*/
> >  		if (!(pdata->trigger_type[i] == HW_TRIP))
> >  			trigger_levs++;

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()
  2014-05-15 14:55   ` Eduardo Valentin
@ 2014-05-15 16:56     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-15 16:56 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On Thursday, May 15, 2014 10:55:52 AM Eduardo Valentin wrote:
> Hello Bartlomiej,

Hi,

> On Mon, May 05, 2014 at 01:15:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Remove runtime checks for negative return values of temp_to_code()
> > from exynos_tmu_initialize().  The current level temperature data
> > hardcoded in pdata will never cause a negative temp_to_code()
> > return values and for the new code potential mistakes should be
> > caught during development/review phases.
> > 
> > Theres should be no functional changes caused by this patch.
> > 
> 
> Same question as in previous patch. Removing defensive code must

Simirarly like in a previous case.  Such verification should not be done
at runtime in a production code because it is already too late for such
checking.  It should be done during development and review phases.

> be done carefully. 

BTW In case of temp_to_code() its users should be audited to pass only
input values that give positive results and later the function itself
may be modified to catch wrong input values by using WARN_ON (or even
BUG_ON).

> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
> >  1 file changed, 1 insertion(+), 15 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 789d745..a415829 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  	if (data->soc == SOC_ARCH_EXYNOS4210) {
> >  		/* Write temperature code for threshold */
> >  		threshold_code = temp_to_code(data, pdata->threshold);
> > -		if (threshold_code < 0) {
> > -			ret = threshold_code;
> > -			goto out;
> > -		}
> >  		writeb(threshold_code,
> >  			data->base + reg->threshold_temp);
> >  		for (i = 0; i < trigger_levs; i++)
> > @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
> >  			threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i]);
> > -			if (threshold_code < 0) {
> > -				ret = threshold_code;
> > -				goto out;
> > -			}
> >  			rising_threshold |= threshold_code << 8 * i;
> >  			if (pdata->threshold_falling) {
> >  				threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i] -
> >  						pdata->threshold_falling);
> > -				if (threshold_code > 0)
> > -					falling_threshold |=
> > -						threshold_code << 8 * i;
> > +				falling_threshold |= threshold_code << 8 * i;
> >  			}
> >  		}
> >  
> > @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  				(pdata->trigger_type[i] == HW_TRIP)) {
> >  			threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i]);
> > -			if (threshold_code < 0) {
> > -				ret = threshold_code;
> > -				goto out;
> > -			}
> >  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
> >  				/* 1-4 level to be assigned in th0 reg */
> >  				rising_threshold |= threshold_code << 8 * i;

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control()
  2014-05-15 15:03   ` Eduardo Valentin
@ 2014-05-15 17:06     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-15 17:06 UTC (permalink / raw)
  To: Eduardo Valentin
  Cc: Eduardo Valentin, Zhang Rui, Amit Daniel Kachhap, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On Thursday, May 15, 2014 11:03:08 AM Eduardo Valentin wrote:
> Hello Bartlomiej,

Hi,

> On Mon, May 05, 2014 at 01:15:38PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > pdata->reference_voltage and pdata->gain are always defined
> > to non-zero values so remove the redundant checks from
> > exynos_tmu_control().
> > 
> 
> In all existing SoCs? 

Yes.

> Should this be considered to any upcoming SoCs?

Yes, if/when it changes we will update the code.

> > 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, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index a8d9524..45d7c6f 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -215,15 +215,11 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
> >  	if (pdata->test_mux)
> >  		con |= (pdata->test_mux << reg->test_mux_addr_shift);
> >  
> > -	if (pdata->reference_voltage) {
> > -		con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> > -		con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
> > -	}
> > +	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> > +	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
> >  
> > -	if (pdata->gain) {
> > -		con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> > -		con |= (pdata->gain << reg->buf_slope_sel_shift);
> > -	}
> > +	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> > +	con |= (pdata->gain << reg->buf_slope_sel_shift);
> 
> The way it is put, looks like gain and reference_voltage are mandatory
> pdata fields. Should these checks be moved to initialization phase?

Yes, they should be considered mandatory pdata fields but I'm against
adding any superfluous pdata checks for initialization phase (I will
update struct exynos_tmu_platform_data documentation to indicate that
these fields cannot be zero instead).

> >  	if (pdata->noise_cancel_mode) {
> >  		con &= ~(reg->therm_trip_mode_mask <<

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries
  2014-05-05 11:15 ` [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries Bartlomiej Zolnierkiewicz
@ 2014-05-19  5:12   ` Amit Kachhap
  0 siblings, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:12 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

Hi Bartlomiej,

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.h      | 40
> -------------------------------
>  drivers/thermal/samsung/exynos_tmu_data.c |  2 --
>  drivers/thermal/samsung/exynos_tmu_data.h |  1 -
>  3 files changed, 43 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.h
> b/drivers/thermal/samsung/exynos_tmu.h
> index 3fb6554..80dc899 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -82,8 +82,6 @@ enum soc_type {
>   * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data
> reg.
>   * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data
> reg.
>   * @triminfo_ctrl: trim info controller register.
> - * @triminfo_reload_shift: shift of triminfo reload enable bit in
> triminfo_ctrl
> -	reg.
>   * @tmu_ctrl: TMU main controller register.
>   * @test_mux_addr_shift: shift bits of test mux address.
>   * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl
> register.
> @@ -98,27 +96,13 @@ enum soc_type {
>  	register.
>   * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
>  	register.
> - * @therm_trip_tq_en_shift: shift bits of thermal trip enable by TQ pin in
> -	tmu_ctrl register.
>   * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
>   * @tmu_status: register drescribing the TMU status.
>   * @tmu_cur_temp: register containing the current temperature of the TMU.
> - * @tmu_cur_temp_shift: shift bits of current temp value in tmu_cur_temp
> -	register.
>   * @threshold_temp: register containing the base threshold level.
>   * @threshold_th0: Register containing first set of rising levels.
> - * @threshold_th0_l0_shift: shift bits of level0 threshold temperature.
> - * @threshold_th0_l1_shift: shift bits of level1 threshold temperature.
> - * @threshold_th0_l2_shift: shift bits of level2 threshold temperature.
> - * @threshold_th0_l3_shift: shift bits of level3 threshold temperature.
>   * @threshold_th1: Register containing second set of rising levels.
> - * @threshold_th1_l0_shift: shift bits of level0 threshold temperature.
> - * @threshold_th1_l1_shift: shift bits of level1 threshold temperature.
> - * @threshold_th1_l2_shift: shift bits of level2 threshold temperature.
> - * @threshold_th1_l3_shift: shift bits of level3 threshold temperature.
>   * @threshold_th2: Register containing third set of rising levels.
> - * @threshold_th2_l0_shift: shift bits of level0 threshold temperature.
> - * @threshold_th3: Register containing fourth set of rising levels.
>   * @threshold_th3_l0_shift: shift bits of level0 threshold temperature.
>   * @tmu_inten: register containing the different threshold interrupt
>  	enable bits.
> @@ -131,15 +115,11 @@ enum soc_type {
>   * @inten_rise2_shift: shift bits of rising 2 interrupt bits.
>   * @inten_rise3_shift: shift bits of rising 3 interrupt bits.
>   * @inten_fall0_shift: shift bits of falling 0 interrupt bits.
> - * @inten_fall1_shift: shift bits of falling 1 interrupt bits.
> - * @inten_fall2_shift: shift bits of falling 2 interrupt bits.
> - * @inten_fall3_shift: shift bits of falling 3 interrupt bits.
>   * @tmu_intstat: Register containing the interrupt status values.
>   * @tmu_intclear: Register for clearing the raised interrupt status.
>   * @emul_con: TMU emulation controller register.
>   * @emul_temp_shift: shift bits of emulation temperature.
>   * @emul_time_shift: shift bits of emulation time.
> - * @emul_time_mask: mask bits of emulation time.
>   * @tmu_irqstatus: register to find which TMU generated interrupts.
>   * @tmu_pmin: register to get/set the Pmin value.

I prefer to have these register description as they describe the h/w
and are used for capturing those parameters. My argument here is that
adding new soc should have minimum changes. However some unused macros
removed below makes sense.
>   */
> @@ -149,7 +129,6 @@ struct exynos_tmu_registers {
>  	u32	triminfo_85_shift;
>
>  	u32	triminfo_ctrl;
> -	u32	triminfo_reload_shift;
>
>  	u32	tmu_ctrl;
>  	u32     test_mux_addr_shift;
> @@ -162,32 +141,17 @@ struct exynos_tmu_registers {
>  	u32	buf_slope_sel_mask;
>  	u32	calib_mode_shift;
>  	u32	calib_mode_mask;
> -	u32	therm_trip_tq_en_shift;
>  	u32	core_en_shift;
>
>  	u32	tmu_status;
>
>  	u32	tmu_cur_temp;
> -	u32	tmu_cur_temp_shift;
>
>  	u32	threshold_temp;
>
>  	u32	threshold_th0;
> -	u32	threshold_th0_l0_shift;
> -	u32	threshold_th0_l1_shift;
> -	u32	threshold_th0_l2_shift;
> -	u32	threshold_th0_l3_shift;
> -
>  	u32	threshold_th1;
> -	u32	threshold_th1_l0_shift;
> -	u32	threshold_th1_l1_shift;
> -	u32	threshold_th1_l2_shift;
> -	u32	threshold_th1_l3_shift;
> -
>  	u32	threshold_th2;
> -	u32	threshold_th2_l0_shift;
> -
> -	u32	threshold_th3;
>  	u32	threshold_th3_l0_shift;
>
>  	u32	tmu_inten;
> @@ -200,9 +164,6 @@ struct exynos_tmu_registers {
>  	u32	inten_rise2_shift;
>  	u32	inten_rise3_shift;
>  	u32	inten_fall0_shift;
> -	u32	inten_fall1_shift;
> -	u32	inten_fall2_shift;
> -	u32	inten_fall3_shift;
>
>  	u32	tmu_intstat;
>
> @@ -211,7 +172,6 @@ struct exynos_tmu_registers {
>  	u32	emul_con;
>  	u32	emul_temp_shift;
>  	u32	emul_time_shift;
> -	u32	emul_time_mask;
>
>  	u32	tmu_irqstatus;
>  	u32	tmu_pmin;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index 476b768..36d64d6 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -96,7 +96,6 @@ static const struct exynos_tmu_registers
> exynos4412_tmu_registers = {
>  	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
>  	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
> -	.triminfo_reload_shift = EXYNOS_TRIMINFO_RELOAD_SHIFT,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
>  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> @@ -126,7 +125,6 @@ static const struct exynos_tmu_registers
> exynos4412_tmu_registers = {
>  	.emul_con = EXYNOS_EMUL_CON,
>  	.emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>  	.emul_time_shift = EXYNOS_EMUL_TIME_SHIFT,
> -	.emul_time_mask = EXYNOS_EMUL_TIME_MASK,
Makes sense.
>  };
>
>  #define EXYNOS4412_TMU_DATA \
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h
> b/drivers/thermal/samsung/exynos_tmu_data.h
> index a1ea19d..06c4345 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -63,7 +63,6 @@
>  #define EXYNOS_THD_TEMP_FALL		0x54
>  #define EXYNOS_EMUL_CON		0x80
>
> -#define EXYNOS_TRIMINFO_RELOAD_SHIFT	1
make sense.

>  #define EXYNOS_TRIMINFO_25_SHIFT	0
>  #define EXYNOS_TRIMINFO_85_SHIFT	8
>  #define EXYNOS_TMU_RISE_INT_MASK	0x111
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/10] thermal: exynos: remove unused defines
  2014-05-05 11:15 ` [PATCH 02/10] thermal: exynos: remove unused defines Bartlomiej Zolnierkiewicz
  2014-05-15 14:07   ` Eduardo Valentin
@ 2014-05-19  5:17   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:17 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

Hi Bartlomiej,

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu_data.h | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h
> b/drivers/thermal/samsung/exynos_tmu_data.h
> index 06c4345..d4eeddb 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -42,20 +42,8 @@
>  /* Exynos4210 specific registers */
>  #define EXYNOS4210_TMU_REG_THRESHOLD_TEMP	0x44
>  #define EXYNOS4210_TMU_REG_TRIG_LEVEL0	0x50
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL1	0x54
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL2	0x58
> -#define EXYNOS4210_TMU_REG_TRIG_LEVEL3	0x5C
> -#define EXYNOS4210_TMU_REG_PAST_TEMP0	0x60
> -#define EXYNOS4210_TMU_REG_PAST_TEMP1	0x64
> -#define EXYNOS4210_TMU_REG_PAST_TEMP2	0x68
> -#define EXYNOS4210_TMU_REG_PAST_TEMP3	0x6C
> -
This removal looks fine as 4210 is an old soc.
> -#define EXYNOS4210_TMU_TRIG_LEVEL0_MASK	0x1
> -#define EXYNOS4210_TMU_TRIG_LEVEL1_MASK	0x10
> -#define EXYNOS4210_TMU_TRIG_LEVEL2_MASK	0x100
> -#define EXYNOS4210_TMU_TRIG_LEVEL3_MASK	0x1000
> +
>  #define EXYNOS4210_TMU_TRIG_LEVEL_MASK	0x1111
> -#define EXYNOS4210_TMU_INTCLEAR_VAL	0x1111
>
>  /* Exynos5250 and Exynos4412 specific registers */
>  #define EXYNOS_TMU_TRIMINFO_CON	0x14
> @@ -69,8 +57,6 @@
>  #define EXYNOS_TMU_RISE_INT_SHIFT	0
>  #define EXYNOS_TMU_FALL_INT_MASK	0x111
>  #define EXYNOS_TMU_FALL_INT_SHIFT	12
> -#define EXYNOS_TMU_CLEAR_RISE_INT	0x111
> -#define EXYNOS_TMU_CLEAR_FALL_INT	(0x111 << 12)
>  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
>  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
>  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> @@ -82,8 +68,6 @@
>  #define EXYNOS_TMU_INTEN_RISE2_SHIFT	8
>  #define EXYNOS_TMU_INTEN_RISE3_SHIFT	12
>  #define EXYNOS_TMU_INTEN_FALL0_SHIFT	16
> -#define EXYNOS_TMU_INTEN_FALL1_SHIFT	20
> -#define EXYNOS_TMU_INTEN_FALL2_SHIFT	24
I suggest to retain this generic soc macros as they might be used in future.
>
>  #define EXYNOS_EMUL_TIME	0x57F0
>  #define EXYNOS_EMUL_TIME_MASK	0xffff
> @@ -107,13 +91,11 @@
>  #define EXYNOS5440_TMU_S0_7_TH0			0x110
>  #define EXYNOS5440_TMU_S0_7_TH1			0x130
>  #define EXYNOS5440_TMU_S0_7_TH2			0x150
> -#define EXYNOS5440_TMU_S0_7_EVTEN		0x1F0
>  #define EXYNOS5440_TMU_S0_7_IRQEN		0x210
>  #define EXYNOS5440_TMU_S0_7_IRQ			0x230
>  /* exynos5440 common registers */
>  #define EXYNOS5440_TMU_IRQ_STATUS		0x000
>  #define EXYNOS5440_TMU_PMIN			0x004
> -#define EXYNOS5440_TMU_TEMP			0x008
>
>  #define EXYNOS5440_TMU_RISE_INT_MASK		0xf
>  #define EXYNOS5440_TMU_RISE_INT_SHIFT		0
> @@ -124,13 +106,6 @@
>  #define EXYNOS5440_TMU_INTEN_RISE2_SHIFT	2
>  #define EXYNOS5440_TMU_INTEN_RISE3_SHIFT	3
>  #define EXYNOS5440_TMU_INTEN_FALL0_SHIFT	4
> -#define EXYNOS5440_TMU_INTEN_FALL1_SHIFT	5
> -#define EXYNOS5440_TMU_INTEN_FALL2_SHIFT	6
> -#define EXYNOS5440_TMU_INTEN_FALL3_SHIFT	7
> -#define EXYNOS5440_TMU_TH_RISE0_SHIFT		0
> -#define EXYNOS5440_TMU_TH_RISE1_SHIFT		8
> -#define EXYNOS5440_TMU_TH_RISE2_SHIFT		16
> -#define EXYNOS5440_TMU_TH_RISE3_SHIFT		24
5440 is an old soc so can be removed.
>  #define EXYNOS5440_TMU_TH_RISE4_SHIFT		24
>  #define EXYNOS5440_EFUSE_SWAP_OFFSET		8
>
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration
  2014-05-05 11:15 ` [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration Bartlomiej Zolnierkiewicz
  2014-05-15 14:14   ` Eduardo Valentin
@ 2014-05-19  5:27   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:27 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

Hi Bartlomiej,

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> 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      | 33
> +------------------------------
>  drivers/thermal/samsung/exynos_tmu.h      | 13 ------------
>  drivers/thermal/samsung/exynos_tmu_data.c |  3 ---
>  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
>  4 files changed, 1 insertion(+), 50 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 0d96a51..9f2a5ee 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -76,9 +76,6 @@ static int temp_to_code(struct exynos_tmu_data *data, u8
> temp)
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	int temp_code;
>
> -	if (pdata->cal_mode == HW_MODE)
> -		return temp;
> -
I suggest to retain the hw mode generic feature as it is provided by
the TMU controller.  However below unused defines for 5440 might be
removed as the h/w mode relevant values were wrongly fused.
>  	if (data->soc == SOC_ARCH_EXYNOS4210)
>  		/* temp should range between 25 and 125 */
>  		if (temp < 25 || temp > 125) {
> @@ -113,9 +110,6 @@ static int code_to_temp(struct exynos_tmu_data *data, u8
> temp_code)
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	int temp;
>
> -	if (pdata->cal_mode == HW_MODE)
> -		return temp_code;
> -
>  	if (data->soc == SOC_ARCH_EXYNOS4210)
>  		/* temp_code should range between 75 and 175 */
>  		if (temp_code < 75 || temp_code > 175) {
> @@ -164,9 +158,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  	if (TMU_SUPPORTS(pdata, TRIM_RELOAD))
>  		__raw_writel(1, data->base + reg->triminfo_ctrl);
>
> -	if (pdata->cal_mode == HW_MODE)
> -		goto skip_calib_data;
> -
>  	/* Save trimming info in order to perform calibration */
>  	if (data->soc == SOC_ARCH_EXYNOS5440) {
>  		/*
> @@ -202,7 +193,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  			(pdata->efuse_value >> reg->triminfo_85_shift) &
>  			EXYNOS_TMU_TEMP_MASK;
>
> -skip_calib_data:
>  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
>  		dev_err(&pdev->dev, "Invalid max trigger level\n");
>  		ret = -EINVAL;
> @@ -311,7 +301,7 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	const struct exynos_tmu_registers *reg = pdata->registers;
> -	unsigned int con, interrupt_en, cal_val;
> +	unsigned int con, interrupt_en;
>
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
> @@ -337,27 +327,6 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  		con |= (pdata->noise_cancel_mode << reg->therm_trip_mode_shift);
>  	}
>
> -	if (pdata->cal_mode == HW_MODE) {
> -		con &= ~(reg->calib_mode_mask << reg->calib_mode_shift);
> -		cal_val = 0;
> -		switch (pdata->cal_type) {
> -		case TYPE_TWO_POINT_TRIMMING:
> -			cal_val = 3;
> -			break;
> -		case TYPE_ONE_POINT_TRIMMING_85:
> -			cal_val = 2;
> -			break;
> -		case TYPE_ONE_POINT_TRIMMING_25:
> -			cal_val = 1;
> -			break;
> -		case TYPE_NONE:
> -			break;
> -		default:
> -			dev_err(&pdev->dev, "Invalid calibration type, using none\n");
> -		}
> -		con |= cal_val << reg->calib_mode_shift;
> -	}
> -
>  	if (on) {
>  		con |= (1 << reg->core_en_shift);
>  		interrupt_en =
> diff --git a/drivers/thermal/samsung/exynos_tmu.h
> b/drivers/thermal/samsung/exynos_tmu.h
> index 80dc899..e417af8 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -34,11 +34,6 @@ enum calibration_type {
>  	TYPE_NONE,
>  };
>
> -enum calibration_mode {
> -	SW_MODE,
> -	HW_MODE,
> -};
> -
>  enum soc_type {
>  	SOC_ARCH_EXYNOS4210 = 1,
>  	SOC_ARCH_EXYNOS4412,
> @@ -92,10 +87,6 @@ enum soc_type {
>   * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
>  	register.
>   * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl
> register.
> - * @calib_mode_shift: shift bits of calibration mode value in tmu_ctrl
> -	register.
> - * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
> -	register.
>   * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
>   * @tmu_status: register drescribing the TMU status.
>   * @tmu_cur_temp: register containing the current temperature of the TMU.
> @@ -139,8 +130,6 @@ struct exynos_tmu_registers {
>  	u32	therm_trip_en_shift;
>  	u32	buf_slope_sel_shift;
>  	u32	buf_slope_sel_mask;
> -	u32	calib_mode_shift;
> -	u32	calib_mode_mask;
>  	u32	core_en_shift;
>
>  	u32	tmu_status;
> @@ -222,7 +211,6 @@ struct exynos_tmu_registers {
>   * @default_temp_offset: default temperature offset in case of no trimming
>   * @test_mux; information if SoC supports test MUX
>   * @cal_type: calibration type for temperature
> - * @cal_mode: calibration mode for temperature
>   * @freq_clip_table: Table representing frequency reduction percentage.
>   * @freq_tab_count: Count of the above table as frequency reduction may
>   *	applicable to only some of the trigger levels.
> @@ -253,7 +241,6 @@ struct exynos_tmu_platform_data {
>  	u8 test_mux;
>
>  	enum calibration_type cal_type;
> -	enum calibration_mode cal_mode;
>  	enum soc_type type;
>  	struct freq_clip_table freq_tab[4];
>  	unsigned int freq_tab_count;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index 36d64d6..4b992d9 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -205,8 +205,6 @@ static const struct exynos_tmu_registers
> exynos5440_tmu_registers = {
>  	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
>  	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
>  	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> -	.calib_mode_shift = EXYNOS_TMU_CALIB_MODE_SHIFT,
> -	.calib_mode_mask = EXYNOS_TMU_CALIB_MODE_MASK,
>  	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
>  	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
>  	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
> @@ -243,7 +241,6 @@ static const struct exynos_tmu_registers
> exynos5440_tmu_registers = {
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
>  	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> -	.cal_mode = 0, \
>  	.efuse_value = 0x5b2d, \
>  	.min_efuse_value = 16, \
>  	.max_efuse_value = 76, \
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h
> b/drivers/thermal/samsung/exynos_tmu_data.h
> index d4eeddb..1fed00d 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -60,8 +60,6 @@
>  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
>  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
>  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> -#define EXYNOS_TMU_CALIB_MODE_SHIFT	4
> -#define EXYNOS_TMU_CALIB_MODE_MASK	0x3
>
>  #define EXYNOS_TMU_INTEN_RISE0_SHIFT	0
>  #define EXYNOS_TMU_INTEN_RISE1_SHIFT	4
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration
  2014-05-05 11:15 ` [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration Bartlomiej Zolnierkiewicz
  2014-05-15 14:31   ` Eduardo Valentin
@ 2014-05-19  5:40   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:40 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> Only TYPE_ONE_POINT_TRIMMING calibration is used so remove
> the dead code for TYPE_TWO_POINT_TRIMMING calibration.
I prefer to retain this feature as it is provided by the TMU
controller. This will avoid unnecessary churning of code when some new
soc wants to use it. 2 point trimming should ideally give best
compensated thermal values.
>
> 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      | 55
> ++++++-------------------------
>  drivers/thermal/samsung/exynos_tmu.h      | 20 +----------
>  drivers/thermal/samsung/exynos_tmu_data.c | 17 +---------
>  drivers/thermal/samsung/exynos_tmu_data.h |  2 --
>  4 files changed, 12 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 9f2a5ee..903566f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -47,8 +47,7 @@
>   * @irq_work: pointer to the irq work structure.
>   * @lock: lock to implement synchronization.
>   * @clk: pointer to the clock structure.
> - * @temp_error1: fused value of the first point trim.
> - * @temp_error2: fused value of the second point trim.
> + * @temp_error: fused value of the first point trim.
>   * @regulator: pointer to the TMU regulator structure.
>   * @reg_conf: pointer to structure to register with core thermal.
>   */
> @@ -62,14 +61,13 @@ struct exynos_tmu_data {
>  	struct work_struct irq_work;
>  	struct mutex lock;
>  	struct clk *clk;
> -	u8 temp_error1, temp_error2;
> +	u8 temp_error;
>  	struct regulator *regulator;
>  	struct thermal_sensor_conf *reg_conf;
>  };
>
>  /*
>   * TMU treats temperature as a mapped temperature code.
> - * The temperature is converted differently depending on the calibration
> type.
>   */
>  static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>  {
> @@ -83,20 +81,7 @@ static int temp_to_code(struct exynos_tmu_data *data, u8
> temp)
>  			goto out;
>  		}
>
> -	switch (pdata->cal_type) {
> -	case TYPE_TWO_POINT_TRIMMING:
> -		temp_code = (temp - pdata->first_point_trim) *
> -			(data->temp_error2 - data->temp_error1) /
> -			(pdata->second_point_trim - pdata->first_point_trim) +
> -			data->temp_error1;
> -		break;
> -	case TYPE_ONE_POINT_TRIMMING:
> -		temp_code = temp + data->temp_error1 - pdata->first_point_trim;
> -		break;
> -	default:
> -		temp_code = temp + pdata->default_temp_offset;
> -		break;
> -	}
> +	temp_code = temp + data->temp_error - pdata->first_point_trim;
>  out:
>  	return temp_code;
>  }
> @@ -117,20 +102,7 @@ static int code_to_temp(struct exynos_tmu_data *data,
> u8 temp_code)
>  			goto out;
>  		}
>
> -	switch (pdata->cal_type) {
> -	case TYPE_TWO_POINT_TRIMMING:
> -		temp = (temp_code - data->temp_error1) *
> -			(pdata->second_point_trim - pdata->first_point_trim) /
> -			(data->temp_error2 - data->temp_error1) +
> -			pdata->first_point_trim;
> -		break;
> -	case TYPE_ONE_POINT_TRIMMING:
> -		temp = temp_code - data->temp_error1 + pdata->first_point_trim;
> -		break;
> -	default:
> -		temp = temp_code - pdata->default_temp_offset;
> -		break;
> -	}
> +	temp = temp_code - data->temp_error + pdata->first_point_trim;
>  out:
>  	return temp;
>  }
> @@ -179,19 +151,12 @@ static int exynos_tmu_initialize(struct
> platform_device *pdev)
>  	} else {
>  		trim_info = readl(data->base + reg->triminfo_data);
>  	}
> -	data->temp_error1 = trim_info & EXYNOS_TMU_TEMP_MASK;
> -	data->temp_error2 = ((trim_info >> reg->triminfo_85_shift) &
> -				EXYNOS_TMU_TEMP_MASK);
> -
> -	if (!data->temp_error1 ||
> -		(pdata->min_efuse_value > data->temp_error1) ||
> -		(data->temp_error1 > pdata->max_efuse_value))
> -		data->temp_error1 = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
> -
> -	if (!data->temp_error2)
> -		data->temp_error2 =
> -			(pdata->efuse_value >> reg->triminfo_85_shift) &
> -			EXYNOS_TMU_TEMP_MASK;
> +	data->temp_error = trim_info & EXYNOS_TMU_TEMP_MASK;
> +
> +	if (!data->temp_error ||
> +	    pdata->min_efuse_value > data->temp_error ||
> +	    data->temp_error > pdata->max_efuse_value)
> +		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
>
>  	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
>  		dev_err(&pdev->dev, "Invalid max trigger level\n");
> diff --git a/drivers/thermal/samsung/exynos_tmu.h
> b/drivers/thermal/samsung/exynos_tmu.h
> index e417af8..186e39e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -26,14 +26,6 @@
>
>  #include "exynos_thermal_common.h"
>
> -enum calibration_type {
> -	TYPE_ONE_POINT_TRIMMING,
> -	TYPE_ONE_POINT_TRIMMING_25,
> -	TYPE_ONE_POINT_TRIMMING_85,
> -	TYPE_TWO_POINT_TRIMMING,
> -	TYPE_NONE,
> -};
> -
>  enum soc_type {
>  	SOC_ARCH_EXYNOS4210 = 1,
>  	SOC_ARCH_EXYNOS4412,
> @@ -74,8 +66,6 @@ enum soc_type {
>   * bitfields. The register validity, offsets and bitfield values may vary
>   * slightly across different exynos SOC's.
>   * @triminfo_data: register containing 2 pont trimming data
> - * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data
> reg.
> - * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data
> reg.
>   * @triminfo_ctrl: trim info controller register.
>   * @tmu_ctrl: TMU main controller register.
>   * @test_mux_addr_shift: shift bits of test mux address.
> @@ -116,8 +106,6 @@ enum soc_type {
>   */
>  struct exynos_tmu_registers {
>  	u32	triminfo_data;
> -	u32	triminfo_25_shift;
> -	u32	triminfo_85_shift;
>
>  	u32	triminfo_ctrl;
>
> @@ -207,10 +195,7 @@ struct exynos_tmu_registers {
>   * @min_efuse_value: minimum valid trimming data
>   * @max_efuse_value: maximum valid trimming data
>   * @first_point_trim: temp value of the first point trimming
> - * @second_point_trim: temp value of the second point trimming
> - * @default_temp_offset: default temperature offset in case of no trimming
>   * @test_mux; information if SoC supports test MUX
> - * @cal_type: calibration type for temperature
>   * @freq_clip_table: Table representing frequency reduction percentage.
>   * @freq_tab_count: Count of the above table as frequency reduction may
>   *	applicable to only some of the trigger levels.
> @@ -232,15 +217,12 @@ struct exynos_tmu_platform_data {
>  	u8 reference_voltage;
>  	u8 noise_cancel_mode;
>
> -	u32 efuse_value;
> +	u8 efuse_value;
>  	u32 min_efuse_value;
>  	u32 max_efuse_value;
>  	u8 first_point_trim;
> -	u8 second_point_trim;
> -	u8 default_temp_offset;
>  	u8 test_mux;
>
> -	enum calibration_type cal_type;
>  	enum soc_type type;
>  	struct freq_clip_table freq_tab[4];
>  	unsigned int freq_tab_count;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index 4b992d9..c32d186 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -27,8 +27,6 @@
>  #if defined(CONFIG_CPU_EXYNOS4210)
>  static const struct exynos_tmu_registers exynos4210_tmu_registers = {
>  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
>  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> @@ -66,12 +64,9 @@ struct exynos_tmu_init_data const
> exynos4210_default_tmu_data = {
>  		.max_trigger_level = 4,
>  		.gain = 15,
>  		.reference_voltage = 7,
> -		.cal_type = TYPE_ONE_POINT_TRIMMING,
>  		.min_efuse_value = 40,
>  		.max_efuse_value = 100,
>  		.first_point_trim = 25,
> -		.second_point_trim = 85,
> -		.default_temp_offset = 50,
>  		.freq_tab[0] = {
>  			.freq_clip_max = 800 * 1000,
>  			.temp_level = 85,
> @@ -93,8 +88,6 @@ struct exynos_tmu_init_data const
> exynos4210_default_tmu_data = {
>  #if defined(CONFIG_SOC_EXYNOS4412) || defined(CONFIG_SOC_EXYNOS5250)
>  static const struct exynos_tmu_registers exynos4412_tmu_registers = {
>  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
> @@ -145,13 +138,10 @@ static const struct exynos_tmu_registers
> exynos4412_tmu_registers = {
>  	.gain = 8, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
>  	.efuse_value = 55, \
>  	.min_efuse_value = 40, \
>  	.max_efuse_value = 100, \
>  	.first_point_trim = 25, \
> -	.second_point_trim = 85, \
> -	.default_temp_offset = 50, \
>  	.freq_tab[0] = { \
>  		.freq_clip_max = 1400 * 1000, \
>  		.temp_level = 70, \
> @@ -195,8 +185,6 @@ struct exynos_tmu_init_data const
> exynos5250_default_tmu_data = {
>  #if defined(CONFIG_SOC_EXYNOS5440)
>  static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
> -	.triminfo_25_shift = EXYNOS_TRIMINFO_25_SHIFT,
> -	.triminfo_85_shift = EXYNOS_TRIMINFO_85_SHIFT,
>  	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
>  	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
>  	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> @@ -240,13 +228,10 @@ static const struct exynos_tmu_registers
> exynos5440_tmu_registers = {
>  	.gain = 5, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> -	.cal_type = TYPE_ONE_POINT_TRIMMING, \
> -	.efuse_value = 0x5b2d, \
> +	.efuse_value = 45, \
>  	.min_efuse_value = 16, \
>  	.max_efuse_value = 76, \
>  	.first_point_trim = 25, \
> -	.second_point_trim = 70, \
> -	.default_temp_offset = 25, \
>  	.type = SOC_ARCH_EXYNOS5440, \
>  	.registers = &exynos5440_tmu_registers, \
>  	.features = (TMU_SUPPORT_EMULATION | TMU_SUPPORT_FALLING_TRIP | \
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h
> b/drivers/thermal/samsung/exynos_tmu_data.h
> index 1fed00d..734d1f9 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.h
> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> @@ -51,8 +51,6 @@
>  #define EXYNOS_THD_TEMP_FALL		0x54
>  #define EXYNOS_EMUL_CON		0x80
>
> -#define EXYNOS_TRIMINFO_25_SHIFT	0
> -#define EXYNOS_TRIMINFO_85_SHIFT	8
>  #define EXYNOS_TMU_RISE_INT_MASK	0x111
>  #define EXYNOS_TMU_RISE_INT_SHIFT	0
>  #define EXYNOS_TMU_FALL_INT_MASK	0x111
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize()
  2014-05-15 16:24     ` Bartlomiej Zolnierkiewicz
@ 2014-05-19  5:47       ` Amit Kachhap
  0 siblings, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:47 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Eduardo Valentin, Zhang Rui, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On 5/15/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> On Thursday, May 15, 2014 10:47:40 AM Eduardo Valentin wrote:
>> Hello Bartlomiej,
>
> Hi,
>
>> On Mon, May 05, 2014 at 01:15:34PM +0200, Bartlomiej Zolnierkiewicz
>> wrote:
>> > Remove runtime checks for pdata sanity from exynos_tmu_initialize().
>> > The current values hardcoded in pdata will never trigger the checks
>> > and for the new code potential mistakes should be caught during
>> > development/review phases.
>> >
>> > There should be no functional changes caused by this patch.
>> >
>> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> > ---
>> >  drivers/thermal/samsung/exynos_thermal_common.h |  1 -
>> >  drivers/thermal/samsung/exynos_tmu.c            | 13 -------------
>> >  2 files changed, 14 deletions(-)
>> >
>> > diff --git a/drivers/thermal/samsung/exynos_thermal_common.h
>> > b/drivers/thermal/samsung/exynos_thermal_common.h
>> > index 3eb2ed9..cd44719 100644
>> > --- a/drivers/thermal/samsung/exynos_thermal_common.h
>> > +++ b/drivers/thermal/samsung/exynos_thermal_common.h
>> > @@ -27,7 +27,6 @@
>> >  #define SENSOR_NAME_LEN	16
>> >  #define MAX_TRIP_COUNT	8
>> >  #define MAX_COOLING_DEVICE 4
>> > -#define MAX_THRESHOLD_LEVS 5
>> >
>> >  #define ACTIVE_INTERVAL 500
>> >  #define IDLE_INTERVAL 10000
>> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
>> > b/drivers/thermal/samsung/exynos_tmu.c
>> > index 903566f..789d745 100644
>> > --- a/drivers/thermal/samsung/exynos_tmu.c
>> > +++ b/drivers/thermal/samsung/exynos_tmu.c
>> > @@ -158,23 +158,10 @@ static int exynos_tmu_initialize(struct
>> > platform_device *pdev)
>> >  	    data->temp_error > pdata->max_efuse_value)
>> >  		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
>> >
>> > -	if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
>> > -		dev_err(&pdev->dev, "Invalid max trigger level\n");
>> > -		ret = -EINVAL;
>> > -		goto out;
>> > -	}
>> > -
>> >  	for (i = 0; i < pdata->max_trigger_level; i++) {
>> >  		if (!pdata->trigger_levels[i])
>> >  			continue;
>> >
>> > -		if ((pdata->trigger_type[i] == HW_TRIP) &&
>> > -		(!pdata->trigger_levels[pdata->max_trigger_level - 1])) {
>> > -			dev_err(&pdev->dev, "Invalid hw trigger level\n");
>> > -			ret = -EINVAL;
>> > -			goto out;
>> > -		}
>> > -
>>
>> Does it mean no new pdata are going to be written? i.e., no new soc is
>> going to be supported by this driver that needs proper pdata checking?
>
> This is not a proper checking.  The checks in question are done at runtime
> in a production code for data that is hardcoded inside driver during
> development time and later it doesn't change.  Such data should be verified
> during development and review time (i.e. by a script parsing relevant data
> from exynos_tmu_data.c, one can also argue that verification to be done is
> so simple that the review by a maintainer should be enough).
Agreed to your arguments. These changes looks fine.
>
>> >  		/* Count trigger levels except the HW trip*/
>> >  		if (!(pdata->trigger_type[i] == HW_TRIP))
>> >  			trigger_levs++;
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()
  2014-05-05 11:15 ` [PATCH 06/10] thermal: exynos: remove redundant threshold_code " Bartlomiej Zolnierkiewicz
  2014-05-15 14:55   ` Eduardo Valentin
@ 2014-05-19  5:50   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:50 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> Remove runtime checks for negative return values of temp_to_code()
> from exynos_tmu_initialize().  The current level temperature data
> hardcoded in pdata will never cause a negative temp_to_code()
> return values and for the new code potential mistakes should be
> caught during development/review phases.
Your arguments looks fine.
>
> Theres should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> ---
>  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 789d745..a415829 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  	if (data->soc == SOC_ARCH_EXYNOS4210) {
>  		/* Write temperature code for threshold */
>  		threshold_code = temp_to_code(data, pdata->threshold);
> -		if (threshold_code < 0) {
> -			ret = threshold_code;
> -			goto out;
> -		}
>  		writeb(threshold_code,
>  			data->base + reg->threshold_temp);
>  		for (i = 0; i < trigger_levs; i++)
> @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct
> platform_device *pdev)
>  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			rising_threshold |= threshold_code << 8 * i;
>  			if (pdata->threshold_falling) {
>  				threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i] -
>  						pdata->threshold_falling);
> -				if (threshold_code > 0)
> -					falling_threshold |=
> -						threshold_code << 8 * i;
> +				falling_threshold |= threshold_code << 8 * i;
>  			}
>  		}
>
> @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  				(pdata->trigger_type[i] == HW_TRIP)) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
>  				/* 1-4 level to be assigned in th0 reg */
>  				rising_threshold |= threshold_code << 8 * i;
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp()
  2014-05-05 11:15 ` [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp() Bartlomiej Zolnierkiewicz
@ 2014-05-19  5:54   ` Amit Kachhap
  0 siblings, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:54 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> * Remove dead temp check from temp_to_code() (this function users
>   in exynos_tmu_initialize() always pass correct temperatures and
>   exynos_tmu_set_emulation() returns early for EXYNOS4210 because
>   TMU_SUPPORT_EMULATION flag is not set on this SoC).
>
> * Move temp_code check from code_to_temp() to exynos_tmu_read()
>   (code_to_temp() only user).
>
> There should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Changes looks fine.
Reviewed-by: Amit Daniel Kachhap<amit.daniel@samsung.com>

> ---
>  drivers/thermal/samsung/exynos_tmu.c | 38
> +++++++++++-------------------------
>  1 file changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index a415829..20379eb 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -71,19 +71,7 @@ struct exynos_tmu_data {
>   */
>  static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>  {
> -	struct exynos_tmu_platform_data *pdata = data->pdata;
> -	int temp_code;
> -
> -	if (data->soc == SOC_ARCH_EXYNOS4210)
> -		/* temp should range between 25 and 125 */
> -		if (temp < 25 || temp > 125) {
> -			temp_code = -EINVAL;
> -			goto out;
> -		}
> -
> -	temp_code = temp + data->temp_error - pdata->first_point_trim;
> -out:
> -	return temp_code;
> +	return temp + data->temp_error - data->pdata->first_point_trim;
>  }
>
>  /*
> @@ -92,19 +80,7 @@ out:
>   */
>  static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
>  {
> -	struct exynos_tmu_platform_data *pdata = data->pdata;
> -	int temp;
> -
> -	if (data->soc == SOC_ARCH_EXYNOS4210)
> -		/* temp_code should range between 75 and 175 */
> -		if (temp_code < 75 || temp_code > 175) {
> -			temp = -ENODATA;
> -			goto out;
> -		}
> -
> -	temp = temp_code - data->temp_error + pdata->first_point_trim;
> -out:
> -	return temp;
> +	return temp_code - data->temp_error + data->pdata->first_point_trim;
>  }
>
>  static int exynos_tmu_initialize(struct platform_device *pdev)
> @@ -297,8 +273,16 @@ static int exynos_tmu_read(struct exynos_tmu_data
> *data)
>  	clk_enable(data->clk);
>
>  	temp_code = readb(data->base + reg->tmu_cur_temp);
> -	temp = code_to_temp(data, temp_code);
>
> +	if (data->soc == SOC_ARCH_EXYNOS4210)
> +		/* temp_code should range between 75 and 175 */
> +		if (temp_code < 75 || temp_code > 175) {
> +			temp = -ENODATA;
> +			goto out;
> +		}
> +
> +	temp = code_to_temp(data, temp_code);
> +out:
>  	clk_disable(data->clk);
>  	mutex_unlock(&data->lock);
>
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata
  2014-05-05 11:15 ` [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata Bartlomiej Zolnierkiewicz
@ 2014-05-19  5:56   ` Amit Kachhap
  0 siblings, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  5:56 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> Cache number of non-hardware trigger levels in a new pdata field
> (non_hw_trigger_levels) and convert code in exynos_tmu_initialize()
> accordingly.
Changes looks fine,
Reviewed-by: Amit Daniel Kachhap<amit.daniel@samsung.com>
>
> 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      | 16 +++-------------
>  drivers/thermal/samsung/exynos_tmu.h      |  2 ++
>  drivers/thermal/samsung/exynos_tmu_data.c |  3 +++
>  3 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 20379eb..a8d9524 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -90,7 +90,7 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  	const struct exynos_tmu_registers *reg = pdata->registers;
>  	unsigned int status, trim_info = 0, con;
>  	unsigned int rising_threshold = 0, falling_threshold = 0;
> -	int ret = 0, threshold_code, i, trigger_levs = 0;
> +	int ret = 0, threshold_code, i;
>
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
> @@ -134,29 +134,19 @@ static int exynos_tmu_initialize(struct
> platform_device *pdev)
>  	    data->temp_error > pdata->max_efuse_value)
>  		data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
>
> -	for (i = 0; i < pdata->max_trigger_level; i++) {
> -		if (!pdata->trigger_levels[i])
> -			continue;
> -
> -		/* Count trigger levels except the HW trip*/
> -		if (!(pdata->trigger_type[i] == HW_TRIP))
> -			trigger_levs++;
> -	}
> -
>  	if (data->soc == SOC_ARCH_EXYNOS4210) {
>  		/* Write temperature code for threshold */
>  		threshold_code = temp_to_code(data, pdata->threshold);
>  		writeb(threshold_code,
>  			data->base + reg->threshold_temp);
> -		for (i = 0; i < trigger_levs; i++)
> +		for (i = 0; i < pdata->non_hw_trigger_levels; i++)
>  			writeb(pdata->trigger_levels[i], data->base +
>  			reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>
>  		writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>  	} else {
>  		/* Write temperature code for rising and falling threshold */
> -		for (i = 0;
> -		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
> +		for (i = 0; i < pdata->non_hw_trigger_levels; i++) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
>  			rising_threshold |= threshold_code << 8 * i;
> diff --git a/drivers/thermal/samsung/exynos_tmu.h
> b/drivers/thermal/samsung/exynos_tmu.h
> index 186e39e..4845171 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -183,6 +183,7 @@ struct exynos_tmu_registers {
>   *	1 = enable trigger_level[] interrupt,
>   *	0 = disable trigger_level[] interrupt
>   * @max_trigger_level: max trigger level supported by the TMU
> + * @non_hw_trigger_levels: number of defined non-hardware trigger levels
>   * @gain: gain of amplifier in the positive-TC generator block
>   *	0 <= gain <= 15
>   * @reference_voltage: reference voltage of amplifier
> @@ -213,6 +214,7 @@ struct exynos_tmu_platform_data {
>  	enum trigger_type trigger_type[MAX_TRIP_COUNT];
>  	bool trigger_enable[MAX_TRIP_COUNT];
>  	u8 max_trigger_level;
> +	u8 non_hw_trigger_levels;
>  	u8 gain;
>  	u8 reference_voltage;
>  	u8 noise_cancel_mode;
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index c32d186..ef7f186 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -62,6 +62,7 @@ struct exynos_tmu_init_data const
> exynos4210_default_tmu_data = {
>  		.trigger_type[1] = THROTTLE_ACTIVE,
>  		.trigger_type[2] = SW_TRIP,
>  		.max_trigger_level = 4,
> +		.non_hw_trigger_levels = 3,
>  		.gain = 15,
>  		.reference_voltage = 7,
>  		.min_efuse_value = 40,
> @@ -135,6 +136,7 @@ static const struct exynos_tmu_registers
> exynos4412_tmu_registers = {
>  	.trigger_type[2] = SW_TRIP, \
>  	.trigger_type[3] = HW_TRIP, \
>  	.max_trigger_level = 4, \
> +	.non_hw_trigger_levels = 3, \
>  	.gain = 8, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> @@ -225,6 +227,7 @@ static const struct exynos_tmu_registers
> exynos5440_tmu_registers = {
>  	.trigger_type[0] = SW_TRIP, \
>  	.trigger_type[4] = HW_TRIP, \
>  	.max_trigger_level = 5, \
> +	.non_hw_trigger_levels = 1, \
>  	.gain = 5, \
>  	.reference_voltage = 16, \
>  	.noise_cancel_mode = 4, \
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control()
  2014-05-05 11:15 ` [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control() Bartlomiej Zolnierkiewicz
  2014-05-15 15:03   ` Eduardo Valentin
@ 2014-05-19  6:05   ` Amit Kachhap
  1 sibling, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  6:05 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

Hi Bartlomiej,
On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> pdata->reference_voltage and pdata->gain are always defined
> to non-zero values so remove the redundant checks from
> exynos_tmu_control().
I prefer to have these checks for the same reason that new soc support
should not add these same code again.
>
> 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, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index a8d9524..45d7c6f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -215,15 +215,11 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  	if (pdata->test_mux)
>  		con |= (pdata->test_mux << reg->test_mux_addr_shift);
>
> -	if (pdata->reference_voltage) {
> -		con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> -		con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
> -	}
> +	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> +	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
>
> -	if (pdata->gain) {
> -		con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> -		con |= (pdata->gain << reg->buf_slope_sel_shift);
> -	}
> +	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> +	con |= (pdata->gain << reg->buf_slope_sel_shift);
>
>  	if (pdata->noise_cancel_mode) {
>  		con &= ~(reg->therm_trip_mode_mask <<
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures
  2014-05-05 11:15 ` [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures Bartlomiej Zolnierkiewicz
@ 2014-05-19  6:11   ` Amit Kachhap
  0 siblings, 0 replies; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  6:11 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Eduardo Valentin, Zhang Rui, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> There is no need for abstracting configuration for registers that
> are identical on all SoC types.
Changes look fine and also that shift and masks may not change in
future socs also.
Reviewed-by: Amit Daniel Kachhap<amit.daniel@samsung.com>
>
> 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 ++++++------
>  drivers/thermal/samsung/exynos_tmu.h      | 11 -----------
>  drivers/thermal/samsung/exynos_tmu_data.c | 15 ---------------
>  3 files changed, 6 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 45d7c6f..d37e755 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -215,11 +215,11 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  	if (pdata->test_mux)
>  		con |= (pdata->test_mux << reg->test_mux_addr_shift);
>
> -	con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
> -	con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
> +	con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK << EXYNOS_TMU_REF_VOLTAGE_SHIFT);
> +	con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT;
>
> -	con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
> -	con |= (pdata->gain << reg->buf_slope_sel_shift);
> +	con &= ~(EXYNOS_TMU_BUF_SLOPE_SEL_MASK <<
> EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
> +	con |= (pdata->gain << EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
>
>  	if (pdata->noise_cancel_mode) {
>  		con &= ~(reg->therm_trip_mode_mask <<
> @@ -228,7 +228,7 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  	}
>
>  	if (on) {
> -		con |= (1 << reg->core_en_shift);
> +		con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
>  		interrupt_en =
>  			pdata->trigger_enable[3] << reg->inten_rise3_shift |
>  			pdata->trigger_enable[2] << reg->inten_rise2_shift |
> @@ -238,7 +238,7 @@ static void exynos_tmu_control(struct platform_device
> *pdev, bool on)
>  			interrupt_en |=
>  				interrupt_en << reg->inten_fall0_shift;
>  	} else {
> -		con &= ~(1 << reg->core_en_shift);
> +		con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
>  		interrupt_en = 0; /* Disable all interrupts */
>  	}
>  	writel(interrupt_en, data->base + reg->tmu_inten);
> diff --git a/drivers/thermal/samsung/exynos_tmu.h
> b/drivers/thermal/samsung/exynos_tmu.h
> index 4845171..5c25a4b 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -69,15 +69,9 @@ enum soc_type {
>   * @triminfo_ctrl: trim info controller register.
>   * @tmu_ctrl: TMU main controller register.
>   * @test_mux_addr_shift: shift bits of test mux address.
> - * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl
> register.
> - * @buf_vref_sel_mask: mask bits of reference voltage in tmu_ctrl
> register.
>   * @therm_trip_mode_shift: shift bits of tripping mode in tmu_ctrl
> register.
>   * @therm_trip_mode_mask: mask bits of tripping mode in tmu_ctrl register.
>   * @therm_trip_en_shift: shift bits of tripping enable in tmu_ctrl
> register.
> - * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
> -	register.
> - * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl
> register.
> - * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
>   * @tmu_status: register drescribing the TMU status.
>   * @tmu_cur_temp: register containing the current temperature of the TMU.
>   * @threshold_temp: register containing the base threshold level.
> @@ -111,14 +105,9 @@ struct exynos_tmu_registers {
>
>  	u32	tmu_ctrl;
>  	u32     test_mux_addr_shift;
> -	u32	buf_vref_sel_shift;
> -	u32	buf_vref_sel_mask;
>  	u32	therm_trip_mode_shift;
>  	u32	therm_trip_mode_mask;
>  	u32	therm_trip_en_shift;
> -	u32	buf_slope_sel_shift;
> -	u32	buf_slope_sel_mask;
> -	u32	core_en_shift;
>
>  	u32	tmu_status;
>
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index ef7f186..32530dc 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -28,11 +28,6 @@
>  static const struct exynos_tmu_registers exynos4210_tmu_registers = {
>  	.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
> -	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> -	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
> -	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
> -	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> -	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
>  	.tmu_status = EXYNOS_TMU_REG_STATUS,
>  	.tmu_cur_temp = EXYNOS_TMU_REG_CURRENT_TEMP,
>  	.threshold_temp = EXYNOS4210_TMU_REG_THRESHOLD_TEMP,
> @@ -92,14 +87,9 @@ static const struct exynos_tmu_registers
> exynos4412_tmu_registers = {
>  	.triminfo_ctrl = EXYNOS_TMU_TRIMINFO_CON,
>  	.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
>  	.test_mux_addr_shift = EXYNOS4412_MUX_ADDR_SHIFT,
> -	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> -	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
>  	.therm_trip_mode_shift = EXYNOS_TMU_TRIP_MODE_SHIFT,
>  	.therm_trip_mode_mask = EXYNOS_TMU_TRIP_MODE_MASK,
>  	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
> -	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
> -	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> -	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
>  	.tmu_status = EXYNOS_TMU_REG_STATUS,
>  	.tmu_cur_temp = EXYNOS_TMU_REG_CURRENT_TEMP,
>  	.threshold_th0 = EXYNOS_THD_TEMP_RISE,
> @@ -188,14 +178,9 @@ struct exynos_tmu_init_data const
> exynos5250_default_tmu_data = {
>  static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>  	.triminfo_data = EXYNOS5440_TMU_S0_7_TRIM,
>  	.tmu_ctrl = EXYNOS5440_TMU_S0_7_CTRL,
> -	.buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
> -	.buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
>  	.therm_trip_mode_shift = EXYNOS_TMU_TRIP_MODE_SHIFT,
>  	.therm_trip_mode_mask = EXYNOS_TMU_TRIP_MODE_MASK,
>  	.therm_trip_en_shift = EXYNOS_TMU_THERM_TRIP_EN_SHIFT,
> -	.buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
> -	.buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
> -	.core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
>  	.tmu_status = EXYNOS5440_TMU_S0_7_STATUS,
>  	.tmu_cur_temp = EXYNOS5440_TMU_S0_7_TEMP,
>  	.threshold_th0 = EXYNOS5440_TMU_S0_7_TH0,
> --
> 1.8.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/10] thermal: exynos: various cleanups
  2014-05-15  9:06 ` [PATCH 00/10] thermal: exynos: various cleanups Zhang Rui
@ 2014-05-19  6:16   ` Amit Kachhap
  2014-05-19 11:05     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 36+ messages in thread
From: Amit Kachhap @ 2014-05-19  6:16 UTC (permalink / raw)
  To: Zhang Rui
  Cc: Bartlomiej Zolnierkiewicz, Eduardo Valentin, Tomasz Figa,
	Rafael J. Wysocki, Kyungmin Park, linux-samsung-soc, linux-pm,
	linux-kernel

On 5/15/14, Zhang Rui <rui.zhang@intel.com> wrote:
> On 一, 2014-05-05 at 13:15 +0200, Bartlomiej Zolnierkiewicz wrote:
>> Hi,
>>
>> This patch series contains various cleanups for EXYNOS thermal
>> driver.  Overall it decreases driver's LOC by 13%.  It is based
>> on next-20140428 kernel.  It should not cause any functionality
>> changes.
>>
> Amit,
>
> what do you think of this patch set?
>
> thanks,
> rui

I agreed to many of the cleanups in the patch but tmu controller
features should be retained as they will allow adding quick soc
support and also avoid unnecessary churning of code in the future.

Thanks,
Amit

>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
>>
>> Bartlomiej Zolnierkiewicz (10):
>>   thermal: exynos: remove unused struct exynos_tmu_registers entries
>>   thermal: exynos: remove unused defines
>>   thermal: exynos: remove dead code for HW_MODE calibration
>>   thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING
>>     calibration
>>   thermal: exynos: remove redundant pdata checks from
>>     exynos_tmu_initialize()
>>   thermal: exynos: remove redundant threshold_code checks from
>>     exynos_tmu_initialize()
>>   thermal: exynos: simplify temp_to_code() and code_to_temp()
>>   thermal: exynos: cache non_hw_trigger_levels in pdata
>>   thermal: exynos: remove redundant pdata checks from
>>     exynos_tmu_control()
>>   thermal: exynos: remove identical values from exynos*_tmu_registers
>>     structures
>>
>>  drivers/thermal/samsung/exynos_thermal_common.h |   1 -
>>  drivers/thermal/samsung/exynos_tmu.c            | 181
>> ++++--------------------
>>  drivers/thermal/samsung/exynos_tmu.h            |  86 +----------
>>  drivers/thermal/samsung/exynos_tmu_data.c       |  40 +-----
>>  drivers/thermal/samsung/exynos_tmu_data.h       |  32 +----
>>  5 files changed, 37 insertions(+), 303 deletions(-)
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/10] thermal: exynos: various cleanups
  2014-05-19  6:16   ` Amit Kachhap
@ 2014-05-19 11:05     ` Bartlomiej Zolnierkiewicz
  2014-05-19 11:09       ` Tomasz Figa
  0 siblings, 1 reply; 36+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-05-19 11:05 UTC (permalink / raw)
  To: Amit Kachhap
  Cc: Zhang Rui, Eduardo Valentin, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel


Hi,

On Monday, May 19, 2014 11:46:10 AM Amit Kachhap wrote:
> On 5/15/14, Zhang Rui <rui.zhang@intel.com> wrote:
> > On 一, 2014-05-05 at 13:15 +0200, Bartlomiej Zolnierkiewicz wrote:
> >> Hi,
> >>
> >> This patch series contains various cleanups for EXYNOS thermal
> >> driver.  Overall it decreases driver's LOC by 13%.  It is based
> >> on next-20140428 kernel.  It should not cause any functionality
> >> changes.
> >>
> > Amit,
> >
> > what do you think of this patch set?
> >
> > thanks,
> > rui
> 
> I agreed to many of the cleanups in the patch but tmu controller
> features should be retained as they will allow adding quick soc
> support and also avoid unnecessary churning of code in the future.

The general rule in the kernel code is not to keep the dead code as
it has a maintainance cost and makes further changes usually more
difficult, not easier.  There is no guarantee that the future SoCs
support will use the dead code (i.e. TYPE_TWO_POINT_TRIMMING support
has been introudced more than 2.5 years but no users of it has been
ever added) and in the meantime all other code changes has to take
the dead code into account.  I also have more Exynos thermal driver
changes in the work and it would be much easier to do them without
the need to support unused SoC features.  Please consider this in
your opinion.

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

> Thanks,
> Amit
> 
> >> Best regards,
> >> --
> >> Bartlomiej Zolnierkiewicz
> >> Samsung R&D Institute Poland
> >> Samsung Electronics
> >>
> >>
> >> Bartlomiej Zolnierkiewicz (10):
> >>   thermal: exynos: remove unused struct exynos_tmu_registers entries
> >>   thermal: exynos: remove unused defines
> >>   thermal: exynos: remove dead code for HW_MODE calibration
> >>   thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING
> >>     calibration
> >>   thermal: exynos: remove redundant pdata checks from
> >>     exynos_tmu_initialize()
> >>   thermal: exynos: remove redundant threshold_code checks from
> >>     exynos_tmu_initialize()
> >>   thermal: exynos: simplify temp_to_code() and code_to_temp()
> >>   thermal: exynos: cache non_hw_trigger_levels in pdata
> >>   thermal: exynos: remove redundant pdata checks from
> >>     exynos_tmu_control()
> >>   thermal: exynos: remove identical values from exynos*_tmu_registers
> >>     structures
> >>
> >>  drivers/thermal/samsung/exynos_thermal_common.h |   1 -
> >>  drivers/thermal/samsung/exynos_tmu.c            | 181
> >> ++++--------------------
> >>  drivers/thermal/samsung/exynos_tmu.h            |  86 +----------
> >>  drivers/thermal/samsung/exynos_tmu_data.c       |  40 +-----
> >>  drivers/thermal/samsung/exynos_tmu_data.h       |  32 +----
> >>  5 files changed, 37 insertions(+), 303 deletions(-)


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/10] thermal: exynos: various cleanups
  2014-05-19 11:05     ` Bartlomiej Zolnierkiewicz
@ 2014-05-19 11:09       ` Tomasz Figa
  0 siblings, 0 replies; 36+ messages in thread
From: Tomasz Figa @ 2014-05-19 11:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Amit Kachhap
  Cc: Zhang Rui, Eduardo Valentin, Tomasz Figa, Rafael J. Wysocki,
	Kyungmin Park, linux-samsung-soc, linux-pm, linux-kernel

Hi,

On 19.05.2014 13:05, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Monday, May 19, 2014 11:46:10 AM Amit Kachhap wrote:
>> On 5/15/14, Zhang Rui <rui.zhang@intel.com> wrote:
>>> On 一, 2014-05-05 at 13:15 +0200, Bartlomiej Zolnierkiewicz wrote:
>>>> Hi,
>>>>
>>>> This patch series contains various cleanups for EXYNOS thermal
>>>> driver.  Overall it decreases driver's LOC by 13%.  It is based
>>>> on next-20140428 kernel.  It should not cause any functionality
>>>> changes.
>>>>
>>> Amit,
>>>
>>> what do you think of this patch set?
>>>
>>> thanks,
>>> rui
>>
>> I agreed to many of the cleanups in the patch but tmu controller
>> features should be retained as they will allow adding quick soc
>> support and also avoid unnecessary churning of code in the future.
> 
> The general rule in the kernel code is not to keep the dead code as
> it has a maintainance cost and makes further changes usually more
> difficult, not easier.  There is no guarantee that the future SoCs
> support will use the dead code (i.e. TYPE_TWO_POINT_TRIMMING support
> has been introudced more than 2.5 years but no users of it has been
> ever added) and in the meantime all other code changes has to take
> the dead code into account.  I also have more Exynos thermal driver
> changes in the work and it would be much easier to do them without
> the need to support unused SoC features.  Please consider this in
> your opinion.

I'd also like to add that I don't see any correlation with mentioned
dead code and possibility of adding new SoCs. The code is mostly related
to unused/untested features and support for new SoCs might just be added
without those optional features. Moreover, less features (especially
untested) means less hassle to add new SoC, because you have less things
to test. Especially considering the fact that you would have to test
originally untested code and fix it if it turns out to be broken.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2014-05-19 11:09 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-05 11:15 [PATCH 00/10] thermal: exynos: various cleanups Bartlomiej Zolnierkiewicz
2014-05-05 11:15 ` [PATCH 01/10] thermal: exynos: remove unused struct exynos_tmu_registers entries Bartlomiej Zolnierkiewicz
2014-05-19  5:12   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 02/10] thermal: exynos: remove unused defines Bartlomiej Zolnierkiewicz
2014-05-15 14:07   ` Eduardo Valentin
2014-05-19  5:17   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 03/10] thermal: exynos: remove dead code for HW_MODE calibration Bartlomiej Zolnierkiewicz
2014-05-15 14:14   ` Eduardo Valentin
2014-05-15 15:06     ` Bartlomiej Zolnierkiewicz
2014-05-19  5:27   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 04/10] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration Bartlomiej Zolnierkiewicz
2014-05-15 14:31   ` Eduardo Valentin
2014-05-15 15:35     ` Bartlomiej Zolnierkiewicz
2014-05-19  5:40   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize() Bartlomiej Zolnierkiewicz
2014-05-15 14:47   ` Eduardo Valentin
2014-05-15 16:24     ` Bartlomiej Zolnierkiewicz
2014-05-19  5:47       ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 06/10] thermal: exynos: remove redundant threshold_code " Bartlomiej Zolnierkiewicz
2014-05-15 14:55   ` Eduardo Valentin
2014-05-15 16:56     ` Bartlomiej Zolnierkiewicz
2014-05-19  5:50   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 07/10] thermal: exynos: simplify temp_to_code() and code_to_temp() Bartlomiej Zolnierkiewicz
2014-05-19  5:54   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 08/10] thermal: exynos: cache non_hw_trigger_levels in pdata Bartlomiej Zolnierkiewicz
2014-05-19  5:56   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 09/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_control() Bartlomiej Zolnierkiewicz
2014-05-15 15:03   ` Eduardo Valentin
2014-05-15 17:06     ` Bartlomiej Zolnierkiewicz
2014-05-19  6:05   ` Amit Kachhap
2014-05-05 11:15 ` [PATCH 10/10] thermal: exynos: remove identical values from exynos*_tmu_registers structures Bartlomiej Zolnierkiewicz
2014-05-19  6:11   ` Amit Kachhap
2014-05-15  9:06 ` [PATCH 00/10] thermal: exynos: various cleanups Zhang Rui
2014-05-19  6:16   ` Amit Kachhap
2014-05-19 11:05     ` Bartlomiej Zolnierkiewicz
2014-05-19 11:09       ` Tomasz Figa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.