[PATCH/RFT v2 0/3] thermal: rcar_gen3_thermal: Update calculation formula due to HW evaluation

[PATCH/RFT v2 0/3] thermal: rcar_gen3_thermal: Update calculation formula due to HW evaluation

[Yoshihiro Kaneko]

This series was inspired by a patch in the BSP by Dien Pham <dien.pham.ry@renesas.com>.

This series is based on the master branch of Linus Torvalds's linux tree.

v2 [Yoshihiro Kaneko]
* As suggested by Simon Horman
- split into three patches
- avoid global variables
- marked thcode const

Yoshihiro Kaneko (3):
  thermal: rcar_gen3_thermal: Update value of Tj_1
  thermal: rcar_gen3_thermal: Update calculation formula of IRQTEMP
  thermal: rcar_gen3_thermal: Update temperature conversion method

 drivers/thermal/rcar_gen3_thermal.c | 93 ++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 32 deletions(-)

-- 
1.9.1

[PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Yoshihiro Kaneko]

As evaluation of hardware team, temperature calculation formula
of M3-W is difference from all other SoCs as below:
- M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
- Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)

Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 88fa41c..a2fd0fd 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
 #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
 
 /* no idea where these constants come from */
-#define TJ_1 116
 #define TJ_3 -41
 
 static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
-					 int *ptat, int *thcode)
+					 int *ptat, int *thcode,
+					 unsigned int ths_tj_1)
 {
 	int tj_2;
 
@@ -139,15 +139,15 @@ static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
 	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
 	 */
 	tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
-		/ (ptat[0] - ptat[2])) - FIXPT_INT(41);
+		/ (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
 
 	coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
 			     tj_2 - FIXPT_INT(TJ_3));
 	coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
 
 	coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
-			     tj_2 - FIXPT_INT(TJ_1));
-	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * TJ_1;
+			     tj_2 - FIXPT_INT(ths_tj_1));
+	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
 }
 
 static int rcar_gen3_thermal_round(int temp)
@@ -318,12 +318,29 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
 	usleep_range(1000, 2000);
 }
 
+static const unsigned int rcar_gen3_ths_tj_1 = 126;
+static const unsigned int rcar_gen3_ths_tj_1_m3_w = 116;
 static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
-	{ .compatible = "renesas,r8a774a1-thermal", },
-	{ .compatible = "renesas,r8a7795-thermal", },
-	{ .compatible = "renesas,r8a7796-thermal", },
-	{ .compatible = "renesas,r8a77965-thermal", },
-	{ .compatible = "renesas,r8a77980-thermal", },
+	{
+		.compatible = "renesas,r8a774a1-thermal",
+		.data = &rcar_gen3_ths_tj_1_m3_w,
+	},
+	{
+		.compatible = "renesas,r8a7795-thermal",
+		.data = &rcar_gen3_ths_tj_1,
+	},
+	{
+		.compatible = "renesas,r8a7796-thermal",
+		.data = &rcar_gen3_ths_tj_1_m3_w,
+	},
+	{
+		.compatible = "renesas,r8a77965-thermal",
+		.data = &rcar_gen3_ths_tj_1,
+	},
+	{
+		.compatible = "renesas,r8a77980-thermal",
+		.data = &rcar_gen3_ths_tj_1,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
@@ -349,6 +366,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 {
 	struct rcar_gen3_thermal_priv *priv;
 	struct device *dev = &pdev->dev;
+	const unsigned int *rcar_gen3_ths_tj_1 = of_device_get_match_data(dev);
 	struct resource *res;
 	struct thermal_zone_device *zone;
 	int ret, irq, i;
@@ -422,7 +440,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		priv->tscs[i] = tsc;
 
 		priv->thermal_init(tsc);
-		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
+		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
+					     *rcar_gen3_ths_tj_1);
 
 		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
 							    &rcar_gen3_tz_of_ops);
-- 
1.9.1

