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. 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. 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