Hi Scott, On 1 September 2015 at 21:00, Scott Wood <[email protected]> wrote: > On Tue, 2015-09-01 at 20:48 -0600, Simon Glass wrote: >> Hi Scott, >> >> On 31 August 2015 at 21:16, Scott Wood <[email protected]> wrote: >> > On Mon, 2015-08-31 at 21:13 -0600, Simon Glass wrote: >> > > Hi Scott, >> > > >> > > On 31 August 2015 at 20:11, Scott Wood <[email protected]> wrote: >> > > > Currently, using fdt_fixup_stdout() on a device tree that is missing >> > > > the relevant alias results in this: >> > > > >> > > > WARNING: could not set linux,stdout-path FDT_ERR_NOTFOUND. >> > > > ERROR: /chosen node create failed >> > > > - must RESET the board to recover. >> > > > >> > > > FDT creation failed! hanging...### ERROR ### Please RESET the board >> > > > ### >> > > > >> > > > There is no reason for this to be a fatal error rather than a warning, >> > > > and removing this allows for a smooth transition on a platform where >> > > > the device tree currently lacks the correct aliases but will have them >> > > > in the future. >> > > >> > > Why do we need this patch - what platform? >> > >> > LS2085A >> > >> > > >> > > > >> > > > Signed-off-by: Scott Wood <[email protected]> >> > > > Cc: Kumar Gala <[email protected]> >> > > > Cc: Simon Glass <[email protected]> >> > > > --- >> > > > Resent with correct address for Simon Glass. >> > > > >> > > > common/fdt_support.c | 3 ++- >> > > > 1 file changed, 2 insertions(+), 1 deletion(-) >> > > > >> > > > diff --git a/common/fdt_support.c b/common/fdt_support.c >> > > > index f86365e..6052c77 100644 >> > > > --- a/common/fdt_support.c >> > > > +++ b/common/fdt_support.c >> > > > @@ -308,7 +308,8 @@ int fdt_chosen(void *fdt) >> > > > } >> > > > } >> > > > >> > > > - return fdt_fixup_stdout(fdt, nodeoffset); >> > > > + fdt_fixup_stdout(fdt, nodeoffset); >> > > >> > > Will some platforms will not boot correctly with this failing? Should >> > > we make your new feature a Kconfig options perhaps? I worry that it >> > > will become the default behaviour and then it will be hard to remove >> > > later. >> > >> > A warning will still be printed. I'm not sure how "### ERROR ### Please >> > RESET the board ###" is more useful than trying to continue and possibly >> > failing. >> >> Only that if it indicates a fatal error the board code can at least >> find out about it and deal with it. Perhaps booting will just result >> in a hang? > > If booting results in a hang, then you're no worse off than the current > > situation. Either way, the user sees a message that indicates the problem. > >> I think ignoring errors is fine but here we make it impossible to >> detect a failure. So I think that a Kconfig is the best idea, so we >> can remove it later. > > It seems excessive... Config options aren't free, from a maintenance > > perspective, and I have a hard time imagining a scenario where proceeding to > > boot causes a real problem. > > Also, from a consistency perspective: > - The OF_STDOUT_PATH version doesn't cause a panic if the destination path > doesn't exist. > - None of the callers of fdt_status_<foo>_by_alias() panic if the alias is > not found. > - do_fixup_by_path() doesn't panic if the node is not found. > - Neither does do_fixup_by_compatible(). > - Neither does fdt_fixup_ethernet().
Yes this code needs cleaning up. The particular problem you have is that there are no aliases, right? How about returning 0 from fdt_fixup_stdout() in that case? Then you will not affect the behaviour of other boards which perhaps do have /aliases but do not have space in their tree for the fdt_setprop(). Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
