Re: [PATCH 2/2] mtd: nand: rearrange ONFI revision checking, add ONFI 2.3

[Florian Fainelli <ffainelli@freebox.fr>]

On Sunday 12 December 2010 09:23:33 Brian Norris wrote:
> In checking for the ONFI revision, the first conditional (for checking
> "unsupported" ONFI) seems unnecessary.  All ONFI revisions should be
> backwards-compatible; even if this is not the case on some newer ONFI
> revision, it should simply fail the second version-checking if-else block
> (i.e., the bit-fields for 1.0, 2.0, etc. would not be set to 1). Thus, we
> move our "unsupported" condition after having checked each bit field.
> 
> Also, it's simple enough to add a condition for ONFI revision 2.3. Note
> that this does *NOT* mean we handle all new features of ONFI versions
> above 1.0.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Acked-by: Florian Fainelli <ffainelli@freebox.fr>

> ---
>  drivers/mtd/nand/nand_base.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 38b5eb0..2237a87 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2872,20 +2872,24 @@ static int nand_flash_detect_onfi(struct mtd_info
> *mtd, struct nand_chip *chip,
> 
>  	/* check version */
>  	val = le16_to_cpu(p->revision);
> -	if (val == 1 || val > (1 << 4)) {
> -		printk(KERN_INFO "%s: unsupported ONFI version: %d\n",
> -								__func__, val);
> -		return 0;
> -	}
> -
> -	if (val & (1 << 4))
> +	if (val & (1 << 5))
> +		chip->onfi_version = 23;
> +	else if (val & (1 << 4))
>  		chip->onfi_version = 22;
>  	else if (val & (1 << 3))
>  		chip->onfi_version = 21;
>  	else if (val & (1 << 2))
>  		chip->onfi_version = 20;
> -	else
> +	else if (val & (1 << 1))
>  		chip->onfi_version = 10;
> +	else
> +		chip->onfi_version = 0;
> +
> +	if (!chip->onfi_version) {
> +		printk(KERN_INFO "%s: unsupported ONFI version: %d\n",
> +								__func__, val);
> +		return 0;
> +	}
> 
>  	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
>  	sanitize_string(p->model, sizeof(p->model));