On 11/7/2012 5:00 AM, Scott Wood wrote:
On 11/02/2012 12:40:02 PM, Vipin Kumar wrote:
imls does not list the images in NAND devices. This patch implements this
support for legacy type images.
Signed-off-by: Vipin Kumar <vipin.ku...@st.com>
---
common/cmd_bootm.c | 98
++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 83fa5d7..ca3c430 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -62,6 +62,11 @@
#include <linux/lzo.h>
#endif /* CONFIG_LZO */
+#if defined(CONFIG_CMD_NAND)
+#include <linux/err.h>
+#include <nand.h>
+#endif
You shouldn't need to ifdef-protect header files.
OK. I would correct this in v2
DECLARE_GLOBAL_DATA_PTR;
#ifndef CONFIG_SYS_BOOTM_LEN
@@ -1221,6 +1226,99 @@ next_sector: ;
next_bank: ;
}
+#if defined(CONFIG_CMD_NAND)
+ printf("\n");
+ nand_info_t *nand;
+ image_header_t image_header;
+ image_header_t *header = &image_header;
+ int nand_dev = nand_curr_device;
+ unsigned long img_size;
+ size_t hdr_size, read_len;
+ loff_t off;
+ unsigned int crc;
+ u_char *data;
+
+ /* the following commands operate on the current device */
+ if (nand_dev < 0 || nand_dev >= CONFIG_SYS_MAX_NAND_DEVICE) {
+ puts("\nNo NAND devices available\n");
+ return 0;
+ }
Please move the NAND and NOR code into their own functions.
You mean I can separate the NOR list images code in one routine and NAND
in another?
+
+ for (nand_dev = 0; nand_dev < CONFIG_SYS_MAX_NAND_DEVICE; nand_dev++) {
+
+ nand = &nand_info[nand_dev];
+ if ((!nand->name) || (!nand->size))
+ continue;
Redundant parentheses.
Accepted
+ data = malloc(nand->writesize);
+ if (!data) {
+ puts("No memory available to list NAND images\n");
+ return 0;
+ }
+
+ for (off = 0; off < nand->size; off += nand->erasesize) {
+ int ret;
+
+ if (nand_block_isbad(nand, off))
+ continue;
+
+ hdr_size = sizeof(image_header_t);
+ ret = nand_read(nand, off, &hdr_size, (u_char *)header);
+ if (ret < 0 && ret != -EUCLEAN)
+ continue;
I guess you don't use nand_read_skip_bad() because you don't want to
allocate a buffer for the whole image... How about moving this code to
nand_util.c? That would at least allow some refactoring rather than
duplication.
+ if (!image_check_hcrc(header))
+ continue;
+
+ printf("Legacy Image at NAND device %d offset %08lX:\n",
+ nand_dev, (ulong)off);
+ image_print_contents(header);
Shouldn't you check for FIT images as well?
Yes. I was a bit reluctant because I don't know about that format.
OK, I would try to add it in v2
+ puts(" Verifying Checksum ... ");
+ crc = 0;
+ img_size = ntohl(header->ih_size);
+ img_size += hdr_size;
+
+ while (img_size > 0) {
+ int blockoff = 0;
+
+ while (nand_block_isbad(nand, off)) {
+ off += nand->erasesize;
+ if (off >= nand->size)
+ goto out;
+ }
+
+ do {
+ read_len = min(img_size,
+ nand->writesize);
+ ret = nand_read(nand, off + blockoff,
+ &read_len, data);
+ if (ret < 0 && ret != -EUCLEAN)
+ break;
+
+ crc = crc32(crc, data + hdr_size,
+ read_len - hdr_size);
+ img_size -= read_len;
+ blockoff += read_len;
+ hdr_size = 0;
+ } while (img_size &&
+ (blockoff < nand->erasesize));
+
+ off += nand->erasesize;
+ if (off >= nand->size)
+ goto out;
+ }
+ off -= nand->erasesize;
+out:
+ if (crc != ntohl(header->ih_dcrc))
+ puts(" Bad Data CRC\n");
+ else
+ puts("OK\n");
+ }
Please refactor this into separate functions to improve readability.
Maybe put a nand_crc_skip_bad() function into nand_util.c?
OK, I would give it a try. Please wait for v2
-Scott
btw, thanks for the review
How about other patches, Albert, Wolfgang ?
Regards
Vipin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot