From: "Nuno Sá" <noname.nuno@gmail.com>
To: "Stan, Liviu" <Liviu.Stan@analog.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
"Hennerich, Michael" <Michael.Hennerich@analog.com>,
"Sa, Nuno" <Nuno.Sa@analog.com>,
Jonathan Cameron <jic23@kernel.org>,
David Lechner <dlechner@baylibre.com>,
Andy Shevchenko <andy@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604
Date: Fri, 8 May 2026 10:19:15 +0100 [thread overview]
Message-ID: <af2no3bJA9MSjXvV@nsa> (raw)
In-Reply-To: <SA5PR03MB83772D8F6A3CC39094DE5241F63C2@SA5PR03MB8377.namprd03.prod.outlook.com>
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
next prev parent reply other threads:[~2026-05-08 9:18 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-27 13:25 [PATCH 0/2] iio: temperature: ltc2983: Add support for ADT7604 Liviu Stan
2026-04-27 13:25 ` [PATCH 1/2] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983 Liviu Stan
2026-04-27 19:34 ` Conor Dooley
2026-05-06 13:06 ` Stan, Liviu
2026-05-06 17:26 ` Conor Dooley
2026-05-07 8:53 ` Stan, Liviu
2026-04-28 14:58 ` Jonathan Cameron
2026-05-06 14:52 ` Stan, Liviu
2026-05-07 10:35 ` Jonathan Cameron
2026-04-27 13:25 ` [PATCH 2/2] iio: temperature: ltc2983: Add support for ADT7604 Liviu Stan
2026-04-27 18:23 ` Andy Shevchenko
2026-05-07 15:31 ` Stan, Liviu
2026-05-08 7:44 ` Andy Shevchenko
2026-05-12 7:12 ` Stan, Liviu
2026-05-12 7:57 ` Andy Shevchenko
2026-05-12 9:37 ` Stan, Liviu
2026-05-12 16:25 ` Andy Shevchenko
2026-04-28 11:14 ` Nuno Sá
2026-05-07 17:25 ` Stan, Liviu
2026-05-08 9:19 ` Nuno Sá [this message]
2026-05-08 11:14 ` Jonathan Cameron
2026-05-08 12:46 ` Stan, Liviu
2026-05-08 13:44 ` Nuno Sá
2026-05-08 14:48 ` Stan, Liviu
2026-05-08 16:13 ` Nuno Sá
2026-05-09 14:46 ` Jonathan Cameron
2026-05-11 7:52 ` Stan, Liviu
2026-05-11 11:18 ` Jonathan Cameron
2026-05-11 12:02 ` Stan, Liviu
2026-05-12 8:24 ` Nuno Sá
2026-05-12 10:55 ` Jonathan Cameron
2026-05-12 11:06 ` Nuno Sá
2026-05-12 11:55 ` Stan, Liviu
2026-05-12 12:06 ` Nuno Sá
2026-05-12 12:26 ` Stan, Liviu
2026-05-12 15:56 ` Jonathan Cameron
2026-05-13 16:08 ` Stan, Liviu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=af2no3bJA9MSjXvV@nsa \
--to=noname.nuno@gmail.com \
--cc=Liviu.Stan@analog.com \
--cc=Michael.Hennerich@analog.com \
--cc=Nuno.Sa@analog.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.