Re: [PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Niklas Söderlund]

Hi Kaneko-san,

Thanks for your work.

On 2019-04-17 02:44:13 +0900, Yoshihiro Kaneko wrote:
> As evaluation of hardware team, temperature calculation formula
> of M3-W is difference from all other SoCs as below:
> - M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
> - Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)
> 
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 88fa41c..a2fd0fd 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
>  #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
>  
>  /* no idea where these constants come from */
> -#define TJ_1 116
>  #define TJ_3 -41
>  
>  static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> -					 int *ptat, int *thcode)
> +					 int *ptat, int *thcode,
> +					 unsigned int ths_tj_1)

I would move tj_1 inside struce rcar_gen3_thermal_tsc as you in 2/3 move 
tj_2 there. You could still keep the value in .data but init the tj_1 in 
the struct at probe instead of passing it as an argument.

>  {
>  	int tj_2;
>  
> @@ -139,15 +139,15 @@ static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
>  	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
>  	 */
>  	tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> -		/ (ptat[0] - ptat[2])) - FIXPT_INT(41);
> +		/ (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
>  
>  	coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
>  			     tj_2 - FIXPT_INT(TJ_3));
>  	coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
>  
>  	coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> -			     tj_2 - FIXPT_INT(TJ_1));
> -	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * TJ_1;
> +			     tj_2 - FIXPT_INT(ths_tj_1));
> +	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
>  }
>  
>  static int rcar_gen3_thermal_round(int temp)
> @@ -318,12 +318,29 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>  	usleep_range(1000, 2000);
>  }
>  
> +static const unsigned int rcar_gen3_ths_tj_1 = 126;
> +static const unsigned int rcar_gen3_ths_tj_1_m3_w = 116;
>  static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> -	{ .compatible = "renesas,r8a774a1-thermal", },
> -	{ .compatible = "renesas,r8a7795-thermal", },
> -	{ .compatible = "renesas,r8a7796-thermal", },
> -	{ .compatible = "renesas,r8a77965-thermal", },
> -	{ .compatible = "renesas,r8a77980-thermal", },
> +	{
> +		.compatible = "renesas,r8a774a1-thermal",
> +		.data = &rcar_gen3_ths_tj_1_m3_w,
> +	},
> +	{
> +		.compatible = "renesas,r8a7795-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
> +	{
> +		.compatible = "renesas,r8a7796-thermal",
> +		.data = &rcar_gen3_ths_tj_1_m3_w,
> +	},
> +	{
> +		.compatible = "renesas,r8a77965-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
> +	{
> +		.compatible = "renesas,r8a77980-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> @@ -349,6 +366,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  {
>  	struct rcar_gen3_thermal_priv *priv;
>  	struct device *dev = &pdev->dev;
> +	const unsigned int *rcar_gen3_ths_tj_1 = of_device_get_match_data(dev);
>  	struct resource *res;
>  	struct thermal_zone_device *zone;
>  	int ret, irq, i;
> @@ -422,7 +440,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  		priv->tscs[i] = tsc;
>  
>  		priv->thermal_init(tsc);
> -		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> +		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
> +					     *rcar_gen3_ths_tj_1);
>  
>  		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
>  							    &rcar_gen3_tz_of_ops);
> -- 
> 1.9.1
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Yoshihiro Kaneko]

Hi Niklas-san,

Thanks for your review!

