Skip to contentSkip to conversation

zynqmp: nand: Add Nand driver support for zynqmp

PATCH Superseded Siva Durga Prasad Paladugu 2 participants 5 messages Nov 6, 2015 – Nov 10, 2015

Subsystems (4)
Arm (32-bit) Memory technology devices (MTD) Arm/Zynq architecture NAND flash

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Siva Durga Prasad Paladugu

author
#0 lore raw

PATCH v2 1/2 zynqmp: nand: Add Nand driver support for zynqmp

Add nand driver support for zynqmp. The Nand
controller used in ZynqMP is Arasan Nand Flash
controller.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

---
Changes from v1:
- Newly added in series

---
 arch/arm/cpu/armv8/zynqmp/Kconfig           | 3 +++
 arch/arm/include/asm/arch-zynqmp/hardware.h | 2 ++
 configs/xilinx_zynqmp_ep_defconfig          | 1 +
 include/configs/xilinx_zynqmp.h             | 9 +++++++++
 4 files changed, 15 insertions(+)
arch/arm/cpu/armv8/zynqmp/Kconfig +3 -0
index e5a4fdd..161954f 100644
@@ -27,4 +27,7 @@ config SECURE_IOU
 config ZYNQMP_USB
 	bool "Configure ZynqMP USB"
 
+config NAND_ARASAN
+	bool "Configure Arasan Nand"
+
 endif
arch/arm/include/asm/arch-zynqmp/hardware.h +2 -0
index 0f03c24..3c9f08c 100644
@@ -25,6 +25,8 @@
 #define ZYNQ_SDHCI_BASEADDR0	0xFF160000
 #define ZYNQ_SDHCI_BASEADDR1	0xFF170000
 
+#define ARASAN_NAND_BASEADDR	0xFF100000
+
 #define ZYNQMP_SATA_BASEADDR	0xFD0C0000
 
 #define ZYNQMP_USB0_XHCI_BASEADDR	0xFE200000
configs/xilinx_zynqmp_ep_defconfig +1 -0
index 79304c1..43a24a7 100644
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_ARCH_ZYNQMP=y
 CONFIG_ZYNQMP_USB=y
+CONFIG_NAND_ARASAN=y
 CONFIG_SYS_TEXT_BASE=0x8000000
 CONFIG_DEFAULT_DEVICE_TREE="zynqmp-ep108"
 CONFIG_SYS_PROMPT="ZynqMP> "
include/configs/xilinx_zynqmp.h +9 -0
index 36c1100..1f17adb 100644
@@ -101,6 +101,15 @@
 # define CONFIG_CMD_EXT4_WRITE
 #endif
 
+#ifdef CONFIG_NAND_ARASAN
+# define CONFIG_CMD_NAND
+# define CONFIG_CMD_NAND_LOCK_UNLOCK
+# define CONFIG_SYS_MAX_NAND_DEVICE	1
+# define CONFIG_SYS_NAND_SELF_INIT
+# define CONFIG_SYS_NAND_ONFI_DETECTION
+# define CONFIG_MTD_DEVICE
+#endif
+
 /* Miscellaneous configurable options */
 #define CONFIG_SYS_LOAD_ADDR		0x8000000
Signature · 1 line
2.1.1
author
#1 lore raw

PATCH v2 2/2 arasan: nfc: Add initial nand driver support for arasan

Added initial nand driver support for arasan nand flash controller.This supports nand erase,nand read, nand write This uses the hardware ECC for read and write operations ZynqMP uses this driver.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

---
Changes from v1:
- Made some variables global to filescope
wherever applicable
- Used real units of time for timeouts
- Handled proper return values
- Removed arasan_read_byte and merged its
functionality to arasan_nand_read_byte
- Fixed coding style changes
- Removed On Die ECC support code for now
- Removed code ReadId from init.

---
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/arasan_nfc.c | 1173 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1174 insertions(+)
 create mode 100644 drivers/mtd/nand/arasan_nfc.c
drivers/mtd/nand/Makefile +1 -0
index b4e5376..6fb3718 100644
@@ -42,6 +42,7 @@ ifdef NORMAL_DRIVERS
 obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
+obj-$(CONFIG_NAND_ARASAN) += arasan_nfc.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
 obj-$(CONFIG_NAND_DENALI) += denali.o
drivers/mtd/nand/arasan_nfc.c new file +1173 -0

1,173 lines not on this page · raw message

Signature · 1 line
2.1.1
#2 lore raw

PATCH v2 2/2 arasan: nfc: Add initial nand driver support for arasan

Quoted 41 lines · Siva Durga Prasad Paladugu

Added initial nand driver support for arasan nand flash controller.This supports nand erase,nand read, nand write This uses the hardware ECC for read and write operations ZynqMP uses this driver.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

---
Changes from v1:
- Made some variables global to filescope
wherever applicable
- Used real units of time for timeouts
- Handled proper return values
- Removed arasan_read_byte and merged its
functionality to arasan_nand_read_byte
- Fixed coding style changes
- Removed On Die ECC support code for now
- Removed code ReadId from init.

---
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/arasan_nfc.c | 1173 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1174 insertions(+)
 create mode 100644 drivers/mtd/nand/arasan_nfc.c
drivers/mtd/nand/Makefile +1 -0
index b4e5376..6fb3718 100644
@@ -42,6 +42,7 @@ ifdef NORMAL_DRIVERS
 obj-$(CONFIG_NAND_ECC_BCH) += nand_bch.o
 
 obj-$(CONFIG_NAND_ATMEL) += atmel_nand.o
+obj-$(CONFIG_NAND_ARASAN) += arasan_nfc.o
 obj-$(CONFIG_DRIVER_NAND_BFIN) += bfin_nand.o
 obj-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
 obj-$(CONFIG_NAND_DENALI) += denali.o
drivers/mtd/nand/arasan_nfc.c new file +0 -0
new file mode 100644
index 0000000..a697cce

drivers/mtd/nand/arasan_nfc.c line 7

Quoted 7 lines
drivers/mtd/nand/arasan_nfc.c new file +7 -0
@@ -0,0 +1,1173 @@
+/*
+ * Arasan NAND Flash Controller Driver
+ *
+ * Copyright (C) 2014 - 2015 Xilinx, Inc.
+ *
+ * SPDX-License-Identifier:     GPL-2.0
+ */

Usually U-Boot contributions are requested to be "GPL-2.0+". Is there any particular reason why this can't be?

Quoted 40 lines
drivers/mtd/nand/arasan_nfc.c new file +40 -0
+struct arasan_ecc_matrix {
+	u32 pagesize;
+	u8 ecc_codeword_size;
+	u8 eccbits;
+	u8 slcmlc;
+	u16 eccaddr;
+	u16 eccsize;
+};
+
+static const struct arasan_ecc_matrix ecc_matrix[] = {
+	{512, 9, 1, 0, 0x20D, 0x3},
+	{512, 9, 4, 1, 0x209, 0x7},
+	{512, 9, 8, 1, 0x203, 0xD},
+	/*
+	 * 2K byte page
+	 */
+	{2048, 9, 1, 0, 0x834, 0xC},
+	{2048, 9, 4, 1, 0x826, 0x1A},
+	{2048, 9, 8, 1, 0x80c, 0x34},
+	{2048, 9, 12, 1, 0x822, 0x4E},
+	{2048, 9, 16, 1, 0x808, 0x68},
+	{2048, 10, 24, 1, 0x81c, 0x54},
+	/*
+	 * 4K byte page
+	 */
+	{4096, 9, 1, 0, 0x1068, 0x18},
+	{4096, 9, 4, 1, 0x104c, 0x34},
+	{4096, 9, 8, 1, 0x1018, 0x68},
+	{4096, 9, 12, 1, 0x1044, 0x9C},
+	{4096, 9, 16, 1, 0x1010, 0xD0},
+	{4096, 10, 24, 1, 0x1038, 0xA8},
+	/*
+	 * 8K byte page
+	 */
+	{8192, 9, 1, 0, 0x20d0, 0x30},
+	{8192, 9, 4, 1, 0x2098, 0x68},
+	{8192, 9, 8, 1, 0x2030, 0xD0},
+	{8192, 9, 12, 1, 0x2088, 0x138},
+	{8192, 9, 16, 1, 0x2020, 0x1A0},
+	{8192, 24, 10, 1, 0x2070, 0x150},

drivers/mtd/nand/arasan_nfc.c line 203

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+	/*
+	 * 16K byte page
+	 */
+	{16384, 9, 1, 0, 0x4460, 0x60},
+	{16384, 9, 4, 1, 0x43f0, 0xD0},
+	{16384, 9, 8, 1, 0x4320, 0x1A0},
+	{16384, 9, 12, 1, 0x4250, 0x270},
+	{16384, 9, 16, 1, 0x4180, 0x340},
+	{16384, 10, 24, 1, 0x4220, 0x2A0}
+};

