Hi Miquel, On Fri, 24 Jan 2025 at 08:20, Miquel Raynal <[email protected]> wrote: > > Hi Simon, > > >> > 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. > > This, I do understand. > > >> > 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. > > Setting up power domains in board code is drawback. It is legacy > behaviour, people should switch to the device model (ie. using a proper > DT description) and stop messing around with board files. It's been like > 5 years since U-Boot forced people to transition, I wouldn't feel bad if > these boards were no longer behaving like expected (mind there are very > little chances this will break anything, as the kernel is supposed to > re-init these domains anyway).
While I agree with you, the message is not fully landing, certainly not in SPL where there are code-size constraints. > > >> > 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 > > Yes, pmdomain (former genpd if I get it correctly), how are they > different? From my point of view it is the same. The same devices are > supported. So why would we want the API to be different? But can you please point me to the API? I am seeing struct generic_pm_domain, which is uncommented, so it isn't clear what the power_off() and power_on() functions actually do. Perhaps they work differently from what you think? > > >> 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. > > Honestly, I am not convinced, but I will anyway assume we need a way to > force a domain off. There is no need to force a power domain on however, > as after power_domain_on(), the power domain cannot be off (except error > condition of course). > > So I'll add a helper to force power off. It will be called > power_domain_off_force() and be preceded with a comment stating that > this is not the standard approach, to guide people towards the > refcounted helper instead. > > Would this address your request? I believe it would be better to have a 'low-level' function which handles the refcounting and returns -EALREADY or -EBUSY if it does nothing, with your 'Linux' functions sitting above that. You can put any comment you like on the low-level function. The idea of 'forcing' something just seems odd to me. I imagine people would call that just to be sure :-) Regards, Simon
