Received-SPF: pass (google.com: domain of linux-kernel+bounces-149668-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <4da3e998-d26d-4200-85ea-b8fd0176dff7@collabora.com>
Date: Thu, 18 Apr 2024 09:42:13 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 1/7] dt-bindings: regulator: Add bindings for MediaTek
 DVFSRC Regulators
To: Rob Herring <robh@kernel.org>
Cc: djakov@kernel.org, krzysztof.kozlowski+dt@linaro.org,
 conor+dt@kernel.org, matthias.bgg@gmail.com, lgirdwood@gmail.com,
 broonie@kernel.org, keescook@chromium.org, gustavoars@kernel.org,
 henryc.chen@mediatek.com, linux-pm@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org,
 kernel@collabora.com, wenst@chromium.org, amergnat@baylibre.com
References: <20240417091442.170505-1-angelogioacchino.delregno@collabora.com>
 <20240417091442.170505-2-angelogioacchino.delregno@collabora.com>
 <20240417145238.GA2348962-robh@kernel.org>
 <b728478d-d9c9-4256-aeab-a234316d81cc@collabora.com>
 <20240417191247.GA3101736-robh@kernel.org>
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Content-Language: en-US
In-Reply-To: <20240417191247.GA3101736-robh@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Il 17/04/24 21:12, Rob Herring ha scritto:
> On Wed, Apr 17, 2024 at 05:39:12PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 17/04/24 16:52, Rob Herring ha scritto:
>>> On Wed, Apr 17, 2024 at 11:14:36AM +0200, AngeloGioacchino Del Regno wrote:
>>>> The Dynamic Voltage and Frequency Scaling Resource Collector Regulators
>>>> are controlled with votes to the DVFSRC hardware.
>>>>
>>>> This adds support for the regulators found in MT6873, MT8183, MT8192
>>>> and MT8195 SoCs.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>>    .../mediatek,mt6873-dvfsrc-regulator.yaml     | 71 +++++++++++++++++++
>>>>    1 file changed, 71 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6873-dvfsrc-regulator.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6873-dvfsrc-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6873-dvfsrc-regulator.yaml
>>>> new file mode 100644
>>>> index 000000000000..446f1dab4d2e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6873-dvfsrc-regulator.yaml
>>>> @@ -0,0 +1,71 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6873-dvfsrc-regulator.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: MediaTek DVFSRC-controlled Regulators
>>>> +
>>>> +description:
>>>> +  The Dynamic Voltage and Frequency Scaling Resource Collector Regulators
>>>> +  are controlled with votes to the DVFSRC hardware.
>>>> +
>>>> +maintainers:
>>>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - mediatek,mt6873-dvfsrc-regulator
>>>> +      - mediatek,mt8183-dvfsrc-regulator
>>>> +      - mediatek,mt8192-dvfsrc-regulator
>>>> +      - mediatek,mt8195-dvfsrc-regulator
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  dvfsrc-vcore:
>>>> +    description: DVFSRC-controlled SoC Vcore regulator
>>>> +    $ref: regulator.yaml#
>>>
>>>          unevaluatedProperties: false
>>>
>>
>> Will do!
>>
>>>> +
>>>> +  dvfsrc-vscp:
>>>> +    description: DVFSRC-controlled System Control Processor regulator
>>>> +    $ref: regulator.yaml#
>>>
>>> ditto
>>>
>>>> +
>>>> +required:
>>>> +  - compatible
>>>
>>> 'reg' is never optional. And how is no regulators at all valid?
>>>
>>
>> The two nodes that I'm adding with this series don't need reg, but others
>> that are not present in this do... but anyway, let's postpone that problem
>> for the future me, or the future-anyone-else implementing the rest, I will
>> remove the 'reg' property as it is indeed not needed for this node.
> 
> That might have been fine, but now that I know you *will* need it, it
> isn't fine. You could wait 1 week to 6 months to repost and hope I
> forget...

My bad, I should've explained a bit better, that was referred to the DVFSRC
main node, not to the REGULATOR node: that one doesn't need it at all.

I get it as it's fine if I drop reg from all of them (here and icc, and the
size/address cells from the parent node) and send a v3.

Did I get that wrong?


> 
> The ideal for these cases is you put the actual sub-range of
> MMIO registers in the child nodes. But sometimes it is just bit soup in
> the register layout and that doesn't work.
> 

I can confirm that there's a bit soup situation on MediaTek - bits for different
devices scattered all around in one register "far away".

>>
>> And yeah it's invalid to add that node without any regulator. Will check the
>> other regulator bindings on how they're doing it.
> 
> 'required' or anyOf with a list of required entries.
> 

That saves me lookup time - greatly appreciated, thank you!

Cheers,
Angelo

> Rob