[PATCH] mtd: nand: hack ONFI for non-power-of-2 dimensions

[PATCH] mtd: nand: hack ONFI for non-power-of-2 dimensions

[Brian Norris]

Some bright specification writers decided to write this in the ONFI spec
(from ONFI 3.0, Section 3.1):

  "The number of blocks and number of pages per block is not required to
  be a power of two. In the case where one of these values is not a
  power of two, the corresponding address shall be rounded to an
  integral number of bits such that it addresses a range up to the
  subsequent power of two value. The host shall not access upper
  addresses in a range that is shown as not supported."

This breaks every assumption MTD makes about NAND block/chip-size
dimensions -- they *must* be a power of two!

And of course, an enterprising manufacturer has made use of this lovely
freedom. Exhibit A: Micron MT29F32G08CBADAWP

  "- Plane size: 2 planes x 1064 blocks per plane
   - Device size: 32Gb: 2128 blockss [sic]"

This quickly hits a BUG() in nand_base.c, since the extra dimensions
overflow so we think it's a second chip (on my single-chip setup):

    ONFI param page 0 valid
    ONFI flash detected
    NAND device: Manufacturer ID: 0x2c, Chip ID: 0x44 (Micron MT29F32G08CBADAWP), 4256MiB, page size: 8192, OOB size: 744
    ------------[ cut here ]------------
    kernel BUG at drivers/mtd/nand/nand_base.c:203!
    Internal error: Oops - BUG: 0 [#1] SMP ARM
    [... trim ...]
    [<c02cf3e4>] (nand_select_chip+0x18/0x2c) from [<c02d25c0>] (nand_do_read_ops+0x90/0x424)
    [<c02d25c0>] (nand_do_read_ops+0x90/0x424) from [<c02d2dd8>] (nand_read+0x54/0x78)
    [<c02d2dd8>] (nand_read+0x54/0x78) from [<c02ad2c8>] (mtd_read+0x84/0xbc)
    [<c02ad2c8>] (mtd_read+0x84/0xbc) from [<c02d4b28>] (scan_read.clone.4+0x4c/0x64)
    [<c02d4b28>] (scan_read.clone.4+0x4c/0x64) from [<c02d4c88>] (search_bbt+0x148/0x290)
    [<c02d4c88>] (search_bbt+0x148/0x290) from [<c02d4ea4>] (nand_scan_bbt+0xd4/0x5c0)
    [... trim ...]
    ---[ end trace 0c9363860d865ff2 ]---

So to fix this, just truncate these dimensions down to the greatest
power-of-2 dimension that is less than or equal to the specified
dimension.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: <stable@vger.kernel.org>
---
 drivers/mtd/nand/nand_base.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 7ed4841..208321a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2983,10 +2983,21 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
 	sanitize_string(p->model, sizeof(p->model));
 	if (!mtd->name)
 		mtd->name = p->model;
+
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
-	mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
+
+	/*
+	 * pages_per_block and blocks_per_lun may not be a power-of-2 size
+	 * (don't ask me who thought of this...). MTD assumes that these
+	 * dimensions will be power-of-2, so just truncate the remaining area.
+	 */
+	mtd->erasesize = 1 << (fls(le32_to_cpu(p->pages_per_block)) - 1);
+	mtd->erasesize *= mtd->writesize;
+
 	mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
-	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
+
+	/* See erasesize comment */
+	chip->chipsize = 1 << (fls(le32_to_cpu(p->blocks_per_lun)) - 1);
 	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
 
 	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
-- 
1.8.4

Re: [PATCH] mtd: nand: hack ONFI for non-power-of-2 dimensions

[Brian Norris]

On Tue, Aug 27, 2013 at 07:19:18PM -0700, Brian Norris wrote:
> Some bright specification writers decided to write this in the ONFI spec
> (from ONFI 3.0, Section 3.1):
> 
>   "The number of blocks and number of pages per block is not required to
>   be a power of two. In the case where one of these values is not a
>   power of two, the corresponding address shall be rounded to an
>   integral number of bits such that it addresses a range up to the
>   subsequent power of two value. The host shall not access upper
>   addresses in a range that is shown as not supported."
> 
> This breaks every assumption MTD makes about NAND block/chip-size
> dimensions -- they *must* be a power of two!
> 
> And of course, an enterprising manufacturer has made use of this lovely
> freedom. Exhibit A: Micron MT29F32G08CBADAWP
> 
>   "- Plane size: 2 planes x 1064 blocks per plane
>    - Device size: 32Gb: 2128 blockss [sic]"
> 
> This quickly hits a BUG() in nand_base.c, since the extra dimensions
> overflow so we think it's a second chip (on my single-chip setup):
> 
>     ONFI param page 0 valid
>     ONFI flash detected
>     NAND device: Manufacturer ID: 0x2c, Chip ID: 0x44 (Micron MT29F32G08CBADAWP), 4256MiB, page size: 8192, OOB size: 744
>     ------------[ cut here ]------------
>     kernel BUG at drivers/mtd/nand/nand_base.c:203!
>     Internal error: Oops - BUG: 0 [#1] SMP ARM
>     [... trim ...]
>     [<c02cf3e4>] (nand_select_chip+0x18/0x2c) from [<c02d25c0>] (nand_do_read_ops+0x90/0x424)
>     [<c02d25c0>] (nand_do_read_ops+0x90/0x424) from [<c02d2dd8>] (nand_read+0x54/0x78)
>     [<c02d2dd8>] (nand_read+0x54/0x78) from [<c02ad2c8>] (mtd_read+0x84/0xbc)
>     [<c02ad2c8>] (mtd_read+0x84/0xbc) from [<c02d4b28>] (scan_read.clone.4+0x4c/0x64)
>     [<c02d4b28>] (scan_read.clone.4+0x4c/0x64) from [<c02d4c88>] (search_bbt+0x148/0x290)
>     [<c02d4c88>] (search_bbt+0x148/0x290) from [<c02d4ea4>] (nand_scan_bbt+0xd4/0x5c0)
>     [... trim ...]
>     ---[ end trace 0c9363860d865ff2 ]---
> 
> So to fix this, just truncate these dimensions down to the greatest
> power-of-2 dimension that is less than or equal to the specified
> dimension.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: <stable@vger.kernel.org>

I've not heard any comments about this, but I've been using this patch
for a few months on most of my kernels, and I just ran into another
(unpatched) kernel/board that trips over this same problem. Unless
someone screams...

... applied to l2-mtd.git, with CC -stable. Although it is a bugfix, I
think it should wait until next merge window so it can sit in -next for
a while.

Brian