> Would you please check the following from checkpatch too? I saw them and chose to ignore them. I am not strict with those warnings within the i2c subsystem as well. I can change the series if your mileage varies, of course. > WARNING: please write a paragraph that describes the config symbol fully > #82: FILE: drivers/thermal/Kconfig:248: > +config RCAR_GEN3_THERMAL I can make up something. > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? Is this mandatory? > CHECK: struct mutex definition without comment > #186: FILE: drivers/thermal/rcar_gen3_thermal.c:75: > + struct mutex lock; Can change, but if there is only one lock, I don't really see much benefit from this check. > WARNING: line over 80 characters > #204: FILE: drivers/thermal/rcar_gen3_thermal.c:93: > +static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc, u32 reg) > > CHECK: Alignment should match open parenthesis > #210: FILE: drivers/thermal/rcar_gen3_thermal.c:99: > +static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc, > + u32 reg, u32 data) I have those warnings (80 chars and open parens) ignored by default but if you think it makes the code more readable, I'll change it. > > +static void _linear_coefficient_calculation(struct rcar_gen3_thermal_tsc *tsc, > > + int *ptat, int *thcode) > > +{ > > + int tj_2; > > + s64 a1, b1; > > + s64 a2, b2; > > + s64 a1_num, a1_den; > > + s64 a2_num, a2_den; > > + > > + tj_2 = (CODETSD((ptat[1] - ptat[2]) * 137) > > + / (ptat[0] - ptat[2])) - CODETSD(41); > > + > > + /* calculate coefficients for linear equation */ > > + a1_num = CODETSD(thcode[1] - thcode[2]); > > + a1_den = tj_2 - TJ_3; > > + a1 = (10000 * a1_num) / a1_den; > > + b1 = (10000 * thcode[2]) - ((a1 * TJ_3) / 1000); > > + > > + a2_num = CODETSD(thcode[1] - thcode[0]); > > + a2_den = tj_2 - TJ_1; > > + a2 = (10000 * a2_num) / a2_den; > > + b2 = (10000 * thcode[0]) - ((a2 * TJ_1) / 1000); > > + > > + tsc->coef.a1 = DIV_ROUND_CLOSEST(a1, 10); > > + tsc->coef.b1 = DIV_ROUND_CLOSEST(b1, 10); > > + tsc->coef.a2 = DIV_ROUND_CLOSEST(a2, 10); > > + tsc->coef.b2 = DIV_ROUND_CLOSEST(b2, 10); > > What is a a1, b1, a2, b2 typical values? > > are you sure they do not fit into int? Looks like you start from pretty small values, > but multiply by 10^3 on num and den to get better precision? Typical values are a few thousand. a1_num uses CODETSD which multiplies by 1000 and makes it a million. a1 then multiplies again by 10000 which makes it 10 billion. No int. I am quite sure the formulas can be rearranged to fit into an int. As mentioned before, I hoped we could start with the already tested formulas since documentation on them is sparse. > > +static int _linear_temp_converter(struct equation_coefs *coef, > > + int temp_code) > > +{ > > + int temp, temp1, temp2; > > + > > + temp1 = MCELSIUS((CODETSD(temp_code) - coef->b1)) / coef->a1; > > + temp2 = MCELSIUS((CODETSD(temp_code) - coef->b2)) / coef->a2; > > aren't we overflowing the result of this 64 bit math assigned into an int? The division ensures that we get an int. Hmmm, not very pretty, I agree. Sigh, maybe it is better to refactor the formulas before submitting upstream :/ Regards, Wolfram