On 11-10-2013 11:57, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On Friday, October 11, 2013 11:10:38 AM Eduardo Valentin wrote: >> Hi Naveen, >> >> On 09-10-2013 10:03, Bartlomiej Zolnierkiewicz wrote: >>> >>> Hi, >>> >>> All patches (#1-#3) look good to me, FWIW you can add: >>> >>> Reviewed-by: Bartlomiej Zolnierkiewicz >>> >>> Please note that (at least) patch #3 conflicts with Lukasz's EXYNOS4412 >>> fixup patchset: >>> >>> https://lkml.org/lkml/2013/10/9/35 >>> >>> It is up to Eduardo to resolve this but it probably would be better to >>> merge EXYNOS4412 fixes first and then add EXYNOS5420 support. This would >>> require you to port patch #3 over Lukasz's patchset though. >> >> My question is if this fix applies also to EXYNOS4412, as it is not >> mentioned in the patch description and the change affects all supported > > This patch doesn't affect EXYNOS4210 as exynos4210_tmu_registers struct I was, at least for now, worried about 4412, as I mentioned above. > uses the default zero value for inten_fall_mask, inten_fall_shift and > intclr_fall_shift. > >> chip version deliberately. Has this change been validated on all >> supported chip versions? >> >> Amit, I saw you ack, but still, it is not clear how this change behaves >> across supported hardware. > > For EXYNOS4412 and EXYNOS5250 this patch doesn't cause any functionality > changes because while the patch changes inten_fall_shift usage to > intclr_fall_shift one in exynos_tmu_initialize() it defines > EXYNOS_TMU_CLEAR_FALL_INT_SHIFT to 12 (old EXYNOS_TMU_FALL_INT_SHIFT > value). > OK. Then the patch is about a symbol rename, right? > This patch only changes driver behavior for EXYNOS5440 on which the > used shift value changes from 4 to 12. I see. > > PS I've only noticed it now but after this patch inten_fall_shift becomes > unused and can be removed. > Then we should remove it. > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > >>> >>> Best regards, >>> -- >>> Bartlomiej Zolnierkiewicz >>> Samsung R&D Institute Poland >>> Samsung Electronics >>> >>> On Wednesday, October 09, 2013 05:38:27 PM Naveen Krishna Chatradhi wrote: >>>> The FALL interrupt related en, status bits are available at an offset of >>>> 16 on INTEN, INTSTAT registers and at an offset of >>>> 12 on INTCLEAR register. >>>> >>>> This patch corrects the same for exyns5250 and exynos5440 >>>> >>>> Signed-off-by: Naveen Krishna Chatradhi >>>> --- >>>> Changes since v1: >>>> Changes since v2: >>>> Changes since v3: >>>> None >>>> >>>> drivers/thermal/samsung/exynos_tmu.c | 2 +- >>>> drivers/thermal/samsung/exynos_tmu.h | 2 ++ >>>> drivers/thermal/samsung/exynos_tmu_data.c | 2 ++ >>>> drivers/thermal/samsung/exynos_tmu_data.h | 3 ++- >>>> 4 files changed, 7 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c >>>> index b43afda..af69209 100644 >>>> --- a/drivers/thermal/samsung/exynos_tmu.c >>>> +++ b/drivers/thermal/samsung/exynos_tmu.c >>>> @@ -265,7 +265,7 @@ skip_calib_data: >>>> data->base + reg->threshold_th1); >>>> >>>> writel((reg->inten_rise_mask << reg->inten_rise_shift) | >>>> - (reg->inten_fall_mask << reg->inten_fall_shift), >>>> + (reg->inten_fall_mask << reg->intclr_fall_shift), >>>> data->base + reg->tmu_intclear); >>>> >>>> /* if last threshold limit is also present */ >>>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h >>>> index b364c9e..7c6c34a 100644 >>>> --- a/drivers/thermal/samsung/exynos_tmu.h >>>> +++ b/drivers/thermal/samsung/exynos_tmu.h >>>> @@ -134,6 +134,7 @@ enum soc_type { >>>> * @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. >>>> + * @intclr_fall_shift: shift bits for interrupt clear fall 0 >>>> * @emul_con: TMU emulation controller register. >>>> * @emul_temp_shift: shift bits of emulation temperature. >>>> * @emul_time_shift: shift bits of emulation time. >>>> @@ -204,6 +205,7 @@ struct exynos_tmu_registers { >>>> u32 tmu_intstat; >>>> >>>> u32 tmu_intclear; >>>> + u32 intclr_fall_shift; >>>> >>>> u32 emul_con; >>>> u32 emul_temp_shift; >>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c >>>> index 9002499..23fea23 100644 >>>> --- a/drivers/thermal/samsung/exynos_tmu_data.c >>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c >>>> @@ -122,6 +122,7 @@ static const struct exynos_tmu_registers exynos5250_tmu_registers = { >>>> .inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT, >>>> .tmu_intstat = EXYNOS_TMU_REG_INTSTAT, >>>> .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR, >>>> + .intclr_fall_shift = EXYNOS_TMU_CLEAR_FALL_INT_SHIFT, >>>> .emul_con = EXYNOS_EMUL_CON, >>>> .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT, >>>> .emul_time_shift = EXYNOS_EMUL_TIME_SHIFT, >>>> @@ -210,6 +211,7 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = { >>>> .inten_fall0_shift = EXYNOS5440_TMU_INTEN_FALL0_SHIFT, >>>> .tmu_intstat = EXYNOS5440_TMU_S0_7_IRQ, >>>> .tmu_intclear = EXYNOS5440_TMU_S0_7_IRQ, >>>> + .intclr_fall_shift = EXYNOS_TMU_CLEAR_FALL_INT_SHIFT, >>>> .tmu_irqstatus = EXYNOS5440_TMU_IRQ_STATUS, >>>> .emul_con = EXYNOS5440_TMU_S0_7_DEBUG, >>>> .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT, >>>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h >>>> index dc7feb5..8788a87 100644 >>>> --- a/drivers/thermal/samsung/exynos_tmu_data.h >>>> +++ b/drivers/thermal/samsung/exynos_tmu_data.h >>>> @@ -69,9 +69,10 @@ >>>> #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_FALL_INT_SHIFT 16 >>>> #define EXYNOS_TMU_CLEAR_RISE_INT 0x111 >>>> #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 12) >>>> +#define EXYNOS_TMU_CLEAR_FALL_INT_SHIFT 12 >>>> #define EXYNOS_TMU_TRIP_MODE_SHIFT 13 >>>> #define EXYNOS_TMU_TRIP_MODE_MASK 0x7 >>>> #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT 12 > > > -- You have got to be excited about what you are doing. (L. Lamport) Eduardo Valentin