diff mbox series

[v2] iio: cros_ec: Fix the maths for gyro scale calculation

Message ID 20190307151041.10113-1-enric.balletbo@collabora.com (mailing list archive)
State New, archived
Headers show
Series [v2] iio: cros_ec: Fix the maths for gyro scale calculation | expand

Commit Message

Enric Balletbo i Serra March 7, 2019, 3:10 p.m. UTC 
From: Gwendal Grignou <gwendal@chromium.org>

Calculation did not use IIO_DEGREE_TO_RAD and implemented a variant to
avoid precision loss as we aim a nano value. The offset added to avoid
rounding error, though, doesn't give us a close result to the expected
value. E.g.

For 1000dps, the result should be:

    (1000 * pi ) / 180 >> 15 ~= 0.000532632218

But with current calculation we get

    $ cat scale
    0.000547890

Fix the calculation by just doing the maths involved for a nano value

   val * pi * 10e12 / (180 * 2^15)

so we get a closer result.

    $ cat scale
    0.000532632

Fixes: c14dca07a31d ("iio: cros_ec_sensors: add ChromeOS EC Contiguous Sensors driver")
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v2:
- Do the maths instead of play with dividers as pointed by Jonathan.

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

kernel test robot March 10, 2019, 8:40 a.m. UTC | #1 
Hi Gwendal,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on v5.0 next-20190306]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/iio-cros_ec-Fix-the-maths-for-gyro-scale-calculation/20190310-155057
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-x005-201910 (attached as .config)
compiler: gcc-8 (Debian 8.3.0-2) 8.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c: In function 'cros_ec_sensors_read':
>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c:107:4: warning: this decimal constant is unsigned only in ISO C90
       *val2 = div_s64(val64 * 3141592653,
       ^

vim +107 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c

    44	
    45	static int cros_ec_sensors_read(struct iio_dev *indio_dev,
    46				  struct iio_chan_spec const *chan,
    47				  int *val, int *val2, long mask)
    48	{
    49		struct cros_ec_sensors_state *st = iio_priv(indio_dev);
    50		s16 data = 0;
    51		s64 val64;
    52		int i;
    53		int ret;
    54		int idx = chan->scan_index;
    55	
    56		mutex_lock(&st->core.cmd_lock);
    57	
    58		switch (mask) {
    59		case IIO_CHAN_INFO_RAW:
    60			ret = st->core.read_ec_sensors_data(indio_dev, 1 << idx, &data);
    61			if (ret < 0)
    62				break;
    63			ret = IIO_VAL_INT;
    64			*val = data;
    65			break;
    66		case IIO_CHAN_INFO_CALIBBIAS:
    67			st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
    68			st->core.param.sensor_offset.flags = 0;
    69	
    70			ret = cros_ec_motion_send_host_cmd(&st->core, 0);
    71			if (ret < 0)
    72				break;
    73	
    74			/* Save values */
    75			for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
    76				st->core.calib[i] =
    77					st->core.resp->sensor_offset.offset[i];
    78			ret = IIO_VAL_INT;
    79			*val = st->core.calib[idx];
    80			break;
    81		case IIO_CHAN_INFO_SCALE:
    82			st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_RANGE;
    83			st->core.param.sensor_range.data = EC_MOTION_SENSE_NO_VALUE;
    84	
    85			ret = cros_ec_motion_send_host_cmd(&st->core, 0);
    86			if (ret < 0)
    87				break;
    88	
    89			val64 = st->core.resp->sensor_range.ret;
    90			switch (st->core.type) {
    91			case MOTIONSENSE_TYPE_ACCEL:
    92				/*
    93				 * EC returns data in g, iio exepects m/s^2.
    94				 * Do not use IIO_G_TO_M_S_2 to avoid precision loss.
    95				 */
    96				*val = div_s64(val64 * 980665, 10);
    97				*val2 = 10000 << (CROS_EC_SENSOR_BITS - 1);
    98				ret = IIO_VAL_FRACTIONAL;
    99				break;
   100			case MOTIONSENSE_TYPE_GYRO:
   101				/*
   102				 * EC returns data in dps, iio expects rad/s.
   103				 * Do not use IIO_DEGREE_TO_RAD to avoid precision
   104				 * loss. Round to the nearest integer.
   105				 */
   106				*val = 0;
 > 107				*val2 = div_s64(val64 * 3141592653,
   108						180 << (CROS_EC_SENSOR_BITS - 1));
   109				ret = IIO_VAL_INT_PLUS_NANO;
   110				break;
   111			case MOTIONSENSE_TYPE_MAG:
   112				/*
   113				 * EC returns data in 16LSB / uT,
   114				 * iio expects Gauss
   115				 */
   116				*val = val64;
   117				*val2 = 100 << (CROS_EC_SENSOR_BITS - 1);
   118				ret = IIO_VAL_FRACTIONAL;
   119				break;
   120			default:
   121				ret = -EINVAL;
   122			}
   123			break;
   124		default:
   125			ret = cros_ec_sensors_core_read(&st->core, chan, val, val2,
   126							mask);
   127			break;
   128		}
   129		mutex_unlock(&st->core.cmd_lock);
   130	
   131		return ret;
   132	}
   133	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 89cb0066a6e0..401b22e08d48 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -103,9 +103,10 @@  static int cros_ec_sensors_read(struct iio_dev *indio_dev,
 			 * Do not use IIO_DEGREE_TO_RAD to avoid precision
 			 * loss. Round to the nearest integer.
 			 */
-			*val = div_s64(val64 * 314159 + 9000000ULL, 1000);
-			*val2 = 18000 << (CROS_EC_SENSOR_BITS - 1);
-			ret = IIO_VAL_FRACTIONAL;
+			*val = 0;
+			*val2 = div_s64(val64 * 3141592653,
+					180 << (CROS_EC_SENSOR_BITS - 1));
+			ret = IIO_VAL_INT_PLUS_NANO;
 			break;
 		case MOTIONSENSE_TYPE_MAG:
 			/*