[PATCH v3 00/10] mtd: nand: add Broadcom NAND controller support

[PATCH v3 00/10] mtd: nand: add Broadcom NAND controller support

[Brian Norris]

Hi,

This is version 3 of support for the Broadcom BCM7xxx Set-Top Box NAND
controller. This controller has been used in a variety of Broadcom SoCs.

This series now adds support for a few new chips: BCM63138, and the iProc chip
family. These add an additional 6 new patches to the original 4. If the only
comments end up on the latter 6 patches, the first 4 might be worth merging
independently.

Summary changelog:

v1 -> v2:
 * add NAND to DTS for BCM7445 / BCM97445SVMB
 * rename DT binding file to have 'brcm,' prefix
 * catch DMA mapping errors
 * fixup timeout / error messages (hex, remove misleading info)
 * MODULE_LICENSE("GPL v2")
 * fix incorrect comments
 * print why we fail, when checking for supported controller revisions
 * disable prefetch when using Flash DMA (see FIXME); will re-enable once we
   get a good erased-page verification scheme merged

v2 -> v3:
 * rebase to v4.1-rc1
 * add SoC-specific infrastructure, to help support other SoCs:
   - add BCM63138 support
   - add iProc/Cygnus support
 * disable prefetch on v6.1

Brian Norris (8):
  mtd: nand: add common DT init code
  Documentation: devicetree: add binding doc for Broadcom NAND
    controller
  mtd: nand: add NAND driver for Broadcom STB NAND controller
  ARM: bcm7445: add NAND to DTS
  Documentation: devicetree: brcmstb_nand: add 'brcm,nand-soc' bindings
  mtd: brcmstb_nand: add SoC-specific support
  mtd: brcsmtb_nand_soc: add support for BCM63138
  ARM: bcm63138: add NAND DT support

Ray Jui (2):
  mtd: brcsmtb_nand_soc: add iProc support
  ARM: dts: cygnus: Enable NAND support for Cygnus

 .../devicetree/bindings/mtd/brcm,brcmstb-nand.txt  |  147 ++
 arch/arm/boot/dts/bcm-cygnus.dtsi                  |   20 +
 arch/arm/boot/dts/bcm63138.dtsi                    |   17 +
 arch/arm/boot/dts/bcm7445-bcm97445svmb.dts         |   23 +
 arch/arm/boot/dts/bcm7445.dtsi                     |   22 +
 arch/arm/boot/dts/bcm958300k.dts                   |   16 +
 arch/arm/boot/dts/bcm963138dvt.dts                 |   12 +
 drivers/mtd/nand/Kconfig                           |    8 +
 drivers/mtd/nand/Makefile                          |    2 +
 drivers/mtd/nand/brcmnand.h                        |   56 +
 drivers/mtd/nand/brcmstb_nand.c                    | 2263 ++++++++++++++++++++
 drivers/mtd/nand/brcmstb_nand_soc.c                |  244 +++
 drivers/mtd/nand/nand_base.c                       |   41 +
 include/linux/mtd/nand.h                           |    5 +
 14 files changed, 2876 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
 create mode 100644 drivers/mtd/nand/brcmnand.h
 create mode 100644 drivers/mtd/nand/brcmstb_nand.c
 create mode 100644 drivers/mtd/nand/brcmstb_nand_soc.c

-- 
1.9.1

[PATCH v3 01/10] mtd: nand: add common DT init code

[Brian Norris]

These are already-documented common bindings for NAND chips. Let's
handle them in nand_base.

If NAND controller drivers need to act on this data before bringing up
the NAND chip (e.g., fill out ECC callback functions, change HW modes,
etc.), then they can do so between calling nand_scan_ident() and
nand_scan_tail().

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h     |  5 +++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c2e1232cd45c..c6cf775ee431 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -48,6 +48,7 @@
 #include <linux/leds.h>
 #include <linux/io.h>
 #include <linux/mtd/partitions.h>
+#include <linux/of_mtd.h>
 
 /* Define default oob placement schemes for large and small page devices */
 static struct nand_ecclayout nand_oob_8 = {
@@ -3798,6 +3799,39 @@ ident_done:
 	return type;
 }
 
+static int nand_dt_init(struct mtd_info *mtd, struct nand_chip *chip,
+			struct device_node *dn)
+{
+	int ecc_mode, ecc_strength, ecc_step;
+
+	if (of_get_nand_bus_width(dn) == 16)
+		chip->options |= NAND_BUSWIDTH_16;
+
+	if (of_get_nand_on_flash_bbt(dn))
+		chip->bbt_options |= NAND_BBT_USE_FLASH;
+
+	ecc_mode = of_get_nand_ecc_mode(dn);
+	ecc_strength = of_get_nand_ecc_strength(dn);
+	ecc_step = of_get_nand_ecc_step_size(dn);
+
+	if ((ecc_step >= 0 && !(ecc_strength >= 0)) ||
+	    (!(ecc_step >= 0) && ecc_strength >= 0)) {
+		pr_err("must set both strength and step size in DT\n");
+		return -EINVAL;
+	}
+
+	if (ecc_mode >= 0)
+		chip->ecc.mode = ecc_mode;
+
+	if (ecc_strength >= 0)
+		chip->ecc.strength = ecc_strength;
+
+	if (ecc_step > 0)
+		chip->ecc.size = ecc_step;
+
+	return 0;
+}
+
 /**
  * nand_scan_ident - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -3815,6 +3849,13 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
 	int i, nand_maf_id, nand_dev_id;
 	struct nand_chip *chip = mtd->priv;
 	struct nand_flash_dev *type;
+	int ret;
+
+	if (chip->dn) {
+		ret = nand_dt_init(mtd, chip, chip->dn);
+		if (ret)
+			return ret;
+	}
 
 	/* Set the default functions */
 	nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16);
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3d4ea7eb2b68..e0f40e12a2c8 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -26,6 +26,8 @@
 
 struct mtd_info;
 struct nand_flash_dev;
+struct device_node;
+
 /* Scan and identify a NAND device */
 extern int nand_scan(struct mtd_info *mtd, int max_chips);
 /*
@@ -542,6 +544,7 @@ struct nand_buffers {
  *			flash device
  * @IO_ADDR_W:		[BOARDSPECIFIC] address to write the 8 I/O lines of the
  *			flash device.
+ * @dn:			[BOARDSPECIFIC] device node describing this instance
  * @read_byte:		[REPLACEABLE] read one byte from the chip
  * @read_word:		[REPLACEABLE] read one word from the chip
  * @write_byte:		[REPLACEABLE] write a single byte to the chip on the
@@ -644,6 +647,8 @@ struct nand_chip {
 	void __iomem *IO_ADDR_R;
 	void __iomem *IO_ADDR_W;
 
+	struct device_node *dn;
+
 	uint8_t (*read_byte)(struct mtd_info *mtd);
 	u16 (*read_word)(struct mtd_info *mtd);
 	void (*write_byte)(struct mtd_info *mtd, uint8_t byte);
-- 
1.9.1

[PATCH v3 02/10] Documentation: devicetree: add binding doc for Broadcom NAND controller

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 .../devicetree/bindings/mtd/brcm,brcmstb-nand.txt  | 110 +++++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
new file mode 100644
index 000000000000..662c857e74fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
@@ -0,0 +1,110 @@
+* Broadcom STB NAND Controller
+
+The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
+flash chips. It has a memory-mapped register interface for both control
+registers and for its data input/output buffer. On some SoCs, this controller is
+paired with a custom DMA engine (inventively named "Flash DMA") which supports
+basic PROGRAM and READ functions, among other features.
+
+This controller was originally designed for STB SoCs (BCM7xxx) but is now
+available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
+iProc/Cygnus. Its history includes several similar (but not fully register
+compatible) versions.
+
+Required properties:
+- compatible       : should contain "brcm,brcmnand" and an appropriate version
+                      compatibility string, like "brcm,brcmnand-v7.0"
+                      Possible values:
+                         brcm,brcmnand-v4.0
+                         brcm,brcmnand-v5.0
+                         brcm,brcmnand-v6.0
+                         brcm,brcmnand-v6.1
+                         brcm,brcmnand-v7.0
+                         brcm,brcmnand-v7.1
+                         brcm,brcmnand
+- reg              : the register start and length for NAND register region.
+                     (optional) Flash DMA register range (if present)
+                     (optional) NAND flash cache range (if at non-standard offset)
+- reg-names        : a list of the names corresponding to the previous register
+                     ranges. Should contain "nand" and (optionally)
+                     "flash-dma" and/or "nand-cache".
+- interrupts       : The NAND CTLRDY interrupt and (if Flash DMA is available)
+                     FLASH_DMA_DONE
+- interrupt-names  : May be "nand_ctlrdy" or "flash_dma_done"
+- interrupt-parent : See standard interrupt bindings
+- #address-cells   : <1> - subnodes give the chip-select number
+- #size-cells      : <0>
+
+Optional properties:
+- brcm,nand-has-wp          : Some versions of this IP include a write-protect
+                              (WP) control bit. It is always available on >=
+                              v7.0. Use this property to describe the rare
+                              earlier versions of this core that include WP
+
+* NAND chip-select
+
+Each controller (compatible: "brcm,brcmnand") may contain one or more subnodes
+to represent enabled chip-selects which (may) contain NAND flash chips. Their
+properties are as follows.
+
+Required properties:
+- compatible                : should contain "brcm,nandcs"
+- reg                       : a single integer representing the chip-select
+                              number (e.g., 0, 1, 2, etc.)
+- #address-cells            : see partition.txt
+- #size-cells               : see partition.txt
+- nand-ecc-strength         : see nand.txt
+- nand-ecc-step-size        : must be 512 or 1024. See nand.txt
+
+Optional properties:
+- nand-on-flash-bbt         : boolean, to enable the on-flash BBT for this
+                              chip-select. See nand.txt
+- brcm,nand-oob-sector-size : integer, to denote the spare area sector size
+                              expected for the ECC layout in use. This size, in
+                              addition to the strength and step-size,
+                              determines how the hardware BCH engine will lay
+                              out the parity bytes it stores on the flash.
+                              This property can be automatically determined by
+                              the flash geometry (particularly the NAND page
+                              and OOB size) in many cases, but when booting
+                              from NAND, the boot controller has only a limited
+                              number of available options for its default ECC
+                              layout.
+
+Each nandcs device node may optionally contain sub-nodes describing the flash
+partition mapping. See partition.txt for more detail.
+
+Example:
+
+nand@f0442800 {
+	compatible = "brcm,brcmnand-v7.0", "brcm,brcmnand";
+	reg = <0xF0442800 0x600>,
+	      <0xF0443000 0x100>;
+	reg-names = "nand", "flash-dma";
+	interrupt-parent = <&hif_intr2_intc>;
+	interrupts = <24>, <4>;
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	nandcs@1 {
+		compatible = "brcm,nandcs";
+		reg = <1>; // Chip select 1
+		nand-on-flash-bbt;
+		nand-ecc-strength = <12>;
+		nand-ecc-step-size = <512>;
+
+		// Partitions
+		#address-cells = <1>;  // <2>, for 64-bit offset
+		#size-cells = <1>;     // <2>, for 64-bit length
+		flash0.rootfs@0 {
+			reg = <0 0x10000000>;
+		};
+		flash0@0 {
+			reg = <0 0>; // MTDPART_SIZ_FULL
+		};
+		flash0.kernel@10000000 {
+			reg = <0x10000000 0x400000>;
+		};
+	};
+};
-- 
1.9.1

[PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Brian Norris]

This core originated in Set-Top Box chips (BCM7xxx) but is used in a
variety of other Broadcom chips, including some BCM63xxx, BCM33xx, and
iProc/Cygnus. It's been used only on ARM and MIPS SoCs, so restrict it
to those architectures.

There are multiple revisions of this core throughout the years, and
almost every version broke register compatibility in some small way, but
with some effort, this driver is able to support v4.0, v5.0, v6.x, v7.0,
and v7.1. It's been tested on v5.0, v6.0, v7.0, and v7.1 recently, so
there hopefully are no more lurking inconsistencies.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/Kconfig        |    8 +
 drivers/mtd/nand/Makefile       |    1 +
 drivers/mtd/nand/brcmstb_nand.c | 2198 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 2207 insertions(+)
 create mode 100644 drivers/mtd/nand/brcmstb_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5897d8d8fa5a..3587017b209a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -394,6 +394,14 @@ config MTD_NAND_GPMI_NAND
 	 block, such as SD card. So pay attention to it when you enable
 	 the GPMI.
 
+config MTD_NAND_BRCMSTB
+	tristate "Broadcom STB NAND controller"
+	depends on ARM || MIPS
+	help
+	  Enables the Broadcom NAND controller driver. The controller was
+	  originally designed for Set-Top Box but is used on various BCM7xxx,
+	  BCM3xxx, BCM63xxx, iProc/Cygnus and more.
+
 config MTD_NAND_BCM47XXNFLASH
 	tristate "Support for NAND flash on BCM4706 BCMA bus"
 	depends on BCMA_NFLASH
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 582bbd05aff7..3b1adddc83dd 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -52,5 +52,6 @@ obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
 obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
+obj-$(CONFIG_MTD_NAND_BRCMSTB)		+= brcmstb_nand.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/brcmstb_nand.c b/drivers/mtd/nand/brcmstb_nand.c
new file mode 100644
index 000000000000..ec65a48d2487
--- /dev/null
+++ b/drivers/mtd/nand/brcmstb_nand.c
@@ -0,0 +1,2198 @@
+/*
+ * Copyright © 2010-2015 Broadcom Corporation
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/dma-mapping.h>
+#include <linux/ioport.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/mm.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_mtd.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/log2.h>
+
+/*
+ * This flag controls if WP stays on between erase/write commands to mitigate
+ * flash corruption due to power glitches. Values:
+ * 0: NAND_WP is not used or not available
+ * 1: NAND_WP is set by default, cleared for erase/write operations
+ * 2: NAND_WP is always cleared
+ */
+static int wp_on = 1;
+module_param(wp_on, int, 0444);
+
+/***********************************************************************
+ * Definitions
+ ***********************************************************************/
+
+#define DRV_NAME			"brcmstb_nand"
+
+#define CMD_NULL			0x00
+#define CMD_PAGE_READ			0x01
+#define CMD_SPARE_AREA_READ		0x02
+#define CMD_STATUS_READ			0x03
+#define CMD_PROGRAM_PAGE		0x04
+#define CMD_PROGRAM_SPARE_AREA		0x05
+#define CMD_COPY_BACK			0x06
+#define CMD_DEVICE_ID_READ		0x07
+#define CMD_BLOCK_ERASE			0x08
+#define CMD_FLASH_RESET			0x09
+#define CMD_BLOCKS_LOCK			0x0a
+#define CMD_BLOCKS_LOCK_DOWN		0x0b
+#define CMD_BLOCKS_UNLOCK		0x0c
+#define CMD_READ_BLOCKS_LOCK_STATUS	0x0d
+#define CMD_PARAMETER_READ		0x0e
+#define CMD_PARAMETER_CHANGE_COL	0x0f
+#define CMD_LOW_LEVEL_OP		0x10
+
+struct brcm_nand_dma_desc {
+	u32 next_desc;
+	u32 next_desc_ext;
+	u32 cmd_irq;
+	u32 dram_addr;
+	u32 dram_addr_ext;
+	u32 tfr_len;
+	u32 total_len;
+	u32 flash_addr;
+	u32 flash_addr_ext;
+	u32 cs;
+	u32 pad2[5];
+	u32 status_valid;
+} __packed;
+
+/* Bitfields for brcm_nand_dma_desc::status_valid */
+#define FLASH_DMA_ECC_ERROR	(1 << 8)
+#define FLASH_DMA_CORR_ERROR	(1 << 9)
+
+/* 512B flash cache in the NAND controller HW */
+#define FC_SHIFT		9U
+#define FC_BYTES		512U
+#define FC_WORDS		(FC_BYTES >> 2)
+
+#define BRCMNAND_MIN_PAGESIZE	512
+#define BRCMNAND_MIN_BLOCKSIZE	(8 * 1024)
+#define BRCMNAND_MIN_DEVSIZE	(4ULL * 1024 * 1024)
+
+/* Controller feature flags */
+enum {
+	BRCMNAND_HAS_1K_SECTORS			= BIT(0),
+	BRCMNAND_HAS_PREFETCH			= BIT(1),
+	BRCMNAND_HAS_CACHE_MODE			= BIT(2),
+	BRCMNAND_HAS_WP				= BIT(3),
+};
+
+struct brcmnand_controller {
+	struct device		*dev;
+	struct nand_hw_control	controller;
+	void __iomem		*nand_base;
+	void __iomem		*nand_fc; /* flash cache */
+	void __iomem		*flash_dma_base;
+	unsigned int		irq;
+	unsigned int		dma_irq;
+	int			nand_version;
+
+	int			cmd_pending;
+	bool			dma_pending;
+	struct completion	done;
+	struct completion	dma_done;
+
+	/* List of NAND hosts (one for each chip-select) */
+	struct list_head host_list;
+
+	struct brcm_nand_dma_desc *dma_desc;
+	dma_addr_t		dma_pa;
+
+	/* in-memory cache of the FLASH_CACHE, used only for some commands */
+	u32			flash_cache[FC_WORDS];
+
+	/* Controller revision details */
+	const u16		*reg_offsets;
+	unsigned int		reg_spacing; /* between CS1, CS2, ... regs */
+	const u8		*cs_offsets; /* within each chip-select */
+	const u8		*cs0_offsets; /* within CS0, if different */
+	unsigned int		max_block_size;
+	const unsigned int	*block_sizes;
+	unsigned int		max_page_size;
+	const unsigned int	*page_sizes;
+	unsigned int		max_oob;
+	u32			features;
+
+	/* for low-power standby/resume only */
+	u32			nand_cs_nand_select;
+	u32			nand_cs_nand_xor;
+	u32			corr_stat_threshold;
+	u32			flash_dma_mode;
+};
+
+struct brcmnand_cfg {
+	u64			device_size;
+	unsigned int		block_size;
+	unsigned int		page_size;
+	unsigned int		spare_area_size;
+	unsigned int		device_width;
+	unsigned int		col_adr_bytes;
+	unsigned int		blk_adr_bytes;
+	unsigned int		ful_adr_bytes;
+	unsigned int		sector_size_1k;
+	unsigned int		ecc_level;
+	/* use for low-power standby/resume only */
+	u32			acc_control;
+	u32			config;
+	u32			config_ext;
+	u32			timing_1;
+	u32			timing_2;
+};
+
+struct brcmnand_host {
+	struct list_head	node;
+	struct device_node	*of_node;
+
+	struct nand_chip	chip;
+	struct mtd_info		mtd;
+	struct platform_device	*pdev;
+	int			cs;
+
+	unsigned int		last_cmd;
+	unsigned int		last_byte;
+	u64			last_addr;
+	struct brcmnand_cfg	hwcfg;
+	struct brcmnand_controller *ctrl;
+};
+
+enum brcmnand_reg {
+	BRCMNAND_CMD_START = 0,
+	BRCMNAND_CMD_EXT_ADDRESS,
+	BRCMNAND_CMD_ADDRESS,
+	BRCMNAND_INTFC_STATUS,
+	BRCMNAND_CS_SELECT,
+	BRCMNAND_CS_XOR,
+	BRCMNAND_LL_OP,
+	BRCMNAND_CS0_BASE,
+	BRCMNAND_CS1_BASE,		/* CS1 regs, if non-contiguous */
+	BRCMNAND_CORR_THRESHOLD,
+	BRCMNAND_CORR_THRESHOLD_EXT,
+	BRCMNAND_UNCORR_COUNT,
+	BRCMNAND_CORR_COUNT,
+	BRCMNAND_CORR_EXT_ADDR,
+	BRCMNAND_CORR_ADDR,
+	BRCMNAND_UNCORR_EXT_ADDR,
+	BRCMNAND_UNCORR_ADDR,
+	BRCMNAND_SEMAPHORE,
+	BRCMNAND_ID,
+	BRCMNAND_ID_EXT,
+	BRCMNAND_LL_RDATA,
+	BRCMNAND_OOB_READ_BASE,
+	BRCMNAND_OOB_READ_10_BASE,	/* offset 0x10, if non-contiguous */
+	BRCMNAND_OOB_WRITE_BASE,
+	BRCMNAND_OOB_WRITE_10_BASE,	/* offset 0x10, if non-contiguous */
+	BRCMNAND_FC_BASE,
+};
+
+/* BRCMNAND v4.0 */
+static const u16 brcmnand_regs_v40[] = {
+	[BRCMNAND_CMD_START]		=  0x04,
+	[BRCMNAND_CMD_EXT_ADDRESS]	=  0x08,
+	[BRCMNAND_CMD_ADDRESS]		=  0x0c,
+	[BRCMNAND_INTFC_STATUS]		=  0x6c,
+	[BRCMNAND_CS_SELECT]		=  0x14,
+	[BRCMNAND_CS_XOR]		=  0x18,
+	[BRCMNAND_LL_OP]		= 0x178,
+	[BRCMNAND_CS0_BASE]		=  0x40,
+	[BRCMNAND_CS1_BASE]		=  0xd0,
+	[BRCMNAND_CORR_THRESHOLD]	=  0x84,
+	[BRCMNAND_CORR_THRESHOLD_EXT]	=     0,
+	[BRCMNAND_UNCORR_COUNT]		=     0,
+	[BRCMNAND_CORR_COUNT]		=     0,
+	[BRCMNAND_CORR_EXT_ADDR]	=  0x70,
+	[BRCMNAND_CORR_ADDR]		=  0x74,
+	[BRCMNAND_UNCORR_EXT_ADDR]	=  0x78,
+	[BRCMNAND_UNCORR_ADDR]		=  0x7c,
+	[BRCMNAND_SEMAPHORE]		=  0x58,
+	[BRCMNAND_ID]			=  0x60,
+	[BRCMNAND_ID_EXT]		=  0x64,
+	[BRCMNAND_LL_RDATA]		= 0x17c,
+	[BRCMNAND_OOB_READ_BASE]	=  0x20,
+	[BRCMNAND_OOB_READ_10_BASE]	= 0x130,
+	[BRCMNAND_OOB_WRITE_BASE]	=  0x30,
+	[BRCMNAND_OOB_WRITE_10_BASE]	=     0,
+	[BRCMNAND_FC_BASE]		= 0x200,
+};
+
+/* BRCMNAND v5.0 */
+static const u16 brcmnand_regs_v50[] = {
+	[BRCMNAND_CMD_START]		=  0x04,
+	[BRCMNAND_CMD_EXT_ADDRESS]	=  0x08,
+	[BRCMNAND_CMD_ADDRESS]		=  0x0c,
+	[BRCMNAND_INTFC_STATUS]		=  0x6c,
+	[BRCMNAND_CS_SELECT]		=  0x14,
+	[BRCMNAND_CS_XOR]		=  0x18,
+	[BRCMNAND_LL_OP]		= 0x178,
+	[BRCMNAND_CS0_BASE]		=  0x40,
+	[BRCMNAND_CS1_BASE]		=  0xd0,
+	[BRCMNAND_CORR_THRESHOLD]	=  0x84,
+	[BRCMNAND_CORR_THRESHOLD_EXT]	=     0,
+	[BRCMNAND_UNCORR_COUNT]		=     0,
+	[BRCMNAND_CORR_COUNT]		=     0,
+	[BRCMNAND_CORR_EXT_ADDR]	=  0x70,
+	[BRCMNAND_CORR_ADDR]		=  0x74,
+	[BRCMNAND_UNCORR_EXT_ADDR]	=  0x78,
+	[BRCMNAND_UNCORR_ADDR]		=  0x7c,
+	[BRCMNAND_SEMAPHORE]		=  0x58,
+	[BRCMNAND_ID]			=  0x60,
+	[BRCMNAND_ID_EXT]		=  0x64,
+	[BRCMNAND_LL_RDATA]		= 0x17c,
+	[BRCMNAND_OOB_READ_BASE]	=  0x20,
+	[BRCMNAND_OOB_READ_10_BASE]	= 0x130,
+	[BRCMNAND_OOB_WRITE_BASE]	=  0x30,
+	[BRCMNAND_OOB_WRITE_10_BASE]	= 0x140,
+	[BRCMNAND_FC_BASE]		= 0x200,
+};
+
+/* BRCMNAND v6.0 - v7.1 */
+static const u16 brcmnand_regs_v60[] = {
+	[BRCMNAND_CMD_START]		=  0x04,
+	[BRCMNAND_CMD_EXT_ADDRESS]	=  0x08,
+	[BRCMNAND_CMD_ADDRESS]		=  0x0c,
+	[BRCMNAND_INTFC_STATUS]		=  0x14,
+	[BRCMNAND_CS_SELECT]		=  0x18,
+	[BRCMNAND_CS_XOR]		=  0x1c,
+	[BRCMNAND_LL_OP]		=  0x20,
+	[BRCMNAND_CS0_BASE]		=  0x50,
+	[BRCMNAND_CS1_BASE]		=     0,
+	[BRCMNAND_CORR_THRESHOLD]	=  0xc0,
+	[BRCMNAND_CORR_THRESHOLD_EXT]	=  0xc4,
+	[BRCMNAND_UNCORR_COUNT]		=  0xfc,
+	[BRCMNAND_CORR_COUNT]		= 0x100,
+	[BRCMNAND_CORR_EXT_ADDR]	= 0x10c,
+	[BRCMNAND_CORR_ADDR]		= 0x110,
+	[BRCMNAND_UNCORR_EXT_ADDR]	= 0x114,
+	[BRCMNAND_UNCORR_ADDR]		= 0x118,
+	[BRCMNAND_SEMAPHORE]		= 0x150,
+	[BRCMNAND_ID]			= 0x194,
+	[BRCMNAND_ID_EXT]		= 0x198,
+	[BRCMNAND_LL_RDATA]		= 0x19c,
+	[BRCMNAND_OOB_READ_BASE]	= 0x200,
+	[BRCMNAND_OOB_READ_10_BASE]	=     0,
+	[BRCMNAND_OOB_WRITE_BASE]	= 0x280,
+	[BRCMNAND_OOB_WRITE_10_BASE]	=     0,
+	[BRCMNAND_FC_BASE]		= 0x400,
+};
+
+enum brcmnand_cs_reg {
+	BRCMNAND_CS_CFG_EXT = 0,
+	BRCMNAND_CS_CFG,
+	BRCMNAND_CS_ACC_CONTROL,
+	BRCMNAND_CS_TIMING1,
+	BRCMNAND_CS_TIMING2,
+};
+
+/* Per chip-select offsets for v7.1 */
+static const u8 brcmnand_cs_offsets_v71[] = {
+	[BRCMNAND_CS_ACC_CONTROL]	= 0x00,
+	[BRCMNAND_CS_CFG_EXT]		= 0x04,
+	[BRCMNAND_CS_CFG]		= 0x08,
+	[BRCMNAND_CS_TIMING1]		= 0x0c,
+	[BRCMNAND_CS_TIMING2]		= 0x10,
+};
+
+/* Per chip-select offsets for pre v7.1, except CS0 on <= v5.0 */
+static const u8 brcmnand_cs_offsets[] = {
+	[BRCMNAND_CS_ACC_CONTROL]	= 0x00,
+	[BRCMNAND_CS_CFG_EXT]		= 0x04,
+	[BRCMNAND_CS_CFG]		= 0x04,
+	[BRCMNAND_CS_TIMING1]		= 0x08,
+	[BRCMNAND_CS_TIMING2]		= 0x0c,
+};
+
+/* Per chip-select offset for <= v5.0 on CS0 only */
+static const u8 brcmnand_cs_offsets_cs0[] = {
+	[BRCMNAND_CS_ACC_CONTROL]	= 0x00,
+	[BRCMNAND_CS_CFG_EXT]		= 0x08,
+	[BRCMNAND_CS_CFG]		= 0x08,
+	[BRCMNAND_CS_TIMING1]		= 0x10,
+	[BRCMNAND_CS_TIMING2]		= 0x14,
+};
+
+/* BRCMNAND_INTFC_STATUS */
+enum {
+	INTFC_FLASH_STATUS		= GENMASK(7, 0),
+
+	INTFC_ERASED			= BIT(27),
+	INTFC_OOB_VALID			= BIT(28),
+	INTFC_CACHE_VALID		= BIT(29),
+	INTFC_FLASH_READY		= BIT(30),
+	INTFC_CTLR_READY		= BIT(31),
+};
+
+static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
+{
+	return __raw_readl(ctrl->nand_base + offs);
+}
+
+static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
+				 u32 val)
+{
+	__raw_writel(val, ctrl->nand_base + offs);
+}
+
+static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
+{
+	static const unsigned int block_sizes_v6[] = { 8, 16, 128, 256, 512, 1024, 2048, 0 };
+	static const unsigned int block_sizes_v4[] = { 16, 128, 8, 512, 256, 1024, 2048, 0 };
+	static const unsigned int page_sizes[] = { 512, 2048, 4096, 8192, 0 };
+
+	ctrl->nand_version = nand_readreg(ctrl, 0) & 0xffff;
+
+	/* Only support v4.0+? */
+	if (ctrl->nand_version < 0x0400) {
+		dev_err(ctrl->dev, "version %#x not supported\n",
+			ctrl->nand_version);
+		return -ENODEV;
+	}
+
+	/* Register offsets */
+	if (ctrl->nand_version >= 0x0600)
+		ctrl->reg_offsets = brcmnand_regs_v60;
+	else if (ctrl->nand_version >= 0x0500)
+		ctrl->reg_offsets = brcmnand_regs_v50;
+	else if (ctrl->nand_version >= 0x0400)
+		ctrl->reg_offsets = brcmnand_regs_v40;
+
+	/* Chip-select stride */
+	if (ctrl->nand_version >= 0x0701)
+		ctrl->reg_spacing = 0x14;
+	else
+		ctrl->reg_spacing = 0x10;
+
+	/* Per chip-select registers */
+	if (ctrl->nand_version >= 0x0701) {
+		ctrl->cs_offsets = brcmnand_cs_offsets_v71;
+	} else {
+		ctrl->cs_offsets = brcmnand_cs_offsets;
+
+		/* v5.0 and earlier has a different CS0 offset layout */
+		if (ctrl->nand_version <= 0x0500)
+			ctrl->cs0_offsets = brcmnand_cs_offsets_cs0;
+	}
+
+	/* Page / block sizes */
+	if (ctrl->nand_version >= 0x0701) {
+		/* >= v7.1 use nice power-of-2 values! */
+		ctrl->max_page_size = 16 * 1024;
+		ctrl->max_block_size = 2 * 1024 * 1024;
+	} else {
+		ctrl->page_sizes = page_sizes;
+		if (ctrl->nand_version >= 0x0600)
+			ctrl->block_sizes = block_sizes_v6;
+		else
+			ctrl->block_sizes = block_sizes_v4;
+
+		if (ctrl->nand_version < 0x0400) {
+			ctrl->max_page_size = 4096;
+			ctrl->max_block_size = 512 * 1024;
+		}
+	}
+
+	/* Maximum spare area sector size (per 512B) */
+	if (ctrl->nand_version >= 0x0600)
+		ctrl->max_oob = 64;
+	else if (ctrl->nand_version >= 0x0500)
+		ctrl->max_oob = 32;
+	else
+		ctrl->max_oob = 16;
+
+	/* v6.0 and newer (except v6.1) have prefetch support */
+	if (ctrl->nand_version >= 0x0600 && ctrl->nand_version != 0x0601)
+		ctrl->features |= BRCMNAND_HAS_PREFETCH;
+
+	/*
+	 * v6.x has cache mode, but it's implemented differently. Ignore it for
+	 * now.
+	 */
+	if (ctrl->nand_version >= 0x0700)
+		ctrl->features |= BRCMNAND_HAS_CACHE_MODE;
+
+	if (ctrl->nand_version >= 0x0500)
+		ctrl->features |= BRCMNAND_HAS_1K_SECTORS;
+
+	if (ctrl->nand_version >= 0x0700)
+		ctrl->features |= BRCMNAND_HAS_WP;
+	else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
+		ctrl->features |= BRCMNAND_HAS_WP;
+
+	return 0;
+}
+
+static inline u32 brcmnand_read_reg(struct brcmnand_controller *ctrl,
+		enum brcmnand_reg reg)
+{
+	u16 offs = ctrl->reg_offsets[reg];
+
+	if (offs)
+		return nand_readreg(ctrl, offs);
+	else
+		return 0;
+}
+
+static inline void brcmnand_write_reg(struct brcmnand_controller *ctrl,
+				      enum brcmnand_reg reg, u32 val)
+{
+	u16 offs = ctrl->reg_offsets[reg];
+
+	if (offs)
+		nand_writereg(ctrl, offs, val);
+}
+
+static inline void brcmnand_rmw_reg(struct brcmnand_controller *ctrl,
+				    enum brcmnand_reg reg, u32 mask, unsigned
+				    int shift, u32 val)
+{
+	u32 tmp = brcmnand_read_reg(ctrl, reg);
+
+	tmp &= ~mask;
+	tmp |= val << shift;
+	brcmnand_write_reg(ctrl, reg, tmp);
+}
+
+static inline u32 brcmnand_read_fc(struct brcmnand_controller *ctrl, int word)
+{
+	return __raw_readl(ctrl->nand_fc + word * 4);
+}
+
+static inline void brcmnand_write_fc(struct brcmnand_controller *ctrl,
+				     int word, u32 val)
+{
+	__raw_writel(val, ctrl->nand_fc + word * 4);
+}
+
+static inline u16 brcmnand_cs_offset(struct brcmnand_controller *ctrl, int cs,
+				     enum brcmnand_cs_reg reg)
+{
+	u16 offs_cs0 = ctrl->reg_offsets[BRCMNAND_CS0_BASE];
+	u16 offs_cs1 = ctrl->reg_offsets[BRCMNAND_CS1_BASE];
+	u8 cs_offs;
+
+	if (cs == 0 && ctrl->cs0_offsets)
+		cs_offs = ctrl->cs0_offsets[reg];
+	else
+		cs_offs = ctrl->cs_offsets[reg];
+
+	if (cs && offs_cs1)
+		return offs_cs1 + (cs - 1) * ctrl->reg_spacing + cs_offs;
+
+	return offs_cs0 + cs * ctrl->reg_spacing + cs_offs;
+}
+
+static inline u32 brcmnand_count_corrected(struct brcmnand_controller *ctrl)
+{
+	if (ctrl->nand_version < 0x0600)
+		return 1;
+	return brcmnand_read_reg(ctrl, BRCMNAND_CORR_COUNT);
+}
+
+static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	unsigned int shift = 0, bits;
+	enum brcmnand_reg reg = BRCMNAND_CORR_THRESHOLD;
+	int cs = host->cs;
+
+	if (ctrl->nand_version >= 0x0600)
+		bits = 6;
+	else if (ctrl->nand_version >= 0x0500)
+		bits = 5;
+	else
+		bits = 4;
+
+	if (ctrl->nand_version >= 0x0600) {
+		if (cs >= 5)
+			reg = BRCMNAND_CORR_THRESHOLD_EXT;
+		shift = (cs % 5) * bits;
+	}
+	brcmnand_rmw_reg(ctrl, reg, (bits - 1) << shift, shift, val);
+}
+
+static inline int brcmnand_cmd_shift(struct brcmnand_controller *ctrl)
+{
+	if (ctrl->nand_version < 0x0700)
+		return 24;
+	return 0;
+}
+
+/***********************************************************************
+ * NAND ACC CONTROL bitfield
+ *
+ * Some bits have remained constant throughout hardware revision, while
+ * others have shifted around.
+ ***********************************************************************/
+
+/* Constant for all versions (where supported) */
+enum {
+	/* See BRCMNAND_HAS_CACHE_MODE */
+	ACC_CONTROL_CACHE_MODE				= BIT(22),
+
+	/* See BRCMNAND_HAS_PREFETCH */
+	ACC_CONTROL_PREFETCH				= BIT(23),
+
+	ACC_CONTROL_PAGE_HIT				= BIT(24),
+	ACC_CONTROL_WR_PREEMPT				= BIT(25),
+	ACC_CONTROL_PARTIAL_PAGE			= BIT(26),
+	ACC_CONTROL_RD_ERASED				= BIT(27),
+	ACC_CONTROL_FAST_PGM_RDIN			= BIT(28),
+	ACC_CONTROL_WR_ECC				= BIT(30),
+	ACC_CONTROL_RD_ECC				= BIT(31),
+};
+
+static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
+{
+	if (ctrl->nand_version >= 0x0600)
+		return GENMASK(6, 0);
+	else
+		return GENMASK(5, 0);
+}
+
+#define NAND_ACC_CONTROL_ECC_SHIFT	16
+
+static inline u32 brcmnand_ecc_level_mask(struct brcmnand_controller *ctrl)
+{
+	u32 mask = (ctrl->nand_version >= 0x0600) ? 0x1f : 0x0f;
+
+	return mask << NAND_ACC_CONTROL_ECC_SHIFT;
+}
+
+static void brcmnand_set_ecc_enabled(struct brcmnand_host *host, int en)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u16 offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_ACC_CONTROL);
+	u32 acc_control = nand_readreg(ctrl, offs);
+	u32 ecc_flags = ACC_CONTROL_WR_ECC | ACC_CONTROL_RD_ECC;
+
+	if (en) {
+		acc_control |= ecc_flags; /* enable RD/WR ECC */
+		acc_control |= host->hwcfg.ecc_level
+			       << NAND_ACC_CONTROL_ECC_SHIFT;
+	} else {
+		acc_control &= ~ecc_flags; /* disable RD/WR ECC */
+		acc_control &= ~brcmnand_ecc_level_mask(ctrl);
+	}
+
+	nand_writereg(ctrl, offs, acc_control);
+}
+
+static inline int brcmnand_sector_1k_shift(struct brcmnand_controller *ctrl)
+{
+	if (ctrl->nand_version >= 0x0600)
+		return 7;
+	else if (ctrl->nand_version >= 0x0500)
+		return 6;
+	else
+		return -1;
+}
+
+static int brcmnand_get_sector_size_1k(struct brcmnand_host *host)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	int shift = brcmnand_sector_1k_shift(ctrl);
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+						  BRCMNAND_CS_ACC_CONTROL);
+
+	if (shift < 0)
+		return 0;
+
+	return (nand_readreg(ctrl, acc_control_offs) >> shift) & 0x1;
+}
+
+static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	int shift = brcmnand_sector_1k_shift(ctrl);
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+						  BRCMNAND_CS_ACC_CONTROL);
+	u32 tmp;
+
+	if (shift < 0)
+		return;
+
+	tmp = nand_readreg(ctrl, acc_control_offs);
+	tmp &= ~(1 << shift);
+	tmp |= (!!val) << shift;
+	nand_writereg(ctrl, acc_control_offs, tmp);
+}
+
+/***********************************************************************
+ * CS_NAND_SELECT
+ ***********************************************************************/
+
+enum {
+	CS_SELECT_NAND_WP			= BIT(29),
+	CS_SELECT_AUTO_DEVICE_ID_CFG		= BIT(30),
+};
+
+static inline void brcmnand_set_wp(struct brcmnand_controller *ctrl, bool en)
+{
+	u32 val = en ? CS_SELECT_NAND_WP : 0;
+
+	brcmnand_rmw_reg(ctrl, BRCMNAND_CS_SELECT, CS_SELECT_NAND_WP, 0, val);
+}
+
+/***********************************************************************
+ * Flash DMA
+ ***********************************************************************/
+
+enum flash_dma_reg {
+	FLASH_DMA_REVISION		= 0x00,
+	FLASH_DMA_FIRST_DESC		= 0x04,
+	FLASH_DMA_FIRST_DESC_EXT	= 0x08,
+	FLASH_DMA_CTRL			= 0x0c,
+	FLASH_DMA_MODE			= 0x10,
+	FLASH_DMA_STATUS		= 0x14,
+	FLASH_DMA_INTERRUPT_DESC	= 0x18,
+	FLASH_DMA_INTERRUPT_DESC_EXT	= 0x1c,
+	FLASH_DMA_ERROR_STATUS		= 0x20,
+	FLASH_DMA_CURRENT_DESC		= 0x24,
+	FLASH_DMA_CURRENT_DESC_EXT	= 0x28,
+};
+
+static inline bool has_flash_dma(struct brcmnand_controller *ctrl)
+{
+	return ctrl->flash_dma_base;
+}
+
+static inline bool flash_dma_buf_ok(const void *buf)
+{
+	return buf && !is_vmalloc_addr(buf) &&
+		likely(IS_ALIGNED((uintptr_t)buf, 4));
+}
+
+static inline void flash_dma_writel(struct brcmnand_controller *ctrl, u8 offs,
+				    u32 val)
+{
+	__raw_writel(val, ctrl->flash_dma_base + offs);
+}
+
+static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl, u8 offs)
+{
+	return __raw_readl(ctrl->flash_dma_base + offs);
+}
+
+/* Low-level operation types: command, address, write, or read */
+enum brcmnand_llop_type {
+	LL_OP_CMD,
+	LL_OP_ADDR,
+	LL_OP_WR,
+	LL_OP_RD,
+};
+
+/***********************************************************************
+ * Internal support functions
+ ***********************************************************************/
+
+static inline bool is_hamming_ecc(struct brcmnand_cfg *cfg)
+{
+	return cfg->sector_size_1k == 0 && cfg->spare_area_size == 16 &&
+		cfg->ecc_level == 15;
+}
+
+/*
+ * Returns a nand_ecclayout strucutre for the given layout/configuration.
+ * Returns NULL on failure.
+ */
+static struct nand_ecclayout *brcmnand_create_layout(int ecc_level,
+						     struct brcmnand_host *host)
+{
+	struct brcmnand_cfg *cfg = &host->hwcfg;
+	int i, j;
+	struct nand_ecclayout *layout;
+	int req;
+	int sectors;
+	int sas;
+	int idx1, idx2;
+
+	layout = devm_kzalloc(&host->pdev->dev, sizeof(*layout), GFP_KERNEL);
+	if (!layout)
+		return NULL;
+
+	sectors = cfg->page_size / (512 << cfg->sector_size_1k);
+	sas = cfg->spare_area_size << cfg->sector_size_1k;
+
+	/* Hamming */
+	if (is_hamming_ecc(cfg)) {
+		for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) {
+			/* First sector of each page may have BBI */
+			if (i == 0) {
+				layout->oobfree[idx2].offset = i * sas + 1;
+				/* Small-page NAND use byte 6 for BBI */
+				if (cfg->page_size == 512)
+					layout->oobfree[idx2].offset--;
+				layout->oobfree[idx2].length = 5;
+			} else {
+				layout->oobfree[idx2].offset = i * sas;
+				layout->oobfree[idx2].length = 6;
+			}
+			idx2++;
+			layout->eccpos[idx1++] = i * sas + 6;
+			layout->eccpos[idx1++] = i * sas + 7;
+			layout->eccpos[idx1++] = i * sas + 8;
+			layout->oobfree[idx2].offset = i * sas + 9;
+			layout->oobfree[idx2].length = 7;
+			idx2++;
+			/* Leave zero-terminated entry for OOBFREE */
+			if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE ||
+				    idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1)
+				break;
+		}
+		goto out;
+	}
+
+	/*
+	 * CONTROLLER_VERSION:
+	 *   < v5.0: ECC_REQ = ceil(BCH_T * 13/8)
+	 *  >= v5.0: ECC_REQ = ceil(BCH_T * 14/8)
+	 * But we will just be conservative.
+	 */
+	req = DIV_ROUND_UP(ecc_level * 14, 8);
+	if (req >= sas) {
+		dev_err(&host->pdev->dev,
+			"error: ECC too large for OOB (ECC bytes %d, spare sector %d)\n",
+			req, sas);
+		return NULL;
+	}
+
+	layout->eccbytes = req * sectors;
+	for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) {
+		for (j = sas - req; j < sas && idx1 <
+				MTD_MAX_ECCPOS_ENTRIES_LARGE; j++, idx1++)
+			layout->eccpos[idx1] = i * sas + j;
+
+		/* First sector of each page may have BBI */
+		if (i == 0) {
+			if (cfg->page_size == 512 && (sas - req >= 6)) {
+				/* Small-page NAND use byte 6 for BBI */
+				layout->oobfree[idx2].offset = 0;
+				layout->oobfree[idx2].length = 5;
+				idx2++;
+				if (sas - req > 6) {
+					layout->oobfree[idx2].offset = 6;
+					layout->oobfree[idx2].length =
+						sas - req - 6;
+					idx2++;
+				}
+			} else if (sas > req + 1) {
+				layout->oobfree[idx2].offset = i * sas + 1;
+				layout->oobfree[idx2].length = sas - req - 1;
+				idx2++;
+			}
+		} else if (sas > req) {
+			layout->oobfree[idx2].offset = i * sas;
+			layout->oobfree[idx2].length = sas - req;
+			idx2++;
+		}
+		/* Leave zero-terminated entry for OOBFREE */
+		if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE ||
+				idx2 >= MTD_MAX_OOBFREE_ENTRIES_LARGE - 1)
+			break;
+	}
+out:
+	/* Sum available OOB */
+	for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES_LARGE; i++)
+		layout->oobavail += layout->oobfree[i].length;
+	return layout;
+}
+
+static struct nand_ecclayout *brcmstb_choose_ecc_layout(
+		struct brcmnand_host *host)
+{
+	struct nand_ecclayout *layout;
+	struct brcmnand_cfg *p = &host->hwcfg;
+	unsigned int ecc_level = p->ecc_level;
+
+	if (p->sector_size_1k)
+		ecc_level <<= 1;
+
+	layout = brcmnand_create_layout(ecc_level, host);
+	if (!layout) {
+		dev_err(&host->pdev->dev,
+				"no proper ecc_layout for this NAND cfg\n");
+		return NULL;
+	}
+
+	return layout;
+}
+
+static void brcmnand_wp(struct mtd_info *mtd, int wp)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+
+	if ((ctrl->features & BRCMNAND_HAS_WP) && wp_on == 1) {
+		static int old_wp = -1;
+
+		if (old_wp != wp) {
+			dev_dbg(ctrl->dev, "WP %s\n", wp ? "on" : "off");
+			old_wp = wp;
+		}
+		brcmnand_set_wp(ctrl, wp);
+	}
+}
+
+/* Helper functions for reading and writing OOB registers */
+static inline u8 oob_reg_read(struct brcmnand_controller *ctrl, u32 offs)
+{
+	u16 offset0, offset10, reg_offs;
+
+	offset0 = ctrl->reg_offsets[BRCMNAND_OOB_READ_BASE];
+	offset10 = ctrl->reg_offsets[BRCMNAND_OOB_READ_10_BASE];
+
+	if (offs >= ctrl->max_oob)
+		return 0x77;
+
+	if (offs >= 16 && offset10)
+		reg_offs = offset10 + ((offs - 0x10) & ~0x03);
+	else
+		reg_offs = offset0 + (offs & ~0x03);
+
+	return nand_readreg(ctrl, reg_offs) >> (24 - ((offs & 0x03) << 3));
+}
+
+static inline void oob_reg_write(struct brcmnand_controller *ctrl, u32 offs,
+				 u32 data)
+{
+	u16 offset0, offset10, reg_offs;
+
+	offset0 = ctrl->reg_offsets[BRCMNAND_OOB_WRITE_BASE];
+	offset10 = ctrl->reg_offsets[BRCMNAND_OOB_WRITE_10_BASE];
+
+	if (offs >= ctrl->max_oob)
+		return;
+
+	if (offs >= 16 && offset10)
+		reg_offs = offset10 + ((offs - 0x10) & ~0x03);
+	else
+		reg_offs = offset0 + (offs & ~0x03);
+
+	nand_writereg(ctrl, reg_offs, data);
+}
+
+/*
+ * read_oob_from_regs - read data from OOB registers
+ * @ctrl: NAND controller
+ * @i: sub-page sector index
+ * @oob: buffer to read to
+ * @sas: spare area sector size (i.e., OOB size per FLASH_CACHE)
+ * @sector_1k: 1 for 1KiB sectors, 0 for 512B, other values are illegal
+ */
+static int read_oob_from_regs(struct brcmnand_controller *ctrl, int i, u8 *oob,
+			      int sas, int sector_1k)
+{
+	int tbytes = sas << sector_1k;
+	int j;
+
+	/* Adjust OOB values for 1K sector size */
+	if (sector_1k && (i & 0x01))
+		tbytes = max(0, tbytes - (int)ctrl->max_oob);
+	tbytes = min_t(int, tbytes, ctrl->max_oob);
+
+	for (j = 0; j < tbytes; j++)
+		oob[j] = oob_reg_read(ctrl, j);
+	return tbytes;
+}
+
+/*
+ * write_oob_to_regs - write data to OOB registers
+ * @i: sub-page sector index
+ * @oob: buffer to write from
+ * @sas: spare area sector size (i.e., OOB size per FLASH_CACHE)
+ * @sector_1k: 1 for 1KiB sectors, 0 for 512B, other values are illegal
+ */
+static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
+			     const u8 *oob, int sas, int sector_1k)
+{
+	int tbytes = sas << sector_1k;
+	int j;
+
+	/* Adjust OOB values for 1K sector size */
+	if (sector_1k && (i & 0x01))
+		tbytes = max(0, tbytes - (int)ctrl->max_oob);
+	tbytes = min_t(int, tbytes, ctrl->max_oob);
+
+	for (j = 0; j < tbytes; j += 4)
+		oob_reg_write(ctrl, j,
+				(oob[j + 0] << 24) |
+				(oob[j + 1] << 16) |
+				(oob[j + 2] <<  8) |
+				(oob[j + 3] <<  0));
+	return tbytes;
+}
+
+static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
+{
+	struct brcmnand_controller *ctrl = data;
+
+	/* Discard all NAND_CTLRDY interrupts during DMA */
+	if (ctrl->dma_pending)
+		return IRQ_HANDLED;
+
+	complete(&ctrl->done);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t brcmnand_dma_irq(int irq, void *data)
+{
+	struct brcmnand_controller *ctrl = data;
+
+	complete(&ctrl->dma_done);
+
+	return IRQ_HANDLED;
+}
+
+static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u32 intfc;
+
+	dev_dbg(ctrl->dev, "send native cmd %d addr_lo 0x%x\n", cmd,
+		brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS));
+	BUG_ON(ctrl->cmd_pending != 0);
+	ctrl->cmd_pending = cmd;
+
+	intfc = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS);
+	BUG_ON(!(intfc & INTFC_CTLR_READY));
+
+	mb(); /* flush previous writes */
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_START,
+			   cmd << brcmnand_cmd_shift(ctrl));
+}
+
+/***********************************************************************
+ * NAND MTD API: read/program/erase
+ ***********************************************************************/
+
+static void brcmnand_cmd_ctrl(struct mtd_info *mtd, int dat,
+	unsigned int ctrl)
+{
+	/* intentionally left blank */
+}
+
+static int brcmnand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	unsigned long timeo = msecs_to_jiffies(100);
+
+	dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending);
+	if (ctrl->cmd_pending &&
+			wait_for_completion_timeout(&ctrl->done, timeo) <= 0) {
+		u32 cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START)
+					>> brcmnand_cmd_shift(ctrl);
+
+		dev_err_ratelimited(ctrl->dev,
+			"timeout waiting for command %#02x\n", cmd);
+		dev_err_ratelimited(ctrl->dev, "intfc status %08x\n",
+			brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS));
+	}
+	ctrl->cmd_pending = 0;
+	return brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) &
+				 INTFC_FLASH_STATUS;
+}
+
+enum {
+	LLOP_RE				= BIT(16),
+	LLOP_WE				= BIT(17),
+	LLOP_ALE			= BIT(18),
+	LLOP_CLE			= BIT(19),
+	LLOP_RETURN_IDLE		= BIT(31),
+
+	LLOP_DATA_MASK			= GENMASK(15, 0),
+};
+
+static int brcmnand_low_level_op(struct brcmnand_host *host,
+				 enum brcmnand_llop_type type, u32 data,
+				 bool last_op)
+{
+	struct mtd_info *mtd = &host->mtd;
+	struct nand_chip *chip = &host->chip;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u32 tmp;
+
+	tmp = data & LLOP_DATA_MASK;
+	switch (type) {
+	case LL_OP_CMD:
+		tmp |= LLOP_WE | LLOP_CLE;
+		break;
+	case LL_OP_ADDR:
+		/* WE | ALE */
+		tmp |= LLOP_WE | LLOP_ALE;
+		break;
+	case LL_OP_WR:
+		/* WE */
+		tmp |= LLOP_WE;
+		break;
+	case LL_OP_RD:
+		/* RE */
+		tmp |= LLOP_RE;
+		break;
+	}
+	if (last_op)
+		/* RETURN_IDLE */
+		tmp |= LLOP_RETURN_IDLE;
+
+	dev_dbg(ctrl->dev, "ll_op cmd %#x\n", tmp);
+
+	brcmnand_write_reg(ctrl, BRCMNAND_LL_OP, tmp);
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_LL_OP);
+
+	brcmnand_send_cmd(host, CMD_LOW_LEVEL_OP);
+	return brcmnand_waitfunc(mtd, chip);
+}
+
+static void brcmnand_cmdfunc(struct mtd_info *mtd, unsigned command,
+			     int column, int page_addr)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u64 addr = (u64)page_addr << chip->page_shift;
+	int native_cmd = 0;
+
+	if (command == NAND_CMD_READID || command == NAND_CMD_PARAM ||
+			command == NAND_CMD_RNDOUT)
+		addr = (u64)column;
+	/* Avoid propagating a negative, don't-care address */
+	else if (page_addr < 0)
+		addr = 0;
+
+	dev_dbg(ctrl->dev, "cmd 0x%x addr 0x%llx\n", command,
+		(unsigned long long)addr);
+
+	host->last_cmd = command;
+	host->last_byte = 0;
+	host->last_addr = addr;
+
+	switch (command) {
+	case NAND_CMD_RESET:
+		native_cmd = CMD_FLASH_RESET;
+		break;
+	case NAND_CMD_STATUS:
+		native_cmd = CMD_STATUS_READ;
+		break;
+	case NAND_CMD_READID:
+		native_cmd = CMD_DEVICE_ID_READ;
+		break;
+	case NAND_CMD_READOOB:
+		native_cmd = CMD_SPARE_AREA_READ;
+		break;
+	case NAND_CMD_ERASE1:
+		native_cmd = CMD_BLOCK_ERASE;
+		brcmnand_wp(mtd, 0);
+		break;
+	case NAND_CMD_PARAM:
+		native_cmd = CMD_PARAMETER_READ;
+		break;
+	case NAND_CMD_SET_FEATURES:
+	case NAND_CMD_GET_FEATURES:
+		brcmnand_low_level_op(host, LL_OP_CMD, command, false);
+		brcmnand_low_level_op(host, LL_OP_ADDR, column, false);
+		break;
+	case NAND_CMD_RNDOUT:
+		native_cmd = CMD_PARAMETER_CHANGE_COL;
+		addr &= ~((u64)(FC_BYTES - 1));
+		/*
+		 * HW quirk: PARAMETER_CHANGE_COL requires SECTOR_SIZE_1K=0
+		 * NB: hwcfg.sector_size_1k may not be initialized yet
+		 */
+		if (brcmnand_get_sector_size_1k(host)) {
+			host->hwcfg.sector_size_1k =
+				brcmnand_get_sector_size_1k(host);
+			brcmnand_set_sector_size_1k(host, 0);
+		}
+		break;
+	}
+
+	if (!native_cmd)
+		return;
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
+		(host->cs << 16) | ((addr >> 32) & 0xffff));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS, lower_32_bits(addr));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+
+	brcmnand_send_cmd(host, native_cmd);
+	brcmnand_waitfunc(mtd, chip);
+
+	if (native_cmd == CMD_PARAMETER_READ ||
+			native_cmd == CMD_PARAMETER_CHANGE_COL) {
+		int i;
+		/*
+		 * Must cache the FLASH_CACHE now, since changes in
+		 * SECTOR_SIZE_1K may invalidate it
+		 */
+		for (i = 0; i < FC_WORDS; i++)
+			ctrl->flash_cache[i] = brcmnand_read_fc(ctrl, i);
+		/* Cleanup from HW quirk: restore SECTOR_SIZE_1K */
+		if (host->hwcfg.sector_size_1k)
+			brcmnand_set_sector_size_1k(host,
+						    host->hwcfg.sector_size_1k);
+	}
+
+	/* Re-enable protection is necessary only after erase */
+	if (command == NAND_CMD_ERASE1)
+		brcmnand_wp(mtd, 1);
+}
+
+static uint8_t brcmnand_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	uint8_t ret = 0;
+	int addr, offs;
+
+	switch (host->last_cmd) {
+	case NAND_CMD_READID:
+		if (host->last_byte < 4)
+			ret = brcmnand_read_reg(ctrl, BRCMNAND_ID) >>
+				(24 - (host->last_byte << 3));
+		else if (host->last_byte < 8)
+			ret = brcmnand_read_reg(ctrl, BRCMNAND_ID_EXT) >>
+				(56 - (host->last_byte << 3));
+		break;
+
+	case NAND_CMD_READOOB:
+		ret = oob_reg_read(ctrl, host->last_byte);
+		break;
+
+	case NAND_CMD_STATUS:
+		ret = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS) &
+					INTFC_FLASH_STATUS;
+		if (wp_on) /* hide WP status */
+			ret |= NAND_STATUS_WP;
+		break;
+
+	case NAND_CMD_PARAM:
+	case NAND_CMD_RNDOUT:
+		addr = host->last_addr + host->last_byte;
+		offs = addr & (FC_BYTES - 1);
+
+		/* At FC_BYTES boundary, switch to next column */
+		if (host->last_byte > 0 && offs == 0)
+			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, addr, -1);
+
+		ret = ctrl->flash_cache[offs >> 2] >>
+					(24 - ((offs & 0x03) << 3));
+		break;
+	case NAND_CMD_GET_FEATURES:
+		if (host->last_byte >= ONFI_SUBFEATURE_PARAM_LEN) {
+			ret = 0;
+		} else {
+			bool last = host->last_byte ==
+				ONFI_SUBFEATURE_PARAM_LEN - 1;
+			brcmnand_low_level_op(host, LL_OP_RD, 0, last);
+			ret = brcmnand_read_reg(ctrl, BRCMNAND_LL_RDATA) & 0xff;
+		}
+	}
+
+	dev_dbg(ctrl->dev, "read byte = 0x%02x\n", ret);
+	host->last_byte++;
+
+	return ret;
+}
+
+static void brcmnand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++, buf++)
+		*buf = brcmnand_read_byte(mtd);
+}
+
+static void brcmnand_write_buf(struct mtd_info *mtd, const uint8_t *buf,
+				   int len)
+{
+	int i;
+	struct nand_chip *chip = mtd->priv;
+	struct brcmnand_host *host = chip->priv;
+
+	switch (host->last_cmd) {
+	case NAND_CMD_SET_FEATURES:
+		for (i = 0; i < len; i++)
+			brcmnand_low_level_op(host, LL_OP_WR, buf[i],
+						  (i + 1) == len);
+		break;
+	default:
+		BUG();
+		break;
+	}
+}
+
+/**
+ * Construct a FLASH_DMA descriptor as part of a linked list. You must know the
+ * following ahead of time:
+ *  - Is this descriptor the beginning or end of a linked list?
+ *  - What is the (DMA) address of the next descriptor in the linked list?
+ */
+static int brcmnand_fill_dma_desc(struct brcmnand_host *host,
+				  struct brcm_nand_dma_desc *desc, u64 addr,
+				  dma_addr_t buf, u32 len, u8 dma_cmd,
+				  bool begin, bool end,
+				  dma_addr_t next_desc)
+{
+	memset(desc, 0, sizeof(*desc));
+	/* Descriptors are written in native byte order (wordwise) */
+	desc->next_desc = lower_32_bits(next_desc);
+	desc->next_desc_ext = upper_32_bits(next_desc);
+	desc->cmd_irq = (dma_cmd << 24) |
+		(end ? (0x03 << 8) : 0) | /* IRQ | STOP */
+		(!!begin) | ((!!end) << 1); /* head, tail */
+#ifdef CONFIG_CPU_BIG_ENDIAN
+	desc->cmd_irq |= 0x01 << 12;
+#endif
+	desc->dram_addr = lower_32_bits(buf);
+	desc->dram_addr_ext = upper_32_bits(buf);
+	desc->tfr_len = len;
+	desc->total_len = len;
+	desc->flash_addr = lower_32_bits(addr);
+	desc->flash_addr_ext = upper_32_bits(addr);
+	desc->cs = host->cs;
+	desc->status_valid = 0x01;
+	return 0;
+}
+
+/**
+ * Kick the FLASH_DMA engine, with a given DMA descriptor
+ */
+static void brcmnand_dma_run(struct brcmnand_host *host, dma_addr_t desc)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	unsigned long timeo = msecs_to_jiffies(100);
+
+	flash_dma_writel(ctrl, FLASH_DMA_FIRST_DESC, lower_32_bits(desc));
+	(void)flash_dma_readl(ctrl, FLASH_DMA_FIRST_DESC);
+	flash_dma_writel(ctrl, FLASH_DMA_FIRST_DESC_EXT, upper_32_bits(desc));
+	(void)flash_dma_readl(ctrl, FLASH_DMA_FIRST_DESC_EXT);
+
+	/* Start FLASH_DMA engine */
+	ctrl->dma_pending = true;
+	mb(); /* flush previous writes */
+	flash_dma_writel(ctrl, FLASH_DMA_CTRL, 0x03); /* wake | run */
+
+	if (wait_for_completion_timeout(&ctrl->dma_done, timeo) <= 0) {
+		dev_err(ctrl->dev,
+				"timeout waiting for DMA; status %#x, error status %#x\n",
+				flash_dma_readl(ctrl, FLASH_DMA_STATUS),
+				flash_dma_readl(ctrl, FLASH_DMA_ERROR_STATUS));
+	}
+	ctrl->dma_pending = false;
+	flash_dma_writel(ctrl, FLASH_DMA_CTRL, 0); /* force stop */
+}
+
+static int brcmnand_dma_trans(struct brcmnand_host *host, u64 addr, u32 *buf,
+			      u32 len, u8 dma_cmd)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	dma_addr_t buf_pa;
+	int dir = dma_cmd == CMD_PAGE_READ ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+	buf_pa = dma_map_single(ctrl->dev, buf, len, dir);
+	if (dma_mapping_error(ctrl->dev, buf_pa)) {
+		dev_err(ctrl->dev, "unable to map buffer for DMA\n");
+		return -ENOMEM;
+	}
+
+	brcmnand_fill_dma_desc(host, ctrl->dma_desc, addr, buf_pa, len,
+				   dma_cmd, true, true, 0);
+
+	brcmnand_dma_run(host, ctrl->dma_pa);
+
+	dma_unmap_single(ctrl->dev, buf_pa, len, dir);
+
+	if (ctrl->dma_desc->status_valid & FLASH_DMA_ECC_ERROR)
+		return -EBADMSG;
+	else if (ctrl->dma_desc->status_valid & FLASH_DMA_CORR_ERROR)
+		return -EUCLEAN;
+
+	return 0;
+}
+
+/*
+ * Assumes proper CS is already set
+ */
+static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
+				u64 addr, unsigned int trans, u32 *buf,
+				u8 *oob, u64 *err_addr)
+{
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	int i, j, ret = 0;
+
+	/* Clear error addresses */
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_ADDR, 0);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_ADDR, 0);
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
+			(host->cs << 16) | ((addr >> 32) & 0xffff));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+
+	for (i = 0; i < trans; i++, addr += FC_BYTES) {
+		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
+				   lower_32_bits(addr));
+		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+		/* SPARE_AREA_READ does not use ECC, so just use PAGE_READ */
+		brcmnand_send_cmd(host, CMD_PAGE_READ);
+		brcmnand_waitfunc(mtd, chip);
+
+		if (likely(buf))
+			for (j = 0; j < FC_WORDS; j++, buf++)
+				*buf = brcmnand_read_fc(ctrl, j);
+
+		if (oob)
+			oob += read_oob_from_regs(ctrl, i, oob,
+					mtd->oobsize / trans,
+					host->hwcfg.sector_size_1k);
+
+		if (!ret) {
+			*err_addr = brcmnand_read_reg(ctrl,
+					BRCMNAND_UNCORR_ADDR) |
+				((u64)(brcmnand_read_reg(ctrl,
+						BRCMNAND_UNCORR_EXT_ADDR)
+					& 0xffff) << 32);
+			if (*err_addr)
+				ret = -EBADMSG;
+		}
+
+		if (!ret) {
+			*err_addr = brcmnand_read_reg(ctrl,
+					BRCMNAND_CORR_ADDR) |
+				((u64)(brcmnand_read_reg(ctrl,
+						BRCMNAND_CORR_EXT_ADDR)
+					& 0xffff) << 32);
+			if (*err_addr)
+				ret = -EUCLEAN;
+		}
+	}
+
+	return ret;
+}
+
+static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
+			 u64 addr, unsigned int trans, u32 *buf, u8 *oob)
+{
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u64 err_addr = 0;
+	int err;
+
+	dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long long)addr, buf);
+
+	brcmnand_write_reg(ctrl, BRCMNAND_UNCORR_COUNT, 0);
+
+	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
+		err = brcmnand_dma_trans(host, addr, buf, trans * FC_BYTES,
+					     CMD_PAGE_READ);
+		if (err) {
+			if (mtd_is_bitflip_or_eccerr(err))
+				err_addr = addr;
+			else
+				return -EIO;
+		}
+	} else {
+		if (oob)
+			memset(oob, 0x99, mtd->oobsize);
+
+		err = brcmnand_read_by_pio(mtd, chip, addr, trans, buf,
+					       oob, &err_addr);
+	}
+
+	if (mtd_is_eccerr(err)) {
+		dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
+			(unsigned long long)err_addr);
+		mtd->ecc_stats.failed++;
+		/* NAND layer expects zero on ECC errors */
+		return 0;
+	}
+
+	if (mtd_is_bitflip(err)) {
+		unsigned int corrected = brcmnand_count_corrected(ctrl);
+
+		dev_dbg(ctrl->dev, "corrected error at 0x%llx\n",
+			(unsigned long long)err_addr);
+		mtd->ecc_stats.corrected += corrected;
+		/* Always exceed the software-imposed threshold */
+		return max(mtd->bitflip_threshold, corrected);
+	}
+
+	return 0;
+}
+
+static int brcmnand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			      uint8_t *buf, int oob_required, int page)
+{
+	struct brcmnand_host *host = chip->priv;
+	u8 *oob = oob_required ? (u8 *)chip->oob_poi : NULL;
+
+	return brcmnand_read(mtd, chip, host->last_addr,
+			mtd->writesize >> FC_SHIFT, (u32 *)buf, oob);
+}
+
+static int brcmnand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				  uint8_t *buf, int oob_required, int page)
+{
+	struct brcmnand_host *host = chip->priv;
+	u8 *oob = oob_required ? (u8 *)chip->oob_poi : NULL;
+	int ret;
+
+	brcmnand_set_ecc_enabled(host, 0);
+	ret = brcmnand_read(mtd, chip, host->last_addr,
+			mtd->writesize >> FC_SHIFT, (u32 *)buf, oob);
+	brcmnand_set_ecc_enabled(host, 1);
+	return ret;
+}
+
+static int brcmnand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			     int page)
+{
+	return brcmnand_read(mtd, chip, (u64)page << chip->page_shift,
+			mtd->writesize >> FC_SHIFT,
+			NULL, (u8 *)chip->oob_poi);
+}
+
+static int brcmnand_read_oob_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				 int page)
+{
+	struct brcmnand_host *host = chip->priv;
+
+	brcmnand_set_ecc_enabled(host, 0);
+	brcmnand_read(mtd, chip, (u64)page << chip->page_shift,
+		mtd->writesize >> FC_SHIFT,
+		NULL, (u8 *)chip->oob_poi);
+	brcmnand_set_ecc_enabled(host, 1);
+	return 0;
+}
+
+static int brcmnand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
+				 uint32_t data_offs, uint32_t readlen,
+				 uint8_t *bufpoi, int page)
+{
+	struct brcmnand_host *host = chip->priv;
+
+	return brcmnand_read(mtd, chip, host->last_addr + data_offs,
+			readlen >> FC_SHIFT, (u32 *)bufpoi, NULL);
+}
+
+static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
+			  u64 addr, const u32 *buf, u8 *oob)
+{
+	struct brcmnand_host *host = chip->priv;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	unsigned int i, j, trans = mtd->writesize >> FC_SHIFT;
+	int status, ret = 0;
+
+	dev_dbg(ctrl->dev, "write %llx <- %p\n", (unsigned long long)addr, buf);
+
+	if (unlikely((u32)buf & 0x03)) {
+		dev_warn(ctrl->dev, "unaligned buffer: %p\n", buf);
+		buf = (u32 *)((u32)buf & ~0x03);
+	}
+
+	brcmnand_wp(mtd, 0);
+
+	for (i = 0; i < ctrl->max_oob; i += 4)
+		oob_reg_write(ctrl, i, 0xffffffff);
+
+	if (has_flash_dma(ctrl) && !oob && flash_dma_buf_ok(buf)) {
+		if (brcmnand_dma_trans(host, addr, (u32 *)buf,
+					mtd->writesize, CMD_PROGRAM_PAGE))
+			ret = -EIO;
+		goto out;
+	}
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS,
+			(host->cs << 16) | ((addr >> 32) & 0xffff));
+	(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_EXT_ADDRESS);
+
+	for (i = 0; i < trans; i++, addr += FC_BYTES) {
+		/* full address MUST be set before populating FC */
+		brcmnand_write_reg(ctrl, BRCMNAND_CMD_ADDRESS,
+				   lower_32_bits(addr));
+		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
+
+		if (buf)
+			for (j = 0; j < FC_WORDS; j++, buf++)
+				brcmnand_write_fc(ctrl, j, *buf);
+		else if (oob)
+			for (j = 0; j < FC_WORDS; j++)
+				brcmnand_write_fc(ctrl, j, 0xffffffff);
+
+		if (oob) {
+			oob += write_oob_to_regs(ctrl, i, oob,
+					mtd->oobsize / trans,
+					host->hwcfg.sector_size_1k);
+		}
+
+		/* we cannot use SPARE_AREA_PROGRAM when PARTIAL_PAGE_EN=0 */
+		brcmnand_send_cmd(host, CMD_PROGRAM_PAGE);
+		status = brcmnand_waitfunc(mtd, chip);
+
+		if (status & NAND_STATUS_FAIL) {
+			dev_info(ctrl->dev, "program failed at %llx\n",
+				(unsigned long long)addr);
+			ret = -EIO;
+			goto out;
+		}
+	}
+out:
+	brcmnand_wp(mtd, 1);
+	return ret;
+}
+
+static int brcmnand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+			       const uint8_t *buf, int oob_required)
+{
+	struct brcmnand_host *host = chip->priv;
+	void *oob = oob_required ? chip->oob_poi : NULL;
+
+	brcmnand_write(mtd, chip, host->last_addr, (const u32 *)buf, oob);
+	return 0;
+}
+
+static int brcmnand_write_page_raw(struct mtd_info *mtd,
+				   struct nand_chip *chip, const uint8_t *buf,
+				   int oob_required)
+{
+	struct brcmnand_host *host = chip->priv;
+	void *oob = oob_required ? chip->oob_poi : NULL;
+
+	brcmnand_set_ecc_enabled(host, 0);
+	brcmnand_write(mtd, chip, host->last_addr, (const u32 *)buf, oob);
+	brcmnand_set_ecc_enabled(host, 1);
+	return 0;
+}
+
+static int brcmnand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+				  int page)
+{
+	return brcmnand_write(mtd, chip, (u64)page << chip->page_shift,
+				  NULL, chip->oob_poi);
+}
+
+static int brcmnand_write_oob_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				  int page)
+{
+	struct brcmnand_host *host = chip->priv;
+	int ret;
+
+	brcmnand_set_ecc_enabled(host, 0);
+	ret = brcmnand_write(mtd, chip, (u64)page << chip->page_shift, NULL,
+				 (u8 *)chip->oob_poi);
+	brcmnand_set_ecc_enabled(host, 1);
+
+	return ret;
+}
+
+/***********************************************************************
+ * Per-CS setup (1 NAND device)
+ ***********************************************************************/
+
+static int brcmnand_set_cfg(struct brcmnand_host *host,
+			    struct brcmnand_cfg *cfg)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	struct nand_chip *chip = &host->chip;
+	u16 cfg_offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_CFG);
+	u16 cfg_ext_offs = brcmnand_cs_offset(ctrl, host->cs,
+			BRCMNAND_CS_CFG_EXT);
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+			BRCMNAND_CS_ACC_CONTROL);
+	u8 block_size = 0, page_size = 0, device_size = 0;
+	u32 tmp;
+
+	if (ctrl->block_sizes) {
+		int i, found;
+
+		for (i = 0, found = 0; ctrl->block_sizes[i]; i++)
+			if (ctrl->block_sizes[i] * 1024 == cfg->block_size) {
+				block_size = i;
+				found = 1;
+			}
+		if (!found) {
+			dev_warn(ctrl->dev, "invalid block size %u\n",
+					cfg->block_size);
+			return -EINVAL;
+		}
+	} else {
+		block_size = ffs(cfg->block_size) - ffs(BRCMNAND_MIN_BLOCKSIZE);
+	}
+
+	if (cfg->block_size < BRCMNAND_MIN_BLOCKSIZE || (ctrl->max_block_size &&
+				cfg->block_size > ctrl->max_block_size)) {
+		dev_warn(ctrl->dev, "invalid block size %u\n",
+				cfg->block_size);
+		block_size = 0;
+	}
+
+	if (ctrl->page_sizes) {
+		int i, found;
+
+		for (i = 0, found = 0; ctrl->page_sizes[i]; i++)
+			if (ctrl->page_sizes[i] == cfg->page_size) {
+				page_size = i;
+				found = 1;
+			}
+		if (!found) {
+			dev_warn(ctrl->dev, "invalid page size %u\n",
+					cfg->page_size);
+			return -EINVAL;
+		}
+	} else {
+		page_size = ffs(cfg->page_size) - ffs(BRCMNAND_MIN_PAGESIZE);
+	}
+
+	if (cfg->page_size < BRCMNAND_MIN_PAGESIZE || (ctrl->max_page_size &&
+				cfg->page_size > ctrl->max_page_size)) {
+		dev_warn(ctrl->dev, "invalid page size %u\n", cfg->page_size);
+		return -EINVAL;
+	}
+
+	if (fls64(cfg->device_size) < fls64(BRCMNAND_MIN_DEVSIZE)) {
+		dev_warn(ctrl->dev, "invalid device size 0x%llx\n",
+			(unsigned long long)cfg->device_size);
+		return -EINVAL;
+	}
+	device_size = fls64(cfg->device_size) - fls64(BRCMNAND_MIN_DEVSIZE);
+
+	tmp = (cfg->blk_adr_bytes << 8) |
+		(cfg->col_adr_bytes << 12) |
+		(cfg->ful_adr_bytes << 16) |
+		(!!(cfg->device_width == 16) << 23) |
+		(device_size << 24);
+	if (cfg_offs == cfg_ext_offs) {
+		tmp |= (page_size << 20) | (block_size << 28);
+		nand_writereg(ctrl, cfg_offs, tmp);
+	} else {
+		nand_writereg(ctrl, cfg_offs, tmp);
+		tmp = page_size | (block_size << 4);
+		nand_writereg(ctrl, cfg_ext_offs, tmp);
+	}
+
+	tmp = nand_readreg(ctrl, acc_control_offs);
+	tmp &= ~brcmnand_ecc_level_mask(ctrl);
+	tmp |= cfg->ecc_level << NAND_ACC_CONTROL_ECC_SHIFT;
+	tmp &= ~brcmnand_spare_area_mask(ctrl);
+	tmp |= cfg->spare_area_size;
+	nand_writereg(ctrl, acc_control_offs, tmp);
+
+	brcmnand_set_sector_size_1k(host, cfg->sector_size_1k);
+
+	/* threshold = ceil(BCH-level * 0.75) */
+	brcmnand_wr_corr_thresh(host, DIV_ROUND_UP(chip->ecc.strength * 3, 4));
+
+	return 0;
+}
+
+static void brcmnand_print_cfg(char *buf, struct brcmnand_cfg *cfg)
+{
+	buf += sprintf(buf,
+		"%lluMiB total, %uKiB blocks, %u%s pages, %uB OOB, %u-bit",
+		(unsigned long long)cfg->device_size >> 20,
+		cfg->block_size >> 10,
+		cfg->page_size >= 1024 ? cfg->page_size >> 10 : cfg->page_size,
+		cfg->page_size >= 1024 ? "KiB" : "B",
+		cfg->spare_area_size, cfg->device_width);
+
+	/* Account for Hamming ECC and for BCH 512B vs 1KiB sectors */
+	if (is_hamming_ecc(cfg))
+		sprintf(buf, ", Hamming ECC");
+	else if (cfg->sector_size_1k)
+		sprintf(buf, ", BCH-%u (1KiB sector)", cfg->ecc_level << 1);
+	else
+		sprintf(buf, ", BCH-%u\n", cfg->ecc_level);
+}
+
+/*
+ * Minimum number of bytes to address a page. Calculated as:
+ *     roundup(log2(size / page-size) / 8)
+ *
+ * NB: the following does not "round up" for non-power-of-2 'size'; but this is
+ *     OK because many other things will break if 'size' is irregular...
+ */
+static inline int get_blk_adr_bytes(u64 size, u32 writesize)
+{
+	return ALIGN(ilog2(size) - ilog2(writesize), 8) >> 3;
+}
+
+static int brcmnand_setup_dev(struct brcmnand_host *host)
+{
+	struct mtd_info *mtd = &host->mtd;
+	struct nand_chip *chip = &host->chip;
+	struct brcmnand_controller *ctrl = host->ctrl;
+	struct brcmnand_cfg *cfg = &host->hwcfg;
+	char msg[128];
+	u32 offs, tmp, oob_sector;
+	int ret;
+
+	memset(cfg, 0, sizeof(*cfg));
+
+	ret = of_property_read_u32(chip->dn, "brcm,nand-oob-sector-size",
+				   &oob_sector);
+	if (ret) {
+		/* Use detected size */
+		cfg->spare_area_size = mtd->oobsize /
+					(mtd->writesize >> FC_SHIFT);
+	} else {
+		cfg->spare_area_size = oob_sector;
+	}
+	if (cfg->spare_area_size > ctrl->max_oob)
+		cfg->spare_area_size = ctrl->max_oob;
+	/*
+	 * Set oobsize to be consistent with controller's spare_area_size, as
+	 * the rest is inaccessible.
+	 */
+	mtd->oobsize = cfg->spare_area_size * (mtd->writesize >> FC_SHIFT);
+
+	cfg->device_size = mtd->size;
+	cfg->block_size = mtd->erasesize;
+	cfg->page_size = mtd->writesize;
+	cfg->device_width = (chip->options & NAND_BUSWIDTH_16) ? 16 : 8;
+	cfg->col_adr_bytes = 2;
+	cfg->blk_adr_bytes = get_blk_adr_bytes(mtd->size, mtd->writesize);
+
+	switch (chip->ecc.size) {
+	case 512:
+		if (chip->ecc.strength == 1) /* Hamming */
+			cfg->ecc_level = 15;
+		else
+			cfg->ecc_level = chip->ecc.strength;
+		cfg->sector_size_1k = 0;
+		break;
+	case 1024:
+		if (!(ctrl->features & BRCMNAND_HAS_1K_SECTORS)) {
+			dev_err(ctrl->dev, "1KB sectors not supported\n");
+			return -EINVAL;
+		}
+		if (chip->ecc.strength & 0x1) {
+			dev_err(ctrl->dev,
+				"odd ECC not supported with 1KB sectors\n");
+			return -EINVAL;
+		}
+
+		cfg->ecc_level = chip->ecc.strength >> 1;
+		cfg->sector_size_1k = 1;
+		break;
+	default:
+		dev_err(ctrl->dev, "unsupported ECC size: %d\n",
+			chip->ecc.size);
+		return -EINVAL;
+	}
+
+	cfg->ful_adr_bytes = cfg->blk_adr_bytes;
+	if (mtd->writesize > 512)
+		cfg->ful_adr_bytes += cfg->col_adr_bytes;
+	else
+		cfg->ful_adr_bytes += 1;
+
+	ret = brcmnand_set_cfg(host, cfg);
+	if (ret)
+		return ret;
+
+	brcmnand_set_ecc_enabled(host, 1);
+
+	brcmnand_print_cfg(msg, cfg);
+	dev_info(ctrl->dev, "detected %s\n", msg);
+
+	/* Configure ACC_CONTROL */
+	offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_ACC_CONTROL);
+	tmp = nand_readreg(ctrl, offs);
+	tmp &= ~ACC_CONTROL_PARTIAL_PAGE;
+	tmp &= ~ACC_CONTROL_RD_ERASED;
+	tmp &= ~ACC_CONTROL_FAST_PGM_RDIN;
+	if (ctrl->features & BRCMNAND_HAS_PREFETCH) {
+		/*
+		 * FIXME: Flash DMA + prefetch may see spurious erased-page ECC
+		 * errors
+		 */
+		if (has_flash_dma(ctrl))
+			tmp &= ~ACC_CONTROL_PREFETCH;
+		else
+			tmp |= ACC_CONTROL_PREFETCH;
+	}
+	nand_writereg(ctrl, offs, tmp);
+
+	return 0;
+}
+
+static int brcmnand_init_cs(struct brcmnand_host *host)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	struct device_node *dn = host->of_node;
+	struct platform_device *pdev = host->pdev;
+	struct mtd_info *mtd;
+	struct nand_chip *chip;
+	int ret = 0;
+	struct mtd_part_parser_data ppdata = { .of_node = dn };
+
+	ret = of_property_read_u32(dn, "reg", &host->cs);
+	if (ret) {
+		dev_err(&pdev->dev, "can't get chip-select\n");
+		return -ENXIO;
+	}
+
+	mtd = &host->mtd;
+	chip = &host->chip;
+
+	chip->dn = dn;
+	chip->priv = host;
+	mtd->priv = chip;
+	mtd->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "brcmnand.%d",
+				   host->cs);
+	mtd->owner = THIS_MODULE;
+	mtd->dev.parent = &pdev->dev;
+
+	chip->IO_ADDR_R = (void __iomem *)0xdeadbeef;
+	chip->IO_ADDR_W = (void __iomem *)0xdeadbeef;
+
+	chip->cmd_ctrl = brcmnand_cmd_ctrl;
+	chip->cmdfunc = brcmnand_cmdfunc;
+	chip->waitfunc = brcmnand_waitfunc;
+	chip->read_byte = brcmnand_read_byte;
+	chip->read_buf = brcmnand_read_buf;
+	chip->write_buf = brcmnand_write_buf;
+
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.read_page = brcmnand_read_page;
+	chip->ecc.read_subpage = brcmnand_read_subpage;
+	chip->ecc.write_page = brcmnand_write_page;
+	chip->ecc.read_page_raw = brcmnand_read_page_raw;
+	chip->ecc.write_page_raw = brcmnand_write_page_raw;
+	chip->ecc.write_oob_raw = brcmnand_write_oob_raw;
+	chip->ecc.read_oob_raw = brcmnand_read_oob_raw;
+	chip->ecc.read_oob = brcmnand_read_oob;
+	chip->ecc.write_oob = brcmnand_write_oob;
+
+	chip->controller = &ctrl->controller;
+
+	if (nand_scan_ident(mtd, 1, NULL))
+		return -ENXIO;
+
+	chip->options |= NAND_NO_SUBPAGE_WRITE;
+	/*
+	 * Avoid (for instance) kmap()'d buffers from JFFS2, which we can't DMA
+	 * to/from, and have nand_base pass us a bounce buffer instead, as
+	 * needed.
+	 */
+	chip->options |= NAND_USE_BOUNCE_BUFFER;
+
+	if (of_get_nand_on_flash_bbt(dn))
+		chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
+
+	if (brcmnand_setup_dev(host))
+		return -ENXIO;
+
+	chip->ecc.size = host->hwcfg.sector_size_1k ? 1024 : 512;
+	/* only use our internal HW threshold */
+	mtd->bitflip_threshold = 1;
+
+	chip->ecc.layout = brcmstb_choose_ecc_layout(host);
+	if (!chip->ecc.layout)
+		return -ENXIO;
+
+	if (nand_scan_tail(mtd))
+		return -ENXIO;
+
+	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
+}
+
+static void brcmnand_save_restore_cs_config(struct brcmnand_host *host,
+					    int restore)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u16 cfg_offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_CFG);
+	u16 cfg_ext_offs = brcmnand_cs_offset(ctrl, host->cs,
+			BRCMNAND_CS_CFG_EXT);
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+			BRCMNAND_CS_ACC_CONTROL);
+	u16 t1_offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_TIMING1);
+	u16 t2_offs = brcmnand_cs_offset(ctrl, host->cs, BRCMNAND_CS_TIMING2);
+
+	if (restore) {
+		nand_writereg(ctrl, cfg_offs, host->hwcfg.config);
+		if (cfg_offs != cfg_ext_offs)
+			nand_writereg(ctrl, cfg_ext_offs,
+				      host->hwcfg.config_ext);
+		nand_writereg(ctrl, acc_control_offs, host->hwcfg.acc_control);
+		nand_writereg(ctrl, t1_offs, host->hwcfg.timing_1);
+		nand_writereg(ctrl, t2_offs, host->hwcfg.timing_2);
+	} else {
+		host->hwcfg.config = nand_readreg(ctrl, cfg_offs);
+		if (cfg_offs != cfg_ext_offs)
+			host->hwcfg.config_ext =
+				nand_readreg(ctrl, cfg_ext_offs);
+		host->hwcfg.acc_control = nand_readreg(ctrl, acc_control_offs);
+		host->hwcfg.timing_1 = nand_readreg(ctrl, t1_offs);
+		host->hwcfg.timing_2 = nand_readreg(ctrl, t2_offs);
+	}
+}
+
+static int brcmnand_suspend(struct device *dev)
+{
+	struct brcmnand_controller *ctrl = dev_get_drvdata(dev);
+	struct brcmnand_host *host;
+
+	list_for_each_entry(host, &ctrl->host_list, node)
+		brcmnand_save_restore_cs_config(host, 0);
+
+	ctrl->nand_cs_nand_select = brcmnand_read_reg(ctrl, BRCMNAND_CS_SELECT);
+	ctrl->nand_cs_nand_xor = brcmnand_read_reg(ctrl, BRCMNAND_CS_XOR);
+	ctrl->corr_stat_threshold =
+		brcmnand_read_reg(ctrl, BRCMNAND_CORR_THRESHOLD);
+
+	if (has_flash_dma(ctrl))
+		ctrl->flash_dma_mode = flash_dma_readl(ctrl, FLASH_DMA_MODE);
+
+	return 0;
+}
+
+static int brcmnand_resume(struct device *dev)
+{
+	struct brcmnand_controller *ctrl = dev_get_drvdata(dev);
+	struct brcmnand_host *host;
+
+	if (has_flash_dma(ctrl)) {
+		flash_dma_writel(ctrl, FLASH_DMA_MODE, ctrl->flash_dma_mode);
+		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
+	}
+
+	brcmnand_write_reg(ctrl, BRCMNAND_CS_SELECT, ctrl->nand_cs_nand_select);
+	brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
+	brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
+			ctrl->corr_stat_threshold);
+
+	list_for_each_entry(host, &ctrl->host_list, node) {
+		struct mtd_info *mtd = &host->mtd;
+		struct nand_chip *chip = mtd->priv;
+
+		brcmnand_save_restore_cs_config(host, 1);
+
+		/* Reset the chip, required by some chips after power-up */
+		chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops brcmnand_pm_ops = {
+	.suspend		= brcmnand_suspend,
+	.resume			= brcmnand_resume,
+};
+
+/***********************************************************************
+ * Platform driver setup (per controller)
+ ***********************************************************************/
+
+static int brcmnand_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node, *child;
+	static struct brcmnand_controller *ctrl;
+	struct resource *res;
+	int ret;
+
+	/* We only support device-tree instantiation */
+	if (!dn)
+		return -ENODEV;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, ctrl);
+	ctrl->dev = dev;
+
+	init_completion(&ctrl->done);
+	init_completion(&ctrl->dma_done);
+	spin_lock_init(&ctrl->controller.lock);
+	init_waitqueue_head(&ctrl->controller.wq);
+	INIT_LIST_HEAD(&ctrl->host_list);
+
+	/* NAND register range */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctrl->nand_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(ctrl->nand_base))
+		return PTR_ERR(ctrl->nand_base);
+
+	/* Initialize NAND revision */
+	ret = brcmnand_revision_init(ctrl);
+	if (ret)
+		return ret;
+
+	/*
+	 * Most chips have this cache at a fixed offset within 'nand' block.
+	 * Some must specify this region separately.
+	 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand-cache");
+	if (res) {
+		ctrl->nand_fc = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ctrl->nand_fc))
+			return PTR_ERR(ctrl->nand_fc);
+	} else {
+		ctrl->nand_fc = ctrl->nand_base +
+				ctrl->reg_offsets[BRCMNAND_FC_BASE];
+	}
+
+	/* FLASH_DMA */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "flash-dma");
+	if (res) {
+		ctrl->flash_dma_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ctrl->flash_dma_base))
+			return PTR_ERR(ctrl->flash_dma_base);
+
+		flash_dma_writel(ctrl, FLASH_DMA_MODE, 1); /* linked-list */
+		flash_dma_writel(ctrl, FLASH_DMA_ERROR_STATUS, 0);
+
+		/* Allocate descriptor(s) */
+		ctrl->dma_desc = dmam_alloc_coherent(dev,
+						     sizeof(*ctrl->dma_desc),
+						     &ctrl->dma_pa, GFP_KERNEL);
+		if (!ctrl->dma_desc)
+			return -ENOMEM;
+
+		ctrl->dma_irq = platform_get_irq(pdev, 1);
+		if ((int)ctrl->dma_irq < 0) {
+			dev_err(dev, "missing FLASH_DMA IRQ\n");
+			return -ENODEV;
+		}
+
+		ret = devm_request_irq(dev, ctrl->dma_irq,
+				brcmnand_dma_irq, 0, DRV_NAME,
+				ctrl);
+		if (ret < 0) {
+			dev_err(dev, "can't allocate IRQ %d: error %d\n",
+					ctrl->dma_irq, ret);
+			return ret;
+		}
+
+		dev_info(dev, "enabling FLASH_DMA\n");
+	}
+
+	/* Disable automatic device ID config, direct addressing */
+	brcmnand_rmw_reg(ctrl, BRCMNAND_CS_SELECT,
+			 CS_SELECT_AUTO_DEVICE_ID_CFG | 0xff, 0, 0);
+	/* Disable XOR addressing */
+	brcmnand_rmw_reg(ctrl, BRCMNAND_CS_XOR, 0xff, 0, 0);
+
+	if (ctrl->features & BRCMNAND_HAS_WP) {
+		/* Permanently disable write protection */
+		if (wp_on == 2)
+			brcmnand_set_wp(ctrl, false);
+	} else {
+		wp_on = 0;
+	}
+
+	/* IRQ */
+	ctrl->irq = platform_get_irq(pdev, 0);
+	if ((int)ctrl->irq < 0) {
+		dev_err(dev, "no IRQ defined\n");
+		return -ENODEV;
+	}
+
+	ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
+			DRV_NAME, ctrl);
+	if (ret < 0) {
+		dev_err(dev, "can't allocate IRQ %d: error %d\n",
+			ctrl->irq, ret);
+		return ret;
+	}
+
+	for_each_available_child_of_node(dn, child) {
+		if (of_device_is_compatible(child, "brcm,nandcs")) {
+			struct brcmnand_host *host;
+
+			host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+			if (!host)
+				return -ENOMEM;
+			host->pdev = pdev;
+			host->ctrl = ctrl;
+			host->of_node = child;
+
+			ret = brcmnand_init_cs(host);
+			if (ret)
+				continue; /* Try all chip-selects */
+
+			list_add_tail(&host->node, &ctrl->host_list);
+		}
+	}
+
+	/* No chip-selects could initialize properly */
+	if (list_empty(&ctrl->host_list))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int brcmnand_remove(struct platform_device *pdev)
+{
+	struct brcmnand_controller *ctrl = dev_get_drvdata(&pdev->dev);
+	struct brcmnand_host *host;
+
+	list_for_each_entry(host, &ctrl->host_list, node)
+		nand_release(&host->mtd);
+
+	dev_set_drvdata(&pdev->dev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id brcmnand_of_match[] = {
+	{ .compatible = "brcm,brcmnand-v4.0" },
+	{ .compatible = "brcm,brcmnand-v5.0" },
+	{ .compatible = "brcm,brcmnand-v6.0" },
+	{ .compatible = "brcm,brcmnand-v6.1" },
+	{ .compatible = "brcm,brcmnand-v7.0" },
+	{ .compatible = "brcm,brcmnand-v7.1" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, brcmnand_of_match);
+
+static struct platform_driver brcmstb_nand_driver = {
+	.probe			= brcmnand_probe,
+	.remove			= brcmnand_remove,
+	.driver = {
+		.name		= DRV_NAME,
+		.pm		= &brcmnand_pm_ops,
+		.of_match_table = of_match_ptr(brcmnand_of_match),
+	}
+};
+module_platform_driver(brcmstb_nand_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Kevin Cernekee");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("NAND driver for Broadcom STB chips");
+MODULE_ALIAS("platform:brcmnand");
-- 
1.9.1

[PATCH v3 04/10] ARM: bcm7445: add NAND to DTS

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 arch/arm/boot/dts/bcm7445-bcm97445svmb.dts | 23 +++++++++++++++++++++++
 arch/arm/boot/dts/bcm7445.dtsi             | 22 ++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts b/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
index 9eec2ac1112f..0bb8d17e4c2d 100644
--- a/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
+++ b/arch/arm/boot/dts/bcm7445-bcm97445svmb.dts
@@ -12,3 +12,26 @@
 		      <0x00 0x80000000 0x00 0x40000000>;
 	};
 };
+
+&nand {
+	status = "okay";
+
+	nandcs@1 {
+		compatible = "brcm,nandcs";
+		reg = <1>;
+		nand-ecc-step-size = <512>;
+		nand-ecc-strength = <8>;
+		nand-on-flash-bbt;
+
+		#size-cells = <2>;
+		#address-cells = <2>;
+
+		flash1.rootfs0@0 {
+			reg = <0x0 0x0 0x0 0x80000000>;
+		};
+
+		flash1.rootfs1@80000000 {
+			reg = <0x0 0x80000000 0x0 0x80000000>;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/bcm7445.dtsi b/arch/arm/boot/dts/bcm7445.dtsi
index 39ac7840d7ee..33447d82b7cf 100644
--- a/arch/arm/boot/dts/bcm7445.dtsi
+++ b/arch/arm/boot/dts/bcm7445.dtsi
@@ -108,6 +108,28 @@
 			brcm,int-map-mask = <0x25c>, <0x7000000>;
 			brcm,int-fwd-mask = <0x70000>;
 		};
+
+		hif_intr2_intc: interrupt-controller@3e1000 {
+			compatible = "brcm,l2-intc";
+			reg = <0x3e1000 0x30>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupts = <GIC_SPI 0x20 0x0>;
+			interrupt-parent = <&gic>;
+			interrupt-names = "hif";
+		};
+
+		nand: nand@3e2800 {
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg-names = "nand", "flash-dma";
+			reg = <0x3e2800 0x600>, <0x3e3000 0x2c>;
+			interrupt-parent = <&hif_intr2_intc>;
+			interrupts = <24>, <4>;
+			interrupt-names = "nand_ctlrdy", "flash_dma_done";
+		};
 	};
 
 	smpboot {
-- 
1.9.1

[PATCH v3 05/10] Documentation: devicetree: brcmstb_nand: add 'brcm,nand-soc' bindings

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 .../devicetree/bindings/mtd/brcm,brcmstb-nand.txt  | 39 +++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt b/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
index 662c857e74fe..6a3ab751db99 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
@@ -30,7 +30,10 @@ Required properties:
                      "flash-dma" and/or "nand-cache".
 - interrupts       : The NAND CTLRDY interrupt and (if Flash DMA is available)
                      FLASH_DMA_DONE
-- interrupt-names  : May be "nand_ctlrdy" or "flash_dma_done"
+- interrupt-names  : For hardware without a dedicated 'brcm,nand-soc' node, may
+                     be "nand_ctlrdy" or "flash_dma_done"
+                     For hardware with a dedicated 'brcm,nand-soc' node for
+                     breaking out individual interrupt types, may be "nand"
 - interrupt-parent : See standard interrupt bindings
 - #address-cells   : <1> - subnodes give the chip-select number
 - #size-cells      : <0>
@@ -40,6 +43,10 @@ Optional properties:
                               (WP) control bit. It is always available on >=
                               v7.0. Use this property to describe the rare
                               earlier versions of this core that include WP
+- brcm,nand-soc             : Phandle to SoC control node. This is necessary
+                              for SoCs where NAND interrupts and bus
+                              infrastructure are integrated in non-standard
+                              ways.
 
 * NAND chip-select
 
@@ -74,6 +81,36 @@ Optional properties:
 Each nandcs device node may optionally contain sub-nodes describing the flash
 partition mapping. See partition.txt for more detail.
 
+
+* NAND SoC control node:
+
+The NAND controller is integrated differently on the variety of SoCs on which it
+is found. Part of this integration involves providing status and enable bits
+with which to control the 8 exposed NAND interrupts, as well as hardware for
+configuring the endianness of the data bus. On some SoCs, these features are
+handled via standard, modular components (e.g., their interrupts look like a
+normal IRQ chip), but on others, they are controlled in unique and interesting
+ways, sometimes with registers that lump multiple NAND-related functions
+together. The former case can be described simply by the standard interrupts
+properties in the main controller node. But for the latter exceptional cases,
+we can describe these extra SoC-specific integration hardware via the following
+node, referenced from the brcm,brcmnand node above.
+
+ - compatible: Can be one of several SoC-specific strings. Each SoC may have
+   different requirements for its additional properties, as described below each
+   bullet point below.
+
+   * "brcm,nand-soc-bcm63138"
+     - reg: (required) the 'NAND_INT_BASE' register range, with separate status
+       and enable registers
+
+   * "brcm,nand-soc-iproc"
+     - reg: (required) the "IDM" register range, for interrupt enable and APB
+       bus access endianness configuration, and the "EXT" register range,
+       for interrupt status/ack.
+     - reg-names: (required) a list of the names corresponding to the previous
+       register ranges. Should contain "idm" and "ext".
+
 Example:
 
 nand@f0442800 {
-- 
1.9.1

[PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

The STB NAND controller is integrated into various non-STB SoCs, and
those SoCs integrate things like interrupts and busing differently. Add
support for masking/clearing interrupts via a custom hook, as well as
preparing/unpreparing the data bus for data transfers.

Does not support any new SoCs yet; support will be added incrementally.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/Makefile           |  3 +-
 drivers/mtd/nand/brcmnand.h         | 56 +++++++++++++++++++++++++++
 drivers/mtd/nand/brcmstb_nand.c     | 75 ++++++++++++++++++++++++++++++++++---
 drivers/mtd/nand/brcmstb_nand_soc.c | 65 ++++++++++++++++++++++++++++++++
 4 files changed, 193 insertions(+), 6 deletions(-)
 create mode 100644 drivers/mtd/nand/brcmnand.h
 create mode 100644 drivers/mtd/nand/brcmstb_nand_soc.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 3b1adddc83dd..806727d5a84b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_MTD_NAND_XWAY)		+= xway_nand.o
 obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)	+= bcm47xxnflash/
 obj-$(CONFIG_MTD_NAND_SUNXI)		+= sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504)	        += hisi504_nand.o
-obj-$(CONFIG_MTD_NAND_BRCMSTB)		+= brcmstb_nand.o
+obj-$(CONFIG_MTD_NAND_BRCMSTB)		+= brcmnand.o
+brcmnand-objs				:= brcmstb_nand.o brcmstb_nand_soc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/brcmnand.h b/drivers/mtd/nand/brcmnand.h
new file mode 100644
index 000000000000..59e0bfef2331
--- /dev/null
+++ b/drivers/mtd/nand/brcmnand.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright © 2015 Broadcom Corporation
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __BRCMNAND_H__
+#define __BRCMNAND_H__
+
+#include <linux/types.h>
+
+struct device;
+struct device_node;
+
+struct brcmnand_soc {
+	struct device *dev; /* parent device */
+	struct device_node *dn;
+	bool (*ctlrdy_ack)(struct brcmnand_soc *soc);
+	void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
+	void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare);
+	void *priv;
+};
+
+/**
+ * Probe for a custom Broadcom SoC
+ *
+ * @dev: device to bind devres objects to
+ * @dn: DT node for the custom SoC
+ *
+ * Return a new soc struct if successful. Should be freed with
+ * brcmnand_remove_soc().
+ * Return NULL for all other errors
+ */
+struct brcmnand_soc *devm_brcmnand_probe_soc(struct device *dev,
+					     struct device_node *dn);
+
+static inline void brcmnand_soc_data_bus_prepare(struct brcmnand_soc *soc)
+{
+	if (soc && soc->prepare_data_bus)
+		soc->prepare_data_bus(soc, true);
+}
+
+static inline void brcmnand_soc_data_bus_unprepare(struct brcmnand_soc *soc)
+{
+	if (soc && soc->prepare_data_bus)
+		soc->prepare_data_bus(soc, false);
+}
+
+#endif /* __BRCMNAND_H__ */
diff --git a/drivers/mtd/nand/brcmstb_nand.c b/drivers/mtd/nand/brcmstb_nand.c
index ec65a48d2487..5abc88cfe702 100644
--- a/drivers/mtd/nand/brcmstb_nand.c
+++ b/drivers/mtd/nand/brcmstb_nand.c
@@ -37,6 +37,8 @@
 #include <linux/list.h>
 #include <linux/log2.h>
 
+#include "brcmnand.h"
+
 /*
  * This flag controls if WP stays on between erase/write commands to mitigate
  * flash corruption due to power glitches. Values:
@@ -117,6 +119,9 @@ struct brcmnand_controller {
 	unsigned int		dma_irq;
 	int			nand_version;
 
+	/* Some SoCs provide custom interrupt status register(s) */
+	struct brcmnand_soc	*soc;
+
 	int			cmd_pending;
 	bool			dma_pending;
 	struct completion	done;
@@ -963,6 +968,17 @@ static irqreturn_t brcmnand_ctlrdy_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+/* Handle SoC-specific interrupt hardware */
+static irqreturn_t brcmnand_irq(int irq, void *data)
+{
+	struct brcmnand_controller *ctrl = data;
+
+	if (ctrl->soc->ctlrdy_ack(ctrl->soc))
+		return brcmnand_ctlrdy_irq(irq, data);
+
+	return IRQ_NONE;
+}
+
 static irqreturn_t brcmnand_dma_irq(int irq, void *data)
 {
 	struct brcmnand_controller *ctrl = data;
@@ -1151,12 +1167,18 @@ static void brcmnand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	if (native_cmd == CMD_PARAMETER_READ ||
 			native_cmd == CMD_PARAMETER_CHANGE_COL) {
 		int i;
+
+		brcmnand_soc_data_bus_prepare(ctrl->soc);
+
 		/*
 		 * Must cache the FLASH_CACHE now, since changes in
 		 * SECTOR_SIZE_1K may invalidate it
 		 */
 		for (i = 0; i < FC_WORDS; i++)
 			ctrl->flash_cache[i] = brcmnand_read_fc(ctrl, i);
+
+		brcmnand_soc_data_bus_unprepare(ctrl->soc);
+
 		/* Cleanup from HW quirk: restore SECTOR_SIZE_1K */
 		if (host->hwcfg.sector_size_1k)
 			brcmnand_set_sector_size_1k(host,
@@ -1369,10 +1391,15 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
 		brcmnand_send_cmd(host, CMD_PAGE_READ);
 		brcmnand_waitfunc(mtd, chip);
 
-		if (likely(buf))
+		if (likely(buf)) {
+			brcmnand_soc_data_bus_prepare(ctrl->soc);
+
 			for (j = 0; j < FC_WORDS; j++, buf++)
 				*buf = brcmnand_read_fc(ctrl, j);
 
+			brcmnand_soc_data_bus_unprepare(ctrl->soc);
+		}
+
 		if (oob)
 			oob += read_oob_from_regs(ctrl, i, oob,
 					mtd->oobsize / trans,
@@ -1544,12 +1571,17 @@ static int brcmnand_write(struct mtd_info *mtd, struct nand_chip *chip,
 				   lower_32_bits(addr));
 		(void)brcmnand_read_reg(ctrl, BRCMNAND_CMD_ADDRESS);
 
-		if (buf)
+		if (buf) {
+			brcmnand_soc_data_bus_prepare(ctrl->soc);
+
 			for (j = 0; j < FC_WORDS; j++, buf++)
 				brcmnand_write_fc(ctrl, j, *buf);
-		else if (oob)
+
+			brcmnand_soc_data_bus_unprepare(ctrl->soc);
+		} else if (oob) {
 			for (j = 0; j < FC_WORDS; j++)
 				brcmnand_write_fc(ctrl, j, 0xffffffff);
+		}
 
 		if (oob) {
 			oob += write_oob_to_regs(ctrl, i, oob,
@@ -1993,6 +2025,11 @@ static int brcmnand_resume(struct device *dev)
 	brcmnand_write_reg(ctrl, BRCMNAND_CS_XOR, ctrl->nand_cs_nand_xor);
 	brcmnand_write_reg(ctrl, BRCMNAND_CORR_THRESHOLD,
 			ctrl->corr_stat_threshold);
+	if (ctrl->soc) {
+		/* Clear/re-enable interrupt */
+		ctrl->soc->ctlrdy_ack(ctrl->soc);
+		ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
+	}
 
 	list_for_each_entry(host, &ctrl->host_list, node) {
 		struct mtd_info *mtd = &host->mtd;
@@ -2122,8 +2159,36 @@ static int brcmnand_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
-			DRV_NAME, ctrl);
+	/*
+	 * Some SoCs integrate this controller (e.g., its interrupt bits) in
+	 * interesting ways
+	 */
+	if (of_property_read_bool(dn, "brcm,nand-soc")) {
+		struct device_node *soc_dn;
+
+		soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
+		if (!soc_dn)
+			return -ENODEV;
+
+		ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
+		if (!ctrl->soc) {
+			dev_err(dev, "could not probe SoC data\n");
+			of_node_put(soc_dn);
+			return -ENODEV;
+		}
+
+		ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
+				       DRV_NAME, ctrl);
+
+		/* Enable interrupt */
+		ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
+
+		of_node_put(soc_dn);
+	} else {
+		/* Use standard interrupt infrastructure */
+		ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
+				       DRV_NAME, ctrl);
+	}
 	if (ret < 0) {
 		dev_err(dev, "can't allocate IRQ %d: error %d\n",
 			ctrl->irq, ret);
diff --git a/drivers/mtd/nand/brcmstb_nand_soc.c b/drivers/mtd/nand/brcmstb_nand_soc.c
new file mode 100644
index 000000000000..970912c690a7
--- /dev/null
+++ b/drivers/mtd/nand/brcmstb_nand_soc.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright © 2015 Broadcom Corporation
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include "brcmnand.h"
+
+#define DRV_NAME	"brcmnand-soc"
+
+struct brcmnand_soc_ofdata {
+	int (*init)(struct brcmnand_soc *soc);
+	bool (*ctlrdy_ack)(struct brcmnand_soc *soc);
+	void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
+	void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare);
+};
+
+static const struct of_device_id brcmnand_soc_ofmatch[] = {
+	{},
+};
+
+struct brcmnand_soc *devm_brcmnand_probe_soc(struct device *dev,
+					     struct device_node *dn)
+{
+	const struct brcmnand_soc_ofdata *soc_data;
+	const struct of_device_id *match;
+	struct brcmnand_soc *soc;
+	int ret;
+
+	match = of_match_node(brcmnand_soc_ofmatch, dn);
+	if (!match)
+		return NULL;
+
+	soc_data = match->data;
+
+	soc = devm_kzalloc(dev, sizeof(*soc), GFP_KERNEL);
+	if (!soc)
+		return NULL;
+
+	soc->dev = dev;
+	soc->dn = dn;
+	soc->ctlrdy_ack = soc_data->ctlrdy_ack;
+	soc->ctlrdy_set_enabled = soc_data->ctlrdy_set_enabled;
+	soc->prepare_data_bus = soc_data->prepare_data_bus;
+	if (soc_data->init) {
+		ret = soc_data->init(soc);
+		if (ret)
+			return NULL;
+	}
+
+	return soc;
+}
-- 
1.9.1

[PATCH v3 07/10] mtd: brcsmtb_nand_soc: add support for BCM63138

[Brian Norris]

This SoC just has its custom interrupt status bits.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/brcmstb_nand_soc.c | 69 +++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/mtd/nand/brcmstb_nand_soc.c b/drivers/mtd/nand/brcmstb_nand_soc.c
index 970912c690a7..c878334edb1a 100644
--- a/drivers/mtd/nand/brcmstb_nand_soc.c
+++ b/drivers/mtd/nand/brcmstb_nand_soc.c
@@ -28,7 +28,76 @@ struct brcmnand_soc_ofdata {
 	void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare);
 };
 
+struct bcm63138_nand_soc_priv {
+	void __iomem *base;
+};
+
+#define BCM63138_NAND_INT_STATUS		0x00
+#define BCM63138_NAND_INT_EN			0x04
+
+enum {
+	BCM63138_CTLRDY		= BIT(4),
+};
+
+static bool bcm63138_nand_intc_ack(struct brcmnand_soc *soc)
+{
+	struct bcm63138_nand_soc_priv *priv = soc->priv;
+	void __iomem *mmio = priv->base + BCM63138_NAND_INT_STATUS;
+	u32 val = __raw_readl(mmio);
+
+	if (val & BCM63138_CTLRDY) {
+		__raw_writel(val & ~BCM63138_CTLRDY, mmio);
+		return true;
+	}
+
+	return false;
+}
+
+static void bcm63138_nand_intc_set(struct brcmnand_soc *soc, bool en)
+{
+	struct bcm63138_nand_soc_priv *priv = soc->priv;
+	void __iomem *mmio = priv->base + BCM63138_NAND_INT_EN;
+	u32 val = __raw_readl(mmio);
+
+	if (en)
+		val |= BCM63138_CTLRDY;
+	else
+		val &= ~BCM63138_CTLRDY;
+
+	__raw_writel(val, mmio);
+}
+
+static int bcm63138_nand_soc_init(struct brcmnand_soc *soc)
+{
+	struct bcm63138_nand_soc_priv *priv;
+
+	priv = devm_kzalloc(soc->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = of_io_request_and_map(soc->dn, 0, DRV_NAME);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	soc->priv = priv;
+
+	/* Clear the interrupt */
+	bcm63138_nand_intc_ack(soc);
+
+	return 0;
+}
+
+static const struct brcmnand_soc_ofdata bcm63138_nand_soc = {
+	.init			= bcm63138_nand_soc_init,
+	.ctlrdy_ack		= bcm63138_nand_intc_ack,
+	.ctlrdy_set_enabled	= bcm63138_nand_intc_set,
+};
+
 static const struct of_device_id brcmnand_soc_ofmatch[] = {
+	{
+		.compatible	= "brcm,nand-soc-bcm63138",
+		.data		= &bcm63138_nand_soc,
+	},
 	{},
 };
 
-- 
1.9.1

[PATCH v3 08/10] mtd: brcsmtb_nand_soc: add iProc support

[Brian Norris]

From: Ray Jui <rjui@broadcom.com>

Signed-off-by: Ray Jui <rjui@broadcom.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/brcmstb_nand_soc.c | 110 ++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/drivers/mtd/nand/brcmstb_nand_soc.c b/drivers/mtd/nand/brcmstb_nand_soc.c
index c878334edb1a..5fb851c655f2 100644
--- a/drivers/mtd/nand/brcmstb_nand_soc.c
+++ b/drivers/mtd/nand/brcmstb_nand_soc.c
@@ -93,11 +93,121 @@ static const struct brcmnand_soc_ofdata bcm63138_nand_soc = {
 	.ctlrdy_set_enabled	= bcm63138_nand_intc_set,
 };
 
+struct iproc_nand_soc_priv {
+	void __iomem *idm_base;
+	void __iomem *ext_base;
+	spinlock_t idm_lock;
+};
+
+#define IPROC_NAND_CTLR_READY_OFFSET       0x10
+#define IPROC_NAND_CTLR_READY              BIT(0)
+
+#define IPROC_NAND_IO_CTRL_OFFSET          0x00
+#define IPROC_NAND_APB_LE_MODE             BIT(24)
+#define IPROC_NAND_INT_CTRL_READ_ENABLE    BIT(6)
+
+static bool iproc_nand_intc_ack(struct brcmnand_soc *soc)
+{
+	struct iproc_nand_soc_priv *priv = soc->priv;
+	void __iomem *mmio = priv->ext_base + IPROC_NAND_CTLR_READY_OFFSET;
+	u32 val = __raw_readl(mmio);
+
+	if (val & IPROC_NAND_CTLR_READY) {
+		__raw_writel(IPROC_NAND_CTLR_READY, mmio);
+		return true;
+	}
+
+	return false;
+}
+
+static void iproc_nand_intc_set(struct brcmnand_soc *soc, bool en)
+{
+	struct iproc_nand_soc_priv *priv = soc->priv;
+	void __iomem *mmio = priv->idm_base + IPROC_NAND_IO_CTRL_OFFSET;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->idm_lock, flags);
+
+	val = __raw_readl(mmio);
+
+	if (en)
+		val |= IPROC_NAND_INT_CTRL_READ_ENABLE;
+	else
+		val &= ~IPROC_NAND_INT_CTRL_READ_ENABLE;
+
+	__raw_writel(val, mmio);
+
+	spin_unlock_irqrestore(&priv->idm_lock, flags);
+}
+
+static int iproc_nand_soc_init(struct brcmnand_soc *soc)
+{
+	struct iproc_nand_soc_priv *priv;
+
+	priv = devm_kzalloc(soc->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	spin_lock_init(&priv->idm_lock);
+
+	priv->idm_base = of_io_request_and_map(soc->dn, 0, DRV_NAME);
+	if (IS_ERR(priv->idm_base))
+		return PTR_ERR(priv->idm_base);
+
+	priv->ext_base = of_io_request_and_map(soc->dn, 1, DRV_NAME);
+	if (IS_ERR(priv->ext_base)) {
+		struct resource res;
+
+		of_address_to_resource(soc->dn, 0, &res);
+		release_mem_region(res.start, resource_size(&res));
+		iounmap(priv->idm_base);
+		return PTR_ERR(priv->ext_base);
+	}
+
+	soc->priv = priv;
+	iproc_nand_intc_ack(soc);
+
+	return 0;
+}
+
+static void iproc_nand_apb_access(struct brcmnand_soc *soc, bool prepare)
+{
+	struct iproc_nand_soc_priv *priv = soc->priv;
+	void __iomem *mmio = priv->idm_base + IPROC_NAND_IO_CTRL_OFFSET;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->idm_lock, flags);
+
+	val = __raw_readl(mmio);
+
+	if (prepare)
+		val |= IPROC_NAND_APB_LE_MODE;
+	else
+		val &= ~IPROC_NAND_APB_LE_MODE;
+
+	__raw_writel(val, mmio);
+
+	spin_unlock_irqrestore(&priv->idm_lock, flags);
+}
+
+static const struct brcmnand_soc_ofdata iproc_nand_soc = {
+	.init			= iproc_nand_soc_init,
+	.ctlrdy_ack		= iproc_nand_intc_ack,
+	.ctlrdy_set_enabled	= iproc_nand_intc_set,
+	.prepare_data_bus	= iproc_nand_apb_access,
+};
+
 static const struct of_device_id brcmnand_soc_ofmatch[] = {
 	{
 		.compatible	= "brcm,nand-soc-bcm63138",
 		.data		= &bcm63138_nand_soc,
 	},
+	{
+		.compatible	= "brcm,nand-soc-iproc",
+		.data		= &iproc_nand_soc,
+	},
 	{},
 };
 
-- 
1.9.1

[PATCH v3 09/10] ARM: bcm63138: add NAND DT support

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 arch/arm/boot/dts/bcm63138.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm963138dvt.dts | 12 ++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/bcm63138.dtsi b/arch/arm/boot/dts/bcm63138.dtsi
index f46329c8ad75..5fb6f6d750f1 100644
--- a/arch/arm/boot/dts/bcm63138.dtsi
+++ b/arch/arm/boot/dts/bcm63138.dtsi
@@ -131,5 +131,22 @@
 			clock-names = "periph";
 			status = "disabled";
 		};
+
+		nand: nand@2000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,brcmnand-v7.0", "brcm,brcmnand";
+			reg = <0x2000 0x600>;
+			reg-names = "nand";
+			status = "disabled";
+			interrupts = <GIC_SPI 38 0>;
+			interrupt-names = "nand";
+			brcm,nand-soc = <&nand_int_base>;
+		};
+
+		nand_int_base: nand-soc@f0 {
+			compatible = "brcm,nand-soc-bcm63138";
+			reg = <0xf0 0x10>;
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/bcm963138dvt.dts b/arch/arm/boot/dts/bcm963138dvt.dts
index 69c93395ecd2..370aa2cfddf2 100644
--- a/arch/arm/boot/dts/bcm963138dvt.dts
+++ b/arch/arm/boot/dts/bcm963138dvt.dts
@@ -28,3 +28,15 @@
 &serial1 {
 	status = "okay";
 };
+
+&nand {
+	status = "okay";
+
+	nandcs@0 {
+		compatible = "brcm,nandcs";
+		reg = <0>;
+		nand-ecc-strength = <4>;
+		nand-ecc-step-size = <512>;
+		brcm,nand-oob-sectors-size = <16>;
+	};
+};
-- 
1.9.1

[PATCH v3 10/10] ARM: dts: cygnus: Enable NAND support for Cygnus

[Brian Norris]

From: Ray Jui <rjui@broadcom.com>

Enable NAND support for Broadcom Cygnus SoC

Signed-off-by: Ray Jui <rjui@broadcom.com>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 arch/arm/boot/dts/bcm-cygnus.dtsi | 20 ++++++++++++++++++++
 arch/arm/boot/dts/bcm958300k.dts  | 16 ++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 7b52c33ea69a..8aabebc67632 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -212,6 +212,26 @@
 		status = "disabled";
 	};
 
+	nand_soc: nand_soc@f8105408 {
+		compatible = "brcm,nand-soc-iproc";
+		reg = <0xf8105408 0x600>,
+		      <0x18046f00 0x20>;
+		reg-names = "idm", "ext";
+	};
+
+	nand: nand@18046000 {
+		compatible = "brcm,brcmnand-v6.1", "brcm,brcmnand";
+		reg = <0x18046000 0x600>;
+		reg-names = "nand";
+		interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		brcm,nand-has-wp;
+		brcm,nand-soc = <&nand_soc>;
+	};
+
 	gic: interrupt-controller@19021000 {
 		compatible = "arm,cortex-a9-gic";
 		#interrupt-cells = <3>;
diff --git a/arch/arm/boot/dts/bcm958300k.dts b/arch/arm/boot/dts/bcm958300k.dts
index c9eb8565eac5..2f63052f9d48 100644
--- a/arch/arm/boot/dts/bcm958300k.dts
+++ b/arch/arm/boot/dts/bcm958300k.dts
@@ -58,4 +58,20 @@
 	uart3: serial@18023000 {
 		status = "okay";
 	};
+
+	nand: nand@18046000 {
+		nandcs@1 {
+			compatible = "brcm,nandcs";
+			reg = <0>;
+			nand-on-flash-bbt;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			nand-ecc-strength = <24>;
+			nand-ecc-step-size = <1024>;
+
+			brcm,nand-oob-sector-size = <27>;
+		};
+	};
 };
-- 
1.9.1

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Arnd Bergmann]

On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
> +       /*
> +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
> +        * interesting ways
> +        */
> +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
> +               struct device_node *soc_dn;
> +
> +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
> +               if (!soc_dn)
> +                       return -ENODEV;
> +
> +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
> +               if (!ctrl->soc) {
> +                       dev_err(dev, "could not probe SoC data\n");
> +                       of_node_put(soc_dn);
> +                       return -ENODEV;
> +               }
> +
> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
> +                                      DRV_NAME, ctrl);
> +
> +               /* Enable interrupt */
> +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
> +
> +               of_node_put(soc_dn);
> +       } else {
> +               /* Use standard interrupt infrastructure */
> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
> +                                      DRV_NAME, ctrl);
> +       }
> 

It looks to me like this should be handled as a nested irqchip, so the node
you look up gets used as the "interrupt-parent" instead, making the behavior
of this SoC transparent to the nand driver.

We recently merged nested irqdomain support as well, which might help here,
or might not be needed.

	Arnd

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Arnd Bergmann]

On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
> +
> +static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
> +{
> +       return __raw_readl(ctrl->nand_base + offs);
> +}
> +
> +static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
> +                                u32 val)
> +{
> +       __raw_writel(val, ctrl->nand_base + offs);
> +}
> +
> 

You had mentioned previously that there might be an endianess issue in this
driver. I think this won't work on big-endian architectures other than MIPS,
so it would be good to either list in the DT the endianess of the device
and use appropriate accessors here, or hardcode it based on the architecture
(using ioread32_be in big-endian mips, but readl elsewhere).

Using __raw_writel has another problem regarding the DMA capability of this
driver, as it will not flush any write buffers or synchronize caches before
sending data off to the device, so you risk data corruption. Also, the
compiler can choose to split up the 32-bit word access into byte accesses,
which on most hardware does not do what you want.

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
> > +       /*
> > +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
> > +        * interesting ways
> > +        */
> > +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
> > +               struct device_node *soc_dn;
> > +
> > +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
> > +               if (!soc_dn)
> > +                       return -ENODEV;
> > +
> > +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
> > +               if (!ctrl->soc) {
> > +                       dev_err(dev, "could not probe SoC data\n");
> > +                       of_node_put(soc_dn);
> > +                       return -ENODEV;
> > +               }
> > +
> > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
> > +                                      DRV_NAME, ctrl);
> > +
> > +               /* Enable interrupt */
> > +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
> > +
> > +               of_node_put(soc_dn);
> > +       } else {
> > +               /* Use standard interrupt infrastructure */
> > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
> > +                                      DRV_NAME, ctrl);
> > +       }
> > 
> 
> It looks to me like this should be handled as a nested irqchip, so the node
> you look up gets used as the "interrupt-parent" instead, making the behavior
> of this SoC transparent to the nand driver.

