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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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!



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux