| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 19 Jun 2013 16:52:27 -0400
From: Eduardo Valentin <eduardo.valentin@...com>
To: Eduardo Valentin <eduardo.valentin@...com>,
Amit Daniel Kachhap <amit.daniel@...sung.com>
CC: <linux-pm@...r.kernel.org>, Zhang Rui <rui.zhang@...el.com>,
<linux-samsung-soc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <amit.kachhap@...il.com>,
Kukjin Kim <kgene.kim@...sung.com>, <jonghwa3.lee@...sung.com>
Subject: Re: [PATCH V6 09/30] thermal: exynos: Add extra entries in the tmu
platform data
On 19-06-2013 16:19, Eduardo Valentin wrote:
> On 17-06-2013 02:46, Amit Daniel Kachhap wrote:
>> This patch adds entries min_efuse_value, max_efuse_value, default_temp_offset,
>> trigger_type, cal_type, trim_first_point, trim_second_point, max_trigger_level
>> trigger_enable in the TMU platform data structure. Also the driver is modified
>> to use the data passed by these new platform memebers instead of the constant
>> macros. All these changes helps in separating the SOC specific data part from
>> the TMU driver.
>>
>> Acked-by: Kukjin Kim <kgene.kim@...sung.com>
>> Acked-by: Jonghwa Lee <jonghwa3.lee@...sung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@...sung.com>
>> ---
>> drivers/thermal/samsung/exynos_thermal_common.h | 7 +++
>> drivers/thermal/samsung/exynos_tmu.c | 43 ++++++++++----------
>> drivers/thermal/samsung/exynos_tmu.h | 49 ++++++++++++++--------
>> drivers/thermal/samsung/exynos_tmu_data.c | 35 ++++++++++++----
>> 4 files changed, 86 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h
>> index 068f56c..fd789a5 100644
>> --- a/drivers/thermal/samsung/exynos_thermal_common.h
>> +++ b/drivers/thermal/samsung/exynos_thermal_common.h
>> @@ -44,6 +44,13 @@
>>
>> #define EXYNOS_ZONE_COUNT 3
>>
>> +enum trigger_type {
>> + THROTTLE_ACTIVE = 1,
>> + THROTTLE_PASSIVE,
>> + SW_TRIP,
>> + HW_TRIP,
>> +};
>> +
>> /**
>> * struct freq_clip_table
>> * @freq_clip_max: maximum frequency allowed for this cooling state.
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index fa33a48..401ec98 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -49,7 +49,6 @@
>> #define EXYNOS_TMU_BUF_SLOPE_SEL_MASK 0xf
>> #define EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT 8
>> #define EXYNOS_TMU_CORE_EN_SHIFT 0
>> -#define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
>>
>> /* Exynos4210 specific registers */
>> #define EXYNOS4210_TMU_REG_THRESHOLD_TEMP 0x44
>> @@ -94,9 +93,6 @@
>> #define EXYNOS_TMU_INTEN_FALL1_SHIFT 20
>> #define EXYNOS_TMU_INTEN_FALL2_SHIFT 24
>>
>> -#define EFUSE_MIN_VALUE 40
>> -#define EFUSE_MAX_VALUE 100
>> -
>> #ifdef CONFIG_THERMAL_EMULATION
>> #define EXYNOS_EMUL_TIME 0x57F0
>> #define EXYNOS_EMUL_TIME_MASK 0xffff
>> @@ -136,15 +132,16 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
>>
>> switch (pdata->cal_type) {
>> case TYPE_TWO_POINT_TRIMMING:
>> - temp_code = (temp - 25) *
>> - (data->temp_error2 - data->temp_error1) /
>> - (85 - 25) + data->temp_error1;
>> + 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 - 25;
>> + temp_code = temp + data->temp_error1 - pdata->first_point_trim;
>> break;
>> default:
>> - temp_code = temp + EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET;
>> + temp_code = temp + pdata->default_temp_offset;
>> break;
>> }
>> out:
>> @@ -169,14 +166,16 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
>>
>> switch (pdata->cal_type) {
>> case TYPE_TWO_POINT_TRIMMING:
>> - temp = (temp_code - data->temp_error1) * (85 - 25) /
>> - (data->temp_error2 - data->temp_error1) + 25;
>> + 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 + 25;
>> + temp = temp_code - data->temp_error1 + pdata->first_point_trim;
>> break;
>> default:
>> - temp = temp_code - EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET;
>> + temp = temp_code - pdata->default_temp_offset;
>> break;
>> }
>> out:
>> @@ -209,8 +208,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>> data->temp_error1 = trim_info & EXYNOS_TMU_TRIM_TEMP_MASK;
>> data->temp_error2 = ((trim_info >> 8) & EXYNOS_TMU_TRIM_TEMP_MASK);
>>
>> - if ((EFUSE_MIN_VALUE > data->temp_error1) ||
>> - (data->temp_error1 > EFUSE_MAX_VALUE) ||
>> + if ((pdata->min_efuse_value > data->temp_error1) ||
>> + (data->temp_error1 > pdata->max_efuse_value) ||
>> (data->temp_error2 != 0))
>> data->temp_error1 = pdata->efuse_value;
>>
>> @@ -300,10 +299,10 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>> if (on) {
>> con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
>> interrupt_en =
>> - pdata->trigger_level3_en << EXYNOS_TMU_INTEN_RISE3_SHIFT |
>> - pdata->trigger_level2_en << EXYNOS_TMU_INTEN_RISE2_SHIFT |
>> - pdata->trigger_level1_en << EXYNOS_TMU_INTEN_RISE1_SHIFT |
>> - pdata->trigger_level0_en << EXYNOS_TMU_INTEN_RISE0_SHIFT;
>> + pdata->trigger_enable[3] << EXYNOS_TMU_INTEN_RISE3_SHIFT |
>> + pdata->trigger_enable[2] << EXYNOS_TMU_INTEN_RISE2_SHIFT |
>> + pdata->trigger_enable[1] << EXYNOS_TMU_INTEN_RISE1_SHIFT |
>> + pdata->trigger_enable[0] << EXYNOS_TMU_INTEN_RISE0_SHIFT;
>> if (pdata->threshold_falling)
>> interrupt_en |=
>> interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
>> @@ -533,9 +532,9 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>>
>> /* Register the sensor with thermal management interface */
>> (&exynos_sensor_conf)->private_data = data;
>> - exynos_sensor_conf.trip_data.trip_count = pdata->trigger_level0_en +
>> - pdata->trigger_level1_en + pdata->trigger_level2_en +
>> - pdata->trigger_level3_en;
>> + exynos_sensor_conf.trip_data.trip_count = pdata->trigger_enable[0] +
>> + pdata->trigger_enable[1] + pdata->trigger_enable[2]+
>> + pdata->trigger_enable[3];
>>
>> for (i = 0; i < exynos_sensor_conf.trip_data.trip_count; i++)
>> exynos_sensor_conf.trip_data.trip_val[i] =
>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
>> index 9e0f887..45c697d 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.h
>> +++ b/drivers/thermal/samsung/exynos_tmu.h
>> @@ -30,6 +30,11 @@ enum calibration_type {
>> TYPE_NONE,
>> };
>>
>> +enum calibration_mode {
>> + SW_MODE,
>> + HW_MODE,
>> +};
>> +
>> enum soc_type {
>> SOC_ARCH_EXYNOS4210 = 1,
>> SOC_ARCH_EXYNOS,
>> @@ -55,18 +60,15 @@ enum soc_type {
>> * 3: temperature for trigger_level3 interrupt
>> * condition for trigger_level3 interrupt:
>> * current temperature > threshold + trigger_levels[3]
>> - * @trigger_level0_en:
>> - * 1 = enable trigger_level0 interrupt,
>> - * 0 = disable trigger_level0 interrupt
>> - * @trigger_level1_en:
>> - * 1 = enable trigger_level1 interrupt,
>> - * 0 = disable trigger_level1 interrupt
>> - * @trigger_level2_en:
>> - * 1 = enable trigger_level2 interrupt,
>> - * 0 = disable trigger_level2 interrupt
>> - * @trigger_level3_en:
>> - * 1 = enable trigger_level3 interrupt,
>> - * 0 = disable trigger_level3 interrupt
>> + * @trigger_type: defines the type of trigger. Possible values are,
>> + * THROTTLE_ACTIVE trigger type
>> + * THROTTLE_PASSIVE trigger type
>> + * SW_TRIP trigger type
>> + * HW_TRIP
>> + * @trigger_enable[]: array to denote which trigger levels are enabled.
>> + * 1 = enable trigger_level[] interrupt,
>> + * 0 = disable trigger_level[] interrupt
>> + * @max_trigger_level: max trigger level supported by the TMU
>> * @gain: gain of amplifier in the positive-TC generator block
>> * 0 <= gain <= 15
>> * @reference_voltage: reference voltage of amplifier
>> @@ -76,7 +78,13 @@ enum soc_type {
>> * 000, 100, 101, 110 and 111 can be different modes
>> * @type: determines the type of SOC
>> * @efuse_value: platform defined fuse value
>> + * @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
>> * @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.
>> @@ -86,18 +94,23 @@ enum soc_type {
>> struct exynos_tmu_platform_data {
>> u8 threshold;
>> u8 threshold_falling;
>> - u8 trigger_levels[4];
>> - bool trigger_level0_en;
>> - bool trigger_level1_en;
>> - bool trigger_level2_en;
>> - bool trigger_level3_en;
>> -
>> + u8 trigger_levels[MAX_TRIP_COUNT];
>> + enum trigger_type trigger_type[MAX_TRIP_COUNT];
>> + bool trigger_enable[MAX_TRIP_COUNT];
>> + u8 max_trigger_level;
>> u8 gain;
>> u8 reference_voltage;
>> u8 noise_cancel_mode;
>> +
>> u32 efuse_value;
>> + u32 min_efuse_value;
>> + u32 max_efuse_value;
>> + u8 first_point_trim;
>> + u8 second_point_trim;
>> + u8 default_temp_offset;
>>
>> 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 13a60ca..a187043 100644
>> --- a/drivers/thermal/samsung/exynos_tmu_data.c
>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
>> @@ -22,6 +22,7 @@
>>
>> #include "exynos_thermal_common.h"
>> #include "exynos_tmu.h"
>> +#include "exynos_tmu_data.h"
>
> This change needs to be moved to the patch that you added this file.
> Check comment on patch 07/30.
>>
>> #if defined(CONFIG_CPU_EXYNOS4210)
>> struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
>> @@ -29,13 +30,22 @@ struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
>> .trigger_levels[0] = 5,
>> .trigger_levels[1] = 20,
>> .trigger_levels[2] = 30,
>> - .trigger_level0_en = 1,
>> - .trigger_level1_en = 1,
>> - .trigger_level2_en = 1,
>> - .trigger_level3_en = 0,
>> + .trigger_enable[0] = 1,
>> + .trigger_enable[1] = 1,
>> + .trigger_enable[2] = 1,
>> + .trigger_enable[3] = 0,
This change added this sparse warning on your driver:
drivers/thermal/samsung/exynos_tmu_data.c:34:10: warning: Initializer
entry defined twice
drivers/thermal/samsung/exynos_tmu_data.c:35:10: also defined here
>> + .trigger_type[0] = THROTTLE_ACTIVE,
>> + .trigger_type[1] = THROTTLE_ACTIVE,
>> + .trigger_type[2] = SW_TRIP,
>
> is there any issues if trigger_type[3] is 0? there is no defined value
> for 0. (0 means undefined on your enum definition).
>
>
>> + .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,
>> @@ -55,15 +65,24 @@ struct exynos_tmu_platform_data const exynos5250_default_tmu_data = {
>> .trigger_levels[0] = 85,
>> .trigger_levels[1] = 103,
>> .trigger_levels[2] = 110,
>> - .trigger_level0_en = 1,
>> - .trigger_level1_en = 1,
>> - .trigger_level2_en = 1,
>> - .trigger_level3_en = 0,
>> + .trigger_enable[0] = 1,
>> + .trigger_enable[1] = 1,
>> + .trigger_enable[2] = 1,
>> + .trigger_enable[3] = 0,
This change add this sparse warning on your driver:
drivers/thermal/samsung/exynos_tmu_data.c:69:10: warning: Initializer
entry defined twice
drivers/thermal/samsung/exynos_tmu_data.c:70:10: also defined here
>> + .trigger_type[0] = THROTTLE_ACTIVE,
>> + .trigger_type[1] = THROTTLE_ACTIVE,
>> + .trigger_type[2] = SW_TRIP,
>> + .max_trigger_level = 4,
>> .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 = 800 * 1000,
>> .temp_level = 85,
>>
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
Download attachment "signature.asc" of type "application/pgp-signature" (296 bytes)
Powered by blists - more mailing lists