2019年4月18日(木) 17:40 Niklas Söderlund <niklas.soderlund@ragnatech.se>:
>
> Hi Kaneko-san,
>
> Thanks for your work.
>
> On 2019-04-17 02:44:13 +0900, Yoshihiro Kaneko wrote:
> > As evaluation of hardware team, temperature calculation formula
> > of M3-W is difference from all other SoCs as below:
> > - M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
> > - Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)
> >
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > ---
> >  drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
> >  1 file changed, 30 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> > index 88fa41c..a2fd0fd 100644
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> >  #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
> >
> >  /* no idea where these constants come from */
> > -#define TJ_1 116
> >  #define TJ_3 -41
> >
> >  static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> > -                                      int *ptat, int *thcode)
> > +                                      int *ptat, int *thcode,
> > +                                      unsigned int ths_tj_1)
>
> I would move tj_1 inside struce rcar_gen3_thermal_tsc as you in 2/3 move
> tj_2 there. You could still keep the value in .data but init the tj_1 in
> the struct at probe instead of passing it as an argument.

This function is a simple subroutine, is not called back.
Therefore, I think that it is not necessary to move tj_1 into
rcar_gen3_thermal_tsc.

Thanks,
Kaneko

>
> >  {
> >       int tj_2;
> >
> > @@ -139,15 +139,15 @@ static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> >        * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
> >        */
> >       tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> > -             / (ptat[0] - ptat[2])) - FIXPT_INT(41);
> > +             / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
> >
> >       coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
> >                            tj_2 - FIXPT_INT(TJ_3));
> >       coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
> >
> >       coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> > -                          tj_2 - FIXPT_INT(TJ_1));
> > -     coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * TJ_1;
> > +                          tj_2 - FIXPT_INT(ths_tj_1));
> > +     coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
> >  }
> >
> >  static int rcar_gen3_thermal_round(int temp)
> > @@ -318,12 +318,29 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> >       usleep_range(1000, 2000);
> >  }
> >
> > +static const unsigned int rcar_gen3_ths_tj_1 = 126;
> > +static const unsigned int rcar_gen3_ths_tj_1_m3_w = 116;
> >  static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> > -     { .compatible = "renesas,r8a774a1-thermal", },
> > -     { .compatible = "renesas,r8a7795-thermal", },
> > -     { .compatible = "renesas,r8a7796-thermal", },
> > -     { .compatible = "renesas,r8a77965-thermal", },
> > -     { .compatible = "renesas,r8a77980-thermal", },
> > +     {
> > +             .compatible = "renesas,r8a774a1-thermal",
> > +             .data = &rcar_gen3_ths_tj_1_m3_w,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a7795-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a7796-thermal",
> > +             .data = &rcar_gen3_ths_tj_1_m3_w,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a77965-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a77980-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> >       {},
> >  };
> >  MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> > @@ -349,6 +366,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> >  {
> >       struct rcar_gen3_thermal_priv *priv;
> >       struct device *dev = &pdev->dev;
> > +     const unsigned int *rcar_gen3_ths_tj_1 = of_device_get_match_data(dev);
> >       struct resource *res;
> >       struct thermal_zone_device *zone;
> >       int ret, irq, i;
> > @@ -422,7 +440,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> >               priv->tscs[i] = tsc;
> >
> >               priv->thermal_init(tsc);
> > -             rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> > +             rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
> > +                                          *rcar_gen3_ths_tj_1);
> >
> >               zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> >                                                           &rcar_gen3_tz_of_ops);
> > --
> > 1.9.1
> >
>
> --
> Regards,
> Niklas Söderlund

Re: [PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Simon Horman]

Hi Kaneko-san,

On Wed, Apr 17, 2019 at 02:44:13AM +0900, Yoshihiro Kaneko wrote:
> As evaluation of hardware team, temperature calculation formula
> of M3-W is difference from all other SoCs as below:
> - M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
> - Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)
> 
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index 88fa41c..a2fd0fd 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
>  #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
>  
>  /* no idea where these constants come from */
> -#define TJ_1 116
>  #define TJ_3 -41
>  
>  static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> -					 int *ptat, int *thcode)
> +					 int *ptat, int *thcode,
> +					 unsigned int ths_tj_1)

While testing I found that the type of ths_tj_1 needs to be int
rather than unsigned int, in order for the FIXPT logic to work correctly.

And with that change in place the entire series appears to work correctly.

My suggestion is to change the types of ths_tj_1 here, rcar_gen3_ths_tj_1
in rcar_gen3_thermal_probe(), and rcar_gen3_ths_tj_1 and
rcar_gen3_ths_tj_1_m3_w, which are gloabl to this file accordingly.

>  {
>  	int tj_2;
>  
> @@ -139,15 +139,15 @@ static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
>  	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
>  	 */
>  	tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> -		/ (ptat[0] - ptat[2])) - FIXPT_INT(41);
> +		/ (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
>  
>  	coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
>  			     tj_2 - FIXPT_INT(TJ_3));
>  	coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
>  
>  	coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> -			     tj_2 - FIXPT_INT(TJ_1));
> -	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * TJ_1;
> +			     tj_2 - FIXPT_INT(ths_tj_1));
> +	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
>  }
>  
>  static int rcar_gen3_thermal_round(int temp)
> @@ -318,12 +318,29 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>  	usleep_range(1000, 2000);
>  }
>  
> +static const unsigned int rcar_gen3_ths_tj_1 = 126;
> +static const unsigned int rcar_gen3_ths_tj_1_m3_w = 116;
>  static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> -	{ .compatible = "renesas,r8a774a1-thermal", },
> -	{ .compatible = "renesas,r8a7795-thermal", },
> -	{ .compatible = "renesas,r8a7796-thermal", },
> -	{ .compatible = "renesas,r8a77965-thermal", },
> -	{ .compatible = "renesas,r8a77980-thermal", },
> +	{
> +		.compatible = "renesas,r8a774a1-thermal",
> +		.data = &rcar_gen3_ths_tj_1_m3_w,
> +	},
> +	{
> +		.compatible = "renesas,r8a7795-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
> +	{
> +		.compatible = "renesas,r8a7796-thermal",
> +		.data = &rcar_gen3_ths_tj_1_m3_w,
> +	},
> +	{
> +		.compatible = "renesas,r8a77965-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
> +	{
> +		.compatible = "renesas,r8a77980-thermal",
> +		.data = &rcar_gen3_ths_tj_1,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> @@ -349,6 +366,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  {
>  	struct rcar_gen3_thermal_priv *priv;
>  	struct device *dev = &pdev->dev;
> +	const unsigned int *rcar_gen3_ths_tj_1 = of_device_get_match_data(dev);
>  	struct resource *res;
>  	struct thermal_zone_device *zone;
>  	int ret, irq, i;
> @@ -422,7 +440,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  		priv->tscs[i] = tsc;
>  
>  		priv->thermal_init(tsc);
> -		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> +		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
> +					     *rcar_gen3_ths_tj_1);
>  
>  		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
>  							    &rcar_gen3_tz_of_ops);
> -- 
> 1.9.1
> 

Re: [PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Yoshihiro Kaneko]

Hi Simon-san,

Thanks for testing this patch!

2019年4月24日(水) 23:54 Simon Horman <horms@verge.net.au>:
>
> Hi Kaneko-san,
>
> On Wed, Apr 17, 2019 at 02:44:13AM +0900, Yoshihiro Kaneko wrote:
> > As evaluation of hardware team, temperature calculation formula
> > of M3-W is difference from all other SoCs as below:
> > - M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
> > - Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)
> >
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > ---
> >  drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
> >  1 file changed, 30 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> > index 88fa41c..a2fd0fd 100644
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> >  #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
> >
> >  /* no idea where these constants come from */
> > -#define TJ_1 116
> >  #define TJ_3 -41
> >
> >  static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> > -                                      int *ptat, int *thcode)
> > +                                      int *ptat, int *thcode,
> > +                                      unsigned int ths_tj_1)
>
> While testing I found that the type of ths_tj_1 needs to be int
> rather than unsigned int, in order for the FIXPT logic to work correctly.
>
> And with that change in place the entire series appears to work correctly.
>
> My suggestion is to change the types of ths_tj_1 here, rcar_gen3_ths_tj_1
> in rcar_gen3_thermal_probe(), and rcar_gen3_ths_tj_1 and
> rcar_gen3_ths_tj_1_m3_w, which are gloabl to this file accordingly.

