Re: [PATCH] iio: cros_ec: Add calibscale for 3d MEMS

From: Jonathan Cameron
Date: Sun Jul 21 2019 - 13:34:53 EST


On Thu, 18 Jul 2019 16:28:24 -0700
Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote:

> Add calibration scale support to accel, gyro and magnetometer.
>
> Check on eve with current firmware, check reading calibscale returns 1.0,
> check with newer firmware values are applied.
>
> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
Looks good to me.

Applied to the togreg branch of iio.git and pushed out as testing.
If anyone wants to comment, there is still time as I won't be
pushing that out as non rebasing for a while yet.

Thanks,

Jonathan

> ---
>
> .../common/cros_ec_sensors/cros_ec_sensors.c | 51 +++++++++++++++++--
> .../cros_ec_sensors/cros_ec_sensors_core.c | 2 +-
> drivers/iio/light/cros_ec_light_prox.c | 12 ++---
> drivers/iio/pressure/cros_ec_baro.c | 2 -
> .../linux/iio/common/cros_ec_sensors_core.h | 5 +-
> 5 files changed, 57 insertions(+), 15 deletions(-)
>
> 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 17af4e0fd5f8..2af09606c438 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -63,10 +63,35 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
>
> /* Save values */
> for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
> - st->core.calib[i] =
> + st->core.calib[i].offset =
> st->core.resp->sensor_offset.offset[i];
> ret = IIO_VAL_INT;
> - *val = st->core.calib[idx];
> + *val = st->core.calib[idx].offset;
> + break;
> + case IIO_CHAN_INFO_CALIBSCALE:
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_SCALE;
> + st->core.param.sensor_offset.flags = 0;
> +
> + ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> + if (ret == -EPROTO) {
> + /* Reading calibscale is not supported on older EC. */
> + *val = 1;
> + *val2 = 0;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + break;
> + } else if (ret) {
> + break;
> + }
> +
> + /* Save values */
> + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
> + st->core.calib[i].scale =
> + st->core.resp->sensor_scale.scale[i];
> +
> + *val = st->core.calib[idx].scale >> 15;
> + *val2 = ((st->core.calib[idx].scale & 0x7FFF) * 1000000LL) /
> + MOTION_SENSE_DEFAULT_SCALE;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> break;
> case IIO_CHAN_INFO_SCALE:
> st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_RANGE;
> @@ -134,7 +159,7 @@ static int cros_ec_sensors_write(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_CALIBBIAS:
> - st->core.calib[idx] = val;
> + st->core.calib[idx].offset = val;
>
> /* Send to EC for each axis, even if not complete */
> st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> @@ -142,10 +167,25 @@ static int cros_ec_sensors_write(struct iio_dev *indio_dev,
> MOTION_SENSE_SET_OFFSET;
> for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
> st->core.param.sensor_offset.offset[i] =
> - st->core.calib[i];
> + st->core.calib[i].offset;
> st->core.param.sensor_offset.temp =
> EC_MOTION_SENSE_INVALID_CALIB_TEMP;
>
> + ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> + break;
> + case IIO_CHAN_INFO_CALIBSCALE:
> + st->core.calib[idx].scale = val;
> + /* Send to EC for each axis, even if not complete */
> +
> + st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_SCALE;
> + st->core.param.sensor_offset.flags =
> + MOTION_SENSE_SET_OFFSET;
> + for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
> + st->core.param.sensor_scale.scale[i] =
> + st->core.calib[i].scale;
> + st->core.param.sensor_scale.temp =
> + EC_MOTION_SENSE_INVALID_CALIB_TEMP;
> +
> ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> break;
> case IIO_CHAN_INFO_SCALE:
> @@ -206,7 +246,8 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> /* Common part */
> channel->info_mask_separate =
> BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_CALIBBIAS);
> + BIT(IIO_CHAN_INFO_CALIBBIAS) |
> + BIT(IIO_CHAN_INFO_CALIBSCALE);
> channel->info_mask_shared_by_all =
> BIT(IIO_CHAN_INFO_SCALE) |
> BIT(IIO_CHAN_INFO_FREQUENCY) |
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 130362ca421b..96d5aa1f4bd5 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -118,7 +118,7 @@ static ssize_t cros_ec_sensors_calibrate(struct iio_dev *indio_dev,
> } else {
> /* Save values */
> for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
> - st->calib[i] = st->resp->perform_calib.offset[i];
> + st->calib[i].offset = st->resp->perform_calib.offset[i];
> }
> mutex_unlock(&st->cmd_lock);
>
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 308ee6ff2e22..b81746a99f1f 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -88,9 +88,10 @@ static int cros_ec_light_prox_read(struct iio_dev *indio_dev,
> }
>
> /* Save values */
> - st->core.calib[0] = st->core.resp->sensor_offset.offset[0];
> + st->core.calib[0].offset =
> + st->core.resp->sensor_offset.offset[0];
>
> - *val = st->core.calib[idx];
> + *val = st->core.calib[idx].offset;
> break;
> case IIO_CHAN_INFO_CALIBSCALE:
> /*
> @@ -134,11 +135,12 @@ static int cros_ec_light_prox_write(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_CALIBBIAS:
> - st->core.calib[idx] = val;
> + st->core.calib[idx].offset = val;
> /* Send to EC for each axis, even if not complete */
> st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_OFFSET;
> st->core.param.sensor_offset.flags = MOTION_SENSE_SET_OFFSET;
> - st->core.param.sensor_offset.offset[0] = st->core.calib[0];
> + st->core.param.sensor_offset.offset[0] =
> + st->core.calib[0].offset;
> st->core.param.sensor_offset.temp =
> EC_MOTION_SENSE_INVALID_CALIB_TEMP;
> if (cros_ec_motion_send_host_cmd(&st->core, 0))
> @@ -205,8 +207,6 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
> channel->ext_info = cros_ec_sensors_ext_info;
> channel->scan_type.sign = 'u';
>
> - state->core.calib[0] = 0;
> -
> /* Sensor specific */
> switch (state->core.type) {
> case MOTIONSENSE_TYPE_LIGHT:
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 034ce98d6e97..d3acba7ba582 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -152,8 +152,6 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
> channel->ext_info = cros_ec_sensors_ext_info;
> channel->scan_type.sign = 'u';
>
> - state->core.calib[0] = 0;
> -
> /* Sensor specific */
> switch (state->core.type) {
> case MOTIONSENSE_TYPE_BARO:
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index 0c636b9fe8d7..bb03a252bd04 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -62,7 +62,10 @@ struct cros_ec_sensors_core_state {
> enum motionsensor_type type;
> enum motionsensor_location loc;
>
> - s16 calib[CROS_EC_SENSOR_MAX_AXIS];
> + struct calib_data {
> + s16 offset;
> + u16 scale;
> + } calib[CROS_EC_SENSOR_MAX_AXIS];
>
> u8 samples[CROS_EC_SAMPLE_SIZE];
>