Re: [PATCH 1/3 v5] thermal: samsung: correct the fall interrupt en, status bit fields

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

[ I was a bit too quick with hitting the Send button.. ]

On Wednesday, October 16, 2013 12:06:21 PM Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> On Wednesday, October 16, 2013 08:21:30 AM Naveen Krishna Chatradhi wrote:
> > On Exynos5250, the FALL interrupt related en, status and clear bits are
> > available at an offset of
> > 16 on INTEN, INTSTAT registers and at an offset of
> > 12 on INTCLEAR register.
> > 
> > On Exynos5420, the FALL interrupt related en, status and clear bits are
> > available at an offset of
> > 16 on INTEN, INTSTAT and INTCLEAR registers.
> > 
> > On Exynos5440,
> > the FALL_IRQEN bits are at an offset of 4
> > and the RISE_IRQEN bits are at an offset of 0
> > 
> > This patch corrects the same for exyns5250 and exynos5440
> 
> No, it doesn't correct anything on EXYNOS5250 or EXYNOS5440. It just
> does rename of used exynos_tmu_registers register and related defines.

s/register/field/

> IOW This patch is just a preparation patch for EXYNOS5420 support, on

Sorry, it isn't even that. This patch is just a cleanup patch and should
be presented as such.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> its own it shouldn't cause any functionality changes.
> 
> Please update the patch description to mention this.
> 
> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> > ---
> > Changes since v1:
> > Changes since v2:
> > Changes since v3:
> >   None
> > Changes since v4:
> > Correct the CLEAR_FALL_INT_SHIFT for Exynos5250/Exynos5440
> > 
> >  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 |    4 +++-
> >  4 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 32f38b9..b2202fa 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 3fb6554..5f4fe6c 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.h
> > +++ b/drivers/thermal/samsung/exynos_tmu.h
> > @@ -136,6 +136,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.
> > @@ -207,6 +208,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 073c292..09a8a27 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.c
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> > @@ -123,6 +123,7 @@ static const struct exynos_tmu_registers exynos4412_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 = EXYNOS5250_TMU_CLEAR_FALL_INT_SHIFT,
> >  	.emul_con = EXYNOS_EMUL_CON,
> >  	.emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
> >  	.emul_time_shift = EXYNOS_EMUL_TIME_SHIFT,
> > @@ -228,6 +229,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 = EXYNOS5440_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 a1ea19d..9c1e2c8 100644
> > --- a/drivers/thermal/samsung/exynos_tmu_data.h
> > +++ b/drivers/thermal/samsung/exynos_tmu_data.h
> > @@ -69,9 +69,11 @@
> >  #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 EXYNOS5250_TMU_CLEAR_FALL_INT_SHIFT	12
> 
> The better name would be EXYNOS_TMU_CLEAR_FALL_INT_SHIFT because
> it is also used on EXYNOS4412.
> 
> Also in patch #3 you should define EXYNOS5420_TMU_CLEAR_FALL_INT_SHIFT
> instead of re-using EXYNOS_TMU_FALL_INT_SHIFT.
> 
> In this patch (#1) EXYNOS*_TMU_FALL_INT_SHIFT should be removed together
> with no longer needed inten_fall_shift.
> 
> > +#define EXYNOS5440_TMU_CLEAR_FALL_INT_SHIFT	4
> >  #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
> >  #define EXYNOS_TMU_TRIP_MODE_MASK	0x7
> >  #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT	12
> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics