[U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS

[U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Simon Glass]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Simon Glass]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[York Sun]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Simon Glass]

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

[U-Boot] [PATCH 2/2] fdt_support: Don't panic if unable to perform stdout fixup

[Scott Wood]

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

[U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing

[Scott Wood]

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

[U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing

[Simon Glass]

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>

[U-Boot] [PATCH 1/2] arm/fsl-ls: Add CONFIG_OF_STDOUT_VIA_ALIAS

[York Sun]

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

[U-Boot] [PATCH v2 2/2] fdt_support: Don't panic if stdout alias is missing

[York Sun]

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