| Message ID | 20220817054321.6519-13-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Variety of fixes and new features for mr75203 driver | expand |
On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote: > Modify the equation and coefficients to convert the digital output to > temperature according to series 5 of the Moortec Embedded Temperature > Sensor (METS) datasheet: > T = G + H * (n / cal5 - 0.5) + J * F > > The G, H and J coefficients are multiplied by 1000 to get the temperature > in milli-Celsius. > This is, at the very least, confusing. It doesn't explain the discrepancy to the old code nor the change in constant values. I have no idea if this change would result in erroneous readings on some other system where the existing calculation may be the correct one. On top of that, it seems overflow-prune in 32 bit systems. Guenter > Signed-off-by: Eliav Farber <farbere@amazon.com> > --- > drivers/hwmon/mr75203.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c > index e3191f590167..e500897585e4 100644 > --- a/drivers/hwmon/mr75203.c > +++ b/drivers/hwmon/mr75203.c > @@ -102,9 +102,10 @@ > > #define PVT_POLL_DELAY_US 20 > #define PVT_POLL_TIMEOUT_US 20000 > -#define PVT_H_CONST 100000 > -#define PVT_CAL5_CONST 2047 > -#define PVT_G_CONST 40000 > +#define PVT_H_CONST 60000 > +#define PVT_G_CONST 200000 > +#define PVT_J_CONST -100 > +#define PVT_CAL5_CONST 4094 > #define PVT_CONV_BITS 10 > #define PVT_N_CONST 90 > #define PVT_R_CONST 245805 > @@ -158,7 +159,6 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) > struct regmap *t_map = pvt->t_map; > u32 stat, nbs; > int ret; > - u64 tmp; > > switch (attr) { > case hwmon_temp_input: > @@ -176,12 +176,13 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) > nbs &= SAMPLE_DATA_MSK; > > /* > - * Convert the register value to > - * degrees centigrade temperature > + * Convert the register value to degrees centigrade temperature: > + * T = G + H * (n / cal5 - 0.5) + J * F > */ > - tmp = nbs * PVT_H_CONST; > - do_div(tmp, PVT_CAL5_CONST); > - *val = tmp - PVT_G_CONST - pvt->ip_freq; > + *val = PVT_G_CONST; > + *val += PVT_H_CONST * nbs / PVT_CAL5_CONST; > + *val -= PVT_H_CONST / 2; > + *val += PVT_J_CONST * pvt->ip_freq / HZ_PER_MHZ; > > return 0; > default: > @@ -313,7 +314,7 @@ static int pvt_init(struct pvt_device *pvt) > (key >> 1) << CLK_SYNTH_HI_SFT | > (key >> 1) << CLK_SYNTH_HOLD_SFT | CLK_SYNTH_EN; > > - pvt->ip_freq = sys_freq * 100 / (key + 2); > + pvt->ip_freq = clk_get_rate(pvt->clk) / (key + 2); > > if (t_num) { > ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0);
On 8/18/2022 11:23 PM, Guenter Roeck wrote: > On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote: >> Modify the equation and coefficients to convert the digital output to >> temperature according to series 5 of the Moortec Embedded Temperature >> Sensor (METS) datasheet: >> T = G + H * (n / cal5 - 0.5) + J * F >> >> The G, H and J coefficients are multiplied by 1000 to get the >> temperature >> in milli-Celsius. >> > > This is, at the very least, confusing. It doesn't explain the discrepancy > to the old code nor the change in constant values. I have no idea if this > change would result in erroneous readings on some other system where > the existing calculation may be the correct one. When I tested the driver it was also not clear to me why the equation and coefficients in the code don't match the specifications in the data sheet. I reached out to Maxlinear engineers (@rtanwar) and they also couldn't explain the discrepancy. After further correspondence I aligned both the equation and coefficients in the driver code to the equation and coefficients defined in series 5 of the Moortec Embedded Temperature Sensor (METS) datasheet which they provided. > On top of that, it seems overflow-prune in 32 bit systems. I'll check if it can overflow, and if it can I'll fix in next version. -- Regards, Eliav
On Fri, Aug 19, 2022 at 10:44:06AM +0300, Farber, Eliav wrote: > On 8/18/2022 11:23 PM, Guenter Roeck wrote: > > On Wed, Aug 17, 2022 at 05:43:17AM +0000, Eliav Farber wrote: > > > Modify the equation and coefficients to convert the digital output to > > > temperature according to series 5 of the Moortec Embedded Temperature > > > Sensor (METS) datasheet: > > > T = G + H * (n / cal5 - 0.5) + J * F > > > > > > The G, H and J coefficients are multiplied by 1000 to get the > > > temperature > > > in milli-Celsius. > > > > > > > This is, at the very least, confusing. It doesn't explain the discrepancy > > to the old code nor the change in constant values. I have no idea if this > > change would result in erroneous readings on some other system where > > the existing calculation may be the correct one. > > When I tested the driver it was also not clear to me why the equation > and coefficients in the code don't match the specifications in the data > sheet. > I reached out to Maxlinear engineers (@rtanwar) and they also couldn't > explain the discrepancy. > After further correspondence I aligned both the equation and coefficients > in the driver code to the equation and coefficients defined in series 5 > of the Moortec Embedded Temperature Sensor (METS) datasheet which they > provided. > At least some of the discrepancy is because the original code is more optimized and avoids overflow. Either case, the above needs to be explained in the commit description. > > On top of that, it seems overflow-prune in 32 bit systems. > > I'll check if it can overflow, and if it can I'll fix in next version. > > -- > Regards, Eliav >
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c index e3191f590167..e500897585e4 100644 --- a/drivers/hwmon/mr75203.c +++ b/drivers/hwmon/mr75203.c @@ -102,9 +102,10 @@ #define PVT_POLL_DELAY_US 20 #define PVT_POLL_TIMEOUT_US 20000 -#define PVT_H_CONST 100000 -#define PVT_CAL5_CONST 2047 -#define PVT_G_CONST 40000 +#define PVT_H_CONST 60000 +#define PVT_G_CONST 200000 +#define PVT_J_CONST -100 +#define PVT_CAL5_CONST 4094 #define PVT_CONV_BITS 10 #define PVT_N_CONST 90 #define PVT_R_CONST 245805 @@ -158,7 +159,6 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) struct regmap *t_map = pvt->t_map; u32 stat, nbs; int ret; - u64 tmp; switch (attr) { case hwmon_temp_input: @@ -176,12 +176,13 @@ static int pvt_read_temp(struct device *dev, u32 attr, int channel, long *val) nbs &= SAMPLE_DATA_MSK; /* - * Convert the register value to - * degrees centigrade temperature + * Convert the register value to degrees centigrade temperature: + * T = G + H * (n / cal5 - 0.5) + J * F */ - tmp = nbs * PVT_H_CONST; - do_div(tmp, PVT_CAL5_CONST); - *val = tmp - PVT_G_CONST - pvt->ip_freq; + *val = PVT_G_CONST; + *val += PVT_H_CONST * nbs / PVT_CAL5_CONST; + *val -= PVT_H_CONST / 2; + *val += PVT_J_CONST * pvt->ip_freq / HZ_PER_MHZ; return 0; default: @@ -313,7 +314,7 @@ static int pvt_init(struct pvt_device *pvt) (key >> 1) << CLK_SYNTH_HI_SFT | (key >> 1) << CLK_SYNTH_HOLD_SFT | CLK_SYNTH_EN; - pvt->ip_freq = sys_freq * 100 / (key + 2); + pvt->ip_freq = clk_get_rate(pvt->clk) / (key + 2); if (t_num) { ret = regmap_write(t_map, SDIF_SMPL_CTRL, 0x0);
Modify the equation and coefficients to convert the digital output to temperature according to series 5 of the Moortec Embedded Temperature Sensor (METS) datasheet: T = G + H * (n / cal5 - 0.5) + J * F The G, H and J coefficients are multiplied by 1000 to get the temperature in milli-Celsius. Signed-off-by: Eliav Farber <farbere@amazon.com> --- drivers/hwmon/mr75203.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)