[U-Boot] [PATCH 0/2] mtd: denali: Use SELF_INIT to fix up registers after nand_scan_ident()
This patch series is here because Scott Wood recommended me to use CONFIG_SYS_NAND_SELF_INIT to solve my problem: http://patchwork.ozlabs.org/patch/402462/
Masahiro Yamada (2): mtd: denali: use CONFIG_SYS_NAND_SELF_INIT mtd: denali: set some registers after nand_scan_ident()
drivers/mtd/nand/Kconfig | 7 +++ drivers/mtd/nand/denali.c | 131 +++++++++++++++++++++++++++++++++------------- drivers/mtd/nand/denali.h | 5 +- 3 files changed, 103 insertions(+), 40 deletions(-)
Some variants of the Denali NAND controller need some registers set up based on the device information that has been detected during nand_scan_ident().
CONFIG_SYS_NAND_SELF_INIT has to be defined to insert code between nand_scan_ident() and nand_scan_tail(). It is also helpful to reduce the difference between this driver and its Linux counterpart because this driver was ported from Linux. Moreover, doc/README.nand recommends to use CONFIG_SYS_NAND_SELF_INIT.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Cc: Scott Wood scottwood@freescale.com Cc: Chin Liang See clsee@altera.com ---
drivers/mtd/nand/Kconfig | 7 +++ drivers/mtd/nand/denali.c | 120 ++++++++++++++++++++++++++++++++-------------- drivers/mtd/nand/denali.h | 5 +- 3 files changed, 92 insertions(+), 40 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 75c2c06..c242214 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -1,9 +1,16 @@ menu "NAND Device Support"
+config SYS_NAND_SELF_INIT + bool + help + This option, if enabled, provides more flexible and linux-like + NAND initialization process. + if !SPL_BUILD
config NAND_DENALI bool "Support Denali NAND controller" + select SYS_NAND_SELF_INIT help Enable support for the Denali NAND controller.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index 308b784..a9838d8 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -44,7 +44,7 @@ static int onfi_timing_mode = NAND_DEFAULT_TIMINGS; * this macro allows us to convert from an MTD structure to our own * device context (denali) structure. */ -#define mtd_to_denali(m) (((struct nand_chip *)mtd->priv)->priv) +#define mtd_to_denali(m) container_of(m->priv, struct denali_nand_info, nand)
/* These constants are defined by the driver to enable common driver * configuration options. */ @@ -1144,70 +1144,116 @@ static void denali_hw_init(struct denali_nand_info *denali)
static struct nand_ecclayout nand_oob;
-static int denali_nand_init(struct nand_chip *nand) +static int denali_init(struct denali_nand_info *denali) { - struct denali_nand_info *denali; + int ret;
- denali = malloc(sizeof(*denali)); - if (!denali) - return -ENOMEM; + denali_hw_init(denali);
- nand->priv = denali; + denali->mtd->name = "denali-nand"; + denali->mtd->owner = THIS_MODULE; + denali->mtd->priv = &denali->nand;
- denali->flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE; - denali->flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE; + /* register the driver with the NAND core subsystem */ + denali->nand.select_chip = denali_select_chip; + denali->nand.cmdfunc = denali_cmdfunc; + denali->nand.read_byte = denali_read_byte; + denali->nand.read_buf = denali_read_buf; + denali->nand.waitfunc = denali_waitfunc; + + /* + * scan for NAND devices attached to the controller + * this is the first stage in a two step process to register + * with the nand subsystem + */ + if (nand_scan_ident(denali->mtd, denali->max_banks, NULL)) { + ret = -ENXIO; + goto fail; + }
#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT /* check whether flash got BBT table (located at end of flash). As we * use NAND_BBT_NO_OOB, the BBT page will start with * bbt_pattern. We will have mirror pattern too */ - nand->bbt_options |= NAND_BBT_USE_FLASH; + denali->nand.bbt_options |= NAND_BBT_USE_FLASH; /* * We are using main + spare with ECC support. As BBT need ECC support, * we need to ensure BBT code don't write to OOB for the BBT pattern. * All BBT info will be stored into data area with ECC support. */ - nand->bbt_options |= NAND_BBT_NO_OOB; + denali->nand.bbt_options |= NAND_BBT_NO_OOB; #endif
- nand->ecc.mode = NAND_ECC_HW; - nand->ecc.size = CONFIG_NAND_DENALI_ECC_SIZE; - nand->ecc.read_oob = denali_read_oob; - nand->ecc.write_oob = denali_write_oob; - nand->ecc.read_page = denali_read_page; - nand->ecc.read_page_raw = denali_read_page_raw; - nand->ecc.write_page = denali_write_page; - nand->ecc.write_page_raw = denali_write_page_raw; + denali->nand.ecc.mode = NAND_ECC_HW; + denali->nand.ecc.size = CONFIG_NAND_DENALI_ECC_SIZE; + /* * Tell driver the ecc strength. This register may be already set * correctly. So we read this value out. */ - nand->ecc.strength = readl(denali->flash_reg + ECC_CORRECTION); - switch (nand->ecc.size) { + denali->nand.ecc.strength = readl(denali->flash_reg + ECC_CORRECTION); + switch (denali->nand.ecc.size) { case 512: - nand->ecc.bytes = (nand->ecc.strength * 13 + 15) / 16 * 2; + denali->nand.ecc.bytes = + (denali->nand.ecc.strength * 13 + 15) / 16 * 2; break; case 1024: - nand->ecc.bytes = (nand->ecc.strength * 14 + 15) / 16 * 2; + denali->nand.ecc.bytes = + (denali->nand.ecc.strength * 14 + 15) / 16 * 2; break; default: pr_err("Unsupported ECC size\n"); - return -EINVAL; + ret = -EINVAL; + goto fail; } - nand_oob.eccbytes = nand->ecc.bytes; - nand->ecc.layout = &nand_oob; - - /* Set address of hardware control function */ - nand->cmdfunc = denali_cmdfunc; - nand->read_byte = denali_read_byte; - nand->read_buf = denali_read_buf; - nand->select_chip = denali_select_chip; - nand->waitfunc = denali_waitfunc; - denali_hw_init(denali); - return 0; + nand_oob.eccbytes = denali->nand.ecc.bytes; + denali->nand.ecc.layout = &nand_oob; + + /* override the default operations */ + denali->nand.ecc.read_page = denali_read_page; + denali->nand.ecc.read_page_raw = denali_read_page_raw; + denali->nand.ecc.write_page = denali_write_page; + denali->nand.ecc.write_page_raw = denali_write_page_raw; + denali->nand.ecc.read_oob = denali_read_oob; + denali->nand.ecc.write_oob = denali_write_oob; + + if (nand_scan_tail(denali->mtd)) { + ret = -ENXIO; + goto fail; + } + + ret = nand_register(0); + +fail: + return ret; +} + +static int __board_nand_init(void) +{ + struct denali_nand_info *denali; + + denali = kzalloc(sizeof(*denali), GFP_KERNEL); + if (!denali) + return -ENOMEM; + + /* + * If CONFIG_SYS_NAND_SELF_INIT is defined, each driver is responsible + * for instantiating struct nand_chip, while drivers/mtd/nand/nand.c + * still provides a "struct mtd_info nand_info" instance. + */ + denali->mtd = nand_info; + + /* + * In the future, these base addresses should be taken from + * Device Tree or platform data. + */ + denali->flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE; + denali->flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE; + + return denali_init(denali); }
-int board_nand_init(struct nand_chip *chip) +void board_nand_init(void) { - return denali_nand_init(chip); + __board_nand_init(); } diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h index 3277da7..a258df0 100644 --- a/drivers/mtd/nand/denali.h +++ b/drivers/mtd/nand/denali.h @@ -434,9 +434,8 @@ struct nand_buf { #define DT 3
struct denali_nand_info { - struct mtd_info mtd; - struct nand_chip *nand; - + struct mtd_info *mtd; + struct nand_chip nand; int flash_bank; /* currently selected chip */ int status; int platform;
On Tue, 2014-11-11 at 22:05 +0900, Masahiro Yamada wrote:
- /*
* If CONFIG_SYS_NAND_SELF_INIT is defined, each driver is responsible* for instantiating struct nand_chip, while drivers/mtd/nand/nand.c* still provides a "struct mtd_info nand_info" instance.*/- denali->mtd = nand_info;
&nand_info[0] would be clearer.
- /*
* In the future, these base addresses should be taken from* Device Tree or platform data.*/- denali->flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE;
- denali->flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE;
- return denali_init(denali);
}
-int board_nand_init(struct nand_chip *chip) +void board_nand_init(void) {
- return denali_nand_init(chip);
- __board_nand_init();
}
Why do you need this wrapper rather than putting the contents of __board_nand_init() here?
Also, you might want to print an error if denali_init() returns an error, rather than just discarding it (or, make sure denali_init() prints for error conditions, and have it return void). I realize that the existing self-init drivers aren't perfect in this regard. :-)
-Scott
Hi Scott,
2014-11-12 12:06 GMT+09:00 Scott Wood scottwood@freescale.com:
On Tue, 2014-11-11 at 22:05 +0900, Masahiro Yamada wrote:
/** If CONFIG_SYS_NAND_SELF_INIT is defined, each driver is responsible* for instantiating struct nand_chip, while drivers/mtd/nand/nand.c* still provides a "struct mtd_info nand_info" instance.*/denali->mtd = nand_info;&nand_info[0] would be clearer.
OK, I will fix it.
/** In the future, these base addresses should be taken from* Device Tree or platform data.*/denali->flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE;denali->flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE;return denali_init(denali);}
-int board_nand_init(struct nand_chip *chip) +void board_nand_init(void) {
return denali_nand_init(chip);
__board_nand_init();}
Why do you need this wrapper rather than putting the contents of __board_nand_init() here?
I'd like to return -ENOMEM rather than printing "No memory" without return code.
"int __board_nand_init(void)" is a function where I write my best code and "void board_nand_init(void)" is a wrapper to adjust it into a frame work.
Also, you might want to print an error if denali_init() returns an error, rather than just discarding it (or, make sure denali_init() prints for error conditions, and have it return void). I realize that the existing self-init drivers aren't perfect in this regard. :-)
I will print an error message in v2.
Some but not all of implementations of the Denali NAND controller have hardware circuits to detect the device parameters such as page_size, erase_size, etc. Even on those SoCs with such hardware supported, the hardware is known to detect wrong parameters for some nasty (almost buggy) NAND devices. The device parameters detected during nand_scan_ident() are more trustworthy.
This commit sets some hardware registers to mtd->pagesize, mtd->oobsize, etc. in the code between nand_scan_ident() and nand_scan_tail().
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com Cc: Scott Wood scottwood@freescale.com Cc: Chin Liang See clsee@altera.com ---
drivers/mtd/nand/denali.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index a9838d8..a2b346e 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -1209,6 +1209,17 @@ static int denali_init(struct denali_nand_info *denali) nand_oob.eccbytes = denali->nand.ecc.bytes; denali->nand.ecc.layout = &nand_oob;
+ writel(denali->mtd->erasesize / denali->mtd->writesize, + denali->flash_reg + PAGES_PER_BLOCK); + writel(denali->nand.options & NAND_BUSWIDTH_16 ? 1 : 0, + denali->flash_reg + DEVICE_WIDTH); + writel(denali->mtd->writesize, + denali->flash_reg + DEVICE_MAIN_AREA_SIZE); + writel(denali->mtd->oobsize, + denali->flash_reg + DEVICE_SPARE_AREA_SIZE); + if (readl(denali->flash_reg + DEVICES_CONNECTED) == 0) + writel(1, denali->flash_reg + DEVICES_CONNECTED); + /* override the default operations */ denali->nand.ecc.read_page = denali_read_page; denali->nand.ecc.read_page_raw = denali_read_page_raw;
participants (3)
-
Masahiro YAMADA -
Masahiro Yamada
-
Scott Wood