Le 10/07/2024 à 15:24, Joshua Felmeden a écrit :
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-tUaQ5FxYRYX4aQPF92CzsNBc4/FLrbF6@xxxxxxxxxxxxxxxx>
---
drivers/iio/humidity/Kconfig | 11 ++
drivers/iio/humidity/Makefile | 1 +
drivers/iio/humidity/ens21x.c | 346 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 358 insertions(+)
Hi,
as kernel test robot complained, there will be a v4.
So here are a few nitpicks/questions, in case it helps.
...
+#include <linux/types.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/crc7.h>
Nitpick: usually, it is prefered to keep #include alphabetically ordered.
...
+
+/* magic constants */
+#define ENS21X_CONST_TEMP_SCALE_INT 15 /* integer part of temperature scale (1/64) */
+#define ENS21X_CONST_TEMP_SCALE_DEC 625000 /* decimal part of temperature scale */
+#define ENS21X_CONST_HUM_SCALE_INT 1 /* integer part of humidity scale (1/512) */
+#define ENS21X_CONST_HUM_SCALE_DEC 953125 /* decimal part of humidity scale */
+#define ENS21X_CONST_TEMP_OFFSET_INT -17481 /* temperature offset (64 * -273.15) */
+#define ENS21X_CONST_TEMP_OFFSET_DEC 600000 /* decimal part of offset */
+#define ENS210_CONST_CONVERSION_TIME 130
+#define ENS212_CONST_CONVERSION_TIME 32
+#define ENS215_CONST_CONVERSION_TIME 132
Datasheet says 130 for ENS213A and ENS215.
Is it a typo?
If 132 is intentional, maybe a samll comment explaining why would be
welcomed?
...
+static int ens21x_get_measurement(struct iio_dev *indio_dev, bool temp, int *val)
+{
+ u32 regval, regval_le;
+ int ret, tries;
+ struct ens21x_dev *dev_data = iio_priv(indio_dev);
+
+ /* assert read */
+ i2c_smbus_write_byte_data(dev_data->client, ENS21X_REG_SENS_START,
+ temp ? ENS21X_SENS_START_T_START :
+ ENS21X_SENS_START_H_START);
+
+ /* wait for conversion to be ready */
+ switch (dev_data->part_id) {
+ case ENS210:
+ case ENS210A:
+ msleep(ENS210_CONST_CONVERSION_TIME);
+ break;
+ case ENS211:
+ case ENS212:
+ msleep(ENS212_CONST_CONVERSION_TIME);
+ break;
+ case ENS213A:
+ case ENS215:
+ msleep(ENS215_CONST_CONVERSION_TIME);
+ break;
+ default:
+ dev_err(&dev_data->client->dev, "unrecognised device");
+ return -ENODEV;
+ }
+
+ tries = 10;
+ while (tries-- > 0) {
+ usleep_range(4000, 5000);
We just msleep()'ed the max expected time for the conversion. So, maybe
the code could be re-arranged so that this delay is done only if we retry?
+ ret = i2c_smbus_read_byte_data(dev_data->client,
+ ENS21X_REG_SENS_STAT);
+ if (ret < 0)
+ continue;
+ if (!(ret & (temp ? ENS21X_SENS_STAT_T_ACTIVE :
+ ENS21X_SENS_STAT_H_ACTIVE)))
+ break;
+ }
+ if (tries < 0) {
+ dev_err(&indio_dev->dev, "timeout waiting for sensor reading\n");
+ return -EIO;
+ }
...
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = ens21x_channels;
+ indio_dev->num_channels = ARRAY_SIZE(ens21x_channels);
+ indio_dev->info = &ens21x_info;
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+
Nitpick: unneeded 2nd new line.
+static const struct of_device_id ens21x_of_match[] = {
+ { .compatible = "sciosense,ens210", .data = (void *)ENS210},
+ { .compatible = "sciosense,ens210a", .data = (void *)ENS210A },
+ { .compatible = "sciosense,ens211", .data = (void *)ENS211},
...
CJ
