diff mbox

[V3,10/21] thermal: exynos: Fix to clear only the generated interrupts

Message ID 1367931671-3906-11-git-send-email-amit.daniel@samsung.com (mailing list archive)
State Changes Requested
Delegated to: Eduardo Valentin
Headers show

Commit Message

Amit Kachhap May 7, 2013, 1:01 p.m. UTC 
This patch uses the TMU status register to know the generated interrupts
and only clear them in the interrupt handler.

Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c      |   10 ++++------
 drivers/thermal/samsung/exynos_tmu_data.c |    2 ++
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Eduardo Valentin May 9, 2013, 2:33 p.m. UTC | #1 
On 07-05-2013 09:01, Amit Daniel Kachhap wrote:
> This patch uses the TMU status register to know the generated interrupts
> and only clear them in the interrupt handler.
> 
> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c      |   10 ++++------
>  drivers/thermal/samsung/exynos_tmu_data.c |    2 ++
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 97b87aa..082d5a2 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -325,17 +325,15 @@ static void exynos_tmu_work(struct work_struct *work)
>  			struct exynos_tmu_data, irq_work);
>  	struct exynos_tmu_platform_data *pdata = data->pdata;
>  	struct exynos_tmu_registers *reg = pdata->registers;
> +	unsigned int val_irq;
>  
>  	exynos_report_trigger();
>  	mutex_lock(&data->lock);
>  	clk_enable(data->clk);
>  
> -	if (data->soc == SOC_ARCH_EXYNOS)
> -		writel((reg->inten_rise_mask << reg->inten_rise_shift) |
> -			(reg->inten_fall_mask << reg->inten_fall_shift),
> -				data->base + reg->tmu_intclear);
> -	else
> -		writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
> +	val_irq = readl(data->base + reg->tmu_intstat);
> +	/* clear the interrupts */
> +	writel(val_irq, data->base + reg->tmu_intclear);
>  

hmmm.. Don t you need to act accordingly depending on what is in your
int status?

>  	clk_disable(data->clk);
>  	mutex_unlock(&data->lock);
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> index 3a1ded1..7322597 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -45,6 +45,7 @@ static struct exynos_tmu_registers exynos4210_tmu_registers = {
>  	.inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>  	.inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>  	.inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
> +	.tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>  	.tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>  };
>  struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
> @@ -111,6 +112,7 @@ static struct exynos_tmu_registers exynos5250_tmu_registers = {
>  	.inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>  	.inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>  	.inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
> +	.tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>  	.tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>  	.emul_con = EXYNOS_EMUL_CON,
>  	.emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>
Amit Kachhap May 10, 2013, 2:22 a.m. UTC | #2 
Hi,

On Thu, May 9, 2013 at 8:03 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> On 07-05-2013 09:01, Amit Daniel Kachhap wrote:
>> This patch uses the TMU status register to know the generated interrupts
>> and only clear them in the interrupt handler.
>>
>> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/thermal/samsung/exynos_tmu.c      |   10 ++++------
>>  drivers/thermal/samsung/exynos_tmu_data.c |    2 ++
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index 97b87aa..082d5a2 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -325,17 +325,15 @@ static void exynos_tmu_work(struct work_struct *work)
>>                       struct exynos_tmu_data, irq_work);
>>       struct exynos_tmu_platform_data *pdata = data->pdata;
>>       struct exynos_tmu_registers *reg = pdata->registers;
>> +     unsigned int val_irq;
>>
>>       exynos_report_trigger();
>>       mutex_lock(&data->lock);
>>       clk_enable(data->clk);
>>
>> -     if (data->soc == SOC_ARCH_EXYNOS)
>> -             writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>> -                     (reg->inten_fall_mask << reg->inten_fall_shift),
>> -                             data->base + reg->tmu_intclear);
>> -     else
>> -             writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>> +     val_irq = readl(data->base + reg->tmu_intstat);
>> +     /* clear the interrupts */
>> +     writel(val_irq, data->base + reg->tmu_intclear);
>>
>
> hmmm.. Don t you need to act accordingly depending on what is in your
> int status?

