Re: [PATCH v2 3/6] dt-bindings: regulator: Document MediaTek MT6363 PMIC Regulators

[Chen-Yu Tsai <wenst@chromium.org>]

Hi,

On Mon, Jun 30, 2025 at 3:52 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 30/06/25 05:25, Chen-Yu Tsai ha scritto:
> > On Fri, Jun 27, 2025 at 4:24 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Tue, Jun 24, 2025 at 09:35:45AM +0200, AngeloGioacchino Del Regno wrote:
> >>> Add bindings for the regulators found in the MediaTek MT6363 PMIC,
> >>> usually found in board designs using the MT6991 Dimensity 9400 and
> >>> on MT8196 Kompanio SoC for Chromebooks, along with the MT6316 and
> >>> MT6373 PMICs.
> >>>
> >>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>> ---
> >>>   .../regulator/mediatek,mt6363-regulator.yaml  | 123 ++++++++++++++++++
> >>>   1 file changed, 123 insertions(+)
> >>>   create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6363-regulator.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6363-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6363-regulator.yaml
> >>> new file mode 100644
> >>> index 000000000000..f866c89c56f7
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6363-regulator.yaml
> >>> @@ -0,0 +1,123 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6363-regulator.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: MediaTek MT6363 PMIC Regulators
> >>> +
> >>> +maintainers:
> >>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >>> +
> >>> +description:
> >>> +  The MT6363 SPMI PMIC provides 10 BUCK and 26 LDO (Low Dropout) regulators
> >>> +  and can optionally provide overcurrent warnings with one ocp interrupt
> >>> +  for each voltage regulator.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: mediatek,mt6363-regulator
> >>> +
> >>> +  interrupts:
> >>> +    description: Overcurrent warning interrupts
> >>
> >> Are you sure interrupts are physically not connected?
>
> Yes, I'm sure, they are not.
>
> >
> > Side note:
> >
> > I wonder if we really need to describe _all_ the interrupts here.
> >
> > Looking at the PMIC as a whole, the interrupt tree is something like
> >
> > SoC <- SPMI inband IRQ - PMIC top level IRQ block <- sub-function IRQ blocks:
> >
> >      - BUCK (buck regulator over current)
> >      - LDO (LDO regulator over current)
> >      - PSC (key press / system low voltage)
> >      - MISC (protected registers accessed / SPMI stuff)
> >
> > And some other blocks that may apply to other MediaTek PMICs:
> >
> >      - HK (some threshold triggered interrupt)
> >      - BM (battery management related)
> >
> > The thing I'm trying to get to is that all these interrupt vectors are
> > internal to the whole PMIC. Do we really need to spell them out in the
> > device tree? The top level compatible should already imply how all the
> > internals are wired up.
> >
>
> Chen-Yu:
>
> Yes, we do: not all boards need overcurrent protection on all of the rails, but
> especially, in the past I have seen (multiple times) board designs (not MediaTek,
> but that doesn't mean anything) that will trigger the overcurrent protection due
> to a high inrush upon rail enablement - in these cases, the ocp would have to be
> either ignored completely or reset and read after a while.
>
> Not only that: since not all rails are actually used, due to EMI (and other issues
> which usually mean suboptimally built boards) some of those may randomly trigger
> OCP, and that's another case in which that should be ignored.
>
> So... yes, we want to define the overcurrent interrupts in the devicetree.

I understand the use case, but I think that's kind of giving the interrupts
property a second meaning.

Instead, if you look at the common regulator bindings, there is a
"regulator-over-current-protection" which signals that over current
protection should be enabled for a given regulator. Perhaps you could
use that? I think this common property also implies that over current
protection has to be explicitly enabled.

> >
> > ChenYu
> >
> >>> +    minItems: 1
> >>> +    maxItems: 36
> >>> +
> >>> +  interrupt-names:
> >>> +    description:
> >>> +      Names for the overcurrent interrupts are the same as the name
> >>> +      of a regulator (hence the same as each regulator's node name).
> >>> +      For example, the interrupt name for regulator vs2 will be "vs2".
> >>
> >> You need to define the items or pattern if this is really flexible in
> >> the hardware (not drivers).
>
> krzk:
>
> It's flexible in the hardware... but how do I define a pattern here?
> I avoided to define the items because you can miss some; I mean....
>
> You may have, on one board:
> "vs1", "vsram", "someother", "another"
>
> on another: "vsram", "another"
>
> ...and another: "vs1", "another"
>
> (etc etc)
>
> Is there any way to allow missing items in between?
> Because then there's 36 possible items, so there are more than 100 possible
> combinations (keeping the order, but missing something in between..!).

I recently saw in the net/snps,dwmac.yaml binding the following:

  clock-names:
    minItems: 1
    maxItems: 10
    additionalItems: true
    contains:
      enum:
        - stmmaceth
        - pclk
        - ptp_ref

I suppose you could adapt this pattern, list all the possibilities, and
set additionalItems to false. I don't think it can pick out duplicates
though.


ChenYu

> Cheers,
> Angelo
>
>
>