[PATCH] iio: chemical: scd4x: Add pressure compensation

Roan van Dijk posted 1 patch 2 years, 7 months ago
drivers/iio/chemical/scd4x.c | 41 +++++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
[PATCH] iio: chemical: scd4x: Add pressure compensation
Posted by Roan van Dijk 2 years, 7 months ago
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@protonic.nl>
---
 drivers/iio/chemical/scd4x.c | 41 +++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
index a4f22d926400..fe6b3f3f7186 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_AMB_PRESSURE_MIN 700
+#define SCD4X_AMB_PRESSURE_MAX 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,
@@ -373,7 +377,10 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_CALIBBIAS:
 		mutex_lock(&state->lock);
-		ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
+		if (chan->type == IIO_TEMP)
+			ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
+		else if (chan->type == IIO_PRESSURE)
+			ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));
 		mutex_unlock(&state->lock);
 		if (ret)
 			return ret;
@@ -386,6 +393,25 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static const int scd4x_pressure_calibbias_available[] = {
+	SCD4X_AMB_PRESSURE_MIN, 1, SCD4X_AMB_PRESSURE_MAX,
+};
+
+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_CALIBBIAS:
+		*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)
 {
@@ -395,9 +421,11 @@ static int scd4x_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
 	switch (mask) {
 	case IIO_CHAN_INFO_CALIBBIAS:
 		mutex_lock(&state->lock);
-		ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
+		if (chan->type == IIO_TEMP)
+			ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
+		else if (chan->type == IIO_PRESSURE)
+			ret = scd4x_write(state, CMD_SET_AMB_PRESSURE, val);
 		mutex_unlock(&state->lock);
-
 		return ret;
 	default:
 		return -EINVAL;
@@ -503,9 +531,16 @@ 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[] = {
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.scan_index = -1,
+	},
 	{
 		.type = IIO_CONCENTRATION,
 		.channel2 = IIO_MOD_CO2,
-- 
2.39.2
Re: [PATCH] iio: chemical: scd4x: Add pressure compensation
Posted by kernel test robot 2 years, 7 months ago
Hi Roan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.4 next-20230704]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Roan-van-Dijk/iio-chemical-scd4x-Add-pressure-compensation/20230704-170621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20230704084706.370637-1-roan%40protonic.nl
patch subject: [PATCH] iio: chemical: scd4x: Add pressure compensation
config: powerpc-randconfig-r001-20230704 (https://download.01.org/0day-ci/archive/20230704/202307041943.RlL71CKd-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230704/202307041943.RlL71CKd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307041943.RlL71CKd-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:677:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
      47 | DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      48 |                  (p, b, c), pio, p)
         |                  ~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:674:3: note: expanded from macro 'DEF_PCI_AC_NORET'
     674 |                 __do_##name al;                                 \
         |                 ^~~~~~~~~~~~~~
   <scratch space>:139:1: note: expanded from here
     139 | __do_insl
         | ^
   arch/powerpc/include/asm/io.h:616:56: note: expanded from macro '__do_insl'
     616 | #define __do_insl(p, b, n)      readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
         |                                        ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/iio/chemical/scd4x.c:18:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:677:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
      49 | DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      50 |                  (p, b, c), pio, p)
         |                  ~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:674:3: note: expanded from macro 'DEF_PCI_AC_NORET'
     674 |                 __do_##name al;                                 \
         |                 ^~~~~~~~~~~~~~
   <scratch space>:143:1: note: expanded from here
     143 | __do_outsb
         | ^
   arch/powerpc/include/asm/io.h:617:58: note: expanded from macro '__do_outsb'
     617 | #define __do_outsb(p, b, n)     writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
         |                                         ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/iio/chemical/scd4x.c:18:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:677:
   arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
      51 | DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      52 |                  (p, b, c), pio, p)
         |                  ~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:674:3: note: expanded from macro 'DEF_PCI_AC_NORET'
     674 |                 __do_##name al;                                 \
         |                 ^~~~~~~~~~~~~~
   <scratch space>:147:1: note: expanded from here
     147 | __do_outsw
         | ^
   arch/powerpc/include/asm/io.h:618:58: note: expanded from macro '__do_outsw'
     618 | #define __do_outsw(p, b, n)     writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
         |                                         ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/iio/chemical/scd4x.c:18:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:677:
   arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
      53 | DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      54 |                  (p, b, c), pio, p)
         |                  ~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:674:3: note: expanded from macro 'DEF_PCI_AC_NORET'
     674 |                 __do_##name al;                                 \
         |                 ^~~~~~~~~~~~~~
   <scratch space>:151:1: note: expanded from here
     151 | __do_outsl
         | ^
   arch/powerpc/include/asm/io.h:619:58: note: expanded from macro '__do_outsl'
     619 | #define __do_outsl(p, b, n)     writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
         |                                         ~~~~~~~~~~~~~~~~~~~~~^
>> drivers/iio/chemical/scd4x.c:382:8: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     382 |                 else if (chan->type == IIO_PRESSURE)
         |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:28: note: expanded from macro 'if'
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:57:30: note: expanded from macro '__trace_if_var'
      57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/chemical/scd4x.c:385:7: note: uninitialized use occurs here
     385 |                 if (ret)
         |                     ^~~
   include/linux/compiler.h:55:47: note: expanded from macro 'if'
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                                               ^~~~
   include/linux/compiler.h:57:52: note: expanded from macro '__trace_if_var'
      57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
         |                                                    ^~~~
   drivers/iio/chemical/scd4x.c:382:8: note: remove the 'if' if its condition is always true
     382 |                 else if (chan->type == IIO_PRESSURE)
         |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     383 |                         ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));
         | ~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:55:23: note: expanded from macro 'if'
      55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
         |                       ^
   drivers/iio/chemical/scd4x.c:340:9: note: initialize the variable 'ret' to silence this warning
     340 |         int ret;
         |                ^
         |                 = 0
   7 warnings generated.


vim +382 drivers/iio/chemical/scd4x.c

   334	
   335	static int scd4x_read_raw(struct iio_dev *indio_dev,
   336				struct iio_chan_spec const *chan, int *val,
   337				int *val2, long mask)
   338	{
   339		struct scd4x_state *state = iio_priv(indio_dev);
   340		int ret;
   341		__be16 tmp;
   342	
   343		switch (mask) {
   344		case IIO_CHAN_INFO_RAW:
   345			ret = iio_device_claim_direct_mode(indio_dev);
   346			if (ret)
   347				return ret;
   348	
   349			mutex_lock(&state->lock);
   350			ret = scd4x_read_channel(state, chan->address);
   351			mutex_unlock(&state->lock);
   352	
   353			iio_device_release_direct_mode(indio_dev);
   354			if (ret < 0)
   355				return ret;
   356	
   357			*val = ret;
   358			return IIO_VAL_INT;
   359		case IIO_CHAN_INFO_SCALE:
   360			if (chan->type == IIO_CONCENTRATION) {
   361				*val = 0;
   362				*val2 = 100;
   363				return IIO_VAL_INT_PLUS_MICRO;
   364			} else if (chan->type == IIO_TEMP) {
   365				*val = 175000;
   366				*val2 = 65536;
   367				return IIO_VAL_FRACTIONAL;
   368			} else if (chan->type == IIO_HUMIDITYRELATIVE) {
   369				*val = 100000;
   370				*val2 = 65536;
   371				return IIO_VAL_FRACTIONAL;
   372			}
   373			return -EINVAL;
   374		case IIO_CHAN_INFO_OFFSET:
   375			*val = -16852;
   376			*val2 = 114286;
   377			return IIO_VAL_INT_PLUS_MICRO;
   378		case IIO_CHAN_INFO_CALIBBIAS:
   379			mutex_lock(&state->lock);
   380			if (chan->type == IIO_TEMP)
   381				ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
 > 382			else if (chan->type == IIO_PRESSURE)
   383				ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));
   384			mutex_unlock(&state->lock);
   385			if (ret)
   386				return ret;
   387	
   388			*val = be16_to_cpu(tmp);
   389	
   390			return IIO_VAL_INT;
   391		default:
   392			return -EINVAL;
   393		}
   394	}
   395	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] iio: chemical: scd4x: Add pressure compensation
Posted by Dan Carpenter 2 years, 7 months ago
Hi Roan,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Roan-van-Dijk/iio-chemical-scd4x-Add-pressure-compensation/20230704-170621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20230704084706.370637-1-roan%40protonic.nl
patch subject: [PATCH] iio: chemical: scd4x: Add pressure compensation
config: x86_64-randconfig-m001-20230705 (https://download.01.org/0day-ci/archive/20230705/202307052018.7DV5CAOH-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230705/202307052018.7DV5CAOH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202307052018.7DV5CAOH-lkp@intel.com/

smatch warnings:
drivers/iio/chemical/scd4x.c:385 scd4x_read_raw() error: uninitialized symbol 'ret'.
drivers/iio/chemical/scd4x.c:388 scd4x_read_raw() error: uninitialized symbol 'tmp'.

vim +/ret +385 drivers/iio/chemical/scd4x.c

49d22b695cbb69 Roan van Dijk 2021-10-08  335  static int scd4x_read_raw(struct iio_dev *indio_dev,
49d22b695cbb69 Roan van Dijk 2021-10-08  336  			struct iio_chan_spec const *chan, int *val,
49d22b695cbb69 Roan van Dijk 2021-10-08  337  			int *val2, long mask)
49d22b695cbb69 Roan van Dijk 2021-10-08  338  {
49d22b695cbb69 Roan van Dijk 2021-10-08  339  	struct scd4x_state *state = iio_priv(indio_dev);
49d22b695cbb69 Roan van Dijk 2021-10-08  340  	int ret;
49d22b695cbb69 Roan van Dijk 2021-10-08  341  	__be16 tmp;
49d22b695cbb69 Roan van Dijk 2021-10-08  342  
49d22b695cbb69 Roan van Dijk 2021-10-08  343  	switch (mask) {
49d22b695cbb69 Roan van Dijk 2021-10-08  344  	case IIO_CHAN_INFO_RAW:
49d22b695cbb69 Roan van Dijk 2021-10-08  345  		ret = iio_device_claim_direct_mode(indio_dev);
49d22b695cbb69 Roan van Dijk 2021-10-08  346  		if (ret)
49d22b695cbb69 Roan van Dijk 2021-10-08  347  			return ret;
49d22b695cbb69 Roan van Dijk 2021-10-08  348  
49d22b695cbb69 Roan van Dijk 2021-10-08  349  		mutex_lock(&state->lock);
49d22b695cbb69 Roan van Dijk 2021-10-08  350  		ret = scd4x_read_channel(state, chan->address);
49d22b695cbb69 Roan van Dijk 2021-10-08  351  		mutex_unlock(&state->lock);
49d22b695cbb69 Roan van Dijk 2021-10-08  352  
49d22b695cbb69 Roan van Dijk 2021-10-08  353  		iio_device_release_direct_mode(indio_dev);
49d22b695cbb69 Roan van Dijk 2021-10-08  354  		if (ret < 0)
49d22b695cbb69 Roan van Dijk 2021-10-08  355  			return ret;
49d22b695cbb69 Roan van Dijk 2021-10-08  356  
49d22b695cbb69 Roan van Dijk 2021-10-08  357  		*val = ret;
49d22b695cbb69 Roan van Dijk 2021-10-08  358  		return IIO_VAL_INT;
49d22b695cbb69 Roan van Dijk 2021-10-08  359  	case IIO_CHAN_INFO_SCALE:
e46e2512ac84bd Roan van Dijk 2021-10-21  360  		if (chan->type == IIO_CONCENTRATION) {
e46e2512ac84bd Roan van Dijk 2021-10-21  361  			*val = 0;
e46e2512ac84bd Roan van Dijk 2021-10-21  362  			*val2 = 100;
e46e2512ac84bd Roan van Dijk 2021-10-21  363  			return IIO_VAL_INT_PLUS_MICRO;
e46e2512ac84bd Roan van Dijk 2021-10-21  364  		} else if (chan->type == IIO_TEMP) {
49d22b695cbb69 Roan van Dijk 2021-10-08  365  			*val = 175000;
49d22b695cbb69 Roan van Dijk 2021-10-08  366  			*val2 = 65536;
49d22b695cbb69 Roan van Dijk 2021-10-08  367  			return IIO_VAL_FRACTIONAL;
49d22b695cbb69 Roan van Dijk 2021-10-08  368  		} else if (chan->type == IIO_HUMIDITYRELATIVE) {
49d22b695cbb69 Roan van Dijk 2021-10-08  369  			*val = 100000;
49d22b695cbb69 Roan van Dijk 2021-10-08  370  			*val2 = 65536;
49d22b695cbb69 Roan van Dijk 2021-10-08  371  			return IIO_VAL_FRACTIONAL;
49d22b695cbb69 Roan van Dijk 2021-10-08  372  		}
49d22b695cbb69 Roan van Dijk 2021-10-08  373  		return -EINVAL;
49d22b695cbb69 Roan van Dijk 2021-10-08  374  	case IIO_CHAN_INFO_OFFSET:
49d22b695cbb69 Roan van Dijk 2021-10-08  375  		*val = -16852;
49d22b695cbb69 Roan van Dijk 2021-10-08  376  		*val2 = 114286;
49d22b695cbb69 Roan van Dijk 2021-10-08  377  		return IIO_VAL_INT_PLUS_MICRO;
49d22b695cbb69 Roan van Dijk 2021-10-08  378  	case IIO_CHAN_INFO_CALIBBIAS:
49d22b695cbb69 Roan van Dijk 2021-10-08  379  		mutex_lock(&state->lock);
8e4a309948cffe Roan van Dijk 2023-07-04  380  		if (chan->type == IIO_TEMP)
49d22b695cbb69 Roan van Dijk 2021-10-08  381  			ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
8e4a309948cffe Roan van Dijk 2023-07-04  382  		else if (chan->type == IIO_PRESSURE)
8e4a309948cffe Roan van Dijk 2023-07-04  383  			ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));

ret is uninitialized if type != IIO_TEMP and != IIO_PRESSURE.

49d22b695cbb69 Roan van Dijk 2021-10-08  384  		mutex_unlock(&state->lock);
49d22b695cbb69 Roan van Dijk 2021-10-08 @385  		if (ret)
49d22b695cbb69 Roan van Dijk 2021-10-08  386  			return ret;
49d22b695cbb69 Roan van Dijk 2021-10-08  387  
49d22b695cbb69 Roan van Dijk 2021-10-08 @388  		*val = be16_to_cpu(tmp);
49d22b695cbb69 Roan van Dijk 2021-10-08  389  
49d22b695cbb69 Roan van Dijk 2021-10-08  390  		return IIO_VAL_INT;
49d22b695cbb69 Roan van Dijk 2021-10-08  391  	default:
49d22b695cbb69 Roan van Dijk 2021-10-08  392  		return -EINVAL;
49d22b695cbb69 Roan van Dijk 2021-10-08  393  	}
49d22b695cbb69 Roan van Dijk 2021-10-08  394  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] iio: chemical: scd4x: Add pressure compensation
Posted by Jonathan Cameron 2 years, 7 months ago
On Tue,  4 Jul 2023 10:47:06 +0200
Roan van Dijk <roan@protonic.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@protonic.nl>

