On 01/16/2013 10:56 PM, Sebastian Andrzej Siewior wrote:
* Suzuki K. Poulose | 2013-01-11 12:12:11 [+0530]:
only some stylish comments…
diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c
b/kexec/arch/ppc/kexec-uImage-ppc.c
index eaea3c1..58935c0 100644
--- a/kexec/arch/ppc/kexec-uImage-ppc.c
+++ b/kexec/arch/ppc/kexec-uImage-ppc.c
@@ -46,6 +46,26 @@ void uImage_ppc_usage(void)
);
}
+char* slurp_ramdisk_ppc(const char *filename, off_t *r_size)
char *slurp_ramdisk_ppc(const char *filename, off_t *r_size)
?
OK
+{
+ struct Image_info img;
+ off_t size;
+ const unsigned char *buf =
+ (const unsigned char *)slurp_file(filename, &size);
is the const cast required here?
Yes, the uImage_probe() expects (const unsigned char *) as the first
argument.
+ /* Check if this is a uImage RAMDisk */
+ if (buf != (void*)0 &&
this should be NULL if at all
OK
+ uImage_probe(buf, size, IH_ARCH_PPC) == IH_TYPE_RAMDISK) {
and maybe splitted in more than one line
OK
+ if (uImage_load(buf, size, &img) != 0)
+ die ("uImage: Reading %ld bytes from %s failed\n",
size, filename);
no space between the function.
OK
+ buf = img.buf;
+ size = img.len;
+ }
+
+ *r_size = size;
+ return buf;
+}
+
int uImage_ppc_probe(const char *buf, off_t len)
{
int type;
@@ -196,7 +216,7 @@ static int ppc_load_bare_bits(int argc, char **argv, const
char *buf,
blob_buf = fixup_dtb_init(info, blob_buf, &blob_size, load_addr,
&dtb_addr);
if (ramdisk) {
- seg_buf = slurp_file(ramdisk, &seg_size);
+ seg_buf = slurp_ramdisk_ppc(ramdisk, &seg_size);
I'm not sure but this kinda breaks the case where someone was using a
plain file without the uImage header. I don't know if someone used this
at all but it seems we have code for this. The way you load the uImage
is very generic and I would expect to load it the same way on ARM or SH.
If the ramdisk doesn't have uImage header, the slurp_ramdisk_ppc()
defaults to the original behaviour. i.e, it just does a slurp_file()
as it used to do earlier.
One thing that is different compared to kernel: If the compression is
set to GZ on a ramdisk image you should not uncompress it. I think that
uImage_load() decompresses the .gz part for you but I am not sure.
Did you mean, uImage_load() shouldn't do a decompression for ramdisk ?
We rely on uImage_load to get what is required.
/* Load ramdisk at top of memory */
hole_addr = add_buffer(info, seg_buf, seg_size, seg_size,
0, dtb_addr + blob_size, max_addr, -1);
Thanks for the detailed review.
Suzuki
Sebastian
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
