| Message ID | 20170112161955.14631-1-afd@ti.com |
|---|---|
| State | Accepted |
| Commit | cf947da19aecd1b430f22d73ad109af64a6051b5 |
| Delegated to: | Tom Rini |
| Headers | show |
On Thursday 12 January 2017 09:49 PM, Andrew F. Davis wrote: > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > common/spl/spl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index a76ea3a603..e43718de62 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, > loader = spl_ll_find_loader(spl_boot_list[i]); > #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > if (loader) > - printf("Trying to boot from %s", loader->name); > + printf("Trying to boot from %s\n", loader->name); > else > puts("SPL: Unsupported Boot Device!\n"); > #endif > @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) > gd->malloc_ptr / 1024); > #endif > > - debug("loaded - jumping to U-Boot..."); > + debug("loaded - jumping to U-Boot...\n"); Acked-by: Lokesh Vutla <lokeshvutla@ti.com> Thanks and regards, Lokesh
2017-01-12 17:19 GMT+01:00 Andrew F. Davis <afd@ti.com>: > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > common/spl/spl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index a76ea3a603..e43718de62 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info > *spl_image, > loader = spl_ll_find_loader(spl_boot_list[i]); > #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_ > SUPPORT) > if (loader) > - printf("Trying to boot from %s", loader->name); > + printf("Trying to boot from %s\n", loader->name); > else > puts("SPL: Unsupported Boot Device!\n"); > #endif > @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) > gd->malloc_ptr / 1024); > #endif > > - debug("loaded - jumping to U-Boot..."); > + debug("loaded - jumping to U-Boot...\n"); > spl_board_prepare_for_boot(); > jump_to_image_no_args(&spl_image); > } > -- > 2.11.0 > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On Thu, Jan 12, 2017 at 10:19:55AM -0600, Andrew F. Davis wrote: > Signed-off-by: Andrew F. Davis <afd@ti.com> > Acked-by: Lokesh Vutla <lokeshvutla@ti.com> > Acked-by: Michal Simek <michal.simek@xilinx.com> Applied to u-boot/master, thanks!
Hi Andrew, On 12 January 2017 at 09:19, Andrew F. Davis <afd@ti.com> wrote: > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > common/spl/spl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl.c b/common/spl/spl.c > index a76ea3a603..e43718de62 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, > loader = spl_ll_find_loader(spl_boot_list[i]); > #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) > if (loader) > - printf("Trying to boot from %s", loader->name); > + printf("Trying to boot from %s\n", loader->name); > else > puts("SPL: Unsupported Boot Device!\n"); > #endif > @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) > gd->malloc_ptr / 1024); > #endif > > - debug("loaded - jumping to U-Boot..."); > + debug("loaded - jumping to U-Boot...\n"); I prefer this one as it is, since U-Boot prints a few newlines anyway, and this way we can have the cursor at the end of the 'jumping' line until U-Boot starts. What's the rationale for changing it. Could you add a commit message? > spl_board_prepare_for_boot(); > jump_to_image_no_args(&spl_image); > } > -- > 2.11.0 > Regards, Simon
On 01/20/2017 09:51 PM, Simon Glass wrote: > Hi Andrew, > > On 12 January 2017 at 09:19, Andrew F. Davis <afd@ti.com> wrote: >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> --- >> common/spl/spl.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/common/spl/spl.c b/common/spl/spl.c >> index a76ea3a603..e43718de62 100644 >> --- a/common/spl/spl.c >> +++ b/common/spl/spl.c >> @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, >> loader = spl_ll_find_loader(spl_boot_list[i]); >> #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) >> if (loader) >> - printf("Trying to boot from %s", loader->name); >> + printf("Trying to boot from %s\n", loader->name); >> else >> puts("SPL: Unsupported Boot Device!\n"); >> #endif >> @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) >> gd->malloc_ptr / 1024); >> #endif >> >> - debug("loaded - jumping to U-Boot..."); >> + debug("loaded - jumping to U-Boot...\n"); > > I prefer this one as it is, since U-Boot prints a few newlines anyway, > and this way we can have the cursor at the end of the 'jumping' line > until U-Boot starts. > > What's the rationale for changing it. Could you add a commit message? > Looks like this already has be taken, but I'll explain myself anyway. The way I see it, for consistency sake, the only reason a print statement should not end in a newline is iff they expect something to be printed on the same line after. This is not the case here, we *do* want a newline after this statement, we are just expecting it to be handled later (hopefully). Not sticking to this standard will lead to a lot of print statements starting with '\n' to be safe. For instance even if we knew what follows should emit some newlines, this is a debug statement, it may not printed, so the following line would still have to begin with a newline "just in-case", we would end up with half our print out lines with two new lines above them. Andrew
Hi Andrew, On 22 January 2017 at 14:35, Andrew F. Davis <afd@ti.com> wrote: > On 01/20/2017 09:51 PM, Simon Glass wrote: >> Hi Andrew, >> >> On 12 January 2017 at 09:19, Andrew F. Davis <afd@ti.com> wrote: >>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>> --- >>> common/spl/spl.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/common/spl/spl.c b/common/spl/spl.c >>> index a76ea3a603..e43718de62 100644 >>> --- a/common/spl/spl.c >>> +++ b/common/spl/spl.c >>> @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, >>> loader = spl_ll_find_loader(spl_boot_list[i]); >>> #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) >>> if (loader) >>> - printf("Trying to boot from %s", loader->name); >>> + printf("Trying to boot from %s\n", loader->name); >>> else >>> puts("SPL: Unsupported Boot Device!\n"); >>> #endif >>> @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) >>> gd->malloc_ptr / 1024); >>> #endif >>> >>> - debug("loaded - jumping to U-Boot..."); >>> + debug("loaded - jumping to U-Boot...\n"); >> >> I prefer this one as it is, since U-Boot prints a few newlines anyway, >> and this way we can have the cursor at the end of the 'jumping' line >> until U-Boot starts. >> >> What's the rationale for changing it. Could you add a commit message? >> > > Looks like this already has be taken, but I'll explain myself anyway. > > The way I see it, for consistency sake, the only reason a print > statement should not end in a newline is iff they expect something to be > printed on the same line after. This is not the case here, we *do* want > a newline after this statement, we are just expecting it to be handled > later (hopefully). Not sticking to this standard will lead to a lot of > print statements starting with '\n' to be safe. For instance even if we > knew what follows should emit some newlines, this is a debug statement, > it may not printed, so the following line would still have to begin with > a newline "just in-case", we would end up with half our print out lines > with two new lines above them. Of course you are right in general and I agree with your rule. But in this case we know we are jumping to U-Boot, and that U-Boot prints a few newlines at the start. I suppose you could argue that you might turn on some debug UART output early in U-Boot which would mess that up. If you made that argument then I might agree with you :-) But for most users this avoids an unnecessary newline. Regards, Simon
diff --git a/common/spl/spl.c b/common/spl/spl.c index a76ea3a603..e43718de62 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -316,7 +316,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, loader = spl_ll_find_loader(spl_boot_list[i]); #if defined(CONFIG_SPL_SERIAL_SUPPORT) && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) if (loader) - printf("Trying to boot from %s", loader->name); + printf("Trying to boot from %s\n", loader->name); else puts("SPL: Unsupported Boot Device!\n"); #endif @@ -389,7 +389,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024); #endif - debug("loaded - jumping to U-Boot..."); + debug("loaded - jumping to U-Boot...\n"); spl_board_prepare_for_boot(); jump_to_image_no_args(&spl_image); }
Signed-off-by: Andrew F. Davis <afd@ti.com> --- common/spl/spl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)