* [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
2015-06-02 1:32 ` Brian Norris
` (2 more replies)
2015-03-25 16:28 ` [PATCH v4 2/6] mtd: nand: vf610_nfc: add hardware BCH-ECC support Stefan Agner
` (4 subsequent siblings)
5 siblings, 3 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
This driver supports Freescale NFC (NAND flash controller) found on
Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
Limitations:
- DMA and pipelining not used
- Pages larger than 2k are not supported
- No hardware ECC
The driver has only been tested on Vybrid (VF610).
Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
MAINTAINERS | 6 +
drivers/mtd/nand/Kconfig | 12 +
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/vf610_nfc.c | 686 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 705 insertions(+)
create mode 100644 drivers/mtd/nand/vf610_nfc.c
diff --git a/MAINTAINERS b/MAINTAINERS
index eaf9996..d8cbaee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10363,6 +10363,12 @@ S: Maintained
F: Documentation/fb/uvesafb.txt
F: drivers/video/fbdev/uvesafb.*
+VF610 NAND DRIVER
+M: Stefan Agner <stefan@agner.ch>
+L: linux-mtd@lists.infradead.org
+S: Supported
+F: drivers/mtd/nand/vf610_nfc.c
+
VFAT/FAT/MSDOS FILESYSTEM
M: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
S: Maintained
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a17..d0badda 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
This enables the driver for the NAND flash controller on the
MPC5121 SoC.
+config HAVE_NAND_VF610_NFC
+ bool
+
+config MTD_NAND_VF610_NFC
+ tristate "Support for Freescale NFC for VF610/MPC5125"
+ depends on (HAVE_NAND_VF610_NFC || COMPILE_TEST)
+ help
+ Enables support for NAND Flash Controller on some Freescale
+ processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+ The driver supports a maximum 2k page size. The driver
+ currently does not support hardware ECC.
+
config MTD_NAND_MXC
tristate "MXC NAND support"
depends on ARCH_MXC
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 582bbd05..e97ca7b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES) += socrates_nand.o
obj-$(CONFIG_MTD_NAND_TXX9NDFMC) += txx9ndfmc.o
obj-$(CONFIG_MTD_NAND_NUC900) += nuc900_nand.o
obj-$(CONFIG_MTD_NAND_MPC5121_NFC) += mpc5121_nfc.o
+obj-$(CONFIG_MTD_NAND_VF610_NFC) += vf610_nfc.o
obj-$(CONFIG_MTD_NAND_RICOH) += r852.o
obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
new file mode 100644
index 0000000..23c1510
--- /dev/null
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -0,0 +1,686 @@
+/*
+ * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
+ *
+ * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
+ * Jason ported to M54418TWR and MVFA5 (VF610).
+ * Authors: Stefan Agner <stefan.agner@toradex.com>
+ * Bill Pringlemeir <bpringlemeir@nbsps.com>
+ * Shaohui Xie <b21989@freescale.com>
+ * Jason Jin <Jason.jin@freescale.com>
+ *
+ * Based on original driver mpc5121_nfc.c.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Limitations:
+ * - Untested on MPC5125 and M54418.
+ * - DMA not used.
+ * - 2K pages or less.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of_mtd.h>
+
+#define DRV_NAME "vf610_nfc"
+
+/* Register Offsets */
+#define NFC_FLASH_CMD1 0x3F00
+#define NFC_FLASH_CMD2 0x3F04
+#define NFC_COL_ADDR 0x3F08
+#define NFC_ROW_ADDR 0x3F0c
+#define NFC_ROW_ADDR_INC 0x3F14
+#define NFC_FLASH_STATUS1 0x3F18
+#define NFC_FLASH_STATUS2 0x3F1c
+#define NFC_CACHE_SWAP 0x3F28
+#define NFC_SECTOR_SIZE 0x3F2c
+#define NFC_FLASH_CONFIG 0x3F30
+#define NFC_IRQ_STATUS 0x3F38
+
+/* Addresses for NFC MAIN RAM BUFFER areas */
+#define NFC_MAIN_AREA(n) ((n) * 0x1000)
+
+#define PAGE_2K 0x0800
+#define OOB_64 0x0040
+
+/*
+ * NFC_CMD2[CODE] values. See section:
+ * - 31.4.7 Flash Command Code Description, Vybrid manual
+ * - 23.8.6 Flash Command Sequencer, MPC5125 manual
+ *
+ * Briefly these are bitmasks of controller cycles.
+ */
+#define READ_PAGE_CMD_CODE 0x7EE0
+#define PROGRAM_PAGE_CMD_CODE 0x7FC0
+#define ERASE_CMD_CODE 0x4EC0
+#define READ_ID_CMD_CODE 0x4804
+#define RESET_CMD_CODE 0x4040
+#define STATUS_READ_CMD_CODE 0x4068
+
+/* NFC ECC mode define */
+#define ECC_BYPASS 0
+
+/*** Register Mask and bit definitions */
+
+/* NFC_FLASH_CMD1 Field */
+#define CMD_BYTE2_MASK 0xFF000000
+#define CMD_BYTE2_SHIFT 24
+
+/* NFC_FLASH_CM2 Field */
+#define CMD_BYTE1_MASK 0xFF000000
+#define CMD_BYTE1_SHIFT 24
+#define CMD_CODE_MASK 0x00FFFF00
+#define CMD_CODE_SHIFT 8
+#define BUFNO_MASK 0x00000006
+#define BUFNO_SHIFT 1
+#define START_BIT (1<<0)
+
+/* NFC_COL_ADDR Field */
+#define COL_ADDR_MASK 0x0000FFFF
+#define COL_ADDR_SHIFT 0
+
+/* NFC_ROW_ADDR Field */
+#define ROW_ADDR_MASK 0x00FFFFFF
+#define ROW_ADDR_SHIFT 0
+#define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000
+#define ROW_ADDR_CHIP_SEL_RB_SHIFT 28
+#define ROW_ADDR_CHIP_SEL_MASK 0x0F000000
+#define ROW_ADDR_CHIP_SEL_SHIFT 24
+
+/* NFC_FLASH_STATUS2 Field */
+#define STATUS_BYTE1_MASK 0x000000FF
+
+/* NFC_FLASH_CONFIG Field */
+#define CONFIG_ECC_SRAM_REQ_BIT (1<<21)
+#define CONFIG_DMA_REQ_BIT (1<<20)
+#define CONFIG_ECC_MODE_MASK 0x000E0000
+#define CONFIG_ECC_MODE_SHIFT 17
+#define CONFIG_FAST_FLASH_BIT (1<<16)
+#define CONFIG_16BIT (1<<7)
+#define CONFIG_BOOT_MODE_BIT (1<<6)
+#define CONFIG_ADDR_AUTO_INCR_BIT (1<<5)
+#define CONFIG_BUFNO_AUTO_INCR_BIT (1<<4)
+#define CONFIG_PAGE_CNT_MASK 0xF
+#define CONFIG_PAGE_CNT_SHIFT 0
+
+/* NFC_IRQ_STATUS Field */
+#define IDLE_IRQ_BIT (1<<29)
+#define IDLE_EN_BIT (1<<20)
+#define CMD_DONE_CLEAR_BIT (1<<18)
+#define IDLE_CLEAR_BIT (1<<17)
+
+#define NFC_TIMEOUT (HZ)
+
+struct vf610_nfc_config {
+ int width;
+ int flash_bbt;
+};
+
+struct vf610_nfc {
+ struct mtd_info mtd;
+ struct nand_chip chip;
+ struct device *dev;
+ void __iomem *regs;
+ wait_queue_head_t irq_waitq;
+ uint column;
+ int spareonly;
+ int page_sz;
+ int page;
+ /* Status and ID are in alternate locations. */
+ int alt_buf;
+#define ALT_BUF_ID 1
+#define ALT_BUF_STAT 2
+ struct clk *clk;
+
+ struct vf610_nfc_config cfg;
+};
+
+#define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
+
+static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
+{
+ return readl(nfc->regs + reg);
+}
+
+static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
+{
+ writel(val, nfc->regs + reg);
+}
+
+static inline void vf610_nfc_set(struct vf610_nfc *nfc, uint reg, u32 bits)
+{
+ vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) | bits);
+}
+
+static inline void vf610_nfc_clear(struct vf610_nfc *nfc, uint reg, u32 bits)
+{
+ vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) & ~bits);
+}
+
+static inline void vf610_nfc_set_field(struct vf610_nfc *nfc, u32 reg,
+ u32 mask, u32 shift, u32 val)
+{
+ vf610_nfc_write(nfc, reg,
+ (vf610_nfc_read(nfc, reg) & (~mask)) | val << shift);
+}
+
+static inline void vf610_nfc_memcpy(void *dst, const void *src, size_t n)
+{
+ /*
+ * Use this accessor for the interal SRAM buffers. On ARM we can
+ * treat the SRAM buffer as if its memory, hence use memcpy
+ */
+ memcpy(dst, src, n);
+}
+
+/* Clear flags for upcoming command */
+static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc)
+{
+ void __iomem *reg = nfc->regs + NFC_IRQ_STATUS;
+ u32 tmp = __raw_readl(reg);
+
+ tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
+ __raw_writel(tmp, reg);
+}
+
+static inline void vf610_nfc_done(struct vf610_nfc *nfc)
+{
+ int rv;
+
+ /*
+ * Barrier is needed after this write. This write need
+ * to be done before reading the next register the first
+ * time.
+ * vf610_nfc_set implicates such a barrier by using writel
+ * to write to the register.
+ */
+ vf610_nfc_set(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
+ vf610_nfc_set(nfc, NFC_FLASH_CMD2, START_BIT);
+
+ if (!(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT)) {
+ rv = wait_event_timeout(nfc->irq_waitq,
+ (vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT),
+ NFC_TIMEOUT);
+ if (!rv)
+ dev_warn(nfc->dev, "Timeout while waiting for BUSY.\n");
+ }
+ vf610_nfc_clear_status(nfc);
+}
+
+static u8 vf610_nfc_get_id(struct vf610_nfc *nfc, int col)
+{
+ u32 flash_id;
+
+ if (col < 4) {
+ flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS1);
+ return (flash_id >> (3-col)*8) & 0xff;
+ } else {
+ flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS2);
+ return flash_id >> 24;
+ }
+}
+
+static u8 vf610_nfc_get_status(struct vf610_nfc *nfc)
+{
+ return vf610_nfc_read(nfc, NFC_FLASH_STATUS2) & STATUS_BYTE1_MASK;
+}
+
+static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
+ u32 cmd_code)
+{
+ void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
+ u32 tmp;
+
+ vf610_nfc_clear_status(nfc);
+
+ tmp = __raw_readl(reg);
+ tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
+ tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
+ tmp |= cmd_code << CMD_CODE_SHIFT;
+ __raw_writel(tmp, reg);
+}
+
+static void vf610_nfc_send_commands(struct vf610_nfc *nfc, u32 cmd_byte1,
+ u32 cmd_byte2, u32 cmd_code)
+{
+ void __iomem *reg = nfc->regs + NFC_FLASH_CMD1;
+ u32 tmp;
+
+ vf610_nfc_send_command(nfc, cmd_byte1, cmd_code);
+
+ tmp = __raw_readl(reg);
+ tmp &= ~CMD_BYTE2_MASK;
+ tmp |= cmd_byte2 << CMD_BYTE2_SHIFT;
+ __raw_writel(tmp, reg);
+}
+
+static irqreturn_t vf610_nfc_irq(int irq, void *data)
+{
+ struct mtd_info *mtd = data;
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ vf610_nfc_clear(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
+ wake_up(&nfc->irq_waitq);
+
+ return IRQ_HANDLED;
+}
+
+static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
+{
+ if (column != -1) {
+ if (nfc->chip.options | NAND_BUSWIDTH_16)
+ column = column/2;
+ vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
+ COL_ADDR_SHIFT, column);
+ }
+ if (page != -1)
+ vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
+ ROW_ADDR_SHIFT, page);
+}
+
+static inline void vf610_nfc_transfer_size(void __iomem *regbase, int size)
+{
+ __raw_writel(size, regbase + NFC_SECTOR_SIZE);
+}
+
+static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
+ int column, int page)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ nfc->column = max(column, 0);
+ nfc->spareonly = 0;
+ nfc->alt_buf = 0;
+
+ switch (command) {
+ case NAND_CMD_PAGEPROG:
+ nfc->page = -1;
+ vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
+ vf610_nfc_send_commands(nfc, NAND_CMD_SEQIN,
+ command, PROGRAM_PAGE_CMD_CODE);
+ vf610_nfc_addr_cycle(nfc, column, page);
+ break;
+
+ case NAND_CMD_RESET:
+ vf610_nfc_transfer_size(nfc->regs, 0);
+ vf610_nfc_send_command(nfc, command, RESET_CMD_CODE);
+ break;
+ /*
+ * NFC does not support sub-page reads and writes,
+ * so emulate them using full page transfers.
+ */
+ case NAND_CMD_READOOB:
+ nfc->spareonly = 1;
+ case NAND_CMD_SEQIN: /* Pre-read for partial writes. */
+ case NAND_CMD_READ0:
+ column = 0;
+ /* Already read? */
+ if (nfc->page == page)
+ return;
+ nfc->page = page;
+ vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
+ vf610_nfc_send_commands(nfc, NAND_CMD_READ0,
+ NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
+ vf610_nfc_addr_cycle(nfc, column, page);
+ break;
+
+ case NAND_CMD_ERASE1:
+ nfc->page = -1;
+ vf610_nfc_transfer_size(nfc->regs, 0);
+ vf610_nfc_send_commands(nfc, command,
+ NAND_CMD_ERASE2, ERASE_CMD_CODE);
+ vf610_nfc_addr_cycle(nfc, column, page);
+ break;
+
+ case NAND_CMD_READID:
+ nfc->alt_buf = ALT_BUF_ID;
+ vf610_nfc_transfer_size(nfc->regs, 0);
+ vf610_nfc_send_command(nfc, command, READ_ID_CMD_CODE);
+ break;
+
+ case NAND_CMD_STATUS:
+ nfc->alt_buf = ALT_BUF_STAT;
+ vf610_nfc_transfer_size(nfc->regs, 0);
+ vf610_nfc_send_command(nfc, command, STATUS_READ_CMD_CODE);
+ break;
+ default:
+ return;
+ }
+
+ vf610_nfc_done(nfc);
+}
+
+static inline void vf610_nfc_read_spare(struct mtd_info *mtd, void *buf,
+ int len)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ len = min_t(uint, mtd->oobsize, len);
+ if (len > 0)
+ vf610_nfc_memcpy(buf, nfc->regs + mtd->writesize, len);
+}
+
+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;
+ uint l;
+
+ /* Handle main area */
+ if (!nfc->spareonly) {
+ l = min_t(uint, len, mtd->writesize - c);
+ nfc->column += l;
+
+ if (!nfc->alt_buf)
+ vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, l);
+ else
+ if (nfc->alt_buf & ALT_BUF_ID)
+ *buf = vf610_nfc_get_id(nfc, c);
+ else
+ *buf = vf610_nfc_get_status(nfc);
+
+ buf += l;
+ len -= l;
+ }
+
+ /* Handle spare area access */
+ if (len) {
+ nfc->column += len;
+ vf610_nfc_read_spare(mtd, buf, len);
+ }
+}
+
+static void vf610_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+ uint c = nfc->column;
+ uint l;
+
+ l = min_t(uint, len, mtd->writesize + mtd->oobsize - c);
+ nfc->column += l;
+ vf610_nfc_memcpy(nfc->regs + NFC_MAIN_AREA(0) + c, buf, l);
+}
+
+static uint8_t vf610_nfc_read_byte(struct mtd_info *mtd)
+{
+ u8 tmp;
+
+ vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
+ return tmp;
+}
+
+static u16 vf610_nfc_read_word(struct mtd_info *mtd)
+{
+ u16 tmp;
+
+ vf610_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
+ return tmp;
+}
+
+/* If not provided, upper layers apply a fixed delay. */
+static int vf610_nfc_dev_ready(struct mtd_info *mtd)
+{
+ /* NFC handles R/B internally; always ready. */
+ return 1;
+}
+
+/*
+ * This function supports Vybrid only (MPC5125 would have full RB and four CS)
+ */
+static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
+{
+#ifdef CONFIG_SOC_VF610
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+ u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
+
+ tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
+ tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
+
+ if (chip == 0)
+ tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
+ else if (chip == 1)
+ tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;
+
+ vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
+#endif
+}
+
+#ifdef CONFIG_OF_MTD
+static const struct of_device_id vf610_nfc_dt_ids[] = {
+ { .compatible = "fsl,vf610-nfc" },
+ { .compatible = "fsl,mpc5125-nfc" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vf610_nfc_dt_ids);
+
+static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
+{
+ struct device_node *np = dev->of_node;
+ int buswidth;
+
+ if (!np)
+ return 1;
+
+ cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
+
+ buswidth = of_get_nand_bus_width(np);
+ if (buswidth < 0)
+ return buswidth;
+
+ cfg->width = buswidth;
+
+ return 0;
+}
+#else
+static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
+{
+ return 0;
+}
+#endif
+
+static int vf610_nfc_init_controller(struct vf610_nfc *nfc)
+{
+ struct vf610_nfc_config *cfg = &nfc->cfg;
+
+ if (cfg->width == 16)
+ vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
+ else
+ vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
+
+ /* Set configuration register. */
+ vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
+ vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
+ vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
+ vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
+ vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
+
+ /* PAGE_CNT = 1 */
+ vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
+ CONFIG_PAGE_CNT_SHIFT, 1);
+
+ return 0;
+}
+
+static int vf610_nfc_probe(struct platform_device *pdev)
+{
+ struct vf610_nfc *nfc;
+ struct resource *res;
+ struct mtd_info *mtd;
+ struct nand_chip *chip;
+ struct vf610_nfc_config *cfg;
+ int err = 0;
+ int irq;
+
+ nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
+ if (!nfc)
+ return -ENOMEM;
+
+ cfg = &nfc->cfg;
+
+ nfc->dev = &pdev->dev;
+ nfc->page = -1;
+ mtd = &nfc->mtd;
+ chip = &nfc->chip;
+
+ mtd->priv = chip;
+ mtd->owner = THIS_MODULE;
+ mtd->dev.parent = nfc->dev;
+ mtd->name = DRV_NAME;
+
+ err = vf610_nfc_probe_dt(nfc->dev, cfg);
+ if (err)
+ return -ENODEV;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq <= 0)
+ return -EINVAL;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ nfc->regs = devm_ioremap_resource(nfc->dev, res);
+ if (IS_ERR(nfc->regs))
+ return PTR_ERR(nfc->regs);
+
+ nfc->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(nfc->clk))
+ return PTR_ERR(nfc->clk);
+
+ err = clk_prepare_enable(nfc->clk);
+ if (err) {
+ dev_err(nfc->dev, "Unable to enable clock!\n");
+ return err;
+ }
+
+ if (cfg->width == 16)
+ chip->options |= NAND_BUSWIDTH_16;
+ else
+ chip->options &= ~NAND_BUSWIDTH_16;
+
+ chip->dev_ready = vf610_nfc_dev_ready;
+ chip->cmdfunc = vf610_nfc_command;
+ chip->read_byte = vf610_nfc_read_byte;
+ chip->read_word = vf610_nfc_read_word;
+ chip->read_buf = vf610_nfc_read_buf;
+ chip->write_buf = vf610_nfc_write_buf;
+ chip->select_chip = vf610_nfc_select_chip;
+
+ /* Bad block options. */
+ if (cfg->flash_bbt)
+ chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB |
+ NAND_BBT_CREATE;
+
+ init_waitqueue_head(&nfc->irq_waitq);
+
+ err = devm_request_irq(nfc->dev, irq, vf610_nfc_irq, 0, DRV_NAME, mtd);
+ if (err) {
+ dev_err(nfc->dev, "Error requesting IRQ!\n");
+ goto error;
+ }
+
+ nfc->page_sz = PAGE_2K + OOB_64;
+ nfc->page_sz += cfg->width == 16 ? 1 : 0;
+
+ vf610_nfc_init_controller(nfc);
+
+ /* first scan to find the device and get the page size */
+ if (nand_scan_ident(mtd, 1, NULL)) {
+ err = -ENXIO;
+ goto error;
+ }
+
+ chip->ecc.mode = NAND_ECC_SOFT; /* default */
+
+ nfc->page_sz = mtd->writesize + mtd->oobsize;
+
+ /* Single buffer only, max 256 OOB minus ECC status */
+ if (nfc->page_sz > PAGE_2K + 256 - 8) {
+ dev_err(nfc->dev, "Unsupported flash page size\n");
+ err = -ENXIO;
+ goto error;
+ }
+ nfc->page_sz += cfg->width == 16 ? 1 : 0;
+
+ /* second phase scan */
+ if (nand_scan_tail(mtd)) {
+ err = -ENXIO;
+ goto error;
+ }
+
+ /* Register device in MTD */
+ mtd_device_parse_register(mtd, NULL,
+ &(struct mtd_part_parser_data){
+ .of_node = pdev->dev.of_node,
+ },
+ NULL, 0);
+
+ platform_set_drvdata(pdev, mtd);
+
+ return 0;
+
+error:
+ clk_disable_unprepare(nfc->clk);
+ return err;
+}
+
+static int vf610_nfc_remove(struct platform_device *pdev)
+{
+ struct mtd_info *mtd = platform_get_drvdata(pdev);
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ nand_release(mtd);
+ clk_disable_unprepare(nfc->clk);
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int vf610_nfc_suspend(struct device *dev)
+{
+ struct mtd_info *mtd = dev_get_drvdata(dev);
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ clk_disable_unprepare(nfc->clk);
+ return 0;
+}
+
+static int vf610_nfc_resume(struct device *dev)
+{
+ struct mtd_info *mtd = dev_get_drvdata(dev);
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ pinctrl_pm_select_default_state(dev);
+
+ clk_prepare_enable(nfc->clk);
+
+ vf610_nfc_init_controller(nfc);
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(vf610_nfc_pm_ops, vf610_nfc_suspend, vf610_nfc_resume);
+
+static struct platform_driver vf610_nfc_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = vf610_nfc_dt_ids,
+ .pm = &vf610_nfc_pm_ops,
+ },
+ .probe = vf610_nfc_probe,
+ .remove = vf610_nfc_remove,
+};
+
+module_platform_driver(vf610_nfc_driver);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("Freescale VF610/MPC5125 NFC MTD NAND driver");
+MODULE_LICENSE("GPL");
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-03-25 16:28 ` [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Stefan Agner
@ 2015-06-02 1:32 ` Brian Norris
2015-06-02 9:01 ` Stefan Agner
2015-06-03 8:10 ` Sebastian Andrzej Siewior
2015-06-03 13:08 ` Sebastian Andrzej Siewior
2 siblings, 1 reply; 21+ messages in thread
From: Brian Norris @ 2015-06-02 1:32 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel
Hi Stefan,
Late review...
On Wed, Mar 25, 2015 at 05:28:24PM +0100, Stefan Agner wrote:
> This driver supports Freescale NFC (NAND flash controller) found on
> Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
>
> Limitations:
> - DMA and pipelining not used
> - Pages larger than 2k are not supported
> - No hardware ECC
>
> The driver has only been tested on Vybrid (VF610).
>
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> MAINTAINERS | 6 +
> drivers/mtd/nand/Kconfig | 12 +
> drivers/mtd/nand/Makefile | 1 +
> drivers/mtd/nand/vf610_nfc.c | 686 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 705 insertions(+)
> create mode 100644 drivers/mtd/nand/vf610_nfc.c
>
...
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 0000000..23c1510
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,686 @@
> +/*
> + * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
> + *
> + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
> + * Jason ported to M54418TWR and MVFA5 (VF610).
> + * Authors: Stefan Agner <stefan.agner@toradex.com>
> + * Bill Pringlemeir <bpringlemeir@nbsps.com>
> + * Shaohui Xie <b21989@freescale.com>
> + * Jason Jin <Jason.jin@freescale.com>
> + *
> + * Based on original driver mpc5121_nfc.c.
It's not supremely obvious to me: what's the relationship between these
drivers? How similar is the IP, if at all?
> + *
> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Limitations:
> + * - Untested on MPC5125 and M54418.
> + * - DMA not used.
> + * - 2K pages or less.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of_mtd.h>
> +
> +#define DRV_NAME "vf610_nfc"
> +
> +/* Register Offsets */
> +#define NFC_FLASH_CMD1 0x3F00
> +#define NFC_FLASH_CMD2 0x3F04
> +#define NFC_COL_ADDR 0x3F08
> +#define NFC_ROW_ADDR 0x3F0c
> +#define NFC_ROW_ADDR_INC 0x3F14
> +#define NFC_FLASH_STATUS1 0x3F18
> +#define NFC_FLASH_STATUS2 0x3F1c
> +#define NFC_CACHE_SWAP 0x3F28
> +#define NFC_SECTOR_SIZE 0x3F2c
> +#define NFC_FLASH_CONFIG 0x3F30
> +#define NFC_IRQ_STATUS 0x3F38
> +
> +/* Addresses for NFC MAIN RAM BUFFER areas */
> +#define NFC_MAIN_AREA(n) ((n) * 0x1000)
> +
> +#define PAGE_2K 0x0800
> +#define OOB_64 0x0040
> +
> +/*
> + * NFC_CMD2[CODE] values. See section:
> + * - 31.4.7 Flash Command Code Description, Vybrid manual
> + * - 23.8.6 Flash Command Sequencer, MPC5125 manual
> + *
> + * Briefly these are bitmasks of controller cycles.
> + */
> +#define READ_PAGE_CMD_CODE 0x7EE0
> +#define PROGRAM_PAGE_CMD_CODE 0x7FC0
> +#define ERASE_CMD_CODE 0x4EC0
> +#define READ_ID_CMD_CODE 0x4804
> +#define RESET_CMD_CODE 0x4040
> +#define STATUS_READ_CMD_CODE 0x4068
> +
> +/* NFC ECC mode define */
> +#define ECC_BYPASS 0
> +
> +/*** Register Mask and bit definitions */
> +
> +/* NFC_FLASH_CMD1 Field */
> +#define CMD_BYTE2_MASK 0xFF000000
> +#define CMD_BYTE2_SHIFT 24
> +
> +/* NFC_FLASH_CM2 Field */
> +#define CMD_BYTE1_MASK 0xFF000000
> +#define CMD_BYTE1_SHIFT 24
> +#define CMD_CODE_MASK 0x00FFFF00
> +#define CMD_CODE_SHIFT 8
> +#define BUFNO_MASK 0x00000006
> +#define BUFNO_SHIFT 1
> +#define START_BIT (1<<0)
> +
> +/* NFC_COL_ADDR Field */
> +#define COL_ADDR_MASK 0x0000FFFF
> +#define COL_ADDR_SHIFT 0
> +
> +/* NFC_ROW_ADDR Field */
> +#define ROW_ADDR_MASK 0x00FFFFFF
> +#define ROW_ADDR_SHIFT 0
> +#define ROW_ADDR_CHIP_SEL_RB_MASK 0xF0000000
> +#define ROW_ADDR_CHIP_SEL_RB_SHIFT 28
> +#define ROW_ADDR_CHIP_SEL_MASK 0x0F000000
> +#define ROW_ADDR_CHIP_SEL_SHIFT 24
> +
> +/* NFC_FLASH_STATUS2 Field */
> +#define STATUS_BYTE1_MASK 0x000000FF
> +
> +/* NFC_FLASH_CONFIG Field */
> +#define CONFIG_ECC_SRAM_REQ_BIT (1<<21)
> +#define CONFIG_DMA_REQ_BIT (1<<20)
> +#define CONFIG_ECC_MODE_MASK 0x000E0000
> +#define CONFIG_ECC_MODE_SHIFT 17
> +#define CONFIG_FAST_FLASH_BIT (1<<16)
> +#define CONFIG_16BIT (1<<7)
> +#define CONFIG_BOOT_MODE_BIT (1<<6)
> +#define CONFIG_ADDR_AUTO_INCR_BIT (1<<5)
> +#define CONFIG_BUFNO_AUTO_INCR_BIT (1<<4)
> +#define CONFIG_PAGE_CNT_MASK 0xF
> +#define CONFIG_PAGE_CNT_SHIFT 0
> +
> +/* NFC_IRQ_STATUS Field */
> +#define IDLE_IRQ_BIT (1<<29)
> +#define IDLE_EN_BIT (1<<20)
> +#define CMD_DONE_CLEAR_BIT (1<<18)
> +#define IDLE_CLEAR_BIT (1<<17)
> +
> +#define NFC_TIMEOUT (HZ)
> +
> +struct vf610_nfc_config {
> + int width;
> + int flash_bbt;
> +};
> +
> +struct vf610_nfc {
> + struct mtd_info mtd;
> + struct nand_chip chip;
> + struct device *dev;
> + void __iomem *regs;
> + wait_queue_head_t irq_waitq;
> + uint column;
> + int spareonly;
> + int page_sz;
> + int page;
> + /* Status and ID are in alternate locations. */
> + int alt_buf;
> +#define ALT_BUF_ID 1
> +#define ALT_BUF_STAT 2
> + struct clk *clk;
> +
> + struct vf610_nfc_config cfg;
> +};
> +
> +#define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
> +
> +static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
> +{
> + return readl(nfc->regs + reg);
> +}
> +
> +static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
> +{
> + writel(val, nfc->regs + reg);
> +}
> +
> +static inline void vf610_nfc_set(struct vf610_nfc *nfc, uint reg, u32 bits)
> +{
> + vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) | bits);
> +}
> +
> +static inline void vf610_nfc_clear(struct vf610_nfc *nfc, uint reg, u32 bits)
> +{
> + vf610_nfc_write(nfc, reg, vf610_nfc_read(nfc, reg) & ~bits);
> +}
> +
> +static inline void vf610_nfc_set_field(struct vf610_nfc *nfc, u32 reg,
> + u32 mask, u32 shift, u32 val)
> +{
> + vf610_nfc_write(nfc, reg,
> + (vf610_nfc_read(nfc, reg) & (~mask)) | val << shift);
> +}
> +
> +static inline void vf610_nfc_memcpy(void *dst, const void *src, size_t n)
> +{
> + /*
> + * Use this accessor for the interal SRAM buffers. On ARM we can
> + * treat the SRAM buffer as if its memory, hence use memcpy
s/its/it's/
or
s/its/it is/
> + */
> + memcpy(dst, src, n);
I'm not clear on all the reasoning in your comment. Is this really
specific to ARM, that you can treat SRAM like memory? If so, should this
driver have some dependency on ARM?
Also, if you're really trying to handle iomem, it makes sense to
annotate the 2nd argument with __iomem. Then sparse will only complain
on the memcpy(), which has a proper comment explanation, rather than on
every call site for vf610_nfc_memcpy(). i.e.:
static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src,
size_t n)
> +}
> +
> +/* Clear flags for upcoming command */
> +static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc)
> +{
> + void __iomem *reg = nfc->regs + NFC_IRQ_STATUS;
> + u32 tmp = __raw_readl(reg);
Why are you using __raw_xxx variants here, whereas you use readl() in
others? The __raw_xxx variants do not have the normal endian awareness,
nor do they have implicit I/O barriers. I'd bet you should be using the
{read,write}l_relaxed() or {read,write}l(), depending on whether this is
performance critical and you handle the consistency issues elsewhere.
(Same comment applies to all the __raw_xxx usages in this driver, I
think.)
> +
> + tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
> + __raw_writel(tmp, reg);
> +}
> +
> +static inline void vf610_nfc_done(struct vf610_nfc *nfc)
> +{
> + int rv;
> +
> + /*
> + * Barrier is needed after this write. This write need
> + * to be done before reading the next register the first
> + * time.
> + * vf610_nfc_set implicates such a barrier by using writel
> + * to write to the register.
> + */
> + vf610_nfc_set(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
> + vf610_nfc_set(nfc, NFC_FLASH_CMD2, START_BIT);
> +
> + if (!(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT)) {
> + rv = wait_event_timeout(nfc->irq_waitq,
> + (vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT),
> + NFC_TIMEOUT);
This looks like you're doing a standard IRQ-based wait-for-completion,
except that you're also including some extra register polling. Would
this work just as well using a struct completion
(wait_for_completion_timeout() and complete())? Or is the register poll
necessary?
> + if (!rv)
> + dev_warn(nfc->dev, "Timeout while waiting for BUSY.\n");
> + }
> + vf610_nfc_clear_status(nfc);
> +}
> +
> +static u8 vf610_nfc_get_id(struct vf610_nfc *nfc, int col)
> +{
> + u32 flash_id;
> +
> + if (col < 4) {
> + flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS1);
> + return (flash_id >> (3-col)*8) & 0xff;
Add spaces around the arithmetic, please.
> + } else {
> + flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS2);
> + return flash_id >> 24;
> + }
> +}
> +
> +static u8 vf610_nfc_get_status(struct vf610_nfc *nfc)
> +{
> + return vf610_nfc_read(nfc, NFC_FLASH_STATUS2) & STATUS_BYTE1_MASK;
> +}
> +
> +static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
> + u32 cmd_code)
> +{
> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
> + u32 tmp;
> +
> + vf610_nfc_clear_status(nfc);
> +
> + tmp = __raw_readl(reg);
> + tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
> + tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
> + tmp |= cmd_code << CMD_CODE_SHIFT;
> + __raw_writel(tmp, reg);
> +}
> +
> +static void vf610_nfc_send_commands(struct vf610_nfc *nfc, u32 cmd_byte1,
> + u32 cmd_byte2, u32 cmd_code)
> +{
> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD1;
> + u32 tmp;
> +
> + vf610_nfc_send_command(nfc, cmd_byte1, cmd_code);
> +
> + tmp = __raw_readl(reg);
> + tmp &= ~CMD_BYTE2_MASK;
> + tmp |= cmd_byte2 << CMD_BYTE2_SHIFT;
> + __raw_writel(tmp, reg);
> +}
> +
> +static irqreturn_t vf610_nfc_irq(int irq, void *data)
> +{
> + struct mtd_info *mtd = data;
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + vf610_nfc_clear(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
> + wake_up(&nfc->irq_waitq);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
> +{
> + if (column != -1) {
> + if (nfc->chip.options | NAND_BUSWIDTH_16)
smatch caught a hard-to-spot one here:
drivers/mtd/nand/vf610_nfc.c:335 vf610_nfc_addr_cycle() warn: suspicious bitop condition [smatch]
You probably meant:
if (nfc->chip.options & NAND_BUSWIDTH_16)
> + column = column/2;
> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
> + COL_ADDR_SHIFT, column);
> + }
> + if (page != -1)
> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
> + ROW_ADDR_SHIFT, page);
> +}
> +
> +static inline void vf610_nfc_transfer_size(void __iomem *regbase, int size)
> +{
> + __raw_writel(size, regbase + NFC_SECTOR_SIZE);
> +}
> +
> +static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
> + int column, int page)
> +{
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + nfc->column = max(column, 0);
> + nfc->spareonly = 0;
> + nfc->alt_buf = 0;
> +
> + switch (command) {
> + case NAND_CMD_PAGEPROG:
> + nfc->page = -1;
> + vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
> + vf610_nfc_send_commands(nfc, NAND_CMD_SEQIN,
> + command, PROGRAM_PAGE_CMD_CODE);
> + vf610_nfc_addr_cycle(nfc, column, page);
> + break;
> +
> + case NAND_CMD_RESET:
> + vf610_nfc_transfer_size(nfc->regs, 0);
> + vf610_nfc_send_command(nfc, command, RESET_CMD_CODE);
> + break;
> + /*
> + * NFC does not support sub-page reads and writes,
> + * so emulate them using full page transfers.
> + */
> + case NAND_CMD_READOOB:
> + nfc->spareonly = 1;
> + case NAND_CMD_SEQIN: /* Pre-read for partial writes. */
> + case NAND_CMD_READ0:
> + column = 0;
> + /* Already read? */
> + if (nfc->page == page)
> + return;
> + nfc->page = page;
> + vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
> + vf610_nfc_send_commands(nfc, NAND_CMD_READ0,
> + NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
> + vf610_nfc_addr_cycle(nfc, column, page);
> + break;
> +
> + case NAND_CMD_ERASE1:
> + nfc->page = -1;
> + vf610_nfc_transfer_size(nfc->regs, 0);
> + vf610_nfc_send_commands(nfc, command,
> + NAND_CMD_ERASE2, ERASE_CMD_CODE);
> + vf610_nfc_addr_cycle(nfc, column, page);
> + break;
> +
> + case NAND_CMD_READID:
> + nfc->alt_buf = ALT_BUF_ID;
> + vf610_nfc_transfer_size(nfc->regs, 0);
> + vf610_nfc_send_command(nfc, command, READ_ID_CMD_CODE);
> + break;
> +
> + case NAND_CMD_STATUS:
> + nfc->alt_buf = ALT_BUF_STAT;
> + vf610_nfc_transfer_size(nfc->regs, 0);
> + vf610_nfc_send_command(nfc, command, STATUS_READ_CMD_CODE);
> + break;
> + default:
> + return;
> + }
> +
> + vf610_nfc_done(nfc);
> +}
> +
> +static inline void vf610_nfc_read_spare(struct mtd_info *mtd, void *buf,
> + int len)
> +{
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + len = min_t(uint, mtd->oobsize, len);
> + if (len > 0)
> + vf610_nfc_memcpy(buf, nfc->regs + mtd->writesize, len);
> +}
> +
> +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;
> + uint l;
> +
> + /* Handle main area */
> + if (!nfc->spareonly) {
> + l = min_t(uint, len, mtd->writesize - c);
> + nfc->column += l;
> +
> + if (!nfc->alt_buf)
> + vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, l);
> + else
> + if (nfc->alt_buf & ALT_BUF_ID)
> + *buf = vf610_nfc_get_id(nfc, c);
> + else
> + *buf = vf610_nfc_get_status(nfc);
> +
> + buf += l;
> + len -= l;
> + }
> +
> + /* Handle spare area access */
> + if (len) {
> + nfc->column += len;
> + vf610_nfc_read_spare(mtd, buf, len);
> + }
> +}
> +
> +static void vf610_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> +{
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> + uint c = nfc->column;
> + uint l;
> +
> + l = min_t(uint, len, mtd->writesize + mtd->oobsize - c);
> + nfc->column += l;
> + vf610_nfc_memcpy(nfc->regs + NFC_MAIN_AREA(0) + c, buf, l);
> +}
> +
> +static uint8_t vf610_nfc_read_byte(struct mtd_info *mtd)
> +{
> + u8 tmp;
> +
> + vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
> + return tmp;
> +}
> +
> +static u16 vf610_nfc_read_word(struct mtd_info *mtd)
> +{
> + u16 tmp;
> +
> + vf610_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
> + return tmp;
> +}
> +
> +/* If not provided, upper layers apply a fixed delay. */
> +static int vf610_nfc_dev_ready(struct mtd_info *mtd)
> +{
> + /* NFC handles R/B internally; always ready. */
> + return 1;
> +}
> +
> +/*
> + * This function supports Vybrid only (MPC5125 would have full RB and four CS)
> + */
> +static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
> +{
> +#ifdef CONFIG_SOC_VF610
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> + u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
> +
> + tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
> + tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
> +
> + if (chip == 0)
> + tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
> + else if (chip == 1)
> + tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;
> +
> + vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
> +#endif
> +}
> +
> +#ifdef CONFIG_OF_MTD
Don't use CONFIG_OF_MTD. Maybe CONFIG_OF, but even that doesn't seem
necessary. The table is tiny, and the code should fail gracefully (and
reduce to pretty simple / negligible code) if you don't have OF enabled.
> +static const struct of_device_id vf610_nfc_dt_ids[] = {
> + { .compatible = "fsl,vf610-nfc" },
> + { .compatible = "fsl,mpc5125-nfc" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, vf610_nfc_dt_ids);
> +
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> +{
> + struct device_node *np = dev->of_node;
> + int buswidth;
> +
> + if (!np)
> + return 1;
Try:
return -EINVAL;
> +
> + cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> +
> + buswidth = of_get_nand_bus_width(np);
> + if (buswidth < 0)
> + return buswidth;
This function should mostly come for free as part of this:
commit 5844feeaa4154d1c46d3462c7a4653d22356d8b4
Author: Brian Norris <computersforpeace@gmail.com>
Date: Fri Jan 23 00:22:27 2015 -0800
mtd: nand: add common DT init code
in l2-mtd.git and linux-next.git. Can you try just assigning chip->dn
instead?
> +
> + cfg->width = buswidth;
> +
> + return 0;
> +}
> +#else
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
> +{
> + return 0;
Really? I doubt you want to silently do nothing if you don't have OF
enabled. It seems to make more sense to just kill the #ifdef, and have
the registration fail on either the '!np' case or the
'of_get_nand_bus_width(np) returns -ENOSYS' case.
> +}
> +#endif
> +
> +static int vf610_nfc_init_controller(struct vf610_nfc *nfc)
> +{
> + struct vf610_nfc_config *cfg = &nfc->cfg;
> +
> + if (cfg->width == 16)
> + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
> + else
> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
> +
> + /* Set configuration register. */
> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
> + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
> +
> + /* PAGE_CNT = 1 */
> + vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
> + CONFIG_PAGE_CNT_SHIFT, 1);
> +
> + return 0;
> +}
> +
> +static int vf610_nfc_probe(struct platform_device *pdev)
> +{
> + struct vf610_nfc *nfc;
> + struct resource *res;
> + struct mtd_info *mtd;
> + struct nand_chip *chip;
> + struct vf610_nfc_config *cfg;
> + int err = 0;
> + int irq;
> +
> + nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> + cfg = &nfc->cfg;
> +
> + nfc->dev = &pdev->dev;
> + nfc->page = -1;
> + mtd = &nfc->mtd;
> + chip = &nfc->chip;
> +
> + mtd->priv = chip;
> + mtd->owner = THIS_MODULE;
> + mtd->dev.parent = nfc->dev;
> + mtd->name = DRV_NAME;
> +
> + err = vf610_nfc_probe_dt(nfc->dev, cfg);
> + if (err)
> + return -ENODEV;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0)
> + return -EINVAL;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + nfc->regs = devm_ioremap_resource(nfc->dev, res);
> + if (IS_ERR(nfc->regs))
> + return PTR_ERR(nfc->regs);
> +
> + nfc->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(nfc->clk))
> + return PTR_ERR(nfc->clk);
> +
> + err = clk_prepare_enable(nfc->clk);
> + if (err) {
> + dev_err(nfc->dev, "Unable to enable clock!\n");
> + return err;
> + }
> +
> + if (cfg->width == 16)
> + chip->options |= NAND_BUSWIDTH_16;
> + else
> + chip->options &= ~NAND_BUSWIDTH_16;
Is the else clause needed? Also, you'll get the above for free with the
aforementioned commit ("mtd: nand: add common DT init code").
> +
> + chip->dev_ready = vf610_nfc_dev_ready;
> + chip->cmdfunc = vf610_nfc_command;
> + chip->read_byte = vf610_nfc_read_byte;
> + chip->read_word = vf610_nfc_read_word;
> + chip->read_buf = vf610_nfc_read_buf;
> + chip->write_buf = vf610_nfc_write_buf;
> + chip->select_chip = vf610_nfc_select_chip;
> +
> + /* Bad block options. */
> + if (cfg->flash_bbt)
> + chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB |
> + NAND_BBT_CREATE;
You probably don't need NAND_BBT_CREATE here. It's added automatically
to the BBT descriptors in nand_bbt.c.
You can also get the NAND_BBT_USE_FLASH part for free from the
aforementioned patch ("mtd: nand: add common DT init code").
> +
> + init_waitqueue_head(&nfc->irq_waitq);
> +
> + err = devm_request_irq(nfc->dev, irq, vf610_nfc_irq, 0, DRV_NAME, mtd);
> + if (err) {
> + dev_err(nfc->dev, "Error requesting IRQ!\n");
> + goto error;
> + }
> +
> + nfc->page_sz = PAGE_2K + OOB_64;
> + nfc->page_sz += cfg->width == 16 ? 1 : 0;
> +
> + vf610_nfc_init_controller(nfc);
> +
> + /* first scan to find the device and get the page size */
> + if (nand_scan_ident(mtd, 1, NULL)) {
> + err = -ENXIO;
> + goto error;
> + }
> +
> + chip->ecc.mode = NAND_ECC_SOFT; /* default */
> +
> + nfc->page_sz = mtd->writesize + mtd->oobsize;
> +
> + /* Single buffer only, max 256 OOB minus ECC status */
> + if (nfc->page_sz > PAGE_2K + 256 - 8) {
> + dev_err(nfc->dev, "Unsupported flash page size\n");
> + err = -ENXIO;
> + goto error;
> + }
> + nfc->page_sz += cfg->width == 16 ? 1 : 0;
> +
> + /* second phase scan */
> + if (nand_scan_tail(mtd)) {
> + err = -ENXIO;
> + goto error;
> + }
> +
> + /* Register device in MTD */
> + mtd_device_parse_register(mtd, NULL,
> + &(struct mtd_part_parser_data){
> + .of_node = pdev->dev.of_node,
> + },
> + NULL, 0);
Please check the return code of mtd_device_parse_register().
> +
> + platform_set_drvdata(pdev, mtd);
If you move this up, then you can do:
return mtd_device_parse_register(...);
> +
> + return 0;
> +
> +error:
> + clk_disable_unprepare(nfc->clk);
> + return err;
> +}
> +
> +static int vf610_nfc_remove(struct platform_device *pdev)
> +{
> + struct mtd_info *mtd = platform_get_drvdata(pdev);
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + nand_release(mtd);
> + clk_disable_unprepare(nfc->clk);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int vf610_nfc_suspend(struct device *dev)
> +{
> + struct mtd_info *mtd = dev_get_drvdata(dev);
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + clk_disable_unprepare(nfc->clk);
> + return 0;
> +}
> +
> +static int vf610_nfc_resume(struct device *dev)
> +{
> + struct mtd_info *mtd = dev_get_drvdata(dev);
> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +
> + pinctrl_pm_select_default_state(dev);
> +
> + clk_prepare_enable(nfc->clk);
> +
> + vf610_nfc_init_controller(nfc);
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(vf610_nfc_pm_ops, vf610_nfc_suspend, vf610_nfc_resume);
> +
> +static struct platform_driver vf610_nfc_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .of_match_table = vf610_nfc_dt_ids,
> + .pm = &vf610_nfc_pm_ops,
> + },
> + .probe = vf610_nfc_probe,
> + .remove = vf610_nfc_remove,
> +};
> +
> +module_platform_driver(vf610_nfc_driver);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
> +MODULE_DESCRIPTION("Freescale VF610/MPC5125 NFC MTD NAND driver");
> +MODULE_LICENSE("GPL");
Brian
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-02 1:32 ` Brian Norris
@ 2015-06-02 9:01 ` Stefan Agner
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-06-02 9:01 UTC (permalink / raw)
To: Brian Norris
Cc: dwmw2, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel
Hi Brian,
Thanks for the review!
FYI, in between I have some patches pending which enable ONFI and some
other minor improvements. Those changes already went upstream in U-Boot,
I will port them to the Linux version.
Hence the next version will also include those changes...
FWIW, the U-boot thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/220416
On 2015-06-02 03:32, Brian Norris wrote:
> Hi Stefan,
>
> Late review...
>
> On Wed, Mar 25, 2015 at 05:28:24PM +0100, Stefan Agner wrote:
>> This driver supports Freescale NFC (NAND flash controller) found on
>> Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
>>
>> Limitations:
>> - DMA and pipelining not used
>> - Pages larger than 2k are not supported
>> - No hardware ECC
>>
>> The driver has only been tested on Vybrid (VF610).
>>
>> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> MAINTAINERS | 6 +
>> drivers/mtd/nand/Kconfig | 12 +
>> drivers/mtd/nand/Makefile | 1 +
>> drivers/mtd/nand/vf610_nfc.c | 686 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 705 insertions(+)
>> create mode 100644 drivers/mtd/nand/vf610_nfc.c
>>
> ...
>> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
>> new file mode 100644
>> index 0000000..23c1510
>> --- /dev/null
>> +++ b/drivers/mtd/nand/vf610_nfc.c
>> @@ -0,0 +1,686 @@
>> +/*
>> + * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
>> + *
>> + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
>> + * Jason ported to M54418TWR and MVFA5 (VF610).
>> + * Authors: Stefan Agner <stefan.agner@toradex.com>
>> + * Bill Pringlemeir <bpringlemeir@nbsps.com>
>> + * Shaohui Xie <b21989@freescale.com>
>> + * Jason Jin <Jason.jin@freescale.com>
>> + *
>> + * Based on original driver mpc5121_nfc.c.
>
> It's not supremely obvious to me: what's the relationship between these
> drivers? How similar is the IP, if at all?
I think this is the driver the initial authors templated from (Jason,
Shaohui).
It appears to me that the NFC in MPC5121 is a remote relative of this
IP: There are some register which are named similar, but usually the
layout is quite different. Also the command sequencing seems to be
handled differently...
Note that Bill Pringlemeir shares this opinion, as he stated in a thread
related to his initial RFC patch submission:
http://thread.gmane.org/gmane.linux.drivers.mtd/49878/focus=49885
<snip>
>> +static inline void vf610_nfc_memcpy(void *dst, const void *src, size_t n)
>> +{
>> + /*
>> + * Use this accessor for the interal SRAM buffers. On ARM we can
>> + * treat the SRAM buffer as if its memory, hence use memcpy
>
> s/its/it's/
> or
> s/its/it is/
>
>> + */
>> + memcpy(dst, src, n);
>
> I'm not clear on all the reasoning in your comment. Is this really
> specific to ARM, that you can treat SRAM like memory? If so, should this
> driver have some dependency on ARM?
I do not have a PowerPC here, so I can't test it actually. My guess
would be that the PowerPC will behave similar and hence allow to use
memcpy too.
This memcpy encapsulation was suggested by Bill, and I think it makes
sense since the SRAM is part of the IP register map, it _could_ probably
behave different than normal SRAM and hence need special treatment...
However, at least on ARM memcpy works fine (and we can make use of the
assembler optimized variants of it).
I will alter the comment to read somewhat like:
/*
* Use this accessor for the internal SRAM buffers. On ARM platforms,
it's
* known that the driver can treat the SRAM buffer as if it's memory.
Other
* platform might need to treat that buffer differently.
*
* For the time being, use memcpy
*/
>
> Also, if you're really trying to handle iomem, it makes sense to
> annotate the 2nd argument with __iomem. Then sparse will only complain
> on the memcpy(), which has a proper comment explanation, rather than on
> every call site for vf610_nfc_memcpy(). i.e.:
>
> static inline void vf610_nfc_memcpy(void *dst, const void __iomem *src,
> size_t n)
>
Agreed.
>> +}
>> +
>> +/* Clear flags for upcoming command */
>> +static inline void vf610_nfc_clear_status(struct vf610_nfc *nfc)
>> +{
>> + void __iomem *reg = nfc->regs + NFC_IRQ_STATUS;
>> + u32 tmp = __raw_readl(reg);
>
> Why are you using __raw_xxx variants here, whereas you use readl() in
> others? The __raw_xxx variants do not have the normal endian awareness,
> nor do they have implicit I/O barriers. I'd bet you should be using the
> {read,write}l_relaxed() or {read,write}l(), depending on whether this is
> performance critical and you handle the consistency issues elsewhere.
>
> (Same comment applies to all the __raw_xxx usages in this driver, I
> think.)
>
>> +
>> + tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
>> + __raw_writel(tmp, reg);
>> +}
>> +
>> +static inline void vf610_nfc_done(struct vf610_nfc *nfc)
>> +{
>> + int rv;
>> +
>> + /*
>> + * Barrier is needed after this write. This write need
>> + * to be done before reading the next register the first
>> + * time.
>> + * vf610_nfc_set implicates such a barrier by using writel
>> + * to write to the register.
>> + */
>> + vf610_nfc_set(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
>> + vf610_nfc_set(nfc, NFC_FLASH_CMD2, START_BIT);
>> +
>> + if (!(vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT)) {
>> + rv = wait_event_timeout(nfc->irq_waitq,
>> + (vf610_nfc_read(nfc, NFC_IRQ_STATUS) & IDLE_IRQ_BIT),
>> + NFC_TIMEOUT);
>
> This looks like you're doing a standard IRQ-based wait-for-completion,
> except that you're also including some extra register polling. Would
> this work just as well using a struct completion
> (wait_for_completion_timeout() and complete())? Or is the register poll
> necessary?
>
>> + if (!rv)
>> + dev_warn(nfc->dev, "Timeout while waiting for BUSY.\n");
>> + }
>> + vf610_nfc_clear_status(nfc);
>> +}
>> +
>> +static u8 vf610_nfc_get_id(struct vf610_nfc *nfc, int col)
>> +{
>> + u32 flash_id;
>> +
>> + if (col < 4) {
>> + flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS1);
>> + return (flash_id >> (3-col)*8) & 0xff;
>
> Add spaces around the arithmetic, please.
>
>> + } else {
>> + flash_id = vf610_nfc_read(nfc, NFC_FLASH_STATUS2);
>> + return flash_id >> 24;
>> + }
>> +}
>> +
>> +static u8 vf610_nfc_get_status(struct vf610_nfc *nfc)
>> +{
>> + return vf610_nfc_read(nfc, NFC_FLASH_STATUS2) & STATUS_BYTE1_MASK;
>> +}
>> +
>> +static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
>> + u32 cmd_code)
>> +{
>> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
>> + u32 tmp;
>> +
>> + vf610_nfc_clear_status(nfc);
>> +
>> + tmp = __raw_readl(reg);
>> + tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
>> + tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
>> + tmp |= cmd_code << CMD_CODE_SHIFT;
>> + __raw_writel(tmp, reg);
>> +}
>> +
>> +static void vf610_nfc_send_commands(struct vf610_nfc *nfc, u32 cmd_byte1,
>> + u32 cmd_byte2, u32 cmd_code)
>> +{
>> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD1;
>> + u32 tmp;
>> +
>> + vf610_nfc_send_command(nfc, cmd_byte1, cmd_code);
>> +
>> + tmp = __raw_readl(reg);
>> + tmp &= ~CMD_BYTE2_MASK;
>> + tmp |= cmd_byte2 << CMD_BYTE2_SHIFT;
>> + __raw_writel(tmp, reg);
>> +}
>> +
>> +static irqreturn_t vf610_nfc_irq(int irq, void *data)
>> +{
>> + struct mtd_info *mtd = data;
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + vf610_nfc_clear(nfc, NFC_IRQ_STATUS, IDLE_EN_BIT);
>> + wake_up(&nfc->irq_waitq);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
>> +{
>> + if (column != -1) {
>> + if (nfc->chip.options | NAND_BUSWIDTH_16)
>
> smatch caught a hard-to-spot one here:
>
> drivers/mtd/nand/vf610_nfc.c:335 vf610_nfc_addr_cycle() warn:
> suspicious bitop condition [smatch]
>
> You probably meant:
>
> if (nfc->chip.options & NAND_BUSWIDTH_16)
>
>> + column = column/2;
>> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
>> + COL_ADDR_SHIFT, column);
>> + }
>> + if (page != -1)
>> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
>> + ROW_ADDR_SHIFT, page);
>> +}
>> +
>> +static inline void vf610_nfc_transfer_size(void __iomem *regbase, int size)
>> +{
>> + __raw_writel(size, regbase + NFC_SECTOR_SIZE);
>> +}
>> +
>> +static void vf610_nfc_command(struct mtd_info *mtd, unsigned command,
>> + int column, int page)
>> +{
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + nfc->column = max(column, 0);
>> + nfc->spareonly = 0;
>> + nfc->alt_buf = 0;
>> +
>> + switch (command) {
>> + case NAND_CMD_PAGEPROG:
>> + nfc->page = -1;
>> + vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
>> + vf610_nfc_send_commands(nfc, NAND_CMD_SEQIN,
>> + command, PROGRAM_PAGE_CMD_CODE);
>> + vf610_nfc_addr_cycle(nfc, column, page);
>> + break;
>> +
>> + case NAND_CMD_RESET:
>> + vf610_nfc_transfer_size(nfc->regs, 0);
>> + vf610_nfc_send_command(nfc, command, RESET_CMD_CODE);
>> + break;
>> + /*
>> + * NFC does not support sub-page reads and writes,
>> + * so emulate them using full page transfers.
>> + */
>> + case NAND_CMD_READOOB:
>> + nfc->spareonly = 1;
>> + case NAND_CMD_SEQIN: /* Pre-read for partial writes. */
>> + case NAND_CMD_READ0:
>> + column = 0;
>> + /* Already read? */
>> + if (nfc->page == page)
>> + return;
>> + nfc->page = page;
>> + vf610_nfc_transfer_size(nfc->regs, nfc->page_sz);
>> + vf610_nfc_send_commands(nfc, NAND_CMD_READ0,
>> + NAND_CMD_READSTART, READ_PAGE_CMD_CODE);
>> + vf610_nfc_addr_cycle(nfc, column, page);
>> + break;
>> +
>> + case NAND_CMD_ERASE1:
>> + nfc->page = -1;
>> + vf610_nfc_transfer_size(nfc->regs, 0);
>> + vf610_nfc_send_commands(nfc, command,
>> + NAND_CMD_ERASE2, ERASE_CMD_CODE);
>> + vf610_nfc_addr_cycle(nfc, column, page);
>> + break;
>> +
>> + case NAND_CMD_READID:
>> + nfc->alt_buf = ALT_BUF_ID;
>> + vf610_nfc_transfer_size(nfc->regs, 0);
>> + vf610_nfc_send_command(nfc, command, READ_ID_CMD_CODE);
>> + break;
>> +
>> + case NAND_CMD_STATUS:
>> + nfc->alt_buf = ALT_BUF_STAT;
>> + vf610_nfc_transfer_size(nfc->regs, 0);
>> + vf610_nfc_send_command(nfc, command, STATUS_READ_CMD_CODE);
>> + break;
>> + default:
>> + return;
>> + }
>> +
>> + vf610_nfc_done(nfc);
>> +}
>> +
>> +static inline void vf610_nfc_read_spare(struct mtd_info *mtd, void *buf,
>> + int len)
>> +{
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + len = min_t(uint, mtd->oobsize, len);
>> + if (len > 0)
>> + vf610_nfc_memcpy(buf, nfc->regs + mtd->writesize, len);
>> +}
>> +
>> +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;
>> + uint l;
>> +
>> + /* Handle main area */
>> + if (!nfc->spareonly) {
>> + l = min_t(uint, len, mtd->writesize - c);
>> + nfc->column += l;
>> +
>> + if (!nfc->alt_buf)
>> + vf610_nfc_memcpy(buf, nfc->regs + NFC_MAIN_AREA(0) + c, l);
>> + else
>> + if (nfc->alt_buf & ALT_BUF_ID)
>> + *buf = vf610_nfc_get_id(nfc, c);
>> + else
>> + *buf = vf610_nfc_get_status(nfc);
>> +
>> + buf += l;
>> + len -= l;
>> + }
>> +
>> + /* Handle spare area access */
>> + if (len) {
>> + nfc->column += len;
>> + vf610_nfc_read_spare(mtd, buf, len);
>> + }
>> +}
>> +
>> +static void vf610_nfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>> +{
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> + uint c = nfc->column;
>> + uint l;
>> +
>> + l = min_t(uint, len, mtd->writesize + mtd->oobsize - c);
>> + nfc->column += l;
>> + vf610_nfc_memcpy(nfc->regs + NFC_MAIN_AREA(0) + c, buf, l);
>> +}
>> +
>> +static uint8_t vf610_nfc_read_byte(struct mtd_info *mtd)
>> +{
>> + u8 tmp;
>> +
>> + vf610_nfc_read_buf(mtd, &tmp, sizeof(tmp));
>> + return tmp;
>> +}
>> +
>> +static u16 vf610_nfc_read_word(struct mtd_info *mtd)
>> +{
>> + u16 tmp;
>> +
>> + vf610_nfc_read_buf(mtd, (u_char *)&tmp, sizeof(tmp));
>> + return tmp;
>> +}
>> +
>> +/* If not provided, upper layers apply a fixed delay. */
>> +static int vf610_nfc_dev_ready(struct mtd_info *mtd)
>> +{
>> + /* NFC handles R/B internally; always ready. */
>> + return 1;
>> +}
>> +
>> +/*
>> + * This function supports Vybrid only (MPC5125 would have full RB and four CS)
>> + */
>> +static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
>> +{
>> +#ifdef CONFIG_SOC_VF610
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> + u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
>> +
>> + tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
>> + tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
>> +
>> + if (chip == 0)
>> + tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
>> + else if (chip == 1)
>> + tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;
>> +
>> + vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
>> +#endif
>> +}
>> +
>> +#ifdef CONFIG_OF_MTD
>
> Don't use CONFIG_OF_MTD. Maybe CONFIG_OF, but even that doesn't seem
> necessary. The table is tiny, and the code should fail gracefully (and
> reduce to pretty simple / negligible code) if you don't have OF enabled.
>
>> +static const struct of_device_id vf610_nfc_dt_ids[] = {
>> + { .compatible = "fsl,vf610-nfc" },
>> + { .compatible = "fsl,mpc5125-nfc" },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, vf610_nfc_dt_ids);
>> +
>> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
>> +{
>> + struct device_node *np = dev->of_node;
>> + int buswidth;
>> +
>> + if (!np)
>> + return 1;
>
> Try:
>
> return -EINVAL;
>
>> +
>> + cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>> +
>> + buswidth = of_get_nand_bus_width(np);
>> + if (buswidth < 0)
>> + return buswidth;
>
> This function should mostly come for free as part of this:
>
> commit 5844feeaa4154d1c46d3462c7a4653d22356d8b4
> Author: Brian Norris <computersforpeace@gmail.com>
> Date: Fri Jan 23 00:22:27 2015 -0800
>
> mtd: nand: add common DT init code
>
> in l2-mtd.git and linux-next.git. Can you try just assigning chip->dn
> instead?
>
>> +
>> + cfg->width = buswidth;
>> +
>> + return 0;
>> +}
>> +#else
>> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
>> +{
>> + return 0;
>
> Really? I doubt you want to silently do nothing if you don't have OF
> enabled. It seems to make more sense to just kill the #ifdef, and have
> the registration fail on either the '!np' case or the
> 'of_get_nand_bus_width(np) returns -ENOSYS' case.
The driver might be used on a platform which does not support DT
(ColdFire). However, I don't think anyone tried and I guess it doesn't
work as is, hence the driver would need an update anyway. So I'm fine
with just returning with an error in the non-DT case...
>
>> +}
>> +#endif
>> +
>> +static int vf610_nfc_init_controller(struct vf610_nfc *nfc)
>> +{
>> + struct vf610_nfc_config *cfg = &nfc->cfg;
>> +
>> + if (cfg->width == 16)
>> + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
>> + else
>> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_16BIT);
>> +
>> + /* Set configuration register. */
>> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
>> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
>> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
>> + vf610_nfc_clear(nfc, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
>> + vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
>> +
>> + /* PAGE_CNT = 1 */
>> + vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
>> + CONFIG_PAGE_CNT_SHIFT, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int vf610_nfc_probe(struct platform_device *pdev)
>> +{
>> + struct vf610_nfc *nfc;
>> + struct resource *res;
>> + struct mtd_info *mtd;
>> + struct nand_chip *chip;
>> + struct vf610_nfc_config *cfg;
>> + int err = 0;
>> + int irq;
>> +
>> + nfc = devm_kzalloc(&pdev->dev, sizeof(*nfc), GFP_KERNEL);
>> + if (!nfc)
>> + return -ENOMEM;
>> +
>> + cfg = &nfc->cfg;
>> +
>> + nfc->dev = &pdev->dev;
>> + nfc->page = -1;
>> + mtd = &nfc->mtd;
>> + chip = &nfc->chip;
>> +
>> + mtd->priv = chip;
>> + mtd->owner = THIS_MODULE;
>> + mtd->dev.parent = nfc->dev;
>> + mtd->name = DRV_NAME;
>> +
>> + err = vf610_nfc_probe_dt(nfc->dev, cfg);
>> + if (err)
>> + return -ENODEV;
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq <= 0)
>> + return -EINVAL;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + nfc->regs = devm_ioremap_resource(nfc->dev, res);
>> + if (IS_ERR(nfc->regs))
>> + return PTR_ERR(nfc->regs);
>> +
>> + nfc->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(nfc->clk))
>> + return PTR_ERR(nfc->clk);
>> +
>> + err = clk_prepare_enable(nfc->clk);
>> + if (err) {
>> + dev_err(nfc->dev, "Unable to enable clock!\n");
>> + return err;
>> + }
>> +
>> + if (cfg->width == 16)
>> + chip->options |= NAND_BUSWIDTH_16;
>> + else
>> + chip->options &= ~NAND_BUSWIDTH_16;
>
> Is the else clause needed? Also, you'll get the above for free with the
> aforementioned commit ("mtd: nand: add common DT init code").
>
>> +
>> + chip->dev_ready = vf610_nfc_dev_ready;
>> + chip->cmdfunc = vf610_nfc_command;
>> + chip->read_byte = vf610_nfc_read_byte;
>> + chip->read_word = vf610_nfc_read_word;
>> + chip->read_buf = vf610_nfc_read_buf;
>> + chip->write_buf = vf610_nfc_write_buf;
>> + chip->select_chip = vf610_nfc_select_chip;
>> +
>> + /* Bad block options. */
>> + if (cfg->flash_bbt)
>> + chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB |
>> + NAND_BBT_CREATE;
>
> You probably don't need NAND_BBT_CREATE here. It's added automatically
> to the BBT descriptors in nand_bbt.c.
>
> You can also get the NAND_BBT_USE_FLASH part for free from the
> aforementioned patch ("mtd: nand: add common DT init code").
>
>> +
>> + init_waitqueue_head(&nfc->irq_waitq);
>> +
>> + err = devm_request_irq(nfc->dev, irq, vf610_nfc_irq, 0, DRV_NAME, mtd);
>> + if (err) {
>> + dev_err(nfc->dev, "Error requesting IRQ!\n");
>> + goto error;
>> + }
>> +
>> + nfc->page_sz = PAGE_2K + OOB_64;
>> + nfc->page_sz += cfg->width == 16 ? 1 : 0;
>> +
>> + vf610_nfc_init_controller(nfc);
>> +
>> + /* first scan to find the device and get the page size */
>> + if (nand_scan_ident(mtd, 1, NULL)) {
>> + err = -ENXIO;
>> + goto error;
>> + }
>> +
>> + chip->ecc.mode = NAND_ECC_SOFT; /* default */
>> +
>> + nfc->page_sz = mtd->writesize + mtd->oobsize;
>> +
>> + /* Single buffer only, max 256 OOB minus ECC status */
>> + if (nfc->page_sz > PAGE_2K + 256 - 8) {
>> + dev_err(nfc->dev, "Unsupported flash page size\n");
>> + err = -ENXIO;
>> + goto error;
>> + }
>> + nfc->page_sz += cfg->width == 16 ? 1 : 0;
>> +
>> + /* second phase scan */
>> + if (nand_scan_tail(mtd)) {
>> + err = -ENXIO;
>> + goto error;
>> + }
>> +
>> + /* Register device in MTD */
>> + mtd_device_parse_register(mtd, NULL,
>> + &(struct mtd_part_parser_data){
>> + .of_node = pdev->dev.of_node,
>> + },
>> + NULL, 0);
>
> Please check the return code of mtd_device_parse_register().
>
>> +
>> + platform_set_drvdata(pdev, mtd);
>
> If you move this up, then you can do:
>
> return mtd_device_parse_register(...);
>
>> +
>> + return 0;
>> +
>> +error:
>> + clk_disable_unprepare(nfc->clk);
>> + return err;
>> +}
>> +
>> +static int vf610_nfc_remove(struct platform_device *pdev)
>> +{
>> + struct mtd_info *mtd = platform_get_drvdata(pdev);
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + nand_release(mtd);
>> + clk_disable_unprepare(nfc->clk);
>> + return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int vf610_nfc_suspend(struct device *dev)
>> +{
>> + struct mtd_info *mtd = dev_get_drvdata(dev);
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + clk_disable_unprepare(nfc->clk);
>> + return 0;
>> +}
>> +
>> +static int vf610_nfc_resume(struct device *dev)
>> +{
>> + struct mtd_info *mtd = dev_get_drvdata(dev);
>> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
>> +
>> + pinctrl_pm_select_default_state(dev);
>> +
>> + clk_prepare_enable(nfc->clk);
>> +
>> + vf610_nfc_init_controller(nfc);
>> + return 0;
>> +}
>> +#endif
>> +
>> +static SIMPLE_DEV_PM_OPS(vf610_nfc_pm_ops, vf610_nfc_suspend, vf610_nfc_resume);
>> +
>> +static struct platform_driver vf610_nfc_driver = {
>> + .driver = {
>> + .name = DRV_NAME,
>> + .of_match_table = vf610_nfc_dt_ids,
>> + .pm = &vf610_nfc_pm_ops,
>> + },
>> + .probe = vf610_nfc_probe,
>> + .remove = vf610_nfc_remove,
>> +};
>> +
>> +module_platform_driver(vf610_nfc_driver);
>> +
>> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
>> +MODULE_DESCRIPTION("Freescale VF610/MPC5125 NFC MTD NAND driver");
>> +MODULE_LICENSE("GPL");
Otherwise agreed, will look into implementing your changes and rebase
the driver on-top of l2-mtd.git/linux-next.git.
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-03-25 16:28 ` [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Stefan Agner
2015-06-02 1:32 ` Brian Norris
@ 2015-06-03 8:10 ` Sebastian Andrzej Siewior
2015-06-03 9:45 ` Stefan Agner
2015-06-03 13:08 ` Sebastian Andrzej Siewior
2 siblings, 1 reply; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-03 8:10 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 0000000..23c1510
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
…
> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
> +{
> + if (column != -1) {
> + if (nfc->chip.options | NAND_BUSWIDTH_16)
> + column = column/2;
> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
> + COL_ADDR_SHIFT, column);
> + }
> + if (page != -1)
> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
> + ROW_ADDR_SHIFT, page);
> +}
Do you have here also a different NAND layout on your boot-page vs
remaining pages? The mpc5125 has this different layout.
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 8:10 ` Sebastian Andrzej Siewior
@ 2015-06-03 9:45 ` Stefan Agner
2015-06-03 12:03 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Agner @ 2015-06-03 9:45 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-03 10:10, Sebastian Andrzej Siewior wrote:
> On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
>> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
>> new file mode 100644
>> index 0000000..23c1510
>> --- /dev/null
>> +++ b/drivers/mtd/nand/vf610_nfc.c
> …
>> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
>> +{
>> + if (column != -1) {
>> + if (nfc->chip.options | NAND_BUSWIDTH_16)
>> + column = column/2;
>> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
>> + COL_ADDR_SHIFT, column);
>> + }
>> + if (page != -1)
>> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
>> + ROW_ADDR_SHIFT, page);
>> +}
>
> Do you have here also a different NAND layout on your boot-page vs
> remaining pages? The mpc5125 has this different layout.
There is the boot configuration block which uses a Parity 13/8
algorithm, is it that you are referring to? The format parity is in the
main area, hence it doesn't clash with the HW ECC of the controller. One
can use normal NAND write functions to write this page. See the
implementation in our downstream U-Boot:
http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex-next&id=b004926168f3bc15f9e26f14c16ae60a252b1304
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 9:45 ` Stefan Agner
@ 2015-06-03 12:03 ` Sebastian Andrzej Siewior
2015-06-03 15:09 ` Stefan Agner
0 siblings, 1 reply; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-03 12:03 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On Wed, Jun 03, 2015 at 11:45:50AM +0200, Stefan Agner wrote:
> On 2015-06-03 10:10, Sebastian Andrzej Siewior wrote:
> > On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
> >> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> >> new file mode 100644
> >> index 0000000..23c1510
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/vf610_nfc.c
> > …
> >> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
> >> +{
> >> + if (column != -1) {
> >> + if (nfc->chip.options | NAND_BUSWIDTH_16)
> >> + column = column/2;
> >> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
> >> + COL_ADDR_SHIFT, column);
> >> + }
> >> + if (page != -1)
> >> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
> >> + ROW_ADDR_SHIFT, page);
> >> +}
> >
> > Do you have here also a different NAND layout on your boot-page vs
> > remaining pages? The mpc5125 has this different layout.
>
> There is the boot configuration block which uses a Parity 13/8
> algorithm, is it that you are referring to? The format parity is in the
> main area, hence it doesn't clash with the HW ECC of the controller. One
> can use normal NAND write functions to write this page. See the
No, this sounds different. The first few pages which are used for nand-boot
are using 1056 for sector size with 32bit-ecc. This configuration is always
used depsite what the physical NAND offers. Which means if you configure later
the proper sector size for your NAND with 2K (or 4K) pages the content in the
first page(s) can not be read. Even if you disable ECC correction on read the
content looks a little different due to the two sector size beeing involved.
And I am not sure if the layout if data/ECC is the same here, to. On top of
this writting without ECC is not usefull.
The FSL's BSP for mpc5125 had some funny commands in their u-boot in order to
write the boot-page(s).
That said, I added here in the addr cycle a check for the lower page numbers
to switch the sector size at runtime. With the proper partition entry I am
able to read/write the boot pages(s). The only extra magic involved is the
rearrangement of the binary data before writing it into the boot page because
the SoC's bootmode uses a different layout here.
Since you probably not doing this at all it probably remains unique to the
mpc5125. Please keep this in mind if someone with mpc5125 comes along that it
might be usefull to switch the sector size for the boot-pages so one can
easily read/write it from linux.
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 12:03 ` Sebastian Andrzej Siewior
@ 2015-06-03 15:09 ` Stefan Agner
2015-06-09 20:05 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Agner @ 2015-06-03 15:09 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-03 14:03, Sebastian Andrzej Siewior wrote:
> On Wed, Jun 03, 2015 at 11:45:50AM +0200, Stefan Agner wrote:
>> On 2015-06-03 10:10, Sebastian Andrzej Siewior wrote:
>> > On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
>> >> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
>> >> new file mode 100644
>> >> index 0000000..23c1510
>> >> --- /dev/null
>> >> +++ b/drivers/mtd/nand/vf610_nfc.c
>> > …
>> >> +static void vf610_nfc_addr_cycle(struct vf610_nfc *nfc, int column, int page)
>> >> +{
>> >> + if (column != -1) {
>> >> + if (nfc->chip.options | NAND_BUSWIDTH_16)
>> >> + column = column/2;
>> >> + vf610_nfc_set_field(nfc, NFC_COL_ADDR, COL_ADDR_MASK,
>> >> + COL_ADDR_SHIFT, column);
>> >> + }
>> >> + if (page != -1)
>> >> + vf610_nfc_set_field(nfc, NFC_ROW_ADDR, ROW_ADDR_MASK,
>> >> + ROW_ADDR_SHIFT, page);
>> >> +}
>> >
>> > Do you have here also a different NAND layout on your boot-page vs
>> > remaining pages? The mpc5125 has this different layout.
>>
>> There is the boot configuration block which uses a Parity 13/8
>> algorithm, is it that you are referring to? The format parity is in the
>> main area, hence it doesn't clash with the HW ECC of the controller. One
>> can use normal NAND write functions to write this page. See the
>
> No, this sounds different. The first few pages which are used for nand-boot
> are using 1056 for sector size with 32bit-ecc. This configuration is always
> used depsite what the physical NAND offers. Which means if you configure later
> the proper sector size for your NAND with 2K (or 4K) pages the content in the
> first page(s) can not be read. Even if you disable ECC correction on read the
> content looks a little different due to the two sector size beeing involved.
> And I am not sure if the layout if data/ECC is the same here, to. On top of
> this writting without ECC is not usefull.
Hm, so on PPC the boot ROM is making use of the HW ECC controller, is
this correct?
I guess if this is a hard wired behavior of the boot ROM for the first
few pages, one could add such hard wired behavior for the affected pages
in the driver too. However, since other platforms don't use that, it
probably would need to be done in a device specific way (e.g. quirks
field which is set according to compatible...)
> The FSL's BSP for mpc5125 had some funny commands in their u-boot in order to
> write the boot-page(s).
>
> That said, I added here in the addr cycle a check for the lower page numbers
> to switch the sector size at runtime. With the proper partition entry I am
> able to read/write the boot pages(s). The only extra magic involved is the
> rearrangement of the binary data before writing it into the boot page because
> the SoC's bootmode uses a different layout here.
> Since you probably not doing this at all it probably remains unique to the
> mpc5125. Please keep this in mind if someone with mpc5125 comes along that it
> might be usefull to switch the sector size for the boot-pages so one can
> easily read/write it from linux.
In the end, if somebody wants that feature, somebody will have to do
that change to the driver... But thanks for the information, will keep
that in mind.
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 15:09 ` Stefan Agner
@ 2015-06-09 20:05 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-09 20:05 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-03 17:09:33 [+0200], Stefan Agner wrote:
> Hm, so on PPC the boot ROM is making use of the HW ECC controller, is
> this correct?
On MPC5125, yes.
> --
> Stefan
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-03-25 16:28 ` [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Stefan Agner
2015-06-02 1:32 ` Brian Norris
2015-06-03 8:10 ` Sebastian Andrzej Siewior
@ 2015-06-03 13:08 ` Sebastian Andrzej Siewior
2015-06-03 15:05 ` Stefan Agner
2 siblings, 1 reply; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-03 13:08 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 0000000..23c1510
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,686 @@
…
> +static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
> +{
> + return readl(nfc->regs + reg);
> +}
> +
> +static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
> +{
> + writel(val, nfc->regs + reg);
> +}
…
> +static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
> + u32 cmd_code)
> +{
> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
> + u32 tmp;
> +
> + vf610_nfc_clear_status(nfc);
> +
> + tmp = __raw_readl(reg);
> + tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
> + tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
> + tmp |= cmd_code << CMD_CODE_SHIFT;
> + __raw_writel(tmp, reg);
> +}
Why readl() vs __raw_readl() dito for write?
vf610_nfc_{read|write} is good since for PPC we would need out_be32()
here instead.
It would be nice if you could abstract the __raw_ once as well. And I am
not sure if you need those at all since the former functions should work
here just fine.
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 13:08 ` Sebastian Andrzej Siewior
@ 2015-06-03 15:05 ` Stefan Agner
2015-06-09 20:07 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Agner @ 2015-06-03 15:05 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-03 15:08, Sebastian Andrzej Siewior wrote:
> On 2015-03-25 17:28:24 [+0100], Stefan Agner wrote:
>> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
>> new file mode 100644
>> index 0000000..23c1510
>> --- /dev/null
>> +++ b/drivers/mtd/nand/vf610_nfc.c
>> @@ -0,0 +1,686 @@
> …
>> +static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
>> +{
>> + return readl(nfc->regs + reg);
>> +}
>> +
>> +static inline void vf610_nfc_write(struct vf610_nfc *nfc, uint reg, u32 val)
>> +{
>> + writel(val, nfc->regs + reg);
>> +}
> …
>
>> +static void vf610_nfc_send_command(struct vf610_nfc *nfc, u32 cmd_byte1,
>> + u32 cmd_code)
>> +{
>> + void __iomem *reg = nfc->regs + NFC_FLASH_CMD2;
>> + u32 tmp;
>> +
>> + vf610_nfc_clear_status(nfc);
>> +
>> + tmp = __raw_readl(reg);
>> + tmp &= ~(CMD_BYTE1_MASK | CMD_CODE_MASK | BUFNO_MASK);
>> + tmp |= cmd_byte1 << CMD_BYTE1_SHIFT;
>> + tmp |= cmd_code << CMD_CODE_SHIFT;
>> + __raw_writel(tmp, reg);
>> +}
>
> Why readl() vs __raw_readl() dito for write?
> vf610_nfc_{read|write} is good since for PPC we would need out_be32()
> here instead.
> It would be nice if you could abstract the __raw_ once as well. And I am
> not sure if you need those at all since the former functions should work
> here just fine.
As Boris guessed correctly, the reason I used the raw variant was due to
performance improvements due to the barrier. However, I will use
{read|write}l_relaxed instead, which should offer endian abstraction
while not having the performance penalty due to extensive barriers...
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-03 15:05 ` Stefan Agner
@ 2015-06-09 20:07 ` Sebastian Andrzej Siewior
2015-06-09 20:31 ` Stefan Agner
2015-06-17 13:02 ` Stefan Agner
0 siblings, 2 replies; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-09 20:07 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-03 17:05:16 [+0200], Stefan Agner wrote:
> As Boris guessed correctly, the reason I used the raw variant was due to
> performance improvements due to the barrier. However, I will use
yeah, do you have any numbers by chance?
> {read|write}l_relaxed instead, which should offer endian abstraction
> while not having the performance penalty due to extensive barriers...
well, even those
|$ git grep readl_relaxed arch/powerpc/
|arch/powerpc/include/asm/io.h:#define readl_relaxed(addr) readl(addr)
have the endian swap. So an abstraction like you provided earlier
would be nice :)
> --
> Stefan
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-09 20:07 ` Sebastian Andrzej Siewior
@ 2015-06-09 20:31 ` Stefan Agner
2015-06-12 19:44 ` Sebastian Andrzej Siewior
2015-06-17 13:02 ` Stefan Agner
1 sibling, 1 reply; 21+ messages in thread
From: Stefan Agner @ 2015-06-09 20:31 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-09 22:07, Sebastian Andrzej Siewior wrote:
> On 2015-06-03 17:05:16 [+0200], Stefan Agner wrote:
>> As Boris guessed correctly, the reason I used the raw variant was due to
>> performance improvements due to the barrier. However, I will use
>
> yeah, do you have any numbers by chance?
Not current ones for Linux. I added this in the U-Boot variant of the
driver.
>> {read|write}l_relaxed instead, which should offer endian abstraction
>> while not having the performance penalty due to extensive barriers...
>
> well, even those
> |$ git grep readl_relaxed arch/powerpc/
> |arch/powerpc/include/asm/io.h:#define readl_relaxed(addr) readl(addr)
>
> have the endian swap. So an abstraction like you provided earlier
> would be nice :)
What do you mean by that?
Btw, I sent v5 of the patchset which use the relaxed variants. As
expected, performance did not suffer by that change on ARM.
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-09 20:31 ` Stefan Agner
@ 2015-06-12 19:44 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-12 19:44 UTC (permalink / raw)
To: Stefan Agner
Cc: Sebastian Andrzej Siewior, dwmw2, computersforpeace,
mark.rutland, boris.brezillon, aaron, marb, pawel.moll,
ijc+devicetree, linux-kernel, devicetree, robh+dt, linux-mtd,
kernel, galak, shawn.guo, linux-arm-kernel, bpringlemeir
On 2015-06-09 22:31:07 [+0200], Stefan Agner wrote:
> > have the endian swap. So an abstraction like you provided earlier
> > would be nice :)
>
> What do you mean by that?
Something like you did for the reader where you have:
|static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
|{
| return readl(nfc->regs + reg);
|}
instead using readl() directly. So you could also have
|static inline u32 vf610_nfc_read_relaxed(struct vf610_nfc *nfc, uint reg)
|{
| return readl_relaxed(nfc->regs + reg);
|}
Instead of using readl_relaxed(). Unless I'm mistaken, that function was
used more than once. If someone plugs in PPC support he does not need to
add this function anymore but but simply add an ifdef in vf610_nfc_read()
and vf610_nfc_read_relaxed() and replace it whatever works for him.
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-09 20:07 ` Sebastian Andrzej Siewior
2015-06-09 20:31 ` Stefan Agner
@ 2015-06-17 13:02 ` Stefan Agner
2015-06-18 18:27 ` Sebastian Andrzej Siewior
1 sibling, 1 reply; 21+ messages in thread
From: Stefan Agner @ 2015-06-17 13:02 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On 2015-06-09 22:07, Sebastian Andrzej Siewior wrote:
> On 2015-06-03 17:05:16 [+0200], Stefan Agner wrote:
>> As Boris guessed correctly, the reason I used the raw variant was due to
>> performance improvements due to the barrier. However, I will use
>
> yeah, do you have any numbers by chance?
>
Just reevaluated the "performance optimizations". On a VF610 SoC (with
L2 cache) the improvements increase performance by 2.5% and below. On a
VF500 SoC (without L2 cache) it seems to have slightly more impact, up
to 4%. Overall, it seems to influence write more than read.
Back then, when I implemented the improvements it certainly had a bigger
impact. I don't have strong opinion on that...
=> VF610, with relaxed, optimized
[ 45.554246]
[ 45.555934] =================================================
[ 45.561762] mtd_speedtest: MTD device: 3
[ 45.576627] mtd_speedtest: MTD device size 104857600, eraseblock size
131072, page size 2048, count of eraseblocks 800, pages per eraseblock
64, OOB size 64
[ 45.595350] mtd_test: scanning for bad eraseblocks
[ 45.602177] mtd_test: scanned 800 eraseblocks, 0 are bad
[ 46.404682] mtd_speedtest: testing eraseblock write speed
[ 65.650704] mtd_speedtest: eraseblock write speed is 5344 KiB/s
[ 65.656738] mtd_speedtest: testing eraseblock read speed
[ 73.480687] mtd_speedtest: eraseblock read speed is 13118 KiB/s
[ 74.264593] mtd_speedtest: testing page write speed
[ 94.444190] mtd_speedtest: page write speed is 5087 KiB/s
[ 94.449674] mtd_speedtest: testing page read speed
[ 102.760387] mtd_speedtest: page read speed is 12343 KiB/s
[ 103.544838] mtd_speedtest: testing 2 page write speed
[ 123.257753] mtd_speedtest: 2 page write speed is 5199 KiB/s
[ 123.263440] mtd_speedtest: testing 2 page read speed
[ 131.171313] mtd_speedtest: 2 page read speed is 12963 KiB/s
[ 131.176981] mtd_speedtest: Testing erase speed
[ 131.968196] mtd_speedtest: erase speed is 130279 KiB/s
[ 131.973442] mtd_speedtest: Testing 2x multi-block erase speed
[ 132.153410] mtd_speedtest: 2x multi-block erase speed is 585142 KiB/s
[ 132.159924] mtd_speedtest: Testing 4x multi-block erase speed
[ 132.334864] mtd_speedtest: 4x multi-block erase speed is 609523 KiB/s
[ 132.341369] mtd_speedtest: Testing 8x multi-block erase speed
[ 132.516091] mtd_speedtest: 8x multi-block erase speed is 609523 KiB/s
[ 132.522602] mtd_speedtest: Testing 16x multi-block erase speed
[ 132.695800] mtd_speedtest: 16x multi-block erase speed is 613173
KiB/s
[ 132.702389] mtd_speedtest: Testing 32x multi-block erase speed
[ 132.874831] mtd_speedtest: 32x multi-block erase speed is 616867
KiB/s
[ 132.881445] mtd_speedtest: Testing 64x multi-block erase speed
[ 133.053647] mtd_speedtest: 64x multi-block erase speed is 616867
KiB/s
[ 133.060267] mtd_speedtest: finished
[ 133.063812] =================================================
=> VF610, without relaxed and using accessors
vf610_nfc_set/vf610_nfc_set_field and friends:
[ 60.015797]
[ 60.017481] =================================================
[ 60.023320] mtd_speedtest: MTD device: 3
[ 60.037232] mtd_speedtest: MTD device size 104857600, eraseblock size
131072, page size 2048, count of eraseblocks 800, pages per eraseblock
64, OOB size 64
[ 60.066359] mtd_test: scanning for bad eraseblocks
[ 60.074016] mtd_test: scanned 800 eraseblocks, 0 are bad
[ 60.286730] mtd_speedtest: testing eraseblock write speed
[ 79.679892] mtd_speedtest: eraseblock write speed is 5281 KiB/s
[ 79.685930] mtd_speedtest: testing eraseblock read speed
[ 87.563845] mtd_speedtest: eraseblock read speed is 13008 KiB/s
[ 88.353390] mtd_speedtest: testing page write speed
[ 108.984528] mtd_speedtest: page write speed is 4965 KiB/s
[ 108.990041] mtd_speedtest: testing page read speed
[ 117.012486] mtd_speedtest: page read speed is 12774 KiB/s
[ 117.801663] mtd_speedtest: testing 2 page write speed
[ 137.674009] mtd_speedtest: 2 page write speed is 5154 KiB/s
[ 137.679696] mtd_speedtest: testing 2 page read speed
[ 145.643303] mtd_speedtest: 2 page read speed is 12865 KiB/s
[ 145.648982] mtd_speedtest: Testing erase speed
[ 146.444423] mtd_speedtest: erase speed is 129456 KiB/s
[ 146.449669] mtd_speedtest: Testing 2x multi-block erase speed
[ 146.629634] mtd_speedtest: 2x multi-block erase speed is 588505 KiB/s
[ 146.636142] mtd_speedtest: Testing 4x multi-block erase speed
[ 146.813027] mtd_speedtest: 4x multi-block erase speed is 598830 KiB/s
[ 146.819577] mtd_speedtest: Testing 8x multi-block erase speed
[ 146.996654] mtd_speedtest: 8x multi-block erase speed is 595348 KiB/s
[ 147.003192] mtd_speedtest: Testing 16x multi-block erase speed
[ 147.178085] mtd_speedtest: 16x multi-block erase speed is 609523
KiB/s
[ 147.184703] mtd_speedtest: Testing 32x multi-block erase speed
[ 147.358306] mtd_speedtest: 32x multi-block erase speed is 613173
KiB/s
[ 147.364929] mtd_speedtest: Testing 64x multi-block erase speed
[ 147.540505] mtd_speedtest: 64x multi-block erase speed is 605917
KiB/s
[ 147.547106] mtd_speedtest: finished
[ 147.558336] =================================================
=> VF500, with relaxed, optimized
[ 42.878713]
[ 42.880775] =================================================
[ 42.886589] mtd_speedtest: MTD device: 3
[ 42.933491] mtd_speedtest: MTD device size 132120576, eraseblock size
131072, page size 2048, count of eraseblocks 1008, pages per eraseblock
64, OOB size 64
[ 42.981413] mtd_test: scanning for bad eraseblocks
[ 42.987505] mtd_test: block 142 is bad
[ 43.001342] mtd_test: block 1004 is bad
[ 43.024184] mtd_test: block 1005 is bad
[ 43.029184] mtd_test: block 1006 is bad
[ 43.045070] mtd_test: block 1007 is bad
[ 43.048975] mtd_test: scanned 1008 eraseblocks, 5 are bad
[ 43.854654] mtd_speedtest: testing eraseblock write speed
[ 68.873797] mtd_speedtest: eraseblock write speed is 5150 KiB/s
[ 68.879915] mtd_speedtest: testing eraseblock read speed
[ 80.409787] mtd_speedtest: eraseblock read speed is 11171 KiB/s
[ 81.053244] mtd_speedtest: testing page write speed
[ 109.851897] mtd_speedtest: page write speed is 4462 KiB/s
[ 109.857496] mtd_speedtest: testing page read speed
[ 121.964415] mtd_speedtest: page read speed is 10616 KiB/s
[ 122.603068] mtd_speedtest: testing 2 page write speed
[ 148.929905] mtd_speedtest: 2 page write speed is 4880 KiB/s
[ 148.935696] mtd_speedtest: testing 2 page read speed
[ 160.908554] mtd_speedtest: 2 page read speed is 10731 KiB/s
[ 160.914236] mtd_speedtest: Testing erase speed
[ 161.579429] mtd_speedtest: erase speed is 194226 KiB/s
[ 161.584680] mtd_speedtest: Testing 2x multi-block erase speed
[ 162.226599] mtd_speedtest: 2x multi-block erase speed is 201861 KiB/s
[ 162.233153] mtd_speedtest: Testing 4x multi-block erase speed
[ 162.870923] mtd_speedtest: 4x multi-block erase speed is 202818 KiB/s
[ 162.877591] mtd_speedtest: Testing 8x multi-block erase speed
[ 163.503176] mtd_speedtest: 8x multi-block erase speed is 207070 KiB/s
[ 163.509826] mtd_speedtest: Testing 16x multi-block erase speed
[ 164.138392] mtd_speedtest: 16x multi-block erase speed is 206405
KiB/s
[ 164.145128] mtd_speedtest: Testing 32x multi-block erase speed
[ 164.768705] mtd_speedtest: 32x multi-block erase speed is 207741
KiB/s
[ 164.775475] mtd_speedtest: Testing 64x multi-block erase speed
[ 165.394716] mtd_speedtest: 64x multi-block erase speed is 209094
KiB/s
[ 165.401347] mtd_speedtest: finished
[ 165.415581] =================================================
=> VF500, without relaxed and using accessors
[ 93.466642]
[ 93.468578] =================================================
[ 93.474563] mtd_speedtest: MTD device: 3
[ 93.510699] mtd_speedtest: MTD device size 132120576, eraseblock size
131072, page size 2048, count of eraseblocks 1008, pages per eraseblock
64, OOB size 64
[ 93.534490] mtd_test: scanning for bad eraseblocks
[ 93.553066] mtd_test: block 142 is bad
[ 93.586518] mtd_test: block 1004 is bad
[ 93.590559] mtd_test: block 1005 is bad
[ 93.595654] mtd_test: block 1006 is bad
[ 93.607090] mtd_test: block 1007 is bad
[ 93.630667] mtd_test: scanned 1008 eraseblocks, 5 are bad
[ 94.457510] mtd_speedtest: testing eraseblock write speed
[ 120.495695] mtd_speedtest: eraseblock write speed is 4932 KiB/s
[ 120.501726] mtd_speedtest: testing eraseblock read speed
[ 131.937006] mtd_speedtest: eraseblock read speed is 11233 KiB/s
[ 132.576468] mtd_speedtest: testing page write speed
[ 160.663012] mtd_speedtest: page write speed is 4572 KiB/s
[ 160.668608] mtd_speedtest: testing page read speed
[ 172.624020] mtd_speedtest: page read speed is 10743 KiB/s
[ 173.263221] mtd_speedtest: testing 2 page write speed
[ 199.811255] mtd_speedtest: 2 page write speed is 4837 KiB/s
[ 199.816933] mtd_speedtest: testing 2 page read speed
[ 211.558975] mtd_speedtest: 2 page read speed is 10939 KiB/s
[ 211.564657] mtd_speedtest: Testing erase speed
[ 212.227608] mtd_speedtest: erase speed is 194816 KiB/s
[ 212.232951] mtd_speedtest: Testing 2x multi-block erase speed
[ 212.875607] mtd_speedtest: 2x multi-block erase speed is 201544 KiB/s
[ 212.882254] mtd_speedtest: Testing 4x multi-block erase speed
[ 213.521870] mtd_speedtest: 4x multi-block erase speed is 202818 KiB/s
[ 213.528471] mtd_speedtest: Testing 8x multi-block erase speed
[ 214.154043] mtd_speedtest: 8x multi-block erase speed is 207070 KiB/s
[ 214.160690] mtd_speedtest: Testing 16x multi-block erase speed
[ 214.785910] mtd_speedtest: 16x multi-block erase speed is 207405
KiB/s
[ 214.792652] mtd_speedtest: Testing 32x multi-block erase speed
[ 215.418150] mtd_speedtest: 32x multi-block erase speed is 207405
KiB/s
[ 215.424883] mtd_speedtest: Testing 64x multi-block erase speed
[ 216.051367] mtd_speedtest: 64x multi-block erase speed is 206737
KiB/s
[ 216.058056] mtd_speedtest: finished
[ 216.069363] =================================================
--
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others
2015-06-17 13:02 ` Stefan Agner
@ 2015-06-18 18:27 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 21+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-06-18 18:27 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, mark.rutland, boris.brezillon, aaron,
marb, pawel.moll, ijc+devicetree, linux-kernel, devicetree,
robh+dt, linux-mtd, kernel, galak, shawn.guo, linux-arm-kernel,
bpringlemeir
On Wed, Jun 17, 2015 at 03:02:03PM +0200, Stefan Agner wrote:
> On 2015-06-09 22:07, Sebastian Andrzej Siewior wrote:
> > yeah, do you have any numbers by chance?
>
> Just reevaluated the "performance optimizations". On a VF610 SoC (with
> L2 cache) the improvements increase performance by 2.5% and below. On a
> VF500 SoC (without L2 cache) it seems to have slightly more impact, up
> to 4%. Overall, it seems to influence write more than read.
it could have something to do with the CPU clock and how long it is blocked
due to the sync operation.
> Back then, when I implemented the improvements it certainly had a bigger
> impact. I don't have strong opinion on that...
Thanks for doing that. If you could take those numbers, put them in a table
like with/without relaxed and so one, write how you got them (modprobe bla)
and make it part of the commit message then everybody could see about how much
we talk here. Thanks again.
> --
> Stefan
Sebastian
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 2/6] mtd: nand: vf610_nfc: add hardware BCH-ECC support
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
2015-03-25 16:28 ` [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
2015-03-25 16:28 ` [PATCH v4 3/6] mtd: nand: vf610_nfc: add device tree bindings Stefan Agner
` (3 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
This adds hardware ECC support using the BCH encoder in the NFC IP.
The ECC encoder supports up to 32-bit correction by using 60 error
correction bytes. There is no sub-page ECC step, ECC is calculated
always accross the whole page (up to 2k pages).
Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
drivers/mtd/nand/Kconfig | 6 +-
drivers/mtd/nand/vf610_nfc.c | 173 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 177 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d0badda..824c343 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -464,8 +464,10 @@ config MTD_NAND_VF610_NFC
help
Enables support for NAND Flash Controller on some Freescale
processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
- The driver supports a maximum 2k page size. The driver
- currently does not support hardware ECC.
+ The driver supports a maximum 2k page size. With 2k pages and
+ 64 bytes or more of OOB, hardware ECC with up to 32-bit error
+ correction is supported. Hardware ECC is only enabled through
+ device tree.
config MTD_NAND_MXC
tristate "MXC NAND support"
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
index 23c1510..d273c27 100644
--- a/drivers/mtd/nand/vf610_nfc.c
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -19,6 +19,7 @@
* - Untested on MPC5125 and M54418.
* - DMA not used.
* - 2K pages or less.
+ * - Only 2K page w. 64+OOB and hardware ECC.
*/
#include <linux/module.h>
@@ -71,6 +72,8 @@
/* NFC ECC mode define */
#define ECC_BYPASS 0
+#define ECC_45_BYTE 6
+#define ECC_60_BYTE 7
/*** Register Mask and bit definitions */
@@ -103,6 +106,8 @@
#define STATUS_BYTE1_MASK 0x000000FF
/* NFC_FLASH_CONFIG Field */
+#define CONFIG_ECC_SRAM_ADDR_MASK 0x7FC00000
+#define CONFIG_ECC_SRAM_ADDR_SHIFT 22
#define CONFIG_ECC_SRAM_REQ_BIT (1<<21)
#define CONFIG_DMA_REQ_BIT (1<<20)
#define CONFIG_ECC_MODE_MASK 0x000E0000
@@ -123,9 +128,28 @@
#define NFC_TIMEOUT (HZ)
+/* ECC status placed at end of buffers. */
+#define ECC_SRAM_ADDR ((PAGE_2K+256-8) >> 3)
+#define ECC_STATUS_MASK 0x80
+#define ECC_ERR_COUNT 0x3F
+
+/*
+ * ECC status is stored at NFC_CFG[ECCADD] +4 for little-endian
+ * and +7 for big-endian SoCs.
+ */
+#ifdef CONFIG_SOC_VF610
+#define ECC_OFFSET 4
+#else
+#define ECC_OFFSET 7
+#endif
+
struct vf610_nfc_config {
+ bool hardware_ecc;
+ int ecc_strength;
+ int ecc_step_size;
int width;
int flash_bbt;
+ u32 ecc_mode;
};
struct vf610_nfc {
@@ -149,6 +173,34 @@ struct vf610_nfc {
#define mtd_to_nfc(_mtd) container_of(_mtd, struct vf610_nfc, mtd)
+static struct nand_ecclayout vf610_nfc_ecc45 = {
+ .eccbytes = 45,
+ .eccpos = {19, 20, 21, 22, 23,
+ 24, 25, 26, 27, 28, 29, 30, 31,
+ 32, 33, 34, 35, 36, 37, 38, 39,
+ 40, 41, 42, 43, 44, 45, 46, 47,
+ 48, 49, 50, 51, 52, 53, 54, 55,
+ 56, 57, 58, 59, 60, 61, 62, 63},
+ .oobfree = {
+ {.offset = 2,
+ .length = 17} }
+};
+
+static struct nand_ecclayout vf610_nfc_ecc60 = {
+ .eccbytes = 60,
+ .eccpos = { 4, 5, 6, 7, 8, 9, 10, 11,
+ 12, 13, 14, 15, 16, 17, 18, 19,
+ 20, 21, 22, 23, 24, 25, 26, 27,
+ 28, 29, 30, 31, 32, 33, 34, 35,
+ 36, 37, 38, 39, 40, 41, 42, 43,
+ 44, 45, 46, 47, 48, 49, 50, 51,
+ 52, 53, 54, 55, 56, 57, 58, 59,
+ 60, 61, 62, 63 },
+ .oobfree = {
+ {.offset = 2,
+ .length = 2} }
+};
+
static inline u32 vf610_nfc_read(struct vf610_nfc *nfc, uint reg)
{
return readl(nfc->regs + reg);
@@ -457,6 +509,60 @@ static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
#endif
}
+/* Count the number of 0's in buff up to max_bits */
+static inline int count_written_bits(uint8_t *buff, int size, int max_bits)
+{
+ uint32_t *buff32 = (uint32_t *)buff;
+ int k, written_bits = 0;
+
+ for (k = 0; k < (size / 4); k++) {
+ written_bits += hweight32(~buff32[k]);
+ if (written_bits > max_bits)
+ break;
+ }
+
+ return written_bits;
+}
+
+static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t *dat,
+ uint8_t *read_ecc, uint8_t *calc_ecc)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+ u8 ecc_status;
+ u8 ecc_count;
+ int flip;
+
+ ecc_status = __raw_readb(nfc->regs + ECC_SRAM_ADDR * 8 + ECC_OFFSET);
+ ecc_count = ecc_status & ECC_ERR_COUNT;
+ if (!(ecc_status & ECC_STATUS_MASK))
+ return ecc_count;
+
+ /*
+ * On an erased page, bit count should be zero or at least
+ * less then half of the ECC strength
+ */
+ flip = count_written_bits(dat, nfc->chip.ecc.size, ecc_count);
+
+ if (flip > ecc_count && flip > (nfc->chip.ecc.strength / 2)) {
+ nfc->page = -1;
+ return -1;
+ }
+
+ /* Erased page. */
+ memset(dat, 0xff, nfc->chip.ecc.size);
+ return 0;
+}
+
+static int vf610_nfc_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
+ u_char *ecc_code)
+{
+ return 0;
+}
+
+static void vf610_nfc_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+}
+
#ifdef CONFIG_OF_MTD
static const struct of_device_id vf610_nfc_dt_ids[] = {
{ .compatible = "fsl,vf610-nfc" },
@@ -473,7 +579,17 @@ static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config *cfg)
if (!np)
return 1;
+ if (of_get_nand_ecc_mode(np) >= NAND_ECC_HW)
+ cfg->hardware_ecc = 1;
+
cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
+ cfg->ecc_strength = of_get_nand_ecc_strength(np);
+ if (cfg->ecc_strength < 0)
+ cfg->ecc_strength = 0;
+
+ cfg->ecc_step_size = of_get_nand_ecc_step_size(np);
+ if (cfg->ecc_step_size < 0)
+ cfg->ecc_step_size = 0;
buswidth = of_get_nand_bus_width(np);
if (buswidth < 0)
@@ -510,6 +626,20 @@ static int vf610_nfc_init_controller(struct vf610_nfc *nfc)
vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
CONFIG_PAGE_CNT_SHIFT, 1);
+ if (cfg->hardware_ecc) {
+ /* Set ECC_STATUS offset */
+ vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+ CONFIG_ECC_SRAM_ADDR_MASK,
+ CONFIG_ECC_SRAM_ADDR_SHIFT, ECC_SRAM_ADDR);
+
+ vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+ CONFIG_ECC_MODE_MASK,
+ CONFIG_ECC_MODE_SHIFT, cfg->ecc_mode);
+
+ /* Enable ECC_STATUS */
+ vf610_nfc_set(nfc, NFC_FLASH_CONFIG, CONFIG_ECC_SRAM_REQ_BIT);
+ }
+
return 0;
}
@@ -611,6 +741,49 @@ static int vf610_nfc_probe(struct platform_device *pdev)
}
nfc->page_sz += cfg->width == 16 ? 1 : 0;
+ if (cfg->hardware_ecc) {
+ if (mtd->writesize != PAGE_2K && mtd->oobsize < 64) {
+ dev_err(nfc->dev, "Unsupported flash with hwecc\n");
+ err = -ENXIO;
+ goto error;
+ }
+
+ if (cfg->ecc_step_size != mtd->writesize) {
+ dev_err(nfc->dev, "Step size needs to be page size\n");
+ err = -ENXIO;
+ goto error;
+ }
+
+ if (cfg->ecc_strength == 32) {
+ cfg->ecc_mode = ECC_60_BYTE;
+ chip->ecc.bytes = 60;
+ chip->ecc.layout = &vf610_nfc_ecc60;
+ } else if (cfg->ecc_strength == 24) {
+ cfg->ecc_mode = ECC_45_BYTE;
+ chip->ecc.bytes = 45;
+ chip->ecc.layout = &vf610_nfc_ecc45;
+ } else {
+ dev_err(nfc->dev, "Unsupported ECC strength\n");
+ err = -ENXIO;
+ goto error;
+ }
+
+ /* propagate ecc.layout to mtd_info */
+ mtd->ecclayout = chip->ecc.layout;
+ chip->ecc.calculate = vf610_nfc_calculate_ecc;
+ chip->ecc.hwctl = vf610_nfc_enable_hwecc;
+ chip->ecc.correct = vf610_nfc_correct_data;
+ chip->ecc.mode = NAND_ECC_HW;
+
+ chip->ecc.size = PAGE_2K;
+ chip->ecc.strength = cfg->ecc_strength;
+
+ /* set ECC mode according to required ECC strength */
+ vf610_nfc_set_field(nfc, NFC_FLASH_CONFIG,
+ CONFIG_ECC_MODE_MASK,
+ CONFIG_ECC_MODE_SHIFT, cfg->ecc_mode);
+ }
+
/* second phase scan */
if (nand_scan_tail(mtd)) {
err = -ENXIO;
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 3/6] mtd: nand: vf610_nfc: add device tree bindings
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
2015-03-25 16:28 ` [PATCH v4 1/6] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others Stefan Agner
2015-03-25 16:28 ` [PATCH v4 2/6] mtd: nand: vf610_nfc: add hardware BCH-ECC support Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
2015-03-25 16:28 ` [PATCH v4 4/6] ARM: vf610: enable NAND Flash Controller Stefan Agner
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
.../devicetree/bindings/mtd/vf610-nfc.txt | 45 ++++++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/vf610-nfc.txt
diff --git a/Documentation/devicetree/bindings/mtd/vf610-nfc.txt b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
new file mode 100644
index 0000000..5f0d344
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
@@ -0,0 +1,45 @@
+Freescale's NAND flash controller (NFC)
+
+This variant of the Freescale NAND flash controller (NFC) is on Vybrid (vf610),
+MPC5125, MCF54418 and Kinetis K70.
+
+Required properties:
+- compatible: "fsl,vf610-nfc"
+- reg: address range of the NFC
+- interrupts: interrupt of the NFC
+- nand-bus-width: see nand.txt
+- nand-ecc-mode: see nand.txt
+- nand-on-flash-bbt: see nand.txt
+- assigned-clocks: main clock from the SoC, for Vybrid <&clks VF610_CLK_NFC>;
+- assigned-clock-rates: The NAND bus timing is derived from this clock
+ rate and should not exceed maximum timing for any NAND memory chip
+ in a board stuffing. Typical NAND memory timings derived from this
+ clock are found in the SoC hardware reference manual. Furthermore,
+ there might be restrictions on maximum rates when using hardware ECC.
+
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+ representing partitions.
+
+Required properties for hardware ECC:
+- nand-ecc-strength: supported strengths are 24 and 32 bit (see nand.txt)
+- nand-ecc-step-size: step size equals page size, currently only 2k pages are
+ supported
+
+Example:
+
+ nfc: nand@400e0000 {
+ compatible = "fsl,vf610-nfc";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x400e0000 0x4000>;
+ interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks VF610_CLK_NFC>;
+ clock-names = "nfc";
+ assigned-clocks = <&clks VF610_CLK_NFC>;
+ assigned-clock-rates = <33000000>;
+ nand-bus-width = <8>;
+ nand-ecc-mode = "hw";
+ nand-on-flash-bbt;
+ nand-ecc-strength = <32>;
+ nand-ecc-step-size = <2048>;
+ };
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 4/6] ARM: vf610: enable NAND Flash Controller
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
` (2 preceding siblings ...)
2015-03-25 16:28 ` [PATCH v4 3/6] mtd: nand: vf610_nfc: add device tree bindings Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
2015-03-25 16:28 ` [PATCH v4 5/6] ARM: dts: vf610: add NAND flash controller peripherial Stefan Agner
2015-03-25 16:28 ` [PATCH v4 6/6] ARM: dts: vf-colibri: enable NAND flash controller Stefan Agner
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
Enable the NAND Flash Controller driver which is part of the Vybrid
SoC by default.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/mach-imx/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index c8dffce..182e30e 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -635,6 +635,7 @@ config SOC_VF610
select ARM_GIC
select PINCTRL_VF610
select PL310_ERRATA_769419 if CACHE_L2X0
+ select HAVE_NAND_VF610_NFC
help
This enable support for Freescale Vybrid VF610 processor.
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 5/6] ARM: dts: vf610: add NAND flash controller peripherial
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
` (3 preceding siblings ...)
2015-03-25 16:28 ` [PATCH v4 4/6] ARM: vf610: enable NAND Flash Controller Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
2015-03-25 16:28 ` [PATCH v4 6/6] ARM: dts: vf-colibri: enable NAND flash controller Stefan Agner
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
This adds the NAND flash controller (NFC) peripherial. The driver
supports the SLC NAND chips found on Freescale's Vybrid Tower System
Module. The Micron NAND chip on the module needs 4-bit ECC per 512
byte page. Use 24-bit ECC per 2k page, which is supported by the
driver.
Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/vf610-twr.dts | 44 +++++++++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/vfxxx.dtsi | 8 ++++++++
2 files changed, 52 insertions(+)
diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index f64fddc..f42d155 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -287,6 +287,50 @@
status = "okay";
};
+&iomuxc {
+ vf610-twr {
+ pinctrl_nfc_1: nfcgrp_1 {
+ fsl,pins = <
+ VF610_PAD_PTD31__NF_IO15 0x28df
+ VF610_PAD_PTD30__NF_IO14 0x28df
+ VF610_PAD_PTD29__NF_IO13 0x28df
+ VF610_PAD_PTD28__NF_IO12 0x28df
+ VF610_PAD_PTD27__NF_IO11 0x28df
+ VF610_PAD_PTD26__NF_IO10 0x28df
+ VF610_PAD_PTD25__NF_IO9 0x28df
+ VF610_PAD_PTD24__NF_IO8 0x28df
+ VF610_PAD_PTD23__NF_IO7 0x28df
+ VF610_PAD_PTD22__NF_IO6 0x28df
+ VF610_PAD_PTD21__NF_IO5 0x28df
+ VF610_PAD_PTD20__NF_IO4 0x28df
+ VF610_PAD_PTD19__NF_IO3 0x28df
+ VF610_PAD_PTD18__NF_IO2 0x28df
+ VF610_PAD_PTD17__NF_IO1 0x28df
+ VF610_PAD_PTD16__NF_IO0 0x28df
+ VF610_PAD_PTB24__NF_WE_B 0x28c2
+ VF610_PAD_PTB25__NF_CE0_B 0x28c2
+ VF610_PAD_PTB27__NF_RE_B 0x28c2
+ VF610_PAD_PTC26__NF_RB_B 0x283d
+ VF610_PAD_PTC27__NF_ALE 0x28c2
+ VF610_PAD_PTC28__NF_CLE 0x28c2
+ >;
+ };
+ };
+};
+
+&nfc {
+ assigned-clocks = <&clks VF610_CLK_NFC>;
+ assigned-clock-rates = <33000000>;
+ nand-bus-width = <16>;
+ nand-ecc-mode = "hw";
+ nand-on-flash-bbt;
+ nand-ecc-strength = <24>;
+ nand-ecc-step-size = <2048>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_nfc_1>;
+ status = "okay";
+};
+
&uart1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1>;
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 32de809..8c85972 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -507,6 +507,14 @@
status = "disabled";
};
+ nfc: nand@400e0000 {
+ compatible = "fsl,vf610-nfc";
+ reg = <0x400e0000 0x4000>;
+ interrupts = <83 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks VF610_CLK_NFC>;
+ clock-names = "nfc";
+ status = "disabled";
+ };
};
};
};
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 6/6] ARM: dts: vf-colibri: enable NAND flash controller
2015-03-25 16:28 [PATCH v4 0/6] mtd: nand: vf610_nfc: Freescale NFC for VF610 Stefan Agner
` (4 preceding siblings ...)
2015-03-25 16:28 ` [PATCH v4 5/6] ARM: dts: vf610: add NAND flash controller peripherial Stefan Agner
@ 2015-03-25 16:28 ` Stefan Agner
5 siblings, 0 replies; 21+ messages in thread
From: Stefan Agner @ 2015-03-25 16:28 UTC (permalink / raw)
To: dwmw2, computersforpeace
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
shawn.guo, kernel, boris.brezillon, marb, aaron, bpringlemeir,
linux-mtd, devicetree, linux-arm-kernel, linux-kernel,
Stefan Agner
Enable NAND access by adding pinmux and NAND flash controller node
to device tree. The NAND chips currently used on the Colibri VF61
requires 8-bit ECC per 512 byte page, hence specify 32-bit ECC
strength per 2k page size.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/vf-colibri.dtsi | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/vf-colibri.dtsi b/arch/arm/boot/dts/vf-colibri.dtsi
index 5c2b732..76139d7 100644
--- a/arch/arm/boot/dts/vf-colibri.dtsi
+++ b/arch/arm/boot/dts/vf-colibri.dtsi
@@ -46,6 +46,19 @@
pinctrl-0 = <&pinctrl_i2c0>;
};
+&nfc {
+ assigned-clocks = <&clks VF610_CLK_NFC>;
+ assigned-clock-rates = <33000000>;
+ nand-bus-width = <8>;
+ nand-ecc-mode = "hw";
+ nand-on-flash-bbt;
+ nand-ecc-strength = <32>;
+ nand-ecc-step-size = <2048>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_nfc_1>;
+ status = "okay";
+};
+
&pwm0 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm0>;
@@ -141,6 +154,25 @@
>;
};
+ pinctrl_nfc_1: nfcgrp_1 {
+ fsl,pins = <
+ VF610_PAD_PTD23__NF_IO7 0x28df
+ VF610_PAD_PTD22__NF_IO6 0x28df
+ VF610_PAD_PTD21__NF_IO5 0x28df
+ VF610_PAD_PTD20__NF_IO4 0x28df
+ VF610_PAD_PTD19__NF_IO3 0x28df
+ VF610_PAD_PTD18__NF_IO2 0x28df
+ VF610_PAD_PTD17__NF_IO1 0x28df
+ VF610_PAD_PTD16__NF_IO0 0x28df
+ VF610_PAD_PTB24__NF_WE_B 0x28c2
+ VF610_PAD_PTB25__NF_CE0_B 0x28c2
+ VF610_PAD_PTB27__NF_RE_B 0x28c2
+ VF610_PAD_PTC26__NF_RB_B 0x283d
+ VF610_PAD_PTC27__NF_ALE 0x28c2
+ VF610_PAD_PTC28__NF_CLE 0x28c2
+ >;
+ };
+
pinctrl_pwm0: pwm0grp {
fsl,pins = <
VF610_PAD_PTB0__FTM0_CH0 0x1182
--
2.3.3
^ permalink raw reply related [flat|nested] 21+ messages in thread