On Tue, 2015-09-01 at 20:48 -0600, Simon Glass wrote:
Hi Scott,
On 31 August 2015 at 21:16, Scott Wood scottwood@freescale.com wrote:
On Mon, 2015-08-31 at 21:13 -0600, Simon Glass wrote:
Hi Scott,
On 31 August 2015 at 20:11, Scott Wood scottwood@freescale.com 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 scottwood@freescale.com Cc: Kumar Gala galak@kernel.crashing.org Cc: Simon Glass sjg@chromium.org
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().
-Scott