From: Liam Beguin <redacted>
When a consumer calls iio_read_channel_processed() and the channel has
an integer scale, the scale channel scale is applied and the processed
value is returned as expected.
On the other hand, if the consumer calls iio_convert_raw_to_processed()
the scaling factor requested by the consumer is not applied.
This for example causes the consumer to process mV when expecting uV.
Make sure to always apply the scaling factor requested by the consumer.
Fixes: 48e44ce0f881 ("iio:inkern: Add function to read the processed value")
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/inkern.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Liam Beguin <redacted>
When a consumer calls iio_read_channel_processed() and no channel scale
is available, it's assumed that the scale is one and the raw value is
returned as expected.
On the other hand, if the consumer calls iio_convert_raw_to_processed()
the scaling factor requested by the consumer is not applied.
This for example causes the consumer to process mV when expecting uV.
Make sure to always apply the scaling factor requested by the consumer.
Fixes: adc8ec5ff183 ("iio: inkern: pass through raw values if no scaling")
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/inkern.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: Liam Beguin <redacted>
iio_convert_raw_to_processed_unlocked() assumes the offset is an
integer. Make a best effort to get a valid offset value for fractional
cases without breaking implicit truncations.
Fixes: 48e44ce0f881 ("iio:inkern: Add function to read the processed value")
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/inkern.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
From: Liam Beguin <redacted>
Reduce the risk of integer overflow by doing the scale calculation with
64bit integers and looking for a Greatest Common Divider for both parts
of the fractional value.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
From: Peter Rosin <hidden> Date: 2021-07-09 16:24:27
On 2021-07-06 18:09, Liam Beguin wrote:
quoted hunk
From: Liam Beguin <redacted>
Reduce the risk of integer overflow by doing the scale calculation with
64bit integers and looking for a Greatest Common Divider for both parts
of the fractional value.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
@@ -39,7 +39,8 @@ static int rescale_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structrescale*rescale=iio_priv(indio_dev);-unsignedlonglongtmp;+s64tmp,tmp2;+u32factor;intret;switch(mask){
@@ -67,8 +68,13 @@ static int rescale_read_raw(struct iio_dev *indio_dev,}switch(ret){caseIIO_VAL_FRACTIONAL:-*val*=rescale->numerator;-*val2*=rescale->denominator;+tmp=(s64)*val*rescale->numerator;+tmp2=(s64)*val2*rescale->denominator;+factor=gcd(tmp,tmp2);
Hi!
gcd() isn't exactly free. I do not think it is suitable to call it for each
and every value. So, if you really need it, then it should only be used
when there is an actual overflow (or if there is a high risk if that's
somehow easier).
Cheers,
Peter
On Fri Jul 9, 2021 at 12:24 PM EDT, Peter Rosin wrote:
On 2021-07-06 18:09, Liam Beguin wrote:
quoted
From: Liam Beguin <redacted>
Reduce the risk of integer overflow by doing the scale calculation with
64bit integers and looking for a Greatest Common Divider for both parts
of the fractional value.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
@@ -39,7 +39,8 @@ static int rescale_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structrescale*rescale=iio_priv(indio_dev);-unsignedlonglongtmp;+s64tmp,tmp2;+u32factor;intret;switch(mask){
@@ -67,8 +68,13 @@ static int rescale_read_raw(struct iio_dev *indio_dev,}switch(ret){caseIIO_VAL_FRACTIONAL:-*val*=rescale->numerator;-*val2*=rescale->denominator;+tmp=(s64)*val*rescale->numerator;+tmp2=(s64)*val2*rescale->denominator;+factor=gcd(tmp,tmp2);
Hi Peter,
Hi!
gcd() isn't exactly free. I do not think it is suitable to call it for
each
and every value. So, if you really need it, then it should only be used
when there is an actual overflow (or if there is a high risk if that's
somehow easier).
Understood, digging into this a little bit, it seems like
check_mul_overflow() could be used here.
I'll give it a try and will look at implementing Jonathan's suggestion
in case we're dealing with a case where gcd() returns 1.
Thanks,
Liam
From: Liam Beguin <redacted>
An ADC is often used to measure other quantities indirectly. This
binding describe one case, the measurement of a temperature through the
voltage across an RTD resistor such as a PT1000.
Signed-off-by: Liam Beguin <redacted>
---
.../iio/afe/temperature-sense-rtd.yaml | 101 ++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
@@ -0,0 +1,101 @@+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iio/afe/temperature-sense-rtd.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Temperature Sense RTD++maintainers:+-Liam Beguin <lvb@xiphos.com>++description:|+RTDs (Resistance Temperature Detectors) are a kind of temperature sensors+used to get a linear voltage to temperature reading within a give range+(usually 0 to 100 degrees Celsius).++When an io-channel measures the output voltage across an RTD such as a+PT1000, the interesting measurement is almost always the corresponding+temperature, not the voltage output. This binding describes such a circuit.++The general transfer function here is (using SI units)++V = R(T) * iexc+R(T) = r0 * (1 + alpha * T)+T = 1 / (alpha * r0 * iexc) * (V - r0 * iexc)++The following circuit matches what's in the examples section.++5V0+-----+|++---+----++| R 5k |++---+----++|+V 1mA+|++---- Vout+|++---+----++| PT1000 |++---+----++|+-----+GND++properties:+compatible:+const:temperature-sense-rtd++io-channels:+maxItems:1+description:|+Channel node of a voltage io-channel.++'#io-channel-cells':+const:0++excitation-current-microamp:+description:The current fed through the RTD sensor.++alpha-ppm-per-celsius:+description:|+alpha can also be expressed in micro-ohms per ohm Celsius. It's a linear+approximation of the resistance versus temperature relationship+between 0 and 100 degrees Celsius.++alpha = (R_100 - R_0) / (100 * R_0)++Where, R_100 is the resistance of the sensor at 100 degrees Celsius, and+R_0 (or r-naught-ohms) is the resistance of the sensor at 0 degrees+Celsius.++Pure platinum has an alpha of 3925. Industry standards such as IEC60751+and ASTM E-1137 specify an alpha of 3850.++r-naught-ohms:+description:|+Resistance of the sensor at 0 degrees Celsius.+Common values are 100 for PT100, 500 for PT500, and 1000 for PT1000++additionalProperties:false+required:+-compatible+-io-channels+-excitation-current-microamp+-alpha-ppm-per-celsius+-r-naught-ohms++examples:+-|+pt1000_1:temperature-sensor0 {+compatible = "temperature-sense-rtd";+#io-channel-cells = <0>;+io-channels = <&temp_adc1 0>;++excitation-current-microamp = <1000>; /* i = U/R = 5 / 5000 */+alpha-ppm-per-celsius = <3908>;+r-naught-ohms = <1000>;+};+...
From: Rob Herring <robh@kernel.org> Date: 2021-07-12 16:08:24
On Tue, 06 Jul 2021 12:09:41 -0400, Liam Beguin wrote:
From: Liam Beguin <redacted>
An ADC is often used to measure other quantities indirectly. This
binding describe one case, the measurement of a temperature through the
voltage across an RTD resistor such as a PT1000.
Signed-off-by: Liam Beguin <redacted>
---
.../iio/afe/temperature-sense-rtd.yaml | 101 ++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-sense-rtd.yaml
From: Liam Beguin <redacted>
A temperature transducer is a device that converts a thermal quantity
into any other physical quantity. This patch add support for temperature
to voltage (like the LTC2997) and temperature to current (like the
AD590) linear transducers.
In both cases these are assumed to be connected to a voltage ADC.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
From: Liam Beguin <redacted>
An ADC is often used to measure other quantities indirectly.
This binding describe one case, the measurement of a temperature
through a temperature transducer (either voltage or current).
Signed-off-by: Liam Beguin <redacted>
---
.../iio/afe/temperature-transducer.yaml | 111 ++++++++++++++++++
1 file changed, 111 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-transducer.yaml
@@ -0,0 +1,111 @@+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iio/afe/temperature-transducer.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Temperature Transducer++maintainers:+-Liam Beguin <lvb@xiphos.com>++description:|+A temperature transducer is a device that converts a thermal quantity+into any other physical quantity. This binding applies to temperature to+voltage (like the LTC2997), and temperature to current (like the AD590)+linear transducers.+In both cases these are assumed to be connected to a voltage ADC.++When an io-channel measures the output voltage of a temperature analog front+end such as a temperature transducer, the interesting measurement is almost+always the corresponding temperature, not the voltage output. This binding+describes such a circuit.++The general transfer function here is (using SI units)+V(T) = Rsense * Isense(T)+T = (Isense(T) / alpha) + offset+T = 1 / (Rsense * alpha) * (V + offset * Rsense * alpha)++When using a temperature to voltage transducer, Rsense is set to 1.++The following circuits show a temperature to current and a temperature to+voltage transducer that can be used with this binding.++VCC+-----+|++---+---++| AD590 | VCC++---+---+ -----+| |+V proportional to T +----+----++| D+ --+ |++---- Vout | LTC2997 +--- Vout+| D- --+ |++---+----+ +---------++| Rsense | |++---+----+ -----+| GND+-----+GND++properties:+compatible:+const:temperature-transducer++io-channels:+maxItems:1+description:|+Channel node of a voltage io-channel.++'#io-channel-cells':+const:0++sense-offset-millicelsius:+description:|+Temperature offset. The default is <0>.+This offset is commonly used to convert from Kelvins to degrees Celsius.+In that case, sense-offset-millicelsius would be set to <(-273150)>.++sense-resistor-ohms:+description:|+The sense resistor. Defaults to <1>.+Set sense-resistor-ohms to <1> when using a temperature to voltage+transducer.++alpha-ppm-per-celsius:+description:|+Sometimes referred to as output gain, slope, or temperature coefficient.++alpha is expressed in parts per million which can be micro-amps per+degrees Celsius or micro-volts per degrees Celsius. The is the main+characteristic of a temperature transducer and should be stated in the+datasheet.++additionalProperties:false+required:+-compatible+-io-channels+-alpha-ppm-per-celsius++examples:+-|+ad950:temperature-sensor-0 {+compatible = "temperature-transducer";+#io-channel-cells = <0>;+io-channels = <&temp_adc 3>;++sense-offset-millicelsius = <(-273150)>; /* Kelvin to degrees Celsius */+sense-resistor-ohms = <8060>;+alpha-ppm-per-celsius = <1>; /* 1 uA/K */+};+-|+znq_tmp:temperature-sensor-1 {+compatible = "temperature-transducer";+#io-channel-cells = <0>;+io-channels = <&temp_adc 2>;++sense-offset-millicelsius = <(-273150)>; /* Kelvin to degrees Celsius */+alpha-ppm-per-celsius = <4000>; /* 4 mV/K */+};+...
From: Rob Herring <robh@kernel.org> Date: 2021-07-12 16:12:07
On Tue, Jul 06, 2021 at 12:09:42PM -0400, Liam Beguin wrote:
quoted hunk
From: Liam Beguin <redacted>
An ADC is often used to measure other quantities indirectly.
This binding describe one case, the measurement of a temperature
through a temperature transducer (either voltage or current).
Signed-off-by: Liam Beguin <redacted>
---
.../iio/afe/temperature-transducer.yaml | 111 ++++++++++++++++++
1 file changed, 111 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-transducer.yaml
@@ -0,0 +1,111 @@+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iio/afe/temperature-transducer.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Temperature Transducer++maintainers:+-Liam Beguin <lvb@xiphos.com>++description:|+A temperature transducer is a device that converts a thermal quantity+into any other physical quantity. This binding applies to temperature to+voltage (like the LTC2997), and temperature to current (like the AD590)+linear transducers.+In both cases these are assumed to be connected to a voltage ADC.++When an io-channel measures the output voltage of a temperature analog front+end such as a temperature transducer, the interesting measurement is almost+always the corresponding temperature, not the voltage output. This binding+describes such a circuit.++The general transfer function here is (using SI units)+V(T) = Rsense * Isense(T)+T = (Isense(T) / alpha) + offset+T = 1 / (Rsense * alpha) * (V + offset * Rsense * alpha)++When using a temperature to voltage transducer, Rsense is set to 1.++The following circuits show a temperature to current and a temperature to+voltage transducer that can be used with this binding.++VCC+-----+|++---+---++| AD590 | VCC++---+---+ -----+| |+V proportional to T +----+----++| D+ --+ |++---- Vout | LTC2997 +--- Vout+| D- --+ |++---+----+ +---------++| Rsense | |++---+----+ -----+| GND+-----+GND++properties:+compatible:+const:temperature-transducer++io-channels:+maxItems:1+description:|+Channel node of a voltage io-channel.++'#io-channel-cells':+const:0
This is a io-channel consumer and producer?
+
+ sense-offset-millicelsius:
+ description: |
+ Temperature offset. The default is <0>.
+ This offset is commonly used to convert from Kelvins to degrees Celsius.
+ In that case, sense-offset-millicelsius would be set to <(-273150)>.
default: 0
+
+ sense-resistor-ohms:
+ description: |
+ The sense resistor. Defaults to <1>.
+ Set sense-resistor-ohms to <1> when using a temperature to voltage
+ transducer.
default: 1
Though why would we set the value to 1 if the default is 1?
+
+ alpha-ppm-per-celsius:
+ description: |
+ Sometimes referred to as output gain, slope, or temperature coefficient.
+
+ alpha is expressed in parts per million which can be micro-amps per
+ degrees Celsius or micro-volts per degrees Celsius. The is the main
+ characteristic of a temperature transducer and should be stated in the
+ datasheet.
+
+additionalProperties: false
On Mon Jul 12, 2021 at 12:11 PM EDT, Rob Herring wrote:
On Tue, Jul 06, 2021 at 12:09:42PM -0400, Liam Beguin wrote:
quoted
From: Liam Beguin <redacted>
An ADC is often used to measure other quantities indirectly.
This binding describe one case, the measurement of a temperature
through a temperature transducer (either voltage or current).
Signed-off-by: Liam Beguin <redacted>
---
.../iio/afe/temperature-transducer.yaml | 111 ++++++++++++++++++
1 file changed, 111 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/afe/temperature-transducer.yaml
@@ -0,0 +1,111 @@+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iio/afe/temperature-transducer.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Temperature Transducer++maintainers:+-Liam Beguin <lvb@xiphos.com>++description:|+A temperature transducer is a device that converts a thermal quantity+into any other physical quantity. This binding applies to temperature to+voltage (like the LTC2997), and temperature to current (like the AD590)+linear transducers.+In both cases these are assumed to be connected to a voltage ADC.++When an io-channel measures the output voltage of a temperature analog front+end such as a temperature transducer, the interesting measurement is almost+always the corresponding temperature, not the voltage output. This binding+describes such a circuit.++The general transfer function here is (using SI units)+V(T) = Rsense * Isense(T)+T = (Isense(T) / alpha) + offset+T = 1 / (Rsense * alpha) * (V + offset * Rsense * alpha)++When using a temperature to voltage transducer, Rsense is set to 1.++The following circuits show a temperature to current and a temperature to+voltage transducer that can be used with this binding.++VCC+-----+|++---+---++| AD590 | VCC++---+---+ -----+| |+V proportional to T +----+----++| D+ --+ |++---- Vout | LTC2997 +--- Vout+| D- --+ |++---+----+ +---------++| Rsense | |++---+----+ -----+| GND+-----+GND++properties:+compatible:+const:temperature-transducer++io-channels:+maxItems:1+description:|+Channel node of a voltage io-channel.++'#io-channel-cells':+const:0
Hi Rob,
This is a io-channel consumer and producer?
Yes, this is a consumer and a producer.
It consumes a single ADC channel and can be fed to something like hwmon.
quoted
+
+ sense-offset-millicelsius:
+ description: |
+ Temperature offset. The default is <0>.
+ This offset is commonly used to convert from Kelvins to degrees Celsius.
+ In that case, sense-offset-millicelsius would be set to <(-273150)>.
default: 0
quoted
+
+ sense-resistor-ohms:
+ description: |
+ The sense resistor. Defaults to <1>.
+ Set sense-resistor-ohms to <1> when using a temperature to voltage
+ transducer.
default: 1
Though why would we set the value to 1 if the default is 1?
I can rephrase this. I meant to say that the default will make this
behave like a temperature to voltage transducer.
Liam
quoted
+
+ alpha-ppm-per-celsius:
+ description: |
+ Sometimes referred to as output gain, slope, or temperature coefficient.
+
+ alpha is expressed in parts per million which can be micro-amps per
+ degrees Celsius or micro-volts per degrees Celsius. The is the main
+ characteristic of a temperature transducer and should be stated in the
+ datasheet.
+
+additionalProperties: false
From: Liam Beguin <redacted>
This is a preparatory change required for the addition of temperature
sensing front ends.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 72 +++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
@@ -39,6 +40,8 @@ static int rescale_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structrescale*rescale=iio_priv(indio_dev);+intscale,scale2;+intschan_off=0;s64tmp,tmp2;u32factor;intret;
@@ -100,6 +103,71 @@ static int rescale_read_raw(struct iio_dev *indio_dev,dev_err(&indio_dev->dev,"unsupported type %d\n",ret);return-EOPNOTSUPP;}+caseIIO_CHAN_INFO_OFFSET:+/*+*Processedchannelsarescaled1-to-1andsourceoffsetis+*alreadytakenintoaccount.+*+*Inothercases,realworldmeasurementareexpressedas:+*+*schan_scale*(raw+schan_offset)+*+*Giventhattherescalerparametersareappliedrecursively:+*+*rescaler_scale*(schan_scale*(raw+schan_offset)++*rescaler_offset)+*+*Or,+*+*(rescaler_scale*schan_scale)*(raw++*(schan_offset+rescaler_offset/schan_scale)+*+*Thus,reusingtheoriginalexpressiontheparametersexposed+*touserspaceare:+*+*scale=schan_scale*rescaler_scale+*offset=schan_offset+rescaler_offset/schan_scale+*/+if(rescale->chan_processed){+*val=rescale->offset;+returnIIO_VAL_INT;+}++if(iio_channel_has_info(rescale->source->channel,+IIO_CHAN_INFO_OFFSET)){+ret=iio_read_channel_offset(rescale->source,+&schan_off,NULL);+if(ret!=IIO_VAL_INT)+returnret<0?ret:-EOPNOTSUPP;+}++ret=iio_read_channel_scale(rescale->source,&scale,&scale2);+switch(ret){+caseIIO_VAL_FRACTIONAL:+tmp=(s64)rescale->offset*scale2;+*val=div_s64(tmp,scale)+schan_off;+returnIIO_VAL_INT;+caseIIO_VAL_INT:+*val=div_s64(rescale->offset,scale)+schan_off;+returnIIO_VAL_INT;+caseIIO_VAL_FRACTIONAL_LOG2:+tmp=(s64)rescale->offset*(1<<scale2);+*val=div_s64(tmp,scale)+schan_off;+returnIIO_VAL_INT;+caseIIO_VAL_INT_PLUS_NANO:+tmp=(s64)rescale->offset*1000000000UL;+tmp2=((s64)scale*1000000000UL)+scale2;+*val=div_s64(tmp,tmp2)+schan_off;+returnIIO_VAL_INT;+caseIIO_VAL_INT_PLUS_MICRO:+tmp=(s64)rescale->offset*1000000UL;+tmp2=((s64)scale*1000000UL)+scale2;+*val=div_s64(tmp,tmp2)+schan_off;+returnIIO_VAL_INT;+default:+dev_err(&indio_dev->dev,"unsupported type %d\n",ret);+return-EOPNOTSUPP;+}default:return-EINVAL;}
@@ -176,6 +244,9 @@ static int rescale_configure_channel(struct device *dev,chan->info_mask_separate=BIT(IIO_CHAN_INFO_RAW)|BIT(IIO_CHAN_INFO_SCALE);+if(rescale->offset)+chan->info_mask_separate|=BIT(IIO_CHAN_INFO_OFFSET);+/**Using.read_avail()isfringetobeginwithandmakesnosense*whatsoeverforprocessedchannels,sowemakesurethatthiscannot
@@ -340,6 +411,7 @@ static int rescale_probe(struct platform_device *pdev)rescale->cfg=of_device_get_match_data(dev);rescale->numerator=1;rescale->denominator=1;+rescale->offset=0;ret=rescale->cfg->props(dev,rescale);if(ret)
From: Liam Beguin <redacted>
An RTD (Resistance Temperature Detector) is a kind of temperature
sensor used to get a linear voltage to temperature reading within a
give range (usually 0 to 100 degrees Celsius). Common types of RTDs
include PT100, PT500, and PT1000.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 48 +++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
From: Liam Beguin <redacted>
Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
Scale the integer part and the decimal parts individually and keep the
original scaling type.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 8 ++++++++
1 file changed, 8 insertions(+)
From: Peter Rosin <hidden> Date: 2021-07-09 16:29:22
On 2021-07-06 18:09, Liam Beguin wrote:
quoted hunk
From: Liam Beguin <redacted>
Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
Scale the integer part and the decimal parts individually and keep the
original scaling type.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -89,7 +89,15 @@ static int rescale_read_raw(struct iio_dev *indio_dev,do_div(tmp,1000000000LL);*val=tmp;returnret;+caseIIO_VAL_INT_PLUS_NANO:+caseIIO_VAL_INT_PLUS_MICRO:+tmp=(s64)*val*rescale->numerator;+*val=div_s64(tmp,rescale->denominator);+tmp=(s64)*val2*rescale->numerator;+*val2=div_s64(tmp,rescale->denominator);
Hi!
You are losing precision, and you are not mormalising after the calculation.
I think it's better to not even attempt this given that the results can be
really poor.
Cheers,
Peter
On Fri Jul 9, 2021 at 12:29 PM EDT, Peter Rosin wrote:
On 2021-07-06 18:09, Liam Beguin wrote:
quoted
From: Liam Beguin <redacted>
Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
Scale the integer part and the decimal parts individually and keep the
original scaling type.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -89,7 +89,15 @@ static int rescale_read_raw(struct iio_dev *indio_dev,do_div(tmp,1000000000LL);*val=tmp;returnret;+caseIIO_VAL_INT_PLUS_NANO:+caseIIO_VAL_INT_PLUS_MICRO:+tmp=(s64)*val*rescale->numerator;+*val=div_s64(tmp,rescale->denominator);+tmp=(s64)*val2*rescale->numerator;+*val2=div_s64(tmp,rescale->denominator);
Hi Peter,
Hi!
You are losing precision, and you are not mormalising after the
calculation.
Can you elaborate a little on what you mean here?
Do you mean that I should make sure that *val2, the PLUS_{NANO,MICRO}
part, doesn't contain an integer part? And if so transfer that part back
to *val?
I think it's better to not even attempt this given that the results can
be
really poor.
Unfortunatelly, I'm kinda stuck with this as some of my ADC use these
types.
Thanks,
Liam
From: Peter Rosin <hidden> Date: 2021-07-10 08:14:31
On 2021-07-09 21:30, Liam Beguin wrote:
On Fri Jul 9, 2021 at 12:29 PM EDT, Peter Rosin wrote:
quoted
On 2021-07-06 18:09, Liam Beguin wrote:
quoted
From: Liam Beguin <redacted>
Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
Scale the integer part and the decimal parts individually and keep the
original scaling type.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -89,7 +89,15 @@ static int rescale_read_raw(struct iio_dev *indio_dev,do_div(tmp,1000000000LL);*val=tmp;returnret;+caseIIO_VAL_INT_PLUS_NANO:+caseIIO_VAL_INT_PLUS_MICRO:+tmp=(s64)*val*rescale->numerator;+*val=div_s64(tmp,rescale->denominator);+tmp=(s64)*val2*rescale->numerator;+*val2=div_s64(tmp,rescale->denominator);
Hi Peter,
quoted
Hi!
You are losing precision, and you are not mormalising after the
calculation.
Can you elaborate a little on what you mean here?
Do you mean that I should make sure that *val2, the PLUS_{NANO,MICRO}
part, doesn't contain an integer part? And if so transfer that part back
to *val?
Yes. On 32-bit, you will easily wrap, especially for PLUS_NANO. You'd
only need a scale factor of 10 or so and a fractional part above .5 to
hit the roof (10 * 500000000 > 2^32).
But I also mean that you are losing precision when you are scaling
the integer part and the fractional part separately. That deserves
at least a comment, but ideally it should be handled correctly.
quoted
I think it's better to not even attempt this given that the results can
be
really poor.
Unfortunatelly, I'm kinda stuck with this as some of my ADC use these
types.
On Sat Jul 10, 2021 at 4:14 AM EDT, Peter Rosin wrote:
On 2021-07-09 21:30, Liam Beguin wrote:
quoted
On Fri Jul 9, 2021 at 12:29 PM EDT, Peter Rosin wrote:
quoted
On 2021-07-06 18:09, Liam Beguin wrote:
quoted
From: Liam Beguin <redacted>
Add IIO_VAL_INT_PLUS_{NANO,MICRO} scaling support.
Scale the integer part and the decimal parts individually and keep the
original scaling type.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/afe/iio-rescale.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -89,7 +89,15 @@ static int rescale_read_raw(struct iio_dev *indio_dev,do_div(tmp,1000000000LL);*val=tmp;returnret;+caseIIO_VAL_INT_PLUS_NANO:+caseIIO_VAL_INT_PLUS_MICRO:+tmp=(s64)*val*rescale->numerator;+*val=div_s64(tmp,rescale->denominator);+tmp=(s64)*val2*rescale->numerator;+*val2=div_s64(tmp,rescale->denominator);
Hi Peter,
quoted
Hi!
You are losing precision, and you are not mormalising after the
calculation.
Can you elaborate a little on what you mean here?
Do you mean that I should make sure that *val2, the PLUS_{NANO,MICRO}
part, doesn't contain an integer part? And if so transfer that part back
to *val?
Hi Peter,
Yes. On 32-bit, you will easily wrap, especially for PLUS_NANO. You'd
only need a scale factor of 10 or so and a fractional part above .5 to
hit the roof (10 * 500000000 > 2^32).
Right, That makes sense!
But I also mean that you are losing precision when you are scaling
the integer part and the fractional part separately. That deserves
at least a comment, but ideally it should be handled correctly.