Re: [PATCH] mtd: nand: stm_nand_bch: add new driver

[Lee Jones]

Finally getting round to this ...

On Wed, 02 Jul 2014, Brian Norris wrote:
> On Wed, May 28, 2014 at 10:20:05AM +0100, Lee Jones wrote:
> > This is a squashed version of the submission to avoid re-sending the
> > entire set over and over, essentially clogging up the MLs.
> 
> Thanks. I think I'd prefer to accept your driver in a form like this
> too.

You mean squashed?  I'd _really_ like _not_ to do that.  We've spoken
about this in reasonable depth before.  I agreed to squash it for this
review to keep the submissions nice and light, but we did have an
agreement that once accepted the final delivery could be via
pull-request.  I'd really like it if you honoured that.

> A few comments below.
> And I'll get one big comment out of the way here: can you abstract your
> ST BBT code into its own self-contained portion, preferably in a
> separate source file, a la nand_bbt.c? Then, provide a way to optionally
> use either your ST BBT or the existing BBT -- perhaps a NAND_BBT_ST flag
> for chip->bbt_options, and a matching device tree property. That way,
> even though you require a legacy format for bootloader interoperability,
> someone can theoretically utilize more mainstream (albeit, not
> necessarily better...) BBT support from nand_bbt.c. I think this will
> provide the best balance between your existing product support and
> upstream-friendly modularity/flexibility. I'm open to other suggestions,
> of course.

Sounds reasonable.  I can pull the vendor specific (rather than
legacy :) ) BBT handling out and protect it with a CONFIG option.

> > Cc: computersforpeace@xxxxxxxxx
> > Cc: Gupta, Pekon" <pekon@xxxxxx>
> > Cc: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
> > Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
> > ---
> 
> Please add versioning to your next patch(es), and describe changes here.

Certainly.

> >  Documentation/devicetree/bindings/mtd/stm-nand.txt |   87 +
> 
> See:
> 
>   Documentation/devicetree/bindings/submitting-patches.txt

This file doesn't exist.  What are you referencing?

> You're missing devicetree@xxxxxxxxxxxxxxx on CC, and the binding doc
> needs a separate patch. (Sorry if I confused this one earlier.)

The binding document will certainly be a separate patch.  This patch
is the 'everything squashed' version which was requested.

> >  arch/arm/boot/dts/stih41x-b2020.dtsi               |   40 +
> 
> This will need refreshed and sent as a separate patch, to go through the
> appropriate ARM tree.

As above.

> >  drivers/mtd/nand/Kconfig                           |   14 +
> >  drivers/mtd/nand/Makefile                          |    2 +
> >  drivers/mtd/nand/stm_nand_bch.c                    | 2415 ++++++++++++++++++++
> >  drivers/mtd/nand/stm_nand_dt.c                     |  116 +
> >  drivers/mtd/nand/stm_nand_dt.h                     |   39 +
> >  drivers/mtd/nand/stm_nand_regs.h                   |  302 +++
> >  include/linux/mtd/stm_nand.h                       |  104 +
> >  9 files changed, 3119 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/stm-nand.txt
> >  create mode 100644 drivers/mtd/nand/stm_nand_bch.c
> >  create mode 100644 drivers/mtd/nand/stm_nand_dt.c
> >  create mode 100644 drivers/mtd/nand/stm_nand_dt.h
> >  create mode 100644 drivers/mtd/nand/stm_nand_regs.h
> >  create mode 100644 include/linux/mtd/stm_nand.h
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/stm-nand.txt b/Documentation/devicetree/bindings/mtd/stm-nand.txt
> > new file mode 100644
> > index 0000000..d957f49
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/stm-nand.txt
> > @@ -0,0 +1,87 @@
> > +STM BCH NAND Support
> > +--------------------
> > +
> > +Required properties:
> > +
> > +- compatible		: Should be "st,nand-bch"
> > +- reg			: Should contain register's location and length
> > +- reg-names		: "nand_mem" - NAND Controller register map
> > +			  "nand_dma" - BCH Controller DMA configuration map
> > +- interrupts		: Interrupt number
> > +- interrupt-names	: "nand_irq" - NAND Controller IRQ
> > +- st,nand-banks		: Subnode representing one or more "banks" of NAND
> > +			  Flash, connected to an STM NAND Controller (see
> > +			  description below).
> > +- nand-ecc-strength	: Generic NAND property (See mtd/nand.txt)
> > +			  Options are; 0, 18, 30 or 0xFF (AUTO)
> 
> What is 0xFF (AUTO)?
> 
> > +
> > +Properties describing Bank of NAND Flash ("st,nand-banks"):
> > +
> > +- st,nand-csn		: Chip select associated with the Bank.
> > +
> > +- st,nand-timing-relax	: [Optional] Number of IP clock cycles by which to
> > +			  "relax" timing configuration.  Required on some boards
> > +			  to accommodate board-level limitations. Applies to
> > +			  ONFI timing mode configuration.
> > +
> > +- nand-on-flash-bbt	: Generic NAND property (See mtd/nand.txt)
> > +
> > +- partitions		: [Optional] Subnode describing MTD partition map
> > +			  (see mtd/partition.txt)
> > +
> > +Note, during initialisation, the NAND Controller timing registers are configured
> > +according to one of the following methods, in order of precedence:
> > +
> > +	   1. Configuration based on ONFI timing mode, as advertised by the
> > +	      device during ONFI-probing (ONFI-compliant NAND only).
> > +
> > +	   2. Use reset/safe timing values
> > +
> > +Example:
> > +
> > +	nandbch: nand-bch {
> > +		compatible = "st,nand-bch";
> > +		reg = <0xfe901000 0x1000>, <0xfef00800 0x0800>;
> > +		reg-names = "nand_mem", "nand_dma";
> > +		interrupts = <0 139 0x0>;
> > +		interrupt-names = "nand_irq";
> > +		nand-ecc-strength = <30>;
> > +		st,nand-banks = <&nand_banks>;
> 
> Is there a good reason to be a phandle, vs. just a subnode? I think
> a subnode might describe the parent/child relationship a little better.

I think it can be a child node.  Will see to it.

> > +		status = "okay";
> > +	};
> > +
> > +	nand_banks: nand-banks {
> > +		bank0 {
> > +			/* NAND_BBT_USE_FLASH */
> > +			nand-on-flash-bbt;
> > +			st,nand-csn		= <0>;
> > +			st,nand-timing-data	= <&nand_timing0>;
> 
> Where is this property documented?

I think it's legacy and can actually be removed altogether.

> > +
> > +			partitions {
> > +				#address-cells = <1>;
> > +				#size-cells = <1>;
> > +
> > +				partition@0{
> > +					label = "NAND Flash 1";
> 
> Do you really want spaces in your partition names?

No clue to be honest.  What are the known ramifications?

> > +					reg = <0x00000000 0x00800000>;
> > +				};
> > +				partition@800000{
> > +					label = "NAND Flash 2";
> > +					reg = <0x00800000 0x0F800000>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> > +	nand_timing0: nand-timing {
> > +		sig-setup	= <10>;
> > +		sig-hold	= <10>;
> > +		CE-deassert	= <0>;
> > +		WE-to-RBn	= <100>;
> > +		wr-on		= <10>;
> > +		wr-off		= <30>;
> > +		rd-on		= <10>;
> > +		rd-off		= <30>;
> > +		chip-delay	= <30>;		/* delay in us */
> > +	};
> 
> You didn't document any of this node. And I don't think we want to
> specify every single timing parameter in DT; it may make sense to use
> Boris Brezillon's approach (I note this further down, in the driver
> code) for mapping non-ONFI NAND timings into a compatible ONFI timing
> mode. This will greatly simplify the bindings needed, since it's
> standardized and auto-detectable in many cases.

It doesn't appear to be used at all - will check my previous versions
and figure something out.

> > diff --git a/arch/arm/boot/dts/stih41x-b2020.dtsi b/arch/arm/boot/dts/stih41x-b2020.dtsi
> > index bc5818d..7a6a6e8 100644
> > --- a/arch/arm/boot/dts/stih41x-b2020.dtsi
> > +++ b/arch/arm/boot/dts/stih41x-b2020.dtsi
> > @@ -52,5 +52,45 @@
> >  			pinctrl-0	= <&pinctrl_rgmii1>;
> >  		};
> >  
> > +		nandbch: nand-bch {
> 
> Shouldn't this be named 'nand@xxxxxxxx', 'flash@xxxxxxxx', or similar?

Yes, will fix.

> > +			compatible = "st,nand-bch";
> > +			reg = <0xfe901000 0x1000>, <0xfef00800 0x0800>;
> > +			reg-names = "nand_mem", "nand_dma";
> > +			interrupts = <0 139 0x0>;
> > +			interrupt-names = "nand_irq";
> > +			st,nand-banks = <&nand_banks>;
> > +			nand-ecc-strength = <0xFF>;
> > +
> > +			status = "okay";
> > +		};
> > +
> > +		nand_banks: nand-banks {
> > +			/*
> > +			 * Micron MT29F8G08ABABAWP:
> > +			 *  - Size = 8Gib(1GiB); Page = 4096+224; Block = 512KiB
> > +			 *  - ECC = 4-bit/540B min
> > +			 *  - ONFI 2.1 (timing parameters retrieved during probe)
> > +			 */
> > +			bank0 {
> > +				nand-on-flash-bbt;
> > +				st,nand-csn		= <0>;
> > +				st,nand-timing-relax	= <0>;
> > +
> > +				partitions {
> > +					#address-cells	= <1>;
> > +					#size-cells	= <1>;
> > +					partition@0 {
> > +						/* 8MB */
> > +						label = "NAND Flash 1";
> > +						reg = <0x00000000 0x00800000>;
> > +					};
> > +					partition@800000 {
> > +						/* 1GB - 8MB */
> > +						label = "NAND Flash 2";
> > +						reg = <0x00800000 0x1F000000>;
> > +					};
> > +				};
> > +			};
> > +		};
> >  	};
> >  };
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index 93ae6a6..119aed5 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -510,4 +510,18 @@ 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
> > +	depends on OF
> > +	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
> 
> Do you really need a second Kconfig symbol? Separate C files is OK, if
> it provides good separation, but since MTD_NAND_STM_BCH is already
> dependent on CONFIG_OF, would you ever really want to build the main
> driver without the DT support? Or maybe MTD_NAND_STM_BCH shouldn't
> depend on CONFIG_OF?

Again, this is legacy stuff.  Our platform is now DT only, so the
extra Kconfig entry can be removed.

> > +	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 542b568..890f47f 100644
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > @@ -46,6 +46,8 @@ 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_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
> > new file mode 100644
> > index 0000000..5ad78ce
> > --- /dev/null
> > +++ b/drivers/mtd/nand/stm_nand_bch.c
> > @@ -0,0 +1,2415 @@
> > +/*
> > + * drivers/mtd/nand/stm_nand_bch.c
> 
> This line's a bit redundant and prone to error if things are renamed.
> Drop it? (And ditto for the other files?)

It's quite common to list the filename at the top of a file.
Although, mentioning its full patch is a little unorthodox, will fix.

> > + *
> > + * Support for STMicroelectronics NANDi BCH Controller
> > + *
> > + * Copyright (c) 2014 STMicroelectronics Limited
> > + * Author: Angus Clark <Angus.Clark@xxxxxx>
> > + *
> > + * 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/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/clk.h>
> > +#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>
> > +#include <linux/mtd/partitions.h>
> > +#include <generated/utsrelease.h>
> > +
> > +#include "stm_nand_regs.h"
> > +#include "stm_nand_dt.h"
> > +
> > +/* 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[] = {
> > +	[BCH_18BIT_ECC] = 32,
> > +	[BCH_30BIT_ECC] = 54,
> > +	[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)
> > + */
> > +#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'},
> > +};
> > +
> > +static char *bbt_strs[] = {
> > +	"primary",
> > +	"mirror",
> > +};
> > +
> > +/* 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 */
> > +	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*/
> > +	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 */
> > +
> > +	struct device		*dev;
> > +
> > +	int			bch_ecc_mode;	/* ECC mode */
> > +	bool			extra_addr;	/* Extra address cycle */
> > +
> > +	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'             */
> > +
> > +	struct nandi_info	info;		/* NAND device info */
> > +};
> > +
> > +/* ONFI define 6 timing modes */
> > +#define ST_NAND_ONFI_TIMING_MODES		6
> > +
> > +/*
> > + * ONFI NAND Timing Mode Specifications
> > + *
> > + * Note, 'tR' field (maximum page read time) is extracted from the ONFI
> > + * parameter page during device probe.
> > + */
> > +const struct nand_sdr_timings st_nand_onfi_timing_specs[] = {
> 
> Did you ever take a look at Boris Brezillon's timing patch series? He
> hasn't submitted an update recently, but it was looking good:
> 
>   http://article.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/7976
> 
> Perhaps you could even modify/test/resubmit it (it's got a GPL Sign-off,
> after all!). I'd prefer not to stick this stuff in your driver.

Not yet.  Will do that now.

> > +	/*
> > +	 * ONFI Timing Mode '0' (supported on all ONFI compliant devices)
> > +	 */
> > +	[0] = {
> > +		.tCLS_min	= 50,
> > +		.tCS_min	= 70,
> > +		.tALS_min	= 50,
> > +		.tDS_min	= 40,
> > +		.tWP_min	= 50,
> > +		.tCLH_min	= 20,
> > +		.tCH_min	= 20,
> > +		.tALH_min	= 20,
> > +		.tDH_min	= 20,
> > +		.tWB_max	= 200,
> > +		.tWH_min	= 30,
> > +		.tWC_min	= 100,
> > +		.tRP_min	= 50,
> > +		.tREH_min	= 30,
> > +		.tRC_min	= 100,
> > +		.tREA_max	= 40,
> > +		.tRHOH_min	= 0,
> > +		.tCEA_max	= 100,
> > +		.tCOH_min	= 0,
> > +		.tCHZ_max	= 100,
> > +	},
> > +
> > +	/*
> > +	 * ONFI Timing Mode '1'
> > +	 */
> > +	[1] = {
> > +		.tCLS_min	= 25,
> > +		.tCS_min	= 35,
> > +		.tALS_min	= 25,
> > +		.tDS_min	= 20,
> > +		.tWP_min	= 25,
> > +		.tCLH_min	= 10,
> > +		.tCH_min	= 10,
> > +		.tALH_min	= 10,
> > +		.tDH_min	= 10,
> > +		.tWB_max	= 100,
> > +		.tWH_min	= 15,
> > +		.tWC_min	= 45,
> > +		.tRP_min	= 25,
> > +		.tREH_min	= 15,
> > +		.tRC_min	= 50,
> > +		.tREA_max	= 30,
> > +		.tRHOH_min	= 15,
> > +		.tCEA_max	= 45,
> > +		.tCOH_min	= 15,
> > +		.tCHZ_max	= 50,
> > +	},
> > +
> > +	/*
> > +	 * ONFI Timing Mode '2'
> > +	 */
> > +	[2] = {
> > +		.tCLS_min	= 15,
> > +		.tCS_min	= 25,
> > +		.tALS_min	= 15,
> > +		.tDS_min	= 15,
> > +		.tWP_min	= 17,
> > +		.tCLH_min	= 10,
> > +		.tCH_min	= 10,
> > +		.tALH_min	= 10,
> > +		.tDH_min	= 5,
> > +		.tWB_max	= 100,
> > +		.tWH_min	= 15,
> > +		.tWC_min	= 35,
> > +		.tRP_min	= 17,
> > +		.tREH_min	= 16,
> > +		.tRC_min	= 35,
> > +		.tREA_max	= 25,
> > +		.tRHOH_min	= 15,
> > +		.tCEA_max	= 30,
> > +		.tCOH_min	= 15,
> > +		.tCHZ_max	= 50,
> > +	},
> > +
> > +	/*
> > +	 * ONFI Timing Mode '3'
> > +	 */
> > +	[3] = {
> > +		.tCLS_min	= 10,
> > +		.tCS_min	= 25,
> > +		.tALS_min	= 10,
> > +		.tDS_min	= 10,
> > +		.tWP_min	= 15,
> > +		.tCLH_min	= 5,
> > +		.tCH_min	= 5,
> > +		.tALH_min	= 5,
> > +		.tDH_min	= 5,
> > +		.tWB_max	= 100,
> > +		.tWH_min	= 10,
> > +		.tWC_min	= 30,
> > +		.tRP_min	= 15,
> > +		.tREH_min	= 10,
> > +		.tRC_min	= 30,
> > +		.tREA_max	= 20,
> > +		.tRHOH_min	= 15,
> > +		.tCEA_max	= 25,
> > +		.tCOH_min	= 15,
> > +		.tCHZ_max	= 50,
> > +	},
> > +
> > +	/*
> > +	 * ONFI Timing Mode '4' (EDO only)
> > +	 */
> > +	[4] = {
> > +		.tCLS_min	= 10,
> > +		.tCS_min	= 20,
> > +		.tALS_min	= 10,
> > +		.tDS_min	= 10,
> > +		.tWP_min	= 12,
> > +		.tCLH_min	= 5,
> > +		.tCH_min	= 5,
> > +		.tALH_min	= 5,
> > +		.tDH_min	= 5,
> > +		.tWB_max	= 100,
> > +		.tWH_min	= 10,
> > +		.tWC_min	= 25,
> > +		.tRP_min	= 12,
> > +		.tREH_min	= 10,
> > +		.tRC_min	= 25,
> > +		.tREA_max	= 20,
> > +		.tRHOH_min	= 15,
> > +		.tCEA_max	= 25,
> > +		.tCOH_min	= 15,
> > +		.tCHZ_max	= 30,
> > +	},
> > +
> > +	/*
> > +	 * ONFI Timing Mode '5' (EDO only)
> > +	 */
> > +	[5] = {
> > +		.tCLS_min	= 10,
> > +		.tCS_min	= 15,
> > +		.tALS_min	= 10,
> > +		.tDS_min	= 7,
> > +		.tWP_min	= 10,
> > +		.tCLH_min	= 5,
> > +		.tCH_min	= 5,
> > +		.tALH_min	= 5,
> > +		.tDH_min	= 5,
> > +		.tWB_max	= 100,
> > +		.tWH_min	= 7,
> > +		.tWC_min	= 20,
> > +		.tRP_min	= 10,
> > +		.tREH_min	= 7,
> > +		.tRC_min	= 20,
> > +		.tREA_max	= 16,
> > +		.tRHOH_min	= 15,
> > +		.tCEA_max	= 25,
> > +		.tCOH_min	= 15,
> > +		.tCHZ_max	= 30,
> > +	}
> > +};
> > +
> > +/* 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)
> > + */
> > +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);
> > +}
> > +
> > +/*
> > + * 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 nand_chip *chip = &nandi->info.chip;
> > +	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 >> chip->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;
> > +}
> > +
> > +static int bch_erase(struct mtd_info *mtd, int page)
> > +{
> > +	struct nand_chip *chip = mtd->priv;
> > +	struct nandi_controller *nandi = chip->priv;
> > +	uint32_t page_size = mtd->writesize;
> > +	loff_t offs = page * page_size;
> 
> You might have an overflow bug here, for >= 4GB NAND. You should cast to
> loff_t before the multiplication. Examine the rest of your driver for
> similar bugs; I think there's at least two repeats of this.
> 
> (Related: Coverity caught a whole bunch of these type of bugs in the MTD
> test modules. I have fixes queued up that I meant to test and submit
> soon.)

I will attempt to play around with Coverity.

> > +
> > +	return bch_erase_block(nandi, offs);
> > +}
> > +
> > +/*
> > + * 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)
> 
> Another "check erased page" implementation? Sigh... it would be nice if
> we could agree on a common implementation to share. My last attempt was
> unsuccessful, since some drivers have some very odd requirements.
> 
> > +{
> > +	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);
> 
> It seems like you're not considering the spare area at all. What if this
> is a mostly-blank page, with ECC data, but most of the bitflips are in
> the spare area? Then you will "correct" this page to all 0xFF, not
> noticing that this was really not a blank page at all.

I could really use Angus' advice on this one.  Although, I fear he may
have disappeared into the cosmos.  If I remember correctly (I'm
getting rusty on this now), this controller doesn't allow access to
the spare area easily.  I think it's all handled automatically
i.e. without intervention.

> > +
> > +	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 nand_chip *chip = &nandi->info.chip;
> > +	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(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 >> (chip->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 'bch_ecc_strength' bits at zero.
> 
> Couldn't this (more straightforwarly) just be chip->ecc.strength?
> 
> But then, do you really want to "correct" that many bits in an erased
> page? What if there are stuck 1 bits, too? Then when you finally program
> the page, you may have more errors than you can actually correct.
> 
> Maybe chip->ecc.strength / 2 would be a good compromise, allowing for a
> 50/50 distribution of 0->1 and 1->0 errors.

As you wish.

> > +		 */
> > +		ret = check_erased_page(buf, page_size,
> > +					bch_ecc_strength[nandi->bch_ecc_mode]);
> > +		if (ret >= 0)
> > +			dev_dbg(nandi->dev,
> > +				"%s: erased page detected: \n"
> > +				"  downgrading uncorrectable ECC error.\n",
> > +				__func__);
> 
> You seem to be missing all handling of mtd->ecc_stats; you need to
> increment mtd->ecc_stats.corrected or mtd->ecc_stats.failed for
> corrected bitlips and ECC failure reporting. Otherwise, the MTD API will
> not report -EUCLEAN and -EBADMSG properly.

