linux-kernel - Re: [PATCH 1/3 v8] thermal: samsung: add intclr_fall_shift bit in exynos_tmu

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <1458475.5nSWLtVcXi@amdc1227>
Date:	Thu, 07 Nov 2013 15:47:45 +0100
From:	Tomasz Figa <t.figa@...sung.com>
To:	Naveen Krishna Chatradhi <ch.naveen@...sung.com>
Cc:	linux-pm@...r.kernel.org, naveenkrishna.ch@...il.com,
	rui.zhang@...el.com, eduardo.valentin@...com,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	amit.daniel@...sung.com, kgene.kim@...sung.com,
	devicetree@...r.kernel.org, b.zolnierkie@...sung.com,
	cpgs@...sung.com
Subject: Re: [PATCH 1/3 v8] thermal: samsung: add intclr_fall_shift bit in
 exynos_tmu_register struct

Hi Naveen,

On Thursday 07 of November 2013 11:22:42 Naveen Krishna Chatradhi wrote:
> On Exynos5250, the FALL interrupt related en, status and clear bits are
> available at an offset of
> 16 in INTEN, INTSTAT registers and at an offset of
> 12 in INTCLEAR register.
> 
> On Exynos5420, the FALL interrupt related en, status and clear bits are
> available at an offset of
> 16 in 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 introduces a new bit field intclr_fall_shift to handle the
> offset for exyns5250 and exynos5440
> Also removes the unused macros EXYNOS_TMU_FALL_INT_SHIFT and
> EXYNOS5440_TMU_FALL_INT_SHIFT, inten_fall_shift field

>From what I can see in this patch, the field intclr_fall_shift is not
really introduced, but rather inten_fall_shift is renamed to it. Please
match patch description with what the patch actually does.

I believe this patch is also touches code and data related to Exynos 4x12
SoCs, but the description only covers Exynos 5 SoCs.

In addition, if this patch does not introduce any functional changes,
but only refactors some code, the description should say so.

Also, please see my comment below.

> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@...sung.com>
> ---
> Changes since v1:
> Changes since v2:
> Changes since v3:
>   None
> Changes since v4:
>  Correct the CLEAR_FALL_INT_SHIFT for Exynos5250/Exynos5440
> Changes since v5:
>  Modify the commit message
> Changes since v6:
>  - Use EXYNOS_TMU_CLEAR_FALL_INT_SHIFT instead of EXYNOS5250_TMU_CLEAR_FALL_INT_SHIFT
>  as the same is being used for Exynos4412
> Changes since v7:
>  - also removes the unused macros EXYNOS_TMU_FALL_INT_SHIFT and
>  EXYNOS5440_TMU_FALL_INT_SHIFT, inten_fall_shift field
> 
>  
>  drivers/thermal/samsung/exynos_tmu.c      |    2 +-
>  drivers/thermal/samsung/exynos_tmu.h      |    4 ++--
>  drivers/thermal/samsung/exynos_tmu_data.c |    4 ++--
>  drivers/thermal/samsung/exynos_tmu_data.h |    4 ++--
>  4 files changed, 7 insertions(+), 7 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),

Shouldn't also the mask values be called intclr_*_mask? They seem to
be used only with tmu_intclear register. Same goes for inten_fall_shift,

Best regards,
Tomasz

--
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/