* [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS
@ 2015-09-01 2:05 Scott Wood
2015-09-01 2:05 ` [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup Scott Wood
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Scott Wood @ 2015-09-01 2:05 UTC (permalink / raw)
To: u-boot
This will allow OF-based earlycon to be used once the appropriate
aliases are added to the device tree and kernel support is fixed.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
include/configs/ls1021aqds.h | 1 +
include/configs/ls1021atwr.h | 1 +
include/configs/ls2085a_common.h | 1 +
3 files changed, 3 insertions(+)
diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h
index dfaffa1..a7d12b8 100644
--- a/include/configs/ls1021aqds.h
+++ b/include/configs/ls1021aqds.h
@@ -654,6 +654,7 @@ unsigned long get_board_ddr_clk(void);
#define CONFIG_OF_LIBFDT
#define CONFIG_OF_BOARD_SETUP
+#define CONFIG_OF_STDOUT_VIA_ALIAS
#define CONFIG_CMD_BOOTZ
#define CONFIG_MISC_INIT_R
diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
index 3299a9f..f318162 100644
--- a/include/configs/ls1021atwr.h
+++ b/include/configs/ls1021atwr.h
@@ -496,6 +496,7 @@
#define CONFIG_OF_LIBFDT
#define CONFIG_OF_BOARD_SETUP
+#define CONFIG_OF_STDOUT_VIA_ALIAS
#define CONFIG_CMD_BOOTZ
#define CONFIG_MISC_INIT_R
diff --git a/include/configs/ls2085a_common.h b/include/configs/ls2085a_common.h
index 39fb464..96abb92 100644
--- a/include/configs/ls2085a_common.h
+++ b/include/configs/ls2085a_common.h
@@ -47,6 +47,7 @@
/* Flat Device Tree Definitions */
#define CONFIG_OF_LIBFDT
#define CONFIG_OF_BOARD_SETUP
+#define CONFIG_OF_STDOUT_VIA_ALIAS
/* new uImage format support */
#define CONFIG_FIT
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-01 2:05 [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS Scott Wood @ 2015-09-01 2:05 ` Scott Wood 2015-09-01 2:11 ` Scott Wood 2015-10-30 16:11 ` [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS York Sun 2 siblings, 0 replies; 15+ messages in thread From: Scott Wood @ 2015-09-01 2:05 UTC (permalink / raw) To: u-boot 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. Signed-off-by: Scott Wood <scottwood@freescale.com> Cc: Kumar Gala <galak@kernel.crashing.org> Cc: Simon Glass <sgj@chromium.org> --- 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); + return 0; } void do_fixup_by_path(void *fdt, const char *path, const char *prop, -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-01 2:05 [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS Scott Wood 2015-09-01 2:05 ` [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup Scott Wood @ 2015-09-01 2:11 ` Scott Wood 2015-09-01 3:13 ` Simon Glass 2015-09-02 3:48 ` [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing Scott Wood 2015-10-30 16:11 ` [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS York Sun 2 siblings, 2 replies; 15+ messages in thread From: Scott Wood @ 2015-09-01 2:11 UTC (permalink / raw) To: u-boot 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. 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); + return 0; } void do_fixup_by_path(void *fdt, const char *path, const char *prop, -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-01 2:11 ` Scott Wood @ 2015-09-01 3:13 ` Simon Glass 2015-09-01 3:16 ` Scott Wood 2015-09-02 3:48 ` [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing Scott Wood 1 sibling, 1 reply; 15+ messages in thread From: Simon Glass @ 2015-09-01 3:13 UTC (permalink / raw) To: u-boot 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? > > 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. > + return 0; > } > > void do_fixup_by_path(void *fdt, const char *path, const char *prop, > -- > 2.1.4 > Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-01 3:13 ` Simon Glass @ 2015-09-01 3:16 ` Scott Wood 2015-09-02 2:48 ` Simon Glass 0 siblings, 1 reply; 15+ messages in thread From: Scott Wood @ 2015-09-01 3:16 UTC (permalink / raw) To: u-boot 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. -Scott ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-01 3:16 ` Scott Wood @ 2015-09-02 2:48 ` Simon Glass 2015-09-02 3:00 ` Scott Wood 2015-09-02 3:04 ` York Sun 0 siblings, 2 replies; 15+ messages in thread From: Simon Glass @ 2015-09-02 2:48 UTC (permalink / raw) To: u-boot 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? 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. Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-02 2:48 ` Simon Glass @ 2015-09-02 3:00 ` Scott Wood 2015-09-02 3:10 ` Simon Glass 2015-09-02 3:04 ` York Sun 1 sibling, 1 reply; 15+ messages in thread From: Scott Wood @ 2015-09-02 3:00 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-02 3:00 ` Scott Wood @ 2015-09-02 3:10 ` Simon Glass 2015-09-02 3:14 ` Scott Wood 0 siblings, 1 reply; 15+ messages in thread From: Simon Glass @ 2015-09-02 3:10 UTC (permalink / raw) To: u-boot Hi Scott, On 1 September 2015 at 21:00, Scott Wood <scottwood@freescale.com> wrote: > 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(). 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-02 3:10 ` Simon Glass @ 2015-09-02 3:14 ` Scott Wood 0 siblings, 0 replies; 15+ messages in thread From: Scott Wood @ 2015-09-02 3:14 UTC (permalink / raw) To: u-boot On Tue, 2015-09-01 at 21:10 -0600, Simon Glass wrote: > Hi Scott, > > On 1 September 2015 at 21:00, Scott Wood <scottwood@freescale.com> wrote: > > 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(). > > Yes this code needs cleaning up. I wasn't citing them as problems. :-P > 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(). That would be fine. -Scott ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-02 2:48 ` Simon Glass 2015-09-02 3:00 ` Scott Wood @ 2015-09-02 3:04 ` York Sun 2015-09-02 3:06 ` Scott Wood 1 sibling, 1 reply; 15+ messages in thread From: York Sun @ 2015-09-02 3:04 UTC (permalink / raw) To: u-boot On 09/01/2015 09:48 PM, 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? > > 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. How about a big warning instead? In general, having the message to reset the board doesn't help much if it is a fatal condition. We have to use external tool to recover the board. On the other side, if the error is not fatal, continue to boot may give the user a chance to reflash an update. York ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup 2015-09-02 3:04 ` York Sun @ 2015-09-02 3:06 ` Scott Wood 0 siblings, 0 replies; 15+ messages in thread From: Scott Wood @ 2015-09-02 3:06 UTC (permalink / raw) To: u-boot On Tue, 2015-09-01 at 22:04 -0500, York Sun wrote: > On 09/01/2015 09:48 PM, 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? > > > > 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. > > How about a big warning instead? There is still a warning with this patch applied. > In general, having the message to reset the > board doesn't help much if it is a fatal condition. We have to use external > tool > to recover the board. > On the other side, if the error is not fatal, continue to > boot may give the user a chance to reflash an update. That doesn't really apply to errors that happen when loading a kernel... -Scott ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing 2015-09-01 2:11 ` Scott Wood 2015-09-01 3:13 ` Simon Glass @ 2015-09-02 3:48 ` Scott Wood 2015-09-02 14:05 ` Simon Glass 2015-10-30 16:12 ` York Sun 1 sibling, 2 replies; 15+ messages in thread From: Scott Wood @ 2015-09-02 3:48 UTC (permalink / raw) To: u-boot 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. Signed-off-by: Scott Wood <scottwood@freescale.com> Cc: Kumar Gala <galak@kernel.crashing.org> Cc: Simon Glass <sjg@chromium.org> --- v2: Only continue booting if the problem was a missing alias, not the inability to write to the device tree. --- common/fdt_support.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/common/fdt_support.c b/common/fdt_support.c index f86365e..a7ff2df 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -158,25 +158,30 @@ static int fdt_fixup_stdout(void *fdt, int chosenoff) aliasoff = fdt_path_offset(fdt, "/aliases"); if (aliasoff < 0) { err = aliasoff; - goto error; + goto noalias; } path = fdt_getprop(fdt, aliasoff, sername, &len); if (!path) { err = len; - goto error; + goto noalias; } /* fdt_setprop may break "path" so we copy it to tmp buffer */ memcpy(tmp, path, len); err = fdt_setprop(fdt, chosenoff, "linux,stdout-path", tmp, len); -error: if (err < 0) printf("WARNING: could not set linux,stdout-path %s.\n", fdt_strerror(err)); return err; + +noalias: + printf("WARNING: %s: could not read %s alias: %s\n", + __func__, sername, fdt_strerror(err)); + + return 0; } #else static int fdt_fixup_stdout(void *fdt, int chosenoff) -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing 2015-09-02 3:48 ` [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing Scott Wood @ 2015-09-02 14:05 ` Simon Glass 2015-10-30 16:12 ` York Sun 1 sibling, 0 replies; 15+ messages in thread From: Simon Glass @ 2015-09-02 14:05 UTC (permalink / raw) To: u-boot On 1 September 2015 at 21:48, 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. > > Signed-off-by: Scott Wood <scottwood@freescale.com> > Cc: Kumar Gala <galak@kernel.crashing.org> > Cc: Simon Glass <sjg@chromium.org> > --- > v2: Only continue booting if the problem was a missing alias, not > the inability to write to the device tree. > --- > common/fdt_support.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing 2015-09-02 3:48 ` [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing Scott Wood 2015-09-02 14:05 ` Simon Glass @ 2015-10-30 16:12 ` York Sun 1 sibling, 0 replies; 15+ messages in thread From: York Sun @ 2015-10-30 16:12 UTC (permalink / raw) To: u-boot On 09/01/2015 08:48 PM, Scott Wood 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. > > Signed-off-by: Scott Wood <scottwood@freescale.com> > Cc: Kumar Gala <galak@kernel.crashing.org> > Cc: Simon Glass <sjg@chromium.org> > --- > v2: Only continue booting if the problem was a missing alias, not > the inability to write to the device tree. > --- > common/fdt_support.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > Applied to u-boot-fsl-qoriq. Awaiting upstream. Thanks. York ^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS 2015-09-01 2:05 [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS Scott Wood 2015-09-01 2:05 ` [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup Scott Wood 2015-09-01 2:11 ` Scott Wood @ 2015-10-30 16:11 ` York Sun 2 siblings, 0 replies; 15+ messages in thread From: York Sun @ 2015-10-30 16:11 UTC (permalink / raw) To: u-boot On 08/31/2015 07:05 PM, Scott Wood wrote: > This will allow OF-based earlycon to be used once the appropriate > aliases are added to the device tree and kernel support is fixed. > > Signed-off-by: Scott Wood <scottwood@freescale.com> > --- Applied to u-boot-fsl-qoriq. Awaiting upstream. Thanks. York ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-10-30 16:12 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-01 2:05 [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS Scott Wood 2015-09-01 2:05 ` [U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup Scott Wood 2015-09-01 2:11 ` Scott Wood 2015-09-01 3:13 ` Simon Glass 2015-09-01 3:16 ` Scott Wood 2015-09-02 2:48 ` Simon Glass 2015-09-02 3:00 ` Scott Wood 2015-09-02 3:10 ` Simon Glass 2015-09-02 3:14 ` Scott Wood 2015-09-02 3:04 ` York Sun 2015-09-02 3:06 ` Scott Wood 2015-09-02 3:48 ` [U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing Scott Wood 2015-09-02 14:05 ` Simon Glass 2015-10-30 16:12 ` York Sun 2015-10-30 16:11 ` [U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS York Sun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox