[RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Lee Jones]

All,

This patch-set was written against kernel version v3.4, but applies
cleanly to v3.13-rc7 which it is currently based. I understand that
the core has changed significantly since and that some of the
infrastructure this driver provides is now available either through
the framework or similar means. I'm looking for someone with inside
knowledge to draw attention to those areas so we can align with the
most recent API.

Any help would be gratefully received.

 drivers/mtd/nand/Kconfig         |   13 +
 drivers/mtd/nand/Makefile        |    2 +
 drivers/mtd/nand/nand_base.c     |   18 +-
 drivers/mtd/nand/nand_ids.c      |  165 ++++++++
 drivers/mtd/nand/stm_nand_bch.c  | 2769 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/stm_nand_dt.c   |  146 +++++++
 drivers/mtd/nand/stm_nand_dt.h   |   39 ++
 drivers/mtd/nand/stm_nand_regs.h |  302 ++++++++++++++
 include/linux/mtd/nand.h         |   40 ++
 include/linux/mtd/stm_nand.h     |  104 +++++
 10 files changed, 3591 insertions(+), 7 deletions(-)

[RFC 01/47] mtd: nand: export useful functions from core driver

[Lee Jones]

These functions are utilised by the STM BCH NAND Controller driver.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/nand_base.c | 18 +++++++++++-------
 include/linux/mtd/nand.h     |  6 ++++++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index bd39f7b..070505d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -90,7 +90,7 @@ static struct nand_ecclayout nand_oob_128 = {
 		 .length = 78} }
 };
 
-static int nand_get_device(struct mtd_info *mtd, int new_state);
+int nand_get_device(struct mtd_info *mtd, int new_state);
 
 static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 			     struct mtd_oob_ops *ops);
@@ -128,7 +128,7 @@ static int check_offs_len(struct mtd_info *mtd,
  *
  * Release chip lock and wake up anyone waiting on the device.
  */
-static void nand_release_device(struct mtd_info *mtd)
+void nand_release_device(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 
@@ -139,6 +139,7 @@ static void nand_release_device(struct mtd_info *mtd)
 	wake_up(&chip->controller->wq);
 	spin_unlock(&chip->controller->lock);
 }
+EXPORT_SYMBOL_GPL(nand_release_device);
 
 /**
  * nand_read_byte - [DEFAULT] read one byte from the chip
@@ -736,8 +737,7 @@ static void panic_nand_get_device(struct nand_chip *chip,
  *
  * Get the device and lock it for exclusive access
  */
-static int
-nand_get_device(struct mtd_info *mtd, int new_state)
+int nand_get_device(struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *chip = mtd->priv;
 	spinlock_t *lock = &chip->controller->lock;
@@ -769,6 +769,7 @@ retry:
 	remove_wait_queue(wq, &wait);
 	goto retry;
 }
+EXPORT_SYMBOL_GPL(nand_get_device);
 
 /**
  * panic_nand_wait - [GENERIC] wait until the command is done
@@ -2665,7 +2666,7 @@ erase_exit:
  *
  * Sync is actually a wait for chip ready function.
  */
-static void nand_sync(struct mtd_info *mtd)
+void nand_sync(struct mtd_info *mtd)
 {
 	pr_debug("%s: called\n", __func__);
 
@@ -2674,6 +2675,7 @@ static void nand_sync(struct mtd_info *mtd)
 	/* Release it and go back */
 	nand_release_device(mtd);
 }
+EXPORT_SYMBOL_GPL(nand_sync);
 
 /**
  * nand_block_isbad - [MTD Interface] Check if block at offset is bad
@@ -2757,16 +2759,17 @@ static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
  * nand_suspend - [MTD Interface] Suspend the NAND flash
  * @mtd: MTD device structure
  */
-static int nand_suspend(struct mtd_info *mtd)
+int nand_suspend(struct mtd_info *mtd)
 {
 	return nand_get_device(mtd, FL_PM_SUSPENDED);
 }
+EXPORT_SYMBOL(nand_suspend);
 
 /**
  * nand_resume - [MTD Interface] Resume the NAND flash
  * @mtd: MTD device structure
  */
-static void nand_resume(struct mtd_info *mtd)
+void nand_resume(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 
@@ -2776,6 +2779,7 @@ static void nand_resume(struct mtd_info *mtd)
 		pr_err("%s called for a chip which is not in suspended state\n",
 			__func__);
 }
+EXPORT_SYMBOL(nand_resume);
 
 /* Set default functions */
 static void nand_set_defaults(struct nand_chip *chip, int busw)
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 9e6c8f9..f7b6a53 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -698,6 +698,12 @@ extern int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 			   int allowbbt);
 extern int nand_do_read(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, uint8_t *buf);
+extern void nand_sync(struct mtd_info *mtd);
+extern int nand_suspend(struct mtd_info *mtd);
+extern void nand_resume(struct mtd_info *mtd);
+
+extern int nand_get_device(struct mtd_info *mtd, int new_state);
+extern void nand_release_device(struct mtd_info *mtd);
 
 /**
  * struct platform_nand_chip - chip level device structure
-- 
1.8.3.2

Re: [RFC 01/47] mtd: nand: export useful functions from core driver

[Ezequiel Garcia]

On Mar 25, Lee Jones wrote:
> These functions are utilised by the STM BCH NAND Controller driver.
> 
[..]
> -static int nand_suspend(struct mtd_info *mtd)
> +int nand_suspend(struct mtd_info *mtd)
>  {
>  	return nand_get_device(mtd, FL_PM_SUSPENDED);
>  }
> +EXPORT_SYMBOL(nand_suspend);
>  
>  /**
>   * nand_resume - [MTD Interface] Resume the NAND flash
>   * @mtd: MTD device structure
>   */
> -static void nand_resume(struct mtd_info *mtd)
> +void nand_resume(struct mtd_info *mtd)
>  {
>  	struct nand_chip *chip = mtd->priv;
>  
> @@ -2776,6 +2779,7 @@ static void nand_resume(struct mtd_info *mtd)
>  		pr_err("%s called for a chip which is not in suspended state\n",
>  			__func__);
>  }
> +EXPORT_SYMBOL(nand_resume);
>  

The patch that adds the PM support for the driver makes no use of these.

Can you detail why do you need to export these?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [RFC 01/47] mtd: nand: export useful functions from core driver

[Lee Jones]

On Tue, 25 Mar 2014, Ezequiel Garcia wrote:

> On Mar 25, Lee Jones wrote:
> > These functions are utilised by the STM BCH NAND Controller driver.
> > 
> [..]
> > -static int nand_suspend(struct mtd_info *mtd)
> > +int nand_suspend(struct mtd_info *mtd)
> >  {
> >  	return nand_get_device(mtd, FL_PM_SUSPENDED);
> >  }
> > +EXPORT_SYMBOL(nand_suspend);
> >  
> >  /**
> >   * nand_resume - [MTD Interface] Resume the NAND flash
> >   * @mtd: MTD device structure
> >   */
> > -static void nand_resume(struct mtd_info *mtd)
> > +void nand_resume(struct mtd_info *mtd)
> >  {
> >  	struct nand_chip *chip = mtd->priv;
> >  
> > @@ -2776,6 +2779,7 @@ static void nand_resume(struct mtd_info *mtd)
> >  		pr_err("%s called for a chip which is not in suspended state\n",
> >  			__func__);
> >  }
> > +EXPORT_SYMBOL(nand_resume);
> >  
> 
> The patch that adds the PM support for the driver makes no use of these.
> 
> Can you detail why do you need to export these?

Please see RFC PATCH 9:

mtd: nand: stm_nand_bch: introduce and initialise some important data structures

static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
                                   struct mtd_info *mtd, struct nand_chip *chip)
{

[...]
       mtd->_sync = nand_sync;
       mtd->_suspend = nand_suspend;
       mtd->_resume = nand_resume;
}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 02/47] mtd: nand: add ONFI NAND Timing Mode Specifications

[Lee Jones]

This patch adds a new structure, 'nand_timing_spec', to capture the A/C
timing characteristics of NAND devices.

Unfortunately, there is no universally accepted standard for defining
NAND timing parameters. Different datasheets list different sets of
parameters. The ONFI standard gets close, but support is missing from
some of the major NAND manufacturers (e.g. Samsung, Toshiba). Here we
have followed broadly the ONFI timing definitions.

We also provide specifications for the six standard ONFI timing modes,
in terms of the new nand_timing_spec structure. This will allow fully
automated configuration of the timing registers when using
ONFI-compliant NAND.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/nand_ids.c | 165 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h    |  34 +++++++++
 2 files changed, 199 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index a87b0a3..12f769d 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -175,6 +175,171 @@ struct nand_manufacturers nand_manuf_ids[] = {
 EXPORT_SYMBOL(nand_manuf_ids);
 EXPORT_SYMBOL(nand_flash_ids);
 
+/*
+ * ONFI NAND Timing Mode Specifications
+ *
+ * Note, 'tR' field (maximum page read time) is extracted from the ONFI
+ * parameter page during device probe.
+ */
+struct nand_timing_spec nand_onfi_timing_specs[] = {
+	/*
+	 * ONFI Timing Mode '0' (supported on all ONFI compliant devices)
+	 */
+	[0] = {
+		.tCLS	= 50,
+		.tCS	= 70,
+		.tALS	= 50,
+		.tDS	= 40,
+		.tWP	= 50,
+		.tCLH	= 20,
+		.tCH	= 20,
+		.tALH	= 20,
+		.tDH	= 20,
+		.tWB	= 200,
+		.tWH	= 30,
+		.tWC	= 100,
+		.tRP	= 50,
+		.tREH	= 30,
+		.tRC	= 100,
+		.tREA	= 40,
+		.tRHOH	= 0,
+		.tCEA	= 100,
+		.tCOH	= 0,
+		.tCHZ	= 100,
+	},
+
+	/*
+	 * ONFI Timing Mode '1'
+	 */
+	[1] = {
+		.tCLS	= 25,
+		.tCS	= 35,
+		.tALS	= 25,
+		.tDS	= 20,
+		.tWP	= 25,
+		.tCLH	= 10,
+		.tCH	= 10,
+		.tALH	= 10,
+		.tDH	= 10,
+		.tWB	= 100,
+		.tWH	= 15,
+		.tWC	= 45,
+		.tRP	= 25,
+		.tREH	= 15,
+		.tRC	= 50,
+		.tREA	= 30,
+		.tRHOH	= 15,
+		.tCEA	= 45,
+		.tCOH	= 15,
+		.tCHZ	= 50,
+	},
+
+	/*
+	 * ONFI Timing Mode '2'
+	 */
+	[2] = {
+		.tCLS	= 15,
+		.tCS	= 25,
+		.tALS	= 15,
+		.tDS	= 15,
+		.tWP	= 17,
+		.tCLH	= 10,
+		.tCH	= 10,
+		.tALH	= 10,
+		.tDH	= 5,
+		.tWB	= 100,
+		.tWH	= 15,
+		.tWC	= 35,
+		.tRP	= 17,
+		.tREH	= 16,
+		.tRC	= 35,
+		.tREA	= 25,
+		.tRHOH	= 15,
+		.tCEA	= 30,
+		.tCOH	= 15,
+		.tCHZ	= 50,
+	},
+
+	/*
+	 * ONFI Timing Mode '3'
+	 */
+	[3] = {
+		.tCLS	= 10,
+		.tCS	= 25,
+		.tALS	= 10,
+		.tDS	= 10,
+		.tWP	= 15,
+		.tCLH	= 5,
+		.tCH	= 5,
+		.tALH	= 5,
+		.tDH	= 5,
+		.tWB	= 100,
+		.tWH	= 10,
+		.tWC	= 30,
+		.tRP	= 15,
+		.tREH	= 10,
+		.tRC	= 30,
+		.tREA	= 20,
+		.tRHOH	= 15,
+		.tCEA	= 25,
+		.tCOH	= 15,
+		.tCHZ	= 50,
+	},
+
+	/*
+	 * ONFI Timing Mode '4' (EDO only)
+	 */
+	[4] = {
+		.tCLS	= 10,
+		.tCS	= 20,
+		.tALS	= 10,
+		.tDS	= 10,
+		.tWP	= 12,
+		.tCLH	= 5,
+		.tCH	= 5,
+		.tALH	= 5,
+		.tDH	= 5,
+		.tWB	= 100,
+		.tWH	= 10,
+		.tWC	= 25,
+		.tRP	= 12,
+		.tREH	= 10,
+		.tRC	= 25,
+		.tREA	= 20,
+		.tRHOH	= 15,
+		.tCEA	= 25,
+		.tCOH	= 15,
+		.tCHZ	= 30,
+	},
+
+	/*
+	 * ONFI Timing Mode '5' (EDO only)
+	 */
+	[5] = {
+		.tCLS	= 10,
+		.tCS	= 15,
+		.tALS	= 10,
+		.tDS	= 7,
+		.tWP	= 10,
+		.tCLH	= 5,
+		.tCH	= 5,
+		.tALH	= 5,
+		.tDH	= 5,
+		.tWB	= 100,
+		.tWH	= 7,
+		.tWC	= 20,
+		.tRP	= 10,
+		.tREH	= 7,
+		.tRC	= 20,
+		.tREA	= 16,
+		.tRHOH	= 15,
+		.tCEA	= 25,
+		.tCOH	= 15,
+		.tCHZ	= 30,
+	}
+};
+EXPORT_SYMBOL(nand_onfi_timing_specs);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
 MODULE_DESCRIPTION("Nand device & manufacturer IDs");
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f7b6a53..0d456b4 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -326,6 +326,40 @@ struct onfi_ext_param_page {
 	 */
 } __packed;
 
+/*
+ * NAND Device Timing Specification
+ *
+ * All values in nano seconds, except where specified.
+ */
+struct nand_timing_spec {
+	int	tR;		/* Max Page Read delay [us]*/
+	int	tCLS;		/* Min CLE setup time */
+	int	tCS;		/* Min CE setup time */
+	int	tALS;		/* Min ALE setup time */
+	int	tDS;		/* Min Data setup time */
+	int	tWP;		/* Min WE pulse width */
+	int	tCLH;		/* Min CLE hold time */
+	int	tCH;		/* Min CE hold time */
+	int	tALH;		/* Min ALE hold time */
+	int	tDH;		/* Min Data hold time */
+	int	tWB;		/* Max WE high to busy */
+	int	tWH;		/* Min WE hold time */
+	int	tWC;		/* Min Write cycle time */
+	int	tRP;		/* Min RE pulse width */
+	int	tREH;		/* Min RE high hold time */
+	int	tRC;		/* Min Read cycle time */
+	int	tREA;		/* Max Read access time */
+	int	tRHOH;		/* Min RE high to output hold */
+	int	tCEA;		/* Max CE access time */
+	int	tCOH;		/* Min CE high to output hold */
+	int	tCHZ;		/* Max CE high to output high Z */
+	int	tCSD;		/* Min CE high to ALE/CLE don't care */
+};
+
+/* ONFI define 6 timing modes */
+#define NAND_ONFI_TIMING_MODES		6
+extern struct nand_timing_spec nand_onfi_timing_specs[];
+
 /**
  * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices
  * @lock:               protection lock
-- 
1.8.3.2

Re: [RFC 02/47] mtd: nand: add ONFI NAND Timing Mode Specifications

[Jason Gunthorpe]

On Tue, Mar 25, 2014 at 08:19:19AM +0000, Lee Jones wrote:
> This patch adds a new structure, 'nand_timing_spec', to capture the A/C
> timing characteristics of NAND devices.

Boris BREZILLON <b.brezillon.dev@gmail.com> has been working on a
similar patch for a time now, and I think has an implementation for
the sunxi driver.
 
> some of the major NAND manufacturers (e.g. Samsung, Toshiba). Here we
> have followed broadly the ONFI timing definitions.

Timing specifications that are not precisely defined are
useless.. I would stick entirely with the well defined ONFI ones
until a need arises..

> + * Note, 'tR' field (maximum page read time) is extracted from the ONFI
> + * parameter page during device probe.
> + */
> +struct nand_timing_spec nand_onfi_timing_specs[] = {

const?

> +	/*
> +	 * ONFI Timing Mode '0' (supported on all ONFI compliant devices)
> +	 */
> +	[0] = {
> +		.tCLS	= 50,
> +		.tCS	= 70,
> +		.tALS	= 50,

Use picoseconds, ns is already loosing precision at the higher timing
modes.

Cheers,
Jason

[RFC 03/47] mtd: nand: add shared register defines for ST's NAND Controller drivers

[Lee Jones]

Provide register and bit definitions used by STM's BCH Controller driver.
We place these into a shared location as they will be used by other STM
NAND Controllers as they appear.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_regs.h | 302 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)
 create mode 100644 drivers/mtd/nand/stm_nand_regs.h

diff --git a/drivers/mtd/nand/stm_nand_regs.h b/drivers/mtd/nand/stm_nand_regs.h
new file mode 100644
index 0000000..2b0e069
--- /dev/null
+++ b/drivers/mtd/nand/stm_nand_regs.h
@@ -0,0 +1,302 @@
+/*
+ * drivers/mtd/nand/stm_nand_regs.h
+ *
+ * STMicroelectronics NAND Controller register definitions
+ *
+ * Copyright (c) 2008-2014 STMicroelectronics Limited
+ * Author: Angus Clark <Angus.Clark@st.com>
+ *
+ * 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.
+ *
+ */
+
+#ifndef STM_NANDC_REGS_H
+#define STM_NANDC_REGS_H
+
+/* Hamming Controller Registers (Offsets from EMINAND_BASE) */
+#define NANDHAM_BOOTBANK_CFG				0x000
+#define NANDHAM_RBN_STA					0x004
+#define NANDHAM_INT_EN					0x010
+#define NANDHAM_INT_STA					0x014
+#define NANDHAM_INT_CLR					0x018
+#define NANDHAM_INT_EDGE_CFG				0x01C
+#define NANDHAM_CTL_TIMING				0x040
+#define NANDHAM_WEN_TIMING				0x044
+#define NANDHAM_REN_TIMING				0x048
+#define NANDHAM_BLOCK_ZERO_REMAP_REG			0x04C
+#define NANDHAM_FLEXMODE_CFG				0x100
+#define NANDHAM_FLEX_MUXCTRL				0x104
+#define NANDHAM_FLEX_DATAWRITE_CONFIG			0x10C
+#define NANDHAM_FLEX_DATAREAD_CONFIG			0x110
+#define NANDHAM_FLEX_CMD				0x114
+#define NANDHAM_FLEX_ADD				0x118
+#define NANDHAM_FLEX_DATA				0x120
+#define NANDHAM_VERSION_REG				0x144
+#define NANDHAM_MULTI_CS_CONFIG_REG			0x1EC
+#define NANDHAM_AFM_SEQ_REG_1				0x200
+#define NANDHAM_AFM_SEQ_REG_2				0x204
+#define NANDHAM_AFM_SEQ_REG_3				0x208
+#define NANDHAM_AFM_SEQ_REG_4				0x20C
+#define NANDHAM_AFM_ADD					0x210
+#define NANDHAM_AFM_EXTRA				0x214
+#define NANDHAM_AFM_CMD					0x218
+#define NANDHAM_AFM_SEQ_CFG				0x21C
+#define NANDHAM_AFM_GEN_CFG				0x220
+#define NANDHAM_AFM_SEQ_STA				0x240
+#define NANDHAM_AFM_ECC_REG_0				0x280
+#define NANDHAM_AFM_ECC_REG_1				0x284
+#define NANDHAM_AFM_ECC_REG_2				0x288
+#define NANDHAM_AFM_ECC_REG_3				0x28C
+#define NANDHAM_AFM_DATA_FIFO				0x300
+
+/* BCH Controller Registers (Offsets from EMI_NAND) */
+#define NANDBCH_BOOTBANK_CFG				0x000
+#define NANDBCH_RBN_STA					0x004
+#define NANDBCH_INT_EN					0x010
+#define NANDBCH_INT_STA					0x014
+#define NANDBCH_INT_CLR					0x018
+#define NANDBCH_INT_EDGE_CFG				0x01C
+#define NANDBCH_CTL_TIMING				0x040
+#define NANDBCH_WEN_TIMING				0x044
+#define NANDBCH_REN_TIMING				0x048
+#define NANDBCH_BLOCK_ZERO_REMAP_REG			0x04C
+#define NANDBCH_BOOT_STATUS				0x050
+#define NANDBCH_FALSE_BOOT_REG				0x054
+#define NANDBCH_FALSE_BOOT_STATUS			0x058
+#define NANDBCH_CONTROLLER_CFG				0x100
+#define NANDBCH_FLEX_MUXCTRL				0x104
+#define NANDBCH_FLEX_DATAWRITE_CONFIG			0x10C
+#define NANDBCH_FLEX_DATAREAD_CONFIG			0x110
+#define NANDBCH_VERSION_REG				0x144
+#define NANDBCH_ADDRESS_REG_1				0x1F0
+#define NANDBCH_ADDRESS_REG_2				0x1F4
+#define NANDBCH_ADDRESS_REG_3				0x1F8
+#define NANDBCH_MULTI_CS_CONFIG_REG			0x1FC
+#define NANDBCH_SEQ_REG_1				0x200
+#define NANDBCH_SEQ_REG_2				0x204
+#define NANDBCH_SEQ_REG_3				0x208
+#define NANDBCH_SEQ_REG_4				0x20C
+#define NANDBCH_ADD					0x210
+#define NANDBCH_EXTRA_REG				0x214
+#define NANDBCH_CMD					0x218
+#define NANDBCH_GEN_CFG					0x220
+#define NANDBCH_DELAY_REG				0x224
+#define NANDBCH_SEQ_CFG					0x22C
+#define NANDBCH_SEQ_STA					0x270
+#define NANDBCH_DATA_BUFFER_ENTRY_0			0x280
+#define NANDBCH_DATA_BUFFER_ENTRY_1			0x284
+#define NANDBCH_DATA_BUFFER_ENTRY_2			0x288
+#define NANDBCH_DATA_BUFFER_ENTRY_3			0x28C
+#define NANDBCH_DATA_BUFFER_ENTRY_4			0x290
+#define NANDBCH_DATA_BUFFER_ENTRY_5			0x294
+#define NANDBCH_DATA_BUFFER_ENTRY_6			0x298
+#define NANDBCH_DATA_BUFFER_ENTRY_7			0x29C
+#define NANDBCH_ECC_SCORE_REG_A				0x2A0
+#define NANDBCH_ECC_SCORE_REG_B				0x2A4
+#define NANDBCH_CHECK_STATUS_REG_A			0x2A8
+#define NANDBCH_CHECK_STATUS_REG_B			0x2AC
+#define NANDBCH_BUFFER_LIST_PTR				0x300
+#define NANDBCH_SEQ_PTR_REG				0x304
+#define NANDBCH_ERROR_THRESHOLD_REG			0x308
+
+/* EMISS NAND BCH STPLUG Registers (Offsets from EMISS_NAND_DMA) */
+#define EMISS_NAND_RD_DMA_PAGE_SIZE			0x000
+#define EMISS_NAND_RD_DMA_MAX_OPCODE_SIZE		0x004
+#define EMISS_NAND_RD_DMA_MIN_OPCODE_SIZE		0x008
+#define EMISS_NAND_RD_DMA_MAX_CHUNK_SIZE		0x00C
+#define EMISS_NAND_RD_DMA_MAX_MESSAGE_SIZE		0x010
+
+#define EMISS_NAND_WR_DMA_PAGE_SIZE			0x100
+#define EMISS_NAND_WR_DMA_MAX_OPCODE_SIZE		0x104
+#define EMISS_NAND_WR_DMA_MIN_OPCODE_SIZE		0x108
+#define EMISS_NAND_WR_DMA_MAX_CHUNK_SIZE		0x10C
+#define EMISS_NAND_WR_DMA_MAX_MESSAGE_SIZE		0x110
+
+
+/*
+ * Hamming/BCH controller interrupts
+ */
+
+/* NANDxxx_INT_EN/NANDxxx_INT_STA */
+/*      Common */
+#define NAND_INT_ENABLE				(0x1 << 0)
+#define NAND_INT_RBN				(0x1 << 2)
+#define NAND_INT_SEQCHECK			(0x1 << 5)
+/*      Hamming only */
+#define NANDHAM_INT_DATA_DREQ			(0x1 << 3)
+#define NANDHAM_INT_SEQ_DREQ			(0x1 << 4)
+#define NANDHAM_INT_ECC_FIX_REQ			(0x1 << 6)
+/*      BCH only */
+#define NANDBCH_INT_SEQNODESOVER		(0x1 << 7)
+#define NANDBCH_INT_ECCTHRESHOLD		(0x1 << 8)
+
+/* NANDxxx_INT_CLR */
+/*      Common */
+#define NAND_INT_CLR_RBN			(0x1 << 2)
+#define NAND_INT_CLR_SEQCHECK			(0x1 << 3)
+/*      Hamming only */
+#define NANDHAM_INT_CLR_ECC_FIX_REQ		(0x1 << 4)
+#define NANDHAM_INT_CLR_DATA_DREQ		(0x1 << 5)
+#define NANDHAM_INT_CLR_SEQ_DREQ		(0x1 << 6)
+/*      BCH only */
+#define NANDBCH_INT_CLR_SEQNODESOVER		(0x1 << 5)
+#define NANDBCH_INT_CLR_ECCTHRESHOLD		(0x1 << 6)
+
+/* NANDxxx_INT_EDGE_CFG */
+#define NAND_EDGE_CFG_RBN_RISING		0x1
+#define NAND_EDGE_CFG_RBN_FALLING		0x2
+#define NAND_EDGE_CFG_RBN_ANY			0x3
+
+/* NANDBCH_CONTROLLER_CFG/NANDHAM_FLEXMODE_CFG */
+#define CFG_ENABLE_FLEX				0x1
+#define CFG_ENABLE_AFM				0x2
+#define CFG_RESET				(0x1 << 3)
+#define CFG_RESET_ECC(x)			(0x1 << (7 + (x)))
+#define CFG_RESET_ECC_ALL			(0xff << 7)
+
+
+/*
+ * BCH Controller
+ */
+
+/* NANDBCH_BOOTBANK_CFG */
+#define BOOT_CFG_RESET				(0x1 << 3)
+
+/* NANDBCH_CTL_TIMING */
+#define NANDBCH_CTL_SETUP(x)			((x) & 0xff)
+#define NANDBCH_CTL_HOLD(x)			(((x) & 0xff) << 8)
+#define NANDBCH_CTL_WERBN(x)			(((x) & 0xff) << 24)
+
+/* NANDBCH_WEN_TIMING */
+#define NANDBCH_WEN_ONTIME(x)			((x) & 0xff)
+#define NANDBCH_WEN_OFFTIME(x)			(((x) & 0xff) << 8)
+#define NANDBCH_WEN_ONHALFCYCLE			(0x1 << 16)
+#define NANDBCH_WEN_OFFHALFCYCLE		(0x1 << 17)
+
+/* NANDBCH_REN_TIMING */
+#define NANDBCH_REN_ONTIME(x)			((x) & 0xff)
+#define NANDBCH_REN_OFFTIME(x)			(((x) & 0xff) << 8)
+#define NANDBCH_REN_ONHALFCYCLE			(0x1 << 16)
+#define NANDBCH_REN_OFFHALFCYCLE		(0x1 << 17)
+#define NANDBCH_REN_TELQV(x)			(((x) & 0xff) << 24)
+
+/* NANDBCH_BLOCK_ZERO_REMAP_REG */
+#define NANDBCH_BACKUP_COPY_FOUND		(0x1 << 0)
+#define NANDBCH_ORIG_CODE_CORRUPTED		(0x1 << 1)
+#define NANDBCH_BLK_ZERO_REMAP(x)		((x) >> 14)
+
+/* NANDBCH_BOOT_STATUS */
+#define NANDBCH_BOOT_MAX_ERRORS(x)		((x) & 0x1f)
+
+/* NANDBCH_GEN_CFG */
+#define GEN_CFG_DATA_8_NOT_16			(0x1 << 16)
+#define GEN_CFG_EXTRA_ADD_CYCLE			(0x1 << 18)
+#define GEN_CFG_2X8_MODE			(0x1 << 19)
+#define GEN_CFG_ECC_SHIFT			20
+#define GEN_CFG_18BIT_ECC			(BCH_18BIT_ECC << \
+						GEN_CFG_ECC_SHIFT)
+#define GEN_CFG_30BIT_ECC			(BCH_30BIT_ECC << \
+						GEN_CFG_ECC_SHIFT)
+#define GEN_CFG_NO_ECC				(BCH_NO_ECC    << \
+						GEN_CFG_ECC_SHIFT)
+#define GEN_CFG_LAST_SEQ_NODE			(0x1 << 22)
+
+/* NANDBCH_SEQ_CFG */
+#define SEQ_CFG_REPEAT_COUNTER(x)		((x) & 0xffff)
+#define SEQ_CFG_SEQ_IDENT(x)			(((x) & 0xff) << 16)
+#define SEQ_CFG_DATA_WRITE			(0x1 << 24)
+#define SEQ_CFG_ERASE				(0x1 << 25)
+#define SEQ_CFG_GO_STOP				(0x1 << 26)
+
+/* NANDBCH_SEQ_STA */
+#define SEQ_STA_RUN				(0x1 << 4)
+
+/*
+ * BCH Commands
+ */
+#define BCH_OPC_STOP			0x0
+#define BCH_OPC_CMD			0x1
+#define BCH_OPC_INC			0x2
+#define BCH_OPC_DEC_JUMP		0x3
+#define BCH_OPC_DATA			0x4
+#define BCH_OPC_DELAY			0x5
+#define BCH_OPC_CHECK			0x6
+#define BCH_OPC_ADDR			0x7
+#define BCH_OPC_NEXT_CHIP_ON		0x8
+#define BCH_OPC_DEC_JMP_MCS		0x9
+#define BCH_OPC_ECC_SCORE		0xA
+
+#define BCH_INSTR(opc, opr)		((opc) | ((opr) << 4))
+
+#define BCH_CMD_ADDR			BCH_INSTR(BCH_OPC_CMD, 0)
+#define BCH_CL_CMD_1			BCH_INSTR(BCH_OPC_CMD, 1)
+#define BCH_CL_CMD_2			BCH_INSTR(BCH_OPC_CMD, 2)
+#define BCH_CL_CMD_3			BCH_INSTR(BCH_OPC_CMD, 3)
+#define BCH_CL_EX_0			BCH_INSTR(BCH_OPC_CMD, 4)
+#define BCH_CL_EX_1			BCH_INSTR(BCH_OPC_CMD, 5)
+#define BCH_CL_EX_2			BCH_INSTR(BCH_OPC_CMD, 6)
+#define BCH_CL_EX_3			BCH_INSTR(BCH_OPC_CMD, 7)
+#define BCH_INC(x)			BCH_INSTR(BCH_OPC_INC, (x))
+#define BCH_DEC_JUMP(x)			BCH_INSTR(BCH_OPC_DEC_JUMP, (x))
+#define BCH_STOP			BCH_INSTR(BCH_OPC_STOP, 0)
+#define BCH_DATA_1_SECTOR		BCH_INSTR(BCH_OPC_DATA, 0)
+#define BCH_DATA_2_SECTOR		BCH_INSTR(BCH_OPC_DATA, 1)
+#define BCH_DATA_4_SECTOR		BCH_INSTR(BCH_OPC_DATA, 2)
+#define BCH_DATA_8_SECTOR		BCH_INSTR(BCH_OPC_DATA, 3)
+#define BCH_DATA_16_SECTOR		BCH_INSTR(BCH_OPC_DATA, 4)
+#define BCH_DATA_32_SECTOR		BCH_INSTR(BCH_OPC_DATA, 5)
+#define BCH_DELAY_0			BCH_INSTR(BCH_OPC_DELAY, 0)
+#define BCH_DELAY_1			BCH_INSTR(BCH_OPC_DELAY, 1)
+#define BCH_OP_ERR			BCH_INSTR(BCH_OPC_CHECK, 0)
+#define BCH_CACHE_ERR			BCH_INSTR(BCH_OPC_CHECK, 1)
+#define BCH_ERROR			BCH_INSTR(BCH_OPC_CHECK, 2)
+#define BCH_AL_EX_0			BCH_INSTR(BCH_OPC_ADDR, 0)
+#define BCH_AL_EX_1			BCH_INSTR(BCH_OPC_ADDR, 1)
+#define BCH_AL_EX_2			BCH_INSTR(BCH_OPC_ADDR, 2)
+#define BCH_AL_EX_3			BCH_INSTR(BCH_OPC_ADDR, 3)
+#define BCH_AL_AD_0			BCH_INSTR(BCH_OPC_ADDR, 4)
+#define BCH_AL_AD_1			BCH_INSTR(BCH_OPC_ADDR, 5)
+#define BCH_AL_AD_2			BCH_INSTR(BCH_OPC_ADDR, 6)
+#define BCH_AL_AD_3			BCH_INSTR(BCH_OPC_ADDR, 7)
+#define BCH_NEXT_CHIP_ON		BCH_INSTR(BCH_OPC_NEXT_CHIP_ON, 0)
+#define BCH_DEC_JMP_MCS(x)		BCH_INSTR(BCH_OPC_DEC_JMP_MCS, (x))
+#define BCH_ECC_SCORE(x)		BCH_INSTR(BCH_OPC_ECC_SCORE, (x))
+
+
+/*
+ * Hamming-FLEX register fields
+ */
+
+/* NANDHAM_FLEX_DATAREAD/WRITE_CONFIG */
+#define FLEX_DATA_CFG_WAIT_RBN			(0x1 << 27)
+#define FLEX_DATA_CFG_BEATS_1			(0x1 << 28)
+#define FLEX_DATA_CFG_BEATS_2			(0x2 << 28)
+#define FLEX_DATA_CFG_BEATS_3			(0x3 << 28)
+#define FLEX_DATA_CFG_BEATS_4			(0x0 << 28)
+#define FLEX_DATA_CFG_BYTES_1			(0x0 << 30)
+#define FLEX_DATA_CFG_BYTES_2			(0x1 << 30)
+#define FLEX_DATA_CFG_CSN			(0x1 << 31)
+
+/* NANDHAM_FLEX_CMD */
+#define FLEX_CMD_RBN				(0x1 << 27)
+#define FLEX_CMD_BEATS_1			(0x1 << 28)
+#define FLEX_CMD_BEATS_2			(0x2 << 28)
+#define FLEX_CMD_BEATS_3			(0x3 << 28)
+#define FLEX_CMD_BEATS_4			(0x0 << 28)
+#define FLEX_CMD_CSN				(0x1 << 31)
+#define FLEX_CMD(x)				(((x) & 0xff) |	\
+						 FLEX_CMD_RBN |	\
+						 FLEX_CMD_BEATS_1 |	\
+						 FLEX_CMD_CSN)
+/* NANDHAM_FLEX_ADD */
+#define FLEX_ADDR_RBN				(0x1 << 27)
+#define FLEX_ADDR_BEATS_1			(0x1 << 28)
+#define FLEX_ADDR_BEATS_2			(0x2 << 28)
+#define FLEX_ADDR_BEATS_3			(0x3 << 28)
+#define FLEX_ADDR_BEATS_4			(0x0 << 28)
+#define FLEX_ADDR_ADD8_VALID			(0x1 << 30)
+#define FLEX_ADDR_CSN				(0x1 << 31)
+
+#endif /* STM_NANDC_REGS_H */
-- 
1.8.3.2

[RFC 04/47] mtd: nand: adding ST's BCH NAND Controller driver

[Lee Jones]

First introduction of the driver. Includes the basic device struct
(some functionality isn't utilised as of yet) and supplies some of the
important resources required for basic running of the Controller.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/Kconfig        |   6 ++
 drivers/mtd/nand/Makefile       |   1 +
 drivers/mtd/nand/stm_nand_bch.c | 151 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/stm_nand.h    |  33 +++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 drivers/mtd/nand/stm_nand_bch.c
 create mode 100644 include/linux/mtd/stm_nand.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 93ae6a6..2bf972c 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -510,4 +510,10 @@ config MTD_NAND_XWAY
 	  Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached
 	  to the External Bus Unit (EBU).
 
+config MTD_NAND_STM_BCH
+	tristate "STMicroelectronics: NANDi BCH Controller"
+	depends on ARM
+	help
+	  Adds support for the STMicroelectronics NANDi BCH Controller.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 542b568..94e7737 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_MTD_NAND_NUC900)		+= nuc900_nand.o
 obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
 obj-$(CONFIG_MTD_NAND_RICOH)		+= r852.o
 obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
+obj-$(CONFIG_MTD_NAND_STM_BCH)		+= stm_nand_bch.o
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
 obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
new file mode 100644
index 0000000..bd11070
--- /dev/null
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -0,0 +1,151 @@
+/*
+ * drivers/mtd/nand/stm_nand_bch.c
+ *
+ * Support for STMicroelectronics NANDi BCH Controller
+ *
+ * Copyright (c) 2014 STMicroelectronics Limited
+ * Author: Angus Clark <Angus.Clark@st.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/completion.h>
+#include <linux/mtd/stm_nand.h>
+
+/* NANDi Controller (Hamming/BCH) */
+struct nandi_controller {
+	void __iomem		*base;		/* Controller base*/
+	void __iomem		*dma;		/* DMA control base */
+						/* IRQ-triggered Completions: */
+	struct completion	seq_completed;	/*   SEQ Over */
+	struct completion	rbn_completed;	/*   RBn */
+
+	struct device		*dev;
+
+	int			bch_ecc_mode;	/* ECC mode */
+	bool			extra_addr;	/* Extra address cycle */
+
+	/*
+	 * The threshold at which the number of corrected bit-flips per sector
+	 * is deemed to have reached an excessive level (triggers '-EUCLEAN'
+	 * return code).
+	 */
+	int			bitflip_threshold;
+
+	uint32_t		page_shift;	/* Some working variables */
+	uint32_t		block_shift;
+	uint32_t		blocks_per_device;
+	uint32_t		sectors_per_page;
+
+	uint8_t			*buf;		/* Some buffers to use */
+	uint8_t			*page_buf;
+	uint8_t			*oob_buf;
+	uint32_t		*buf_list;
+
+	int			cached_page;	/* page number of page in */
+						/* 'page_buf'             */
+};
+
+static int remap_named_resource(struct platform_device *pdev,
+				char *name,
+				void __iomem **io_ptr)
+{
+	struct resource *res, *mem;
+	resource_size_t size;
+	void __iomem *p;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
+	if (!res)
+		return -ENXIO;
+
+	size = resource_size(res);
+
+	mem = devm_request_mem_region(&pdev->dev, res->start, size, name);
+	if (!mem)
+		return -EBUSY;
+
+	p = devm_ioremap_nocache(&pdev->dev, res->start, size);
+	if (!p)
+		return -ENOMEM;
+
+	*io_ptr = p;
+
+	return 0;
+}
+
+static struct nandi_controller *
+nandi_init_resources(struct platform_device *pdev)
+{
+	struct nandi_controller *nandi;
+	int err;
+
+	nandi = devm_kzalloc(&pdev->dev, sizeof(*nandi), GFP_KERNEL);
+	if (!nandi) {
+		dev_err(&pdev->dev,
+			"failed to allocate NANDi controller data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	nandi->dev = &pdev->dev;
+
+	err = remap_named_resource(pdev, "nand_mem", &nandi->base);
+	if (err)
+		return ERR_PTR(err);
+
+	err = remap_named_resource(pdev, "nand_dma", &nandi->dma);
+	if (err)
+		return ERR_PTR(err);
+
+	platform_set_drvdata(pdev, nandi);
+
+	return nandi;
+}
+
+static int stm_nand_bch_probe(struct platform_device *pdev)
+{
+	struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data;
+	struct nandi_controller *nandi;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform data found\n");
+		return -EINVAL;
+	}
+
+	nandi = nandi_init_resources(pdev);
+	if (IS_ERR(nandi)) {
+		dev_err(&pdev->dev, "failed to initialise NANDi resources\n");
+		return PTR_ERR(nandi);
+	}
+
+	init_completion(&nandi->seq_completed);
+	init_completion(&nandi->rbn_completed);
+
+	return 0;
+}
+
+static int stm_nand_bch_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver stm_nand_bch_driver = {
+	.probe	= stm_nand_bch_probe ,
+	.remove	= stm_nand_bch_remove,
+	.driver	= {
+		.name	= "stm-nand-bch",
+		.owner	= THIS_MODULE,
+	},
+};
+module_platform_driver(stm_nand_bch_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Angus Clark");
+MODULE_DESCRIPTION("STM NAND BCH driver");
diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h
new file mode 100644
index 0000000..9210d5c
--- /dev/null
+++ b/include/linux/mtd/stm_nand.h
@@ -0,0 +1,33 @@
+/*
+ * include/linux/mtd/stm_mtd.h
+ *
+ * Support for STMicroelectronics NAND Controllers
+ *
+ * Copyright (c) 2014 STMicroelectronics Limited
+ * Author: Angus Clark <Angus.Clark@st.com>
+ *
+ * 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.
+ *
+ */
+
+#ifndef __LINUX_STM_NAND_H
+#define __LINUX_STM_NAND_H
+
+struct stm_plat_nand_bch_data {
+	struct stm_nand_bank_data *bank;
+	enum stm_nand_bch_ecc_config bch_ecc_cfg;
+
+	/* The threshold at which the number of corrected bit-flips per sector
+	 * is deemed to have reached an excessive level (triggers '-EUCLEAN' to
+	 * be returned to the caller).  The value should be in the range 1 to
+	 * <ecc-strength> where <ecc-strength> is 18 or 30, depending on the BCH
+	 * ECC mode in operation.  A value of 0 is interpreted by the driver as
+	 * <ecc-strength>.
+	 */
+	unsigned int bch_bitflip_threshold;
+	bool flashss;
+};
+
+#endif /* __LINUX_STM_NAND_H */
-- 
1.8.3.2

[RFC 05/47] mtd: nand: stm_nand_bch: IRQ support for ST's BCH NAND Controller driver

[Lee Jones]

Obtain IRQ number and request IRQ resource via the usual methods. We're
also registering an IRQ handler to inform us of any completed tasks.
Notice that we're starting to make use of the device struct that we
defined before. In keeping with the subject of the patch, we're also
adding the related local enable_irq() and disable_irq() methods. Again,
these will be utilised in a greater capacity in latter commits.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 62 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index bd11070..76a0d02 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -15,11 +15,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <linux/completion.h>
 #include <linux/mtd/stm_nand.h>
 
+#include "stm_nand_regs.h"
+
 /* NANDi Controller (Hamming/BCH) */
 struct nandi_controller {
 	void __iomem		*base;		/* Controller base*/
@@ -54,6 +57,51 @@ struct nandi_controller {
 						/* 'page_buf'             */
 };
 
+/*
+ * NANDi Interrupts (shared by Hamming and BCH controllers)
+ */
+static irqreturn_t nandi_irq_handler(int irq, void *dev)
+{
+	struct nandi_controller *nandi = dev;
+	unsigned int status;
+
+	status = readl(nandi->base + NANDBCH_INT_STA);
+
+	if (status & NANDBCH_INT_SEQNODESOVER) {
+		/* BCH */
+		writel(NANDBCH_INT_CLR_SEQNODESOVER,
+		       nandi->base + NANDBCH_INT_CLR);
+		complete(&nandi->seq_completed);
+	}
+	if (status & NAND_INT_RBN) {
+		/* Hamming */
+		writel(NAND_INT_CLR_RBN, nandi->base + NANDHAM_INT_CLR);
+		complete(&nandi->rbn_completed);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void nandi_enable_interrupts(struct nandi_controller *nandi,
+				    uint32_t irqs)
+{
+	uint32_t val;
+
+	val = readl(nandi->base + NANDBCH_INT_EN);
+	val |= irqs;
+	writel(val, nandi->base + NANDBCH_INT_EN);
+}
+
+static void nandi_disable_interrupts(struct nandi_controller *nandi,
+				     uint32_t irqs)
+{
+	uint32_t val;
+
+	val = readl(nandi->base + NANDBCH_INT_EN);
+	val &= ~irqs;
+	writel(val, nandi->base + NANDBCH_INT_EN);
+}
+
 static int remap_named_resource(struct platform_device *pdev,
 				char *name,
 				void __iomem **io_ptr)
@@ -85,6 +133,7 @@ static struct nandi_controller *
 nandi_init_resources(struct platform_device *pdev)
 {
 	struct nandi_controller *nandi;
+	int irq;
 	int err;
 
 	nandi = devm_kzalloc(&pdev->dev, sizeof(*nandi), GFP_KERNEL);
@@ -104,6 +153,19 @@ nandi_init_resources(struct platform_device *pdev)
 	if (err)
 		return ERR_PTR(err);
 
+	irq = platform_get_irq_byname(pdev, "nand_irq");
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to find IRQ resource\n");
+		return ERR_PTR(irq);
+	}
+
+	err = devm_request_irq(&pdev->dev, irq, nandi_irq_handler,
+			       IRQF_DISABLED, dev_name(&pdev->dev), nandi);
+	if (err) {
+		dev_err(&pdev->dev, "irq request failed\n");
+		return ERR_PTR(err);
+	}
+
 	platform_set_drvdata(pdev, nandi);
 
 	return nandi;
-- 
1.8.3.2

RE: [RFC 05/47] mtd: nand: stm_nand_bch: IRQ support for ST's BCH NAND Controller driver

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>Obtain IRQ number and request IRQ resource via the usual methods. We're
>also registering an IRQ handler to inform us of any completed tasks.
>Notice that we're starting to make use of the device struct that we
>defined before. In keeping with the subject of the patch, we're also
>adding the related local enable_irq() and disable_irq() methods. Again,
>these will be utilised in a greater capacity in latter commits.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/mtd/nand/stm_nand_bch.c | 62 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index bd11070..76a0d02 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -15,11 +15,14 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/io.h>
>+#include <linux/interrupt.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
> #include <linux/completion.h>
> #include <linux/mtd/stm_nand.h>
>
>+#include "stm_nand_regs.h"
>+
> /* NANDi Controller (Hamming/BCH) */
> struct nandi_controller {
> 	void __iomem		*base;		/* Controller base*/
>@@ -54,6 +57,51 @@ struct nandi_controller {
> 						/* 'page_buf'             */
> };
>
>+/*
>+ * NANDi Interrupts (shared by Hamming and BCH controllers)
>+ */
>+static irqreturn_t nandi_irq_handler(int irq, void *dev)
>+{
>+	struct nandi_controller *nandi = dev;
>+	unsigned int status;
>+
>+	status = readl(nandi->base + NANDBCH_INT_STA);
>+
>+	if (status & NANDBCH_INT_SEQNODESOVER) {

You should also check, if BCH ECC mode is actually set to 
	struct nandi_controller *nandi = dev;
	if (nandi->bch_ecc_mode && (status & NANDBCH_INT_SEQNODESOVER)) {
>+		/* BCH */
>+		writel(NANDBCH_INT_CLR_SEQNODESOVER,
>+		       nandi->base + NANDBCH_INT_CLR);
>+		complete(&nandi->seq_completed);

At a given time only one of the two controllers (HAM/BCH) would be active. right ?
So, do don't need to check for both interrupts. you can return if either one is successful.
		return IRQ_HANDLED;  /* BCH ECC IRQ handles successfully */
>+	}

>+	if (status & NAND_INT_RBN) {
>+		/* Hamming */
>+		writel(NAND_INT_CLR_RBN, nandi->base + NANDHAM_INT_CLR);
>+		complete(&nandi->rbn_completed);
 -- same for this --
		return IRQ_HANDLED;   /* HAM ECC IRQ handled successfully */
>+	}
>+
>+	return IRQ_HANDLED;

And if no valid source is found then
	return IRQ_NONE;  /* spurious interrupt */

>+}
>+


with regards, pekon

[RFC 06/47] mtd: nand: stm_nand_bch: change between BCH and Hamming modes

[Lee Jones]

The EMISS is a device which, amongst other things, controls various
flash memory modes. We make use of it here merely to flip the
HAMMING_NOT_BCH bit dependant on whether we wish to operate in Hamming
or BCH modes. The STM BCH driver makes good use of the helper introduced
here.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/mtd/stm_nand.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h
index 9210d5c..da2006c 100644
--- a/include/linux/mtd/stm_nand.h
+++ b/include/linux/mtd/stm_nand.h
@@ -15,6 +15,8 @@
 #ifndef __LINUX_STM_NAND_H
 #define __LINUX_STM_NAND_H
 
+#include <linux/io.h>
+
 struct stm_plat_nand_bch_data {
 	struct stm_nand_bank_data *bank;
 	enum stm_nand_bch_ecc_config bch_ecc_cfg;
@@ -30,4 +32,44 @@ struct stm_plat_nand_bch_data {
 	bool flashss;
 };
 
+#define EMISS_BASE				0xfef01000
+#define EMISS_CONFIG				0x0000
+#define EMISS_CONFIG_HAMMING_NOT_BCH		(0x1 << 6)
+
+enum nandi_controllers {
+	STM_NANDI_UNCONFIGURED,
+	STM_NANDI_HAMMING,
+	STM_NANDI_BCH
+};
+
+static inline void emiss_nandi_select(enum nandi_controllers controller)
+{
+	unsigned v;
+	void __iomem *emiss_config_base;
+
+	emiss_config_base = ioremap(EMISS_BASE, 4);
+	if (!emiss_config_base) {
+		pr_err("%s: failed to ioremap EMISS\n", __func__);
+		return;
+	}
+
+	v = readl(emiss_config_base + EMISS_CONFIG);
+
+	if (controller == STM_NANDI_HAMMING) {
+		if (v & EMISS_CONFIG_HAMMING_NOT_BCH)
+			goto out;
+		v |= EMISS_CONFIG_HAMMING_NOT_BCH;
+	} else {
+		if (!(v & EMISS_CONFIG_HAMMING_NOT_BCH))
+			goto out;
+		v &= ~EMISS_CONFIG_HAMMING_NOT_BCH;
+	}
+
+	writel(v, emiss_config_base + EMISS_CONFIG);
+	readl(emiss_config_base + EMISS_CONFIG);
+
+out:
+	iounmap(emiss_config_base);
+}
+
 #endif /* __LINUX_STM_NAND_H */
-- 
1.8.3.2

[RFC 07/47] mtd: nand: stm_nand_bch: initialise the BCH Controller

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/stm_nand.h    | 20 +++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 76a0d02..1a93f8d 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -14,6 +14,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
@@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 	writel(val, nandi->base + NANDBCH_INT_EN);
 }
 
+static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank)
+{
+	dev_dbg(nandi->dev, "%s\n", __func__);
+
+	/* Initialise BCH Controller */
+	emiss_nandi_select(STM_NANDI_BCH);
+
+	/* Reset and disable boot-mode controller */
+	writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG);
+	udelay(1);
+	writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG);
+
+	/* Reset AFM controller */
+	writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG);
+	udelay(1);
+	writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG);
+
+	/* Set EMI Bank */
+	writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL);
+
+	/* Reset ECC stats */
+	writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG);
+	udelay(1);
+
+	/* Enable AFM */
+	writel(CFG_ENABLE_AFM, nandi->base + NANDBCH_CONTROLLER_CFG);
+
+	/* Configure Read DMA Plugs (values supplied by Validation) */
+	writel(0x00000005, nandi->dma + EMISS_NAND_RD_DMA_PAGE_SIZE);
+	writel(0x00000005, nandi->dma + EMISS_NAND_RD_DMA_MAX_OPCODE_SIZE);
+	writel(0x00000002, nandi->dma + EMISS_NAND_RD_DMA_MIN_OPCODE_SIZE);
+	writel(0x00000001, nandi->dma + EMISS_NAND_RD_DMA_MAX_CHUNK_SIZE);
+	writel(0x00000000, nandi->dma + EMISS_NAND_RD_DMA_MAX_MESSAGE_SIZE);
+
+	/* Configure Write DMA Plugs (values supplied by Validation) */
+	writel(0x00000005, nandi->dma + EMISS_NAND_WR_DMA_PAGE_SIZE);
+	writel(0x00000005, nandi->dma + EMISS_NAND_WR_DMA_MAX_OPCODE_SIZE);
+	writel(0x00000002, nandi->dma + EMISS_NAND_WR_DMA_MIN_OPCODE_SIZE);
+	writel(0x00000001, nandi->dma + EMISS_NAND_WR_DMA_MAX_CHUNK_SIZE);
+	writel(0x00000000, nandi->dma + EMISS_NAND_WR_DMA_MAX_MESSAGE_SIZE);
+
+	nandi_enable_interrupts(nandi, NAND_INT_ENABLE);
+}
+
+static void nandi_init_controller(struct nandi_controller *nandi,
+				  int emi_bank)
+{
+	nandi_init_bch(nandi, emi_bank);
+}
+
 static int remap_named_resource(struct platform_device *pdev,
 				char *name,
 				void __iomem **io_ptr)
@@ -174,6 +225,7 @@ nandi_init_resources(struct platform_device *pdev)
 static int stm_nand_bch_probe(struct platform_device *pdev)
 {
 	struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data;
+	struct stm_nand_bank_data *bank;
 	struct nandi_controller *nandi;
 
 	if (!pdata) {
@@ -190,6 +242,10 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	init_completion(&nandi->seq_completed);
 	init_completion(&nandi->rbn_completed);
 
+	bank = pdata->bank;
+	if (bank)
+		nandi_init_controller(nandi, bank->csn);
+
 	return 0;
 }
 
diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h
index da2006c..99a69ca 100644
--- a/include/linux/mtd/stm_nand.h
+++ b/include/linux/mtd/stm_nand.h
@@ -17,6 +17,26 @@
 
 #include <linux/io.h>
 
+/*
+ * Board-level specification relating to a 'bank' of NAND Flash
+ */
+struct stm_nand_bank_data {
+	int			csn;
+	int			nr_partitions;
+	struct mtd_partition	*partitions;
+	unsigned int		options;
+	unsigned int		bbt_options;
+
+	struct nand_timing_spec *timing_spec;
+
+	/*
+	 * No. of IP clk cycles by which to 'relax' the timing configuration.
+	 * Required on some boards to to accommodate board-level limitations.
+	 * Used in conjunction with 'nand_timing_spec' and ONFI configuration.
+	 */
+	int			timing_relax;
+};
+
 struct stm_plat_nand_bch_data {
 	struct stm_nand_bank_data *bank;
 	enum stm_nand_bch_ecc_config bch_ecc_cfg;
-- 
1.8.3.2

RE: [RFC 07/47] mtd: nand: stm_nand_bch: initialise the BCH Controller

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
missing commit log :-)
Though $subject is self-explanatory, but you can add more description about
assumption and hardware caveats about the controller, and its use.

>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++
> include/linux/mtd/stm_nand.h    | 20 +++++++++++++++
> 2 files changed, 76 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index 76a0d02..1a93f8d 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -14,6 +14,7 @@
>
> #include <linux/kernel.h>
> #include <linux/module.h>
>+#include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/interrupt.h>
> #include <linux/device.h>
>@@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
> 	writel(val, nandi->base + NANDBCH_INT_EN);
> }
>
>+static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank)
>+{
>+	dev_dbg(nandi->dev, "%s\n", __func__);
>+
>+	/* Initialise BCH Controller */
>+	emiss_nandi_select(STM_NANDI_BCH);
>+
>+	/* Reset and disable boot-mode controller */
>+	writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG);
>+	udelay(1);
>+	writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG);

Why using 'udelay' ?
Isn't there any status register which tells you that controller is reset / initialized ?
Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status.

>+
>+	/* Reset AFM controller */
>+	writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG);
>+	udelay(1);
>+	writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG);
>+
>+	/* Set EMI Bank */
>+	writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL);
>+
>+	/* Reset ECC stats */
>+	writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG);
>+	udelay(1);
>+
"0x7f0" ?? please use Macro instead.


with regards, pekon

Re: [RFC 07/47] mtd: nand: stm_nand_bch: initialise the BCH Controller

[Lee Jones]

On Wed, 26 Mar 2014, Gupta, Pekon wrote:

> Hi Lee,
> 
> >From: Lee Jones [mailto:lee.jones@linaro.org]
> missing commit log :-)
> Though $subject is self-explanatory, but you can add more description about
> assumption and hardware caveats about the controller, and its use.
> 
> >
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> > drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++
> > include/linux/mtd/stm_nand.h    | 20 +++++++++++++++
> > 2 files changed, 76 insertions(+)
> >
> >diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
> >index 76a0d02..1a93f8d 100644
> >--- a/drivers/mtd/nand/stm_nand_bch.c
> >+++ b/drivers/mtd/nand/stm_nand_bch.c
> >@@ -14,6 +14,7 @@
> >
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> >+#include <linux/delay.h>
> > #include <linux/io.h>
> > #include <linux/interrupt.h>
> > #include <linux/device.h>
> >@@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
> > 	writel(val, nandi->base + NANDBCH_INT_EN);
> > }
> >
> >+static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank)
> >+{
> >+	dev_dbg(nandi->dev, "%s\n", __func__);
> >+
> >+	/* Initialise BCH Controller */
> >+	emiss_nandi_select(STM_NANDI_BCH);
> >+
> >+	/* Reset and disable boot-mode controller */
> >+	writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG);
> >+	udelay(1);
> >+	writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG);
> 
> Why using 'udelay' ?
> Isn't there any status register which tells you that controller is reset / initialized ?
> Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status.

Documenation says:

  "The soft reset bit has to be reset to ‘0’ to de-assert the soft
   reset. The soft reset bit is expected to be asserted for at least
   one clock cycle for proper reset"

> >+
> >+	/* Reset AFM controller */
> >+	writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG);
> >+	udelay(1);
> >+	writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG);
> >+
> >+	/* Set EMI Bank */
> >+	writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL);
> >+
> >+	/* Reset ECC stats */
> >+	writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG);
> >+	udelay(1);
> >+
> "0x7f0" ?? please use Macro instead.

Sure.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [RFC 07/47] mtd: nand: stm_nand_bch: initialise the BCH Controller

[Gupta, Pekon]

>From: Lee Jones [mailto:lee.jones@linaro.org]
>>On Wed, 26 Mar 2014, Gupta, Pekon wrote:
[...]

>> >+	/* Reset and disable boot-mode controller */
>> >+	writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG);
>> >+	udelay(1);
>> >+	writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG);
>>
>> Why using 'udelay' ?
>> Isn't there any status register which tells you that controller is reset / initialized ?
>> Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status.
>
>Documenation says:
>
>  "The soft reset bit has to be reset to ‘0’ to de-assert the soft
>   reset. The soft reset bit is expected to be asserted for at least
>   one clock cycle for proper reset"
>
That’s the hardware way of saying that 'enable the clock before applying reset'.
Clock is required to propagate reset-logic to flip-flops in pipeline, which do not get direct reset.

However that apart. You may safely drop udelay(1) because this 'udelay' is at
CPU side and won't guarantee anything about clocks at your controller side.
But I leave it to you as this delay is pretty small.


with regards, pekon

Re: [RFC 07/47] mtd: nand: stm_nand_bch: initialise the BCH Controller

[Lee Jones]

> >> >+	/* Reset and disable boot-mode controller */
> >> >+	writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG);
> >> >+	udelay(1);
> >> >+	writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG);
> >>
> >> Why using 'udelay' ?
> >> Isn't there any status register which tells you that controller is reset / initialized ?
> >> Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status.
> >
> >Documenation says:
> >
> >  "The soft reset bit has to be reset to ‘0’ to de-assert the soft
> >   reset. The soft reset bit is expected to be asserted for at least
> >   one clock cycle for proper reset"
> >
> That’s the hardware way of saying that 'enable the clock before applying reset'.
> Clock is required to propagate reset-logic to flip-flops in pipeline, which do not get direct reset.
> 
> However that apart. You may safely drop udelay(1) because this 'udelay' is at
> CPU side and won't guarantee anything about clocks at your controller side.
> But I leave it to you as this delay is pretty small.

I'd like to keep it in if it's all the same to you.  The original
author is pretty competent and I like to think that it's there for a
reason - and as you rightly say, the delay is pretty small.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 08/47] mtd: nand: stm_nand_bch: supply clock support

[Lee Jones]

Add support for clocks when, and only when, they are supplied. It is
not yet compulsory to provide the BCH and EMI clocks, as Common Clk isn't
supported Mainline yet. Until an implementation lands upstream all clocks
located on STM boards default to always-on.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 49 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 1a93f8d..cc0159e 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
@@ -28,6 +29,9 @@
 struct nandi_controller {
 	void __iomem		*base;		/* Controller base*/
 	void __iomem		*dma;		/* DMA control base */
+
+	struct clk		*bch_clk;
+	struct clk		*emi_clk;
 						/* IRQ-triggered Completions: */
 	struct completion	seq_completed;	/*   SEQ Over */
 	struct completion	rbn_completed;	/*   RBn */
@@ -103,6 +107,44 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 	writel(val, nandi->base + NANDBCH_INT_EN);
 }
 
+static void nandi_clk_enable(struct nandi_controller *nandi)
+{
+	if (nandi->emi_clk)
+		clk_prepare_enable(nandi->emi_clk);
+	if (nandi->bch_clk)
+		clk_prepare_enable(nandi->bch_clk);
+}
+
+static void nandi_clk_disable(struct nandi_controller *nandi)
+{
+	if (nandi->emi_clk)
+		clk_disable_unprepare(nandi->emi_clk);
+	if (nandi->bch_clk)
+		clk_disable_unprepare(nandi->bch_clk);
+}
+
+static struct clk *nandi_clk_setup(struct nandi_controller *nandi,
+				   char *clkname)
+{
+	struct clk *clk;
+	int ret;
+
+	clk = clk_get(nandi->dev, clkname);
+	if (IS_ERR_OR_NULL(clk)) {
+		dev_warn(nandi->dev, "Failed to get %s clock\n", clkname);
+		return NULL;
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_warn(nandi->dev, "Failed to enable %s clock\n", clkname);
+		clk_put(clk);
+		return NULL;
+	}
+
+	return clk;
+}
+
 static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank)
 {
 	dev_dbg(nandi->dev, "%s\n", __func__);
@@ -217,6 +259,9 @@ nandi_init_resources(struct platform_device *pdev)
 		return ERR_PTR(err);
 	}
 
+	nandi->emi_clk = nandi_clk_setup(nandi, "emi_clk");
+	nandi->bch_clk = nandi_clk_setup(nandi, "bch_clk");
+
 	platform_set_drvdata(pdev, nandi);
 
 	return nandi;
@@ -251,6 +296,10 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 
 static int stm_nand_bch_remove(struct platform_device *pdev)
 {
+	struct nandi_controller *nandi = platform_get_drvdata(pdev);
+
+	nandi_clk_disable(nandi);
+
 	return 0;
 }
 
-- 
1.8.3.2

RE: [RFC 08/47] mtd: nand: stm_nand_bch: supply clock support

[Gupta, Pekon]

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>Add support for clocks when, and only when, they are supplied. It is
>not yet compulsory to provide the BCH and EMI clocks, as Common Clk isn't
>supported Mainline yet. Until an implementation lands upstream all clocks
>located on STM boards default to always-on.
>
Good to put this information in comments tagged with "FixMe" at relevant place in the code.

>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/mtd/nand/stm_nand_bch.c | 49 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index 1a93f8d..cc0159e 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/io.h>
>+#include <linux/clk.h>
> #include <linux/interrupt.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
>@@ -28,6 +29,9 @@
> struct nandi_controller {
> 	void __iomem		*base;		/* Controller base*/
> 	void __iomem		*dma;		/* DMA control base */
>+
>+	struct clk		*bch_clk;
>+	struct clk		*emi_clk;
> 						/* IRQ-triggered Completions: */
> 	struct completion	seq_completed;	/*   SEQ Over */
> 	struct completion	rbn_completed;	/*   RBn */
>@@ -103,6 +107,44 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
> 	writel(val, nandi->base + NANDBCH_INT_EN);
> }
>
>+static void nandi_clk_enable(struct nandi_controller *nandi)
>+{
>+	if (nandi->emi_clk)
>+		clk_prepare_enable(nandi->emi_clk);
>+	if (nandi->bch_clk)
>+		clk_prepare_enable(nandi->bch_clk);
>+}
>+
You are using nandi_clk_enable() only in your PM patch.
So better introduce above functions there ..
	[RFC 11/47] mtd: nand: stm_nand_bch: add Power Management

Also, you have some un-related comment changes for nandi_clk_enable()
in below patch. All that can be merges into single one.
	[RFC 14/47] mtd: nand: stm_nand_bch: configure BCH and FLEX by ONFI timing mode


>+static void nandi_clk_disable(struct nandi_controller *nandi)
>+{
>+	if (nandi->emi_clk)
>+		clk_disable_unprepare(nandi->emi_clk);
>+	if (nandi->bch_clk)
>+		clk_disable_unprepare(nandi->bch_clk);
>+}
>+
same, please move this to
	[RFC 11/47] mtd: nand: stm_nand_bch: add Power Management

Also as Ezequiel suggested.
You can trim down the number of patches by submitting only the main portion
of driver first. PM other features can be added as separate patch-set.


with regards, pekon

[RFC 09/47] mtd: nand: stm_nand_bch: introduce and initialise some important data structures

[Lee Jones]

Provide some more in-depth structures which will be used heavily within
the driver. We also add a convenience function, used to set the default
values.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 76 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index cc0159e..7333ad6 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -21,10 +21,30 @@
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <linux/completion.h>
+#include <linux/mtd/nand.h>
 #include <linux/mtd/stm_nand.h>
 
 #include "stm_nand_regs.h"
 
+/* Bad Block Table (BBT) */
+struct nandi_bbt_info {
+	uint32_t	bbt_size;		/* Size of bad-block table */
+	uint32_t	bbt_vers[2];		/* Version (Primary/Mirror) */
+	uint32_t	bbt_block[2];		/* Block No. (Primary/Mirror) */
+	uint8_t		*bbt;			/* Table data */
+};
+
+/* Collection of MTD/NAND device information */
+struct nandi_info {
+	struct mtd_info		mtd;		/* MTD info */
+	struct nand_chip	chip;		/* NAND chip info */
+
+	struct nand_ecclayout	ecclayout;	/* MTD ECC layout */
+	struct nandi_bbt_info	bbt_info;	/* Bad Block Table */
+	int			nr_parts;	/* Number of MTD partitions */
+	struct	mtd_partition	*parts;		/* MTD partitions */
+};
+
 /* NANDi Controller (Hamming/BCH) */
 struct nandi_controller {
 	void __iomem		*base;		/* Controller base*/
@@ -60,6 +80,8 @@ struct nandi_controller {
 
 	int			cached_page;	/* page number of page in */
 						/* 'page_buf'             */
+
+	struct nandi_info	info;		/* NAND device info */
 };
 
 /*
@@ -107,6 +129,46 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 	writel(val, nandi->base + NANDBCH_INT_EN);
 }
 
+static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
+				   struct mtd_info *mtd, struct nand_chip *chip)
+{
+	struct nandi_info *info = &nandi->info;
+	int i;
+
+	/* ecclayout */
+	info->ecclayout.eccbytes = mtd->oobsize;
+	for (i = 0; i < 64; i++)
+		info->ecclayout.eccpos[i] = i;
+	info->ecclayout.oobfree[0].offset = 0;
+	info->ecclayout.oobfree[0].length = 0;
+	info->ecclayout.oobavail = 0;
+
+	/* nand_chip */
+	chip->controller = &chip->hwcontrol;
+	spin_lock_init(&chip->controller->lock);
+	init_waitqueue_head(&chip->controller->wq);
+	chip->state = FL_READY;
+	chip->priv = nandi;
+	chip->ecc.layout = &info->ecclayout;
+
+	/* mtd_info */
+	mtd->owner = THIS_MODULE;
+	mtd->type = MTD_NANDFLASH;
+	mtd->flags = MTD_CAP_NANDFLASH;
+	mtd->ecclayout = &info->ecclayout;
+	mtd->oobavail = 0;
+	mtd->subpage_sft = 0;
+
+	mtd->_point = NULL;
+	mtd->_unpoint = NULL;
+	mtd->_lock = NULL;
+	mtd->_unlock = NULL;
+
+	mtd->_sync = nand_sync;
+	mtd->_suspend = nand_suspend;
+	mtd->_resume = nand_resume;
+}
+
 static void nandi_clk_enable(struct nandi_controller *nandi)
 {
 	if (nandi->emi_clk)
@@ -271,7 +333,11 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 {
 	struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data;
 	struct stm_nand_bank_data *bank;
+	struct nandi_bbt_info *bbt_info;
 	struct nandi_controller *nandi;
+	struct nandi_info *info;
+	struct nand_chip *chip;
+	struct mtd_info *mtd;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "no platform data found\n");
@@ -291,6 +357,16 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	if (bank)
 		nandi_init_controller(nandi, bank->csn);
 
+	info            = &nandi->info;
+	chip            = &info->chip;
+	bbt_info        = &info->bbt_info;
+	mtd             = &info->mtd;
+	mtd->priv       = chip;
+	mtd->name       = dev_name(&pdev->dev);
+	mtd->dev.parent = &pdev->dev;
+
+	nandi_set_mtd_defaults(nandi, mtd, chip);
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 10/47] mtd: nand: stm_nand_bch: initialise the Hamming Controller

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 7333ad6..8ab8e7b 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -207,6 +207,41 @@ static struct clk *nandi_clk_setup(struct nandi_controller *nandi,
 	return clk;
 }
 
+static void nandi_init_hamming(struct nandi_controller *nandi, int emi_bank)
+{
+	dev_dbg(nandi->dev, "%s\n", __func__);
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	/* Reset and disable boot-mode controller */
+	writel(BOOT_CFG_RESET, nandi->base + NANDHAM_BOOTBANK_CFG);
+	udelay(1);
+	writel(0x00000000, nandi->base + NANDHAM_BOOTBANK_CFG);
+
+	/* Reset controller */
+	writel(CFG_RESET, nandi->base + NANDHAM_FLEXMODE_CFG);
+	udelay(1);
+	writel(0x00000000, nandi->base + NANDHAM_FLEXMODE_CFG);
+
+	/* Set EMI Bank */
+	writel(0x1 << emi_bank, nandi->base + NANDHAM_FLEX_MUXCTRL);
+
+	/* Enable FLEX mode */
+	writel(CFG_ENABLE_FLEX, nandi->base + NANDHAM_FLEXMODE_CFG);
+
+	/* Configure FLEX_DATA_READ/WRITE for 1-byte access */
+	writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+	writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+
+	/* RBn interrupt on rising edge */
+	writel(NAND_EDGE_CFG_RBN_RISING, nandi->base + NANDHAM_INT_EDGE_CFG);
+
+	/* Enable interrupts */
+	nandi_enable_interrupts(nandi, NAND_INT_ENABLE);
+}
+
 static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank)
 {
 	dev_dbg(nandi->dev, "%s\n", __func__);
@@ -255,6 +290,7 @@ static void nandi_init_controller(struct nandi_controller *nandi,
 				  int emi_bank)
 {
 	nandi_init_bch(nandi, emi_bank);
+	nandi_init_hamming(nandi, emi_bank);
 }
 
 static int remap_named_resource(struct platform_device *pdev,
-- 
1.8.3.2

[RFC 11/47] mtd: nand: stm_nand_bch: add Power Management

[Lee Jones]

Populate the standard PM call-backs for; suspend, resume and restore.

Signed-off-by: Lee Jones <lee.jones@linaro.org>

q
---
 drivers/mtd/nand/stm_nand_bch.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 8ab8e7b..e1ebf92 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -415,12 +415,51 @@ static int stm_nand_bch_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int stm_nand_bch_suspend(struct device *dev)
+{
+	struct nandi_controller *nandi = dev_get_drvdata(dev);
+
+	nandi_clk_disable(nandi);
+
+	return 0;
+}
+static int stm_nand_bch_resume(struct device *dev)
+{
+	struct nandi_controller *nandi = dev_get_drvdata(dev);
+
+	nandi_clk_enable(nandi);
+
+	return 0;
+}
+
+static int stm_nand_bch_restore(struct device *dev)
+{
+	struct nandi_controller *nandi = dev_get_drvdata(dev);
+	struct stm_plat_nand_bch_data *pdata = dev->platform_data;
+	struct stm_nand_bank_data *bank = pdata->bank;
+
+	nandi_init_controller(nandi, bank->csn);
+
+	return 0;
+}
+
+static const struct dev_pm_ops stm_nand_bch_pm_ops = {
+	.suspend = stm_nand_bch_suspend,
+	.resume = stm_nand_bch_resume,
+	.restore = stm_nand_bch_restore,
+};
+#else
+static const struct dev_pm_ops stm_nand_bch_pm_ops;
+#endif
+
 static struct platform_driver stm_nand_bch_driver = {
 	.probe	= stm_nand_bch_probe ,
 	.remove	= stm_nand_bch_remove,
 	.driver	= {
 		.name	= "stm-nand-bch",
 		.owner	= THIS_MODULE,
+		.pm	= &stm_nand_bch_pm_ops,
 	},
 };
 module_platform_driver(stm_nand_bch_driver);
-- 
1.8.3.2

[RFC 12/47] mtd: nand: stm_nand_bch: scan for NAND devices

[Lee Jones]

Use the core nand_scan_ident() routine to locate connected NAND chips.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index e1ebf92..1de13bd 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -374,6 +374,7 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	struct nandi_info *info;
 	struct nand_chip *chip;
 	struct mtd_info *mtd;
+	int err;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "no platform data found\n");
@@ -403,6 +404,10 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 
 	nandi_set_mtd_defaults(nandi, mtd, chip);
 
+	err = nand_scan_ident(mtd, 1, NULL);
+	if (err)
+		return err;
+
 	return 0;
 }
 
@@ -410,6 +415,8 @@ static int stm_nand_bch_remove(struct platform_device *pdev)
 {
 	struct nandi_controller *nandi = platform_get_drvdata(pdev);
 
+	nand_release(&nandi->info.mtd);
+
 	nandi_clk_disable(nandi);
 
 	return 0;
-- 
1.8.3.2

[RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Lee Jones]

Fetch platform specific data from Device Tree. Any functions which
are useful to other STM NAND Controllers have been separated into a
separate file so they can be easily referenced by them as they
appear.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/Kconfig        |   7 ++
 drivers/mtd/nand/Makefile       |   1 +
 drivers/mtd/nand/stm_nand_bch.c |  68 ++++++++++++++++++-
 drivers/mtd/nand/stm_nand_dt.c  | 146 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/nand/stm_nand_dt.h  |  39 +++++++++++
 include/linux/mtd/stm_nand.h    |   9 +++
 6 files changed, 268 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mtd/nand/stm_nand_dt.c
 create mode 100644 drivers/mtd/nand/stm_nand_dt.h

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 2bf972c..28155c0 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -516,4 +516,11 @@ config MTD_NAND_STM_BCH
 	help
 	  Adds support for the STMicroelectronics NANDi BCH Controller.
 
+config MTD_NAND_STM_BCH_DT
+	tristate "STMicroelectronics: NANDi BCH Controller Device Tree support"
+	default MTD_NAND_STM_BCH if OF
+	help
+	  Adds support for the STMicroelectronics NANDi BCH Controller's
+	  Device Tree component.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 94e7737..890f47f 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MTD_NAND_MPC5121_NFC)	+= mpc5121_nfc.o
 obj-$(CONFIG_MTD_NAND_RICOH)		+= r852.o
 obj-$(CONFIG_MTD_NAND_JZ4740)		+= jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_STM_BCH)		+= stm_nand_bch.o
+obj-$(CONFIG_MTD_NAND_STM_BCH_DT)	+= stm_nand_dt.o
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)	+= gpmi-nand/
 obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 1de13bd..3115fb4 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
@@ -23,8 +24,10 @@
 #include <linux/completion.h>
 #include <linux/mtd/nand.h>
 #include <linux/mtd/stm_nand.h>
+#include <linux/mtd/partitions.h>
 
 #include "stm_nand_regs.h"
+#include "stm_nand_dt.h"
 
 /* Bad Block Table (BBT) */
 struct nandi_bbt_info {
@@ -365,9 +368,51 @@ nandi_init_resources(struct platform_device *pdev)
 	return nandi;
 }
 
