[PATCH 0/2] Fix erroneous pressure scaling for the lps331ap driver

[PATCH 0/2] Fix erroneous pressure scaling for the lps331ap driver

[Jacek Anaszewski]

This series fixes pressure scale value calculation and modifies the
way how the temperature scale value is calculated, for clarity
and consistency reasons. Below the detailed explanation
for the fix is provided.

Because of the IIO_VAL_INT_PLUS_NANO value returned by the
read_raw callback for the IIO_CHAN_INFO_SCALE mask case
the returned scale value has to be expressed in nanopascal
units. The scale value is used then for obtaining pressure
value in kilopascal units. Currently this value is 2441410,
whereas it should be 24414.

1. Scaling of exemplary read_raw value:

exemplary read raw result: 4067471
buggy_scale: 0.002441410 => 2441410 * 10^-9
buggy_pressure: 4067471 * 0.002441410 = 
9930.4
According to the documentation file
Documentation/ABI/testing/sysfs-bus-iio the scaled pressure
value is expressed in kilopascals, so the result should
be interpreted as 9930.4 kPa

2. Scaling of the device output value according to the data sheet

In order to get the result in mbars according to the device
data sheet the output value has to be divided by 4096 (LSB/mbar)

pressure [mbar] = 4067471 / 4096 = 993.0349

Given that 1 bar = 10^5 Pa the result in pascal units is
993.0349 * 10^-3 * 10^5 Pa = 993.0349 * 10^2 Pa = 993 hPa =>
99.3 kPa

whereas 4067471 * 0.002441410 (the buggy_scale value) = 9930.4,
which when considered as a result in kilopascals is not true
according to the above reasoning.

3. Proposed fix

The proposed fix introduces following constants:

ST_PRESS_LSB_PER_MBAR (4096.0) - LSB/mbar

ST_PRESS_KPASCAL_NANO_SCALE (100000000 / ST_PRESS_LSB_PER_MBAR) -
actual scale factor to be returned by in_pressure_scale which after
performing division gives the value 24414.
in_pressure_scale returns 0.000024414 after applying nano scale.

The final result in kilopascal units can be obtained as follows:

in_pressure_raw * in_pressure_scale = 
= 4067471 * 24414 * 10^-9 = 4.067471 * 10^6 * 2.4414 * 10^4 * 10^-9 =
= 9.930 * 10^1 = 99.3 kPa => 993 hPa (same as the pressure result
obtained in the paragraph 2.)

Summarizing, the bug was in the ST_PRESS_MBAR_TO_KPASCAL macro
which performed multiplication instead of division.

The second patch applies similar operations for the temperature scale
calculaton. Previous implementation was correct but the modification
is made for consistency and clarity.

Thanks,
Jacek

Jacek Anaszewski (2):
  iio: lps331ap: Fix wrong in_pressure_scale output value
  iio: lps331ap: Modify in_temp_scale calculation way

 drivers/iio/pressure/st_pressure_core.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] iio: lps331ap: Fix wrong in_pressure_scale output value

[Jacek Anaszewski]

This patch fixes improper in_pressure_scale output that is
returned by the lps331ap barometer sensor driver. According
to the documentation the pressure after applying the scale has to
be expressed in kilopascal units. With erroneous implementation
the scale value larger by two orders of magnitude is returned -
2441410 instead of 24414.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/iio/pressure/st_pressure_core.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index aacea2a..89eb40a 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -28,7 +28,9 @@
 #include <linux/iio/common/st_sensors.h>
 #include "st_pressure.h"
 
-#define ST_PRESS_MBAR_TO_KPASCAL(x)		(x * 10)
+#define ST_PRESS_LSB_PER_MBAR			4096UL
+#define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
+						 ST_PRESS_LSB_PER_MBAR)
 #define ST_PRESS_NUMBER_DATA_CHANNELS		1
 
 /* DEFAULT VALUE FOR SENSORS */
@@ -51,8 +53,8 @@
 #define ST_PRESS_1_FS_ADDR			0x23
 #define ST_PRESS_1_FS_MASK			0x30
 #define ST_PRESS_1_FS_AVL_1260_VAL		0x00
-#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_MBAR_TO_KPASCAL(244141)
 #define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
+#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_KPASCAL_NANO_SCALE
 #define ST_PRESS_1_BDU_ADDR			0x20
 #define ST_PRESS_1_BDU_MASK			0x04
 #define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
-- 
1.7.5.4

Re: [PATCH 1/2] iio: lps331ap: Fix wrong in_pressure_scale output value

[Jonathan Cameron]

On 07/02/2013 11:13 AM, Jacek Anaszewski wrote:
> This patch fixes improper in_pressure_scale output that is
> returned by the lps331ap barometer sensor driver. According
> to the documentation the pressure after applying the scale has to
> be expressed in kilopascal units. With erroneous implementation
> the scale value larger by two orders of magnitude is returned -
> 2441410 instead of 24414.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Applied to the fixes-togreg branch of iio.git

Patch 2 doesn't want to go in as a fix so I may end up holding that
one for a while until I can apply it without a whole pile of merge
conflicts.

Jonathan
> ---
>  drivers/iio/pressure/st_pressure_core.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index aacea2a..89eb40a 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -28,7 +28,9 @@
>  #include <linux/iio/common/st_sensors.h>
>  #include "st_pressure.h"
>  
> -#define ST_PRESS_MBAR_TO_KPASCAL(x)		(x * 10)
> +#define ST_PRESS_LSB_PER_MBAR			4096UL
> +#define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
> +						 ST_PRESS_LSB_PER_MBAR)
>  #define ST_PRESS_NUMBER_DATA_CHANNELS		1
>  
>  /* DEFAULT VALUE FOR SENSORS */
> @@ -51,8 +53,8 @@
>  #define ST_PRESS_1_FS_ADDR			0x23
>  #define ST_PRESS_1_FS_MASK			0x30
>  #define ST_PRESS_1_FS_AVL_1260_VAL		0x00
> -#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_MBAR_TO_KPASCAL(244141)
>  #define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
> +#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_KPASCAL_NANO_SCALE
>  #define ST_PRESS_1_BDU_ADDR			0x20
>  #define ST_PRESS_1_BDU_MASK			0x04
>  #define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
> 

[PATCH 2/2] iio: lps331ap: Modify in_temp_scale calculation way

[Jacek Anaszewski]

This patch modifies the way how the in_temp_scale output value is
calculated. With this implementation it is more clear how the value
is obtained.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/iio/pressure/st_pressure_core.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 89eb40a..09d94a6 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -31,6 +31,9 @@
 #define ST_PRESS_LSB_PER_MBAR			4096UL
 #define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
 						 ST_PRESS_LSB_PER_MBAR)
+#define ST_PRESS_LSB_PER_CELSIUS		480UL
+#define ST_PRESS_CELSIUS_NANO_SCALE		(1000000000UL / \
+						 ST_PRESS_LSB_PER_CELSIUS)
 #define ST_PRESS_NUMBER_DATA_CHANNELS		1
 
 /* DEFAULT VALUE FOR SENSORS */
@@ -53,8 +56,8 @@
 #define ST_PRESS_1_FS_ADDR			0x23
 #define ST_PRESS_1_FS_MASK			0x30
 #define ST_PRESS_1_FS_AVL_1260_VAL		0x00
-#define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
 #define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_KPASCAL_NANO_SCALE
+#define ST_PRESS_1_FS_AVL_TEMP_GAIN		ST_PRESS_CELSIUS_NANO_SCALE
 #define ST_PRESS_1_BDU_ADDR			0x20
 #define ST_PRESS_1_BDU_MASK			0x04
 #define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
-- 
1.7.5.4

Re: [PATCH 2/2] iio: lps331ap: Modify in_temp_scale calculation way

[Jonathan Cameron]

On 07/02/13 11:13, Jacek Anaszewski wrote:
> This patch modifies the way how the in_temp_scale output value is
> calculated. With this implementation it is more clear how the value
> is obtained.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

The fix (patch 1) has worked it's way through now to IIOs upstream so
this patch has now been added to the togreg branch of iio.git.

Thanks,

Jonathan