That last line of the 8k section looks suspicious with 24, 10 being reversed compared to the others.

drivers/mtd/nand/arasan_nfc.c line 393

Quoted 3 lines
drivers/mtd/nand/arasan_nfc.c new file +3 -0
+static void arasan_nand_fill_tx(const u8 *buf, int len)
+{
+	u32 __iomem *nand = (u32 __iomem *)&arasan_nand_base->buf_dataport;

Unnecessary cast.

drivers/mtd/nand/arasan_nfc.c line 435

Quoted 5 lines
drivers/mtd/nand/arasan_nfc.c new file +5 -0
+static int arasan_nand_write_page_hwecc(struct mtd_info *mtd,
+		struct nand_chip *chip, const u8 *buf, int oob_required)
+{
+	u32 reg_val, i, pktsize, pktnum;
+	const u32 *bufptr = (u32 *)buf;

Cast is missing "const".

drivers/mtd/nand/arasan_nfc.c line 1036

Quoted 8 lines
drivers/mtd/nand/arasan_nfc.c new file +8 -0
+static void arasan_nand_ecc_init(struct mtd_info *mtd)
+{
+	u32 found = 0;
+	u8 bchmodeval = 0;
+	u32 regval, eccpos_start, i;
+	struct nand_chip *nand_chip = mtd->priv;
+
+	for (i = 0; i < ARRAY_SIZE(ecc_matrix);i++) {

Space after ;

drivers/mtd/nand/arasan_nfc.c line 1049

Quoted 13 lines
drivers/mtd/nand/arasan_nfc.c new file +13 -0
+		if ((ecc_matrix[i].pagesize == mtd->writesize) &&
+		    ((1 << ecc_matrix[i].ecc_codeword_size) >=
+		     nand_chip->ecc_step_ds)) {
+			if (ecc_matrix[i].eccbits >=
+			    nand_chip->ecc_strength_ds) {
+				found = i;
+				break;
+			}
+			found = i;
+		}
+	}
+
+	if (found) {

	if (!found)
		return;

...rather than indenting the rest of the function.

Shouldn't there be some sort of warning if you're unable to provide an ECC strength that meets the chips requirement? Or if ECC was not initialized at all?

Quoted 14 lines
drivers/mtd/nand/arasan_nfc.c new file +14 -0
+		regval = ecc_matrix[i].eccaddr | (ecc_matrix[i].eccsize << 16) |
+			 (ecc_matrix[i].slcmlc << 27);
+		writel(regval, &arasan_nand_base->ecc_reg);
+
+		if (ecc_matrix[i].slcmlc) {
+			switch (ecc_matrix[i].eccbits) {
+			case 16:
+				bchmodeval = 0x0;
+				break;
+			case 12:
+				bchmodeval = 0x1;
+				break;
+			case 8:
+				bchmodeval = 0x2;

drivers/mtd/nand/arasan_nfc.c line 1073

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+				break;
+			case 4:
+				bchmodeval = 0x3;
+				break;
+			case 24:
+				bchmodeval = 0x4;
+				break;
+			default:
+				break;
+			}

This is a useless default clause.

Shouldn't there be some sort of error handling there instead of silently using zero?

Or better yet, why isn't bchmodeval a field in struct arasan_ecc_matrix?

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+			regval = readl(&arasan_nand_base->memadr_reg2);
+			regval &= ~ARASAN_NAND_MEM_ADDR2_BCH_MASK;
+			regval |= (bchmodeval <<
+				   ARASAN_NAND_MEM_ADDR2_BCH_SHIFT);
+			writel(regval, &arasan_nand_base->memadr_reg2);
+		}
+
+		nand_oob.eccbytes = ecc_matrix[i].eccsize;
+		eccpos_start = mtd->oobsize - nand_oob.eccbytes;
+

drivers/mtd/nand/arasan_nfc.c line 1093

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+		for (i = 0; i < nand_oob.eccbytes; i++)
+			nand_oob.eccpos[i] = eccpos_start + i;
+
+		nand_oob.oobfree[0].offset = 2;
+		nand_oob.oobfree[0].length = eccpos_start - 2;
+
+		if (ecc_matrix[i].eccbits == 24)
+			nand_chip->ecc.size = 1024;
+		else
+			nand_chip->ecc.size = 512;

Why not just use 1 << ecc_matrix[i].ecc_codeword_size?

Why is ecc_codeword_size stored as a logarithm when the only place that uses it turns it back into a byte count?

Quoted 8 lines
drivers/mtd/nand/arasan_nfc.c new file +8 -0
+		nand_chip->ecc.bytes = ecc_matrix[i].eccsize;
+		nand_chip->ecc.layout = &nand_oob;
+	}
+}
+
+static int arasan_nand_init(struct nand_chip *nand_chip, int devnum)
+{
+	struct arasan_nand_info *nand;

drivers/mtd/nand/arasan_nfc.c line 1112

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+	struct mtd_info *mtd;
+	int err = -1;
+
+	nand = calloc(1, sizeof(struct arasan_nand_info));
+	if (!nand) {
+		printf("%s: failed to allocate\n", __func__);
+		return err;
+	}
+
+	nand->nand_base = (void *)ARASAN_NAND_BASEADDR;

You already have arsan_nand_base with the proper cast. Why not use it?

drivers/mtd/nand/arasan_nfc.c line 1119

Quoted 7 lines
drivers/mtd/nand/arasan_nfc.c new file +7 -0
+	mtd = &nand_info[0];
+	nand_chip->priv = nand;
+	mtd->priv = nand_chip;
+
+	/* Set address of NAND IO lines */
+	nand_chip->IO_ADDR_R = (void *)&arasan_nand_base->buf_dataport;
+	nand_chip->IO_ADDR_W = (void *)&arasan_nand_base->buf_dataport;

Where do these get used?

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+	/* Set the driver entry points for MTD */
+	nand_chip->cmdfunc = arasan_nand_cmd_function;
+	nand_chip->select_chip = arasan_nand_select_chip;
+	nand_chip->read_byte = arasan_nand_read_byte;
+
+	/* Buffer read/write routines */
+	nand_chip->read_buf = arasan_nand_read_buf;
+	nand_chip->write_buf = arasan_nand_write_buf;
+	nand_chip->bbt_options = NAND_BBT_USE_FLASH;
+

drivers/mtd/nand/arasan_nfc.c line 1140

Quoted 10 lines
drivers/mtd/nand/arasan_nfc.c new file +10 -0
+	writel(0x0, &arasan_nand_base->cmd_reg);
+	writel(0x0, &arasan_nand_base->pgm_reg);
+
+	/* first scan to find the device and get the page size */
+	if (nand_scan_ident(mtd, 1, NULL)) {
+		printf("%s: nand_scan_ident failed\n", __func__);
+		goto fail;
+	}
+
+	mtd->size = nand_chip->chipsize;

nand_scan_ident() already takes care of setting mtd->size.

drivers/mtd/nand/arasan_nfc.c line 1148

Quoted 8 lines
drivers/mtd/nand/arasan_nfc.c new file +8 -0
+
+	nand_chip->ecc.mode = NAND_ECC_HW;
+	nand_chip->ecc.strength = 1;
+	nand_chip->ecc.hwctl = NULL;
+	nand_chip->ecc.read_page = arasan_nand_read_page_hwecc;
+	nand_chip->ecc.write_page = arasan_nand_write_page_hwecc;
+	nand_chip->ecc.read_oob = arasan_nand_read_oob;
+	nand_chip->ecc.write_oob = arasan_nand_write_oob;

None of this needs to be after nand_scan_ident(). Why not keep it together with the other nand_chip init?

Why is ecc.strength being set to 1 when other parts of the code make it look like stronger ECC is supported?

-Scott

#3 lore raw

PATCH v2 1/2 zynqmp: nand: Add Nand driver support for zynqmp

In reply to Siva Durga Prasad Paladugu
Quoted 21 lines · Siva Durga Prasad Paladugu

Add nand driver support for zynqmp. The Nand
controller used in ZynqMP is Arasan Nand Flash
controller.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

---
Changes from v1:
- Newly added in series

---
 arch/arm/cpu/armv8/zynqmp/Kconfig           | 3 +++
 arch/arm/include/asm/arch-zynqmp/hardware.h | 2 ++
 configs/xilinx_zynqmp_ep_defconfig          | 1 +
 include/configs/xilinx_zynqmp.h             | 9 +++++++++
 4 files changed, 15 insertions(+)
arch/arm/cpu/armv8/zynqmp/Kconfig +0 -0
index e5a4fdd..161954f 100644

arch/arm/cpu/armv8/zynqmp/Kconfig config SECURE_IOU · line 33

Quoted 7 lines
arch/arm/cpu/armv8/zynqmp/Kconfig +3 -0
@@ -27,4 +27,7 @@ config SECURE_IOU
 config ZYNQMP_USB
      bool "Configure ZynqMP USB"
 
+config NAND_ARASAN
+     bool "Configure Arasan Nand"
+
 endif

This should go in drivers/mtd/nand/Kconfig.

-Scott

author
#4 lore raw

PATCH v2 1/2 zynqmp: nand: Add Nand driver support for zynqmp

Hi Scott,

Quoted 32 lines

-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com]
Sent: Friday, November 06, 2015 10:33 PM
To: Siva Durga Prasad Paladugu
Cc: u-boot at lists.denx.de; Michal Simek; Siva Durga Prasad Paladugu
Subject: Re: [UBOOT PATCH v2 1/2] zynqmp: nand: Add Nand driver support for zynqmp

Quoted 23 lines · Siva Durga Prasad Paladugu

Add nand driver support for zynqmp. The Nand controller used in ZynqMP is Arasan Nand Flash controller.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

---
Changes from v1:
- Newly added in series

---
 arch/arm/cpu/armv8/zynqmp/Kconfig           | 3 +++
 arch/arm/include/asm/arch-zynqmp/hardware.h | 2 ++
 configs/xilinx_zynqmp_ep_defconfig          | 1 +
 include/configs/xilinx_zynqmp.h             | 9 +++++++++
 4 files changed, 15 insertions(+)
arch/arm/cpu/armv8/zynqmp/Kconfig +0 -0
index e5a4fdd..161954f 100644
@@ -27,4 +27,7 @@ config SECURE_IOU
 config ZYNQMP_USB
      bool "Configure ZynqMP USB"
Quoted 6 lines

arch/arm/cpu/armv8/zynqmp/Kconfig config SECURE_IOU · line 33

Quoted 4 lines
arch/arm/cpu/armv8/zynqmp/Kconfig +3 -0
+config NAND_ARASAN
+     bool "Configure Arasan Nand"
+
 endif

This should go in drivers/mtd/nand/Kconfig.

Ok, I will consider it in V2.
Do you have any further comments on 2nd patch in the series? If not, please take it up.

Regards,
Siva

Quoted 2 lines

-Scott