iio: temperature: mlx90635 Driver for MLX90635 IR temperature sensor

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]>

drivers/iio/temperature/mlx90635.c +0 -0

drivers/iio/temperature/mlx90635.c +5 -0

+
 
 
 
+static int mlx90635_wakeup(struct mlx90635_data *data)
+{
+	s16 Fb, Ga, Gb, Ha, Hb, PG, PO;
+	u32 Ea, Eb, Fa;

drivers/iio/temperature/mlx90635.c +6 -0

+	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);
+

drivers/iio/temperature/mlx90635.c +11 -0

+	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;

drivers/iio/temperature/mlx90635.c +8 -0

+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"Failed to complete sync: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}

drivers/iio/temperature/mlx90635.c +6 -0

+	struct mlx90635_data *mlx90635;
+	struct iio_dev *indio_dev;
+	unsigned int dsp_version;
+	struct regmap *regmap;
+	int ret;
+

drivers/iio/temperature/mlx90635.c +32 -0

+	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);

drivers/iio/temperature/mlx90635.c +2 -0

+		return -EPROTONOSUPPORT;
+	}

Quoted 6 lines · 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.

Quoted 7 lines

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

Quoted 11 lines

...

drivers/iio/temperature/mlx90635.c new file +10 -0

+     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) {

drivers/iio/temperature/mlx90635.c line 872

Quoted 6 lines

drivers/iio/temperature/mlx90635.c new file +6 -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?

Quoted 11 lines

drivers/iio/temperature/mlx90635.c new file +11 -0

+     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;

drivers/iio/temperature/mlx90635.c line 893

Quoted 10 lines

drivers/iio/temperature/mlx90635.c new file +10 -0

+     }
+
+     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.

Quoted 8 lines

drivers/iio/temperature/mlx90635.c new file +8 -0

+     if (ret < 0) {
+             dev_err(&data->client->dev,
+                     "Failed to complete sync: %d\n", ret);
+             return ret;
+     }
+
+     return ret;
+}

...

drivers/iio/temperature/mlx90635.c line 958

Quoted 8 lines

drivers/iio/temperature/mlx90635.c new file +8 -0

+     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.

Quoted 5 lines

drivers/iio/temperature/mlx90635.c new file +5 -0

+     indio_dev->modes = INDIO_DIRECT_MODE;
+     indio_dev->info = &mlx90635_info;
+     indio_dev->channels = mlx90635_channels;
+     indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
+

drivers/iio/temperature/mlx90635.c line 1003

Quoted 5 lines

drivers/iio/temperature/mlx90635.c new file +4 -0

+     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

drivers/iio/temperature/mlx90635.c line 1009

Quoted 6 lines

drivers/iio/temperature/mlx90635.c new file +6 -0

+             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.

Quoted 2 lines

drivers/iio/temperature/mlx90635.c new file +2 -0

+             return -EPROTONOSUPPORT;
+     }

...

Quoted 19 lines · Jonathan Cameron

Quoted 6 lines · 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.

Quoted 7 lines

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,

Quoted 8 lines

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

Quoted 11 lines

Quoted 11 lines

...

drivers/iio/temperature/mlx90635.c new file +10 -0

+     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) {

Quoted 9 lines

drivers/iio/temperature/mlx90635.c line 872

Quoted 6 lines

drivers/iio/temperature/mlx90635.c new file +6 -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.

...

Quoted 12 lines

drivers/iio/temperature/mlx90635.c line 958

Quoted 8 lines

drivers/iio/temperature/mlx90635.c new file +8 -0

+     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.

Quoted 2 lines

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?

Quoted 6 lines

Quoted 6 lines

drivers/iio/temperature/mlx90635.c new file +5 -0

+     indio_dev->modes = INDIO_DIRECT_MODE;
+     indio_dev->info = &mlx90635_info;
+     indio_dev->channels = mlx90635_channels;
+     indio_dev->num_channels = ARRAY_SIZE(mlx90635_channels);
+  

...

Quoted 15 lines

drivers/iio/temperature/mlx90635.c line 1003

Quoted 5 lines

drivers/iio/temperature/mlx90635.c new file +4 -0

+     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

drivers/iio/temperature/mlx90635.c line 1009

Quoted 6 lines

drivers/iio/temperature/mlx90635.c new file +6 -0

+             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);  

Quoted 4 lines

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.

Quoted 42 lines · On Sat, 25 Nov 2023 22:26:46 +0100 Crt Mori <[email protected]> wrote:

Quoted 19 lines · Jonathan Cameron

Quoted 6 lines · 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.

Quoted 7 lines

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,

Quoted 8 lines

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

Quoted 2 lines

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).

Quoted 7 lines

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.

Quoted 13 lines

...

Quoted 12 lines

drivers/iio/temperature/mlx90635.c line 958

Quoted 8 lines

drivers/iio/temperature/mlx90635.c new file +8 -0

+     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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 1 line

Quoted 5 lines

Quoted 5 lines

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.

Quoted 10 lines

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

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

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml new file +23 -0

new file mode 100644
index 000000000000..96463121a806
@@ -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.
+

Quoted 2 lines · Crt Mori

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.

Quoted 40 lines

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

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml new file +27 -0

new file mode 100644
index 000000000000..96463121a806
@@ -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.
+

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml line 33

Quoted 6 lines

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml new file +6 -0

+  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

Quoted 9 lines

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml line 33

Quoted 6 lines

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml new file +6 -0

+  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,

Quoted 19 lines · Crt Mori

Quoted 9 lines

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml line 33

Quoted 6 lines

Documentation/devicetree/bindings/iio/temperature/melexis,mlx90635.yaml new file +6 -0

+  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.

Quoted 1 line

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