+#ifdef CONFIG_OF
+static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct stm_plat_nand_bch_data *pdata;
+	const char *ecc_config;
+	int ret;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	of_property_read_string(np, "st,bch-ecc-mode", &ecc_config);
+	if (!strcmp("noecc", ecc_config))
+		pdata->bch_ecc_cfg = BCH_NO_ECC;
+	else if (!strcmp("18bit", ecc_config))
+		pdata->bch_ecc_cfg = BCH_18BIT_ECC;
+	else if (!strcmp("30bit", ecc_config))
+		pdata->bch_ecc_cfg = BCH_30BIT_ECC;
+	else
+		pdata->bch_ecc_cfg = BCH_ECC_AUTO;
+
+	ret = stm_of_get_nand_banks(&pdev->dev, np, &pdata->bank);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	pdata->flashss = of_property_read_bool(np, "st,nand-flashss");
+
+	of_property_read_u32(np, "st,bch-bitflip-threshold",
+			     &pdata->bch_bitflip_threshold);
+
+	return pdata;
+}
+#else
+static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
 static int stm_nand_bch_probe(struct platform_device *pdev)
 {
 	struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data;
+	struct device_node *np = pdev->dev.of_node;
+	struct mtd_part_parser_data ppdata;
 	struct stm_nand_bank_data *bank;
 	struct nandi_bbt_info *bbt_info;
 	struct nandi_controller *nandi;
@@ -377,8 +422,18 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	int err;
 
 	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data found\n");
-		return -EINVAL;
+		if (!np) {
+			dev_err(&pdev->dev, "no pdata or DT found\n");
+			return -EINVAL;
+		}
+
+		pdata = stm_bch_dt_get_pdata(pdev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+
+		ppdata.of_node = stm_of_get_partitions_node(np, 0);
+
+		pdev->dev.platform_data = pdata;
 	}
 
 	nandi = nandi_init_resources(pdev);
@@ -460,12 +515,21 @@ static const struct dev_pm_ops stm_nand_bch_pm_ops = {
 static const struct dev_pm_ops stm_nand_bch_pm_ops;
 #endif
 
+#ifdef CONFIG_OF
+static struct of_device_id nand_bch_match[] = {
+	{ .compatible = "st,nand-bch", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, nand_bch_match);
+#endif
+
 static struct platform_driver stm_nand_bch_driver = {
 	.probe	= stm_nand_bch_probe ,
 	.remove	= stm_nand_bch_remove,
 	.driver	= {
 		.name	= "stm-nand-bch",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(nand_bch_match),
 		.pm	= &stm_nand_bch_pm_ops,
 	},
 };
diff --git a/drivers/mtd/nand/stm_nand_dt.c b/drivers/mtd/nand/stm_nand_dt.c
new file mode 100644
index 0000000..0e91bba
--- /dev/null
+++ b/drivers/mtd/nand/stm_nand_dt.c
@@ -0,0 +1,146 @@
+/*
+ * drivers/mtd/nand/stm_nand_dt.c
+ *
+ * Support for NANDi BCH Controller Device Tree component
+ *
+ * Copyright (c) 2014 STMicroelectronics Limited
+ * Author: Author: Srinivas Kandagatla <srinivas.kandagatla@st.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/byteorder/generic.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/stm_nand.h>
+
+/**
+* stm_of_get_partitions_node - get partitions node from stm-nand type devices.
+*
+* @dev		device pointer to use for devm allocations.
+* @np		device node of the driver.
+* @bank_nr	which bank number to use to get partitions.
+*
+* Returns a node pointer if found, with refcount incremented, use
+* of_node_put() on it when done.
+*
+*/
+struct device_node *stm_of_get_partitions_node(struct device_node *np,
+					       int bank_nr)
+{
+	struct device_node *banksnp, *banknp, *partsnp = NULL;
+	char name[10];
+
+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
+	if (!banksnp)
+		return NULL;
+
+	sprintf(name, "bank%d", bank_nr);
+	banknp = of_get_child_by_name(banksnp, name);
+	if (banknp)
+		return NULL;
+
+	partsnp = of_get_child_by_name(banknp, "partitions");
+	of_node_put(banknp);
+
+	return partsnp;
+}
+EXPORT_SYMBOL(stm_of_get_partitions_node);
+
+/**
+ * stm_of_get_nand_banks - Get nand banks info from a given device node.
+ *
+ * @dev			device pointer to use for devm allocations.
+ * @np			device node of the driver.
+ * @banksptr		double pointer to banks which is allocated
+ *			and filled with bank data.
+ *
+ * Returns a count of banks found in the given device node.
+ *
+ */
+int stm_of_get_nand_banks(struct device *dev, struct device_node *np,
+			  struct stm_nand_bank_data **banksptr)
+{
+	struct stm_nand_bank_data *banks;
+	struct device_node *banknp, *banksnp;
+	int nr_banks = 0;
+
+	if (!np)
+		return -ENODEV;
+
+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
+	if (!banksnp) {
+		dev_warn(dev, "No NAND banks specified in DT: %s\n",
+			 np->full_name);
+		return -ENODEV;
+	}
+
+	for_each_child_of_node(banksnp, banknp)
+		nr_banks++;
+
+	*banksptr = devm_kzalloc(dev, sizeof(*banks) * nr_banks, GFP_KERNEL);
+	banks = *banksptr;
+	banknp = NULL;
+
+	for_each_child_of_node(banksnp, banknp) {
+		struct device_node *timingnp;
+		int bank = 0;
+
+		of_property_read_u32(banknp, "st,nand-csn", &banks[bank].csn);
+
+		if (of_get_nand_bus_width(banknp) == 16)
+			banks[bank].options |= NAND_BUSWIDTH_16;
+		if (of_get_nand_on_flash_bbt(banknp))
+			banks[bank].bbt_options |= NAND_BBT_USE_FLASH;
+
+		banks[bank].nr_partitions = 0;
+		banks[bank].partitions = NULL;
+
+		timingnp = of_parse_phandle(banknp, "st,nand-timing-spec", 0);
+		if (timingnp) {
+			struct nand_timing_spec *ts;
+
+			ts = devm_kzalloc(dev, sizeof(struct nand_timing_spec),
+					  GFP_KERNEL);
+
+			of_property_read_u32(timingnp, "tR", &ts->tR);
+			of_property_read_u32(timingnp, "tCLS", &ts->tCLS);
+			of_property_read_u32(timingnp, "tCS", &ts->tCS);
+			of_property_read_u32(timingnp, "tALS", &ts->tALS);
+			of_property_read_u32(timingnp, "tDS", &ts->tDS);
+			of_property_read_u32(timingnp, "tWP", &ts->tWP);
+			of_property_read_u32(timingnp, "tCLH", &ts->tCLH);
+			of_property_read_u32(timingnp, "tCH", &ts->tCH);
+			of_property_read_u32(timingnp, "tALH", &ts->tALH);
+			of_property_read_u32(timingnp, "tDH", &ts->tDH);
+			of_property_read_u32(timingnp, "tWB", &ts->tWB);
+			of_property_read_u32(timingnp, "tWH", &ts->tWH);
+			of_property_read_u32(timingnp, "tWC", &ts->tWC);
+			of_property_read_u32(timingnp, "tRP", &ts->tRP);
+			of_property_read_u32(timingnp, "tREH", &ts->tREH);
+			of_property_read_u32(timingnp, "tRC", &ts->tRC);
+			of_property_read_u32(timingnp, "tREA", &ts->tREA);
+			of_property_read_u32(timingnp, "tRHOH", &ts->tRHOH);
+			of_property_read_u32(timingnp, "tCEA", &ts->tCEA);
+			of_property_read_u32(timingnp, "tCOH", &ts->tCOH);
+			of_property_read_u32(timingnp, "tCHZ", &ts->tCHZ);
+			of_property_read_u32(timingnp, "tCSD", &ts->tCSD);
+			banks[bank].timing_spec = ts;
+		}
+		of_property_read_u32(banknp, "st,nand-timing-relax",
+				     &banks[bank].timing_relax);
+		bank++;
+	}
+
+	return nr_banks;
+}
+EXPORT_SYMBOL(stm_of_get_nand_banks);
diff --git a/drivers/mtd/nand/stm_nand_dt.h b/drivers/mtd/nand/stm_nand_dt.h
new file mode 100644
index 0000000..de4507c
--- /dev/null
+++ b/drivers/mtd/nand/stm_nand_dt.h
@@ -0,0 +1,39 @@
+/*
+ * drivers/mtd/nand/stm_nand_dt.h
+ *
+ * Support for NANDi BCH Controller Device Tree component
+ *
+ * Copyright (c) 2014 STMicroelectronics Limited
+ * Author: Author: Srinivas Kandagatla <srinivas.kandagatla@st.com>
+ *
+ * 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.
+ *
+ */
+
+#ifndef STM_NAND_DT_H
+#define STM_NAND_DT_H
+
+#if defined(CONFIG_MTD_NAND_STM_BCH_DT)
+struct device_node *stm_of_get_partitions_node(struct device_node *np,
+					       int bank_nr);
+
+int stm_of_get_nand_banks(struct device *dev, struct device_node *np,
+			  struct stm_nand_bank_data **banksp);
+#else
+static inline
+struct device_node *stm_of_get_partitions_node(struct device_node *np,
+					       int bank_nr)
+{
+	return NULL;
+}
+
+static inline int  stm_of_get_nand_banks(struct device *dev,
+					 struct device_node *np,
+					 struct stm_nand_bank_data **banksp)
+{
+	return 0;
+}
+#endif /* CONFIG_MTD_NAND_STM_BCH_DT */
+#endif /* STM_NAND_DT_H */
diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h
index 99a69ca..f82f9f7 100644
--- a/include/linux/mtd/stm_nand.h
+++ b/include/linux/mtd/stm_nand.h
@@ -37,6 +37,15 @@ struct stm_nand_bank_data {
 	int			timing_relax;
 };
 
+/* ECC Modes */
+enum stm_nand_bch_ecc_config {
+	BCH_18BIT_ECC = 0,
+	BCH_30BIT_ECC,
+	BCH_NO_ECC,
+	BCH_ECC_RSRV,
+	BCH_ECC_AUTO,
+};
+
 struct stm_plat_nand_bch_data {
 	struct stm_nand_bank_data *bank;
 	enum stm_nand_bch_ecc_config bch_ecc_cfg;
-- 
1.8.3.2

RE: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>Fetch platform specific data from Device Tree. Any functions which
>are useful to other STM NAND Controllers have been separated into a
>separate file so they can be easily referenced by them as they
>appear.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
[...]

>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -16,6 +16,7 @@
> #include <linux/module.h>
> #include <linux/delay.h>
> #include <linux/io.h>
>+#include <linux/of.h>
> #include <linux/clk.h>
> #include <linux/interrupt.h>
> #include <linux/device.h>
>@@ -23,8 +24,10 @@
> #include <linux/completion.h>
> #include <linux/mtd/nand.h>
> #include <linux/mtd/stm_nand.h>
>+#include <linux/mtd/partitions.h>
>
> #include "stm_nand_regs.h"
>+#include "stm_nand_dt.h"
>
> /* Bad Block Table (BBT) */
> struct nandi_bbt_info {
>@@ -365,9 +368,51 @@ nandi_init_resources(struct platform_device *pdev)
> 	return nandi;
> }
>
>+#ifdef CONFIG_OF
>+static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
>+{
>+	struct device_node *np = pdev->dev.of_node;
>+	struct stm_plat_nand_bch_data *pdata;
>+	const char *ecc_config;
>+	int ret;
>+
>+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>+	if (!pdata)
>+		return ERR_PTR(-ENOMEM);
>+
>+	of_property_read_string(np, "st,bch-ecc-mode", &ecc_config);

Can you use any of the existing generic NAND bindings like combination of
"nand-ecc-mode" and "nand-ecc-strength" ?
Example: nand-ecc-mode = "bch" & nand-ecc-strength=18
Some of generic bindings are recently added
	commit 8dd49165ef5f46b5ad9ba296c559ccff315f9421  (currently in l2-mtd tree)
	mtd: nand: Add a devicetree binding for ECC strength and ECC step size

It's good to use generic bindings as much as you can, and you don't need
any approvals from DT Maintainers too. Though you may need to add
new values for "nand-ecc-mode" like {auto, hw-ham, hw-bch}
 

>+	if (!strcmp("noecc", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_NO_ECC;
>+	else if (!strcmp("18bit", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_18BIT_ECC;
>+	else if (!strcmp("30bit", ecc_config))
>+		pdata->bch_ecc_cfg = BCH_30BIT_ECC;
>+	else
>+		pdata->bch_ecc_cfg = BCH_ECC_AUTO;
>+
>+	ret = stm_of_get_nand_banks(&pdev->dev, np, &pdata->bank);
>+	if (ret < 0)
>+		return ERR_PTR(ret);
>+
>+	pdata->flashss = of_property_read_bool(np, "st,nand-flashss");
>+
>+	of_property_read_u32(np, "st,bch-bitflip-threshold",
>+			     &pdata->bch_bitflip_threshold);
>+
mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
And then can be re-configured for each MTD partition separately 
	/sys/class/mtd/mtdX/bitflip_threshold
	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
So, I don't think this is a HW parameter, and so should not be passed from DT.


>+	return pdata;
>+}
>+#else

[...]

>+struct device_node *stm_of_get_partitions_node(struct device_node *np,
>+					       int bank_nr)
>+{
>+	struct device_node *banksnp, *banknp, *partsnp = NULL;
>+	char name[10];
>+
>+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
>+	if (!banksnp)
>+		return NULL;
>+
>+	sprintf(name, "bank%d", bank_nr);
>+	banknp = of_get_child_by_name(banksnp, name);
>+	if (banknp)
>+		return NULL;
>+
>+	partsnp = of_get_child_by_name(banknp, "partitions");
>+	of_node_put(banknp);
>+
Sorry, I'm bit confused here .. I think you don't need to find children of
Your bank node. This should already taken care in default parser
drivers/mtd/ofpart.c : parse_ofpart_partitions()
And all you need to pass is 'of_node' of bank (device).
Is my understanding correct ?

>+	return partsnp;
>+}
>+EXPORT_SYMBOL(stm_of_get_partitions_node);
>+


with regards, pekon

Re: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Lee Jones]

> >From: Lee Jones [mailto:lee.jones@linaro.org]
> >
> >Fetch platform specific data from Device Tree. Any functions which
> >are useful to other STM NAND Controllers have been separated into a
> >separate file so they can be easily referenced by them as they
> >appear.
> >
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>

[...]

> >+#ifdef CONFIG_OF
> >+static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
> >+{
> >+	struct device_node *np = pdev->dev.of_node;
> >+	struct stm_plat_nand_bch_data *pdata;
> >+	const char *ecc_config;
> >+	int ret;
> >+
> >+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> >+	if (!pdata)
> >+		return ERR_PTR(-ENOMEM);
> >+
> >+	of_property_read_string(np, "st,bch-ecc-mode", &ecc_config);
> 
> Can you use any of the existing generic NAND bindings like combination of
> "nand-ecc-mode" and "nand-ecc-strength" ?
> Example: nand-ecc-mode = "bch" & nand-ecc-strength=18
> Some of generic bindings are recently added
> 	commit 8dd49165ef5f46b5ad9ba296c559ccff315f9421  (currently in l2-mtd tree)
> 	mtd: nand: Add a devicetree binding for ECC strength and ECC step size
> 
> It's good to use generic bindings as much as you can, and you don't need
> any approvals from DT Maintainers too. Though you may need to add
> new values for "nand-ecc-mode" like {auto, hw-ham, hw-bch}

Sure, I can make use of the generic bindings already offered here.

> >+	if (!strcmp("noecc", ecc_config))
> >+		pdata->bch_ecc_cfg = BCH_NO_ECC;
> >+	else if (!strcmp("18bit", ecc_config))
> >+		pdata->bch_ecc_cfg = BCH_18BIT_ECC;
> >+	else if (!strcmp("30bit", ecc_config))
> >+		pdata->bch_ecc_cfg = BCH_30BIT_ECC;
> >+	else
> >+		pdata->bch_ecc_cfg = BCH_ECC_AUTO;
> >+
> >+	ret = stm_of_get_nand_banks(&pdev->dev, np, &pdata->bank);
> >+	if (ret < 0)
> >+		return ERR_PTR(ret);
> >+
> >+	pdata->flashss = of_property_read_bool(np, "st,nand-flashss");
> >+
> >+	of_property_read_u32(np, "st,bch-bitflip-threshold",
> >+			     &pdata->bch_bitflip_threshold);
> >+
> mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
> And then can be re-configured for each MTD partition separately 
> 	/sys/class/mtd/mtdX/bitflip_threshold
> 	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
> So, I don't think this is a HW parameter, and so should not be passed from DT.

I think the bit-flip threshold is/can be chip specific, and as we know
which chip we're likely to be booting on, we can specify this via the
hardware description.  Thus, I think it's perfectly viable for an
option to pass through DT to exist.


> >+struct device_node *stm_of_get_partitions_node(struct device_node *np,
> >+					       int bank_nr)
> >+{
> >+	struct device_node *banksnp, *banknp, *partsnp = NULL;
> >+	char name[10];
> >+
> >+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
> >+	if (!banksnp)
> >+		return NULL;
> >+
> >+	sprintf(name, "bank%d", bank_nr);
> >+	banknp = of_get_child_by_name(banksnp, name);
> >+	if (banknp)
> >+		return NULL;
> >+
> >+	partsnp = of_get_child_by_name(banknp, "partitions");
> >+	of_node_put(banknp);
> >+
> Sorry, I'm bit confused here .. I think you don't need to find children of
> Your bank node. This should already taken care in default parser
> drivers/mtd/ofpart.c : parse_ofpart_partitions()
> And all you need to pass is 'of_node' of bank (device).
> Is my understanding correct ?

We have 3 options here, you _can_ use parse_ofpart_partitions() if
your partition information conforms to its schema, but said schema
does not support banks and/or other information that we choose to
place within the bank node.  The second option is to register a
parser.  My personal view is that registering a parser is using the
framework for 'using the framework's' sake i.e. doesn't actually
achieve anything special.  We've chosen the third option, which is to
parse and extract the information ourselves - which is actually fairly
trivial, and pass the required partition data in through the
mtd_device_parse_register() call - which is where the information
would be parsed in the case of the first two options.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>> >From: Lee Jones [mailto:lee.jones@linaro.org]

[...]

>> >+	of_property_read_u32(np, "st,bch-bitflip-threshold",
>> >+			     &pdata->bch_bitflip_threshold);
>> >+
>> mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
>> And then can be re-configured for each MTD partition separately
>> 	/sys/class/mtd/mtdX/bitflip_threshold
>> 	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
>> So, I don't think this is a HW parameter, and so should not be passed from DT.
>
>I think the bit-flip threshold is/can be chip specific, and as we know
>which chip we're likely to be booting on, we can specify this via the
>hardware description.  Thus, I think it's perfectly viable for an
>option to pass through DT to exist.
>
I don't think that’s the correct interpretation of bitflip_threshold.

(1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver.
MTD layers uses bitflip_threshold to warn above layers that number of
correctable bit-flips have reached a dangerous level beyond which driver's
ECC scheme may not be able to correct them. So above layers should start
taking additional corrective action like scrubbing.
	@@drivers/mtd/mtdcore.c: mtd_read()
		return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;

(2) Also, user-space may control it based on how your device ages on field.
A fresh silicon may not show too many bitflips. But as device ages the
probability of simultaneous bitflips will increase. So user-space may lower
the bitflip_threshold to avoid accumulation of bitflips in a single page.
 
Thus, bitflip_threshold should not be passed via DT.
It's neither a hardware parameter, nor it’s a static constant.

>
>> >+struct device_node *stm_of_get_partitions_node(struct device_node *np,
>> >+					       int bank_nr)
>> >+{
>> >+	struct device_node *banksnp, *banknp, *partsnp = NULL;
>> >+	char name[10];
>> >+
>> >+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
>> >+	if (!banksnp)
>> >+		return NULL;
>> >+
>> >+	sprintf(name, "bank%d", bank_nr);
>> >+	banknp = of_get_child_by_name(banksnp, name);
>> >+	if (banknp)
>> >+		return NULL;
>> >+
>> >+	partsnp = of_get_child_by_name(banknp, "partitions");
>> >+	of_node_put(banknp);
>> >+
>> Sorry, I'm bit confused here .. I think you don't need to find children of
>> Your bank node. This should already taken care in default parser
>> drivers/mtd/ofpart.c : parse_ofpart_partitions()
>> And all you need to pass is 'of_node' of bank (device).
>> Is my understanding correct ?
>
>We have 3 options here, you _can_ use parse_ofpart_partitions() if
>your partition information conforms to its schema, but said schema
>does not support banks and/or other information that we choose to
>place within the bank node.  The second option is to register a
>parser.  My personal view is that registering a parser is using the
>framework for 'using the framework's' sake i.e. doesn't actually
>achieve anything special.  We've chosen the third option, which is to
>parse and extract the information ourselves - which is actually fairly
>trivial, and pass the required partition data in through the
>mtd_device_parse_register() call - which is where the information
>would be parsed in the case of the first two options.
>
Do you really want to have *custom* partition format ?
If your previous code was non-DT (platform file based), then I think
It's good point to move to unified generic partition format which others
are following, as you make your driver DT compliant, and in mainline.

I understand you primary objective would be to get ST driver work
out of mainline asap, but if you upstream too many custom stuff you
are only adding maintenance burden for your code. This is where
most of my comments originate.
However, I leave it to Brian to decide, if he is okay with these.


with regards, pekon

Re: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Lee Jones]

> >> >+	of_property_read_u32(np, "st,bch-bitflip-threshold",
> >> >+			     &pdata->bch_bitflip_threshold);
> >> >+
> >> mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
> >> And then can be re-configured for each MTD partition separately
> >> 	/sys/class/mtd/mtdX/bitflip_threshold
> >> 	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
> >> So, I don't think this is a HW parameter, and so should not be passed from DT.
> >
> >I think the bit-flip threshold is/can be chip specific, and as we know
> >which chip we're likely to be booting on, we can specify this via the
> >hardware description.  Thus, I think it's perfectly viable for an
> >option to pass through DT to exist.
> >
> I don't think that’s the correct interpretation of bitflip_threshold.
> 
> (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver.
> MTD layers uses bitflip_threshold to warn above layers that number of
> correctable bit-flips have reached a dangerous level beyond which driver's
> ECC scheme may not be able to correct them. So above layers should start
> taking additional corrective action like scrubbing.
> 	@@drivers/mtd/mtdcore.c: mtd_read()
> 		return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
> 
> (2) Also, user-space may control it based on how your device ages on field.
> A fresh silicon may not show too many bitflips. But as device ages the
> probability of simultaneous bitflips will increase. So user-space may lower
> the bitflip_threshold to avoid accumulation of bitflips in a single page.
>  
> Thus, bitflip_threshold should not be passed via DT.
> It's neither a hardware parameter, nor it’s a static constant.

Ah, I see.  I will fixup, thanks for the explanation.

> >> >+struct device_node *stm_of_get_partitions_node(struct device_node *np,
> >> >+					       int bank_nr)
> >> >+{
> >> >+	struct device_node *banksnp, *banknp, *partsnp = NULL;
> >> >+	char name[10];
> >> >+
> >> >+	banksnp = of_parse_phandle(np, "st,nand-banks", 0);
> >> >+	if (!banksnp)
> >> >+		return NULL;
> >> >+
> >> >+	sprintf(name, "bank%d", bank_nr);
> >> >+	banknp = of_get_child_by_name(banksnp, name);
> >> >+	if (banknp)
> >> >+		return NULL;
> >> >+
> >> >+	partsnp = of_get_child_by_name(banknp, "partitions");
> >> >+	of_node_put(banknp);
> >> >+
> >> Sorry, I'm bit confused here .. I think you don't need to find children of
> >> Your bank node. This should already taken care in default parser
> >> drivers/mtd/ofpart.c : parse_ofpart_partitions()
> >> And all you need to pass is 'of_node' of bank (device).
> >> Is my understanding correct ?
> >
> >We have 3 options here, you _can_ use parse_ofpart_partitions() if
> >your partition information conforms to its schema, but said schema
> >does not support banks and/or other information that we choose to
> >place within the bank node.  The second option is to register a
> >parser.  My personal view is that registering a parser is using the
> >framework for 'using the framework's' sake i.e. doesn't actually
> >achieve anything special.  We've chosen the third option, which is to
> >parse and extract the information ourselves - which is actually fairly
> >trivial, and pass the required partition data in through the
> >mtd_device_parse_register() call - which is where the information
> >would be parsed in the case of the first two options.
> >
> Do you really want to have *custom* partition format ?
> If your previous code was non-DT (platform file based), then I think
> It's good point to move to unified generic partition format which others
> are following, as you make your driver DT compliant, and in mainline.
> 
> I understand you primary objective would be to get ST driver work
> out of mainline asap, but if you upstream too many custom stuff you
> are only adding maintenance burden for your code. This is where
> most of my comments originate.
> However, I leave it to Brian to decide, if he is okay with these.
> 
> 
> with regards, pekon

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Gupta, Pekon]

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>> >> >+	of_property_read_u32(np, "st,bch-bitflip-threshold",
>> >> >+			     &pdata->bch_bitflip_threshold);
>> >> >+
>> >> mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
>> >> And then can be re-configured for each MTD partition separately
>> >> 	/sys/class/mtd/mtdX/bitflip_threshold
>> >> 	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
>> >> So, I don't think this is a HW parameter, and so should not be passed from DT.
>> >
>> >I think the bit-flip threshold is/can be chip specific, and as we know
>> >which chip we're likely to be booting on, we can specify this via the
>> >hardware description.  Thus, I think it's perfectly viable for an
>> >option to pass through DT to exist.
>> >
>> I don't think that’s the correct interpretation of bitflip_threshold.
>>
>> (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver.
>> MTD layers uses bitflip_threshold to warn above layers that number of
>> correctable bit-flips have reached a dangerous level beyond which driver's
>> ECC scheme may not be able to correct them. So above layers should start
>> taking additional corrective action like scrubbing.
>> 	@@drivers/mtd/mtdcore.c: mtd_read()
>> 		return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
>>
>> (2) Also, user-space may control it based on how your device ages on field.
>> A fresh silicon may not show too many bitflips. But as device ages the
>> probability of simultaneous bitflips will increase. So user-space may lower
>> the bitflip_threshold to avoid accumulation of bitflips in a single page.
>>
>> Thus, bitflip_threshold should not be passed via DT.
>> It's neither a hardware parameter, nor it’s a static constant.
>
>Ah, I see.  I will fixup, thanks for the explanation.
>

Please wait, I'll review your [v2] series also, then you can further
send all fixes together. I'm bit caught in other commitments for 3.16,
so hopefully I'll be able to review your patches by next week.


with regards, pekon

Re: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Lee Jones]

> >Ah, I see.  I will fixup, thanks for the explanation.
> >
> 
> Please wait, I'll review your [v2] series also, then you can further
> send all fixes together. I'm bit caught in other commitments for 3.16,
> so hopefully I'll be able to review your patches by next week.

I have already fixed up, but I won't resubmit until you have
reviewed.

FYI: I have made some changes since v2 including moving over to the
recently submitted core timing structure submitted by Boris Brezillon
and removal bitflip_threshold as an internal variable.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [RFC 13/47] mtd: nand: stm_nand_bch: provide Device Tree support

[Lee Jones]

> >> >> >+	of_property_read_u32(np, "st,bch-bitflip-threshold",
> >> >> >+			     &pdata->bch_bitflip_threshold);
> >> >> >+
> >> >> mtd->bitflip_threshold is by default set to ecc.strength (unless a driver initializes it).
> >> >> And then can be re-configured for each MTD partition separately
> >> >> 	/sys/class/mtd/mtdX/bitflip_threshold
> >> >> 	Refer: $kernel/Documentation/ABI/testing/sysfs-class-mtd
> >> >> So, I don't think this is a HW parameter, and so should not be passed from DT.
> >> >
> >> >I think the bit-flip threshold is/can be chip specific, and as we know
> >> >which chip we're likely to be booting on, we can specify this via the
> >> >hardware description.  Thus, I think it's perfectly viable for an
> >> >option to pass through DT to exist.
> >> >
> >> I don't think that’s the correct interpretation of bitflip_threshold.
> >>
> >> (1) bitflip_threshold is dependent on ecc.strength (ECC scheme) of your driver.
> >> MTD layers uses bitflip_threshold to warn above layers that number of
> >> correctable bit-flips have reached a dangerous level beyond which driver's
> >> ECC scheme may not be able to correct them. So above layers should start
> >> taking additional corrective action like scrubbing.
> >> 	@@drivers/mtd/mtdcore.c: mtd_read()
> >> 		return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
> >>
> >> (2) Also, user-space may control it based on how your device ages on field.
> >> A fresh silicon may not show too many bitflips. But as device ages the
> >> probability of simultaneous bitflips will increase. So user-space may lower
> >> the bitflip_threshold to avoid accumulation of bitflips in a single page.
> >>
> >> Thus, bitflip_threshold should not be passed via DT.
> >> It's neither a hardware parameter, nor it’s a static constant.
> >
> >Ah, I see.  I will fixup, thanks for the explanation.
> 
> Please wait, I'll review your [v2] series also, then you can further
> send all fixes together. I'm bit caught in other commitments for 3.16,
> so hopefully I'll be able to review your patches by next week.

Did you manage to find some time to review the driver at all last week?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 14/47] mtd: nand: stm_nand_bch: configure BCH and FLEX by ONFI timing mode

[Lee Jones]

This patch adds support for configuring the NAND Controller timing
registers according to 'nand_timing_spec' data. Since the stm_nand_bch
driver falls back to Hamming FLEX mode for certain operations, it is
necessary to configure the timing registers of both the Hamming
Controller and the BCH Controller.

The device initialisation is now performed in two stages,
nand_scan_ident() and nand_scan_tail(), rather than a single call to
nand_scan(). This allows information obtained during device discovery
(e.g. ONFI parameter data) to help optimise the timing configuration
prior to performing some of the more data intensive tasks found in
nand_scan_tail() (e.g. bad block scanning).

If 'nand_timing_spec' is supplied via platform data, then this is used
in preference for configuring the timing registers. Failing that, we
test for the timing mode advertised by ONFI-compliant NAND, and select
one of the predefined timing specifications. Finally, if no timing
data is available, the timing registers are left unmodified
(i.e. reset values, or as programmed by the boot-loader).

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 311 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 311 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 3115fb4..fbcd1b9 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -172,6 +172,10 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	mtd->_resume = nand_resume;
 }
 
+/*
+ * Timing and Clocks
+ */
+
 static void nandi_clk_enable(struct nandi_controller *nandi)
 {
 	if (nandi->emi_clk)
@@ -210,6 +214,291 @@ static struct clk *nandi_clk_setup(struct nandi_controller *nandi,
 	return clk;
 }
 
+/* Derive Hamming-FLEX timing register values from 'nand_timing_spec' data */
+static void flex_calc_timing_registers(struct nand_timing_spec *spec,
+				       int tCLK, int relax,
+				       uint32_t *ctl_timing,
+				       uint32_t *wen_timing,
+				       uint32_t *ren_timing)
+{
+	int tMAX_HOLD;
+	int n_ctl_setup;
+	int n_ctl_hold;
+	int n_ctl_wb;
+
+	int tMAX_WEN_OFF;
+	int n_wen_on;
+	int n_wen_off;
+
+	int tMAX_REN_OFF;
+	int n_ren_on;
+	int n_ren_off;
+
+	/*
+	 * CTL_TIMING
+	 */
+
+	/*	- SETUP */
+	n_ctl_setup = (spec->tCLS - spec->tWP + tCLK - 1)/tCLK;
+	if (n_ctl_setup < 1)
+		n_ctl_setup = 1;
+	n_ctl_setup += relax;
+
+	/*	- HOLD */
+	tMAX_HOLD = spec->tCLH;
+	if (spec->tCH > tMAX_HOLD)
+		tMAX_HOLD = spec->tCH;
+	if (spec->tALH > tMAX_HOLD)
+		tMAX_HOLD = spec->tALH;
+	if (spec->tDH > tMAX_HOLD)
+		tMAX_HOLD = spec->tDH;
+	n_ctl_hold = (tMAX_HOLD + tCLK - 1)/tCLK + relax;
+
+	/*	- CE_deassert_hold = 0 */
+
+	/*	- WE_high_to_RBn_low */
+	n_ctl_wb = (spec->tWB + tCLK - 1)/tCLK;
+
+	*ctl_timing = ((n_ctl_setup & 0xff) |
+		       (n_ctl_hold & 0xff) << 8 |
+		       (n_ctl_wb & 0xff) << 24);
+
+	/*
+	 * WEN_TIMING
+	 */
+
+	/*	- ON */
+	n_wen_on = (spec->tWH + tCLK - 1)/tCLK + relax;
+
+	/*	- OFF */
+	tMAX_WEN_OFF = spec->tWC - spec->tWH;
+	if (spec->tWP > tMAX_WEN_OFF)
+		tMAX_WEN_OFF = spec->tWP;
+	n_wen_off = (tMAX_WEN_OFF + tCLK - 1)/tCLK + relax;
+
+	*wen_timing = ((n_wen_on & 0xff) |
+		       (n_wen_off & 0xff) << 8);
+
+	/*
+	 * REN_TIMING
+	 */
+
+	/*	- ON */
+	n_ren_on = (spec->tREH + tCLK - 1)/tCLK + relax;
+
+	/*	- OFF */
+	tMAX_REN_OFF = spec->tRC - spec->tREH;
+	if (spec->tRP > tMAX_REN_OFF)
+		tMAX_REN_OFF = spec->tRP;
+	if (spec->tREA > tMAX_REN_OFF)
+		tMAX_REN_OFF = spec->tREA;
+	n_ren_off = (tMAX_REN_OFF + tCLK - 1)/tCLK + 1 + relax;
+
+	*ren_timing = ((n_ren_on & 0xff) |
+		       (n_ren_off & 0xff) << 8);
+}
+
+/* Derive BCH timing register values from 'nand_timing_spec' data */
+static void bch_calc_timing_registers(struct nand_timing_spec *spec,
+				      int tCLK, int relax,
+				      uint32_t *ctl_timing,
+				      uint32_t *wen_timing,
+				      uint32_t *ren_timing)
+{
+	int tMAX_HOLD;
+	int n_ctl_setup;
+	int n_ctl_hold;
+	int n_ctl_wb;
+
+	int n_wen_on;
+	int n_wen_off;
+	int wen_half_on;
+	int wen_half_off;
+
+	int tMAX_REN_ON;
+	int tMAX_CS_DEASSERT;
+	int n_d_latch;
+	int n_telqv;
+	int n_ren_on;
+	int n_ren_off;
+	int ren_half_on;
+	int ren_half_off;
+
+	/*
+	 * CTL_TIMING
+	 */
+
+	/*	- SETUP */
+	if (spec->tCLS > spec->tWP)
+		n_ctl_setup = (spec->tCLS - spec->tWP + tCLK - 1)/tCLK;
+	else
+		n_ctl_setup = 0;
+	n_ctl_setup += relax;
+
+	/*	- HOLD */
+	tMAX_HOLD = spec->tCLH;
+	if (spec->tCH > tMAX_HOLD)
+		tMAX_HOLD = spec->tCH;
+	if (spec->tALH > tMAX_HOLD)
+		tMAX_HOLD = spec->tALH;
+	if (spec->tDH > tMAX_HOLD)
+		tMAX_HOLD = spec->tDH;
+	n_ctl_hold = (tMAX_HOLD + tCLK - 1)/tCLK + relax;
+	/*	- CE_deassert_hold = 0 */
+
+	/*	- WE_high_to_RBn_low */
+	n_ctl_wb = (spec->tWB + tCLK - 1)/tCLK;
+
+	*ctl_timing = ((n_ctl_setup & 0xff) |
+		       (n_ctl_hold & 0xff) << 8 |
+		       (n_ctl_wb & 0xff) << 24);
+
+	/*
+	 * WEN_TIMING
+	 */
+
+	/*	- ON */
+	n_wen_on = (2 * spec->tWH + tCLK - 1)/tCLK;
+	wen_half_on = n_wen_on % 2;
+	n_wen_on /= 2;
+	n_wen_on += relax;
+
+	/*	- OFF */
+	n_wen_off = (2 * spec->tWP + tCLK - 1)/tCLK;
+	wen_half_off = n_wen_off % 2;
+	n_wen_off /= 2;
+	n_wen_off += relax;
+
+	*wen_timing = ((n_wen_on & 0xff) |
+		       (n_wen_off & 0xff) << 8 |
+		       (wen_half_on << 16) |
+		       (wen_half_off << 17));
+
+	/*
+	 * REN_TIMING
+	 */
+
+	/*	- ON */
+	tMAX_REN_ON = spec->tRC - spec->tRP;
+	if (spec->tREH > tMAX_REN_ON)
+		tMAX_REN_ON = spec->tREH;
+
+	n_ren_on = (2 * tMAX_REN_ON + tCLK - 1)/tCLK;
+	ren_half_on = n_ren_on % 2;
+	n_ren_on /= 2;
+	n_ren_on += relax;
+
+	/*	- OFF */
+	n_ren_off = (2 * spec->tREA + tCLK - 1)/tCLK;
+	ren_half_off = n_ren_off % 2;
+	n_ren_off /= 2;
+	n_ren_off += relax;
+
+	/*	- DATA_LATCH */
+	if (spec->tREA <= (spec->tRP - (2 * tCLK)))
+		n_d_latch = 0;
+	else if (spec->tREA <= (spec->tRP - tCLK))
+		n_d_latch = 1;
+	else if ((spec->tREA <= spec->tRP) && (spec->tRHOH >= 2 * tCLK))
+		n_d_latch = 2;
+	else
+		n_d_latch = 3;
+
+	/*	- TELQV */
+	tMAX_CS_DEASSERT = spec->tCOH;
+	if (spec->tCHZ > tMAX_CS_DEASSERT)
+		tMAX_CS_DEASSERT = spec->tCHZ;
+	if (spec->tCSD > tMAX_CS_DEASSERT)
+		tMAX_CS_DEASSERT = spec->tCSD;
+
+	n_telqv = (tMAX_CS_DEASSERT + tCLK - 1)/tCLK;
+
+	*ren_timing = ((n_ren_on & 0xff) |
+		       (n_ren_off & 0xff) << 8 |
+		       (n_d_latch & 0x3) << 16 |
+		       (wen_half_on << 18) |
+		       (wen_half_off << 19) |
+		       (n_telqv & 0xff) << 24);
+}
+
+static void flex_configure_timing_registers(struct nandi_controller *nandi,
+					    struct nand_timing_spec *spec,
+					    int relax)
+{
+	uint32_t ctl_timing;
+	uint32_t wen_timing;
+	uint32_t ren_timing;
+	int emi_t_ns;
+
+	/* Select Hamming Controller */
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	/* Get EMI clock (default 100MHz) */
+	if (nandi->emi_clk)
+		emi_t_ns = 1000000000UL / clk_get_rate(nandi->emi_clk);
+	else {
+		dev_warn(nandi->dev,
+			 "No EMI clock available; assuming default 100MHz\n");
+		emi_t_ns = 10;
+	}
+
+	/* Derive timing register values from specification */
+	flex_calc_timing_registers(spec, emi_t_ns, relax,
+				   &ctl_timing, &wen_timing, &ren_timing);
+
+	dev_dbg(nandi->dev,
+		"updating FLEX timing configuration [0x%08x, 0x%08x, 0x%08x]\n",
+		ctl_timing, wen_timing, ren_timing);
+
+	/* Program timing registers */
+	writel(ctl_timing, nandi->base + NANDHAM_CTL_TIMING);
+	writel(wen_timing, nandi->base + NANDHAM_WEN_TIMING);
+	writel(ren_timing, nandi->base + NANDHAM_REN_TIMING);
+}
+
+static void bch_configure_timing_registers(struct nandi_controller *nandi,
+					   struct nand_timing_spec *spec,
+					   int relax)
+{
+	uint32_t ctl_timing;
+	uint32_t wen_timing;
+	uint32_t ren_timing;
+	int bch_t_ns;
+
+	/* Select BCH Controller */
+	emiss_nandi_select(STM_NANDI_BCH);
+
+	/* Get BCH clock (default 200MHz) */
+	if (nandi->bch_clk)
+		bch_t_ns = 1000000000UL / clk_get_rate(nandi->bch_clk);
+	else {
+		dev_warn(nandi->dev,
+			 "No BCH clock available; assuming default 200MHz\n");
+		bch_t_ns = 5;
+	}
+
+	/* Derive timing register values from specification */
+	bch_calc_timing_registers(spec, bch_t_ns, relax,
+				  &ctl_timing, &wen_timing, &ren_timing);
+
+	dev_dbg(nandi->dev,
+		"updating BCH timing configuration [0x%08x, 0x%08x, 0x%08x]\n",
+		ctl_timing, wen_timing, ren_timing);
+
+	/* Program timing registers */
+	writel(ctl_timing, nandi->base + NANDBCH_CTL_TIMING);
+	writel(wen_timing, nandi->base + NANDBCH_WEN_TIMING);
+	writel(ren_timing, nandi->base + NANDBCH_REN_TIMING);
+}
+
+static void nandi_configure_timing_registers(struct nandi_controller *nandi,
+					     struct nand_timing_spec *spec,
+					     int relax)
+{
+	bch_configure_timing_registers(nandi, spec, relax);
+	flex_configure_timing_registers(nandi, spec, relax);
+}
+
 static void nandi_init_hamming(struct nandi_controller *nandi, int emi_bank)
 {
 	dev_dbg(nandi->dev, "%s\n", __func__);
@@ -463,6 +752,28 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/*
+	 * Configure timing registers
+	 */
+	if (bank && bank->timing_spec) {
+		dev_info(&pdev->dev, "Using platform timing data\n");
+		nandi_configure_timing_registers(nandi, bank->timing_spec,
+						 bank->timing_relax);
+	} else if (chip->onfi_version) {
+		int mode = fls(onfi_get_async_timing_mode(chip) - 1);
+
+		/* Modes 4 and 5 (EDO) are not supported on our H/W */
+		if (mode > 3)
+			mode = 3;
+
+		dev_info(&pdev->dev, "Using ONFI Timing Mode %d\n", mode);
+		nandi_configure_timing_registers(nandi,
+						 &nand_onfi_timing_specs[mode],
+						 bank ? bank->timing_relax : 0);
+	} else {
+		dev_warn(&pdev->dev, "No timing data available\n");
+	}
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 15/47] mtd: nand: stm_nand_bch: add compatible page size check

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index fbcd1b9..93fe835 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -29,6 +29,9 @@
 #include "stm_nand_regs.h"
 #include "stm_nand_dt.h"
 
+/* NANDi BCH Controller properties */
+#define NANDI_BCH_SECTOR_SIZE			1024
+
 /* Bad Block Table (BBT) */
 struct nandi_bbt_info {
 	uint32_t	bbt_size;		/* Size of bad-block table */
@@ -774,6 +777,12 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "No timing data available\n");
 	}
 
+	if (mtd->writesize < NANDI_BCH_SECTOR_SIZE) {
+		dev_err(nandi->dev,
+			"page size incompatible with BCH ECC sector\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 16/47] mtd: nand: stm_nand_bch: derive some working variables for latter use

[Lee Jones]

A few of the working variables can either be taken or derived from
existing knowledge about the connected chip. Rather than attempt to
provide each specification manually, here we make assumptions based
on information already obtained.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 93fe835..f5a3e80 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -783,6 +783,15 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	/* Derive some working variables */
+	nandi->sectors_per_page = mtd->writesize / NANDI_BCH_SECTOR_SIZE;
+	nandi->blocks_per_device = mtd->size >> chip->phys_erase_shift;
+	nandi->page_shift = chip->page_shift;
+	nandi->block_shift = chip->phys_erase_shift;
+	nandi->extra_addr = ((chip->chipsize >> nandi->page_shift) >
+			     0x10000) ? true : false;
+	mtd->writebufsize = mtd->writesize;
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 17/47] mtd: nand: stm_nand_bch: automatically set EEC mode if requested

[Lee Jones]

Here we automatically select the strongest ECC scheme compatible with
the size of the OOB.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index f5a3e80..60a7800 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -32,6 +32,13 @@
 /* NANDi BCH Controller properties */
 #define NANDI_BCH_SECTOR_SIZE			1024
 
+/* BCH ECC sizes */
+static int bch_ecc_sizes[] = {
+	[BCH_18BIT_ECC] = 32,
+	[BCH_30BIT_ECC] = 54,
+	[BCH_NO_ECC] = 0,
+};
+
 /* Bad Block Table (BBT) */
 struct nandi_bbt_info {
 	uint32_t	bbt_size;		/* Size of bad-block table */
@@ -135,6 +142,25 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 	writel(val, nandi->base + NANDBCH_INT_EN);
 }
 
+/* Select strongest ECC scheme compatible with OOB size */
+static int bch_set_ecc_auto(struct nandi_controller *nandi,
+			    struct mtd_info *mtd)
+{
+	int oob_bytes_per_sector = mtd->oobsize / nandi->sectors_per_page;
+	int try_ecc_modes[] = { BCH_30BIT_ECC, BCH_18BIT_ECC, -1 };
+	int m, ecc_mode;
+
+	for (m = 0; try_ecc_modes[m] >= 0; m++) {
+		ecc_mode = try_ecc_modes[m];
+		if (oob_bytes_per_sector >= bch_ecc_sizes[ecc_mode]) {
+			nandi->bch_ecc_mode = ecc_mode;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 				   struct mtd_info *mtd, struct nand_chip *chip)
 {
@@ -792,6 +818,20 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 			     0x10000) ? true : false;
 	mtd->writebufsize = mtd->writesize;
 
+	/* Set ECC mode */
+	if (pdata->bch_ecc_cfg == BCH_ECC_AUTO) {
+		err = bch_set_ecc_auto(nandi, mtd);
+		if (err) {
+			dev_err(nandi->dev, "insufficient OOB for BCH ECC\n");
+			return err;
+		}
+	} else {
+		nandi->bch_ecc_mode = pdata->bch_ecc_cfg;
+	}
+
+	info->ecclayout.eccbytes =
+		nandi->sectors_per_page * bch_ecc_sizes[nandi->bch_ecc_mode];
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 18/47] mtd: nand: stm_nand_bch: ensure configuration is compatible with this driver

[Lee Jones]

Some chip characteristics have known incompatibilities with the function
of this driver. Here we check for this characteristics and refuse to run
if they are present.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 60a7800..365a73b 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -142,6 +142,36 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 	writel(val, nandi->base + NANDBCH_INT_EN);
 }
 
+/*
+ * Initialisation
+ */
+static int bch_check_compatibility(struct nandi_controller *nandi,
+				   struct mtd_info *mtd,
+				   struct nand_chip *chip)
+{
+	if (chip->bits_per_cell > 1)
+		dev_warn(nandi->dev, "MLC NAND not fully supported\n");
+
+	if (chip->options & NAND_BUSWIDTH_16) {
+		dev_err(nandi->dev, "x16 NAND not supported\n");
+		return false;
+	}
+
+	if (nandi->blocks_per_device / 4 > mtd->writesize) {
+		/* Need to implement multi-page BBT support... */
+		dev_err(nandi->dev, "BBT too big to fit in single page\n");
+		return false;
+	}
+
+	if (bch_ecc_sizes[nandi->bch_ecc_mode] * nandi->sectors_per_page >
+	    mtd->oobsize) {
+		dev_err(nandi->dev, "insufficient OOB for selected ECC\n");
+		return false;
+	}
+
+	return true;
+}
+
 /* Select strongest ECC scheme compatible with OOB size */
 static int bch_set_ecc_auto(struct nandi_controller *nandi,
 			    struct mtd_info *mtd)
@@ -737,7 +767,7 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	struct nandi_info *info;
 	struct nand_chip *chip;
 	struct mtd_info *mtd;
-	int err;
+	int compatible, err;
 
 	if (!pdata) {
 		if (!np) {
@@ -832,6 +862,13 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	info->ecclayout.eccbytes =
 		nandi->sectors_per_page * bch_ecc_sizes[nandi->bch_ecc_mode];
 
+	compatible = bch_check_compatibility(nandi, mtd, chip);
+	if (!compatible) {
+		dev_err(nandi->dev,
+			"NAND device incompatible with NANDi/BCH Controller\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 19/47] mtd: nand: stm_nand_bch: configure BCH read/write/erase programs

[Lee Jones]

Tune BCH programs according to device found and ECC mode.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 121 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 365a73b..351f5e2 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -97,6 +97,124 @@ struct nandi_controller {
 	struct nandi_info	info;		/* NAND device info */
 };
 
+/* BCH 'program' structure */
+struct bch_prog {
+	u32	multi_cs_addr[3];
+	u32	multi_cs_config;
+	u8	seq[16];
+	u32	addr;
+	u32	extra;
+	u8	cmd[4];
+	u32	reserved1;
+	u32	gen_cfg;
+	u32	delay;
+	u32	reserved2;
+	u32	seq_cfg;
+};
+
+/* BCH template programs (modified on-the-fly) */
+static struct bch_prog bch_prog_read_page = {
+	.cmd = {
+		NAND_CMD_READ0,
+		NAND_CMD_READSTART,
+	},
+	.seq = {
+		BCH_ECC_SCORE(0),
+		BCH_CMD_ADDR,
+		BCH_CL_CMD_1,
+		BCH_DATA_2_SECTOR,
+		BCH_STOP,
+	},
+	.gen_cfg = (GEN_CFG_DATA_8_NOT_16 |
+		    GEN_CFG_EXTRA_ADD_CYCLE |
+		    GEN_CFG_LAST_SEQ_NODE),
+	.seq_cfg = SEQ_CFG_GO_STOP,
+};
+
+static struct bch_prog bch_prog_write_page = {
+	.cmd = {
+		NAND_CMD_SEQIN,
+		NAND_CMD_PAGEPROG,
+		NAND_CMD_STATUS,
+	},
+	.seq = {
+		BCH_CMD_ADDR,
+		BCH_DATA_4_SECTOR,
+		BCH_CL_CMD_1,
+		BCH_CL_CMD_2,
+		BCH_OP_ERR,
+		BCH_STOP,
+	},
+	.gen_cfg = (GEN_CFG_DATA_8_NOT_16 |
+		    GEN_CFG_EXTRA_ADD_CYCLE |
+		    GEN_CFG_LAST_SEQ_NODE),
+	.seq_cfg = (SEQ_CFG_GO_STOP |
+		    SEQ_CFG_DATA_WRITE),
+};
+
+static struct bch_prog bch_prog_erase_block = {
+	.seq = {
+		BCH_CL_CMD_1,
+		BCH_AL_EX_0,
+		BCH_AL_EX_1,
+		BCH_AL_EX_2,
+		BCH_CL_CMD_2,
+		BCH_CL_CMD_3,
+		BCH_OP_ERR,
+		BCH_STOP,
+	},
+	.cmd = {
+		NAND_CMD_ERASE1,
+		NAND_CMD_ERASE1,
+		NAND_CMD_ERASE2,
+		NAND_CMD_STATUS,
+	},
+	.gen_cfg = (GEN_CFG_DATA_8_NOT_16 |
+		    GEN_CFG_EXTRA_ADD_CYCLE |
+		    GEN_CFG_LAST_SEQ_NODE),
+	.seq_cfg = (SEQ_CFG_GO_STOP |
+		    SEQ_CFG_ERASE),
+};
+
+/* Configure BCH read/write/erase programs */
+static void bch_configure_progs(struct nandi_controller *nandi)
+{
+	uint8_t data_opa = ffs(nandi->sectors_per_page) - 1;
+	uint8_t data_instr = BCH_INSTR(BCH_OPC_DATA, data_opa);
+	uint32_t gen_cfg_ecc = nandi->bch_ecc_mode << GEN_CFG_ECC_SHIFT;
+
+	/* Set 'DATA' instruction */
+	bch_prog_read_page.seq[3] = data_instr;
+	bch_prog_write_page.seq[1] = data_instr;
+
+	/* Set ECC mode */
+	bch_prog_read_page.gen_cfg |= gen_cfg_ecc;
+	bch_prog_write_page.gen_cfg |= gen_cfg_ecc;
+	bch_prog_erase_block.gen_cfg |= gen_cfg_ecc;
+
+	/*
+	 * Template sequences above are defined for devices that use 5 address
+	 * cycles for page Read/Write operations (and 3 for Erase operations).
+	 * Update sequences for devices that use 4 address cycles.
+	 */
+	if (!nandi->extra_addr) {
+		/* Clear 'GEN_CFG_EXTRA_ADD_CYCLE' flag */
+		bch_prog_read_page.gen_cfg &= ~GEN_CFG_EXTRA_ADD_CYCLE;
+		bch_prog_write_page.gen_cfg &= ~GEN_CFG_EXTRA_ADD_CYCLE;
+		bch_prog_erase_block.gen_cfg &= ~GEN_CFG_EXTRA_ADD_CYCLE;
+
+		/* Configure Erase sequence for 2 address cycles */
+		/* (page address) */
+		bch_prog_erase_block.seq[0] = BCH_CL_CMD_1;
+		bch_prog_erase_block.seq[1] = BCH_AL_EX_0;
+		bch_prog_erase_block.seq[2] = BCH_AL_EX_1;
+		bch_prog_erase_block.seq[3] = BCH_CL_CMD_2;
+		bch_prog_erase_block.seq[4] = BCH_CL_CMD_3;
+		bch_prog_erase_block.seq[5] = BCH_OP_ERR;
+		bch_prog_erase_block.seq[6] = BCH_STOP;
+	}
+}
+
 /*
  * NANDi Interrupts (shared by Hamming and BCH controllers)
  */
@@ -869,6 +987,9 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	/* Tune BCH programs according to device found and ECC mode */
+	bch_configure_progs(nandi);
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 20/47] mtd: nand: stm_nand_bch: initialise working buffers

[Lee Jones]

Initialise working buffers, accomodating DMA alignment constraints.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 45 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 351f5e2..fafde22 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -31,6 +31,9 @@
 
 /* NANDi BCH Controller properties */
 #define NANDI_BCH_SECTOR_SIZE			1024
+#define NANDI_BCH_DMA_ALIGNMENT			64
+#define NANDI_BCH_MAX_BUF_LIST			8
+#define NANDI_BCH_BUF_LIST_SIZE			(4 * NANDI_BCH_MAX_BUF_LIST)
 
 /* BCH ECC sizes */
 static int bch_ecc_sizes[] = {
@@ -762,6 +765,44 @@ static void nandi_init_controller(struct nandi_controller *nandi,
 	nandi_init_hamming(nandi, emi_bank);
 }
 
+/* Initialise working buffers, accomodating DMA alignment constraints */
+static int nandi_init_working_buffers(struct nandi_controller *nandi,
+				      struct nandi_bbt_info *bbt_info,
+				      struct mtd_info *mtd)
+{
+	uint32_t bbt_buf_size;
+	uint32_t buf_size;
+
+	/*	- Page and OOB */
+	buf_size = mtd->writesize + mtd->oobsize + NANDI_BCH_DMA_ALIGNMENT;
+
+	/*	- BBT data (page-size aligned) */
+	bbt_info->bbt_size = nandi->blocks_per_device >> 2; /* 2 bits/block */
+	bbt_buf_size = ALIGN(bbt_info->bbt_size, mtd->writesize);
+	buf_size += bbt_buf_size + NANDI_BCH_DMA_ALIGNMENT;
+
+	/*	- BCH BUF list */
+	buf_size += NANDI_BCH_BUF_LIST_SIZE + NANDI_BCH_DMA_ALIGNMENT;
+
+	/* Allocate bufffer */
+	nandi->buf = devm_kzalloc(nandi->dev, buf_size, GFP_KERNEL);
+	if (!nandi->buf) {
+		dev_err(nandi->dev, "failed to allocate working buffers\n");
+		return -ENOMEM;
+	}
+
+	/* Set/Align buffer pointers */
+	nandi->page_buf = PTR_ALIGN(nandi->buf, NANDI_BCH_DMA_ALIGNMENT);
+	nandi->oob_buf  = nandi->page_buf + mtd->writesize;
+	bbt_info->bbt   = PTR_ALIGN(nandi->oob_buf + mtd->oobsize,
+				    NANDI_BCH_DMA_ALIGNMENT);
+	nandi->buf_list = (uint32_t *)PTR_ALIGN(bbt_info->bbt + bbt_buf_size,
+						NANDI_BCH_DMA_ALIGNMENT);
+	nandi->cached_page = -1;
+
+	return 0;
+}
+
 static int remap_named_resource(struct platform_device *pdev,
 				char *name,
 				void __iomem **io_ptr)
@@ -990,6 +1031,10 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	/* Tune BCH programs according to device found and ECC mode */
 	bch_configure_progs(nandi);
 
+	err = nandi_init_working_buffers(nandi, bbt_info, mtd);
+	if (err)
+		return err;
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 21/47] mtd: nand: stm_nand_bch: provide shared BCH operations

[Lee Jones]

Including one for programming the BCH sequence, taking care not to
program two registers marked 'reserved' in the NANDi specification.
Failing to take this precaution results in an imprecise data exception
on ARM platforms. We also add an operation which waits on a completion
timeout (released by the IRQ handler) and finally another which
utilises the two aforementioned operations in order to erase a single
block.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 55 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index fafde22..2042cdd 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -264,6 +264,61 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi,
 }
 
 /*
+ * BCH Operations
+ */
+static inline void bch_load_prog_cpu(struct nandi_controller *nandi,
+				     struct bch_prog *prog)
+{
+	uint32_t *src = (uint32_t *)prog;
+	uint32_t *dst = (uint32_t *)(nandi->base + NANDBCH_ADDRESS_REG_1);
+	int i;
+
+	for (i = 0; i < 16; i++) {
+		/* Skip registers marked as "reserved" */
+		if (i != 11 && i != 14)
+			writel(*src, dst);
+		dst++;
+		src++;
+	}
+}
+
+static void bch_wait_seq(struct nandi_controller *nandi)
+{
+	int ret;
+
+	ret = wait_for_completion_timeout(&nandi->seq_completed, HZ/2);
+	if (!ret)
+		dev_err(nandi->dev, "BCH Seq timeout\n");
+}
+
+static uint8_t bch_erase_block(struct nandi_controller *nandi,
+			       loff_t offs)
+{
+	struct bch_prog *prog = &bch_prog_erase_block;
+	uint8_t status;
+
+	dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
+
+	prog->extra = (uint32_t)(offs >> nandi->page_shift);
+
+	emiss_nandi_select(STM_NANDI_BCH);
+
+	nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+	reinit_completion(&nandi->seq_completed);
+
+	bch_load_prog_cpu(nandi, prog);
+
+	bch_wait_seq(nandi);
+
+	nandi_disable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+
+	status = (uint8_t)(readl(nandi->base +
+				 NANDBCH_CHECK_STATUS_REG_A) & 0xff);
+
+	return status;
+}
+
+/*
  * Initialisation
  */
 static int bch_check_compatibility(struct nandi_controller *nandi,
-- 
1.8.3.2

[RFC 22/47] mtd: nand: stm_nand_bch: check erased page for zeros

[Lee Jones]

Detect an erased page, tolerating and correcting up to a specified number
of bits at '0'. Downgrade uncorrectable ECC error for an erased page,
tolerating 'sectors_per_page' bits at '0'.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 2042cdd..7874d85 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -319,6 +319,33 @@ static uint8_t bch_erase_block(struct nandi_controller *nandi,
 }
 
 /*
+ * Detect an erased page, tolerating and correcting up to a specified number of
+ * bits at '0'.  (For many devices, it is now deemed within spec for an erased
+ * page to include a number of bits at '0', either as a result of read-disturb
+ * behaviour or 'stuck-at-zero' failures.)  Returns the number of corrected
+ * bits, or a '-1' if we have exceeded the maximum number of bits at '0' (likely
+ * to be a genuine uncorrectable ECC error).  In the latter case, the data must
+ * be returned unmodified, in accordance with the MTD API.
+ */
+static int check_erased_page(uint8_t *data, uint32_t page_size, int max_zeros)
+{
+	uint8_t *b = data;
+	int zeros = 0;
+	int i;
+
+	for (i = 0; i < page_size; i++) {
+		zeros += hweight8(~*b++);
+		if (zeros > max_zeros)
+			return -1;
+	}
+
+	if (zeros)
+		memset(data, 0xff, page_size);
+
+	return zeros;
+}
+
+/*
  * Initialisation
  */
 static int bch_check_compatibility(struct nandi_controller *nandi,
-- 
1.8.3.2

[RFC 23/47] mtd: nand: stm_nand_bch: read and write page (BCH)

[Lee Jones]

Use DMA to read and/or write a single page of data.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 119 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 7874d85..6323590 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 #include <linux/completion.h>
 #include <linux/mtd/nand.h>
 #include <linux/mtd/stm_nand.h>
@@ -345,6 +346,124 @@ static int check_erased_page(uint8_t *data, uint32_t page_size, int max_zeros)
 	return zeros;
 }
 
+/* Returns the number of ECC errors, or '-1' for uncorrectable error */
+static int bch_read_page(struct nandi_controller *nandi,
+			 loff_t offs,
+			 uint8_t *buf)
+{
+	struct bch_prog *prog = &bch_prog_read_page;
+	uint32_t page_size = nandi->info.mtd.writesize;
+	unsigned long list_phys;
+	unsigned long buf_phys;
+	uint32_t ecc_err;
+	int ret = 0;
+
+	dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
+
+	BUG_ON((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1));
+	BUG_ON(offs & (NANDI_BCH_DMA_ALIGNMENT - 1));
+
+	emiss_nandi_select(STM_NANDI_BCH);
+
+	nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+	reinit_completion(&nandi->seq_completed);
+
+	/* Reset ECC stats */
+	writel(CFG_RESET_ECC_ALL | CFG_ENABLE_AFM,
+	       nandi->base + NANDBCH_CONTROLLER_CFG);
+	writel(CFG_ENABLE_AFM, nandi->base + NANDBCH_CONTROLLER_CFG);
+
+	prog->addr = (uint32_t)((offs >> (nandi->page_shift - 8)) & 0xffffff00);
+
+	buf_phys = dma_map_single(NULL, buf, page_size, DMA_FROM_DEVICE);
+
+	memset(nandi->buf_list, 0x00, NANDI_BCH_BUF_LIST_SIZE);
+	nandi->buf_list[0] = buf_phys | (nandi->sectors_per_page - 1);
+
+	list_phys = dma_map_single(NULL, nandi->buf_list,
+				   NANDI_BCH_BUF_LIST_SIZE, DMA_TO_DEVICE);
+
+	writel(list_phys, nandi->base + NANDBCH_BUFFER_LIST_PTR);
+
+	bch_load_prog_cpu(nandi, prog);
+
+	bch_wait_seq(nandi);
+
+	nandi_disable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+
+	dma_unmap_single(NULL, list_phys, NANDI_BCH_BUF_LIST_SIZE,
+			 DMA_TO_DEVICE);
+	dma_unmap_single(NULL, buf_phys, page_size, DMA_FROM_DEVICE);
+
+	/* Use the maximum per-sector ECC count! */
+	ecc_err = readl(nandi->base + NANDBCH_ECC_SCORE_REG_A) & 0xff;
+	if (ecc_err == 0xff) {
+		/*
+		 * Downgrade uncorrectable ECC error for an erased page,
+		 * tolerating 'sectors_per_page' bits at zero.
+		 */
+		ret = check_erased_page(buf, page_size,
+					nandi->sectors_per_page);
+		if (ret >= 0)
+			dev_dbg(nandi->dev,
+				"%s: erased page detected: \n"
+				"  downgrading uncorrectable ECC error.\n",
+				__func__);
+	} else {
+		ret = (int)ecc_err;
+	}
+
+	return ret;
+}
+
+/* Returns the status of the NAND device following the write operation */
+static uint8_t bch_write_page(struct nandi_controller *nandi,
+			      loff_t offs, const uint8_t *buf)
+{
+	struct bch_prog *prog = &bch_prog_write_page;
+	uint32_t page_size = nandi->info.mtd.writesize;
+	uint8_t *p = (uint8_t *)buf;
+	unsigned long list_phys;
+	unsigned long buf_phys;
+	uint8_t status;
+
+	dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
+
+	BUG_ON((unsigned int)buf & (NANDI_BCH_DMA_ALIGNMENT - 1));
+	BUG_ON(offs & (page_size - 1));
+
+	emiss_nandi_select(STM_NANDI_BCH);
+
+	nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+	reinit_completion(&nandi->seq_completed);
+
+	prog->addr = (uint32_t)((offs >> (nandi->page_shift - 8)) & 0xffffff00);
+
+	buf_phys = dma_map_single(NULL, p, page_size, DMA_TO_DEVICE);
+	memset(nandi->buf_list, 0x00, NANDI_BCH_BUF_LIST_SIZE);
+	nandi->buf_list[0] = buf_phys | (nandi->sectors_per_page - 1);
+
+	list_phys = dma_map_single(NULL, nandi->buf_list,
+				   NANDI_BCH_BUF_LIST_SIZE, DMA_TO_DEVICE);
+
+	writel(list_phys, nandi->base + NANDBCH_BUFFER_LIST_PTR);
+
+	bch_load_prog_cpu(nandi, prog);
+
+	bch_wait_seq(nandi);
+
+	nandi_disable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
+
+	dma_unmap_single(NULL, list_phys, NANDI_BCH_BUF_LIST_SIZE,
+			 DMA_TO_DEVICE);
+	dma_unmap_single(NULL, buf_phys, page_size, DMA_FROM_DEVICE);
+
+	status = (uint8_t)(readl(nandi->base +
+				 NANDBCH_CHECK_STATUS_REG_A) & 0xff);
+
+	return status;
+}
+
 /*
  * Initialisation
  */
-- 
1.8.3.2

RE: [RFC 23/47] mtd: nand: stm_nand_bch: read and write page (BCH)

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>Use DMA to read and/or write a single page of data.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/mtd/nand/stm_nand_bch.c | 119 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index 7874d85..6323590 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -21,6 +21,7 @@
> #include <linux/interrupt.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
>+#include <linux/dma-mapping.h>
> #include <linux/completion.h>
> #include <linux/mtd/nand.h>
> #include <linux/mtd/stm_nand.h>
>@@ -345,6 +346,124 @@ static int check_erased_page(uint8_t *data, uint32_t page_size, int max_zeros)
> 	return zeros;
> }
>
>+/* Returns the number of ECC errors, or '-1' for uncorrectable error */
>+static int bch_read_page(struct nandi_controller *nandi,
>+			 loff_t offs,
>+			 uint8_t *buf)
>+{
>+	struct bch_prog *prog = &bch_prog_read_page;
>+	uint32_t page_size = nandi->info.mtd.writesize;
>+	unsigned long list_phys;
>+	unsigned long buf_phys;
>+	uint32_t ecc_err;
>+	int ret = 0;
>+
>+	dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
>+
>+	BUG_ON((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1));
>+	BUG_ON(offs & (NANDI_BCH_DMA_ALIGNMENT - 1));
>+
>+	emiss_nandi_select(STM_NANDI_BCH);
>+
>+	nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
>+	reinit_completion(&nandi->seq_completed);
>+
>+	/* Reset ECC stats */
>+	writel(CFG_RESET_ECC_ALL | CFG_ENABLE_AFM,
>+	       nandi->base + NANDBCH_CONTROLLER_CFG);
>+	writel(CFG_ENABLE_AFM, nandi->base + NANDBCH_CONTROLLER_CFG);
>+
>+	prog->addr = (uint32_t)((offs >> (nandi->page_shift - 8)) & 0xffffff00);
>+
>+	buf_phys = dma_map_single(NULL, buf, page_size, DMA_FROM_DEVICE);
>+
>+	memset(nandi->buf_list, 0x00, NANDI_BCH_BUF_LIST_SIZE);
>+	nandi->buf_list[0] = buf_phys | (nandi->sectors_per_page - 1);
>+
>+	list_phys = dma_map_single(NULL, nandi->buf_list,
>+				   NANDI_BCH_BUF_LIST_SIZE, DMA_TO_DEVICE);
>+
>+	writel(list_phys, nandi->base + NANDBCH_BUFFER_LIST_PTR);
>+
>+	bch_load_prog_cpu(nandi, prog);
>+
>+	bch_wait_seq(nandi);
>+
>+	nandi_disable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
>+
>+	dma_unmap_single(NULL, list_phys, NANDI_BCH_BUF_LIST_SIZE,
>+			 DMA_TO_DEVICE);
>+	dma_unmap_single(NULL, buf_phys, page_size, DMA_FROM_DEVICE);
>+
>+	/* Use the maximum per-sector ECC count! */

Firstly, this ecc checking and correction should not be part of bch_read_page().
This should be part of chip->ecc.correct().
But, I don't see your driver using nand_chip->ecc interfaces.
Why do you want to break away from generic driver flow ? any controller limitation ?

I think much of the code in below patch can be reused from nand_base.c
	[RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)


>+	ecc_err = readl(nandi->base + NANDBCH_ECC_SCORE_REG_A) & 0xff;
>+	if (ecc_err == 0xff) {
>+		/*
>+		 * Downgrade uncorrectable ECC error for an erased page,
>+		 * tolerating 'sectors_per_page' bits at zero.
>+		 */
>+		ret = check_erased_page(buf, page_size,
>+					nandi->sectors_per_page);

This is also not correct. Here 'max_zeros' should be ecc.strength


>+		if (ret >= 0)
>+			dev_dbg(nandi->dev,
>+				"%s: erased page detected: \n"
>+				"  downgrading uncorrectable ECC error.\n",
>+				__func__);
>+	} else {
>+		ret = (int)ecc_err;
>+	}
>+
>+	return ret;
>+}
>+

with regards, pekon

Re: [RFC 23/47] mtd: nand: stm_nand_bch: read and write page (BCH)

[Lee Jones]

> >From: Lee Jones [mailto:lee.jones@linaro.org]
> >
> >Use DMA to read and/or write a single page of data.
> >
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> > drivers/mtd/nand/stm_nand_bch.c | 119 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 119 insertions(+)
> >
> >diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
> >index 7874d85..6323590 100644
> >--- a/drivers/mtd/nand/stm_nand_bch.c
> >+++ b/drivers/mtd/nand/stm_nand_bch.c
> >@@ -21,6 +21,7 @@

[...]

> >+/* Returns the number of ECC errors, or '-1' for uncorrectable error */
> >+static int bch_read_page(struct nandi_controller *nandi,
> >+			 loff_t offs,
> >+			 uint8_t *buf)
> >+{

[...]

> >+	/* Use the maximum per-sector ECC count! */
> 
> Firstly, this ecc checking and correction should not be part of bch_read_page().
> This should be part of chip->ecc.correct().

I'm not sure that's true.  The functionality below isn't correcting
any data.  I believe it's preventing a false-positive error return.
All calculate() and correct() behaviour is dealt with internally by
h/w.  There should be no intervention from s/w, hence the lack of
call-back functions of the same name.

> But, I don't see your driver using nand_chip->ecc interfaces.
> Why do you want to break away from generic driver flow ? any controller limitation ?

When this driver was written, a great deal of the framework was not
present.  I believe the author also wanted to prevent any changes in
the framework from adversely affecting the leaf driver.  It was
considered that if there is no intention to upstream, it would be much
better to hack around the framework and have full control of the
driver.

Things are different now, as upstreaming is considered necessary.
It's my job to make this driver acceptable so it can be Mainlined.
This includes a 'port' to ensure the driver is using the correct
interfaces provided by the framework.

> I think much of the code in below patch can be reused from nand_base.c
> 	[RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)

I agree and have taken care of this now.

> >+	ecc_err = readl(nandi->base + NANDBCH_ECC_SCORE_REG_A) & 0xff;
> >+	if (ecc_err == 0xff) {
> >+		/*
> >+		 * Downgrade uncorrectable ECC error for an erased page,
> >+		 * tolerating 'sectors_per_page' bits at zero.
> >+		 */
> >+		ret = check_erased_page(buf, page_size,
> >+					nandi->sectors_per_page);
> 
> This is also not correct. Here 'max_zeros' should be ecc.strength

I need to check this with the original author, but I'm inclined to
agree with you.  I'll make the changes before re-submitting.

> >+		if (ret >= 0)
> >+			dev_dbg(nandi->dev,
> >+				"%s: erased page detected: \n"
> >+				"  downgrading uncorrectable ECC error.\n",
> >+				__func__);
> >+	} else {
> >+		ret = (int)ecc_err;
> >+	}
> >+
> >+	return ret;
> >+}
> >+
> 
> with regards, pekon

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 24/47] mtd: nand: stm_nand_bch: find IBBT signature

[Lee Jones]

Scan block for IBBT signature, either "Bbt0" or "1tbB" from
pre-specified data block.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 91 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 6323590..5bcaabbc 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -43,6 +43,37 @@ static int bch_ecc_sizes[] = {
 	[BCH_NO_ECC] = 0,
 };
 
+/*
+ * Inband Bad Block Table (IBBT)
+ */
+#define NAND_IBBT_NBLOCKS	4
+#define NAND_IBBT_SIGLEN	4
+#define NAND_IBBT_PRIMARY	0
+#define NAND_IBBT_MIRROR	1
+#define NAND_IBBT_SCHEMA	0x10
+#define NAND_IBBT_BCH_SCHEMA	0x10
+
+static uint8_t ibbt_sigs[2][NAND_IBBT_SIGLEN] = {
+	{'B', 'b', 't', '0'},
+	{'1', 't', 'b', 'B'},
+};
+
+/* IBBT header */
+struct nand_ibbt_header {
+	uint8_t	signature[4];		/* "Bbt0" or "1tbB" signature */
+	uint8_t version;		/* BBT version ("age") */
+	uint8_t reserved[3];		/* padding */
+	uint8_t schema[4];		/* "base" schema (x4) */
+} __packed;
+
+/* Extend IBBT header with some stm-nand-bch niceties */
+struct nand_ibbt_bch_header {
+	struct nand_ibbt_header base;
+	uint8_t schema[4];		/* "private" schema (x4) */
+	uint8_t ecc_size[4];		/* ECC bytes (0, 32, 54) (x4) */
+	char	author[64];		/* Arbitrary string for S/W to use */
+} __packed;
+
 /* Bad Block Table (BBT) */
 struct nandi_bbt_info {
 	uint32_t	bbt_size;		/* Size of bad-block table */
@@ -464,6 +495,66 @@ static uint8_t bch_write_page(struct nandi_controller *nandi,
 	return status;
 }
 
+/* Scan block for IBBT signature */
+static int bch_find_ibbt_sig(struct nandi_controller *nandi,
+			     uint32_t block, int *bak, uint8_t *vers,
+			     char *author)
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	struct nand_ibbt_bch_header *ibbt_header;
+	loff_t offs;
+	uint8_t *buf = nandi->page_buf;
+	int match_sig;
+	unsigned int b;
+	unsigned int i;
+
+	nandi->cached_page = -1;
+
+	/* Load last page of block */
+	offs = (loff_t)block << nandi->block_shift;
+	offs += mtd->erasesize - mtd->writesize;
+	if (bch_read_page(nandi, offs, buf) < 0) {
+		dev_info(nandi->dev,
+			 "Uncorrectable ECC error while scanning BBT signature at block %u [0x%012llx]\n",
+			 block, offs);
+		return 0;
+	}
+	ibbt_header = (struct nand_ibbt_bch_header *)buf;
+
+	/* Test IBBT signature */
+	match_sig = 0;
+	for (b = 0; b < 2 && !match_sig; b++) {
+		match_sig = 1;
+		for (i = 0; i < NAND_IBBT_SIGLEN; i++) {
+			if (ibbt_header->base.signature[i] != ibbt_sigs[b][i]) {
+				match_sig = 0;
+				break;
+			}
+		}
+
+	}
+
+	if (!match_sig)
+		return 0; /* Failed to match IBBT signature */
+
+	/* Test IBBT schema */
+	for (i = 0; i < 4; i++)
+		if (ibbt_header->base.schema[i] != NAND_IBBT_SCHEMA)
+			return 0;
+
+	/* Test IBBT BCH schema */
+	for (i = 0; i < 4; i++)
+		if (ibbt_header->schema[i] != NAND_IBBT_BCH_SCHEMA)
+			return 0;
+
+	/* We have a match */
+	*vers = ibbt_header->base.version;
+	*bak = b - 1;
+	strncpy(author, ibbt_header->author, 64);
+
+	return 1;
+}
+
 /*
  * Initialisation
  */
-- 
1.8.3.2

[RFC 25/47] mtd: nand: stm_nand_bch: bad block marking helpers

[Lee Jones]

Bad Block Markers (BBMs) keep track of unusable/unsuitable memory
blocks which can prevent unnecessary data loss. These helpers aid in
reading and writing to/from the Bad Block Tables (BBTs) where the BBMs
are stored.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 5bcaabbc..e1c71be 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -495,6 +495,35 @@ static uint8_t bch_write_page(struct nandi_controller *nandi,
 	return status;
 }
 
+/*
+ * Bad Block Tables/Bad Block Markers
+ */
+#define BBT_MARK_BAD_FACTORY	0x0
+#define BBT_MARK_BAD_WEAR	0x1
+#define BBT_MARK_GOOD		0x3
+
+static void bbt_set_block_mark(uint8_t *bbt, uint32_t block, uint8_t mark)
+{
+	unsigned int byte = block >> 2;
+	unsigned int shift = (block & 0x3) << 1;
+
+	bbt[byte] &= ~(0x3 << shift);
+	bbt[byte] |= ((mark & 0x3) << shift);
+}
+
+static uint8_t bbt_get_block_mark(uint8_t *bbt, uint32_t block)
+{
+	unsigned int byte = block >> 2;
+	unsigned int shift = (block & 0x3) << 1;
+
+	return (bbt[byte] >> shift) & 0x3;
+}
+
+static int bbt_is_block_bad(uint8_t *bbt, uint32_t block)
+{
+	return bbt_get_block_mark(bbt, block) == BBT_MARK_GOOD ? 0 : 1;
+}
+
 /* Scan block for IBBT signature */
 static int bch_find_ibbt_sig(struct nandi_controller *nandi,
 			     uint32_t block, int *bak, uint8_t *vers,
-- 
1.8.3.2

[RFC 26/47] mtd: nand: stm_nand_bch: populate IBBT BCH Header

[Lee Jones]

The STM NAND BCH Controller driver has extended the IBBT header with
some extra convenience properties. This function populates both those
and the standard header specified by 'struct nand_ibbt_header'.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index e1c71be..2d03113 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -26,6 +26,7 @@
 #include <linux/mtd/nand.h>
 #include <linux/mtd/stm_nand.h>
 #include <linux/mtd/partitions.h>
+#include <generated/utsrelease.h>
 
 #include "stm_nand_regs.h"
 #include "stm_nand_dt.h"
@@ -524,6 +525,22 @@ static int bbt_is_block_bad(uint8_t *bbt, uint32_t block)
 	return bbt_get_block_mark(bbt, block) == BBT_MARK_GOOD ? 0 : 1;
 }
 
+/* Populate IBBT BCH Header */
+static void bch_fill_ibbt_header(struct nandi_controller *nandi,
+				 struct nand_ibbt_bch_header *ibbt_header,
+				 int bak, uint8_t vers)
+{
+	const char author[] = "STLinux " UTS_RELEASE " (stm-nand-bch)";
+
+	memcpy(ibbt_header->base.signature, ibbt_sigs[bak], NAND_IBBT_SIGLEN);
+	ibbt_header->base.version = vers;
+	memset(ibbt_header->base.schema, NAND_IBBT_SCHEMA, 4);
+
+	memset(ibbt_header->schema, NAND_IBBT_SCHEMA, 4);
+	memset(ibbt_header->ecc_size, bch_ecc_sizes[nandi->bch_ecc_mode], 4);
+	memcpy(ibbt_header->author, author, sizeof(author));
+}
+
 /* Scan block for IBBT signature */
 static int bch_find_ibbt_sig(struct nandi_controller *nandi,
 			     uint32_t block, int *bak, uint8_t *vers,
-- 
1.8.3.2

[RFC 27/47] mtd: nand: stm_nand_bch: write IBBT to Flash

[Lee Jones]

Write BBT contents to the first page of a specified block, then
update the IBBT header and write it to last page of the same block.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 2d03113..76ed99b 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -541,6 +541,35 @@ static void bch_fill_ibbt_header(struct nandi_controller *nandi,
 	memcpy(ibbt_header->author, author, sizeof(author));
 }
 
+/* Write IBBT to Flash */
+static int bch_write_bbt_data(struct nandi_controller *nandi,
+			      struct nandi_bbt_info *bbt_info,
+			      uint32_t block, int bak, uint8_t vers)
+{
+	uint32_t page_size = nandi->info.mtd.writesize;
+	uint32_t block_size = nandi->info.mtd.erasesize;
+	struct nand_ibbt_bch_header *ibbt_header =
+		(struct nand_ibbt_bch_header *)nandi->page_buf;
+	loff_t offs;
+
+	nandi->cached_page = -1;
+
+	/* Write BBT contents to first page of block */
+	offs = (loff_t)block << nandi->block_shift;
+	if (bch_write_page(nandi, offs, bbt_info->bbt) & NAND_STATUS_FAIL)
+		return 1;
+
+	/* Update IBBT header and write to last page of block */
+	memset(ibbt_header, 0xff, nandi->info.mtd.writesize);
+	bch_fill_ibbt_header(nandi, ibbt_header, bak, vers);
+	offs += block_size - page_size;
+	if (bch_write_page(nandi, offs, (uint8_t *)ibbt_header) &
+	    NAND_STATUS_FAIL)
+		return 1;
+
+	return 0;
+}
+
 /* Scan block for IBBT signature */
 static int bch_find_ibbt_sig(struct nandi_controller *nandi,
 			     uint32_t block, int *bak, uint8_t *vers,
-- 
1.8.3.2

[RFC 28/47] mtd: nand: stm_nand_bch: update flash-resident BBT(s)

[Lee Jones]

Search for suitable block, erase and write table data to flash. If any
errors occur on selected block, mark it as bad and select another one.
Keep trying until both the Primary and Mirror BBTs are a) synced and
b) written successfully to flash.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 143 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 76ed99b..a4f3e46 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -59,6 +59,11 @@ static uint8_t ibbt_sigs[2][NAND_IBBT_SIGLEN] = {
 	{'1', 't', 'b', 'B'},
 };
 
+static char *bbt_strs[] = {
+	"primary",
+	"mirror",
+};
+
 /* IBBT header */
 struct nand_ibbt_header {
 	uint8_t	signature[4];		/* "Bbt0" or "1tbB" signature */
@@ -570,6 +575,144 @@ static int bch_write_bbt_data(struct nandi_controller *nandi,
 	return 0;
 }
 
+/*
+ * Update Flash-resident BBT:
+ *   erase/search suitable block, and write table data to Flash
+ */
+static int bch_update_bbt(struct nandi_controller *nandi,
+			 struct nandi_bbt_info *bbt_info,
+			 int bak, uint8_t vers)
+{
+	loff_t offs;
+	uint32_t block;
+	uint32_t block_lower;
+	uint32_t block_other;
+
+	block_other = bbt_info->bbt_block[(bak+1)%2];
+	block_lower = nandi->blocks_per_device - NAND_IBBT_NBLOCKS;
+
+	for (block = bbt_info->bbt_block[bak]; block >= block_lower;  block--) {
+		offs = (loff_t)block << nandi->block_shift;
+
+		/* Skip if block used by other table */
+		if (block == block_other)
+			continue;
+
+		/* Skip if block is marked bad */
+		if (bbt_is_block_bad(bbt_info->bbt, block))
+			continue;
+
+		/* Erase block, mark bad and skip on failure */
+		if (bch_erase_block(nandi, offs) & NAND_STATUS_FAIL) {
+			dev_info(nandi->dev,
+				 "failed to erase block [%u:0x%012llx] while updating BBT\n",
+				 block, offs);
+			vers++;
+			bbt_set_block_mark(bbt_info->bbt, block,
+					   BBT_MARK_BAD_WEAR);
+			continue;
+		}
+
+		/* Write BBT, mark bad and skip on failure */
+		if (bch_write_bbt_data(nandi, bbt_info, block, bak, vers)) {
+			dev_info(nandi->dev,
+				 "failed to write BBT to block [%u:0x%012llx]\n",
+				 block, offs);
+			vers++;
+			bbt_set_block_mark(bbt_info->bbt, block,
+					   BBT_MARK_BAD_WEAR);
+			continue;
+		}
+
+		/* Success */
+		bbt_info->bbt_block[bak] = block;
+		bbt_info->bbt_vers[bak] = vers;
+		break;
+	}
+
+	/* No space in BBT area */
+	if (block < block_lower) {
+		dev_err(nandi->dev, "no space left in BBT area\n");
+		dev_err(nandi->dev, "failed to update %s BBT\n", bbt_strs[bak]);
+		return -ENOSPC;
+	}
+
+	dev_info(nandi->dev, "wrote BBT [%s:%u] at 0x%012llx [%u]\n",
+		 bbt_strs[bak], vers, offs, block);
+
+	return 0;
+}
+
+#define NAND_IBBT_UPDATE_PRIMARY	0x1
+#define NAND_IBBT_UPDATE_MIRROR		0x2
+#define NAND_IBBT_UPDATE_BOTH		(NAND_IBBT_UPDATE_PRIMARY | \
+					 NAND_IBBT_UPDATE_MIRROR)
+static char *bbt_update_strs[] = {
+	"",
+	"primary",
+	"mirror",
+	"both",
+};
+
+/*
+ * Update Flash-resident BBT(s):
+ *   incrementing 'vers' number if required, and ensuring Primary
+ *   and Mirror are kept in sync
+ */
+static int bch_update_bbts(struct nandi_controller *nandi,
+			   struct nandi_bbt_info *bbt_info,
+			   unsigned int update, uint8_t vers)
+{
+	int err;
+
+	dev_info(nandi->dev, "updating %s BBT(s)\n", bbt_update_strs[update]);
+
+	do {
+		/* Update Primary if specified */
+		if (update & NAND_IBBT_UPDATE_PRIMARY) {
+			err = bch_update_bbt(nandi, bbt_info, NAND_IBBT_PRIMARY,
+					     vers);
+			/* Bail out on error (e.g. no space left in BBT area) */
+			if (err)
+				return err;
+
+			/*
+			 * If update resulted in a new BBT version
+			 * (e.g. Erase/Write fail on BBT block) update version
+			 * here, and force update of other table.
+			 */
+			if (bbt_info->bbt_vers[NAND_IBBT_PRIMARY] != vers) {
+				vers = bbt_info->bbt_vers[NAND_IBBT_PRIMARY];
+				update = NAND_IBBT_UPDATE_MIRROR;
+			}
+		}
+
+		/* Update Mirror if specified */
+		if (update & NAND_IBBT_UPDATE_MIRROR) {
+			err = bch_update_bbt(nandi, bbt_info, NAND_IBBT_MIRROR,
+					     vers);
+			/* Bail out on error (e.g. no space left in BBT area) */
+			if (err)
+				return err;
+
+			/*
+			 * If update resulted in a new BBT version
+			 * (e.g. Erase/Write fail on BBT block) update version
+			 * here, and force update of other table.
+			 */
+			if (bbt_info->bbt_vers[NAND_IBBT_MIRROR] != vers) {
+				vers = bbt_info->bbt_vers[NAND_IBBT_MIRROR];
+				update = NAND_IBBT_UPDATE_PRIMARY;
+			}
+		}
+
+		/* Continue, until Primary and Mirror versions are in sync */
+	} while (bbt_info->bbt_vers[NAND_IBBT_PRIMARY] !=
+		 bbt_info->bbt_vers[NAND_IBBT_MIRROR]);
+
+	return 0;
+}
+
 /* Scan block for IBBT signature */
 static int bch_find_ibbt_sig(struct nandi_controller *nandi,
 			     uint32_t block, int *bak, uint8_t *vers,
-- 
1.8.3.2

[RFC 29/47] mtd: nand: stm_nand_bch: add Hamming-FLEX operations

[Lee Jones]

Including; a check for Ready/NotBusy (RBn) (i.e. wait for current
operation to finish and a method to issue Flex commands to the
controller.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index a4f3e46..baecc40 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -502,6 +502,42 @@ static uint8_t bch_write_page(struct nandi_controller *nandi,
 }
 
 /*
+ * Hamming-FLEX operations
+ */
+static int flex_wait_rbn(struct nandi_controller *nandi)
+{
+	int ret;
+
+	ret = wait_for_completion_timeout(&nandi->rbn_completed, HZ/2);
+	if (!ret)
+		dev_err(nandi->dev, "FLEX RBn timeout\n");
+
+	return ret;
+}
+
+static void flex_cmd(struct nandi_controller *nandi, uint8_t cmd)
+{
+	uint32_t val;
+
+	val = (FLEX_CMD_CSN | FLEX_CMD_BEATS_1 | cmd);
+	writel(val, nandi->base + NANDHAM_FLEX_CMD);
+}
+
+static void flex_addr(struct nandi_controller *nandi,
+		      uint32_t addr, int cycles)
+{
+	addr &= 0x00ffffff;
+
+	BUG_ON(cycles < 1);
+	BUG_ON(cycles > 3);
+
+	addr |= (FLEX_ADDR_CSN | FLEX_ADDR_ADD8_VALID);
+	addr |= (cycles & 0x3) << 28;
+
+	writel(addr, nandi->base + NANDHAM_FLEX_ADD);
+}
+
+/*
  * Bad Block Tables/Bad Block Markers
  */
 #define BBT_MARK_BAD_FACTORY	0x0
-- 
1.8.3.2

[RFC 30/47] mtd: nand: stm_nand_bch: read and write raw (FLEX)

[Lee Jones]

Once the correct READ/WRITE(SEQIN) commands and address locations have
been sent to the Controller, these calls are able to read/write the
requested information from the FLEX_DATA register.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 77 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index baecc40..4812431 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -537,6 +537,83 @@ static void flex_addr(struct nandi_controller *nandi,
 	writel(addr, nandi->base + NANDHAM_FLEX_ADD);
 }
 
+static int flex_read_raw(struct nandi_controller *nandi,
+			 uint32_t page_addr,
+			 uint32_t col_addr,
+			 uint8_t *buf, uint32_t len)
+{
+	dev_dbg(nandi->dev, "%s %u bytes at [0x%06x,0x%04x]\n",
+		__func__, len, page_addr, col_addr);
+
+	BUG_ON(len & 0x3);
+	BUG_ON((unsigned long)buf & 0x3);
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+	nandi_enable_interrupts(nandi, NAND_INT_RBN);
+	reinit_completion(&nandi->rbn_completed);
+
+	writel(FLEX_DATA_CFG_BEATS_4 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+
+	flex_cmd(nandi, NAND_CMD_READ0);
+	flex_addr(nandi, col_addr, 2);
+	flex_addr(nandi, page_addr, nandi->extra_addr ? 3 : 2);
+	flex_cmd(nandi, NAND_CMD_READSTART);
+
+	flex_wait_rbn(nandi);
+
+	readsl(nandi->base + NANDHAM_FLEX_DATA, buf, len / 4);
+
+	nandi_disable_interrupts(nandi, NAND_INT_RBN);
+
+	writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+
+	return 0;
+}
+
+static int flex_write_raw(struct nandi_controller *nandi,
+			  uint32_t page_addr,
+			  uint32_t col_addr,
+			  uint8_t *buf, uint32_t len)
+{
+	uint8_t status;
+
+	dev_dbg(nandi->dev, "%s %u bytes at [0x%06x,0x%04x]\n",
+		__func__, len, page_addr, col_addr);
+
+	BUG_ON(len & 0x3);
+	BUG_ON((unsigned long)buf & 0x3);
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+	nandi_enable_interrupts(nandi, NAND_INT_RBN);
+	reinit_completion(&nandi->rbn_completed);
+
+	writel(FLEX_DATA_CFG_BEATS_4 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAWRITE_CONFIG);
+
+	flex_cmd(nandi, NAND_CMD_SEQIN);
+	flex_addr(nandi, col_addr, 2);
+	flex_addr(nandi, page_addr, nandi->extra_addr ? 3 : 2);
+
+	writesl(nandi->base + NANDHAM_FLEX_DATA, buf, len / 4);
+
+	flex_cmd(nandi, NAND_CMD_PAGEPROG);
+
+	flex_wait_rbn(nandi);
+
+	nandi_disable_interrupts(nandi, NAND_INT_RBN);
+
+	writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+	       nandi->base + NANDHAM_FLEX_DATAWRITE_CONFIG);
+
+	flex_cmd(nandi, NAND_CMD_STATUS);
+
+	status = (uint8_t)(readl(nandi->base + NANDHAM_FLEX_DATA) & 0xff);
+
+	return status;
+}
+
 /*
  * Bad Block Tables/Bad Block Markers
  */
-- 
1.8.3.2

[RFC 31/47] mtd: nand: stm_nand_bch: scan block for BBM(s) according to specified BBT options

[Lee Jones]

Search the specified (first and, second or last) page in a block for
the BBT. Support Hamming or Advanced Flex Mode (AFM) BBMs too.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 62 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 4812431..29eeee8 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -643,6 +643,66 @@ static int bbt_is_block_bad(uint8_t *bbt, uint32_t block)
 	return bbt_get_block_mark(bbt, block) == BBT_MARK_GOOD ? 0 : 1;
 }
 
+/* Scan page for BBM(s), according to specified BBT options */
+static int nandi_scan_bad_block_markers_page(struct nandi_controller *nandi,
+					     uint32_t page)
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	struct nand_chip *chip = mtd->priv;
+	uint8_t *oob_buf = nandi->oob_buf;
+	int i, e;
+
+	/* Read the OOB area */
+	flex_read_raw(nandi, page, mtd->writesize, oob_buf, mtd->oobsize);
+
+	if (oob_buf[chip->badblockpos] == 0xff)
+		return 0;
+
+	/* Tolerate 'alien' Hamming Boot Mode ECC */
+	e = 0;
+	for (i = 0; i < mtd->oobsize; i += 16)
+		e += hweight8(oob_buf[i + 3] ^ 'B');
+	if (e <= 1)
+		return 0;
+
+	/* Tolerate 'alien' Hamming AFM ECC */
+	e = 0;
+	for (i = 0; i < mtd->oobsize; i += 16) {
+		e += hweight8(oob_buf[i + 3] ^ 'A');
+		e += hweight8(oob_buf[i + 4] ^ 'F');
+		e += hweight8(oob_buf[i + 5] ^ 'M');
+		if (e <= 1)
+			return 0;
+	}
+
+	return 1;
+}
+
+/* Scan block for BBM(s), according to specified BBT options */
+static int nandi_scan_bad_block_markers_block(struct nandi_controller *nandi,
+					      uint32_t block)
+
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	struct nand_chip *chip = mtd->priv;
+	uint32_t pages_per_block = mtd->erasesize >> nandi->page_shift;
+	uint32_t page = block << (nandi->block_shift - nandi->page_shift);
+
+	if (nandi_scan_bad_block_markers_page(nandi, page))
+		return 1;
+
+	if ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) &&
+	    nandi_scan_bad_block_markers_page(nandi, page + 1))
+		return 1;
+
+	if ((chip->bbt_options & NAND_BBT_SCANLASTPAGE) &&
+	    nandi_scan_bad_block_markers_page(nandi,
+					      page + pages_per_block - 1))
+		return 1;
+
+	return 0;
+}
+
 /* Populate IBBT BCH Header */
 static void bch_fill_ibbt_header(struct nandi_controller *nandi,
 				 struct nand_ibbt_bch_header *ibbt_header,
@@ -957,6 +1017,8 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	chip->priv = nandi;
 	chip->ecc.layout = &info->ecclayout;
 
+	chip->bbt_options |= NAND_BBT_USE_FLASH;
+
 	/* mtd_info */
 	mtd->owner = THIS_MODULE;
 	mtd->type = MTD_NANDFLASH;
-- 
1.8.3.2

[RFC 32/47] mtd: nand: stm_nand_bch: scan for BBMs and build memory-resident BBT

[Lee Jones]

If at start-up time we can't find our own pre-built memory-resident BBT
(e.g. at first run) this routine will search for factory set BBMs and
duplicate them into our own BBT for local consumption.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 29eeee8..d7d97720 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -703,6 +703,33 @@ static int nandi_scan_bad_block_markers_block(struct nandi_controller *nandi,
 	return 0;
 }
 
+/* Scan for BBMs and build memory-resident BBT */
+static int nandi_scan_build_bbt(struct nandi_controller *nandi,
+				struct nandi_bbt_info *bbt_info)
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	struct nand_chip *chip = mtd->priv;
+	uint32_t page_size = mtd->writesize;
+	uint8_t *bbt = bbt_info->bbt;
+	uint32_t block;
+
+	dev_dbg(nandi->dev,
+		"scan device for bad-block markers [bbt options = 0x%02x]\n",
+		chip->bbt_options);
+
+	memset(bbt, 0xff, page_size);
+	bbt_info->bbt_vers[0] = 0;
+	bbt_info->bbt_vers[1] = 0;
+	bbt_info->bbt_block[0] = nandi->blocks_per_device - 1;
+	bbt_info->bbt_block[1] = nandi->blocks_per_device - 2;
+
+	for (block = 0; block < nandi->blocks_per_device; block++)
+		if (nandi_scan_bad_block_markers_block(nandi, block))
+			bbt_set_block_mark(bbt, block, BBT_MARK_BAD_FACTORY);
+
+	return 0;
+}
+
 /* Populate IBBT BCH Header */
 static void bch_fill_ibbt_header(struct nandi_controller *nandi,
 				 struct nand_ibbt_bch_header *ibbt_header,
-- 
1.8.3.2

[RFC 33/47] mtd: nand: stm_nand_bch: search for and load flash-resident BBT

[Lee Jones]

If a BBT already exists in flash, it's this function's task to locate
it and load it into the driver for local consumption.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 87 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index d7d97720..d1d84c1 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -973,6 +973,93 @@ static int bch_find_ibbt_sig(struct nandi_controller *nandi,
 	return 1;
 }
 
+/* Search for and load Flash-resident BBT, updating Primary/Mirror if req'd */
+static int bch_load_bbt(struct nandi_controller *nandi,
+			struct nandi_bbt_info *bbt_info)
+{
+	unsigned int update = 0;
+	uint32_t block;
+	loff_t offs;
+	uint8_t vers;
+	char author[64];
+	int bak;
+
+	dev_dbg(nandi->dev, "looking for Flash-resident BBTs\n");
+
+	bbt_info->bbt_block[0] = 0;
+	bbt_info->bbt_block[1] = 0;
+	bbt_info->bbt_vers[0] = 0;
+	bbt_info->bbt_vers[1] = 0;
+
+	/* Look for IBBT signatures */
+	for (block = nandi->blocks_per_device - NAND_IBBT_NBLOCKS;
+	     block < nandi->blocks_per_device;
+	     block++) {
+		offs = (loff_t)block << nandi->block_shift;
+
+		if (bch_find_ibbt_sig(nandi, block, &bak, &vers, author)) {
+			dev_dbg(nandi->dev,
+				"found BBT [%s:%u] at 0x%012llx [%u] (%s)\n",
+				bbt_strs[bak], vers, offs, block,
+				author);
+
+			if (bbt_info->bbt_block[bak] == 0 ||
+			    ((int8_t)(bbt_info->bbt_vers[bak] - vers)) < 0) {
+				bbt_info->bbt_block[bak] = block;
+				bbt_info->bbt_vers[bak] = vers;
+			}
+		}
+	}
+
+	/* What have we found? */
+	if (bbt_info->bbt_block[0] == 0 && bbt_info->bbt_block[1] == 0) {
+		/* no primary, no mirror: return error */
+		return 1;
+	} else if (bbt_info->bbt_block[0] == 0) {
+		/* no primary: use mirror, update primary */
+		bak = 1;
+		update = NAND_IBBT_UPDATE_PRIMARY;
+		bbt_info->bbt_block[0] = nandi->blocks_per_device - 1;
+	} else if (bbt_info->bbt_block[1] == 0) {
+		/* no mirror: use primary, update mirror */
+		bak = 0;
+		update = NAND_IBBT_UPDATE_MIRROR;
+		bbt_info->bbt_block[1] = nandi->blocks_per_device - 1;
+	} else if (bbt_info->bbt_vers[0] == bbt_info->bbt_vers[1]) {
+		/* primary == mirror: use primary, no update required */
+		bak = 0;
+	} else if ((int8_t)(bbt_info->bbt_vers[1] -
+			    bbt_info->bbt_vers[0]) < 0) {
+		/* primary > mirror: use primary, update mirror */
+		bak = 0;
+		update = NAND_IBBT_UPDATE_MIRROR;
+	} else {
+		/* mirror > primary: use mirror, update primary */
+		bak = 1;
+		update = NAND_IBBT_UPDATE_PRIMARY;
+	}
+
+	vers = bbt_info->bbt_vers[bak];
+	block = bbt_info->bbt_block[bak];
+	offs = block << nandi->block_shift;
+	dev_info(nandi->dev, "using BBT [%s:%u] at 0x%012llx [%u]\n",
+		 bbt_strs[bak], vers, offs, block);
+
+	/* Read BBT data */
+	if (bch_read_page(nandi, offs, bbt_info->bbt) < 0) {
+		dev_err(nandi->dev,
+			"error while reading BBT %s:%u] at 0x%012llx [%u]\n",
+			bbt_strs[bak], vers, offs, block);
+		return 1;
+	}
+
+	/* Update other BBT if required */
+	if (update)
+		bch_update_bbts(nandi, bbt_info, update, vers);
+
+	return 0;
+}
+
 /*
  * Initialisation
  */
-- 
1.8.3.2

[RFC 34/47] mtd: nand: stm_nand_bch: load flash-resident BBT

[Lee Jones]

If load of existing BBT fails (first run), scan device for factory-set
BBMs (Bad-Block Markers) and create a new flash-resident.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index d1d84c1..6698b1f 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1834,6 +1834,21 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* Load Flash-resident BBT */
+	err = bch_load_bbt(nandi, bbt_info);
+	if (err) {
+		dev_warn(nandi->dev,
+			"failed to find BBTs:"
+			"  scanning device for bad-block markers\n");
+
+		/* Scan, build, and write BBT */
+		nandi_scan_build_bbt(nandi, bbt_info);
+		err = bch_update_bbts(nandi, bbt_info, NAND_IBBT_UPDATE_BOTH,
+				      bbt_info->bbt_vers[0] + 1);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
-- 
1.8.3.2

[RFC 35/47] mtd: nand: stm_nand_bch: dump bad blocks

[Lee Jones]

Inform the user of any known bad blocks during initialisation.
Conversely, if there aren't any known bad blocks, let the user know
the good news.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 6698b1f..abbb4d9 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1060,6 +1060,28 @@ static int bch_load_bbt(struct nandi_controller *nandi,
 	return 0;
 }
 
+static void nandi_dump_bad_blocks(struct nandi_controller *nandi)
+{
+	int bad_count = 0;
+	uint32_t block;
+	uint8_t *bbt = nandi->info.bbt_info.bbt;
+	uint8_t mark;
+
+	pr_info("BBT:\n");
+	for (block = 0; block < nandi->blocks_per_device; block++) {
+		mark = bbt_get_block_mark(bbt, block);
+		if (mark != BBT_MARK_GOOD) {
+			pr_info("\t\tBlock 0x%08x [%05u] marked bad [%s]\n",
+				block << nandi->block_shift, block,
+				(mark == BBT_MARK_BAD_FACTORY) ?
+				"Factory" : "Wear");
+			bad_count++;
+		}
+	}
+	if (bad_count == 0)
+		pr_info("\t\tNo bad blocks listed in BBT\n");
+}
+
 /*
  * Initialisation
  */
@@ -1849,6 +1871,8 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 			return err;
 	}
 
+	nandi_dump_bad_blocks(nandi);
+
 	return 0;
 }
 
-- 
1.8.3.2

Re: [RFC 35/47] mtd: nand: stm_nand_bch: dump bad blocks

[Ezequiel Garcia]

On Mar 25, Lee Jones wrote:
> Inform the user of any known bad blocks during initialisation.
> Conversely, if there aren't any known bad blocks, let the user know
> the good news.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mtd/nand/stm_nand_bch.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
> index 6698b1f..abbb4d9 100644
> --- a/drivers/mtd/nand/stm_nand_bch.c
> +++ b/drivers/mtd/nand/stm_nand_bch.c
> @@ -1060,6 +1060,28 @@ static int bch_load_bbt(struct nandi_controller *nandi,
>  	return 0;
>  }
>  
> +static void nandi_dump_bad_blocks(struct nandi_controller *nandi)
> +{
> +	int bad_count = 0;
> +	uint32_t block;
> +	uint8_t *bbt = nandi->info.bbt_info.bbt;
> +	uint8_t mark;
> +
> +	pr_info("BBT:\n");
> +	for (block = 0; block < nandi->blocks_per_device; block++) {
> +		mark = bbt_get_block_mark(bbt, block);
> +		if (mark != BBT_MARK_GOOD) {
> +			pr_info("\t\tBlock 0x%08x [%05u] marked bad [%s]\n",
> +				block << nandi->block_shift, block,
> +				(mark == BBT_MARK_BAD_FACTORY) ?
> +				"Factory" : "Wear");
> +			bad_count++;
> +		}
> +	}
> +	if (bad_count == 0)
> +		pr_info("\t\tNo bad blocks listed in BBT\n");
> +}
> +
>  /*
>   * Initialisation
>   */
> @@ -1849,6 +1871,8 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
>  			return err;
>  	}
>  
> +	nandi_dump_bad_blocks(nandi);
> +

Hm... this looks suspicious. Are you sure you need this?

The NAND BBT code prints the bad blocks at initialization-time. If that's
not the case for you, perhaps we should be fixing that in the BBT code,
rather than at a driver's level.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[RFC 36/47] mtd: nand: stm_nand_bch: parse partitions and register an MTD device

[Lee Jones]

Issue the core parse partitions and register as MTD device call.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index abbb4d9..a0f0ae2 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1738,6 +1738,7 @@ static void *stm_bch_dt_get_pdata(struct platform_device *pdev)
 
 static int stm_nand_bch_probe(struct platform_device *pdev)
 {
+	const char *part_probes[] = { "cmdlinepart", "ofpart", NULL, };
 	struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data;
 	struct device_node *np = pdev->dev.of_node;
 	struct mtd_part_parser_data ppdata;
@@ -1873,7 +1874,9 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 
 	nandi_dump_bad_blocks(nandi);
 
-	return 0;
+	/* Add partitions */
+	return mtd_device_parse_register(mtd, part_probes, &ppdata,
+					bank->partitions, bank->nr_partitions);
 }
 
 static int stm_nand_bch_remove(struct platform_device *pdev)
-- 
1.8.3.2

[RFC 37/47] mtd: nand: stm_nand_bch: fetch the bit-flips threshold

[Lee Jones]

If none is provided by the platform, the default ECC mode will be used.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index a0f0ae2..3853e7a 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -44,6 +44,12 @@ static int bch_ecc_sizes[] = {
 	[BCH_NO_ECC] = 0,
 };
 
+static int bch_ecc_strength[] = {
+	[BCH_18BIT_ECC] = 18,
+	[BCH_30BIT_ECC] = 30,
+	[BCH_NO_ECC] = 0,
+};
+
 /*
  * Inband Bad Block Table (IBBT)
  */
@@ -1843,6 +1849,21 @@ static int stm_nand_bch_probe(struct platform_device *pdev)
 	info->ecclayout.eccbytes =
 		nandi->sectors_per_page * bch_ecc_sizes[nandi->bch_ecc_mode];
 
+	/*
+	 * Get bit-flips threshold. A value of '0' is interpreted as
+	 * <ecc_strength>.
+	 */
+	if (pdata->bch_bitflip_threshold) {
+		nandi->bitflip_threshold = pdata->bch_bitflip_threshold;
+	} else {
+		dev_warn(nandi->dev,
+			 "WARNING: bit-flips threshold not specified.\n"
+			 "         Defaulting to ECC strength [%d]\n",
+			 bch_ecc_strength[nandi->bch_ecc_mode]);
+		nandi->bitflip_threshold =
+			bch_ecc_strength[nandi->bch_ecc_mode];
+	}
+
 	compatible = bch_check_compatibility(nandi, mtd, chip);
 	if (!compatible) {
 		dev_err(nandi->dev,
-- 
1.8.3.2

[RFC 38/47] mtd: nand: stm_nand_bch: check WP (FLEX)

[Lee Jones]

Return status of the Write Protect bit for the NAND device.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 3853e7a..18601e5 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -543,6 +543,22 @@ static void flex_addr(struct nandi_controller *nandi,
 	writel(addr, nandi->base + NANDHAM_FLEX_ADD);
 }
 
+/*
+ * Hamming-FLEX operations (optimised replacements for nand_base.c versions)
+ */
+static int flex_check_wp(struct nandi_controller *nandi)
+{
+	uint8_t status;
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	flex_cmd(nandi, NAND_CMD_STATUS);
+
+	status = (uint8_t)(readl(nandi->base + NANDHAM_FLEX_DATA) & 0xff);
+
+	return status & NAND_STATUS_WP ? 0 : 1;
+}
+
 static int flex_read_raw(struct nandi_controller *nandi,
 			 uint32_t page_addr,
 			 uint32_t col_addr,
-- 
1.8.3.2

[RFC 39/47] mtd: nand: stm_nand_bch: read and write ops (FLEX)

[Lee Jones]

Helper functions for mtd_write_oob() and mtd_write_oob().
Handles multi-page transfers and mapping between BCH sectors
and MTD page+OOB data.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 136 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 18601e5..75c5c9b 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1082,6 +1082,142 @@ static int bch_load_bbt(struct nandi_controller *nandi,
 	return 0;
 }
 
+/*
+ * Helper function for mtd_read_oob(): handles multi-page transfers
+ * and mapping between BCH sectors and MTD page+OOB data.
+ */
+static int flex_do_read_ops(struct nandi_controller *nandi,
+			    loff_t from,
+			    struct mtd_oob_ops *ops)
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	uint32_t page_addr = from >> nandi->page_shift;
+	uint32_t oob_remainder;
+	uint8_t *oobbuf = ops->oobbuf;
+	uint8_t *datbuf = ops->datbuf;
+	uint8_t *page_buf;
+	int ecc_size;
+	int pages;
+	int s;
+
+	ecc_size = bch_ecc_sizes[nandi->bch_ecc_mode];
+	nandi->cached_page = -1;
+
+	pages = ops->datbuf ?
+		(ops->len >> nandi->page_shift) :
+		(ops->ooblen / mtd->oobsize);
+
+	oob_remainder = mtd->oobsize - (nandi->sectors_per_page * ecc_size);
+
+	while (pages) {
+		page_buf = nandi->page_buf;
+
+		flex_read_raw(nandi, page_addr, 0, page_buf,
+			      mtd->writesize + mtd->oobsize);
+
+		for (s = 0; s < nandi->sectors_per_page; s++) {
+			if (datbuf) {
+				memcpy(datbuf, page_buf, NANDI_BCH_SECTOR_SIZE);
+				datbuf += NANDI_BCH_SECTOR_SIZE;
+				ops->retlen += NANDI_BCH_SECTOR_SIZE;
+			}
+			page_buf += NANDI_BCH_SECTOR_SIZE;
+
+			if (oobbuf) {
+				memcpy(oobbuf, page_buf, ecc_size);
+				ops->oobretlen += ecc_size;
+				oobbuf += ecc_size;
+			}
+			page_buf += ecc_size;
+		}
+
+		if (oob_remainder && oobbuf) {
+			memcpy(oobbuf, page_buf, oob_remainder);
+			oobbuf += oob_remainder;
+			ops->oobretlen += oob_remainder;
+		}
+
+		page_addr++;
+		pages--;
+	}
+
+	return 0;
+}
+
+/*
+ * Helper function for mtd_write_oob(): handles multi-page transfers
+ * and mapping between BCH sectors and MTD page+OOB data.
+*/
+static int flex_do_write_ops(struct nandi_controller *nandi,
+			     loff_t to,
+			     struct mtd_oob_ops *ops)
+{
+	struct mtd_info *mtd = &nandi->info.mtd;
+	uint32_t page_addr = to >> nandi->page_shift;
+	uint32_t oob_remainder;
+	uint8_t *oobbuf = ops->oobbuf;
+	uint8_t *datbuf = ops->datbuf;
+	uint8_t *page_buf;
+	uint8_t status;
+	int ecc_size;
+	int pages;
+	int s;
+
+	ecc_size = bch_ecc_sizes[nandi->bch_ecc_mode];
+	nandi->cached_page = -1;
+
+	pages = ops->datbuf ?
+		(ops->len >> nandi->page_shift) :
+		(ops->ooblen / mtd->oobsize);
+
+	oob_remainder = mtd->oobsize - (nandi->sectors_per_page * ecc_size);
+
+	while (pages) {
+		page_buf = nandi->page_buf;
+
+		for (s = 0; s < nandi->sectors_per_page; s++) {
+			if (datbuf) {
+				memcpy(page_buf, datbuf, NANDI_BCH_SECTOR_SIZE);
+				datbuf += NANDI_BCH_SECTOR_SIZE;
+				ops->retlen += NANDI_BCH_SECTOR_SIZE;
+			} else {
+				memset(page_buf, 0xff, NANDI_BCH_SECTOR_SIZE);
+			}
+			page_buf += NANDI_BCH_SECTOR_SIZE;
+
+			if (oobbuf) {
+				memcpy(page_buf, oobbuf, ecc_size);
+				oobbuf += ecc_size;
+				ops->oobretlen += ecc_size;
+			} else {
+				memset(page_buf, 0xff, ecc_size);
+			}
+			page_buf += ecc_size;
+		}
+
+		if (oob_remainder) {
+			if (oobbuf) {
+				memcpy(page_buf, oobbuf, oob_remainder);
+				oobbuf += oob_remainder;
+				ops->oobretlen += oob_remainder;
+			} else {
+				memset(page_buf, 0xff, oob_remainder);
+			}
+		}
+
+		status = flex_write_raw(nandi, page_addr, 0, nandi->page_buf,
+					mtd->writesize + mtd->oobsize);
+
+		if (status & NAND_STATUS_FAIL)
+			return -EIO;
+
+		page_addr++;
+		pages--;
+	}
+
+	return 0;
+}
+
 static void nandi_dump_bad_blocks(struct nandi_controller *nandi)
 {
 	int bad_count = 0;
-- 
1.8.3.2

[RFC 40/47] mtd: nand: stm_nand_bch: MTD erase (BCH)

[Lee Jones]

Carry out the normal checks, then attempt to erase a given block.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 85 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 75c5c9b..55f4a4c 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1218,6 +1218,89 @@ static int flex_do_write_ops(struct nandi_controller *nandi,
 	return 0;
 }
 
+static int bch_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	uint64_t block_mask = mtd->erasesize - 1;
+	uint64_t offs_cached;
+	loff_t offs = instr->addr;
+	size_t len = instr->len;
+	uint8_t status;
+	int ret;
+
+	dev_dbg(nandi->dev, "%s: 0x%012llx @ 0x%012llx\n", __func__,
+		(unsigned long long)len, offs);
+
+	if (offs & block_mask) {
+		dev_err(nandi->dev,
+			"attempt to erase from non-block-aligned offset\n");
+		return -EINVAL;
+	}
+
+	if (len & block_mask) {
+		dev_err(nandi->dev,
+			"attempt to erase non-block-aligned length\n");
+		return -EINVAL;
+	}
+
+	if ((offs + len) > mtd->size) {
+		dev_err(nandi->dev, "attempt to erase past end of device\n");
+		return -EINVAL;
+	}
+
+	nand_get_device(mtd, FL_ERASING);
+	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+
+	if (flex_check_wp(nandi)) {
+		dev_dbg(nandi->dev, "device is write-protected\n");
+		instr->state = MTD_ERASE_FAILED;
+		goto erase_exit;
+	}
+
+	/* Offset of block containing cached page */
+	offs_cached = ((uint64_t)nandi->cached_page << nandi->page_shift) &
+		~block_mask;
+
+	instr->state = MTD_ERASING;
+	while (len) {
+		if (mtd_block_isbad(mtd, offs)) {
+			dev_err(nandi->dev,
+				"attempt to erase a bad block at 0x%012llx\n",
+				offs);
+			instr->state = MTD_ERASE_FAILED;
+			instr->fail_addr = offs;
+			goto erase_exit;
+		}
+
+		if (offs == offs_cached)
+			nandi->cached_page = -1;
+
+		status = bch_erase_block(nandi, offs);
+
+		if (status & NAND_STATUS_FAIL) {
+			dev_err(nandi->dev,
+				"failed to erase block at 0x%012llx\n", offs);
+			instr->state = MTD_ERASE_FAILED;
+			instr->fail_addr = offs;
+			goto erase_exit;
+		}
+
+		len -= mtd->erasesize;
+		offs += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+
+ erase_exit:
+	nand_release_device(mtd);
+
+	ret = instr->state == MTD_ERASE_DONE ? 0 : -EIO;
+	if (ret == 0)
+		mtd_erase_callback(instr);
+
+	return ret;
+}
+
 static void nandi_dump_bad_blocks(struct nandi_controller *nandi)
 {
 	int bad_count = 0;
@@ -1321,6 +1404,8 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	mtd->oobavail = 0;
 	mtd->subpage_sft = 0;
 
+	mtd->_erase = bch_mtd_erase;
+
 	mtd->_point = NULL;
 	mtd->_unpoint = NULL;
 	mtd->_lock = NULL;
-- 
1.8.3.2

[RFC 41/47] mtd: nand: stm_nand_bch: MTD mark and check for bad blocks (BCH)

[Lee Jones]

Use the local BBM helpers to check for and mark bad blocks.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 51 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 55f4a4c..38658b8 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1218,6 +1218,55 @@ static int flex_do_write_ops(struct nandi_controller *nandi,
 	return 0;
 }
 
+static int bch_mtd_block_isbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+
+	uint32_t block;
+
+	/* Check for invalid offset */
+	if (offs > mtd->size)
+		return -EINVAL;
+
+	block = offs >> nandi->block_shift;
+
+	/* Protect blocks reserved for BBTs */
+	if (block >= (nandi->blocks_per_device - NAND_IBBT_NBLOCKS))
+		return 1;
+
+	return bbt_is_block_bad(nandi->info.bbt_info.bbt, block);
+}
+
+static int bch_mtd_block_markbad(struct mtd_info *mtd, loff_t offs)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+
+	uint32_t block;
+	int ret;
+
+	/* Is block already considered bad? (will also catch invalid offsets) */
+	ret = mtd_block_isbad(mtd, offs);
+	if (ret < 0)
+		return ret;
+	if (ret == 1)
+		return 0;
+
+	/* Mark bad */
+	block = offs >> nandi->block_shift;
+	bbt_set_block_mark(nandi->info.bbt_info.bbt, block, BBT_MARK_BAD_WEAR);
+
+	/* Update BBTs, incrementing bbt_vers */
+	nand_get_device(mtd, FL_WRITING);
+	ret = bch_update_bbts(nandi, &nandi->info.bbt_info,
+			      NAND_IBBT_UPDATE_BOTH,
+			      nandi->info.bbt_info.bbt_vers[0] + 1);
+	nand_release_device(mtd);
+
+	return ret;
+}
+
 static int bch_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct nand_chip *chip = mtd->priv;
@@ -1405,6 +1454,8 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	mtd->subpage_sft = 0;
 
 	mtd->_erase = bch_mtd_erase;
+	mtd->_block_isbad = bch_mtd_block_isbad;
+	mtd->_block_markbad = bch_mtd_block_markbad;
 
 	mtd->_point = NULL;
 	mtd->_unpoint = NULL;
-- 
1.8.3.2

[RFC 42/47] mtd: nand: stm_nand_bch: add read and write OOB (BCH)

[Lee Jones]

Provide functions to read and write to the Out-Of-Bounds (OOB) area.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 107 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 38658b8..389ccee 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1218,6 +1218,111 @@ static int flex_do_write_ops(struct nandi_controller *nandi,
 	return 0;
 }
 
+static char *mtd_oob_mode_strs[] = {"PLACE", "AUTO", "RAW"};
+
+static int bch_mtd_read_and_write_oob(struct mtd_info *mtd, loff_t tofrom,
+				      struct mtd_oob_ops *ops,
+				      bool read)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	uint32_t page_mask = mtd->writesize - 1;
+	int ret;
+
+	dev_dbg(nandi->dev, "%s: 0x%012llx [page = %u, oob = %u mode = %s]\n",
+		__func__, tofrom,
+		(ops->datbuf ? ops->len : 0),
+		(ops->oobbuf ? ops->ooblen : 0),
+		mtd_oob_mode_strs[ops->mode]);
+
+	if (!ops->oobbuf && ops->mode != MTD_OPS_RAW) {
+		if (read)
+			return mtd_read(mtd, tofrom, ops->len,
+					&ops->retlen, ops->datbuf);
+		else
+			return mtd_write(mtd, tofrom, ops->len,
+					 &ops->retlen, ops->datbuf);
+	}
+
+	ops->oobretlen = 0;
+	ops->retlen = 0;
+
+	/*
+	 * We report OOB as unavailable (i.e. oobavail = 0), therefore nothing
+	 * should call this
+	 */
+	if (ops->oobbuf && ops->mode == MTD_OPS_AUTO_OOB)
+		return -ENOTSUPP;
+
+	/* Not currently supported by MTD */
+	if (ops->datbuf && ops->oobbuf && ops->mode == MTD_OPS_PLACE_OOB)
+		return -ENOTSUPP;
+
+	/* Do not allow oob reads with ooboffs */
+	if (ops->oobbuf && ops->ooboffs)
+		return -ENOTSUPP;
+
+	/* Do not allow reads past end of device */
+	if (ops->datbuf && (tofrom + ops->len) > mtd->size) {
+		dev_err(nandi->dev, "attempt %s beyond end of device\n",
+			read ? "read" : "write");
+		return -EINVAL;
+	}
+
+	if (ops->oobbuf &&
+	    (tofrom + mtd->writesize * (ops->ooblen / mtd->oobsize))
+	    > mtd->size) {
+		dev_err(nandi->dev, "attempt %s beyond end of device\n",
+			read ? "read" : "write");
+		return -EINVAL;
+	}
+
+	/* Do not allow non-aligned reads/writes */
+	if ((tofrom & page_mask) ||
+	    (ops->datbuf && (ops->len & page_mask)) ||
+	    (ops->oobbuf && (ops->ooblen % mtd->oobsize))) {
+		dev_err(nandi->dev, "attempt to %s non-aligned data\n",
+			read ? "read" : "write");
+		return -EINVAL;
+	}
+
+	/* Do not allow inconsistent data and oob lengths */
+	if (ops->datbuf && ops->oobbuf &&
+	    (ops->len / mtd->writesize != ops->ooblen / mtd->oobsize)) {
+		dev_err(nandi->dev,
+			"data length inconsistent with oob length\n");
+		return -EINVAL;
+	}
+
+	nand_get_device(mtd, read ? FL_READING : FL_WRITING);
+
+	if (!read && flex_check_wp(nandi)) {
+		dev_dbg(nandi->dev, "device is write-protected\n");
+		return -EIO;
+	}
+
+	if (read)
+		ret = flex_do_read_ops(nandi, tofrom, ops);
+	else
+		ret = flex_do_write_ops(nandi, tofrom, ops);
+
+	nand_release_device(mtd);
+
+	return ret;
+}
+
+static int bch_mtd_read_oob(struct mtd_info *mtd, loff_t from,
+			    struct mtd_oob_ops *ops)
+{
+	return bch_mtd_read_and_write_oob(mtd, from, ops, true);
+}
+
+static int bch_mtd_write_oob(struct mtd_info *mtd, loff_t to,
+			     struct mtd_oob_ops *ops)
+{
+	return bch_mtd_read_and_write_oob(mtd, to, ops, false);
+}
+
 static int bch_mtd_block_isbad(struct mtd_info *mtd, loff_t offs)
 {
 	struct nand_chip *chip = mtd->priv;
@@ -1454,6 +1559,8 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	mtd->subpage_sft = 0;
 
 	mtd->_erase = bch_mtd_erase;
+	mtd->_read_oob = bch_mtd_read_oob;
+	mtd->_write_oob = bch_mtd_write_oob;
 	mtd->_block_isbad = bch_mtd_block_isbad;
 	mtd->_block_markbad = bch_mtd_block_markbad;
 
-- 
1.8.3.2

[RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)

[Lee Jones]

Helper function for bch_mtd_read() and bch_mtd_write() to handle
multi-page or non-aligned reads and writes respectively.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 143 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 389ccee..bcaed32 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -507,6 +507,149 @@ static uint8_t bch_write_page(struct nandi_controller *nandi,
 	return status;
 }
 
+/* Helper function for bch_mtd_read to handle multi-page or non-aligned reads */
+static int bch_read(struct nandi_controller *nandi,
+		    loff_t from, size_t len,
+		    size_t *retlen, u_char *buf)
+{
+	struct mtd_ecc_stats stats;
+	uint32_t page_size = nandi->info.mtd.writesize;
+	uint32_t col_offs;
+	loff_t page_mask;
+	loff_t page_offs;
+	int ecc_errs, max_ecc_errs = 0;
+	int page_num;
+	size_t bytes;
+	uint8_t *p;
+	bool bounce = false;
+
+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
+		(unsigned long long)len, from);
+
+	stats = nandi->info.mtd.ecc_stats;
+	page_mask = (loff_t)page_size - 1;
+	col_offs  = (uint32_t)(from & page_mask);
+	page_offs = from & ~page_mask;
+	page_num  = (int)(page_offs >> nandi->page_shift);
+
+	while (len > 0) {
+		bytes = min((page_size - col_offs), len);
+
+		if ((bytes != page_size) ||
+		    ((unsigned int)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
+		    (!virt_addr_valid(buf))) /* vmalloc'd buffer! */
+			bounce = true;
+
+		if (page_num == nandi->cached_page) {
+			memcpy(buf, nandi->page_buf + col_offs, bytes);
+			goto done;
+		}
+
+		p = bounce ? nandi->page_buf : buf;
+
+		ecc_errs = bch_read_page(nandi, page_offs, p);
+		if (bounce)
+			memcpy(buf, p + col_offs, bytes);
+
+		if (ecc_errs < 0) {
+			dev_err(nandi->dev,
+				"%s: uncorrectable error at 0x%012llx\n",
+				__func__, page_offs);
+			nandi->info.mtd.ecc_stats.failed++;
+
+			/* Do not cache uncorrectable pages */
+			if (bounce)
+				nandi->cached_page = -1;
+
+			goto done;
+		}
+
+		if (ecc_errs) {
+			dev_info(nandi->dev,
+				 "%s: corrected %u error(s) at 0x%012llx\n",
+				 __func__, ecc_errs, page_offs);
+
+			nandi->info.mtd.ecc_stats.corrected += ecc_errs;
+
+			if (ecc_errs > max_ecc_errs)
+				max_ecc_errs = ecc_errs;
+		}
+
+		if (bounce)
+			nandi->cached_page = page_num;
+
+done:
+		buf += bytes;
+		len -= bytes;
+
+		if (retlen)
+			*retlen += bytes;
+
+		/* We are now page-aligned */
+		page_offs += page_size;
+		page_num++;
+		col_offs = 0;
+	}
+
+	/* Return '-EBADMSG' on uncorrectable errors */
+	if (nandi->info.mtd.ecc_stats.failed - stats.failed)
+		return -EBADMSG;
+
+	return max_ecc_errs;
+}
+
+/* Helper function for mtd_write, to handle multi-page and non-aligned writes */
+static int bch_write(struct nandi_controller *nandi,
+		     loff_t to, size_t len,
+		     size_t *retlen, const uint8_t *buf)
+{
+	uint32_t page_size = nandi->info.mtd.writesize;
+	int page_num;
+	bool bounce = false;
+	const uint8_t *p = NULL;
+	uint8_t ret;
+
+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
+		(unsigned long long)len, to);
+
+	BUG_ON(len & (page_size - 1));
+	BUG_ON(to & (page_size - 1));
+
+	if (((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
+	    !virt_addr_valid(buf)) { /* vmalloc'd buffer! */
+		bounce = true;
+	}
+
+	page_num = (int)(to >> nandi->page_shift);
+
+	while (len > 0) {
+		if (bounce) {
+			memcpy(nandi->page_buf, buf, page_size);
+			p = nandi->page_buf;
+			nandi->cached_page = -1;
+		} else {
+			p = buf;
+		}
+
+		if (nandi->cached_page == page_num)
+			nandi->cached_page = -1;
+
+		ret = bch_write_page(nandi, to, p);
+		if (ret & NAND_STATUS_FAIL)
+			return -EIO;
+
+		to += page_size;
+		page_num++;
+		buf += page_size;
+		len -= page_size;
+
+		if (retlen)
+			*retlen += page_size;
+	}
+
+	return 0;
+}
+
 /*
  * Hamming-FLEX operations
  */
-- 
1.8.3.2

RE: [RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)

[Gupta, Pekon]

Hi Lee,

>From: Lee Jones [mailto:lee.jones@linaro.org]
>
>Helper function for bch_mtd_read() and bch_mtd_write() to handle
>multi-page or non-aligned reads and writes respectively.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---

I think below code is duplicate of nand_do_read_ops() and nand_do_write_ops()
in nand_base.c. If you could just populate chip->ecc.read_page and
chip->ecc.write_page much of this could be avoided.
Also, you need to break your bch_read_page() into given generic NAND
driver interfaces chip->ecc.calculate(), chip->ecc.correct() ...

This would make your driver much more simpler, and easy to maintain.


> drivers/mtd/nand/stm_nand_bch.c | 143 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 143 insertions(+)
>
>diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
>index 389ccee..bcaed32 100644
>--- a/drivers/mtd/nand/stm_nand_bch.c
>+++ b/drivers/mtd/nand/stm_nand_bch.c
>@@ -507,6 +507,149 @@ static uint8_t bch_write_page(struct nandi_controller *nandi,
> 	return status;
> }
>
>+/* Helper function for bch_mtd_read to handle multi-page or non-aligned reads */
>+static int bch_read(struct nandi_controller *nandi,
>+		    loff_t from, size_t len,
>+		    size_t *retlen, u_char *buf)
>+{
>+	struct mtd_ecc_stats stats;
>+	uint32_t page_size = nandi->info.mtd.writesize;
>+	uint32_t col_offs;
>+	loff_t page_mask;
>+	loff_t page_offs;
>+	int ecc_errs, max_ecc_errs = 0;
>+	int page_num;
>+	size_t bytes;
>+	uint8_t *p;
>+	bool bounce = false;
>+
>+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
>+		(unsigned long long)len, from);
>+
>+	stats = nandi->info.mtd.ecc_stats;
>+	page_mask = (loff_t)page_size - 1;
>+	col_offs  = (uint32_t)(from & page_mask);
>+	page_offs = from & ~page_mask;
>+	page_num  = (int)(page_offs >> nandi->page_shift);
>+
>+	while (len > 0) {
>+		bytes = min((page_size - col_offs), len);
>+
>+		if ((bytes != page_size) ||
>+		    ((unsigned int)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
>+		    (!virt_addr_valid(buf))) /* vmalloc'd buffer! */
>+			bounce = true;
>+
>+		if (page_num == nandi->cached_page) {
>+			memcpy(buf, nandi->page_buf + col_offs, bytes);
>+			goto done;
>+		}
>+
>+		p = bounce ? nandi->page_buf : buf;
>+
>+		ecc_errs = bch_read_page(nandi, page_offs, p);
>+		if (bounce)
>+			memcpy(buf, p + col_offs, bytes);
>+
>+		if (ecc_errs < 0) {
>+			dev_err(nandi->dev,
>+				"%s: uncorrectable error at 0x%012llx\n",
>+				__func__, page_offs);
>+			nandi->info.mtd.ecc_stats.failed++;
>+
>+			/* Do not cache uncorrectable pages */
>+			if (bounce)
>+				nandi->cached_page = -1;
>+
>+			goto done;
>+		}
>+
>+		if (ecc_errs) {
>+			dev_info(nandi->dev,
>+				 "%s: corrected %u error(s) at 0x%012llx\n",
>+				 __func__, ecc_errs, page_offs);
>+
>+			nandi->info.mtd.ecc_stats.corrected += ecc_errs;
>+
>+			if (ecc_errs > max_ecc_errs)
>+				max_ecc_errs = ecc_errs;
>+		}
>+
>+		if (bounce)
>+			nandi->cached_page = page_num;
>+
>+done:
>+		buf += bytes;
>+		len -= bytes;
>+
>+		if (retlen)
>+			*retlen += bytes;
>+
>+		/* We are now page-aligned */
>+		page_offs += page_size;
>+		page_num++;
>+		col_offs = 0;
>+	}
>+
>+	/* Return '-EBADMSG' on uncorrectable errors */
>+	if (nandi->info.mtd.ecc_stats.failed - stats.failed)
>+		return -EBADMSG;
>+
>+	return max_ecc_errs;
>+}
>+
>+/* Helper function for mtd_write, to handle multi-page and non-aligned writes */
>+static int bch_write(struct nandi_controller *nandi,
>+		     loff_t to, size_t len,
>+		     size_t *retlen, const uint8_t *buf)
>+{
>+	uint32_t page_size = nandi->info.mtd.writesize;
>+	int page_num;
>+	bool bounce = false;
>+	const uint8_t *p = NULL;
>+	uint8_t ret;
>+
>+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
>+		(unsigned long long)len, to);
>+
>+	BUG_ON(len & (page_size - 1));
>+	BUG_ON(to & (page_size - 1));
>+
>+	if (((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
>+	    !virt_addr_valid(buf)) { /* vmalloc'd buffer! */
>+		bounce = true;
>+	}
>+
>+	page_num = (int)(to >> nandi->page_shift);
>+
>+	while (len > 0) {
>+		if (bounce) {
>+			memcpy(nandi->page_buf, buf, page_size);
>+			p = nandi->page_buf;
>+			nandi->cached_page = -1;
>+		} else {
>+			p = buf;
>+		}
>+
>+		if (nandi->cached_page == page_num)
>+			nandi->cached_page = -1;
>+
>+		ret = bch_write_page(nandi, to, p);
>+		if (ret & NAND_STATUS_FAIL)
>+			return -EIO;
>+
>+		to += page_size;
>+		page_num++;
>+		buf += page_size;
>+		len -= page_size;
>+
>+		if (retlen)
>+			*retlen += page_size;
>+	}
>+
>+	return 0;
>+}
>+
> /*
>  * Hamming-FLEX operations
>  */
>--
>1.8.3.2


with regards, pekon

Re: [RFC 43/47] mtd: nand: stm_nand_bch: read and write functions (BCH)

[Lee Jones]

> >From: Lee Jones [mailto:lee.jones@linaro.org]
> >
> >Helper function for bch_mtd_read() and bch_mtd_write() to handle
> >multi-page or non-aligned reads and writes respectively.
> >
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> 
> I think below code is duplicate of nand_do_read_ops() and nand_do_write_ops()
> in nand_base.c. If you could just populate chip->ecc.read_page and
> chip->ecc.write_page much of this could be avoided.

chip->ecc.read_page and chip->ecc.write_page now populated.

> Also, you need to break your bch_read_page() into given generic NAND
> driver interfaces chip->ecc.calculate(), chip->ecc.correct() ...

Our h/w calculates and corrects automatically.  No need for
intervention.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 44/47] mtd: nand: stm_nand_bch: MTD read and write (BCH)

[Lee Jones]

MTD interfaces: Standard set of callbacks for MTD functionality.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 54 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index bcaed32..f5b0a00 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -1226,6 +1226,58 @@ static int bch_load_bbt(struct nandi_controller *nandi,
 }
 
 /*
+ * MTD Interface: Standard set of callbacks for MTD functionality
+ */
+static int bch_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
+			size_t *retlen, uint8_t *buf)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	int ret;
+
+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
+		(unsigned long long)len, from);
+
+	nand_get_device(mtd, FL_READING);
+
+	ret = bch_read(nandi, from, len, retlen, buf);
+
+	nand_release_device(mtd);
+
+	return ret;
+}
+
+static int bch_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
+			 size_t *retlen, const uint8_t *buf)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	uint32_t page_mask = mtd->writesize - 1;
+	int ret;
+
+	dev_dbg(nandi->dev, "%s: %llu @ 0x%012llx\n", __func__,
+		(unsigned long long)len, to);
+
+	if ((to & page_mask) || (len & page_mask)) {
+		dev_err(nandi->dev, "attempt to write non-page-aligned data\n");
+		return -EINVAL;
+	}
+
+	nand_get_device(mtd, FL_WRITING);
+
+	if (flex_check_wp(nandi)) {
+		dev_dbg(nandi->dev, "device is write-protected\n");
+		return -EIO;
+	}
+
+	ret = bch_write(nandi, to, len, retlen, buf);
+
+	nand_release_device(mtd);
+
+	return ret;
+}
+
+/*
  * Helper function for mtd_read_oob(): handles multi-page transfers
  * and mapping between BCH sectors and MTD page+OOB data.
  */
@@ -1701,6 +1753,8 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	mtd->oobavail = 0;
 	mtd->subpage_sft = 0;
 
+	mtd->_read = bch_mtd_read;
+	mtd->_write = bch_mtd_write;
 	mtd->_erase = bch_mtd_erase;
 	mtd->_read_oob = bch_mtd_read_oob;
 	mtd->_write_oob = bch_mtd_write_oob;
-- 
1.8.3.2

[RFC 45/47] mtd: nand: stm_nand_bch: read and write buffers (FLEX)

[Lee Jones]

read or write in 4 Byte chunks at a time until there is less than
8 Bytes remaining, at which point split down into reading the
reset a single Byte at a time.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 72 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index f5b0a00..975326e 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -686,6 +686,75 @@ static void flex_addr(struct nandi_controller *nandi,
 	writel(addr, nandi->base + NANDHAM_FLEX_ADD);
 }
 
+static void flex_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	int aligned;
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	/* Read bytes until buf is 4-byte aligned */
+	while (len && ((unsigned int)buf & 0x3)) {
+		*buf++ = (uint8_t)(readl(nandi->base + NANDHAM_FLEX_DATA)
+				   & 0xff);
+		len--;
+	};
+
+	/* Use 'BEATS_4'/readsl */
+	if (len > 8) {
+		aligned = len & ~0x3;
+		writel(FLEX_DATA_CFG_BEATS_4 | FLEX_DATA_CFG_CSN,
+		       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+
+		readsl(nandi->base + NANDHAM_FLEX_DATA, buf, aligned >> 2);
+
+		buf += aligned;
+		len -= aligned;
+
+		writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+		       nandi->base + NANDHAM_FLEX_DATAREAD_CONFIG);
+	}
+
+	/* Mop up remaining bytes */
+	while (len > 0) {
+		*buf++ = (uint8_t)(readl(nandi->base + NANDHAM_FLEX_DATA)
+				   & 0xff);
+		len--;
+	}
+}
+
+static void flex_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+	int aligned;
+
+	/* Write bytes until buf is 4-byte aligned */
+	while (len && ((unsigned int)buf & 0x3)) {
+		writel(*buf++, nandi->base + NANDHAM_FLEX_DATA);
+		len--;
+	};
+
+	/* USE 'BEATS_4/writesl */
+	if (len > 8) {
+		aligned = len & ~0x3;
+		writel(FLEX_DATA_CFG_BEATS_4 | FLEX_DATA_CFG_CSN,
+		       nandi->base + NANDHAM_FLEX_DATAWRITE_CONFIG);
+		writesl(nandi->base + NANDHAM_FLEX_DATA, buf, aligned >> 2);
+		buf += aligned;
+		len -= aligned;
+		writel(FLEX_DATA_CFG_BEATS_1 | FLEX_DATA_CFG_CSN,
+		       nandi->base + NANDHAM_FLEX_DATAWRITE_CONFIG);
+	}
+
+	/* Mop up remaining bytes */
+	while (len > 0) {
+		writel(*buf++, nandi->base + NANDHAM_FLEX_DATA);
+		len--;
+	}
+}
+
 /*
  * Hamming-FLEX operations (optimised replacements for nand_base.c versions)
  */
@@ -1743,6 +1812,9 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	chip->priv = nandi;
 	chip->ecc.layout = &info->ecclayout;
 
+	chip->read_buf = flex_read_buf;
+	chip->write_buf = flex_write_buf;
+
 	chip->bbt_options |= NAND_BBT_USE_FLASH;
 
 	/* mtd_info */
-- 
1.8.3.2

[RFC 46/47] mtd: nand: mtd_nand_bch: add remaining FLEX functions

[Lee Jones]

Including a dummy function for selecting chips which isn't supported by
our driver, the wait function as requested by the NAND sub system, a read
Byte function also requested by NAND and finally a command function which
provides a partial implementation of MTD/NAND Interface, based on
Hamming-FLEX operation which allows us to make use of nand_base.c functions
where possible (i.e.nand_scan_ident()).

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 101 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 975326e..4103d4f 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -686,6 +686,103 @@ static void flex_addr(struct nandi_controller *nandi,
 	writel(addr, nandi->base + NANDHAM_FLEX_ADD);
 }
 
+/*
+ * Partial implementation of MTD/NAND Interface, based on Hamming-FLEX
+ * operation.
+ *
+ * Allows us to make use of nand_base.c functions where possible
+ * (e.g. nand_scan_ident() and friends).
+ */
+static void flex_command_lp(struct mtd_info *mtd, unsigned int command,
+			    int column, int page)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	switch (command) {
+	case NAND_CMD_READOOB:
+		/* Emulate NAND_CMD_READOOB */
+		column += mtd->writesize;
+		command = NAND_CMD_READ0;
+		break;
+	case NAND_CMD_READ0:
+	case NAND_CMD_ERASE1:
+	case NAND_CMD_SEQIN:
+	case NAND_CMD_RESET:
+	case NAND_CMD_PARAM:
+		/* Prime RBn wait */
+		nandi_enable_interrupts(nandi, NAND_INT_RBN);
+		reinit_completion(&nandi->rbn_completed);
+		break;
+	case NAND_CMD_READID:
+	case NAND_CMD_STATUS:
+	case NAND_CMD_ERASE2:
+		break;
+	default:
+		/* Catch unexpected commands */
+		BUG();
+	}
+
+	/*
+	 * Command Cycle
+	 */
+	flex_cmd(nandi, command);
+
+	/*
+	 * Address Cycles
+	 */
+	if (column != -1)
+		flex_addr(nandi, column,
+			  (command == NAND_CMD_READID) ? 1 : 2);
+
+	if (page != -1)
+		flex_addr(nandi, page, nandi->extra_addr ? 3 : 2);
+
+	/* Complete 'READ0' command */
+	if (command == NAND_CMD_READ0)
+		flex_cmd(nandi, NAND_CMD_READSTART);
+
+	/* Wait for RBn, if required                        */
+	/* (Note, other commands may handle wait elsewhere) */
+	if (command == NAND_CMD_RESET ||
+	    command == NAND_CMD_READ0 ||
+	    command == NAND_CMD_PARAM) {
+		flex_wait_rbn(nandi);
+		nandi_disable_interrupts(nandi, NAND_INT_RBN);
+	}
+}
+
+static uint8_t flex_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct nandi_controller *nandi = chip->priv;
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	return (uint8_t)(readl(nandi->base + NANDHAM_FLEX_DATA) & 0xff);
+}
+
+static int flex_wait_func(struct mtd_info *mtd, struct nand_chip *chip)
+{
+	struct nandi_controller *nandi = chip->priv;
+
+	emiss_nandi_select(STM_NANDI_HAMMING);
+
+	flex_wait_rbn(nandi);
+
+	flex_cmd(nandi, NAND_CMD_STATUS);
+
+	return (int)(readl(nandi->base + NANDHAM_FLEX_DATA) & 0xff);
+}
+
+/* Multi-CS devices not supported */
+static void flex_select_chip(struct mtd_info *mtd, int chipnr)
+{
+
+}
+
 static void flex_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 {
 	struct nand_chip *chip = mtd->priv;
@@ -1812,6 +1909,10 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	chip->priv = nandi;
 	chip->ecc.layout = &info->ecclayout;
 
+	chip->cmdfunc = flex_command_lp;
+	chip->read_byte = flex_read_byte;
+	chip->select_chip = flex_select_chip;
+	chip->waitfunc = flex_wait_func;
 	chip->read_buf = flex_read_buf;
 	chip->write_buf = flex_write_buf;
 
-- 
1.8.3.2

[RFC 47/47] mtd: nand: stm_nand_bch: catch unsupported calls

[Lee Jones]

Display a BUG() message and return appropriate/expected value.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/nand/stm_nand_bch.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c
index 4103d4f..e732bf3 100644
--- a/drivers/mtd/nand/stm_nand_bch.c
+++ b/drivers/mtd/nand/stm_nand_bch.c
@@ -852,6 +852,31 @@ static void flex_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	}
 }
 
+/* Catch calls to non-supported functions */
+static uint16_t flex_read_word_bug(struct mtd_info *mtd)
+{
+	BUG();
+	return 0;
+}
+
+static int flex_block_bad_bug(struct mtd_info *mtd, loff_t ofs, int getchip)
+{
+	BUG();
+	return 1;
+}
+
+static int flex_block_markbad_bug(struct mtd_info *mtd, loff_t ofs)
+{
+	BUG();
+	return 1;
+}
+
+int flex_scan_bbt_bug(struct mtd_info *mtd)
+{
+	BUG();
+	return 1;
+}
+
 /*
  * Hamming-FLEX operations (optimised replacements for nand_base.c versions)
  */
@@ -1916,7 +1941,12 @@ static void nandi_set_mtd_defaults(struct nandi_controller *nandi,
 	chip->read_buf = flex_read_buf;
 	chip->write_buf = flex_write_buf;
 
+	chip->read_word = flex_read_word_bug;
+	chip->block_bad = flex_block_bad_bug;
+	chip->block_markbad = flex_block_markbad_bug;
+
 	chip->bbt_options |= NAND_BBT_USE_FLASH;
+	chip->scan_bbt = flex_scan_bbt_bug;
 
 	/* mtd_info */
 	mtd->owner = THIS_MODULE;
-- 
1.8.3.2

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Ezequiel Garcia]

