| Message ID | 1370974493-21822-18-git-send-email-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Simon, On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> wrote: > Implement this feature on ARM for tracing. > > It would be nice to have generic bootm support so that it is easily > implemented on any arch. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > Changes in v2: None > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > index 1b6e0ac..28fba19 100644 > --- a/arch/arm/lib/bootm.c > +++ b/arch/arm/lib/bootm.c > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t *images) > } > > /* Subcommand: GO */ > -static void boot_jump_linux(bootm_headers_t *images) > +static void boot_jump_linux(bootm_headers_t *image, int flag) What's the rationale of the s/images/image/ ? Amicalement,
Hi Albert, On Tue, Jun 11, 2013 at 12:59 PM, Albert ARIBAUD <albert.u.boot@aribaud.net>wrote: > Hi Simon, > > On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> > wrote: > > > Implement this feature on ARM for tracing. > > > > It would be nice to have generic bootm support so that it is easily > > implemented on any arch. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > Changes in v2: None > > > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > index 1b6e0ac..28fba19 100644 > > --- a/arch/arm/lib/bootm.c > > +++ b/arch/arm/lib/bootm.c > > > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t > *images) > > } > > > > /* Subcommand: GO */ > > -static void boot_jump_linux(bootm_headers_t *images) > > +static void boot_jump_linux(bootm_headers_t *image, int flag) > > What's the rationale of the s/images/image/ ? > Just that the function only accesses a single image, so I felt it was a misnomer. Regards, Simon > > Amicalement, > -- > Albert. >
Hi Simon, On Tue, 11 Jun 2013 13:02:56 -0700, Simon Glass <sjg@chromium.org> wrote: > Hi Albert, > > On Tue, Jun 11, 2013 at 12:59 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net>wrote: > > > Hi Simon, > > > > On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> > > wrote: > > > > > Implement this feature on ARM for tracing. > > > > > > It would be nice to have generic bootm support so that it is easily > > > implemented on any arch. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > Changes in v2: None > > > > > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > > index 1b6e0ac..28fba19 100644 > > > --- a/arch/arm/lib/bootm.c > > > +++ b/arch/arm/lib/bootm.c > > > > > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t > > *images) > > > } > > > > > > /* Subcommand: GO */ > > > -static void boot_jump_linux(bootm_headers_t *images) > > > +static void boot_jump_linux(bootm_headers_t *image, int flag) > > > > What's the rationale of the s/images/image/ ? > > > > Just that the function only accesses a single image, so I felt it was a > misnomer. I wonder if the naming was not initially chosen to remind readers that this single image file may actually contain several images (kernel+initrd, for instance. > Regards, > Simon Amicalement,
Hi Albert, On Tue, Jun 11, 2013 at 1:21 PM, Albert ARIBAUD <albert.u.boot@aribaud.net>wrote: > Hi Simon, > > On Tue, 11 Jun 2013 13:02:56 -0700, Simon Glass <sjg@chromium.org> > wrote: > > > Hi Albert, > > > > On Tue, Jun 11, 2013 at 12:59 PM, Albert ARIBAUD > > <albert.u.boot@aribaud.net>wrote: > > > > > Hi Simon, > > > > > > On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> > > > wrote: > > > > > > > Implement this feature on ARM for tracing. > > > > > > > > It would be nice to have generic bootm support so that it is easily > > > > implemented on any arch. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > Changes in v2: None > > > > > > > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > > > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > > > index 1b6e0ac..28fba19 100644 > > > > --- a/arch/arm/lib/bootm.c > > > > +++ b/arch/arm/lib/bootm.c > > > > > > > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t > > > *images) > > > > } > > > > > > > > /* Subcommand: GO */ > > > > -static void boot_jump_linux(bootm_headers_t *images) > > > > +static void boot_jump_linux(bootm_headers_t *image, int flag) > > > > > > What's the rationale of the s/images/image/ ? > > > > > > > Just that the function only accesses a single image, so I felt it was a > > misnomer. > > I wonder if the naming was not initially chosen to remind readers that > this single image file may actually contain several images > (kernel+initrd, for instance. > Ah yes, that could be it :-) Shall I change it back? Regards, Simon > > > Regards, > > Simon > > Amicalement, > -- > Albert. >
Hi Simon, On Tue, 11 Jun 2013 14:17:42 -0700, Simon Glass <sjg@chromium.org> wrote: > Hi Albert, > > > On Tue, Jun 11, 2013 at 1:21 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net>wrote: > > > Hi Simon, > > > > On Tue, 11 Jun 2013 13:02:56 -0700, Simon Glass <sjg@chromium.org> > > wrote: > > > > > Hi Albert, > > > > > > On Tue, Jun 11, 2013 at 12:59 PM, Albert ARIBAUD > > > <albert.u.boot@aribaud.net>wrote: > > > > > > > Hi Simon, > > > > > > > > On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> > > > > wrote: > > > > > > > > > Implement this feature on ARM for tracing. > > > > > > > > > > It would be nice to have generic bootm support so that it is easily > > > > > implemented on any arch. > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > Changes in v2: None > > > > > > > > > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > > > > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > > > > index 1b6e0ac..28fba19 100644 > > > > > --- a/arch/arm/lib/bootm.c > > > > > +++ b/arch/arm/lib/bootm.c > > > > > > > > > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t > > > > *images) > > > > > } > > > > > > > > > > /* Subcommand: GO */ > > > > > -static void boot_jump_linux(bootm_headers_t *images) > > > > > +static void boot_jump_linux(bootm_headers_t *image, int flag) > > > > > > > > What's the rationale of the s/images/image/ ? > > > > > > > > > > Just that the function only accesses a single image, so I felt it was a > > > misnomer. > > > > I wonder if the naming was not initially chosen to remind readers that > > this single image file may actually contain several images > > (kernel+initrd, for instance. > > > > Ah yes, that could be it :-) Shall I change it back? I'd personally prefer that you keep it plural as it was, yes. > Regards, > Simon Amicalement,
Hi Albert, On Tue, Jun 11, 2013 at 2:36 PM, Albert ARIBAUD <albert.u.boot@aribaud.net>wrote: > Hi Simon, > > On Tue, 11 Jun 2013 14:17:42 -0700, Simon Glass <sjg@chromium.org> > wrote: > > > Hi Albert, > > > > > > On Tue, Jun 11, 2013 at 1:21 PM, Albert ARIBAUD > > <albert.u.boot@aribaud.net>wrote: > > > > > Hi Simon, > > > > > > On Tue, 11 Jun 2013 13:02:56 -0700, Simon Glass <sjg@chromium.org> > > > wrote: > > > > > > > Hi Albert, > > > > > > > > On Tue, Jun 11, 2013 at 12:59 PM, Albert ARIBAUD > > > > <albert.u.boot@aribaud.net>wrote: > > > > > > > > > Hi Simon, > > > > > > > > > > On Tue, 11 Jun 2013 11:14:49 -0700, Simon Glass <sjg@chromium.org> > > > > > wrote: > > > > > > > > > > > Implement this feature on ARM for tracing. > > > > > > > > > > > > It would be nice to have generic bootm support so that it is > easily > > > > > > implemented on any arch. > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > --- > > > > > > Changes in v2: None > > > > > > > > > > > > arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ > > > > > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > > > > > > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > > > > > > index 1b6e0ac..28fba19 100644 > > > > > > --- a/arch/arm/lib/bootm.c > > > > > > +++ b/arch/arm/lib/bootm.c > > > > > > > > > > > @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t > > > > > *images) > > > > > > } > > > > > > > > > > > > /* Subcommand: GO */ > > > > > > -static void boot_jump_linux(bootm_headers_t *images) > > > > > > +static void boot_jump_linux(bootm_headers_t *image, int flag) > > > > > > > > > > What's the rationale of the s/images/image/ ? > > > > > > > > > > > > > Just that the function only accesses a single image, so I felt it > was a > > > > misnomer. > > > > > > I wonder if the naming was not initially chosen to remind readers that > > > this single image file may actually contain several images > > > (kernel+initrd, for instance. > > > > > > > Ah yes, that could be it :-) Shall I change it back? > > I'd personally prefer that you keep it plural as it was, yes. > OK I sent a v3 patch for this one. Regards, Simon
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 1b6e0ac..28fba19 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -68,12 +68,19 @@ void arch_lmb_reserve(struct lmb *lmb) gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size - sp); } -static void announce_and_cleanup(void) +/** + * announce_and_cleanup() - Print message and prepare for kernel boot + * + * @fake: non-zero to do everything except actually boot + */ +static void announce_and_cleanup(int fake) { - printf("\nStarting kernel ...\n\n"); + printf("\nStarting kernel ...%s\n\n", fake ? + "(fake run for tracing)" : ""); bootstage_mark_name(BOOTSTAGE_ID_BOOTM_HANDOFF, "start_kernel"); #ifdef CONFIG_BOOTSTAGE_FDT - bootstage_fdt_add_report(); + if (flag == BOOTM_STATE_OS_FAKE_GO) + bootstage_fdt_add_report(); #endif #ifdef CONFIG_BOOTSTAGE_REPORT bootstage_report(); @@ -225,14 +232,15 @@ static void boot_prep_linux(bootm_headers_t *images) } /* Subcommand: GO */ -static void boot_jump_linux(bootm_headers_t *images) +static void boot_jump_linux(bootm_headers_t *image, int flag) { unsigned long machid = gd->bd->bi_arch_number; char *s; void (*kernel_entry)(int zero, int arch, uint params); unsigned long r2; + int fake = (flag & BOOTM_STATE_OS_FAKE_GO); - kernel_entry = (void (*)(int, int, uint))images->ep; + kernel_entry = (void (*)(int, int, uint))image->ep; s = getenv("machid"); if (s) { @@ -243,14 +251,15 @@ static void boot_jump_linux(bootm_headers_t *images) debug("## Transferring control to Linux (at address %08lx)" \ "...\n", (ulong) kernel_entry); bootstage_mark(BOOTSTAGE_ID_RUN_OS); - announce_and_cleanup(); + announce_and_cleanup(fake); - if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) - r2 = (unsigned long)images->ft_addr; + if (IMAGE_ENABLE_OF_LIBFDT && image->ft_len) + r2 = (unsigned long)image->ft_addr; else r2 = gd->bd->bi_boot_params; - kernel_entry(0, machid, r2); + if (!fake) + kernel_entry(0, machid, r2); } /* Main Entry point for arm bootm implementation @@ -270,13 +279,13 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) return 0; } - if (flag & BOOTM_STATE_OS_GO) { - boot_jump_linux(images); + if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) { + boot_jump_linux(images, flag); return 0; } boot_prep_linux(images); - boot_jump_linux(images); + boot_jump_linux(images, flag); return 0; }
Implement this feature on ARM for tracing. It would be nice to have generic bootm support so that it is easily implemented on any arch. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: None arch/arm/lib/bootm.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-)