On Thu, 2014-06-05 at 17:01 +0800, Fu Wei wrote:
On 06/05/2014 04:56 PM, Fu Wei wrote:
On 06/05/2014 04:23 PM, Ian Campbell wrote:
On Wed, 2014-05-07 at 19:58 +0800, Fu Wei wrote:
+#define XEN_MODULE_IMAGE_NAME "linux-zimage" +#define XEN_MODULE_INITRD_NAME "linux-initrd" +#define XEN_MODULE_XSM_NAME "xsm-policy" +#define XEN_MODULE_OTHER_NAME "multiboot-module"
These get used as the node names I think, as in /chosen/module@linux-zimage. AIUI Device Tree convention would be /chosen/module@<address> where <address> is the physical address (i.e. the same as the base address in the reg property).
yes, my patch is doing /chosen/module@<address>, but not /chosen/module@<module_name>
Please check : static grub_err_t prepare_module_params_xen (multiboot_module_t module) in multiboot_xen.c :
char module_name[XEN_MODULE_NAME_MAX_SIZE];
if (module && module->extra_info) { node_info = (xen_node_info_t) module->extra_info; } else { return (grub_error (GRUB_ERR_BAD_ARGUMENT, N_("failed to get module"))); }
retval = grub_snprintf(module_name, XEN_MODULE_NAME_MAX_SIZE, "module@%d", module_index++);
Sorry, here maybe a bug, I should use "address", but not "index", will fix it in next version.
Thanks for pointing it out. :-)
I was pointing out a bug which didn't exist, but at least I caused you to notice an actual bug ;-)
if (retval <(int) sizeof("module@")) return (grub_error(GRUB_ERR_BAD_OS, N_("failed to set node name for module")));
module_node = grub_fdt_find_subnode (multiboot_fdt, chosen_node, module_name); if (module_node < 0) module_node = grub_fdt_add_subnode (multiboot_fdt, chosen_node, module_name);
These "XEN_MODULE_*_NAME"s is just for the named list I used in multiboot.c/multiboot.h. It's easy to get a module node in the named list by "name".
My mistake. So it's just for debug output then I think? Seems reasonable.
BTW due to the \0 embedded in the compat string you can just printf module->compatible which will give you nearly the same thing. You'd get an extra "multiboot," in each of your debug lines using this, but that's not the end of the world I think and saves you having to worry about another field etc.
Ian.