Andrei/ltc2499 support - #3452
Conversation
gastmaier
left a comment
gastmaier
left a comment
There was a problem hiding this comment.
High, some quick thoughts, the llm also found the provided
so it should be able to dive on the registers for us and provide some feedback on that too.
| * IIO reports temperatures in milli-degrees Celsius and | ||
| * the core computes the processed value as | ||
| * T[m°C] = (raw + offset) * scale |
There was a problem hiding this comment.
You shouldn't explain the subsystem here
| * The LTC2499 PTAT sensor yields the absolute temperature | ||
| * T(K) = DATAOUT24 * Vref / 1570 (Vref in volts) | ||
| * but the raw value exported here is normalised to | ||
| * 2^(resolution + 1) == 2^25, i.e. raw = 2 * DATAOUT24, so | ||
| * T(K) = raw * Vref / 3140. | ||
| * Converting K -> m°C (x1000) and Vref uV -> V (/1e6): | ||
| * scale[m°C/LSB] = Vref_uV / 3140000 |
There was a problem hiding this comment.
Could be more concise. the code is already the formula, describe what is unexpected.
| * time_prev and conv_time_prev are deliberately left untouched: | ||
| * a conversion started under the previous mode may still be in | ||
| * flight, and the next read must wait out its (old-mode) duration | ||
| * from its real start time before reprogramming, or the device | ||
| * NACKs the write with -EIO. |
There was a problem hiding this comment.
Don't describe actions taken.
the device NACKs the write with -EIO
This is incorrect. the device i2c protocol will NACK, and the consequence in the driver is -EIO.
The NACK has no return code by itself.
Waiting for commands to settle is usual device communication flows, nothing special here.
| /* | ||
| * The 32 voltage channels are shared by every part in the family. The LTC2499 | ||
| * appends one temperature channel; it is kept last so that parts without an | ||
| * internal temperature sensor can simply exclude it via num_channels. | ||
| */ |
There was a problem hiding this comment.
Sounds good, but too verbose too.
would be better to have a one liner were this surprising logic actually occurs [1]
| /* | ||
| * The temperature channel is the last entry; drop it for parts without | ||
| * an internal temperature sensor. | ||
| */ | ||
| indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel); | ||
| if (!ddata->chip_info->has_temp) | ||
| indio_dev->num_channels--; |
There was a problem hiding this comment.
[1] here is the ideal one liner location.
/* Only ltc2499 has a temperature channel */
| /* | ||
| * Parts with a speed mode expose in_voltage_sampling_frequency / | ||
| * _available on the voltage channels only. SPD is ignored during a | ||
| * temperature measurement, so the temperature channel deliberately | ||
| * carries no SAMP_FREQ attribute. Patch a private copy of the shared | ||
| * channel array so parts without a speed mode stay untouched. | ||
| */ |
There was a problem hiding this comment.
I don't see how this is the best approach.
why can't the temp channel be correctly const instead of patching it on probe?
There was a problem hiding this comment.
The reason I kept the runtime-patched copy is extensibility: the channel metadata is expected to grow with additional optional features on the LTC2499 family, and keeping a single base channel description avoids maintaining multiple near-duplicate channel tables.
I'm happy to switch to a static const table if you feel that is the preferred direction for this series.
|
|
||
| /* | ||
| * Worst-case conversion times (datasheet t_CONV max, simultaneous 50/60Hz | ||
| * rejection - the only rejection mode reachable today). The 1x value also |
There was a problem hiding this comment.
the only rejection mode reachable today
what does this mean?
There was a problem hiding this comment.
The intention was to describe a driver limitation, not a hardware limitation. The original wording was ambiguous and could also be interpreted as a roadmap reference.
I've reworded the comment to explicitly state that the driver only programs simultaneous 50/60Hz rejection and that FA/FB selection is not implemented.
This keeps the comment focused on the behavior implemented by the current change.
LLM reviewThis series adds an internal PTAT temperature channel and a 2x conversion speed mode ("SPD") for the run: 29905280095 Open review comments still unaddressedAll 6 open comments from
|
The LTC2499 has an internal PTAT (proportional to absolute temperature) sensor that is activated by a second I2C configuration byte (EN2 | IM). Expose it as an IIO_TEMP channel providing raw, scale and offset so the standard IIO formula T[m°C] = (raw + offset) * scale reconstructs the temperature. The PTAT sensor yields the absolute temperature as T(K) = DATAOUT24 * Vref / 1570 (Vref in volts) The raw value exported here is sign-extended and normalised to 2^(resolution + 1) == 2^25, i.e. raw = 2 * DATAOUT24, so on the IIO milli-degree-Celsius convention scale[m°C/LSB] = Vref_uV / 3140000 offset = -273150 * 3140000 / Vref_uV The scale and offset are derived from the reference voltage returned by regulator_get_voltage(); its error is propagated as before, so a board that fails to describe vref-supply gets a clear read error instead of a silently wrong temperature. No board-specific reference value is assumed in the driver. The single temperature channel is appended as the last entry of the shared channel array and excluded via num_channels for parts without an internal sensor, so the existing LTC2497 channel layout and device name are unchanged. The LTC2499 latches its converter configuration from the second command byte and only re-evaluates it when that byte has EN2 set. EN2 | IM selects the internal temperature sensor. Because a single-byte command, or a second byte with EN2 = 0, means "keep previous", a one-byte channel select cannot pull the device back out of temperature mode: after a temperature read every subsequent voltage read would keep returning the PTAT result instead of the selected input. Temperature support is therefore only correct if the voltage path also emits a second command byte that re-selects an external input. Send two-byte commands for all conversions on parts that have the sensor (has_temp): temperature: EN2 | IM voltage: EN2 (IM = 0 -> external input) The LTC2497 and LTC2496, which lack the second-byte mechanism, keep using the original single-byte channel select and are unchanged. Signed-off-by: Andrei Stancovici <andrei.stancovici@analog.com>
…port Add the LTC2499 to the binding title and note that its internal temperature sensor derives its scale and offset from the reference voltage, so an accurate vref-supply is required for the temperature channel to report correctly. vref-supply is already a required property, so no schema change is needed. Signed-off-by: Andrei Stancovici <andrei.stancovici@analog.com>
0421f8f to
0b0047e
Compare
|
v2: Addressed the review comments from @gastmaier and force-pushed an updated series. Changes:
Notes:
The fixes were amended into the original temperature and SPD commits and the series was force-pushed; the overall commit structure remains unchanged. One review item remains open for discussion:
For the SPD-only series, the static table is cleaner in isolation. The current approach was kept for extensibility, but I'm happy to switch if that is the preferred direction for this series. |
Add a Raspberry Pi overlay that instantiates an LTC2499 on I2C1 and wires its reference to the board's existing 3.3 V regulator via vref-supply = <&vdd_3v3_reg>; so that regulator_get_voltage() returns 3300000 and the temperature channel reports correctly without relying on any driver-side fallback. This is a local validation/example artifact for the Raspberry Pi and is not intended for upstream: every board must describe its own actual VREF regulator (2.5 V, 3.3 V, 4.096 V, ...). The 3.3 V value here is specific to this board and is not a universal hardware assumption. Signed-off-by: Andrei Stancovici <andrei.stancovici@analog.com>
The LTC2499 supports a 2x output rate (SPD bit in the second configuration byte). In 2x mode the offset auto-calibration is disabled, roughly doubling the conversion rate (~13.6Hz vs ~6.8Hz in simultaneous 50/60Hz rejection) while leaving linearity and full-scale errors unchanged (datasheet). During a temperature measurement the part always converts at 1x regardless of SPD. Expose the rate through the standard sampling_frequency / sampling_frequency_available ABI on the voltage channels only: SPD is ignored for temperature conversions, so the temperature channel deliberately carries no SAMP_FREQ attribute. A new has_speed_mode capability flag gates the feature (LTC2499); the two-byte command path is now taken for has_temp || has_speed_mode, since both features need the second config byte. The conversion-time wait becomes mode dependent: 150ms at 1x, 76ms at 2x (datasheet t_CONV max, simultaneous rejection, rounded up). The wait is keyed on the conversion currently in flight, whose duration is fixed by the mode that was active when it started - not by the newly selected mode. This matters on a 1x->2x switch: a 1x conversion may still be running when the first 2x read arrives, and reprogramming the device before it finishes would be NACKed with -EIO. Timing is centralized in ltc2497core_conv_time_ms() so a future FA/FB rejection-mode selection can extend it into a [rejection][speed] lookup without touching callers. LTC2496/LTC2497 (no speed mode) keep the single-byte path and the unchanged 150ms wait. Validated on a live LTC2499: 20 reads take ~3.1s at 1x and ~1.6s at 2x (~0.5x, no -EIO), voltage and temperature readings stay sane in both modes, and the temperature/voltage interleave (sticky-PTAT) regression still passes at 1x and 2x. Signed-off-by: Andrei Stancovici <andrei.stancovici@analog.com>
e94c70c to
0c63d33
Compare
|
v3: Minor follow-up: The overlay header still described the file as a local validation/example artifact. Since the overlay is kept as part of the delivered LTC2499 series, I updated the wording to describe its actual role and removed the obsolete "not upstream" / "validation artifact" language. No functional changes. |
2 participants
PR Description
Add LTC2499 support and validation infrastructure to the ADI Linux tree.
This PR introduces:
testing.
sampling_frequency ABI.
The temperature support adds an IIO temperature channel backed by the
device's internal PTAT sensor and includes the sticky-PTAT fix required
to reliably switch between temperature and voltage conversions.
The SPD support adds selectable 1x and 2x conversion rates and adjusts
the conversion timing accordingly. Validation on live LTC2499 hardware
demonstrated approximately a 2x throughput improvement while preserving
correct voltage and temperature operation.
The Raspberry Pi overlay is intended as a downstream validation artifact
used for LTC2499 bring-up and testing on ADI Linux platforms.
Validation
Temperature feature:
SPD feature:
Validation was performed on live LTC2499 hardware connected through an
IIO network backend.
Commits
PR Type
PR Checklist