Hi Fu Wei,
(I don't think we need to cc Andrea, Jon and Ilias on the patch review, they get enough email as it is, and we are posting to linaro-uefi list as well :)
On Tue, Oct 28, 2014 at 01:31:07PM +0800, Fu Wei wrote:
Hi Leif,
Great thanks for your review! I have made some improvements(which marked DONE below), following your suggestion.
I may do one more improvement according to your feedback
There is one question inline below.
node = grub_fdt_find_subnode (fdt, 0, "chosen"); @@ -189,13 +244,6 @@ grub_cmd_devicetree (grub_command_t cmd __attribute__ ((unused)), void *blob = NULL; int size;
- if (!loaded)
- {
grub_error (GRUB_ERR_BAD_ARGUMENT,N_("you need to load the kernel first"));return GRUB_ERR_BAD_OS;- }
This would make sense as a separate patch - it is not logically connected with the new functionality.
what do you mean about "a separate patch"? a patch in the same patchset or outside this patchset?
A separate patch in the same patchset makes sense since it is a prerequisite, but the change is valid even without the multiboot support..
without this change, the multiboot patch won't work.
they don't share loaded status. And this loaded belong to linux.c, so if we do multiboot, this loaded == 0, so boot fail.
Yes.
/ Leif