LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
@ 2015-03-07  1:18 Brian Norris
  2015-03-07  1:18 ` [PATCH 1/3] mtd: nand: add common DT init code Brian Norris
                   ` (4 more replies)
  0 siblings, 5 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-07  1:18 UTC (permalink / raw)
  To: linux-mtd
  Cc: Brian Norris, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

Hi,

This adds (long in coming) support for the Broadcom BCM7xxx Set-Top Box NAND
controller. This controller has been used in a variety of Broadcom SoCs.

There are a few more features I'd like add in the near future, mostly to
support more SoCs, but this is the base set, which should only need relatively
minor additions to support chips like BCM63138, BCM3384, and Cygnus/iProc.
Particularly, we may need to straighten out some endianness issues for the data
path on iProc, and interrupt enabling/acking on iProc, BCM63xxx, BCM3xxx, and
others.

TODO: add this to the DTS(I) files for BCM7445.

Happy reviewing! (Speaking of which, I need to catch up on reviewing everybody
else's MTD submissions...)

Brian

Brian Norris (3):
  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

 .../devicetree/bindings/mtd/brcmstb_nand.txt       |  109 +
 drivers/mtd/nand/Kconfig                           |    8 +
 drivers/mtd/nand/Makefile                          |    1 +
 drivers/mtd/nand/brcmstb_nand.c                    | 2182 ++++++++++++++++++++
 drivers/mtd/nand/nand_base.c                       |   41 +
 include/linux/mtd/nand.h                           |    5 +
 6 files changed, 2346 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
 create mode 100644 drivers/mtd/nand/brcmstb_nand.c

-- 
1.9.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/3] mtd: nand: add common DT init code
  2015-03-07  1:18 [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Brian Norris
@ 2015-03-07  1:18 ` Brian Norris
  2015-03-07  1:18 ` [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller Brian Norris
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-07  1:18 UTC (permalink / raw)
  To: linux-mtd
  Cc: Brian Norris, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

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 df7eb4ff07d1..271866b038b3 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 = {
@@ -3779,6 +3780,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
@@ -3796,6 +3830,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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-07  1:18 [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Brian Norris
  2015-03-07  1:18 ` [PATCH 1/3] mtd: nand: add common DT init code Brian Norris
@ 2015-03-07  1:18 ` Brian Norris
  2015-03-16 18:49   ` Florian Fainelli
  2015-03-16 23:07   ` Scott Branden
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-07  1:18 UTC (permalink / raw)
  To: linux-mtd
  Cc: Brian Norris, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

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

diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
new file mode 100644
index 000000000000..933d44943cbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
@@ -0,0 +1,109 @@
+* 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-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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Brian Norris
  2015-03-07  1:18 ` [PATCH 1/3] mtd: nand: add common DT init code Brian Norris
  2015-03-07  1:18 ` [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller Brian Norris
@ 2015-03-07  1:18 ` Brian Norris
  2015-03-07 12:39   ` Paul Bolle
                     ` (5 more replies)
  2015-03-08  0:01 ` [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Rafał Miłecki
       [not found] ` <CALj_zD5rW3Se27Rh0pL6QTMNGOrrmrvAVLvW3BCuF8RujYQE=g@mail.gmail.com>
  4 siblings, 6 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-07  1:18 UTC (permalink / raw)
  To: linux-mtd
  Cc: Brian Norris, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

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 | 2182 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 2191 insertions(+)
 create mode 100644 drivers/mtd/nand/brcmstb_nand.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a173cd95..6445323a8cff 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..4d74f7a17dc3
--- /dev/null
+++ b/drivers/mtd/nand/brcmstb_nand.c
@@ -0,0 +1,2182 @@
+/*
+ * 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)
+		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;
+
+		/* pre-v5.0 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;
+
+	if (ctrl->nand_version >= 0x0600)
+		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) {
+		unsigned long cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START)
+					>> brcmnand_cmd_shift(ctrl);
+
+		dev_err_ratelimited(ctrl->dev,
+			"timeout waiting for command %u (%ld)\n",
+			host->last_cmd, 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);
+
+	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)
+		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 (!ctrl->nand_base)
+		return -ENODEV;
+
+	/* 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 (!ctrl->nand_fc)
+			return -ENODEV;
+	} 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 (!ctrl->flash_dma_base)
+			return -ENODEV;
+
+		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-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");
+MODULE_AUTHOR("Kevin Cernekee");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("NAND driver for Broadcom STB chips");
+MODULE_ALIAS("platform:brcmnand");
-- 
1.9.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
@ 2015-03-07 12:39   ` Paul Bolle
  2015-03-09 17:30     ` Brian Norris
  2015-03-07 13:21   ` Rafał Miłecki
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 30+ messages in thread
From: Paul Bolle @ 2015-03-07 12:39 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

Only a license nit.

Brian Norris schreef op vr 06-03-2015 om 17:18 [-0800]:

> --- /dev/null
> +++ b/drivers/mtd/nand/brcmstb_nand.c
> @@ -0,0 +1,2182 @@
> +/*
> + * 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.
> + */

This states the license is GPL v2.

> +MODULE_LICENSE("GPL");

So you probably want
   MODULE_LICENSE("GPL v2");

here.


Paul Bolle


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
  2015-03-07 12:39   ` Paul Bolle
@ 2015-03-07 13:21   ` Rafał Miłecki
  2015-03-09 17:31     ` Brian Norris
  2015-03-07 17:39   ` Rafał Miłecki
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-07 13:21 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee

On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> +       /* NAND register range */
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       ctrl->nand_base = devm_ioremap_resource(dev, res);
> +       if (!ctrl->nand_base)
> +               return -ENODEV;

This is what I got during my first test:
brcmstb_nand 11315e0.nand: can't request region for resource [mem
0x011315e0-0x011325df]
Unable to handle kernel paging request at virtual address fffffff0
(not sure why yet).

Hint: devm_ioremap_resource does never return NULL

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
  2015-03-07 12:39   ` Paul Bolle
  2015-03-07 13:21   ` Rafał Miłecki
@ 2015-03-07 17:39   ` Rafał Miłecki
  2015-03-07 21:48     ` Rafał Miłecki
  2015-03-08  0:44     ` Rafał Miłecki
  2015-03-07 22:15   ` Rafał Miłecki
                     ` (2 subsequent siblings)
  5 siblings, 2 replies; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-07 17:39 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens

On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> 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.

Unfortunately, it seems I can't make this driver work with my BCM4708
based router. This is a SoC with 6.01 controller.

Sometimes I get this:
[    0.478443] brcmstb_nand 18028000.nand: timeout waiting for command 255 (9)
[    0.485385] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.588393] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.595326] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.698392] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.705322] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.711051] nand: second ID read did not match 20,20 against 20,00
[    0.717197] nand: No NAND device found

Other time:
[    0.478441] brcmstb_nand 18028000.nand: timeout waiting for command 255 (9)
[    0.485385] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.588396] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.595327] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.698393] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.705321] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.808394] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.815326] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.918392] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    0.925322] brcmstb_nand 18028000.nand: intfc status f0000000
[    0.931047] nand: No NAND device found

(Btw. I guess you could use 0x%x instead of %d for printing commands).

It seems that brcmnand_ctlrdy_irq never fires on my device. Just like
controller was never generating any IRQ.


I started comparing your driver with OpenWrt's bcm_nand.c (which
should be very similar to Broadcom's SDK NAND driver for ARM). Below
are few things I've noticed.

1) In bcm_nand.c IRQ handler also doesn't seem to be fired (or very rarely).

2) In brcmnand_waitfunc you seem to be always waiting for an
interrupt. This is different from OpenWrt's driver and Broadcom's
reference code. In bcm_nand.c in bcmnand_wait_cmd we first check
BIT(31) of IRQ STATUS (0x6c / 0x14 / BRCMNAND_INTFC_STATUS). If it's
set, we assume command was already completed.

3) In bcm_nand.c in IRQ waiting function bcmnand_wait_interrupt we
ack/enable/disable NANDC_IRQ_CONTROLLER_RDY (nand_ctlrdy) IRQ. I don't
see that in your code, not sure if it's important.

4) It seems bcm_nand.c also uses some extra register 0x500 and its bit
0x1 (we call it NANDC_IDM_IO_CTRL_RDY). It this bit it set, we don't
ever care about NANDC_IRQ_CONTROLLER_RDY (nand_ctlrdy) IRQ.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07 17:39   ` Rafał Miłecki
@ 2015-03-07 21:48     ` Rafał Miłecki
  2015-03-08  0:44     ` Rafał Miłecki
  1 sibling, 0 replies; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-07 21:48 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens

On 7 March 2015 at 18:39, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
>> 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.
>
> Unfortunately, it seems I can't make this driver work with my BCM4708
> based router. This is a SoC with 6.01 controller.
>
> Sometimes I get this:
> [    0.478443] brcmstb_nand 18028000.nand: timeout waiting for command 255 (9)
> [    0.485385] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.588393] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.595326] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.698392] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.705322] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.711051] nand: second ID read did not match 20,20 against 20,00
> [    0.717197] nand: No NAND device found
>
> Other time:
> [    0.478441] brcmstb_nand 18028000.nand: timeout waiting for command 255 (9)
> [    0.485385] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.588396] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.595327] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.698393] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.705321] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.808394] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.815326] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.918392] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
> [    0.925322] brcmstb_nand 18028000.nand: intfc status f0000000
> [    0.931047] nand: No NAND device found
>
> (Btw. I guess you could use 0x%x instead of %d for printing commands).
>
> It seems that brcmnand_ctlrdy_irq never fires on my device. Just like
> controller was never generating any IRQ.

Oh, damn, I was doing sth stupid. I copied example from Documentation
which has "brcm,nandcs" with reg = <1>;. That was wrong of course.
after changing chip from 1 to 0 I got:

[    1.688413] brcmstb_nand 18028000.nand: timeout waiting for command 255 (9)
[    1.695355] brcmstb_nand 18028000.nand: intfc status f0000000
[    1.808384] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    1.815320] brcmstb_nand 18028000.nand: intfc status f0000000
[    1.928385] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    1.935312] brcmstb_nand 18028000.nand: intfc status f0000000
[    2.048460] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    2.055392] brcmstb_nand 18028000.nand: intfc status f0000000
[    2.168386] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    2.175315] brcmstb_nand 18028000.nand: intfc status f0000000
[    2.185640] nand: device found, Manufacturer ID: 0x92, Chip ID: 0xf1
[    2.191967] nand: Eon NAND 128MiB 3,3V 8-bit
[    2.196221] nand: 128 MiB, SLC, erase size: 128 KiB, page size:
2048, OOB size: 64
[    2.203784] brcmstb_nand 18028000.nand: detected 128MiB total,
128KiB blocks, 2KiB pages, 16B OOB, 8-bit, BCH-8

So there is still problem with lacking IRQ (and timeouts), but at
least communication with chip looks OK in general.

Of course the lost of log is also spammed in these "timeout waiting"
and driver is unusable.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
                     ` (2 preceding siblings ...)
  2015-03-07 17:39   ` Rafał Miłecki