Hi Lee,

On Mar 25, Lee Jones wrote:
> 
> This patch-set was written against kernel version v3.4, but applies
> cleanly to v3.13-rc7 which it is currently based. I understand that
> the core has changed significantly since and that some of the
> infrastructure this driver provides is now available either through
> the framework or similar means. I'm looking for someone with inside
> knowledge to draw attention to those areas so we can align with the
> most recent API.
> 

Please push a git branch. It will be nice to have a way of seeing the 'big
picture' without applying the 47 patches.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Lee Jones]

On Tue, 25 Mar 2014, Ezequiel Garcia wrote:

> Hi Lee,
> 
> On Mar 25, Lee Jones wrote:
> > 
> > This patch-set was written against kernel version v3.4, but applies
> > cleanly to v3.13-rc7 which it is currently based. I understand that
> > the core has changed significantly since and that some of the
> > infrastructure this driver provides is now available either through
> > the framework or similar means. I'm looking for someone with inside
> > knowledge to draw attention to those areas so we can align with the
> > most recent API.
> > 
> 
> Please push a git branch. It will be nice to have a way of seeing the 'big
> picture' without applying the 47 patches.

Sure:

  git://git.linaro.org/people/lee.jones/linux-3.0-ux500.git tb-st-mtd-nand-bch

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Ezequiel Garcia]

