Hi Miquel,
On Tue, 21 Jan 2025 at 01:34, Miquel Raynal miquel.raynal@bootlin.com wrote:
Hi Simon,
On 20/01/2025 at 12:21:04 -07, Simon Glass sjg@chromium.org wrote:
Hi Miquel,
On Mon, 20 Jan 2025 at 03:34, Miquel Raynal miquel.raynal@bootlin.com wrote:
Hello Simon,
int power_domain_on(struct power_domain *power_domain) {
...
if (priv->on_count++ > 0)return 0;-EALREADY
...
int power_domain_off(struct power_domain *power_domain) {
...
if (priv->on_count-- > 1)return 0;-EBUSY
See how the regulator uclass does it.
I really does not understand why we would like to do that.
It is perfectly normal operation to call power_domain_on/off() on the same power domain several times in a row and there is no reason to return an error code. It is quite the opposite, the main reason for power domains is to act like shared regulators. Se while a regulator has one user and doing the same action on it several times does not make much sense and can be reported, that is not how power domains have been thought about in the first place.
I am not aware of any difference between these two subsystems.
Actually you're right on this point, the regulator API works like the pmdomain one. I had a look: it always returns 0, no matter the state of the regulator after the operation (as long as there is no error of course).
You can see dm_test_power_regulator_set_enable_if_allowed() for a test that relates to this.
If we want a power domain to actually turn off, how many times do we need to call power_domain_off()?
We do not? It is why I add refcounting, if a power domain has 2 consumers, each consumer says whether it needs the power domain to be on or off and the core will handle, but in no case they should force it's state.
This is a bootloader, so we do sometimes need to force something off, or on.
The function silently does nothing in many cases, so it is not deterministic.
It is fully deterministic, as long as consumers call power_on/off evenly (and this is already what we request in U-Boot).
But I foresee people setting up power domains in board code, not drivers. My concern is that this is hiding the real state.
In the case where we *actually* want to turn the power domain off, we are at a loss as to what code to write.
Hence, I do not agree with returning error codes in these situations, they are misleading and they would have to be ignored anyway.
How about creating a power_domain_off_if_allowed() or power_domain_soft_off/on() or power_domain_req_off (for request)?
The power domain logic has a hardware reality in many SoCs but is also a software concept that is shared with Linux. Changing the semantics in U-Boot would be very misleading and if my understanding is correct, this approach would be new.
Yes this is quite a strong argument. Can you point me to the equivalent API in Linux? I see pm_domain but not the same as U-Boot
If you really want a way to track the state of the power domain, however, I can maybe add a helper returning its state, ie:
bool power_domain_is_on(domain)Would that work?
Not really. We should have an API which returns -EALREADY or -EBUSY based on the ref counts.
But yes, we can change the naming, so that this 'deterministic' ones have a different name, with the current names used for your ref-counting one. Then the ref-counting one is a thin layer on top of the deterministic one.
Regards, Simon