Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
On Wed, 30 Nov 2016 15:09:27 +0900 Masahiro Yamada wrote: > Hi Boris, > > > 2016-11-28 1:24 GMT+09:00 Boris Brezillon > : > > On Sun, 27 Nov 2016 03:06:14 +0900 > > Masahiro Yamada wrote: > > > >> Collect multi NAND fixups into a helper function instead of > >> scattering them in denali_init(). > > > > Can you tell me more about this multi-NAND feature? > > The core is already able to detect multi-die NAND chips in a generic > > way, > > This is not the case. > > > but I fear this is something else, like "put two 8-bits chips on a > > 16bits bus to emulate a single 16bits chip". > > Yes, it is. > > (I have never used this controller like that. > But, I am pretty sure it is > from the code and the > Denali's User Guide mentions such usage.) > > > Just in case, I will clearly rephrase the comment block like follows in v2: > > /* > * Support for multi device: > * When the IP configuration is x16 capable and two x8 chips are > * connected in parallel, DEVICES_CONNECTED should be set to 2. > * In this case, the core framework knows nothing about this fact, > * so we should tell it the _logical_ pagesize and anything necessary. > */ > BTW, you should also set the NAND_BUSWIDTH_16 flag in this case. > > > > > If that's a case, and this feature is actually used, then it's a bad > > idea IMHO. > > For example, how do you handle the case where one block is bad on a > > chip but not on the other? And I fear this is not the only problem > > with this approach :-/. > > As you expect, if one block is bad, > the correspond block on the other chip can not be used. > Hm, last time I thought about this usage I found others things that could cause problems, but I can't remember exactly what. Anyway, if this feature is already used, let's keep it.
Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Hi Boris, 2016-11-28 1:24 GMT+09:00 Boris Brezillon : > On Sun, 27 Nov 2016 03:06:14 +0900 > Masahiro Yamada wrote: > >> Collect multi NAND fixups into a helper function instead of >> scattering them in denali_init(). > > Can you tell me more about this multi-NAND feature? > The core is already able to detect multi-die NAND chips in a generic > way, This is not the case. > but I fear this is something else, like "put two 8-bits chips on a > 16bits bus to emulate a single 16bits chip". Yes, it is. (I have never used this controller like that. But, I am pretty sure it is from the code and the Denali's User Guide mentions such usage.) Just in case, I will clearly rephrase the comment block like follows in v2: /* * Support for multi device: * When the IP configuration is x16 capable and two x8 chips are * connected in parallel, DEVICES_CONNECTED should be set to 2. * In this case, the core framework knows nothing about this fact, * so we should tell it the _logical_ pagesize and anything necessary. */ > If that's a case, and this feature is actually used, then it's a bad > idea IMHO. > For example, how do you handle the case where one block is bad on a > chip but not on the other? And I fear this is not the only problem > with this approach :-/. As you expect, if one block is bad, the correspond block on the other chip can not be used. -- Best Regards Masahiro Yamada
Re: [PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
On Sun, 27 Nov 2016 03:06:14 +0900
Masahiro Yamada wrote:
> Collect multi NAND fixups into a helper function instead of
> scattering them in denali_init().
Can you tell me more about this multi-NAND feature?
The core is already able to detect multi-die NAND chips in a generic
way, but I fear this is something else, like "put two 8-bits chips on a
16bits bus to emulate a single 16bits chip".
If that's a case, and this feature is actually used, then it's a bad
idea IMHO.
For example, how do you handle the case where one block is bad on a
chip but not on the other? And I fear this is not the only problem
with this approach :-/.
>
> Signed-off-by: Masahiro Yamada
> ---
>
> drivers/mtd/nand/denali.c | 51
> ---
> 1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 60b0858..54dcd83 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1472,6 +1472,34 @@ static void denali_drv_init(struct denali_nand_info
> *denali)
> denali->irq_status = 0;
> }
>
> +static void denali_multidev_fixup(struct denali_nand_info *denali)
> +{
> + struct nand_chip *chip = &denali->nand;
> + struct mtd_info *mtd = nand_to_mtd(chip);
> +
> + /*
> + * Support for multi NAND:
> + * MTD knows nothing about multi NAND, so we should tell it
> + * the real pagesize and anything necessary
> + */
> + denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
> +
> + mtd->size <<= denali->devnum - 1;
> + mtd->erasesize <<= denali->devnum - 1;
> + mtd->writesize <<= denali->devnum - 1;
> + mtd->oobsize <<= denali->devnum - 1;
> + chip->chipsize <<= denali->devnum - 1;
> + chip->page_shift += denali->devnum - 1;
> + chip->phys_erase_shift += denali->devnum - 1;
> + chip->bbt_erase_shift += denali->devnum - 1;
> + chip->chip_shift += denali->devnum - 1;
> + chip->pagemask <<= denali->devnum - 1;
> + chip->ecc.size *= denali->devnum;
> + chip->ecc.bytes *= denali->devnum;
> + chip->ecc.strength *= denali->devnum;
> + denali->bbtskipbytes *= denali->devnum;
> +}
> +
> int denali_init(struct denali_nand_info *denali)
> {
> struct nand_chip *chip = &denali->nand;
> @@ -1553,23 +1581,6 @@ int denali_init(struct denali_nand_info *denali)
> goto failed_req_irq;
> }
>
> - /*
> - * support for multi nand
> - * MTD known nothing about multi nand, so we should tell it
> - * the real pagesize and anything necessery
> - */
> - denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
> - chip->chipsize <<= denali->devnum - 1;
> - chip->page_shift += denali->devnum - 1;
> - chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
> - chip->bbt_erase_shift += denali->devnum - 1;
> - chip->phys_erase_shift = chip->bbt_erase_shift;
> - chip->chip_shift += denali->devnum - 1;
> - mtd->writesize <<= denali->devnum - 1;
> - mtd->oobsize <<= denali->devnum - 1;
> - mtd->erasesize <<= denali->devnum - 1;
> - mtd->size = chip->numchips * chip->chipsize;
> - denali->bbtskipbytes *= denali->devnum;
>
> /*
>* second stage of the NAND scan
> @@ -1614,11 +1625,9 @@ int denali_init(struct denali_nand_info *denali)
> }
>
> mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
> - chip->ecc.bytes *= denali->devnum;
> - chip->ecc.strength *= denali->devnum;
>
> /* override the default read operations */
> - chip->ecc.size = ECC_SECTOR_SIZE * denali->devnum;
> + chip->ecc.size = ECC_SECTOR_SIZE;
> chip->ecc.read_page = denali_read_page;
> chip->ecc.read_page_raw = denali_read_page_raw;
> chip->ecc.write_page = denali_write_page;
> @@ -1627,6 +1636,8 @@ int denali_init(struct denali_nand_info *denali)
> chip->ecc.write_oob = denali_write_oob;
> chip->erase = denali_erase;
>
> + denali_multidev_fixup(denali);
> +
> ret = nand_scan_tail(mtd);
> if (ret)
> goto failed_req_irq;
[PATCH 28/39] mtd: nand: denali: move multi NAND fixup code to a helper function
Collect multi NAND fixups into a helper function instead of
scattering them in denali_init().
Signed-off-by: Masahiro Yamada
---
drivers/mtd/nand/denali.c | 51 ---
1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 60b0858..54dcd83 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1472,6 +1472,34 @@ static void denali_drv_init(struct denali_nand_info
*denali)
denali->irq_status = 0;
}
+static void denali_multidev_fixup(struct denali_nand_info *denali)
+{
+ struct nand_chip *chip = &denali->nand;
+ struct mtd_info *mtd = nand_to_mtd(chip);
+
+ /*
+* Support for multi NAND:
+* MTD knows nothing about multi NAND, so we should tell it
+* the real pagesize and anything necessary
+*/
+ denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
+
+ mtd->size <<= denali->devnum - 1;
+ mtd->erasesize <<= denali->devnum - 1;
+ mtd->writesize <<= denali->devnum - 1;
+ mtd->oobsize <<= denali->devnum - 1;
+ chip->chipsize <<= denali->devnum - 1;
+ chip->page_shift += denali->devnum - 1;
+ chip->phys_erase_shift += denali->devnum - 1;
+ chip->bbt_erase_shift += denali->devnum - 1;
+ chip->chip_shift += denali->devnum - 1;
+ chip->pagemask <<= denali->devnum - 1;
+ chip->ecc.size *= denali->devnum;
+ chip->ecc.bytes *= denali->devnum;
+ chip->ecc.strength *= denali->devnum;
+ denali->bbtskipbytes *= denali->devnum;
+}
+
int denali_init(struct denali_nand_info *denali)
{
struct nand_chip *chip = &denali->nand;
@@ -1553,23 +1581,6 @@ int denali_init(struct denali_nand_info *denali)
goto failed_req_irq;
}
- /*
-* support for multi nand
-* MTD known nothing about multi nand, so we should tell it
-* the real pagesize and anything necessery
-*/
- denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
- chip->chipsize <<= denali->devnum - 1;
- chip->page_shift += denali->devnum - 1;
- chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
- chip->bbt_erase_shift += denali->devnum - 1;
- chip->phys_erase_shift = chip->bbt_erase_shift;
- chip->chip_shift += denali->devnum - 1;
- mtd->writesize <<= denali->devnum - 1;
- mtd->oobsize <<= denali->devnum - 1;
- mtd->erasesize <<= denali->devnum - 1;
- mtd->size = chip->numchips * chip->chipsize;
- denali->bbtskipbytes *= denali->devnum;
/*
* second stage of the NAND scan
@@ -1614,11 +1625,9 @@ int denali_init(struct denali_nand_info *denali)
}
mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
- chip->ecc.bytes *= denali->devnum;
- chip->ecc.strength *= denali->devnum;
/* override the default read operations */
- chip->ecc.size = ECC_SECTOR_SIZE * denali->devnum;
+ chip->ecc.size = ECC_SECTOR_SIZE;
chip->ecc.read_page = denali_read_page;
chip->ecc.read_page_raw = denali_read_page_raw;
chip->ecc.write_page = denali_write_page;
@@ -1627,6 +1636,8 @@ int denali_init(struct denali_nand_info *denali)
chip->ecc.write_oob = denali_write_oob;
chip->erase = denali_erase;
+ denali_multidev_fixup(denali);
+
ret = nand_scan_tail(mtd);
if (ret)
goto failed_req_irq;
--
2.7.4