You snipped the rest of the patch, which involves more than just IRQ
handling. The same registers touch both interrupts and data bus endian
configuration, so it can't possibly be done transparently to the NAND
driver.

> We recently merged nested irqdomain support as well, which might help here,
> or might not be needed.

I'm not familiar with nested irqdomains. Do they address anything like
the above problem?

Brian

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Brian Norris]

On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
> > +
> > +static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
> > +{
> > +       return __raw_readl(ctrl->nand_base + offs);
> > +}
> > +
> > +static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
> > +                                u32 val)
> > +{
> > +       __raw_writel(val, ctrl->nand_base + offs);
> > +}
> > +
> > 
> 
> You had mentioned previously that there might be an endianess issue in this
> driver.

Might. I have a patch already, but I failed to boot a BE kernel, so I
kept it out for now. If you don't mind, I'd prefer patching something
like this once it's testable on ARM BE. This *is*, however, extensively
tested on MIPS (LE and BE) and ARM (LE).

> I think this won't work on big-endian architectures other than MIPS,
> so it would be good to either list in the DT the endianess of the device
> and use appropriate accessors here, or hardcode it based on the architecture
> (using ioread32_be in big-endian mips, but readl elsewhere).

I suspect we wouldn't need a DT property but could just special-case
MIPS BE, as you note.

> Using __raw_writel has another problem regarding the DMA capability of this
> driver, as it will not flush any write buffers or synchronize caches before
> sending data off to the device, so you risk data corruption.

We use mb() before kicking off DMA or other commands.

> Also, the
> compiler can choose to split up the 32-bit word access into byte accesses,
> which on most hardware does not do what you want.

Huh? Wouldn't that break just about every driver in existence? And how
is writel() any different than __raw_writel() in that regard? From
include/asm-generic/io.h:

static inline void writel(u32 value, volatile void __iomem *addr)
{
        __raw_writel(__cpu_to_le32(value), addr);
}

And BTW, splitting isn't possible on ARM. From
arch/arm/include/asm/io.h:

static inline void __raw_writel(u32 val, volatile void __iomem *addr)
{
        asm volatile("str %1, %0"
                     : "+Qo" (*(volatile u32 __force *)addr)
                     : "r" (val));
}

Brian

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Ray Jui]

On 5/6/2015 2:05 PM, Brian Norris wrote:
> On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
>> On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
>>> +
>>> +static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
>>> +{
>>> +       return __raw_readl(ctrl->nand_base + offs);
>>> +}
>>> +
>>> +static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
>>> +                                u32 val)
>>> +{
>>> +       __raw_writel(val, ctrl->nand_base + offs);
>>> +}
>>> +
>>>
>>
>> You had mentioned previously that there might be an endianess issue in this
>> driver.
> 
> Might. I have a patch already, but I failed to boot a BE kernel, so I
> kept it out for now. If you don't mind, I'd prefer patching something
> like this once it's testable on ARM BE. This *is*, however, extensively
> tested on MIPS (LE and BE) and ARM (LE).

Correct, extensive test and pass all MTD test cases. We should
eventually be able to test this on a working ARM BE platform, within the
next couple months.

> 
>> I think this won't work on big-endian architectures other than MIPS,
>> so it would be good to either list in the DT the endianess of the device
>> and use appropriate accessors here, or hardcode it based on the architecture
>> (using ioread32_be in big-endian mips, but readl elsewhere).
> 
> I suspect we wouldn't need a DT property but could just special-case
> MIPS BE, as you note.
> 
>> Using __raw_writel has another problem regarding the DMA capability of this
>> driver, as it will not flush any write buffers or synchronize caches before
>> sending data off to the device, so you risk data corruption.
> 
> We use mb() before kicking off DMA or other commands.
> 
>> Also, the
>> compiler can choose to split up the 32-bit word access into byte accesses,
>> which on most hardware does not do what you want.
> 
> Huh? Wouldn't that break just about every driver in existence? And how
> is writel() any different than __raw_writel() in that regard? From
> include/asm-generic/io.h:
> 
> static inline void writel(u32 value, volatile void __iomem *addr)
> {
>         __raw_writel(__cpu_to_le32(value), addr);
> }
> 
> And BTW, splitting isn't possible on ARM. From
> arch/arm/include/asm/io.h:
> 
> static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> {
>         asm volatile("str %1, %0"
>                      : "+Qo" (*(volatile u32 __force *)addr)
>                      : "r" (val));
> }
> 
> Brian
> 

Re: [PATCH v3 00/10] mtd: nand: add Broadcom NAND controller support

[Florian Fainelli]

On 06/05/15 10:59, Brian Norris wrote:
> Hi,
> 
> This is version 3 of support for the Broadcom BCM7xxx Set-Top Box NAND
> controller. This controller has been used in a variety of Broadcom SoCs.
> 
> This series now adds support for a few new chips: BCM63138, and the iProc chip
> family. These add an additional 6 new patches to the original 4. If the only
> comments end up on the latter 6 patches, the first 4 might be worth merging
> independently.

This looks great, thanks!

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>

on 63138DVT:

[    0.575390] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xf1
[    0.581885] nand: Micron MT29F1G08ABADAWP
[    0.585979] nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048,
OOB size: 64
[    0.593864] brcmstb_nand fffea000.nand: detected 128MiB total, 128KiB
blocks, 2KiB pages, 164
[    0.593864]
[    0.605742] Scanning device for bad blocks

> 
> Summary changelog:
> 
> v1 -> v2:
>  * add NAND to DTS for BCM7445 / BCM97445SVMB
>  * rename DT binding file to have 'brcm,' prefix
>  * catch DMA mapping errors
>  * fixup timeout / error messages (hex, remove misleading info)
>  * MODULE_LICENSE("GPL v2")
>  * fix incorrect comments
>  * print why we fail, when checking for supported controller revisions
>  * disable prefetch when using Flash DMA (see FIXME); will re-enable once we
>    get a good erased-page verification scheme merged
> 
> v2 -> v3:
>  * rebase to v4.1-rc1
>  * add SoC-specific infrastructure, to help support other SoCs:
>    - add BCM63138 support
>    - add iProc/Cygnus support
>  * disable prefetch on v6.1
> 
> Brian Norris (8):
>   mtd: nand: add common DT init code
>   Documentation: devicetree: add binding doc for Broadcom NAND
>     controller
>   mtd: nand: add NAND driver for Broadcom STB NAND controller
>   ARM: bcm7445: add NAND to DTS
>   Documentation: devicetree: brcmstb_nand: add 'brcm,nand-soc' bindings
>   mtd: brcmstb_nand: add SoC-specific support
>   mtd: brcsmtb_nand_soc: add support for BCM63138
>   ARM: bcm63138: add NAND DT support
> 
> Ray Jui (2):
>   mtd: brcsmtb_nand_soc: add iProc support
>   ARM: dts: cygnus: Enable NAND support for Cygnus
> 
>  .../devicetree/bindings/mtd/brcm,brcmstb-nand.txt  |  147 ++
>  arch/arm/boot/dts/bcm-cygnus.dtsi                  |   20 +
>  arch/arm/boot/dts/bcm63138.dtsi                    |   17 +
>  arch/arm/boot/dts/bcm7445-bcm97445svmb.dts         |   23 +
>  arch/arm/boot/dts/bcm7445.dtsi                     |   22 +
>  arch/arm/boot/dts/bcm958300k.dts                   |   16 +
>  arch/arm/boot/dts/bcm963138dvt.dts                 |   12 +
>  drivers/mtd/nand/Kconfig                           |    8 +
>  drivers/mtd/nand/Makefile                          |    2 +
>  drivers/mtd/nand/brcmnand.h                        |   56 +
>  drivers/mtd/nand/brcmstb_nand.c                    | 2263 ++++++++++++++++++++
>  drivers/mtd/nand/brcmstb_nand_soc.c                |  244 +++
>  drivers/mtd/nand/nand_base.c                       |   41 +
>  include/linux/mtd/nand.h                           |    5 +
>  14 files changed, 2876 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/brcm,brcmstb-nand.txt
>  create mode 100644 drivers/mtd/nand/brcmnand.h
>  create mode 100644 drivers/mtd/nand/brcmstb_nand.c
>  create mode 100644 drivers/mtd/nand/brcmstb_nand_soc.c
> 


-- 
Florian

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Arnd Bergmann]

On Wednesday 06 May 2015 14:18:47 Ray Jui wrote:
> 
> On 5/6/2015 2:05 PM, Brian Norris wrote:
> > On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
> >> On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
> >>> +
> >>> +static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
> >>> +{
> >>> +       return __raw_readl(ctrl->nand_base + offs);
> >>> +}
> >>> +
> >>> +static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
> >>> +                                u32 val)
> >>> +{
> >>> +       __raw_writel(val, ctrl->nand_base + offs);
> >>> +}
> >>> +
> >>>
> >>
> >> You had mentioned previously that there might be an endianess issue in this
> >> driver.
> > 
> > Might. I have a patch already, but I failed to boot a BE kernel, so I
> > kept it out for now. If you don't mind, I'd prefer patching something
> > like this once it's testable on ARM BE. This *is*, however, extensively
> > tested on MIPS (LE and BE) and ARM (LE).
> 
> Correct, extensive test and pass all MTD test cases. We should
> eventually be able to test this on a working ARM BE platform, within the
> next couple months.

I'm fine with not testing it on ARM, as long as the code is written in a
way that is plausible to be correct and not obviously broken.


> >> I think this won't work on big-endian architectures other than MIPS,
> >> so it would be good to either list in the DT the endianess of the device
> >> and use appropriate accessors here, or hardcode it based on the architecture
> >> (using ioread32_be in big-endian mips, but readl elsewhere).
> > 
> > I suspect we wouldn't need a DT property but could just special-case
> > MIPS BE, as you note.

Ok


There is one twist here that I forgot to mention:

This loop in brcmnand_read_by_pio() and the respective one in
brcmnand_write_by_pio():

+               if (likely(buf))
+                       for (j = 0; j < FC_WORDS; j++, buf++)
+                               *buf = brcmnand_read_fc(ctrl, j);

should be converted to use ioread32_rep(). There are two reasons for
this: 

a) accessing the flash data is inherently different from accessing an
   mmio register, and you want the bytes to end up in memory in the same
   order that they are in flash. ioread32_rep() uses __raw_readl()
   internally for this purpose, except on architectures that have a
   byte flipping hardware on the bus interface.

b) The implementation is optimized on ARM and will likely give you
   higher throughput than a manual loop using readl().

> >> Using __raw_writel has another problem regarding the DMA capability of this
> >> driver, as it will not flush any write buffers or synchronize caches before
> >> sending data off to the device, so you risk data corruption.
> > 
> > We use mb() before kicking off DMA or other commands.

Ok, that should work, but will be a stronger barrier than necessary on some
architectures. On ARM, mb() is 'dsb(); outer_sync();', while readl only
needs a 'dsb()' and writel() can use dsb(st) that is slightly weaker than
a full dsb().

> >> Also, the
> >> compiler can choose to split up the 32-bit word access into byte accesses,
> >> which on most hardware does not do what you want.
> > 
> > Huh? Wouldn't that break just about every driver in existence? And how
> > is writel() any different than __raw_writel() in that regard? From
> > include/asm-generic/io.h:
> > 
> > static inline void writel(u32 value, volatile void __iomem *addr)
> > {
> >         __raw_writel(__cpu_to_le32(value), addr);
> > }
> > 
> > And BTW, splitting isn't possible on ARM. From
> > arch/arm/include/asm/io.h:
> > 
> > static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> > {
> >         asm volatile("str %1, %0"
> >                      : "+Qo" (*(volatile u32 __force *)addr)
> >                      : "r" (val));
> > }
> > 

Ah right, we changed that one to simplify KVM support. It used to just
do a volatile store for __raw_* but use an assembly for writel_relaxed().

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Arnd Bergmann]

On Wednesday 06 May 2015 13:49:10 Brian Norris wrote:
> On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
> > On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
> > > +       /*
> > > +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
> > > +        * interesting ways
> > > +        */
> > > +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
> > > +               struct device_node *soc_dn;
> > > +
> > > +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
> > > +               if (!soc_dn)
> > > +                       return -ENODEV;
> > > +
> > > +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
> > > +               if (!ctrl->soc) {
> > > +                       dev_err(dev, "could not probe SoC data\n");
> > > +                       of_node_put(soc_dn);
> > > +                       return -ENODEV;
> > > +               }
> > > +
> > > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
> > > +                                      DRV_NAME, ctrl);
> > > +
> > > +               /* Enable interrupt */
> > > +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
> > > +
> > > +               of_node_put(soc_dn);
> > > +       } else {
> > > +               /* Use standard interrupt infrastructure */
> > > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
> > > +                                      DRV_NAME, ctrl);
> > > +       }
> > > 
> > 
> > It looks to me like this should be handled as a nested irqchip, so the node
> > you look up gets used as the "interrupt-parent" instead, making the behavior
> > of this SoC transparent to the nand driver.
> 
> You snipped the rest of the patch, which involves more than just IRQ
> handling. The same registers touch both interrupts and data bus endian
> configuration, so it can't possibly be done transparently to the NAND
> driver.

Anything else in there? The bus configuration would just involve writing
a constant value in some register, right? Doing that in the irqchip
is also a bit ugly, but may still be better overall than doing it the
way you have above.

> > We recently merged nested irqdomain support as well, which might help here,
> > or might not be needed.
> 
> I'm not familiar with nested irqdomains. Do they address anything like
> the above problem?

The problem that nested irqdomains address is when an interrupt is handled
by two irqchips, in particular when one irqchip handles a virtual interrupt
number that was claimed by another irqchip already.

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

On Thu, May 07, 2015 at 12:01:02PM +0200, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 13:49:10 Brian Norris wrote:
> > On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
> > > > +       /*
> > > > +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
> > > > +        * interesting ways
> > > > +        */
> > > > +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
> > > > +               struct device_node *soc_dn;
> > > > +
> > > > +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
> > > > +               if (!soc_dn)
> > > > +                       return -ENODEV;
> > > > +
> > > > +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
> > > > +               if (!ctrl->soc) {
> > > > +                       dev_err(dev, "could not probe SoC data\n");
> > > > +                       of_node_put(soc_dn);
> > > > +                       return -ENODEV;
> > > > +               }
> > > > +
> > > > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
> > > > +                                      DRV_NAME, ctrl);
> > > > +
> > > > +               /* Enable interrupt */
> > > > +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
> > > > +
> > > > +               of_node_put(soc_dn);
> > > > +       } else {
> > > > +               /* Use standard interrupt infrastructure */
> > > > +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
> > > > +                                      DRV_NAME, ctrl);
> > > > +       }
> > > > 
> > > 
> > > It looks to me like this should be handled as a nested irqchip, so the node
> > > you look up gets used as the "interrupt-parent" instead, making the behavior
> > > of this SoC transparent to the nand driver.
> > 
> > You snipped the rest of the patch, which involves more than just IRQ
> > handling. The same registers touch both interrupts and data bus endian
> > configuration, so it can't possibly be done transparently to the NAND
> > driver.
> 
> Anything else in there?

Looks like miscellaneous NAND-related control bits. AXI and APB endian
configuration; several interrupt-enable bits (we only use one for now);
a clock-enable; and some timing test mode bits.

> The bus configuration would just involve writing
> a constant value in some register, right?

I'm not an expert on the Cygnus/iProc chips, but I believe the answer is
no: we *must* reconfigure the bus before and after each data
transaction, because it affects the rest of the core too. Others on the
CC list can probably elaborate.

> Doing that in the irqchip
> is also a bit ugly, but may still be better overall than doing it the
> way you have above.

Well, the Cygnus/iProc case is more complicated as I mention. But I
agree that other cases could be nicer, like bcm63138 (which only has
separate interrupt status/enable registers). Do you expect a new irqchip
driver for every arrangement of registers like this? There are a few
similar chips that have status/enable registers in different orders, and
some that combine them into a single word. Do we really need a new
irqchip driver for each one? I might have done that for bcm63138 and
bcm3384, except that I thought I'd have to fall back to this extra
per-SoC support driver for Cygnus anyway.

> > > We recently merged nested irqdomain support as well, which might help here,
> > > or might not be needed.
> > 
> > I'm not familiar with nested irqdomains. Do they address anything like
> > the above problem?
> 
> The problem that nested irqdomains address is when an interrupt is handled
> by two irqchips, in particular when one irqchip handles a virtual interrupt
> number that was claimed by another irqchip already.

I'll do some reading on that, but it definitely doesn't address the main
problem here.

Brian

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Ray Jui]

On 5/7/2015 11:42 AM, Brian Norris wrote:
> On Thu, May 07, 2015 at 12:01:02PM +0200, Arnd Bergmann wrote:
>> On Wednesday 06 May 2015 13:49:10 Brian Norris wrote:
>>> On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
>>>> On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
>>>>> +       /*
>>>>> +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
>>>>> +        * interesting ways
>>>>> +        */
>>>>> +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
>>>>> +               struct device_node *soc_dn;
>>>>> +
>>>>> +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
>>>>> +               if (!soc_dn)
>>>>> +                       return -ENODEV;
>>>>> +
>>>>> +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
>>>>> +               if (!ctrl->soc) {
>>>>> +                       dev_err(dev, "could not probe SoC data\n");
>>>>> +                       of_node_put(soc_dn);
>>>>> +                       return -ENODEV;
>>>>> +               }
>>>>> +
>>>>> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
>>>>> +                                      DRV_NAME, ctrl);
>>>>> +
>>>>> +               /* Enable interrupt */
>>>>> +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
>>>>> +
>>>>> +               of_node_put(soc_dn);
>>>>> +       } else {
>>>>> +               /* Use standard interrupt infrastructure */
>>>>> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
>>>>> +                                      DRV_NAME, ctrl);
>>>>> +       }
>>>>>
>>>>
>>>> It looks to me like this should be handled as a nested irqchip, so the node
>>>> you look up gets used as the "interrupt-parent" instead, making the behavior
>>>> of this SoC transparent to the nand driver.
>>>
>>> You snipped the rest of the patch, which involves more than just IRQ
>>> handling. The same registers touch both interrupts and data bus endian
>>> configuration, so it can't possibly be done transparently to the NAND
>>> driver.
>>
>> Anything else in there?
> 
> Looks like miscellaneous NAND-related control bits. AXI and APB endian
> configuration; several interrupt-enable bits (we only use one for now);
> a clock-enable; and some timing test mode bits.
> 
>> The bus configuration would just involve writing
>> a constant value in some register, right?
> 
> I'm not an expert on the Cygnus/iProc chips, but I believe the answer is
> no: we *must* reconfigure the bus before and after each data
> transaction, because it affects the rest of the core too. Others on the
> CC list can probably elaborate.
> 

Yes, we must configure the bus before the after each data/cache
registers access, because it changes the APB bus endianess.

Thanks,

Ray

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Florian Fainelli]

On 07/05/15 03:01, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 13:49:10 Brian Norris wrote:
>> On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
>>> On Wednesday 06 May 2015 10:59:50 Brian Norris wrote:
>>>> +       /*
>>>> +        * Some SoCs integrate this controller (e.g., its interrupt bits) in
>>>> +        * interesting ways
>>>> +        */
>>>> +       if (of_property_read_bool(dn, "brcm,nand-soc")) {
>>>> +               struct device_node *soc_dn;
>>>> +
>>>> +               soc_dn = of_parse_phandle(dn, "brcm,nand-soc", 0);
>>>> +               if (!soc_dn)
>>>> +                       return -ENODEV;
>>>> +
>>>> +               ctrl->soc = devm_brcmnand_probe_soc(dev, soc_dn);
>>>> +               if (!ctrl->soc) {
>>>> +                       dev_err(dev, "could not probe SoC data\n");
>>>> +                       of_node_put(soc_dn);
>>>> +                       return -ENODEV;
>>>> +               }
>>>> +
>>>> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_irq, 0,
>>>> +                                      DRV_NAME, ctrl);
>>>> +
>>>> +               /* Enable interrupt */
>>>> +               ctrl->soc->ctlrdy_set_enabled(ctrl->soc, true);
>>>> +
>>>> +               of_node_put(soc_dn);
>>>> +       } else {
>>>> +               /* Use standard interrupt infrastructure */
>>>> +               ret = devm_request_irq(dev, ctrl->irq, brcmnand_ctlrdy_irq, 0,
>>>> +                                      DRV_NAME, ctrl);
>>>> +       }
>>>>
>>>
>>> It looks to me like this should be handled as a nested irqchip, so the node
>>> you look up gets used as the "interrupt-parent" instead, making the behavior
>>> of this SoC transparent to the nand driver.
>>
>> You snipped the rest of the patch, which involves more than just IRQ
>> handling. The same registers touch both interrupts and data bus endian
>> configuration, so it can't possibly be done transparently to the NAND
>> driver.
> 
> Anything else in there? The bus configuration would just involve writing
> a constant value in some register, right? Doing that in the irqchip
> is also a bit ugly, but may still be better overall than doing it the
> way you have above.

IMHO this is uglier, having a pseudo interrupt controller driver that
also takes care of preparing bus transfers, as opposed to an ad-hoc
piece of code that does not pretend to be an interrupt controller at all
sounds like the former is lying about what it is, while the latter is
pretty straight forward even though it may duplicate an existing subsystem.

I would definitively see some value in writing an irqchip driver if this
was remotely useful outside of the NAND block, e.g: the interrupt bits
would serve other peripherals than NAND, which is not the case for 63138
and 338x AFAICT, Cygnus is a special case.

I could very well imagine a near future where this controller gets added
more features in the DMA/data-path that may require bus/chip-level glue
code to be interfaced properly between Broadcom's different internal
buses. In which case, the interrupt controller portion of the code could
be much smaller than the bus/data-path logic, would it still make sense
to pretend this to be an interrupt controller?
-- 
Florian

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Brian Norris]

