Re: [PATCH v10 1/9] dt-bindings: regulator: Document MediaTek MT6316 PMIC Regulators

[AngeloGioacchino Del Regno]

Il 24/10/25 18:29, Conor Dooley ha scritto:
> On Fri, Oct 24, 2025 at 10:32:13AM +0200, AngeloGioacchino Del Regno wrote:
> > Add bindings for the regulators found in the MediaTek MT6316 PMIC,
> > usually found in board designs using the MT6991 Dimensity 9400 and
> > on MT8196 Kompanio SoC for Chromebooks.
> >
> > This chip is fully controlled by SPMI and has multiple variants
> > providing different phase configurations.
> >
> > Reviewed-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
> > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
> >   .../regulator/mediatek,mt6316b-regulator.yaml | 78 +++++++++++++++++++
> >   .../regulator/mediatek,mt6316c-regulator.yaml | 78 +++++++++++++++++++
> >   .../regulator/mediatek,mt6316d-regulator.yaml | 77 ++++++++++++++++++
> >   3 files changed, 233 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6316b-regulator.yaml
> >   create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6316c-regulator.yaml
> >   create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6316d-regulator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6316b-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6316b-regulator.yaml
> > new file mode 100644
> > index 000000000000..65b70dd90728
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6316b-regulator.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/mediatek,mt6316b-regulator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek MT6316 BP/VP SPMI PMIC Regulators
> > +
> > +maintainers:
> > +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
> > +
> > +description:
> > +  The MediaTek MT6316BP/VP PMICs are fully controlled by SPMI interface, both
> > +  feature four step-down DC/DC (buck) converters, and provides 2+2 Phases,
> > +  joining Buck 1+2 for the first phase, and Buck 3+4 for the second phase.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt6316b-regulator
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  "^vbuck(12|34)$":
> > +    type: object
> > +    $ref: regulator.yaml#
> > +    unevaluatedProperties: false
> > +    properties:
> > +      regulator-allowed-modes:
> > +        description: |
> > +          Allowed Buck regulator operating modes allowed. Valid values below.
> > +            0 - Normal mode with automatic power saving, reducing the switching
> > +                frequency when light load conditions are detected
> > +            1 - Forced Continuous Conduction mode (FCCM) for improved voltage
> > +                regulation accuracy with constant switching frequency but lower
> > +                regulator efficiency
> > +            2 - Forced Low Power mode for improved regulator efficiency, used
> > +                when no heavy load is expected, will shut down unnecessary IP
> > +                blocks and secondary phases to reduce quiescent current.
> > +                This mode does not limit the maximum output current but unless
> > +                only a light load is applied, there will be regulation accuracy
> > +                and efficiency losses.
> > +        minItems: 1
> > +        maxItems: 3
> > +        items:
> > +          enum: [ 0, 1, 2 ]
>
> This property has no default, and the property is not required. Is one
> of these modes the default, or is there another mode beyond what's here
> that is used if the property is absent? Or are all modes allowed with no
> property?

The default is what the bootloader sets before jumping to the kernel... this may
vary from one to the other.

Even though "in theory" the default should be 0, I can't guarantee that this will
really be the default for when Linux boots.... and "resetting" is not possible
(either as a real reset or forcing a default) because those are CPU regulators
and if anything goes wrong the CPUs may freeze execution.

It should be kinda safe to force the default to 0 at boot, but I'm not sure that
I should really take that assumption for granted - I prefer leaving this untouched
and without any "forced" default unless DT specifies some.

So, well, that's why there's no default, and that's why this is not a required
property at the end of the day.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#address-cells'
>
> Why is address-cells required here? Your bucks don't have addresses.

Ugh. I forgot it there. No, that's not intentional.

The #address-cells makes no sense here.

> If it is actually required, Rob's bot has pointed out that the property
> isn't defined for the device anyway.
>
> pw-bot: changes-requested
>
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/spmi/spmi.h>
> > +
> > +    spmi {
> > +      #address-cells = <2>;
> > +      #size-cells = <0>;
> > +
> > +      pmic@8 {
> > +        compatible = "mediatek,mt6316b-regulator";
> > +        reg = <0x8 SPMI_USID>;
> > +        #address-cells = <0>;
> > +
> > +        vbuck12 {
> > +          regulator-name = "dvdd_core";
> > +          regulator-min-microvolt = <450000>;
> > +          regulator-max-microvolt = <965000>;
> > +          regulator-allowed-modes = <0 1 2>;
> > +          regulator-enable-ramp-delay = <256>;
> > +        };
> > +      };
> > +    };
> > +...