Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-17 Thread Scott Wood

On 12/17/2012 02:22:40 AM, Vipin Kumar wrote:

On 12/14/2012 11:40 PM, Scott Wood wrote:

On 12/14/2012 03:32:04 AM, Vipin Kumar wrote:

+
+   switch (genimg_get_format(buffer)) {
+   case IMAGE_FORMAT_LEGACY:
+   header = (const image_header_t *)buffer;
+   len = image_get_image_size(header);
+
+   ret = nand_imls_legacyimage(nand,
nand_dev,
+   off, len);
+   if (ret   0   ret != -ENOMEM)
+   return ret;
+   break;
+#if defined(CONFIG_FIT)
+   case IMAGE_FORMAT_FIT:
+   len = fit_get_size(buffer);
+   ret = nand_imls_fitimage(nand, nand_dev,
+   off, len);
+   if (ret   0   ret != -ENOMEM)
+   return ret;
+   break;
+#endif
+   }


Do you really mean to return from the main imls function just  
because

one image has an error?  By use return I meant return from the
subfunction.



This return only corresponds to the situation when there is an error
returned from nand read routine. In that case, I don't think there  
is

any use reading the NAND any further.


Just because one page has an uncorrectable error doesn't mean the
entire NAND is bad.  Note that this is different from what you
currently do if you get an error on the initial read where you look  
for

a header.



Yes, I got your point.

I would now not announce the uncorrectable errors as they may hog the  
whole stdout and still continue to work for the whole NAND device.  
Please check the implementation in v4


I'd rather see errors be announced, with some reasonable limit on how  
many (and a message indicating if further errors exist that were  
suppressed).


-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-17 Thread Vipin Kumar

On 12/18/2012 5:35 AM, Scott Wood wrote:

On 12/17/2012 02:22:40 AM, Vipin Kumar wrote:

On 12/14/2012 11:40 PM, Scott Wood wrote:

On 12/14/2012 03:32:04 AM, Vipin Kumar wrote:

+
+ switch (genimg_get_format(buffer)) {
+ case IMAGE_FORMAT_LEGACY:
+ header = (const image_header_t *)buffer;
+ len = image_get_image_size(header);
+
+ ret = nand_imls_legacyimage(nand,
nand_dev,
+ off, len);
+ if (ret 0 ret != -ENOMEM)
+ return ret;
+ break;
+#if defined(CONFIG_FIT)
+ case IMAGE_FORMAT_FIT:
+ len = fit_get_size(buffer);
+ ret = nand_imls_fitimage(nand, nand_dev,
+ off, len);
+ if (ret 0 ret != -ENOMEM)
+ return ret;
+ break;
+#endif
+ }


Do you really mean to return from the main imls function just because
one image has an error? By use return I meant return from the
subfunction.



This return only corresponds to the situation when there is an error
returned from nand read routine. In that case, I don't think there is
any use reading the NAND any further.


Just because one page has an uncorrectable error doesn't mean the
entire NAND is bad. Note that this is different from what you
currently do if you get an error on the initial read where you look for
a header.



Yes, I got your point.

I would now not announce the uncorrectable errors as they may hog the
whole stdout and still continue to work for the whole NAND device.
Please check the implementation in v4


I'd rather see errors be announced, with some reasonable limit on how
many (and a message indicating if further errors exist that were
suppressed).



Hmm, OK. I would do it this way in v5


-Scott



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-14 Thread Vipin Kumar

On 12/14/2012 3:29 AM, Scott Wood wrote:

On 12/13/2012 04:38:18 AM, Vipin Kumar wrote:

+ for (off = 0; off  nand-size; off += nand-erasesize) {
+ const image_header_t *header;
+ int ret;
+
+ if (nand_block_isbad(nand, off))
+ continue;
+
+ len = sizeof(buffer);
+
+ ret = nand_read(nand, off, len, (u8 *)buffer);
+ if (ret  0  ret != -EUCLEAN)
+ continue;


Might want to make some noise if you get an uncorrectable error.



OK..


+
+ switch (genimg_get_format(buffer)) {
+ case IMAGE_FORMAT_LEGACY:
+ header = (const image_header_t *)buffer;
+ len = image_get_image_size(header);
+
+ ret = nand_imls_legacyimage(nand, nand_dev,
+ off, len);
+ if (ret  0  ret != -ENOMEM)
+ return ret;
+ break;
+#if defined(CONFIG_FIT)
+ case IMAGE_FORMAT_FIT:
+ len = fit_get_size(buffer);
+ ret = nand_imls_fitimage(nand, nand_dev,
+ off, len);
+ if (ret  0  ret != -ENOMEM)
+ return ret;
+ break;
+#endif
+ }


Do you really mean to return from the main imls function just because
one image has an error? By use return I meant return from the
subfunction.



I thought about it a little bit. How do you suggest


-Scott



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-14 Thread Vipin Kumar

+
+   switch (genimg_get_format(buffer)) {
+   case IMAGE_FORMAT_LEGACY:
+   header = (const image_header_t *)buffer;
+   len = image_get_image_size(header);
+
+   ret = nand_imls_legacyimage(nand,
nand_dev,
+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#if defined(CONFIG_FIT)
+   case IMAGE_FORMAT_FIT:
+   len = fit_get_size(buffer);
+   ret = nand_imls_fitimage(nand, nand_dev,
+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#endif
+   }


Do you really mean to return from the main imls function just because
one image has an error?  By use return I meant return from the
subfunction.



This return only corresponds to the situation when there is an error 
returned from nand read routine. In that case, I don't think there is 
any use reading the NAND any further.


What do you think ?


-Scott



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-14 Thread Scott Wood

On 12/14/2012 03:32:04 AM, Vipin Kumar wrote:

+
+   switch (genimg_get_format(buffer)) {
+   case IMAGE_FORMAT_LEGACY:
+   header = (const image_header_t *)buffer;
+   len = image_get_image_size(header);
+
+   ret = nand_imls_legacyimage(nand,
nand_dev,
+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#if defined(CONFIG_FIT)
+   case IMAGE_FORMAT_FIT:
+   len = fit_get_size(buffer);
+   ret = nand_imls_fitimage(nand, nand_dev,
+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#endif
+   }


Do you really mean to return from the main imls function just because
one image has an error?  By use return I meant return from the
subfunction.



This return only corresponds to the situation when there is an error  
returned from nand read routine. In that case, I don't think there is  
any use reading the NAND any further.


Just because one page has an uncorrectable error doesn't mean the  
entire NAND is bad.  Note that this is different from what you  
currently do if you get an error on the initial read where you look for  
a header.


Might want to bail out if you get an excessive amount of errors  
(particularly consecutive), though, suggesting that the NAND might not  
be working at all.  The user probably doesn't want to wait for  
thousands of error messages to scroll by in that case.


-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-13 Thread Vipin Kumar
This patch adds support to list images in NAND flash through imls

Signed-off-by: Vipin Kumar vipin.ku...@st.com
---
 README |   3 +-
 common/cmd_bootm.c | 166 -
 2 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 2077c3b..46fd21d 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_IMIiminfo
-   CONFIG_CMD_IMLS   List all found images
+   CONFIG_CMD_IMLS   List all images found in NOR flash
+   CONFIG_CMD_IMLS_NAND  List all images found in NAND flash
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..e55becd 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -80,9 +80,15 @@ static int image_info(unsigned long addr);
 #include flash.h
 #include mtd/cfi_flash.h
 extern flash_info_t flash_info[]; /* info for FLASH chips */
+#endif
+
+#if defined(CONFIG_CMD_IMLS) || defined(CONFIG_CMD_IMLS_NAND)
 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 +1181,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 int do_imls_nor(void)
 {
flash_info_t *info;
int i, j;
@@ -1224,6 +1230,164 @@ next_sector:;
}
 next_bank: ;
}
+   return 0;
+}
+#endif
+
+#if defined(CONFIG_CMD_IMLS_NAND)
+static int nand_imls_legacyimage(nand_info_t *nand, int nand_dev, loff_t off,
+   size_t len)
+{
+   void *imgdata;
+   int ret;
+
+   imgdata = malloc(len);
+   if (!imgdata) {
+   printf(May be a Legacy Image at NAND device %d \
+   offset %08llX:\n, nand_dev, off);
+   printf(   Low memory(cannot allocate memory for image)\n);
+   return -ENOMEM;
+   }
+
+   ret = nand_read_skip_bad(nand, off, len,
+   imgdata);
+   if (ret  0  ret != -EUCLEAN) {
+   free(imgdata);
+   return ret;
+   }
+
+   if (!image_check_hcrc(imgdata)) {
+   free(imgdata);
+   return 0;
+   }
+
+   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);
+
+   return 0;
+}
+
+static int nand_imls_fitimage(nand_info_t *nand, int nand_dev, loff_t off,
+   size_t len)
+{
+   void *imgdata;
+   int ret;
+
+   imgdata = malloc(len);
+   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);
+   return -ENOMEM;
+   }
+
+   ret = nand_read_skip_bad(nand, off, len,
+   imgdata);
+   if (ret  0  ret != -EUCLEAN) {
+   free(imgdata);
+   return ret;
+   }
+
+   if (!fit_check_format(imgdata)) {
+   free(imgdata);
+   return 0;
+   }
+
+   printf(FIT Image at NAND device %d offset %08llX:\n, nand_dev, off);
+
+   fit_print_contents(imgdata);
+   free(imgdata);
+
+   return 0;
+}
+
+static int do_imls_nand(void)
+{
+   nand_info_t *nand;
+   int nand_dev = nand_curr_device;
+   size_t len;
+   loff_t off;
+   u32 buffer[16];
+
+   if (nand_dev  0 || nand_dev = CONFIG_SYS_MAX_NAND_DEVICE) {
+   puts(\nNo NAND devices available\n);
+   return -ENODEV;
+   }
+
+   printf(\n);
+
+   for (nand_dev = 0; nand_dev  CONFIG_SYS_MAX_NAND_DEVICE; nand_dev++) {
+   nand = nand_info[nand_dev];
+   if (!nand-name || !nand-size)
+   continue;
+
+   for (off = 0; off  nand-size; off += nand-erasesize) {
+   const image_header_t *header;
+   int ret;
+
+   if (nand_block_isbad(nand, off))
+   continue;
+
+   

Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device

2012-12-13 Thread Scott Wood

On 12/13/2012 04:38:18 AM, Vipin Kumar wrote:
+		for (off = 0; off  nand-size; off += nand-erasesize)  
{

+   const image_header_t *header;
+   int ret;
+
+   if (nand_block_isbad(nand, off))
+   continue;
+
+   len = sizeof(buffer);
+
+   ret = nand_read(nand, off, len, (u8 *)buffer);
+   if (ret  0  ret != -EUCLEAN)
+   continue;


Might want to make some noise if you get an uncorrectable error.


+
+   switch (genimg_get_format(buffer)) {
+   case IMAGE_FORMAT_LEGACY:
+   header = (const image_header_t *)buffer;
+   len = image_get_image_size(header);
+
+ret = nand_imls_legacyimage(nand,  
nand_dev,

+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#if defined(CONFIG_FIT)
+   case IMAGE_FORMAT_FIT:
+   len = fit_get_size(buffer);
+   ret = nand_imls_fitimage(nand, nand_dev,
+   off, len);
+   if (ret  0  ret != -ENOMEM)
+   return ret;
+   break;
+#endif
+   }


Do you really mean to return from the main imls function just because  
one image has an error?  By use return I meant return from the  
subfunction.


-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot