Re: [PATCH RFC 21/27] drivers: cpu-pd: Parse topology to setup CPU PM domains

[Geert Uytterhoeven <geert@linux-m68k.org>]

On Thu, Dec 10, 2015 at 7:11 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, Dec 08, 2015 at 11:05:36AM -0700, Lina Iyer wrote:
>
> [...]
>
>> On Mon, Dec 07 2015 at 07:53 -0700, Lorenzo Pieralisi wrote:
>> >>+/**
>> >>+ * of_setup_cpu_domain_topology() - Setup the CPU domains from the CPU
>> >>+ * topology node in DT.
>> >>+ *
>> >>+ * @ops: The PM domain suspend/resume ops for all the domains in the topology
>> >>+ */
>> >>+int of_setup_cpu_domain_topology(const struct cpu_pd_ops *ops)
>> >>+{
>> >>+   struct device_node *cn, *map;
>> >>+   int ret = 0;
>> >>+
>> >>+   cn = of_find_node_by_path("/cpus");
>> >>+   if (!cn) {
>> >>+           pr_err("No CPU information found in DT\n");
>> >>+           return 0;
>> >>+   }
>> >>+
>> >>+   map = of_get_child_by_name(cn, "cpu-map");
>> >>+   if (!map)
>> >>+           goto out;
>> >
>> >I commented on this before, is this reliance on cpu-map necessary ?
>> >Could not you just rely on the "power-domains" phandle in the cpu
>> >nodes to build the cpumask for a specific power domain ? I think
>> >you should try to decouple the concept of power domain from the cpu-map
>> >cluster and I think this would also simplify your code in the process.
>> >
>> Sorry, I missed seeing your comment on this earlier.
>>
>> Hmm, it can be done, but I felt out of place to define just power
>> domains that have no devices in Linux, since they are handled in PSCI,
>> but also have no relation to PSCI. Hence, I felt cpu-map to be a good
>> place for the cluster domain. But I am not strongly inclined. I can
>> remove them from the cpu-map and keep them as separate nodes. However,
>> it would be nice to have a way to say these nodes are PSCI controlled.
>
> I do not agree with the way you described the system.
>
> Here is how I see it. DT must model HW, and in HW your cpus will be part
> of a power domain(s) that of course can be hierarchical. Every cpus has a
> phandle to the power domain it belongs into, and to group them as "cluster"
> you should follow the power domains hierarchy (ie if you have a cluster
> power domain, it will contain the core power domains, through the
> hierarchy it is easy to detect what cpus are part of the cluster power
> domain by detecting which cpus are part of its children). It is much
> simpler than the current solution I think, you get rid of cpu-map
> dependency (honestly it is a bit weird what you did, with cpu-map
> containing a cluster phandle to a power domain and cpu phandles to
> cpu nodes, and the DT bindings you posted does not seem to match your
> dts).
>
> For PSCI, nothing prevents you from defining the same bindings except
> that the power domain(s) is not explicitly controlled by the OS, still,
> the DT would always describe the HW and you can use it to detect the
> power domain topology and which cpus are part of a given power domain.
>
> Please let me know what you think, thanks.

+1

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds