nand/denali: Adding Denali NAND driver support
Subsystems (2)
PATCH v4 1/2 nand/denali: Adding Denali NAND driver support
To add the Denali NAND driver support into U-Boot. It required information such as register base address from configuration header file within include/configs folder.
---
Changes for v4
- Added cache flush to handle dcache enabled
- Used standard return where 0 for pass
- Removed unnecessary casting
- Used standard readl and writel
Changes for v3
- Fixed coding style
Changes for v2
- Enable this driver support for SOCFPGA
--- drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/denali_nand.c | 1146 ++++++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/denali_nand.h | 493 +++++++++++++++++ 3 files changed, 1640 insertions(+) create mode 100644 drivers/mtd/nand/denali_nand.c create mode 100644 drivers/mtd/nand/denali_nand.h
drivers/mtd/nand/Makefile +1 -0
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o +obj-$(CONFIG_NAND_DENALI) += denali_nand.o obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o obj-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
drivers/mtd/nand/denali_nand.c new file +1146 -0
1,146 lines not on this page · raw message
drivers/mtd/nand/denali_nand.h new file +493 -0
493 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v4 1/2 nand/denali: Adding Denali NAND driver support
Hello Chin,
Quoted 15 lines
drivers/mtd/nand/denali_nand.c new file +15
-0+/* setups the HW to perform the data DMA */ +static void denali_setup_dma_sequence (int op) +{ +const int page_count =1 ; +uint32_t mode; +uint32_t addr = (uint32_t )denali.buf.dma_buf; + +flush_dcache_range (addr, addr +sizeof (denali.buf.dma_buf)); + + mode =MODE_10 |BANK (denali.flash_bank); + +/* DMA is a four step process */ + +/* 1. setup transfer type and # of pages */ +index_addr (mode | denali.page,0x2000 | op | page_count);
drivers/mtd/nand/denali_nand.c line 690
Quoted 10 lines
drivers/mtd/nand/denali_nand.c new file +10
-0+ +/* 2. set memory high address bits 23:8 */ +index_addr (mode | ((uint32_t )(addr >>16 ) <<8 ),0x2200 ); + +/* 3. set memory low address bits 23:8 */ +index_addr (mode | ((uint32_t )addr <<8 ),0x2300 ); + +/* 4. interrupt when complete, burst len = 64 bytes*/ +index_addr (mode |0x14000 ,0x2400 ); +}
This function would not work on my board. Besides, it is completely different from the description written in "Chapter 7. Data DMA" of
"Denali NAND Flash Memory Controller User's Guide".
It looks really weird to me.
So I asked some questions to Cadence (Denali, before M&A).
Accoding to the anwers from Cadence,
in conclusion, I think this function seems wrong.
Please let me confirm once again:
"Did this function really work on your board?"
I had to re-write this function to run on my board.
Quoted 1 line
#define
DENALI_DATA_OFFSET_ADDRESS0x10
This is a bad name.
I can't understand at all what this macro stands for. Besides, you only defined data register, not control register. This is weird.
There exist two registers: control and data
for indexed-addressing of Denali NAND controller.
Denali document shows
Table 8.1
Register | Offset Address
----------------------------------
Control | 0x0
Data | 0x10
How about this?
#define INDEX_CTRL_REG 0x0
#define INDEX_DATA_REG 0x10
And move definition from denali_nand.c to denal_nand.h.
Quoted 3 lines
/* setups the HW to perform the data DMA */
static voiddenali_setup_dma_sequence(int op)
{
Please rename "denali_setup_dma_sequence"
to "denali_setup_dma".
Linux uses the latter.
No reason to change it.
Quoted 3 lines
/* Common DMA function */
staticuint32_tdenali_dma_configuration(uint32_t ops, boolraw_xfer,
uint32_tirq_mask, intoob_required)
Fix indentaion, please.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
It's "
denali.c" in Linux -- why "denali_nand.c" here?It seems all the existing U-Boot nand driver is using this naming standard where <platform>_nand.
Quoted 7 lines
Quoted 4 lines
Not all -- there's
omap_gpmc.c,omap_elm.c,nomadik.c,ndfc.c, etc.A lot of them have the
_nand.csuffix in Linux, too. Personally, I think it's redundant.Sure, I can change to
denali.c
Agreed.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Why are you using raw I/O accessors? The Linux driver doesn't do this.
Add ioread32/iowrite32 to
arch/arm/include/asm/io.hand use them?Changed all to use standard writel and readl.
In my understanding, ioread32/iowrite32 is the same as readl/writel at least memory mapped architectures such ARM.
Because Linux Kernel' denali.c uses ioread32/iowrite32, I thought using them is helpful for easy diffing.
I've posted my feedback.
I hope it is helpful for you.
http://patchwork.ozlabs.org/patch/327943/
Best Regards
Masahiro Yamada
PATCH v4 1/2 nand/denali: Adding Denali NAND driver support
Hi Masahiro,
Quoted 26 lines · Masahiro Yamada
Hello Chin,
Quoted 23 lines
drivers/mtd/nand/denali_nand.c new file +23
-0+/* setups the HW to perform the data DMA */ +static void denali_setup_dma_sequence (int op) +{ +const int page_count =1 ; +uint32_t mode; +uint32_t addr = (uint32_t )denali.buf.dma_buf; + +flush_dcache_range (addr, addr +sizeof (denali.buf.dma_buf)); + + mode =MODE_10 |BANK (denali.flash_bank); + +/* DMA is a four step process */ + +/* 1. setup transfer type and # of pages */ +index_addr (mode | denali.page,0x2000 | op | page_count); + +/* 2. set memory high address bits 23:8 */ +index_addr (mode | ((uint32_t )(addr >>16 ) <<8 ),0x2200 ); + +/* 3. set memory low address bits 23:8 */ +index_addr (mode | ((uint32_t )addr <<8 ),0x2300 ); + +/* 4. interrupt when complete, burst len = 64 bytes*/
Quoted 8 lines
drivers/mtd/nand/denali_nand.c line 690
Quoted 2 lines
drivers/mtd/nand/denali_nand.c new file +2
-0+index_addr (mode |0x14000 ,0x2400 ); +}This function would not work on my board. Besides, it is completely different from the description written in "Chapter 7. Data DMA" of
"Denali NAND Flash Memory Controller User's Guide".
Hmmm... based on past few email exchange, I am wonder there are few different version of Denali. For this section, its located at chapter 7:
Data DMA. Wonder you can facilitate which part if not correct. From there, I can provide what being explain in the datasheet.
Quoted 10 lines
It looks really weird to me.
So I asked some questions to Cadence (Denali, before M&A).Accoding to the anwers from Cadence,
in conclusion, I think this function seems wrong.Please let me confirm once again:
"Did this function really work on your board?"
Yup, if you see this driver, all transaction is being done using DMA function. Its working as some of SOCFPGA users are using that code.
Quoted 2 lines
I had to re-write this function to run on my board.
Can you share your code so we can check the differences. Its really strange.
Quoted 19 lines
Quoted 2 lines
#define
DENALI_DATA_OFFSET_ADDRESS0x10This is a bad name.
I can't understand at all what this macro stands for. Besides, you only defined data register, not control register. This is weird.There exist two registers: control and data
for indexed-addressing of Denali NAND controller.Denali document shows
Table 8.1
Register | Offset Address
----------------------------------
Control |0x0
Data |0x10
Quoted 4 lines
How about this?
#defineINDEX_CTRL_REG0x0
#defineINDEX_DATA_REG0x10
I have no much objection when come to naming. FYI, I just pluck this name from datasheet. I will changed that with file name change.
Quoted 3 lines
And move definition from
denali_nand.ctodenal_nand.h.
Yup, actually waiting more comments from you before sending out next revision. :)
Quoted 11 lines
Quoted 4 lines
/* setups the HW to perform the data DMA */
static voiddenali_setup_dma_sequence(int op)
{Please rename "
denali_setup_dma_sequence"
to "denali_setup_dma".Linux uses the latter.
No reason to change it.
I can change that.
Quoted 7 lines
Quoted 4 lines
/* Common DMA function */
staticuint32_tdenali_dma_configuration(uint32_t ops, boolraw_xfer,
uint32_tirq_mask, intoob_required)Fix indentaion, please.
I can fix that. Hmmm seems checkpatch dun catch this.
Quoted 33 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
It's "
denali.c" in Linux -- why "denali_nand.c" here?It seems all the existing U-Boot nand driver is using this naming standard where <platform>_nand.
Quoted 7 lines
Quoted 4 lines
Not all -- there's
omap_gpmc.c,omap_elm.c,nomadik.c,ndfc.c, etc.A lot of them have the
_nand.csuffix in Linux, too. Personally, I think it's redundant.Sure, I can change to
denali.cAgreed.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Why are you using raw I/O accessors? The Linux driver doesn't do this.
Add ioread32/iowrite32 to
arch/arm/include/asm/io.hand use them?Changed all to use standard writel and readl.
In my understanding, ioread32/iowrite32 is the same as readl/writel at least memory mapped architectures such ARM.
Quoted 8 lines
Because Linux Kernel'
denali.cuses ioread32/iowrite32, I thought using them is helpful for easy diffing.I've posted my feedback.
I hope it is helpful for you.
http://patchwork.ozlabs.org/patch/327943/
Great, I will patch them.
PATCH v4 1/2 nand/denali: Adding Denali NAND driver support
Hello Chin,
Quoted 10 lines
Quoted 4 lines
I've posted my feedback.
I hope it is helpful for you.
http://patchwork.ozlabs.org/patch/327943/Great, I will patch them.
From the patch, I saw your DMA sequence code and the discrepancy. For the data portion, I spotted you set the bit 31:16. But inside my datasheet, its marked as reserved.
Quoted 4 lines
Wonder the version of Denali controller I have is older? Probably we can probe the Cadence then.
At same time, I will send out new patch to capture the fix before forget.
Yes, please.
I am interested in this mystery.
I really suspect there are some
incompatible versions in Denali IP.
I sent a question about this to Cadence.
They did not admit it. (But I doubt it.)
By the way, Panasonic have been using Denali
NAND controller IP since 2010.
I refer to "Denali NAND Flash Memory Controller User's Guide" issued on Nov. 30, 2010.
Unfortunately, I can not share it because it is marked as "confidential".
Best Regards
Masahiro Yamada.
PATCH v4 1/2 nand/denali: Adding Denali NAND driver support
Hi Mashiro,
Quoted 28 lines · Masahiro Yamada
Hello Chin,
Quoted 10 lines
Quoted 4 lines
I've posted my feedback.
I hope it is helpful for you.
http://patchwork.ozlabs.org/patch/327943/Great, I will patch them.
From the patch, I saw your DMA sequence code and the discrepancy. For the data portion, I spotted you set the bit 31:16. But inside my datasheet, its marked as reserved.Quoted 4 lines
Wonder the version of Denali controller I have is older? Probably we can probe the Cadence then.
At same time, I will send out new patch to capture the fix before forget.Yes, please.
I am interested in this mystery.
I really suspect there are some
incompatible versions in Denali IP.I sent a question about this to Cadence.
They did not admit it. (But I doubt it.)
Quoted 6 lines
By the way, Panasonic have been using Denali
NAND controller IP since 2010.
I refer to "Denali NAND Flash Memory Controller User's Guide" issued on Nov. 30, 2010.
Unfortunately, I can not share it because it is marked as "confidential".
Yup, me too. My one is having same title but dated Feb 28, 2011. I also checked the latest Feb 14, 2013 and it matched with 2011. Seems they changed in between 2010 and 2011. Wonder you have any newer version?
To work around this, I presume we might to have some #if #else to separate the version differences.
Thanks
Chin Liang
Quoted 4 lines
Best Regards
Masahiro Yamada.