[U-Boot] [PATCH 2/2] imx: mx7: fix the temperature checking for Rev1.1

[Stefano Babic]

Hi Peng,

On 04/01/2016 06:16, Peng Fan wrote:
> To TO1.0, we can not rely on finish bit to read temperature. But to
> TO1.1, the issue was fixed by IC, we can rely on finish bit for
> temperature reading for TO1.1.
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Tim Harvey <tharvey at gateworks.com>
> Cc: Fabio Estevam <fabio.estevam at freescale.com>
> Cc: Adrian Alonso <aalonso at freescale.com>
> ---
>  drivers/thermal/imx_thermal.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 09a3c52..45d5953 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -169,18 +169,30 @@ static int read_cpu_temperature(struct udevice *dev)
>  	writel(TEMPMON_HW_ANADIG_TEMPSENSE1_FINISHED_MASK, &ccm_anatop->tempsense1_clr);
>  	writel(TEMPMON_HW_ANADIG_TEMPSENSE1_MEASURE_TEMP_MASK, &ccm_anatop->tempsense1_set);
>  
> -	start = get_timer(0);
> -	/* Wait max 100ms */
> -	do {
> -		/*
> -		 * Since we can not rely on finish bit, use 1ms delay to get
> -		 * temperature. From RM, 17us is enough to get data, but
> -		 * to gurantee to get the data, delay 100ms here.
> -		 */
> +	if (soc_rev() >= CHIP_REV_1_1) {
> +		/* make sure that the latest temp is valid */
> +		while ((readl(&ccm_anatop->tempsense1) &
> +		       TEMPMON_HW_ANADIG_TEMPSENSE1_FINISHED_MASK) == 0)
> +			udelay(10000);

If 10ms is the minimal time, you should poll with a timer < 10mS, else
you go out after 20mS. Processor is not doing anything, you can simply
poll the register more often.

>  		reg = readl(&ccm_anatop->tempsense1);
>  		tmp = (reg & TEMPMON_HW_ANADIG_TEMPSENSE1_TEMP_VALUE_MASK)
>  		       >> TEMPMON_HW_ANADIG_TEMPSENSE1_TEMP_VALUE_SHIFT;
> -	} while (get_timer(0) < (start + 100));
> +	} else {
> +		start = get_timer(0);
> +		/* Wait max 100ms */
> +		do {
> +			/*
> +			 * Since we can not rely on finish bit, use 100ms
> +			 * delay to get temperature. From RM, 17us is
> +			 * enough to get data, but to gurantee to get
> +			 * the data, delay 100ms here.
> +			 */
> +			reg = readl(&ccm_anatop->tempsense1);
> +			tmp = (reg &
> +			       TEMPMON_HW_ANADIG_TEMPSENSE1_TEMP_VALUE_MASK)
> +			       >> TEMPMON_HW_ANADIG_TEMPSENSE1_TEMP_VALUE_SHIFT;
> +		} while (get_timer(0) < (start + 100));
> +	}

This is nasty. You read the register, but you do not evaluate the result
and you wait always 100mS. This has drawbacks for the boottime, because
it is not so negligible. What is the reason to poll the register if you
simply wait for timeout ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================