On Fri, May 16, 2014 at 09:26:36PM +0300, Khoronzhuk, Ivan wrote:
> From: WingMan Kwok <w-kw...@ti.com>
>
> This commit adds a nand ecclayout command that allows the ecclayout of
> the current nand device to be changed during run time. This feature is
> useful when using u-boot to write something to nand flash that will be
> read by other applications, such as ROM bootloader, that expects a
> different ECC layout. In that case, change the current nand device
> ecclayout using the "nand ecclayout set" command before writing the
> data to nand flash.
>
> Signed-off-by: WingMan Kwok <w-kw...@ti.com>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@ti.com>
>
> ---
> common/cmd_nand.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> include/nand.h | 10 +++++
> 2 files changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index 04ab0f1..7cbe6fc 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -462,6 +462,53 @@ static void adjust_size_for_badblocks(loff_t *size,
> loff_t offset, int dev)
> }
> }
>
> +#ifdef CONFIG_CMD_NAND_ECCLAYOUT
> +static void nand_print_ecclayout_info(struct nand_ecclayout *p)
> +{
> + int i;
> + struct nand_oobfree *oobfree;
> +
> + if (!p)
> + return;
> +
> + printf(" num ecc bytes: %d\n", p->eccbytes);
> + puts(" ecc pos:\n ");
> +
> + for (i = 0; i < p->eccbytes; i++) {
> + if (i && !(i % 9))
> + printf("\n ");
> +
> + printf("%2d ", p->eccpos[i]);
> + }
Why 9?
> + puts("\n oobfree:\n");
> + puts(" offset length\n");
> +
> + oobfree = p->oobfree;
> + for (i = 0; oobfree->length && i < MTD_MAX_OOBFREE_ENTRIES_LARGE; i++) {
> + printf(" %2d %2d\n", oobfree->offset, oobfree->length);
> + oobfree++;
> + }
> +}
> +
> +static void nand_print_device_ecclayout(int dev)
> +{
> + int idx;
> + nand_info_t *nand = &nand_info[dev];
> + struct nand_chip *chip = nand->priv;
> +
> + idx = board_nand_ecclayout_get_idx(chip, chip->ecc.layout);
> +
> + if (idx < 0) {
> + puts("no ecc layout\n");
> + return;
> + }
> +
> + printf("\necc layout %d:\n", idx);
> + nand_print_ecclayout_info(chip->ecc.layout);
> +}
> +#endif
> +
> static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> {
> int i, ret = 0;
> @@ -506,8 +553,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[])
> putc('\n');
> if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE)
> puts("no devices available\n");
> - else
> + else {
> nand_print_and_set_info(dev);
> +#ifdef CONFIG_CMD_NAND_ECCLAYOUT
> + nand_print_device_ecclayout(dev);
> +#endif
> + }
> return 0;
> }
Braces are needed on both sides of if/else if used on one side.
It would be better to provide an inline no-op stub when
CONFIG_CMD_NAND_ECCLAYOUT, than ifdeffing at the call site.
Does it really make sense to dump this information here, when the user
could use the ecclayout command instead?
> @@ -836,6 +887,60 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[])
> }
> #endif
>
> +#ifdef CONFIG_CMD_NAND_ECCLAYOUT
> + if (strcmp(cmd, "ecclayout") == 0) {
> + int idx;
> + struct nand_ecclayout *p;
> + nand_info_t *nand = &nand_info[dev];
> + struct nand_chip *chip = nand->priv;
> +
> + if (argc < 3) {
> + puts("Current device ecclayout:\n");
> + nand_print_device_ecclayout(dev);
> + return 0;
> + }
> +
> + if (!strcmp(argv[2], "set")) {
> + if (argc < 4)
> + return 1;
> +
> + idx = (int)simple_strtoul(argv[3], NULL, 10);
> + if (!board_nand_ecclayout_set(chip, idx)) {
> + p = chip->ecc.layout;
> + p->oobavail = 0;
> + for (i = 0; p->oobfree[i].length &&
> + i < ARRAY_SIZE(p->oobfree); i++)
> + p->oobavail += p->oobfree[i].length;
> +
> + nand->oobavail = p->oobavail;
Shouldn't board_nand_ecclayout_set() take care of this? Possibly via
common helper functions, but not here.
> + } else {
> + printf("Setting current device"
> + " to ecc layout %d FAILED!\n", idx);
Don't wrap user-visible strings (checkpatch should have warned you about
this).
> + }
> +
> + return 0;
> + }
> +
> + if (strcmp(argv[2], "all") != 0)
> + return 1;
> +
> + /* show all available ecc layouts */
> + puts("Available ecc layouts:\n");
> +
> + idx = 0;
> + while (1) {
> + p = board_nand_ecclayout_get_layout(chip, idx);
> + if (!p)
> + break;
> +
> + printf("\n ecc layout %d:\n", idx++);
> + nand_print_ecclayout_info(p);
> + }
Please use a for loop rather than hiding the increment in the loop body.
> +
> + return 0;
> + }
> +#endif
> +
> usage:
> return CMD_RET_USAGE;
> }
> @@ -890,6 +995,14 @@ static char nand_help_text[] =
> "nand env.oob set off|partition - set enviromnent offset\n"
> "nand env.oob get - get environment offset"
> #endif
> +#ifdef CONFIG_CMD_NAND_ECCLAYOUT
> + "\n"
> + "nand ecclayout <set <idx>|all> - show or set ecclayout"
> + " of current device\n"
> + " 'all' shows all available ecc layouts for setting"
> + " to current device\n"
> + " 'set' sets current device ecc layout to layout indexed by idx\n"
> +#endif
This help text doesn't include the plain "nand ecclayout" to show the
current layout.
> diff --git a/include/nand.h b/include/nand.h
> index fc735d1..0e8a093 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -155,6 +155,16 @@ void nand_deselect(void);
> void board_nand_select_device(struct nand_chip *nand, int chip);
> #endif
>
> +#ifdef CONFIG_CMD_NAND_ECCLAYOUT
> +int board_nand_ecclayout_get_idx(struct nand_chip *nand,
> + struct nand_ecclayout *p);
> +
> +struct nand_ecclayout *board_nand_ecclayout_get_layout(struct nand_chip
> *nand,
> + int idx);
> +
> +int board_nand_ecclayout_set(struct nand_chip *nand, int idx);
> +#endif
Don't ifdef prototypes.
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