Errr... I'll have a look.

> > +	} else {
> > +		ret = (int)ecc_err;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int bch_read(struct mtd_info *mtd, struct nand_chip *chip,
> > +			 uint8_t *buf, int oob_required, int page)
> > +{
> > +	struct nandi_controller *nandi = chip->priv;
> > +	uint32_t page_size = mtd->writesize;
> > +	loff_t offs = page * page_size;
> 
> 32-bit overflow

Okay, will cast to loff_t.

> > +	bool bounce = false;
> > +	uint8_t *p;
> > +	int ret;
> > +
> > +	if (((unsigned int)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
> > +	    (!virt_addr_valid(buf))) /* vmalloc'd buffer! */
> > +		bounce = true;
> > +
> > +	p = bounce ? nandi->page_buf : buf;
> 
> It looks like you're reimplementing NAND_USE_BOUNCE_BUFFER. Can you try
> using that flag? (You may need to extend it to account for your DMA
> alignment, too.)

This didn't exist when I submitted.  I'll see what it's trying to do.

> > +
> > +	ret = bch_read_page(nandi, offs, p);
> > +
> > +	if (bounce)
> > +		memcpy(buf, p, page_size);
> > +
> > +	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 nand_chip *chip = &nandi->info.chip;
> > +	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;
> > +	bool bounce = false;
> > +
> > +	dev_dbg(nandi->dev, "%s: offs = 0x%012llx\n", __func__, offs);
> > +
> > +	BUG_ON(offs & (page_size - 1));
> > +
> > +	if (((unsigned long)buf & (NANDI_BCH_DMA_ALIGNMENT - 1)) ||
> > +	    !virt_addr_valid(buf)) { /* vmalloc'd buffer! */
> > +		bounce = true;
> > +	}
> > +
> > +	if (bounce) {
> > +		memcpy(nandi->page_buf, buf, page_size);
> > +		p = nandi->page_buf;
> > +		nandi->cached_page = -1;
> > +	}
> > +
> > +	emiss_nandi_select(STM_NANDI_BCH);
> > +
> > +	nandi_enable_interrupts(nandi, NANDBCH_INT_SEQNODESOVER);
> > +	reinit_completion(&nandi->seq_completed);
> > +
> > +	prog->addr = (uint32_t)((offs >> (chip->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;
> > +}
> > +
> > +static int bch_write(struct mtd_info *mtd, struct nand_chip *chip,
> > +		     uint32_t offset, int data_len, const uint8_t *buf,
> > +		     int oob_required, int page, int cached, int raw)
> > +{
> > +	struct nandi_controller *nandi = chip->priv;
> > +	uint32_t page_size = mtd->writesize;
> > +	loff_t offs = page * page_size;
> 
> 32-bit overflow

Cast to loff_t, got it.

> > +	int ret;
> > +
> > +	ret = bch_write_page(nandi, offs, buf);
> > +	if (ret & NAND_STATUS_FAIL)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * 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);
> > +}
> > +
> > +/*
> > + * 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;
> > +	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--;
> > +	}
> > +}
> > +
> > +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;
> > +}
> > +
> > +/*
> > + * 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 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 >> chip->page_shift;
> > +	uint32_t page = block << (chip->phys_erase_shift - chip->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;
> > +}
> > +
> > +/* 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,
> > +				 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));
> > +}
> > +
> > +/* 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)
> > +{
> > +	struct nand_chip *chip = &nandi->info.chip;
> > +	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 << chip->phys_erase_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;
> > +}
> > +
> > +/*
> > + * 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)
> > +{
> > +	struct nand_chip *chip = &nandi->info.chip;
> > +	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 << chip->phys_erase_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,
> > +			     char *author)
> > +{
> > +	struct nand_chip *chip = &nandi->info.chip;
> > +	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 << chip->phys_erase_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;
> > +}
> > +
> > +/* 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)
> > +{
> > +	struct nand_chip *chip = &nandi->info.chip;
> > +	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 << chip->phys_erase_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 << chip->phys_erase_shift;
> 
> You need to cast 'block' to loff_t to prevent 32-bit overflow.

Okay.

> > +	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;
> > +}
> > +
> > +static int bch_scan_bbt(struct mtd_info *mtd)
> > +{
> > +	struct nand_chip *chip = mtd->priv;
> > +	struct nandi_controller *nandi = chip->priv;
> > +	struct nandi_bbt_info *bbt_info = &nandi->info.bbt_info;
> > +	int 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;
> > +}
> > +
> > +static int bch_mtd_read_oob(struct mtd_info *mtd,
> > +			    struct nand_chip *chip, int page)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static int bch_mtd_write_oob(struct mtd_info *mtd,
> > +			     struct nand_chip *chip, int page)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static int bch_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> > +			     uint8_t *buf, int oob_required, int page)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static int bch_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> > +			      const uint8_t *buf, int oob_required)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static void bch_hwctl(struct mtd_info *mtd, int mode)
> > +{
> > +	BUG();
> > +}
> > +
> > +static int bch_calculate(struct mtd_info *mtd, const uint8_t *dat,
> > +			 uint8_t *ecc_code)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static int bch_correct(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
> > +		       uint8_t *calc_ecc)
> > +{
> > +	BUG();
> > +	return 0;
> > +}
> > +
> > +static int bch_block_isbad(struct mtd_info *mtd, loff_t offs, int getchip)
> > +{
> > +	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 >> chip->phys_erase_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_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);
> 
> You're violating some of the layering here; the low-level driver
> probably shouldn't be calling the high-level mtd_*() APIs.

I'll try to figure out a way to keep the low-level code, low level.

> On a similar
> note, I'm not 100% confident that the nand_base/nand_bbt separation is
> written cleanly enough for easy maintenance of your nand_base + ST BBT
> code in parallel. I might need a second look at that, and I think
> modularizing your BBT code to a separate file could help ease this a
> little.

I have moved all of the BBT code out, but actually have no intention
of using them separately, as the vendor specific code is believed to
be more robust.

> > +	if (ret < 0)
> > +		return ret;
> > +	if (ret == 1)
> > +		return 0;
> > +
> > +	/* Mark bad */
> > +	block = offs >> chip->phys_erase_shift;
> > +	bbt_set_block_mark(nandi->info.bbt_info.bbt, block, BBT_MARK_BAD_WEAR);
> > +
> > +	/* Update BBTs, incrementing bbt_vers */
> > +	ret = bch_update_bbts(nandi, &nandi->info.bbt_info,
> > +			      NAND_IBBT_UPDATE_BOTH,
> > +			      nandi->info.bbt_info.bbt_vers[0] + 1);
> > +
> > +	return ret;
> > +}
> > +
> > +static void nandi_dump_bad_blocks(struct nandi_controller *nandi)
> > +{
> > +	struct nand_chip *chip = &nandi->info.chip;
> > +	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 << chip->phys_erase_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
> > + */
> > +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)
> > +{
> > +	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)
> > +{
> > +	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;
> > +	chip->ecc.mode = NAND_ECC_HW;
> > +
> > +	/* 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;
> > +	chip->options |= NAND_NO_SUBPAGE_WRITE;
> > +
> > +	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;
> > +
> > +	chip->bbt_options |= NAND_BBT_USE_FLASH;
> > +
> > +	/* mtd_info */
> > +	mtd->owner = THIS_MODULE;
> > +	mtd->type = MTD_NANDFLASH;
> > +	mtd->flags = MTD_CAP_NANDFLASH;
> > +	mtd->ecclayout = &info->ecclayout;
> > +	mtd->subpage_sft = 0;
> 
> Is this necessary? You've already set NAND_NO_SUBPAGE_WRITE, and
> kzalloc() will initialize the field.

It was probably put there as a sort of pseudo-comment, for clarity.
But I have no issue with removing the line.

> > +	chip->ecc.hwctl = bch_hwctl;
> > +	chip->ecc.calculate = bch_calculate;
> > +	chip->ecc.correct = bch_correct;
> > +
> > +	chip->ecc.read_oob = bch_mtd_read_oob;
> > +	chip->ecc.write_oob = bch_mtd_write_oob;
> > +
> > +	chip->ecc.read_page = bch_read;
> > +	chip->ecc.read_page_raw = bch_read_page_raw;
> > +	chip->ecc.write_page_raw = bch_write_page_raw;
> > +	chip->write_page = bch_write;
> > +	chip->erase = bch_erase;
> > +
> > +	chip->scan_bbt = bch_scan_bbt;
> > +	chip->block_bad = bch_block_isbad;
> > +	chip->block_markbad = bch_block_markbad;
> 
> If you make the ST BBT format optional, then these three bad block
> function pointer assignments should be conditional.

Right.  In the BBT separation code, I have handled this.

> > +}
> > +
> > +/*
> > + * Timing and Clocks
> > + */
> > +
> > +static void nandi_clk_enable(struct nandi_controller *nandi)
> > +{
> > +	if (nandi->emi_clk)
> 
> I believe the NULL check is unnecessary.
> 
> > +		clk_prepare_enable(nandi->emi_clk);
> > +	if (nandi->bch_clk)
> 
> Ditto.
> 
> > +		clk_prepare_enable(nandi->bch_clk);
> > +}
> > +
> > +static void nandi_clk_disable(struct nandi_controller *nandi)
> > +{
> > +	if (nandi->emi_clk)
> 
> Ditto.
> 
> > +		clk_disable_unprepare(nandi->emi_clk);
> > +	if (nandi->bch_clk)
> 
> Ditto.

Okay.

> > +		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);
> 
> Try devm_clk_get()? Then you can drop the clock_put()'s in your
> remove().

Yes indeedy.

> > +	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;
> > +}
> > +
> > +/* Derive Hamming-FLEX timing register values from 'nand_sdr_timings' data */
> > +static void flex_calc_timing_registers(const struct nand_sdr_timings *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_min - spec->tWP_min + tCLK - 1)/tCLK;
> > +	if (n_ctl_setup < 1)
> > +		n_ctl_setup = 1;
> > +	n_ctl_setup += relax;
> > +
> > +	/*	- HOLD */
> > +	tMAX_HOLD = spec->tCLH_min;
> > +	if (spec->tCH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tCH_min;
> > +	if (spec->tALH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tALH_min;
> > +	if (spec->tDH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tDH_min;
> > +	n_ctl_hold = (tMAX_HOLD + tCLK - 1)/tCLK + relax;
> > +
> > +	/*	- CE_deassert_hold = 0 */
> > +
> > +	/*	- WE_high_to_RBn_low */
> > +	n_ctl_wb = (spec->tWB_max + 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_min + tCLK - 1)/tCLK + relax;
> > +
> > +	/*	- OFF */
> > +	tMAX_WEN_OFF = spec->tWC_min - spec->tWH_min;
> > +	if (spec->tWP_min > tMAX_WEN_OFF)
> > +		tMAX_WEN_OFF = spec->tWP_min;
> > +	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_min + tCLK - 1)/tCLK + relax;
> > +
> > +	/*	- OFF */
> > +	tMAX_REN_OFF = spec->tRC_min - spec->tREH_min;
> > +	if (spec->tRP_min > tMAX_REN_OFF)
> > +		tMAX_REN_OFF = spec->tRP_min;
> > +	if (spec->tREA_max > tMAX_REN_OFF)
> > +		tMAX_REN_OFF = spec->tREA_max;
> > +	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_sdr_timings' data */
> > +static void bch_calc_timing_registers(const struct nand_sdr_timings *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_min > spec->tWP_min)
> > +		n_ctl_setup = (spec->tCLS_min - spec->tWP_min + tCLK - 1)/tCLK;
> > +	else
> > +		n_ctl_setup = 0;
> > +	n_ctl_setup += relax;
> > +
> > +	/*	- HOLD */
> > +	tMAX_HOLD = spec->tCLH_min;
> > +	if (spec->tCH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tCH_min;
> > +	if (spec->tALH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tALH_min;
> > +	if (spec->tDH_min > tMAX_HOLD)
> > +		tMAX_HOLD = spec->tDH_min;
> > +	n_ctl_hold = (tMAX_HOLD + tCLK - 1)/tCLK + relax;
> > +	/*	- CE_deassert_hold = 0 */
> > +
> > +	/*	- WE_high_to_RBn_low */
> > +	n_ctl_wb = (spec->tWB_max + 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_min + 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_min + 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_min - spec->tRP_min;
> > +	if (spec->tREH_min > tMAX_REN_ON)
> > +		tMAX_REN_ON = spec->tREH_min;
> > +
> > +	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_max + tCLK - 1)/tCLK;
> > +	ren_half_off = n_ren_off % 2;
> > +	n_ren_off /= 2;
> > +	n_ren_off += relax;
> > +
> > +	/*	- DATA_LATCH */
> > +	if (spec->tREA_max <= (spec->tRP_min - (2 * tCLK)))
> > +		n_d_latch = 0;
> > +	else if (spec->tREA_max <= (spec->tRP_min - tCLK))
> > +		n_d_latch = 1;
> > +	else if ((spec->tREA_max <= spec->tRP_min) && (spec->tRHOH_min >= 2 * tCLK))
> > +		n_d_latch = 2;
> > +	else
> > +		n_d_latch = 3;
> > +
> > +	/*	- TELQV */
> > +	tMAX_CS_DEASSERT = spec->tCOH_min;
> > +	if (spec->tCHZ_max > tMAX_CS_DEASSERT)
> > +		tMAX_CS_DEASSERT = spec->tCHZ_max;
> > +
> > +	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,
> > +					    const struct nand_sdr_timings *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,
> > +					   const struct nand_sdr_timings *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,
> > +					const struct nand_sdr_timings *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__);
> > +
> > +	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__);
> > +
> > +	/* 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(CFG_RESET_ECC_ALL, 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);
> > +	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)
> > +{
> > +	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 irq;
> > +	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);
> > +
> > +	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);
> > +	}
> > +
> > +	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;
> > +}
> > +
> > +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;
> > +	int ecc_strength;
> > +	int ret;
> > +
> > +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> > +	if (!pdata)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	of_property_read_u32(np, "nand-ecc-strength", &ecc_strength);
> > +	if (ecc_strength == 0)
> > +		pdata->bch_ecc_cfg = BCH_NO_ECC;
> > +	else if (ecc_strength == 18)
> > +		pdata->bch_ecc_cfg = BCH_18BIT_ECC;
> > +	else if (ecc_strength == 30)
> > +		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);
> > +
> > +	return pdata;
> > +}
> > +
> > +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;
> 
> You're assigning 'pdata' here (probably to NULL, since you're looking
> for DT-enabled platform devices, not legacy-style platform_devices)...
> 
> > +	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;
> > +	struct nandi_info *info;
> > +	struct nand_chip *chip;
> > +	struct mtd_info *mtd;
> > +	int compatible, err;
> > +
> > +	if (!np) {
> > +		dev_err(&pdev->dev, "DT node found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	pdata = stm_bch_dt_get_pdata(pdev);
> 
> ...and then you're reassigning it here. Drop the first assignment?

You're right, will fix.

> > +	if (IS_ERR(pdata))
> > +		return PTR_ERR(pdata);
> > +
> > +	ppdata.of_node = stm_of_get_partitions_node(np, 0);
> 
> So you only support a single bank, at chip-select 0? Is this a
> hardware limitation, or simply software? You might consider whether this
> needs to be noted in the DT binding too -- it currently suggests that
> this can be plural.

Yes, only one bank.  Chip select is not possible.

> > +
> > +	pdev->dev.platform_data = pdata;
> 
> Are you actually looking for driver data, not platform data? (e.g.,
> dev_set_drvdata() / platform_get_drvdata()) The two are a little
> different, but your usage sounds like this more of a driver-private
> description.

It should.  I don't know why I didn't notice this before.

> > +
> > +	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);
> > +
> > +	bank = pdata->bank;
> > +	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);
> > +
> > +	err = nand_scan_ident(mtd, 1, NULL);
> > +	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,
> > +					&st_nand_onfi_timing_specs[mode],
> > +					bank ? bank->timing_relax : 0);
> > +	} else {
> > +		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;
> > +	}
> > +
> > +	/* 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->extra_addr = ((chip->chipsize >> chip->page_shift) >
> > +			     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;
> > +	}
> > +
> > +	chip->ecc.size = NANDI_BCH_SECTOR_SIZE;
> > +	chip->ecc.bytes = mtd->oobsize;
> > +	chip->ecc.strength = bch_ecc_strength[nandi->bch_ecc_mode];
> > +
> > +	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;
> > +	}
> > +
> > +	/* 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;
> > +
> > +	err = nand_scan_tail(mtd);
> > +	if (err)
> > +		return err;
> > +
> > +	nandi_dump_bad_blocks(nandi);
> > +
> > +	/* 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)
> > +{
> > +	struct nandi_controller *nandi = platform_get_drvdata(pdev);
> > +
> > +	nand_release(&nandi->info.mtd);
> > +
> > +	nandi_clk_disable(nandi);
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM
> 
> Should this be CONFIG_PM_SLEEP?
> 
> > +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;
> 
> I think this might as well be:
> 
> #define stm_nand_bch_pm_ops	NULL

Actually, I think it's all better written with SET_SYSTEM_SLEEP_PM_OPS()

> > +#endif
> > +
> > +static struct of_device_id nand_bch_match[] = {
> > +	{ .compatible = "st,nand-bch", },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, nand_bch_match);
> > +
> > +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,
> > +	},
> > +};
> > +module_platform_driver(stm_nand_bch_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Angus Clark");
> > +MODULE_DESCRIPTION("STM NAND BCH driver");
> > diff --git a/drivers/mtd/nand/stm_nand_dt.c b/drivers/mtd/nand/stm_nand_dt.c
> > new file mode 100644
> > index 0000000..21bd20f
> > --- /dev/null
> > +++ b/drivers/mtd/nand/stm_nand_dt.c
> > @@ -0,0 +1,116 @@
> > +/*
> > + * 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@xxxxxx>
> > + *
> > + * 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>
> > +
> > +#include "stm_nand_regs.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) {
> > +		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;
> > +
> > +		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@xxxxxx>
> > + *
> > + * 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/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@xxxxxx>
> > + *
> > + * 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 */
> > diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h
> > new file mode 100644
> > index 0000000..3cd3a14
> > --- /dev/null
> > +++ b/include/linux/mtd/stm_nand.h
> > @@ -0,0 +1,104 @@
> > +/*
> > + * include/linux/mtd/stm_mtd.h
> > + *
> > + * Support for STMicroelectronics NAND Controllers
> > + *
> > + * Copyright (c) 2014 STMicroelectronics Limited
> > + * Author: Angus Clark <Angus.Clark@xxxxxx>
> > + *
> > + * 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
> > +
> > +#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_sdr_timings *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_sdr_timings' and ONFI configuration.
> > +	 */
> > +	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;
> > +
> > +	/* 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;
> 
> This field is never set or used...
> 
> > +	bool flashss;
> 
> Ditto.

Will remove them both.

> > +};
> > +
> > +#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 */
> 
> Alright, that's enough of my review for now. I'm not sure I've gotten to
> everything yet, but that should give you something to chew on.
> 
> Brian

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/