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

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

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
participants (2)
-
Scott Wood
-
Vipin Kumar