Re: [PATCH V6 09/30] thermal: exynos: Add extra entries in the tmuplatform data
From: amit daniel kachhap
Date: Fri Jun 21 2013 - 04:51:22 EST
Hi,
On Thu, Jun 20, 2013 at 2:22 AM, Eduardo Valentin
<eduardo.valentin@xxxxxx> wrote:
> 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@xxxxxxxxxxx>
>>> Acked-by: Jonghwa Lee <jonghwa3.lee@xxxxxxxxxxx>
>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@xxxxxxxxxxx>
>>> ---
>>> 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
It seems sparse tool has some issue in bool assignment checkinh and it
is not fixed yet.
http://lkml.indiana.edu/hypermail/linux/kernel/1005.0/02153.html
so leaving it as of now.
Thanks,
Amit D
>
>
>>> + .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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/