On Thu, 1 Mar 2018, Ivan Gorinov wrote: The patch order is wrong. Fixes first and then features simply because you cannot test the feature patch standalone as the fix (3/3) is missing ....
> Adding code to register the processors described in Device Tree. 'Adding code...' is a pointless filler phrase. > APIC ID is specified in 'intel-apic_id' propery as used in U-Boot. s/propery/property/ Also this has nothing to do with U-Boot. The DT properties are described in the DT bindings and not specified in U-Boot. Where is the matching devicetree binding documented? This should be a separate patch and needs to go into Documentation/devicetree/x86/ Please Cc the DT maintainers as we need their ack for that. > First address specified in 'reg' is used as default APIC ID. Changelogs should describe the context/problem and the approach how this is fixed or made working. Something like this: The current x86 device tree implementation does not support SMP. Use the new DT bindings for describing CPUs and their APIC resources. Hmm? > Signed-off-by: Ivan Gorinov <[email protected]> > --- > arch/x86/kernel/devicetree.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c > index 44189ee..ef1cd85 100644 > --- a/arch/x86/kernel/devicetree.c > +++ b/arch/x86/kernel/devicetree.c > @@ -130,6 +130,29 @@ static void __init dtb_setup_hpet(void) > #endif > } > > +static void __init dtb_cpu_setup(void) > +{ > + struct device_node *dn; > + struct resource r; > + const void *prop; > + int apic_id, version; > + int ret; > + > + version = GET_APIC_VERSION(apic_read(APIC_LVR)); > + for_each_node_by_type(dn, "cpu") { > + prop = of_get_property(dn, "intel,apic-id", NULL); > + if (prop) { > + apic_id = be32_to_cpup(prop); > + } else { > + ret = of_address_to_resource(dn, 0, &r); > + if (WARN_ON_ONCE(ret)) > + continue; > + apic_id = r.start; > + } What kind of logic is this? If a CPU node does not have apic id property then it's invalid. That else clause is just voodoo programming at least without a proper comment explaining it. Thanks, tglx
