RE: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

RE: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[Stan, Liviu]

Thank you for the comments, and I apologize for the delayed answer. 

On Mon, Apr 27, 2026, Conor Dooley wrote:
> I have to wonder if this is the right approach, if it's the same device
> just with a different label and advertised purpose.

Could you expand on this? Are you suggesting the copper-trace@ and
leak-detector@ node types aren't needed, or something about the overall
approach of adding ADT7604 to the existing ltc2983 binding? I want to
make sure I understand the concern.

> Pedantry perhaps, but isn't this an "ohmmeter"?

For the naming I followed the convention used in this binding: rtd@, 
thermocouple@, diode@ are named after what they are, not what
they measure. copper-trace@ names the component being sensed. Jonathan
has a related question below about whether this should just reuse rtd@
with type 18, so the name might change depending on where that lands.

> I want to look into this property, where is the datasheet?

The ADT7604 datasheet is not yet publicly available. Will add the URL
once it is.
 
> Are these ever linked in a different way?
> Ditto here and in the rtd node. Are these fixed linkages or actually
> dynamic?
 
The linkage is board-dependent. Any rsense on channels 2-20 can be
paired with any sensor channel in that range. The example uses two
separate sense resistors because the copper trace and leak detector
channels need different values (100Ω vs 10kΩ, these are the recommended
values in the datasheet). A board where all sensors share the same 
resistance range could use a single rsense.

Thanks,
Liviu

RE: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[Stan, Liviu]

Thanks for the comments, here are my answers:

On 28 Apr 2026, Jonathan Cameron wrote:
> I don't know much about these temp sensors, but how is this different
> in practice from a 2-wire RTD?  Obviously one is copper and the other
> probably much more precise platinum but does that matter to us?

The main practical differences are:

- The primary output is IIO_RESISTANCE, read from the resistance result
   bank (0x0060-0x00AF). This bank is marked as reserved for the other 
   devices
- Sensor configuration bits 21:18 are hardcoded to 0b1001 for all
   copper trace configurations. For the sub-ohm variant, bits 17:0 are 
   also zeroed; a >1Ω trace will have the excitation current and an 
   optional custom table in those bits. For the existing custom RTD and
   thermistor types, the custom table is required by the binding. For
   copper trace, it is optional (and forbidden for the sub-ohm variant).
   And for leak detector as well it is optional.
- When a custom table is present, a second IIO_TEMP channel also
   appears, reading from the temperature bank. Same dual-output
   behavior for leak detector.

That said, the hardware uses the same custom RTD mode (sensor
type 18) internally.

> I'd go with "LTC2983 and similar" for the title now as it's
> to long. Leave the description to list amount more info.
>
> Alphabetical order and it might be worth thinking about switching this
> to a bulleted list with one device per line as it'll make adding new ones
> neater. (obviously they are already not in numeric order, so fix that too ;)

Will do.

> Is the absences of them enough to indicate this mode?  I.e. are there other
> modes
> with no specified excitation mode or custom rtd table?
> 
> I'm trying to work out if we can map this to the existing binding for
> custom rtd just be adding more constraints + making existing ones more
> specific.
> 
> I don't mind if we can't and have to add a new child node definition but
> I'm not yet sure that's the case.

You're right that the absence of both properties could imply sub-ohm mode, 
so I think we could drop the boolean. But the issue with reusing rtd@ is that 
adi,custom-rtd is currently required for sensor-type 18, and several 
RTD-specific properties (adi,number-of-wires, adi,rtd-curve,
adi,rsense-share) have no meaning for copper trace and would need to be 
forbidden (they could also be ignored in the driver). In my opinion, separate 
nodes for both copper trace and leak detectors would make sense, but I'm 
happy to go whichever way you prefer.

> I'd avoid describing things as xx only as that tends to become wrong fast!
> Better to put that as a conditional only (as you have below)
> Maybe here you can say, (some parts only) or something like that.

Will switch to "some parts only". 

Thanks,
Liviu

Re: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2467 bytes --]

On Wed, May 06, 2026 at 01:06:35PM +0000, Stan, Liviu wrote:
> Thank you for the comments, and I apologize for the delayed answer. 
> 
> On Mon, Apr 27, 2026, Conor Dooley wrote:
> > I have to wonder if this is the right approach, if it's the same device
> > just with a different label and advertised purpose.
> 
> Could you expand on this? Are you suggesting the copper-trace@ and
> leak-detector@ node types aren't needed, or something about the overall
> approach of adding ADT7604 to the existing ltc2983 binding? I want to
> make sure I understand the concern.

Yeah, if it is the same device, just with different uses for the same
pins, I was wondering whether the correct approach is to reuse the
existing child nodes, with some way of indicating what they are
measuring (e.g. use the compatible to decide). I think Jonathan
expressed a similar sentiment.

> 
> > Pedantry perhaps, but isn't this an "ohmmeter"?
> 
> For the naming I followed the convention used in this binding: rtd@, 
> thermocouple@, diode@ are named after what they are, not what
> they measure. copper-trace@ names the component being sensed. Jonathan

An "ohmmeter" is what it is, not what it measures. What it measures
would be "ohms".

> has a related question below about whether this should just reuse rtd@
> with type 18, so the name might change depending on where that lands.
> 
> > I want to look into this property, where is the datasheet?
> 
> The ADT7604 datasheet is not yet publicly available. Will add the URL
> once it is.

Right. It'll be hard to provide a full review without being able to look
at the datasheet.

>  
> > Are these ever linked in a different way?
> > Ditto here and in the rtd node. Are these fixed linkages or actually
> > dynamic?

Please don't trim this aggressively, you've removed the context for what
this is about, and it was 2 weeks ago so there's no way I remember what
it was.

> The linkage is board-dependent. Any rsense on channels 2-20 can be
> paired with any sensor channel in that range. The example uses two
> separate sense resistors because the copper trace and leak detector
> channels need different values (100Ω vs 10kΩ, these are the recommended
> values in the datasheet).

This question of mine is fairly moot anyway, given the existing child
nodes all have have it, especially if there ends up being reuse of them,
so you can ignore this comment.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[Stan, Liviu]

On Wed, May 06, 2026, Conor Dooley wrote:
> > > I have to wonder if this is the right approach, if it's the same device
> > > just with a different label and advertised purpose.
> >
> > Could you expand on this? Are you suggesting the copper-trace@ and
> > leak-detector@ node types aren't needed, or something about the overall
> > approach of adding ADT7604 to the existing ltc2983 binding? I want to
> > make sure I understand the concern.
> 
> Yeah, if it is the same device, just with different uses for the same
> pins, I was wondering whether the correct approach is to reuse the
> existing child nodes, with some way of indicating what they are
> measuring (e.g. use the compatible to decide). I think Jonathan
> expressed a similar sentiment.

Yes, as I replied to Jonathan, I think the only bigger issue would be
the adi,custom-rtd and adi,custom-thermistor being required for sensor 
type 18 and 27 respectively, but I am happy to go whichever way you prefer.

> > > Pedantry perhaps, but isn't this an "ohmmeter"?
> >
> > For the naming I followed the convention used in this binding: rtd@,
> > thermocouple@, diode@ are named after what they are, not what
> > they measure. copper-trace@ names the component being sensed.
> Jonathan
> 
> An "ohmmeter" is what it is, not what it measures. What it measures
> would be "ohms".

You're right, my mistake. I used the sensor type names in the datasheet. But if
we end up reusing rtd@ and thermistor@, the naming question resolves 
itself anyway.

> > > Are these ever linked in a different way?
> > > Ditto here and in the rtd node. Are these fixed linkages or actually
> > > dynamic?
> 
> Please don't trim this aggressively, you've removed the context for what
> this is about, and it was 2 weeks ago so there's no way I remember what
> it was.

I'm sorry, I'll keep more context in future replies.

> > The linkage is board-dependent. Any rsense on channels 2-20 can be
> > paired with any sensor channel in that range. The example uses two
> > separate sense resistors because the copper trace and leak detector
> > channels need different values (100Ω vs 10kΩ, these are the recommended
> > values in the datasheet).
> 
> This question of mine is fairly moot anyway, given the existing child
> nodes all have have it, especially if there ends up being reuse of them,
> so you can ignore this comment.

Noted, thank you.

Thanks,
Liviu

Re: [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[Jonathan Cameron]

On Wed, 6 May 2026 14:52:04 +0000
"Stan, Liviu" <Liviu.Stan@analog.com> wrote:

> Thanks for the comments, here are my answers:
> 
> On 28 Apr 2026, Jonathan Cameron wrote:
> > I don't know much about these temp sensors, but how is this different
> > in practice from a 2-wire RTD?  Obviously one is copper and the other
> > probably much more precise platinum but does that matter to us?  
> 
> The main practical differences are:
> 
> - The primary output is IIO_RESISTANCE, read from the resistance result
>    bank (0x0060-0x00AF). This bank is marked as reserved for the other 
>    devices

That bit we can bury in the driver.

> - Sensor configuration bits 21:18 are hardcoded to 0b1001 for all
>    copper trace configurations. For the sub-ohm variant, bits 17:0 are 
>    also zeroed; a >1Ω trace will have the excitation current and an 
>    optional custom table in those bits. For the existing custom RTD and
>    thermistor types, the custom table is required by the binding. For
>    copper trace, it is optional (and forbidden for the sub-ohm variant).
>    And for leak detector as well it is optional.

So working around this would require some constraints in the binding
triggered off the compatible - but doable I think.

> - When a custom table is present, a second IIO_TEMP channel also
>    appears, reading from the temperature bank. Same dual-output
>    behavior for leak detector.

This feels like a driver detail rather than a binding one.

> 
> That said, the hardware uses the same custom RTD mode (sensor
> type 18) internally.
> 
> > I'd go with "LTC2983 and similar" for the title now as it's
> > to long. Leave the description to list amount more info.
> >
> > Alphabetical order and it might be worth thinking about switching this
> > to a bulleted list with one device per line as it'll make adding new ones
> > neater. (obviously they are already not in numeric order, so fix that too ;)  
> 
> Will do.
> 
> > Is the absences of them enough to indicate this mode?  I.e. are there other
> > modes
> > with no specified excitation mode or custom rtd table?
> > 
> > I'm trying to work out if we can map this to the existing binding for
> > custom rtd just be adding more constraints + making existing ones more
> > specific.
> > 
> > I don't mind if we can't and have to add a new child node definition but
> > I'm not yet sure that's the case.  
> 
> You're right that the absence of both properties could imply sub-ohm mode, 
> so I think we could drop the boolean. But the issue with reusing rtd@ is that 
> adi,custom-rtd is currently required for sensor-type 18, and several 
> RTD-specific properties (adi,number-of-wires, adi,rtd-curve,
> adi,rsense-share) have no meaning for copper trace and would need to be 
> forbidden (they could also be ignored in the driver). In my opinion, separate 
> nodes for both copper trace and leak detectors would make sense, but I'm 
> happy to go whichever way you prefer.

Ok. Sounds like we could do either but the different node type is cleaner.
So fair enough - go with that if the DT maintainers are happy with it.
Just make sure to lay out some of this reasoning in the commit message.
> 
> > I'd avoid describing things as xx only as that tends to become wrong fast!
> > Better to put that as a conditional only (as you have below)
> > Maybe here you can say, (some parts only) or something like that.  
> 
> Will switch to "some parts only". 
> 
> Thanks,
> Liviu

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

Thank you for the comments, and I apologize for the late response.

On Mon, Apr 27, 2026 Andy Shevchenko wrote:
...
> >  #define LTC2983_CHAN_START_ADDR(chan) \
> >  			(((chan - 1) * 4) +
> LTC2983_CHAN_ASSIGN_START_REG)
> > -#define LTC2983_CHAN_RES_ADDR(chan) \
> > -			(((chan - 1) * 4) + LTC2983_TEMP_RES_START_REG)
> > +#define LTC2983_CHAN_RES_ADDR(chan, base) \
> > +			((((chan) - 1) * 4) + (base))
> 
> For the sake of consistency I would see (base) also to be in the _START_ADDR()
> macro.

Understood. Will change in v2.

> > +	bool sub_ohm;
> 
> What does this mean? Perhaps rename to is_in_milliohms or something like
> that?

The datasheet describes two cases for the copper trace sensor type: < 1ohm and > 1 
ohm copper traces. The difference between the two is that < 1 ohm copper traces
have bits 17:0 zeroed (excitation current and custom sensor data pointer configuration
bits). For > 1 ohm copper traces an excitation current needs to be specified and the 
custom table bits are optional. "Sub_ohm" reflects the selection of the sub-ohm variant,
not the result units. For me "sub_ohm" or "is_sub_ohm" feels more in relation to the 
datasheet, but if something like "is_in_milliohms" feels more understandable to you I can
change it in v2.

> > +		ret = fwnode_property_read_u32(child, "adi,number-of-
> wires", &n_wires);
> > +		if (!ret) {
> 
> Yeah, this is in the original code. Consider at some point to make it rather
> returning meaningful error codes, id est
>
> 		if (fwnode_property_present(child, "adi,number-of-wires")) {
> 			ret = fwnode_property_read_u32(child, "adi,number-
> of-wires", &n_wires);
> 			if (ret)
> 				return ret; // or with message that we can't
> get property value

Noted, I can add a patch for this in v2.

> > +			if (sensor->chan <
> LTC2983_DIFFERENTIAL_CHAN_MIN)
> > +				return dev_err_ptr_probe(&st->spi->dev, -
> EINVAL,
> 
> Don't you have 'dev' variable to use? If not, maybe makes sense to introduce.

That was already present in the original code, but I can have a patch where I 
clean up all occurrences in v2.

> > +							 "Invalid chann:%d for
> RTD\n",
> 
> chann? Perhaps just "chan"?

This, also, was present in the original code, the error messages I introduced 
for leak detector and copper trace follow the same pattern though. Should I
modify it everywhere?

> > +							 sensor->chan);
> >  		}
> 
> ...
> 
> > +		if (st->info->has_copper_trace) {
> > +			if (fwnode_property_present(child, "adi,custom-
> rtd")) {
> > +				rtd->custom =
> __ltc2983_custom_sensor_new(st, child,
> > +
> "adi,custom-rtd",
> > +									  false,
> 2048,
> > +
> false);
> > +				if (IS_ERR(rtd->custom))
> > +					return ERR_CAST(rtd->custom);
> > +			}
> > +		} else {
> > +			rtd->custom = __ltc2983_custom_sensor_new(st,
> child,
> > +								  "adi,custom-
> rtd",
> > +								  false, 2048,
> false);
> > +			if (IS_ERR(rtd->custom))
> > +				return ERR_CAST(rtd->custom);
> > +		}
> 
> Seeing so many indentation noise, I think this patch starves for some
> preparatory ones that make helper(s) out of the existing rather long functions
> and then in a new code it will much easier to follow what gets changed and
> how.
> 
> ...
> 
> Due to above I stopped here, because patch seems unreviewable to me. If
> others
> are motivated more than me ans see this change nice in terms of readability,
> I won't object. Personally I think it must be refactored (a lot!) before actually
> adding a support of a new HW.

I understand, I will restructure in v2.

Thanks,
Liviu

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

Thank you for the comments, and I apologize for the late reply.

On Mon, Apr 28, 2026,  Nuno Sá wrote:
...
> > Both sensor types expose an IIO_RESISTANCE channel reading from
> > the resistance result register bank (0x060-0x00AF), added to
> > the regmap readable ranges. Scales are 1/1,024,000 for copper
> > trace (result in mOhm) and 1/1024 for leak detector (result
> > in Ohm).
> 
> But for userspace we report both in Ohm? That's the ABI AFAICT. In DT,
> you also mention IIO_TEMP is used:
> "IIO_TEMP reports coverage percentage"
> 
> Can you expand more on what the above means? Are we reporting milli
> degrees celcius to userspace?

Yes, both IIO_RESISTANCE channels report in Ω. The commit message was 
misleading, it described the register's native units (mΩ for copper trace,
Ω for leak detector), not the userspace output. The scales are chosen to 
cancel those units and give Ω in both cases.

As for the IIO_TEMP question, the chip's custom sensor table stores 
temperature in Kelvin (same as the LTC2984 custom RTD table). For the 
leak detector, coverage data is encoded as (P + 273.15) K, so when the 
chip converts Kelvin to Celsius on output, after the driver applies the 
1000/1024 scale, the IIO output is P * 1000 millidegrees C - 0% reads
as ~0 millidegrees, 100% reads as ~100000 millidegrees. But yes, the 
actual useable quantity is coverage percentage, not temperature. Is there
a more suitable existing IIO channel type for coverage percentage?

> I could not find the datasheet so I guess it's not yet public?

Correct, it is not public yet. Will upload the URL once it is.

...

> >  struct ltc2983_data {
> > @@ -272,6 +275,7 @@ struct ltc2983_rtd {
> >  	u32 r_sense_chan;
> >  	u32 excitation_current;
> >  	u32 rtd_curve;
> > +	bool sub_ohm;
> >  };
> >
> >  struct ltc2983_thermistor {
> > @@ -575,6 +579,10 @@ static int ltc2983_rtd_assign_chan(struct
> ltc2983_data *st,
> >  		if (ret)
> >  			return ret;
> >  	}
> > +
> > +	if (rtd->sub_ohm)
> > +		chan_val &= ~GENMASK(17, 0);
> > +
> >  	return __ltc2983_chan_assign_common(st, sensor, chan_val);
> >  }
> 
> I'm not sure if we shouldn't just treat the new types as new sensors
> instead of trying to push them in the existing one. I agree with Andy,
> the patch does not look great with respect to if() else() and going to
> deep in indentation.
> 
> >
> > @@ -758,83 +766,113 @@ ltc2983_rtd_new(const struct fwnode_handle
> *child, struct ltc2983_data *st,
> >  		return dev_err_ptr_probe(dev, ret,
> >  					 "Property reg must be given\n");
> >
> > -	ret = fwnode_property_read_u32(child, "adi,number-of-wires",
> &n_wires);
> > -	if (!ret) {
> > -		switch (n_wires) {
> > -		case 2:
> > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(0);
> > -			break;
> > -		case 3:
> > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(1);
> > -			break;
> > -		case 4:
> > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(2);
> > -			break;
> > -		case 5:
> > -			/* 4 wires, Kelvin Rsense */
> > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(3);
> > -			break;
> > -		default:
> > +	/* ADT7604 requires hardcoding sensor configuration bits to 0b1001
> */
> > +	if (st->info->has_copper_trace &&
> > +	    sensor->type == LTC2983_SENSOR_RTD_CUSTOM) {
> > +		rtd->sensor_config = 0x9;
> > +		if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
> 
> Like the above, we have the following kind of condition all over the
> place. In DT we can just have a different type for these and map it to
> real value when creating the sensor.

I understand, I will introduce new adi,sensor-type enum values for 
copper trace and leak detector. The driver will map these to the 
hardware register values (18 and 27) and handle them in dedicated 
switch cases with dedicated functions (ltc2983_copper_trace_new() 
and ltc2983_leak_detector_new()), removing the has_copper_trace guards
from ltc2983_rtd_new() and ltc2983_thermistor_new() entirely. One 
tradeoff is that the adi,sensor-type values for the new sensors will 
now not coincide with the hardware register values in the ADT7604 
datasheet.

...

I will address the rest of the comments in v2 as part of the restructuring.
Thank you very much.

Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Andy Shevchenko]

On Thu, May 07, 2026 at 03:31:14PM +0000, Stan, Liviu wrote:
> On Mon, Apr 27, 2026 Andy Shevchenko wrote:

...

> > > +	bool sub_ohm;
> > 
> > What does this mean? Perhaps rename to is_in_milliohms or something like
> > that?
> 
> The datasheet describes two cases for the copper trace sensor type: < 1ohm and > 1 
> ohm copper traces. The difference between the two is that < 1 ohm copper traces
> have bits 17:0 zeroed (excitation current and custom sensor data pointer configuration
> bits). For > 1 ohm copper traces an excitation current needs to be specified and the 
> custom table bits are optional. "Sub_ohm" reflects the selection of the sub-ohm variant,
> not the result units. For me "sub_ohm" or "is_sub_ohm" feels more in relation to the 
> datasheet, but if something like "is_in_milliohms" feels more understandable to you I can
> change it in v2.

So, this needs a good comment. And is_sub_ohm is definitely better.

...

> > chann? Perhaps just "chan"?
> 
> This, also, was present in the original code, the error messages I introduced 
> for leak detector and copper trace follow the same pattern though. Should I
> modify it everywhere?

Maybe the idea was to represent 'chan<N>'? Either way, the current message is
unclear, please add a preparatory patch that fixes that ambiguity and use the
same pattern in your new code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Nuno Sá]

On Thu, May 07, 2026 at 05:25:58PM +0000, Stan, Liviu wrote:
> Thank you for the comments, and I apologize for the late reply.
> 
> On Mon, Apr 28, 2026,  Nuno Sá wrote:
> ...
> > > Both sensor types expose an IIO_RESISTANCE channel reading from
> > > the resistance result register bank (0x060-0x00AF), added to
> > > the regmap readable ranges. Scales are 1/1,024,000 for copper
> > > trace (result in mOhm) and 1/1024 for leak detector (result
> > > in Ohm).
> > 
> > But for userspace we report both in Ohm? That's the ABI AFAICT. In DT,
> > you also mention IIO_TEMP is used:
> > "IIO_TEMP reports coverage percentage"
> > 
> > Can you expand more on what the above means? Are we reporting milli
> > degrees celcius to userspace?
> 
> Yes, both IIO_RESISTANCE channels report in Ω. The commit message was 
> misleading, it described the register's native units (mΩ for copper trace,
> Ω for leak detector), not the userspace output. The scales are chosen to 
> cancel those units and give Ω in both cases.
> 

ack

> As for the IIO_TEMP question, the chip's custom sensor table stores 
> temperature in Kelvin (same as the LTC2984 custom RTD table). For the 
> leak detector, coverage data is encoded as (P + 273.15) K, so when the 
> chip converts Kelvin to Celsius on output, after the driver applies the 
> 1000/1024 scale, the IIO output is P * 1000 millidegrees C - 0% reads
> as ~0 millidegrees, 100% reads as ~100000 millidegrees. But yes, the 
> actual useable quantity is coverage percentage, not temperature. Is there
> a more suitable existing IIO channel type for coverage percentage?
> 

Will defer this to Jonathan but if we can have a real of the coverage
given the temperature, I guess this is ok. Given that I think we don't have
a better channel (unless we add one?) for this. Or just extended_info...


> > I could not find the datasheet so I guess it's not yet public?
> 
> Correct, it is not public yet. Will upload the URL once it is.
> 
> ...
> 
> > >  struct ltc2983_data {
> > > @@ -272,6 +275,7 @@ struct ltc2983_rtd {
> > >  	u32 r_sense_chan;
> > >  	u32 excitation_current;
> > >  	u32 rtd_curve;
> > > +	bool sub_ohm;
> > >  };
> > >
> > >  struct ltc2983_thermistor {
> > > @@ -575,6 +579,10 @@ static int ltc2983_rtd_assign_chan(struct
> > ltc2983_data *st,
> > >  		if (ret)
> > >  			return ret;
> > >  	}
> > > +
> > > +	if (rtd->sub_ohm)
> > > +		chan_val &= ~GENMASK(17, 0);
> > > +
> > >  	return __ltc2983_chan_assign_common(st, sensor, chan_val);
> > >  }
> > 
> > I'm not sure if we shouldn't just treat the new types as new sensors
> > instead of trying to push them in the existing one. I agree with Andy,
> > the patch does not look great with respect to if() else() and going to
> > deep in indentation.
> > 
> > >
> > > @@ -758,83 +766,113 @@ ltc2983_rtd_new(const struct fwnode_handle
> > *child, struct ltc2983_data *st,
> > >  		return dev_err_ptr_probe(dev, ret,
> > >  					 "Property reg must be given\n");
> > >
> > > -	ret = fwnode_property_read_u32(child, "adi,number-of-wires",
> > &n_wires);
> > > -	if (!ret) {
> > > -		switch (n_wires) {
> > > -		case 2:
> > > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(0);
> > > -			break;
> > > -		case 3:
> > > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(1);
> > > -			break;
> > > -		case 4:
> > > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(2);
> > > -			break;
> > > -		case 5:
> > > -			/* 4 wires, Kelvin Rsense */
> > > -			rtd->sensor_config = LTC2983_RTD_N_WIRES(3);
> > > -			break;
> > > -		default:
> > > +	/* ADT7604 requires hardcoding sensor configuration bits to 0b1001
> > */
> > > +	if (st->info->has_copper_trace &&
> > > +	    sensor->type == LTC2983_SENSOR_RTD_CUSTOM) {
> > > +		rtd->sensor_config = 0x9;
> > > +		if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN)
> > 
> > Like the above, we have the following kind of condition all over the
> > place. In DT we can just have a different type for these and map it to
> > real value when creating the sensor.
> 
> I understand, I will introduce new adi,sensor-type enum values for 
> copper trace and leak detector. The driver will map these to the 
> hardware register values (18 and 27) and handle them in dedicated 
> switch cases with dedicated functions (ltc2983_copper_trace_new() 
> and ltc2983_leak_detector_new()), removing the has_copper_trace guards
> from ltc2983_rtd_new() and ltc2983_thermistor_new() entirely. One 
> tradeoff is that the adi,sensor-type values for the new sensors will 
> now not coincide with the hardware register values in the ADT7604 
> datasheet.

Yes, I was aware of that but I think (I could be wrong) that the
simplifications it will bring will justify for the small "fixup" we'll
need to do on the driver.

- Nuno Sá
> 
> ...
> 
> I will address the rest of the comments in v2 as part of the restructuring.
> Thank you very much.
> 
> Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Jonathan Cameron]

On Fri, 8 May 2026 10:19:15 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, May 07, 2026 at 05:25:58PM +0000, Stan, Liviu wrote:
> > Thank you for the comments, and I apologize for the late reply.
> > 
> > On Mon, Apr 28, 2026,  Nuno Sá wrote:
> > ...  
> > > > Both sensor types expose an IIO_RESISTANCE channel reading from
> > > > the resistance result register bank (0x060-0x00AF), added to
> > > > the regmap readable ranges. Scales are 1/1,024,000 for copper
> > > > trace (result in mOhm) and 1/1024 for leak detector (result
> > > > in Ohm).  
> > > 
> > > But for userspace we report both in Ohm? That's the ABI AFAICT. In DT,
> > > you also mention IIO_TEMP is used:
> > > "IIO_TEMP reports coverage percentage"
> > > 
> > > Can you expand more on what the above means? Are we reporting milli
> > > degrees celcius to userspace?  
> > 
> > Yes, both IIO_RESISTANCE channels report in Ω. The commit message was 
> > misleading, it described the register's native units (mΩ for copper trace,
> > Ω for leak detector), not the userspace output. The scales are chosen to 
> > cancel those units and give Ω in both cases.
> >   
> 
> ack
> 
> > As for the IIO_TEMP question, the chip's custom sensor table stores 
> > temperature in Kelvin (same as the LTC2984 custom RTD table). For the 
> > leak detector, coverage data is encoded as (P + 273.15) K, so when the 
> > chip converts Kelvin to Celsius on output, after the driver applies the 
> > 1000/1024 scale, the IIO output is P * 1000 millidegrees C - 0% reads
> > as ~0 millidegrees, 100% reads as ~100000 millidegrees. But yes, the 
> > actual useable quantity is coverage percentage, not temperature. Is there
> > a more suitable existing IIO channel type for coverage percentage?
> >   
> 
> Will defer this to Jonathan but if we can have a real of the coverage
> given the temperature, I guess this is ok. Given that I think we don't have
> a better channel (unless we add one?) for this. Or just extended_info...
> 

I have no idea what coverage percentage means in this case.
Can you provide some more details or a reference?  Google isn't giving me
anything useful.

> 
> > > I could not find the datasheet so I guess it's not yet public?  
> > 
> > Correct, it is not public yet. Will upload the URL once it is.
> > 

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

On Fri, 8 May 2026 Jonathan Cameron wrote:
> > > As for the IIO_TEMP question, the chip's custom sensor table stores
> > > temperature in Kelvin (same as the LTC2984 custom RTD table). For the
> > > leak detector, coverage data is encoded as (P + 273.15) K, so when the
> > > chip converts Kelvin to Celsius on output, after the driver applies the
> > > 1000/1024 scale, the IIO output is P * 1000 millidegrees C - 0% reads
> > > as ~0 millidegrees, 100% reads as ~100000 millidegrees. But yes, the
> > > actual useable quantity is coverage percentage, not temperature. Is there
> > > a more suitable existing IIO channel type for coverage percentage?
> > >
> >
> > Will defer this to Jonathan but if we can have a real of the coverage
> > given the temperature, I guess this is ok. Given that I think we don't have
> > a better channel (unless we add one?) for this. Or just extended_info...
> >
> 
> I have no idea what coverage percentage means in this case.
> Can you provide some more details or a reference?  Google isn't giving me
> anything useful.

A leak detector is a sensor like the raindrop detection modules, whose 
resistance decreases as more of its surface is contacted by liquid. In this case, 
for example, if half of the sensor's surface is in contact with a liquid, the 
ADT7604 should, with an appropiate user-provided resistance-vs-coverage 
table, report 50% coverage in the IIO channel.

Thanks,
Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Nuno Sá]

On Fri, May 08, 2026 at 12:46:50PM +0000, Stan, Liviu wrote:
> On Fri, 8 May 2026 Jonathan Cameron wrote:
> > > > As for the IIO_TEMP question, the chip's custom sensor table stores
> > > > temperature in Kelvin (same as the LTC2984 custom RTD table). For the
> > > > leak detector, coverage data is encoded as (P + 273.15) K, so when the
> > > > chip converts Kelvin to Celsius on output, after the driver applies the
> > > > 1000/1024 scale, the IIO output is P * 1000 millidegrees C - 0% reads
> > > > as ~0 millidegrees, 100% reads as ~100000 millidegrees. But yes, the
> > > > actual useable quantity is coverage percentage, not temperature. Is there
> > > > a more suitable existing IIO channel type for coverage percentage?
> > > >
> > >
> > > Will defer this to Jonathan but if we can have a real of the coverage
> > > given the temperature, I guess this is ok. Given that I think we don't have
> > > a better channel (unless we add one?) for this. Or just extended_info...
> > >
> > 
> > I have no idea what coverage percentage means in this case.
> > Can you provide some more details or a reference?  Google isn't giving me
> > anything useful.
> 
> A leak detector is a sensor like the raindrop detection modules, whose 
> resistance decreases as more of its surface is contacted by liquid. In this case, 
> for example, if half of the sensor's surface is in contact with a liquid, the 
> ADT7604 should, with an appropiate user-provided resistance-vs-coverage 
> table, report 50% coverage in the IIO channel.

And, AFAIU, depending on the coverage the resistance changes and that's
how we get into the indirect temperature variations?

- Nuno Sá

> 
> Thanks,
> Liviu

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

On Fri, May 08, 2026 Nuno Sá wrote:
> > On Fri, 8 May 2026 Jonathan Cameron wrote:
> > > > Will defer this to Jonathan but if we can have a real of the coverage
> > > > given the temperature, I guess this is ok. Given that I think we don't have
> > > > a better channel (unless we add one?) for this. Or just extended_info...
> > > >
> > >
> > > I have no idea what coverage percentage means in this case.
> > > Can you provide some more details or a reference?  Google isn't giving me
> > > anything useful.
> >
> > A leak detector is a sensor like the raindrop detection modules, whose
> > resistance decreases as more of its surface is contacted by liquid. In this case,
> > for example, if half of the sensor's surface is in contact with a liquid, the
> > ADT7604 should, with an appropiate user-provided resistance-vs-coverage
> > table, report 50% coverage in the IIO channel.
> 
> And, AFAIU, depending on the coverage the resistance changes and that's
> how we get into the indirect temperature variations?

If I understand your comment correctly, you're implying that there are indirect
temperature variations caused by resistance changes, and that's what the chip is
measuring? The chip does not measure any actual temperature for this sensor
type, the value in the temperature result register bank is solely a result of the 
chip's interpolation of the user-defined custom table.

Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Nuno Sá]

On Fri, May 08, 2026 at 02:48:39PM +0000, Stan, Liviu wrote:
> On Fri, May 08, 2026 Nuno Sá wrote:
> > > On Fri, 8 May 2026 Jonathan Cameron wrote:
> > > > > Will defer this to Jonathan but if we can have a real of the coverage
> > > > > given the temperature, I guess this is ok. Given that I think we don't have
> > > > > a better channel (unless we add one?) for this. Or just extended_info...
> > > > >
> > > >
> > > > I have no idea what coverage percentage means in this case.
> > > > Can you provide some more details or a reference?  Google isn't giving me
> > > > anything useful.
> > >
> > > A leak detector is a sensor like the raindrop detection modules, whose
> > > resistance decreases as more of its surface is contacted by liquid. In this case,
> > > for example, if half of the sensor's surface is in contact with a liquid, the
> > > ADT7604 should, with an appropiate user-provided resistance-vs-coverage
> > > table, report 50% coverage in the IIO channel.
> > 
> > And, AFAIU, depending on the coverage the resistance changes and that's
> > how we get into the indirect temperature variations?
> 
> If I understand your comment correctly, you're implying that there are indirect
> temperature variations caused by resistance changes, and that's what the chip is
> measuring? The chip does not measure any actual temperature for this sensor
> type, the value in the temperature result register bank is solely a result of the 
> chip's interpolation of the user-defined custom table.
> 

I meant that's what the table is trying to accomplish? That's why I kind
of put it as "indirect". My understanding was the chip does not measure
any temperature as well.

- Nuno Sá

> Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Jonathan Cameron]

On Fri, 8 May 2026 17:13:14 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Fri, May 08, 2026 at 02:48:39PM +0000, Stan, Liviu wrote:
> > On Fri, May 08, 2026 Nuno Sá wrote:  
> > > > On Fri, 8 May 2026 Jonathan Cameron wrote:  
> > > > > > Will defer this to Jonathan but if we can have a real of the coverage
> > > > > > given the temperature, I guess this is ok. Given that I think we don't have
> > > > > > a better channel (unless we add one?) for this. Or just extended_info...
> > > > > >  
> > > > >
> > > > > I have no idea what coverage percentage means in this case.
> > > > > Can you provide some more details or a reference?  Google isn't giving me
> > > > > anything useful.  
> > > >
> > > > A leak detector is a sensor like the raindrop detection modules, whose
> > > > resistance decreases as more of its surface is contacted by liquid. In this case,
> > > > for example, if half of the sensor's surface is in contact with a liquid, the
> > > > ADT7604 should, with an appropiate user-provided resistance-vs-coverage
> > > > table, report 50% coverage in the IIO channel.  
> > > 
> > > And, AFAIU, depending on the coverage the resistance changes and that's
> > > how we get into the indirect temperature variations?  
> > 
> > If I understand your comment correctly, you're implying that there are indirect
> > temperature variations caused by resistance changes, and that's what the chip is
> > measuring? The chip does not measure any actual temperature for this sensor
> > type, the value in the temperature result register bank is solely a result of the 
> > chip's interpolation of the user-defined custom table.
> >   
> 
> I meant that's what the table is trying to accomplish? That's why I kind
> of put it as "indirect". My understanding was the chip does not measure
> any temperature as well.

Ok. So what are our options here?  Present it as simple resistance and leave
userspace to figure it out or add a new channel type? To me feels like new
channel type makes sense.

Jonathan


> 
> - Nuno Sá
> 
> > Liviu  

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

On Fri, 8 May 2026, Nuno Sá wrote:
> > > > A leak detector is a sensor like the raindrop detection modules, whose
> > > > resistance decreases as more of its surface is contacted by liquid. In this
> > > > case, for example, if half of the sensor's surface is in contact 
> > > > with a liquid, the ADT7604 should, with an appropiate user-provided 
> > > > resistance-vs-coverage table, report 50% coverage in the IIO channel.
> > >
> > > And, AFAIU, depending on the coverage the resistance changes and that's
> > > how we get into the indirect temperature variations?
> >
> > If I understand your comment correctly, you're implying that there are
> > indirect temperature variations caused by resistance changes, and 
> > that's what the chip is measuring? The chip does not measure any actual
> > temperature for this sensor type, the value in the temperature result 
> > register bank is solely a result of the chip's interpolation of the 
> > user-defined custom table.
>
> I meant that's what the table is trying to accomplish? That's why I kind
> of put it as "indirect". My understanding was the chip does not measure
> any temperature as well.

Oh, okay, I understand now. Yes, this is what the table is trying to accomplish.

On Sat, 9 May 2026, Jonathan Cameron wrote:
> > I meant that's what the table is trying to accomplish? That's why I kind
> > of put it as "indirect". My understanding was the chip does not measure
> > any temperature as well.
> >
> Ok. So what are our options here?  Present it as simple resistance and leave
> userspace to figure it out or add a new channel type? To me feels like new
> channel type makes sense.

The current approach presents it as IIO_TEMP since the chip outputs coverage
(using the custom table interpolation) via the temperature result bank, not the
resistance bank, but I agree a new channel type makes sense. Should I
create a specific type like IIO_COVERAGE_PERCENT or would a general
IIO_PERCENTAGE be better?

Thanks,
Liviu

Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Jonathan Cameron]

On Mon, 11 May 2026 07:52:17 +0000
"Stan, Liviu" <Liviu.Stan@analog.com> wrote:

> On Fri, 8 May 2026, Nuno Sá wrote:
> > > > > A leak detector is a sensor like the raindrop detection modules, whose
> > > > > resistance decreases as more of its surface is contacted by liquid. In this
> > > > > case, for example, if half of the sensor's surface is in contact 
> > > > > with a liquid, the ADT7604 should, with an appropiate user-provided 
> > > > > resistance-vs-coverage table, report 50% coverage in the IIO channel.  
> > > >
> > > > And, AFAIU, depending on the coverage the resistance changes and that's
> > > > how we get into the indirect temperature variations?  
> > >
> > > If I understand your comment correctly, you're implying that there are
> > > indirect temperature variations caused by resistance changes, and 
> > > that's what the chip is measuring? The chip does not measure any actual
> > > temperature for this sensor type, the value in the temperature result 
> > > register bank is solely a result of the chip's interpolation of the 
> > > user-defined custom table.  
> >
> > I meant that's what the table is trying to accomplish? That's why I kind
> > of put it as "indirect". My understanding was the chip does not measure
> > any temperature as well.  
> 
> Oh, okay, I understand now. Yes, this is what the table is trying to accomplish.
> 
> On Sat, 9 May 2026, Jonathan Cameron wrote:
> > > I meant that's what the table is trying to accomplish? That's why I kind
> > > of put it as "indirect". My understanding was the chip does not measure
> > > any temperature as well.
> > >  
> > Ok. So what are our options here?  Present it as simple resistance and leave
> > userspace to figure it out or add a new channel type? To me feels like new
> > channel type makes sense.  
> 
> The current approach presents it as IIO_TEMP since the chip outputs coverage
> (using the custom table interpolation) via the temperature result bank, not the
> resistance bank, but I agree a new channel type makes sense. Should I
> create a specific type like IIO_COVERAGE_PERCENT or would a general
> IIO_PERCENTAGE be better?

For ABI purposes we don't care where it comes from.

We already have some 'ratio' type measurements like concentration which are
percentages and similar to those I think we need some indication of 'what'
is being measured given it's unit free.  Hence IIO_COVERAGE_PERCENT seems
the better choice to me.

Jonathan


> 
> Thanks,
> Liviu

RE: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604

[Stan, Liviu]

On Mon, 11 May 2026 Jonathan Cameron wrote:
> > > Ok. So what are our options here?  Present it as simple resistance and leave
> > > userspace to figure it out or add a new channel type? To me feels like new
> > > channel type makes sense.
> >
> > The current approach presents it as IIO_TEMP since the chip outputs coverage
> > (using the custom table interpolation) via the temperature result bank, not
> > the resistance bank, but I agree a new channel type makes sense. Should I
> > create a specific type like IIO_COVERAGE_PERCENT or would a general
> > IIO_PERCENTAGE be better?
> 
> For ABI purposes we don't care where it comes from.
> 
> We already have some 'ratio' type measurements like concentration which are
> percentages and similar to those I think we need some indication of 'what'
> is being measured given it's unit free.  Hence IIO_COVERAGE_PERCENT seems
> the better choice to me.

Understood. Will do that in v2.

Thank you!

Liviu