Messages in this thread | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH 05/10] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize() | Date | Thu, 15 May 2014 18:24:14 +0200 |
| |
On Thursday, May 15, 2014 10:47:40 AM Eduardo Valentin wrote: > Hello Bartlomiej,
Hi,
> On Mon, May 05, 2014 at 01:15:34PM +0200, Bartlomiej Zolnierkiewicz wrote: > > Remove runtime checks for pdata sanity from exynos_tmu_initialize(). > > The current values hardcoded in pdata will never trigger the checks > > and for the new code potential mistakes should be caught during > > development/review phases. > > > > There should be no functional changes caused by this patch. > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > drivers/thermal/samsung/exynos_thermal_common.h | 1 - > > drivers/thermal/samsung/exynos_tmu.c | 13 ------------- > > 2 files changed, 14 deletions(-) > > > > diff --git a/drivers/thermal/samsung/exynos_thermal_common.h b/drivers/thermal/samsung/exynos_thermal_common.h > > index 3eb2ed9..cd44719 100644 > > --- a/drivers/thermal/samsung/exynos_thermal_common.h > > +++ b/drivers/thermal/samsung/exynos_thermal_common.h > > @@ -27,7 +27,6 @@ > > #define SENSOR_NAME_LEN 16 > > #define MAX_TRIP_COUNT 8 > > #define MAX_COOLING_DEVICE 4 > > -#define MAX_THRESHOLD_LEVS 5 > > > > #define ACTIVE_INTERVAL 500 > > #define IDLE_INTERVAL 10000 > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > > index 903566f..789d745 100644 > > --- a/drivers/thermal/samsung/exynos_tmu.c > > +++ b/drivers/thermal/samsung/exynos_tmu.c > > @@ -158,23 +158,10 @@ static int exynos_tmu_initialize(struct platform_device *pdev) > > data->temp_error > pdata->max_efuse_value) > > data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK; > > > > - if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) { > > - dev_err(&pdev->dev, "Invalid max trigger level\n"); > > - ret = -EINVAL; > > - goto out; > > - } > > - > > for (i = 0; i < pdata->max_trigger_level; i++) { > > if (!pdata->trigger_levels[i]) > > continue; > > > > - if ((pdata->trigger_type[i] == HW_TRIP) && > > - (!pdata->trigger_levels[pdata->max_trigger_level - 1])) { > > - dev_err(&pdev->dev, "Invalid hw trigger level\n"); > > - ret = -EINVAL; > > - goto out; > > - } > > - > > Does it mean no new pdata are going to be written? i.e., no new soc is > going to be supported by this driver that needs proper pdata checking?
This is not a proper checking. The checks in question are done at runtime in a production code for data that is hardcoded inside driver during development time and later it doesn't change. Such data should be verified during development and review time (i.e. by a script parsing relevant data from exynos_tmu_data.c, one can also argue that verification to be done is so simple that the review by a maintainer should be enough).
> > /* Count trigger levels except the HW trip*/ > > if (!(pdata->trigger_type[i] == HW_TRIP)) > > trigger_levs++;
Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
| |