mtd: denali: Use SELF_INIT to fix up registers after nand_scan_ident()
Subsystems (2)
cover 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(-)
Signature · 1 line
1.9.1
PATCH 1/2 mtd: denali: use CONFIG_SYS_NAND_SELF_INIT
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.
---
drivers/mtd/nand/Kconfig | 7 +++ drivers/mtd/nand/denali.c | 120 ++++++++++++++++++++++++++++++++-------------- drivers/mtd/nand/denali.h | 5 +- 3 files changed, 92 insertions(+), 40 deletions(-)
drivers/mtd/nand/Kconfig +7 -0
@@ -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.
drivers/mtd/nand/denali.c +83 -37
160 lines not on this page · raw message
drivers/mtd/nand/denali.h +2 -3
@@ -434,9 +434,8 @@ struct nand_buf {#define DT 3struct 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;
Signature · 1 line
1.9.1
PATCH 1/2 mtd: denali: use CONFIG_SYS_NAND_SELF_INIT
drivers/mtd/nand/denali.c static void denali_hw_init(struct denali_nand_info *denali) · line 1244
Quoted 6 lines · Masahiro Yamada
drivers/mtd/nand/denali.c +6
-0+/* +* 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.
Quoted 6 lines
drivers/mtd/nand/denali.c +6
-0+/* +* 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 ;
drivers/mtd/nand/denali.c static void denali_hw_init(struct denali_nand_info *denali) · line 1259
Quoted 10 lines
drivers/mtd/nand/denali.c +4
-2+ +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
PATCH 1/2 mtd: denali: use CONFIG_SYS_NAND_SELF_INIT
Hi Scott,
2014-11-12 12:06 GMT+09:00 Scott Wood <[email protected]>:
Quoted 9 lines · Masahiro Yamada
drivers/mtd/nand/denali.c static void denali_hw_init(struct denali_nand_info *denali) · line 1244
Quoted 6 lines
drivers/mtd/nand/denali.c +6
-0+/* +* 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.
Quoted 10 lines
Quoted 10 lines
drivers/mtd/nand/denali.c +8
-0+/* +* 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); }
Quoted 9 lines
drivers/mtd/nand/denali.c static void denali_hw_init(struct denali_nand_info *denali) · line 1259
Quoted 6 lines
drivers/mtd/nand/denali.c +2
-2-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.
Quoted 4 lines
Also, you might want to print an error if
denali_init()returns an error, rather than just discarding it (or, make suredenali_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.
Signature · 2 lines
Best Regards Masahiro Yamada
PATCH 2/2 mtd: denali: set some registers after nand_scan_ident()
In reply to Masahiro YamadaSome 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().
---
drivers/mtd/nand/denali.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/mtd/nand/denali.c +11 -0
@@ -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;
Signature · 1 line
1.9.1