[U-Boot] [PATCH V3] BOOT: Add "bootz" command to boot Linux zImage

[Marek Vasut]

Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1331588061-21546-1-git-send-email-marex at denx.de> you wrote:
> > This command boots Linux zImage from where the zImage is loaded to.
> > Passing initrd and fdt is supported.
> 
> I have but a few formal questions / issues.

I had a dilemma about the image.[ch] indeed. I was quite unsure where to put 
those, can you suggest proper location? I didn't want to create new file only 
for that matter.

> 
> ...
> 
> > --- a/common/cmd_bootm.c
> > +++ b/common/cmd_bootm.c
> > @@ -160,35 +160,8 @@ static boot_os_fn *boot_os[] = {
> > 
> >  bootm_headers_t images;		/* pointers to os/initrd/fdt images */
> > 
> > -/* Allow for arch specific config before we boot */
> > -void __arch_preboot_os(void)
> > -{
> > -	/* please define platform specific arch_preboot_os() */
> > -}
> > -void arch_preboot_os(void) __attribute__((weak,
> > alias("__arch_preboot_os"))); -
> > 
> >  #define IH_INITRD_ARCH IH_ARCH_DEFAULT
> > 
> > -static void bootm_start_lmb(void)
> > -{
> 
> ...
> 
> Note: this is bootm (or boot*) related code.
> 
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -3190,3 +3190,28 @@ static int fit_check_ramdisk(const void *fit, int
> > rd_noffset, uint8_t arch,
> 
> This is a file that deals with U-Boot images etc.  It is in no way
> related to any of the "boot*" commands.
> 
> > +#ifdef	CONFIG_LMB
> > +void boot_start_lmb(bootm_headers_t *images)
> > +{
> > +	ulong		mem_start;
> > +	phys_size_t	mem_size;
> > +
> > +	lmb_init(&images->lmb);
> > +
> > +	mem_start = getenv_bootm_low();
> > +	mem_size = getenv_bootm_size();
> > +
> > +	lmb_add(&images->lmb, (phys_addr_t)mem_start, mem_size);
> > +
> > +	arch_lmb_reserve(&images->lmb);
> > +	board_lmb_reserve(&images->lmb);
> > +}
> > +#endif
> > +
> > +/* Allow for arch specific config before we boot */
> > +void __arch_preboot_os(void)
> > +{
> > +	/* please define platform specific arch_preboot_os() */
> > +}
> > +void arch_preboot_os(void) __attribute__((weak,
> > alias("__arch_preboot_os")));
> 
> Putting this code here makes no sense.  You could chose any othewr
> random file as well.
> 
> > diff --git a/include/image.h b/include/image.h
> > index bbf80f0..26b8a20 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -486,6 +486,14 @@ void image_multi_getimg(const image_header_t *hdr,
> > ulong idx,
> > 
> >  void image_print_contents(const void *hdr);
> > 
> > +#ifdef CONFIG_LMB
> > +void boot_start_lmb(bootm_headers_t *images);
> > +#else
> > +static inline void boot_start_lmb(bootm_headers_t *image) { }
> > +#endif
> > +
> > +extern void arch_preboot_os(void);
> 
> Sorry, no.  image.[ch] is definitely not the right place for this
> stuff.
> 
> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut