On 12/12/2012 03:20:24 AM, 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>
---
Hello Scott,
There has been sometime since you reviewed the first version of this
patch.
http://lists.denx.de/pipermail/u-boot/2012-November/139631.html
I am sorry for a late response on this. I was waiting for other
comments if any
on the whole patch-set
Regards
Vipin
README | 3 +-
common/cmd_bootm.c | 133
++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/README b/README
index 2077c3b..ec5c31e 100644
--- a/README
+++ b/README
@@ -831,7 +831,8 @@ The following options need to be configured:
CONFIG_CMD_I2C * I2C serial bus support
CONFIG_CMD_IDE * IDE harddisk support
CONFIG_CMD_IMI iminfo
- CONFIG_CMD_IMLS List all found images
+ CONFIG_CMD_IMLS List all images found in flash
+ CONFIG_CMD_IMLS_NAND List all images found in NAND
device
s/in flash/in NOR flash/
s/in NAND device/in NAND flash/
Or better, just have one CONFIG_CMD_IMLS and have it operate on
whatever flash types are configured into U-Boot.
CONFIG_CMD_IMMAP * IMMR dump support
CONFIG_CMD_IMPORTENV * import an environment
CONFIG_CMD_INI * import data from an ini file
into the env
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d256ddf..8ee562a 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -83,6 +83,9 @@ extern flash_info_t flash_info[]; /* info for FLASH
chips */
static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char *
const argv[]);
#endif
+#include <linux/err.h>
+#include <nand.h>
+
#ifdef CONFIG_SILENT_CONSOLE
static void fixup_silent_linux(void);
#endif
@@ -1175,7 +1178,7 @@ U_BOOT_CMD(
/* imls - list all images found in flash */
/*******************************************************************/
#if defined(CONFIG_CMD_IMLS)
-static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char *
const argv[])
+static void do_imls_flash(void)
s/flash/nor/
{
flash_info_t *info;
int i, j;
@@ -1224,6 +1227,134 @@ next_sector: ;
}
next_bank: ;
}
+}
+#endif
+
+#if defined(CONFIG_CMD_IMLS_NAND)
+static void do_imls_nand(void)
+{
+ nand_info_t *nand;
+ int nand_dev = nand_curr_device;
+ size_t read_size;
+ loff_t off;
+ u8 buffer[512];
Why 512?
+ const image_header_t *header = (const image_header_t *)buffer;
+
+ /* 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;
+ }
"following commands", plural?
And this command seems to operate on all devices, not just the current
one.
+
+ printf("\n");
+
+ for (nand_dev = 0; nand_dev < CONFIG_SYS_MAX_NAND_DEVICE;
nand_dev++) {
+
Don't put a blank line inside braces at the beginning/end of blocks.
+ nand = &nand_info[nand_dev];
+ if (!nand->name || !nand->size)
+ continue;
+
+ for (off = 0; off < nand->size; off += nand->erasesize)
{
+ int ret;
+ void *imgdata;
+
+ if (nand_block_isbad(nand, off))
+ goto next_block;
+
+ read_size = sizeof(buffer);
+
+ ret = nand_read(nand, off, &read_size, buffer);
+ if (ret < 0 && ret != -EUCLEAN)
+ goto next_block;
s/goto next_block/continue/
+ header = (const image_header_t *)buffer;
+
+ switch (genimg_get_format(buffer)) {
+ case IMAGE_FORMAT_LEGACY:
+ if (!image_check_hcrc(header))
+ goto next_block;
+
+ read_size =
image_get_image_size(header);
+
+ imgdata = malloc(read_size);
+ if (!imgdata) {
+ printf("Not able to list all
images " \
+ "(Low memory)\n");
Don't line-wrap error strings.
+ goto next_block;
+ }
+
+ ret = nand_read_skip_bad(nand, off,
&read_size,
+ imgdata);
+ if (ret < 0 && ret != -EUCLEAN) {
+ free(imgdata);
+ goto next_block;
+ }
s/goto next_block/break/g
...or better, factor the switch out to its own function.
+
+ printf("Legacy Image at NAND device %d
" \
+ "offset %08llX:\n", nand_dev,
off);
+ image_print_contents(imgdata);
+
+ puts(" Verifying Checksum ... ");
+ if (!image_check_dcrc(imgdata))
+ puts("Bad Data CRC\n");
+ else
+ puts("OK\n");
+
+ free(imgdata);
+ break;
+#if defined(CONFIG_FIT)
+ case IMAGE_FORMAT_FIT:
+ read_size = fit_get_size(buffer);
+
+ imgdata = malloc(read_size);
+ if (!imgdata) {
+ printf("May be a FIT Image at
NAND " \
+ "device %d offset
%08llX:\n",
+ nand_dev, off);
+ printf(" Low memory(cannot
allocate" \
+ " memory for
image)\n");
+ goto next_block;
Why is the no-memory error message different for FIT versus legacy
images? I realize that at this point you don't know if it's a FIT
versus some other dtb, but why do you print the device and offset here
but not in the legacy case? Why "Low memory(cannot allocate memory for
image)" versus just " (Low memory)"?
+ }
+
+ ret = nand_read_skip_bad(nand, off,
&read_size,
+ imgdata);
+ if (ret < 0 && ret != -EUCLEAN) {
+ free(imgdata);
+ goto next_block;
+ }
+
+ if (!fit_check_format(imgdata)) {
+ free(imgdata);
+ goto next_block;
+ }
+
+ printf("FIT Image at NAND device %d " \
+ "offset %08llX:\n", nand_dev,
off);
+
+ fit_print_contents(imgdata);
+ free(imgdata);
+ break;
+#endif
+ default:
+ goto next_block;
+ }
The default: doesn't do anything -- just leave it out.
+
+next_block: ;
+ }
Instead of using goto please factor out the switch to its own function
and use return.
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
