On 16.11.2012, at 07:11, Yin Olivia-R63875 wrote: > Hi Stuart, > > load_uimage() is a public function which is called by below files: > hw/dummy_m68k.c: kernel_size = load_uimage(kernel_filename, > &entry, NULL, NULL); > hw/an5206.c: kernel_size = load_uimage(kernel_filename, &entry, > NULL, NULL); > hw/ppc440_bamboo.c: success = load_uimage(kernel_filename, > &entry, &loadaddr, NULL); > hw/openrisc_sim.c: kernel_size = > load_uimage(kernel_filename, entry, NULL, NULL); > hw/ppc/e500.c: kernel_size = > load_uimage(params->kernel_filename, &entry, &loadaddr, NULL); > hw/arm_boot.c: kernel_size = load_uimage(info->kernel_filename, > &entry, NULL, &is_linux); > hw/microblaze_boot.c: kernel_size = > load_uimage(kernel_filename, &uentry, &loadaddr, 0); > hw/mcf5208.c: kernel_size = load_uimage(kernel_filename, &entry, > NULL, NULL); > > Most value of *is_linux is NULL, only arm_boot.c will check the value of > is_linux. > I just define an static variable is_linux other than include it into > structure ImageFile.
Just load the uImage once on load_uimage, so that you can populate all the passed down return variables (entry, loadaddr, is_linux). No need for a global here. > To define static variable kernel_loaded because function > uimage_physical_loader() will > be called twice. > 1) load_uimage() which will run only one time when startup. > 2) uimage_reset() which will run once reset, so it maybe run many times. > Since "ROM images must be loaded at startup", it means rom_add_*() should not > be called by any reset handlers. > So I use variable kernel_loaded to decide the booting is startup or reset. Pass it as a function argument then? Alex > > > Best Regards, > Olivia > > >> -----Original Message----- >> From: Stuart Yoder [mailto:b08...@gmail.com] >> Sent: Friday, November 16, 2012 4:46 AM >> To: Yin Olivia-R63875 >> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org >> Subject: Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload >> uimage >> >> On Wed, Nov 14, 2012 at 2:28 PM, Olivia Yin <hong-hua....@freescale.com> >> wrote: >>> Signed-off-by: Olivia Yin <hong-hua....@freescale.com> >>> --- >>> hw/loader.c | 64 ++++++++++++++++++++++++++++++++++++++++++--------- >> ------- >>> 1 files changed, 46 insertions(+), 18 deletions(-) >>> >>> diff --git a/hw/loader.c b/hw/loader.c index a8a0a09..1a909d0 100644 >>> --- a/hw/loader.c >>> +++ b/hw/loader.c >>> @@ -55,6 +55,8 @@ >>> #include <zlib.h> >>> >>> static int roms_loaded; >>> +static int kernel_loaded; >>> +static int *is_linux; >> >> Why do we need these 2 new variables? >> >>> /* return the size or -1 if error */ >>> int get_image_size(const char *filename) @@ -472,15 +474,14 @@ static >>> ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src, >>> return dstbytes; >>> } >>> >>> -/* Load a U-Boot image. */ >>> -int load_uimage(const char *filename, hwaddr *ep, >>> - hwaddr *loadaddr, int *is_linux) >>> +/* write uimage into memory */ >>> +static int uimage_physical_loader(const char *filename, uint8_t **data, >>> + hwaddr *loadaddr, int *is_linux) >>> { >>> int fd; >>> int size; >>> uboot_image_header_t h; >>> uboot_image_header_t *hdr = &h; >>> - uint8_t *data = NULL; >>> int ret = -1; >>> >>> fd = open(filename, O_RDONLY | O_BINARY); @@ -521,10 +522,9 @@ >>> int load_uimage(const char *filename, hwaddr *ep, >>> *is_linux = 0; >>> } >>> >>> - *ep = hdr->ih_ep; >>> - data = g_malloc(hdr->ih_size); >>> + *data = g_malloc(hdr->ih_size); >>> >>> - if (read(fd, data, hdr->ih_size) != hdr->ih_size) { >>> + if (read(fd, *data, hdr->ih_size) != hdr->ih_size) { >>> fprintf(stderr, "Error reading file\n"); >>> goto out; >>> } >>> @@ -534,11 +534,11 @@ int load_uimage(const char *filename, hwaddr *ep, >>> size_t max_bytes; >>> ssize_t bytes; >>> >>> - compressed_data = data; >>> + compressed_data = *data; >>> max_bytes = UBOOT_MAX_GUNZIP_BYTES; >>> - data = g_malloc(max_bytes); >>> + *data = g_malloc(max_bytes); >>> >>> - bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size); >>> + bytes = gunzip(*data, max_bytes, compressed_data, >>> + hdr->ih_size); >>> g_free(compressed_data); >>> if (bytes < 0) { >>> fprintf(stderr, "Unable to decompress gzipped image!\n"); >>> @@ -547,7 +547,9 @@ int load_uimage(const char *filename, hwaddr *ep, >>> hdr->ih_size = bytes; >>> } >>> >>> - rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load); >>> + if (!kernel_loaded) { >>> + rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr- >>> ih_load); >>> + } >> >> Why do we need to keep the rom_add_blob_fixed() call? I thought the >> point of this was to remove adding roms. >> >>> if (loadaddr) >>> *loadaddr = hdr->ih_load; >>> @@ -555,12 +557,41 @@ int load_uimage(const char *filename, hwaddr *ep, >>> ret = hdr->ih_size; >>> >>> out: >>> - if (data) >>> - g_free(data); >>> close(fd); >>> return ret; >>> } >>> >>> +static void uimage_reset(void *opaque) { >>> + ImageFile *image = opaque; >>> + uint8_t *data = NULL; >>> + int size; >>> + >>> + size = uimage_physical_loader(image->name, &data, &image->addr, >>> + is_linux); >> >> Not sure why we need is_linux here. I think you should just set that >> parameter to NULL. >> Nothing will look at is_linux. >> >> Stuart > > >