On Thu, May 07, 2015 at 11:25:29AM +0200, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 14:18:47 Ray Jui wrote:
> > 
> > On 5/6/2015 2:05 PM, Brian Norris wrote:
> > > On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
> > >> On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
[...]
> There is one twist here that I forgot to mention:
> 
> This loop in brcmnand_read_by_pio() and the respective one in
> brcmnand_write_by_pio():
> 
> +               if (likely(buf))
> +                       for (j = 0; j < FC_WORDS; j++, buf++)
> +                               *buf = brcmnand_read_fc(ctrl, j);
> 
> should be converted to use ioread32_rep().

Huh? That's completely wrong. You're assuming I have a single-register
FIFO, when in fact, I have a memory-mapped hardware buffer. Maybe you're
looking for memcpy_{to,from}io()? I see this is not optimized at all,
though.

> There are two reasons for
> this: 
> 
> a) accessing the flash data is inherently different from accessing an
>    mmio register, and you want the bytes to end up in memory in the same
>    order that they are in flash.

Right, which is why it's a separate helper function in my driver, and it
will stay with __raw_{read,write}l().

>    ioread32_rep() uses __raw_readl()
>    internally for this purpose, except on architectures that have a
>    byte flipping hardware on the bus interface.
> 
> b) The implementation is optimized on ARM and will likely give you
>    higher throughput than a manual loop using readl().

You suggested the wrong helper, and the "right" helper is *not*
optimized. It even has comments saying "this needs to be optimized".

> > >> Using __raw_writel has another problem regarding the DMA capability of this
> > >> driver, as it will not flush any write buffers or synchronize caches before
> > >> sending data off to the device, so you risk data corruption.
> > > 
> > > We use mb() before kicking off DMA or other commands.
> 
> Ok, that should work, but will be a stronger barrier than necessary on some
> architectures. On ARM, mb() is 'dsb(); outer_sync();', while readl only
> needs a 'dsb()' and writel() can use dsb(st) that is slightly weaker than
> a full dsb().
> 
> > >> Also, the
> > >> compiler can choose to split up the 32-bit word access into byte accesses,
> > >> which on most hardware does not do what you want.
> > > 
> > > Huh? Wouldn't that break just about every driver in existence? And how
> > > is writel() any different than __raw_writel() in that regard? From
> > > include/asm-generic/io.h:
> > > 
> > > static inline void writel(u32 value, volatile void __iomem *addr)
> > > {
> > >         __raw_writel(__cpu_to_le32(value), addr);
> > > }
> > > 
> > > And BTW, splitting isn't possible on ARM. From
> > > arch/arm/include/asm/io.h:
> > > 
> > > static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> > > {
> > >         asm volatile("str %1, %0"
> > >                      : "+Qo" (*(volatile u32 __force *)addr)
> > >                      : "r" (val));
> > > }
> > > 
> 
> Ah right, we changed that one to simplify KVM support. It used to just
> do a volatile store for __raw_* but use an assembly for writel_relaxed().

While the ARM case is rock-solid in my favor, I would appreciate an
answer to the asm-generic case too; do you really expect that any sane
compiler would break up word-aligned volatile stores into smaller (e.g.,
8-bit) stores? As I said, I think that means every driver written in C
is broken, not just the ones using your pet enemies,
__raw_{read,write}l().

Brian

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Brian Norris]

On Thu, May 07, 2015 at 11:25:29AM +0200, Arnd Bergmann wrote:
> On Wednesday 06 May 2015 14:18:47 Ray Jui wrote:
> > On 5/6/2015 2:05 PM, Brian Norris wrote:
> > > On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
> > >> You had mentioned previously that there might be an endianess issue in this
> > >> driver.
> > > 
> > > Might. I have a patch already, but I failed to boot a BE kernel, so I
> > > kept it out for now. If you don't mind, I'd prefer patching something
> > > like this once it's testable on ARM BE. This *is*, however, extensively
> > > tested on MIPS (LE and BE) and ARM (LE).
> > 
> > Correct, extensive test and pass all MTD test cases. We should
> > eventually be able to test this on a working ARM BE platform, within the
> > next couple months.
> 
> I'm fine with not testing it on ARM, as long as the code is written in a
> way that is plausible to be correct and not obviously broken.

Would this satisfy you?

From: Brian Norris <computersforpeace@gmail.com>
Date: Tue, 5 May 2015 17:46:42 -0700
Subject: [PATCH] mtd: brcmstb_nand: fixup endianness assumptions

All users of this controller (MIPS or ARM) have previously used native
I/O (__raw{read,write}l()) to access registers. This is normal for the
MIPS case, where BMIPS chips often have a boot-strap that configures not
only the CPU, but also all the busing, to use a given endianness.
However, newer ARM cores support switching to big endian mode at
runtime, which would leave us with different bus and CPU endianness. For
these cases, we should use the endian-switching accessors, so we
continue to access the NAND core in little endian mode.

Suggested by Arnd.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/brcmnand.h         | 25 +++++++++++++++++++++++++
 drivers/mtd/nand/brcmstb_nand.c     |  8 ++++----
 drivers/mtd/nand/brcmstb_nand_soc.c | 20 ++++++++++----------
 3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/brcmnand.h b/drivers/mtd/nand/brcmnand.h
index 59e0bfef2331..8fa5213b591d 100644
--- a/drivers/mtd/nand/brcmnand.h
+++ b/drivers/mtd/nand/brcmnand.h
@@ -53,4 +53,29 @@ static inline void brcmnand_soc_data_bus_unprepare(struct brcmnand_soc *soc)
 		soc->prepare_data_bus(soc, false);
 }
 
+static inline u32 brcmnand_readl(void __iomem *addr)
+{
+	/*
+	 * MIPS endianness is configured by boot strap, which also reverses all
+	 * bus endianness (i.e., big-endian CPU + big endian bus ==> native
+	 * endian I/O).
+	 *
+	 * Other architectures (e.g., ARM) either do not support big endian, or
+	 * else leave I/O in little endian mode.
+	 */
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(__BIG_ENDIAN))
+		return __raw_readl(addr);
+	else
+		return readl_relaxed(addr);
+}
+
+static inline void brcmnand_writel(u32 val, void __iomem *addr)
+{
+	/* See brcmnand_readl() comments */
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(__BIG_ENDIAN))
+		__raw_writel(val, addr);
+	else
+		writel_relaxed(val, addr);
+}
+
 #endif /* __BRCMNAND_H__ */
diff --git a/drivers/mtd/nand/brcmstb_nand.c b/drivers/mtd/nand/brcmstb_nand.c
index 5abc88cfe702..cdd53edf4ed3 100644
--- a/drivers/mtd/nand/brcmstb_nand.c
+++ b/drivers/mtd/nand/brcmstb_nand.c
@@ -357,13 +357,13 @@ enum {
 
 static inline u32 nand_readreg(struct brcmnand_controller *ctrl, u32 offs)
 {
-	return __raw_readl(ctrl->nand_base + offs);
+	return brcmnand_readl(ctrl->nand_base + offs);
 }
 
 static inline void nand_writereg(struct brcmnand_controller *ctrl, u32 offs,
 				 u32 val)
 {
-	__raw_writel(val, ctrl->nand_base + offs);
+	brcmnand_writel(val, ctrl->nand_base + offs);
 }
 
 static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
@@ -698,12 +698,12 @@ static inline bool flash_dma_buf_ok(const void *buf)
 static inline void flash_dma_writel(struct brcmnand_controller *ctrl, u8 offs,
 				    u32 val)
 {
-	__raw_writel(val, ctrl->flash_dma_base + offs);
+	brcmnand_writel(val, ctrl->flash_dma_base + offs);
 }
 
 static inline u32 flash_dma_readl(struct brcmnand_controller *ctrl, u8 offs)
 {
-	return __raw_readl(ctrl->flash_dma_base + offs);
+	return brcmnand_readl(ctrl->flash_dma_base + offs);
 }
 
 /* Low-level operation types: command, address, write, or read */
diff --git a/drivers/mtd/nand/brcmstb_nand_soc.c b/drivers/mtd/nand/brcmstb_nand_soc.c
index 5fb851c655f2..09a714ef57dd 100644
--- a/drivers/mtd/nand/brcmstb_nand_soc.c
+++ b/drivers/mtd/nand/brcmstb_nand_soc.c
@@ -43,10 +43,10 @@ static bool bcm63138_nand_intc_ack(struct brcmnand_soc *soc)
 {
 	struct bcm63138_nand_soc_priv *priv = soc->priv;
 	void __iomem *mmio = priv->base + BCM63138_NAND_INT_STATUS;
-	u32 val = __raw_readl(mmio);
+	u32 val = brcmnand_readl(mmio);
 
 	if (val & BCM63138_CTLRDY) {
-		__raw_writel(val & ~BCM63138_CTLRDY, mmio);
+		brcmnand_writel(val & ~BCM63138_CTLRDY, mmio);
 		return true;
 	}
 
@@ -57,14 +57,14 @@ static void bcm63138_nand_intc_set(struct brcmnand_soc *soc, bool en)
 {
 	struct bcm63138_nand_soc_priv *priv = soc->priv;
 	void __iomem *mmio = priv->base + BCM63138_NAND_INT_EN;
-	u32 val = __raw_readl(mmio);
+	u32 val = brcmnand_readl(mmio);
 
 	if (en)
 		val |= BCM63138_CTLRDY;
 	else
 		val &= ~BCM63138_CTLRDY;
 
-	__raw_writel(val, mmio);
+	brcmnand_writel(val, mmio);
 }
 
 static int bcm63138_nand_soc_init(struct brcmnand_soc *soc)
@@ -110,10 +110,10 @@ static bool iproc_nand_intc_ack(struct brcmnand_soc *soc)
 {
 	struct iproc_nand_soc_priv *priv = soc->priv;
 	void __iomem *mmio = priv->ext_base + IPROC_NAND_CTLR_READY_OFFSET;
-	u32 val = __raw_readl(mmio);
+	u32 val = brcmnand_readl(mmio);
 
 	if (val & IPROC_NAND_CTLR_READY) {
-		__raw_writel(IPROC_NAND_CTLR_READY, mmio);
+		brcmnand_writel(IPROC_NAND_CTLR_READY, mmio);
 		return true;
 	}
 
@@ -129,14 +129,14 @@ static void iproc_nand_intc_set(struct brcmnand_soc *soc, bool en)
 
 	spin_lock_irqsave(&priv->idm_lock, flags);
 
-	val = __raw_readl(mmio);
+	val = brcmnand_readl(mmio);
 
 	if (en)
 		val |= IPROC_NAND_INT_CTRL_READ_ENABLE;
 	else
 		val &= ~IPROC_NAND_INT_CTRL_READ_ENABLE;
 
-	__raw_writel(val, mmio);
+	brcmnand_writel(val, mmio);
 
 	spin_unlock_irqrestore(&priv->idm_lock, flags);
 }
@@ -180,14 +180,14 @@ static void iproc_nand_apb_access(struct brcmnand_soc *soc, bool prepare)
 
 	spin_lock_irqsave(&priv->idm_lock, flags);
 
-	val = __raw_readl(mmio);
+	val = brcmnand_readl(mmio);
 
 	if (prepare)
 		val |= IPROC_NAND_APB_LE_MODE;
 	else
 		val &= ~IPROC_NAND_APB_LE_MODE;
 
-	__raw_writel(val, mmio);
+	brcmnand_writel(val, mmio);
 
 	spin_unlock_irqrestore(&priv->idm_lock, flags);
 }
-- 
1.9.1

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Arnd Bergmann]

On Thursday 07 May 2015 11:52:11 Brian Norris wrote:
> On Thu, May 07, 2015 at 11:25:29AM +0200, Arnd Bergmann wrote:
> > On Wednesday 06 May 2015 14:18:47 Ray Jui wrote:
> > > 
> > > On 5/6/2015 2:05 PM, Brian Norris wrote:
> > > > On Wed, May 06, 2015 at 09:17:36PM +0200, Arnd Bergmann wrote:
> > > >> On Wednesday 06 May 2015 10:59:47 Brian Norris wrote:
> [...]
> > There is one twist here that I forgot to mention:
> > 
> > This loop in brcmnand_read_by_pio() and the respective one in
> > brcmnand_write_by_pio():
> > 
> > +               if (likely(buf))
> > +                       for (j = 0; j < FC_WORDS; j++, buf++)
> > +                               *buf = brcmnand_read_fc(ctrl, j);
> > 
> > should be converted to use ioread32_rep().
> 
> Huh? That's completely wrong. You're assuming I have a single-register
> FIFO, when in fact, I have a memory-mapped hardware buffer. Maybe you're
> looking for memcpy_{to,from}io()? I see this is not optimized at all,
> though.

Ah, my mistake. So the brcmnand_read_fc() has to just keep using __raw_readl
here, unlike the other accessors that should (on non-MIPS) use readl_relaxed
or readl.

> 
> > There are two reasons for
> > this: 
> > 
> > a) accessing the flash data is inherently different from accessing an
> >    mmio register, and you want the bytes to end up in memory in the same
> >    order that they are in flash.
> 
> Right, which is why it's a separate helper function in my driver, and it
> will stay with __raw_{read,write}l().

Ok.

> >    ioread32_rep() uses __raw_readl()
> >    internally for this purpose, except on architectures that have a
> >    byte flipping hardware on the bus interface.
> > 
> > b) The implementation is optimized on ARM and will likely give you
> >    higher throughput than a manual loop using readl().
> 
> You suggested the wrong helper, and the "right" helper is *not*
> optimized. It even has comments saying "this needs to be optimized".

Yes, I was misreading the code and missed the fact that you implement
a memcpy, not read-from-fifo in that loop. For the fifo case, the
optimized implementation is in arch/arm/lib/io-readsl.S, while you
are right that the ARM implementation of memcpy_fromio is pessimal
by doing byte sized accesses, so don't use that. Your loop is fine.

> > 
> > > >> Also, the
> > > >> compiler can choose to split up the 32-bit word access into byte accesses,
> > > >> which on most hardware does not do what you want.
> > > > 
> > > > Huh? Wouldn't that break just about every driver in existence? And how
> > > > is writel() any different than __raw_writel() in that regard? From
> > > > include/asm-generic/io.h:
> > > > 
> > > > static inline void writel(u32 value, volatile void __iomem *addr)
> > > > {
> > > >         __raw_writel(__cpu_to_le32(value), addr);
> > > > }
> > > > 
> > > > And BTW, splitting isn't possible on ARM. From
> > > > arch/arm/include/asm/io.h:
> > > > 
> > > > static inline void __raw_writel(u32 val, volatile void __iomem *addr)
> > > > {
> > > >         asm volatile("str %1, %0"
> > > >                      : "+Qo" (*(volatile u32 __force *)addr)
> > > >                      : "r" (val));
> > > > }
> > > > 
> > 
> > Ah right, we changed that one to simplify KVM support. It used to just
> > do a volatile store for __raw_* but use an assembly for writel_relaxed().
> 
> While the ARM case is rock-solid in my favor, I would appreciate an
> answer to the asm-generic case too; do you really expect that any sane
> compiler would break up word-aligned volatile stores into smaller (e.g.,
> 8-bit) stores? As I said, I think that means every driver written in C
> is broken, not just the ones using your pet enemies,
> __raw_{read,write}l().

Yes, we've had actual bugs in the past where it broke from one gcc
release to the next. The reason it broke there was that the pointer
was assigned (in violation of the C standard) from a pointer of smaller
alignment. If you have code that does:


	char __iomem *p1 = ...;
	void __iomem *p2 = p1;
	int __iomem *p3 = p2;
	u32 data = __raw_readl(p3);

then gcc may decide that it is not safe to do 32-bit aligned accesses
because it does not know the alignment of p1.

	Arnd

Re: [PATCH v3 03/10] mtd: nand: add NAND driver for Broadcom STB NAND controller

[Arnd Bergmann]

On Thursday 07 May 2015 19:01:13 Brian Norris wrote:
> 
> Would this satisfy you?
> 
> From: Brian Norris <computersforpeace@gmail.com>
> Date: Tue, 5 May 2015 17:46:42 -0700
> Subject: [PATCH] mtd: brcmstb_nand: fixup endianness assumptions
> 
> All users of this controller (MIPS or ARM) have previously used native
> I/O (__raw{read,write}l()) to access registers. This is normal for the
> MIPS case, where BMIPS chips often have a boot-strap that configures not
> only the CPU, but also all the busing, to use a given endianness.
> However, newer ARM cores support switching to big endian mode at
> runtime, which would leave us with different bus and CPU endianness. For
> these cases, we should use the endian-switching accessors, so we
> continue to access the NAND core in little endian mode.
> 
> Suggested by Arnd.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> 

Yes, looks good, thanks!

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Arnd Bergmann]

On Thursday 07 May 2015 11:42:46 Brian Norris wrote:
> On Thu, May 07, 2015 at 12:01:02PM +0200, Arnd Bergmann wrote:
> > On Wednesday 06 May 2015 13:49:10 Brian Norris wrote:
> > > On Wed, May 06, 2015 at 09:12:43PM +0200, Arnd Bergmann wrote:
> > > > It looks to me like this should be handled as a nested irqchip, so the node
> > > > you look up gets used as the "interrupt-parent" instead, making the behavior
> > > > of this SoC transparent to the nand driver.
> > > 
> > > You snipped the rest of the patch, which involves more than just IRQ
> > > handling. The same registers touch both interrupts and data bus endian
> > > configuration, so it can't possibly be done transparently to the NAND
> > > driver.
> > 
> > Anything else in there?
> 
> Looks like miscellaneous NAND-related control bits. AXI and APB endian
> configuration; several interrupt-enable bits (we only use one for now);
> a clock-enable; and some timing test mode bits.

I see. Describing that as an irqchip would indeed be too much of a stretch.

> > The bus configuration would just involve writing
> > a constant value in some register, right?
> 
> I'm not an expert on the Cygnus/iProc chips, but I believe the answer is
> no: we *must* reconfigure the bus before and after each data
> transaction, because it affects the rest of the core too. Others on the
> CC list can probably elaborate.

That would not be a problem I think: the irqchip driver would always
get initialized first, because the main driver would get an -EPROBE_DEFER
when requesting the interrupt line otherwise.

> > Doing that in the irqchip
> > is also a bit ugly, but may still be better overall than doing it the
> > way you have above.
> 
> Well, the Cygnus/iProc case is more complicated as I mention. But I
> agree that other cases could be nicer, like bcm63138 (which only has
> separate interrupt status/enable registers). Do you expect a new irqchip
> driver for every arrangement of registers like this? There are a few
> similar chips that have status/enable registers in different orders, and
> some that combine them into a single word. Do we really need a new
> irqchip driver for each one? I might have done that for bcm63138 and
> bcm3384, except that I thought I'd have to fall back to this extra
> per-SoC support driver for Cygnus anyway.

I assumed this one was the only odd one.

> > > > We recently merged nested irqdomain support as well, which might help here,
> > > > or might not be needed.
> > > 
> > > I'm not familiar with nested irqdomains. Do they address anything like
> > > the above problem?
> > 
> > The problem that nested irqdomains address is when an interrupt is handled
> > by two irqchips, in particular when one irqchip handles a virtual interrupt
> > number that was claimed by another irqchip already.
> 
> I'll do some reading on that, but it definitely doesn't address the main
> problem here.

Ok, back to the drawing board then: How about turning the probe order around
and splitting the SoC-specific part out into its own platform_driver:

Instead of bus_prepare/bus_unprepare for bcm63138, you'd get a
bcm63138_nand_driver with its own probe() function that calls the
common probe function. That would make the soc specific parts
better contained and match how we normally do abstractions of
similar drivers.

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
> On Thursday 07 May 2015 11:42:46 Brian Norris wrote:
> > On Thu, May 07, 2015 at 12:01:02PM +0200, Arnd Bergmann wrote:
> > > The bus configuration would just involve writing
> > > a constant value in some register, right?
> > 
> > I'm not an expert on the Cygnus/iProc chips, but I believe the answer is
> > no: we *must* reconfigure the bus before and after each data
> > transaction, because it affects the rest of the core too. Others on the
> > CC list can probably elaborate.
> 
> That would not be a problem I think: the irqchip driver would always
> get initialized first, because the main driver would get an -EPROBE_DEFER
> when requesting the interrupt line otherwise.

Huh? I wasn't worried about initialization order. I was worried about
the fact that the "NAND" and "SoC" portions are very integrated when
handling the data path. And I think you agreed that this means we can't
do a straight-up irqchip.

FWIW, I agree that -EPROBE_DEFER can handle initialization ordering
issues...

> > > Doing that in the irqchip
> > > is also a bit ugly, but may still be better overall than doing it the
> > > way you have above.
> > 
> > Well, the Cygnus/iProc case is more complicated as I mention. But I
> > agree that other cases could be nicer, like bcm63138 (which only has
> > separate interrupt status/enable registers). Do you expect a new irqchip
> > driver for every arrangement of registers like this? There are a few
> > similar chips that have status/enable registers in different orders, and
> > some that combine them into a single word. Do we really need a new
> > irqchip driver for each one? I might have done that for bcm63138 and
> > bcm3384, except that I thought I'd have to fall back to this extra
> > per-SoC support driver for Cygnus anyway.
> 
> I assumed this one was the only odd one.

Yes, the Cygnus/iProc are the oddest. The others (BCM33xx (not yet
supported) and BCM63xxx) just have separate one-off interrupt register
blocks.

To be clear, since I'm not sure if you're confused below:

 * Cygnus is a family of chips using the IPROC architecture, coming from
   the Infrastructure/Networking Group; there are BCMxxxx numbers noted
   in arch/arm/mach-bcm/Kconfig for them, but I usually just refer to
   the Cygnus family or the IPROC architecture.

 * BCM63xxx is a class of DSL chips from the Broadband/Connectivity
   Group.

> > > > > We recently merged nested irqdomain support as well, which might help here,
> > > > > or might not be needed.
> > > > 
> > > > I'm not familiar with nested irqdomains. Do they address anything like
> > > > the above problem?
> > > 
> > > The problem that nested irqdomains address is when an interrupt is handled
> > > by two irqchips, in particular when one irqchip handles a virtual interrupt
> > > number that was claimed by another irqchip already.
> > 
> > I'll do some reading on that, but it definitely doesn't address the main
> > problem here.
> 
> Ok, back to the drawing board then: How about turning the probe order around
> and splitting the SoC-specific part out into its own platform_driver:
> 
> Instead of bus_prepare/bus_unprepare for bcm63138, you'd get a

bcm63138 does not need the bus_{,un}prepare. That's for IPROC/Cygnus.

> bcm63138_nand_driver with its own probe() function that calls the
> common probe function. That would make the soc specific parts
> better contained and match how we normally do abstractions of
> similar drivers.

OK, so I can imagine this might require changing the DT binding a bit [1]
(is that your goal?). But what's the intended software difference? [2]
I'll still be passing around the same sorts of callbacks from the
'iproc_nand' probe to the common probe function.

Brian

[1] e.g.:

       nand: nand@18046000 {
               compatible = "brcm,iproc-nand", "brcm,brcmnand-v6.1", "brcm,brcmnand";
               reg = <0x18046000 0x600>, <0xf8105408 0x600>, <0x18046f00 0x20>;
               reg-names = "nand", "iproc-idm", "iproc-ext";
               interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;

               #address-cells = <1>;
               #size-cells = <0>;

               brcm,nand-has-wp;
       };

This captures the extra "iproc-*" register ranges. Then we could have
the iproc_nand driver bind against "brcm,iproc-nand", then call into the
common probe, which would then accept/reject based on
"brcm,brcmnand-vX.Y".

[2] The DT structure from [1] could actually accommodate either driver
structure just fine. So maybe that means it's a better hardware
description?

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Arnd Bergmann]

On Friday 08 May 2015 12:38:50 Brian Norris wrote:
> On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
> > On Thursday 07 May 2015 11:42:46 Brian Norris wrote:
> > > On Thu, May 07, 2015 at 12:01:02PM +0200, Arnd Bergmann wrote:
> > > > The bus configuration would just involve writing
> > > > a constant value in some register, right?
> > > 
> > > I'm not an expert on the Cygnus/iProc chips, but I believe the answer is
> > > no: we *must* reconfigure the bus before and after each data
> > > transaction, because it affects the rest of the core too. Others on the
> > > CC list can probably elaborate.
> > 
> > That would not be a problem I think: the irqchip driver would always
> > get initialized first, because the main driver would get an -EPROBE_DEFER
> > when requesting the interrupt line otherwise.
> 
> Huh? I wasn't worried about initialization order. I was worried about
> the fact that the "NAND" and "SoC" portions are very integrated when
> handling the data path. And I think you agreed that this means we can't
> do a straight-up irqchip.

