linux-kernel - Re: [PATCH v2 1/7] dt-bindings: regulator: Add bindings for MediaTek DVFSRC Regulators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4da3e998-d26d-4200-85ea-b8fd0176dff7@collabora.com>
Date: Thu, 18 Apr 2024 09:42:13 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Rob Herring <robh@...nel.org>
Cc: djakov@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, matthias.bgg@...il.com, lgirdwood@...il.com,
 broonie@...nel.org, keescook@...omium.org, gustavoars@...nel.org,
 henryc.chen@...iatek.com, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 kernel@...labora.com, wenst@...omium.org, amergnat@...libre.com
Subject: Re: [PATCH v2 1/7] dt-bindings: regulator: Add bindings for MediaTek
 DVFSRC Regulators

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@...labora.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@...labora.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