Hi all,
I attempted to send this yesterday, but I guess I leaked some HTML into
the message and it was rejected from the lists. I am resending it now as
plain text. Apologies for any inconvenience or confusion.
On 5/19/24 12:14 PM, Jonathan Cameron wrote:
On Fri, 17 May 2024 10:10:49 +0200
Dimitri Fedrau <dima.fedrau@xxxxxxxxx> wrote:
The mapping from cold junction to ambient temperature is inaccurate. We
provide an index for hot and cold junction temperatures.
Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
Signed-off-by: Dimitri Fedrau <dima.fedrau@xxxxxxxxx>
Hi Dmitri,
I'm not sure you replied to the question in previous review of what
sysfs files exist for this device. Whilst I am at least a little
open to changing the ABI, I'd like to fully understand what
is currently presented and why iio_info is having trouble with it.
I also want an ack from Andrew on this one given might break it existing
usage.
I’m not actively using the cold junction temperature reading, so I would
be happy to see any deficiencies in the ABI corrected.
The current interface is perhaps less than ideal, but I don't think it
is wrong as such. Whilst I wasn't particularly keen on the cold junction
== ambient I'm not sure moving to just indexed is an improvement.
Hence looking for input from Andrew. +CC Nuno as someone who is both
active in IIO and has written thermocouple front end drivers in
the past.
The ABI docs state
The ambient and object modifiers distinguish between ambient
(reference) and distant temperatures for contactless measurements
Reading more of the Linux Driver API docs, those say that .modified is
"used to indicate a physically unique characteristic of the channel”,
and that .indexed is "simply another instance”.
I’m not sure whether measuring temperature at a different location meets
the bar of a “physically unique characteristic”. Maybe it does. But I
don’t think of the cold junction temperature as “simply another
instance”. Perhaps that’s a mistake on my behalf.
Reviewing temperature drivers using IIO_MOD_TEMP_AMBIENT, they all seem
to be reporting die temperatures. Some are IR sensors, but there are a
couple other thermocouples like the MCP9600.
Reviewing drivers using “.indexed”, one is an IR sensor and one is a
thermocouple. In both cases, the indexed channels seem to represent a
“full featured” channel. The IR sensor also reports
IIO_MOD_TEMP_AMBIENT, so they chose not to make it an additional index.
It seems to me that using IIO_MOD_TEMP_AMBIENT is more in line with what
has been done in the past. But I may be misunderstanding something and I
am not opposed to using and index if it’s determined that is more correct.
Thanks,
Andrew
Jonathan
---
drivers/iio/temperature/mcp9600.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 46845804292b..22451d1d9e1f 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -14,6 +14,9 @@
#include <linux/iio/iio.h>
+#define MCP9600_CHAN_HOT_JUNCTION 0
+#define MCP9600_CHAN_COLD_JUNCTION 1
+
/* MCP9600 registers */
#define MCP9600_HOT_JUNCTION 0x0
#define MCP9600_COLD_JUNCTION 0x2
@@ -25,17 +28,19 @@
static const struct iio_chan_spec mcp9600_channels[] = {
{
.type = IIO_TEMP,
+ .channel = MCP9600_CHAN_HOT_JUNCTION,
.address = MCP9600_HOT_JUNCTION,
.info_mask_separate =
BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ .indexed = 1,
},
{
.type = IIO_TEMP,
+ .channel = MCP9600_CHAN_COLD_JUNCTION,
.address = MCP9600_COLD_JUNCTION,
- .channel2 = IIO_MOD_TEMP_AMBIENT,
- .modified = 1,
.info_mask_separate =
BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ .indexed = 1,
},
};