@ 2015-03-07 22:15   ` Rafał Miłecki
  2015-03-16 18:55   ` Florian Fainelli
  2015-03-16 19:58   ` Florian Fainelli
  5 siblings, 0 replies; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-07 22:15 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee

On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> +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) {
> +               unsigned long cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START)
> +                                       >> brcmnand_cmd_shift(ctrl);
> +
> +               dev_err_ratelimited(ctrl->dev,
> +                       "timeout waiting for command %u (%ld)\n",
> +                       host->last_cmd, cmd);

I think that using host->last_cmd in the above message is really
misleading. Please consider following:

[    2.061139] [brcmnand_cmdfunc:1151] command:0x90 native_cmd:0x07
[    2.067123] brcmstb_nand 18028000.nand: send native cmd 7 addr_lo 0x40
[    2.168395] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    2.175321] brcmstb_nand 18028000.nand: intfc status f0000000
[    2.181051] nand: device found, Manufacturer ID: 0x92, Chip ID: 0xf1
[    2.187370] nand: Eon NAND 128MiB 3,3V 8-bit
[    2.191632] nand: 128 MiB, SLC, erase size: 128 KiB, page size:
2048, OOB size: 64
[    2.199192] brcmstb_nand 18028000.nand: detected 128MiB total,
128KiB blocks, 2KiB pages, 16B OOB, 8-bit, BCH-8
[    2.199192]
[    2.210724] Scanning device for bad blocks
[    2.214812] brcmstb_nand 18028000.nand: send native cmd 1 addr_lo 0x0
[    2.318394] brcmstb_nand 18028000.nand: timeout waiting for command 144 (1)

As you can see, first there was a brcmnand_cmdfunc call with 0x90
(NAND_CMD_READID). It was translated into internal command 0x07
(CMD_DEVICE_ID_READ).

Then there was a call to brcmnand_read_by_pio which resulted in
sending internal command 0x01 (CMD_PAGE_READ). This one overwrote
previous command. So the timeout wasn't about 144 (0x90) anymore.

I suggest replacing that with something like
"timeout waiting for internal command 0x%02x\n", cmd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-07  1:18 [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Brian Norris
                   ` (2 preceding siblings ...)
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
@ 2015-03-08  0:01 ` Rafał Miłecki
  2015-03-08  0:57   ` Brian Norris
       [not found] ` <CALj_zD5rW3Se27Rh0pL6QTMNGOrrmrvAVLvW3BCuF8RujYQE=g@mail.gmail.com>
  4 siblings, 1 reply; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-08  0:01 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee

Hi Brian,

Thanks for your work on this. It looks amazing, nice piece of code :)

On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> This adds (long in coming) support for the Broadcom BCM7xxx Set-Top Box NAND
> controller. This controller has been used in a variety of Broadcom SoCs.
>
> There are a few more features I'd like add in the near future, mostly to
> support more SoCs, but this is the base set, which should only need relatively
> minor additions to support chips like BCM63138, BCM3384, and Cygnus/iProc.
> Particularly, we may need to straighten out some endianness issues for the data
> path on iProc, and interrupt enabling/acking on iProc, BCM63xxx, BCM3xxx, and
> others.

After applying workaround for not working IRQ, it seems I have some
problem with endianess on my BCM4708 (SoC with 6.01 controller).

Let me start with dumps of "nvram" MTD partition.
1) Dumping with Netgear's original firmware:
# hexdump -C -n 16 mtdblock1.bin
00000000  46 4c 53 48 40 79 00 00  84 01 00 00 47 01 1c 08  |FLSH@y......G...|
2) Dumping with OpenWrt and its bcm_nand.c:
root@OpenWrt:/# hexdump -C -n 16 /dev/mtdblock1
00000000  46 4c 53 48 38 79 00 00  cb 01 00 00 47 01 1c 08  |FLSH8y......G...|

This makes me feel that bcm_nand.c driver is OK.
Unfortunately when using brcmstb_nand.c my bcm47xxpart partition
driver didn't detect any partition at all. This means I couldn't use
any /dev/mtdblockX, not even user space as it wasn't mounted.

So since I didn't have user space, I decided to add some debugging to
bcm47xxpart itself. There is what I got:
1) OpenWrt and its bcm_nand.c:
[bcm47xxpart_parse] 0x80000  46 4c 53 48  38 79 00 00  cb 01 00 00  47 01 1c 08
2) OpenWrt and brcmstb_nand.c:
[bcm47xxpart_parse] 0x80000  48 53 4c 46  00 00 79 38  00 00 01 cb  08 1c 01 47

So obviously data returned by brcmstb_nand.c seems to be all
endianess-flipped. Could you take a loo at this?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07 17:39   ` Rafał Miłecki
  2015-03-07 21:48     ` Rafał Miłecki
@ 2015-03-08  0:44     ` Rafał Miłecki
  2015-03-09 17:49       ` Brian Norris
  1 sibling, 1 reply; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-08  0:44 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens

On 7 March 2015 at 18:39, Rafał Miłecki <zajec5@gmail.com> wrote:
> It seems that brcmnand_ctlrdy_irq never fires on my device. Just like
> controller was never generating any IRQ.
>
>
> I started comparing your driver with OpenWrt's bcm_nand.c (which
> should be very similar to Broadcom's SDK NAND driver for ARM). Below
> are few things I've noticed.
>
> 1) In bcm_nand.c IRQ handler also doesn't seem to be fired (or very rarely).

Oh, wait, I was wrong there. So in bcm_nand.c IRQ handler fires very
often, just not during the early phase (RESET, READID), when we don't
use IRQs. During standard read/program/etc IRQ is commonly used.

So maybe all we're missing in case of brcmstb_nand.c is
enabling/acking/disabling IRQ?

It seem that my controller 6.01 has:

1) Following IRQs:
DIREC_READ_MISS
ERASE_COMPLETE
COPYBACK_COMPLETE
PROGRAM_COMPLETE
CONTROLLER_RDY
RDBSY_RDY
ECC_UNCORRECTABLE
ECC_CORRECTABLE

2) Registers for reading/acking above IRQs:
0xf00 DIREC_READ_MISS
0xf04 ERASE_COMPLETE
0xf08 COPYBACK_COMPLETE
0xf0c PROGRAM_COMPLETE
0xf10 CONTROLLER_RDY
0xf14 RDBSY_RDY
0xf18 ECC_UNCORRECTABLE
0xf1c ECC_CORRECTABLE
(if 0x1 is set, it means IRQ was raised, writing 0x1 ack-es it)

3) Register 0x408 for enabling/disabling IRQs:
0x00000004 DIREC_READ_MISS
0x00000008 ERASE_COMPLETE
0x00000010 COPYBACK_COMPLETE
0x00000020 PROGRAM_COMPLETE
0x00000040 CONTROLLER_RDY
0x00000080 RDBSY_RDY
0x00000100 ECC_UNCORRECTABLE
0x00000200 ECC_CORRECTABLE

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-08  0:01 ` [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Rafał Miłecki
@ 2015-03-08  0:57   ` Brian Norris
  2015-03-08 10:22     ` Rafał Miłecki
  2015-03-08 11:18     ` Rafał Miłecki
  0 siblings, 2 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-08  0:57 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee

On Sun, Mar 08, 2015 at 01:01:14AM +0100, Rafał Miłecki wrote:
> Thanks for your work on this. It looks amazing, nice piece of code :)

Thanks :) And thanks for testing this. I believe I suggested to you that
I'd be releasing this "soon" several months ago. Better late than never,
I guess?

> On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> > This adds (long in coming) support for the Broadcom BCM7xxx Set-Top Box NAND
> > controller. This controller has been used in a variety of Broadcom SoCs.
> >
> > There are a few more features I'd like add in the near future, mostly to
> > support more SoCs, but this is the base set, which should only need relatively
> > minor additions to support chips like BCM63138, BCM3384, and Cygnus/iProc.
> > Particularly, we may need to straighten out some endianness issues for the data
> > path on iProc, and interrupt enabling/acking on iProc, BCM63xxx, BCM3xxx, and
> > others.
> 
> After applying workaround for not working IRQ, it seems I have some
> problem with endianess on my BCM4708 (SoC with 6.01 controller).
> 
> Let me start with dumps of "nvram" MTD partition.
> 1) Dumping with Netgear's original firmware:
> # hexdump -C -n 16 mtdblock1.bin
> 00000000  46 4c 53 48 40 79 00 00  84 01 00 00 47 01 1c 08  |FLSH@y......G...|
> 2) Dumping with OpenWrt and its bcm_nand.c:
> root@OpenWrt:/# hexdump -C -n 16 /dev/mtdblock1
> 00000000  46 4c 53 48 38 79 00 00  cb 01 00 00 47 01 1c 08  |FLSH8y......G...|
> 
> This makes me feel that bcm_nand.c driver is OK.
> Unfortunately when using brcmstb_nand.c my bcm47xxpart partition
> driver didn't detect any partition at all. This means I couldn't use
> any /dev/mtdblockX, not even user space as it wasn't mounted.
> 
> So since I didn't have user space, I decided to add some debugging to
> bcm47xxpart itself. There is what I got:
> 1) OpenWrt and its bcm_nand.c:
> [bcm47xxpart_parse] 0x80000  46 4c 53 48  38 79 00 00  cb 01 00 00  47 01 1c 08
> 2) OpenWrt and brcmstb_nand.c:
> [bcm47xxpart_parse] 0x80000  48 53 4c 46  00 00 79 38  00 00 01 cb  08 1c 01 47
> 
> So obviously data returned by brcmstb_nand.c seems to be all
> endianess-flipped. Could you take a loo at this?

I'll take a look at all your comments/questions when I get back into the
office, but a few pointers:

1. IRQs are a touchy subject; for platforms I've supported, I've found
it best if the NAND interrupt bits are handled in an irqchip driver.
Particularly, that's one of the use cases for
drivers/irqchip/irq-brcmstb-l2.c. If you can arrange the NAND
enable/ack registers to act as a second-level interrupt controller, that
should hopefully solve your problems.

But if BCM4708's NAND interrupt registers can't be shaped into a sane
irqchip driver, then we can probably handle them in a per-SoC extension
of the driver. I have code already that supports another chip in this
way, so I can point you that way if the first suggestion doesn't work
out.

2. Endianness is a known issue with at least one other platform. On many
chips (spanning MIPS LE, MIPS BE, and ARM LE), NAND has been integrated
such that data can just be read/programmed in the native endianness
through the FLASH_CACHE registers (as this driver does), but there are a
few (on ARM, LE) that require a be32_to_cpu()/cpu_to_be32() swap. I'm
considering supporting DT properties like one of the following:

	brcm,nand-cache-be
	brcm,nand-cache-big-endian
	brcm,nand-cache-reverse-endian

You might also check (though I might actually be better equipped for
this) if there is a separate register that can tell the NAND data bus to
automatically endian-swap data into the native endianness. I know a lot
of the buses and peripherals in BCG, at least, are designed such that
either (1) they can work naturally in the CPU's native endianness or
else (2) they can be configured to swap endianness into either format.

But if such a register does not exist, then we'll definitely have to do
something like the DT property above.

Do the bad block markers look OK without extra endian swapping? I'm
wondering whether the swapping will have to occur on both the
FLASH_CACHE and SPARE_AREA registers.

3. I was told that there were only 2 or 3 chips that were released with
a v6.1 NAND controller, and BCM4708 wasn't one of them. Apparently I was
told wrong... I'll have to see if there are any other quirks we should
be accounting for.

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-08  0:57   ` Brian Norris
@ 2015-03-08 10:22     ` Rafał Miłecki
  2015-03-09 18:04       ` Brian Norris
  2015-03-08 11:18     ` Rafał Miłecki
  1 sibling, 1 reply; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-08 10:22 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee

On 8 March 2015 at 01:57, Brian Norris <computersforpeace@gmail.com> wrote:
> 2. Endianness is a known issue with at least one other platform. On many
> chips (spanning MIPS LE, MIPS BE, and ARM LE), NAND has been integrated
> such that data can just be read/programmed in the native endianness
> through the FLASH_CACHE registers (as this driver does), but there are a
> few (on ARM, LE) that require a be32_to_cpu()/cpu_to_be32() swap. I'm
> considering supporting DT properties like one of the following:
>
>         brcm,nand-cache-be
>         brcm,nand-cache-big-endian
>         brcm,nand-cache-reverse-endian
>
> You might also check (though I might actually be better equipped for
> this) if there is a separate register that can tell the NAND data bus to
> automatically endian-swap data into the native endianness. I know a lot
> of the buses and peripherals in BCG, at least, are designed such that
> either (1) they can work naturally in the CPU's native endianness or
> else (2) they can be configured to swap endianness into either format.
>
> But if such a register does not exist, then we'll definitely have to do
> something like the DT property above.

It seems there is such a magic register. Please take a look at bcm_nand.c:
https://dev.openwrt.org/browser/trunk/target/linux/bcm53xx/patches-3.18/420-mtd-bcm5301x_nand.patch

There are multiple places (data, OOB, reads, writes) with:

/* Set controller to Little Endian mode for copying */
bcmnand_reg_awrite(ctrl, NANDC_IDM_APB_LITTLE_ENDIAN, 1);

/* Return to Big Endian mode for commands etc */
bcmnand_reg_awrite(ctrl, NANDC_IDM_APB_LITTLE_ENDIAN, 0);

That register is 0x408, but it's in "agent" core (AKA wrapper), so
it's separated mapping. I'm not sure what address is it right now, as
we read them from the EROM.


> Do the bad block markers look OK without extra endian swapping? I'm
> wondering whether the swapping will have to occur on both the
> FLASH_CACHE and SPARE_AREA registers.

I don't know, I didn't try nand-on-flash-bbt.

-- 
Rafał

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-08  0:57   ` Brian Norris
  2015-03-08 10:22     ` Rafał Miłecki
@ 2015-03-08 11:18     ` Rafał Miłecki
  2015-03-09 17:59       ` Brian Norris
  1 sibling, 1 reply; 30+ messages in thread
From: Rafał Miłecki @ 2015-03-08 11:18 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens

On 8 March 2015 at 01:57, Brian Norris <computersforpeace@gmail.com> wrote:
> 3. I was told that there were only 2 or 3 chips that were released with
> a v6.1 NAND controller, and BCM4708 wasn't one of them. Apparently I was
> told wrong... I'll have to see if there are any other quirks we should
> be accounting for.

Please note I'm speaking about 6.01, not 6.10. Maybe it make a difference?

Buffalo WZR-600DHP2 (BCM47081)
[    0.325732] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
[    4.838105] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02

Buffalo WZR-1750DHP (BCM4708)
[    0.326823] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
[    6.674288] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02

Netgear R6250 V1 (BCM4708)
[    0.326248] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
[    8.721335] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02

Netgear R8000 (BCM4709)
[    0.241961] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x00
[    1.990330] bcm_nand bcma0:18: NAND Controller rev 6.01

-- 
Rafał

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07 12:39   ` Paul Bolle
@ 2015-03-09 17:30     ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-09 17:30 UTC (permalink / raw)
  To: Paul Bolle
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee, sbranden

On Sat, Mar 07, 2015 at 01:39:43PM +0100, Paul Bolle wrote:
> Only a license nit.
> 
> Brian Norris schreef op vr 06-03-2015 om 17:18 [-0800]:
> 
> > --- /dev/null
> > +++ b/drivers/mtd/nand/brcmstb_nand.c
> > @@ -0,0 +1,2182 @@
> > +/*
> > + * 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.
> > + */
> 
> This states the license is GPL v2.
> 
> > +MODULE_LICENSE("GPL");
> 
> So you probably want
>    MODULE_LICENSE("GPL v2");
> 
> here.

Thanks. Fixed.

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07 13:21   ` Rafał Miłecki
@ 2015-03-09 17:31     ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-09 17:31 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, sbranden

On Sat, Mar 07, 2015 at 02:21:26PM +0100, Rafał Miłecki wrote:
> On 7 March 2015 at 02:18, Brian Norris <computersforpeace@gmail.com> wrote:
> > +       /* NAND register range */
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       ctrl->nand_base = devm_ioremap_resource(dev, res);
> > +       if (!ctrl->nand_base)
> > +               return -ENODEV;
> 
> This is what I got during my first test:
> brcmstb_nand 11315e0.nand: can't request region for resource [mem
> 0x011315e0-0x011325df]
> Unable to handle kernel paging request at virtual address fffffff0
> (not sure why yet).
> 
> Hint: devm_ioremap_resource does never return NULL

Thanks, fixed in 3 places.

I expect you've figured out your problem here by now.

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-08  0:44     ` Rafał Miłecki
@ 2015-03-09 17:49       ` Brian Norris
  2015-03-09 17:57         ` Ray Jui
  0 siblings, 1 reply; 30+ messages in thread
From: Brian Norris @ 2015-03-09 17:49 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens, sbranden

On Sun, Mar 08, 2015 at 01:44:02AM +0100, Rafał Miłecki wrote:
> On 7 March 2015 at 18:39, Rafał Miłecki <zajec5@gmail.com> wrote:
> > It seems that brcmnand_ctlrdy_irq never fires on my device. Just like
> > controller was never generating any IRQ.
> >
> >
> > I started comparing your driver with OpenWrt's bcm_nand.c (which
> > should be very similar to Broadcom's SDK NAND driver for ARM). Below
> > are few things I've noticed.
> >
> > 1) In bcm_nand.c IRQ handler also doesn't seem to be fired (or very rarely).
> 
> Oh, wait, I was wrong there. So in bcm_nand.c IRQ handler fires very
> often, just not during the early phase (RESET, READID), when we don't
> use IRQs. During standard read/program/etc IRQ is commonly used.
> 
> So maybe all we're missing in case of brcmstb_nand.c is
> enabling/acking/disabling IRQ?

Yes, that's likely the main problem.

> It seem that my controller 6.01 has:
> 
> 1) Following IRQs:
> DIREC_READ_MISS
> ERASE_COMPLETE
> COPYBACK_COMPLETE
> PROGRAM_COMPLETE
> CONTROLLER_RDY
> RDBSY_RDY
> ECC_UNCORRECTABLE
> ECC_CORRECTABLE

Right, that's the standard set of NAND interrupts. This driver only uses
CONTROLLER_RDY, BTW.

> 2) Registers for reading/acking above IRQs:
> 0xf00 DIREC_READ_MISS
> 0xf04 ERASE_COMPLETE
> 0xf08 COPYBACK_COMPLETE
> 0xf0c PROGRAM_COMPLETE
> 0xf10 CONTROLLER_RDY
> 0xf14 RDBSY_RDY
> 0xf18 ECC_UNCORRECTABLE
> 0xf1c ECC_CORRECTABLE
> (if 0x1 is set, it means IRQ was raised, writing 0x1 ack-es it)
> 
> 3) Register 0x408 for enabling/disabling IRQs:
> 0x00000004 DIREC_READ_MISS
> 0x00000008 ERASE_COMPLETE
> 0x00000010 COPYBACK_COMPLETE
> 0x00000020 PROGRAM_COMPLETE
> 0x00000040 CONTROLLER_RDY
> 0x00000080 RDBSY_RDY
> 0x00000100 ECC_UNCORRECTABLE
> 0x00000200 ECC_CORRECTABLE

