Hi everybody,
There is a new contactless sensor in Melexis portfolio. MLX90635 is just
1.8x1.8mm in size, but with factory calibration offers instant usage
in every project. It offers wide refresh rate range that is configurable
between 100ms and 4s.
Driver currently provides temperature calculations, power management and
changes to the refresh rate. Since sensor is aimed towards the consumer
market there is really low number of EEPROM write cycles available, so
driver changes refresh rate only in run time registers to avoid writing
to EEPROM. Reading EEPROM is not available in Sleep Step mode, so I am
using caching at the driver initialization to ensure that measurements
can still be taken in Sleep Step mode.
Crt Mori (2):
iio: temperature: mlx90635 MLX90635 IR Temperature sensor
dt-bindings: iio: temperature: add MLX90635 device bindings
.../iio/temperature/melexis,mlx90635.yaml | 60 +
MAINTAINERS | 7 +
drivers/iio/temperature/Kconfig | 12 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/mlx90635.c | 1099 +++++++++++++++++
5 files changed, 1179 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
create mode 100644 drivers/iio/temperature/mlx90635.c
--
2.40.1
MLX90635 is an Infra Red contactless temperature sensor most suitable
for consumer applications where measured object temperature is in range
between -20 to 100 degrees Celsius. It has improved accuracy for
measurements within temperature range of human body and can operate in
ambient temperature range between -20 to 85 degrees Celsius.
Driver provides simple power management possibility as it returns to
lowest possible power mode (Step sleep mode) in which temperature
measurements can still be performed, yet for continuous measuring it
switches to Continuous power mode where measurements constantly change
without triggering.
Signed-off-by: Crt Mori<[email protected]>
---
MAINTAINERS | 7 +
drivers/iio/temperature/Kconfig | 12 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/mlx90635.c | 1099 ++++++++++++++++++++++++++++
4 files changed, 1119 insertions(+)
create mode 100644 drivers/iio/temperature/mlx90635.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e0a91dc8251..ad77cdd1c08e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13725,6 +13725,13 @@ S: Supported
W: http://www.melexis.com
F: drivers/iio/temperature/mlx90632.c
+MELEXIS MLX90635 DRIVER
+M: Crt Mori <[email protected]>
+L: [email protected]
+S: Supported
+W: http://www.melexis.com
+F: drivers/iio/temperature/mlx90635.c
+
MELFAS MIP4 TOUCHSCREEN DRIVER
M: Sangwon Jee <[email protected]>
S: Supported
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index ea2ce364b2e9..ed0e4963362f 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -76,6 +76,18 @@ config MLX90632
This driver can also be built as a module. If so, the module will
be called mlx90632.
+config MLX90635
+ tristate "MLX90635 contact-less infrared sensor with medical accuracy"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ If you say yes here you get support for the Melexis
+ MLX90635 contact-less infrared sensor with medical accuracy
+ connected with I2C.
+
+ This driver can also be built as a module. If so, the module will
+ be called mlx90635.
+
config TMP006
tristate "TMP006 infrared thermopile sensor"
depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 9330d4a39598..07d6e65709f7 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MAX31865) += max31865.o
obj-$(CONFIG_MCP9600) += mcp9600.o
obj-$(CONFIG_MLX90614) += mlx90614.o
obj-$(CONFIG_MLX90632) += mlx90632.o
+obj-$(CONFIG_MLX90632) += mlx90635.o
obj-$(CONFIG_TMP006) += tmp006.o
obj-$(CONFIG_TMP007) += tmp007.o
obj-$(CONFIG_TMP117) += tmp117.o
diff --git a/drivers/iio/temperature/mlx90635.c b/drivers/iio/temperature/mlx90635.c
new file mode 100644
index 000000000000..498f890b951f
--- /dev/null
+++ b/drivers/iio/temperature/mlx90635.c
@@ -0,0 +1,1099 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mlx90635.c - Melexis MLX90635 contactless IR temperature sensor
+ *
+ * Copyright (c) 2023 Melexis <[email protected]>
+ *
+ * Driver for the Melexis MLX90635 I2C 16-bit IR thermopile sensor
+ */
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/math64.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* Memory sections addresses */
+#define MLX90635_ADDR_RAM 0x0000 /* Start address of ram */
+#define MLX90635_ADDR_EEPROM 0x0018 /* Start address of user eeprom */
+
+/* EEPROM addresses - used at startup */
+#define MLX90635_EE_I2C_CFG 0x0018 /* I2C address register initial value */
+#define MLX90635_EE_CTRL1 0x001A /* Control register1 initial value */
+#define MLX90635_EE_CTRL2 0x001C /* Control register2 initial value */
+
+#define MLX90635_EE_Ha 0x001E /* Ha customer calib value reg 16bit */
+#define MLX90635_EE_Hb 0x0020 /* Hb customer calib value reg 16bit */
+#define MLX90635_EE_Fa 0x0026 /* Fa calibration register 32bit */
+#define MLX90635_EE_FASCALE 0x002A /* Scaling coefficient for Fa register 16bit */
+#define MLX90635_EE_Ga 0x002C /* Ga calibration register 16bit */
+#define MLX90635_EE_Fb 0x002E /* Fb calibration register 16bit */
+#define MLX90635_EE_Ea 0x0030 /* Ea calibration register 32bit */
+#define MLX90635_EE_Eb 0x0034 /* Eb calibration register 32bit */
+#define MLX90635_EE_P_G 0x0038 /* P_G calibration register 16bit */
+#define MLX90635_EE_P_O 0x003A /* P_O calibration register 16bit */
+#define MLX90635_EE_Aa 0x003C /* Aa calibration register 16bit */
+#define MLX90635_EE_VERSION 0x003E /* Version bits 4:7 and 12:15 */
+#define MLX90635_EE_Gb 0x0040 /* Gb calibration register 16bit */
+
+/* Device status register - volatile */
+#define MLX90635_REG_STATUS 0x0000 /* Device status register */
+#define MLX90635_STAT_BUSY BIT(6) /* Device busy indicator */
+#define MLX90635_STAT_BRST BIT(5) /* Brown out reset indicator */
+#define MLX90635_STAT_CYCLE_POS GENMASK(4, 2) /* Data position */
+#define MLX90635_STAT_END_CONV BIT(1) /* End of conversion indicator */
+#define MLX90635_STAT_DATA_RDY BIT(0) /* Data ready indicator */
+
+/* EEPROM control register address - volatile */
+#define MLX90635_REG_EE 0x000C /* EEPROM status and control register */
+#define MLX90635_EE_BUSY_SHIFT 15
+#define MLX90635_EE_ACTIVE BIT(4) /* Power-on EEPROM */
+#define MLX90635_EE_BUSY_MASK BIT(MLX90635_EE_BUSY_SHIFT)
+
+#define MLX90635_REG_CMD 0x0010 /* Command register address */
+
+/* Control register1 address - volatile */
+#define MLX90635_REG_CTRL1 0x0014 /* Control Register1 address */
+#define MLX90635_CTRL1_REFRESH_RATE_START 2
+#define MLX90635_CTRL1_REFRESH_RATE_SHIFT 0
+#define MLX90635_CTRL1_REFRESH_RATE_MASK GENMASK(MLX90635_CTRL1_REFRESH_RATE_START, MLX90635_CTRL1_REFRESH_RATE_SHIFT)
+#define MLX90635_CTRL1_RES_CTRL_START 4
+#define MLX90635_CTRL1_RES_CTRL_SHIFT 3
+#define MLX90635_CTRL1_RES_CTRL_MASK GENMASK(MLX90635_CTRL1_RES_CTRL_START, MLX90635_CTRL1_RES_CTRL_SHIFT)
+#define MLX90635_CTRL1_TABLE_SHIFT 15 /* Table select */
+#define MLX90635_CTRL1_TABLE_MASK BIT(MLX90635_CTRL1_TABLE_SHIFT)
+#define MLX90635_CTRL1_TABLE(ctrl_val) (ctrl_val << MLX90635_CTRL1_TABLE_SHIFT)
+
+/* Control register2 address - volatile */
+#define MLX90635_REG_CTRL2 0x0016 /* Control Register2 address */
+#define MLX90635_CTRL2_BURST_CNT_SHIFT 6 /* Burst count */
+#define MLX90635_CTRL2_BURST_CNT_MASK GENMASK(MLX90635_CTRL2_BURST_CNT_SHIFT + 4, MLX90635_CTRL2_BURST_CNT_SHIFT)
+#define MLX90635_CTRL2_BURST(ctrl_val) (ctrl_val << MLX90635_CTRL2_BURST_CNT_SHIFT)
+#define MLX90635_CTRL2_MODE_SHIFT 11 /* Power mode */
+#define MLX90635_CTRL2_MODE_MASK GENMASK(MLX90635_CTRL2_MODE_SHIFT + 1, MLX90635_CTRL2_MODE_SHIFT)
+#define MLX90635_CTRL2_MODE(ctrl_val) (ctrl_val << MLX90635_CTRL2_MODE_SHIFT)
+#define MLX90635_CTRL2_SOB_SHIFT 15 /* Start burst measurement in step mode */
+#define MLX90635_CTRL2_SOB_MASK BIT(MLX90635_CTRL2_SOB_SHIFT)
+#define MLX90635_CTRL2_SOB(ctrl_val) (ctrl_val << MLX90635_CTRL2_SOB_SHIFT)
+
+/* PowerModes statuses */
+#define MLX90635_PWR_STATUS_HALT 0 /* Pwrmode hold */
+#define MLX90635_PWR_STATUS_SLEEP_STEP 1 /* Pwrmode sleep step*/
+#define MLX90635_PWR_STATUS_STEP 2 /* Pwrmode step */
+#define MLX90635_PWR_STATUS_CONTINUOUS 3 /* Pwrmode continuous*/
+
+/* Measurement data addresses */
+#define MLX90635_RESULT_1 0x0002
+#define MLX90635_RESULT_2 0x0004
+#define MLX90635_RESULT_3 0x0006
+#define MLX90635_RESULT_4 0x0008
+#define MLX90635_RESULT_5 0x000A
+
+/* Timings (ms) */
+#define MLX90635_TIMING_RST_MIN 200 /* Minimum time after addressed reset command */
+#define MLX90635_TIMING_RST_MAX 250 /* Maximum time after addressed reset command */
+#define MLX90635_TIMING_POLLING 10000 /* Time between bit polling*/
+#define MLX90635_TIMING_EE_ACTIVE_MIN 100 /* Minimum time after activating the EEPROM for read */
+#define MLX90635_TIMING_EE_ACTIVE_MAX 150 /* Maximum time after activating the EEPROM for read */
+
+/* Magic constants */
+#define MLX90635_ID_DSPv1 0x01 /* EEPROM DSP version */
+#define MLX90635_RESET_CMD 0x0006 /**< Reset sensor (address or global) */
+#define MLX90635_MAX_MEAS_NUM 31 /**< Maximum number of measurements in list */
+#define MLX90635_PTAT_DIV 12 /**< Used to divide the PTAT value in pre-processing */
+#define MLX90635_IR_DIV 24 /**< Used to divide the IR value in pre-processing */
+#define MLX90635_SLEEP_DELAY_MS 6000 /* Autosleep delay */
+#define MLX90635_MEAS_MAX_TIME 2000 /* Max measurement time in ms for the lowest refresh rate */
+#define MLX90635_READ_RETRIES 100 /* Number of read retries before quitting with timeout error */
+#define MLX90635_VERSION_MASK (GENMASK(15, 12) | GENMASK(7, 4))
+#define MLX90635_DSP_VERSION(reg) ((reg & GENMASK(6, 4)) >> 3)
+#define MLX90635_DSP_FIXED (BIT(15))
+
+
+/**
+ * struct mlx90635_data - private data for the MLX90635 device
+ * @client: I2C client of the device
+ * @lock: Internal mutex for multiple reads for single measurement
+ * @regmap: Regmap of the device
+ * @emissivity: Object emissivity from 0 to 1000 where 1000 = 1.
+ * @regulator: Regulator of the device
+ * @powerstatus: Current POWER status of the device
+ * @interaction_ts: Timestamp of the last temperature read that is used
+ * for power management in jiffies
+ */
+struct mlx90635_data {
+ struct i2c_client *client;
+ struct mutex lock;
+ struct regmap *regmap;
+ u16 emissivity;
+ struct regulator *regulator;
+ int powerstatus;
+ unsigned long interaction_ts;
+};
+
+static const struct regmap_range mlx90635_volatile_reg_range[] = {
+ regmap_reg_range(MLX90635_REG_STATUS, MLX90635_REG_STATUS),
+ regmap_reg_range(MLX90635_RESULT_1, MLX90635_RESULT_5),
+ regmap_reg_range(MLX90635_REG_EE, MLX90635_REG_EE),
+ regmap_reg_range(MLX90635_REG_CMD, MLX90635_REG_CMD),
+ regmap_reg_range(MLX90635_REG_CTRL1, MLX90635_REG_CTRL2),
+};
+
+static const struct regmap_access_table mlx90635_volatile_regs_tbl = {
+ .yes_ranges = mlx90635_volatile_reg_range,
+ .n_yes_ranges = ARRAY_SIZE(mlx90635_volatile_reg_range),
+};
+
+static const struct regmap_range mlx90635_read_reg_range[] = {
+ regmap_reg_range(MLX90635_REG_STATUS, MLX90635_REG_STATUS),
+ regmap_reg_range(MLX90635_RESULT_1, MLX90635_RESULT_5),
+ regmap_reg_range(MLX90635_REG_EE, MLX90635_REG_EE),
+ regmap_reg_range(MLX90635_REG_CMD, MLX90635_REG_CMD),
+ regmap_reg_range(MLX90635_REG_CTRL1, MLX90635_REG_CTRL2),
+ regmap_reg_range(MLX90635_EE_I2C_CFG, MLX90635_EE_CTRL2),
+ regmap_reg_range(MLX90635_EE_Ha, MLX90635_EE_Gb),
+};
+
+static const struct regmap_access_table mlx90635_readable_regs_tbl = {
+ .yes_ranges = mlx90635_read_reg_range,
+ .n_yes_ranges = ARRAY_SIZE(mlx90635_read_reg_range),
+};
+
+static const struct regmap_range mlx90635_no_write_reg_range[] = {
+ regmap_reg_range(MLX90635_ADDR_EEPROM, MLX90635_EE_Gb),
+ regmap_reg_range(MLX90635_RESULT_1, MLX90635_RESULT_5),
+};
+
+static const struct regmap_access_table mlx90635_writeable_regs_tbl = {
+ .no_ranges = mlx90635_no_write_reg_range,
+ .n_no_ranges = ARRAY_SIZE(mlx90635_no_write_reg_range),
+};
+
+static const struct regmap_config mlx90635_regmap = {
+ .reg_stride = 1,
+ .reg_bits = 16,
+ .val_bits = 16,
+
+ .volatile_table = &mlx90635_volatile_regs_tbl,
+ .rd_table = &mlx90635_readable_regs_tbl,
+ .wr_table = &mlx90635_writeable_regs_tbl,
+
+ .use_single_read = true,
+ .use_single_write = true,
+ .can_multi_write = false,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .val_format_endian = REGMAP_ENDIAN_BIG,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+/**
+ * mlx90635_reset_delay() - Give the mlx90635 some time to reset properly
+ * If this is not done, the following I2C command(s) will not be accepted.
+ */
+static void mlx90635_reset_delay(void)
+{
+ usleep_range(MLX90635_TIMING_RST_MIN, MLX90635_TIMING_RST_MAX);
+}
+
+static int mlx90635_pwr_sleep_step(struct mlx90635_data *data)
+{
+ int ret;
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ return 0;
+
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_CTRL2, MLX90635_CTRL2_MODE_MASK,
+ MLX90635_CTRL2_MODE(MLX90635_PWR_STATUS_SLEEP_STEP));
+ if (ret < 0)
+ return ret;
+
+ regcache_cache_only(data->regmap, true);
+
+ data->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
+ return 0;
+}
+
+static int mlx90635_pwr_continuous(struct mlx90635_data *data)
+{
+ int ret;
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_CONTINUOUS)
+ return 0;
+
+ regcache_cache_only(data->regmap, false);
+
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_CTRL2, MLX90635_CTRL2_MODE_MASK,
+ MLX90635_CTRL2_MODE(MLX90635_PWR_STATUS_CONTINUOUS));
+ if (ret < 0)
+ return ret;
+
+ data->powerstatus = MLX90635_PWR_STATUS_CONTINUOUS;
+ return 0;
+}
+
+static int mlx90635_read_ee_register(struct regmap *regmap, u16 reg_lsb,
+ s32 *reg_value)
+{
+ unsigned int read;
+ u32 value;
+ int ret;
+
+ ret = regmap_read(regmap, reg_lsb + 2, &read);
+ if (ret < 0)
+ return ret;
+
+ value = read;
+
+ ret = regmap_read(regmap, reg_lsb, &read);
+ if (ret < 0)
+ return ret;
+
+ *reg_value = (read << 16) | (value & 0xffff);
+
+ return 0;
+}
+
+static int mlx90635_read_ee_ambient(struct regmap *regmap, s16 *PG, s16 *PO, s16 *Gb)
+{
+ unsigned int read_tmp;
+ int ret;
+
+ ret = regmap_read(regmap, MLX90635_EE_P_O, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *PO = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_P_G, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *PG = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_Gb, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Gb = (u16)read_tmp;
+
+ return ret;
+}
+
+static int mlx90635_read_ee_object(struct regmap *regmap, u32 *Ea, u32 *Eb, u32 *Fa, s16 *Fb,
+ s16 *Ga, s16 *Gb, s16 *Ha, s16 *Hb, u16 *Fa_scale)
+{
+ unsigned int read_tmp;
+ int ret;
+
+ ret = mlx90635_read_ee_register(regmap, MLX90635_EE_Ea, Ea);
+ if (ret < 0)
+ return ret;
+
+ ret = mlx90635_read_ee_register(regmap, MLX90635_EE_Eb, Eb);
+ if (ret < 0)
+ return ret;
+
+ ret = mlx90635_read_ee_register(regmap, MLX90635_EE_Fa, Fa);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(regmap, MLX90635_EE_Ha, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Ha = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_Hb, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Hb = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_Ga, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Ga = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_Gb, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Gb = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_Fb, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Fb = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_EE_FASCALE, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *Fa_scale = (u16)read_tmp;
+
+ return ret;
+}
+
+static int mlx90635_calculate_dataset_ready_time(struct mlx90635_data *data, int *refresh_time)
+{
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(data->regmap, MLX90635_REG_CTRL1, ®);
+ if (ret < 0)
+ return ret;
+
+ *refresh_time = 2 * (MLX90635_MEAS_MAX_TIME >> FIELD_GET(MLX90635_CTRL1_REFRESH_RATE_MASK, reg)) + 80;
+
+ return 0;
+}
+
+static int mlx90635_perform_measurement_burst(struct mlx90635_data *data)
+{
+ unsigned int reg_status;
+ int refresh_time;
+ int ret;
+
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_STATUS,
+ MLX90635_STAT_END_CONV, MLX90635_STAT_END_CONV);
+ if (ret < 0)
+ return ret;
+
+ ret = mlx90635_calculate_dataset_ready_time(data, &refresh_time);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_CTRL2,
+ MLX90635_CTRL2_SOB(1), MLX90635_CTRL2_SOB(1));
+ if (ret < 0)
+ return ret;
+
+ msleep(refresh_time); /* Wait minimum time for dataset to be ready */
+
+ ret = regmap_read_poll_timeout(data->regmap, MLX90635_REG_STATUS, reg_status,
+ !(reg_status & MLX90635_STAT_END_CONV) == 0,
+ MLX90635_TIMING_POLLING, MLX90635_READ_RETRIES * 10000);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "data not ready");
+ return -ETIMEDOUT;
+ }
+
+ return ret;
+}
+
+static int mlx90635_read_ambient_raw(struct regmap *regmap,
+ s16 *ambient_new_raw, s16 *ambient_old_raw)
+{
+ unsigned int read_tmp;
+ int ret;
+
+ ret = regmap_read(regmap, MLX90635_RESULT_2, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *ambient_new_raw = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_RESULT_3, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *ambient_old_raw = (s16)read_tmp;
+
+ return ret;
+}
+
+static int mlx90635_read_object_raw(struct regmap *regmap, s16 *object_raw)
+{
+ unsigned int read_tmp;
+ s16 read;
+ int ret;
+
+ ret = regmap_read(regmap, MLX90635_RESULT_1, &read_tmp);
+ if (ret < 0)
+ return ret;
+
+ read = (s16)read_tmp;
+
+ ret = regmap_read(regmap, MLX90635_RESULT_4, &read_tmp);
+ if (ret < 0)
+ return ret;
+ *object_raw = (read - (s16)read_tmp) / 2;
+
+ return ret;
+}
+
+static int mlx90635_read_all_channel(struct mlx90635_data *data,
+ s16 *ambient_new_raw, s16 *ambient_old_raw,
+ s16 *object_raw)
+{
+ int ret;
+
+ mutex_lock(&data->lock);
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP) {
+ regcache_cache_only(data->regmap, false);
+ ret = mlx90635_perform_measurement_burst(data);
+ if (ret < 0)
+ goto read_unlock;
+ }
+
+ ret = mlx90635_read_ambient_raw(data->regmap, ambient_new_raw,
+ ambient_old_raw);
+ if (ret < 0)
+ goto read_unlock;
+
+ ret = mlx90635_read_object_raw(data->regmap, object_raw);
+read_unlock:
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, true);
+
+ mutex_unlock(&data->lock);
+ return ret;
+}
+
+static s64 mlx90635_preprocess_temp_amb(s16 ambient_new_raw,
+ s16 ambient_old_raw, s16 Gb)
+{
+ s64 VR_Ta, kGb, tmp;
+
+ kGb = ((s64)Gb * 1000LL) >> 10ULL;
+ VR_Ta = (s64)ambient_old_raw * 1000000LL +
+ kGb * div64_s64(((s64)ambient_new_raw * 1000LL),
+ (MLX90635_PTAT_DIV));
+ tmp = div64_s64(
+ div64_s64(((s64)ambient_new_raw * 1000000000000LL),
+ (MLX90635_PTAT_DIV)), VR_Ta);
+ return div64_s64(tmp << 19ULL, 1000LL);
+}
+
+static s64 mlx90635_preprocess_temp_obj(s16 object_raw,
+ s16 ambient_new_raw,
+ s16 ambient_old_raw, s16 Gb)
+{
+ s64 VR_IR, kGb, tmp;
+
+ kGb = ((s64)Gb * 1000LL) >> 10ULL;
+ VR_IR = (s64)ambient_old_raw * 1000000LL +
+ kGb * (div64_s64((s64)ambient_new_raw * 1000LL,
+ MLX90635_PTAT_DIV));
+ tmp = div64_s64(
+ div64_s64((s64)(object_raw * 1000000LL),
+ MLX90635_IR_DIV) * 1000000LL,
+ VR_IR);
+ return div64_s64((tmp << 19ULL), 1000LL);
+}
+
+static s32 mlx90635_calc_temp_ambient(s16 ambient_new_raw, s16 ambient_old_raw,
+ u16 P_G, u16 P_O, s16 Gb)
+{
+ s64 kPG, kPO, AMB;
+
+ AMB = mlx90635_preprocess_temp_amb(ambient_new_raw, ambient_old_raw,
+ Gb);
+ kPG = ((s64)P_G * 1000000LL) >> 9ULL;
+ kPO = AMB - (((s64)P_O * 1000LL) >> 1ULL);
+
+ return 30 * 1000LL + div64_s64(kPO * 1000000LL, kPG);
+}
+
+static s32 mlx90635_calc_temp_object_iteration(s32 prev_object_temp, s64 object,
+ s64 TAdut, s64 TAdut4, s16 Ga,
+ u32 Fa, u16 Fa_scale, s16 Fb,
+ s16 Ha, s16 Hb, u16 emissivity)
+{
+ s64 calcedGa, calcedGb, calcedFa, Alpha_corr;
+ s64 Ha_customer, Hb_customer;
+
+ Ha_customer = ((s64)Ha * 1000000LL) >> 14ULL;
+ Hb_customer = ((s64)Hb * 100) >> 10ULL;
+
+ calcedGa = ((s64)((s64)Ga * (prev_object_temp - 35 * 1000LL)
+ * 1000LL)) >> 24LL;
+ calcedGb = ((s64)(Fb * (TAdut - 30 * 1000000LL))) >> 24LL;
+
+ Alpha_corr = ((s64)((s64)Fa * Ha_customer * 10000LL) >> Fa_scale);
+ Alpha_corr *= ((s64)(1 * 1000000LL + calcedGa + calcedGb));
+
+ Alpha_corr = div64_s64(Alpha_corr, 1000LL);
+ Alpha_corr *= emissivity;
+ Alpha_corr = div64_s64(Alpha_corr, 100LL);
+ calcedFa = div64_s64((s64)object * 100000000000LL, Alpha_corr);
+
+ return (int_sqrt64(int_sqrt64(calcedFa * 100000000LL + TAdut4))
+ - 27315 - Hb_customer) * 10;
+}
+
+static s64 mlx90635_calc_ta4(s64 TAdut, s64 scale)
+{
+ return (div64_s64(TAdut, scale) + 27315) *
+ (div64_s64(TAdut, scale) + 27315) *
+ (div64_s64(TAdut, scale) + 27315) *
+ (div64_s64(TAdut, scale) + 27315);
+}
+
+static s32 mlx90635_calc_temp_object(s64 object, s64 ambient, u32 Ea, u32 Eb,
+ s16 Ga, u32 Fa, u16 Fa_scale, s16 Fb, s16 Ha, s16 Hb,
+ u16 tmp_emi)
+{
+ s64 kTA, kTA0, TAdut, TAdut4;
+ s64 temp = 35000;
+ s8 i;
+
+ kTA = (Ea * 1000LL) >> 16LL;
+ kTA0 = (Eb * 1000LL) >> 8LL;
+ TAdut = div64_s64(((ambient - kTA0) * 1000000LL), kTA) + 30 * 1000000LL;
+ TAdut4 = mlx90635_calc_ta4(TAdut, 10000LL);
+
+ /* Iterations of calculation as described in datasheet */
+ for (i = 0; i < 5; ++i) {
+ temp = mlx90635_calc_temp_object_iteration(temp, object, TAdut, TAdut4,
+ Ga, Fa, Fa_scale, Fb, Ha, Hb,
+ tmp_emi);
+ }
+ return temp;
+}
+
+static int mlx90635_calc_object(struct mlx90635_data *data, int *val)
+{
+ s16 ambient_new_raw, ambient_old_raw, object_raw;
+ s16 Fb, Ga, Gb, Ha, Hb;
+ s64 object, ambient;
+ u32 Ea, Eb, Fa;
+ u16 Fa_scale;
+ int ret;
+
+ ret = mlx90635_read_ee_object(data->regmap, &Ea, &Eb, &Fa, &Fb, &Ga, &Gb, &Ha, &Hb, &Fa_scale);
+ if (ret < 0)
+ return ret;
+
+ ret = mlx90635_read_all_channel(data,
+ &ambient_new_raw, &ambient_old_raw,
+ &object_raw);
+ if (ret < 0)
+ return ret;
+
+ ambient = mlx90635_preprocess_temp_amb(ambient_new_raw,
+ ambient_old_raw, Gb);
+ object = mlx90635_preprocess_temp_obj(object_raw,
+ ambient_new_raw,
+ ambient_old_raw, Gb);
+
+ *val = mlx90635_calc_temp_object(object, ambient, Ea, Eb, Ga, Fa, Fa_scale, Fb,
+ Ha, Hb, data->emissivity);
+ return 0;
+}
+
+static int mlx90635_calc_ambient(struct mlx90635_data *data, int *val)
+{
+ s16 ambient_new_raw, ambient_old_raw;
+ s16 PG, PO, Gb;
+ int ret;
+
+ ret = mlx90635_read_ee_ambient(data->regmap, &PG, &PO, &Gb);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&data->lock);
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP) {
+ regcache_cache_only(data->regmap, false);
+
+ ret = mlx90635_perform_measurement_burst(data);
+ if (ret < 0)
+ goto read_ambient_unlock;
+ }
+
+ ret = mlx90635_read_ambient_raw(data->regmap, &ambient_new_raw,
+ &ambient_old_raw);
+read_ambient_unlock:
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, true);
+
+ mutex_unlock(&data->lock);
+ if (ret < 0)
+ return ret;
+
+ *val = mlx90635_calc_temp_ambient(ambient_new_raw, ambient_old_raw,
+ PG, PO, Gb);
+ return ret;
+}
+
+static int mlx90635_get_refresh_rate(struct mlx90635_data *data,
+ unsigned int *refresh_rate)
+{
+ unsigned int reg;
+ int ret;
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, false);
+
+ ret = regmap_read(data->regmap, MLX90635_REG_CTRL1, ®);
+ if (ret < 0)
+ return ret;
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, true);
+
+ *refresh_rate = FIELD_GET(MLX90635_CTRL1_REFRESH_RATE_MASK, reg);
+
+ return 0;
+}
+
+static const struct {
+ int val;
+ int val2;
+} mlx90635_freqs[] = {
+ {0, 200000},
+ {0, 500000},
+ {0, 900000},
+ {1, 700000},
+ {3, 0},
+ {4, 800000},
+ {6, 900000},
+ {8, 900000}
+};
+
+/**
+ * mlx90635_pm_interaction_wakeup() - Measure time between user interactions to change powermode
+ * @data: pointer to mlx90635_data object containing interaction_ts information
+ *
+ * Switch to continuous mode when interaction is faster than MLX90635_MEAS_MAX_TIME. Update the
+ * interaction_ts for each function call with the jiffies to enable measurement between function
+ * calls. Initial value of the interaction_ts needs to be set before this function call.
+ */
+static int mlx90635_pm_interaction_wakeup(struct mlx90635_data *data)
+{
+ unsigned long now;
+ int ret;
+
+ now = jiffies;
+ if (time_in_range(now, data->interaction_ts,
+ data->interaction_ts +
+ msecs_to_jiffies(MLX90635_MEAS_MAX_TIME + 100))) {
+ ret = mlx90635_pwr_continuous(data);
+ if (ret < 0)
+ return ret;
+ }
+
+ data->interaction_ts = now;
+
+ return 0;
+}
+
+static int mlx90635_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct mlx90635_data *data = iio_priv(indio_dev);
+ int ret;
+ int cr;
+
+ pm_runtime_get_sync(&data->client->dev);
+ ret = mlx90635_pm_interaction_wakeup(data);
+ if (ret < 0)
+ goto mlx90635_read_raw_pm;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_PROCESSED:
+ switch (channel->channel2) {
+ case IIO_MOD_TEMP_AMBIENT:
+ ret = mlx90635_calc_ambient(data, val);
+ if (ret < 0)
+ goto mlx90635_read_raw_pm;
+
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_MOD_TEMP_OBJECT:
+ ret = mlx90635_calc_object(data, val);
+ if (ret < 0)
+ goto mlx90635_read_raw_pm;
+
+ ret = IIO_VAL_INT;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ break;
+ case IIO_CHAN_INFO_CALIBEMISSIVITY:
+ if (data->emissivity == 1000) {
+ *val = 1;
+ *val2 = 0;
+ } else {
+ *val = 0;
+ *val2 = data->emissivity * 1000;
+ }
+ ret = IIO_VAL_INT_PLUS_MICRO;
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = mlx90635_get_refresh_rate(data, &cr);
+ if (ret < 0)
+ goto mlx90635_read_raw_pm;
+
+ *val = mlx90635_freqs[cr].val;
+ *val2 = mlx90635_freqs[cr].val2;
+ ret = IIO_VAL_INT_PLUS_MICRO;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+mlx90635_read_raw_pm:
+ pm_runtime_mark_last_busy(&data->client->dev);
+ pm_runtime_put_autosuspend(&data->client->dev);
+ return ret;
+}
+
+static int mlx90635_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int val,
+ int val2, long mask)
+{
+ struct mlx90635_data *data = iio_priv(indio_dev);
+ int ret;
+ int i;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBEMISSIVITY:
+ /* Confirm we are within 0 and 1.0 */
+ if (val < 0 || val2 < 0 || val > 1 ||
+ (val == 1 && val2 != 0))
+ return -EINVAL;
+ data->emissivity = val * 1000 + val2 / 1000;
+ return 0;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ for (i = 0; i < ARRAY_SIZE(mlx90635_freqs); i++) {
+ if (val == mlx90635_freqs[i].val &&
+ val2 == mlx90635_freqs[i].val2)
+ break;
+ }
+ if (i == ARRAY_SIZE(mlx90635_freqs))
+ return -EINVAL;
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, false);
+
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_CTRL1,
+ MLX90635_CTRL1_REFRESH_RATE_MASK, i);
+
+ if (data->powerstatus == MLX90635_PWR_STATUS_SLEEP_STEP)
+ regcache_cache_only(data->regmap, true);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mlx90635_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_SAMP_FREQ:
+ *vals = (int *)mlx90635_freqs;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ *length = 2 * ARRAY_SIZE(mlx90635_freqs);
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_chan_spec mlx90635_channels[] = {
+ {
+ .type = IIO_TEMP,
+ .modified = 1,
+ .channel2 = IIO_MOD_TEMP_AMBIENT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ },
+ {
+ .type = IIO_TEMP,
+ .modified = 1,
+ .channel2 = IIO_MOD_TEMP_OBJECT,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_CALIBEMISSIVITY),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ },
+};
+
+static const struct iio_info mlx90635_info = {
+ .read_raw = mlx90635_read_raw,
+ .write_raw = mlx90635_write_raw,
+ .read_avail = mlx90635_read_avail,
+};
+
+static void mlx90635_sleep(void *_data)
+{
+ struct mlx90635_data *data = _data;
+
+ mlx90635_pwr_sleep_step(data);
+}
+
+static int mlx90635_suspend(struct mlx90635_data *data)
+{
+ return mlx90635_pwr_sleep_step(data);
+}
+
+static int mlx90635_wakeup(struct mlx90635_data *data)
+{
+ s16 Fb, Ga, Gb, Ha, Hb, PG, PO;
+ u32 Ea, Eb, Fa;
+ u16 Fa_scale;
+ int ret;
+
+ regcache_cache_bypass(data->regmap, false);
+ ret = mlx90635_pwr_continuous(data);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Switch to continuous mode failed\n");
+ return ret;
+ }
+ ret = regmap_write_bits(data->regmap, MLX90635_REG_EE, MLX90635_EE_ACTIVE, MLX90635_EE_ACTIVE);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Powering EEPROM failed\n");
+ return ret;
+ }
+ usleep_range(MLX90635_TIMING_EE_ACTIVE_MIN, MLX90635_TIMING_EE_ACTIVE_MAX);
+
+ regcache_mark_dirty(data->regmap);
+
+ ret = regcache_sync(data->regmap);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to cache everything: %d\n", ret);
+ return ret;
+ }
+
+ ret = regcache_sync_region(data->regmap, MLX90635_EE_Ha, MLX90635_EE_Gb);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to sync EEEPROM region: %d\n", ret);
+ return ret;
+ }
+
+ ret = mlx90635_read_ee_ambient(data->regmap, &PG, &PO, &Gb);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to read to cache Ambient coefficients EEPROM region: %d\n", ret);
+ return ret;
+ }
+
+ ret = mlx90635_read_ee_object(data->regmap, &Ea, &Eb, &Fa, &Fb, &Ga, &Gb, &Ha, &Hb, &Fa_scale);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to read to cache Object coefficients EEPROM region: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_async_complete(data->regmap);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to complete sync: %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
+static void mlx90635_disable_regulator(void *_data)
+{
+ struct mlx90635_data *data = _data;
+ int ret;
+
+ ret = regulator_disable(data->regulator);
+ if (ret < 0)
+ dev_err(regmap_get_device(data->regmap),
+ "Failed to disable power regulator: %d\n", ret);
+}
+
+static int mlx90635_enable_regulator(struct mlx90635_data *data)
+{
+ int ret;
+
+ ret = regulator_enable(data->regulator);
+ if (ret < 0) {
+ dev_err(regmap_get_device(data->regmap), "Failed to enable power regulator!\n");
+ return ret;
+ }
+
+ mlx90635_reset_delay();
+
+ return ret;
+}
+
+static int mlx90635_probe(struct i2c_client *client)
+{
+ const struct i2c_device_id *id = i2c_client_get_device_id(client);
+ struct mlx90635_data *mlx90635;
+ struct iio_dev *indio_dev;
+ unsigned int dsp_version;
+ struct regmap *regmap;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mlx90635));
+ if (!indio_dev) {
+ dev_err(&client->dev, "Failed to allocate device\n");
+ return -ENOMEM;
+ }
+
+ regmap = devm_regmap_init_i2c(client, &mlx90635_regmap);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ dev_err(&client->dev, "Failed to allocate regmap: %d\n", ret);
+ return ret;
+ }
+
+ mlx90635 = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ mlx90635->client = client;
+ mlx90635->regmap = regmap;
+ mlx90635->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
+
+ mutex_init(&mlx90635->lock);
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &mlx90635_info;
+ indio_dev->channels = mlx90635_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
+
+ mlx90635->regulator = devm_regulator_get(&client->dev, "vdd");
+ if (IS_ERR(mlx90635->regulator))
+ return dev_err_probe(&client->dev, PTR_ERR(mlx90635->regulator),
+ "failed to get vdd regulator");
+
+ ret = mlx90635_enable_regulator(mlx90635);
+ if (ret < 0)
+ return ret;
+
+ ret = devm_add_action_or_reset(&client->dev, mlx90635_disable_regulator,
+ mlx90635);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to setup regulator cleanup action %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = mlx90635_wakeup(mlx90635);
+ if (ret < 0) {
+ dev_err(&client->dev, "Wakeup failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(&client->dev, mlx90635_sleep, mlx90635);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to setup low power cleanup action %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = regmap_read(mlx90635->regmap, MLX90635_EE_VERSION, &dsp_version);
+ if (ret < 0) {
+ dev_err(&client->dev, "read of version failed: %d\n", ret);
+ return ret;
+ }
+ dsp_version = dsp_version & MLX90635_VERSION_MASK;
+ if (MLX90635_DSP_VERSION(dsp_version) == MLX90635_ID_DSPv1) {
+ dev_dbg(&client->dev,
+ "Detected DSP v1 calibration %x\n", dsp_version);
+ } else if ((dsp_version & MLX90635_DSP_FIXED) == MLX90635_DSP_FIXED) {
+ dev_dbg(&client->dev,
+ "Detected Unknown EEPROM calibration %lx\n", MLX90635_DSP_VERSION(dsp_version));
+ } else {
+ dev_err(&client->dev,
+ "Wrong fixed top bit %lx (expected 0x8X0X)\n",
+ dsp_version & MLX90635_DSP_FIXED);
+ return -EPROTONOSUPPORT;
+ }
+
+ mlx90635->emissivity = 1000;
+ mlx90635->interaction_ts = jiffies; /* Set initial value */
+
+ pm_runtime_get_noresume(&client->dev);
+ pm_runtime_set_active(&client->dev);
+
+ ret = devm_pm_runtime_enable(&client->dev);
+ if (ret) {
+ dev_err(&client->dev, "Failed to enable powermanagement %d\n",
+ ret);
+ return ret;
+ }
+
+ pm_runtime_set_autosuspend_delay(&client->dev, MLX90635_SLEEP_DELAY_MS);
+ pm_runtime_use_autosuspend(&client->dev);
+ pm_runtime_put_autosuspend(&client->dev);
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id mlx90635_id[] = {
+ { "mlx90635", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mlx90635_id);
+
+static const struct of_device_id mlx90635_of_match[] = {
+ { .compatible = "melexis,mlx90635" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, mlx90635_of_match);
+
+static int mlx90635_pm_suspend(struct device *dev)
+{
+ struct mlx90635_data *data = iio_priv(dev_get_drvdata(dev));
+ int ret;
+
+ ret = mlx90635_suspend(data);
+ if (ret < 0)
+ return ret;
+
+ ret = regulator_disable(data->regulator);
+ if (ret < 0)
+ dev_err(regmap_get_device(data->regmap),
+ "Failed to disable power regulator: %d\n", ret);
+
+ return ret;
+}
+
+static int mlx90635_pm_resume(struct device *dev)
+{
+ struct mlx90635_data *data = iio_priv(dev_get_drvdata(dev));
+ int ret;
+
+ ret = mlx90635_enable_regulator(data);
+ if (ret < 0)
+ return ret;
+
+ return mlx90635_wakeup(data);
+}
+
+static int mlx90635_pm_runtime_suspend(struct device *dev)
+{
+ struct mlx90635_data *data = iio_priv(dev_get_drvdata(dev));
+
+ return mlx90635_pwr_sleep_step(data);
+}
+
+static const struct dev_pm_ops mlx90635_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(mlx90635_pm_suspend, mlx90635_pm_resume)
+ RUNTIME_PM_OPS(mlx90635_pm_runtime_suspend, NULL, NULL)
+};
+
+static struct i2c_driver mlx90635_driver = {
+ .driver = {
+ .name = "mlx90635",
+ .of_match_table = mlx90635_of_match,
+ .pm = pm_ptr(&mlx90635_pm_ops),
+ },
+ .probe = mlx90635_probe,
+ .id_table = mlx90635_id,
+};
+module_i2c_driver(mlx90635_driver);
+
+MODULE_AUTHOR("Crt Mori <[email protected]>");
+MODULE_DESCRIPTION("Melexis MLX90635 contactless Infra Red temperature sensor driver");
+MODULE_LICENSE("GPL");
--
2.40.1
Add device tree bindings for MLX90635 Infra Red contactless temperature
sensor.
Signed-off-by: Crt Mori <[email protected]>
---
.../iio/temperature/melexis,mlx90635.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
diff --git a/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
new file mode 100644
index 000000000000..96463121a806
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/melexis,mlx90635.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Melexis MLX90635 contactless Infra Red temperature sensor
+
+maintainers:
+ - Crt Mori <[email protected]>
+
+description: |
+ https://www.melexis.com/en/documents/documentation/datasheets/datasheet-mlx90635
+
+ There are various applications for the Infra Red contactless temperature
+ sensor and MLX90635 is most suitable for consumer applications where
+ measured object temperature is in range between -20 to 100 degrees
+ Celsius with relative error of measurement 2 degree Celsius in
+ object temperature range for industrial applications, while just 0.2
+ degree Celsius for human body measurement applications. Since it can
+ operate and measure ambient temperature in range of -20 to 85 degrees
+ Celsius it is suitable also for outdoor use.
+
+ Be aware that electronics surrounding the sensor can increase ambient
+ temperature. MLX90635 can be calibrated to reduce the housing effect via
+ already existing EEPROM parameters.
+
+ Since measured object emissivity effects Infra Red energy emitted,
+ emissivity should be set before requesting the object temperature.
+
+properties:
+ compatible:
+ const: melexis,mlx90635
+
+ reg:
+ maxItems: 1
+ description: Default is 0x3a, but can be reprogrammed.
+
+ vdd-supply:
+ description: provide VDD power to the sensor (check datasheet for voltage).
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ temp-sensor@3a {
+ compatible = "melexis,mlx90635";
+ reg = <0x3a>;
+ vdd-supply = <&ldo4_reg>;
+ };
+ };
+...
--
2.40.1
On 22/11/2023 11:27, Crt Mori wrote:
> Add device tree bindings for MLX90635 Infra Red contactless temperature
> sensor.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.
A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
>
> Signed-off-by: Crt Mori <[email protected]>
> ---
> .../iio/temperature/melexis,mlx90635.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
> new file mode 100644
> index 000000000000..96463121a806
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/melexis,mlx90635.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Melexis MLX90635 contactless Infra Red temperature sensor
> +
> +maintainers:
> + - Crt Mori <[email protected]>
> +
> +description: |
> + https://www.melexis.com/en/documents/documentation/datasheets/datasheet-mlx90635
> +
> + There are various applications for the Infra Red contactless temperature
> + sensor and MLX90635 is most suitable for consumer applications where
> + measured object temperature is in range between -20 to 100 degrees
> + Celsius with relative error of measurement 2 degree Celsius in
> + object temperature range for industrial applications, while just 0.2
> + degree Celsius for human body measurement applications. Since it can
> + operate and measure ambient temperature in range of -20 to 85 degrees
> + Celsius it is suitable also for outdoor use.
> +
> + Be aware that electronics surrounding the sensor can increase ambient
> + temperature. MLX90635 can be calibrated to reduce the housing effect via
> + already existing EEPROM parameters.
> +
> + Since measured object emissivity effects Infra Red energy emitted,
> + emissivity should be set before requesting the object temperature.
> +
> +properties:
> + compatible:
> + const: melexis,mlx90635
It's the same as mlx90632. Add it there (as enum).
Best regards,
Krzysztof
On Wed, 22 Nov 2023 at 12:52, Krzysztof Kozlowski <[email protected]> wrote:
>
> On 22/11/2023 11:27, Crt Mori wrote:
> > Add device tree bindings for MLX90635 Infra Red contactless temperature
> > sensor.
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.
>
OK, will put everyone in that list in next spin.
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
>
Ok, will fix that in next version (probably main driver review will
get some comments).
> >
> > Signed-off-by: Crt Mori <[email protected]>
> > ---
> > .../iio/temperature/melexis,mlx90635.yaml | 60 +++++++++++++++++++
> > 1 file changed, 60 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
> > new file mode 100644
> > index 000000000000..96463121a806
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/temperature/melexis,mlx90635.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Melexis MLX90635 contactless Infra Red temperature sensor
> > +
> > +maintainers:
> > + - Crt Mori <[email protected]>
> > +
> > +description: |
> > + https://www.melexis.com/en/documents/documentation/datasheets/datasheet-mlx90635
> > +
> > + There are various applications for the Infra Red contactless temperature
> > + sensor and MLX90635 is most suitable for consumer applications where
> > + measured object temperature is in range between -20 to 100 degrees
> > + Celsius with relative error of measurement 2 degree Celsius in
> > + object temperature range for industrial applications, while just 0.2
> > + degree Celsius for human body measurement applications. Since it can
> > + operate and measure ambient temperature in range of -20 to 85 degrees
> > + Celsius it is suitable also for outdoor use.
> > +
> > + Be aware that electronics surrounding the sensor can increase ambient
> > + temperature. MLX90635 can be calibrated to reduce the housing effect via
> > + already existing EEPROM parameters.
> > +
> > + Since measured object emissivity effects Infra Red energy emitted,
> > + emissivity should be set before requesting the object temperature.
> > +
> > +properties:
> > + compatible:
> > + const: melexis,mlx90635
>
> It's the same as mlx90632. Add it there (as enum).
>
Properties are the same, but then you can't have much differences for
a temperature sensor. It has a bit worse relative measurement error
outside of the human body range and overall different DSP, register
map, even physical size - it's 1.8x1.8 mm compared to 90632 3x3 mm. I
was not sure how it qualifies for adding it as another enum, but I
went with the feeling that if it can reuse the driver, then it is an
enum, otherwise it is a new file. And I could not reuse anything from
90632.
Thanks for quick feedback and best regards,
Crt
> Best regards,
> Krzysztof
>
On 22/11/2023 13:28, Crt Mori wrote:
>>> + Since measured object emissivity effects Infra Red energy emitted,
>>> + emissivity should be set before requesting the object temperature.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: melexis,mlx90635
>>
>> It's the same as mlx90632. Add it there (as enum).
>>
>
> Properties are the same, but then you can't have much differences for
> a temperature sensor. It has a bit worse relative measurement error
> outside of the human body range and overall different DSP, register
> map, even physical size - it's 1.8x1.8 mm compared to 90632 3x3 mm. I
> was not sure how it qualifies for adding it as another enum, but I
> went with the feeling that if it can reuse the driver, then it is an
> enum, otherwise it is a new file. And I could not reuse anything from
> 90632.
>
> Thanks for quick feedback and best regards,
Driver is independent choice. There is no need for new binding file if
everything is the same from bindings point of view.
Best regards,
Krzysztof
Hi Crt,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.7-rc2 next-20231124]
[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/Crt-Mori/iio-temperature-mlx90635-MLX90635-IR-Temperature-sensor/20231122-202635
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/aa36393700ff783274894186366a152bb27e58ff.1700648165.git.cmo%40melexis.com
patch subject: [PATCH 1/2] iio: temperature: mlx90635 MLX90635 IR Temperature sensor
config: x86_64-randconfig-122-20231123 (https://download.01.org/0day-ci/archive/20231124/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/iio/temperature/mlx90635.c:380:12: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
!(reg_status & MLX90635_STAT_END_CONV) == 0,
^ ~~
include/linux/regmap.h:124:58: note: expanded from macro 'regmap_read_poll_timeout'
__tmp = read_poll_timeout(regmap_read, __ret, __ret || (cond), \
^~~~
include/linux/iopoll.h:47:7: note: expanded from macro 'read_poll_timeout'
if (cond) \
^~~~
drivers/iio/temperature/mlx90635.c:380:12: note: add parentheses after the '!' to evaluate the comparison first
drivers/iio/temperature/mlx90635.c:380:12: note: add parentheses around left hand side expression to silence this warning
>> drivers/iio/temperature/mlx90635.c:380:12: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
!(reg_status & MLX90635_STAT_END_CONV) == 0,
^ ~~
include/linux/regmap.h:124:58: note: expanded from macro 'regmap_read_poll_timeout'
__tmp = read_poll_timeout(regmap_read, __ret, __ret || (cond), \
^~~~
include/linux/iopoll.h:58:3: note: expanded from macro 'read_poll_timeout'
(cond) ? 0 : -ETIMEDOUT; \
^~~~
drivers/iio/temperature/mlx90635.c:380:12: note: add parentheses after the '!' to evaluate the comparison first
drivers/iio/temperature/mlx90635.c:380:12: note: add parentheses around left hand side expression to silence this warning
2 warnings generated.
vim +380 drivers/iio/temperature/mlx90635.c
356
357 static int mlx90635_perform_measurement_burst(struct mlx90635_data *data)
358 {
359 unsigned int reg_status;
360 int refresh_time;
361 int ret;
362
363 ret = regmap_write_bits(data->regmap, MLX90635_REG_STATUS,
364 MLX90635_STAT_END_CONV, MLX90635_STAT_END_CONV);
365 if (ret < 0)
366 return ret;
367
368 ret = mlx90635_calculate_dataset_ready_time(data, &refresh_time);
369 if (ret < 0)
370 return ret;
371
372 ret = regmap_write_bits(data->regmap, MLX90635_REG_CTRL2,
373 MLX90635_CTRL2_SOB(1), MLX90635_CTRL2_SOB(1));
374 if (ret < 0)
375 return ret;
376
377 msleep(refresh_time); /* Wait minimum time for dataset to be ready */
378
379 ret = regmap_read_poll_timeout(data->regmap, MLX90635_REG_STATUS, reg_status,
> 380 !(reg_status & MLX90635_STAT_END_CONV) == 0,
381 MLX90635_TIMING_POLLING, MLX90635_READ_RETRIES * 10000);
382 if (ret < 0) {
383 dev_err(&data->client->dev, "data not ready");
384 return -ETIMEDOUT;
385 }
386
387 return ret;
388 }
389
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, 22 Nov 2023 13:35:19 +0100
Krzysztof Kozlowski <[email protected]> wrote:
> On 22/11/2023 13:28, Crt Mori wrote:
> >>> + Since measured object emissivity effects Infra Red energy emitted,
> >>> + emissivity should be set before requesting the object temperature.
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + const: melexis,mlx90635
> >>
> >> It's the same as mlx90632. Add it there (as enum).
> >>
> >
> > Properties are the same, but then you can't have much differences for
> > a temperature sensor. It has a bit worse relative measurement error
> > outside of the human body range and overall different DSP, register
> > map, even physical size - it's 1.8x1.8 mm compared to 90632 3x3 mm. I
> > was not sure how it qualifies for adding it as another enum, but I
> > went with the feeling that if it can reuse the driver, then it is an
> > enum, otherwise it is a new file. And I could not reuse anything from
> > 90632.
> >
> > Thanks for quick feedback and best regards,
>
> Driver is independent choice. There is no need for new binding file if
> everything is the same from bindings point of view.
>
> Best regards,
> Krzysztof
>
>
We got this wrong in the past in IIO and it's a slow effort to merge
the various very similar bindings. For now we are mostly keeping
to within a vendor though unless a driver supports parts from multiple
vendors. It potentially gets too confusing to maintain otherwise.
This one is easy case though so definitely merge as Krzystof suggested!
Jonathan
On Wed, 22 Nov 2023 11:24:06 +0100
Crt Mori <[email protected]> wrote:
> MLX90635 is an Infra Red contactless temperature sensor most suitable
> for consumer applications where measured object temperature is in range
> between -20 to 100 degrees Celsius. It has improved accuracy for
> measurements within temperature range of human body and can operate in
> ambient temperature range between -20 to 85 degrees Celsius.
>
> Driver provides simple power management possibility as it returns to
> lowest possible power mode (Step sleep mode) in which temperature
> measurements can still be performed, yet for continuous measuring it
> switches to Continuous power mode where measurements constantly change
> without triggering.
>
> Signed-off-by: Crt Mori<[email protected]>
Hi Crt,
Very nice. A few minor bits inline.
Note (as normal for me), I haven't sanity checked any calibration maths - just assuming
you got that bit right as don't want to spend ages comparing datasheet maths to what
you have coded up + I'm not sure I can get the datasheet anyway :)
Jonathan
> diff --git a/drivers/iio/temperature/mlx90635.c b/drivers/iio/temperature/mlx90635.c
...
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
Needed? Rarely needed in a driver that hasn't had to do custom ABI and I don't
see any here.
> +
> +/* Control register2 address - volatile */
> +#define MLX90635_REG_CTRL2 0x0016 /* Control Register2 address */
> +#define MLX90635_CTRL2_BURST_CNT_SHIFT 6 /* Burst count */
> +#define MLX90635_CTRL2_BURST_CNT_MASK GENMASK(MLX90635_CTRL2_BURST_CNT_SHIFT + 4, MLX90635_CTRL2_BURST_CNT_SHIFT)
> +#define MLX90635_CTRL2_BURST(ctrl_val) (ctrl_val << MLX90635_CTRL2_BURST_CNT_SHIFT)
> +#define MLX90635_CTRL2_MODE_SHIFT 11 /* Power mode */
> +#define MLX90635_CTRL2_MODE_MASK GENMASK(MLX90635_CTRL2_MODE_SHIFT + 1, MLX90635_CTRL2_MODE_SHIFT)
> +#define MLX90635_CTRL2_MODE(ctrl_val) (ctrl_val << MLX90635_CTRL2_MODE_SHIFT)
> +#define MLX90635_CTRL2_SOB_SHIFT 15 /* Start burst measurement in step mode */
> +#define MLX90635_CTRL2_SOB_MASK BIT(MLX90635_CTRL2_SOB_SHIFT)
> +#define MLX90635_CTRL2_SOB(ctrl_val) (ctrl_val << MLX90635_CTRL2_SOB_SHIFT)
Can't do these with mask and FIELD_PREP() to simplify the macros + ensure any passed
in value doesn't overwrite neighbouring fields?
> +/* Magic constants */
> +#define MLX90635_ID_DSPv1 0x01 /* EEPROM DSP version */
> +#define MLX90635_RESET_CMD 0x0006 /**< Reset sensor (address or global) */
Why the /**< syntax?
> +#define MLX90635_MAX_MEAS_NUM 31 /**< Maximum number of measurements in list */
> +#define MLX90635_PTAT_DIV 12 /**< Used to divide the PTAT value in pre-processing */
> +#define MLX90635_IR_DIV 24 /**< Used to divide the IR value in pre-processing */
> +#define MLX90635_SLEEP_DELAY_MS 6000 /* Autosleep delay */
> +#define MLX90635_MEAS_MAX_TIME 2000 /* Max measurement time in ms for the lowest refresh rate */
> +#define MLX90635_READ_RETRIES 100 /* Number of read retries before quitting with timeout error */
> +#define MLX90635_VERSION_MASK (GENMASK(15, 12) | GENMASK(7, 4))
> +#define MLX90635_DSP_VERSION(reg) ((reg & GENMASK(6, 4)) >> 3)
> +#define MLX90635_DSP_FIXED (BIT(15))
Possible to have too many brackets. I'd drop them around the BIT()
> +
> +static int mlx90635_wakeup(struct mlx90635_data *data)
> +{
> + s16 Fb, Ga, Gb, Ha, Hb, PG, PO;
> + u32 Ea, Eb, Fa;
> + u16 Fa_scale;
> + int ret;
> +
> + regcache_cache_bypass(data->regmap, false);
> + ret = mlx90635_pwr_continuous(data);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Switch to continuous mode failed\n");
> + return ret;
> + }
> + ret = regmap_write_bits(data->regmap, MLX90635_REG_EE, MLX90635_EE_ACTIVE, MLX90635_EE_ACTIVE);
Long line. Add a break somewhere sensible.
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Powering EEPROM failed\n");
> + return ret;
> + }
> + usleep_range(MLX90635_TIMING_EE_ACTIVE_MIN, MLX90635_TIMING_EE_ACTIVE_MAX);
> +
> + regcache_mark_dirty(data->regmap);
> +
> + ret = regcache_sync(data->regmap);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to cache everything: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regcache_sync_region(data->regmap, MLX90635_EE_Ha, MLX90635_EE_Gb);
Why is this needed given you just synced the whole thing?
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to sync EEEPROM region: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mlx90635_read_ee_ambient(data->regmap, &PG, &PO, &Gb);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to read to cache Ambient coefficients EEPROM region: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mlx90635_read_ee_object(data->regmap, &Ea, &Eb, &Fa, &Fb, &Ga, &Gb, &Ha, &Hb, &Fa_scale);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to read to cache Object coefficients EEPROM region: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_async_complete(data->regmap);
What is this syncing here for? Several calls above include internal calls to this.
My knowledge of this stuff is limited, so I may well be misunderstanding what this does.
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to complete sync: %d\n", ret);
> + return ret;
> + }
> +
> + return ret;
> +}
...
> +static int mlx90635_probe(struct i2c_client *client)
> +{
> + const struct i2c_device_id *id = i2c_client_get_device_id(client);
Shouldn't do this in a modern driver because of fragility I mention below.
Use the generic stuff to access data if you need it (but you don't here).
> + struct mlx90635_data *mlx90635;
> + struct iio_dev *indio_dev;
> + unsigned int dsp_version;
> + struct regmap *regmap;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mlx90635));
> + if (!indio_dev) {
> + dev_err(&client->dev, "Failed to allocate device\n");
> + return -ENOMEM;
> + }
> +
> + regmap = devm_regmap_init_i2c(client, &mlx90635_regmap);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(&client->dev, "Failed to allocate regmap: %d\n", ret);
return dev_err_probe() for all messages in probe and things only called from probe.
It's neater, handles the ret printing and even does deferred handling correctly
if that's relevant. Also, if you used it everywhere I don't have to think about
what might defer which makes for easier reviews :)
> + return ret;
> + }
> +
> + mlx90635 = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + mlx90635->client = client;
> + mlx90635->regmap = regmap;
> + mlx90635->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
> +
> + mutex_init(&mlx90635->lock);
> + indio_dev->name = id->name;
Not keen on doing this as it can be fragile if id and of tables get out of sync
or we are using backwards compatibles in dt bindings.
Given only one part supported, just hard code the name for now.
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &mlx90635_info;
> + indio_dev->channels = mlx90635_channels;
> + indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
> +
> + mlx90635->regulator = devm_regulator_get(&client->dev, "vdd");
> + if (IS_ERR(mlx90635->regulator))
> + return dev_err_probe(&client->dev, PTR_ERR(mlx90635->regulator),
> + "failed to get vdd regulator");
> +
> + ret = mlx90635_enable_regulator(mlx90635);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(&client->dev, mlx90635_disable_regulator,
> + mlx90635);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to setup regulator cleanup action %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = mlx90635_wakeup(mlx90635);
> + if (ret < 0) {
> + dev_err(&client->dev, "Wakeup failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(&client->dev, mlx90635_sleep, mlx90635);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to setup low power cleanup action %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = regmap_read(mlx90635->regmap, MLX90635_EE_VERSION, &dsp_version);
> + if (ret < 0) {
> + dev_err(&client->dev, "read of version failed: %d\n", ret);
> + return ret;
> + }
> + dsp_version = dsp_version & MLX90635_VERSION_MASK;
FIELD_GET() always preferred as then I don't need to wonder if there is an offset for
the field or not as it will be handled correctly either way
> + if (MLX90635_DSP_VERSION(dsp_version) == MLX90635_ID_DSPv1) {
> + dev_dbg(&client->dev,
> + "Detected DSP v1 calibration %x\n", dsp_version);
> + } else if ((dsp_version & MLX90635_DSP_FIXED) == MLX90635_DSP_FIXED) {
FIELD_GET() for that bit then just check if it is 0 or 1
> + dev_dbg(&client->dev,
> + "Detected Unknown EEPROM calibration %lx\n", MLX90635_DSP_VERSION(dsp_version));
> + } else {
> + dev_err(&client->dev,
> + "Wrong fixed top bit %lx (expected 0x8X0X)\n",
> + dsp_version & MLX90635_DSP_FIXED);
I'd like to understand what breaks if this happens but we carry on anyway?
I'd 'hope' that any future DSP version is backwards compatible or that there was some way to know if
the difference between backwards compatible versions and ones that aren't.
> + return -EPROTONOSUPPORT;
> + }
...
> +
> +static const struct i2c_device_id mlx90635_id[] = {
> + { "mlx90635", 0 },
The 0 is unnecessary so I'd drop it.
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mlx90635_id);
> +
> +static const struct of_device_id mlx90635_of_match[] = {
> + { .compatible = "melexis,mlx90635" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, mlx90635_of_match);
Thanks,
Jonathan
On Sat, 25 Nov 2023 at 18:53, Jonathan Cameron <[email protected]> wrote:
>
> On Wed, 22 Nov 2023 11:24:06 +0100
> Crt Mori <[email protected]> wrote:
>
> > MLX90635 is an Infra Red contactless temperature sensor most suitable
> > for consumer applications where measured object temperature is in range
> > between -20 to 100 degrees Celsius. It has improved accuracy for
> > measurements within temperature range of human body and can operate in
> > ambient temperature range between -20 to 85 degrees Celsius.
> >
> > Driver provides simple power management possibility as it returns to
> > lowest possible power mode (Step sleep mode) in which temperature
> > measurements can still be performed, yet for continuous measuring it
> > switches to Continuous power mode where measurements constantly change
> > without triggering.
> >
> > Signed-off-by: Crt Mori<[email protected]>
> Hi Crt,
>
> Very nice. A few minor bits inline.
>
> Note (as normal for me), I haven't sanity checked any calibration maths - just assuming
> you got that bit right as don't want to spend ages comparing datasheet maths to what
> you have coded up + I'm not sure I can get the datasheet anyway :)
>
> Jonathan
>
Hi Jonathan,
Maths I have unit tests where I did floating point (which will be
released as embedded library same as for 90632) to integer conversion
and ensure that the delta is less then the error of the sensor. So
math I take full responsibility :)
Datasheet will be public probably in March, when hopefully the sensor
is already part of the main Android kernel as well. But your review is
anyway very valuable and detailed. Thanks for all the remarks - there
is just one discussion below I would love to complete for future
reference.
Best regards,
Crt
>> ...
> > + if (ret < 0) {
> > + dev_err(&data->client->dev, "Powering EEPROM failed\n");
> > + return ret;
> > + }
> > + usleep_range(MLX90635_TIMING_EE_ACTIVE_MIN, MLX90635_TIMING_EE_ACTIVE_MAX);
> > +
> > + regcache_mark_dirty(data->regmap);
> > +
> > + ret = regcache_sync(data->regmap);
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "Failed to cache everything: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = regcache_sync_region(data->regmap, MLX90635_EE_Ha, MLX90635_EE_Gb);
>
> Why is this needed given you just synced the whole thing?
>
Well, this is the discussion I wanted to have. I expected
regcache_sync to perform i2c read of all the read_regs defined in
regmap to get them to cache - but it didn't. Then I expected the same
from sync_region, but it didn't, so I manually read them below.
So discussion I want to have is: why would we regcache_sync, if no
reads are performed? And second one is why would we return EBUSY for
volatile register range when cache_only is active?
Current driver implementation is very specific in bypassing all these,
so I am quite certain this regcache_sync_region here is not needed and
below regmap_async_complete as well, but I cannot be sure, since
regcache_sync doesn't really do the job I would expect it to do. I
looked into the code, but could not find the reason, why I do not see
any reads on oscilloscope and why I need to physically read registers
below, so that they are cached. regmap totally knows which registers
it should cache, but it does not at init, nor at regcache_sync
request. And if you remember in 90632 I had a similar remark, but
could not reproduce as EEPROM was readable in most powermodes (well
all used in driver), now I checked with scope and since I know this
chip does not allow EEPROM reading during the step sleep mode, so
everything was much easier to conclude.
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "Failed to sync EEEPROM region: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = mlx90635_read_ee_ambient(data->regmap, &PG, &PO, &Gb);
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "Failed to read to cache Ambient coefficients EEPROM region: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = mlx90635_read_ee_object(data->regmap, &Ea, &Eb, &Fa, &Fb, &Ga, &Gb, &Ha, &Hb, &Fa_scale);
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "Failed to read to cache Object coefficients EEPROM region: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = regmap_async_complete(data->regmap);
>
> What is this syncing here for? Several calls above include internal calls to this.
> My knowledge of this stuff is limited, so I may well be misunderstanding what this does.
>
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "Failed to complete sync: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return ret;
> > +}
>
> ...
>
...
> > + mlx90635 = iio_priv(indio_dev);
> > + i2c_set_clientdata(client, indio_dev);
> > + mlx90635->client = client;
> > + mlx90635->regmap = regmap;
> > + mlx90635->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
> > +
> > + mutex_init(&mlx90635->lock);
> > + indio_dev->name = id->name;
>
> Not keen on doing this as it can be fragile if id and of tables get out of sync
> or we are using backwards compatibles in dt bindings.
>
> Given only one part supported, just hard code the name for now.
>
Can you elaborate? Because I have the same thing in 90632 and I would
fix there as well. I assumed this is for linking to dt, to ensure it
is defined there?
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->info = &mlx90635_info;
> > + indio_dev->channels = mlx90635_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
> > +
...
>
> > + if (MLX90635_DSP_VERSION(dsp_version) == MLX90635_ID_DSPv1) {
> > + dev_dbg(&client->dev,
> > + "Detected DSP v1 calibration %x\n", dsp_version);
> > + } else if ((dsp_version & MLX90635_DSP_FIXED) == MLX90635_DSP_FIXED) {
>
> FIELD_GET() for that bit then just check if it is 0 or 1
>
> > + dev_dbg(&client->dev,
> > + "Detected Unknown EEPROM calibration %lx\n", MLX90635_DSP_VERSION(dsp_version));
> > + } else {
> > + dev_err(&client->dev,
> > + "Wrong fixed top bit %lx (expected 0x8X0X)\n",
> > + dsp_version & MLX90635_DSP_FIXED);
>
> I'd like to understand what breaks if this happens but we carry on anyway?
> I'd 'hope' that any future DSP version is backwards compatible or that there was some way to know if
> the difference between backwards compatible versions and ones that aren't.
>
The top bit in high nibble is fixed to 1, to ensure that we have
endianness correct in the wild. We did the same later on in 90632
where we had plenty of trouble the way people read 16 bits. So if that
top bit is not there (bottom nibble has it hardcoded to 0), then for
certain we do not have the correct chip. And as for compatible DSP
versions: when we release incompatible one, I will upgrade the driver,
otherwise we have some more slack in the driver to keep on working,
because that was also lesson learnt from my side in 90632 as there are
compatible DSP versions possible and used, but we are still honest and
bump also the DSP version here.
> > + return -EPROTONOSUPPORT;
> > + }
> ...
>
>
On Sat, 25 Nov 2023 22:26:46 +0100
Crt Mori <[email protected]> wrote:
> On Sat, 25 Nov 2023 at 18:53, Jonathan Cameron <[email protected]> wrote:
> >
> > On Wed, 22 Nov 2023 11:24:06 +0100
> > Crt Mori <[email protected]> wrote:
> >
> > > MLX90635 is an Infra Red contactless temperature sensor most suitable
> > > for consumer applications where measured object temperature is in range
> > > between -20 to 100 degrees Celsius. It has improved accuracy for
> > > measurements within temperature range of human body and can operate in
> > > ambient temperature range between -20 to 85 degrees Celsius.
> > >
> > > Driver provides simple power management possibility as it returns to
> > > lowest possible power mode (Step sleep mode) in which temperature
> > > measurements can still be performed, yet for continuous measuring it
> > > switches to Continuous power mode where measurements constantly change
> > > without triggering.
> > >
> > > Signed-off-by: Crt Mori<[email protected]>
> > Hi Crt,
> >
> > Very nice. A few minor bits inline.
> >
> > Note (as normal for me), I haven't sanity checked any calibration maths - just assuming
> > you got that bit right as don't want to spend ages comparing datasheet maths to what
> > you have coded up + I'm not sure I can get the datasheet anyway :)
> >
> > Jonathan
> >
> Hi Jonathan,
> Maths I have unit tests where I did floating point (which will be
> released as embedded library same as for 90632) to integer conversion
> and ensure that the delta is less then the error of the sensor. So
> math I take full responsibility :)
>
> Datasheet will be public probably in March, when hopefully the sensor
> is already part of the main Android kernel as well. But your review is
> anyway very valuable and detailed. Thanks for all the remarks - there
> is just one discussion below I would love to complete for future
> reference.
>
> Best regards,
> Crt
> >> ...
> > > + if (ret < 0) {
> > > + dev_err(&data->client->dev, "Powering EEPROM failed\n");
> > > + return ret;
> > > + }
> > > + usleep_range(MLX90635_TIMING_EE_ACTIVE_MIN, MLX90635_TIMING_EE_ACTIVE_MAX);
> > > +
> > > + regcache_mark_dirty(data->regmap);
> > > +
> > > + ret = regcache_sync(data->regmap);
> > > + if (ret < 0) {
> > > + dev_err(&data->client->dev,
> > > + "Failed to cache everything: %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = regcache_sync_region(data->regmap, MLX90635_EE_Ha, MLX90635_EE_Gb);
> >
> > Why is this needed given you just synced the whole thing?
> >
>
> Well, this is the discussion I wanted to have. I expected
> regcache_sync to perform i2c read of all the read_regs defined in
> regmap to get them to cache - but it didn't. Then I expected the same
> from sync_region, but it didn't, so I manually read them below.
>
> So discussion I want to have is: why would we regcache_sync, if no
> reads are performed?
I think the intent is to minimise the reads from the device.
If we look at the implmentation (which is probably the default here)
you can see that it's only doing writes where non volatile registers are out
of date. I don't think there are any reads from the device done.
https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regcache.c#L310
If you want to ensure the cache contains the correct values
a read will need to be forced.
> And second one is why would we return EBUSY for
> volatile register range when cache_only is active?
I'd say that's correct. We are in a state where any attempt to read a
volatile register can't get the current correct value, so returning
-EBUSY which basically says come back later is the correct
behaviour.
>
> Current driver implementation is very specific in bypassing all these,
> so I am quite certain this regcache_sync_region here is not needed and
> below regmap_async_complete as well, but I cannot be sure, since
> regcache_sync doesn't really do the job I would expect it to do. I
> looked into the code, but could not find the reason, why I do not see
> any reads on oscilloscope and why I need to physically read registers
> below, so that they are cached.
I agree the docs are a little vague, but looking at the code there
aren't any reads, only writes to the device so I think this behaviour
is by design. The cache is considered correctly synced if an entry
is simply not cached (valid I suppose as no wrong values cached).
> regmap totally knows which registers
> it should cache, but it does not at init, nor at regcache_sync
> request. And if you remember in 90632 I had a similar remark, but
> could not reproduce as EEPROM was readable in most powermodes (well
> all used in driver), now I checked with scope and since I know this
> chip does not allow EEPROM reading during the step sleep mode, so
> everything was much easier to conclude.
I think the key here is that the cache isn't really meant to provide
access to values when the device is powered down; it is there to
reduce bus traffic. Hence the last thing you normally want to do is
read back all the values. There is a way to get that to happen
on init though.
You want to hit this path:
https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regcache.c#L183
I think you set num_reg_defaults_raw = Number of registers, but don't provide any
default values, so as to indicate they should be read from the device.
> ...
> > > + mlx90635 = iio_priv(indio_dev);
> > > + i2c_set_clientdata(client, indio_dev);
> > > + mlx90635->client = client;
> > > + mlx90635->regmap = regmap;
> > > + mlx90635->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
> > > +
> > > + mutex_init(&mlx90635->lock);
> > > + indio_dev->name = id->name;
> >
> > Not keen on doing this as it can be fragile if id and of tables get out of sync
> > or we are using backwards compatibles in dt bindings.
> >
> > Given only one part supported, just hard code the name for now.
> >
>
> Can you elaborate? Because I have the same thing in 90632 and I would
> fix there as well. I assumed this is for linking to dt, to ensure it
> is defined there?
Exactly, the dt table and the i2c_id one can end up out of sync - perhaps
deliberately and when compatible = "device1", "device2" is used
I'm not sure exactly which one will turn up in this id if the dt table
only includes device2.
So rather than arguing that out and pinning down the expected behaviour
we tend to avoid using id->name in new drivers.
It's probably not broken to do so but lets make life easier for any
future cases by not doing it.
>
> > > + indio_dev->modes = INDIO_DIRECT_MODE;
> > > + indio_dev->info = &mlx90635_info;
> > > + indio_dev->channels = mlx90635_channels;
> > > + indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
> > > +
> ...
> >
> > > + if (MLX90635_DSP_VERSION(dsp_version) == MLX90635_ID_DSPv1) {
> > > + dev_dbg(&client->dev,
> > > + "Detected DSP v1 calibration %x\n", dsp_version);
> > > + } else if ((dsp_version & MLX90635_DSP_FIXED) == MLX90635_DSP_FIXED) {
> >
> > FIELD_GET() for that bit then just check if it is 0 or 1
> >
> > > + dev_dbg(&client->dev,
> > > + "Detected Unknown EEPROM calibration %lx\n", MLX90635_DSP_VERSION(dsp_version));
> > > + } else {
> > > + dev_err(&client->dev,
> > > + "Wrong fixed top bit %lx (expected 0x8X0X)\n",
> > > + dsp_version & MLX90635_DSP_FIXED);
> >
> > I'd like to understand what breaks if this happens but we carry on anyway?
> > I'd 'hope' that any future DSP version is backwards compatible or that there was some way to know if
> > the difference between backwards compatible versions and ones that aren't.
> >
> The top bit in high nibble is fixed to 1, to ensure that we have
> endianness correct in the wild. We did the same later on in 90632
> where we had plenty of trouble the way people read 16 bits. So if that
> top bit is not there (bottom nibble has it hardcoded to 0), then for
> certain we do not have the correct chip. And as for compatible DSP
> versions: when we release incompatible one, I will upgrade the driver,
> otherwise we have some more slack in the driver to keep on working,
> because that was also lesson learnt from my side in 90632 as there are
> compatible DSP versions possible and used, but we are still honest and
> bump also the DSP version here.
So there isn't a clear division between 'minor and major' version numbers
as used in many similar cases? Minor can tick without a driver change as the
interface only grows (nothing breaks), but major implies incompatible.
Anyhow, sounds like you carry on with just a dbg print if an unexpected version
seen. That's fine.
Jonathan
On Sun, 26 Nov 2023 at 17:01, Jonathan Cameron <[email protected]> wrote:
>
> On Sat, 25 Nov 2023 22:26:46 +0100
> Crt Mori <[email protected]> wrote:
>
> > On Sat, 25 Nov 2023 at 18:53, Jonathan Cameron <[email protected]> wrote:
> > >
> > > On Wed, 22 Nov 2023 11:24:06 +0100
> > > Crt Mori <[email protected]> wrote:
> > >
> > > > MLX90635 is an Infra Red contactless temperature sensor most suitable
> > > > for consumer applications where measured object temperature is in range
> > > > between -20 to 100 degrees Celsius. It has improved accuracy for
> > > > measurements within temperature range of human body and can operate in
> > > > ambient temperature range between -20 to 85 degrees Celsius.
> > > >
> > > > Driver provides simple power management possibility as it returns to
> > > > lowest possible power mode (Step sleep mode) in which temperature
> > > > measurements can still be performed, yet for continuous measuring it
> > > > switches to Continuous power mode where measurements constantly change
> > > > without triggering.
> > > >
> > > > Signed-off-by: Crt Mori<[email protected]>
> > > Hi Crt,
> > >
> > > Very nice. A few minor bits inline.
> > >
> > > Note (as normal for me), I haven't sanity checked any calibration maths - just assuming
> > > you got that bit right as don't want to spend ages comparing datasheet maths to what
> > > you have coded up + I'm not sure I can get the datasheet anyway :)
> > >
> > > Jonathan
> > >
> > Hi Jonathan,
> > Maths I have unit tests where I did floating point (which will be
> > released as embedded library same as for 90632) to integer conversion
> > and ensure that the delta is less then the error of the sensor. So
> > math I take full responsibility :)
> >
> > Datasheet will be public probably in March, when hopefully the sensor
> > is already part of the main Android kernel as well. But your review is
> > anyway very valuable and detailed. Thanks for all the remarks - there
> > is just one discussion below I would love to complete for future
> > reference.
> >
> > Best regards,
> > Crt
...
> > any reads on oscilloscope and why I need to physically read registers
> > below, so that they are cached.
>
> I agree the docs are a little vague, but looking at the code there
> aren't any reads, only writes to the device so I think this behaviour
> is by design. The cache is considered correctly synced if an entry
> is simply not cached (valid I suppose as no wrong values cached).
>
> > regmap totally knows which registers
> > it should cache, but it does not at init, nor at regcache_sync
> > request. And if you remember in 90632 I had a similar remark, but
> > could not reproduce as EEPROM was readable in most powermodes (well
> > all used in driver), now I checked with scope and since I know this
> > chip does not allow EEPROM reading during the step sleep mode, so
> > everything was much easier to conclude.
>
> I think the key here is that the cache isn't really meant to provide
> access to values when the device is powered down; it is there to
> reduce bus traffic. Hence the last thing you normally want to do is
> read back all the values. There is a way to get that to happen
> on init though.
>
> You want to hit this path:
> https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regcache.c#L183
> I think you set num_reg_defaults_raw = Number of registers, but don't provide any
> default values, so as to indicate they should be read from the device.
>
Ok, then I have understood this correctly I should only retain
regcache_sync and abandon any other call to sync or complete and just
rely on the natural state of the cache (and the toggling of the
cache_only flag I am doing here). Will roll v2 with that, after I
confirm it still works.
> > ...
> > > > + mlx90635 = iio_priv(indio_dev);
> > > > + i2c_set_clientdata(client, indio_dev);
> > > > + mlx90635->client = client;
> > > > + mlx90635->regmap = regmap;
> > > > + mlx90635->powerstatus = MLX90635_PWR_STATUS_SLEEP_STEP;
> > > > +
> > > > + mutex_init(&mlx90635->lock);
> > > > + indio_dev->name = id->name;
> > >
> > > Not keen on doing this as it can be fragile if id and of tables get out of sync
> > > or we are using backwards compatibles in dt bindings.
> > >
> > > Given only one part supported, just hard code the name for now.
> > >
> >
> > Can you elaborate? Because I have the same thing in 90632 and I would
> > fix there as well. I assumed this is for linking to dt, to ensure it
> > is defined there?
>
> Exactly, the dt table and the i2c_id one can end up out of sync - perhaps
> deliberately and when compatible = "device1", "device2" is used
> I'm not sure exactly which one will turn up in this id if the dt table
> only includes device2.
>
> So rather than arguing that out and pinning down the expected behaviour
> we tend to avoid using id->name in new drivers.
> It's probably not broken to do so but lets make life easier for any
> future cases by not doing it.
>
>
Ok, will also roll the fix for the existing two drivers.
> >
...
> > >
> > > I'd like to understand what breaks if this happens but we carry on anyway?
> > > I'd 'hope' that any future DSP version is backwards compatible or that there was some way to know if
> > > the difference between backwards compatible versions and ones that aren't.
> > >
> > The top bit in high nibble is fixed to 1, to ensure that we have
> > endianness correct in the wild. We did the same later on in 90632
> > where we had plenty of trouble the way people read 16 bits. So if that
> > top bit is not there (bottom nibble has it hardcoded to 0), then for
> > certain we do not have the correct chip. And as for compatible DSP
> > versions: when we release incompatible one, I will upgrade the driver,
> > otherwise we have some more slack in the driver to keep on working,
> > because that was also lesson learnt from my side in 90632 as there are
> > compatible DSP versions possible and used, but we are still honest and
> > bump also the DSP version here.
>
> So there isn't a clear division between 'minor and major' version numbers
> as used in many similar cases? Minor can tick without a driver change as the
> interface only grows (nothing breaks), but major implies incompatible.
>
> Anyhow, sounds like you carry on with just a dbg print if an unexpected version
> seen. That's fine.
>
I am trying to convince them to do that, but your comment will provide
me some leverage for my case. As usual, I was assured there would be
no DSPv2 anyway. Thanks for the feedback.
Crt
>
> Jonathan
>
>
>