* [PATCH 0/3] Supporting LTC4284 which is a negative voltage hotswap controller
@ 2026-11-04 3:29 Alexis Czezar Torreno
2026-11-04 3:30 ` [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 Alexis Czezar Torreno
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Alexis Czezar Torreno @ 2026-11-04 3:29 UTC (permalink / raw)
To: Nuno Sá, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan, Randy Dunlap,
Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-hwmon, devicetree, linux-kernel, linux-doc,
Alexis Czezar Torreno
LTC4284 is a negative voltage hot swap controller that drives an external
N-channel MOSFET to allow a board to be safely inserted and removed from a
live backplane.
LTC4284 is very similar to LTC4283 but for higher power. The device features
programmable current limit with foldback and independently adjustable inrush
current to optimize the MOSFET safe operating area (SOA). The SOA timer
limits MOSFET temperature rise for reliable protection against overstresses.
LTC4284 also has up to 8 pins that can be configured as GPIOs.
Uses an I2C interface and onboard ADC allow monitoring of board current,
voltage, power, energy, and fault status.
Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Alexis Czezar Torreno (3):
dt-bindings: hwmon: ltc4283: add ltc4284
hwmon: ltc4283: Add support for the LTC4284 swap controller
gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller
.../devicetree/bindings/hwmon/adi,ltc4283.yaml | 57 ++-
Documentation/hwmon/ltc4283.rst | 54 ++-
drivers/gpio/Kconfig | 4 +-
drivers/hwmon/Kconfig | 6 +-
drivers/hwmon/ltc4283.c | 421 ++++++++++++++++++---
5 files changed, 472 insertions(+), 70 deletions(-)
---
base-commit: 4b8a299cc032d3cbcc68403724740fa14ed7a592
change-id: 20261104-staging-ltc4284-adb24be174a6
Best regards,
--
Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 2026-11-04 3:29 [PATCH 0/3] Supporting LTC4284 which is a negative voltage hotswap controller Alexis Czezar Torreno @ 2026-11-04 3:30 ` Alexis Czezar Torreno 2026-11-04 3:39 ` sashiko-bot 2026-11-04 3:30 ` [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller Alexis Czezar Torreno 2026-11-04 3:30 ` [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller Alexis Czezar Torreno 2 siblings, 1 reply; 9+ messages in thread From: Alexis Czezar Torreno @ 2026-11-04 3:30 UTC (permalink / raw) To: Nuno Sá, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan, Randy Dunlap, Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-hwmon, devicetree, linux-kernel, linux-doc, Alexis Czezar Torreno The LTC4284 is a negative voltage hot swap controller that drives an external N-channel MOSFET to allow a board to be safely inserted and removed from a live backplane. Note that this device has a MODE pin that is determined by differing voltage levels (VEE, VIN, INTVCC, or left open) and cannot be controlled via software or GPIO. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> --- .../devicetree/bindings/hwmon/adi,ltc4283.yaml | 57 ++++++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml index 05e2132ad4d8621ab76ee89aa166c131fed08e85..c850390ddc4a33ed3fd877bf4620f7f009366ff5 100644 --- a/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml @@ -10,22 +10,42 @@ maintainers: - Nuno Sá <nuno.sa@analog.com> description: | - The LTC4283 negative voltage hot swap controller drives an external N-channel - MOSFET to allow a board to be safely inserted and removed from a live - backplane. + The LTC4283 and LTC4284 are negative voltage hot swap controllers that drive + external N-channel MOSFETs to allow a board to be safely inserted and removed + from a live backplane. + + The LTC4284 features dual-gate drivers with four operating modes (single + driver, parallel, high stress staged start, and low stress staged start) + configured via the hardware MODE pin. The MODE pin must be connected to the + appropriate voltage level (VEE, VIN, INTVCC, or left open) and cannot be + controlled via software or GPIO. https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4283.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4284.pdf properties: compatible: enum: - adi,ltc4283 + - adi,ltc4284 reg: maxItems: 1 adi,rsense-nano-ohms: - description: Value of the sense resistor. + description: + Value of the sense resistor. For LTC4283, this is the resistor connected + between SENSE+/SENSE- pins. For LTC4284, this is the resistor connected + between SENSE1+/SENSE1- pins. + + adi,rsense2-nano-ohms: + description: + Value of the SENSE2 current sense resistor. This resistor is connected + between SENSE2+ and SENSE2- pins. + + adi,radc-nano-ohms: + description: + Value of the effective resistance seen by the ADC+/ADC- pins. adi,current-limit-sense-microvolt: description: @@ -195,6 +215,20 @@ required: - adi,rsense-nano-ohms allOf: + - if: + properties: + compatible: + contains: + const: adi,ltc4284 + then: + required: + - adi,rsense2-nano-ohms + - adi,radc-nano-ohms + else: + properties: + adi,rsense2-nano-ohms: false + adi,radc-nano-ohms: false + - if: properties: adi,pgio1-func: @@ -269,4 +303,19 @@ examples: #gpio-cells = <2>; }; }; + + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + swap-controller@20 { + compatible = "adi,ltc4284"; + reg = <0x20>; + + adi,rsense-nano-ohms = <1000>; + adi,rsense2-nano-ohms = <1000>; + adi,radc-nano-ohms = <500>; + }; + }; ... -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 2026-11-04 3:30 ` [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 Alexis Czezar Torreno @ 2026-11-04 3:39 ` sashiko-bot 2026-11-04 4:06 ` Torreno, Alexis Czezar 0 siblings, 1 reply; 9+ messages in thread From: sashiko-bot @ 2026-11-04 3:39 UTC (permalink / raw) To: Alexis Czezar Torreno; +Cc: linux-gpio, linux-hwmon, devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] dt-bindings: hwmon: ltc4283: requiring rsense2 breaks single-driver mode - [Low] dt-bindings: hwmon: ltc4283: ltc4284 example sense resistors below min 50000 -- commit 9676b294cf2ad1168e5ce0d8de5c0cd58e68d9ed Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> dt-bindings: hwmon: ltc4283: add ltc4284 This commit adds device tree binding support for the Analog Devices LTC4284 negative voltage hot swap controller to the adi,ltc4283 schema, including new properties for its second sense resistor and ADC resistance. > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml > index 05e2132ad4d86..c850390ddc4a3 100644 > --- a/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml > @@ -10,22 +10,42 @@ maintainers: > - Nuno Sá <nuno.sa@analog.com> > > description: | > - The LTC4283 negative voltage hot swap controller drives an external N-channel > - MOSFET to allow a board to be safely inserted and removed from a live > - backplane. > + The LTC4283 and LTC4284 are negative voltage hot swap controllers that drive > + external N-channel MOSFETs to allow a board to be safely inserted and removed > + from a live backplane. > + > + The LTC4284 features dual-gate drivers with four operating modes (single > + driver, parallel, high stress staged start, and low stress staged start) > + configured via the hardware MODE pin. The MODE pin must be connected to the > + appropriate voltage level (VEE, VIN, INTVCC, or left open) and cannot be > + controlled via software or GPIO. > > https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4283.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4284.pdf > > properties: > compatible: > enum: > - adi,ltc4283 > + - adi,ltc4284 > > reg: > maxItems: 1 > > adi,rsense-nano-ohms: > - description: Value of the sense resistor. > + description: > + Value of the sense resistor. For LTC4283, this is the resistor connected > + between SENSE+/SENSE- pins. For LTC4284, this is the resistor connected > + between SENSE1+/SENSE1- pins. > + > + adi,rsense2-nano-ohms: > + description: > + Value of the SENSE2 current sense resistor. This resistor is connected > + between SENSE2+ and SENSE2- pins. > + > + adi,radc-nano-ohms: > + description: > + Value of the effective resistance seen by the ADC+/ADC- pins. > > adi,current-limit-sense-microvolt: > description: > @@ -195,6 +215,20 @@ required: > - adi,rsense-nano-ohms > > allOf: > + - if: > + properties: > + compatible: > + contains: > + const: adi,ltc4284 > + then: > + required: > + - adi,rsense2-nano-ohms > + - adi,radc-nano-ohms > + else: > + properties: > + adi,rsense2-nano-ohms: false > + adi,radc-nano-ohms: false [Severity: Medium] As noted in the binding description above, the LTC4284 supports four hardware operating modes configured via the MODE pin, including single driver mode. In single driver mode, only the first MOSFET path (GATE1 and SENSE1+/SENSE1-) is used, and no second sense resistor is populated between SENSE2+ and SENSE2-. If adi,rsense2-nano-ohms is unconditionally required in the allOf block for adi,ltc4284, wouldn't boards wired in single driver mode fail schema validation unless they specify a nonexistent adi,rsense2-nano-ohms property? > @@ -269,4 +303,19 @@ examples: > #gpio-cells = <2>; > }; > }; > + > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + swap-controller@20 { > + compatible = "adi,ltc4284"; > + reg = <0x20>; > + > + adi,rsense-nano-ohms = <1000>; > + adi,rsense2-nano-ohms = <1000>; > + adi,radc-nano-ohms = <500>; > + }; [Severity: Low] In drivers/hwmon/ltc4283.c, ltc4283_setup() validates the sense resistor properties against [LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE], where LTC4283_MIN_RSENSE is 50000 nano-ohms (50 micro-ohms) and LTC4283_MAX_RSENSE is 1677721599 nano-ohms: if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > LTC4283_MAX_RSENSE) return dev_err_probe(dev, -EINVAL, "adi,rsense-nano-ohms(%u) too small or too large [%u %u]\n", st->rsense, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); The example above sets adi,rsense-nano-ohms = <1000>, adi,rsense2-nano-ohms = <1000>, and adi,radc-nano-ohms = <500> (1 micro-ohm and 0.5 micro-ohm), which will cause ltc4283_setup() to fail probe with -EINVAL. Were these example values intended to be 1000 micro-ohms (<1000000> nano-ohms) and 500 micro-ohms (<500000> nano-ohms), and should adi,rsense-nano-ohms, adi,rsense2-nano-ohms, and adi,radc-nano-ohms specify minimum: 50000 and maximum: 1677721599 in the schema? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261104-staging-ltc4284-v1-0-49fce67ebc48@analog.com?part=1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 2026-11-04 3:39 ` sashiko-bot @ 2026-11-04 4:06 ` Torreno, Alexis Czezar 0 siblings, 0 replies; 9+ messages in thread From: Torreno, Alexis Czezar @ 2026-11-04 4:06 UTC (permalink / raw) To: sashiko-reviews@lists.linux.dev Cc: linux-gpio@vger.kernel.org, linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org > [External] > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to > consider: > - [Medium] dt-bindings: hwmon: ltc4283: requiring rsense2 breaks single-driver > mode > - [Low] dt-bindings: hwmon: ltc4283: ltc4284 example sense resistors below min > 50000 > -- > > commit 9676b294cf2ad1168e5ce0d8de5c0cd58e68d9ed > Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> > > dt-bindings: hwmon: ltc4283: add ltc4284 > > This commit adds device tree binding support for the Analog Devices LTC4284 > negative voltage hot swap controller to the adi,ltc4283 schema, including new > properties for its second sense resistor and ADC resistance. > ... > > + > > + adi,rsense2-nano-ohms: > > + description: > > + Value of the SENSE2 current sense resistor. This resistor is connected > > + between SENSE2+ and SENSE2- pins. > > + ... > > allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: adi,ltc4284 > > + then: > > + required: > > + - adi,rsense2-nano-ohms > > + - adi,radc-nano-ohms > > + else: > > + properties: > > + adi,rsense2-nano-ohms: false > > + adi,radc-nano-ohms: false > > [Severity: Medium] > As noted in the binding description above, the LTC4284 supports four hardware > operating modes configured via the MODE pin, including single driver mode. > > In single driver mode, only the first MOSFET path (GATE1 and SENSE1+/SENSE1- > ) is used, and no second sense resistor is populated between SENSE2+ and > SENSE2-. > > If adi,rsense2-nano-ohms is unconditionally required in the allOf block for > adi,ltc4284, wouldn't boards wired in single driver mode fail schema validation > unless they specify a nonexistent adi,rsense2-nano-ohms property? > I suppose I can make rsense2 optional (+ corresponding driver code change)? Or can it be kept as is? > > @@ -269,4 +303,19 @@ examples: > > #gpio-cells = <2>; > > }; > > }; > > + > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + swap-controller@20 { > > + compatible = "adi,ltc4284"; > > + reg = <0x20>; > > + > > + adi,rsense-nano-ohms = <1000>; > > + adi,rsense2-nano-ohms = <1000>; > > + adi,radc-nano-ohms = <500>; > > + }; > > [Severity: Low] > In drivers/hwmon/ltc4283.c, ltc4283_setup() validates the sense resistor > properties against [LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE], where > LTC4283_MIN_RSENSE is 50000 nano-ohms (50 micro-ohms) and > LTC4283_MAX_RSENSE is 1677721599 nano-ohms: > > if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > LTC4283_MAX_RSENSE) > return dev_err_probe(dev, -EINVAL, > "adi,rsense-nano-ohms(%u) too small or too large [%u %u]\n", > st->rsense, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); > > The example above sets adi,rsense-nano-ohms = <1000>, adi,rsense2-nano- > ohms = <1000>, and adi,radc-nano-ohms = <500> (1 micro-ohm and 0.5 micro- > ohm), which will cause ltc4283_setup() to fail probe with -EINVAL. > This was just random numbers to show the example. I can change the values to a more realistic sample if preferred. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller 2026-11-04 3:29 [PATCH 0/3] Supporting LTC4284 which is a negative voltage hotswap controller Alexis Czezar Torreno 2026-11-04 3:30 ` [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 Alexis Czezar Torreno @ 2026-11-04 3:30 ` Alexis Czezar Torreno 2026-11-04 3:41 ` sashiko-bot 2026-11-04 3:30 ` [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller Alexis Czezar Torreno 2 siblings, 1 reply; 9+ messages in thread From: Alexis Czezar Torreno @ 2026-11-04 3:30 UTC (permalink / raw) To: Nuno Sá, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan, Randy Dunlap, Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-hwmon, devicetree, linux-kernel, linux-doc, Alexis Czezar Torreno Support the LTC4284 Hot Swap Controller. It is very Similar to LTC4283 but for higher power. The device features programmable current limit with foldback and independently adjustable inrush current to optimize the MOSFET safe operating area (SOA). The SOA timer limits MOSFET temperature rise for reliable protection against overstresses. An I2C interface and onboard ADC allow monitoring of board current, voltage, power, energy, and fault status. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> --- Documentation/hwmon/ltc4283.rst | 54 ++++-- drivers/hwmon/Kconfig | 6 +- drivers/hwmon/ltc4283.c | 421 +++++++++++++++++++++++++++++++++++----- 3 files changed, 417 insertions(+), 64 deletions(-) diff --git a/Documentation/hwmon/ltc4283.rst b/Documentation/hwmon/ltc4283.rst index 99b1ee05f62993a794a7b2b3455f2b61b55cd68c..9fc615d4e729a83b3f305f7ace7b4064e88931ba 100644 --- a/Documentation/hwmon/ltc4283.rst +++ b/Documentation/hwmon/ltc4283.rst @@ -15,23 +15,37 @@ Supported chips: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4283.pdf + * Analog Devices LTC4284 + + Prefix: 'ltc4284' + + Addresses scanned: - + + Datasheet: + + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4284.pdf + Author: Nuno Sá <nuno.sa@analog.com> Description ___________ -The LTC4283 negative voltage hot swap controller drives an external N-channel -MOSFET to allow a board to be safely inserted and removed from a live backplane. -The device features programmable current limit with foldback and independently -adjustable inrush current to optimize the MOSFET safe operating area (SOA). The -SOA timer limits MOSFET temperature rise for reliable protection against -overstresses. An I2C interface and onboard gear-shift ADC allow monitoring of -board current, voltage, power, energy, and fault status. Additional features -respond to input UV/OV, interrupt the host when a fault has occurred, notify -when output power is good, detect insertion of a board, turn off the MOSFET -if an external supply monitor fails to indicate power good within a timeout -period, and auto-reboot after a programmable delay following a host commanded -turn-off. +The LTC4283 and LTC4284 are negative voltage hot swap controllers that drive +external N-channel MOSFETs to allow a board to be safely inserted and removed +from a live backplane. The devices feature programmable current limit with +foldback and independently adjustable inrush current to optimize the MOSFET +safe operating area (SOA). The SOA timer limits MOSFET temperature rise for +reliable protection against overstresses. An I2C interface and onboard +gear-shift ADC allow monitoring of board current, voltage, power, energy, and +fault status. Additional features respond to input UV/OV, interrupt the host +when a fault has occurred, notify when output power is good, detect insertion +of a board, turn off the MOSFET if an external supply monitor fails to indicate +power good within a timeout period, and auto-reboot after a programmable delay +following a host commanded turn-off. + +The LTC4284 extends the LTC4283 with dual-gate drivers for parallel or staged +start operation modes, configured via the MODE pin. It also provides additional +current sense channels (SENSE1 and SENSE2) for monitoring multiple load paths. Sysfs entries _____________ @@ -41,6 +55,11 @@ attributes are read-only. Note that the VADIOx channels might not be available if the ADIO pins are used as GPIOs (naturally also affects the respective differential channels). +.. note:: + LTC4284 provides two current sense channels (curr1, curr2) for + monitoring SENSE1+/SENSE1- and SENSE2+/SENSE2- respectively. LTC4283 only + provides curr1 for monitoring the main sense input (SENSE+/SENSE-). + ======================= ========================================== in0_lcrit_alarm Critical Undervoltage alarm in0_crit_alarm Critical Overvoltage alarm @@ -225,6 +244,17 @@ curr1_max_alarm Overcurrent alarm curr1_crit_alarm Critical Overcurrent alarm curr1_label Channel label (ISENSE) +curr2_input Sense current (mA) +curr2_min Undercurrent threshold +curr2_max Overcurrent threshold +curr2_lowest Lowest measured current +curr2_highest Highest measured current +curr2_reset_history Write 1 to reset curr2 history. +curr2_min_alarm Undercurrent alarm +curr2_max_alarm Overcurrent alarm +curr2_crit_alarm Critical Overcurrent alarm +curr2_label Channel label (ISENSE2) + power1_input Power (in uW) power1_min Low power threshold power1_max High power threshold diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 3ae676a56d95ca2664c27f9cd82d6c75fd15b7a7..0e5f2fd12d08b609391d520853eeef4203479fac 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1256,13 +1256,13 @@ config SENSORS_LTC4282 be called ltc4282. config SENSORS_LTC4283 - tristate "Analog Devices LTC4283" + tristate "Analog Devices LTC4283 and similar devices" depends on I2C select REGMAP_I2C select AUXILIARY_BUS help - If you say yes here you get support for Analog Devices LTC4283 - Negative Voltage Hot Swap Controller I2C interface. + If you say yes here you get support for Analog Devices LTC4283 and + similar devices Negative Voltage Hot Swap Controller I2C interface. This driver can also be built as a module. If so, the module will be called ltc4283. diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c index 9ec409bc26482260e696bf7a6c40b0dc9e1f55b8..d27c21114615245e8c755f1ae0fede0ee3ee810d 100644 --- a/drivers/hwmon/ltc4283.c +++ b/drivers/hwmon/ltc4283.c @@ -48,6 +48,10 @@ #define LTC4283_ADC_ALM_LOG_2 0x06 #define LTC4283_ADC_ALM_LOG_3 0x07 #define LTC4283_ADC_ALM_LOG_4 0x08 +#define LTC4284_SENSE2_LOW_ALM BIT(0) +#define LTC4284_SENSE2_HIGH_ALM BIT(1) +#define LTC4284_SENSE1_LOW_ALM BIT(2) +#define LTC4284_SENSE1_HIGH_ALM BIT(3) #define LTC4283_ADC_ALM_LOG_5 0x09 #define LTC4283_CONTROL_1 0x0a #define LTC4283_RW_PAGE_MASK BIT(0) @@ -81,8 +85,16 @@ #define LTC4283_ADC_MASK GENMASK(2, 0) #define LTC4283_ADC_SELECT(c) (0x13 + (c) / 8) #define LTC4283_ADC_SELECT_MASK(c) BIT((c) % 8) +#define LTC4283_ADC_SELECT_2 0x14 +#define LTC4284_SENSE1_SELECT_MASK BIT(2) +#define LTC4284_SENSE2_SELECT_MASK BIT(3) #define LTC4283_SENSE_MIN_TH 0x1b #define LTC4283_SENSE_MAX_TH 0x1c +/* LTC4284 only - dual sense channel thresholds */ +#define LTC4284_SENSE1_MIN_TH 0x35 +#define LTC4284_SENSE1_MAX_TH 0x36 +#define LTC4284_SENSE2_MIN_TH 0x37 +#define LTC4284_SENSE2_MAX_TH 0x38 #define LTC4283_VPWR_MIN_TH 0x1d #define LTC4283_VPWR_MAX_TH 0x1e #define LTC4283_POWER_MIN_TH 0x1f @@ -102,6 +114,13 @@ #define LTC4283_POWER_MAX 0x49 #define LTC4283_RESERVED_68 0x68 #define LTC4283_RESERVED_6D 0x6D +/* LTC4284 only - dual sense channels */ +#define LTC4284_SENSE1 0x68 +#define LTC4284_SENSE1_MIN 0x69 +#define LTC4284_SENSE1_MAX 0x6A +#define LTC4284_SENSE2 0x6B +#define LTC4284_SENSE2_MIN 0x6C +#define LTC4284_SENSE2_MAX 0x6D /* get channels from ADC 2 */ #define LTC4283_ADC_2(c) (0x4a + (c) * 3) #define LTC4283_ADC_2_MIN(c) (0x4b + (c) * 3) @@ -173,7 +192,19 @@ enum { #define LTC4283_ADIO34_MAX \ LTC4283_ADC_2_MAX_DIFF(LTC4283_CHAN_ADIO34 - LTC4283_CHAN_ADIN12) +enum ltc4283_chip_id { + ID_LTC4283, + ID_LTC4284, +}; + +struct ltc4283_chip_info { + enum ltc4283_chip_id id; + const char *name; + const struct hwmon_chip_info *hwmon_info; +}; + struct ltc4283_hwmon { + const struct ltc4283_chip_info *info; struct regmap *map; struct i2c_client *client; unsigned long gpio_mask; @@ -182,12 +213,118 @@ struct ltc4283_hwmon { unsigned long power_max; /* in millivolt */ u32 vsense_max; - /* in tenths of microohm*/ + /* in tenths of microohm - For LTC4283 SENSE or LTC4284 SENSE1 */ u32 rsense; + /* in tenths of microohm - For LTC4284 SENSE2 only */ + u32 rsense2; + /* in tenths of microohm - For LTC4284 ADC power measurement */ + u32 radc; bool energy_en; bool ext_fault; }; +static u32 ltc4283_get_rsense(const struct ltc4283_hwmon *st, u32 channel) +{ + if (channel) + return st->rsense2; + + return st->rsense; +} + +static u32 ltc4283_get_rpower(const struct ltc4283_hwmon *st) +{ + /* For LTC4284, ADC uses radc. For LTC4283, ADC uses rsense. */ + if (st->info->id == ID_LTC4284) + return st->radc; + + return st->rsense; +} + +static u8 ltc4283_get_sense_reg(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE; + + if (!channel) + return LTC4284_SENSE1; + + return LTC4284_SENSE2; +} + +static u8 ltc4283_get_sense_min_reg(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_MIN; + + if (!channel) + return LTC4284_SENSE1_MIN; + + return LTC4284_SENSE2_MIN; +} + +static u8 ltc4283_get_sense_max_reg(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_MAX; + + if (!channel) + return LTC4284_SENSE1_MAX; + + return LTC4284_SENSE2_MAX; +} + +static u8 ltc4283_get_sense_min_th_reg(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_MIN_TH; + + if (!channel) + return LTC4284_SENSE1_MIN_TH; + + return LTC4284_SENSE2_MIN_TH; +} + +static u8 ltc4283_get_sense_max_th_reg(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_MAX_TH; + + if (!channel) + return LTC4284_SENSE1_MAX_TH; + + return LTC4284_SENSE2_MAX_TH; +} + +static u8 ltc4283_get_curr_alarm_reg(const struct ltc4283_hwmon *st) +{ + if (st->info->id == ID_LTC4284) + return LTC4283_ADC_ALM_LOG_4; + + return LTC4283_ADC_ALM_LOG_1; +} + +static u8 ltc4283_get_curr_alarm_high_mask(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_HIGH_ALM; + + if (!channel) + return LTC4284_SENSE1_HIGH_ALM; + + return LTC4284_SENSE2_HIGH_ALM; +} + +static u8 ltc4283_get_curr_alarm_low_mask(const struct ltc4283_hwmon *st, u32 channel) +{ + if (st->info->id == ID_LTC4283) + return LTC4283_SENSE_LOW_ALM; + + if (!channel) + return LTC4284_SENSE1_LOW_ALM; + + return LTC4284_SENSE2_LOW_ALM; +} + static int ltc4283_read_voltage_word(const struct ltc4283_hwmon *st, u32 reg, u32 fs, long *val) { @@ -380,56 +517,63 @@ static int ltc4283_read_in(struct ltc4283_hwmon *st, u32 attr, u32 channel, } static int ltc4283_read_current_word(const struct ltc4283_hwmon *st, u32 reg, - long *val) + u32 channel, long *val) { u64 temp = (u64)LTC4283_ADC1_FS_uV * DECA * MILLI; unsigned int __raw; + u32 rsense; int ret; ret = regmap_read(st->map, reg, &__raw); if (ret) return ret; - *val = DIV64_U64_ROUND_CLOSEST(__raw * temp, - BIT_ULL(16) * st->rsense); + rsense = ltc4283_get_rsense(st, channel); + *val = DIV64_U64_ROUND_CLOSEST(__raw * temp, BIT_ULL(16) * rsense); return 0; } static int ltc4283_read_current_byte(const struct ltc4283_hwmon *st, u32 reg, - long *val) + u32 channel, long *val) { u64 temp = (u64)LTC4283_ADC1_FS_uV * DECA * MILLI; - u32 curr; + u32 curr, rsense; int ret; ret = regmap_read(st->map, reg, &curr); if (ret) return ret; - *val = DIV_ROUND_CLOSEST_ULL(curr * temp, BIT(8) * st->rsense); + rsense = ltc4283_get_rsense(st, channel); + *val = DIV_ROUND_CLOSEST_ULL(curr * temp, BIT(8) * rsense); return 0; } -static int ltc4283_read_curr(struct ltc4283_hwmon *st, u32 attr, long *val) +static int ltc4283_read_curr(struct ltc4283_hwmon *st, u32 attr, u32 channel, long *val) { switch (attr) { case hwmon_curr_input: - return ltc4283_read_current_word(st, LTC4283_SENSE, val); + return ltc4283_read_current_word(st, ltc4283_get_sense_reg(st, channel), + channel, val); case hwmon_curr_highest: - return ltc4283_read_current_word(st, LTC4283_SENSE_MAX, val); + return ltc4283_read_current_word(st, ltc4283_get_sense_max_reg(st, channel), + channel, val); case hwmon_curr_lowest: - return ltc4283_read_current_word(st, LTC4283_SENSE_MIN, val); + return ltc4283_read_current_word(st, ltc4283_get_sense_min_reg(st, channel), + channel, val); case hwmon_curr_max: - return ltc4283_read_current_byte(st, LTC4283_SENSE_MAX_TH, val); + return ltc4283_read_current_byte(st, ltc4283_get_sense_max_th_reg(st, channel), + channel, val); case hwmon_curr_min: - return ltc4283_read_current_byte(st, LTC4283_SENSE_MIN_TH, val); + return ltc4283_read_current_byte(st, ltc4283_get_sense_min_th_reg(st, channel), + channel, val); case hwmon_curr_max_alarm: - return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1, - LTC4283_SENSE_HIGH_ALM, val); + return ltc4283_read_alarm(st, ltc4283_get_curr_alarm_reg(st), + ltc4283_get_curr_alarm_high_mask(st, channel), val); case hwmon_curr_min_alarm: - return ltc4283_read_alarm(st, LTC4283_ADC_ALM_LOG_1, - LTC4283_SENSE_LOW_ALM, val); + return ltc4283_read_alarm(st, ltc4283_get_curr_alarm_reg(st), + ltc4283_get_curr_alarm_low_mask(st, channel), val); case hwmon_curr_crit_alarm: return ltc4283_read_alarm(st, LTC4283_FAULT_STATUS, LTC4283_OC_MASK, val); @@ -443,6 +587,7 @@ static int ltc4283_read_power_word(const struct ltc4283_hwmon *st, { u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI; unsigned int __raw; + u32 resistor = ltc4283_get_rpower(st); int ret; ret = regmap_read(st->map, reg, &__raw); @@ -453,7 +598,7 @@ static int ltc4283_read_power_word(const struct ltc4283_hwmon *st, * Power is given by: * P = CODE(16b) * 32.768mV * 2.048V / (2^16 * Rsense) */ - *val = DIV64_U64_ROUND_CLOSEST(temp * __raw, BIT_ULL(16) * st->rsense); + *val = DIV64_U64_ROUND_CLOSEST(temp * __raw, BIT_ULL(16) * resistor); return 0; } @@ -462,6 +607,7 @@ static int ltc4283_read_power_byte(const struct ltc4283_hwmon *st, u32 reg, long *val) { u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI; + u32 resistor = ltc4283_get_rpower(st); u32 power; int ret; @@ -469,7 +615,7 @@ static int ltc4283_read_power_byte(const struct ltc4283_hwmon *st, if (ret) return ret; - *val = DIV_ROUND_CLOSEST_ULL(power * temp, BIT(8) * st->rsense); + *val = DIV_ROUND_CLOSEST_ULL(power * temp, BIT(8) * resistor); return 0; } @@ -501,6 +647,7 @@ static int ltc4283_read_power(struct ltc4283_hwmon *st, u32 attr, long *val) static int ltc4283_read_energy(struct ltc4283_hwmon *st, u32 attr, s64 *val) { u64 temp = LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV, energy; + u32 resistor = ltc4283_get_rpower(st); u8 raw[8] = {}; int ret; @@ -517,20 +664,22 @@ static int ltc4283_read_energy(struct ltc4283_hwmon *st, u32 attr, s64 *val) /* * The formula for energy is given by: - * E = CODE(48b) * 32.768mV * 2.048V * Tconv / 2^24 * Rsense + * E = CODE(48b) * 32.768mV * 2.048V * Tconv / 2^24 * R * - * As Rsense can have tenths of micro-ohm resolution, we need to - * multiply by DECA to get microjoule. + * Energy is accumulated power, so we use the same resistor as power + * measurements (rsense for LTC4283, radc for LTC4284). As the resistor + * can have tenths of micro-ohm resolution, we need to multiply by DECA + * to get microjoule. */ /* * Use mul_u64_u64_div_u64() to handle the 128-bit intermediate * product of energy (up to 48 bits) * temp * Tconv without overflow. - * Multiply rsense by CENTI to convert from tenths-of-microohm back + * Multiply resistor by CENTI to convert from tenths-of-microohm back * to nanoohm so the result comes out in microjoule. */ energy = mul_u64_u64_div_u64(energy, temp * LTC4283_TCONV_uS, - BIT_ULL(24) * st->rsense * CENTI); + BIT_ULL(24) * resistor * CENTI); *val = energy; return 0; @@ -545,7 +694,7 @@ static int ltc4283_read(struct device *dev, enum hwmon_sensor_types type, case hwmon_in: return ltc4283_read_in(st, attr, channel, val); case hwmon_curr: - return ltc4283_read_curr(st, attr, val); + return ltc4283_read_curr(st, attr, channel, val); case hwmon_power: return ltc4283_read_power(st, attr, val); case hwmon_energy: @@ -562,10 +711,11 @@ static int ltc4283_write_power_byte(const struct ltc4283_hwmon *st, u32 reg, long val) { u64 temp = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI; + u32 resistor = ltc4283_get_rpower(st); u32 __raw; val = clamp_val(val, 0, st->power_max); - __raw = DIV64_U64_ROUND_CLOSEST(val * BIT_ULL(8) * st->rsense, temp); + __raw = DIV64_U64_ROUND_CLOSEST(val * BIT_ULL(8) * resistor, temp); return regmap_write(st->map, reg, __raw); } @@ -574,9 +724,10 @@ static int ltc4283_write_power_word(const struct ltc4283_hwmon *st, u32 reg, unsigned long val) { u64 divisor = (u64)LTC4283_ADC1_FS_uV * LTC4283_ADC2_FS_mV * DECA * MILLI; + u32 resistor = ltc4283_get_rpower(st); u16 __raw; - __raw = mul_u64_u64_div_u64(val, st->rsense * BIT_ULL(16), divisor); + __raw = mul_u64_u64_div_u64(val, resistor * BIT_ULL(16), divisor); return regmap_write(st->map, reg, __raw); } @@ -757,23 +908,24 @@ static int ltc4283_write_in(struct ltc4283_hwmon *st, u32 attr, long val, } static int ltc4283_write_curr_byte(const struct ltc4283_hwmon *st, - u32 reg, long val) + u32 reg, u32 channel, long val) { u32 temp = LTC4283_ADC1_FS_uV * DECA * MILLI; - u32 reg_val, isense_max; + u32 reg_val, isense_max, rsense; - isense_max = DIV_ROUND_CLOSEST(st->vsense_max * MICRO * DECA, st->rsense); + rsense = ltc4283_get_rsense(st, channel); + isense_max = DIV_ROUND_CLOSEST(st->vsense_max * MICRO * DECA, rsense); val = clamp_val(val, 0, isense_max); - reg_val = DIV_ROUND_CLOSEST_ULL(val * BIT_ULL(8) * st->rsense, temp); + reg_val = DIV_ROUND_CLOSEST_ULL(val * BIT_ULL(8) * rsense, temp); return regmap_write(st->map, reg, reg_val); } -static int ltc4283_write_curr_history(struct ltc4283_hwmon *st) +static int ltc4283_write_curr_history(struct ltc4283_hwmon *st, u32 channel) { int ret; - ret = ltc4283_write_in_history(st, LTC4283_SENSE_MIN, + ret = ltc4283_write_in_history(st, ltc4283_get_sense_min_reg(st, channel), st->vsense_max * MILLI, LTC4283_ADC1_FS_uV); if (ret) @@ -784,15 +936,17 @@ static int ltc4283_write_curr_history(struct ltc4283_hwmon *st) LTC4283_OC_FAULT_MASK); } -static int ltc4283_write_curr(struct ltc4283_hwmon *st, u32 attr, long val) +static int ltc4283_write_curr(struct ltc4283_hwmon *st, u32 attr, u32 channel, long val) { switch (attr) { case hwmon_curr_max: - return ltc4283_write_curr_byte(st, LTC4283_SENSE_MAX_TH, val); + return ltc4283_write_curr_byte(st, ltc4283_get_sense_max_th_reg(st, channel), + channel, val); case hwmon_curr_min: - return ltc4283_write_curr_byte(st, LTC4283_SENSE_MIN_TH, val); + return ltc4283_write_curr_byte(st, ltc4283_get_sense_min_th_reg(st, channel), + channel, val); case hwmon_curr_reset_history: - return ltc4283_write_curr_history(st); + return ltc4283_write_curr_history(st, channel); default: return -EOPNOTSUPP; } @@ -826,7 +980,7 @@ static int ltc4283_write(struct device *dev, enum hwmon_sensor_types type, case hwmon_in: return ltc4283_write_in(st, attr, val, channel); case hwmon_curr: - return ltc4283_write_curr(st, attr, val); + return ltc4283_write_curr(st, attr, channel, val); case hwmon_energy: return ltc4283_energy_enable_set(st, val); default: @@ -945,6 +1099,11 @@ static const char * const ltc4283_in_strs[] = { "ADIO2-ADIO1", "ADIO4-ADIO3" }; +/* LTC4283 has 1 channel (SENSE), LTC4284 has 2 channels (SENSE1, SENSE2) */ +static const char * const ltc4284_curr_strs[] = { + "ISENSE", "ISENSE2" +}; + static int ltc4283_read_labels(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, const char **str) @@ -954,7 +1113,7 @@ static int ltc4283_read_labels(struct device *dev, *str = ltc4283_in_strs[channel]; return 0; case hwmon_curr: - *str = "ISENSE"; + *str = ltc4284_curr_strs[channel]; return 0; case hwmon_power: *str = "Power"; @@ -974,15 +1133,32 @@ static int ltc4283_read_labels(struct device *dev, static int ltc4283_set_max_limits(struct ltc4283_hwmon *st, struct device *dev) { u32 temp = st->vsense_max * DECA * MICRO; + u32 resistor = ltc4283_get_rpower(st); int ret; - ret = ltc4283_write_in_byte(st, LTC4283_SENSE_MAX_TH, LTC4283_ADC1_FS_uV, - st->vsense_max * MILLI); - if (ret) - return ret; + if (st->info->id == ID_LTC4283) { + ret = ltc4283_write_in_byte(st, LTC4283_SENSE_MAX_TH, LTC4283_ADC1_FS_uV, + st->vsense_max * MILLI); + if (ret) + return ret; + } else { + /* LTC4284: Set max limits for SENSE1 and SENSE2 */ + ret = ltc4283_write_in_byte(st, LTC4284_SENSE1_MAX_TH, + LTC4283_ADC1_FS_uV, + st->vsense_max * MILLI); + if (ret) + return ret; + + ret = ltc4283_write_in_byte(st, LTC4284_SENSE2_MAX_TH, + LTC4283_ADC1_FS_uV, + st->vsense_max * MILLI); + if (ret) + return ret; + } /* Power is given by ISENSE * Vout. */ - st->power_max = DIV_ROUND_CLOSEST(temp, st->rsense) * LTC4283_ADC2_FS_mV; + st->power_max = DIV_ROUND_CLOSEST(temp, resistor) * LTC4283_ADC2_FS_mV; + return ltc4283_write_power_byte(st, LTC4283_POWER_MAX_TH, st->power_max); } @@ -1271,6 +1447,45 @@ static int ltc4283_setup(struct ltc4283_hwmon *st, struct device *dev) */ st->rsense /= CENTI; + /* + * For LTC4284, rsense is for SENSE1, rsense2 for SENSE2, and radc is + * the effective sense resistance for ADC+- pin. + */ + if (st->info->id == ID_LTC4284) { + ret = device_property_read_u32(dev, "adi,rsense2-nano-ohms", + &st->rsense2); + if (ret) + return dev_err_probe(dev, ret, + "Failed to read adi,rsense2-nano-ohms\n"); + + if (st->rsense2 < LTC4283_MIN_RSENSE || st->rsense2 > LTC4283_MAX_RSENSE) + return dev_err_probe(dev, -EINVAL, + "adi,rsense2-nano-ohms(%u) too small or too large [%u %u]\n", + st->rsense2, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); + + st->rsense2 /= CENTI; + + ret = device_property_read_u32(dev, "adi,radc-nano-ohms", + &st->radc); + if (ret) + return dev_err_probe(dev, ret, + "Failed to read adi,radc-nano-ohms\n"); + + if (st->radc < LTC4283_MIN_RSENSE || st->radc > LTC4283_MAX_RSENSE) + return dev_err_probe(dev, -EINVAL, + "adi,radc-nano-ohms(%u) too small or too large [%u %u]\n", + st->radc, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); + + st->radc /= CENTI; + + /* Enable SENSE1 and SENSE2 */ + ret = regmap_set_bits(st->map, LTC4283_ADC_SELECT_2, + LTC4284_SENSE1_SELECT_MASK | + LTC4284_SENSE2_SELECT_MASK); + if (ret) + return ret; + } + ret = device_property_read_u32(dev, "adi,current-limit-sense-microvolt", &st->vsense_max); if (!ret) { @@ -1558,6 +1773,91 @@ static const struct hwmon_channel_info * const ltc4283_info[] = { NULL }; +static const struct hwmon_channel_info * const ltc4284_info[] = { + HWMON_CHANNEL_INFO(in, + HWMON_I_LCRIT_ALARM | HWMON_I_CRIT_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_MAX_ALARM | HWMON_I_RESET_HISTORY | + HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_FAULT | HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL, + HWMON_I_INPUT | HWMON_I_LOWEST | HWMON_I_HIGHEST | + HWMON_I_MAX | HWMON_I_MIN | HWMON_I_MIN_ALARM | + HWMON_I_RESET_HISTORY | HWMON_I_MAX_ALARM | + HWMON_I_ENABLE | HWMON_I_LABEL), + HWMON_CHANNEL_INFO(curr, + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM | + HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM | + HWMON_C_RESET_HISTORY | HWMON_C_LABEL, + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM | + HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM | + HWMON_C_RESET_HISTORY | HWMON_C_LABEL), + HWMON_CHANNEL_INFO(power, + HWMON_P_INPUT | HWMON_P_INPUT_LOWEST | + HWMON_P_INPUT_HIGHEST | HWMON_P_MAX | HWMON_P_MIN | + HWMON_P_MAX_ALARM | HWMON_P_MIN_ALARM | + HWMON_P_RESET_HISTORY | HWMON_P_LABEL), + HWMON_CHANNEL_INFO(energy, + HWMON_E_ENABLE), + HWMON_CHANNEL_INFO(energy64, + HWMON_E_INPUT), + NULL +}; + static const struct hwmon_ops ltc4283_ops = { .read = ltc4283_read, .write = ltc4283_write, @@ -1565,11 +1865,16 @@ static const struct hwmon_ops ltc4283_ops = { .read_string = ltc4283_read_labels, }; -static const struct hwmon_chip_info ltc4283_chip_info = { +static const struct hwmon_chip_info ltc4283_hwmon_chip_info = { .ops = <c4283_ops, .info = ltc4283_info, }; +static const struct hwmon_chip_info ltc4284_hwmon_chip_info = { + .ops = <c4283_ops, + .info = ltc4284_info, +}; + static int ltc4283_show_fault_log(void *arg, u64 *val, u32 mask) { struct ltc4283_hwmon *st = arg; @@ -1720,6 +2025,18 @@ static const struct regmap_config ltc4283_regmap_config = { .writeable_reg = ltc4283_writable_reg, }; +static const struct ltc4283_chip_info chip_info_ltc4283 = { + .id = ID_LTC4283, + .name = "ltc4283", + .hwmon_info = <c4283_hwmon_chip_info, +}; + +static const struct ltc4283_chip_info chip_info_ltc4284 = { + .id = ID_LTC4284, + .name = "ltc4284", + .hwmon_info = <c4284_hwmon_chip_info, +}; + static int ltc4283_probe(struct i2c_client *client) { struct device *dev = &client->dev, *hwmon; @@ -1731,6 +2048,10 @@ static int ltc4283_probe(struct i2c_client *client) if (!st) return -ENOMEM; + st->info = i2c_get_match_data(client); + if (!st->info) + return -ENODEV; + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | @@ -1748,8 +2069,8 @@ static int ltc4283_probe(struct i2c_client *client) if (ret) return ret; - hwmon = devm_hwmon_device_register_with_info(dev, "ltc4283", st, - <c4283_chip_info, NULL); + hwmon = devm_hwmon_device_register_with_info(dev, st->info->name, st, + st->info->hwmon_info, NULL); if (IS_ERR(hwmon)) return PTR_ERR(hwmon); @@ -1769,13 +2090,15 @@ static int ltc4283_probe(struct i2c_client *client) } static const struct of_device_id ltc4283_of_match[] = { - { .compatible = "adi,ltc4283" }, + { .compatible = "adi,ltc4283", .data = &chip_info_ltc4283 }, + { .compatible = "adi,ltc4284", .data = &chip_info_ltc4284 }, { } }; MODULE_DEVICE_TABLE(of, ltc4283_of_match); static const struct i2c_device_id ltc4283_i2c_id[] = { - { "ltc4283" }, + { "ltc4283", (kernel_ulong_t)&chip_info_ltc4283 }, + { "ltc4284", (kernel_ulong_t)&chip_info_ltc4284 }, { } }; MODULE_DEVICE_TABLE(i2c, ltc4283_i2c_id); -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller 2026-11-04 3:30 ` [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller Alexis Czezar Torreno @ 2026-11-04 3:41 ` sashiko-bot 2026-11-04 4:58 ` Torreno, Alexis Czezar 0 siblings, 1 reply; 9+ messages in thread From: sashiko-bot @ 2026-11-04 3:41 UTC (permalink / raw) To: Alexis Czezar Torreno; +Cc: linux-gpio, linux-hwmon, devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Low] hwmon: ltc4283: positional initializers in ltc4283_i2c_id table entries - [Low] hwmon: ltc4283: duplicated channel info table and chip-ID helpers - [Low] hwmon: ltc4283: missing default resistor fallback in ltc4283_setup() - [Low] hwmon: ltc4283: commit message omits functional differences for LTC4284 -- commit 07a1775aa285e31dccd905e19fc784e472406b97 Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> hwmon: ltc4283: Add support for the LTC4284 swap controller This commit adds support for the Analog Devices LTC4284 negative voltage hot swap controller to the ltc4283 hwmon driver, including dual current sense channels and separate ADC power sense resistor handling. > hwmon: ltc4283: Add support for the LTC4284 swap controller > > Support the LTC4284 Hot Swap Controller. It is very Similar to LTC4283 but > for higher power. The device features programmable current limit with > foldback and independently adjustable inrush current to optimize the > MOSFET safe operating area (SOA). The SOA timer limits MOSFET temperature > rise for reliable protection against overstresses. > > An I2C interface and onboard ADC allow monitoring of board current, > voltage, power, energy, and fault status. > > Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> [Severity: Low] This isn't a bug, but the commit message mostly repeats generic hot-swap controller features already shared with the LTC4283. Could the changelog also describe the functional differences added to the driver for the LTC4284, specifically the two current sense channels (SENSE1 and SENSE2) with separate sense resistors (rsense and rsense2) and the dedicated ADC sense resistor (radc) used for power and energy calculations? > diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c > index 9ec409bc26482..d27c211146152 100644 > --- a/drivers/hwmon/ltc4283.c > +++ b/drivers/hwmon/ltc4283.c [ ... ] > @@ -173,7 +192,19 @@ enum { > #define LTC4283_ADIO34_MAX \ > LTC4283_ADC_2_MAX_DIFF(LTC4283_CHAN_ADIO34 - LTC4283_CHAN_ADIN12) > > +enum ltc4283_chip_id { > + ID_LTC4283, > + ID_LTC4284, > +}; > + > +struct ltc4283_chip_info { > + enum ltc4283_chip_id id; > + const char *name; > + const struct hwmon_chip_info *hwmon_info; > +}; > + > struct ltc4283_hwmon { > + const struct ltc4283_chip_info *info; > struct regmap *map; > struct i2c_client *client; > unsigned long gpio_mask; > @@ -182,12 +213,118 @@ struct ltc4283_hwmon { > unsigned long power_max; > /* in millivolt */ > u32 vsense_max; > - /* in tenths of microohm*/ > + /* in tenths of microohm - For LTC4283 SENSE or LTC4284 SENSE1 */ > u32 rsense; > + /* in tenths of microohm - For LTC4284 SENSE2 only */ > + u32 rsense2; > + /* in tenths of microohm - For LTC4284 ADC power measurement */ > + u32 radc; > bool energy_en; > bool ext_fault; > }; > > +static u32 ltc4283_get_rsense(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (channel) > + return st->rsense2; > + > + return st->rsense; > +} > + > +static u32 ltc4283_get_rpower(const struct ltc4283_hwmon *st) > +{ > + /* For LTC4284, ADC uses radc. For LTC4283, ADC uses rsense. */ > + if (st->info->id == ID_LTC4284) > + return st->radc; > + > + return st->rsense; > +} > + > +static u8 ltc4283_get_sense_reg(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE; > + > + if (!channel) > + return LTC4284_SENSE1; > + > + return LTC4284_SENSE2; > +} > + > +static u8 ltc4283_get_sense_min_reg(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_MIN; > + > + if (!channel) > + return LTC4284_SENSE1_MIN; > + > + return LTC4284_SENSE2_MIN; > +} > + > +static u8 ltc4283_get_sense_max_reg(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_MAX; > + > + if (!channel) > + return LTC4284_SENSE1_MAX; > + > + return LTC4284_SENSE2_MAX; > +} > + > +static u8 ltc4283_get_sense_min_th_reg(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_MIN_TH; > + > + if (!channel) > + return LTC4284_SENSE1_MIN_TH; > + > + return LTC4284_SENSE2_MIN_TH; > +} > + > +static u8 ltc4283_get_sense_max_th_reg(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_MAX_TH; > + > + if (!channel) > + return LTC4284_SENSE1_MAX_TH; > + > + return LTC4284_SENSE2_MAX_TH; > +} > + > +static u8 ltc4283_get_curr_alarm_reg(const struct ltc4283_hwmon *st) > +{ > + if (st->info->id == ID_LTC4284) > + return LTC4283_ADC_ALM_LOG_4; > + > + return LTC4283_ADC_ALM_LOG_1; > +} > + > +static u8 ltc4283_get_curr_alarm_high_mask(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_HIGH_ALM; > + > + if (!channel) > + return LTC4284_SENSE1_HIGH_ALM; > + > + return LTC4284_SENSE2_HIGH_ALM; > +} > + > +static u8 ltc4283_get_curr_alarm_low_mask(const struct ltc4283_hwmon *st, u32 channel) > +{ > + if (st->info->id == ID_LTC4283) > + return LTC4283_SENSE_LOW_ALM; > + > + if (!channel) > + return LTC4284_SENSE1_LOW_ALM; > + > + return LTC4284_SENSE2_LOW_ALM; > +} [ ... ] > @@ -974,15 +1133,32 @@ static int ltc4283_read_labels(struct device *dev, > static int ltc4283_set_max_limits(struct ltc4283_hwmon *st, struct device *dev) > { > u32 temp = st->vsense_max * DECA * MICRO; > + u32 resistor = ltc4283_get_rpower(st); > int ret; > > - ret = ltc4283_write_in_byte(st, LTC4283_SENSE_MAX_TH, LTC4283_ADC1_FS_uV, > - st->vsense_max * MILLI); > - if (ret) > - return ret; > + if (st->info->id == ID_LTC4283) { > + ret = ltc4283_write_in_byte(st, LTC4283_SENSE_MAX_TH, LTC4283_ADC1_FS_uV, > + st->vsense_max * MILLI); > + if (ret) > + return ret; > + } else { > + /* LTC4284: Set max limits for SENSE1 and SENSE2 */ > + ret = ltc4283_write_in_byte(st, LTC4284_SENSE1_MAX_TH, > + LTC4283_ADC1_FS_uV, > + st->vsense_max * MILLI); > + if (ret) > + return ret; > + > + ret = ltc4283_write_in_byte(st, LTC4284_SENSE2_MAX_TH, > + LTC4283_ADC1_FS_uV, > + st->vsense_max * MILLI); > + if (ret) > + return ret; > + } > > /* Power is given by ISENSE * Vout. */ > - st->power_max = DIV_ROUND_CLOSEST(temp, st->rsense) * LTC4283_ADC2_FS_mV; > + st->power_max = DIV_ROUND_CLOSEST(temp, resistor) * LTC4283_ADC2_FS_mV; > + > return ltc4283_write_power_byte(st, LTC4283_POWER_MAX_TH, st->power_max); > } > > @@ -1271,6 +1447,45 @@ static int ltc4283_setup(struct ltc4283_hwmon *st, struct device *dev) > */ > st->rsense /= CENTI; > > + /* > + * For LTC4284, rsense is for SENSE1, rsense2 for SENSE2, and radc is > + * the effective sense resistance for ADC+- pin. > + */ > + if (st->info->id == ID_LTC4284) { > + ret = device_property_read_u32(dev, "adi,rsense2-nano-ohms", > + &st->rsense2); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to read adi,rsense2-nano-ohms\n"); > + > + if (st->rsense2 < LTC4283_MIN_RSENSE || st->rsense2 > LTC4283_MAX_RSENSE) > + return dev_err_probe(dev, -EINVAL, > + "adi,rsense2-nano-ohms(%u) too small or too large [%u %u]\n", > + st->rsense2, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); > + > + st->rsense2 /= CENTI; > + > + ret = device_property_read_u32(dev, "adi,radc-nano-ohms", > + &st->radc); > + if (ret) > + return dev_err_probe(dev, ret, > + "Failed to read adi,radc-nano-ohms\n"); > + > + if (st->radc < LTC4283_MIN_RSENSE || st->radc > LTC4283_MAX_RSENSE) > + return dev_err_probe(dev, -EINVAL, > + "adi,radc-nano-ohms(%u) too small or too large [%u %u]\n", > + st->radc, LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE); > + > + st->radc /= CENTI; [Severity: Low] In ltc4283_setup(), st->rsense is initialized to LTC4283_MIN_RSENSE before calling device_property_read_u32(dev, "adi,rsense-nano-ohms", &st->rsense) so that the driver can probe without firmware properties: st->rsense = LTC4283_MIN_RSENSE; ret = device_property_read_u32(dev, "adi,rsense-nano-ohms", &st->rsense); if (!ret) { if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > LTC4283_MAX_RSENSE) return dev_err_probe(...); } When an LTC4284 device is instantiated via ltc4283_i2c_id without firmware properties, won't ltc4283_probe() -> ltc4283_setup() fail because device_property_read_u32() is required to succeed for adi,rsense2-nano-ohms and adi,radc-nano-ohms instead of defaulting to LTC4283_MIN_RSENSE? > + > + /* Enable SENSE1 and SENSE2 */ > + ret = regmap_set_bits(st->map, LTC4283_ADC_SELECT_2, > + LTC4284_SENSE1_SELECT_MASK | > + LTC4284_SENSE2_SELECT_MASK); > + if (ret) > + return ret; > + } > + > ret = device_property_read_u32(dev, "adi,current-limit-sense-microvolt", > &st->vsense_max); > if (!ret) { > @@ -1558,6 +1773,91 @@ static const struct hwmon_channel_info * const ltc4283_info[] = { > NULL > }; > > +static const struct hwmon_channel_info * const ltc4284_info[] = { [ ... ] > + HWMON_CHANNEL_INFO(curr, > + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | > + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM | > + HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM | > + HWMON_C_RESET_HISTORY | HWMON_C_LABEL, > + HWMON_C_INPUT | HWMON_C_LOWEST | HWMON_C_HIGHEST | > + HWMON_C_MAX | HWMON_C_MIN | HWMON_C_MIN_ALARM | > + HWMON_C_MAX_ALARM | HWMON_C_CRIT_ALARM | > + HWMON_C_RESET_HISTORY | HWMON_C_LABEL), > + HWMON_CHANNEL_INFO(power, > + HWMON_P_INPUT | HWMON_P_INPUT_LOWEST | > + HWMON_P_INPUT_HIGHEST | HWMON_P_MAX | HWMON_P_MIN | > + HWMON_P_MAX_ALARM | HWMON_P_MIN_ALARM | > + HWMON_P_RESET_HISTORY | HWMON_P_LABEL), > + HWMON_CHANNEL_INFO(energy, > + HWMON_E_ENABLE), > + HWMON_CHANNEL_INFO(energy64, > + HWMON_E_INPUT), > + NULL > +}; [Severity: Low] This isn't a bug, but there is quite a bit of duplication here: 1. ltc4284_info[] copies all 16 voltage channels, the power channel, and the energy channels from ltc4283_info[] just to add a second current channel. Could both current channels be defined in ltc4283_info[] and channel 1 hidden for ID_LTC4283 in ltc4283_is_visible() / ltc4283_curr_is_visible(), similar to how ltc4283_in_is_visible() hides voltage channels? 2. Eight separate helper functions (ltc4283_get_sense_reg(), ltc4283_get_sense_min_reg(), ltc4283_get_sense_max_reg(), ltc4283_get_sense_min_th_reg(), ltc4283_get_sense_max_th_reg(), ltc4283_get_curr_alarm_reg(), ltc4283_get_curr_alarm_high_mask(), and ltc4283_get_curr_alarm_low_mask()) branch on st->info->id. Could these register addresses and masks be stored in struct ltc4283_chip_info instead? 3. ltc4283_set_max_limits() branches on st->info->id == ID_LTC4283 to select LTC4283_SENSE_MAX_TH vs LTC4284_SENSE1_MAX_TH instead of reusing ltc4283_get_sense_max_th_reg(st, 0). 4. ltc4283_setup() repeats the property read, [LTC4283_MIN_RSENSE, LTC4283_MAX_RSENSE] range check, and division by CENTI three times for adi,rsense-nano-ohms, adi,rsense2-nano-ohms, and adi,radc-nano-ohms. Could that be factored into a small helper? [ ... ] > @@ -1769,13 +2090,15 @@ static int ltc4283_probe(struct i2c_client *client) > } > > static const struct of_device_id ltc4283_of_match[] = { > - { .compatible = "adi,ltc4283" }, > + { .compatible = "adi,ltc4283", .data = &chip_info_ltc4283 }, > + { .compatible = "adi,ltc4284", .data = &chip_info_ltc4284 }, > { } > }; > MODULE_DEVICE_TABLE(of, ltc4283_of_match); > > static const struct i2c_device_id ltc4283_i2c_id[] = { > - { "ltc4283" }, > + { "ltc4283", (kernel_ulong_t)&chip_info_ltc4283 }, > + { "ltc4284", (kernel_ulong_t)&chip_info_ltc4284 }, > { } > }; > MODULE_DEVICE_TABLE(i2c, ltc4283_i2c_id); [Severity: Low] This isn't a bug, but should the modified and new entries in ltc4283_i2c_id[] use named initializers (.name and .driver_data) to match I2C subsystem conventions? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261104-staging-ltc4284-v1-0-49fce67ebc48@analog.com?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller 2026-11-04 3:41 ` sashiko-bot @ 2026-11-04 4:58 ` Torreno, Alexis Czezar 0 siblings, 0 replies; 9+ messages in thread From: Torreno, Alexis Czezar @ 2026-11-04 4:58 UTC (permalink / raw) To: sashiko-reviews@lists.linux.dev Cc: linux-gpio@vger.kernel.org, linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org > [External] > > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to > consider: > - [Low] hwmon: ltc4283: positional initializers in ltc4283_i2c_id table entries > - [Low] hwmon: ltc4283: duplicated channel info table and chip-ID helpers > - [Low] hwmon: ltc4283: missing default resistor fallback in ltc4283_setup() > - [Low] hwmon: ltc4283: commit message omits functional differences for > LTC4284 > -- > > commit 07a1775aa285e31dccd905e19fc784e472406b97 > Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> > > hwmon: ltc4283: Add support for the LTC4284 swap controller > > This commit adds support for the Analog Devices LTC4284 negative voltage hot > swap controller to the ltc4283 hwmon driver, including dual current sense > channels and separate ADC power sense resistor handling. > > > hwmon: ltc4283: Add support for the LTC4284 swap controller > > > > Support the LTC4284 Hot Swap Controller. It is very Similar to LTC4283 but > > for higher power. The device features programmable current limit with > > foldback and independently adjustable inrush current to optimize the > > MOSFET safe operating area (SOA). The SOA timer limits MOSFET > temperature > > rise for reliable protection against overstresses. > > > > An I2C interface and onboard ADC allow monitoring of board current, > > voltage, power, energy, and fault status. > > > > Signed-off-by: Alexis Czezar Torreno > > <alexisczezar.torreno@analog.com> > > [Severity: Low] > This isn't a bug, but the commit message mostly repeats generic hot-swap > controller features already shared with the LTC4283. > > Could the changelog also describe the functional differences added to the driver > for the LTC4284, specifically the two current sense channels (SENSE1 and > SENSE2) with separate sense resistors (rsense and rsense2) and the dedicated > ADC sense resistor (radc) used for power and energy calculations? > Yes minor, but can add this edit to show better difference between the two parts. > > diff --git a/drivers/hwmon/ltc4283.c b/drivers/hwmon/ltc4283.c index ... > > > > @@ -1271,6 +1447,45 @@ static int ltc4283_setup(struct ltc4283_hwmon > *st, struct device *dev) > > */ > > st->rsense /= CENTI; > > > > + /* > > + * For LTC4284, rsense is for SENSE1, rsense2 for SENSE2, and radc is > > + * the effective sense resistance for ADC+- pin. > > + */ > > + if (st->info->id == ID_LTC4284) { > > + ret = device_property_read_u32(dev, "adi,rsense2-nano-ohms", > > + &st->rsense2); > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "Failed to read adi,rsense2-nano- > ohms\n"); > > + > > + if (st->rsense2 < LTC4283_MIN_RSENSE || st->rsense2 > > LTC4283_MAX_RSENSE) > > + return dev_err_probe(dev, -EINVAL, > > + "adi,rsense2-nano-ohms(%u) too > small or too large [%u %u]\n", > > + st->rsense2, LTC4283_MIN_RSENSE, > LTC4283_MAX_RSENSE); > > + > > + st->rsense2 /= CENTI; > > + > > + ret = device_property_read_u32(dev, "adi,radc-nano-ohms", > > + &st->radc); > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "Failed to read adi,radc-nano- > ohms\n"); > > + > > + if (st->radc < LTC4283_MIN_RSENSE || st->radc > > LTC4283_MAX_RSENSE) > > + return dev_err_probe(dev, -EINVAL, > > + "adi,radc-nano-ohms(%u) too small > or too large [%u %u]\n", > > + st->radc, LTC4283_MIN_RSENSE, > LTC4283_MAX_RSENSE); > > + > > + st->radc /= CENTI; > > [Severity: Low] > In ltc4283_setup(), st->rsense is initialized to LTC4283_MIN_RSENSE before > calling device_property_read_u32(dev, "adi,rsense-nano-ohms", &st->rsense) so > that the driver can probe without firmware properties: > > st->rsense = LTC4283_MIN_RSENSE; > ret = device_property_read_u32(dev, "adi,rsense-nano-ohms", > &st->rsense); > if (!ret) { > if (st->rsense < LTC4283_MIN_RSENSE || st->rsense > > LTC4283_MAX_RSENSE) > return dev_err_probe(...); > } > > When an LTC4284 device is instantiated via ltc4283_i2c_id without firmware > properties, won't ltc4283_probe() -> ltc4283_setup() fail because > device_property_read_u32() is required to succeed for adi,rsense2-nano-ohms > and adi,radc-nano-ohms instead of defaulting to LTC4283_MIN_RSENSE? > Will add the missing default. If as discussed in binding this maybe also become optional that defaults to 0. > > + > > + /* Enable SENSE1 and SENSE2 */ > > + ret = regmap_set_bits(st->map, LTC4283_ADC_SELECT_2, > > + LTC4284_SENSE1_SELECT_MASK | > > + LTC4284_SENSE2_SELECT_MASK); > > + if (ret) > > + return ret; > > + } > > + > > ret = device_property_read_u32(dev, "adi,current-limit-sense- > microvolt", > > &st->vsense_max); > > if (!ret) { > > @@ -1558,6 +1773,91 @@ static const struct hwmon_channel_info * const > ltc4283_info[] = { > > NULL > > }; > > > > +static const struct hwmon_channel_info * const ltc4284_info[] = { > [ ... ] > > + HWMON_CHANNEL_INFO(curr, > > + HWMON_C_INPUT | HWMON_C_LOWEST | > HWMON_C_HIGHEST | > > + HWMON_C_MAX | HWMON_C_MIN | > HWMON_C_MIN_ALARM | > > + HWMON_C_MAX_ALARM | > HWMON_C_CRIT_ALARM | > > + HWMON_C_RESET_HISTORY | HWMON_C_LABEL, > > + HWMON_C_INPUT | HWMON_C_LOWEST | > HWMON_C_HIGHEST | > > + HWMON_C_MAX | HWMON_C_MIN | > HWMON_C_MIN_ALARM | > > + HWMON_C_MAX_ALARM | > HWMON_C_CRIT_ALARM | > > + HWMON_C_RESET_HISTORY | HWMON_C_LABEL), > > + HWMON_CHANNEL_INFO(power, > > + HWMON_P_INPUT | HWMON_P_INPUT_LOWEST | > > + HWMON_P_INPUT_HIGHEST | HWMON_P_MAX | > HWMON_P_MIN | > > + HWMON_P_MAX_ALARM | > HWMON_P_MIN_ALARM | > > + HWMON_P_RESET_HISTORY | HWMON_P_LABEL), > > + HWMON_CHANNEL_INFO(energy, > > + HWMON_E_ENABLE), > > + HWMON_CHANNEL_INFO(energy64, > > + HWMON_E_INPUT), > > + NULL > > +}; > > [Severity: Low] > This isn't a bug, but there is quite a bit of duplication here: > > 1. ltc4284_info[] copies all 16 voltage channels, the power channel, and the > energy channels from ltc4283_info[] just to add a second current channel. > Could both current channels be defined in ltc4283_info[] and channel 1 hidden > for ID_LTC4283 in ltc4283_is_visible() / ltc4283_curr_is_visible(), similar to how > ltc4283_in_is_visible() hides voltage channels? > > 2. Eight separate helper functions (ltc4283_get_sense_reg(), > ltc4283_get_sense_min_reg(), ltc4283_get_sense_max_reg(), > ltc4283_get_sense_min_th_reg(), ltc4283_get_sense_max_th_reg(), > ltc4283_get_curr_alarm_reg(), ltc4283_get_curr_alarm_high_mask(), and > ltc4283_get_curr_alarm_low_mask()) branch on st->info->id. Could these > register addresses and masks be stored in struct ltc4283_chip_info instead? > > 3. ltc4283_set_max_limits() branches on st->info->id == ID_LTC4283 to select > LTC4283_SENSE_MAX_TH vs LTC4284_SENSE1_MAX_TH instead of reusing > ltc4283_get_sense_max_th_reg(st, 0). > > 4. ltc4283_setup() repeats the property read, [LTC4283_MIN_RSENSE, > LTC4283_MAX_RSENSE] range check, and division by CENTI three times for > adi,rsense-nano-ohms, adi,rsense2-nano-ohms, and adi,radc-nano-ohms. Could > that be factored into a small helper? > A few interesting suggestions here that's worth applying on the next version to lessen code. Will try > [ ... ] > > > @@ -1769,13 +2090,15 @@ static int ltc4283_probe(struct i2c_client > > *client) } > > > > static const struct of_device_id ltc4283_of_match[] = { > > - { .compatible = "adi,ltc4283" }, > > + { .compatible = "adi,ltc4283", .data = &chip_info_ltc4283 }, > > + { .compatible = "adi,ltc4284", .data = &chip_info_ltc4284 }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, ltc4283_of_match); > > > > static const struct i2c_device_id ltc4283_i2c_id[] = { > > - { "ltc4283" }, > > + { "ltc4283", (kernel_ulong_t)&chip_info_ltc4283 }, > > + { "ltc4284", (kernel_ulong_t)&chip_info_ltc4284 }, > > { } > > }; > > MODULE_DEVICE_TABLE(i2c, ltc4283_i2c_id); > > [Severity: Low] > This isn't a bug, but should the modified and new entries in ltc4283_i2c_id[] use > named initializers (.name and .driver_data) to match I2C subsystem > conventions? > Will fix to use named initializers. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller 2026-11-04 3:29 [PATCH 0/3] Supporting LTC4284 which is a negative voltage hotswap controller Alexis Czezar Torreno 2026-11-04 3:30 ` [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 Alexis Czezar Torreno 2026-11-04 3:30 ` [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller Alexis Czezar Torreno @ 2026-11-04 3:30 ` Alexis Czezar Torreno 2026-11-04 14:07 ` Bartosz Golaszewski 2 siblings, 1 reply; 9+ messages in thread From: Alexis Czezar Torreno @ 2026-11-04 3:30 UTC (permalink / raw) To: Nuno Sá, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan, Randy Dunlap, Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-hwmon, devicetree, linux-kernel, linux-doc, Alexis Czezar Torreno Similar to LTC4283, LTC4284 has up to 8 pins that can be configured as GPIOs and behaves the same. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> --- drivers/gpio/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 55e4280029d86e7701d9a043c89537f60bb70c6e..de9226619ef2f77fdded123657a14c15cbba07b1 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1864,11 +1864,11 @@ config GPIO_AD7768 called gpio-ad7768. config GPIO_LTC4283 - tristate "Analog Devices LTC4283 GPIO support" + tristate "Analog Devices LTC4283 and similar devices GPIO support" depends on SENSORS_LTC4283 help If you say yes here you want the GPIO function available in Analog - Devices LTC4283 Negative Voltage Hot Swap Controller. + Devices LTC4283 and similar devices Negative Voltage Hot Swap Controller. This driver can also be built as a module. If so, the module will be called gpio-ltc4283. -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller 2026-11-04 3:30 ` [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller Alexis Czezar Torreno @ 2026-11-04 14:07 ` Bartosz Golaszewski 0 siblings, 0 replies; 9+ messages in thread From: Bartosz Golaszewski @ 2026-11-04 14:07 UTC (permalink / raw) To: Alexis Czezar Torreno Cc: Nuno Sá, Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan, Randy Dunlap, Linus Walleij, linux-gpio, linux-hwmon, devicetree, linux-kernel, linux-doc On Wed, Nov 04, 2026 at 11:30:02AM +0800, Alexis Czezar Torreno wrote: > Similar to LTC4283, LTC4284 has up to 8 pins that can be configured > as GPIOs and behaves the same. > > Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com> > --- > drivers/gpio/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 55e4280029d86e7701d9a043c89537f60bb70c6e..de9226619ef2f77fdded123657a14c15cbba07b1 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1864,11 +1864,11 @@ config GPIO_AD7768 > called gpio-ad7768. > > config GPIO_LTC4283 > - tristate "Analog Devices LTC4283 GPIO support" > + tristate "Analog Devices LTC4283 and similar devices GPIO support" > depends on SENSORS_LTC4283 > help > If you say yes here you want the GPIO function available in Analog > - Devices LTC4283 Negative Voltage Hot Swap Controller. > + Devices LTC4283 and similar devices Negative Voltage Hot Swap Controller. > > This driver can also be built as a module. If so, the module will > be called gpio-ltc4283. > > -- > 2.34.1 > Is there something missing from this commit? The commit message suggests there would be some code changes? Bart ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-11-04 14:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-04 3:29 [PATCH 0/3] Supporting LTC4284 which is a negative voltage hotswap controller Alexis Czezar Torreno 2026-11-04 3:30 ` [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284 Alexis Czezar Torreno 2026-11-04 3:39 ` sashiko-bot 2026-11-04 4:06 ` Torreno, Alexis Czezar 2026-11-04 3:30 ` [PATCH 2/3] hwmon: ltc4283: Add support for the LTC4284 swap controller Alexis Czezar Torreno 2026-11-04 3:41 ` sashiko-bot 2026-11-04 4:58 ` Torreno, Alexis Czezar 2026-11-04 3:30 ` [PATCH 3/3] gpio: gpio-ltc4283: Add support for the LTC4284 Swap Controller Alexis Czezar Torreno 2026-11-04 14:07 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox