Linux Documentation
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: 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, 16 Mar 2026 08:59:56 -0700	[thread overview]
Message-ID: <c395fad0-ca24-448a-a77f-ddac1cd9f809@roeck-us.net> (raw)
In-Reply-To: <20260314-ltc4283-support-v7-1-1cda48e93802@analog.com>

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.
> +
> +  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" ]

> +
> +  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.

> +
> +  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?

> +
> +  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
> 
> 
> 

  reply	other threads:[~2026-03-16 16:00 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 [this message]
2026-03-23 10:31     ` Nuno Sá
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=c395fad0-ca24-448a-a77f-ddac1cd9f809@roeck-us.net \
    --to=linux@roeck-us.net \
    --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=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