* Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 [not found] <20260521191828.F20931F000E9@smtp.kernel.org> @ 2026-05-22 13:23 ` Liviu Stan 2026-05-22 14:09 ` David Lechner 0 siblings, 1 reply; 6+ messages in thread From: Liviu Stan @ 2026-05-22 13:23 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Antoniu Miclaus, Francesco Lavra, Liviu Stan, linux-iio, linux-kernel, linux, devicetree On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > > New issues: > - [High] Reading the IIO_RESISTANCE channel bypasses hardware fault detection, causing hardware errors (like broken sensor traces) to be silently reported as valid data. > > Pre-existing issues: > - [High] reinit_completion() is called after initiating the hardware conversion, creating a race condition that can cause spurious read timeouts. > -- > > commit d382a4cbefd41723ab5ddd91fd399d716c8faaec > Author: Liviu Stan <liviu.stan@analog.com> > Subject: iio: temperature: ltc2983: Add support for ADT7604 > > This commit adds support for the ADT7604 temperature sensor by repurposing > custom RTD and thermistor types as copper trace resistance and leak detector > sensors. It introduces two new software sensor types and exposes a new > IIO_RESISTANCE channel reading from a separate result register bank. The > ltc2983_chip_info struct is also extended to use a 64-bit supported_sensors > bitmask for the new types. > > > diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c > > index 326f843f42713..e2ffeee026eef 100644 > > --- a/drivers/iio/temperature/ltc2983.c > > +++ b/drivers/iio/temperature/ltc2983.c > > [ ... ] > > > @@ -1205,7 +1502,8 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child, > > } > > > > static int ltc2983_chan_read(struct ltc2983_data *st, > > - const struct ltc2983_sensor *sensor, int *val) > > + const struct ltc2983_sensor *sensor, > > + u32 base_reg, int *val) > > { > > struct device *dev = &st->spi->dev; > > u32 start_conversion = 0; > > [Severity: High] > This is a pre-existing issue, but can this function experience spurious > read timeouts due to a race condition with the completion? > > Looking at the rest of this function: > > ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion); > if (ret) > return ret; > > reinit_completion(&st->completion); > > If the hardware conversion finishes very quickly after regmap_write(), > could the interrupt fire and call complete() before the CPU executes > reinit_completion()? This would overwrite the completion event, causing the > subsequent wait_for_completion_timeout() to spuriously time out. > > Would it be safer to call reinit_completion() before initiating the hardware > action? This is theoretically valid but in practice, the conversion requires at minimum two 82ms cycles (167ms), so there is no realistic window for the interrupt to race with reinit_completion(). What do you guys think? > > [ ... ] > > > @@ -1236,13 +1534,23 @@ static int ltc2983_chan_read(struct ltc2983_data *st, > > } > > > > /* read the converted data */ > > - ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan), > > + ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan, base_reg), > > &st->temp, sizeof(st->temp)); > > if (ret) > > return ret; > > > > *val = __be32_to_cpu(st->temp); > > > > + if (base_reg = ADT7604_RES_RES_START_REG) { > > + /* > > + * Resistance result register gives a plain unsigned value, > > + * D31 is always 0, no valid bit, no fault bits. Read bits[30:0] > > + * directly — the temperature result format does not apply here. > > + */ > > + *val &= GENMASK(30, 0); > > + return 0; > > [Severity: High] > Will returning early here bypass hardware fault detection for resistance > channels? > > As the comment notes, the resistance result register does not contain fault > bits. However, hardware fault indicators (like Hard Fault, Soft Fault, or > Over Range) are stored in the primary Temperature Result register. > > By returning before reading the Temperature Result register and calling > sensor->fault_handler(), does this code silently ignore sensor faults? This > might be problematic for sub-ohm copper traces, which only expose the > IIO_RESISTANCE channel, leaving users with no mechanism to detect a broken > trace. > First, I don't understand why Sashiko wrote "base_reg = ADT7604_RES_RES_START_REG", the patch has a "==" there. Second, the temperature result register stores fault indicator bits for temperature reads, not resistance. Thanks, Liviu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 2026-05-22 13:23 ` [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 Liviu Stan @ 2026-05-22 14:09 ` David Lechner 0 siblings, 0 replies; 6+ messages in thread From: David Lechner @ 2026-05-22 14:09 UTC (permalink / raw) To: Liviu Stan, Jonathan Cameron, Nuno Sá, Andy Shevchenko, Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Antoniu Miclaus, Francesco Lavra, linux-iio, linux-kernel, linux, devicetree On 5/22/26 8:23 AM, Liviu Stan wrote: > On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote: > >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: >> >> New issues: >> - [High] Reading the IIO_RESISTANCE channel bypasses hardware fault detection, causing hardware errors (like broken sensor traces) to be silently reported as valid data. >> >> Pre-existing issues: >> - [High] reinit_completion() is called after initiating the hardware conversion, creating a race condition that can cause spurious read timeouts. >> -- >> >> commit d382a4cbefd41723ab5ddd91fd399d716c8faaec >> Author: Liviu Stan <liviu.stan@analog.com> >> Subject: iio: temperature: ltc2983: Add support for ADT7604 >> >> This commit adds support for the ADT7604 temperature sensor by repurposing >> custom RTD and thermistor types as copper trace resistance and leak detector >> sensors. It introduces two new software sensor types and exposes a new >> IIO_RESISTANCE channel reading from a separate result register bank. The >> ltc2983_chip_info struct is also extended to use a 64-bit supported_sensors >> bitmask for the new types. >> >>> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c >>> index 326f843f42713..e2ffeee026eef 100644 >>> --- a/drivers/iio/temperature/ltc2983.c >>> +++ b/drivers/iio/temperature/ltc2983.c >> >> [ ... ] >> >>> @@ -1205,7 +1502,8 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child, >>> } >>> >>> static int ltc2983_chan_read(struct ltc2983_data *st, >>> - const struct ltc2983_sensor *sensor, int *val) >>> + const struct ltc2983_sensor *sensor, >>> + u32 base_reg, int *val) >>> { >>> struct device *dev = &st->spi->dev; >>> u32 start_conversion = 0; >> >> [Severity: High] >> This is a pre-existing issue, but can this function experience spurious >> read timeouts due to a race condition with the completion? >> >> Looking at the rest of this function: >> >> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion); >> if (ret) >> return ret; >> >> reinit_completion(&st->completion); >> >> If the hardware conversion finishes very quickly after regmap_write(), >> could the interrupt fire and call complete() before the CPU executes >> reinit_completion()? This would overwrite the completion event, causing the >> subsequent wait_for_completion_timeout() to spuriously time out. >> >> Would it be safer to call reinit_completion() before initiating the hardware >> action? > > This is theoretically valid but in practice, the conversion requires at minimum > two 82ms cycles (167ms), so there is no realistic window for the interrupt to > race with reinit_completion(). What do you guys think? I would still move it before the write so that it looks correct and others can copy/paste the logic on other devices. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 0/8] iio: temperature: ltc2983: Add support for ADT7604
@ 2026-05-21 16:42 Liviu Stan
2026-05-21 16:43 ` [PATCH v3 8/8] " Liviu Stan
0 siblings, 1 reply; 6+ messages in thread
From: Liviu Stan @ 2026-05-21 16:42 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Michael Hennerich, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Antoniu Miclaus,
Francesco Lavra, Liviu Stan, linux-iio, linux-kernel, linux,
devicetree
This series adds support for the ADT7604 multi-sensor temperature
measurement and leak detection system to the existing ltc2983 driver.
The ADT7604 shares the same die as the LTC2984, reusing its register
map and SPI interface. It repurposes the custom RTD sensor type (18)
as a copper trace resistance sensor and the custom thermistor type (27)
as a leak detector, removing thermocouple, diode and direct ADC support.
Patches 1-5 fix pre-existing bugs in the ltc2983 driver: macro
parenthesization and renaming, inconsistent use of the local device
pointer, inconsistent channel wording in log messages, missing
fwnode_property_present() guards for optional properties, and an
n_wires default that silently bypassed the current-rotate validation.
Patch 6 adds IIO_COVERAGE, a new channel type for sensors reporting
fractional surface coverage as a percentage.
Patch 7 updates the device tree bindings: adds adi,adt7604 compatible,
copper-trace@ and leak-detector@ sensor node types with their respective
properties, and an ADT7604 example.
Patch 8 updates the driver: introduces two new software sensor type
values (LTC2983_SENSOR_COPPER_TRACE = 32, LTC2983_SENSOR_LEAK_DETECTOR
= 33) with dedicated structs and parser functions rather than extending
the existing RTD and thermistor paths. The hardware configuration bits
are fully hardcoded for both sensor types, and several RTD/thermistor
DT properties have no meaning for them. A u64 supported_sensors bitmask
in ltc2983_chip_info gates sensor type validation per chip, replacing
the has_temp bool pattern. BIT_ULL() is used for the new type values
at bits 32 and 33 to avoid shifting beyond 32 bits on 32-bit builds.
Tested on EVAL-ADT7604-AZ connected to Raspberry Pi 5 via SPI.
Changes in v3:
Patch 1 - macro parenthesization and rename:
- Dropped the Fixes: tag
- Removed the "base" parameter from ADT7604_RES_RES_ADDR(); the resistance
result memory region is (currently) ADT7604-specific so the macro's
modification belongs in the driver patch
Patch 2 - use local device pointer consistently:
- Dropped the Fixes: tag
- Fixed one remaining dev_dbg() call in __ltc2983_chan_assign_common()
that was still using the raw device pointer instead of the dev local
variable introduced by this patch
Patch 3 - fix inconsistent channel wording in messages:
- Dropped both Fixes: tags
- Removed the "all" from "all occurrences" in the commit message
- Fixed some missed "chan" / "chann" occurrences
Patch 4 - use fwnode_property_present() for optional properties:
- Dropped the Fixes: tag
- Fixed other occurrences brought up by sashiko
Patch 5 - Fix n_wires default bypassing rotation check:
- New patch
Patch 6 - IIO_COVERAGE channel type:
- Renamed the sysfs attribute from in_coveragepercentX_raw to
in_coverageX_raw
- Added a _scale ABI documentation entry
- Corrected KernelVersion in the ABI documentation from 6.15 to 7.2
- Added IIO_COVERAGE to the event_is_known() switch in
tools/iio/iio_event_monitor.c
Patch 7 - DT bindings:
- Changed the custom leak detector table encoding, users now specify
plain coverage percentage values (0-100) in the device tree and the
driver applies the +273.15 C offset internally before writing the
hardware table
- Changed custom-rtd to custom-copper-trace to better represent the
copper trace custom table, and made it required for > 1 ohm variants
- Made custom-leak-detector required for leak detector sensors
- Updated commit message to reflect the changes
- Updated leak detector node description
- Updated adi,custom-leak-detector description
- Modified the example leak detector custom table to match the datasheet
Patch 8 - driver:
- Relocated the "base" parameter addition from patch 1 to this one since
the resistance result memory region is (currently) ADT7604-specific
- Updated the commit message to mention the base and base_reg parameters
added to the result-bank read helpers
- Stacked the IIO_RESISTANCE and IIO_COVERAGE case labels in
ltc2983_read_raw()
- Added LTC2983_SENSOR_NUM sentinel at the end of the sensor-type enum
- Changed the sensor-type bounds check from > LTC2983_SENSOR_LEAK_DETECTOR
to >= LTC2983_SENSOR_NUM
- Reordered the ltm2985_chip_info_data struct initializer so has_eeprom
doesn't look like it was added to the ltm2985 in the diff
- Changed the custom leak detector table encoding, users now specify
plain coverage percentage values (0-100) in the device tree and the
driver applies the +273.15 C offset internally before writing the
hardware table
- Changed custom-rtd to custom-copper-trace to better represent the
copper trace custom table, and made it required for > 1 ohm variants
- Made custom-leak-detector required for leak detector sensors, _new
and _assign_chan functions were updated for both sensor types
- Updated commit message to reflect the changes
- Removed "..." from "rsense channel must be configured" error message
from ltc2983_leak_detector_new()
- Removed blank line before hw_type declaration in
__ltc2983_chan_assign_common
Liviu Stan (8):
iio: temperature: ltc2983: Fix macro parenthesization and rename
iio: temperature: ltc2983: Use local device pointer consistently
iio: temperature: ltc2983: Fix inconsistent channel wording in
messages
iio: temperature: ltc2983: Use fwnode_property_present() for optional
properties
iio: temperature: ltc2983: Fix n_wires default bypassing rotation
check
iio: core: Add IIO_COVERAGE channel type
dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983
iio: temperature: ltc2983: Add support for ADT7604
Documentation/ABI/testing/sysfs-bus-iio | 17 +
.../bindings/iio/temperature/adi,ltc2983.yaml | 207 +++++-
drivers/iio/industrialio-core.c | 1 +
drivers/iio/temperature/ltc2983.c | 614 +++++++++++++++---
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 +
6 files changed, 741 insertions(+), 101 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 2026-05-21 16:42 [PATCH v3 0/8] " Liviu Stan @ 2026-05-21 16:43 ` Liviu Stan 2026-05-22 13:34 ` Jonathan Cameron 0 siblings, 1 reply; 6+ messages in thread From: Liviu Stan @ 2026-05-21 16:43 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Antoniu Miclaus, Francesco Lavra, Liviu Stan, linux-iio, linux-kernel, linux, devicetree The ADT7604 shares the same die as the LTC2984. It repurposes the custom RTD sensor type (18) as a copper trace resistance sensor and the custom thermistor type (27) as a leak detector, and removes thermocouple, diode and direct ADC sensor types. Two new software sensor type values are introduced (LTC2983_SENSOR_COPPER_TRACE = 32, LTC2983_SENSOR_LEAK_DETECTOR = 33) that map to the hardware register values 18 and 27 respectively. Dedicated structs (ltc2983_copper_trace, ltc2983_leak_detector) and parser functions are added rather than extending the existing RTD and thermistor paths, as the hardware configuration bits are fully hardcoded and several RTD/thermistor properties would need to be explicitly forbidden or ignored. Custom RTD (type 18) becomes the copper trace sensor. Sensor configuration bits are hardcoded to 0b1001 per the datasheet. Two variants are supported via the adi,copper-trace-sub-ohm DT property: sub-ohm traces (< 1 ohm) have bits 17:0 cleared with no excitation current or custom table; standard traces (> 1 ohm) have a required resistance-to-temperature table. Custom thermistor (type 27) becomes the leak detector. Sensor configuration bits are hardcoded to 0b001. The custom table uses a resolution of 16 instead of 64, and is specified via the required adi,custom-leak-detector DT property. Both sensor types expose an IIO_RESISTANCE channel reading from the resistance result register bank (0x0060-0x00AF). Added a "base" parameter to the LTC2983_RESULT_ADDR macro and a "base_reg" parameter to the ltc2983_chan_read function so we can read from both result register banks. The resistance register encodes the measured resistance with 10 fractional bits, so dividing by 1024 gives ohms. Since the sense resistor is specified in ohms, the output is in ohms for both sensor types and a single 1/1024 scale applies to both. For > 1 ohm copper traces and for leak detectors, a secondary channel also appears: IIO_TEMP (millidegrees Celsius) for copper trace and IIO_COVERAGE (percent) for leak detector. The ltc2983_chip_info struct is extended with a u64 supported_sensors bitmask using BIT_ULL() to safely represent the new sensor type bits 32 and 33 on 32-bit builds. A LTC2983_SENSOR_NUM sentinel is added to the enum so that the bounds check uses >= LTC2983_SENSOR_NUM rather than hardcoding the last sensor type. Tested on EVAL-ADT7604-AZ connected to Raspberry Pi 5 via SPI. Signed-off-by: Liviu Stan <liviu.stan@analog.com> --- Changes in v3: - Relocated the "base" parameter addition from patch 1 to this one since the resistance result memory region is (currently) ADT7604-specific - Updated the commit message to mention the base and base_reg parameters added to the result-bank read helpers - Stacked the IIO_RESISTANCE and IIO_COVERAGE case labels in ltc2983_read_raw() - Added LTC2983_SENSOR_NUM sentinel at the end of the sensor-type enum - Changed the sensor-type bounds check from > LTC2983_SENSOR_LEAK_DETECTOR to >= LTC2983_SENSOR_NUM - Reordered the ltm2985_chip_info_data struct initializer so has_eeprom doesn't look like it was added to the ltm2985 in the diff - Changed the custom leak detector table encoding, users now specify plain coverage percentage values (0-100) in the device tree and the driver applies the +273.15 C offset internally before writing the hardware table - Changed custom-rtd to custom-copper-trace to better represent the copper trace custom table, and made it required for > 1 ohm variants - Made custom-leak-detector required for leak detector sensors, _new and _assign_chan functions were updated for both sensor types - Updated commit message to reflect the changes - Removed "..." from "rsense channel must be configured" error message from ltc2983_leak_detector_new() - Removed blank line before hw_type declaration in __ltc2983_chan_assign_common drivers/iio/temperature/ltc2983.c | 413 ++++++++++++++++++++++++++++-- 1 file changed, 394 insertions(+), 19 deletions(-) diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c index 326f843f4271..e2ffeee026ee 100644 --- a/drivers/iio/temperature/ltc2983.c +++ b/drivers/iio/temperature/ltc2983.c @@ -28,6 +28,8 @@ #define LTC2983_STATUS_REG 0x0000 #define LTC2983_TEMP_RES_START_REG 0x0010 #define LTC2983_TEMP_RES_END_REG 0x005F +#define ADT7604_RES_RES_START_REG 0x0060 +#define ADT7604_RES_RES_END_REG 0x00AF #define LTC2983_EEPROM_KEY_REG 0x00B0 #define LTC2983_EEPROM_READ_STATUS_REG 0x00D0 #define LTC2983_GLOBAL_CONFIG_REG 0x00F0 @@ -58,8 +60,8 @@ #define LTC2983_CHAN_ASSIGN_ADDR(chan) \ ((((chan) - 1) * 4) + LTC2983_CHAN_ASSIGN_START_REG) -#define LTC2983_RESULT_ADDR(chan) \ - ((((chan) - 1) * 4) + LTC2983_TEMP_RES_START_REG) +#define LTC2983_RESULT_ADDR(chan, base) \ + ((((chan) - 1) * 4) + (base)) #define LTC2983_THERMOCOUPLE_DIFF_MASK BIT(3) #define LTC2983_THERMOCOUPLE_SGL(x) \ FIELD_PREP(LTC2983_THERMOCOUPLE_DIFF_MASK, x) @@ -186,17 +188,44 @@ enum { LTC2983_SENSOR_SENSE_RESISTOR = 29, LTC2983_SENSOR_DIRECT_ADC = 30, LTC2983_SENSOR_ACTIVE_TEMP = 31, + /* Sensor types for some parts only; map to RTD_CUSTOM/THERMISTOR_CUSTOM in HW */ + LTC2983_SENSOR_COPPER_TRACE = 32, + LTC2983_SENSOR_LEAK_DETECTOR = 33, + LTC2983_SENSOR_NUM, }; +/* Bitmask of sensor types supported by LTC2983/LTC2984 and derivatives */ +#define LTC2983_COMMON_SENSORS \ + (GENMASK_ULL(LTC2983_SENSOR_THERMOCOUPLE_CUSTOM, LTC2983_SENSOR_THERMOCOUPLE) | \ + GENMASK_ULL(LTC2983_SENSOR_RTD_CUSTOM, LTC2983_SENSOR_RTD) | \ + GENMASK_ULL(LTC2983_SENSOR_THERMISTOR_CUSTOM, LTC2983_SENSOR_THERMISTOR) | \ + BIT_ULL(LTC2983_SENSOR_DIODE) | \ + BIT_ULL(LTC2983_SENSOR_SENSE_RESISTOR) | \ + BIT_ULL(LTC2983_SENSOR_DIRECT_ADC)) + +/* Bitmask of sensor types supported by ADT7604 */ +#define ADT7604_SENSORS \ + (GENMASK_ULL(LTC2983_SENSOR_RTD_CUSTOM - 1, LTC2983_SENSOR_RTD) | \ + GENMASK_ULL(LTC2983_SENSOR_THERMISTOR_CUSTOM - 1, LTC2983_SENSOR_THERMISTOR) | \ + BIT_ULL(LTC2983_SENSOR_SENSE_RESISTOR) | \ + BIT_ULL(LTC2983_SENSOR_COPPER_TRACE) | \ + BIT_ULL(LTC2983_SENSOR_LEAK_DETECTOR)) + #define to_thermocouple(_sensor) \ container_of(_sensor, struct ltc2983_thermocouple, sensor) #define to_rtd(_sensor) \ container_of(_sensor, struct ltc2983_rtd, sensor) +#define to_copper_trace(_sensor) \ + container_of(_sensor, struct ltc2983_copper_trace, sensor) + #define to_thermistor(_sensor) \ container_of(_sensor, struct ltc2983_thermistor, sensor) +#define to_leak_detector(_sensor) \ + container_of(_sensor, struct ltc2983_leak_detector, sensor) + #define to_diode(_sensor) \ container_of(_sensor, struct ltc2983_diode, sensor) @@ -212,7 +241,7 @@ enum { struct ltc2983_chip_info { const char *name; unsigned int max_channels_nr; - bool has_temp; + u64 supported_sensors; bool has_eeprom; }; @@ -247,6 +276,8 @@ struct ltc2983_sensor { u32 chan; /* sensor type */ u32 type; + /* number of IIO channels this sensor produces */ + u8 n_iio_chan; }; struct ltc2983_custom_sensor { @@ -274,6 +305,25 @@ struct ltc2983_rtd { u32 rtd_curve; }; +struct ltc2983_copper_trace { + struct ltc2983_sensor sensor; + struct ltc2983_custom_sensor *custom; + u32 r_sense_chan; + u32 excitation_current; + /* selects the <1Ω variant: bits 17:0 of the channel word are zeroed, + * disabling excitation current and custom table fields (ADT7604 + * datasheet Table 26) + */ + bool is_sub_ohm; +}; + +struct ltc2983_leak_detector { + struct ltc2983_sensor sensor; + struct ltc2983_custom_sensor *custom; + u32 r_sense_chan; + u32 excitation_current; +}; + struct ltc2983_thermistor { struct ltc2983_sensor sensor; struct ltc2983_custom_sensor *custom; @@ -353,8 +403,14 @@ static int __ltc2983_chan_assign_common(struct ltc2983_data *st, { struct device *dev = &st->spi->dev; u32 reg = LTC2983_CHAN_ASSIGN_ADDR(sensor->chan); + u32 hw_type = sensor->type; - chan_val |= LTC2983_CHAN_TYPE(sensor->type); + if (hw_type == LTC2983_SENSOR_COPPER_TRACE) + hw_type = LTC2983_SENSOR_RTD_CUSTOM; + else if (hw_type == LTC2983_SENSOR_LEAK_DETECTOR) + hw_type = LTC2983_SENSOR_THERMISTOR_CUSTOM; + + chan_val |= LTC2983_CHAN_TYPE(hw_type); dev_dbg(dev, "Assign reg:0x%04X, val:0x%08X\n", reg, chan_val); st->chan_val = cpu_to_be32(chan_val); @@ -486,6 +542,14 @@ __ltc2983_custom_sensor_new(struct ltc2983_data *st, const struct fwnode_handle for (index = 0; index < n_entries; index++) { u64 temp = ((u64 *)new_custom->table)[index]; + /* + * Users specify plain coverage percentage (0-100). Convert + * to µK so __convert_to_raw() produces the correct hardware + * encoding: P + 273.15 K. + */ + if ((index % 2) != 0 && !strcmp(propname, "adi,custom-leak-detector")) + temp = temp * 1000000 + 273150000; + if ((index % 2) != 0) temp = __convert_to_raw(temp, 1024); else if (has_signed && (s64)temp < 0) @@ -579,6 +643,31 @@ static int ltc2983_rtd_assign_chan(struct ltc2983_data *st, return __ltc2983_chan_assign_common(st, sensor, chan_val); } +static int ltc2983_copper_trace_assign_chan(struct ltc2983_data *st, + const struct ltc2983_sensor *sensor) +{ + struct ltc2983_copper_trace *ct = to_copper_trace(sensor); + u32 chan_val; + + chan_val = LTC2983_CHAN_ASSIGN(ct->r_sense_chan); + /* Sensor config bits 21:18 must be 0b1001 (ADT7604 datasheet Table 26) */ + chan_val |= LTC2983_RTD_CFG(0x9); + + if (ct->is_sub_ohm) { + chan_val &= ~GENMASK(17, 0); + } else { + int ret; + + chan_val |= LTC2983_RTD_EXC_CURRENT(ct->excitation_current); + ret = __ltc2983_chan_custom_sensor_assign(st, ct->custom, + &chan_val); + if (ret) + return ret; + } + + return __ltc2983_chan_assign_common(st, sensor, chan_val); +} + static int ltc2983_thermistor_assign_chan(struct ltc2983_data *st, const struct ltc2983_sensor *sensor) { @@ -602,6 +691,25 @@ static int ltc2983_thermistor_assign_chan(struct ltc2983_data *st, return __ltc2983_chan_assign_common(st, sensor, chan_val); } +static int ltc2983_leak_detector_assign_chan(struct ltc2983_data *st, + const struct ltc2983_sensor *sensor) +{ + struct ltc2983_leak_detector *ld = to_leak_detector(sensor); + u32 chan_val; + int ret; + + chan_val = LTC2983_CHAN_ASSIGN(ld->r_sense_chan); + /* bits 21:19 must be 0b001 (ADT7604 datasheet Table 38) */ + chan_val |= LTC2983_THERMISTOR_CFG(1); + chan_val |= LTC2983_THERMISTOR_EXC_CURRENT(ld->excitation_current); + + ret = __ltc2983_chan_custom_sensor_assign(st, ld->custom, &chan_val); + if (ret) + return ret; + + return __ltc2983_chan_assign_common(st, sensor, chan_val); +} + static int ltc2983_diode_assign_chan(struct ltc2983_data *st, const struct ltc2983_sensor *sensor) { @@ -1037,6 +1145,195 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s return &thermistor->sensor; } +static struct ltc2983_sensor * +ltc2983_copper_trace_new(const struct fwnode_handle *child, struct ltc2983_data *st, + const struct ltc2983_sensor *sensor) +{ + struct device *dev = &st->spi->dev; + struct ltc2983_copper_trace *ct; + int ret; + + if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) + return dev_err_ptr_probe(dev, -EINVAL, + "Invalid channel %d for copper trace\n", + sensor->chan); + + ct = devm_kzalloc(dev, sizeof(*ct), GFP_KERNEL); + if (!ct) + return ERR_PTR(-ENOMEM); + + struct fwnode_handle *ref __free(fwnode_handle) = + fwnode_find_reference(child, "adi,rsense-handle", 0); + if (IS_ERR(ref)) + return dev_err_cast_probe(dev, ref, + "Property adi,rsense-handle missing or invalid\n"); + + ret = fwnode_property_read_u32(ref, "reg", &ct->r_sense_chan); + if (ret) + return dev_err_ptr_probe(dev, ret, "Property reg must be given\n"); + + ct->is_sub_ohm = fwnode_property_read_bool(child, "adi,copper-trace-sub-ohm"); + + if (ct->is_sub_ohm && fwnode_property_present(child, "adi,custom-copper-trace")) + return dev_err_ptr_probe(dev, -EINVAL, + "sub-ohm copper trace cannot have a custom table\n"); + + if (!ct->is_sub_ohm) { + u32 excitation_current = 0; + + if (!fwnode_property_present(child, "adi,custom-copper-trace")) + return dev_err_ptr_probe(dev, -EINVAL, + "adi,custom-copper-trace is required for >1 ohm copper trace\n"); + + ct->custom = __ltc2983_custom_sensor_new(st, child, "adi,custom-copper-trace", + false, 2048, false); + if (IS_ERR(ct->custom)) + return ERR_CAST(ct->custom); + + if (fwnode_property_present(child, "adi,excitation-current-microamp")) { + ret = fwnode_property_read_u32(child, "adi,excitation-current-microamp", + &excitation_current); + if (ret) + return dev_err_ptr_probe(dev, ret, + "Failed to read adi,excitation-current-microamp\n"); + + switch (excitation_current) { + case 5: + ct->excitation_current = 0x01; + break; + case 10: + ct->excitation_current = 0x02; + break; + case 25: + ct->excitation_current = 0x03; + break; + case 50: + ct->excitation_current = 0x04; + break; + case 100: + ct->excitation_current = 0x05; + break; + case 250: + ct->excitation_current = 0x06; + break; + case 500: + ct->excitation_current = 0x07; + break; + case 1000: + ct->excitation_current = 0x08; + break; + default: + return dev_err_ptr_probe(dev, -EINVAL, + "Invalid value for excitation current(%u)\n", + excitation_current); + } + } else { + /* default to 1mA per datasheet recommendation for copper trace */ + ct->excitation_current = 0x08; + } + } + + ct->sensor.fault_handler = ltc2983_common_fault_handler; + ct->sensor.assign_chan = ltc2983_copper_trace_assign_chan; + if (ct->is_sub_ohm) + ct->sensor.n_iio_chan = 1; + else + ct->sensor.n_iio_chan = 2; + + return &ct->sensor; +} + +static struct ltc2983_sensor * +ltc2983_leak_detector_new(const struct fwnode_handle *child, struct ltc2983_data *st, + const struct ltc2983_sensor *sensor) +{ + struct device *dev = &st->spi->dev; + struct ltc2983_leak_detector *ld; + int ret; + u32 excitation_current = 0; + + if (sensor->chan < LTC2983_DIFFERENTIAL_CHAN_MIN) + return dev_err_ptr_probe(dev, -EINVAL, + "Invalid channel %d for leak detector\n", + sensor->chan); + + ld = devm_kzalloc(dev, sizeof(*ld), GFP_KERNEL); + if (!ld) + return ERR_PTR(-ENOMEM); + + struct fwnode_handle *ref __free(fwnode_handle) = + fwnode_find_reference(child, "adi,rsense-handle", 0); + if (IS_ERR(ref)) + return dev_err_cast_probe(dev, ref, + "Property adi,rsense-handle missing or invalid\n"); + + ret = fwnode_property_read_u32(ref, "reg", &ld->r_sense_chan); + if (ret) + return dev_err_ptr_probe(dev, ret, + "rsense channel must be configured\n"); + + if (!fwnode_property_present(child, "adi,custom-leak-detector")) + return dev_err_ptr_probe(dev, -EINVAL, + "adi,custom-leak-detector is required for leak detectors\n"); + + ld->custom = __ltc2983_custom_sensor_new(st, child, "adi,custom-leak-detector", + false, 16, false); + if (IS_ERR(ld->custom)) + return ERR_CAST(ld->custom); + + ret = fwnode_property_read_u32(child, "adi,excitation-current-nanoamp", + &excitation_current); + if (ret) + return dev_err_ptr_probe(dev, ret, + "adi,excitation-current-nanoamp is required for leak detectors\n"); + + switch (excitation_current) { + case 250: + ld->excitation_current = 0x01; + break; + case 500: + ld->excitation_current = 0x02; + break; + case 1000: + ld->excitation_current = 0x03; + break; + case 5000: + ld->excitation_current = 0x04; + break; + case 10000: + ld->excitation_current = 0x05; + break; + case 25000: + ld->excitation_current = 0x06; + break; + case 50000: + ld->excitation_current = 0x07; + break; + case 100000: + ld->excitation_current = 0x08; + break; + case 250000: + ld->excitation_current = 0x09; + break; + case 500000: + ld->excitation_current = 0x0a; + break; + case 1000000: + ld->excitation_current = 0x0b; + break; + default: + return dev_err_ptr_probe(dev, -EINVAL, + "Invalid value for excitation current(%u)\n", + excitation_current); + } + + ld->sensor.fault_handler = ltc2983_common_fault_handler; + ld->sensor.assign_chan = ltc2983_leak_detector_assign_chan; + ld->sensor.n_iio_chan = 2; + + return &ld->sensor; +} + static struct ltc2983_sensor * ltc2983_diode_new(const struct fwnode_handle *child, const struct ltc2983_data *st, const struct ltc2983_sensor *sensor) @@ -1205,7 +1502,8 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child, } static int ltc2983_chan_read(struct ltc2983_data *st, - const struct ltc2983_sensor *sensor, int *val) + const struct ltc2983_sensor *sensor, + u32 base_reg, int *val) { struct device *dev = &st->spi->dev; u32 start_conversion = 0; @@ -1236,13 +1534,23 @@ static int ltc2983_chan_read(struct ltc2983_data *st, } /* read the converted data */ - ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan), + ret = regmap_bulk_read(st->regmap, LTC2983_RESULT_ADDR(sensor->chan, base_reg), &st->temp, sizeof(st->temp)); if (ret) return ret; *val = __be32_to_cpu(st->temp); + if (base_reg == ADT7604_RES_RES_START_REG) { + /* + * Resistance result register gives a plain unsigned value, + * D31 is always 0, no valid bit, no fault bits. Read bits[30:0] + * directly — the temperature result format does not apply here. + */ + *val &= GENMASK(30, 0); + return 0; + } + if (!(LTC2983_RES_VALID_MASK & *val)) { dev_err(dev, "Invalid conversion detected\n"); return -EIO; @@ -1274,7 +1582,16 @@ static int ltc2983_read_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_RAW: mutex_lock(&st->lock); - ret = ltc2983_chan_read(st, st->sensors[chan->address], val); + switch (chan->type) { + case IIO_RESISTANCE: + ret = ltc2983_chan_read(st, st->sensors[chan->address], + ADT7604_RES_RES_START_REG, val); + break; + default: + ret = ltc2983_chan_read(st, st->sensors[chan->address], + LTC2983_TEMP_RES_START_REG, val); + break; + } mutex_unlock(&st->lock); return ret ?: IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: @@ -1291,6 +1608,13 @@ static int ltc2983_read_raw(struct iio_dev *indio_dev, /* 2^21 */ *val2 = 2097152; return IIO_VAL_FRACTIONAL; + case IIO_RESISTANCE: + case IIO_COVERAGE: + /* value in ohm/percent */ + *val = 1; + /* 2^10 */ + *val2 = 1024; + return IIO_VAL_FRACTIONAL; default: return -EINVAL; } @@ -1351,7 +1675,7 @@ static int ltc2983_parse_fw(struct ltc2983_data *st) if (!st->sensors) return -ENOMEM; - st->iio_channels = st->num_channels; + st->iio_channels = 0; device_for_each_child_node_scoped(dev, child) { struct ltc2983_sensor sensor; @@ -1379,6 +1703,12 @@ static int ltc2983_parse_fw(struct ltc2983_data *st) return dev_err_probe(dev, ret, "adi,sensor-type property must given for child nodes\n"); + if (sensor.type >= LTC2983_SENSOR_NUM || + !(st->info->supported_sensors & BIT_ULL(sensor.type))) + return dev_err_probe(dev, -EINVAL, + "sensor type %d not supported on %s\n", + sensor.type, st->info->name); + dev_dbg(dev, "Create new sensor, type %u, channel %u", sensor.type, sensor.chan); @@ -1399,13 +1729,14 @@ static int ltc2983_parse_fw(struct ltc2983_data *st) } else if (sensor.type == LTC2983_SENSOR_SENSE_RESISTOR) { st->sensors[chan] = ltc2983_r_sense_new(child, st, &sensor); - /* don't add rsense to iio */ - st->iio_channels--; } else if (sensor.type == LTC2983_SENSOR_DIRECT_ADC) { st->sensors[chan] = ltc2983_adc_new(child, st, &sensor); - } else if (st->info->has_temp && - sensor.type == LTC2983_SENSOR_ACTIVE_TEMP) { + } else if (sensor.type == LTC2983_SENSOR_ACTIVE_TEMP) { st->sensors[chan] = ltc2983_temp_new(child, st, &sensor); + } else if (sensor.type == LTC2983_SENSOR_COPPER_TRACE) { + st->sensors[chan] = ltc2983_copper_trace_new(child, st, &sensor); + } else if (sensor.type == LTC2983_SENSOR_LEAK_DETECTOR) { + st->sensors[chan] = ltc2983_leak_detector_new(child, st, &sensor); } else { return dev_err_probe(dev, -EINVAL, "Unknown sensor type %d\n", @@ -1420,6 +1751,16 @@ static int ltc2983_parse_fw(struct ltc2983_data *st) st->sensors[chan]->chan = sensor.chan; st->sensors[chan]->type = sensor.type; + /* + * Dedicated functions set n_iio_chan themselves; for all other + * sensor types rsense produces 0 channels, everything else 1. + */ + if (!st->sensors[chan]->n_iio_chan) { + if (sensor.type != LTC2983_SENSOR_SENSE_RESISTOR) + st->sensors[chan]->n_iio_chan = 1; + } + st->iio_channels += st->sensors[chan]->n_iio_chan; + channel_avail_mask |= BIT(sensor.chan); chan++; } @@ -1467,8 +1808,9 @@ static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd, static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio) { - u32 iio_chan_t = 0, iio_chan_v = 0, chan, iio_idx = 0, status; struct device *dev = &st->spi->dev; + u32 iio_chan_t = 0, iio_chan_v = 0, iio_chan_r = 0, iio_chan_c = 0; + u32 chan, iio_idx = 0, status; int ret; /* make sure the device is up: start bit (7) is 0 and done bit (6) is 1 */ @@ -1516,12 +1858,33 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio) continue; /* assign iio channel */ - if (st->sensors[chan]->type != LTC2983_SENSOR_DIRECT_ADC) { - chan_type = IIO_TEMP; - iio_chan = &iio_chan_t; - } else { + switch (st->sensors[chan]->type) { + case LTC2983_SENSOR_COPPER_TRACE: + if (st->sensors[chan]->n_iio_chan == 1) { + /* sub-ohm copper traces produce only a resistance result */ + st->iio_chan[iio_idx++] = + LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan); + } else { + st->iio_chan[iio_idx++] = + LTC2983_CHAN(IIO_TEMP, iio_chan_t++, chan); + st->iio_chan[iio_idx++] = + LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan); + } + continue; + case LTC2983_SENSOR_LEAK_DETECTOR: + st->iio_chan[iio_idx++] = + LTC2983_CHAN(IIO_COVERAGE, iio_chan_c++, chan); + st->iio_chan[iio_idx++] = + LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan); + continue; + case LTC2983_SENSOR_DIRECT_ADC: chan_type = IIO_VOLTAGE; iio_chan = &iio_chan_v; + break; + default: + chan_type = IIO_TEMP; + iio_chan = &iio_chan_t; + break; } /* @@ -1538,6 +1901,7 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio) static const struct regmap_range ltc2983_reg_ranges[] = { regmap_reg_range(LTC2983_STATUS_REG, LTC2983_STATUS_REG), regmap_reg_range(LTC2983_TEMP_RES_START_REG, LTC2983_TEMP_RES_END_REG), + regmap_reg_range(ADT7604_RES_RES_START_REG, ADT7604_RES_RES_END_REG), regmap_reg_range(LTC2983_EEPROM_KEY_REG, LTC2983_EEPROM_KEY_REG), regmap_reg_range(LTC2983_EEPROM_READ_STATUS_REG, LTC2983_EEPROM_READ_STATUS_REG), @@ -1680,26 +2044,35 @@ static DEFINE_SIMPLE_DEV_PM_OPS(ltc2983_pm_ops, ltc2983_suspend, static const struct ltc2983_chip_info ltc2983_chip_info_data = { .name = "ltc2983", .max_channels_nr = 20, + .supported_sensors = LTC2983_COMMON_SENSORS, }; static const struct ltc2983_chip_info ltc2984_chip_info_data = { .name = "ltc2984", .max_channels_nr = 20, .has_eeprom = true, + .supported_sensors = LTC2983_COMMON_SENSORS, }; static const struct ltc2983_chip_info ltc2986_chip_info_data = { .name = "ltc2986", .max_channels_nr = 10, - .has_temp = true, .has_eeprom = true, + .supported_sensors = LTC2983_COMMON_SENSORS | BIT_ULL(LTC2983_SENSOR_ACTIVE_TEMP), }; static const struct ltc2983_chip_info ltm2985_chip_info_data = { .name = "ltm2985", .max_channels_nr = 10, - .has_temp = true, .has_eeprom = true, + .supported_sensors = LTC2983_COMMON_SENSORS | BIT_ULL(LTC2983_SENSOR_ACTIVE_TEMP), +}; + +static const struct ltc2983_chip_info adt7604_chip_info_data = { + .name = "adt7604", + .max_channels_nr = 20, + .has_eeprom = true, + .supported_sensors = ADT7604_SENSORS, }; static const struct spi_device_id ltc2983_id_table[] = { @@ -1707,6 +2080,7 @@ static const struct spi_device_id ltc2983_id_table[] = { { "ltc2984", (kernel_ulong_t)<c2984_chip_info_data }, { "ltc2986", (kernel_ulong_t)<c2986_chip_info_data }, { "ltm2985", (kernel_ulong_t)<m2985_chip_info_data }, + { "adt7604", (kernel_ulong_t)&adt7604_chip_info_data }, { } }; MODULE_DEVICE_TABLE(spi, ltc2983_id_table); @@ -1716,6 +2090,7 @@ static const struct of_device_id ltc2983_of_match[] = { { .compatible = "adi,ltc2984", .data = <c2984_chip_info_data }, { .compatible = "adi,ltc2986", .data = <c2986_chip_info_data }, { .compatible = "adi,ltm2985", .data = <m2985_chip_info_data }, + { .compatible = "adi,adt7604", .data = &adt7604_chip_info_data }, { } }; MODULE_DEVICE_TABLE(of, ltc2983_of_match); -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 2026-05-21 16:43 ` [PATCH v3 8/8] " Liviu Stan @ 2026-05-22 13:34 ` Jonathan Cameron 2026-05-22 14:24 ` Stan, Liviu 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron @ 2026-05-22 13:34 UTC (permalink / raw) To: Liviu Stan Cc: David Lechner, Nuno Sá, Andy Shevchenko, Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Antoniu Miclaus, Francesco Lavra, linux-iio, linux-kernel, linux, devicetree On Thu, 21 May 2026 19:43:01 +0300 Liviu Stan <liviu.stan@analog.com> wrote: > The ADT7604 shares the same die as the LTC2984. It repurposes the > custom RTD sensor type (18) as a copper trace resistance sensor > and the custom thermistor type (27) as a leak detector, and > removes thermocouple, diode and direct ADC sensor types. > > Two new software sensor type values are introduced > (LTC2983_SENSOR_COPPER_TRACE = 32, LTC2983_SENSOR_LEAK_DETECTOR = 33) > that map to the hardware register values 18 and 27 respectively. > Dedicated structs (ltc2983_copper_trace, ltc2983_leak_detector) and > parser functions are added rather than extending the existing RTD and > thermistor paths, as the hardware configuration bits are fully > hardcoded and several RTD/thermistor properties would need to be > explicitly forbidden or ignored. > > Custom RTD (type 18) becomes the copper trace sensor. Sensor > configuration bits are hardcoded to 0b1001 per the datasheet. > Two variants are supported via the adi,copper-trace-sub-ohm DT > property: sub-ohm traces (< 1 ohm) have bits 17:0 cleared with no > excitation current or custom table; standard traces (> 1 ohm) have > a required resistance-to-temperature table. > > Custom thermistor (type 27) becomes the leak detector. Sensor > configuration bits are hardcoded to 0b001. The custom table uses > a resolution of 16 instead of 64, and is specified via the > required adi,custom-leak-detector DT property. > > Both sensor types expose an IIO_RESISTANCE channel reading from > the resistance result register bank (0x0060-0x00AF). Added a > "base" parameter to the LTC2983_RESULT_ADDR macro and a "base_reg" > parameter to the ltc2983_chan_read function so we can read from > both result register banks. The resistance register encodes the > measured resistance with 10 fractional bits, so dividing by 1024 > gives ohms. Since the sense resistor is specified in ohms, the > output is in ohms for both sensor types and a single 1/1024 > scale applies to both. For > 1 ohm copper traces and for leak > detectors, a secondary channel also appears: IIO_TEMP > (millidegrees Celsius) for copper trace and IIO_COVERAGE (percent) > for leak detector. > > The ltc2983_chip_info struct is extended with a u64 supported_sensors > bitmask using BIT_ULL() to safely represent the new sensor type bits > 32 and 33 on 32-bit builds. A LTC2983_SENSOR_NUM sentinel is added > to the enum so that the bounds check uses >= LTC2983_SENSOR_NUM > rather than hardcoding the last sensor type. > > Tested on EVAL-ADT7604-AZ connected to Raspberry Pi 5 via SPI. > > Signed-off-by: Liviu Stan <liviu.stan@analog.com> The sashiko comment about the completion race looks real to me. Can you take a look at that. Might be some odd reason why we can't reinit_completion() before triggering the capture but that would be very unusual. Otherwise just two really small things I missed in earlier reviews. If everything else was ready to go I'd have tweaked these whilst applying but seems you'll be doing a v4 so over to you! Jonathan > drivers/iio/temperature/ltc2983.c | 413 ++++++++++++++++++++++++++++-- > 1 file changed, 394 insertions(+), 19 deletions(-) > > diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c > index 326f843f4271..e2ffeee026ee 100644 > --- a/drivers/iio/temperature/ltc2983.c > +++ b/drivers/iio/temperature/ltc2983.c > @@ -186,17 +188,44 @@ enum { > LTC2983_SENSOR_SENSE_RESISTOR = 29, > LTC2983_SENSOR_DIRECT_ADC = 30, > LTC2983_SENSOR_ACTIVE_TEMP = 31, > + /* Sensor types for some parts only; map to RTD_CUSTOM/THERMISTOR_CUSTOM in HW */ > + LTC2983_SENSOR_COPPER_TRACE = 32, > + LTC2983_SENSOR_LEAK_DETECTOR = 33, > + LTC2983_SENSOR_NUM, That's a terminating entry so no trailing comma. It always has to be last. > }; > static const struct spi_device_id ltc2983_id_table[] = { > @@ -1707,6 +2080,7 @@ static const struct spi_device_id ltc2983_id_table[] = { > { "ltc2984", (kernel_ulong_t)<c2984_chip_info_data }, > { "ltc2986", (kernel_ulong_t)<c2986_chip_info_data }, > { "ltm2985", (kernel_ulong_t)<m2985_chip_info_data }, > + { "adt7604", (kernel_ulong_t)&adt7604_chip_info_data }, As below. > { } > }; > MODULE_DEVICE_TABLE(spi, ltc2983_id_table); > @@ -1716,6 +2090,7 @@ static const struct of_device_id ltc2983_of_match[] = { > { .compatible = "adi,ltc2984", .data = <c2984_chip_info_data }, > { .compatible = "adi,ltc2986", .data = <c2986_chip_info_data }, > { .compatible = "adi,ltm2985", .data = <m2985_chip_info_data }, > + { .compatible = "adi,adt7604", .data = &adt7604_chip_info_data }, Oops I missed this before. Alphabetical order. Jonathan > { } ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 2026-05-22 13:34 ` Jonathan Cameron @ 2026-05-22 14:24 ` Stan, Liviu 2026-05-22 17:31 ` Jonathan Cameron 0 siblings, 1 reply; 6+ messages in thread From: Stan, Liviu @ 2026-05-22 14:24 UTC (permalink / raw) To: Jonathan Cameron, David Lechner Cc: David Lechner, Sa, Nuno, Andy Shevchenko, Hennerich, Michael, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Miclaus, Antoniu, Francesco Lavra, linux-iio, linux-kernel, linux, devicetree On Fri, 22 May 2026 14:34:44 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Thu, 21 May 2026 19:43:01 +0300 > Liviu Stan <liviu.stan@analog.com> wrote: > ... > > The sashiko comment about the completion race looks real to me. > Can you take a look at that. Might be some odd reason why we can't > reinit_completion() before triggering the capture but that would > be very unusual. > I don't see any reason why we couldn't put it before the regmap_write, it has also already been done in ltc2983_eeprom_cmd(). So, I will change it in v4. > Otherwise just two really small things I missed in earlier reviews. > If everything else was ready to go I'd have tweaked these whilst > applying but seems you'll be doing a v4 so over to you! Understood, will change them in v4!:) Thanks! On Fri, 22 May 2026 09:09:33 -0500 David Lechner <dlechner@baylibre.com> wrote: > On 5/22/26 8:23 AM, Liviu Stan wrote: > > On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote: > > > >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: ... > >> [Severity: High] > >> This is a pre-existing issue, but can this function experience spurious > >> read timeouts due to a race condition with the completion? > >> > >> Looking at the rest of this function: > >> > >> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion); > >> if (ret) > >> return ret; > >> > >> reinit_completion(&st->completion); > >> > >> If the hardware conversion finishes very quickly after regmap_write(), > >> could the interrupt fire and call complete() before the CPU executes > >> reinit_completion()? This would overwrite the completion event, causing the > >> subsequent wait_for_completion_timeout() to spuriously time out. > >> > >> Would it be safer to call reinit_completion() before initiating the hardware > >> action? > > > > This is theoretically valid but in practice, the conversion requires at minimum > > two 82ms cycles (167ms), so there is no realistic window for the interrupt to > > race with reinit_completion(). What do you guys think? > > I would still move it before the write so that it looks correct > and others can copy/paste the logic on other devices. That makes sense. Thanks! Would this be considered a fix? Liviu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 2026-05-22 14:24 ` Stan, Liviu @ 2026-05-22 17:31 ` Jonathan Cameron 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Cameron @ 2026-05-22 17:31 UTC (permalink / raw) To: Stan, Liviu Cc: David Lechner, Sa, Nuno, Andy Shevchenko, Hennerich, Michael, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Miclaus, Antoniu, Francesco Lavra, linux-iio, linux-kernel, linux, devicetree On Fri, 22 May 2026 14:24:27 +0000 "Stan, Liviu" <Liviu.Stan@analog.com> wrote: > On Fri, 22 May 2026 14:34:44 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > > > On Thu, 21 May 2026 19:43:01 +0300 > > Liviu Stan <liviu.stan@analog.com> wrote: > > > ... > > > > The sashiko comment about the completion race looks real to me. > > Can you take a look at that. Might be some odd reason why we can't > > reinit_completion() before triggering the capture but that would > > be very unusual. > > > > I don't see any reason why we couldn't put it before the regmap_write, > it has also already been done in ltc2983_eeprom_cmd(). So, I will change > it in v4. > > > Otherwise just two really small things I missed in earlier reviews. > > If everything else was ready to go I'd have tweaked these whilst > > applying but seems you'll be doing a v4 so over to you! > > Understood, will change them in v4!:) Thanks! > > On Fri, 22 May 2026 09:09:33 -0500 David Lechner <dlechner@baylibre.com> wrote: > > > On 5/22/26 8:23 AM, Liviu Stan wrote: > > > On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote: > > > > > >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > ... > > >> [Severity: High] > > >> This is a pre-existing issue, but can this function experience spurious > > >> read timeouts due to a race condition with the completion? > > >> > > >> Looking at the rest of this function: > > >> > > >> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion); > > >> if (ret) > > >> return ret; > > >> > > >> reinit_completion(&st->completion); > > >> > > >> If the hardware conversion finishes very quickly after regmap_write(), > > >> could the interrupt fire and call complete() before the CPU executes > > >> reinit_completion()? This would overwrite the completion event, causing the > > >> subsequent wait_for_completion_timeout() to spuriously time out. > > >> > > >> Would it be safer to call reinit_completion() before initiating the hardware > > >> action? > > > > > > This is theoretically valid but in practice, the conversion requires at minimum > > > two 82ms cycles (167ms), so there is no realistic window for the interrupt to > > > race with reinit_completion(). What do you guys think? > > > > I would still move it before the write so that it looks correct > > and others can copy/paste the logic on other devices. > > That makes sense. Thanks! > > Would this be considered a fix? Yes, though not an urgent one given no particular reports of it being hit. Just put it at the start of the series and I'll apply it with the rest. Can get backported after next merge window. Thanks, Jonathan > > Liviu ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-22 17:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260521191828.F20931F000E9@smtp.kernel.org>
2026-05-22 13:23 ` [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604 Liviu Stan
2026-05-22 14:09 ` David Lechner
2026-05-21 16:42 [PATCH v3 0/8] " Liviu Stan
2026-05-21 16:43 ` [PATCH v3 8/8] " Liviu Stan
2026-05-22 13:34 ` Jonathan Cameron
2026-05-22 14:24 ` Stan, Liviu
2026-05-22 17:31 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®