Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Zhang Rui]

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@xxxxxxxxx>
> > 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@xxxxxxxxx>
> 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@xxxxxxxxx>
> ---
> Â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)