Re: [PATCH v3 3/3] dt-bindings: regulator: Add MAX77675 regulator binding

[Joan Na <joan.na.devcode@gmail.com>]

On Thu, Nov 06, 2025 at 08:08:36AM +0100, Krzysztof Kozlowski wrote:
> On 06/11/2025 06:29, Joan Na wrote:
> >>> +  maxim,bias-low-power-request:
> >>> +    type: boolean
> >>> +    description: |
> >>> +      Request low-power bias mode.
> >>> +      When set, the device enters low-power bias mode.
> >>> +      Defaults to normal bias mode if this property is not specified.
> >>> +    default: false
> >>> +
> >>> +  maxim,simo-int-ldo-always-on:
> >>> +    type: boolean
> >>> +    description: |
> >>> +      Set internal LDO to always supply 1.8V
> >>> +      When set, the internal LDO always supplies 1.8V.
> >>> +      By default, the SIMO internal channel supplies 1.8V during low-power mode
> >>> +    default: false
> >>> +
> >>> +  regulators:
> >>> +    type: object
> >>> +    description: Regulator child nodes
> >>> +    patternProperties:
> >>> +      "^sbb[0-3]$":
> >>> +        type: object
> >>> +        $ref: regulator.yaml#
> >>> +    properties:
> >>> +      maxim,fps-slot:
> >>
> >> That's not property of regulators. Totally messed indentation.
> >>
> >>
> > 
> > The maxim,fps-slot property is specific to the MAX77675 regulators 
> > and is used to configure FPS slots individually for each regulator (e.g., sbb0–sbb3). 
> > As this represents a device-specific extension rather than a generic regulator property, 
> > it is defined under each regulator node.
> 
> But you did not define it under each regulator node. That would be fine.
> You defined it under regulators. So again that is not a property of
> regulators. That's a property of each regulator.
> 
> If you bothered to test it, you would see warnings.
>

I am sorry if it seemed like I was suggesting no testing was done.
I have now updated it under the regulator node as shown below.

  regulators:
    type: object
    description: Regulator child nodes
    patternProperties:
      "^sbb[0-3]$":
        type: object
        $ref: regulator.yaml#
        properties:
          adi,fps-slot:
            description: |
              FPS (Flexible Power Sequencer) slot selection.
              The Flexible Power Sequencer allows resources to power up under
              hardware or software control. Additionally, each resource can
              power up independently or among a group of other regulators with
              adjustable power-up and power-down slots.
              "slot0"   - Assign to FPS Slot 0
              "slot1"   - Assign to FPS Slot 1
              "slot2"   - Assign to FPS Slot 2
              "slot3"   - Assign to FPS Slot 3
              "default" - Use the default FPS slot value stored in OTP and read from the register
            $ref: /schemas/types.yaml#/definitions/string
            enum: [slot0, slot1, slot2, slot3, default]
            default: default

          adi,fixed-slew-rate:
            type: boolean
            description:
              When this property is present, the device uses a constant 2 mV/μs
              slew rate and ignores any dynamic slew rate configuration.
              When absent, the device uses the dynamic slew rate specified
              by 'adi,dvs-slew-rate-mv-per-us'
            default: true

