[PATCH 2/4] arm64: topology: Add support for topology DT bindings

[Mark Brown]

On Thu, Jan 09, 2014 at 12:50:52PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Jan 08, 2014 at 07:12:08PM +0000, Mark Brown wrote:

> > +#ifdef CONFIG_OF
> > +static int cluster_id;

> This is __initdata. I think it is better to move it to parse_cluster
> and pass it to parse_core as a parameter instead of using a global.

Should be __initdata, yes.  I actually started using pointers but
passing the pointer around just seemed more ugly, it made the code in
the functions look more scary due to the dereferences.

> > +	cpu_node = of_parse_phandle(node, "cpu", 0);
> > +	if (!cpu_node) {
> > +		pr_crit("%s: Unable to parse CPU phandle\n", node->full_name);

> This messages is spit out anytime you try to grab a cpu phandle and it
> is not there. In particular, in parse_core if core is not a leaf but you
> still try to grab a cpu phandle this message is spit out and that's not
> nice at all. Either we remove the message or we do not check the phandle
> in core nodes that are not leaves.

I'll just drop it.

> > +			cpu = get_cpu_for_node(t);
> > +			if (cpu >= 0) {
> > +				pr_info("CPU%d: socket %d core %d thread %d\n",
> > +					cpu, cluster_id, core_id, i);

> Since all these variables are internal values that have no real meaning
> I am not sure it is useful to print them out. Probably better to print
> the resulting cpu masks on every cpu, problem is when to do that.

I found it was useful for getting a quick overview of the topology from
the logs - the actual numbers don't matter but being able to see which
cores are grouped together seemed useful.  I'm not sure the masks would
be an improvement for the human reader, I'd expect that presenting more
decoded information would be more helpful.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140109/64be5a90/attachment.sig>