Re: [PATCH v6 05/12] power-domain: Add refcounting

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 15/04/2025 at 15:29:09 +0530, Neha Malcom Francis <n-francis@ti.com> wrote:

> Hi Miquel
>
> On 15/04/25 15:20, Miquel Raynal wrote:
>>>> Francesco, are you also testing on K3 platforms?
>>>>
>>>
>>> Yes he would be, since the firmware print is K3 firmware.
>>>
>>>> Can one of you boot with the patch below applied? It should partially
>>>> revert the commit to the ancient behaviour, while adding more debug
>>>> traces (please enable the debug logs as well). This should clarify
>>>> which one of the 3 different path is likely failing. It should also help
>>>> identify who's the user that fails to enable/disable its own power
>>>> domain. I will need to know what board/SoC was used for the test, so I
>>>> can look the relevant driver up.
>>>
>>> Booted on j784s4_evm
>>>
>>> https://gist.github.com/nehamalcom/b09687a523bec89f9df3537fdd99b6f3
>
> ^

Ah, sorry! This is very interesting.

There are two things which I do not understand:
- if we look at the very first power domains, their id is 15 and 14
  respectively. I do not see these numbers in any device tree, am I
  looking incorrectly? Could you help me identify what are the devices
  requesting these IDs?
- all power domains appear to be already enabled the very first time we
  enable them. I believe this is the root cause of the problem, as they
  should have count_on to 0 when booting. I checked again,
  'per_device_auto' is a field that is zeroed when allocated (using
  calloc() in this case) so I am really puzzled about this. It looks
  like "something else" is using this data field, which would be
  wrong.

>>>
>>> Currently debugging on my build where I hang in console_init itself, I
>>> think the path for failure is different here, will confirm.
>> 
>> Thanks a lot for your feedback. Normally by applying the diff shared in
>> my previous e-mail, you should have the console back. If not, maybe
>> there is something else involved and the blamed commit is just the first
>> showing an underlying problem.
>
> I do have console back with that patch, see the gist link above. What I
> meant was the path of failure seems to be different, it's looking like
> with my version of firmware the serial driver probe
> (ns16550_serial_probe) tries to do a readb() and fails, this could
> possibly be because the device was not powered on to begin with. Whereas
> in another case (what Francesco is seeing), this issue is not run
> into.

Clear.

Could you make another round with this extra change?

Thanks!
Miquèl

--- a/drivers/power/domain/power-domain-uclass.c
+++ b/drivers/power/domain/power-domain-uclass.c
@@ -119,14 +119,18 @@ int power_domain_on_lowlevel(struct power_domain *power_domain)
 
        debug("%s(power_domain=%p, id %d)\n", __func__, power_domain, power_domain->id);
 
-       if (priv->on_count++ > 0) {
-               debug("Power domain %s already on.\n", power_domain->dev->name);
+       if (priv->on_count > 0) {
+               debug("Power domain %s already on (usage count: %d).\n", power_domain->dev->name, priv->on_count);
+               priv->on_count++;
                //return -EALREADY;
+       } else {
+               priv->on_count++;
        }
 
        ret = ops->on ? ops->on(power_domain) : 0;
        if (ret) {
                priv->on_count--;
+               debug("Power domain %s on failed (usage count: %d).\n", power_domain->dev->name, priv->on_count);
                return ret;
        }