Re: [PATCH 1/3] dt-bindings: hwmon: ltc4283: add ltc4284

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3785 bytes --]

On Wed, Nov 04, 2026 at 04:06:13AM +0000, Torreno, Alexis Czezar wrote:
> > [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?

If it's actually optional, the dt should reflect that.

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

Random numbers are fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]