diff mbox series

[v4,2/2] iio: humidity: Add support for ENS210

Message ID 20240719-ens21x-v4-2-6044e48a376a@thegoodpenguin.co.uk (mailing list archive)
State Changes Requested
Headers show
Series iio: humidity: Add support for en210 sensor family | expand

Commit Message

Joshua Felmeden July 19, 2024, 12:50 p.m. UTC 
Add support for ENS210/ENS210A/ENS211/ENS212/ENS213A/ENS215.

The ENS21x is a family of temperature and relative humidity sensors with
accuracies tailored to the needs of specific applications.

Signed-off-by: Joshua Felmeden <jfelmeden@thegoodpenguin.co.uk>
---
 drivers/iio/humidity/Kconfig  |  11 ++
 drivers/iio/humidity/Makefile |   1 +
 drivers/iio/humidity/ens210.c | 341 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 353 insertions(+)

Comments

kernel test robot July 20, 2024, 9:19 a.m. UTC | #1 
Hi Joshua,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1ebab783647a9e3bf357002d5c4ff060c8474a0a]

url:    https://github.com/intel-lab-lkp/linux/commits/Joshua-Felmeden/dt-bindings-iio-humidity-add-ENS210-sensor-family/20240719-210648
base:   1ebab783647a9e3bf357002d5c4ff060c8474a0a
patch link:    https://lore.kernel.org/r/20240719-ens21x-v4-2-6044e48a376a%40thegoodpenguin.co.uk
patch subject: [PATCH v4 2/2] iio: humidity: Add support for ENS210
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20240720/202407201602.KrJ889wu-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project ad154281230d83ee551e12d5be48bb956ef47ed3)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240720/202407201602.KrJ889wu-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/202407201602.KrJ889wu-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/iio/humidity/ens210.c:19:
   In file included from include/linux/i2c.h:13:
   In file included from include/linux/acpi.h:14:
   In file included from include/linux/device.h:32:
   In file included from include/linux/device/driver.h:21:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:173:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2258:
   include/linux/vmstat.h:500:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     500 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     501 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:507:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     507 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     508 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:519:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     519 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     520 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:528:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     528 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     529 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/humidity/ens210.c:252:4: warning: format specifies type 'unsigned long' but the argument has underlying type 'unsigned int' [-Wformat]
     251 |                         "Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
         |                                                              ~~~~~
         |                                                              %04x
     252 |                         data->chip_info->part_id);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:160:67: note: expanded from macro 'dev_info'
     160 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                  ~~~     ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ~~~    ^~~~~~~~~~~
   drivers/iio/humidity/ens210.c:200:30: warning: unused variable 'id' [-Wunused-variable]
     200 |         const struct i2c_device_id *id = i2c_client_get_device_id(client);
         |                                     ^~
   7 warnings generated.


vim +252 drivers/iio/humidity/ens210.c

   197	
   198	static int ens210_probe(struct i2c_client *client)
   199	{
   200		const struct i2c_device_id *id = i2c_client_get_device_id(client);
   201		struct ens210_data *data;
   202		struct iio_dev *indio_dev;
   203		uint16_t part_id;
   204		int ret;
   205	
   206		if (!i2c_check_functionality(client->adapter,
   207				I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
   208				I2C_FUNC_SMBUS_WRITE_BYTE |
   209				I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
   210			dev_err_probe(&client->dev, -EOPNOTSUPP,
   211				"adapter does not support some i2c transactions\n");
   212			return -EOPNOTSUPP;
   213		}
   214	
   215		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
   216		if (!indio_dev)
   217			return -ENOMEM;
   218	
   219		data = iio_priv(indio_dev);
   220		i2c_set_clientdata(client, indio_dev);
   221		data->client = client;
   222		mutex_init(&data->lock);
   223		data->chip_info = i2c_get_match_data(client);
   224	
   225		ret = devm_regulator_get_enable(&client->dev, "vdd");
   226		if (ret)
   227			return ret;
   228	
   229		/* reset device */
   230		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
   231						ENS210_SYS_CTRL_SYS_RESET);
   232		if (ret)
   233			return ret;
   234	
   235		/* wait for device to become active */
   236		usleep_range(4000, 5000);
   237	
   238		/* disable low power mode */
   239		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL, 0x00);
   240		if (ret)
   241			return ret;
   242	
   243		/* wait for device to finish */
   244		usleep_range(4000, 5000);
   245	
   246		/* get part_id */
   247		part_id = i2c_smbus_read_word_data(client, ENS210_REG_PART_ID);
   248	
   249		if (part_id != data->chip_info->part_id) {
   250			dev_info(&client->dev,
   251				"Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
 > 252				data->chip_info->part_id);
   253		}
   254	
   255		/* reenable low power */
   256		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
   257						ENS210_SYS_CTRL_LOW_POWER_ENABLE);
   258		if (ret)
   259			return ret;
   260	
   261		indio_dev->name = data->chip_info->name;
   262		indio_dev->modes = INDIO_DIRECT_MODE;
   263		indio_dev->channels = ens210_channels;
   264		indio_dev->num_channels = ARRAY_SIZE(ens210_channels);
   265		indio_dev->info = &ens210_info;
   266	
   267		return devm_iio_device_register(&client->dev, indio_dev);
   268	}
   269
kernel test robot July 20, 2024, 9:29 a.m. UTC | #2 
Hi Joshua,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1ebab783647a9e3bf357002d5c4ff060c8474a0a]

url:    https://github.com/intel-lab-lkp/linux/commits/Joshua-Felmeden/dt-bindings-iio-humidity-add-ENS210-sensor-family/20240719-210648
base:   1ebab783647a9e3bf357002d5c4ff060c8474a0a
patch link:    https://lore.kernel.org/r/20240719-ens21x-v4-2-6044e48a376a%40thegoodpenguin.co.uk
patch subject: [PATCH v4 2/2] iio: humidity: Add support for ENS210
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20240720/202407201729.dZc0rJ8y-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240720/202407201729.dZc0rJ8y-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/202407201729.dZc0rJ8y-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/acpi.h:14,
                    from include/linux/i2c.h:13,
                    from drivers/iio/humidity/ens210.c:19:
   drivers/iio/humidity/ens210.c: In function 'ens210_probe':
>> drivers/iio/humidity/ens210.c:251:25: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
     251 |                         "Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:160:58: note: in expansion of macro 'dev_fmt'
     160 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/iio/humidity/ens210.c:250:17: note: in expansion of macro 'dev_info'
     250 |                 dev_info(&client->dev,
         |                 ^~~~~~~~
   drivers/iio/humidity/ens210.c:251:66: note: format string is defined here
     251 |                         "Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
         |                                                              ~~~~^
         |                                                                  |
         |                                                                  long unsigned int
         |                                                              %04x
>> drivers/iio/humidity/ens210.c:200:37: warning: unused variable 'id' [-Wunused-variable]
     200 |         const struct i2c_device_id *id = i2c_client_get_device_id(client);
         |                                     ^~
--
>> drivers/iio/humidity/ens210.c:83: warning: Function parameter or struct member 'chip_info' not described in 'ens210_data'
>> drivers/iio/humidity/ens210.c:83: warning: Excess struct member 'res_index' description in 'ens210_data'


vim +251 drivers/iio/humidity/ens210.c

    72	
    73	/**
    74	 * struct ens210_data - Humidity/Temperature sensor device structure
    75	 * @client:	i2c client
    76	 * @lock:	lock protecting the i2c conversion
    77	 * @res_index:	index to selected sensor resolution
    78	 */
    79	struct ens210_data {
    80		struct i2c_client *client;
    81		const struct ens210_chip_info *chip_info;
    82		struct mutex lock;
  > 83	};
    84	
    85	/* calculate 17-bit crc7 */
    86	static u8 ens210_crc7(u32 val)
    87	{
    88		__be32 val_be = (cpu_to_be32(val & 0x1ffff) >> 0x8);
    89	
    90		return crc7_be(0xde, (u8 *)&val_be, 3) >> 1;
    91	}
    92	
    93	static int ens210_get_measurement(struct iio_dev *indio_dev, bool temp, int *val)
    94	{
    95		u32 regval;
    96		u8 regval_le[3];
    97		int ret;
    98		struct ens210_data *data = iio_priv(indio_dev);
    99	
   100		/* assert read */
   101		ret = i2c_smbus_write_byte_data(data->client, ENS210_REG_SENS_START,
   102					  temp ? ENS210_SENS_START_T_START :
   103						 ENS210_SENS_START_H_START);
   104		if (ret)
   105			return ret;
   106	
   107		/* wait for conversion to be ready */
   108		msleep(data->chip_info->conv_time_msec);
   109	
   110		ret = i2c_smbus_read_byte_data(data->client,
   111					       ENS210_REG_SENS_STAT);
   112		if (ret < 0)
   113			return ret;
   114	
   115		/* perform read */
   116		ret = i2c_smbus_read_i2c_block_data(
   117			data->client, temp ? ENS210_REG_T_VAL : ENS210_REG_H_VAL, 3,
   118			(u8 *)&regval_le);
   119		if (ret < 0) {
   120			dev_err(&data->client->dev, "failed to read register");
   121			return -EIO;
   122		} else if (ret != 3) {
   123			dev_err(&indio_dev->dev, "expected 3 bytes, received %d\n", ret);
   124			return -EIO;
   125		}
   126	
   127		regval = get_unaligned_le24(regval_le);
   128		if (ens210_crc7(regval) != ((regval >> 17) & 0x7f)) {
   129			/* crc fail */
   130			dev_err(&indio_dev->dev, "ens invalid crc\n");
   131			return -EIO;
   132		}
   133	
   134		*val = regval & 0xffff;
   135		return IIO_VAL_INT;
   136	}
   137	
   138	static int ens210_read_raw(struct iio_dev *indio_dev,
   139				   struct iio_chan_spec const *channel, int *val,
   140				   int *val2, long mask)
   141	{
   142		struct ens210_data *data = iio_priv(indio_dev);
   143		int ret = -EINVAL;
   144	
   145		switch (mask) {
   146		case IIO_CHAN_INFO_RAW:
   147			scoped_guard(mutex, &data->lock) {
   148				ret = ens210_get_measurement(
   149					indio_dev, channel->type == IIO_TEMP, val);
   150				if (ret)
   151					return ret;
   152				return IIO_VAL_INT;
   153			}
   154			return ret;
   155		case IIO_CHAN_INFO_SCALE:
   156			if (channel->type == IIO_TEMP) {
   157				*val = 15;
   158				*val2 = 625000;
   159			} else {
   160				*val = 1;
   161				*val2 = 953125;
   162			}
   163			return IIO_VAL_INT_PLUS_MICRO;
   164		case IIO_CHAN_INFO_OFFSET:
   165			if (channel->type == IIO_TEMP) {
   166				*val = -17481;
   167				*val2 = 600000;
   168				ret = IIO_VAL_INT_PLUS_MICRO;
   169				break;
   170			}
   171			*val = 0;
   172			return IIO_VAL_INT;
   173		default:
   174			return -EINVAL;
   175		}
   176		return ret;
   177	}
   178	
   179	static const struct iio_chan_spec ens210_channels[] = {
   180		{
   181			.type = IIO_TEMP,
   182			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
   183					      BIT(IIO_CHAN_INFO_SCALE) |
   184					      BIT(IIO_CHAN_INFO_OFFSET),
   185		},
   186		{
   187			.type = IIO_HUMIDITYRELATIVE,
   188			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
   189					      BIT(IIO_CHAN_INFO_SCALE) |
   190					      BIT(IIO_CHAN_INFO_OFFSET),
   191		}
   192	};
   193	
   194	static const struct iio_info ens210_info = {
   195		.read_raw = ens210_read_raw,
   196	};
   197	
   198	static int ens210_probe(struct i2c_client *client)
   199	{
 > 200		const struct i2c_device_id *id = i2c_client_get_device_id(client);
   201		struct ens210_data *data;
   202		struct iio_dev *indio_dev;
   203		uint16_t part_id;
   204		int ret;
   205	
   206		if (!i2c_check_functionality(client->adapter,
   207				I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
   208				I2C_FUNC_SMBUS_WRITE_BYTE |
   209				I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
   210			dev_err_probe(&client->dev, -EOPNOTSUPP,
   211				"adapter does not support some i2c transactions\n");
   212			return -EOPNOTSUPP;
   213		}
   214	
   215		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
   216		if (!indio_dev)
   217			return -ENOMEM;
   218	
   219		data = iio_priv(indio_dev);
   220		i2c_set_clientdata(client, indio_dev);
   221		data->client = client;
   222		mutex_init(&data->lock);
   223		data->chip_info = i2c_get_match_data(client);
   224	
   225		ret = devm_regulator_get_enable(&client->dev, "vdd");
   226		if (ret)
   227			return ret;
   228	
   229		/* reset device */
   230		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
   231						ENS210_SYS_CTRL_SYS_RESET);
   232		if (ret)
   233			return ret;
   234	
   235		/* wait for device to become active */
   236		usleep_range(4000, 5000);
   237	
   238		/* disable low power mode */
   239		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL, 0x00);
   240		if (ret)
   241			return ret;
   242	
   243		/* wait for device to finish */
   244		usleep_range(4000, 5000);
   245	
   246		/* get part_id */
   247		part_id = i2c_smbus_read_word_data(client, ENS210_REG_PART_ID);
   248	
   249		if (part_id != data->chip_info->part_id) {
 > 250			dev_info(&client->dev,
 > 251				"Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
   252				data->chip_info->part_id);
   253		}
   254	
   255		/* reenable low power */
   256		ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
   257						ENS210_SYS_CTRL_LOW_POWER_ENABLE);
   258		if (ret)
   259			return ret;
   260	
   261		indio_dev->name = data->chip_info->name;
   262		indio_dev->modes = INDIO_DIRECT_MODE;
   263		indio_dev->channels = ens210_channels;
   264		indio_dev->num_channels = ARRAY_SIZE(ens210_channels);
   265		indio_dev->info = &ens210_info;
   266	
   267		return devm_iio_device_register(&client->dev, indio_dev);
   268	}
   269
Jonathan Cameron July 20, 2024, 4:54 p.m. UTC | #3 
On Fri, 19 Jul 2024 13:50:54 +0100
Joshua Felmeden <jfelmeden@thegoodpenguin.co.uk> wrote:

> Add support for ENS210/ENS210A/ENS211/ENS212/ENS213A/ENS215.
> 
> The ENS21x is a family of temperature and relative humidity sensors with
> accuracies tailored to the needs of specific applications.
> 
> Signed-off-by: Joshua Felmeden <jfelmeden@thegoodpenguin.co.uk>
Hi Joshua

Looking pretty good, but a few more minor comments.

Jonathan


> +
> +/**
> + * struct ens210_data - Humidity/Temperature sensor device structure
> + * @client:	i2c client
> + * @lock:	lock protecting the i2c conversion
> + * @res_index:	index to selected sensor resolution
> + */
> +struct ens210_data {
> +	struct i2c_client *client;
> +	const struct ens210_chip_info *chip_info;
> +	struct mutex lock;

Docs rather different to content. Make sure to run kernel-doc script
over the files and fix warnings.

> +};
> +
> +/* calculate 17-bit crc7 */
> +static u8 ens210_crc7(u32 val)
> +{
> +	__be32 val_be = (cpu_to_be32(val & 0x1ffff) >> 0x8);

drop excess outer brackets.

> +
> +	return crc7_be(0xde, (u8 *)&val_be, 3) >> 1;
> +}
> +
> +static int ens210_get_measurement(struct iio_dev *indio_dev, bool temp, int *val)
> +{
> +	u32 regval;
> +	u8 regval_le[3];
> +	int ret;
> +	struct ens210_data *data = iio_priv(indio_dev);
> +
> +	/* assert read */
> +	ret = i2c_smbus_write_byte_data(data->client, ENS210_REG_SENS_START,
> +				  temp ? ENS210_SENS_START_T_START :
> +					 ENS210_SENS_START_H_START);
> +	if (ret)
> +		return ret;
> +
> +	/* wait for conversion to be ready */
> +	msleep(data->chip_info->conv_time_msec);
> +
> +	ret = i2c_smbus_read_byte_data(data->client,
> +				       ENS210_REG_SENS_STAT);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* perform read */
> +	ret = i2c_smbus_read_i2c_block_data(
> +		data->client, temp ? ENS210_REG_T_VAL : ENS210_REG_H_VAL, 3,
> +		(u8 *)&regval_le);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "failed to read register");
> +		return -EIO;
> +	} else if (ret != 3) {
> +		dev_err(&indio_dev->dev, "expected 3 bytes, received %d\n", ret);

Use the i2c dev for all error messages.
The indio_dev->dev one is only available in some paths and not generally
as useful.

Add a local 
struct device *dev = &data->client->dev;
in places where you use it lots of times.

> +		return -EIO;
> +	}
> +
> +	regval = get_unaligned_le24(regval_le);
> +	if (ens210_crc7(regval) != ((regval >> 17) & 0x7f)) {

> +		/* crc fail */
> +		dev_err(&indio_dev->dev, "ens invalid crc\n");
> +		return -EIO;
> +	}
> +
> +	*val = regval & 0xffff;
I wondered what the 17th bit is and it seems to be a valid flag.
Should we be checking that before returning the data?

> +	return IIO_VAL_INT;
> +}
> +
> +static int ens210_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *channel, int *val,
> +			   int *val2, long mask)
> +{
> +	struct ens210_data *data = iio_priv(indio_dev);
> +	int ret = -EINVAL;

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		scoped_guard(mutex, &data->lock) {
> +			ret = ens210_get_measurement(
> +				indio_dev, channel->type == IIO_TEMP, val);
Odd line wrapping. I'd just use a slightly long line and format as
>			ret = ens210_get_measurement(indio_dev,
						     channel->type == IIO_TEMP,
						     val);


> +			if (ret)
> +				return ret;
> +			return IIO_VAL_INT;
> +		}
> +		return ret;
		unreachable();
The compiler may fail to figure out we can't get here so we tell it.
The drop the return ret;

> +	case IIO_CHAN_INFO_SCALE:
> +		if (channel->type == IIO_TEMP) {
> +			*val = 15;
> +			*val2 = 625000;
> +		} else {
> +			*val = 1;
> +			*val2 = 953125;
> +		}
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (channel->type == IIO_TEMP) {
> +			*val = -17481;
> +			*val2 = 600000;
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +			break;
> +		}
> +		*val = 0;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +	return ret;
Unreachable code I think, so drop it.

> +}
> +

> +static int ens210_probe(struct i2c_client *client)
> +{
> +	const struct i2c_device_id *id = i2c_client_get_device_id(client);
Don't think this is used any more. 
> +	struct ens210_data *data;
> +	struct iio_dev *indio_dev;
> +	uint16_t part_id;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +			I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
> +			I2C_FUNC_SMBUS_WRITE_BYTE |
> +			I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err_probe(&client->dev, -EOPNOTSUPP,
> +			"adapter does not support some i2c transactions\n");
> +		return -EOPNOTSUPP;
		return dev_err_probe()
don't worry about the slightly long line.

> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);

