2007-09-03 07:25:43

by Bryan Wu

[permalink] [raw]
Subject: [PATCH] Blackfin BF54x NAND Flash Controller driver

This is the driver for latest Blackfin BF54x nand flash controller

- use nand_chip and mtd_info common nand driver interface
- provide both PIO and dma operation
- compiled with ezkit bf548 configuration
- use hardware 1-bit ECC
- tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs

Signed-off-by: Bryan Wu <[email protected]>
---
drivers/mtd/nand/Kconfig | 18 +
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/bf54x_nand.c | 796 ++++++++++++++++++++++++++++++++
include/asm-blackfin/mach-bf548/dma.h | 1 +
include/asm-blackfin/mach-bf548/nand.h | 47 ++
5 files changed, 863 insertions(+), 0 deletions(-)
create mode 100644 drivers/mtd/nand/bf54x_nand.c
create mode 100644 include/asm-blackfin/mach-bf548/nand.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 7992c43..87e9f80 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -131,6 +131,24 @@ config MTD_NAND_AU1550
This enables the driver for the NAND flash controller on the
AMD/Alchemy 1550 SOC.

+config MTD_NAND_BF54X
+ tristate "NAND Flash support for Blackfin BF54X SoC DSP"
+ depends on BF54x && MTD_NAND
+ help
+ This enables the NAND flash controller on the BF54X SoC DPSs
+
+ No board specific support is done by this driver, each board
+ must advertise a platform_device for the driver to attach.
+
+config MTD_NAND_BF54X_HWECC
+ bool "BF54X NAND Hardware ECC"
+ depends on MTD_NAND_BF54X
+ help
+ Enable the use of the BF54X's internal ECC generator when
+ using NAND. Early versions of the chip have had problems with
+ incorrect ECC generation, and if using these, the default of
+ software ECC is preferable.
+
config MTD_NAND_RTC_FROM4
tristate "Renesas Flash ROM 4-slot interface board (FROM_BOARD4)"
depends on SH_SOLUTION_ENGINE
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index b16d234..cbf2bab 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MTD_NAND_TOTO) += toto.o
obj-$(CONFIG_MTD_NAND_AUTCPU12) += autcpu12.o
obj-$(CONFIG_MTD_NAND_EDB7312) += edb7312.o
obj-$(CONFIG_MTD_NAND_AU1550) += au1550nd.o
+obj-$(CONFIG_MTD_NAND_BF54X) += bf54x_nand.o
obj-$(CONFIG_MTD_NAND_PPCHAMELEONEVB) += ppchameleonevb.o
obj-$(CONFIG_MTD_NAND_S3C2410) += s3c2410.o
obj-$(CONFIG_MTD_NAND_DISKONCHIP) += diskonchip.o
diff --git a/drivers/mtd/nand/bf54x_nand.c b/drivers/mtd/nand/bf54x_nand.c
new file mode 100644
index 0000000..dccb94a
--- /dev/null
+++ b/drivers/mtd/nand/bf54x_nand.c
@@ -0,0 +1,796 @@
+/* linux/drivers/mtd/nand/bf54x_nand.c
+ *
+ * Copyright 2006-2007 Analog Devices Inc.
+ * http://blackfin.uclinux.org/
+ * Bryan Wu <[email protected]>
+ *
+ * Blackfin BF54x on-chip NAND flash controler driver
+ *
+ * Derived from drivers/mtd/nand/s3c2410.c
+ * Copyright (c) 2007 Ben Dooks <[email protected]>
+ *
+ * Derived from drivers/mtd/nand/cafe.c
+ * Copyright © 2006 Red Hat, Inc.
+ * Copyright © 2006 David Woodhouse <[email protected]>
+ *
+ * Changelog:
+ * 12-Jun-2007 Bryan Wu: Initial version
+ * 18-Jul-2007 Bryan Wu:
+ * - ECC_HW and ECC_SW supported
+ * - DMA supported in ECC_HW
+ * - YAFFS tested as rootfs in both ECC_HW and ECC_SW
+ *
+ * TODO:
+ * Enable JFFS2 over NAND as rootfs
+ *
+ * This program 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+*/
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/partitions.h>
+
+#include <asm/blackfin.h>
+#include <asm/dma.h>
+#include <asm/cacheflush.h>
+#include <asm/mach/nand.h>
+#include <asm/portmux.h>
+
+#define DRV_NAME "bf54x-nand"
+#define DRV_VERSION "1.0"
+#define DRV_AUTHOR "Bryan Wu <[email protected]>"
+#define DRV_DESC "BF54x on-chip NAND FLash Controller Driver"
+
+#ifdef CONFIG_MTD_NAND_BF54X_HWECC
+int hardware_ecc = 1;
+#else
+int hardware_ecc = 0;
+#endif
+
+unsigned short bfin_nfc_pin_req[] = {P_NAND_CE, P_NAND_RB, 0};
+
+/*----------------------------------------------------------------------------
+ * Data structures for bf54x nand flash controller driver
+ */
+
+/* bf54x nand info */
+struct bf54x_nand_info {
+ /* mtd info */
+ struct nand_hw_control controller;
+ struct mtd_info mtd;
+ struct nand_chip chip;
+
+ /* platform info */
+ struct bf54x_nand_platform *platform;
+
+ /* device info */
+ struct device *device;
+
+ /* DMA stuff */
+ struct completion dma_completion;
+};
+
+/*----------------------------------------------------------------------------
+ * Conversion functions
+ */
+static struct bf54x_nand_info *mtd_to_nand_info(struct mtd_info *mtd)
+{
+ return container_of(mtd, struct bf54x_nand_info, mtd);
+}
+
+static struct bf54x_nand_info *to_nand_info(struct platform_device *pdev)
+{
+ return platform_get_drvdata(pdev);
+}
+
+static struct bf54x_nand_platform *to_nand_plat(struct platform_device *pdev)
+{
+ return pdev->dev.platform_data;
+}
+
+
+/*----------------------------------------------------------------------------
+ * struct nand_chip interface function pointers
+ */
+
+/*
+ * bf54x_nand_hwcontrol
+ *
+ * Issue command and address cycles to the chip
+ */
+static void bf54x_nand_hwcontrol(struct mtd_info *mtd, int cmd,
+ unsigned int ctrl)
+{
+ if (cmd == NAND_CMD_NONE)
+ return;
+
+ while (bfin_read_NFC_STAT() & WB_FULL)
+ continue;
+
+ if (ctrl & NAND_CLE)
+ bfin_write_NFC_CMD(cmd);
+ else
+ bfin_write_NFC_ADDR(cmd);
+ SSYNC();
+}
+
+/*
+ * bf54x_nand_devready()
+ *
+ * returns 0 if the nand is busy, 1 if it is ready
+ */
+static int bf54x_nand_devready(struct mtd_info *mtd)
+{
+ unsigned short val = bfin_read_NFC_IRQSTAT();
+
+ if ((val & NBUSYIRQ) == NBUSYIRQ)
+ return 1;
+ else
+ return 0;
+}
+
+/*----------------------------------------------------------------------------
+ * ECC functions
+ *
+ * These allow the bf54x to use the controller's ECC
+ * generator block to ECC the data as it passes through
+ */
+static inline int count_bits(uint32_t byte)
+{
+ int res = 0;
+
+ for (; byte; byte >>= 1)
+ res += byte & 0x01;
+ return res;
+}
+
+/*
+ * ECC error correction function
+ */
+static int bf54x_nand_correct_data_256(struct mtd_info *mtd, u_char *dat,
+ u_char *read_ecc, u_char *calc_ecc)
+{
+ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ u32 syndrome[5];
+ u32 calced, stored;
+ int i;
+ unsigned short failing_bit, failing_byte;
+ u_char data;
+
+ calced = calc_ecc[0] | (calc_ecc[1] << 8) | (calc_ecc[2] << 16);
+ stored = read_ecc[0] | (read_ecc[1] << 8) | (read_ecc[2] << 16);
+
+ syndrome[0] = (calced ^ stored);
+
+ /*
+ * syndrome 0: all zero
+ * No error in data
+ * No action
+ */
+ if (!syndrome[0] || !calced || !stored)
+ return 0;
+
+ /*
+ * sysdrome 0: only one bit is one
+ * ECC data was incorrect
+ * No action
+ */
+ if (count_bits(syndrome[0]) == 1) {
+ dev_err(info->device, "ECC data was incorrect!\n");
+ return 1;
+ }
+
+ syndrome[1] = (calced & 0x7FF) ^ (stored & 0x7FF);
+ syndrome[2] = (calced & 0x7FF) ^ ((calced >> 11) & 0x7FF);
+ syndrome[3] = (stored & 0x7FF) ^ ((stored >> 11) & 0x7FF);
+ syndrome[4] = syndrome[2] ^ syndrome[3];
+
+ for (i = 0; i < 5; i++)
+ dev_info(info->device, "syndrome[%d] 0x%08x\n", i, syndrome[i]);
+
+ dev_info(info->device, "calced[0x%08x], stored[0x%08x]\n", calced, stored);
+
+ /*
+ * sysdrome 0: exactly 11 bits are one, each parity
+ * and parity' pair is 1 & 0 or 0 & 1.
+ * 1-bit correctable error
+ * Correct the error
+ */
+ if (count_bits(syndrome[0]) == 11 && syndrome[4] == 0x7FF) {
+ dev_info(info->device, "1-bit correctable error, correct it.\n");
+ dev_info(info->device, "syndrome[1] 0x%08x\n", syndrome[1]);
+
+ failing_bit = syndrome[1] & 0x7;
+ failing_byte = syndrome[1] >> 0x3;
+ data = *(dat + failing_byte);
+ data = data ^ (0x1 << failing_bit);
+ *(dat + failing_byte) = data;
+
+ return 0;
+ }
+
+ /*
+ * sysdrome 0: random data
+ * More than 1-bit error, non-correctable error
+ * Discard data, mark bad block
+ */
+ dev_err(info->device,
+ "More than 1-bit error, non-correctable error.\n");
+ dev_err(info->device,
+ "Please discard data, mark bad block\n");
+
+ return 1;
+}
+
+static int bf54x_nand_correct_data(struct mtd_info *mtd, u_char *dat,
+ u_char *read_ecc, u_char *calc_ecc)
+{ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ struct bf54x_nand_platform *plat = info->platform;
+ unsigned short page_size = (plat->page_size ? 512 : 256);
+
+ int ret;
+
+ ret = bf54x_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
+
+ /* If page size is 512, correct second 256 bytes */
+ if (page_size == 512) {
+ dat += 256;
+ read_ecc += 8;
+ calc_ecc += 8;
+ ret = bf54x_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
+ }
+
+ return ret;
+}
+
+static void bf54x_nand_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+ /*
+ * This register must be written before each page is
+ * transferred to generate the correct ECC register
+ * values.
+ */
+ return;
+
+}
+
+static int bf54x_nand_calculate_ecc(struct mtd_info *mtd,
+ const u_char *dat, u_char *ecc_code)
+{
+ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ struct bf54x_nand_platform *plat = info->platform;
+ u16 page_size = (plat->page_size ? 512 : 256);
+ u16 ecc0, ecc1;
+ u32 code[2];
+ u8 *p;
+ int bytes = 3, i;
+
+ /* first 4 bytes ECC code for 256 page size */
+ ecc0 = bfin_read_NFC_ECC0();
+ ecc1 = bfin_read_NFC_ECC1();
+
+ code[0] = (ecc0 & 0x3FF) | ((ecc1 & 0x3FF) << 11);
+
+ dev_dbg(info->device, "returning ecc 0x%08x\n", code[0]);
+
+ /* second 4 bytes ECC code for 512 page size */
+ if (page_size == 512) {
+ ecc0 = bfin_read_NFC_ECC2();
+ ecc1 = bfin_read_NFC_ECC3();
+ code[1] = (ecc0 & 0x3FF) | ((ecc1 & 0x3FF) << 11);
+ bytes = 6;
+ dev_dbg(info->device, "returning ecc 0x%08x\n", code[1]);
+ }
+
+ p = (u8 *)code;
+ for (i = 0; i < bytes; i++)
+ ecc_code[i] = p[i];
+
+ return 0;
+}
+
+/*----------------------------------------------------------------------------
+ * PIO mode for buffer writing and reading
+ */
+static void bf54x_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ int i;
+ unsigned short val;
+
+ /*
+ * Data reads are requested by first writing to NFC_DATA_RD
+ * and then reading back from NFC_READ.
+ */
+ for (i = 0; i < len; i++) {
+ while (bfin_read_NFC_STAT() & WB_FULL)
+ continue;
+
+ /* Contents do not matter */
+ bfin_write_NFC_DATA_RD(0x0000);
+ SSYNC();
+
+ while ((bfin_read_NFC_IRQSTAT() & RD_RDY) != RD_RDY)
+ continue;
+
+ buf[i] = bfin_read_NFC_READ();
+
+ val = bfin_read_NFC_IRQSTAT();
+ val |= RD_RDY;
+ bfin_write_NFC_IRQSTAT(val);
+ SSYNC();
+ }
+}
+
+static uint8_t bf54x_nand_read_byte(struct mtd_info *mtd)
+{
+ uint8_t val;
+
+ bf54x_nand_read_buf(mtd, &val, 1);
+
+ return val;
+}
+
+static void bf54x_nand_write_buf(struct mtd_info *mtd,
+ const uint8_t *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ while (bfin_read_NFC_STAT() & WB_FULL)
+ continue;
+
+ bfin_write_NFC_DATA_WR(buf[i]);
+ SSYNC();
+ }
+}
+
+static void bf54x_nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ int i;
+ u16 *p = (u16 *) buf;
+ len >>= 1;
+
+ /*
+ * Data reads are requested by first writing to NFC_DATA_RD
+ * and then reading back from NFC_READ.
+ */
+ bfin_write_NFC_DATA_RD(0x5555);
+
+ SSYNC();
+
+ for (i = 0; i < len; i++)
+ p[i] = bfin_read_NFC_READ();
+}
+
+static void bf54x_nand_write_buf16(struct mtd_info *mtd,
+ const uint8_t *buf, int len)
+{
+ int i;
+ u16 *p = (u16 *) buf;
+ len >>= 1;
+
+ for (i = 0; i < len; i++)
+ bfin_write_NFC_DATA_WR(p[i]);
+
+ SSYNC();
+}
+
+/*----------------------------------------------------------------------------
+ * DMA functions for buffer writing and reading
+ */
+static irqreturn_t bf54x_nand_dma_irq (int irq, void *dev_id)
+{
+ struct bf54x_nand_info *info = (struct bf54x_nand_info *) dev_id;
+
+ clear_dma_irqstat(CH_NFC);
+ disable_dma(CH_NFC);
+ complete(&info->dma_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int bf54x_nand_dma_rw(struct mtd_info *mtd,
+ uint8_t *buf, int is_read)
+{
+ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ struct bf54x_nand_platform *plat = info->platform;
+ unsigned short page_size = (plat->page_size ? 512 : 256);
+ unsigned short val;
+
+ dev_dbg(info->device, " mtd->%p, buf->%p, len %d, is_read %d\n",
+ mtd, buf, is_read);
+
+ if (is_read)
+ invalidate_dcache_range((unsigned int)buf,
+ (unsigned int)(buf + page_size));
+ else
+ flush_dcache_range((unsigned int)buf,
+ (unsigned int)(buf + page_size));
+
+ bfin_write_NFC_RST(0x1);
+ SSYNC();
+
+ disable_dma(CH_NFC);
+ clear_dma_irqstat(CH_NFC);
+
+ /* setup DMA register with Blackfin DMA API */
+ set_dma_config(CH_NFC, 0x0);
+ set_dma_start_addr(CH_NFC, (unsigned long) buf);
+ set_dma_x_count(CH_NFC, (page_size >> 2));
+ set_dma_x_modify(CH_NFC, 4);
+
+ /* setup write or read operation */
+ val = DI_EN | WDSIZE_32;
+ if (is_read)
+ val |= WNR;
+ set_dma_config(CH_NFC, val);
+ enable_dma(CH_NFC);
+
+ /* Start PAGE read/write operation */
+ if (is_read)
+ bfin_write_NFC_PGCTL(0x1);
+ else
+ bfin_write_NFC_PGCTL(0x2);
+ wait_for_completion(&info->dma_completion);
+
+ return 0;
+}
+
+static void bf54x_nand_dma_read_buf(struct mtd_info *mtd,
+ uint8_t *buf, int len)
+{
+ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ struct bf54x_nand_platform *plat = info->platform;
+ unsigned short page_size = (plat->page_size ? 512 : 256);
+
+ dev_dbg(info->device, "mtd->%p, buf->%p, int %d\n", mtd, buf, len);
+
+ if (len == page_size)
+ bf54x_nand_dma_rw(mtd, buf, 1);
+ else
+ bf54x_nand_read_buf(mtd, buf, len);
+}
+
+static void bf54x_nand_dma_write_buf(struct mtd_info *mtd,
+ const uint8_t *buf, int len)
+{
+ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
+ struct bf54x_nand_platform *plat = info->platform;
+ unsigned short page_size = (plat->page_size ? 512 : 256);
+
+ dev_dbg(info->device, "mtd->%p, buf->%p, len %d\n", mtd, buf, len);
+
+ if (len == page_size)
+ bf54x_nand_dma_rw(mtd, (uint8_t *)buf, 0);
+ else
+ bf54x_nand_write_buf(mtd, buf, len);
+}
+
+/*----------------------------------------------------------------------------
+ * System initialization functions
+ */
+
+static int bf54x_nand_dma_init(struct bf54x_nand_info *info)
+{
+ int ret;
+ unsigned short val;
+
+ /* Do not use dma */
+ if (!hardware_ecc)
+ return 0;
+
+ init_completion(&info->dma_completion);
+
+ /* Setup DMAC1 channel mux for NFC which shared with SDH */
+ val = bfin_read_DMAC1_PERIMUX();
+ val &= 0xFFFE;
+ bfin_write_DMAC1_PERIMUX(val);
+ SSYNC();
+
+ /* Request NFC DMA channel */
+ ret = request_dma(CH_NFC, "BF54X NFC driver");
+ if (ret < 0) {
+ dev_err(info->device, " unable to get DMA channel\n");
+ return ret;
+ }
+
+ set_dma_callback(CH_NFC, (void *) bf54x_nand_dma_irq, (void *) info);
+
+ /* Turn off the DMA channel first */
+ disable_dma(CH_NFC);
+ return 0;
+}
+
+/*
+ * BF54X NFC hardware initialization
+ * - pin mux setup
+ * - clear interrupt status
+ */
+static int bf54x_nand_hw_init(struct bf54x_nand_info *info)
+{
+ int err = 0;
+ unsigned short val;
+ struct bf54x_nand_platform *plat = info->platform;
+
+ if (!info)
+ return -EINVAL;
+
+ /* setup NFC_CTL register */
+ dev_info(info->device,
+ "page_size=%d, data_width=%d, wr_dly=%d, rd_dly=%d\n",
+ (plat->page_size ? 512 : 256),
+ (plat->data_width ? 16 : 8),
+ plat->wr_dly, plat->rd_dly);
+
+ val = (plat->page_size << NFC_PG_SIZE_OFFSET) |
+ (plat->data_width << NFC_NWIDTH_OFFSET) |
+ (plat->rd_dly << NFC_RDDLY_OFFSET) |
+ (plat->rd_dly << NFC_WRDLY_OFFSET);
+ dev_dbg(info->device, "NFC_CTL is 0x%04x\n", val);
+
+ bfin_write_NFC_CTL(val);
+ SSYNC();
+
+ /* clear interrupt status */
+ bfin_write_NFC_IRQMASK(0x0);
+ SSYNC();
+ val = bfin_read_NFC_IRQSTAT();
+ bfin_write_NFC_IRQSTAT(val);
+ SSYNC();
+
+ if (peripheral_request_list(bfin_nfc_pin_req, DRV_NAME)) {
+ printk(KERN_ERR DRV_NAME
+ ": Requesting Peripherals failed\n");
+ return -EFAULT;
+ }
+
+
+ /* DMA initialization */
+ if (bf54x_nand_dma_init(info)) {
+ err = -ENXIO;
+ }
+
+ return err;
+}
+
+/*----------------------------------------------------------------------------
+ * Device management interface
+ */
+static int bf54x_nand_add_partition(struct bf54x_nand_info *info)
+{
+ struct mtd_info *mtd = &info->mtd;
+
+#ifdef CONFIG_MTD_PARTITIONS
+ struct mtd_partition *parts = info->platform->partitions;
+ int nr = info->platform->nr_partitions;
+
+ return add_mtd_partitions(mtd, parts, nr);
+#else
+ return add_mtd_device(mtd);
+#endif
+}
+
+static int bf54x_nand_remove(struct platform_device *pdev)
+{
+ struct bf54x_nand_info *info = to_nand_info(pdev);
+ struct mtd_info *mtd = NULL;
+
+ platform_set_drvdata(pdev, NULL);
+
+ if (!info)
+ return 0;
+
+ /* first thing we need to do is release all our mtds
+ * and their partitions, then go through freeing the
+ * resources used
+ */
+ mtd = &info->mtd;
+ if (mtd) {
+ nand_release(mtd);
+ kfree(mtd);
+ }
+
+ peripheral_free_list(bfin_nfc_pin_req);
+
+ /* free the common resources */
+ kfree(info);
+
+ return 0;
+}
+
+/*
+ * bf54x_nand_probe
+ *
+ * called by device layer when it finds a device matching
+ * one our driver can handled. This code checks to see if
+ * it can allocate all necessary resources then calls the
+ * nand layer to look for devices
+ */
+static int bf54x_nand_probe(struct platform_device *pdev)
+{
+ struct bf54x_nand_platform *plat = to_nand_plat(pdev);
+ struct bf54x_nand_info *info = NULL;
+ struct nand_chip *chip = NULL;
+ struct mtd_info *mtd = NULL;
+ int err = 0;
+
+ dev_dbg(&pdev->dev, "(%p)\n", pdev);
+
+ if (!plat) {
+ dev_err(&pdev->dev, "no platform specific information\n");
+ goto exit_error;
+ }
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (info == NULL) {
+ dev_err(&pdev->dev, "no memory for flash info\n");
+ err = -ENOMEM;
+ goto exit_error;
+ }
+
+ platform_set_drvdata(pdev, info);
+
+ spin_lock_init(&info->controller.lock);
+ init_waitqueue_head(&info->controller.wq);
+
+ info->device = &pdev->dev;
+ info->platform = plat;
+
+ /* initialise chip data struct */
+ chip = &info->chip;
+
+ if (plat->data_width)
+ chip->options |= NAND_BUSWIDTH_16;
+
+ chip->options |= NAND_CACHEPRG | NAND_SKIP_BBTSCAN;
+
+ chip->read_buf = (plat->data_width) ?
+ bf54x_nand_read_buf16 : bf54x_nand_read_buf;
+ chip->write_buf = (plat->data_width) ?
+ bf54x_nand_write_buf16 : bf54x_nand_write_buf;
+
+ chip->read_byte = bf54x_nand_read_byte;
+
+ chip->cmd_ctrl = bf54x_nand_hwcontrol;
+ chip->dev_ready = bf54x_nand_devready;
+
+ chip->priv = &info->mtd;
+ chip->controller = &info->controller;
+
+ chip->IO_ADDR_R = (void __iomem *) NFC_READ;
+ chip->IO_ADDR_W = (void __iomem *) NFC_DATA_WR;
+
+ chip->chip_delay = 0;
+
+ /* initialise mtd info data struct */
+ mtd = &info->mtd;
+ mtd->priv = chip;
+ mtd->owner = THIS_MODULE;
+
+ /* initialise the hardware */
+ err = bf54x_nand_hw_init(info);
+ if (err != 0)
+ goto exit_error;
+
+ /* setup hardware ECC data struct */
+ if (hardware_ecc) {
+ if (plat->page_size == NFC_PG_SIZE_256) {
+ chip->ecc.bytes = 3;
+ chip->ecc.size = 256;
+ } else if (mtd->writesize == NFC_PG_SIZE_512) {
+ chip->ecc.bytes = 6;
+ chip->ecc.size = 512;
+ }
+
+ chip->read_buf = bf54x_nand_dma_read_buf;
+ chip->write_buf = bf54x_nand_dma_write_buf;
+ chip->ecc.calculate = bf54x_nand_calculate_ecc;
+ chip->ecc.correct = bf54x_nand_correct_data;
+ chip->ecc.mode = NAND_ECC_HW;
+ chip->ecc.hwctl = bf54x_nand_enable_hwecc;
+ } else {
+ chip->ecc.mode = NAND_ECC_SOFT;
+ }
+
+ /* scan hardware nand chip and setup mtd info data struct */
+ if (nand_scan(mtd, 1)) {
+ err = -ENXIO;
+ goto exit_error;
+ }
+
+ /* add NAND partition */
+ bf54x_nand_add_partition(info);
+
+ dev_dbg(&pdev->dev, "initialised ok\n");
+ return 0;
+
+exit_error:
+ bf54x_nand_remove(pdev);
+
+ if (err == 0)
+ err = -EINVAL;
+ return err;
+}
+
+/* PM Support */
+#ifdef CONFIG_PM
+
+static int bf54x_nand_suspend(struct platform_device *dev, pm_message_t pm)
+{
+ struct bf54x_nand_info *info = platform_get_drvdata(dev);
+
+ return 0;
+}
+
+static int bf54x_nand_resume(struct platform_device *dev)
+{
+ struct bf54x_nand_info *info = platform_get_drvdata(dev);
+
+ if (info)
+ bf54x_nand_hw_init(info);
+
+ return 0;
+}
+
+#else
+#define bf54x_nand_suspend NULL
+#define bf54x_nand_resume NULL
+#endif
+
+/* driver device registration */
+static struct platform_driver bf54x_nand_driver = {
+ .probe = bf54x_nand_probe,
+ .remove = bf54x_nand_remove,
+ .suspend = bf54x_nand_suspend,
+ .resume = bf54x_nand_resume,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init bf54x_nand_init(void)
+{
+ printk(KERN_INFO "%s, Version %s (c) 2007 Analog Devices, Inc.\n",
+ DRV_DESC, DRV_VERSION);
+
+ return platform_driver_register(&bf54x_nand_driver);
+}
+
+static void __exit bf54x_nand_exit(void)
+{
+ platform_driver_unregister(&bf54x_nand_driver);
+}
+
+module_init(bf54x_nand_init);
+module_exit(bf54x_nand_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR(DRV_AUTHOR);
+MODULE_DESCRIPTION(DRV_DESC);
diff --git a/include/asm-blackfin/mach-bf548/dma.h b/include/asm-blackfin/mach-bf548/dma.h
index fcc8b4c..14cb10c 100644
--- a/include/asm-blackfin/mach-bf548/dma.h
+++ b/include/asm-blackfin/mach-bf548/dma.h
@@ -55,6 +55,7 @@
#define CH_SPORT3_RX 20
#define CH_SPORT3_TX 21
#define CH_SDH 22
+#define CH_NFC 22
#define CH_SPI2 23

#define CH_MEM_STREAM0_DEST 24
diff --git a/include/asm-blackfin/mach-bf548/nand.h b/include/asm-blackfin/mach-bf548/nand.h
new file mode 100644
index 0000000..a77fd46
--- /dev/null
+++ b/include/asm-blackfin/mach-bf548/nand.h
@@ -0,0 +1,47 @@
+/* linux/include/asm-blackfin/mach-bf548/nand.h
+ *
+ * Copyright (c) 2007 Analog Devices, Inc.
+ * Bryan Wu <[email protected]>
+ *
+ * BF54X - NAND flash controller platfrom_device info
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* struct bf54x_nand_platform
+ *
+ * define a interface between platfrom board specific code and
+ * bf54x NFC driver.
+ *
+ * nr_partitions = number of partitions pointed to be partitoons (or zero)
+ * partitions = mtd partition list
+ */
+
+#define NFC_PG_SIZE_256 0
+#define NFC_PG_SIZE_512 1
+#define NFC_PG_SIZE_OFFSET 9
+
+#define NFC_NWIDTH_8 0
+#define NFC_NWIDTH_16 1
+#define NFC_NWIDTH_OFFSET 8
+
+#define NFC_RDDLY_OFFSET 4
+#define NFC_WRDLY_OFFSET 0
+
+#define NFC_STAT_NBUSY 1
+
+struct bf54x_nand_platform {
+ /* NAND chip information */
+ unsigned short page_size;
+ unsigned short data_width;
+
+ /* RD/WR strobe delay timing information, all times in SCLK cycles */
+ unsigned short rd_dly;
+ unsigned short wr_dly;
+
+ /* NAND MTD partition information */
+ int nr_partitions;
+ struct mtd_partition *partitions;
+};
--
1.5.2


2007-09-03 10:14:21

by Clemens Koller

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

Hi, Bryan!

Bryan Wu schrieb:
> This is the driver for latest Blackfin BF54x nand flash controller
>
> - use nand_chip and mtd_info common nand driver interface
> - provide both PIO and dma operation
> - compiled with ezkit bf548 configuration
> - use hardware 1-bit ECC
> - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs

Looks great!

Will this driver also work for the new BF52x series of the Blackfin Series?
(Or is it planned to make it work for... anytime soon.)

Thank you!
--
Clemens Koller
__________________________________
R&D Imaging Devices
Anagramm GmbH
Rupert-Mayer-Straße 45/1
Linhof Werksgelände
D-81379 München
Tel.089-741518-50
Fax 089-741518-19
http://www.anagramm-technology.com

2007-09-03 16:46:46

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Mon, 2007-09-03 at 15:25 +0800, Bryan Wu wrote:
> + if (hardware_ecc) {
> + if (plat->page_size == NFC_PG_SIZE_256) {
> + chip->ecc.bytes = 3;
> + chip->ecc.size = 256;
> + } else if (mtd->writesize == NFC_PG_SIZE_512) {

Comparing against plat->page_size in one case, but mtd->writesize in the
other? And elsewhere it seems plat->page_size is treated as a boolean,
indicating only 256-byte vs. 512-byte pages (you don't support 2KiB or
other page sizes at all?)

--
dwmw2

2007-09-03 17:34:17

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Mon, 2007-09-03 at 17:46 +0100, David Woodhouse wrote:
> On Mon, 2007-09-03 at 15:25 +0800, Bryan Wu wrote:
> > + if (hardware_ecc) {
> > + if (plat->page_size == NFC_PG_SIZE_256) {
> > + chip->ecc.bytes = 3;
> > + chip->ecc.size = 256;
> > + } else if (mtd->writesize == NFC_PG_SIZE_512) {
>
> Comparing against plat->page_size in one case, but mtd->writesize in the
> other?

This is a typo in development. I will fix it ASAP. -:)

> And elsewhere it seems plat->page_size is treated as a boolean,
> indicating only 256-byte vs. 512-byte pages (you don't support 2KiB or
> other page sizes at all?)
>

When enabled hardware ECC, NFC of BF54x supports 256 and 512 bytes page
size ECC.
it's maybe little confusing with the NAND chip's pagesize. When NAND
chip is 2KiB or other page size, this driver can use multiple 256/512
pages to do hardware ECC. And it's handled by the driver software here.

-Bryan Wu

2007-09-03 17:58:19

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On 9/3/07, Bryan Wu <[email protected]> wrote:
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -131,6 +131,24 @@ config MTD_NAND_AU1550
> +config MTD_NAND_BF54X
> + tristate "NAND Flash support for Blackfin BF54X SoC DSP"

i'd just describe it as "Blackfin on-chip NAND" rather than sticking
in exact part numbers as i imagine we can extend it down the line

> + This enables the NAND flash controller on the BF54X SoC DPSs

"BF54X SoC DPSs" -> "BF54x SoC DSPs"

> + No board specific support is done by this driver, each board
> + must advertise a platform_device for the driver to attach.

should mention the module name when built as a module

> +config MTD_NAND_BF54X_HWECC
> + bool "BF54X NAND Hardware ECC"
> + depends on MTD_NAND_BF54X
> + help
> + Enable the use of the BF54X's internal ECC generator when
> + using NAND. Early versions of the chip have had problems with
> + incorrect ECC generation, and if using these, the default of
> + software ECC is preferable.

rather than advertising this, i'd just keep it in the driver ...
presumably there are anomaly #'s for when the ECC is wrong, so we can
make the code depend on those
-mike

2007-09-03 17:59:09

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On 9/3/07, Clemens Koller <[email protected]> wrote:
> Bryan Wu schrieb:
> > This is the driver for latest Blackfin BF54x nand flash controller
> >
> > - use nand_chip and mtd_info common nand driver interface
> > - provide both PIO and dma operation
> > - compiled with ezkit bf548 configuration
> > - use hardware 1-bit ECC
> > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
>
> Looks great!
>
> Will this driver also work for the new BF52x series of the Blackfin Series?
> (Or is it planned to make it work for... anytime soon.)

the order for porting of new chips is:
u-boot -> core kernel -> peripherals

we generally dont bother looking at peripherals until the earlier stuff is done
-mike

2007-09-04 03:25:00

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Mon, 2007-09-03 at 13:57 -0400, Mike Frysinger wrote:
> On 9/3/07, Bryan Wu <[email protected]> wrote:
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -131,6 +131,24 @@ config MTD_NAND_AU1550
> > +config MTD_NAND_BF54X
> > + tristate "NAND Flash support for Blackfin BF54X SoC DSP"
>
> i'd just describe it as "Blackfin on-chip NAND" rather than sticking
> in exact part numbers as i imagine we can extend it down the line
>
> > + This enables the NAND flash controller on the BF54X SoC DPSs
>
> "BF54X SoC DPSs" -> "BF54x SoC DSPs"
>
> > + No board specific support is done by this driver, each board
> > + must advertise a platform_device for the driver to attach.
>
> should mention the module name when built as a module
>

I will get rid of this BF54X and use BF5XX for (BF54x and BF52x and
future processor)

> > +config MTD_NAND_BF54X_HWECC
> > + bool "BF54X NAND Hardware ECC"
> > + depends on MTD_NAND_BF54X
> > + help
> > + Enable the use of the BF54X's internal ECC generator when
> > + using NAND. Early versions of the chip have had problems with
> > + incorrect ECC generation, and if using these, the default of
> > + software ECC is preferable.
>
> rather than advertising this, i'd just keep it in the driver ...
> presumably there are anomaly #'s for when the ECC is wrong, so we can
> make the code depend on those
> -mike

This option is quite common in the NAND driver. If there are anomaly
#'s, code can still depend on those by using hardware_ecc varible. This
option is very useful when someone need software_ecc.

Thanks
- Bryan Wu

2007-09-13 08:38:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:

> This is the driver for latest Blackfin BF54x nand flash controller
>
> - use nand_chip and mtd_info common nand driver interface
> - provide both PIO and dma operation
> - compiled with ezkit bf548 configuration
> - use hardware 1-bit ECC
> - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
>
> ...
>
> +int hardware_ecc = 0;

scripts/checkpatch.pl, please.

> +#endif
> +
> +unsigned short bfin_nfc_pin_req[] = {P_NAND_CE, P_NAND_RB, 0};

static. Please review whole patch for this.

> +/*
> + * bf54x_nand_hwcontrol
> + *
> + * Issue command and address cycles to the chip
> + */
> +static void bf54x_nand_hwcontrol(struct mtd_info *mtd, int cmd,
> + unsigned int ctrl)
> +{
> + if (cmd == NAND_CMD_NONE)
> + return;
> +
> + while (bfin_read_NFC_STAT() & WB_FULL)
> + continue;

cpu_relax(). Please check the whole patch for this too.

> + if (ctrl & NAND_CLE)
> + bfin_write_NFC_CMD(cmd);
> + else
> + bfin_write_NFC_ADDR(cmd);
> + SSYNC();
> +}
> +
>
> ...
>
> +/*----------------------------------------------------------------------------
> + * ECC functions
> + *
> + * These allow the bf54x to use the controller's ECC
> + * generator block to ECC the data as it passes through
> + */
> +static inline int count_bits(uint32_t byte)
> +{
> + int res = 0;
> +
> + for (; byte; byte >>= 1)
> + res += byte & 0x01;
> + return res;
> +}

delete this, use hweight32() at its callsites.

> +
> +static int bf54x_nand_correct_data(struct mtd_info *mtd, u_char *dat,
> + u_char *read_ecc, u_char *calc_ecc)
> +{ struct bf54x_nand_info *info = mtd_to_nand_info(mtd);

missing newline

> + struct bf54x_nand_platform *plat = info->platform;
> + unsigned short page_size = (plat->page_size ? 512 : 256);
> +
> + int ret;

extraneous newline

> + ret = bf54x_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
> +
> + /* If page size is 512, correct second 256 bytes */
> + if (page_size == 512) {
> + dat += 256;
> + read_ecc += 8;
> + calc_ecc += 8;
> + ret = bf54x_nand_correct_data_256(mtd, dat, read_ecc, calc_ecc);
> + }
> +
> + return ret;
> +}
> +
> +static void bf54x_nand_enable_hwecc(struct mtd_info *mtd, int mode)
> +{
> + /*
> + * This register must be written before each page is
> + * transferred to generate the correct ECC register
> + * values.
> + */
> + return;
> +
> +}

comment doesn't match code.

extraneous newline

> +
> +/*----------------------------------------------------------------------------
> + * PIO mode for buffer writing and reading
> + */

remove the ------------------------------ thingy. (whole patch)

> +static void bf54x_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> + int i;
> + unsigned short val;
> +
> + /*
> + * Data reads are requested by first writing to NFC_DATA_RD
> + * and then reading back from NFC_READ.
> + */
> + for (i = 0; i < len; i++) {
> + while (bfin_read_NFC_STAT() & WB_FULL)
> + continue;

cpu_relax()

> + /* Contents do not matter */
> + bfin_write_NFC_DATA_RD(0x0000);
> + SSYNC();
> +
> + while ((bfin_read_NFC_IRQSTAT() & RD_RDY) != RD_RDY)
> + continue;
> +
> + buf[i] = bfin_read_NFC_READ();
> +
> + val = bfin_read_NFC_IRQSTAT();
> + val |= RD_RDY;
> + bfin_write_NFC_IRQSTAT(val);
> + SSYNC();
> + }
> +}
> +
> +static uint8_t bf54x_nand_read_byte(struct mtd_info *mtd)
> +{
> + uint8_t val;
> +
> + bf54x_nand_read_buf(mtd, &val, 1);
> +
> + return val;
> +}
> +
> +static void bf54x_nand_write_buf(struct mtd_info *mtd,
> + const uint8_t *buf, int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++) {
> + while (bfin_read_NFC_STAT() & WB_FULL)
> + continue;

dittoes

> + bfin_write_NFC_DATA_WR(buf[i]);
> + SSYNC();
> + }
> +}
> +
>
> ...
>
> +/*----------------------------------------------------------------------------
> + * DMA functions for buffer writing and reading
> + */
> +static irqreturn_t bf54x_nand_dma_irq (int irq, void *dev_id)
> +{
> + struct bf54x_nand_info *info = (struct bf54x_nand_info *) dev_id;

unneeded and undesirable cast of void*

> +
> + clear_dma_irqstat(CH_NFC);
> + disable_dma(CH_NFC);
> + complete(&info->dma_completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int bf54x_nand_dma_rw(struct mtd_info *mtd,
> + uint8_t *buf, int is_read)
> +{
> + struct bf54x_nand_info *info = mtd_to_nand_info(mtd);
> + struct bf54x_nand_platform *plat = info->platform;
> + unsigned short page_size = (plat->page_size ? 512 : 256);
> + unsigned short val;
> +
> + dev_dbg(info->device, " mtd->%p, buf->%p, len %d, is_read %d\n",
> + mtd, buf, is_read);
> +
> + if (is_read)
> + invalidate_dcache_range((unsigned int)buf,
> + (unsigned int)(buf + page_size));
> + else
> + flush_dcache_range((unsigned int)buf,
> + (unsigned int)(buf + page_size));

I'd have though that the MM tricks here need a comment so readers know
what's going on?

> + bfin_write_NFC_RST(0x1);
> + SSYNC();
> +
> + disable_dma(CH_NFC);
> + clear_dma_irqstat(CH_NFC);
> +
> + /* setup DMA register with Blackfin DMA API */
> + set_dma_config(CH_NFC, 0x0);
> + set_dma_start_addr(CH_NFC, (unsigned long) buf);
> + set_dma_x_count(CH_NFC, (page_size >> 2));
> + set_dma_x_modify(CH_NFC, 4);
> +
> + /* setup write or read operation */
> + val = DI_EN | WDSIZE_32;
> + if (is_read)
> + val |= WNR;
> + set_dma_config(CH_NFC, val);
> + enable_dma(CH_NFC);
> +
> + /* Start PAGE read/write operation */
> + if (is_read)
> + bfin_write_NFC_PGCTL(0x1);
> + else
> + bfin_write_NFC_PGCTL(0x2);
> + wait_for_completion(&info->dma_completion);
> +
> + return 0;
> +}
> +
>
> ...
>
> +/*
> + * BF54X NFC hardware initialization
> + * - pin mux setup
> + * - clear interrupt status
> + */
> +static int bf54x_nand_hw_init(struct bf54x_nand_info *info)
> +{
> + int err = 0;
> + unsigned short val;
> + struct bf54x_nand_platform *plat = info->platform;
> +
> + if (!info)
> + return -EINVAL;

can this happen?

> + /* setup NFC_CTL register */
> + dev_info(info->device,
> + "page_size=%d, data_width=%d, wr_dly=%d, rd_dly=%d\n",
> + (plat->page_size ? 512 : 256),
> + (plat->data_width ? 16 : 8),
> + plat->wr_dly, plat->rd_dly);
> +
> + val = (plat->page_size << NFC_PG_SIZE_OFFSET) |
> + (plat->data_width << NFC_NWIDTH_OFFSET) |
> + (plat->rd_dly << NFC_RDDLY_OFFSET) |
> + (plat->rd_dly << NFC_WRDLY_OFFSET);
> + dev_dbg(info->device, "NFC_CTL is 0x%04x\n", val);
> +
> + bfin_write_NFC_CTL(val);
> + SSYNC();
> +
> + /* clear interrupt status */
> + bfin_write_NFC_IRQMASK(0x0);
> + SSYNC();
> + val = bfin_read_NFC_IRQSTAT();
> + bfin_write_NFC_IRQSTAT(val);
> + SSYNC();
> +
> + if (peripheral_request_list(bfin_nfc_pin_req, DRV_NAME)) {
> + printk(KERN_ERR DRV_NAME
> + ": Requesting Peripherals failed\n");
> + return -EFAULT;
> + }
> +
> +

extraneous newline

> + /* DMA initialization */
> + if (bf54x_nand_dma_init(info)) {
> + err = -ENXIO;
> + }
> +
> + return err;
> +}
> +
>
> ...
>
> +static int bf54x_nand_remove(struct platform_device *pdev)
> +{
> + struct bf54x_nand_info *info = to_nand_info(pdev);
> + struct mtd_info *mtd = NULL;
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + if (!info)
> + return 0;

can this happen?

> + /* first thing we need to do is release all our mtds
> + * and their partitions, then go through freeing the
> + * resources used
> + */
> + mtd = &info->mtd;
> + if (mtd) {
> + nand_release(mtd);
> + kfree(mtd);
> + }
> +
> + peripheral_free_list(bfin_nfc_pin_req);
> +
> + /* free the common resources */
> + kfree(info);
> +
> + return 0;
> +}
> +
> +/*
> + * bf54x_nand_probe
> + *
> + * called by device layer when it finds a device matching
> + * one our driver can handled. This code checks to see if
> + * it can allocate all necessary resources then calls the
> + * nand layer to look for devices
> + */
> +static int bf54x_nand_probe(struct platform_device *pdev)
> +{
> + struct bf54x_nand_platform *plat = to_nand_plat(pdev);
> + struct bf54x_nand_info *info = NULL;
> + struct nand_chip *chip = NULL;
> + struct mtd_info *mtd = NULL;
> + int err = 0;
> +
> + dev_dbg(&pdev->dev, "(%p)\n", pdev);
> +
> + if (!plat) {
> + dev_err(&pdev->dev, "no platform specific information\n");
> + goto exit_error;

and can this?

> + }
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (info == NULL) {
> + dev_err(&pdev->dev, "no memory for flash info\n");
> + err = -ENOMEM;
> + goto exit_error;
> + }
> +
> + platform_set_drvdata(pdev, info);
> +
> + spin_lock_init(&info->controller.lock);
> + init_waitqueue_head(&info->controller.wq);
> +
> + info->device = &pdev->dev;
> + info->platform = plat;
> +
> + /* initialise chip data struct */
> + chip = &info->chip;
> +
> + if (plat->data_width)
> + chip->options |= NAND_BUSWIDTH_16;
> +
> + chip->options |= NAND_CACHEPRG | NAND_SKIP_BBTSCAN;
> +
> + chip->read_buf = (plat->data_width) ?
> + bf54x_nand_read_buf16 : bf54x_nand_read_buf;
> + chip->write_buf = (plat->data_width) ?
> + bf54x_nand_write_buf16 : bf54x_nand_write_buf;
> +
> + chip->read_byte = bf54x_nand_read_byte;
> +
> + chip->cmd_ctrl = bf54x_nand_hwcontrol;
> + chip->dev_ready = bf54x_nand_devready;
> +
> + chip->priv = &info->mtd;
> + chip->controller = &info->controller;
> +
> + chip->IO_ADDR_R = (void __iomem *) NFC_READ;
> + chip->IO_ADDR_W = (void __iomem *) NFC_DATA_WR;
> +
> + chip->chip_delay = 0;
> +
> + /* initialise mtd info data struct */
> + mtd = &info->mtd;
> + mtd->priv = chip;
> + mtd->owner = THIS_MODULE;
> +
> + /* initialise the hardware */
> + err = bf54x_nand_hw_init(info);
> + if (err != 0)
> + goto exit_error;
> +
> + /* setup hardware ECC data struct */
> + if (hardware_ecc) {
> + if (plat->page_size == NFC_PG_SIZE_256) {
> + chip->ecc.bytes = 3;
> + chip->ecc.size = 256;
> + } else if (mtd->writesize == NFC_PG_SIZE_512) {
> + chip->ecc.bytes = 6;
> + chip->ecc.size = 512;
> + }
> +
> + chip->read_buf = bf54x_nand_dma_read_buf;
> + chip->write_buf = bf54x_nand_dma_write_buf;
> + chip->ecc.calculate = bf54x_nand_calculate_ecc;
> + chip->ecc.correct = bf54x_nand_correct_data;
> + chip->ecc.mode = NAND_ECC_HW;
> + chip->ecc.hwctl = bf54x_nand_enable_hwecc;
> + } else {
> + chip->ecc.mode = NAND_ECC_SOFT;
> + }
> +
> + /* scan hardware nand chip and setup mtd info data struct */
> + if (nand_scan(mtd, 1)) {
> + err = -ENXIO;
> + goto exit_error;
> + }
> +
> + /* add NAND partition */
> + bf54x_nand_add_partition(info);
> +
> + dev_dbg(&pdev->dev, "initialised ok\n");
> + return 0;
> +
> +exit_error:
> + bf54x_nand_remove(pdev);
> +
> + if (err == 0)
> + err = -EINVAL;
> + return err;
> +}
> +
> + if (info)
> + bf54x_nand_hw_init(info);
> +
> + return 0;
> +}
> +

2007-09-13 08:45:25

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On 9/13/07, Andrew Morton <[email protected]> wrote:
> On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:
> > This is the driver for latest Blackfin BF54x nand flash controller
> >
> > - use nand_chip and mtd_info common nand driver interface
> > - provide both PIO and dma operation
> > - compiled with ezkit bf548 configuration
> > - use hardware 1-bit ECC
> > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> >
> > ...
> >
> > +int hardware_ecc = 0;
>
> scripts/checkpatch.pl, please.

that isnt static so checkpatch.pl wouldnt have caught it ... it should
be static though of course and then checkpatch.pl would have thrown a
static init 0 warning :)
-mike

2007-09-13 08:49:00

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Thu, 2007-09-13 at 04:45 -0400, Mike Frysinger wrote:
> On 9/13/07, Andrew Morton <[email protected]> wrote:
> > On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:
> > > This is the driver for latest Blackfin BF54x nand flash controller
> > >
> > > - use nand_chip and mtd_info common nand driver interface
> > > - provide both PIO and dma operation
> > > - compiled with ezkit bf548 configuration
> > > - use hardware 1-bit ECC
> > > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> > >
> > > ...
> > >
> > > +int hardware_ecc = 0;
> >
> > scripts/checkpatch.pl, please.
>
> that isnt static so checkpatch.pl wouldnt have caught it ... it should
> be static though of course and then checkpatch.pl would have thrown a
> static init 0 warning :)

Actually, it would caught by checkpatch.pl even without static.

Andrew, I think I sent out the try#2 version of this patch which fixed
these checkpatch.pl issues. But I will continue do some cleanup as you
point out.

Thanks
-Bryan Wu

2007-09-13 08:51:34

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Thu, 13 Sep 2007 04:45:13 -0400 "Mike Frysinger" <[email protected]> wrote:

> On 9/13/07, Andrew Morton <[email protected]> wrote:
> > On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:
> > > This is the driver for latest Blackfin BF54x nand flash controller
> > >
> > > - use nand_chip and mtd_info common nand driver interface
> > > - provide both PIO and dma operation
> > > - compiled with ezkit bf548 configuration
> > > - use hardware 1-bit ECC
> > > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> > >
> > > ...
> > >
> > > +int hardware_ecc = 0;
> >
> > scripts/checkpatch.pl, please.
>
> that isnt static so checkpatch.pl wouldnt have caught it ...

box:/usr/src/25> perl scripts/checkpatch.pl ~/x
ERROR: do not initialise externals to 0 or NULL
#186: FILE: drivers/mtd/nand/bf54x_nand.c:73:
+int hardware_ecc = 0;

> it should be static though of course

good point.

> and then checkpatch.pl would have thrown a
> static init 0 warning :)
> -mike

2007-09-13 08:51:55

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On 9/13/07, Bryan Wu <[email protected]> wrote:
> On Thu, 2007-09-13 at 04:45 -0400, Mike Frysinger wrote:
> > On 9/13/07, Andrew Morton <[email protected]> wrote:
> > > On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:
> > > > This is the driver for latest Blackfin BF54x nand flash controller
> > > >
> > > > - use nand_chip and mtd_info common nand driver interface
> > > > - provide both PIO and dma operation
> > > > - compiled with ezkit bf548 configuration
> > > > - use hardware 1-bit ECC
> > > > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> > > >
> > > > ...
> > > >
> > > > +int hardware_ecc = 0;
> > >
> > > scripts/checkpatch.pl, please.
> >
> > that isnt static so checkpatch.pl wouldnt have caught it ... it should
> > be static though of course and then checkpatch.pl would have thrown a
> > static init 0 warning :)
>
> Actually, it would caught by checkpatch.pl even without static.

it doesnt for me, but maybe i'm just using an older version
-mike

2007-09-13 08:53:42

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Thu, 2007-09-13 at 04:51 -0400, Mike Frysinger wrote:
> On 9/13/07, Bryan Wu <[email protected]> wrote:
> > On Thu, 2007-09-13 at 04:45 -0400, Mike Frysinger wrote:
> > > On 9/13/07, Andrew Morton <[email protected]> wrote:
> > > > On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]> wrote:
> > > > > This is the driver for latest Blackfin BF54x nand flash controller
> > > > >
> > > > > - use nand_chip and mtd_info common nand driver interface
> > > > > - provide both PIO and dma operation
> > > > > - compiled with ezkit bf548 configuration
> > > > > - use hardware 1-bit ECC
> > > > > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> > > > >
> > > > > ...
> > > > >
> > > > > +int hardware_ecc = 0;
> > > >
> > > > scripts/checkpatch.pl, please.
> > >
> > > that isnt static so checkpatch.pl wouldnt have caught it ... it should
> > > be static though of course and then checkpatch.pl would have thrown a
> > > static init 0 warning :)
> >
> > Actually, it would caught by checkpatch.pl even without static.
>
> it doesnt for me, but maybe i'm just using an older version
> -mike

