diff mbox

[U-Boot,v2,8/8] mtd: vf610_nfc: enable ONFI detection

Message ID 1428504281-30214-9-git-send-email-stefan@agner.ch
State Superseded
Delegated to: Scott Wood
Headers show

Commit Message

Stefan Agner April 8, 2015, 2:44 p.m. UTC 
This changes enable ONFI detection. The Read ID command now allows
one address byte which is needed for ONFI detection. To read the
ONFI parameter page, the NAND_CMD_PARAM need to be supported. The
CMD code enables one command and one address byte along with reading
data from flash using R/B#, as specified by ONFI.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mtd/nand/vf610_nfc.c | 65 ++++++++++++++++++++++++++++++++------------
 include/configs/vf610twr.h   |  1 +
 2 files changed, 48 insertions(+), 18 deletions(-)

Comments

Scott Wood April 20, 2015, 11:17 p.m. UTC | #1 
On Wed, 2015-04-08 at 16:44 +0200, Stefan Agner wrote:
> +	case ALT_BUF_ONFI:
> +		/* Reverse byte since the controller uses big endianness */
> +		c = nfc->column % 4;
> +		c = nfc->column - c + (3 - c);

These two lines can be simplified to "c = nfc->column ^ 3;"

Doesn't this driver run on some big-endian targets, in which case you
wouldn't want to reverse?  I think you should instead be using in_be32()
and then extracting the byte within the word after it's been put into
cpu byte order.

> +		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
> +		break;
> +	default:
> +		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
> +		break;

Why is the byte order different here?  I guess you've been writing data
backwards onto the NAND chip?  Won't that mess up factory bad block
markers?

-Scott
Stefan Agner May 8, 2015, 3:49 p.m. UTC | #2 
On 2015-04-21 01:17, Scott Wood wrote:
> On Wed, 2015-04-08 at 16:44 +0200, Stefan Agner wrote:
>> +	case ALT_BUF_ONFI:
>> +		/* Reverse byte since the controller uses big endianness */
>> +		c = nfc->column % 4;
>> +		c = nfc->column - c + (3 - c);
> 
> These two lines can be simplified to "c = nfc->column ^ 3;"
> 
> Doesn't this driver run on some big-endian targets, in which case you
> wouldn't want to reverse?  I think you should instead be using in_be32()
> and then extracting the byte within the word after it's been put into
> cpu byte order.

Yes agreed, big endian platforms shouldn't be left out here, will fix
that.

>> +		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
>> +		break;
>> +	default:
>> +		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
>> +		break;
> 
> Why is the byte order different here?  I guess you've been writing data
> backwards onto the NAND chip?  Won't that mess up factory bad block
> markers?

Yes, the data end up backward in the NAND chip. I asked that myself to,
however, I have devices which definitely come with "naturally" looking
bad blocks. It seems that the whole pages are actually set to 0x00 on
those chips, although this is not specified in the data sheet. I will
try to figure out more...

--
Stefan
diff mbox

Patch

diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
index 2c02ff5..99457d4 100644
--- a/drivers/mtd/nand/vf610_nfc.c
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -62,6 +62,7 @@ 
  * Briefly these are bitmasks of controller cycles.
  */
 #define READ_PAGE_CMD_CODE		0x7EE0
+#define READ_ONFI_PARAM_CMD_CODE	0x4860
 #define PROGRAM_PAGE_CMD_CODE		0x7FC0
 #define ERASE_CMD_CODE			0x4EC0
 #define READ_ID_CMD_CODE		0x4804
@@ -150,6 +151,7 @@  struct vf610_nfc {
 	int                alt_buf;
 #define ALT_BUF_ID   1
 #define ALT_BUF_STAT 2
+#define ALT_BUF_ONFI 3
 	struct clk        *clk;
 };
 
@@ -390,6 +392,15 @@  static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
 		vf610_nfc_ecc_mode(mtd, ECC_HW_MODE);
 		break;
 