Earlier also it is done like this way. Basically core thermal layer
checks temp for each trip so even if I check for which threshold
interrupt is raised it is of no use. Anyway this is a TODO for me.

Thanks,
Amit Daniel
>
>>       clk_disable(data->clk);
>>       mutex_unlock(&data->lock);
>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
>> index 3a1ded1..7322597 100644
>> --- a/drivers/thermal/samsung/exynos_tmu_data.c
>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
>> @@ -45,6 +45,7 @@ static struct exynos_tmu_registers exynos4210_tmu_registers = {
>>       .inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>  };
>>  struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
>> @@ -111,6 +112,7 @@ static struct exynos_tmu_registers exynos5250_tmu_registers = {
>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>>       .inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>       .emul_con = EXYNOS_EMUL_CON,
>>       .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>>
>
>
--
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
Eduardo Valentin May 10, 2013, 3:40 p.m. UTC | #3 
On 09-05-2013 22:22, amit daniel kachhap wrote:
> Hi,
> 
> On Thu, May 9, 2013 at 8:03 PM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
>> On 07-05-2013 09:01, Amit Daniel Kachhap wrote:
>>> This patch uses the TMU status register to know the generated interrupts
>>> and only clear them in the interrupt handler.
>>>
>>> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>> ---
>>>  drivers/thermal/samsung/exynos_tmu.c      |   10 ++++------
>>>  drivers/thermal/samsung/exynos_tmu_data.c |    2 ++
>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>> index 97b87aa..082d5a2 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>> @@ -325,17 +325,15 @@ static void exynos_tmu_work(struct work_struct *work)
>>>                       struct exynos_tmu_data, irq_work);
>>>       struct exynos_tmu_platform_data *pdata = data->pdata;
>>>       struct exynos_tmu_registers *reg = pdata->registers;
>>> +     unsigned int val_irq;
>>>
>>>       exynos_report_trigger();
>>>       mutex_lock(&data->lock);
>>>       clk_enable(data->clk);
>>>
>>> -     if (data->soc == SOC_ARCH_EXYNOS)
>>> -             writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>>> -                     (reg->inten_fall_mask << reg->inten_fall_shift),
>>> -                             data->base + reg->tmu_intclear);
>>> -     else
>>> -             writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>>> +     val_irq = readl(data->base + reg->tmu_intstat);
>>> +     /* clear the interrupts */
>>> +     writel(val_irq, data->base + reg->tmu_intclear);
>>>
>>
>> hmmm.. Don t you need to act accordingly depending on what is in your
>> int status?
> 
> Earlier also it is done like this way. Basically core thermal layer
> checks temp for each trip so even if I check for which threshold
> interrupt is raised it is of no use. Anyway this is a TODO for me.
> 

Please add  a /* TODO: */ entry then.