Used?  If not drop it.

> +	data->client = client;
> +	mutex_init(&data->lock);
> +	data->chip_info = i2c_get_match_data(client);
> +
> +	ret = devm_regulator_get_enable(&client->dev, "vdd");
> +	if (ret)
> +		return ret;
> +
> +	/* reset device */
> +	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
> +					ENS210_SYS_CTRL_SYS_RESET);
> +	if (ret)
> +		return ret;
> +
> +	/* wait for device to become active */
> +	usleep_range(4000, 5000);
> +
> +	/* disable low power mode */
> +	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL, 0x00);
> +	if (ret)
> +		return ret;
> +
> +	/* wait for device to finish */
> +	usleep_range(4000, 5000);
> +
> +	/* get part_id */
> +	part_id = i2c_smbus_read_word_data(client, ENS210_REG_PART_ID);
> +
> +	if (part_id != data->chip_info->part_id) {
> +		dev_info(&client->dev,
> +			"Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
%04x as per the build bot warning.

> +			data->chip_info->part_id);
> +	}
> +
> +	/* reenable low power */
> +	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
> +					ENS210_SYS_CTRL_LOW_POWER_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->name = data->chip_info->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ens210_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ens210_channels);
> +	indio_dev->info = &ens210_info;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
kernel test robot July 21, 2024, 5:28 a.m. UTC | #4 
Hi Joshua,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1ebab783647a9e3bf357002d5c4ff060c8474a0a]

url:    https://github.com/intel-lab-lkp/linux/commits/Joshua-Felmeden/dt-bindings-iio-humidity-add-ENS210-sensor-family/20240719-210648
base:   1ebab783647a9e3bf357002d5c4ff060c8474a0a
patch link:    https://lore.kernel.org/r/20240719-ens21x-v4-2-6044e48a376a%40thegoodpenguin.co.uk
patch subject: [PATCH v4 2/2] iio: humidity: Add support for ENS210
config: x86_64-randconfig-122-20240721 (https://download.01.org/0day-ci/archive/20240721/202407211229.nP7WkSo5-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240721/202407211229.nP7WkSo5-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/202407211229.nP7WkSo5-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/humidity/ens210.c:88:26: sparse: sparse: restricted __be32 degrades to integer
>> drivers/iio/humidity/ens210.c:88:53: sparse: sparse: incorrect type in initializer (different base types) @@     expected restricted __be32 [usertype] val_be @@     got unsigned int @@
   drivers/iio/humidity/ens210.c:88:53: sparse:     expected restricted __be32 [usertype] val_be
   drivers/iio/humidity/ens210.c:88:53: sparse:     got unsigned int

vim +88 drivers/iio/humidity/ens210.c

    84	
    85	/* calculate 17-bit crc7 */
    86	static u8 ens210_crc7(u32 val)
    87	{
  > 88		__be32 val_be = (cpu_to_be32(val & 0x1ffff) >> 0x8);
    89	
    90		return crc7_be(0xde, (u8 *)&val_be, 3) >> 1;
    91	}
    92
diff mbox series

Patch

diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
index b15b7a3b66d5..54f11f000b6f 100644
--- a/drivers/iio/humidity/Kconfig
+++ b/drivers/iio/humidity/Kconfig
@@ -25,6 +25,17 @@  config DHT11
 	  Other sensors should work as well as long as they speak the
 	  same protocol.
 
+config ENS210
+	tristate "ENS210 temperature and humidity sensor"
+	depends on I2C
+	select CRC7
+	help
+	  Say yes here to get support for the ScioSense ENS210 family of
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ens210.
+
 config HDC100X
 	tristate "TI HDC100x relative humidity and temperature sensor"
 	depends on I2C
diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
index 5fbeef299f61..34b3dc749466 100644
--- a/drivers/iio/humidity/Makefile
+++ b/drivers/iio/humidity/Makefile
@@ -5,6 +5,7 @@ 
 
 obj-$(CONFIG_AM2315) += am2315.o
 obj-$(CONFIG_DHT11) += dht11.o
+obj-$(CONFIG_ENS210) += ens210.o
 obj-$(CONFIG_HDC100X) += hdc100x.o
 obj-$(CONFIG_HDC2010) += hdc2010.o
 obj-$(CONFIG_HDC3020) += hdc3020.o
diff --git a/drivers/iio/humidity/ens210.c b/drivers/iio/humidity/ens210.c
new file mode 100644
index 000000000000..4655e8cb4f51
--- /dev/null
+++ b/drivers/iio/humidity/ens210.c
@@ -0,0 +1,341 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ens210.c - Support for ScioSense ens210 temperature & humidity sensor family
+ *
+ * (7-bit I2C slave address 0x43 ENS210)
+ * (7-bit I2C slave address 0x43 ENS210A)
+ * (7-bit I2C slave address 0x44 ENS211)
+ * (7-bit I2C slave address 0x45 ENS212)
+ * (7-bit I2C slave address 0x46 ENS213A)
+ * (7-bit I2C slave address 0x47 ENS215)
+ *
+ * Datasheet:
+ *  https://www.sciosense.com/wp-content/uploads/2024/04/ENS21x-Datasheet.pdf
+ *  https://www.sciosense.com/wp-content/uploads/2023/12/ENS210-Datasheet.pdf
+ */
+
+#include <linux/crc7.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include <asm/unaligned.h>
+
+/* register definitions */
+#define ENS210_REG_PART_ID		0x00
+#define ENS210_REG_DIE_REV		0x02
+#define ENS210_REG_UID			0x04
+#define ENS210_REG_SYS_CTRL		0x10
+#define ENS210_REG_SYS_STAT		0x11
+#define ENS210_REG_SENS_RUN		0x21
+#define ENS210_REG_SENS_START		0x22
+#define ENS210_REG_SENS_STOP		0x23
+#define ENS210_REG_SENS_STAT		0x24
+#define ENS210_REG_T_VAL		0x30
+#define ENS210_REG_H_VAL		0x33
+
+/* value definitions */
+#define ENS210_SENS_START_T_START		BIT(0)
+#define ENS210_SENS_START_H_START		BIT(1)
+
+#define ENS210_SENS_STAT_T_ACTIVE		BIT(0)
+#define ENS210_SENS_STAT_H_ACTIVE		BIT(1)
+
+#define ENS210_SYS_CTRL_LOW_POWER_ENABLE	BIT(0)
+#define ENS210_SYS_CTRL_SYS_RESET		BIT(7)
+
+#define ENS210_SYS_STAT_SYS_ACTIVE		BIT(0)
+
+enum ens210_partnumber {
+	ENS210	= 0x0210,
+	ENS210A	= 0xa210,
+	ENS211	= 0x0211,
+	ENS212	= 0x0212,
+	ENS213A	= 0xa213,
+	ENS215	= 0x0215,
+};
+
+/**
+ * struct ens210_chip_info - Humidity/Temperature chip specific information
+ * @name:		name of device
+ * @part_id:		chip identifier
+ * @conv_time_msec:	time for conversion calculation in m/s
+ */
+struct ens210_chip_info {
+	const char *name;
+	enum ens210_partnumber part_id;
+	unsigned int conv_time_msec;
+};
+
+/**
+ * struct ens210_data - Humidity/Temperature sensor device structure
+ * @client:	i2c client
+ * @lock:	lock protecting the i2c conversion
+ * @res_index:	index to selected sensor resolution
+ */
+struct ens210_data {
+	struct i2c_client *client;
+	const struct ens210_chip_info *chip_info;
+	struct mutex lock;
+};
+
+/* calculate 17-bit crc7 */
+static u8 ens210_crc7(u32 val)
+{
+	__be32 val_be = (cpu_to_be32(val & 0x1ffff) >> 0x8);
+
+	return crc7_be(0xde, (u8 *)&val_be, 3) >> 1;
+}
+
+static int ens210_get_measurement(struct iio_dev *indio_dev, bool temp, int *val)
+{
+	u32 regval;
+	u8 regval_le[3];
+	int ret;
+	struct ens210_data *data = iio_priv(indio_dev);
+
+	/* assert read */
+	ret = i2c_smbus_write_byte_data(data->client, ENS210_REG_SENS_START,
+				  temp ? ENS210_SENS_START_T_START :
+					 ENS210_SENS_START_H_START);
+	if (ret)
+		return ret;
+
+	/* wait for conversion to be ready */
+	msleep(data->chip_info->conv_time_msec);
+
+	ret = i2c_smbus_read_byte_data(data->client,
+				       ENS210_REG_SENS_STAT);
+	if (ret < 0)
+		return ret;
+
+	/* perform read */
+	ret = i2c_smbus_read_i2c_block_data(
+		data->client, temp ? ENS210_REG_T_VAL : ENS210_REG_H_VAL, 3,
+		(u8 *)&regval_le);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "failed to read register");
+		return -EIO;
+	} else if (ret != 3) {
+		dev_err(&indio_dev->dev, "expected 3 bytes, received %d\n", ret);
+		return -EIO;
+	}
+
+	regval = get_unaligned_le24(regval_le);
+	if (ens210_crc7(regval) != ((regval >> 17) & 0x7f)) {
+		/* crc fail */
+		dev_err(&indio_dev->dev, "ens invalid crc\n");
+		return -EIO;
+	}
+
+	*val = regval & 0xffff;
+	return IIO_VAL_INT;
+}
+
+static int ens210_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *channel, int *val,
+			   int *val2, long mask)
+{
+	struct ens210_data *data = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		scoped_guard(mutex, &data->lock) {
+			ret = ens210_get_measurement(
+				indio_dev, channel->type == IIO_TEMP, val);
+			if (ret)
+				return ret;
+			return IIO_VAL_INT;
+		}
+		return ret;
+	case IIO_CHAN_INFO_SCALE:
+		if (channel->type == IIO_TEMP) {
+			*val = 15;
+			*val2 = 625000;
+		} else {
+			*val = 1;
+			*val2 = 953125;
+		}
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OFFSET:
+		if (channel->type == IIO_TEMP) {
+			*val = -17481;
+			*val2 = 600000;
+			ret = IIO_VAL_INT_PLUS_MICRO;
+			break;
+		}
+		*val = 0;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+	return ret;
+}
+
+static const struct iio_chan_spec ens210_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+	},
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+	}
+};
+
+static const struct iio_info ens210_info = {
+	.read_raw = ens210_read_raw,
+};
+
+static int ens210_probe(struct i2c_client *client)
+{
+	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	struct ens210_data *data;
+	struct iio_dev *indio_dev;
+	uint16_t part_id;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+			I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+			I2C_FUNC_SMBUS_WRITE_BYTE |
+			I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+		dev_err_probe(&client->dev, -EOPNOTSUPP,
+			"adapter does not support some i2c transactions\n");
+		return -EOPNOTSUPP;
+	}
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	mutex_init(&data->lock);
+	data->chip_info = i2c_get_match_data(client);
+
+	ret = devm_regulator_get_enable(&client->dev, "vdd");
+	if (ret)
+		return ret;
+
+	/* reset device */
+	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
+					ENS210_SYS_CTRL_SYS_RESET);
+	if (ret)
+		return ret;
+
+	/* wait for device to become active */
+	usleep_range(4000, 5000);
+
+	/* disable low power mode */
+	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL, 0x00);
+	if (ret)
+		return ret;
+
+	/* wait for device to finish */
+	usleep_range(4000, 5000);
+
+	/* get part_id */
+	part_id = i2c_smbus_read_word_data(client, ENS210_REG_PART_ID);
+
+	if (part_id != data->chip_info->part_id) {
+		dev_info(&client->dev,
+			"Part ID does not match (0x%04x != 0x%04lx)\n", part_id,
+			data->chip_info->part_id);
+	}
+
+	/* reenable low power */
+	ret = i2c_smbus_write_byte_data(client, ENS210_REG_SYS_CTRL,
+					ENS210_SYS_CTRL_LOW_POWER_ENABLE);
+	if (ret)
+		return ret;
+
+	indio_dev->name = data->chip_info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ens210_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ens210_channels);
+	indio_dev->info = &ens210_info;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct ens210_chip_info ens210_chip_info_data = {
+	.name = "ens210",
+	.part_id = ENS210,
+	.conv_time_msec = 130,
+};
+
+static const struct ens210_chip_info ens210a_chip_info_data = {
+	.name = "ens210a",
+	.part_id = ENS210A,
+	.conv_time_msec = 130,
+};
+
+static const struct ens210_chip_info ens211_chip_info_data = {
+	.name = "ens211",
+	.part_id = ENS211,
+	.conv_time_msec = 32,
+};
+
+static const struct ens210_chip_info ens212_chip_info_data = {
+	.name = "ens212",
+	.part_id = ENS212,
+	.conv_time_msec = 32,
+};
+
+static const struct ens210_chip_info ens213a_chip_info_data = {
+	.name = "ens213a",
+	.part_id = ENS213A,
+	.conv_time_msec = 130,
+};
+
+static const struct ens210_chip_info ens215_chip_info_data = {
+	.name = "ens215",
+	.part_id = ENS215,
+	.conv_time_msec = 130,
+};
+
+static const struct of_device_id ens210_of_match[] = {
+	{ .compatible = "sciosense,ens210", .data = &ens210_chip_info_data},
+	{ .compatible = "sciosense,ens210a", .data = &ens210a_chip_info_data },
+	{ .compatible = "sciosense,ens211", .data = &ens211_chip_info_data},
+	{ .compatible = "sciosense,ens212", .data = &ens212_chip_info_data},
+	{ .compatible = "sciosense,ens213a", .data = &ens213a_chip_info_data },
+	{ .compatible = "sciosense,ens215", .data = &ens215_chip_info_data },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ens210_of_match);
+
+static const struct i2c_device_id ens210_id_table[] = {
+	{ "ens210", (kernel_ulong_t)&ens210_chip_info_data },
+	{ "ens210a", (kernel_ulong_t)&ens210a_chip_info_data },
+	{ "ens211", (kernel_ulong_t)&ens211_chip_info_data },
+	{ "ens212", (kernel_ulong_t)&ens212_chip_info_data },
+	{ "ens213a", (kernel_ulong_t)&ens213a_chip_info_data },
+	{ "ens215", (kernel_ulong_t)&ens215_chip_info_data },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ens210_id_table);
+
+static struct i2c_driver ens210_driver = {
+	.probe = ens210_probe,
+	.id_table = ens210_id_table,
+	.driver = {
+		.name = "ens210",
+		.of_match_table = ens210_of_match,
+	},
+};
+
+module_i2c_driver(ens210_driver);
+
+MODULE_DESCRIPTION("ScioSense ENS210 temperature and humidity sensor driver");
+MODULE_AUTHOR("Joshua Felmeden <jfelmeden@thegoodpenguin.co.uk>");
+MODULE_LICENSE("GPL");