+	case NAND_CMD_PARAM:
+		nfc->alt_buf = ALT_BUF_ONFI;
+		vf610_nfc_transfer_size(nfc->regs, 768);
+		vf610_nfc_send_command(nfc->regs, NAND_CMD_PARAM, READ_ONFI_PARAM_CMD_CODE);
+		vf610_nfc_set_field(mtd, NFC_ROW_ADDR, ROW_ADDR_MASK,
+				    ROW_ADDR_SHIFT, column);
+		vf610_nfc_ecc_mode(mtd, ECC_BYPASS);
+		break;
+
 	case NAND_CMD_ERASE1:
 		vf610_nfc_transfer_size(nfc->regs, 0);
 		vf610_nfc_send_commands(nfc->regs, command,
@@ -399,8 +410,11 @@  static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
 
 	case NAND_CMD_READID:
 		nfc->alt_buf = ALT_BUF_ID;
+		nfc->column = 0;
 		vf610_nfc_transfer_size(nfc->regs, 0);
 		vf610_nfc_send_command(nfc->regs, command, READ_ID_CMD_CODE);
+		vf610_nfc_set_field(mtd, NFC_ROW_ADDR, ROW_ADDR_MASK,
+				    ROW_ADDR_SHIFT, column);
 		break;
 
 	case NAND_CMD_STATUS:
@@ -422,17 +436,11 @@  static void vf610_nfc_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
 	uint c = nfc->column;
 
-	switch (nfc->alt_buf) {
-	case ALT_BUF_ID:
-		*buf = vf610_nfc_get_id(mtd, c);
-		break;
-	case ALT_BUF_STAT:
-		*buf = vf610_nfc_get_status(mtd);
-		break;
-	default:
-		vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, len);
-		break;
-	}
+	/* Alternate buffers are only supported through read_byte */
+	if (nfc->alt_buf)
+		return;
+
+	vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, len);
 
 	nfc->column += len;
 }
@@ -453,8 +461,28 @@  static void vf610_nfc_write_buf(struct mtd_info *mtd, const u_char *buf,
 /* Read byte from NFC buffers */
 static u8 vf610_nfc_read_byte(struct mtd_info *mtd)
 {
+	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
 	u8 tmp;
-	vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
+	uint c = nfc->column;
+
+	switch (nfc->alt_buf) {
+	case ALT_BUF_ID:
+		tmp = vf610_nfc_get_id(mtd, c);
+		break;
+	case ALT_BUF_STAT:
+		tmp = vf610_nfc_get_status(mtd);
+		break;
+	case ALT_BUF_ONFI:
+		/* Reverse byte since the controller uses big endianness */
+		c = nfc->column % 4;
+		c = nfc->column - c + (3 - c);
+		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
+		break;
+	default:
+		tmp = *((u8 *)(nfc->regs + NFC_MAIN_AREA(0) + c));
+		break;
+	}
+	nfc->column++;
 	return tmp;
 }
 
@@ -602,13 +630,11 @@  static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 	mtd->priv = chip;
 	chip->priv = nfc;
 
-	if (cfg.width == 16) {
+	if (cfg.width == 16)
 		chip->options |= NAND_BUSWIDTH_16;
-		vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
-	} else {
-		chip->options &= ~NAND_BUSWIDTH_16;
-		vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
-	}
+
+	/* Use 8-bit mode during initialization */
+	vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
 
 	/* Disable subpage writes as we do not provide ecc->hwctl */
 	chip->options |= NAND_NO_SUBPAGE_WRITE;
@@ -651,6 +677,9 @@  static int vf610_nfc_nand_init(int devnum, void __iomem *addr)
 		goto error;
 	}
 
+	if (cfg.width == 16)
+		vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
+
 	chip->ecc.mode = NAND_ECC_SOFT; /* default */
 
 	/* Single buffer only, max 256 OOB minus ECC status */
diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
index 621aa13..aa31041 100644
--- a/include/configs/vf610twr.h
+++ b/include/configs/vf610twr.h
@@ -48,6 +48,7 @@ 
 /* NAND support */
 #define CONFIG_CMD_NAND
 #define CONFIG_CMD_NAND_TRIMFFS
+#define CONFIG_SYS_NAND_ONFI_DETECTION
 
 #ifdef CONFIG_CMD_NAND
 #define CONFIG_USE_ARCH_MEMCPY