[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c5dffc8c-2abe-4fd3-a062-6d1adb417d27@collabora.com>
Date: Mon, 30 Jun 2025 09:52:53 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>, Krzysztof Kozlowski <krzk@...nel.org>
Cc: broonie@...nel.org, lgirdwood@...il.com, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, matthias.bgg@...il.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
kernel@...labora.com
Subject: Re: [PATCH v2 3/6] dt-bindings: regulator: Document MediaTek MT6363
PMIC Regulators
Il 30/06/25 05:25, Chen-Yu Tsai ha scritto:
> On Fri, Jun 27, 2025 at 4:24 PM Krzysztof Kozlowski <krzk@...nel.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@...labora.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@...labora.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.
>
> 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..!).
Cheers,
Angelo
Powered by blists - more mailing lists