| Message ID | 20220208020441.3081162-7-liambeguin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: afe: add temperature rescaling support | expand |
Hi! On 2022-02-08 03:04, Liam Beguin wrote: > Make use of well-defined SI metric prefixes to improve code readability. > > Signed-off-by: Liam Beguin <liambeguin@gmail.com> > --- > drivers/iio/afe/iio-rescale.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c > index 67273de46843..4601f84a83c2 100644 > --- a/drivers/iio/afe/iio-rescale.c > +++ b/drivers/iio/afe/iio-rescale.c > @@ -51,11 +51,16 @@ int rescale_process_scale(struct rescale *rescale, int scale_type, > } > fallthrough; > case IIO_VAL_FRACTIONAL_LOG2: > - tmp = (s64)*val * 1000000000LL; > + /* > + * GIGA is used here as an arbitrarily large multiplier to avoid s/arbitrarily/arbitrary/, however... > + * precision loss in the division. It doesn't have any physical > + * meaning attached to it. ... 1000000000 is NOT arbitrary. Before patch 4/11 that was true, but with that patch the multiplier MUST match the multiplier used below when calculating with the decimals for the IIO_VAL_INT_PLUS_NANO value. Again, the connection with that name makes me think that NANO is just so much better that GIGA. Still fine with raw numbers of course. So, the comment is actively misleading. How about the following? /* * We need to multiply by something large to avoid loss of * precision, and NANO fits that while at the same time * being compatible with the conversion to * IIO_VAL_INT_PLUS_NANO for cases where that maintains even * more precision. */ > + */ > + tmp = (s64)*val * GIGA; > tmp = div_s64(tmp, rescale->denominator); > tmp *= rescale->numerator; > > - tmp = div_s64_rem(tmp, 1000000000LL, &rem); > + tmp = div_s64_rem(tmp, GIGA, &rem); > *val = tmp; > > if (!rem) > @@ -71,7 +76,7 @@ int rescale_process_scale(struct rescale *rescale, int scale_type, > > *val2 = rem / (int)tmp; > if (rem2) > - *val2 += div_s64((s64)rem2 * 1000000000LL, tmp); > + *val2 += div_s64((s64)rem2 * GIGA, tmp); NANO again, because IIO_VAL_INT_PLUS_NANO. > > return IIO_VAL_INT_PLUS_NANO; > case IIO_VAL_INT_PLUS_NANO: > @@ -332,8 +337,8 @@ static int rescale_current_sense_amplifier_props(struct device *dev, > * gain_div / (gain_mult * sense), while trying to keep the > * numerator/denominator from overflowing. > */ > - factor = gcd(sense, 1000000); > - rescale->numerator = 1000000 / factor; > + factor = gcd(sense, MEGA); > + rescale->numerator = MEGA / factor; Here, we actually have a connection with prefix of a unit of a physical quantity. microohms. If anything, why not MICRO? > rescale->denominator = sense / factor; > > factor = gcd(rescale->numerator, gain_mult); > @@ -361,8 +366,8 @@ static int rescale_current_sense_shunt_props(struct device *dev, > return ret; > } > > - factor = gcd(shunt, 1000000); > - rescale->numerator = 1000000 / factor; > + factor = gcd(shunt, MEGA); > + rescale->numerator = MEGA / factor; MICRO again, bacause microohms. > rescale->denominator = shunt / factor; > > return 0; Cheers, Peter
diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c index 67273de46843..4601f84a83c2 100644 --- a/drivers/iio/afe/iio-rescale.c +++ b/drivers/iio/afe/iio-rescale.c @@ -51,11 +51,16 @@ int rescale_process_scale(struct rescale *rescale, int scale_type, } fallthrough; case IIO_VAL_FRACTIONAL_LOG2: - tmp = (s64)*val * 1000000000LL; + /* + * GIGA is used here as an arbitrarily large multiplier to avoid + * precision loss in the division. It doesn't have any physical + * meaning attached to it. + */ + tmp = (s64)*val * GIGA; tmp = div_s64(tmp, rescale->denominator); tmp *= rescale->numerator; - tmp = div_s64_rem(tmp, 1000000000LL, &rem); + tmp = div_s64_rem(tmp, GIGA, &rem); *val = tmp; if (!rem) @@ -71,7 +76,7 @@ int rescale_process_scale(struct rescale *rescale, int scale_type, *val2 = rem / (int)tmp; if (rem2) - *val2 += div_s64((s64)rem2 * 1000000000LL, tmp); + *val2 += div_s64((s64)rem2 * GIGA, tmp); return IIO_VAL_INT_PLUS_NANO; case IIO_VAL_INT_PLUS_NANO: @@ -332,8 +337,8 @@ static int rescale_current_sense_amplifier_props(struct device *dev, * gain_div / (gain_mult * sense), while trying to keep the * numerator/denominator from overflowing. */ - factor = gcd(sense, 1000000); - rescale->numerator = 1000000 / factor; + factor = gcd(sense, MEGA); + rescale->numerator = MEGA / factor; rescale->denominator = sense / factor; factor = gcd(rescale->numerator, gain_mult); @@ -361,8 +366,8 @@ static int rescale_current_sense_shunt_props(struct device *dev, return ret; } - factor = gcd(shunt, 1000000); - rescale->numerator = 1000000 / factor; + factor = gcd(shunt, MEGA); + rescale->numerator = MEGA / factor; rescale->denominator = shunt / factor; return 0;
Make use of well-defined SI metric prefixes to improve code readability. Signed-off-by: Liam Beguin <liambeguin@gmail.com> --- drivers/iio/afe/iio-rescale.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)