[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230716144827.5b084922@jic23-huawei>
Date: Sun, 16 Jul 2023 14:48:27 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Roan van Dijk <roan@...tonic.nl>
Cc: lars@...afoo.de, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: chemical: scd4x: Add pressure compensation
On Tue, 11 Jul 2023 12:14:19 +0200
Roan van Dijk <roan@...tonic.nl> wrote:
> This patch adds pressure compensation to the scd4x driver. The pressure can
> be written to the sensor in hPa. The pressure will be compensated
> internally by the sensor.
>
> Signed-off-by: Roan van Dijk <roan@...tonic.nl>
Hi Roan,
I made a tiny tweak to alignment.. (see below) and applied to the togreg
branch of iio.git which I'll initially push out as testing for 0-day to
see if it can find anything we missed.
Thanks,
Jonathan
> ---
>
> Changes since v2:
> - Fix for kernel test robot errors of uninitialized symbols
> - Changed pressure compensation to output channel instead of a channel
> with calibbias
> - Pressure can now be compensated during measurements
>
> drivers/iio/chemical/scd4x.c | 79 +++++++++++++++++++++++++++++++++---
> 1 file changed, 73 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index a4f22d926400..4bb5d215c54c 100644
> --- a/drivers/iio/chemical/scd4x.c
> +++ b/drivers/iio/chemical/scd4x.c
> @@ -36,6 +36,8 @@
> #define SCD4X_WRITE_BUF_SIZE 5
> #define SCD4X_FRC_MIN_PPM 0
> #define SCD4X_FRC_MAX_PPM 2000
> +#define SCD4X_PRESSURE_COMP_MIN_MBAR 700
> +#define SCD4X_PRESSURE_COMP_MAX_MBAR 1200
> #define SCD4X_READY_MASK 0x01
>
> /*Commands SCD4X*/
> @@ -45,6 +47,8 @@ enum scd4x_cmd {
> CMD_STOP_MEAS = 0x3f86,
> CMD_SET_TEMP_OFFSET = 0x241d,
> CMD_GET_TEMP_OFFSET = 0x2318,
> + CMD_SET_AMB_PRESSURE = 0xe000,
> + CMD_GET_AMB_PRESSURE = 0xe000,
> CMD_FRC = 0x362f,
> CMD_SET_ASC = 0x2416,
> CMD_GET_ASC = 0x2313,
> @@ -137,7 +141,8 @@ static int scd4x_read(struct scd4x_state *state, enum scd4x_cmd cmd,
> * Measurement needs to be stopped before sending commands.
> * Except for reading measurement and data ready command.
> */
> - if ((cmd != CMD_GET_DATA_READY) && (cmd != CMD_READ_MEAS)) {
> + if ((cmd != CMD_GET_DATA_READY) && (cmd != CMD_READ_MEAS) &&
> + (cmd != CMD_GET_AMB_PRESSURE)) {
Alignment here odd...
> ret = scd4x_send_command(state, CMD_STOP_MEAS);
> if (ret)
> return ret;
> @@ -166,7 +171,8 @@ static int scd4x_read(struct scd4x_state *state, enum scd4x_cmd cmd,
> }
>
> /* start measurement */
> - if ((cmd != CMD_GET_DATA_READY) && (cmd != CMD_READ_MEAS)) {
> + if ((cmd != CMD_GET_DATA_READY) && (cmd != CMD_READ_MEAS) &&
> + (cmd != CMD_GET_AMB_PRESSURE)) {
Also here
> ret = scd4x_send_command(state, CMD_START_MEAS);
> if (ret)
> return ret;
> @@ -188,9 +194,11 @@ static int scd4x_write(struct scd4x_state *state, enum scd4x_cmd cmd, uint16_t a
> buf[4] = crc;
>
> /* measurement needs to be stopped before sending commands */
> - ret = scd4x_send_command(state, CMD_STOP_MEAS);
> - if (ret)
> - return ret;
> + if (cmd != CMD_SET_AMB_PRESSURE) {
> + ret = scd4x_send_command(state, CMD_STOP_MEAS);
> + if (ret)
> + return ret;
> + }
>
> /* execution time */
> msleep_interruptible(500);
> @@ -200,7 +208,7 @@ static int scd4x_write(struct scd4x_state *state, enum scd4x_cmd cmd, uint16_t a
> return ret;
>
> /* start measurement, except for forced calibration command */
> - if (cmd != CMD_FRC) {
> + if ((cmd != CMD_FRC) && (cmd != CMD_SET_AMB_PRESSURE)) {
> ret = scd4x_send_command(state, CMD_START_MEAS);
> if (ret)
> return ret;
> @@ -338,6 +346,18 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> + if (chan->output) {
> + mutex_lock(&state->lock);
> + ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));
> + mutex_unlock(&state->lock);
> +
> + if (ret)
> + return ret;
> +
> + *val = be16_to_cpu(tmp);
> + return IIO_VAL_INT;
> + }
> +
> ret = iio_device_claim_direct_mode(indio_dev);
> if (ret)
> return ret;
> @@ -386,6 +406,25 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static const int scd4x_pressure_calibbias_available[] = {
> + SCD4X_PRESSURE_COMP_MIN_MBAR, 1, SCD4X_PRESSURE_COMP_MAX_MBAR,
> +};
> +
> +static int scd4x_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *vals = scd4x_pressure_calibbias_available;
> + *type = IIO_VAL_INT;
> +
> + return IIO_AVAIL_RANGE;
> + }
> +
> + return -EINVAL;
> +}
> +
> +
> static int scd4x_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> int val, int val2, long mask)
> {
> @@ -399,6 +438,21 @@ static int scd4x_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
> mutex_unlock(&state->lock);
>
> return ret;
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_PRESSURE:
> + if (val < SCD4X_PRESSURE_COMP_MIN_MBAR ||
> + val > SCD4X_PRESSURE_COMP_MAX_MBAR)
> + return -EINVAL;
> +
> + mutex_lock(&state->lock);
> + ret = scd4x_write(state, CMD_SET_AMB_PRESSURE, val);
> + mutex_unlock(&state->lock);
> +
> + return ret;
> + default:
> + return -EINVAL;
> + }
> default:
> return -EINVAL;
> }
> @@ -503,9 +557,22 @@ static const struct iio_info scd4x_info = {
> .attrs = &scd4x_attr_group,
> .read_raw = scd4x_read_raw,
> .write_raw = scd4x_write_raw,
> + .read_avail = scd4x_read_avail,
> };
>
> static const struct iio_chan_spec scd4x_channels[] = {
> + {
> + /*
> + * this channel is special in a sense we are pretending that
> + * sensor is able to change measurement chamber pressure but in
> + * fact we're just setting pressure compensation value
> + */
> + .type = IIO_PRESSURE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
> + .output = 1,
> + .scan_index = -1,
> + },
> {
> .type = IIO_CONCENTRATION,
> .channel2 = IIO_MOD_CO2,
Powered by blists - more mailing lists