Hi Kaneko-san, Thanks for your work. On 2019-04-01 15:37:57 +0200, Simon Horman wrote: > On Fri, Mar 22, 2019 at 05:47:53AM +0900, Yoshihiro Kaneko wrote: > > From: Dien Pham <dien.pham.ry@xxxxxxxxxxx> > > > > 1/ 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) > > > > 2/ 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. > > > > 3/ 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: Dien Pham <dien.pham.ry@xxxxxxxxxxx> > > [ykaneko0929@xxxxxxxxx: use the data field of the of_device_id for Tj_1] > > [ykaneko0929@xxxxxxxxx: revise a description of case 1 of the commit log] > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@xxxxxxxxx> > > I'm wondering if we could split this up into three patches, > one for each problem it solves? Partly because I think it is good to fix > one problem per patch. And partly because am having trouble > verifying this patch - the new if statement in rcar_gen3_thermal_get_temp() > seems to result in 0 temperature readings when the else case is used. I think the changes in this patch are good and correct but I agree with Simon here, this patch should be split. > > > --- > > drivers/thermal/rcar_gen3_thermal.c | 86 +++++++++++++++++++++++++------------ > > 1 file changed, 58 insertions(+), 28 deletions(-) > > > > This patch is based on the master branch of Linus Torvalds's linux tree. > > > > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c > > index 88fa41c..de6f244 100644 > > --- a/drivers/thermal/rcar_gen3_thermal.c > > +++ b/drivers/thermal/rcar_gen3_thermal.c > > @@ -63,6 +63,15 @@ > > > > #define TSC_MAX_NUM 3 > > > > +static int tj_2; > > We need to avoid global variables. There can be multiple > instances of this driver. (Although they will all get the same > value for tj_2. > > Perhaps it can be added to struct rcar_gen3_thermal_tsc? > > ... -- Regards, Niklas Söderlund
