From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Fri, 17 Aug 2012 15:53:06 -0500
Subject: [U-Boot] [PATCH 1/3] nand: Add support for unlock.invert
In-Reply-To: <1345235492-15043-1-git-send-email-joe.hershberger@ni.com>
References: <1345235492-15043-1-git-send-email-joe.hershberger@ni.com>
Message-ID: <502EAF32.1070106@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 08/17/2012 03:31 PM, Joe Hershberger wrote:
> NAND unlock command allows an invert bit to be set to unlock all but
> the selected page range.
> 
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---
>  common/cmd_nand.c            | 13 ++++++++++---
>  drivers/mtd/nand/nand_util.c |  9 ++++++---
>  include/nand.h               |  4 ++--
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index a91ccf4..9acac9f 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -749,11 +749,18 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  		return 0;
>  	}
>  
> -	if (strcmp(cmd, "unlock") == 0) {
> +	if (strncmp(cmd, "unlock", 5) == 0) {
> +		int invert = 0;
> +
> +		s = strchr(cmd, '.');
> +
> +		if (s && !strcmp(s, ".invert"))
> +			invert = 1;
> +

How about something like ".allexcept"?  "Invert" is less clear about
what's being inverted.

>  		if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size) < 0)
>  			return 1;
>  
> -		if (!nand_unlock(&nand_info[dev], off, size)) {
> +		if (!nand_unlock(&nand_info[dev], off, size, invert)) {
>  			puts("NAND flash successfully unlocked\n");
>  		} else {
>  			puts("Error unlocking NAND flash, "
> @@ -807,7 +814,7 @@ U_BOOT_CMD(
>  	"\n"
>  	"nand lock [tight] [status]\n"
>  	"    bring nand to lock state or display locked pages\n"
> -	"nand unlock [offset] [size] - unlock section"
> +	"nand unlock[.invert] [offset] [size] - unlock section"
>  #endif
>  #ifdef CONFIG_ENV_OFFSET_OOB
>  	"\n"
> diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
> index 7ed8b18..87caa62 100644
> --- a/drivers/mtd/nand/nand_util.c
> +++ b/drivers/mtd/nand/nand_util.c
> @@ -317,18 +317,19 @@ int nand_get_lock_status(struct mtd_info *mtd, loff_t offset)
>   * @param start		start byte address
>   * @param length	number of bytes to unlock (must be a multiple of
>   *			page size nand->writesize)
> + * @param invert	if set, unlock everything not selected
>   *
>   * @return		0 on success, -1 in case of error
>   */
> -int nand_unlock(struct mtd_info *mtd, ulong start, ulong length)
> +int nand_unlock(struct mtd_info *mtd, ulong start, ulong length, int invert)

Hmm, seems this got missed in the loff_t conversion.

>  {
>  	int ret = 0;
>  	int chipnr;
>  	int status;
>  	int page;
>  	struct nand_chip *chip = mtd->priv;
> -	printf ("nand_unlock: start: %08x, length: %d!\n",
> -		(int)start, (int)length);
> +	printf("nand_unlock%s: start: %08x, length: %d!\n",
> +		invert ? " (invert)" : "", (int)start, (int)length);

This looks like it should be a debug message rather than a normal printf
(and the (int) casts should go).

>  	/* select the NAND device */
>  	chipnr = (int)(start >> chip->chip_shift);
> @@ -368,6 +369,8 @@ int nand_unlock(struct mtd_info *mtd, ulong start, ulong length)
>  
>  	/* submit ADDRESS of LAST page to unlock */
>  	page += (int)(length >> chip->page_shift);
> +	if (invert)
> +		page |= 1;
>  	chip->cmdfunc(mtd, NAND_CMD_UNLOCK2, -1, page & chip->pagemask);

Why |= 1?  Is this some magic that the chip recognizes to implement
"invert"?  Do all chips that support lock/unlock support this (none of
the NAND chip manuals I have document lock/unlock at all as far as I
could find)?  What if you want to unlock a non-inverted range that ends
in a page with the low bit set?

-Scott