| 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: Mon, 24 Jun 2013 09:27:36 +0530
From: amit daniel kachhap <amit.daniel@...sung.com>
To: Eduardo Valentin <eduardo.valentin@...com>
Cc: linux-pm@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
Kukjin Kim <kgene.kim@...sung.com>, jonghwa3.lee@...sung.com
Subject: Re: [PATCH V6 08/30] thermal: exynos: Add missing definations and
code cleanup
On Fri, Jun 21, 2013 at 8:01 PM, Eduardo Valentin
<eduardo.valentin@...com> wrote:
> On 20-06-2013 22:01, amit daniel kachhap wrote:
>> Hi Eduardo,
>>
>> On Thu, Jun 20, 2013 at 1:25 AM, Eduardo Valentin
>> <eduardo.valentin@...com> wrote:
>>> On 17-06-2013 02:46, Amit Daniel Kachhap wrote:
>>>> This patch adds some extra register bitfield definations and cleans
>>>> up the code to prepare for moving register macros and definations inside
>>>> the TMU data section.
>>>>
>>>> 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_tmu.c | 62 +++++++++++++++++++++++++---------
>>>> 1 files changed, 46 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>>> index 5df04a1..fa33a48 100644
>>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>>> @@ -43,9 +43,12 @@
>>>>
>>>> #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
>>>> #define EXYNOS_TMU_GAIN_SHIFT 8
>>>> +#define EXYNOS_TMU_GAIN_MASK 0xf
>>>> #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
>>>> -#define EXYNOS_TMU_CORE_ON 3
>>>> -#define EXYNOS_TMU_CORE_OFF 2
>>>> +#define EXYNOS_TMU_REF_VOLTAGE_MASK 0x1f
>>>> +#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 */
>>>> @@ -63,6 +66,7 @@
>>>> #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 */
>>>> @@ -72,17 +76,30 @@
>>>> #define EXYNOS_EMUL_CON 0x80
>>>>
>>>> #define EXYNOS_TRIMINFO_RELOAD 0x1
>>>> +#define EXYNOS_TRIMINFO_SHIFT 0x0
>>>> +#define EXYNOS_TMU_RISE_INT_MASK 0x111
>>>> +#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_MUX_ADDR_VALUE 6
>>>> -#define EXYNOS_MUX_ADDR_SHIFT 20
>>>> #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
>>>> +#define EXYNOS_TMU_TRIP_MODE_MASK 0x7
>>>> +
>>>> +#define EXYNOS_TMU_INTEN_RISE0_SHIFT 0
>>>> +#define EXYNOS_TMU_INTEN_RISE1_SHIFT 4
>>>> +#define EXYNOS_TMU_INTEN_RISE2_SHIFT 8
>>>> +#define EXYNOS_TMU_INTEN_RISE3_SHIFT 12
>>>> +#define EXYNOS_TMU_INTEN_FALL0_SHIFT 16
>>>> +#define EXYNOS_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
>>>> #define EXYNOS_EMUL_TIME_SHIFT 16
>>>> #define EXYNOS_EMUL_DATA_SHIFT 8
>>>> #define EXYNOS_EMUL_DATA_MASK 0xFF
>>>> @@ -261,24 +278,37 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
>>>> mutex_lock(&data->lock);
>>>> clk_enable(data->clk);
>>>>
>>>> - con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
>>>> - pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
>>>> + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
>>>>
>>>> - if (data->soc == SOC_ARCH_EXYNOS) {
>>>> - con |= pdata->noise_cancel_mode << EXYNOS_TMU_TRIP_MODE_SHIFT;
>>>> - con |= (EXYNOS_MUX_ADDR_VALUE << EXYNOS_MUX_ADDR_SHIFT);
>>>> + if (pdata->reference_voltage) {
>>>> + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK <<
>>>> + EXYNOS_TMU_REF_VOLTAGE_SHIFT);
>>>> + con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT;
>>>> + }
>>>> +
>>>> + if (pdata->gain) {
>>>> + con &= ~(EXYNOS_TMU_GAIN_MASK << EXYNOS_TMU_GAIN_SHIFT);
>>>> + con |= (pdata->gain << EXYNOS_TMU_GAIN_SHIFT);
>>>> + }
>>>> +
>>>> + if (pdata->noise_cancel_mode) {
>>>> + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK <<
>>>> + EXYNOS_TMU_TRIP_MODE_SHIFT);
>>>> + con |= (pdata->noise_cancel_mode << EXYNOS_TMU_TRIP_MODE_SHIFT);
>>>> }
>>>>
>>>> if (on) {
>>>> - con |= EXYNOS_TMU_CORE_ON;
>>>
>>>
>>>
>>> Before, in order to turn core on you had:
>>> con = con | 3;
>>>
>>> now you do:
>>> con = con | (1 << 0);
>>>
>>> To me, before you would set bit 1 and 0, now you set bit 0.
>>>
>>>
>>>> - interrupt_en = pdata->trigger_level3_en << 12 |
>>>> - pdata->trigger_level2_en << 8 |
>>>> - pdata->trigger_level1_en << 4 |
>>>> - pdata->trigger_level0_en;
>>>> + 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;
>>>> if (pdata->threshold_falling)
>>>> - interrupt_en |= interrupt_en << 16;
>>>> + interrupt_en |=
>>>> + interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT;
>>>> } else {
>>>> - con |= EXYNOS_TMU_CORE_OFF;
>>>> + con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
>>>
>>>
>>> Before, in order to turno core off you had:
>>> con = con | 2;
>>>
>>> now you do:
>>> con = con & ~(1 << 0);
>>>
>>> To me, before you would set bit 2, now you clear bit 0.
>>>
>>> Using the approach on this patch looks correct to me if you have 1 bit
>>> core_en for instance.
>>>
>>> so, Is this a fix?
>>>
>>> Just to be clear, is this what you want ?
>> Yes you are right. Bit 0 is the actual enable bit. Bit 1 is the
>> reserve bit and its default value is 1. Earlier both bits are used to
>> turn on/off the controller which was wrong so this patch rectifies it.
>
> Care to mention this in your patch description? It is good for code history.
Ok
>
>>
>> Thanks,
>> Amit
>>>
>>>> interrupt_en = 0; /* Disable all interrupts */
>>>> }
>>>> writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
>>>>
>>>
>>>
>>> --
>>> You have got to be excited about what you are doing. (L. Lamport)
>>>
>>> Eduardo Valentin
>>>
>>
>>
>
>
> --
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists