| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 13 Apr 2018 13:39:05 +0800
From: Zhang Rui <rui.zhang@...el.com>
To: Eduardo Valentin <edubezval@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux PM list <linux-pm@...r.kernel.org>,
"Li, Philip" <philip.li@...el.com>
Subject: Re: [GIT PULL] Thermal management updates for v4.17-rc1
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@...el.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@...il.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@...il.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)
Powered by blists - more mailing lists