diff mbox

[06/10] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()

Message ID 1399288539-1793-7-git-send-email-b.zolnierkie@samsung.com (mailing list archive)
State Superseded, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Bartlomiej Zolnierkiewicz May 5, 2014, 11:15 a.m. UTC
Remove runtime checks for negative return values of temp_to_code()
from exynos_tmu_initialize().  The current level temperature data
hardcoded in pdata will never cause a negative temp_to_code()
return values and for the new code potential mistakes should be
caught during development/review phases.

Theres should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

Comments

Eduardo Valentin May 15, 2014, 2:55 p.m. UTC | #1
Hello Bartlomiej,

On Mon, May 05, 2014 at 01:15:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Remove runtime checks for negative return values of temp_to_code()
> from exynos_tmu_initialize().  The current level temperature data
> hardcoded in pdata will never cause a negative temp_to_code()
> return values and for the new code potential mistakes should be
> caught during development/review phases.
> 
> Theres should be no functional changes caused by this patch.
> 

Same question as in previous patch. Removing defensive code must
be done carefully. 

> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 789d745..a415829 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	if (data->soc == SOC_ARCH_EXYNOS4210) {
>  		/* Write temperature code for threshold */
>  		threshold_code = temp_to_code(data, pdata->threshold);
> -		if (threshold_code < 0) {
> -			ret = threshold_code;
> -			goto out;
> -		}
>  		writeb(threshold_code,
>  			data->base + reg->threshold_temp);
>  		for (i = 0; i < trigger_levs; i++)
> @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			rising_threshold |= threshold_code << 8 * i;
>  			if (pdata->threshold_falling) {
>  				threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i] -
>  						pdata->threshold_falling);
> -				if (threshold_code > 0)
> -					falling_threshold |=
> -						threshold_code << 8 * i;
> +				falling_threshold |= threshold_code << 8 * i;
>  			}
>  		}
>  
> @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  				(pdata->trigger_type[i] == HW_TRIP)) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
>  				/* 1-4 level to be assigned in th0 reg */
>  				rising_threshold |= threshold_code << 8 * i;
> -- 
> 1.8.2.3
> 
> --
> 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
--
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
Bartlomiej Zolnierkiewicz May 15, 2014, 4:56 p.m. UTC | #2
On Thursday, May 15, 2014 10:55:52 AM Eduardo Valentin wrote:
> Hello Bartlomiej,

Hi,

> On Mon, May 05, 2014 at 01:15:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Remove runtime checks for negative return values of temp_to_code()
> > from exynos_tmu_initialize().  The current level temperature data
> > hardcoded in pdata will never cause a negative temp_to_code()
> > return values and for the new code potential mistakes should be
> > caught during development/review phases.
> > 
> > Theres should be no functional changes caused by this patch.
> > 
> 
> Same question as in previous patch. Removing defensive code must

Simirarly like in a previous case.  Such verification should not be done
at runtime in a production code because it is already too late for such
checking.  It should be done during development and review phases.

> be done carefully. 

BTW In case of temp_to_code() its users should be audited to pass only
input values that give positive results and later the function itself
may be modified to catch wrong input values by using WARN_ON (or even
BUG_ON).

> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > ---
> >  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
> >  1 file changed, 1 insertion(+), 15 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> > index 789d745..a415829 100644
> > --- a/drivers/thermal/samsung/exynos_tmu.c
> > +++ b/drivers/thermal/samsung/exynos_tmu.c
> > @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  	if (data->soc == SOC_ARCH_EXYNOS4210) {
> >  		/* Write temperature code for threshold */
> >  		threshold_code = temp_to_code(data, pdata->threshold);
> > -		if (threshold_code < 0) {
> > -			ret = threshold_code;
> > -			goto out;
> > -		}
> >  		writeb(threshold_code,
> >  			data->base + reg->threshold_temp);
> >  		for (i = 0; i < trigger_levs; i++)
> > @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
> >  			threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i]);
> > -			if (threshold_code < 0) {
> > -				ret = threshold_code;
> > -				goto out;
> > -			}
> >  			rising_threshold |= threshold_code << 8 * i;
> >  			if (pdata->threshold_falling) {
> >  				threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i] -
> >  						pdata->threshold_falling);
> > -				if (threshold_code > 0)
> > -					falling_threshold |=
> > -						threshold_code << 8 * i;
> > +				falling_threshold |= threshold_code << 8 * i;
> >  			}
> >  		}
> >  
> > @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
> >  				(pdata->trigger_type[i] == HW_TRIP)) {
> >  			threshold_code = temp_to_code(data,
> >  						pdata->trigger_levels[i]);
> > -			if (threshold_code < 0) {
> > -				ret = threshold_code;
> > -				goto out;
> > -			}
> >  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
> >  				/* 1-4 level to be assigned in th0 reg */
> >  				rising_threshold |= threshold_code << 8 * i;

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

--
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
amit kachhap May 19, 2014, 5:50 a.m. UTC | #3
On 5/5/14, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote:
> Remove runtime checks for negative return values of temp_to_code()
> from exynos_tmu_initialize().  The current level temperature data
> hardcoded in pdata will never cause a negative temp_to_code()
> return values and for the new code potential mistakes should be
> caught during development/review phases.
Your arguments looks fine.
>
> Theres should be no functional changes caused by this patch.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> ---
>  drivers/thermal/samsung/exynos_tmu.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 789d745..a415829 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -170,10 +170,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  	if (data->soc == SOC_ARCH_EXYNOS4210) {
>  		/* Write temperature code for threshold */
>  		threshold_code = temp_to_code(data, pdata->threshold);
> -		if (threshold_code < 0) {
> -			ret = threshold_code;
> -			goto out;
> -		}
>  		writeb(threshold_code,
>  			data->base + reg->threshold_temp);
>  		for (i = 0; i < trigger_levs; i++)
> @@ -187,18 +183,12 @@ static int exynos_tmu_initialize(struct
> platform_device *pdev)
>  		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			rising_threshold |= threshold_code << 8 * i;
>  			if (pdata->threshold_falling) {
>  				threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i] -
>  						pdata->threshold_falling);
> -				if (threshold_code > 0)
> -					falling_threshold |=
> -						threshold_code << 8 * i;
> +				falling_threshold |= threshold_code << 8 * i;
>  			}
>  		}
>
> @@ -217,10 +207,6 @@ static int exynos_tmu_initialize(struct platform_device
> *pdev)
>  				(pdata->trigger_type[i] == HW_TRIP)) {
>  			threshold_code = temp_to_code(data,
>  						pdata->trigger_levels[i]);
> -			if (threshold_code < 0) {
> -				ret = threshold_code;
> -				goto out;
> -			}
>  			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
>  				/* 1-4 level to be assigned in th0 reg */
>  				rising_threshold |= threshold_code << 8 * i;
> --
> 1.8.2.3
>
> --
> 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
>
--
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 789d745..a415829 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -170,10 +170,6 @@  static int exynos_tmu_initialize(struct platform_device *pdev)
 	if (data->soc == SOC_ARCH_EXYNOS4210) {
 		/* Write temperature code for threshold */
 		threshold_code = temp_to_code(data, pdata->threshold);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
 		writeb(threshold_code,
 			data->base + reg->threshold_temp);
 		for (i = 0; i < trigger_levs; i++)
@@ -187,18 +183,12 @@  static int exynos_tmu_initialize(struct platform_device *pdev)
 		i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
 			threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i]);
-			if (threshold_code < 0) {
-				ret = threshold_code;
-				goto out;
-			}
 			rising_threshold |= threshold_code << 8 * i;
 			if (pdata->threshold_falling) {
 				threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i] -
 						pdata->threshold_falling);
-				if (threshold_code > 0)
-					falling_threshold |=
-						threshold_code << 8 * i;
+				falling_threshold |= threshold_code << 8 * i;
 			}
 		}
 
@@ -217,10 +207,6 @@  static int exynos_tmu_initialize(struct platform_device *pdev)
 				(pdata->trigger_type[i] == HW_TRIP)) {
 			threshold_code = temp_to_code(data,
 						pdata->trigger_levels[i]);
-			if (threshold_code < 0) {
-				ret = threshold_code;
-				goto out;
-			}
 			if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
 				/* 1-4 level to be assigned in th0 reg */
 				rising_threshold |= threshold_code << 8 * i;