On Mar 25, Lee Jones wrote:
> On Tue, 25 Mar 2014, Ezequiel Garcia wrote:
> 
> > Hi Lee,
> > 
> > On Mar 25, Lee Jones wrote:
> > > 
> > > This patch-set was written against kernel version v3.4, but applies
> > > cleanly to v3.13-rc7 which it is currently based. I understand that
> > > the core has changed significantly since and that some of the
> > > infrastructure this driver provides is now available either through
> > > the framework or similar means. I'm looking for someone with inside
> > > knowledge to draw attention to those areas so we can align with the
> > > most recent API.
> > > 
> > 
> > Please push a git branch. It will be nice to have a way of seeing the 'big
> > picture' without applying the 47 patches.
[..]
> 
>   git://git.linaro.org/people/lee.jones/linux-3.0-ux500.git tb-st-mtd-nand-bch
> 

Lee,

After taking a quick glance at the whole driver I noticed you have something
strange going on. AFAIK, the typical NAND driver probe() should be one of
these two:

* Call nand_scan() which calls nand_scan_ident() + nand_scan_tail().

* Call nand_scan_ident() to identify the NAND device geometry, do some
  driver specific initialization, fill some hooks, and finally call
  nand_scan_tail() to complete the initialization.

You driver call nand_scan_ident() and then does some bad block scan, and
fills some callbacks on its own, but never calls nand_scan_tail().

The call to nand_scan_tail() would remove the need to export those NAND
core functions, and remove the need to scan and print the bad blocks.
I don't know if you have a real reason for not doing it this way, or
maybe it's the way this driver was originally written.

Care to review this and re-spin the driver? You'll have a more nicer
driver, and more framework-compliant.

Also, if you plan to target v3.16 on this, I'd suggest that you pick
some pack of features and submit those first, reducing the amount of code
to be reviewed. For instance, you may choose to leave some of the ECC bits
aside for now.

It's just a suggestion to get at least some of the code merged quicker,
don't take me too seriously on this.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Brian Norris]

On Tue, Mar 25, 2014 at 07:00:45PM -0300, Ezequiel Garcia wrote:
> After taking a quick glance at the whole driver I noticed you have something
> strange going on. AFAIK, the typical NAND driver probe() should be one of
> these two:
> 
> * Call nand_scan() which calls nand_scan_ident() + nand_scan_tail().
> 
> * Call nand_scan_ident() to identify the NAND device geometry, do some
>   driver specific initialization, fill some hooks, and finally call
>   nand_scan_tail() to complete the initialization.
> 
> You driver call nand_scan_ident() and then does some bad block scan, and
> fills some callbacks on its own, but never calls nand_scan_tail().
> 
> The call to nand_scan_tail() would remove the need to export those NAND
> core functions, and remove the need to scan and print the bad blocks.
> I don't know if you have a real reason for not doing it this way, or
> maybe it's the way this driver was originally written.
> 
> Care to review this and re-spin the driver? You'll have a more nicer
> driver, and more framework-compliant.

A hearty +1 to this. You are avoiding much of the core of the NAND
framework by avoiding the nand_chip callbacks and nand_scan_tail(), and
by reimplementing the BBT. I will have to NAK to some of the patches
that EXPORT the nand_base private core (e.g., nand_get_device()), and I
will most likely NAK the custom BBT implementation (please improve
nand_bbt.c as needed).

> Also, if you plan to target v3.16 on this, I'd suggest that you pick
> some pack of features and submit those first, reducing the amount of code
> to be reviewed. For instance, you may choose to leave some of the ECC bits
> aside for now.
> 
> It's just a suggestion to get at least some of the code merged quicker,
> don't take me too seriously on this.

That's a possible approach if it still leaves your driver functional.
But I wouldn't trim the driver too much just for sake of reviewing.

BTW, why do you call this driver stm_nand_bch? BCH is a particular type
of ECC algorithm, not unique at all to ST's hardware. Can you drop the
_bch and make it just stm_nand? Also, you might want to change the
namespacing on some of your functions; for instance, I don't think you
can own the name bch_write(). Possibly prefix things with stm_* or
stm_nand_* where reasonable.

Brian

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Lee Jones]

> > After taking a quick glance at the whole driver I noticed you have something
> > strange going on. AFAIK, the typical NAND driver probe() should be one of
> > these two:
> > 
> > * Call nand_scan() which calls nand_scan_ident() + nand_scan_tail().
> > 
> > * Call nand_scan_ident() to identify the NAND device geometry, do some
> >   driver specific initialization, fill some hooks, and finally call
> >   nand_scan_tail() to complete the initialization.
> > 
> > You driver call nand_scan_ident() and then does some bad block scan, and
> > fills some callbacks on its own, but never calls nand_scan_tail().
> > 
> > The call to nand_scan_tail() would remove the need to export those NAND
> > core functions, and remove the need to scan and print the bad blocks.
> > I don't know if you have a real reason for not doing it this way, or
> > maybe it's the way this driver was originally written.
> > 
> > Care to review this and re-spin the driver? You'll have a more nicer
> > driver, and more framework-compliant.
> 
> A hearty +1 to this. You are avoiding much of the core of the NAND
> framework by avoiding the nand_chip callbacks and nand_scan_tail(), and
> by reimplementing the BBT. I will have to NAK to some of the patches
> that EXPORT the nand_base private core (e.g., nand_get_device()), and I
> will most likely NAK the custom BBT implementation (please improve
> nand_bbt.c as needed).

This is a good catch. I will attempt to reimplement the driver's
initialisation steps to utilise more of the core infrastructure in an
attempt to mitigate the requirement for exportation of private
routines.

The BBT requirements are somewhere more complex. To provide you with
the complete picture, a little knowledge of driver history is
required. When it was initially created the MTD core only supported
OOB BBTs, but the ST BCH Controller doesn't support OOB access, so
Angus wrote his on In-Band (IB) implementation. Unfortunately the IB
support which _is_ now present in the kernel doesn't match the
internal implementation. Normally this wouldn't be an issue in itself,
but ST's boot-stack and tooling (Primary Bootloader, U-Boot, various
Programmers, etc) are aware of the internal IB BTT and utilise it
in varying ways. Shifting over to the Mainline version in
one-foul-swoop _will_ cause lots of pain and will probably result in
the disownership of driver we're trying to Mainline today. Naturally
I'm keen to avoid this.

> > Also, if you plan to target v3.16 on this, I'd suggest that you pick
> > some pack of features and submit those first, reducing the amount of code
> > to be reviewed. For instance, you may choose to leave some of the ECC bits
> > aside for now.
> > 
> > It's just a suggestion to get at least some of the code merged quicker,
> > don't take me too seriously on this.
> 
> That's a possible approach if it still leaves your driver functional.
> But I wouldn't trim the driver too much just for sake of reviewing.
> 
> BTW, why do you call this driver stm_nand_bch? BCH is a particular type
> of ECC algorithm, not unique at all to ST's hardware. Can you drop the
> _bch and make it just stm_nand?

>From my knowledge (Angus feel free to jump in anywhere you like), ST
have 4 NAND drivers. This is just one of them. The others are EMI,
Flex and Advanced Flex (AFM). This particular controller is described
as the BCH throughout ST's documentation. Much of this documentation is
available freely online [1].

> Also, you might want to change the
> namespacing on some of your functions; for instance, I don't think you
> can own the name bch_write(). Possibly prefix things with stm_* or
> stm_nand_* where reasonable.

Yes, absolutely.

[1] http://www.stlinux.com/howto/NAND

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Lee Jones]

> > > The call to nand_scan_tail() would remove the need to export those NAND
> > > core functions, and remove the need to scan and print the bad blocks.
> > > I don't know if you have a real reason for not doing it this way, or
> > > maybe it's the way this driver was originally written.
> > > 
> > > Care to review this and re-spin the driver? You'll have a more nicer
> > > driver, and more framework-compliant.
> > 
> > A hearty +1 to this. You are avoiding much of the core of the NAND
> > framework by avoiding the nand_chip callbacks and nand_scan_tail(), and
> > by reimplementing the BBT. I will have to NAK to some of the patches
> > that EXPORT the nand_base private core (e.g., nand_get_device()), and I
> > will most likely NAK the custom BBT implementation (please improve
> > nand_bbt.c as needed).
> 
> This is a good catch. I will attempt to reimplement the driver's
> initialisation steps to utilise more of the core infrastructure in an
> attempt to mitigate the requirement for exportation of private
> routines.
> 
> The BBT requirements are somewhere more complex. To provide you with
> the complete picture, a little knowledge of driver history is
> required. When it was initially created the MTD core only supported
> OOB BBTs, but the ST BCH Controller doesn't support OOB access, so
> Angus wrote his on In-Band (IB) implementation. Unfortunately the IB
> support which _is_ now present in the kernel doesn't match the
> internal implementation. Normally this wouldn't be an issue in itself,
> but ST's boot-stack and tooling (Primary Bootloader, U-Boot, various
> Programmers, etc) are aware of the internal IB BTT and utilise it
> in varying ways. Shifting over to the Mainline version in
> one-foul-swoop _will_ cause lots of pain and will probably result in
> the disownership of driver we're trying to Mainline today. Naturally
> I'm keen to avoid this.

Just looking into this now. Can I add support for a vendor specific
signature extension? ST's flashers, bootloaders and tooling currently
use the format:

/* Extend IBBT header with some stm-nand-bch niceties */
struct nand_ibbt_bch_header {
	uint8_t signature[4];           /* "Bbt0" or "1tbB" signature */
	uint8_t version;                /* BBT version ("age") */
	uint8_t reserved[3];            /* padding */
	uint8_t baseschema[4];          /* "base" schema (x4) */
	uint8_t privschema[4];          /* "private" schema (x4) */
	uint8_t ecc_size[4];            /* ECC bytes (0, 32, 54) (x4) */
	char    author[64];             /* Arbitrary string for S/W to use */
}; __attribute__((__packed__))

It would be great if we can support this with a descriptor option or
suchlike, as it would a) save me a lot of aggravation and b) continue
to support ST with their current use-case.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Brian Norris]

On Tue, Apr 01, 2014 at 12:29:26PM +0100, Lee Jones wrote:
> > The BBT requirements are somewhere more complex. To provide you with
> > the complete picture, a little knowledge of driver history is
> > required. When it was initially created the MTD core only supported
> > OOB BBTs, but the ST BCH Controller doesn't support OOB access, so
> > Angus wrote his on In-Band (IB) implementation. Unfortunately the IB
> > support which _is_ now present in the kernel doesn't match the
> > internal implementation. Normally this wouldn't be an issue in itself,
> > but ST's boot-stack and tooling (Primary Bootloader, U-Boot, various
> > Programmers, etc) are aware of the internal IB BTT and utilise it
> > in varying ways. Shifting over to the Mainline version in
> > one-foul-swoop _will_ cause lots of pain and will probably result in
> > the disownership of driver we're trying to Mainline today. Naturally
> > I'm keen to avoid this.
> 
> Just looking into this now. Can I add support for a vendor specific
> signature extension? ST's flashers, bootloaders and tooling currently
> use the format:
> 
> /* Extend IBBT header with some stm-nand-bch niceties */
> struct nand_ibbt_bch_header {
> 	uint8_t signature[4];           /* "Bbt0" or "1tbB" signature */
> 	uint8_t version;                /* BBT version ("age") */
> 	uint8_t reserved[3];            /* padding */
> 	uint8_t baseschema[4];          /* "base" schema (x4) */
> 	uint8_t privschema[4];          /* "private" schema (x4) */

Not sure what these schema mean.

> 	uint8_t ecc_size[4];            /* ECC bytes (0, 32, 54) (x4) */
> 	char    author[64];             /* Arbitrary string for S/W to use */
> }; __attribute__((__packed__))

Nit: that would just be __packed (see compiler-gcc.h).

In principle, I'm OK with extending the BBT somewhat. Preferably, this
would provide some extensibility, so that other custom formats can use
the same base code. For instance, it looks like many of these fields
would be fixed, and specific to your platform. So (from nand_bbt's
perspective) these could just be consolidated int a field:

	u8 custom[76];

Or make it variable-length, with the length provided by the driver?
nand_bbt would just know not to check for it when scanning, and it
would know to program it to flash when updating.

> It would be great if we can support this with a descriptor option or
> suchlike, as it would a) save me a lot of aggravation and b) continue
> to support ST with their current use-case.

Yeah, I realize you can't just jump over to the current format for
production systems. And there are admittedly some rough spots in our
current nand_bbt; it's not perfect.

Some random notes (not necessarily your problem; but things to be aware
of): nand_bbt could use some additional robustness checks, I think. Like
a CRC field, and maybe a versioning system for allowing
(backwards-compatible) changes in the format. To make
backwards-compatible changes, though, the original format needs to have
reserved space, or at least an 'offset' field, which would point to
where the actual BBT starts--not just a fixed offset.

There's also still a bit of cruft that really can be removed from
nand_bbt (the handling of bad block markers, which is duplicated in
nand_base). It's a low-priority item on my plate, but I think it might
be a good first step before trying to expand nand_bbt much.

Brian

Re: [RFC 00/47] mtd: nand: Add new driver supporting ST's BCH h/w

[Lee Jones]

> > > The BBT requirements are somewhere more complex. To provide you with
> > > the complete picture, a little knowledge of driver history is
> > > required. When it was initially created the MTD core only supported
> > > OOB BBTs, but the ST BCH Controller doesn't support OOB access, so
> > > Angus wrote his on In-Band (IB) implementation. Unfortunately the IB
> > > support which _is_ now present in the kernel doesn't match the
> > > internal implementation. Normally this wouldn't be an issue in itself,
> > > but ST's boot-stack and tooling (Primary Bootloader, U-Boot, various
> > > Programmers, etc) are aware of the internal IB BTT and utilise it
> > > in varying ways. Shifting over to the Mainline version in
> > > one-foul-swoop _will_ cause lots of pain and will probably result in
> > > the disownership of driver we're trying to Mainline today. Naturally
> > > I'm keen to avoid this.
> > 
> > Just looking into this now. Can I add support for a vendor specific
> > signature extension? ST's flashers, bootloaders and tooling currently
> > use the format:
> > 
> > /* Extend IBBT header with some stm-nand-bch niceties */
> > struct nand_ibbt_bch_header {
> > 	uint8_t signature[4];           /* "Bbt0" or "1tbB" signature */
> > 	uint8_t version;                /* BBT version ("age") */
> > 	uint8_t reserved[3];            /* padding */
> > 	uint8_t baseschema[4];          /* "base" schema (x4) */
> > 	uint8_t privschema[4];          /* "private" schema (x4) */
> 
> Not sure what these schema mean.

To be honest, me either, but I know that they are used by ST's
tooling; flashers, bootloaders and debuggers.

> > 	uint8_t ecc_size[4];            /* ECC bytes (0, 32, 54) (x4) */
> > 	char    author[64];             /* Arbitrary string for S/W to use */
> > }; __attribute__((__packed__))
> 
> Nit: that would just be __packed (see compiler-gcc.h).

Okay.

> In principle, I'm OK with extending the BBT somewhat. Preferably, this
> would provide some extensibility, so that other custom formats can use
> the same base code. For instance, it looks like many of these fields
> would be fixed, and specific to your platform. So (from nand_bbt's
> perspective) these could just be consolidated int a field:
> 
> 	u8 custom[76];
> 
> Or make it variable-length, with the length provided by the driver?
> nand_bbt would just know not to check for it when scanning, and it
> would know to program it to flash when updating.
> 
> > It would be great if we can support this with a descriptor option or
> > suchlike, as it would a) save me a lot of aggravation and b) continue
> > to support ST with their current use-case.
> 
> Yeah, I realize you can't just jump over to the current format for
> production systems. And there are admittedly some rough spots in our
> current nand_bbt; it's not perfect.
> 
> Some random notes (not necessarily your problem; but things to be aware
> of): nand_bbt could use some additional robustness checks, I think. Like
> a CRC field, and maybe a versioning system for allowing
> (backwards-compatible) changes in the format. To make
> backwards-compatible changes, though, the original format needs to have
> reserved space, or at least an 'offset' field, which would point to
> where the actual BBT starts--not just a fixed offset.
> 
> There's also still a bit of cruft that really can be removed from
> nand_bbt (the handling of bad block markers, which is duplicated in
> nand_base). It's a low-priority item on my plate, but I think it might
> be a good first step before trying to expand nand_bbt much.

So I've been looking into the differences between the Mainline and
ST's implementation.  I've concluded that the transition over to
Mainline's version is best dealt with in the device driver.

I have depicted the differences between the two versions at [1].
You'll see that the BBT header and BBT data have been separated into
different pages the ST version.  This hardens the process against
power failures whilst creating the BBT by only writing the pattern
once the BBT data has been successfully applied to flash.  A small
corner-case perhaps, but these things do happen.

What I'd like to do is supply our own scan_bbt() call.  This is
supported by the framework already, so no extra modifications are
required.  Converting nand_bbt to add support for our (and other)
formats would be fairly intrusive.  The framework already allows us to
search from the last block backwards, which is great, but that same
functionality is missing for searching from the last page in the
block.  The ability to separate header from data is also vacant,
which we require for the aforementioned reasons.

In our scan_bbt() I would like to start off by only supporting solely
the ST schema, then over time search in both locations but preferring
ST's.  Subsequently the choice should be version number based, with a
view to phasing out ST's implementation completely once we've had a
change to adapt the tooling which currently use only ST's
implementation.

I'm hoping to submit the next version either today or tomorrow, which
will conform to the proposal above.  I hope that you find this
adequate.

[1] goo.gl/WnGLVQ

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog