[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5c0dfcad-956d-e3cd-fd06-7671b85c4ae7@linaro.org>
Date: Tue, 15 Nov 2022 16:16:58 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
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
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.
Best regards,
Krzysztof
Powered by blists - more mailing lists