Sorry, I missed the part about "each data transaction", got it now.
 
> > > > Doing that in the irqchip
> > > > is also a bit ugly, but may still be better overall than doing it the
> > > > way you have above.
> > > 
> > > Well, the Cygnus/iProc case is more complicated as I mention. But I
> > > agree that other cases could be nicer, like bcm63138 (which only has
> > > separate interrupt status/enable registers). Do you expect a new irqchip
> > > driver for every arrangement of registers like this? There are a few
> > > similar chips that have status/enable registers in different orders, and
> > > some that combine them into a single word. Do we really need a new
> > > irqchip driver for each one? I might have done that for bcm63138 and
> > > bcm3384, except that I thought I'd have to fall back to this extra
> > > per-SoC support driver for Cygnus anyway.
> > 
> > I assumed this one was the only odd one.
> 
> Yes, the Cygnus/iProc are the oddest. The others (BCM33xx (not yet
> supported) and BCM63xxx) just have separate one-off interrupt register
> blocks.
> 
> To be clear, since I'm not sure if you're confused below:
> 
>  * Cygnus is a family of chips using the IPROC architecture, coming from
>    the Infrastructure/Networking Group; there are BCMxxxx numbers noted
>    in arch/arm/mach-bcm/Kconfig for them, but I usually just refer to
>    the Cygnus family or the IPROC architecture.
> 
>  * BCM63xxx is a class of DSL chips from the Broadband/Connectivity
>    Group.

Thanks for the clarification, I think that is roughly what I thought it was,
but I'm still not sure about brcmstb. Is that related to bcm63xxx or separate?

> > > > > > We recently merged nested irqdomain support as well, which might help here,
> > > > > > or might not be needed.
> > > > > 
> > > > > I'm not familiar with nested irqdomains. Do they address anything like
> > > > > the above problem?
> > > > 
> > > > The problem that nested irqdomains address is when an interrupt is handled
> > > > by two irqchips, in particular when one irqchip handles a virtual interrupt
> > > > number that was claimed by another irqchip already.
> > > 
> > > I'll do some reading on that, but it definitely doesn't address the main
> > > problem here.
> > 
> > Ok, back to the drawing board then: How about turning the probe order around
> > and splitting the SoC-specific part out into its own platform_driver:
> > 
> > Instead of bus_prepare/bus_unprepare for bcm63138, you'd get a
> 
> bcm63138 does not need the bus_{,un}prepare. That's for IPROC/Cygnus.

Ok.

> > bcm63138_nand_driver with its own probe() function that calls the
> > common probe function. That would make the soc specific parts
> > better contained and match how we normally do abstractions of
> > similar drivers.
> 
> OK, so I can imagine this might require changing the DT binding a bit [1]
> (is that your goal?). But what's the intended software difference? [2]
> I'll still be passing around the same sorts of callbacks from the
> 'iproc_nand' probe to the common probe function.
> 
> Brian
> 
> [1] e.g.:
> 
>        nand: nand@18046000 {
>                compatible = "brcm,iproc-nand", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>                reg = <0x18046000 0x600>, <0xf8105408 0x600>, <0x18046f00 0x20>;
>                reg-names = "nand", "iproc-idm", "iproc-ext";
>                interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> 
>                #address-cells = <1>;
>                #size-cells = <0>;
> 
>                brcm,nand-has-wp;
>        };
> 
> This captures the extra "iproc-*" register ranges. Then we could have
> the iproc_nand driver bind against "brcm,iproc-nand", then call into the
> common probe, which would then accept/reject based on
> "brcm,brcmnand-vX.Y".
> 
> [2] The DT structure from [1] could actually accommodate either driver
> structure just fine. So maybe that means it's a better hardware
> description?

Yes, I think this makes sense overall. Regarding the specific example, can you
clarify how the register areas in iproc are structured?

The 0xf8105408 and 0x18046f00 start addresses are not aligned to large powers
of two, which often indicates that they are part of some other, larger,
unit that might need to have a driver of its own, so before we specify
a binding like the one you proposed above I'd like to make sure we're not
getting ourselves into trouble later.

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

On Fri, May 08, 2015 at 09:49:02PM +0200, Arnd Bergmann wrote:
> On Friday 08 May 2015 12:38:50 Brian Norris wrote:
> > On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
[...]

> > To be clear, since I'm not sure if you're confused below:
> > 
> >  * Cygnus is a family of chips using the IPROC architecture, coming from
> >    the Infrastructure/Networking Group; there are BCMxxxx numbers noted
> >    in arch/arm/mach-bcm/Kconfig for them, but I usually just refer to
> >    the Cygnus family or the IPROC architecture.
> > 
> >  * BCM63xxx is a class of DSL chips from the Broadband/Connectivity
> >    Group.
> 
> Thanks for the clarification, I think that is roughly what I thought it was,
> but I'm still not sure about brcmstb. Is that related to bcm63xxx or separate?

I think arch/arm/mach-bcm/Kconfig has the best summary. brcmstb is
separate; BCM7xxx is generally (always?) Set-Top Box.

Another potentially confusing point: the main driver is named
'brcsmtb_nand' since the NAND core (and driver) originated from STB
chips. But that core was applied to other non-STB chips, and so the
driver has been extended.

> > > bcm63138_nand_driver with its own probe() function that calls the
> > > common probe function. That would make the soc specific parts
> > > better contained and match how we normally do abstractions of
> > > similar drivers.
> > 
> > OK, so I can imagine this might require changing the DT binding a bit [1]
> > (is that your goal?). But what's the intended software difference? [2]
> > I'll still be passing around the same sorts of callbacks from the
> > 'iproc_nand' probe to the common probe function.

^^ before getting bogged down on the DT details (which can be changed
independently), I'd like to address this point.

> > Brian
> > 
> > [1] e.g.:
> > 
> >        nand: nand@18046000 {
> >                compatible = "brcm,iproc-nand", "brcm,brcmnand-v6.1", "brcm,brcmnand";
> >                reg = <0x18046000 0x600>, <0xf8105408 0x600>, <0x18046f00 0x20>;
> >                reg-names = "nand", "iproc-idm", "iproc-ext";
> >                interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> > 
> >                #address-cells = <1>;
> >                #size-cells = <0>;
> > 
> >                brcm,nand-has-wp;
> >        };
> > 
> > This captures the extra "iproc-*" register ranges. Then we could have
> > the iproc_nand driver bind against "brcm,iproc-nand", then call into the
> > common probe, which would then accept/reject based on
> > "brcm,brcmnand-vX.Y".
> > 
> > [2] The DT structure from [1] could actually accommodate either driver
> > structure just fine. So maybe that means it's a better hardware
> > description?
> 
> Yes, I think this makes sense overall. Regarding the specific example, can you
> clarify how the register areas in iproc are structured?
> 
> The 0xf8105408 and 0x18046f00 start addresses are not aligned to large powers
> of two, which often indicates that they are part of some other, larger,
> unit that might need to have a driver of its own, so before we specify
> a binding like the one you proposed above I'd like to make sure we're not
> getting ourselves into trouble later.

I may want the Cygnus guys to speak up here, partly for technical
expertise and partly to know how much they care to share...

<0xf8105408 0x600>: covers a series of NAND_IDM registers. NAND has a
few bits we don't care about (for debugging, logging, and resetting), as
well as its interrupt enable bits. The adjacent blocks cover similar IDM
blocks for other cores (SPI, PNOR, DDR), and they are similarly
unaligned. Not sure why, exactly; probably just a compact layout.

<0x18046f00 0x20>: a series of 8 NAND interrupt registers, each word
containing a single bit representing status/clear. There is nothing
between the "nand" range and this range, and the SPI core register range
follows.

So I think these are pretty clearly-delineated register ranges for NAND,
and the alignment is not really missing anything. Adjacent hardware
(e.g., SPI) is independent, though pieces look similar. For one, it has
similar:

 * interrupt enable bits in the IDM range (0xf8106408 to 0xf8106a00);
   and
 * interrupt status/clear following the SPI block (0x180473a0 to
   0x180473b8)

Brian

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Arnd Bergmann]

On Friday 08 May 2015 13:47:25 Brian Norris wrote:
> On Fri, May 08, 2015 at 09:49:02PM +0200, Arnd Bergmann wrote:
> > On Friday 08 May 2015 12:38:50 Brian Norris wrote:
> > > On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
> [...]
> 
> > > To be clear, since I'm not sure if you're confused below:
> > > 
> > >  * Cygnus is a family of chips using the IPROC architecture, coming from
> > >    the Infrastructure/Networking Group; there are BCMxxxx numbers noted
> > >    in arch/arm/mach-bcm/Kconfig for them, but I usually just refer to
> > >    the Cygnus family or the IPROC architecture.
> > > 
> > >  * BCM63xxx is a class of DSL chips from the Broadband/Connectivity
> > >    Group.
> > 
> > Thanks for the clarification, I think that is roughly what I thought it was,
> > but I'm still not sure about brcmstb. Is that related to bcm63xxx or separate?
> 
> I think arch/arm/mach-bcm/Kconfig has the best summary. brcmstb is
> separate; BCM7xxx is generally (always?) Set-Top Box.
> 
> Another potentially confusing point: the main driver is named
> 'brcsmtb_nand' since the NAND core (and driver) originated from STB
> chips. But that core was applied to other non-STB chips, and so the
> driver has been extended.

Ok, I see.

> > > > bcm63138_nand_driver with its own probe() function that calls the
> > > > common probe function. That would make the soc specific parts
> > > > better contained and match how we normally do abstractions of
> > > > similar drivers.
> > > 
> > > OK, so I can imagine this might require changing the DT binding a bit [1]
> > > (is that your goal?). But what's the intended software difference? [2]
> > > I'll still be passing around the same sorts of callbacks from the
> > > 'iproc_nand' probe to the common probe function.
> 
> ^^ before getting bogged down on the DT details (which can be changed
> independently), I'd like to address this point.

The intended change is to make it work according to
Documentation/driver-model/design-patterns.txt

basically, by having all the shared code be a "library" module that gets
called by the actual hardware specific drivers, rather than having the
shared code be the central driver that fans out into all possible subdrivers.

> > 
> > Yes, I think this makes sense overall. Regarding the specific example, can you
> > clarify how the register areas in iproc are structured?
> > 
> > The 0xf8105408 and 0x18046f00 start addresses are not aligned to large powers
> > of two, which often indicates that they are part of some other, larger,
> > unit that might need to have a driver of its own, so before we specify
> > a binding like the one you proposed above I'd like to make sure we're not
> > getting ourselves into trouble later.
> 
> I may want the Cygnus guys to speak up here, partly for technical
> expertise and partly to know how much they care to share...
> 
> <0xf8105408 0x600>: covers a series of NAND_IDM registers. NAND has a
> few bits we don't care about (for debugging, logging, and resetting), as
> well as its interrupt enable bits. The adjacent blocks cover similar IDM
> blocks for other cores (SPI, PNOR, DDR), and they are similarly
> unaligned. Not sure why, exactly; probably just a compact layout.
> 
> <0x18046f00 0x20>: a series of 8 NAND interrupt registers, each word
> containing a single bit representing status/clear. There is nothing
> between the "nand" range and this range, and the SPI core register range
> follows.
> 
> So I think these are pretty clearly-delineated register ranges for NAND,
> and the alignment is not really missing anything. Adjacent hardware
> (e.g., SPI) is independent, though pieces look similar. For one, it has
> similar:
> 
>  * interrupt enable bits in the IDM range (0xf8106408 to 0xf8106a00);
>    and
>  * interrupt status/clear following the SPI block (0x180473a0 to
>    0x180473b8)

This would in turn indicate that we should treat these ranges as
an irqchip that handles all sorts of devices, but it really depends
on the particular register layout.

	Arnd

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Brian Norris]

On Fri, May 08, 2015 at 11:38:11PM +0200, Arnd Bergmann wrote:
> On Friday 08 May 2015 13:47:25 Brian Norris wrote:
> > On Fri, May 08, 2015 at 09:49:02PM +0200, Arnd Bergmann wrote:
> > > On Friday 08 May 2015 12:38:50 Brian Norris wrote:
> > > > On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
> > > > > bcm63138_nand_driver with its own probe() function that calls the
> > > > > common probe function. That would make the soc specific parts
> > > > > better contained and match how we normally do abstractions of
> > > > > similar drivers.
> > > > 
> > > > OK, so I can imagine this might require changing the DT binding a bit [1]
> > > > (is that your goal?). But what's the intended software difference? [2]
> > > > I'll still be passing around the same sorts of callbacks from the
> > > > 'iproc_nand' probe to the common probe function.
> > 
> > ^^ before getting bogged down on the DT details (which can be changed
> > independently), I'd like to address this point.
> 
> The intended change is to make it work according to
> Documentation/driver-model/design-patterns.txt

Huh? There are two bullet points in that file, and neither are
particularly enlightening for this case. Maybe you're referring to your
mental design patterns documentation? :)

> basically, by having all the shared code be a "library" module that gets
> called by the actual hardware specific drivers, rather than having the
> shared code be the central driver that fans out into all possible subdrivers.

OK, I'll see what I can do. It will be a fairly opaque "library" though,
consisting largely of a single monolithic core driver. Might just move
to a whole drivers/mtd/nand/brcmnand/ subdirectory at the same time...

> > > Yes, I think this makes sense overall. Regarding the specific example, can you
> > > clarify how the register areas in iproc are structured?
> > > 
> > > The 0xf8105408 and 0x18046f00 start addresses are not aligned to large powers
> > > of two, which often indicates that they are part of some other, larger,
> > > unit that might need to have a driver of its own, so before we specify
> > > a binding like the one you proposed above I'd like to make sure we're not
> > > getting ourselves into trouble later.
> > 
> > I may want the Cygnus guys to speak up here, partly for technical
> > expertise and partly to know how much they care to share...
> > 
> > <0xf8105408 0x600>: covers a series of NAND_IDM registers. NAND has a
> > few bits we don't care about (for debugging, logging, and resetting), as
> > well as its interrupt enable bits. The adjacent blocks cover similar IDM
> > blocks for other cores (SPI, PNOR, DDR), and they are similarly
> > unaligned. Not sure why, exactly; probably just a compact layout.
> > 
> > <0x18046f00 0x20>: a series of 8 NAND interrupt registers, each word
> > containing a single bit representing status/clear. There is nothing
> > between the "nand" range and this range, and the SPI core register range
> > follows.
> > 
> > So I think these are pretty clearly-delineated register ranges for NAND,
> > and the alignment is not really missing anything. Adjacent hardware
> > (e.g., SPI) is independent, though pieces look similar. For one, it has
> > similar:
> > 
> >  * interrupt enable bits in the IDM range (0xf8106408 to 0xf8106a00);
> >    and
> >  * interrupt status/clear following the SPI block (0x180473a0 to
> >    0x180473b8)
> 
> This would in turn indicate that we should treat these ranges as
> an irqchip that handles all sorts of devices, but it really depends
> on the particular register layout.

OK, sure. But this has nothing to do with NAND (which we established
cannot be an irqchip on Cygnus). I think SPI is coming through the
pipeline soon, though, and that's a good point.

Brian

Re: [PATCH v3 06/10] mtd: brcmstb_nand: add SoC-specific support

[Ray Jui]

On 5/8/2015 1:47 PM, Brian Norris wrote:
> On Fri, May 08, 2015 at 09:49:02PM +0200, Arnd Bergmann wrote:
>> On Friday 08 May 2015 12:38:50 Brian Norris wrote:
>>> On Fri, May 08, 2015 at 03:41:10PM +0200, Arnd Bergmann wrote:
> [...]
> 
>>> To be clear, since I'm not sure if you're confused below:
>>>
>>>  * Cygnus is a family of chips using the IPROC architecture, coming from
>>>    the Infrastructure/Networking Group; there are BCMxxxx numbers noted
>>>    in arch/arm/mach-bcm/Kconfig for them, but I usually just refer to
>>>    the Cygnus family or the IPROC architecture.
>>>
>>>  * BCM63xxx is a class of DSL chips from the Broadband/Connectivity
>>>    Group.
>>
>> Thanks for the clarification, I think that is roughly what I thought it was,
>> but I'm still not sure about brcmstb. Is that related to bcm63xxx or separate?
> 
> I think arch/arm/mach-bcm/Kconfig has the best summary. brcmstb is
> separate; BCM7xxx is generally (always?) Set-Top Box.
> 
> Another potentially confusing point: the main driver is named
> 'brcsmtb_nand' since the NAND core (and driver) originated from STB
> chips. But that core was applied to other non-STB chips, and so the
> driver has been extended.
> 
>>>> bcm63138_nand_driver with its own probe() function that calls the
>>>> common probe function. That would make the soc specific parts
>>>> better contained and match how we normally do abstractions of
>>>> similar drivers.
>>>
>>> OK, so I can imagine this might require changing the DT binding a bit [1]
>>> (is that your goal?). But what's the intended software difference? [2]
>>> I'll still be passing around the same sorts of callbacks from the
>>> 'iproc_nand' probe to the common probe function.
> 
> ^^ before getting bogged down on the DT details (which can be changed
> independently), I'd like to address this point.
> 
>>> Brian
>>>
>>> [1] e.g.:
>>>
>>>        nand: nand@18046000 {
>>>                compatible = "brcm,iproc-nand", "brcm,brcmnand-v6.1", "brcm,brcmnand";
>>>                reg = <0x18046000 0x600>, <0xf8105408 0x600>, <0x18046f00 0x20>;
>>>                reg-names = "nand", "iproc-idm", "iproc-ext";
>>>                interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
>>>
>>>                #address-cells = <1>;
>>>                #size-cells = <0>;
>>>
>>>                brcm,nand-has-wp;
>>>        };
>>>
>>> This captures the extra "iproc-*" register ranges. Then we could have
>>> the iproc_nand driver bind against "brcm,iproc-nand", then call into the
>>> common probe, which would then accept/reject based on
>>> "brcm,brcmnand-vX.Y".
>>>
>>> [2] The DT structure from [1] could actually accommodate either driver
>>> structure just fine. So maybe that means it's a better hardware
>>> description?
>>
>> Yes, I think this makes sense overall. Regarding the specific example, can you
>> clarify how the register areas in iproc are structured?
>>
>> The 0xf8105408 and 0x18046f00 start addresses are not aligned to large powers
>> of two, which often indicates that they are part of some other, larger,
>> unit that might need to have a driver of its own, so before we specify
>> a binding like the one you proposed above I'd like to make sure we're not
>> getting ourselves into trouble later.
> 
> I may want the Cygnus guys to speak up here, partly for technical
> expertise and partly to know how much they care to share...
> 
> <0xf8105408 0x600>: covers a series of NAND_IDM registers. NAND has a
> few bits we don't care about (for debugging, logging, and resetting), as
> well as its interrupt enable bits. The adjacent blocks cover similar IDM
> blocks for other cores (SPI, PNOR, DDR), and they are similarly
> unaligned. Not sure why, exactly; probably just a compact layout.

Yes, starting from 0xf8105408, within the range of 0x600, there are
various NAND_IDM registers scattered, which is indeed a very weird
register layout.

Like Brian said, most of those NAND_IDM registers are for debugging,
logging, or status reporting. As of today, we only care about the first
register, that contains a bunch of bits that allow you to configure the
endianness of the AXI/APB bus, enabling NAND interrupts and clocks.

> 
> <0x18046f00 0x20>: a series of 8 NAND interrupt registers, each word
> containing a single bit representing status/clear. There is nothing
> between the "nand" range and this range, and the SPI core register range
> follows.

Correct.

> 
> So I think these are pretty clearly-delineated register ranges for NAND,
> and the alignment is not really missing anything. Adjacent hardware
> (e.g., SPI) is independent, though pieces look similar. For one, it has
> similar:
> 
>  * interrupt enable bits in the IDM range (0xf8106408 to 0xf8106a00);
>    and
>  * interrupt status/clear following the SPI block (0x180473a0 to
>    0x180473b8)
> 
> Brian
> 

Re: [PATCH v3 01/10] mtd: nand: add common DT init code

[Brian Norris]

On Wed, May 06, 2015 at 10:59:45AM -0700, Brian Norris wrote:
> These are already-documented common bindings for NAND chips. Let's
> handle them in nand_base.
> 
> If NAND controller drivers need to act on this data before bringing up
> the NAND chip (e.g., fill out ECC callback functions, change HW modes,
> etc.), then they can do so between calling nand_scan_ident() and
> nand_scan_tail().
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Pushed just this patch to l2-mtd.git, since it's relatively
uncontroversial and hasn't changed across the first three revisions. It
should be of benefit to a few other driver writers out there that I've
reviewed.

Brian

> ---
>  drivers/mtd/nand/nand_base.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/nand.h     |  5 +++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index c2e1232cd45c..c6cf775ee431 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -48,6 +48,7 @@
>  #include <linux/leds.h>
>  #include <linux/io.h>
>  #include <linux/mtd/partitions.h>
> +#include <linux/of_mtd.h>
>  
>  /* Define default oob placement schemes for large and small page devices */
>  static struct nand_ecclayout nand_oob_8 = {
> @@ -3798,6 +3799,39 @@ ident_done:
>  	return type;
>  }
>  
> +static int nand_dt_init(struct mtd_info *mtd, struct nand_chip *chip,
> +			struct device_node *dn)
> +{
> +	int ecc_mode, ecc_strength, ecc_step;
> +
> +	if (of_get_nand_bus_width(dn) == 16)
> +		chip->options |= NAND_BUSWIDTH_16;
> +
> +	if (of_get_nand_on_flash_bbt(dn))
> +		chip->bbt_options |= NAND_BBT_USE_FLASH;
> +
> +	ecc_mode = of_get_nand_ecc_mode(dn);
> +	ecc_strength = of_get_nand_ecc_strength(dn);
> +	ecc_step = of_get_nand_ecc_step_size(dn);
> +
> +	if ((ecc_step >= 0 && !(ecc_strength >= 0)) ||
> +	    (!(ecc_step >= 0) && ecc_strength >= 0)) {
> +		pr_err("must set both strength and step size in DT\n");
> +		return -EINVAL;
> +	}
> +
> +	if (ecc_mode >= 0)
> +		chip->ecc.mode = ecc_mode;
> +
> +	if (ecc_strength >= 0)
> +		chip->ecc.strength = ecc_strength;
> +
> +	if (ecc_step > 0)
> +		chip->ecc.size = ecc_step;
> +
> +	return 0;
> +}
> +
>  /**
>   * nand_scan_ident - [NAND Interface] Scan for the NAND device
>   * @mtd: MTD device structure
> @@ -3815,6 +3849,13 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips,
>  	int i, nand_maf_id, nand_dev_id;
>  	struct nand_chip *chip = mtd->priv;
>  	struct nand_flash_dev *type;
> +	int ret;
> +
> +	if (chip->dn) {
> +		ret = nand_dt_init(mtd, chip, chip->dn);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/* Set the default functions */
>  	nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16);
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 3d4ea7eb2b68..e0f40e12a2c8 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -26,6 +26,8 @@
>  
>  struct mtd_info;
>  struct nand_flash_dev;
> +struct device_node;
> +
>  /* Scan and identify a NAND device */
>  extern int nand_scan(struct mtd_info *mtd, int max_chips);
>  /*
> @@ -542,6 +544,7 @@ struct nand_buffers {
>   *			flash device
>   * @IO_ADDR_W:		[BOARDSPECIFIC] address to write the 8 I/O lines of the
>   *			flash device.
> + * @dn:			[BOARDSPECIFIC] device node describing this instance
>   * @read_byte:		[REPLACEABLE] read one byte from the chip
>   * @read_word:		[REPLACEABLE] read one word from the chip
>   * @write_byte:		[REPLACEABLE] write a single byte to the chip on the
> @@ -644,6 +647,8 @@ struct nand_chip {
>  	void __iomem *IO_ADDR_R;
>  	void __iomem *IO_ADDR_W;
>  
> +	struct device_node *dn;
> +
>  	uint8_t (*read_byte)(struct mtd_info *mtd);
>  	u16 (*read_word)(struct mtd_info *mtd);
>  	void (*write_byte)(struct mtd_info *mtd, uint8_t byte);
> -- 
> 1.9.1
>