Messages in this thread | | | Date | Tue, 15 Nov 2022 16:34:33 +0100 | Subject | Re: [PATCH v2 1/2] dt-bindings: soc: qcom: Add bindings for Qualcomm Ramp Controller | From | AngeloGioacchino Del Regno <> |
| |
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@xxxx or qcom-rc@xxxx 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!
| |