On Fri, 4 Sep 2015 00:30:56 +0200 Bernhard Nortmann <[email protected]> wrote:
> This patch moves retrieving the image header type into a function > of its own. The idea is that aw_fel_write() can and will also make > use of this function to detect boot scripts (by IH_TYPE_SCRIPT). > > Signed-off-by: Bernhard Nortmann <[email protected]> > --- > fel.c | 71 > ++++++++++++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 53 insertions(+), 18 deletions(-) > > diff --git a/fel.c b/fel.c > index 78c1465..c27ebe3 100644 > --- a/fel.c > +++ b/fel.c > @@ -106,6 +106,54 @@ void usb_bulk_recv(libusb_device_handle *usb, int ep, > void *data, int length) > } > } > > +/* Constants taken from ${U-BOOT}/include/image.h */ > +#define IH_MAGIC 0x27051956 /* Image Magic Number */ > +#define IH_ARCH_ARM 2 /* ARM */ > +#define IH_TYPE_INVALID 0 /* Invalid Image */ > +#define IH_TYPE_FIRMWARE 5 /* Firmware Image */ > +#define IH_TYPE_SCRIPT 6 /* Script file */ > +#define IH_NMLEN 32 /* Image Name Length */ > + > +#define HEADER_NAME_OFFSET 32 /* offset of name field */ > +#define HEADER_SIZE (HEADER_NAME_OFFSET + IH_NMLEN) > + > +/* > + * Utility function to determine the image type from a mkimage-compatible > + * header at given buffer (address). Use a non-zero "error" parameter if > + * you want to propagate any mismatch (header magic, ARM architecture) > + * with an actual error message and program termination. > + * Otherwise the function will either return the "ih_type" field from > + * a valid header, or IH_TYPE_INVALID in case of any mismatch. > + */ > +uint8_t get_image_type(const uint8_t *buf, int error) IMHO, this is not the nicest looking API. We get a buffer pointer here, but not the buffer size, so it is implicitly assumed to be large enough. The use of the error flag is not very readable in the code, that is calling this function if it is just 0 or 1. > +{ > + uint32_t *buf32 = (uint32_t *)buf; > + > + if (be32toh(buf32[0]) != IH_MAGIC) { > + if (error) { > + fprintf(stderr, "Image (%p) verification failure: " > + "expected IH_MAGIC, got 0x%X\n", > + buf, be32toh(buf32[0])); > + exit(1); If the 'get_image_type()' function is going to be called from multiple places, then this error message text is too generic and does not provide enough context. Maybe error reporting should be just done by the caller of this function? Or maybe this whole function can be replaced with just a structure declaration (if the primary intention is to avoid using the magic array indexes too much)? > + } else { > + return IH_TYPE_INVALID; > + } > + } > + /* For sunxi, we always expect ARM architecture here */ > + if (buf[29] != IH_ARCH_ARM) { > + if (error) { > + fprintf(stderr, "Image (%p) verification failure: " > + "expected IH_ARCH_ARM, got %02X\n", > + buf, buf[29]); > + exit(1); > + } else { > + return IH_TYPE_INVALID; > + } > + } > + /* assume a valid header, and return ih_type */ > + return buf[30]; > +} > + > void aw_send_usb_request(libusb_device_handle *usb, int type, int length) > { > struct aw_usb_request req; > @@ -758,15 +806,6 @@ void aw_fel_write_and_execute_spl(libusb_device_handle > *usb, > aw_restore_and_enable_mmu(usb, tt); > } > > -/* Constants taken from ${U-BOOT}/include/image.h */ > -#define IH_MAGIC 0x27051956 /* Image Magic Number */ > -#define IH_ARCH_ARM 2 /* ARM */ > -#define IH_TYPE_FIRMWARE 5 /* Firmware Image */ > -#define IH_NMLEN 32 /* Image Name Length */ > - > -#define HEADER_NAME_OFFSET 32 /* offset of name field */ > -#define HEADER_SIZE (HEADER_NAME_OFFSET + IH_NMLEN) > - > /* > * This function tests a given buffer address and length for a valid U-Boot > * image. Upon success, the image data gets transferred to the default memory > @@ -779,17 +818,13 @@ void aw_fel_write_uboot_image(libusb_device_handle *usb, > if (len <= HEADER_SIZE) > return; /* Insufficient size (no actual data), just bail out */ > > - /* Check for a valid mkimage header */ > uint32_t *buf32 = (uint32_t *)buf; > > - if (be32toh(buf32[0]) != IH_MAGIC) { > - fprintf(stderr, "U-Boot image verification failure: " > - "expected IH_MAGIC, got 0x%X\n", be32toh(buf32[0])); > - exit(1); > - } > - if (buf[29] != IH_ARCH_ARM|| buf[30] != IH_TYPE_FIRMWARE) { > - fprintf(stderr, "U-Boot image verification failure: " > - "expected ARM firmware, got %02X %02X\n", buf[29], > buf[30]); > + /* Check for a valid mkimage header */ > + uint8_t image_type = get_image_type(buf, 1); > + if (image_type != IH_TYPE_FIRMWARE) { > + fprintf(stderr, "U-Boot image type mismatch: " > + "expected IH_TYPE_FIRMWARE, got %02X\n", image_type); > exit(1); > } > uint32_t data_size = be32toh(buf32[3]); /* Image Data Size */ -- Best regards, Siarhei Siamashka -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