I understood. Why did I decide to use unsigned?
I will fix it in v3.

Thanks,
Kaneko

>
> >  {
> >       int tj_2;
> >
> > @@ -139,15 +139,15 @@ static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> >        * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
> >        */
> >       tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> > -             / (ptat[0] - ptat[2])) - FIXPT_INT(41);
> > +             / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
> >
> >       coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
> >                            tj_2 - FIXPT_INT(TJ_3));
> >       coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
> >
> >       coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> > -                          tj_2 - FIXPT_INT(TJ_1));
> > -     coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * TJ_1;
> > +                          tj_2 - FIXPT_INT(ths_tj_1));
> > +     coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
> >  }
> >
> >  static int rcar_gen3_thermal_round(int temp)
> > @@ -318,12 +318,29 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> >       usleep_range(1000, 2000);
> >  }
> >
> > +static const unsigned int rcar_gen3_ths_tj_1 = 126;
> > +static const unsigned int rcar_gen3_ths_tj_1_m3_w = 116;
> >  static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> > -     { .compatible = "renesas,r8a774a1-thermal", },
> > -     { .compatible = "renesas,r8a7795-thermal", },
> > -     { .compatible = "renesas,r8a7796-thermal", },
> > -     { .compatible = "renesas,r8a77965-thermal", },
> > -     { .compatible = "renesas,r8a77980-thermal", },
> > +     {
> > +             .compatible = "renesas,r8a774a1-thermal",
> > +             .data = &rcar_gen3_ths_tj_1_m3_w,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a7795-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a7796-thermal",
> > +             .data = &rcar_gen3_ths_tj_1_m3_w,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a77965-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> > +     {
> > +             .compatible = "renesas,r8a77980-thermal",
> > +             .data = &rcar_gen3_ths_tj_1,
> > +     },
> >       {},
> >  };
> >  MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);
> > @@ -349,6 +366,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> >  {
> >       struct rcar_gen3_thermal_priv *priv;
> >       struct device *dev = &pdev->dev;
> > +     const unsigned int *rcar_gen3_ths_tj_1 = of_device_get_match_data(dev);
> >       struct resource *res;
> >       struct thermal_zone_device *zone;
> >       int ret, irq, i;
> > @@ -422,7 +440,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> >               priv->tscs[i] = tsc;
> >
> >               priv->thermal_init(tsc);
> > -             rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i]);
> > +             rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
> > +                                          *rcar_gen3_ths_tj_1);
> >
> >               zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> >                                                           &rcar_gen3_tz_of_ops);
> > --
> > 1.9.1
> >

Re: [PATCH/RFT v2 1/3] thermal: rcar_gen3_thermal: Update value of Tj_1

[Simon Horman]