(2) and (3) look pretty similar to the Cygnus chips. We should probably
end up using the same solution for both. Several related to the Cygnus
project are on CC.

One problem for Cygnus is that some of the other bits in their register
0x408 deal with non-interrupt-related functionality (endianness and
clocks), so it might not make the cleanest design to handle this with an
irqchip driver.

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-09 17:49       ` Brian Norris
@ 2015-03-09 17:57         ` Ray Jui
  0 siblings, 0 replies; 30+ messages in thread
From: Ray Jui @ 2015-03-09 17:57 UTC (permalink / raw)
  To: Brian Norris, Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Corneliu Doban,
	Jonathan Richardson, Florian Fainelli, bcm-kernel-feedback-list,
	devicetree, Linux Kernel Mailing List, Kevin Cernekee,
	Hauke Mehrtens, sbranden



On 3/9/2015 10:49 AM, Brian Norris wrote:
> On Sun, Mar 08, 2015 at 01:44:02AM +0100, Rafał Miłecki wrote:
>> On 7 March 2015 at 18:39, Rafał Miłecki <zajec5@gmail.com> wrote:
>>> It seems that brcmnand_ctlrdy_irq never fires on my device. Just like
>>> controller was never generating any IRQ.
>>>
>>>
>>> I started comparing your driver with OpenWrt's bcm_nand.c (which
>>> should be very similar to Broadcom's SDK NAND driver for ARM). Below
>>> are few things I've noticed.
>>>
>>> 1) In bcm_nand.c IRQ handler also doesn't seem to be fired (or very rarely).
>>
>> Oh, wait, I was wrong there. So in bcm_nand.c IRQ handler fires very
>> often, just not during the early phase (RESET, READID), when we don't
>> use IRQs. During standard read/program/etc IRQ is commonly used.
>>
>> So maybe all we're missing in case of brcmstb_nand.c is
>> enabling/acking/disabling IRQ?
> 
> Yes, that's likely the main problem.
> 
>> It seem that my controller 6.01 has:
>>
>> 1) Following IRQs:
>> DIREC_READ_MISS
>> ERASE_COMPLETE
>> COPYBACK_COMPLETE
>> PROGRAM_COMPLETE
>> CONTROLLER_RDY
>> RDBSY_RDY
>> ECC_UNCORRECTABLE
>> ECC_CORRECTABLE
> 
> Right, that's the standard set of NAND interrupts. This driver only uses
> CONTROLLER_RDY, BTW.
> 
>> 2) Registers for reading/acking above IRQs:
>> 0xf00 DIREC_READ_MISS
>> 0xf04 ERASE_COMPLETE
>> 0xf08 COPYBACK_COMPLETE
>> 0xf0c PROGRAM_COMPLETE
>> 0xf10 CONTROLLER_RDY
>> 0xf14 RDBSY_RDY
>> 0xf18 ECC_UNCORRECTABLE
>> 0xf1c ECC_CORRECTABLE
>> (if 0x1 is set, it means IRQ was raised, writing 0x1 ack-es it)
>>
>> 3) Register 0x408 for enabling/disabling IRQs:
>> 0x00000004 DIREC_READ_MISS
>> 0x00000008 ERASE_COMPLETE
>> 0x00000010 COPYBACK_COMPLETE
>> 0x00000020 PROGRAM_COMPLETE
>> 0x00000040 CONTROLLER_RDY
>> 0x00000080 RDBSY_RDY
>> 0x00000100 ECC_UNCORRECTABLE
>> 0x00000200 ECC_CORRECTABLE
> 
> (2) and (3) look pretty similar to the Cygnus chips. We should probably
> end up using the same solution for both. Several related to the Cygnus
> project are on CC.
> 
> One problem for Cygnus is that some of the other bits in their register
> 0x408 deal with non-interrupt-related functionality (endianness and
> clocks), so it might not make the cleanest design to handle this with an
> irqchip driver.

It's not just similar, but it's identical to Cygnus.

The register offset to read/ack the IRQ are the same. and Cygnus also
uses a standard alone register to enable/disable IRQ (and their bit
field is the same). Based on the link from Rafal, this chip also have
the same issue with APB endianess control, clock control, interrupt
control all stuffed in the same register:

#define NANDC_IDM_AXI_BIG_ENDIAN        IDMREG_BIT_FIELD(0x408, 28, 1)
#define NANDC_IDM_APB_LITTLE_ENDIAN     IDMREG_BIT_FIELD(0x408, 24, 1)
#define NANDC_IDM_TM                    IDMREG_BIT_FIELD(0x408, 16, 5)
#define NANDC_IDM_IRQ_CORRECABLE_EN     IDMREG_BIT_FIELD(0x408, 9, 1)
#define NANDC_IDM_IRQ_UNCORRECABLE_EN   IDMREG_BIT_FIELD(0x408, 8, 1)
#define NANDC_IDM_IRQ_RDYBSY_RDY_EN     IDMREG_BIT_FIELD(0x408, 7, 1)
#define NANDC_IDM_IRQ_CONTROLLER_RDY_EN IDMREG_BIT_FIELD(0x408, 6, 1)
#define NANDC_IDM_IRQ_PRPOGRAM_COMP_EN  IDMREG_BIT_FIELD(0x408, 5, 1)
#define NANDC_IDM_IRQ_COPYBK_COMP_EN    IDMREG_BIT_FIELD(0x408, 4, 1)
#define NANDC_IDM_IRQ_ERASE_COMP_EN     IDMREG_BIT_FIELD(0x408, 3, 1)
#define NANDC_IDM_IRQ_READ_MISS_EN      IDMREG_BIT_FIELD(0x408, 2, 1)
#define NANDC_IDM_IRQ_N_EN(n)           IDMREG_BIT_FIELD(0x408, 2+(n), 1)
#define NANDC_IDM_CLOCK_EN              IDMREG_BIT_FIELD(0x408, 0, 1)


> 
> Brian
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-08 11:18     ` Rafał Miłecki
@ 2015-03-09 17:59       ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-09 17:59 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, Hauke Mehrtens

On Sun, Mar 08, 2015 at 12:18:39PM +0100, Rafał Miłecki wrote:
> On 8 March 2015 at 01:57, Brian Norris <computersforpeace@gmail.com> wrote:
> > 3. I was told that there were only 2 or 3 chips that were released with
> > a v6.1 NAND controller, and BCM4708 wasn't one of them. Apparently I was
> > told wrong... I'll have to see if there are any other quirks we should
> > be accounting for.
> 
> Please note I'm speaking about 6.01, not 6.10. Maybe it make a difference?
> 
> Buffalo WZR-600DHP2 (BCM47081)
> [    0.325732] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
> [    4.838105] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02
> 
> Buffalo WZR-1750DHP (BCM4708)
> [    0.326823] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
> [    6.674288] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02
> 
> Netgear R6250 V1 (BCM4708)
> [    0.326248] [brcmnand_revision_init:371] ctrl->nand_version:0x0601
> [    8.721335] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x02
> 
> Netgear R8000 (BCM4709)
> [    0.241961] bcma: bus0: Found chip with id 53010, rev 0x00 and package 0x00
> [    1.990330] bcm_nand bcma0:18: NAND Controller rev 6.01

We've never had a MINOR number larger than 4 or 5, I think, so I haven't
actually bothered with two digits past the decimal. It's 8 bits of MAJOR
and 8 bits of MINOR, but we just call it 6.1 in all our documentation.

0x0601 == v6.1
0x0701 == v7.1

I suppose if we ever have to care (e.g., we get past 7.9), we'd probably
call it 7.10 (0x070a). Kinda like kernel versioning (and not like Ubuntu
versioning).

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
  2015-03-08 10:22     ` Rafał Miłecki
@ 2015-03-09 18:04       ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-09 18:04 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	bcm-kernel-feedback-list, devicetree, Linux Kernel Mailing List,
	Kevin Cernekee, sbranden

On Sun, Mar 08, 2015 at 11:22:40AM +0100, Rafał Miłecki wrote:
> On 8 March 2015 at 01:57, Brian Norris <computersforpeace@gmail.com> wrote:
> > 2. Endianness is a known issue with at least one other platform. On many
> > chips (spanning MIPS LE, MIPS BE, and ARM LE), NAND has been integrated
> > such that data can just be read/programmed in the native endianness
> > through the FLASH_CACHE registers (as this driver does), but there are a
> > few (on ARM, LE) that require a be32_to_cpu()/cpu_to_be32() swap. I'm
> > considering supporting DT properties like one of the following:
> >
> >         brcm,nand-cache-be
> >         brcm,nand-cache-big-endian
> >         brcm,nand-cache-reverse-endian
> >
> > You might also check (though I might actually be better equipped for
> > this) if there is a separate register that can tell the NAND data bus to
> > automatically endian-swap data into the native endianness. I know a lot
> > of the buses and peripherals in BCG, at least, are designed such that
> > either (1) they can work naturally in the CPU's native endianness or
> > else (2) they can be configured to swap endianness into either format.
> >
> > But if such a register does not exist, then we'll definitely have to do
> > something like the DT property above.
> 
> It seems there is such a magic register. Please take a look at bcm_nand.c:
> https://dev.openwrt.org/browser/trunk/target/linux/bcm53xx/patches-3.18/420-mtd-bcm5301x_nand.patch
> 
> There are multiple places (data, OOB, reads, writes) with:

So you do data and OOB in little endian? At least that seems consistent.

> /* Set controller to Little Endian mode for copying */
> bcmnand_reg_awrite(ctrl, NANDC_IDM_APB_LITTLE_ENDIAN, 1);
> 
> /* Return to Big Endian mode for commands etc */
> bcmnand_reg_awrite(ctrl, NANDC_IDM_APB_LITTLE_ENDIAN, 0);
> 
> That register is 0x408, but it's in "agent" core (AKA wrapper), so
> it's separated mapping. I'm not sure what address is it right now, as
> we read them from the EROM.
> 
> 
> > Do the bad block markers look OK without extra endian swapping? I'm
> > wondering whether the swapping will have to occur on both the
> > FLASH_CACHE and SPARE_AREA registers.
> 
> I don't know, I didn't try nand-on-flash-bbt.

You don't have to use on-flash BBT to notice. Without a flash-based BBT,
you should just be scanning for bad block markers on *every* boot. Do
you read factory-marked bad block markers correctly? Or maybe you see no
factory bad blocks?

Note that this is sometimes hard to tell, if the factory just programmed
the entire page + OOB to 0x00; then you obviously don't have to worry
about endianness. But if they programmed a word of 0xffffff00, then you
definitely do need to care!

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-07  1:18 ` [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller Brian Norris
@ 2015-03-16 18:49   ` Florian Fainelli
  2015-03-16 23:07   ` Scott Branden
  1 sibling, 0 replies; 30+ messages in thread
From: Florian Fainelli @ 2015-03-16 18:49 UTC (permalink / raw)
  To: Brian Norris, linux-mtd
  Cc: Dmitry Torokhov, Anatol Pomazao, Ray Jui, Corneliu Doban,
	Jonathan Richardson, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

On 06/03/15 17:18, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

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

> ---
>  .../devicetree/bindings/mtd/brcmstb_nand.txt       | 109 +++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> new file mode 100644
> index 000000000000..933d44943cbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> @@ -0,0 +1,109 @@
> +* 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-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>;
> +		};
> +	};
> +};
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
                     ` (3 preceding siblings ...)
  2015-03-07 22:15   ` Rafał Miłecki
@ 2015-03-16 18:55   ` Florian Fainelli
  2015-03-19  1:49     ` Brian Norris
  2015-03-16 19:58   ` Florian Fainelli
  5 siblings, 1 reply; 30+ messages in thread
From: Florian Fainelli @ 2015-03-16 18:55 UTC (permalink / raw)
  To: Brian Norris, linux-mtd
  Cc: Dmitry Torokhov, Anatol Pomazao, Ray Jui, Corneliu Doban,
	Jonathan Richardson, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

On 06/03/15 17:18, Brian Norris wrote:
> 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>
> ---

Looks good to me, just one nitpick below:

[snip]

> +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)
> +		return -ENODEV;

It could be nice to have an informative error message here that this is
either:

- an unknown controller revision (> 7.1)
- an older controller revision
- a check against the compatible property, just in case?

[snip]

