Re: [PATCH v3 2/3] cpuidle: psci: Move enabling OSI mode after power domains creation

[Maulik Shah (mkshah)]

Hi Ulf,

On 4/18/2023 1:42 PM, Ulf Hansson wrote:
> O
>
>          /* Bail out if not using the hierarchical CPU topology. */
>          if (!pd_count)
> -               goto no_pd;
> +               goto remove_pd;
> We should return 0 here instead, right?
right. will fix in next revision.
> >          /* Link genpd masters/subdomains to model the CPU topology. */
> >          ret = dt_idle_pd_init_topology(np);
> >          if (ret)
> > -               goto remove_pd;
> > +               goto remove_pd_topology;
> This looks wrong to me. Shouldn't we continue to goto the "remove_pd"
> label for this error path?
We should need to remove already added subdomains via 
of_genpd_add_subdomain() if one of them fails.
So this look ok to me.
> > +
> > +       /* let's try to enable OSI. */
> > +       ret = psci_set_osi_mode(use_osi);
> > +       if (ret)
> > +               goto remove_pd_topology;
> >
> >          pr_info("Initialized CPU PM domain topology using %s mode\n",
> >                  use_osi ? "OSI" : "PC");
> >          return 0;
> >
> > -put_node:
> > -       of_node_put(node);
> > +remove_pd_topology:
> > +       dt_idle_pd_remove_topology(np);
> >   remove_pd:
> >          psci_pd_remove();
> > +put_node:
> > +       of_node_put(node);
> This of_node_put() should only be called if we break the
> "for_each_child_of_node" loop above because of an error, I think.
> Perhaps it's cleaner to just move this within the loop?
yes will move inside loop.

Thanks,
Maulik