On 10/28/2014 06:27 PM, Leif Lindholm wrote:
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 :)
Hi Leif, np, will do this way.
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,
OK, that makes sense, you will see this in next patchset.
but the change is valid even without the multiboot support..
Yes, I see.
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