linux-kernel - Re: [PATCH v2 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Ramp Controller

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] [day] [month] [year] [list]

Date:   Tue, 15 Nov 2022 16:34:33 +0100
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     marijn.suijten@...ainline.org, konrad.dybcio@...ainline.org,
        kernel@...labora.com, andersson@...nel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, robh+dt@...nel.org,
        agross@...nel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm
 Ramp Controller

Il 15/11/22 16:16, Krzysztof Kozlowski ha scritto:
> On 15/11/2022 15:44, AngeloGioacchino Del Regno wrote:
> 
>>>>> Please check and re-submit.
>>>>>
>>>>
>>>> I'm unsure about what I should do about this one.
>>>> This is a power-controller, but does *not* need any #power-domain-cells, as it is
>>>> standalone and doesn't require being attached to anything.
>>>
>>> power-domain-cells are for power domain providers, not consumers. The
>>> generic binding expect that nodes called power-controller are exactly
>>> like that.
>>>
>>> Solutions could be:
>>> 1. Rename the node to something else. I cannot deduct the type of the
>>> device based on description. What is "sequence ID" and how is it even
>>> closely related to power control?
>>
>> This uC is mainly controlling DCVS, automagically plays with voltages for
>> each ramp up/down step and from what I understand also decides to shut down
>> or bring up *power* to "certain clocks" before ungating (CPU related, mainly
>> big cluster).
>> This also interacts with LMH - setting the LMH part makes it possible to
>> later use CPR (otherwise CPR errors out internally and won't start, as it
>> requires this controller, SAW and LMH to be set up in order to work).
>>
>> What I've seen is that without it I can't bring up the big cluster at all,
>> not even at minimum frequency, as the HF2PLL (a clock source for that cluster)
>> will not power up.
>> All it takes is to initialize these params and start the controller, then
>> everything goes as it should.
>>
>> If you're wondering why my explanation may not be particularly satisfying,
>> that's because downstream contains practically no information about this
>> one, apart from a bunch of lines of code and because this controller is
>> just a big black box.
>>
>>>
>>> 2. Narrow the node name in power-domain.yaml which would require changes
>>> in multiple DTS and bindings.
>>>
>>> 3. Do not require power-domain-cells for power-controllers, only for
>>> power-domains.
>>>
>>
>> Solutions 2 and 3... well, I don't think that this would be really feasible
>> as I envision this being the one and only driver that will ever require
>> that kind of thing.
>> Also, this programming was later moved to bootloaders and the only SoCs that
>> will ever require this are MSM8956/76, MSM8953 and.. I think MSM8952 as well,
>> but nothing more.
>>
>> Even if I can imagine the answer, I'm still tempted to ask: can we eventually
>> just name it ramp-controller@...x or qcom-rc@...x or something "special" like
>> that to overcome to this binding issue?
> 
> So maybe "cpu-power-controller"? This should already help for this warning.
> 

Agreed. Thanks for the advice!

Sending a v3 asap!