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