[PATCH] [MTD] NAND: Wrong calculation of page number in nand_block_bad()

[PATCH] [MTD] NAND: Wrong calculation of page number in nand_block_bad()

[Knobloch, Thomas]

In case that there is no memory based bad block table available the
function nand_block_checkbad() in drivers/mtd/nand/nand_base.c will call
nand_block_bad() directly. When parameter 'getchip' is set to zero,
nand_block_bad() will not right shift the offset to calculate the
correct page number.

Signed-off-by: Thomas Knobloch <knobloch@siemens.com>

diff -uNr a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c	2007-04-27 12:44:23.345198284
+0200
+++ b/drivers/mtd/nand/nand_base.c	2007-04-27 12:46:57.916226928
+0200
@@ -312,7 +312,7 @@
 		/* Select the NAND device */
 		chip->select_chip(mtd, chipnr);
 	} else
-		page = (int)ofs;
+		page = (int)(ofs >> chip->page_shift);
 
 	if (chip->options & NAND_BUSWIDTH_16) {
 		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos &
0xFE,

Re: [PATCH] [MTD] NAND: Wrong calculation of page number in nand_block_bad()

[Thomas Gleixner]

On Fri, 2007-04-27 at 13:19 +0200, Knobloch, Thomas wrote:
> In case that there is no memory based bad block table available the
> function nand_block_checkbad() in drivers/mtd/nand/nand_base.c will call
> nand_block_bad() directly. When parameter 'getchip' is set to zero,
> nand_block_bad() will not right shift the offset to calculate the
> correct page number.
> 
> Signed-off-by: Thomas Knobloch <knobloch@siemens.com>
>
> diff -uNr a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> --- a/drivers/mtd/nand/nand_base.c	2007-04-27 12:44:23.345198284
> +0200
> +++ b/drivers/mtd/nand/nand_base.c	2007-04-27 12:46:57.916226928
> +0200
> @@ -312,7 +312,7 @@
>  		/* Select the NAND device */
>  		chip->select_chip(mtd, chipnr);
>  	} else
> -		page = (int)ofs;
> +		page = (int)(ofs >> chip->page_shift);

Can you please make this outside the else path, so we can remove same
code in the if (getchip) path.

Please also apply the pagemask right there so we don't have it in both
cmdfunc() calls.

	tglx

AW: [PATCH] [MTD] NAND: Wrong calculation of page number innand_block_bad()

[Knobloch, Thomas]

On Fri, 2007-04-27 at 19:04, Thomas Gleixner wrote:
> Can you please make this outside the else path, so we can remove same
> code in the if (getchip) path.
> 
> Please also apply the pagemask right there so we don't have it in both
> cmdfunc() calls.

I added the requested code cleanup to the original patch:

diff -auNr a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c	2007-04-27 12:44:23.000000000
+0200
+++ b/drivers/mtd/nand/nand_base.c	2007-05-02 13:19:19.080023100
+0200
@@ -303,28 +303,27 @@
 	struct nand_chip *chip = mtd->priv;
 	u16 bad;
 
+	page = (int)(ofs >> chip->page_shift) & chip->pagemask;
+
 	if (getchip) {
-		page = (int)(ofs >> chip->page_shift);
 		chipnr = (int)(ofs >> chip->chip_shift);
 
 		nand_get_device(chip, mtd, FL_READING);
 
 		/* Select the NAND device */
 		chip->select_chip(mtd, chipnr);
-	} else
-		page = (int)ofs;
+	}
 
 	if (chip->options & NAND_BUSWIDTH_16) {
 		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos &
0xFE,
-			      page & chip->pagemask);
+			      page);
 		bad = cpu_to_le16(chip->read_word(mtd));
 		if (chip->badblockpos & 0x1)
 			bad >>= 8;
 		if ((bad & 0xFF) != 0xff)
 			res = 1;
 	} else {
-		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
-			      page & chip->pagemask);
+		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
page);
 		if (chip->read_byte(mtd) != 0xff)
 			res = 1;
 	}

Re: AW: [PATCH] [MTD] NAND: Wrong calculation of page number innand_block_bad()

[Thomas Gleixner]

On Wed, 2007-05-02 at 14:14 +0200, Knobloch, Thomas wrote:
> On Fri, 2007-04-27 at 19:04, Thomas Gleixner wrote:
> > Can you please make this outside the else path, so we can remove same
> > code in the if (getchip) path.
> > 
> > Please also apply the pagemask right there so we don't have it in both
> > cmdfunc() calls.
> 
> I added the requested code cleanup to the original patch:

If you now manage to send a non line wrapped patch with the log message
and your Signed-off-by, then I can apply it right from the mail.

Thanks,

	tglx

>  	if (chip->options & NAND_BUSWIDTH_16) {
>  		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos &
> 0xFE,
> +		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
> page);
>  		if (chip->read_byte(mtd) != 0xff)
>  			res = 1;
>  	}

Re: AW: [PATCH] [MTD] NAND: Wrong calculation of page number innand_block_bad()

[Thomas Knobloch]

On Wed, 2007-05-02 at 17:18 +0200, Thomas Gleixner wrote:
> If you now manage to send a non line wrapped patch with the log message
> and your Signed-off-by, then I can apply it right from the mail.

Sorry for the inconvinience. Here comes the patch again (hopefully right
this time):


In case that there is no memory based bad block table available the
function nand_block_checkbad() in drivers/mtd/nand/nand_base.c will call
nand_block_bad() directly. When parameter 'getchip' is set to zero,
nand_block_bad() will not right shift the offset to calculate the
correct page number.

Signed-off-by: Thomas Knobloch <knobloch@siemens.com>

diff -auNr a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c	2007-04-27 12:44:23.000000000 +0200
+++ b/drivers/mtd/nand/nand_base.c	2007-05-02 13:19:19.080023100 +0200
@@ -303,28 +303,27 @@
 	struct nand_chip *chip = mtd->priv;
 	u16 bad;
 
+	page = (int)(ofs >> chip->page_shift) & chip->pagemask;
+
 	if (getchip) {
-		page = (int)(ofs >> chip->page_shift);
 		chipnr = (int)(ofs >> chip->chip_shift);
 
 		nand_get_device(chip, mtd, FL_READING);
 
 		/* Select the NAND device */
 		chip->select_chip(mtd, chipnr);
-	} else
-		page = (int)ofs;
+	}
 
 	if (chip->options & NAND_BUSWIDTH_16) {
 		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos & 0xFE,
-			      page & chip->pagemask);
+			      page);
 		bad = cpu_to_le16(chip->read_word(mtd));
 		if (chip->badblockpos & 0x1)
 			bad >>= 8;
 		if ((bad & 0xFF) != 0xff)
 			res = 1;
 	} else {
-		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos,
-			      page & chip->pagemask);
+		chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos, page);
 		if (chip->read_byte(mtd) != 0xff)
 			res = 1;
 	}