> Thanks,
> Amit Daniel
>>
>>>       clk_disable(data->clk);
>>>       mutex_unlock(&data->lock);
>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
>>> index 3a1ded1..7322597 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu_data.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
>>> @@ -45,6 +45,7 @@ static struct exynos_tmu_registers exynos4210_tmu_registers = {
>>>       .inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>>  };
>>>  struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
>>> @@ -111,6 +112,7 @@ static struct exynos_tmu_registers exynos5250_tmu_registers = {
>>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>>>       .inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
>>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>>       .emul_con = EXYNOS_EMUL_CON,
>>>       .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>>>
>>
>>
> 
>
Amit Kachhap May 13, 2013, 1:36 a.m. UTC | #4 
On Fri, May 10, 2013 at 9:10 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> On 09-05-2013 22:22, amit daniel kachhap wrote:
>> Hi,
>>
>> On Thu, May 9, 2013 at 8:03 PM, Eduardo Valentin
>> <eduardo.valentin@ti.com> wrote:
>>> On 07-05-2013 09:01, Amit Daniel Kachhap wrote:
>>>> This patch uses the TMU status register to know the generated interrupts
>>>> and only clear them in the interrupt handler.
>>>>
>>>> Acked-by: Kukjin Kim <kgene.kim@samsung.com>
>>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>>> ---
>>>>  drivers/thermal/samsung/exynos_tmu.c      |   10 ++++------
>>>>  drivers/thermal/samsung/exynos_tmu_data.c |    2 ++
>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>>>> index 97b87aa..082d5a2 100644
>>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>>> @@ -325,17 +325,15 @@ static void exynos_tmu_work(struct work_struct *work)
>>>>                       struct exynos_tmu_data, irq_work);
>>>>       struct exynos_tmu_platform_data *pdata = data->pdata;
>>>>       struct exynos_tmu_registers *reg = pdata->registers;
>>>> +     unsigned int val_irq;
>>>>
>>>>       exynos_report_trigger();
>>>>       mutex_lock(&data->lock);
>>>>       clk_enable(data->clk);
>>>>
>>>> -     if (data->soc == SOC_ARCH_EXYNOS)
>>>> -             writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>>>> -                     (reg->inten_fall_mask << reg->inten_fall_shift),
>>>> -                             data->base + reg->tmu_intclear);
>>>> -     else
>>>> -             writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>>>> +     val_irq = readl(data->base + reg->tmu_intstat);
>>>> +     /* clear the interrupts */
>>>> +     writel(val_irq, data->base + reg->tmu_intclear);
>>>>
>>>
>>> hmmm.. Don t you need to act accordingly depending on what is in your
>>> int status?
>>
>> Earlier also it is done like this way. Basically core thermal layer
>> checks temp for each trip so even if I check for which threshold
>> interrupt is raised it is of no use. Anyway this is a TODO for me.
>>
>
> Please add  a /* TODO: */ entry then.
ok,

Thanks,
Amit Daniel
>
>> Thanks,
>> Amit Daniel
>>>
>>>>       clk_disable(data->clk);
>>>>       mutex_unlock(&data->lock);
>>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
>>>> index 3a1ded1..7322597 100644
>>>> --- a/drivers/thermal/samsung/exynos_tmu_data.c
>>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
>>>> @@ -45,6 +45,7 @@ static struct exynos_tmu_registers exynos4210_tmu_registers = {
>>>>       .inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>>>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>>>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>>>  };
>>>>  struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
>>>> @@ -111,6 +112,7 @@ static struct exynos_tmu_registers exynos5250_tmu_registers = {
>>>>       .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>>>>       .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>>>>       .inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
>>>> +     .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>>>>       .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>>>>       .emul_con = EXYNOS_EMUL_CON,
>>>>       .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>>>>
>>>
>>>
>>
>>
>
>
--
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
diff mbox

Patch

diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
index 97b87aa..082d5a2 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -325,17 +325,15 @@  static void exynos_tmu_work(struct work_struct *work)
 			struct exynos_tmu_data, irq_work);
 	struct exynos_tmu_platform_data *pdata = data->pdata;
 	struct exynos_tmu_registers *reg = pdata->registers;
+	unsigned int val_irq;
 
 	exynos_report_trigger();
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
 
-	if (data->soc == SOC_ARCH_EXYNOS)
-		writel((reg->inten_rise_mask << reg->inten_rise_shift) |
-			(reg->inten_fall_mask << reg->inten_fall_shift),
-				data->base + reg->tmu_intclear);
-	else
-		writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
+	val_irq = readl(data->base + reg->tmu_intstat);
+	/* clear the interrupts */
+	writel(val_irq, data->base + reg->tmu_intclear);
 
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
index 3a1ded1..7322597 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -45,6 +45,7 @@  static struct exynos_tmu_registers exynos4210_tmu_registers = {
 	.inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
 	.inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
 	.inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
+	.tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
 	.tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
 };
 struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
@@ -111,6 +112,7 @@  static struct exynos_tmu_registers exynos5250_tmu_registers = {
 	.inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
 	.inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
 	.inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
+	.tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
 	.tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
 	.emul_con = EXYNOS_EMUL_CON,
 	.emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,