Rob Herring <robh@xxxxxxxxxx> writes:
> On Wed, Oct 31, 2018 at 7:46 AM Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
>> Rob Herring <robh@xxxxxxxxxx> writes:
>> > Use the for_each_of_cpu_node iterator to iterate over cpu nodes. This
>> > has the side effect of defaulting to iterating using "cpu" node names in
>> > preference to the deprecated (for FDT) device_type == "cpu".
>> >
>> > Cc: Frank Rowand <frowand.list@xxxxxxxxx>
>> > Cc: devicetree@xxxxxxxxxxxxxxx
>> > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>> > ---
>> > Please ack and I will take via the DT tree. This is dependent on the
>> > first 2 patches.
>> >
>> > drivers/of/base.c | 2 +-
>> > drivers/of/of_numa.c | 15 ++-------------
>> > 2 files changed, 3 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/drivers/of/base.c b/drivers/of/base.c
>> > index 6389aeb2f48c..8285c07cab44 100644
>> > --- a/drivers/of/base.c
>> > +++ b/drivers/of/base.c
>> > @@ -389,7 +389,7 @@ struct device_node *of_get_cpu_node(int cpu, unsigned int *thread)
>> > {
>> > struct device_node *cpun;
>> >
>> > - for_each_node_by_type(cpun, "cpu") {
>> > + for_each_of_cpu_node(cpun) {
>> > if (arch_find_n_match_cpu_physical_id(cpun, cpu, thread))
>> > return cpun;
>> > }
>>
>> Previously we just looked for any node with a type of "cpu", but now
>> we're using for_each_of_cpu_node(), which does:
>>
>> for (; next; next = next->sibling) {
>> if (!(of_node_name_eq(next, "cpu") ||
>> (next->type && !of_node_cmp(next->type, "cpu"))))
>> continue;
>>
>> if (!__of_device_is_available(next))
>> continue;
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> if (of_node_get(next))
>> break;
>> }
>>
>>
>> ie. the available check is new.
>>
>> On this machine the 2nd CPU is not marked as available:
>>
>> root@p5020ds:/proc/device-tree/cpus/PowerPC,e5500@1# lsprop status
>> status "disabled"
>>
>> This has the effect of preventing the SMP code from finding the 2nd CPU
>> in order to bring it up (in smp_85xx_start_cpu()). And so only the boot
>> CPU is onlined.
>>
>> The device tree is built from a dts:
>>
>> arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi
>>
>> But we don't set the status in there, so presumably u-boot is changing
>> the status during boot? (not a u-boot expert).
>
> Ah, status for cpus is a bit different. For most nodes, it should be
> equivalent to the node not being present, but for cpus it means
> offline if disabled. Though ARM platforms have never used it in that
> way.
Aha. We don't use it like that on server CPUs either, so perhaps it's
just a u-boot thing.
>> We could work around this in the platform code presumably, but I'm
>> worried this might break other things as well. You didn't mention the
>> addition of the available check in the change log so I wonder if it was
>> deliberate or just seemed like a good idea?
>
> Just seemed like a good idea...
Yeah fair enough.
> I'll send a patch now dropping those 2 lines.
Awesome, thanks.
cheers