Why treat this as a channel with just calibbias?
From what I can recall we've previous treated such cases as an
output channel with the advantage that the units are then fully
defined.  I may well be forgetting some argument or a case that
does it with calibbias though.

Jonathan


> ---
>  drivers/iio/chemical/scd4x.c | 41 +++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/chemical/scd4x.c b/drivers/iio/chemical/scd4x.c
> index a4f22d926400..fe6b3f3f7186 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_AMB_PRESSURE_MIN 700
> +#define SCD4X_AMB_PRESSURE_MAX 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,
> @@ -373,7 +377,10 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		mutex_lock(&state->lock);
> -		ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
> +		if (chan->type == IIO_TEMP)
> +			ret = scd4x_read(state, CMD_GET_TEMP_OFFSET, &tmp, sizeof(tmp));
> +		else if (chan->type == IIO_PRESSURE)
> +			ret = scd4x_read(state, CMD_GET_AMB_PRESSURE, &tmp, sizeof(tmp));
>  		mutex_unlock(&state->lock);
>  		if (ret)
>  			return ret;
> @@ -386,6 +393,25 @@ static int scd4x_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static const int scd4x_pressure_calibbias_available[] = {
> +	SCD4X_AMB_PRESSURE_MIN, 1, SCD4X_AMB_PRESSURE_MAX,
> +};
> +
> +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_CALIBBIAS:
> +		*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)
>  {
> @@ -395,9 +421,11 @@ static int scd4x_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const
>  	switch (mask) {
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		mutex_lock(&state->lock);
> -		ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
> +		if (chan->type == IIO_TEMP)
> +			ret = scd4x_write(state, CMD_SET_TEMP_OFFSET, val);
> +		else if (chan->type == IIO_PRESSURE)
> +			ret = scd4x_write(state, CMD_SET_AMB_PRESSURE, val);
>  		mutex_unlock(&state->lock);
> -
>  		return ret;
>  	default:
>  		return -EINVAL;
> @@ -503,9 +531,16 @@ 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[] = {
> +	{
> +		.type = IIO_PRESSURE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_CALIBBIAS),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBBIAS),
> +		.scan_index = -1,
> +	},
>  	{
>  		.type = IIO_CONCENTRATION,
>  		.channel2 = IIO_MOD_CO2,