On Wed, May 08, 2019 at 03:36:36PM +0900, Yoshihiro Kaneko wrote:
> Hi Simon-san,
> 
> Thanks for testing this patch!
> 
> 2019年4月24日(水) 23:54 Simon Horman <horms@verge.net.au>:
> >
> > Hi Kaneko-san,
> >
> > On Wed, Apr 17, 2019 at 02:44:13AM +0900, Yoshihiro Kaneko wrote:
> > > As evaluation of hardware team, temperature calculation formula
> > > of M3-W is difference from all other SoCs as below:
> > > - M3-W: Tj_1: 116 (so Tj_1 - Tj_3 = 157)
> > > - Others: Tj_1: 126 (so Tj_1 - Tj_3 = 167)
> > >
> > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > > ---
> > >  drivers/thermal/rcar_gen3_thermal.c | 41 +++++++++++++++++++++++++++----------
> > >  1 file changed, 30 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> > > index 88fa41c..a2fd0fd 100644
> > > --- a/drivers/thermal/rcar_gen3_thermal.c
> > > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > > @@ -124,11 +124,11 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
> > >  #define RCAR3_THERMAL_GRAN 500 /* mili Celsius */
> > >
> > >  /* no idea where these constants come from */
> > > -#define TJ_1 116
> > >  #define TJ_3 -41
> > >
> > >  static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> > > -                                      int *ptat, int *thcode)
> > > +                                      int *ptat, int *thcode,
> > > +                                      unsigned int ths_tj_1)
> >
> > While testing I found that the type of ths_tj_1 needs to be int
> > rather than unsigned int, in order for the FIXPT logic to work correctly.
> >
> > And with that change in place the entire series appears to work correctly.
> >
> > My suggestion is to change the types of ths_tj_1 here, rcar_gen3_ths_tj_1
> > in rcar_gen3_thermal_probe(), and rcar_gen3_ths_tj_1 and
> > rcar_gen3_ths_tj_1_m3_w, which are gloabl to this file accordingly.
> 
> I understood. Why did I decide to use unsigned?
> I will fix it in v3.

Probably due to an earlier suggestion by me.
Sorry about that.

...

Could you post v3 with this fix and the changes suggested by Niklas
for patch 2/3?

[PATCH/RFT v2 2/3] thermal: rcar_gen3_thermal: Update calculation formula of IRQTEMP

[Yoshihiro Kaneko]

Update the formula to calculate CTEMP:
Currently, the CTEMP is average of val1 (is calculated by
formula 1) and val2 (is calculated by formula 2). But,
as description in HWM (chapter 10A.3.1.1 Setting of Normal Mode)

If (STEMP < Tj_T) CTEMP value should be val1.
If (STEMP > Tj_T) CTEMP value should be val2.

Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index a2fd0fd..97cf3cd 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -77,6 +77,7 @@ struct rcar_gen3_thermal_tsc {
 	struct equation_coefs coef;
 	int low;
 	int high;
+	int tj_2;
 };
 
 struct rcar_gen3_thermal_priv {
@@ -126,28 +127,27 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
 /* no idea where these constants come from */
 #define TJ_3 -41
 
-static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
+
+static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_tsc *tsc,
 					 int *ptat, int *thcode,
 					 unsigned int ths_tj_1)
 {
-	int tj_2;
-
 	/* TODO: Find documentation and document constant calculation formula */
 
 	/*
 	 * Division is not scaled in BSP and if scaled it might overflow
 	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
 	 */
-	tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
-		/ (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
+	tsc->tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
+		     / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
 
-	coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
-			     tj_2 - FIXPT_INT(TJ_3));
-	coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
+	tsc->coef.a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
+				 tsc->tj_2 - FIXPT_INT(TJ_3));
+	tsc->coef.b1 = FIXPT_INT(thcode[2]) - tsc->coef.a1 * TJ_3;
 
-	coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
-			     tj_2 - FIXPT_INT(ths_tj_1));
-	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
+	tsc->coef.a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
+				 tsc->tj_2 - FIXPT_INT(ths_tj_1));
+	tsc->coef.b2 = FIXPT_INT(thcode[0]) - tsc->coef.a2 * ths_tj_1;
 }
 
 static int rcar_gen3_thermal_round(int temp)
@@ -186,13 +186,15 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
 static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
 					      int mcelsius)
 {
-	int celsius, val1, val2;
+	int celsius, val;
 
 	celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
-	val1 = celsius * tsc->coef.a1 + tsc->coef.b1;
-	val2 = celsius * tsc->coef.a2 + tsc->coef.b2;
+	if (celsius <= INT_FIXPT(tsc->tj_2))
+		val = celsius * tsc->coef.a1 + tsc->coef.b1;
+	else
+		val = celsius * tsc->coef.a2 + tsc->coef.b2;
 
-	return INT_FIXPT((val1 + val2) / 2);
+	return INT_FIXPT(val);
 }
 
 static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high)
