From: "Nuno Sá" <noname.nuno@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>, nuno.sa@analog.com
Cc: linux-gpio@vger.kernel.org, linux-hwmon@vger.kernel.org,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Shuah Khan <skhan@linuxfoundation.org>,
Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>
Subject: Re: [PATCH v7 1/3] dt-bindings: hwmon: Document the LTC4283 Swap Controller
Date: Mon, 23 Mar 2026 10:31:42 +0000 [thread overview]
Message-ID: <77cd7e879a10df791d9d5eb1f16f1654e9904199.camel@gmail.com> (raw)
In-Reply-To: <c395fad0-ca24-448a-a77f-ddac1cd9f809@roeck-us.net>
On Mon, 2026-03-16 at 08:59 -0700, Guenter Roeck wrote:
> On Sat, Mar 14, 2026 at 10:52:19AM +0000, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> >
> > The LTC4283 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.
> >
> > Special note for the "adi,vpower-drns-enable" property. It allows to choose
> > between the attenuated MOSFET drain voltage or the attenuated input
> > voltage at the RTNS pin (effectively choosing between input or output
> > power). This is a system level decision not really intended to change at
> > runtime and hence is being added as a Firmware property.
> >
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
>
> Some AI review feedback inline. Feel free to ignore if wrong, but please let me know
> to help improve it.
>
> Thanks,
> Guenter
>
> > ---
> > .../devicetree/bindings/hwmon/adi,ltc4283.yaml | 272 +++++++++++++++++++++
> > MAINTAINERS | 6 +
> > 2 files changed, 278 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml
> > b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..f82fff1ec7e4407ed63d00f8b1281db459d7221b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml
> > @@ -0,0 +1,272 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/adi,ltc4283.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LTC4283 Negative Voltage Hot Swap Controller
> > +
> > +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.
> > +
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4283.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,ltc4283
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + adi,rsense-nano-ohms:
> > + description: Value of the sense resistor.
> > +
> > + adi,current-limit-sense-microvolt:
> > + description:
> > + The current limit sense voltage of the chip is adjustable between
> > + 15mV and 30mV in 1mV steps. This effectively limits the current
> > + on the load.
> > + minimum: 15000
> > + maximum: 30000
> > + default: 15000
> > +
> > + adi,current-limit-foldback-factor:
> > + description:
> > + Specifies the foldback factor for the current limit. The current limit
> > + can be reduced (folded back) to one of four preset levels. The value
> > + represents the percentage of the current limit sense voltage to use
> > + during foldback. A value of 100 means no foldback.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [10, 20, 50, 100]
> > + default: 100
> > +
> > + adi,cooling-delay-ms:
> > + description:
> > + Cooling time to apply after an overcurrent fault, FET bad or
> > + external fault.
> > + enum: [512, 1002, 2005, 4100, 8190, 16400, 32800, 65600]
> > + default: 512
> > +
> > + adi,fet-bad-timer-delay-ms:
> > + description:
> > + FET bad timer delay. After a FET bad status condition is detected,
> > + this timer is started. If the condition persists for the
> > + specified time, the FET is turned off and a fault is logged.
> > + enum: [256, 512, 1002, 2005]
> > + default: 256
> > +
> > + adi,power-good-reset-on-fet:
> > + description:
> > + If set, resets the power good status when the MOSFET is turned off.
> > + Otherwise, it resets when a low output voltage is detected.
> > + type: boolean
> > +
> > + adi,fet-turn-off-disable:
> > + description:
> > + If set, the MOSFET is turned off immediately when a FET fault is detected.
> > + type: boolean
>
> Is there a logic inversion between the property name and its description?
> The property name uses a -disable suffix, but the description says "If set,
> the MOSFET is turned off immediately", which sounds like it is enabling the
> behavior rather than disabling it.
Yes. Misleading description. This has -disable because the default is the -enable case which
indeed is the case when the FET is turned off. Will update the description so that is not
confusing.
> > +
> > + adi,tmr-pull-down-disable:
> > + description: Disables 2uA pull-down current on the TMR pin.
> > + type: boolean
> > +
> > + adi,dvdt-inrush-control-disable:
> > + description:
> > + Enables dV/dt inrush control during startup. In dV/dt mode, the inrush
> > + current is limited by controlling a constant output voltage ramp rate.
> > + If not set, the inrush control mechanism is active current limiting.
> > + type: boolean
>
> Does this description contradict the property name?
> The -disable suffix implies the property turns off the dV/dt inrush control,
> but the description states that setting the property enables it.
>
> [ Non-AI note: It seems to me that the description contradicts itself.
> It first says "_Enables_ ...", then it says "If _not_ set, the inrush control
> mechanism is active current limiting" ]
Yeah same thing as above. I will update this one just by s/Enables/Disables/
>
> > +
> > + adi,fault-log-enable:
> > + description:
> > + If set, enables logging fault registers and ADC data into EEPROM upon a
> > + fault.
> > + type: boolean
> > +
> > + adi,vpower-drns-enable:
> > + description:
> > + If set, enables the attenuated MOSFET drain voltage to be monitored. This
> > + effectively means that the MOSFET power is monitored. If not set, the
> > + attenuated input voltage (and hence input power) is monitored.
> > + type: boolean
> > +
> > + adi,external-fault-fet-off-enable:
> > + description: Turns MOSFET off following an external fault.
> > + type: boolean
> > +
> > + adi,undervoltage-retry-disable:
> > + description: Do not retry to turn on the MOSFET after an undervoltage fault.
> > + type: boolean
> > +
> > + adi,overvoltage-retry-disable:
> > + description: Do not retry to turn on the MOSFET after an overvoltage fault.
> > + type: boolean
> > +
> > + adi,external-fault-retry-enable:
> > + description: Retry to turn on the MOSFET retry after an external fault.
> > + type: boolean
>
> This isn't a bug, but there's a typo in the description where the word
> "retry" is repeated.
ack.
>
> > +
> > + adi,overcurrent-retries:
> > + description: Configures auto-retry following an Overcurrent fault.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [latch-off, "1", "7", unlimited]
> > + default: latch-off
> > +
> > + adi,fet-bad-retries:
> > + description:
> > + Configures auto-retry following a FET bad fault and a consequent MOSFET
> > + turn off.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [latch-off, "1", "7", unlimited]
> > + default: latch-off
> > +
> > + adi,pgio1-func:
> > + description: Configures the function of the PGIO1 pin.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [inverted_power_good, power_good, gpio]
> > + default: inverted_power_good
> > +
> > + adi,pgio2-func:
> > + description: Configures the function of the PGIO2 pin.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [inverted_power_good, power_good, gpio, active_current_limiting]
> > + default: inverted_power_good
> > +
> > + adi,pgio3-func:
> > + description: Configures the function of the PGIO3 pin.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [inverted_power_good_input, power_good_input, gpio]
> > + default: inverted_power_good_input
> > +
> > + adi,pgio4-func:
> > + description: Configures the function of the PGIO4 pin.
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [inverted_external_fault, external_fault, gpio]
> > + default: inverted_external_fault
> > +
> > + adi,gpio-on-adio1:
> > + description: If set, the ADIO1 pin is used as a GPIO.
> > + type: boolean
> > +
> > + adi,gpio-on-adio2:
> > + description: If set, the ADIO2 pin is used as a GPIO.
> > + type: boolean
> > +
> > + adi,gpio-on-adio3:
> > + description: If set, the ADIO3 pin is used as a GPIO.
> > + type: boolean
> > +
> > + adi,gpio-on-adio4:
> > + description: If set, the ADIO4 pin is used as a GPIO.
> > + type: boolean
>
> Does this dependency block force a redundant specification of adi,pgio4-func?
> The default for adi,pgio4-func is inverted_external_fault, which means the
> default hardware state already supports external fault features.
> If a device tree legitimately omits adi,pgio4-func to rely on that default,
> will it fail schema validation here since the dependencies keyword strictly
> checks for the literal presence of properties without injecting defaults?
Fair point. I guess it will fail but the alternative is to not have any constrain at all so
maybe worth it to be explicit in here?
- Nuno Sá
>
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > +dependencies:
> > + adi,gpio-on-adio1:
> > + - gpio-controller
> > + - '#gpio-cells'
> > + adi,gpio-on-adio2:
> > + - gpio-controller
> > + - '#gpio-cells'
> > + adi,gpio-on-adio3:
> > + - gpio-controller
> > + - '#gpio-cells'
> > + adi,gpio-on-adio4:
> > + - gpio-controller
> > + - '#gpio-cells'
> > + adi,external-fault-retry-enable:
> > + - adi,pgio4-func
> > + adi,external-fault-fet-off-enable:
> > + - adi,pgio4-func
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - adi,rsense-nano-ohms
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + adi,pgio1-func:
> > + const: gpio
> > + required:
> > + - adi,pgio1-func
> > + then:
> > + required:
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > + - if:
> > + properties:
> > + adi,pgio2-func:
> > + const: gpio
> > + required:
> > + - adi,pgio2-func
> > + then:
> > + required:
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > + - if:
> > + properties:
> > + adi,pgio3-func:
> > + const: gpio
> > + required:
> > + - adi,pgio3-func
> > + then:
> > + required:
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > + - if:
> > + properties:
> > + adi,pgio4-func:
> > + const: gpio
> > + required:
> > + - adi,pgio4-func
> > + then:
> > + properties:
> > + adi,external-fault-retry-enable: false
> > + adi,external-fault-fet-off-enable: false
> > + required:
> > + - gpio-controller
> > + - '#gpio-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + swap-controller@15 {
> > + compatible = "adi,ltc4283";
> > + reg = <0x15>;
> > +
> > + adi,rsense-nano-ohms = <500>;
> > + adi,current-limit-sense-microvolt = <25000>;
> > + adi,current-limit-foldback-factor = <10>;
> > + adi,cooling-delay-ms = <8190>;
> > + adi,fet-bad-timer-delay-ms = <512>;
> > +
> > + adi,external-fault-fet-off-enable;
> > + adi,pgio4-func = "external_fault";
> > +
> > + adi,gpio-on-adio1;
> > + adi,pgio1-func = "gpio";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + };
> > + };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 830c6f076b0029f0ff1abee148ad0e1905a60e82..13ae2f3db449e5fd3a7d0fbac92aabdc01734ba9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15141,6 +15141,12 @@ F: Documentation/devicetree/bindings/hwmon/adi,ltc4282.yaml
> > F: Documentation/hwmon/ltc4282.rst
> > F: drivers/hwmon/ltc4282.c
> >
> > +LTC4283 HARDWARE MONITOR AND GPIO DRIVER
> > +M: Nuno Sá <nuno.sa@analog.com>
> > +L: linux-hwmon@vger.kernel.org
> > +S: Supported
> > +F: Documentation/devicetree/bindings/hwmon/adi,ltc4283.yaml
> > +
> > LTC4286 HARDWARE MONITOR DRIVER
> > M: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>
> > L: linux-hwmon@vger.kernel.org
> >
> > --
> > 2.51.0
> >
> >
> >
next prev parent reply other threads:[~2026-03-23 10:30 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-14 10:52 [PATCH v7 0/3] hwmon: Add support for the LTC4283 Hot Swap Controller Nuno Sá via B4 Relay
2026-03-14 10:52 ` [PATCH v7 1/3] dt-bindings: hwmon: Document the LTC4283 " Nuno Sá via B4 Relay
2026-03-16 15:59 ` Guenter Roeck
2026-03-23 10:31 ` Nuno Sá [this message]
2026-03-23 14:33 ` Guenter Roeck
2026-03-23 15:17 ` Nuno Sá
2026-03-23 15:27 ` Guenter Roeck
2026-03-23 16:07 ` Nuno Sá
2026-03-14 10:52 ` [PATCH v7 2/3] hwmon: ltc4283: Add support for " Nuno Sá via B4 Relay
2026-03-18 1:17 ` Guenter Roeck
2026-03-23 10:21 ` Nuno Sá
2026-03-14 10:52 ` [PATCH v7 3/3] gpio: gpio-ltc4283: " Nuno Sá via B4 Relay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=77cd7e879a10df791d9d5eb1f16f1654e9904199.camel@gmail.com \
--to=noname.nuno@gmail.com \
--cc=brgl@kernel.org \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=nuno.sa@analog.com \
--cc=robh@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox