imls: Add support to list images in NAND device
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v3 1/1 imls: Add support to list images in NAND device
Quoted 43 lines · Vipin Kumar
Quoted 35 lines · Scott Wood
Quoted 29 lines · Vipin Kumar
Quoted 19 lines
Quoted 19 lines
(quoted diff) +7
-0+ + 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,(quoted diff) +11
-0+ 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;Quoted 9 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ 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.Quoted 4 lines
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.Quoted 6 lines
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.
Quoted 5 lines
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
PATCH v3 1/1 imls: Add support to list images in NAND device
Quoted 43 lines · Scott Wood
Quoted 42 lines · Vipin Kumar
Quoted 29 lines · Scott Wood
Quoted 28 lines · Vipin Kumar
Quoted 27 lines
Quoted 12 lines
(quoted diff) +7
-0+ + 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;Review
Quoted 10 lines
(quoted diff) +10
-0+#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.
Quoted 9 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 3 lines
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
Quoted 2 lines
-Scott