> ---
>  drivers/iio/pressure/st_pressure_core.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index 89eb40a..09d94a6 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -31,6 +31,9 @@
>  #define ST_PRESS_LSB_PER_MBAR			4096UL
>  #define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
>  						 ST_PRESS_LSB_PER_MBAR)
> +#define ST_PRESS_LSB_PER_CELSIUS		480UL
> +#define ST_PRESS_CELSIUS_NANO_SCALE		(1000000000UL / \
> +						 ST_PRESS_LSB_PER_CELSIUS)
>  #define ST_PRESS_NUMBER_DATA_CHANNELS		1
>  
>  /* DEFAULT VALUE FOR SENSORS */
> @@ -53,8 +56,8 @@
>  #define ST_PRESS_1_FS_ADDR			0x23
>  #define ST_PRESS_1_FS_MASK			0x30
>  #define ST_PRESS_1_FS_AVL_1260_VAL		0x00
> -#define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
>  #define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_KPASCAL_NANO_SCALE
> +#define ST_PRESS_1_FS_AVL_TEMP_GAIN		ST_PRESS_CELSIUS_NANO_SCALE
>  #define ST_PRESS_1_BDU_ADDR			0x20
>  #define ST_PRESS_1_BDU_MASK			0x04
>  #define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
> 

Re: [PATCH 0/2] Fix erroneous pressure scaling for the lps331ap driver

[Jacek Anaszewski]

Adding Denis.

On 07/02/2013 12:13 PM, Jacek Anaszewski wrote:
> This series fixes pressure scale value calculation and modifies the
> way how the temperature scale value is calculated, for clarity
> and consistency reasons. Below the detailed explanation
> for the fix is provided.
>
> Because of the IIO_VAL_INT_PLUS_NANO value returned by the
> read_raw callback for the IIO_CHAN_INFO_SCALE mask case
> the returned scale value has to be expressed in nanopascal
> units. The scale value is used then for obtaining pressure
> value in kilopascal units. Currently this value is 2441410,
> whereas it should be 24414.
>
> 1. Scaling of exemplary read_raw value:
>
> exemplary read raw result: 4067471
> buggy_scale: 0.002441410 =>  2441410 * 10^-9
> buggy_pressure: 4067471 * 0.002441410 =
> 9930.4
> According to the documentation file
> Documentation/ABI/testing/sysfs-bus-iio the scaled pressure
> value is expressed in kilopascals, so the result should
> be interpreted as 9930.4 kPa
>
> 2. Scaling of the device output value according to the data sheet
>
> In order to get the result in mbars according to the device
> data sheet the output value has to be divided by 4096 (LSB/mbar)
>
> pressure [mbar] = 4067471 / 4096 = 993.0349
>
> Given that 1 bar = 10^5 Pa the result in pascal units is
> 993.0349 * 10^-3 * 10^5 Pa = 993.0349 * 10^2 Pa = 993 hPa =>
> 99.3 kPa
>
> whereas 4067471 * 0.002441410 (the buggy_scale value) = 9930.4,
> which when considered as a result in kilopascals is not true
> according to the above reasoning.
>
> 3. Proposed fix
>
> The proposed fix introduces following constants:
>
> ST_PRESS_LSB_PER_MBAR (4096.0) - LSB/mbar
>
> ST_PRESS_KPASCAL_NANO_SCALE (100000000 / ST_PRESS_LSB_PER_MBAR) -
> actual scale factor to be returned by in_pressure_scale which after
> performing division gives the value 24414.
> in_pressure_scale returns 0.000024414 after applying nano scale.
>
> The final result in kilopascal units can be obtained as follows:
>
> in_pressure_raw * in_pressure_scale =
> = 4067471 * 24414 * 10^-9 = 4.067471 * 10^6 * 2.4414 * 10^4 * 10^-9 =
> = 9.930 * 10^1 = 99.3 kPa =>  993 hPa (same as the pressure result
> obtained in the paragraph 2.)
>
> Summarizing, the bug was in the ST_PRESS_MBAR_TO_KPASCAL macro
> which performed multiplication instead of division.
>
> The second patch applies similar operations for the temperature scale
> calculaton. Previous implementation was correct but the modification
> is made for consistency and clarity.
>
> Thanks,
> Jacek
>
> Jacek Anaszewski (2):
>    iio: lps331ap: Fix wrong in_pressure_scale output value
>    iio: lps331ap: Modify in_temp_scale calculation way
>
>   drivers/iio/pressure/st_pressure_core.c |   11 ++++++++---
>   1 files changed, 8 insertions(+), 3 deletions(-)
>

Re: [PATCH 0/2] Fix erroneous pressure scaling for the lps331ap driver

[Jonathan Cameron]

Denis,

Any comments on this series?

(Jacek, I'm not entirely sure if you managed to add Denis as intended
in your last email - certainly no sign of his email when it got to me.)

On 07/02/2013 12:06 PM, Jacek Anaszewski wrote:
> Adding Denis.
> 
> On 07/02/2013 12:13 PM, Jacek Anaszewski wrote:
>> This series fixes pressure scale value calculation and modifies the
>> way how the temperature scale value is calculated, for clarity
>> and consistency reasons. Below the detailed explanation
>> for the fix is provided.
>>
>> Because of the IIO_VAL_INT_PLUS_NANO value returned by the
>> read_raw callback for the IIO_CHAN_INFO_SCALE mask case
>> the returned scale value has to be expressed in nanopascal
>> units. The scale value is used then for obtaining pressure
>> value in kilopascal units. Currently this value is 2441410,
>> whereas it should be 24414.
>>
>> 1. Scaling of exemplary read_raw value:
>>
>> exemplary read raw result: 4067471
>> buggy_scale: 0.002441410 =>  2441410 * 10^-9
>> buggy_pressure: 4067471 * 0.002441410 =
>> 9930.4
>> According to the documentation file
>> Documentation/ABI/testing/sysfs-bus-iio the scaled pressure
>> value is expressed in kilopascals, so the result should
>> be interpreted as 9930.4 kPa
>>
>> 2. Scaling of the device output value according to the data sheet
>>
>> In order to get the result in mbars according to the device
>> data sheet the output value has to be divided by 4096 (LSB/mbar)
>>
>> pressure [mbar] = 4067471 / 4096 = 993.0349
>>
>> Given that 1 bar = 10^5 Pa the result in pascal units is
>> 993.0349 * 10^-3 * 10^5 Pa = 993.0349 * 10^2 Pa = 993 hPa =>
>> 99.3 kPa
>>
>> whereas 4067471 * 0.002441410 (the buggy_scale value) = 9930.4,
>> which when considered as a result in kilopascals is not true
>> according to the above reasoning.
>>
>> 3. Proposed fix
>>
>> The proposed fix introduces following constants:
>>
>> ST_PRESS_LSB_PER_MBAR (4096.0) - LSB/mbar
>>
>> ST_PRESS_KPASCAL_NANO_SCALE (100000000 / ST_PRESS_LSB_PER_MBAR) -
>> actual scale factor to be returned by in_pressure_scale which after
>> performing division gives the value 24414.
>> in_pressure_scale returns 0.000024414 after applying nano scale.
>>
>> The final result in kilopascal units can be obtained as follows:
>>
>> in_pressure_raw * in_pressure_scale =
>> = 4067471 * 24414 * 10^-9 = 4.067471 * 10^6 * 2.4414 * 10^4 * 10^-9 =
>> = 9.930 * 10^1 = 99.3 kPa =>  993 hPa (same as the pressure result
>> obtained in the paragraph 2.)
>>
>> Summarizing, the bug was in the ST_PRESS_MBAR_TO_KPASCAL macro
>> which performed multiplication instead of division.
>>
>> The second patch applies similar operations for the temperature scale
>> calculaton. Previous implementation was correct but the modification
>> is made for consistency and clarity.
>>
>> Thanks,
>> Jacek
>>
>> Jacek Anaszewski (2):
>>    iio: lps331ap: Fix wrong in_pressure_scale output value
>>    iio: lps331ap: Modify in_temp_scale calculation way
>>
>>   drivers/iio/pressure/st_pressure_core.c |   11 ++++++++---
>>   1 files changed, 8 insertions(+), 3 deletions(-)
>>
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html