> > 
> >>> +        description: |
> >>> +          FPS (Flexible Power Sequencer) slot selection.
> >>> +          The Flexible Power Sequencer allows resources to power up under hardware or software control.
> >>> +          Additionally, each resource can power up independently or among a group of other regulators
> >>> +          with adjustable power-up and power-down slots.
> >>> +          This device's regulators provide an additional property to configure the FPS parameters,
> >>> +          allowing each regulator to be assigned to an FPS slot for proper power management control.
> >>> +          "slot0"   - Assign to FPS Slot 0
> >>> +          "slot1"   - Assign to FPS Slot 1
> >>> +          "slot2"   - Assign to FPS Slot 2
> >>> +          "slot3"   - Assign to FPS Slot 3
> >>> +          "default" - Use the default FPS slot value stored in OTP and read from the register
> >>> +        $ref: /schemas/types.yaml#/definitions/string
> >>> +        enum: ["slot0", "slot1", "slot2", "slot3", "default"]
> >>> +        default: default
> >>> +
> >>> +      maxim,fixed-slew-rate:
> >>> +        type: boolean
> >>> +        description: |
> >>> +          Use fixed slew rate of 2 mV/μs for output voltage transitions.
> >>> +          When this property is present, the device uses a constant 2 mV/μs slew rate
> >>> +          and ignores any dynamic slew rate configuration.
> >>> +          When absent, the device uses the dynamic slew rate specified
> >>> +          by 'maxim,dvs-slew-rate-mv-per-us'
> >>> +        default: true
> >>> +
> >>> +    additionalProperties: false
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - regulators
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> +  - |
> >>> +    #include <dt-bindings/regulator/maxim,max77675-regulator.h>
> >>> +
> >>> +    i2c {
> >>> +      #address-cells = <1>;
> >>> +      #size-cells = <0>;
> >>> +
> >>> +      max77675: pmic@44 {
> >>> +        compatible = "maxim,max77675";
> >>> +        reg = <0x44>;
> >>> +
> >>> +        maxim,en-mode = "slide-switch";
> >>> +        maxim,latency-mode = "high";
> >>> +        maxim,drv-sbb-strength = "max";
> >>> +        maxim,dvs-slew-rate-mv-per-us = <5>;
> >>> +        maxim,manual-reset-time-sec = <4>;
> >>> +        maxim,en-debounce-time-us = <100>;
> >>> +
> >>> +        regulators {
> >>> +          sbb0: sbb0 {
> >>> +            regulator-name = "sbb0";
> >>> +            regulator-min-microvolt = <500000>;
> >>> +            regulator-max-microvolt = <5500000>;
> >>> +            maxim,fps-slot = "default";
> >>
> >> I don't think this was tested.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> > 
> > Testing on the actual EVKit has been conducted since PATCH V4
> 
> I have proofs this was not tested - see email from Rob.
> 
> But if you claim it was tested, then please explain me how can you
> possible test a binding on EVKit (it is impossible) and how could your
> testing miss such obvious errors?
> 
> 
> > 
> 
> 
> Best regards,
> Krzysztof

I will do my best to prepare everything properly for the next version.
In the meantime, I’d like to share the following test results that demonstrate the current behavior.
Please let me know if you need any additional information or clarification.

[    3.527749] max77675 1-0044: max77675_parse_config : MAX77675 PMIC configuration:
[    3.527753] max77675 1-0044:   en_mode                = 0
[    3.527758] max77675 1-0044:   voltage_change_latency = 1
[    3.527762] max77675 1-0044:   drv_sbb_strength     = 3
[    3.527766] max77675 1-0044:   dvs_slew_rate        = 1
[    3.527769] max77675 1-0044:   debounce_time        = 1 us
[    3.527773] max77675 1-0044:   manual_reset_time    = 1 sec
[    3.527776] max77675 1-0044:   en_pullup_disable    = true
[    3.527779] max77675 1-0044:   bias_low_power_req   = true
[    3.527782] max77675 1-0044:   simo_ldo_always  = true
[    3.531250] max77675 1-0044: max77675_of_parse_cb() called for regulator: sbb0 (id=0)
[    3.531265] max77675 1-0044:   DT node: /soc/i2c@7e804000/pmic@44/regulators/sbb0
[    3.531281] max77675 1-0044:   Init data: (none)
[    3.531286] max77675 1-0044:   Parsed 'adi,fps-slot' = 'slot0' (slot=0)
[    3.531291] max77675 1-0044:   'adi,fixed-slew-rate' = 1
[    3.531294] max77675 1-0044:   Applying parsed config for regulator id=0 (fps_slot=0, fixed_slew_rate=1)
[    3.532553] sbb0: 500 <--> 4700 mV at 1800 mV, disabled
[    3.545628] max77675 1-0044: max77675_of_parse_cb() called for regulator: sbb1 (id=1)
[    3.545647] max77675 1-0044:   DT node: /soc/i2c@7e804000/pmic@44/regulators/sbb1
[    3.545661] max77675 1-0044:   Init data: (none)
[    3.545666] max77675 1-0044:   Parsed 'adi,fps-slot' = 'slot1' (slot=1)
[    3.545670] max77675 1-0044:   'adi,fixed-slew-rate' = 0
[    3.545674] max77675 1-0044:   Applying parsed config for regulator id=1 (fps_slot=1, fixed_slew_rate=0)
[    3.546991] sbb1: 600 <--> 4800 mV at 1100 mV, disabled
[    3.547235] max77675 1-0044: max77675_of_parse_cb() called for regulator: sbb2 (id=2)
[    3.547245] max77675 1-0044:   DT node: /soc/i2c@7e804000/pmic@44/regulators/sbb2
[    3.547258] max77675 1-0044:   Init data: (none)
[    3.547263] max77675 1-0044:   Parsed 'adi,fps-slot' = 'slot2' (slot=2)
[    3.547267] max77675 1-0044:   'adi,fixed-slew-rate' = 1
[    3.547271] max77675 1-0044:   Applying parsed config for regulator id=2 (fps_slot=2, fixed_slew_rate=1)
[    3.553864] sbb2: 700 <--> 4900 mV at 700 mV, disabled
[    3.562943] max77675 1-0044: max77675_of_parse_cb() called for regulator: sbb3 (id=3)
[    3.562965] max77675 1-0044:   DT node: /soc/i2c@7e804000/pmic@44/regulators/sbb3
[    3.562979] max77675 1-0044:   Init data: (none)
[    3.562984] max77675 1-0044:   Parsed 'adi,fps-slot' = 'slot3' (slot=3)
[    3.562988] max77675 1-0044:   'adi,fixed-slew-rate' = 0
[    3.562991] max77675 1-0044:   Applying parsed config for regulator id=3 (fps_slot=3, fixed_slew_rate=0)
[    3.565702] sbb3: 800 <--> 5000 mV at 3300 mV, disabled

Thank you again for taking the time to review this.

-- 
Best regards,
Joan Na