[PATCH 1/2] mtd: nand: choose correct chip name (ONFI bug)

[PATCH 1/2] mtd: nand: choose correct chip name (ONFI bug)

[Brian Norris]

We have the order of the conditional wrong for choosing the ONFI chip name
vs. the ID table name. Without this fix, we will almost *always* choose a
NULL string to print out instead of the correct one.

This has already been suggested by Matthieu Castet.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 33550c4..38b5eb0 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3164,7 +3164,7 @@ ident_done:
 	printk(KERN_INFO "NAND device: Manufacturer ID:"
 		" 0x%02x, Chip ID: 0x%02x (%s %s)\n", *maf_id, *dev_id,
 		nand_manuf_ids[maf_idx].name,
-	chip->onfi_version ? type->name : chip->onfi_params.model);
+		chip->onfi_version ? chip->onfi_params.model : type->name);
 
 	return type;
 }
-- 
1.7.0.4

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

[Brian Norris]

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>
---
 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));
-- 
1.7.0.4

Re: [PATCH 1/2] mtd: nand: choose correct chip name (ONFI bug)

[Florian Fainelli]

On Sunday 12 December 2010 09:23:32 Brian Norris wrote:
> We have the order of the conditional wrong for choosing the ONFI chip name
> vs. the ID table name. Without this fix, we will almost *always* choose a
> NULL string to print out instead of the correct one.
> 
> This has already been suggested by Matthieu Castet.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

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

> ---
>  drivers/mtd/nand/nand_base.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 33550c4..38b5eb0 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3164,7 +3164,7 @@ ident_done:
>  	printk(KERN_INFO "NAND device: Manufacturer ID:"
>  		" 0x%02x, Chip ID: 0x%02x (%s %s)\n", *maf_id, *dev_id,
>  		nand_manuf_ids[maf_idx].name,
> -	chip->onfi_version ? type->name : chip->onfi_params.model);
> +		chip->onfi_version ? chip->onfi_params.model : type->name);
> 
>  	return type;
>  }

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

[Florian Fainelli]

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));

Re: [PATCH 1/2] mtd: nand: choose correct chip name (ONFI bug)

[Artem Bityutskiy]

On Sun, 2010-12-12 at 00:23 -0800, Brian Norris wrote:
> We have the order of the conditional wrong for choosing the ONFI chip name
> vs. the ID table name. Without this fix, we will almost *always* choose a
> NULL string to print out instead of the correct one.
> 
> This has already been suggested by Matthieu Castet.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Pushed both to l2-mtd-2.6.git, thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)