@@ -440,7 +442,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		priv->tscs[i] = tsc;
 
 		priv->thermal_init(tsc);
-		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
+		rcar_gen3_thermal_calc_coefs(tsc, ptat, thcode[i],
 					     *rcar_gen3_ths_tj_1);
 
 		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
-- 
1.9.1

Re: [PATCH/RFT v2 2/3] thermal: rcar_gen3_thermal: Update calculation formula of IRQTEMP

[Niklas Söderlund]

Hi Kaneko-san,

Thanks for your work.

On 2019-04-17 02:44:14 +0900, Yoshihiro Kaneko wrote:
> Update the formula to calculate CTEMP:
> Currently, the CTEMP is average of val1 (is calculated by
> formula 1) and val2 (is calculated by formula 2). But,
> as description in HWM (chapter 10A.3.1.1 Setting of Normal Mode)
> 
> If (STEMP < Tj_T) CTEMP value should be val1.
> If (STEMP > Tj_T) CTEMP value should be val2.
> 
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
> index a2fd0fd..97cf3cd 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -77,6 +77,7 @@ struct rcar_gen3_thermal_tsc {
>  	struct equation_coefs coef;
>  	int low;
>  	int high;
> +	int tj_2;
>  };
>  
>  struct rcar_gen3_thermal_priv {
> @@ -126,28 +127,27 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
>  /* no idea where these constants come from */
>  #define TJ_3 -41
>  
> -static void rcar_gen3_thermal_calc_coefs(struct equation_coefs *coef,
> +
> +static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_tsc *tsc,
>  					 int *ptat, int *thcode,
>  					 unsigned int ths_tj_1)
>  {
> -	int tj_2;
> -
>  	/* TODO: Find documentation and document constant calculation formula */

Maybe you cold also fix this todo now that the datasheet have been 
updated with 10A.3.1.1 ? This might also impact the equation_coefs 
structure.

>  
>  	/*
>  	 * Division is not scaled in BSP and if scaled it might overflow
>  	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
>  	 */
> -	tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> -		/ (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
> +	tsc->tj_2 = (FIXPT_INT((ptat[1] - ptat[2]) * 157)
> +		     / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);

I think tj_2 should be renamed to match the names in the datasheet, it's 
Tj_T from 10A.3.1.1 right?

>  
> -	coef->a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
> -			     tj_2 - FIXPT_INT(TJ_3));
> -	coef->b1 = FIXPT_INT(thcode[2]) - coef->a1 * TJ_3;
> +	tsc->coef.a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
> +				 tsc->tj_2 - FIXPT_INT(TJ_3));
> +	tsc->coef.b1 = FIXPT_INT(thcode[2]) - tsc->coef.a1 * TJ_3;
>  
> -	coef->a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> -			     tj_2 - FIXPT_INT(ths_tj_1));
> -	coef->b2 = FIXPT_INT(thcode[0]) - coef->a2 * ths_tj_1;
> +	tsc->coef.a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
> +				 tsc->tj_2 - FIXPT_INT(ths_tj_1));
> +	tsc->coef.b2 = FIXPT_INT(thcode[0]) - tsc->coef.a2 * ths_tj_1;
>  }
>  
>  static int rcar_gen3_thermal_round(int temp)
> @@ -186,13 +186,15 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
>  static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
>  					      int mcelsius)
>  {
> -	int celsius, val1, val2;
> +	int celsius, val;
>  
>  	celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
> -	val1 = celsius * tsc->coef.a1 + tsc->coef.b1;
> -	val2 = celsius * tsc->coef.a2 + tsc->coef.b2;
> +	if (celsius <= INT_FIXPT(tsc->tj_2))
> +		val = celsius * tsc->coef.a1 + tsc->coef.b1;
> +	else
> +		val = celsius * tsc->coef.a2 + tsc->coef.b2;
>  
> -	return INT_FIXPT((val1 + val2) / 2);
> +	return INT_FIXPT(val);
>  }
>  
>  static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high)
> @@ -440,7 +442,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>  		priv->tscs[i] = tsc;
>  
>  		priv->thermal_init(tsc);
> -		rcar_gen3_thermal_calc_coefs(&tsc->coef, ptat, thcode[i],
> +		rcar_gen3_thermal_calc_coefs(tsc, ptat, thcode[i],
>  					     *rcar_gen3_ths_tj_1);
>  
>  		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
> -- 
> 1.9.1
> 

-- 
Regards,
Niklas Söderlund

[PATCH/RFT v2 3/3] thermal: rcar_gen3_thermal: Update temperature conversion method

[Yoshihiro Kaneko]

Update the formula to calculate temperature:
Currently, current TEMP is calculated as
average of val1 (is calculated by formula 1)
and val2 (is calculated by formula 2). But,
as description in HWM (chapter 10A.3.1.2 Normal Mode.)

If (TEMP_CODE < THCODE2[11:0]) CTEMP value should be val1.
If (TEMP_CODE > THCODE2[11:0]) CTEMP value should be val2.

Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---
 drivers/thermal/rcar_gen3_thermal.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 97cf3cd..9b95263 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -63,6 +63,13 @@
 
 #define TSC_MAX_NUM	3
 
+/* default THCODE values if FUSEs are missing */
+static const int thcode[TSC_MAX_NUM][3] = {
+	{ 3397, 2800, 2221 },
+	{ 3393, 2795, 2216 },
+	{ 3389, 2805, 2237 },
+};
+
 /* Structure for thermal temperature calculation */
 struct equation_coefs {
 	int a1;
@@ -78,6 +85,7 @@ struct rcar_gen3_thermal_tsc {
 	int low;
 	int high;
 	int tj_2;
+	int id; /* thermal channel id */
 };
 
 struct rcar_gen3_thermal_priv {
@@ -129,7 +137,7 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
 
 
 static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_tsc *tsc,
-					 int *ptat, int *thcode,
+					 int *ptat, const int *thcode,
 					 unsigned int ths_tj_1)
 {
 	/* TODO: Find documentation and document constant calculation formula */
@@ -163,15 +171,19 @@ static int rcar_gen3_thermal_round(int temp)
 static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
 {
 	struct rcar_gen3_thermal_tsc *tsc = devdata;
-	int mcelsius, val1, val2;
+	int mcelsius, val;
 	u32 reg;
 
 	/* Read register and convert to mili Celsius */
 	reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
 
-	val1 = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1, tsc->coef.a1);
-	val2 = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2, tsc->coef.a2);
-	mcelsius = FIXPT_TO_MCELSIUS((val1 + val2) / 2);
+	if (reg <= thcode[tsc->id][1])
+		val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
+				tsc->coef.a1);
+	else
+		val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b2,
+				tsc->coef.a2);
+	mcelsius = FIXPT_TO_MCELSIUS(val);
 
 	/* Make sure we are inside specifications */
 	if ((mcelsius < MCELSIUS(-40)) || (mcelsius > MCELSIUS(125)))
@@ -377,11 +389,6 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	/* default values if FUSEs are missing */
 	/* TODO: Read values from hardware on supported platforms */
 	int ptat[3] = { 2631, 1509, 435 };
-	int thcode[TSC_MAX_NUM][3] = {
-		{ 3397, 2800, 2221 },
-		{ 3393, 2795, 2216 },
-		{ 3389, 2805, 2237 },
-	};
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -438,6 +445,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 			ret = PTR_ERR(tsc->base);
 			goto error_unregister;
 		}
+		tsc->id = i;
 
 		priv->tscs[i] = tsc;
 
-- 
1.9.1