| Message ID | 1303225269-10092-1-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Shawn, > It fixes the return value of funciont early_init_dt_scan_memory on > the success return path. [In general, the changelog should explain why you're making this change, not just re-iterate what the patch does. Does this fix a problem you were seeing?] With regards to this specific patch - I don't think this is correct; if we return 1 here, we'll abort the of_scan_flat_dt loop after successfully parsing one memory node, whereas machines may have multiple nodes. This change will break booting on those machines. Cheers, Jeremy
On Tue, Apr 19, 2011 at 6:47 PM, Jeremy Kerr <jeremy.kerr@canonical.com> wrote: > Hi Shawn, > >> It fixes the return value of funciont early_init_dt_scan_memory on >> the success return path. > > [In general, the changelog should explain why you're making this change, > not just re-iterate what the patch does. Does this fix a problem you > were seeing?] > > With regards to this specific patch - I don't think this is correct; if > we return 1 here, we'll abort the of_scan_flat_dt loop after > successfully parsing one memory node, whereas machines may have multiple > nodes. This change will break booting on those machines. > > Cheers, Yes, the current code is correct. All of the memory nodes are supposed to be processed. g.
Hi Jeremy, On Wed, Apr 20, 2011 at 08:47:26AM +0800, Jeremy Kerr wrote: > Hi Shawn, > > > It fixes the return value of funciont early_init_dt_scan_memory on > > the success return path. > > [In general, the changelog should explain why you're making this change, > not just re-iterate what the patch does. Does this fix a problem you > were seeing?] > Thanks for the guide. > With regards to this specific patch - I don't think this is correct; if > we return 1 here, we'll abort the of_scan_flat_dt loop after > successfully parsing one memory node, whereas machines may have multiple > nodes. This change will break booting on those machines. > I really did not think of the case that there are multiple memory nodes. Please ignore it, and sorry for the noise.
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index c9db49c..387336d 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -659,7 +659,7 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname, early_init_dt_add_memory_arch(base, size); } - return 0; + return 1; } int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
It fixes the return value of funciont early_init_dt_scan_memory on the success return path. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/of/fdt.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)