[PATCH v2] ARM: remove name from machine_desc for DT platforms
Rob Herring
robherring2 at gmail.com
Fri Nov 15 11:08:14 EST 2013
On 11/14/2013 02:33 PM, Matt Sealey wrote:
> On Sun, Nov 3, 2013 at 2:50 PM, Rob Herring <robherring2 at gmail.com> wrote:
>> From: Rob Herring <rob.herring at calxeda.com>
>
> Snip.. I actually reviewed what you're doing since I was
> understandably curious as to what the "plan" is for removing
> machine_desc and how you'd potentially avoid end up stashing global
> variables all over the place for early init code..
>
>> --- a/arch/arm/kernel/devtree.c
>> +++ b/arch/arm/kernel/devtree.c
>> @@ -199,7 +199,7 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
>> const struct machine_desc *mdesc, *mdesc_best = NULL;
>>
>> #ifdef CONFIG_ARCH_MULTIPLATFORM
>> - DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
>> + DT_MACHINE_START(GENERIC_DT)
>> MACHINE_END
>
> This machine doesn't have a .dt_compat so...
>
>> + early_print("%08x\t%s\n", p->nr, p->name ? p->name : *p->dt_compat);
>
> In your case here, isn't this going to print "calxeda,highbank" for
> you right now, with a broken tree, since it's the first in your
> dt_compat list?
>
> In the generic case, as noted above, the compatibility list usually comes out as
>
> ID (hex) NAME
> :
> 0xffffffff Highbank
> 0xffffffff Generic DT based system
>
> But you've made it say:
>
> ID (hex) NAME
> :
> 0xffffffff calxeda,highbank
> 0xffffffff
>
> You didn't fix your customer issue there, at least, if they're messing
> with their DT and break it ;)
This is not the issue I'm trying to fix as this is a debug early print.
It is what /proc/cpuinfo displays.
I don't think your example is even possible. If we have the default
descriptor enabled (i.e. all multi-platform builds), then we will always
match a machine descriptor and try to continue to boot never reaching
this code path.
>> @@ -859,7 +859,7 @@ void __init setup_arch(char **cmdline_p)
>> if (!mdesc)
>> mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
>> machine_desc = mdesc;
>> - machine_name = mdesc->name;
>> + machine_name = mdesc->name ? mdesc->name : of_flat_dt_get_machine_name();
>
> Also you missed around line 242 of kernel/devtree.c which prints
> mdesc_best->name;
You are looking at the wrong version. See the DT clean-up that I did
which is in mainline now. This patch is dependent on that.
>
> model = of_get_flat_dt_prop(dt_root, "model", NULL);
> if (!model)
> model = of_get_flat_dt_prop(dt_root, "compatible", NULL);
> if (!model)
> model = "<unknown>";
> pr_info("Machine: %s, model: %s\n", mdesc_best->name, model);
>
> Which is now going to say "Machine: , model: Calxeda Foo Board", isn't it?
>
> Should just use machine_name, if mdesc_best->name isn't set, shouldn't it?
>
> I think there's a bunch of weirdness here.. I just checked the PAPR
> and ePAPR specs, and the original IEEE1275 and ARM bindings and
> finding board information like you want from the DT is an unholy
> crapshoot.
Maybe in the past, but we require a root compatible string. If there is
no model property, then the compatible string is the machine name.
> In this case I think we need more information, and to only print it
> once per boot if that's the goal here. Why not make machine_name an
> argument to setup_machine_fdt (or just don't have it be static in
> setup.c) and have that function fill in the right thing rather than
> having a DT parsing function in setup_arch where before it was totally
> boot-method agnostic?
Well, that's an implementation detail. First we have to agree on which
string gets used and where.
Rob
More information about the linux-arm-kernel
mailing list