lkml.org 
[lkml]   [2018]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [GIT PULL] Thermal management updates for v4.17-rc1
From
Date
Hi, Eduardo,

On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> Hello,
>
> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> >
> > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui <rui.zhang@intel.com>
> > wrote:
> > >
> > >
> > > could you please illustrate me what the kconfig & warning is?
> > Just "make allmodconfig" and the warning is about a uninitialized
> > variable.
> >
> > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > history
> > is to be believed.
> >
> >                 Linus
> Yeah, this has also passed my local compilation error. Somehow my
> gcc4.9
> is not catching it. Using an older gcc (gcc4.6) does catch it.
>
> Anyways, given that the conversion functions are written to cover
> for unexpected cal_type, the right way of fixing this is to rewrite
> the conversion functions to allow for returning error codes and
> adjusting the callers as expected.
>
> Rui, bzolnier, please consider the following fix:
>
as it is late in this merge window, I'd prefer to
1. drop all the thermal-soc material in the first pull request which I
will send out soon.
2. you can prepare another pull request containing the thermal-soc
materials except the exynos fixes
3. exynos fixes with the problem solved can be queued for -rc2 or
later.

thanks,
rui

> From 2aaf94f80c0021a21b4122c9f4197acff08ea398 Mon Sep 17 00:00:00
> 2001
> From: Eduardo Valentin <edubezval@gmail.com>
> Date: Thu, 12 Apr 2018 21:00:48 -0700
> Subject: [PATCH 1/1] thermal: exynos: fix compilation warning around
>  conversion functions
>
> In order to fix the warns:
> drivers/thermal/samsung/exynos_tmu.c:931:37: warning: 'temp' may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/thermal/samsung/exynos_tmu.c:304:9: warning: 'temp_code' may
> be used uninitialized in this function [-Wmaybe-uninitialized]
>
> the conversion functions should allow return error codes
> and the not mix the converted value with error code.
>
> This patch change the conversion functions to return
> error code or success and adjusts the callers accordingly.
>
> Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 120 ++++++++++++++++++++++++-
> ----------
>  1 file changed, 84 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 2ec8548..b3f0704 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -282,52 +282,54 @@ static void exynos_report_trigger(struct
> exynos_tmu_data *p)
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the
> calibration type.
>   */
> -static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> +static int temp_to_code(struct exynos_tmu_data *data, u8 temp, int
> *temp_code)
>  {
> - int temp_code;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
> + *temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
>   (data->temp_error2 - data->temp_error1) /
>   (EXYNOS_SECOND_POINT_TRIM -
> EXYNOS_FIRST_POINT_TRIM) +
>   data->temp_error1;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp_code = temp + data->temp_error1 -
> EXYNOS_FIRST_POINT_TRIM;
> + *temp_code = temp + data->temp_error1 -
> EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:
>   WARN_ON(1);
> + ret = -EINVAL;
>   break;
>   }
>  
> - return temp_code;
> + return ret;
>  }
>  
>  /*
>   * Calculate a temperature value from a temperature code.
>   * The unit of the temperature is degree Celsius.
>   */
> -static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
> +static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code,
> int *temp)
>  {
> - int temp;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp = (temp_code - data->temp_error1) *
> + *temp = (temp_code - data->temp_error1) *
>   (EXYNOS_SECOND_POINT_TRIM -
> EXYNOS_FIRST_POINT_TRIM) /
>   (data->temp_error2 - data->temp_error1) +
>   EXYNOS_FIRST_POINT_TRIM;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp = temp_code - data->temp_error1 +
> EXYNOS_FIRST_POINT_TRIM;
> + *temp = temp_code - data->temp_error1 +
> EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:
>   WARN_ON(1);
> + ret = -EINVAL;
>   break;
>   }
>  
> - return temp;
> + return ret;
>  }
>  
>  static void sanitize_temp_error(struct exynos_tmu_data *data, u32
> trim_info)
> @@ -352,7 +354,7 @@ static u32 get_th_reg(struct exynos_tmu_data
> *data, u32 threshold, bool falling)
>   struct thermal_zone_device *tz = data->tzd;
>   const struct thermal_trip * const trips =
>   of_thermal_get_trip_points(tz);
> - unsigned long temp;
> + int temp;
>   int i;
>  
>   if (!trips) {
> @@ -362,6 +364,8 @@ static u32 get_th_reg(struct exynos_tmu_data
> *data, u32 threshold, bool falling)
>   }
>  
>   for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
> + int val, ret;
> +
>   if (trips[i].type == THERMAL_TRIP_CRITICAL)
>   continue;
>  
> @@ -371,7 +375,14 @@ static u32 get_th_reg(struct exynos_tmu_data
> *data, u32 threshold, bool falling)
>   else
>   threshold &= ~(0xff << 8 * i);
>  
> - threshold |= temp_to_code(data, temp) << 8 * i;
> + ret = temp_to_code(data, temp, &val);
> + if (ret) {
> + pr_err("%s: Convertion error from temp (%d)
> to code: %d!\n",
> + __func__, temp, ret);
> + return 0;
> + }
> +
> + threshold |= val << 8 * i;
>   }
>  
>   return threshold;
> @@ -460,11 +471,10 @@ static int exynos4210_tmu_initialize(struct
> platform_device *pdev)
>  
>   /* Write temperature code for threshold */
>   reference = trips[0].temperature / MCELSIUS;
> - threshold_code = temp_to_code(data, reference);
> - if (threshold_code < 0) {
> - ret = threshold_code;
> + ret = temp_to_code(data, reference, &threshold_code);
> + if (ret < 0 || threshold_code < 0)
>   goto out;
> - }
> +
>   writeb(threshold_code, data->base +
> EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
>  
>   for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
> @@ -537,7 +547,10 @@ static int exynos4412_tmu_initialize(struct
> platform_device *pdev)
>   goto out;
>   }
>  
> - threshold_code = temp_to_code(data, crit_temp / MCELSIUS);
> + ret = temp_to_code(data, crit_temp / MCELSIUS,
> &threshold_code);
> + if (ret)
> + goto out;
> +
>   /* 1-4 level to be assigned in th0 reg */
>   rising_threshold &= ~(0xff << 8 * i);
>   rising_threshold |= threshold_code << 8 * i;
> @@ -620,7 +633,9 @@ static int exynos5433_tmu_initialize(struct
> platform_device *pdev)
>   /* Write temperature code for rising threshold */
>   tz->ops->get_trip_temp(tz, i, &temp);
>   temp /= MCELSIUS;
> - threshold_code = temp_to_code(data, temp);
> + ret = temp_to_code(data, temp, &threshold_code);
> + if (ret)
> + goto out;
>  
>   rising_threshold = readl(data->base +
> rising_reg_offset);
>   rising_threshold |= (threshold_code << j * 8);
> @@ -629,7 +644,9 @@ static int exynos5433_tmu_initialize(struct
> platform_device *pdev)
>   /* Write temperature code for falling threshold */
>   tz->ops->get_trip_hyst(tz, i, &temp_hist);
>   temp_hist = temp - (temp_hist / MCELSIUS);
> - threshold_code = temp_to_code(data, temp_hist);
> + ret = temp_to_code(data, temp_hist,
> &threshold_code);
> + if (ret)
> + goto out;
>  
>   falling_threshold = readl(data->base +
> falling_reg_offset);
>   falling_threshold &= ~(0xff << j * 8);
> @@ -677,7 +694,12 @@ static int exynos5440_tmu_initialize(struct
> platform_device *pdev)
>  
>   /* if last threshold limit is also present */
>   if (!data->tzd->ops->get_crit_temp(data->tzd, &crit_temp)) {
> - threshold_code = temp_to_code(data, crit_temp /
> MCELSIUS);
> + int ret;
> +
> + ret = temp_to_code(data, crit_temp / MCELSIUS,
> &threshold_code);
> + if (ret)
> + return ret;
> +
>   /* 5th level to be assigned in th2 reg */
>   rising_threshold =
>   threshold_code <<
> EXYNOS5440_TMU_TH_RISE4_SHIFT;
> @@ -749,7 +771,10 @@ static int exynos7_tmu_initialize(struct
> platform_device *pdev)
>   temp_hist = temp - (temp_hist / MCELSIUS);
>  
>   /* Set 9-bit temperature code for rising threshold
> levels */
> - threshold_code = temp_to_code(data, temp);
> + ret = temp_to_code(data, temp, &threshold_code);
> + if (ret)
> + goto out;
> +
>   rising_threshold = readl(data->base +
>   EXYNOS7_THD_TEMP_RISE7_6 + reg_off);
>   rising_threshold &= ~(EXYNOS7_TMU_TEMP_MASK << (16 *
> bit_off));
> @@ -758,7 +783,9 @@ static int exynos7_tmu_initialize(struct
> platform_device *pdev)
>          data->base + EXYNOS7_THD_TEMP_RISE7_6 +
> reg_off);
>  
>   /* Set 9-bit temperature code for falling threshold
> levels */
> - threshold_code = temp_to_code(data, temp_hist);
> + ret = temp_to_code(data, temp_hist,
> &threshold_code);
> + if (ret)
> + goto out;
>   falling_threshold &= ~(EXYNOS7_TMU_TEMP_MASK << (16
> * bit_off));
>   falling_threshold |= threshold_code << (16 *
> bit_off);
>   writel(falling_threshold,
> @@ -925,11 +952,18 @@ static int exynos_get_temp(void *p, int *temp)
>   clk_enable(data->clk);
>  
>   value = data->tmu_read(data);
> - if (value < 0)
> + if (value < 0) {
>   ret = value;
> - else
> - *temp = code_to_temp(data, value) * MCELSIUS;
> + goto out;
> + }
> +
> + ret = code_to_temp(data, value, temp);
> + if (ret)
> + goto out;
>  
> + *temp *= MCELSIUS;
> +
> +out:
>   clk_disable(data->clk);
>   mutex_unlock(&data->lock);
>  
> @@ -937,9 +971,11 @@ static int exynos_get_temp(void *p, int *temp)
>  }
>  
>  #ifdef CONFIG_THERMAL_EMULATION
> -static u32 get_emul_con_reg(struct exynos_tmu_data *data, unsigned
> int val,
> -     int temp)
> +static int get_emul_con_reg(struct exynos_tmu_data *data, unsigned
> int val,
> +     int temp, u32 *con_reg)
>  {
> + int code, ret = 0;
> +
>   if (temp) {
>   temp /= MCELSIUS;
>  
> @@ -950,27 +986,36 @@ static u32 get_emul_con_reg(struct
> exynos_tmu_data *data, unsigned int val,
>   if (data->soc == SOC_ARCH_EXYNOS7) {
>   val &= ~(EXYNOS7_EMUL_DATA_MASK <<
>   EXYNOS7_EMUL_DATA_SHIFT);
> - val |= (temp_to_code(data, temp) <<
> - EXYNOS7_EMUL_DATA_SHIFT) |
> + ret = temp_to_code(data, temp, &code);
> + if (ret)
> + goto out;
> +
> + val |= (code << EXYNOS7_EMUL_DATA_SHIFT) |
>   EXYNOS_EMUL_ENABLE;
>   } else {
>   val &= ~(EXYNOS_EMUL_DATA_MASK <<
>   EXYNOS_EMUL_DATA_SHIFT);
> - val |= (temp_to_code(data, temp) <<
> - EXYNOS_EMUL_DATA_SHIFT) |
> + ret = temp_to_code(data, temp, &code);
> + if (ret)
> + goto out;
> +
> + val |= (code << EXYNOS_EMUL_DATA_SHIFT) |
>   EXYNOS_EMUL_ENABLE;
>   }
>   } else {
>   val &= ~EXYNOS_EMUL_ENABLE;
>   }
>  
> - return val;
> + *con_reg = val;
> +out:
> + return ret;
>  }
>  
>  static void exynos4412_tmu_set_emulation(struct exynos_tmu_data
> *data,
>    int temp)
>  {
>   unsigned int val;
> + int ret;
>   u32 emul_con;
>  
>   if (data->soc == SOC_ARCH_EXYNOS5260)
> @@ -983,18 +1028,21 @@ static void
> exynos4412_tmu_set_emulation(struct exynos_tmu_data *data,
>   emul_con = EXYNOS_EMUL_CON;
>  
>   val = readl(data->base + emul_con);
> - val = get_emul_con_reg(data, val, temp);
> - writel(val, data->base + emul_con);
> + ret = get_emul_con_reg(data, val, temp, &val);
> + if (!ret)
> + writel(val, data->base + emul_con);
>  }
>  
>  static void exynos5440_tmu_set_emulation(struct exynos_tmu_data
> *data,
>    int temp)
>  {
>   unsigned int val;
> + int ret;
>  
>   val = readl(data->base + EXYNOS5440_TMU_S0_7_DEBUG);
> - val = get_emul_con_reg(data, val, temp);
> - writel(val, data->base + EXYNOS5440_TMU_S0_7_DEBUG);
> + ret = get_emul_con_reg(data, val, temp, &val);
> + if (!ret)
> + writel(val, data->base + EXYNOS5440_TMU_S0_7_DEBUG);
>  }
>  
>  static int exynos_tmu_set_emulation(void *drv_data, int temp)

\
 
 \ /
  Last update: 2018-04-13 07:39    [W:1.093 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site