> +		ctrl->cs_offsets = brcmnand_cs_offsets_v71;
> +	} else {
> +		ctrl->cs_offsets = brcmnand_cs_offsets;
> +
> +		/* pre-v5.0 has a different CS0 offset layout */
> +		if (ctrl->nand_version <= 0x0500)
> +			ctrl->cs0_offsets = brcmnand_cs_offsets_cs0;

Based on this check, should the comment should be "pre-v5.0 and v5.0
have a different CS0 offset layout"?
-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
                     ` (4 preceding siblings ...)
  2015-03-16 18:55   ` Florian Fainelli
@ 2015-03-16 19:58   ` Florian Fainelli
  5 siblings, 0 replies; 30+ messages in thread
From: Florian Fainelli @ 2015-03-16 19:58 UTC (permalink / raw)
  To: Brian Norris, linux-mtd
  Cc: Dmitry Torokhov, Anatol Pomazao, Ray Jui, Corneliu Doban,
	Jonathan Richardson, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

[snip]

> +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);

We are missing a dma_mapping_error() check here.
-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-07  1:18 ` [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller Brian Norris
  2015-03-16 18:49   ` Florian Fainelli
@ 2015-03-16 23:07   ` Scott Branden
  2015-03-16 23:37     ` Brian Norris
  1 sibling, 1 reply; 30+ messages in thread
From: Scott Branden @ 2015-03-16 23:07 UTC (permalink / raw)
  To: Brian Norris, linux-mtd
  Cc: Dmitry Torokhov, Anatol Pomazao, Ray Jui, Corneliu Doban,
	Jonathan Richardson, Florian Fainelli, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

Hi Brian,


On 15-03-06 05:18 PM, Brian Norris wrote:
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>   .../devicetree/bindings/mtd/brcmstb_nand.txt       | 109 +++++++++++++++++++++
>   1 file changed, 109 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> new file mode 100644
> index 000000000000..933d44943cbb
> --- /dev/null

Could you change the binding document with the vendor prefix to match 
all the other drivers we have been submitting? ie. "brcm,brcmnand.txt"

Regards,
  Scott

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-16 23:07   ` Scott Branden
@ 2015-03-16 23:37     ` Brian Norris
  2015-03-16 23:40       ` Scott Branden
  0 siblings, 1 reply; 30+ messages in thread
From: Brian Norris @ 2015-03-16 23:37 UTC (permalink / raw)
  To: Scott Branden
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

On Mon, Mar 16, 2015 at 04:07:51PM -0700, Scott Branden wrote:
> On 15-03-06 05:18 PM, Brian Norris wrote:
> >Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> >---
> >  .../devicetree/bindings/mtd/brcmstb_nand.txt       | 109 +++++++++++++++++++++
> >  1 file changed, 109 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> >
> >diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> >new file mode 100644
> >index 000000000000..933d44943cbb
> >--- /dev/null
> 
> Could you change the binding document with the vendor prefix to
> match all the other drivers we have been submitting? ie.
> "brcm,brcmnand.txt"

$ find Documentation/devicetree/ -name 'broadcom*'
Documentation/devicetree/bindings/net/broadcom-systemport.txt
Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
Documentation/devicetree/bindings/net/broadcom-mdio-unimac.txt
Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
Documentation/devicetree/bindings/net/broadcom-sf2.txt

$ find Documentation/devicetree/ -name 'brcm[^,]*'
Documentation/devicetree/bindings/arm/brcm-brcmstb.txt

$ find Documentation/devicetree/ -name 'brcm,*'
Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
Documentation/devicetree/bindings/i2c/brcm,bcm2835-i2c.txt
Documentation/devicetree/bindings/spi/brcm,bcm2835-spi.txt
Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
Documentation/devicetree/bindings/pinctrl/brcm,bcm11351-pinctrl.txt
Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
Documentation/devicetree/bindings/arm/bcm/brcm,bcm11351-cpu-method
Documentation/devicetree/bindings/mmc/brcm,bcm2835-sdhci.txt
Documentation/devicetree/bindings/timer/brcm,bcm2835-system-timer.txt

$ find Documentation/devicetree/bindings -name '*.txt' | grep -v ','| wc -l
1227
$ find Documentation/devicetree/bindings -name '*.txt' | grep ','| wc -l
352

So, what's the standard? Company prefix? Long name? Commas? Hyphens?

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-16 23:37     ` Brian Norris
@ 2015-03-16 23:40       ` Scott Branden
  2015-03-16 23:46         ` Brian Norris
  0 siblings, 1 reply; 30+ messages in thread
From: Scott Branden @ 2015-03-16 23:40 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee


On 15-03-16 04:37 PM, Brian Norris wrote:
> On Mon, Mar 16, 2015 at 04:07:51PM -0700, Scott Branden wrote:
>> On 15-03-06 05:18 PM, Brian Norris wrote:
>>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>>> ---
>>>   .../devicetree/bindings/mtd/brcmstb_nand.txt       | 109 +++++++++++++++++++++
>>>   1 file changed, 109 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
>>> new file mode 100644
>>> index 000000000000..933d44943cbb
>>> --- /dev/null
>>
>> Could you change the binding document with the vendor prefix to
>> match all the other drivers we have been submitting? ie.
>> "brcm,brcmnand.txt"
>
> $ find Documentation/devicetree/ -name 'broadcom*'
> Documentation/devicetree/bindings/net/broadcom-systemport.txt
> Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
> Documentation/devicetree/bindings/net/broadcom-mdio-unimac.txt
> Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> Documentation/devicetree/bindings/net/broadcom-sf2.txt
>
> $ find Documentation/devicetree/ -name 'brcm[^,]*'
> Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
>
> $ find Documentation/devicetree/ -name 'brcm,*'
> Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
> Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
> Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
> Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
> Documentation/devicetree/bindings/i2c/brcm,bcm2835-i2c.txt
> Documentation/devicetree/bindings/spi/brcm,bcm2835-spi.txt
> Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
> Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> Documentation/devicetree/bindings/pinctrl/brcm,bcm11351-pinctrl.txt
> Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
> Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> Documentation/devicetree/bindings/arm/bcm/brcm,bcm11351-cpu-method
> Documentation/devicetree/bindings/mmc/brcm,bcm2835-sdhci.txt
> Documentation/devicetree/bindings/timer/brcm,bcm2835-system-timer.txt
>
> $ find Documentation/devicetree/bindings -name '*.txt' | grep -v ','| wc -l
> 1227
> $ find Documentation/devicetree/bindings -name '*.txt' | grep ','| wc -l
> 352
>
> So, what's the standard? Company prefix? Long name? Commas? Hyphens?

The problem with devicetree binding is there is no standard for much of 
anything.  Some others use the vendor prefix for all their binding 
documentation.  We can try to do the same?

>
> Brian
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] mtd: nand: add Broadcom NAND controller support
       [not found] ` <CALj_zD5rW3Se27Rh0pL6QTMNGOrrmrvAVLvW3BCuF8RujYQE=g@mail.gmail.com>
@ 2015-03-16 23:44   ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-16 23:44 UTC (permalink / raw)
  To: Anatol Pomazau
  Cc: linux-mtd, Dmitry Torokhov, Ray Jui, Corneliu Doban,
	Jonathan Richardson, Florian Fainelli, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

(Your HTML mail will likely get blocked by a lot of filters)

Hi Anatol,

On Mon, Mar 16, 2015 at 04:24:20PM -0700, Anatol Pomazau wrote:
> Hi Brian
> 
> Thanks for the patch series. I am going to test it on a Broadcom dev board.
> 
> Do you plan to add a patch for bcm-cygnus.dtsi similar to what is done
> here https://github.com/Broadcom/cygnus-linux/commit/
> 1de02697829c3a2ce48a1bc29a8535e4961999ca
> 
> ?

Not yet. As I note below, this does not work as-is on Cygnus. I wanted
to get the initial code out there, as this already supports some chips.
There as still some additions necessary to get iProc/Cygnus and BCM53xxx
chips working (WIP).

The code you link to is a hacked version from Ray. His changes have not
been integrated yet.

> On Fri, Mar 6, 2015 at 5:18 PM, Brian Norris <computersforpeace@gmail.com>
> wrote:
>      Hi,
> 
>      This adds (long in coming) support for the Broadcom BCM7xxx Set-Top
>      Box NAND
>      controller. This controller has been used in a variety of Broadcom
>      SoCs.
> 
>      There are a few more features I'd like add in the near future, mostly
>      to
>      support more SoCs, but this is the base set, which should only need
>      relatively
>      minor additions to support chips like BCM63138, BCM3384, and Cygnus/
>      iProc.
>      Particularly, we may need to straighten out some endianness issues
>      for the data
>      path on iProc, and interrupt enabling/acking on iProc, BCM63xxx,
>      BCM3xxx, and
>      others.

^^^ Note the previous 2 sentences.


>      TODO: add this to the DTS(I) files for BCM7445.
> 
>      Happy reviewing! (Speaking of which, I need to catch up on reviewing
>      everybody
>      else's MTD submissions...)
> 
>      Brian

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-16 23:40       ` Scott Branden
@ 2015-03-16 23:46         ` Brian Norris
  2015-03-16 23:52           ` Scott Branden
  0 siblings, 1 reply; 30+ messages in thread
From: Brian Norris @ 2015-03-16 23:46 UTC (permalink / raw)
  To: Scott Branden
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

On Mon, Mar 16, 2015 at 04:40:13PM -0700, Scott Branden wrote:
> 
> On 15-03-16 04:37 PM, Brian Norris wrote:
> >On Mon, Mar 16, 2015 at 04:07:51PM -0700, Scott Branden wrote:
> >>On 15-03-06 05:18 PM, Brian Norris wrote:
> >>>Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> >>>---
> >>>  .../devicetree/bindings/mtd/brcmstb_nand.txt       | 109 +++++++++++++++++++++
> >>>  1 file changed, 109 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> >>>
> >>>diff --git a/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt b/Documentation/devicetree/bindings/mtd/brcmstb_nand.txt
> >>>new file mode 100644
> >>>index 000000000000..933d44943cbb
> >>>--- /dev/null
> >>
> >>Could you change the binding document with the vendor prefix to
> >>match all the other drivers we have been submitting? ie.
> >>"brcm,brcmnand.txt"
> >
> >$ find Documentation/devicetree/ -name 'broadcom*'
> >Documentation/devicetree/bindings/net/broadcom-systemport.txt
> >Documentation/devicetree/bindings/net/broadcom-bcm87xx.txt
> >Documentation/devicetree/bindings/net/broadcom-mdio-unimac.txt
> >Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> >Documentation/devicetree/bindings/net/broadcom-sf2.txt
> >
> >$ find Documentation/devicetree/ -name 'brcm[^,]*'
> >Documentation/devicetree/bindings/arm/brcm-brcmstb.txt
> >
> >$ find Documentation/devicetree/ -name 'brcm,*'
> >Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
> >Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
> >Documentation/devicetree/bindings/interrupt-controller/brcm,bcm7120-l2-intc.txt
> >Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> >Documentation/devicetree/bindings/i2c/brcm,iproc-i2c.txt
> >Documentation/devicetree/bindings/i2c/brcm,bcm2835-i2c.txt
> >Documentation/devicetree/bindings/spi/brcm,bcm2835-spi.txt
> >Documentation/devicetree/bindings/rng/brcm,bcm2835.txt
> >Documentation/devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt
> >Documentation/devicetree/bindings/pinctrl/brcm,bcm11351-pinctrl.txt
> >Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
> >Documentation/devicetree/bindings/watchdog/brcm,bcm2835-pm-wdog.txt
> >Documentation/devicetree/bindings/arm/bcm/brcm,bcm11351-cpu-method
> >Documentation/devicetree/bindings/mmc/brcm,bcm2835-sdhci.txt
> >Documentation/devicetree/bindings/timer/brcm,bcm2835-system-timer.txt
> >
> >$ find Documentation/devicetree/bindings -name '*.txt' | grep -v ','| wc -l
> >1227
> >$ find Documentation/devicetree/bindings -name '*.txt' | grep ','| wc -l
> >352
> >
> >So, what's the standard? Company prefix? Long name? Commas? Hyphens?
> 
> The problem with devicetree binding is there is no standard for much
> of anything.  Some others use the vendor prefix for all their
> binding documentation.  We can try to do the same?

Ok, but we already have 3 versions for Broadcom. I can use 'brcm,' if
that helps...

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller
  2015-03-16 23:46         ` Brian Norris
@ 2015-03-16 23:52           ` Scott Branden
  0 siblings, 0 replies; 30+ messages in thread
From: Scott Branden @ 2015-03-16 23:52 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Florian Fainelli,
	Rafał Miłecki, bcm-kernel-feedback-list, devicetree,
	linux-kernel, Kevin Cernekee

Hi Brian,


On 15-03-16 04:46 PM, Brian Norris wrote:
> On Mon, Mar 16, 2015 at 04:40:13PM -0700, Scott Branden wrote:
>>
>> On 15-03-16 04:37 PM, Brian Norris wrote:
>>> On Mon, Mar 16, 2015 at 04:07:51PM -0700, Scott Branden wrote:
>>> So, what's the standard? Company prefix? Long name? Commas? Hyphens?
>>
>> The problem with devicetree binding is there is no standard for much
>> of anything.  Some others use the vendor prefix for all their
>> binding documentation.  We can try to do the same?
>
> Ok, but we already have 3 versions for Broadcom. I can use 'brcm,' if
> that helps...

Yes, it helps if we try standardizing on the brcm vendor prefix in
Documentation/devicetree/bindings/vendor-prefixes.txt.  If you sift
through the bindings directory you will find some of the other vendors
have done the same.

Also, of note.
In Documentation/devicetree/bindings/submitting-patches.txt
it states the Documentation should be the first patch in the series.
Perhaps this helps the devicetree maintainer in reviewing bindings?

Scott

>
> Brian
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller
  2015-03-16 18:55   ` Florian Fainelli
@ 2015-03-19  1:49     ` Brian Norris
  0 siblings, 0 replies; 30+ messages in thread
From: Brian Norris @ 2015-03-19  1:49 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-mtd, Dmitry Torokhov, Anatol Pomazao, Ray Jui,
	Corneliu Doban, Jonathan Richardson, Rafał Miłecki,
	bcm-kernel-feedback-list, devicetree, linux-kernel,
	Kevin Cernekee

On Mon, Mar 16, 2015 at 11:55:16AM -0700, Florian Fainelli wrote:
> On 06/03/15 17:18, Brian Norris wrote:
> > +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)
> > +		return -ENODEV;
> 
> It could be nice to have an informative error message here that this is
> either:
> 
> - an unknown controller revision (> 7.1)
> - an older controller revision
> - a check against the compatible property, just in case?

I'll add a message that the revision is not supported. The DT binding
should catch this too, so this check is just an extra safeguard.

> [snip]
> 
> > +		ctrl->cs_offsets = brcmnand_cs_offsets_v71;
> > +	} else {
> > +		ctrl->cs_offsets = brcmnand_cs_offsets;
> > +
> > +		/* pre-v5.0 has a different CS0 offset layout */
> > +		if (ctrl->nand_version <= 0x0500)
> > +			ctrl->cs0_offsets = brcmnand_cs_offsets_cs0;
> 
> Based on this check, should the comment should be "pre-v5.0 and v5.0
> have a different CS0 offset layout"?

Yes. Fixed.

Brian

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-03-19  1:49 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-07  1:18 [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Brian Norris
2015-03-07  1:18 ` [PATCH 1/3] mtd: nand: add common DT init code Brian Norris
2015-03-07  1:18 ` [PATCH 2/3] Documentation: devicetree: add binding doc for Broadcom NAND controller Brian Norris
2015-03-16 18:49   ` Florian Fainelli
2015-03-16 23:07   ` Scott Branden
2015-03-16 23:37     ` Brian Norris
2015-03-16 23:40       ` Scott Branden
2015-03-16 23:46         ` Brian Norris
2015-03-16 23:52           ` Scott Branden
2015-03-07  1:18 ` [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB " Brian Norris
2015-03-07 12:39   ` Paul Bolle
2015-03-09 17:30     ` Brian Norris
2015-03-07 13:21   ` Rafał Miłecki
2015-03-09 17:31     ` Brian Norris
2015-03-07 17:39   ` Rafał Miłecki
2015-03-07 21:48     ` Rafał Miłecki
2015-03-08  0:44     ` Rafał Miłecki
2015-03-09 17:49       ` Brian Norris
2015-03-09 17:57         ` Ray Jui
2015-03-07 22:15   ` Rafał Miłecki
2015-03-16 18:55   ` Florian Fainelli
2015-03-19  1:49     ` Brian Norris
2015-03-16 19:58   ` Florian Fainelli
2015-03-08  0:01 ` [PATCH 0/3] mtd: nand: add Broadcom NAND controller support Rafał Miłecki
2015-03-08  0:57   ` Brian Norris
2015-03-08 10:22     ` Rafał Miłecki
2015-03-09 18:04       ` Brian Norris
2015-03-08 11:18     ` Rafał Miłecki
2015-03-09 17:59       ` Brian Norris
     [not found] ` <CALj_zD5rW3Se27Rh0pL6QTMNGOrrmrvAVLvW3BCuF8RujYQE=g@mail.gmail.com>
2015-03-16 23:44   ` Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).