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
___
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
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
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
+
+ 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
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
Re: [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
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
