Hi Niklas-san, Thanks for your review. 2019年4月12日(金) 1:50 Niklas Söderlund <niklas.soderlund@xxxxxxxxxxxx>: > > 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. I have split this patch and made fixes in v2. Thanks, Kaneko > > > > > > --- > > > 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
