[U-Boot-Users] [PATCH] JFFS2 command support on OneNAND (take #2)

[Wolfgang Denk]

In message <200807300920.41289.fabo at debian.org> you wrote:
> JFFS2 command support on OneNAND.
> 
> - Fix typo.
> - Fix nand_bbt_descr redefinition when NAND and OneNAND are enabled.
> 
> TODO: Use NAND command interface (e.g.: part_validate_*nand()). 
> 
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Signed-off-by: Fathi Boudra <fabo at debian.org>
> ---
> diff --git a/common/cmd_jffs2.c b/common/cmd_jffs2.c
> index b4698be..43e33dd 100644
> --- a/common/cmd_jffs2.c
> +++ b/common/cmd_jffs2.c
> @@ -51,7 +51,7 @@
>   * mtdids=<idmap>[,<idmap>,...]
>   *
>   * <idmap>    := <dev-id>=<mtd-id>
> - * <dev-id>   := 'nand'|'nor'<dev-num>
> + * <dev-id>   := 'nand'|'nor'|'onenand'<dev-num>
>   * <dev-num>  := mtd device number, 0...
>   * <mtd-id>   := unique device tag used by linux kernel to find mtd device (mtd->name)
>   *
> @@ -103,6 +103,13 @@
>  #include <nand.h>
>  #endif /* !CFG_NAND_LEGACY */
>  #endif
> +
> +#if defined(CONFIG_CMD_ONENAND)
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/onenand.h>
> +#include <onenand_uboot.h>
> +#endif
> +
>  /* enable/disable debugging messages */
>  #define	DEBUG_JFFS
>  #undef	DEBUG_JFFS
> @@ -401,6 +408,42 @@ static int part_validate_nand(struct mtdids *id, struct part_info *part)
>  }
>  
>  /**
> + * Performs sanity check for supplied OneNAND flash partition.
> + * Table of existing OneNAND flash devices is searched and partition device
> + * is located. Alignment with the granularity of nand erasesize is verified.
> + *
> + * @param id of the parent device
> + * @param part partition to validate
> + * @return 0 if partition is valid, 1 otherwise
> + */
> +static int part_validate_onenand(struct mtdids *id, struct part_info *part)
> +{
> +#if defined(CONFIG_CMD_ONENAND)

The whole function should only be defined...

> @@ -436,6 +479,8 @@ static int part_validate(struct mtdids *id, struct part_info *part)
>  		return part_validate_nand(id, part);
>  	else if (id->type == MTD_DEV_TYPE_NOR)
>  		return part_validate_nor(id, part);
> +	else if (id->type == MTD_DEV_TYPE_ONENAND)
> +		return part_validate_onenand(id, part);

... and called when CONFIG_CMD_ONENAND is set.

> @@ -755,7 +800,15 @@ static int device_validate(u8 type, u8 num, u32 *size)
>  #else
>  		printf("support for NAND devices not present\n");
>  #endif
> -	}
> +	} else if (type == MTD_DEV_TYPE_ONENAND) {
> +#if defined(CONFIG_CMD_ONENAND)
> +		*size = onenand_mtd.size;
> +		return 0;
> +#else
> +		printf("support for OneNAND devices not present\n");
> +#endif
> +	} else
> +		printf("Unknown device type %d\n", type);

Same here - if CONFIG_CMD_ONENAND is not set, no new code should be
added.

> @@ -1065,8 +1118,8 @@ static struct mtdids* id_find_by_mtd_id(const char *mtd_id, unsigned int mtd_id_
>  #endif /* #ifdef CONFIG_JFFS2_CMDLINE */
>  
>  /**
> - * Parse device id string <dev-id> := 'nand'|'nor'<dev-num>, return device
> - * type and number.
> + * Parse device id string <dev-id> := 'nand'|'nor'|'onenand'<dev-num>,
> + * return device type and number.
>   *
>   * @param id string describing device id
>   * @param ret_id output pointer to next char after parse completes (output)
> @@ -1085,6 +1138,9 @@ int id_parse(const char *id, const char **ret_id, u8 *dev_type, u8 *dev_num)
>  	} else if (strncmp(p, "nor", 3) == 0) {
>  		*dev_type = MTD_DEV_TYPE_NOR;
>  		p += 3;
> +	} else if (strncmp(p, "onenand", 7) == 0) {
> +		*dev_type = MTD_DEV_TYPE_ONENAND;
> +		p += 7;

Ditto here.

> +static int read_onenand_cached(u32 off, u32 size, u_char *buf)
> +{
> +	u32 bytes_read = 0;
> +	size_t retlen;
> +	int cpy_bytes;
> +
> +	while (bytes_read < size) {
> +		if ((off + bytes_read < onenand_cache_off) ||
> +		    (off + bytes_read >= onenand_cache_off + ONENAND_CACHE_SIZE)) {
> +			onenand_cache_off = (off + bytes_read) & ONENAND_PAGE_MASK;
> +			if (!onenand_cache) {
> +				/* This memory never gets freed but 'cause
> +				   it's a bootloader, nobody cares */
> +				onenand_cache = malloc(ONENAND_CACHE_SIZE);
> +				if (!onenand_cache) {
> +					printf("read_onenand_cached: can't alloc cache size %d bytes\n",

Lines too long.

...
> +				printf("read_onenand_cached: error reading nand off %#x size %d bytes\n",
> +					onenand_cache_off, ONENAND_CACHE_SIZE);

Ditto.

> diff --git a/include/jffs2/load_kernel.h b/include/jffs2/load_kernel.h
> index 882a80e..37871de 100644
> --- a/include/jffs2/load_kernel.h
> +++ b/include/jffs2/load_kernel.h
> @@ -28,9 +28,14 @@
>  #include <linux/list.h>
>  
>  /* mtd device types */
> -#define MTD_DEV_TYPE_NOR      0x0001
> -#define MTD_DEV_TYPE_NAND     0x0002
> -#define MTD_DEV_TYPE(type) ((type == MTD_DEV_TYPE_NAND) ? "nand" : "nor")
> +enum {
> +	MTD_DEV_TYPE_NOR,
> +	MTD_DEV_TYPE_NAND,
> +	MTD_DEV_TYPE_ONENAND,
> +};

Please don't. Leave as it was.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I don't mind criticism. You know me. I've  never  been  one  to  take
offence  at  criticism. No one could say I'm the sort to take offence
at criticism -- Not twice, anyway. Not without blowing bubbles.
                                  - Terry Pratchett, _Witches Abroad_