Skip to contentSkip to conversation

nand/denali: Adding Denali NAND driver support

PATCH Superseded Chin Liang See 2 participants 5 messages Mar 10, 2014 – Mar 12, 2014

Subsystems (2)
Memory technology devices (MTD) NAND flash

v5 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Chin Liang See

author
#0 lore raw

PATCH v5 1/1 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.

Signed-off-by Chin Liang See <[email protected]>
Cc Artem Bityutskiy <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Brian Norris <[email protected]>
Cc Scott Wood <[email protected]>
Cc Masahiro Yamada <[email protected]>

---
Changes for v5
- Rename denali_nand to denali only
- Rename the macro for ctrl and data address
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.c | 1144 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/denali.h |  496 ++++++++++++++++++++
 3 files changed, 1641 insertions(+)
 create mode 100644 drivers/mtd/nand/denali.c
 create mode 100644 drivers/mtd/nand/denali.h
drivers/mtd/nand/Makefile +1 -0
index 02b149c..76ae105 100644
@@ -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.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.c new file +1144 -0

1,144 lines not on this page · raw message

drivers/mtd/nand/denali.h new file +496 -0

496 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v5 1/1 nand/denali: Adding Denali NAND driver support

Hello Chin,

drivers/mtd/nand/denali.c line 692

Quoted 2 lines
drivers/mtd/nand/denali.c new file +2 -0
+static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
+	uint32_t irq_mask, int oob_required)

Indentation not fixed yet.
I mean, only one tab is too small for the second line.

Rationale: "Documentation/CodingStyle" of Linux Kernel

Accoding to it, the following lines should be "placed to substantially to the right."

Chapter 2: Breaking long lines and strings

Coding style is all about readability and maintainability using commonly available tools.

The limit on the length of lines is 80 columns and this is a strongly preferred limit.

Statements longer than 80 columns will be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. Descendants are always substantially shorter than the parent and are placed substantially to the right. The same applies to function headers with a long argument list. However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them.

drivers/mtd/nand/denali.c line 1094

Quoted 4 lines
drivers/mtd/nand/denali.c new file +4 -0
+void denali_nand_init(struct nand_chip *nand)
+{
+	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
+	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;

denali_nand_init() is used only inside this file. I'd like to suggest to add "static".

drivers/mtd/nand/denali.c line 1144

Quoted 6 lines
drivers/mtd/nand/denali.c new file +6 -0
+int board_nand_init(struct nand_chip *chip)
+{
+	puts("NAND:  Denali NAND controller\n");
+	denali_nand_init(chip);
+	return 0;
+}

Please delete
puts("NAND: Denali NAND controller\n");

With this patch, the boot messages on my board is like this:

    U-Boot 2014.04-rc2-00056-g05775a7-dirty (Mar 12 2014 - 13:04:36)
    
    CPU:   Peaks LD4 (MN2WS0250) (rev. 3)
    Board: Peaks LD4 Board
    SC:    Micro Support Card (DCC CPLD version 3.6.9)
    DRAM:  512 MiB
    NAND:  NAND:  Denali NAND controller
    2048 MiB
    In:    serial
    Out:   serial
    Err:   serial
    Net:   smc911x-0

"NAND: Denali NAND controller" is interrupting
between "NAND: " and "2048 Mib"

I think it is supposed be seen like this:

    DRAM:  512 MiB
    NAND:  2048 MiB
    In:    serial
    Out:   serial
    Err:   serial

Best Regards
Masahiro Yamada

author
#2 lore raw

PATCH v5 1/1 nand/denali: Adding Denali NAND driver support

Hi Masahiro,

Quoted 11 lines · Masahiro Yamada

Hello Chin,

drivers/mtd/nand/denali.c line 692

Quoted 2 lines
drivers/mtd/nand/denali.c new file +2 -0
+static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
+	uint32_t irq_mask, int oob_required)

Indentation not fixed yet.
I mean, only one tab is too small for the second line.

oops, actually I miss this one.
Fixed in next revision

Quoted 27 lines

Rationale: "Documentation/CodingStyle" of Linux Kernel

Accoding to it, the following lines should be "placed to substantially to the right."

Chapter 2: Breaking long lines and strings

Coding style is all about readability and maintainability using commonly available tools.

The limit on the length of lines is 80 columns and this is a strongly preferred limit.

Statements longer than 80 columns will be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. Descendants are always substantially shorter than the parent and are placed substantially to the right. The same applies to function headers with a long argument list. However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them.

Quoted 10 lines

drivers/mtd/nand/denali.c line 1094

Quoted 4 lines
drivers/mtd/nand/denali.c new file +4 -0
+void denali_nand_init(struct nand_chip *nand)
+{
+	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
+	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;

denali_nand_init() is used only inside this file. I'd like to suggest to add "static".

Fixed

Quoted 12 lines

drivers/mtd/nand/denali.c line 1144

Quoted 8 lines
drivers/mtd/nand/denali.c new file +6 -0
+int board_nand_init(struct nand_chip *chip)
+{
+	puts("NAND:  Denali NAND controller\n");
+	denali_nand_init(chip);
+	return 0;
+}

Please delete
puts("NAND: Denali NAND controller\n");

Actually I added this during debugging and forget remove this. Removed for next patch

Thanks

Chin Liang

Quoted 36 lines

With this patch, the boot messages on my board is like this:

    U-Boot 2014.04-rc2-00056-g05775a7-dirty (Mar 12 2014 - 13:04:36)
    
    CPU:   Peaks LD4 (MN2WS0250) (rev. 3)
    Board: Peaks LD4 Board
    SC:    Micro Support Card (DCC CPLD version 3.6.9)
    DRAM:  512 MiB
    NAND:  NAND:  Denali NAND controller
    2048 MiB
    In:    serial
    Out:   serial
    Err:   serial
    Net:   smc911x-0

"NAND: Denali NAND controller" is interrupting
between "NAND: " and "2048 Mib"

I think it is supposed be seen like this:

    DRAM:  512 MiB
    NAND:  2048 MiB
    In:    serial
    Out:   serial
    Err:   serial

Best Regards
Masahiro Yamada

#3 lore raw

PATCH v5 1/1 nand/denali: Adding Denali NAND driver support

In reply to Chin Liang See

Hello Chin,

Here is a little more comments about v5.

Quoted 8 lines
drivers/mtd/nand/denali.c new file +8 -0
+static void get_toshiba_nand_para(void)
+{
+	uint32_t tmp;
+
+	/* Workaround to fix a controller bug which reports a wrong */
+	/* spare area size for some kind of Toshiba NAND device */
+	if ((readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
+	    (readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)

drivers/mtd/nand/denali.c line 351

Quoted 10 lines
drivers/mtd/nand/denali.c new file +10 -0
+		== 64)){
+		writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		tmp = readl(denali.flash_reg + DEVICES_CONNECTED) *
+			readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		writel(tmp, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+#if SUPPORT_15BITECC
+		writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+		writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif

I can't understand why ECC_CORRECTION must be set here. Can you?
If not, could you delete it?

Quoted 12 lines
drivers/mtd/nand/denali.c new file +12 -0
+static void get_hynix_nand_para(uint8_t device_id)
+{
+	uint32_t main_size, spare_size;
+
+	switch (device_id) {
+	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
+	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
+		writel(128, denali.flash_reg + PAGES_PER_BLOCK);
+		writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
+		writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		main_size = 4096 *
+			readl(denali.flash_reg + DEVICES_CONNECTED);

drivers/mtd/nand/denali.c line 376

Quoted 10 lines
drivers/mtd/nand/denali.c new file +10 -0
+		spare_size = 224 *
+			readl(denali.flash_reg + DEVICES_CONNECTED);
+		writel(main_size, denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
+		writel(spare_size, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+		writel(0, denali.flash_reg + DEVICE_WIDTH);
+#if SUPPORT_15BITECC
+		writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+		writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif

Ditto.

drivers/mtd/nand/denali.c line 1099

Quoted 9 lines
drivers/mtd/nand/denali.c new file +9 -0
+void denali_nand_init(struct nand_chip *nand)
+{
+	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
+	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
+
+	nand->chip_delay  = 0;
+#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

	nand->chip_delay  = 0;

is not necessary.
Please delete.

if chip_delay is zero, chip_delay is set to 20
in nand_set_defaults() function.

Anyway it is never used in Denali NAND driver, because we are overriding cmdfunc with denali_cmdfunc().

Best Regards
Masahiro Yamada

author
#4 lore raw

PATCH v5 1/1 nand/denali: Adding Denali NAND driver support

Hi Masahiro,

Quoted 19 lines · Masahiro Yamada

Hello Chin,

Here is a little more comments about v5.

Quoted 14 lines
drivers/mtd/nand/denali.c new file +14 -0
+static void get_toshiba_nand_para(void)
+{
+	uint32_t tmp;
+
+	/* Workaround to fix a controller bug which reports a wrong */
+	/* spare area size for some kind of Toshiba NAND device */
+	if ((readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
+	    (readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)
+		== 64)){
+		writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		tmp = readl(denali.flash_reg + DEVICES_CONNECTED) *
+			readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		writel(tmp, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
+#if SUPPORT_15BITECC
Quoted 9 lines

drivers/mtd/nand/denali.c line 351

Quoted 4 lines
drivers/mtd/nand/denali.c new file +4 -0
+		writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+		writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif

I can't understand why ECC_CORRECTION must be set here. Can you?
If not, could you delete it?

Yup, this is not necessary as we setup this at init function. Removed

Quoted 18 lines
Quoted 18 lines
drivers/mtd/nand/denali.c new file +16 -0
+static void get_hynix_nand_para(uint8_t device_id)
+{
+	uint32_t main_size, spare_size;
+
+	switch (device_id) {
+	case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
+	case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
+		writel(128, denali.flash_reg + PAGES_PER_BLOCK);
+		writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
+		writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
+		main_size = 4096 *
+			readl(denali.flash_reg + DEVICES_CONNECTED);
+		spare_size = 224 *
+			readl(denali.flash_reg + DEVICES_CONNECTED);
+		writel(main_size, denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
+		writel(spare_size, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
Quoted 9 lines

drivers/mtd/nand/denali.c line 376

Quoted 6 lines
drivers/mtd/nand/denali.c new file +6 -0
+		writel(0, denali.flash_reg + DEVICE_WIDTH);
+#if SUPPORT_15BITECC
+		writel(15, denali.flash_reg + ECC_CORRECTION);
+#elif SUPPORT_8BITECC
+		writel(8, denali.flash_reg + ECC_CORRECTION);
+#endif

Ditto.

Removed

Quoted 19 lines

drivers/mtd/nand/denali.c line 1099

Quoted 11 lines
drivers/mtd/nand/denali.c new file +9 -0
+void denali_nand_init(struct nand_chip *nand)
+{
+	denali.flash_reg = (void  __iomem *)CONFIG_SYS_NAND_REGS_BASE;
+	denali.flash_mem = (void  __iomem *)CONFIG_SYS_NAND_DATA_BASE;
+
+	nand->chip_delay  = 0;
+#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

	nand->chip_delay  = 0;

is not necessary.
Please delete.

Quoted 6 lines

if chip_delay is zero, chip_delay is set to 20
in nand_set_defaults() function.

Anyway it is never used in Denali NAND driver, because we are overriding cmdfunc with denali_cmdfunc().

Good catch, this is not required.
Initially I would let user to use other delay value. As we are using own cmdfunc, this is not relevant.

Thanks
Chin Liang

Quoted 7 lines

Best Regards
Masahiro Yamada