Re: [PATCH] ARM/multiboot: use more flexible node naming

[Andre Przywara <andre.przywara@linaro.org>]

On 09/09/2013 02:00 PM, Ian Campbell wrote:
> On Thu, 2013-09-05 at 15:43 +0200, Andre Przywara wrote:
>> For the current "multiboot" on ARM support we look for a compatible
>> string of "xen,multiboot-module" in the device tree, and then
>> use "xen,linux-zimage" and "xen,linux-initrd" to differentiate
>> between the two supported module types.
>> To meet the more generic multiboot proposal in the device tree [1],
>> allow Xen to be more flexible in the compatible naming and also use
>> the new generic base name "boot,module".
>> The mapping to either Dom0 kernel or RAM disk works either by
>> providing a more specific name ("xen,dom0-kernel" and "xen,ramdisk"),
>> or by using the enumeration order of the device tree nodes
>> (module@0 = kernel, module@1 = initrd). This allows bootloaders
>> without any specific Xen knowledge to boot Xen anyways.
>>
>> [1] http://lists.xen.org/archives/html/xen-devel/2013-09/msg00083.html
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   xen/common/device_tree.c | 57 ++++++++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 50 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index ec0d5e2..e10c035 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -439,22 +439,63 @@ static void __init process_cpu_node(const void *fdt, int node,
>>       cpumask_set_cpu(start, &cpu_possible_map);
>>   }
>>
>> +static const char * const kernel_module_names[] = {
>> +	"xen,linux-zimage",
>> +	"xen,dom0-kernel",
>> +	"boot,kernel",
>> +	NULL
>> +};
>> +
>> +static const char * const initrd_module_names[] = {
>> +	"xen,linux-initrd",
>> +	"xen,ramdisk",
>> +	"boot,ramdisk",
>> +	NULL
>> +};
>> +
>>   static void __init process_multiboot_node(const void *fdt, int node,
>>                                             const char *name,
>>                                             u32 address_cells, u32 size_cells)
>>   {
>>       const struct fdt_property *prop;
>>       const u32 *cell;
>> -    int nr;
>> +    int nr = -1;
>>       struct dt_mb_module *mod;
>>       int len;
>> +    const char* const * name_list;
>>
>> -    if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
>> -        nr = MOD_KERNEL;
>> -    else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
>> -        nr = MOD_INITRD;
>> -    else
>> -        early_panic("%s not a known xen multiboot type\n", name);
>> +    for (name_list = kernel_module_names; *name_list != NULL; name_list++)
>> +        if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 ) {
>> +            nr = MOD_KERNEL;
>> +            break;
>> +        }
>> +
>> +    for (name_list = initrd_module_names; *name_list != NULL; name_list++)
>> +        if ( fdt_node_check_compatible(fdt, node, *name_list) == 0 ) {
>
> I think Julien's big dtb series adds a helper which would make this
> simpler?
>
> That could be a later fix up, are there any other ordering constraints
> with that series?

Not that I am aware of. I think I will send out my new version so far 
for discussion and review and will happily rebase it later on if needed.

Thanks,
Andre.