On 08/25/2014 08:19 PM, Bartlomiej Zolnierkiewicz wrote: > On Monday, August 25, 2014 07:37:25 PM Chanwoo Choi wrote: >> Hi Bartlomiej, >> >> On 08/25/2014 07:15 PM, Bartlomiej Zolnierkiewicz wrote: >>> >>> Hi, >>> >>> On Monday, August 25, 2014 04:30:23 PM Chanwoo Choi wrote: >>>> This patch support many TRIMINFO_CTRL registers if specific Exynos SoC >>>> has one more TRIMINFO_CTRL registers. Also this patch uses proper 'RELOAD' >>>> shift/mask bit operation to set RELOAD feature instead of static value. >>>> >>>> Signed-off-by: Chanwoo Choi <[email protected]> >>>> Acked-by: Kyungmin Park <[email protected]> >>>> Cc: Zhang Rui <[email protected]> >>>> Cc: Eduardo Valentin <[email protected]> >>>> Cc: Amit Daniel Kachhap <[email protected]> >>>> Reviewed-by: Amit Daniel Kachhap <[email protected]> >>>> --- >>>> drivers/thermal/samsung/exynos_thermal_common.h | 1 + >>>> drivers/thermal/samsung/exynos_tmu.c | 23 >>>> ++++++++++++++++++++--- >>>> drivers/thermal/samsung/exynos_tmu.h | 9 +++++++-- >>>> drivers/thermal/samsung/exynos_tmu_data.c | 5 ++++- >>>> drivers/thermal/samsung/exynos_tmu_data.h | 3 +++ >>>> 5 files changed, 35 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/thermal/samsung/exynos_thermal_common.h >>>> b/drivers/thermal/samsung/exynos_thermal_common.h >>>> index 3eb2ed9..b211976 100644 >>>> --- a/drivers/thermal/samsung/exynos_thermal_common.h >>>> +++ b/drivers/thermal/samsung/exynos_thermal_common.h >>>> @@ -28,6 +28,7 @@ >>>> #define MAX_TRIP_COUNT 8 >>>> #define MAX_COOLING_DEVICE 4 >>>> #define MAX_THRESHOLD_LEVS 5 >>>> +#define MAX_TRIMINFO_CTRL_REG 2 >>>> >>>> #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 acbff14..7234f38 100644 >>>> --- a/drivers/thermal/samsung/exynos_tmu.c >>>> +++ b/drivers/thermal/samsung/exynos_tmu.c >>>> @@ -147,7 +147,7 @@ static int exynos_tmu_initialize(struct >>>> platform_device *pdev) >>>> struct exynos_tmu_data *data = platform_get_drvdata(pdev); >>>> struct exynos_tmu_platform_data *pdata = data->pdata; >>>> const struct exynos_tmu_registers *reg = pdata->registers; >>>> - unsigned int status, trim_info = 0, con; >>>> + unsigned int status, trim_info = 0, con, ctrl; >>>> unsigned int rising_threshold = 0, falling_threshold = 0; >>>> int ret = 0, threshold_code, i, trigger_levs = 0; >>>> >>>> @@ -164,8 +164,25 @@ static int exynos_tmu_initialize(struct >>>> platform_device *pdev) >>>> } >>>> } >>>> >>>> - if (TMU_SUPPORTS(pdata, TRIM_RELOAD)) >>>> - __raw_writel(1, data->base + reg->triminfo_ctrl); >>>> + if (TMU_SUPPORTS(pdata, TRIM_RELOAD)) { >>>> + if (reg->triminfo_ctrl_count > MAX_TRIMINFO_CTRL_REG) { >>> >>> Please remove this check and MAX_TRIMINFO_CTRL_REG define. >>> >>> We do not want such runtime checks for development time errors. >> >> OK, I'll remove it. >> >>> >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>>> + >>>> + for (i = 0; i < reg->triminfo_ctrl_count; i++) { >>>> + if (pdata->triminfo_reload[i]) { >>>> + ctrl = readl(data->base + >>>> + reg->triminfo_ctrl[i]); >>>> + ctrl &= ~(reg->triminfo_reload_mask << >>>> + reg->triminfo_reload_shift); >>>> + ctrl |= pdata->triminfo_reload[i] << >>>> + reg->triminfo_reload_shift; >>> >>> triminfo_reload_shift and triminfo_reload_mask variables have always >>> the same values when this code is run so there is no need for them. >> >> I don't understand. Do you mean that timinfo_reload_{shift/mask} variable is >> un-needed? > > Yes. > >> If you possible, I need more detailed comment. > > Currently triminfo_reload_shift is always "0" and triminfo_reload_mask > is "1" so there is no need to add an abstraction for different SoCs > (it should be added only when there is a real need for it). > > Please just rewrite this code as: > > if (pdata->triminfo_reload[i]) { > ctrl = readl(data->base + > reg->triminfo_ctrl[i]); > ctrl |= pdata->triminfo_reload[i]; > __raw_writel(ctrl, data->base + > reg->triminfo_ctrl[i]); > } > > Then you can remove unused triminfo_reload_shift and > EXYNOS_TRIMINFO_RELOAD_SHIFT. > > Please also include my patch (https://lkml.org/lkml/2014/8/20/481) in > your patch series (or at least mark it in the cover letter that my > patch should be merged before your patch #2/4).
OK. thanks for your comment. Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