Yes, should use the latest git-tree checkpatch.pl
Thanks
-Bryan

2007-09-14 07:23:38

by Bryan Wu

[permalink] [raw]
Subject: Re: [PATCH] Blackfin BF54x NAND Flash Controller driver

On Thu, 2007-09-13 at 01:37 -0700, Andrew Morton wrote:
> On Mon, 03 Sep 2007 15:25:23 +0800 Bryan Wu <[email protected]>
> wrote:
>
> > This is the driver for latest Blackfin BF54x nand flash controller
> >
> > - use nand_chip and mtd_info common nand driver interface
> > - provide both PIO and dma operation
> > - compiled with ezkit bf548 configuration
> > - use hardware 1-bit ECC
> > - tested with YAFFS2 and can mount YAFFS2 filesystem as rootfs
> >
> > ...
> >
> > +int hardware_ecc = 0;
>
> scripts/checkpatch.pl, please.
>
Things are fixed in the try#3 version of the driver, which will be sent
out soon.

> > +#endif
> > +
> > +unsigned short bfin_nfc_pin_req[] = {P_NAND_CE, P_NAND_RB, 0};
>
> static. Please review whole patch for this.
>
[!snip!]

> > +
> > +/*
> > + * bf54x_nand_probe
> > + *
> > + * called by device layer when it finds a device matching
> > + * one our driver can handled. This code checks to see if
> > + * it can allocate all necessary resources then calls the
> > + * nand layer to look for devices
> > + */
> > +static int bf54x_nand_probe(struct platform_device *pdev)
> > +{
> > + struct bf54x_nand_platform *plat = to_nand_plat(pdev);
> > + struct bf54x_nand_info *info = NULL;
> > + struct nand_chip *chip = NULL;
> > + struct mtd_info *mtd = NULL;
> > + int err = 0;
> > +
> > + dev_dbg(&pdev->dev, "(%p)\n", pdev);
> > +
> > + if (!plat) {
> > + dev_err(&pdev->dev, "no platform specific information\n");
> > + goto exit_error;
>
> and can this?
>

Try to prevent board configuration missing the bf54x_nand_platform
information.

> > + }
> > +
> > + info = kzalloc(sizeof(*info), GFP_KERNEL);
> > + if (info == NULL) {
> > + dev_err(&pdev->dev, "no memory for flash info\n");
> > + err = -ENOMEM;
> > + goto exit_error;
> > + }


Thanks
-Bryan Wu