* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
@ 2012-12-13 10:38 Vipin Kumar
2012-12-13 21:59 ` Scott Wood
0 siblings, 1 reply; 7+ messages in thread
From: Vipin Kumar @ 2012-12-13 10:38 UTC (permalink / raw)
To: u-boot
This patch adds support to list images in NAND flash through imls
Signed-off-by: Vipin Kumar <vipin.kumar@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_IMI iminfo
- 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@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;
+
+ len = sizeof(buffer);
+
+ ret = nand_read(nand, off, &len, (u8 *)buffer);
+ if (ret < 0 && ret != -EUCLEAN)
+ continue;
+
+ 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
+ }
+ }
+ }
+
+ return 0;
+}
+#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[])
+{
+ int ret_nor = 0, ret_nand = 0;
+
+#if defined(CONFIG_CMD_IMLS)
+ ret_nor = do_imls_nor();
+#endif
+
+#if defined(CONFIG_CMD_IMLS_NAND)
+ ret_nand = do_imls_nand();
+#endif
+
+ if (ret_nor)
+ return ret_nor;
+
+ if (ret_nand)
+ return ret_nand;
return (0);
}
--
1.8.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
2012-12-13 10:38 [U-Boot] [PATCH v3] imls: Add support to list images in NAND device Vipin Kumar
@ 2012-12-13 21:59 ` Scott Wood
2012-12-14 8:57 ` Vipin Kumar
2012-12-14 9:32 ` Vipin Kumar
0 siblings, 2 replies; 7+ messages in thread
From: Scott Wood @ 2012-12-13 21:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
2012-12-13 21:59 ` Scott Wood
@ 2012-12-14 8:57 ` Vipin Kumar
2012-12-14 9:32 ` Vipin Kumar
1 sibling, 0 replies; 7+ messages in thread
From: Vipin Kumar @ 2012-12-14 8:57 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
2012-12-13 21:59 ` Scott Wood
2012-12-14 8:57 ` Vipin Kumar
@ 2012-12-14 9:32 ` Vipin Kumar
2012-12-14 18:10 ` Scott Wood
1 sibling, 1 reply; 7+ messages in thread
From: Vipin Kumar @ 2012-12-14 9:32 UTC (permalink / raw)
To: u-boot
>> +
>> + 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
2012-12-14 9:32 ` Vipin Kumar
@ 2012-12-14 18:10 ` Scott Wood
0 siblings, 0 replies; 7+ messages in thread
From: Scott Wood @ 2012-12-14 18:10 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
[not found] <50CED650.3090000@st.com>
@ 2012-12-18 0:05 ` Scott Wood
2012-12-18 5:04 ` Vipin Kumar
0 siblings, 1 reply; 7+ messages in thread
From: Scott Wood @ 2012-12-18 0:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v3] imls: Add support to list images in NAND device
2012-12-18 0:05 ` Scott Wood
@ 2012-12-18 5:04 ` Vipin Kumar
0 siblings, 0 replies; 7+ messages in thread
From: Vipin Kumar @ 2012-12-18 5:04 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-12-18 5:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-13 10:38 [U-Boot] [PATCH v3] imls: Add support to list images in NAND device Vipin Kumar
2012-12-13 21:59 ` Scott Wood
2012-12-14 8:57 ` Vipin Kumar
2012-12-14 9:32 ` Vipin Kumar
2012-12-14 18:10 ` Scott Wood
[not found] <50CED650.3090000@st.com>
2012-12-18 0:05 ` Scott Wood
2012-12-18 5:04 ` Vipin Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox