mtd: Qualcomm NAND controller driver

Hi Archit,

Again, sorry for the late review. It's probably not exhaustive but points a few things that should be fixed.

Quoted 355 lines · On Wed, 19 Aug 2015 10:19:03 +0530 Archit Taneja <[email protected]> wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx, MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2) and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a broader interface for external slow peripheral devices such as LCD and NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs of our interest, we only use the NAND controller within EBI2. Therefore, it's safe for us to assume that the NAND controller is a standalone block within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main and spare data. The controller contains an internal 512 byte page buffer to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA for register read/write and data transfers. The controller performs page reads and writes at a codeword/step level of 512 bytes. It can support up to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for each codeword, followed by data descriptors to read or write data from the controller's internal buffer. It uses a single ADM DMA channel that we get via dmaengine API. The controller requires 2 ADM CRCIs for command and data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use the standard syndrome ecc ops because of several reasons. First, the amount of data bytes covered by ecc isn't same in each step. Second, writing to free oob space requires us writing to the entire step in which the oob lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker. The controller ignores reading the marker when ECC is enabled. ECC needs to be explicity disabled to read or write to the bad block marker. For this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by Stephen Boyd <[email protected]>

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by Andy Gross <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
  drivers/mtd/nand/Kconfig      |    7 +
  drivers/mtd/nand/Makefile     |    1 +
  drivers/mtd/nand/qcom_nandc.c | 1910 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 1918 insertions(+)

create mode 100644 drivers/mtd/nand/qcom_nandc.c

drivers/mtd/nand/Kconfig +7 -0

index 5b2806a..6085b8a 100644
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
  	help
  	  Enables support for NAND controller on Hisilicon SoC Hip04.
 
+config MTD_NAND_QCOM
+	tristate "Support for NAND on QCOM SoCs"
+	depends on ARCH_QCOM
+	help
+	  Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+	  controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND

drivers/mtd/nand/Makefile +1 -0

index 1f897ec..87b6a1d 100644
@@ -53,5 +53,6 @@ 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_BRCMNAND)		+= brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
  nand-objs := nand_base.o nand_bbt.o nand_timings.o

drivers/mtd/nand/qcom_nandc.c new file +250 -0

new file mode 100644
index 0000000..2337731
@@ -0,0 +1,1910 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/clk.h>
+#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_mtd.h>
+#include <linux/delay.h>
+
+/* NANDc reg offsets */
+#define NAND_FLASH_CMD			0x00
+#define NAND_ADDR0			0x04
+#define NAND_ADDR1			0x08
+#define NAND_FLASH_CHIP_SELECT		0x0c
+#define NAND_EXEC_CMD			0x10
+#define NAND_FLASH_STATUS		0x14
+#define NAND_BUFFER_STATUS		0x18
+#define NAND_DEV0_CFG0			0x20
+#define NAND_DEV0_CFG1			0x24
+#define NAND_DEV0_ECC_CFG		0x28
+#define NAND_DEV1_ECC_CFG		0x2c
+#define NAND_DEV1_CFG0			0x30
+#define NAND_DEV1_CFG1			0x34
+#define NAND_READ_ID			0x40
+#define NAND_READ_STATUS		0x44
+#define NAND_DEV_CMD0			0xa0
+#define NAND_DEV_CMD1			0xa4
+#define NAND_DEV_CMD2			0xa8
+#define NAND_DEV_CMD_VLD		0xac
+#define SFLASHC_BURST_CFG		0xe0
+#define NAND_ERASED_CW_DETECT_CFG	0xe8
+#define NAND_ERASED_CW_DETECT_STATUS	0xec
+#define NAND_EBI2_ECC_BUF_CFG		0xf0
+#define FLASH_BUF_ACC			0x100
+
+#define NAND_CTRL			0xf00
+#define NAND_VERSION			0xf08
+#define NAND_READ_LOCATION_0		0xf20
+#define NAND_READ_LOCATION_1		0xf24
+
+/* dummy register offsets, used by write_reg_dma */
+#define NAND_DEV_CMD1_RESTORE		0xdead
+#define NAND_DEV_CMD_VLD_RESTORE	0xbeef
+
+/* NAND_FLASH_CMD bits */
+#define PAGE_ACC			BIT(4)
+#define LAST_PAGE			BIT(5)
+
+/* NAND_FLASH_CHIP_SELECT bits */
+#define NAND_DEV_SEL			0
+#define DM_EN				BIT(2)
+
+/* NAND_FLASH_STATUS bits */
+#define FS_OP_ERR			BIT(4)
+#define FS_READY_BSY_N			BIT(5)
+#define FS_MPU_ERR			BIT(8)
+#define FS_DEVICE_STS_ERR		BIT(16)
+#define FS_DEVICE_WP			BIT(23)
+
+/* NAND_BUFFER_STATUS bits */
+#define BS_UNCORRECTABLE_BIT		BIT(8)
+#define BS_CORRECTABLE_ERR_MSK		0x1f
+
+/* NAND_DEVn_CFG0 bits */
+#define DISABLE_STATUS_AFTER_WRITE	4
+#define CW_PER_PAGE			6
+#define UD_SIZE_BYTES			9
+#define ECC_PARITY_SIZE_BYTES_RS	19
+#define SPARE_SIZE_BYTES		23
+#define NUM_ADDR_CYCLES			27
+#define STATUS_BFR_READ			30
+#define SET_RD_MODE_AFTER_STATUS	31
+
+/* NAND_DEVn_CFG0 bits */
+#define DEV0_CFG1_ECC_DISABLE		0
+#define WIDE_FLASH			1
+#define NAND_RECOVERY_CYCLES		2
+#define CS_ACTIVE_BSY			5
+#define BAD_BLOCK_BYTE_NUM		6
+#define BAD_BLOCK_IN_SPARE_AREA		16
+#define WR_RD_BSY_GAP			17
+#define ENABLE_BCH_ECC			27
+
+/* NAND_DEV0_ECC_CFG bits */
+#define ECC_CFG_ECC_DISABLE		0
+#define ECC_SW_RESET			1
+#define ECC_MODE			4
+#define ECC_PARITY_SIZE_BYTES_BCH	8
+#define ECC_NUM_DATA_BYTES		16
+#define ECC_FORCE_CLK_OPEN		30
+
+/* NAND_DEV_CMD1 bits */
+#define READ_ADDR			0
+
+/* NAND_DEV_CMD_VLD bits */
+#define READ_START_VLD			0
+
+/* NAND_EBI2_ECC_BUF_CFG bits */
+#define NUM_STEPS			0
+
+/* NAND_ERASED_CW_DETECT_CFG bits */
+#define ERASED_CW_ECC_MASK		1
+#define AUTO_DETECT_RES			0
+#define MASK_ECC			(1 << ERASED_CW_ECC_MASK)
+#define RESET_ERASED_DET		(1 << AUTO_DETECT_RES)
+#define ACTIVE_ERASED_DET		(0 << AUTO_DETECT_RES)
+#define CLR_ERASED_PAGE_DET		(RESET_ERASED_DET | MASK_ECC)
+#define SET_ERASED_PAGE_DET		(ACTIVE_ERASED_DET | MASK_ECC)
+
+/* NAND_ERASED_CW_DETECT_STATUS bits */
+#define PAGE_ALL_ERASED			BIT(7)
+#define CODEWORD_ALL_ERASED		BIT(6)
+#define PAGE_ERASED			BIT(5)
+#define CODEWORD_ERASED			BIT(4)
+#define ERASED_PAGE			(PAGE_ALL_ERASED | PAGE_ERASED)
+#define ERASED_CW			(CODEWORD_ALL_ERASED | CODEWORD_ERASED)
+
+/* Version Mask */
+#define NAND_VERSION_MAJOR_MASK		0xf0000000
+#define NAND_VERSION_MAJOR_SHIFT	28
+#define NAND_VERSION_MINOR_MASK		0x0fff0000
+#define NAND_VERSION_MINOR_SHIFT	16
+
+/* NAND OP_CMDs */
+#define PAGE_READ			0x2
+#define PAGE_READ_WITH_ECC		0x3
+#define PAGE_READ_WITH_ECC_SPARE	0x4
+#define PROGRAM_PAGE			0x6
+#define PAGE_PROGRAM_WITH_ECC		0x7
+#define PROGRAM_PAGE_SPARE		0x9
+#define BLOCK_ERASE			0xa
+#define FETCH_ID			0xb
+#define RESET_DEVICE			0xd
+
+/*
+ * the NAND controller performs reads/writes with ECC in 516 byte chunks.
+ * the driver calls the chunks 'step' or 'codeword' interchangeably
+ */
+#define NANDC_STEP_SIZE			512
+
+/*
+ * the largest page size we support is 8K, this will have 16 steps/codewords
+ * of 512 bytes each
+ */
+#define	MAX_NUM_STEPS			(SZ_8K / NANDC_STEP_SIZE)
+
+/* we read at most 3 registers per codeword scan */
+#define MAX_REG_RD			(3 * MAX_NUM_STEPS)
+
+/* ECC modes */
+#define ECC_NONE	BIT(0)
+#define ECC_RS_4BIT	BIT(1)
+#define	ECC_BCH_4BIT	BIT(2)
+#define	ECC_BCH_8BIT	BIT(3)
+
+struct desc_info {
+	struct list_head list;
+
+	enum dma_data_direction dir;
+	struct scatterlist sgl;
+	struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a contiguous
+ * chunk of memory which we use to write the controller registers through DMA.
+ */
+struct nandc_regs {
+	__le32 cmd;
+	__le32 addr0;
+	__le32 addr1;
+	__le32 chip_sel;
+	__le32 exec;
+
+	__le32 cfg0;
+	__le32 cfg1;
+	__le32 ecc_bch_cfg;
+
+	__le32 clrflashstatus;
+	__le32 clrreadstatus;
+
+	__le32 cmd1;
+	__le32 vld;
+
+	__le32 orig_cmd1;
+	__le32 orig_vld;
+
+	__le32 ecc_buf_cfg;
+};
+
+/*
+ * @cmd_crci:			ADM DMA CRCI for command flow control
+ * @data_crci:			ADM DMA CRCI for data flow control
+ * @list:			DMA descriptor list (list of desc_infos)
+ * @data_buffer:		our local DMA buffer for page read/writes,
+ *				used when we can't use the buffer provided
+ *				by upper layers directly
+ * @buf_size/count/start:	markers for chip->read_buf/write_buf functions
+ * @reg_read_buf:		buffer for reading register data via DMA
+ * @reg_read_pos:		marker for data read in reg_read_buf
+ * @cfg0, cfg1, cfg0_raw..:	NANDc register configurations needed for
+ *				ecc/non-ecc mode for the current nand flash
+ *				device
+ * @regs:			a contiguous chunk of memory for DMA register
+ *				writes
+ * @ecc_strength:		4 bit or 8 bit ecc, received via DT
+ * @bus_width:			8 bit or 16 bit NAND bus width, received via DT
+ * @ecc_modes:			supported ECC modes by the current controller,
+ *				initialized via DT match data
+ * @cw_size:			the number of bytes in a single step/codeword
+ *				of a page, consisting of all data, ecc, spare
+ *				and reserved bytes
+ * @cw_data:			the number of bytes within a codeword protected
+ *				by ECC
+ * @bch_enabled:		flag to tell whether BCH or RS ECC mode is used
+ * @status:			value to be returned if NAND_CMD_STATUS command
+ *				is executed
+ */
+struct qcom_nandc_data {
+	struct platform_device *pdev;
+	struct device *dev;
+
+	void __iomem *base;
+	struct resource *res;
+
+	struct clk *core_clk;
+	struct clk *aon_clk;
+
+	/* DMA stuff */
+	struct dma_chan *chan;
+	struct dma_slave_config	slave_conf;
+	unsigned int cmd_crci;
+	unsigned int data_crci;

drivers/mtd/nand/qcom_nandc.c line 255

Quoted 5 lines

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+	struct list_head list;
+
+	/* MTD stuff */
+	struct nand_chip chip;
+	struct mtd_info mtd;

You can drop this field, since nand_chip now embeds its own mtd_info instance (accessible through the nand_to_mtd() helper).

Quoted 23 lines

drivers/mtd/nand/qcom_nandc.c new file +22 -0

+
+	/* local data buffer and markers */
+	u8		*data_buffer;
+	int		buf_size;
+	int		buf_count;
+	int		buf_start;
+
+	/* local buffer to read back registers */
+	__le32 *reg_read_buf;
+	int reg_read_pos;
+
+	/* required configs */
+	u32 cfg0, cfg1;
+	u32 cfg0_raw, cfg1_raw;
+	u32 ecc_buf_cfg;
+	u32 ecc_bch_cfg;
+	u32 clrflashstatus;
+	u32 clrreadstatus;
+	u32 sflashc_burst_cfg;
+	u32 cmd1, vld;
+
+	/* register state */

drivers/mtd/nand/qcom_nandc.c line 282

Quoted 5 lines

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+	struct nandc_regs *regs;
+
+	/* things we get from DT */
+	int ecc_strength;
+	int bus_width;

Do you really need these fields? You can directly change the ->chip.ecc.strength and ->chip.options field instead.

drivers/mtd/nand/qcom_nandc.c line 293

Quoted 12 lines

drivers/mtd/nand/qcom_nandc.c new file +11 -0

+
+	u32 ecc_modes;
+
+	/* misc params */
+	int cw_size;
+	int cw_data;
+	bool use_ecc;
+	bool bch_enabled;
+	u8 status;
+	int last_command;
+};

You're mixing controller and chip stuff in this structure. As said in my answer to your DT bindings proposal, I think it would be clearer to separate the controller and chip specific fields.

Note that a NAND chip can be attached to a NAND controller through its ->controller field.

So here is how it would look like:

struct qcom_nandc_data {
	struct nand_hw_control controller;

	struct list_head chips;
	/* controller specific fields */
};

struct qcom_nand_data {
	struct list_head node;
	struct nand_chip chip;

	/* chip specific fields */
}

[...]

drivers/mtd/nand/qcom_nandc.c line 810

Quoted 11 lines

drivers/mtd/nand/qcom_nandc.c new file +11 -0

+
+/*
+ * Implements chip->cmdfunc. It's  only used for a limited set of commands.
+ * The rest of the commands wouldn't be called by upper layers. For example,
+ * NAND_CMD_READOOB would never be called because we have our own versions
+ * of read_oob ops for nand_ecc_ctrl.
+ */
+static void qcom_nandc_command(struct mtd_info *mtd, unsigned int command,
+			 int column, int page_addr)
+{
+	struct nand_chip *chip = mtd->priv;

	struct nand_chip *chip = mtd_to_nand(mtd);

Quoted 70 lines

drivers/mtd/nand/qcom_nandc.c new file +70 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct qcom_nandc_data *this = chip->priv;
+	bool wait = false;
+	int r = 0;
+
+	pre_command(this, command);
+
+	switch (command) {
+	case NAND_CMD_RESET:
+		r = reset(this);
+		wait = true;
+		break;
+
+	case NAND_CMD_READID:
+		this->buf_count = 4;
+		r = read_id(this, column);
+		wait = true;
+		break;
+
+	case NAND_CMD_PARAM:
+		r = nandc_param(this);
+		wait = true;
+		break;
+
+	case NAND_CMD_ERASE1:
+		r = erase_block(this, page_addr);
+		wait = true;
+		break;
+
+	case NAND_CMD_READ0:
+		/* we read the entire page for now */
+		WARN_ON(column != 0);
+
+		this->use_ecc = true;
+		set_address(this, 0, page_addr);
+		update_rw_regs(this, ecc->steps, true);
+		break;
+
+	case NAND_CMD_SEQIN:
+		WARN_ON(column != 0);
+		set_address(this, 0, page_addr);
+		break;
+
+	case NAND_CMD_PAGEPROG:
+	case NAND_CMD_STATUS:
+	case NAND_CMD_NONE:
+	default:
+		break;
+	}
+
+	if (r) {
+		dev_err(this->dev, "failure executing command %d\n",
+			command);
+		free_descs(this);
+		return;
+	}
+
+	if (wait) {
+		r = submit_descs(this);
+		if (r)
+			dev_err(this->dev,
+				"failure submitting descs for command %d\n",
+				command);
+	}
+
+	free_descs(this);
+
+	post_command(this, command);
+}
+

drivers/mtd/nand/qcom_nandc.c line 890

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+/*
+ * when using RS ECC, the NAND controller flags an error when reading an
+ * erased page. however, there are special characters at certain offsets when
+ * we read the erased page. we check here if the page is really empty. if so,
+ * we replace the magic characters with 0xffs
+ */
+static bool empty_page_fixup(struct qcom_nandc_data *this, u8 *data_buf)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 63 lines

drivers/mtd/nand/qcom_nandc.c new file +63 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage = ecc->steps;
+	u8 orig1[MAX_NUM_STEPS], orig2[MAX_NUM_STEPS];
+	int i, j;
+
+	/* if BCH is enabled, HW will take care of detecting erased pages */
+	if (this->bch_enabled || !this->use_ecc)
+		return false;
+
+	for (i = 0; i < cwperpage; i++) {
+		u8 *empty1, *empty2;
+		u32 flash_status = le32_to_cpu(this->reg_read_buf[3 * i]);
+
+		/*
+		 * an erased page flags an error in NAND_FLASH_STATUS, check if
+		 * the page is erased by looking for 0x54s at offsets 3 and 175
+		 * from the beginning of each codeword
+		 */
+		if (!(flash_status & FS_OP_ERR))
+			break;
+
+		empty1 = &data_buf[3 + i * this->cw_data];
+		empty2 = &data_buf[175 + i * this->cw_data];
+
+		/*
+		 * if the error wasn't because of an erased page, bail out and
+		 * and let someone else do the error checking
+		 */
+		if ((*empty1 == 0x54 && *empty2 == 0xff) ||
+				(*empty1 == 0xff && *empty2 == 0x54)) {
+			orig1[i] = *empty1;
+			orig2[i] = *empty2;
+
+			*empty1 = 0xff;
+			*empty2 = 0xff;
+		} else {
+			break;
+		}
+	}
+
+	if (i < cwperpage || memchr_inv(data_buf, 0xff, mtd->writesize))
+		goto not_empty;
+
+	/*
+	 * tell the caller that the page was empty and is fixed up, so that
+	 * parse_read_errors() doesn't think it's an error
+	 */
+	return true;
+
+not_empty:
+	/* restore original values if not empty*/
+	for (j = 0; j < i; j++) {
+		data_buf[3 + j * this->cw_data] = orig1[j];
+		data_buf[175 + j * this->cw_data] = orig2[j];
+	}
+
+	return false;
+}
+
+struct read_stats {
+	__le32 flash;
+	__le32 buffer;
+	__le32 erased_cw;

drivers/mtd/nand/qcom_nandc.c line 963

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+};
+
+/*
+ * reads back status registers set by the controller to notify page read
+ * errors. this is equivalent to what 'ecc->correct()' would do.
+ */
+static int parse_read_errors(struct qcom_nandc_data *this, bool erased_page)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 40 lines

drivers/mtd/nand/qcom_nandc.c new file +40 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage = ecc->steps;
+	unsigned int max_bitflips = 0;
+	int i;
+	struct read_stats *buf;
+
+	buf = (struct read_stats *)this->reg_read_buf;
+	for (i = 0; i < cwperpage; i++, buf++) {
+		unsigned int stat;
+		u32 flash, buffer, erased_cw;
+
+		flash = le32_to_cpu(buf->flash);
+		buffer = le32_to_cpu(buf->buffer);
+		erased_cw = le32_to_cpu(buf->erased_cw);
+
+		if (flash & (FS_OP_ERR | FS_MPU_ERR)) {
+
+			/* ignore erased codeword errors */
+			if (this->bch_enabled) {
+				if ((erased_cw & ERASED_CW) == ERASED_CW)
+					continue;
+			} else if (erased_page) {
+				continue;
+			}
+
+			if (buffer & BS_UNCORRECTABLE_BIT) {
+				mtd->ecc_stats.failed++;
+				continue;
+			}
+		}
+
+		stat = buffer & BS_CORRECTABLE_ERR_MSK;
+		mtd->ecc_stats.corrected += stat;
+
+		max_bitflips = max(max_bitflips, stat);
+	}
+
+	return max_bitflips;
+}
+

[...]

drivers/mtd/nand/qcom_nandc.c line 1350

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+
+/*
+ * the three functions below implement chip->read_byte(), chip->read_buf()
+ * and chip->write_buf() respectively. these aren't used for
+ * reading/writing page data, they are used for smaller data like reading
+ * id, status etc
+ */
+static uint8_t qcom_nandc_read_byte(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;

	struct nand_chip *chip = mtd_to_nand(mtd);

Quoted 11 lines

drivers/mtd/nand/qcom_nandc.c new file +11 -0

+	struct qcom_nandc_data *this = chip->priv;
+	uint8_t *buf = this->data_buffer;
+	uint8_t ret = 0x0;
+
+	if (this->last_command == NAND_CMD_STATUS) {
+		ret = this->status;
+
+		this->status = NAND_STATUS_READY | NAND_STATUS_WP;
+
+		return ret;
+	}

drivers/mtd/nand/qcom_nandc.c line 1371

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+
+	if (this->buf_start < this->buf_count)
+		ret = buf[this->buf_start++];
+
+	return ret;
+}
+
+static void qcom_nandc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;

	struct nand_chip *chip = mtd_to_nand(mtd);

drivers/mtd/nand/qcom_nandc.c line 1382

Quoted 11 lines

drivers/mtd/nand/qcom_nandc.c new file +11 -0

+	struct qcom_nandc_data *this = chip->priv;
+	int real_len = min_t(size_t, len, this->buf_count - this->buf_start);
+
+	memcpy(buf, this->data_buffer + this->buf_start, real_len);
+	this->buf_start += real_len;
+}
+
+static void qcom_nandc_write_buf(struct mtd_info *mtd, const uint8_t *buf,
+		int len)
+{
+	struct nand_chip *chip = mtd->priv;

	struct nand_chip *chip = mtd_to_nand(mtd);

drivers/mtd/nand/qcom_nandc.c line 1394

Quoted 12 lines

drivers/mtd/nand/qcom_nandc.c new file +12 -0

+	struct qcom_nandc_data *this = chip->priv;
+	int real_len = min_t(size_t, len, this->buf_count - this->buf_start);
+
+	memcpy(this->data_buffer + this->buf_start, buf, real_len);
+
+	this->buf_start += real_len;
+}
+
+/* we support only one external chip for now */
+static void qcom_nandc_select_chip(struct mtd_info *mtd, int chipnr)
+{
+	struct nand_chip *chip = mtd->priv;

	struct nand_chip *chip = mtd_to_nand(mtd);

Quoted 18 lines

drivers/mtd/nand/qcom_nandc.c new file +18 -0

+	struct qcom_nandc_data *this = chip->priv;
+
+	if (chipnr <= 0)
+		return;
+
+	dev_warn(this->dev, "invalid chip select\n");
+}
+
+/*
+ * NAND controller page layout info
+ *
+ * |-----------------------|	  |---------------------------------|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |	DATA	xx..ECC..xx|	  |	DATA	**SPARE**xx..ECC..xx|
+ * |   (516)	xx.......xx|	  |  (516-n*4)	**(n*4)**xx.......xx|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |-----------------------|	  |---------------------------------|
+ *     codeword 1,2..n-1			codeword n

drivers/mtd/nand/qcom_nandc.c line 1422

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+ *  <---(528/532 Bytes)---->	   <-------(528/532 Bytes)---------->
+ *
+ * n = number of codewords in the page
+ * . = ECC bytes
+ * * = spare bytes
+ * x = unused/reserved bytes
+ *
+ * 2K page: n = 4, spare = 16 bytes
+ * 4K page: n = 8, spare = 32 bytes
+ * 8K page: n = 16, spare = 64 bytes

Is there a reason not to use the following layout?

(
n x (
data = 512 bytes
protected OOB data = 4 bytes
ECC bytes = 12 or 16
)

+

remaining unprotected OOB bytes
)

This way the ECC layout definition would be easier to define, and you'll have something that is closer to what a NAND chip expect (ECC block/step size of 512 or 1024).

I know this is also dependent on the bootloader, hence my question.

Quoted 15 lines

+ *
+ * the qcom nand controller operates at a sub page/codeword level. each
+ * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
+ * the number of ECC bytes vary based on the ECC strength and the bus width.
+ *
+ * the first n - 1 codewords contains 516 bytes of user data, the remaining
+ * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
+ * both user data and spare(oobavail) bytes that sum up to 516 bytes.
+ *
+ * the layout described above is used by the controller when the ECC block is
+ * enabled. When we read a page with ECC enabled, the unused/reserved bytes are
+ * skipped and not copied to our internal buffer. therefore, the nand_ecclayout
+ * layouts defined below doesn't consider the positions occupied by the reserved
+ * bytes

You could just read this portion with the ECC engine disabled when you're asked for OOB data.

Quoted 5 lines

+ *
+ * when the ECC block is disabled, one unused byte (or two for 16 bit bus width)
+ * in the last codeword is the position of bad block marker. the bad block
+ * marker cannot be accessed when ECC is enabled.

So, you're switching the BBM with the data at the BBM position (possibly some in-band data), right?

drivers/mtd/nand/qcom_nandc.c line 1447

Quoted 8 lines

+ *
+ */

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+
+/*
+ * Layouts for different page sizes and ecc modes. We skip the eccpos field
+ * since it isn't needed for this driver
+ */

If you know where they are stored, please specify them, even if they are not used by the upper layers (this helps analyzing raw nand dumps).

drivers/mtd/nand/qcom_nandc.c line 1455

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c new file +8 -0

+
+/* 2K page, 4 bit ECC */
+static struct nand_ecclayout layout_oob_64 = {
+	.eccbytes	= 40,
+	.oobfree	= {
+				{ 30, 16 },
+			  },
+};

According to your description it should either be eccbytes = 48 (if you're considering reserved bytes as ECC bytes) or 32 (if you're not counting reserved bytes).

BTW, is the oobfree portion really starting at offset 30?

I'd say that in the 2K page, 4 bit ECC you don't have any oobfree bytes (528 * 4 == 2048 + 64).

Quoted 29 lines

drivers/mtd/nand/qcom_nandc.c new file +28 -0

+
+/* 4K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_128 = {
+	.eccbytes	= 80,
+	.oobfree	= {
+				{ 70, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 8 bit bus width */
+static struct nand_ecclayout layout_oob_224_x8 = {
+	.eccbytes	= 104,
+	.oobfree	= {
+				{ 91, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 16 bit bus width */
+static struct nand_ecclayout layout_oob_224_x16 = {
+	.eccbytes	= 112,
+	.oobfree	= {
+				{ 98, 32 },
+			  },
+};
+
+/* 8K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_256 = {
+	.eccbytes	= 160,

drivers/mtd/nand/qcom_nandc.c line 1487

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+	.oobfree	= {
+				{ 151, 64 },
+			  },
+};

Those ECC layout definitions could probably be dynamically created based on the detected ECC strength, bus-width and page size, instead of defining a new one for each new combination.

Quoted 16 lines

drivers/mtd/nand/qcom_nandc.c new file +15 -0

+
+/*
+ * this is called before scan_ident, we do some minimal configurations so
+ * that reading ID and ONFI params work
+ */
+static void qcom_nandc_pre_init(struct qcom_nandc_data *this)
+{
+	/* kill onenand */
+	nandc_write(this, SFLASHC_BURST_CFG, 0);
+
+	/* enable ADM DMA */
+	nandc_write(this, NAND_FLASH_CHIP_SELECT, DM_EN);
+
+	/* save the original values of these registers */
+	this->cmd1 = nandc_read(this, NAND_DEV_CMD1);

drivers/mtd/nand/qcom_nandc.c line 1512

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	this->vld = nandc_read(this, NAND_DEV_CMD_VLD);
+
+	/* initial status value */
+	this->status = NAND_STATUS_READY | NAND_STATUS_WP;
+}
+
+static int qcom_nandc_ecc_init(struct qcom_nandc_data *this)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage;
+	bool wide_bus;
+
+	/* the nand controller fetches codewords/chunks of 512 bytes */
+	cwperpage = mtd->writesize >> 9;
+
+	ecc->strength = this->ecc_strength;
+
+	wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;

drivers/mtd/nand/qcom_nandc.c line 1526

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+
+	if (ecc->strength >= 8) {
+		/* 8 bit ECC defaults to BCH ECC on all platforms */
+		ecc->bytes = wide_bus ? 14 : 13;

Maybe you'd better consider that reserved bytes (after the ECC bytes) are actually ECC bytes. So, according to your description you would always have 16 here.

drivers/mtd/nand/qcom_nandc.c line 1534

Quoted 9 lines

drivers/mtd/nand/qcom_nandc.c new file +8 -0

+	} else {
+		/*
+		 * if the controller supports BCH for 4 bit ECC, the controller
+		 * uses lesser bytes for ECC. If RS is used, the ECC bytes is
+		 * always 10 bytes
+		 */
+		if (this->ecc_modes & ECC_BCH_4BIT)
+			ecc->bytes = wide_bus ? 8 : 7;

Ditto, except it's 12 here.

Quoted 44 lines

drivers/mtd/nand/qcom_nandc.c new file +44 -0

+		else
+			ecc->bytes = 10;
+	}
+
+	/* each step consists of 512 bytes of data */
+	ecc->size = NANDC_STEP_SIZE;
+
+	ecc->read_page		= qcom_nandc_read_page;
+	ecc->read_oob		= qcom_nandc_read_oob;
+	ecc->write_page		= qcom_nandc_write_page;
+	ecc->write_oob		= qcom_nandc_write_oob;
+
+	/*
+	 * the bad block marker is readable only when we read the page with ECC
+	 * disabled. all the ops above run with ECC enabled. We need raw read
+	 * and write function for oob in order to access bad block marker.
+	 */
+	ecc->read_oob_raw	= qcom_nandc_read_oob_raw;
+	ecc->write_oob_raw	= qcom_nandc_write_oob_raw;
+
+	switch (mtd->oobsize) {
+	case 64:
+		ecc->layout = &layout_oob_64;
+		break;
+	case 128:
+		ecc->layout = &layout_oob_128;
+		break;
+	case 224:
+		if (wide_bus)
+			ecc->layout = &layout_oob_224_x16;
+		else
+			ecc->layout = &layout_oob_224_x8;
+		break;
+	case 256:
+		ecc->layout = &layout_oob_256;
+		break;
+	default:
+		dev_err(this->dev, "unsupported NAND device, oobsize %d\n",
+			mtd->oobsize);
+		return -ENODEV;
+	}
+
+	ecc->mode = NAND_ECC_HW;
+

drivers/mtd/nand/qcom_nandc.c line 1588

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	/* enable ecc by default */
+	this->use_ecc = true;
+
+	return 0;
+}
+
+static void qcom_nandc_hw_post_init(struct qcom_nandc_data *this)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 128 lines

drivers/mtd/nand/qcom_nandc.c new file +128 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage = mtd->writesize / ecc->size;
+	int spare_bytes, bad_block_byte;
+	bool wide_bus;
+	int ecc_mode = 0;
+
+	wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
+
+	if (ecc->strength >= 8) {
+		this->cw_size = 532;
+
+		spare_bytes = wide_bus ? 0 : 2;
+
+		this->bch_enabled = true;
+		ecc_mode = 1;
+	} else {
+		this->cw_size = 528;
+
+		if (this->ecc_modes & ECC_BCH_4BIT) {
+			spare_bytes = wide_bus ? 2 : 4;
+
+			this->bch_enabled = true;
+			ecc_mode = 0;
+		} else {
+			spare_bytes = wide_bus ? 0 : 1;
+		}
+	}
+
+	/*
+	 * DATA_UD_BYTES varies based on whether the read/write command protects
+	 * spare data with ECC too. We protect spare data by default, so we set
+	 * it to main + spare data, which are 512 and 4 bytes respectively.
+	 */
+	this->cw_data = 516;
+
+	bad_block_byte = mtd->writesize - this->cw_size * (cwperpage - 1) + 1;
+
+	this->cfg0 = (cwperpage - 1) << CW_PER_PAGE
+				| this->cw_data << UD_SIZE_BYTES
+				| 0 << DISABLE_STATUS_AFTER_WRITE
+				| 5 << NUM_ADDR_CYCLES
+				| ecc->bytes << ECC_PARITY_SIZE_BYTES_RS
+				| 0 << STATUS_BFR_READ
+				| 1 << SET_RD_MODE_AFTER_STATUS
+				| spare_bytes << SPARE_SIZE_BYTES;
+
+	this->cfg1 = 7 << NAND_RECOVERY_CYCLES
+				| 0 <<  CS_ACTIVE_BSY
+				| bad_block_byte << BAD_BLOCK_BYTE_NUM
+				| 0 << BAD_BLOCK_IN_SPARE_AREA
+				| 2 << WR_RD_BSY_GAP
+				| wide_bus << WIDE_FLASH
+				| this->bch_enabled << ENABLE_BCH_ECC;
+
+	this->cfg0_raw = (cwperpage - 1) << CW_PER_PAGE
+				| this->cw_size << UD_SIZE_BYTES
+				| 5 << NUM_ADDR_CYCLES
+				| 0 << SPARE_SIZE_BYTES;
+
+	this->cfg1_raw = 7 << NAND_RECOVERY_CYCLES
+				| 0 << CS_ACTIVE_BSY
+				| 17 << BAD_BLOCK_BYTE_NUM
+				| 1 << BAD_BLOCK_IN_SPARE_AREA
+				| 2 << WR_RD_BSY_GAP
+				| wide_bus << WIDE_FLASH
+				| 1 << DEV0_CFG1_ECC_DISABLE;
+
+	this->ecc_bch_cfg = this->bch_enabled << ECC_CFG_ECC_DISABLE
+				| 0 << ECC_SW_RESET
+				| this->cw_data << ECC_NUM_DATA_BYTES
+				| 1 << ECC_FORCE_CLK_OPEN
+				| ecc_mode << ECC_MODE
+				| ecc->bytes << ECC_PARITY_SIZE_BYTES_BCH;
+
+	this->ecc_buf_cfg = 0x203 << NUM_STEPS;
+
+	this->clrflashstatus = FS_READY_BSY_N;
+	this->clrreadstatus = 0xc0;
+
+	dev_dbg(this->dev,
+		"cfg0 %x cfg1 %x ecc_buf_cfg %x ecc_bch cfg %x cw_size %d cw_data %d strength %d parity_bytes %d steps %d\n",
+		this->cfg0, this->cfg1, this->ecc_buf_cfg,
+		this->ecc_bch_cfg, this->cw_size, this->cw_data,
+		ecc->strength, ecc->bytes, cwperpage);
+}
+
+static int qcom_nandc_alloc(struct qcom_nandc_data *this)
+{
+	int r;
+
+	r = dma_set_coherent_mask(this->dev, DMA_BIT_MASK(32));
+	if (r) {
+		dev_err(this->dev, "failed to set DMA mask\n");
+		return r;
+	}
+
+	/*
+	 * we use the internal buffer for reading ONFI params, reading small
+	 * data like ID and status, and preforming read-copy-write operations
+	 * when writing to a codeword partially. 532 is the maximum possible
+	 * size of a codeword for our nand controller
+	 */
+	this->buf_size = 532;
+
+	this->data_buffer = devm_kzalloc(this->dev, this->buf_size, GFP_KERNEL);
+	if (!this->data_buffer)
+		return -ENOMEM;
+
+	this->regs = devm_kzalloc(this->dev, sizeof(*this->regs), GFP_KERNEL);
+	if (!this->regs)
+		return -ENOMEM;
+
+	this->reg_read_buf = devm_kzalloc(this->dev,
+				MAX_REG_RD * sizeof(*this->reg_read_buf),
+				GFP_KERNEL);
+	if (!this->reg_read_buf)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&this->list);
+
+	this->chan = dma_request_slave_channel(this->dev, "rxtx");
+	if (!this->chan) {
+		dev_err(this->dev, "failed to request slave channel\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}

drivers/mtd/nand/qcom_nandc.c line 1726

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+
+static void qcom_nandc_unalloc(struct qcom_nandc_data *this)
+{
+	dma_release_channel(this->chan);
+}
+
+static int qcom_nandc_init(struct qcom_nandc_data *this)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

drivers/mtd/nand/qcom_nandc.c line 1728

Quoted 2 lines

drivers/mtd/nand/qcom_nandc.c new file +2 -0

+	struct device_node *np = this->dev->of_node;
+	struct mtd_part_parser_data ppdata = { .of_node = np };

You don't need it anymore, because it's automatically extracted from the ->flash_node field.
Just call, nand_set_flash_node() before calling nand_scan_ident().

Quoted 17 lines

drivers/mtd/nand/qcom_nandc.c new file +16 -0

+	int r;
+
+	mtd->priv = chip;
+	mtd->name = "qcom-nandc";
+	mtd->owner = THIS_MODULE;
+
+	chip->priv = this;
+
+	chip->cmdfunc		= qcom_nandc_command;
+	chip->select_chip	= qcom_nandc_select_chip;
+	chip->read_byte		= qcom_nandc_read_byte;
+	chip->read_buf		= qcom_nandc_read_buf;
+	chip->write_buf		= qcom_nandc_write_buf;
+
+	chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
+	if (this->bus_width == 16)

drivers/mtd/nand/qcom_nandc.c line 1749

Quoted 5 lines

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+		chip->options |= NAND_BUSWIDTH_16;
+
+	chip->bbt_options = NAND_BBT_ACCESS_BBM_RAW;
+	if (of_get_nand_on_flash_bbt(np))
+		chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;

Ditto: rely on the work done by nand_dt_init() which is called by nand_scan_ident().

Quoted 13 lines

drivers/mtd/nand/qcom_nandc.c new file +12 -0

+
+	qcom_nandc_pre_init(this);
+
+	r = nand_scan_ident(mtd, 1, NULL);
+	if (r)
+		return r;
+
+	r = qcom_nandc_ecc_init(this);
+	if (r)
+		return r;
+
+	qcom_nandc_hw_post_init(this);

drivers/mtd/nand/qcom_nandc.c line 1767

Quoted 6 lines

drivers/mtd/nand/qcom_nandc.c new file +6 -0

+
+	r = nand_scan_tail(mtd);
+	if (r)
+		return r;
+
+	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);

	return mtd_device_parse_register(mtd, NULL, NULL, NULL 0);

Quoted 16 lines

drivers/mtd/nand/qcom_nandc.c new file +15 -0

+}
+
+static int qcom_nandc_parse_dt(struct platform_device *pdev)
+{
+	struct qcom_nandc_data *this = platform_get_drvdata(pdev);
+	struct device_node *np = this->dev->of_node;
+	int r;
+
+	this->ecc_strength = of_get_nand_ecc_strength(np);
+	if (this->ecc_strength < 0) {
+		dev_warn(this->dev,
+			"incorrect ecc strength, setting to 4 bits/step\n");
+		this->ecc_strength = 4;
+	}
+

drivers/mtd/nand/qcom_nandc.c line 1787

Quoted 5 lines

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+	this->bus_width = of_get_nand_bus_width(np);
+	if (this->bus_width < 0) {
+		dev_warn(this->dev, "incorrect bus width, setting to 8\n");
+		this->bus_width = 8;
+	}

Those two properties are extracted when calling nand_scan_ident() if you've called nand_set_flash_node() before that.

Quoted 85 lines

drivers/mtd/nand/qcom_nandc.c new file +84 -0

+
+	r = of_property_read_u32(np, "qcom,cmd-crci", &this->cmd_crci);
+	if (r) {
+		dev_err(this->dev, "command CRCI unspecified\n");
+		return r;
+	}
+
+	r = of_property_read_u32(np, "qcom,data-crci", &this->data_crci);
+	if (r) {
+		dev_err(this->dev, "data CRCI unspecified\n");
+		return r;
+	}
+
+	return 0;
+}
+
+static int qcom_nandc_probe(struct platform_device *pdev)
+{
+	struct qcom_nandc_data *this;
+	const void *dev_data;
+	int r;
+
+	this = devm_kzalloc(&pdev->dev, sizeof(*this), GFP_KERNEL);
+	if (!this)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, this);
+
+	this->pdev = pdev;
+	this->dev  = &pdev->dev;
+
+	dev_data = of_device_get_match_data(&pdev->dev);
+	if (!dev_data) {
+		dev_err(&pdev->dev, "failed to get device data\n");
+		return -ENODEV;
+	}
+
+	this->ecc_modes = (unsigned long)dev_data;
+
+	this->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	this->base = devm_ioremap_resource(&pdev->dev, this->res);
+	if (IS_ERR(this->base))
+		return PTR_ERR(this->base);
+
+	this->core_clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(this->core_clk))
+		return PTR_ERR(this->core_clk);
+
+	this->aon_clk = devm_clk_get(&pdev->dev, "aon");
+	if (IS_ERR(this->aon_clk))
+		return PTR_ERR(this->aon_clk);
+
+	r = qcom_nandc_parse_dt(pdev);
+	if (r)
+		return r;
+
+	r = qcom_nandc_alloc(this);
+	if (r)
+		return r;
+
+	r = clk_prepare_enable(this->core_clk);
+	if (r)
+		goto err_core_clk;
+
+	r = clk_prepare_enable(this->aon_clk);
+	if (r)
+		goto err_aon_clk;
+
+	r = qcom_nandc_init(this);
+	if (r)
+		goto err_init;
+
+	return 0;
+
+err_init:
+	clk_disable_unprepare(this->aon_clk);
+err_aon_clk:
+	clk_disable_unprepare(this->core_clk);
+err_core_clk:
+	qcom_nandc_unalloc(this);
+
+	return r;
+}
+

drivers/mtd/nand/qcom_nandc.c line 1875

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+static int qcom_nandc_remove(struct platform_device *pdev)
+{
+	struct qcom_nandc_data *this = platform_get_drvdata(pdev);
+

You miss a call to nand_release() here, otherwise your device is still registered to the MTD/NAND layer, even though you've released all the resources attached to it.

That's all I got for now.

Quoted 39 lines

Quoted 9 lines

+/*
+ * NAND controller page layout info
+ *

+ * |-----------------------|	  |---------------------------------|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |	DATA	xx..ECC..xx|	  |	DATA	**SPARE**xx..ECC..xx|
+ * |   (516)	xx.......xx|	  |  (516-n*4)	**(n*4)**xx.......xx|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |-----------------------|	  |---------------------------------|

Quoted 11 lines

+ * codeword 1,2..n-1 codeword n
+ * <---(528/532 Bytes)----> <-------(528/532 Bytes)---------->
+ *
+ * n = number of codewords in the page
+ * . = ECC bytes
+ * * = spare bytes
+ * x = unused/reserved bytes
+ *
+ * 2K page: n = 4, spare = 16 bytes
+ * 4K page: n = 8, spare = 32 bytes
+ * 8K page: n = 16, spare = 64 bytes

Is there a reason not to use the following layout?

(
n x (
data = 512 bytes
protected OOB data = 4 bytes
ECC bytes = 12 or 16
)

+

remaining unprotected OOB bytes
)

This way the ECC layout definition would be easier to define, and you'll have something that is closer to what a NAND chip expect (ECC block/step size of 512 or 1024).

Quoted 1 line

I know this is also dependent on the bootloader, hence my question.

I tried to figure this out looking at documentation and the downstream drivers. What I understood was that all the OOB was intentionally kept in the last step, so that things are faster when we only want to access OOB. In that case, the controller will need to write to only one step/codeword.

The bootloaders also use the same layout.

Quoted 17 lines

Quoted 16 lines

+ *
+ * the qcom nand controller operates at a sub page/codeword level. each
+ * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
+ * the number of ECC bytes vary based on the ECC strength and the bus width.
+ *
+ * the first n - 1 codewords contains 516 bytes of user data, the remaining
+ * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
+ * both user data and spare(oobavail) bytes that sum up to 516 bytes.
+ *
+ * the layout described above is used by the controller when the ECC block is
+ * enabled. When we read a page with ECC enabled, the unused/reserved bytes are
+ * skipped and not copied to our internal buffer. therefore, the nand_ecclayout
+ * layouts defined below doesn't consider the positions occupied by the reserved
+ * bytes

Quoted 2 lines

You could just read this portion with the ECC engine disabled when you're asked for OOB data.

Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that have an argument called 'oob_required'. We need to have ECC enabled when running these ops.

In order to read this additional portion, I'll need to read/write each step again with ECC disabled, which would really slow things down.

Quoted 9 lines

Quoted 6 lines

+ *
+ * when the ECC block is disabled, one unused byte (or two for 16 bit bus width)
+ * in the last codeword is the position of bad block marker. the bad block
+ * marker cannot be accessed when ECC is enabled.

So, you're switching the BBM with the data at the BBM position (possibly some in-band data), right?

Yes. When ECC isn't enabled, the BBM byte lies within the in-band data of the last step. In fact, there are dummy BBM bytes in the previous steps at the same offset.

With ECC enabled, the controller just skips that position (and the dummy BBM bytes in previous steps) altogether.

Quoted 13 lines

drivers/mtd/nand/qcom_nandc.c line 1447

Quoted 9 lines

+ *
+ */

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+
+/*
+ * Layouts for different page sizes and ecc modes. We skip the eccpos field
+ * since it isn't needed for this driver
+ */

If you know where they are stored, please specify them, even if they are not used by the upper layers (this helps analyzing raw nand dumps).

Quoted 12 lines

drivers/mtd/nand/qcom_nandc.c line 1455

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c new file +8 -0

+
+/* 2K page, 4 bit ECC */
+static struct nand_ecclayout layout_oob_64 = {
+	.eccbytes	= 40,
+	.oobfree	= {
+				{ 30, 16 },
+			  },
+};

According to your description it should either be eccbytes = 48 (if you're considering reserved bytes as ECC bytes) or 32 (if you're not counting reserved bytes).

Each step is 528 bytes in total. The first 3 steps contain 516 bytes of data, 10 bytes of ECC and 2 bytes of resrved data. The last step contains 500 bytes of data, 16 bytes of OOB, 10 bytes of ECC and 2 reserved bytes.

If I don't count the reserved bytes as part of ECC, I get 40. If I do count it as part of ECC, I get 48. In the way I described layouts, I ignored the ECC parts. How did you get 32?

From the ecc.bytes value you're filling in qcom_nandc_pre_init(). Just

Quoted 3 lines

BTW, is the oobfree portion really starting at offset 30?

I thought the offsets mentioned here also had to incorporate positions taken by ECC bytes? If I strip all the the in-band data (real data) from each step, we get:

ECC(10 bytes).ECC(10 bytes).ECC(10 bytes).OOB(16 bytes).ECC(10 bytes)

Wouldn't this result in the offset as 30?

We are still only taking into account 56 bytes out of the 64 bytes in the chip's OOB. This is because I'm discaring the 2 bytes from each step (summing up to 8) which aren't accessible when ECC is enabled.

Quoted 4 lines

I'd say that in the 2K page, 4 bit ECC you don't have any oobfree bytes (528 * 4 == 2048 + 64).

528 contains both oob and in-band data. If you ignore the weird layout and assume we have at an average 512 bytes for each step, we get:

512 * 4 == 2048 bytes of data, and 64 bytes of OOB (16 bytes free, 40 ECC, and 8 reserved/unused).

Quoted 30 lines

Quoted 30 lines

drivers/mtd/nand/qcom_nandc.c new file +28 -0

+
+/* 4K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_128 = {
+	.eccbytes	= 80,
+	.oobfree	= {
+				{ 70, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 8 bit bus width */
+static struct nand_ecclayout layout_oob_224_x8 = {
+	.eccbytes	= 104,
+	.oobfree	= {
+				{ 91, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 16 bit bus width */
+static struct nand_ecclayout layout_oob_224_x16 = {
+	.eccbytes	= 112,
+	.oobfree	= {
+				{ 98, 32 },
+			  },
+};
+
+/* 8K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_256 = {
+	.eccbytes	= 160,

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c line 1487

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+	.oobfree	= {
+				{ 151, 64 },
+			  },
+};

Those ECC layout definitions could probably be dynamically created based on the detected ECC strength, bus-width and page size, instead of defining a new one for each new combination.

That's true. I can try that out.

Quoted 41 lines

Quoted 17 lines

drivers/mtd/nand/qcom_nandc.c new file +15 -0

+
+/*
+ * this is called before scan_ident, we do some minimal configurations so
+ * that reading ID and ONFI params work
+ */
+static void qcom_nandc_pre_init(struct qcom_nandc_data *this)
+{
+	/* kill onenand */
+	nandc_write(this, SFLASHC_BURST_CFG, 0);
+
+	/* enable ADM DMA */
+	nandc_write(this, NAND_FLASH_CHIP_SELECT, DM_EN);
+
+	/* save the original values of these registers */
+	this->cmd1 = nandc_read(this, NAND_DEV_CMD1);

drivers/mtd/nand/qcom_nandc.c line 1512

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	this->vld = nandc_read(this, NAND_DEV_CMD_VLD);
+
+	/* initial status value */
+	this->status = NAND_STATUS_READY | NAND_STATUS_WP;
+}
+
+static int qcom_nandc_ecc_init(struct qcom_nandc_data *this)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage;
+	bool wide_bus;
+
+	/* the nand controller fetches codewords/chunks of 512 bytes */
+	cwperpage = mtd->writesize >> 9;
+
+	ecc->strength = this->ecc_strength;
+
+	wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c line 1526

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+
+	if (ecc->strength >= 8) {
+		/* 8 bit ECC defaults to BCH ECC on all platforms */
+		ecc->bytes = wide_bus ? 14 : 13;

Maybe you'd better consider that reserved bytes (after the ECC bytes) are actually ECC bytes. So, according to your description you would always have 16 here.

The thing is that if I consider the reserved bytes as a part of the ECC bytes, and if I use this bigger value when configuring the controller and dma, I will get bad results; becase the hardware doesn't touch these when ECC is enabled.

I could set the ecc->bytes to '16' and still use the actual values when configuring the controller. Do you think that will help in any way?

Quoted 3 lines

That's all I got for now.

Thanks again for taking the time for the review. Really appreciate it.

Quoted 41 lines · On Wed, 16 Dec 2015 17:27:48 +0530 Archit Taneja <[email protected]> wrote:

Quoted 40 lines

Quoted 9 lines

+/*
+ * NAND controller page layout info
+ *

+ * |-----------------------|	  |---------------------------------|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |	DATA	xx..ECC..xx|	  |	DATA	**SPARE**xx..ECC..xx|
+ * |   (516)	xx.......xx|	  |  (516-n*4)	**(n*4)**xx.......xx|
+ * |		xx.......xx|	  |		*********xx.......xx|
+ * |-----------------------|	  |---------------------------------|

Quoted 11 lines

+ * codeword 1,2..n-1 codeword n
+ * <---(528/532 Bytes)----> <-------(528/532 Bytes)---------->
+ *
+ * n = number of codewords in the page
+ * . = ECC bytes
+ * * = spare bytes
+ * x = unused/reserved bytes
+ *
+ * 2K page: n = 4, spare = 16 bytes
+ * 4K page: n = 8, spare = 32 bytes
+ * 8K page: n = 16, spare = 64 bytes

Is there a reason not to use the following layout?

(
n x (
data = 512 bytes
protected OOB data = 4 bytes
ECC bytes = 12 or 16
)

+

remaining unprotected OOB bytes
)

This way the ECC layout definition would be easier to define, and you'll have something that is closer to what a NAND chip expect (ECC block/step size of 512 or 1024).

I know this is also dependent on the bootloader, hence my question.

Quoted 5 lines

I tried to figure this out looking at documentation and the downstream drivers. What I understood was that all the OOB was intentionally kept in the last step, so that things are faster when we only want to access OOB. In that case, the controller will need to write to only one step/codeword.

Well, I don't think sending a column change command is that costly compared to a page retrieval command or ECC calculation.

Quoted 3 lines

The bootloaders also use the same layout.

Ok, then I guess we'll have to live with that (but it complicates a lot the driver logic :-/)

Quoted 25 lines

Quoted 18 lines

Quoted 17 lines

+ *
+ * the qcom nand controller operates at a sub page/codeword level. each
+ * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
+ * the number of ECC bytes vary based on the ECC strength and the bus width.
+ *
+ * the first n - 1 codewords contains 516 bytes of user data, the remaining
+ * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
+ * both user data and spare(oobavail) bytes that sum up to 516 bytes.
+ *
+ * the layout described above is used by the controller when the ECC block is
+ * enabled. When we read a page with ECC enabled, the unused/reserved bytes are
+ * skipped and not copied to our internal buffer. therefore, the nand_ecclayout
+ * layouts defined below doesn't consider the positions occupied by the reserved
+ * bytes

Quoted 2 lines

You could just read this portion with the ECC engine disabled when you're asked for OOB data.

Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that have an argument called 'oob_required'. We need to have ECC enabled when running these ops.

Quoted 2 lines

In order to read this additional portion, I'll need to read/write each step again with ECC disabled, which would really slow things down.

Nowadays, MTD FS are not using the OOB area, and oob_required is only passed if the MTD user asked for OOB data, so that can be an acceptable penalty. Anyway, that's not really important if we loose a few OOB bytes.

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

+ *
+ * when the ECC block is disabled, one unused byte (or two for 16 bit bus width)
+ * in the last codeword is the position of bad block marker. the bad block
+ * marker cannot be accessed when ECC is enabled.

So, you're switching the BBM with the data at the BBM position (possibly some in-band data), right?

Quoted 6 lines

Yes. When ECC isn't enabled, the BBM byte lies within the in-band data of the last step. In fact, there are dummy BBM bytes in the previous steps at the same offset.

With ECC enabled, the controller just skips that position (and the dummy BBM bytes in previous steps) altogether.

Okay.

Quoted 31 lines

Quoted 13 lines

drivers/mtd/nand/qcom_nandc.c line 1447

Quoted 9 lines

+ *
+ */

drivers/mtd/nand/qcom_nandc.c new file +5 -0

+
+/*
+ * Layouts for different page sizes and ecc modes. We skip the eccpos field
+ * since it isn't needed for this driver
+ */

If you know where they are stored, please specify them, even if they are not used by the upper layers (this helps analyzing raw nand dumps).

Quoted 12 lines

drivers/mtd/nand/qcom_nandc.c line 1455

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c new file +8 -0

+
+/* 2K page, 4 bit ECC */
+static struct nand_ecclayout layout_oob_64 = {
+	.eccbytes	= 40,
+	.oobfree	= {
+				{ 30, 16 },
+			  },
+};

According to your description it should either be eccbytes = 48 (if you're considering reserved bytes as ECC bytes) or 32 (if you're not counting reserved bytes).

Each step is 528 bytes in total. The first 3 steps contain 516 bytes of data, 10 bytes of ECC and 2 bytes of resrved data. The last step contains 500 bytes of data, 16 bytes of OOB, 10 bytes of ECC and 2 reserved bytes.

Quoted 3 lines

If I don't count the reserved bytes as part of ECC, I get 40. If I do count it as part of ECC, I get 48. In the way I described layouts, I ignored the ECC parts. How did you get 32?

From the ecc.bytes value you're filling in qcom_nandc_pre_init(). Just
multiplied it by 4 (the number of ECC steps).

And here is where weird things happen: you're setting ecc.size to 512 and ecc.strength to 4. But in reality, what you have is 4bits/516bytes for the first 3 blocks, and 4bits/500bytes for the last one. You're not only fooling the MTD user when faking a 4bits/512byte strength, you're also sightly modifying the logic supposed to detect the maximum number of bitflips found in a page.

This being said, I understand your constraints, just trying to explain why we're trying to use the symmetric ECC block size.

Quoted 9 lines

Quoted 4 lines

BTW, is the oobfree portion really starting at offset 30?

I thought the offsets mentioned here also had to incorporate positions taken by ECC bytes? If I strip all the the in-band data (real data) from each step, we get:

Quoted 3 lines

ECC(10 bytes).ECC(10 bytes).ECC(10 bytes).OOB(16 bytes).ECC(10 bytes)

Wouldn't this result in the offset as 30?

Yep, as I said I thought it was 32 because of what you put in ecc.size. And, you're putting OOB bytes before the last chunk of ECC bytes, because they are protected, right?

Note that you could put the oobfree area at the end of the OOB area, since this 10-10-10-16-10 representation is already a virtual representation of the OOB area (ECC bytes are actually interleaved with in-band data on the flash).

Quoted 6 lines

We are still only taking into account 56 bytes out of the 64 bytes in the chip's OOB. This is because I'm discaring the 2 bytes from each step (summing up to 8) which aren't accessible when ECC is enabled.

Okay. As said above, those two bytes could be exposed without two much overhead for most operations, but that's your call to make.

Quoted 6 lines

Quoted 5 lines

I'd say that in the 2K page, 4 bit ECC you don't have any oobfree bytes (528 * 4 == 2048 + 64).

Quoted 5 lines

528 contains both oob and in-band data. If you ignore the weird layout and assume we have at an average 512 bytes for each step, we get:

512 * 4 == 2048 bytes of data, and 64 bytes of OOB (16 bytes free, 40 ECC, and 8 reserved/unused).

Okay. Still think the ECC block asymmetry is not a good idea, but I get your point ;-).

Quoted 40 lines

Quoted 31 lines

Quoted 31 lines

drivers/mtd/nand/qcom_nandc.c new file +28 -0

+
+/* 4K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_128 = {
+	.eccbytes	= 80,
+	.oobfree	= {
+				{ 70, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 8 bit bus width */
+static struct nand_ecclayout layout_oob_224_x8 = {
+	.eccbytes	= 104,
+	.oobfree	= {
+				{ 91, 32 },
+			  },
+};
+
+/* 4K page, 8 bit ECC, 16 bit bus width */
+static struct nand_ecclayout layout_oob_224_x16 = {
+	.eccbytes	= 112,
+	.oobfree	= {
+				{ 98, 32 },
+			  },
+};
+
+/* 8K page, 4 bit ECC, 8/16 bit bus width */
+static struct nand_ecclayout layout_oob_256 = {
+	.eccbytes	= 160,

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c line 1487

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+	.oobfree	= {
+				{ 151, 64 },
+			  },
+};

Those ECC layout definitions could probably be dynamically created based on the detected ECC strength, bus-width and page size, instead of defining a new one for each new combination.

Quoted 1 line

That's true. I can try that out.

Cool.

Quoted 50 lines

Quoted 41 lines

Quoted 17 lines

drivers/mtd/nand/qcom_nandc.c new file +15 -0

+
+/*
+ * this is called before scan_ident, we do some minimal configurations so
+ * that reading ID and ONFI params work
+ */
+static void qcom_nandc_pre_init(struct qcom_nandc_data *this)
+{
+	/* kill onenand */
+	nandc_write(this, SFLASHC_BURST_CFG, 0);
+
+	/* enable ADM DMA */
+	nandc_write(this, NAND_FLASH_CHIP_SELECT, DM_EN);
+
+	/* save the original values of these registers */
+	this->cmd1 = nandc_read(this, NAND_DEV_CMD1);

drivers/mtd/nand/qcom_nandc.c line 1512

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	this->vld = nandc_read(this, NAND_DEV_CMD_VLD);
+
+	/* initial status value */
+	this->status = NAND_STATUS_READY | NAND_STATUS_WP;
+}
+
+static int qcom_nandc_ecc_init(struct qcom_nandc_data *this)
+{
+	struct mtd_info *mtd = &this->mtd;
+	struct nand_chip *chip = &this->chip;

	struct nand_chip *chip = &this->chip;
	struct mtd_info *mtd = nand_to_mtd(chip);

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int cwperpage;
+	bool wide_bus;
+
+	/* the nand controller fetches codewords/chunks of 512 bytes */
+	cwperpage = mtd->writesize >> 9;
+
+	ecc->strength = this->ecc_strength;
+
+	wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;

Quoted 8 lines

drivers/mtd/nand/qcom_nandc.c line 1526

Quoted 4 lines

drivers/mtd/nand/qcom_nandc.c new file +4 -0

+
+	if (ecc->strength >= 8) {
+		/* 8 bit ECC defaults to BCH ECC on all platforms */
+		ecc->bytes = wide_bus ? 14 : 13;

Maybe you'd better consider that reserved bytes (after the ECC bytes) are actually ECC bytes. So, according to your description you would always have 16 here.

Quoted 4 lines

The thing is that if I consider the reserved bytes as a part of the ECC bytes, and if I use this bigger value when configuring the controller and dma, I will get bad results; becase the hardware doesn't touch these when ECC is enabled.

You should at least be consistent with what you put in your ECC layout. Choose one solution and stick to it. Since reserved bytes are not accessible I would suggest to count them in the number of ECC bytes.

Quoted 1 line

Quoted 2 lines

I could set the ecc->bytes to '16' and still use the actual values when configuring the controller. Do you think that will help in any way?

You can have you own private field(s) to store you controller config, if that helps, or create a function which would convert ecc.bytes into something appropriate.

Quoted 6 lines

Note that you could put the oobfree area at the end of the OOB area, since this 10-10-10-16-10 representation is already a virtual representation of the OOB area (ECC bytes are actually interleaved with in-band data on the flash).

But, when I read from the controller's internal buffer via DMA, I first get the oobfree area, and only then the last step's ECC bytes. So, the content in chip->oob_poi is in the same order as mentioned above (10-10-10-16-10).

If the upper layers uses MTD_OPS_AUTO_OOB, and if I have a different layout as what it is in the oob_poi buffer, then it'll end up reading/writing the wrong bytes in nand_transfer_oob/nand_fill_oob.

Are you suggesting that I modify the contents of oob_poi by hand such that it has a cleaner 10-10-10-10-16 representation?

Quoted 6 lines · Boris Brezillon

drivers/mtd/nand/qcom_nandc.c line 1767

Quoted 1 line · On Wed, 19 Aug 2015 10:19:03 +0530 Archit Taneja <[email protected]> wrote:

drivers/mtd/nand/qcom_nandc.c new file +1 -0

+	return mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);

	return mtd_device_parse_register(mtd, NULL, NULL, NULL 0);

Will fix.

Add DT bindings document for the Qualcomm NAND controller driver.

Cc [email protected]

v4:
- No changes

v3:
- Don't use '0x' when specifying nand controller address space
- Add optional property for on-flash bbt usage

Acked-by Andy Gross <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
 .../devicetree/bindings/mtd/qcom_nandc.txt         | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt

Documentation/devicetree/bindings/mtd/qcom_nandc.txt new file +39 -0

new file mode 100644
index 0000000..1de4643
@@ -0,0 +1,49 @@
+* Qualcomm NAND controller
+
+Required properties:
+- compatible:		should be "qcom,ebi2-nand" for IPQ806x
+- reg:			MMIO address range
+- clocks:		must contain core clock and always on clock
+- clock-names:		must contain "core" for the core clock and "aon" for the
+			always on clock
+- dmas:			DMA specifier, consisting of a phandle to the ADM DMA
+			controller node and the channel number to be used for
+			NAND. Refer to dma.txt and qcom_adm.txt for more details
+- dma-names:		must be "rxtx"
+- qcom,cmd-crci:	must contain the ADM command type CRCI block instance
+			number specified for the NAND controller on the given
+			platform
+- qcom,data-crci:	must contain the ADM data type CRCI block instance
+			number specified for the NAND controller on the given
+			platform
+
+Optional properties:
+- nand-bus-width:	bus width. Must be 8 or 16. If not present, 8 is chosen
+			as default
+
+- nand-ecc-strength:	number of bits to correct per ECC step. Must be 4 or 8
+			bits. If not present, 4 is chosen as default
+- nand-on-flash-bbt:	Create/use on-flash bad block table
+
+The device tree may optionally contain sub-nodes describing partitions of the
+address space. See partition.txt for more detail.
+
+Example:
+
+nand@1ac00000 {
+	compatible = "qcom,ebi2-nandc";
+	reg = <0x1ac00000 0x800>;
+
+	clocks = <&gcc EBI2_CLK>,
+		 <&gcc EBI2_AON_CLK>;
+	clock-names = "core", "aon";

Hi Archit,

Sorry for the late review, but there are a few things I think should be addressed.

Quoted 72 lines · On Wed, 19 Aug 2015 10:19:04 +0530 Archit Taneja <[email protected]> wrote:

Add DT bindings document for the Qualcomm NAND controller driver.

Cc [email protected]

v4:
- No changes

v3:
- Don't use '0x' when specifying nand controller address space
- Add optional property for on-flash bbt usage

Acked-by Andy Gross <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
  .../devicetree/bindings/mtd/qcom_nandc.txt         | 49 ++++++++++++++++++++++
  1 file changed, 49 insertions(+)

create mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt

Documentation/devicetree/bindings/mtd/qcom_nandc.txt new file +48 -0

new file mode 100644
index 0000000..1de4643
@@ -0,0 +1,49 @@
+* Qualcomm NAND controller
+
+Required properties:
+- compatible:		should be "qcom,ebi2-nand" for IPQ806x
+- reg:			MMIO address range
+- clocks:		must contain core clock and always on clock
+- clock-names:		must contain "core" for the core clock and "aon" for the
+			always on clock
+- dmas:			DMA specifier, consisting of a phandle to the ADM DMA
+			controller node and the channel number to be used for
+			NAND. Refer to dma.txt and qcom_adm.txt for more details
+- dma-names:		must be "rxtx"
+- qcom,cmd-crci:	must contain the ADM command type CRCI block instance
+			number specified for the NAND controller on the given
+			platform
+- qcom,data-crci:	must contain the ADM data type CRCI block instance
+			number specified for the NAND controller on the given
+			platform
+
+Optional properties:
+- nand-bus-width:	bus width. Must be 8 or 16. If not present, 8 is chosen
+			as default
+
+- nand-ecc-strength:	number of bits to correct per ECC step. Must be 4 or 8
+			bits. If not present, 4 is chosen as default
+- nand-on-flash-bbt:	Create/use on-flash bad block table
+
+The device tree may optionally contain sub-nodes describing partitions of the
+address space. See partition.txt for more detail.
+
+Example:
+
+nand@1ac00000 {
+	compatible = "qcom,ebi2-nandc";
+	reg = <0x1ac00000 0x800>;
+
+	clocks = <&gcc EBI2_CLK>,
+		 <&gcc EBI2_AON_CLK>;
+	clock-names = "core", "aon";
+
+	dmas = <&adm_dma 3>;
+	dma-names = "rxtx";
+	qcom,cmd-crci = <15>;
+	qcom,data-crci = <3>;
+
+	partition@0 {
+	...
+	};

Documentation/devicetree/bindings/mtd/qcom_nandc.txt line 49

Quoted 1 line

Documentation/devicetree/bindings/mtd/qcom_nandc.txt new file +1 -0

+};

According to the registers layout defined in your driver, your NAND controller can address multiple chips (NAND_DEV_SEL register). Since DT bindings are supposed to be as stable as possible, I would recommend separating the NAND controller and NAND chip declaration (as done here
[1] and here [2]).

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt [2]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt

One of the arguments passed to struct nand_chip's block_bad op is 'getchip', which, if true, is supposed to get and select the nand device, and later unselect and release the device.

This op is intended to be replaceable by drivers. The drivers shouldn't be responsible for selecting/unselecting chip. Like other ops, the chip should already be selected before the block_bad op is called.

Remove the getchip argument from the block_bad op and move the chip selection to nand_block_checkbad (the only user of chip->block_bad). Modify nand_block_bad (the default function for the op) such that it doesn't select the chip.

Signed-off-by Archit Taneja <[email protected]>

---
 drivers/mtd/nand/nand_base.c | 41 +++++++++++++++++++++++------------------
 include/linux/mtd/nand.h     |  2 +-
 2 files changed, 24 insertions(+), 19 deletions(-)

drivers/mtd/nand/nand_base.c +23 -18

index 928081b..42aa441 100644
@@ -317,9 +317,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
  *
  * Check, if the block is bad.
  */
-static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
+static int nand_block_bad(struct mtd_info *mtd, loff_t ofs)
 {
-	int page, chipnr, res = 0, i = 0;
+	int page, res = 0, i = 0;
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	u16 bad;
 

@@ -328,15 +328,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
 
 	page = (int)(ofs >> chip->page_shift) & chip->pagemask;
 
-	if (getchip) {
-		chipnr = (int)(ofs >> chip->chip_shift);
-
-		nand_get_device(mtd, FL_READING);
-
-		/* Select the NAND device */
-		chip->select_chip(mtd, chipnr);
-	}
-
 	do {
 		if (chip->options & NAND_BUSWIDTH_16) {
 			chip->cmdfunc(mtd, NAND_CMD_READOOB,

@@ -361,11 +352,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
 		i++;
 	} while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE));
 
-	if (getchip) {
-		chip->select_chip(mtd, -1);
-		nand_release_device(mtd);
-	}
-
 	return res;
 }
 

@@ -514,8 +500,27 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip,
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (!chip->bbt)
-		return chip->block_bad(mtd, ofs, getchip);
+	if (!chip->bbt) {
+		int ret;
+
+		if (getchip) {
+			int chipnr = (int)(ofs >> chip->chip_shift);
+
+			nand_get_device(mtd, FL_READING);
+
+			/* Select the NAND device */
+			chip->select_chip(mtd, chipnr);
+		}
+
+		ret = chip->block_bad(mtd, ofs);
+
+		if (getchip) {
+			chip->select_chip(mtd, -1);
+			nand_release_device(mtd);
+		}
+
+		return ret;
+	}
 
 	/* Return info from the table */
 	return nand_isbad_bbt(mtd, ofs, allowbbt);

include/linux/mtd/nand.h +1 -1

index 3e92be1..c15818e 100644
@@ -650,7 +650,7 @@ struct nand_chip {
 	void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
 	void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len);
 	void (*select_chip)(struct mtd_info *mtd, int chip);
-	int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip);
+	int (*block_bad)(struct mtd_info *mtd, loff_t ofs);
 	int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
 	void (*cmd_ctrl)(struct mtd_info *mtd, int dat, unsigned int ctrl);
 	int (*dev_ready)(struct mtd_info *mtd);

Quoted 8 lines · On Tue, 5 Jan 2016 10:54:59 +0530 Archit Taneja <[email protected]> wrote:

One of the arguments passed to struct nand_chip's block_bad op is 'getchip', which, if true, is supposed to get and select the nand device, and later unselect and release the device.

This op is intended to be replaceable by drivers. The drivers shouldn't be responsible for selecting/unselecting chip. Like other ops, the chip should already be selected before the block_bad op is called.

Quoted 4 lines

Remove the getchip argument from the block_bad op and move the chip selection to nand_block_checkbad (the only user of chip->block_bad). Modify nand_block_bad (the default function for the op) such that it doesn't select the chip.

I would go even further and move the chip selection into nand_block_isbad(), who is the only caller that requires this chip selection.

Quoted 95 lines

Signed-off-by Archit Taneja <[email protected]>

---
  drivers/mtd/nand/nand_base.c | 41 +++++++++++++++++++++++------------------
  include/linux/mtd/nand.h     |  2 +-
  2 files changed, 24 insertions(+), 19 deletions(-)

drivers/mtd/nand/nand_base.c +23 -18

index 928081b..42aa441 100644
@@ -317,9 +317,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
   *
   * Check, if the block is bad.
   */
-static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
+static int nand_block_bad(struct mtd_info *mtd, loff_t ofs)
  {
-	int page, chipnr, res = 0, i = 0;
+	int page, res = 0, i = 0;
  	struct nand_chip *chip = mtd_to_nand(mtd);
  	u16 bad;
 

@@ -328,15 +328,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
 
  	page = (int)(ofs >> chip->page_shift) & chip->pagemask;
 
-	if (getchip) {
-		chipnr = (int)(ofs >> chip->chip_shift);
-
-		nand_get_device(mtd, FL_READING);
-
-		/* Select the NAND device */
-		chip->select_chip(mtd, chipnr);
-	}
-
  	do {
  		if (chip->options & NAND_BUSWIDTH_16) {
  			chip->cmdfunc(mtd, NAND_CMD_READOOB,

@@ -361,11 +352,6 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs, int getchip)
  		i++;
  	} while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE));
 
-	if (getchip) {
-		chip->select_chip(mtd, -1);
-		nand_release_device(mtd);
-	}
-
  	return res;
  }
 

@@ -514,8 +500,27 @@ static int nand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip,
  {
  	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (!chip->bbt)
-		return chip->block_bad(mtd, ofs, getchip);
+	if (!chip->bbt) {
+		int ret;
+
+		if (getchip) {
+			int chipnr = (int)(ofs >> chip->chip_shift);
+
+			nand_get_device(mtd, FL_READING);
+
+			/* Select the NAND device */
+			chip->select_chip(mtd, chipnr);
+		}
+
+		ret = chip->block_bad(mtd, ofs);
+
+		if (getchip) {
+			chip->select_chip(mtd, -1);
+			nand_release_device(mtd);
+		}
+
+		return ret;
+	}
 
  	/* Return info from the table */
  	return nand_isbad_bbt(mtd, ofs, allowbbt);

include/linux/mtd/nand.h +1 -1

index 3e92be1..c15818e 100644
@@ -650,7 +650,7 @@ struct nand_chip {
  	void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
  	void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len);
  	void (*select_chip)(struct mtd_info *mtd, int chip);
-	int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip);
+	int (*block_bad)(struct mtd_info *mtd, loff_t ofs);
  	int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
  	void (*cmd_ctrl)(struct mtd_info *mtd, int dat, unsigned int ctrl);
  	int (*dev_ready)(struct mtd_info *mtd);

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx, MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2) and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a broader interface for external slow peripheral devices such as LCD and NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs of our interest, we only use the NAND controller within EBI2. Therefore, it's safe for us to assume that the NAND controller is a standalone block within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main and spare data. The controller contains an internal 512 byte page buffer to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA for register read/write and data transfers. The controller performs page reads and writes at a codeword/step level of 512 bytes. It can support up to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for each codeword, followed by data descriptors to read or write data from the controller's internal buffer. It uses a single ADM DMA channel that we get via dmaengine API. The controller requires 2 ADM CRCIs for command and data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use the standard syndrome ecc ops because of several reasons. First, the amount of data bytes covered by ecc isn't same in each step. Second, writing to free oob space requires us writing to the entire step in which the oob lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker. The controller ignores reading the marker when ECC is enabled. ECC needs to be explicity disabled to read or write to the bad block marker. The nand_bbt helpers library hence can't access BBMs for the controller. For now, we skip the creation of BBT and populate chip->block_bad and chip->block_markbad helpers instead.

Reviewed-by Andy Gross <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
v5:
- split chip/controller structs
- simplify layout by considering reserved bytes as part of ECC
- create ecc layouts automatically
- implement block_bad and block_markbad chip ops instead of
- read_oob_raw/write_oob_raw ecc ops to access BBMs.
- Add NAND_SKIP_BBTSCAN flag until we get badblockbits support.
- misc clean ups

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes
- Add Stephen's Signed-off since he provided a patch to fix endianness problems

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

 drivers/mtd/nand/Kconfig      |    7 +
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/qcom_nandc.c | 1981 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1989 insertions(+)
 create mode 100644 drivers/mtd/nand/qcom_nandc.c

drivers/mtd/nand/Kconfig +7 -0

index 95b8d2b..2fccdfb 100644
@@ -546,4 +546,11 @@ config MTD_NAND_HISI504
 	help
 	  Enables support for NAND controller on Hisilicon SoC Hip04.
 
+config MTD_NAND_QCOM
+	tristate "Support for NAND on QCOM SoCs"
+	depends on ARCH_QCOM
+	help
+	  Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+	  controller. This controller is found on IPQ806x SoC.
+
 endif # MTD_NAND

drivers/mtd/nand/Makefile +1 -0

index 2c7f014..9450cdc 100644
@@ -55,5 +55,6 @@ 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_BRCMNAND)		+= brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o

drivers/mtd/nand/qcom_nandc.c new file +146 -0

new file mode 100644
index 0000000..a4dd922
@@ -0,0 +1,1981 @@
+/*
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/clk.h>
+#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_mtd.h>
+#include <linux/delay.h>
+
+/* NANDc reg offsets */
+#define NAND_FLASH_CMD			0x00
+#define NAND_ADDR0			0x04
+#define NAND_ADDR1			0x08
+#define NAND_FLASH_CHIP_SELECT		0x0c
+#define NAND_EXEC_CMD			0x10
+#define NAND_FLASH_STATUS		0x14
+#define NAND_BUFFER_STATUS		0x18
+#define NAND_DEV0_CFG0			0x20
+#define NAND_DEV0_CFG1			0x24
+#define NAND_DEV0_ECC_CFG		0x28
+#define NAND_DEV1_ECC_CFG		0x2c
+#define NAND_DEV1_CFG0			0x30
+#define NAND_DEV1_CFG1			0x34
+#define NAND_READ_ID			0x40
+#define NAND_READ_STATUS		0x44
+#define NAND_DEV_CMD0			0xa0
+#define NAND_DEV_CMD1			0xa4
+#define NAND_DEV_CMD2			0xa8
+#define NAND_DEV_CMD_VLD		0xac
+#define SFLASHC_BURST_CFG		0xe0
+#define NAND_ERASED_CW_DETECT_CFG	0xe8
+#define NAND_ERASED_CW_DETECT_STATUS	0xec
+#define NAND_EBI2_ECC_BUF_CFG		0xf0
+#define FLASH_BUF_ACC			0x100
+
+#define NAND_CTRL			0xf00
+#define NAND_VERSION			0xf08
+#define NAND_READ_LOCATION_0		0xf20
+#define NAND_READ_LOCATION_1		0xf24
+
+/* dummy register offsets, used by write_reg_dma */
+#define NAND_DEV_CMD1_RESTORE		0xdead
+#define NAND_DEV_CMD_VLD_RESTORE	0xbeef
+
+/* NAND_FLASH_CMD bits */
+#define PAGE_ACC			BIT(4)
+#define LAST_PAGE			BIT(5)
+
+/* NAND_FLASH_CHIP_SELECT bits */
+#define NAND_DEV_SEL			0
+#define DM_EN				BIT(2)
+
+/* NAND_FLASH_STATUS bits */
+#define FS_OP_ERR			BIT(4)
+#define FS_READY_BSY_N			BIT(5)
+#define FS_MPU_ERR			BIT(8)
+#define FS_DEVICE_STS_ERR		BIT(16)
+#define FS_DEVICE_WP			BIT(23)
+
+/* NAND_BUFFER_STATUS bits */
+#define BS_UNCORRECTABLE_BIT		BIT(8)
+#define BS_CORRECTABLE_ERR_MSK		0x1f
+
+/* NAND_DEVn_CFG0 bits */
+#define DISABLE_STATUS_AFTER_WRITE	4
+#define CW_PER_PAGE			6
+#define UD_SIZE_BYTES			9
+#define ECC_PARITY_SIZE_BYTES_RS	19
+#define SPARE_SIZE_BYTES		23
+#define NUM_ADDR_CYCLES			27
+#define STATUS_BFR_READ			30
+#define SET_RD_MODE_AFTER_STATUS	31
+
+/* NAND_DEVn_CFG0 bits */
+#define DEV0_CFG1_ECC_DISABLE		0
+#define WIDE_FLASH			1
+#define NAND_RECOVERY_CYCLES		2
+#define CS_ACTIVE_BSY			5
+#define BAD_BLOCK_BYTE_NUM		6
+#define BAD_BLOCK_IN_SPARE_AREA		16
+#define WR_RD_BSY_GAP			17
+#define ENABLE_BCH_ECC			27
+
+/* NAND_DEV0_ECC_CFG bits */
+#define ECC_CFG_ECC_DISABLE		0
+#define ECC_SW_RESET			1
+#define ECC_MODE			4
+#define ECC_PARITY_SIZE_BYTES_BCH	8
+#define ECC_NUM_DATA_BYTES		16
+#define ECC_FORCE_CLK_OPEN		30
+
+/* NAND_DEV_CMD1 bits */
+#define READ_ADDR			0
+
+/* NAND_DEV_CMD_VLD bits */
+#define READ_START_VLD			0
+
+/* NAND_EBI2_ECC_BUF_CFG bits */
+#define NUM_STEPS			0
+
+/* NAND_ERASED_CW_DETECT_CFG bits */
+#define ERASED_CW_ECC_MASK		1
+#define AUTO_DETECT_RES			0
+#define MASK_ECC			(1 << ERASED_CW_ECC_MASK)
+#define RESET_ERASED_DET		(1 << AUTO_DETECT_RES)
+#define ACTIVE_ERASED_DET		(0 << AUTO_DETECT_RES)
+#define CLR_ERASED_PAGE_DET		(RESET_ERASED_DET | MASK_ECC)
+#define SET_ERASED_PAGE_DET		(ACTIVE_ERASED_DET | MASK_ECC)
+
+/* NAND_ERASED_CW_DETECT_STATUS bits */
+#define PAGE_ALL_ERASED			BIT(7)
+#define CODEWORD_ALL_ERASED		BIT(6)
+#define PAGE_ERASED			BIT(5)
+#define CODEWORD_ERASED			BIT(4)
+#define ERASED_PAGE			(PAGE_ALL_ERASED | PAGE_ERASED)
+#define ERASED_CW			(CODEWORD_ALL_ERASED | CODEWORD_ERASED)
+
+/* Version Mask */
+#define NAND_VERSION_MAJOR_MASK		0xf0000000
+#define NAND_VERSION_MAJOR_SHIFT	28
+#define NAND_VERSION_MINOR_MASK		0x0fff0000
+#define NAND_VERSION_MINOR_SHIFT	16
+
+/* NAND OP_CMDs */
+#define PAGE_READ			0x2
+#define PAGE_READ_WITH_ECC		0x3
+#define PAGE_READ_WITH_ECC_SPARE	0x4
+#define PROGRAM_PAGE			0x6
+#define PAGE_PROGRAM_WITH_ECC		0x7

drivers/mtd/nand/qcom_nandc.c new file +80 -0

+
+struct desc_info {
+	struct list_head node;
+
+	enum dma_data_direction dir;
+	struct scatterlist sgl;
+	struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a contiguous
+ * chunk of memory which we use to write the controller registers through DMA.
+ */
+struct nandc_regs {
+	__le32 cmd;
+	__le32 addr0;
+	__le32 addr1;
+	__le32 chip_sel;
+	__le32 exec;
+
+	__le32 cfg0;
+	__le32 cfg1;
+	__le32 ecc_bch_cfg;
+
+	__le32 clrflashstatus;
+	__le32 clrreadstatus;
+
+	__le32 cmd1;
+	__le32 vld;
+
+	__le32 orig_cmd1;
+	__le32 orig_vld;
+
+	__le32 ecc_buf_cfg;
+};
+
+/*
+ * NAND controller data struct
+ *
+ * @controller:			base controller structure
+ * @host_list:			list containing all the chips attached to the
+ *				controller
+ * @dev:			parent device
+ * @res:			resource pointer for platform device (used to
+ *				retrieve the physical addresses of registers
+ *				for DMA)
+ * @base:			MMIO base
+ * @core_clk:			controller clock
+ * @aon_clk:			another controller clock
+ *
+ * @chan:			dma channel
+ * @cmd_crci:			ADM DMA CRCI for command flow control
+ * @data_crci:			ADM DMA CRCI for data flow control
+ * @desc_list:			DMA descriptor list (list of desc_infos)
+ *
+ * @data_buffer:		our local DMA buffer for page read/writes,
+ *				used when we can't use the buffer provided
+ *				by upper layers directly
+ * @buf_size/count/start:	markers for chip->read_buf/write_buf functions
+ * @reg_read_buf:		local buffer for reading back registers via DMA
+ * @reg_read_pos:		marker for data read in reg_read_buf
+ *
+ * @regs:			a contiguous chunk of memory for DMA register
+ *				writes. contains the register values to be
+ *				written to controller
+ * @cmd1/vld:			some fixed controller register values
+ * @ecc_modes:			supported ECC modes by the current controller,
+ *				initialized via DT match data
+ */
+struct qcom_nand_controller {
+	struct nand_hw_control controller;
+	struct list_head host_list;
+
+	struct device *dev;
+
+	struct resource *res;
+	void __iomem *base;
+
+	struct clk *core_clk;
+	struct clk *aon_clk;

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+
+	__le32 *reg_read_buf;
+	int reg_read_pos;
+
+	struct nandc_regs *regs;
+
+	u32 cmd1, vld;
+	u32 ecc_modes;
+};
+

drivers/mtd/nand/qcom_nandc.c new file +59 -0

+
+/*
+ * when using RS ECC, the NAND controller flags an error when reading an
+ * erased page. however, there are special characters at certain offsets when
+ * we read the erased page. we check here if the page is really empty. if so,
+ * we replace the magic characters with 0xffs
+ */
+static bool empty_page_fixup(struct qcom_nand_host *host, u8 *data_buf)
+{
+	struct qcom_nand_controller *nandc = host->nandc;
+	struct nand_chip *chip = &host->chip;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int cwperpage = chip->ecc.steps;
+	u8 orig1[MAX_NUM_STEPS], orig2[MAX_NUM_STEPS];
+	int i, j;
+
+	/* if BCH is enabled, HW will take care of detecting erased pages */
+	if (host->bch_enabled || !host->use_ecc)
+		return false;
+
+	for (i = 0; i < cwperpage; i++) {
+		u8 *empty1, *empty2;
+		u32 flash_status = le32_to_cpu(nandc->reg_read_buf[3 * i]);
+
+		/*
+		 * an erased page flags an error in NAND_FLASH_STATUS, check if
+		 * the page is erased by looking for 0x54s at offsets 3 and 175
+		 * from the beginning of each codeword
+		 */
+		if (!(flash_status & FS_OP_ERR))
+			break;
+
+		empty1 = &data_buf[3 + i * host->cw_data];
+		empty2 = &data_buf[175 + i * host->cw_data];
+
+		/*
+		 * if the error wasn't because of an erased page, bail out and
+		 * and let someone else do the error checking
+		 */
+		if ((*empty1 == 0x54 && *empty2 == 0xff) ||
+				(*empty1 == 0xff && *empty2 == 0x54)) {
+			orig1[i] = *empty1;
+			orig2[i] = *empty2;
+
+			*empty1 = 0xff;
+			*empty2 = 0xff;
+		} else {
+			break;
+		}
+	}
+
+	if (i < cwperpage || memchr_inv(data_buf, 0xff, mtd->writesize))
+		goto not_empty;
+
+	/*
+	 * tell the caller that the page was empty and is fixed up, so that
+	 * parse_read_errors() doesn't think it's an error
+	 */
+	return true;

drivers/mtd/nand/qcom_nandc.c new file +67 -0

+
+static int qcom_nand_host_setup(struct qcom_nand_host *host)
+{
+	struct qcom_nand_controller *nandc = host->nandc;
+	struct nand_chip *chip = &host->chip;
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int cwperpage, spare_bytes, bbm_size, bad_block_byte;
+	bool wide_bus;
+	int ecc_mode = 1;
+
+	/*
+	 * the controller requires each step consists of 512 bytes of data.
+	 * bail out if DT has populated a wrong step size.
+	 */
+	if (ecc->size != NANDC_STEP_SIZE) {
+		dev_err(nandc->dev, "invalid ecc size\n");
+		return -EINVAL;
+	}
+
+	wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
+
+	if (ecc->strength >= 8) {
+		/* 8 bit ECC defaults to BCH ECC on all platforms */
+		host->bch_enabled = true;
+		ecc_mode = 1;
+
+		if (wide_bus) {
+			host->ecc_bytes_hw = 14;
+			spare_bytes = 0;
+			bbm_size = 2;
+		} else {
+			host->ecc_bytes_hw = 13;
+			spare_bytes = 2;
+			bbm_size = 1;
+		}
+	} else {
+		/*
+		 * if the controller supports BCH for 4 bit ECC, the controller
+		 * uses lesser bytes for ECC. If RS is used, the ECC bytes is
+		 * always 10 bytes
+		 */
+		if (nandc->ecc_modes & ECC_BCH_4BIT) {
+			/* BCH */
+			host->bch_enabled = true;
+			ecc_mode = 0;
+
+			if (wide_bus) {
+				host->ecc_bytes_hw = 8;
+				spare_bytes = 2;
+				bbm_size = 2;
+			} else {
+				host->ecc_bytes_hw = 7;
+				spare_bytes = 4;
+				bbm_size = 1;
+			}
+		} else {
+			/* RS */
+			host->ecc_bytes_hw = 10;
+
+			if (wide_bus) {
+				spare_bytes = 0;
+				bbm_size = 2;
+			} else {
+				spare_bytes = 1;
+				bbm_size = 1;
+			}

drivers/mtd/nand/qcom_nandc.c new file +79 -0

+
+	ecc->read_page		= qcom_nandc_read_page;
+	ecc->read_oob		= qcom_nandc_read_oob;
+	ecc->write_page		= qcom_nandc_write_page;
+	ecc->write_oob		= qcom_nandc_write_oob;
+
+	ecc->mode = NAND_ECC_HW;
+
+	ecc->layout = qcom_nand_create_layout(host);
+	if (!ecc->layout)
+		return -ENOMEM;
+
+	cwperpage = mtd->writesize / ecc->size;
+
+	/*
+	 * DATA_UD_BYTES varies based on whether the read/write command protects
+	 * spare data with ECC too. We protect spare data by default, so we set
+	 * it to main + spare data, which are 512 and 4 bytes respectively.
+	 */
+	host->cw_data = 516;
+
+	/*
+	 * total bytes in a step, either 528 bytes for 4 bit ECC, or 532 bytes
+	 * for 8 bit ECC
+	 */
+	host->cw_size = host->cw_data + ecc->bytes;
+
+	bad_block_byte = mtd->writesize - host->cw_size * (cwperpage - 1) + 1;
+
+	host->cfg0 = (cwperpage - 1) << CW_PER_PAGE
+				| host->cw_data << UD_SIZE_BYTES
+				| 0 << DISABLE_STATUS_AFTER_WRITE
+				| 5 << NUM_ADDR_CYCLES
+				| host->ecc_bytes_hw << ECC_PARITY_SIZE_BYTES_RS
+				| 0 << STATUS_BFR_READ
+				| 1 << SET_RD_MODE_AFTER_STATUS
+				| spare_bytes << SPARE_SIZE_BYTES;
+
+	host->cfg1 = 7 << NAND_RECOVERY_CYCLES
+				| 0 <<  CS_ACTIVE_BSY
+				| bad_block_byte << BAD_BLOCK_BYTE_NUM
+				| 0 << BAD_BLOCK_IN_SPARE_AREA
+				| 2 << WR_RD_BSY_GAP
+				| wide_bus << WIDE_FLASH
+				| host->bch_enabled << ENABLE_BCH_ECC;
+
+	host->cfg0_raw = (cwperpage - 1) << CW_PER_PAGE
+				| host->cw_size << UD_SIZE_BYTES
+				| 5 << NUM_ADDR_CYCLES
+				| 0 << SPARE_SIZE_BYTES;
+
+	host->cfg1_raw = 7 << NAND_RECOVERY_CYCLES
+				| 0 << CS_ACTIVE_BSY
+				| 17 << BAD_BLOCK_BYTE_NUM
+				| 1 << BAD_BLOCK_IN_SPARE_AREA
+				| 2 << WR_RD_BSY_GAP
+				| wide_bus << WIDE_FLASH
+				| 1 << DEV0_CFG1_ECC_DISABLE;
+
+	host->ecc_bch_cfg = host->bch_enabled << ECC_CFG_ECC_DISABLE
+				| 0 << ECC_SW_RESET
+				| host->cw_data << ECC_NUM_DATA_BYTES
+				| 1 << ECC_FORCE_CLK_OPEN
+				| ecc_mode << ECC_MODE
+				| host->ecc_bytes_hw << ECC_PARITY_SIZE_BYTES_BCH;
+
+	host->ecc_buf_cfg = 0x203 << NUM_STEPS;
+
+	host->clrflashstatus = FS_READY_BSY_N;
+	host->clrreadstatus = 0xc0;
+
+	dev_dbg(nandc->dev,
+		"cfg0 %x cfg1 %x ecc_buf_cfg %x ecc_bch cfg %x cw_size %d cw_data %d strength %d parity_bytes %d steps %d\n",
+		host->cfg0, host->cfg1, host->ecc_buf_cfg, host->ecc_bch_cfg,
+		host->cw_size, host->cw_data, ecc->strength, ecc->bytes,
+		cwperpage);
+
+	return 0;
+}

drivers/mtd/nand/qcom_nandc.c new file +77 -0

+
+static int qcom_nandc_probe(struct platform_device *pdev)
+{
+	struct qcom_nand_controller *nandc;
+	const void *dev_data;
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node, *child;
+	int ret;
+
+	nandc = devm_kzalloc(&pdev->dev, sizeof(*nandc), GFP_KERNEL);
+	if (!nandc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, nandc);
+	nandc->dev = dev;
+
+	dev_data = of_device_get_match_data(dev);
+	if (!dev_data) {
+		dev_err(&pdev->dev, "failed to get device data\n");
+		return -ENODEV;
+	}
+
+	nandc->ecc_modes = (unsigned long) dev_data;
+
+	nandc->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	nandc->base = devm_ioremap_resource(dev, nandc->res);
+	if (IS_ERR(nandc->base))
+		return PTR_ERR(nandc->base);
+
+	nandc->core_clk = devm_clk_get(dev, "core");
+	if (IS_ERR(nandc->core_clk))
+		return PTR_ERR(nandc->core_clk);
+
+	nandc->aon_clk = devm_clk_get(dev, "aon");
+	if (IS_ERR(nandc->aon_clk))
+		return PTR_ERR(nandc->aon_clk);
+
+	ret = qcom_nandc_parse_dt(pdev);
+	if (ret)
+		return ret;
+
+	ret = qcom_nandc_alloc(nandc);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(nandc->core_clk);
+	if (ret)
+		goto err_core_clk;
+
+	ret = clk_prepare_enable(nandc->aon_clk);
+	if (ret)
+		goto err_aon_clk;
+
+	ret = qcom_nandc_setup(nandc);
+	if (ret)
+		goto err_setup;
+
+	for_each_available_child_of_node(dn, child) {
+		if (of_device_is_compatible(child, "qcom,nandcs")) {
+			struct qcom_nand_host *host;
+
+			host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+			if (!host) {
+				of_node_put(child);
+				ret = -ENOMEM;
+				goto err_setup;
+			}
+
+			host->nandc = nandc;
+			ret = qcom_nand_host_init(host, child);
+			if (ret) {
+				devm_kfree(dev, host);
+				continue;
+			}
+
+			list_add_tail(&host->node, &nandc->host_list);
+		}

drivers/mtd/nand/qcom_nandc.c new file +42 -0

+	clk_disable_unprepare(nandc->aon_clk);
+err_aon_clk:
+	clk_disable_unprepare(nandc->core_clk);
+err_core_clk:
+	qcom_nandc_unalloc(nandc);
+
+	return ret;
+}
+
+static int qcom_nandc_remove(struct platform_device *pdev)
+{
+	struct qcom_nand_controller *nandc = platform_get_drvdata(pdev);
+	struct qcom_nand_host *host;
+
+	list_for_each_entry(host, &nandc->host_list, node)
+		nand_release(nand_to_mtd(&host->chip));
+
+	qcom_nandc_unalloc(nandc);
+
+	clk_disable_unprepare(nandc->aon_clk);
+	clk_disable_unprepare(nandc->core_clk);
+
+	return 0;
+}
+
+#define EBI2_NANDC_ECC_MODES	(ECC_RS_4BIT | ECC_BCH_8BIT)
+
+/*
+ * data will hold a struct pointer containing more differences once we support
+ * more controller variants
+ */
+static const struct of_device_id qcom_nandc_of_match[] = {
+	{	.compatible = "qcom,ebi2-nandc",
+		.data = (void *) EBI2_NANDC_ECC_MODES,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_nandc_of_match);
+
+static struct platform_driver qcom_nandc_driver = {
+	.driver = {
+		.name = "qcom-nandc",

Hi Archit,

Quoted 263 lines · On Tue, 5 Jan 2016 10:55:00 +0530 Archit Taneja <[email protected]> wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx, MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2) and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a broader interface for external slow peripheral devices such as LCD and NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs of our interest, we only use the NAND controller within EBI2. Therefore, it's safe for us to assume that the NAND controller is a standalone block within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main and spare data. The controller contains an internal 512 byte page buffer to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA for register read/write and data transfers. The controller performs page reads and writes at a codeword/step level of 512 bytes. It can support up to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for each codeword, followed by data descriptors to read or write data from the controller's internal buffer. It uses a single ADM DMA channel that we get via dmaengine API. The controller requires 2 ADM CRCIs for command and data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use the standard syndrome ecc ops because of several reasons. First, the amount of data bytes covered by ecc isn't same in each step. Second, writing to free oob space requires us writing to the entire step in which the oob lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker. The controller ignores reading the marker when ECC is enabled. ECC needs to be explicity disabled to read or write to the bad block marker. The nand_bbt helpers library hence can't access BBMs for the controller. For now, we skip the creation of BBT and populate chip->block_bad and chip->block_markbad helpers instead.

Reviewed-by Andy Gross <[email protected]>

Reviewed-by Stephen Boyd <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
v5:
- split chip/controller structs
- simplify layout by considering reserved bytes as part of ECC
- create ecc layouts automatically
- implement block_bad and block_markbad chip ops instead of
- read_oob_raw/write_oob_raw ecc ops to access BBMs.
- Add NAND_SKIP_BBTSCAN flag until we get badblockbits support.
- misc clean ups

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes
- Add Stephen's Signed-off since he provided a patch to fix endianness problems

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

  drivers/mtd/nand/Kconfig      |    7 +
  drivers/mtd/nand/Makefile     |    1 +
  drivers/mtd/nand/qcom_nandc.c | 1981 +++++++++++++++++++++++++++++++++++++++++
  3 files changed, 1989 insertions(+)

create mode 100644 drivers/mtd/nand/qcom_nandc.c

drivers/mtd/nand/Kconfig +7 -0

index 95b8d2b..2fccdfb 100644
@@ -546,4 +546,11 @@ config MTD_NAND_HISI504
  	help
  	  Enables support for NAND controller on Hisilicon SoC Hip04.
 
+config MTD_NAND_QCOM
+	tristate "Support for NAND on QCOM SoCs"
+	depends on ARCH_QCOM
+	help
+	  Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+	  controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND

drivers/mtd/nand/Makefile +1 -0

index 2c7f014..9450cdc 100644
@@ -55,5 +55,6 @@ 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_BRCMNAND)		+= brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
 
  nand-objs := nand_base.o nand_bbt.o nand_timings.o

drivers/mtd/nand/qcom_nandc.c new file +146 -0

new file mode 100644
index 0000000..a4dd922
@@ -0,0 +1,1981 @@
+/*
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/clk.h>
+#include <linux/slab.h>
+#include <linux/bitops.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_mtd.h>
+#include <linux/delay.h>
+
+/* NANDc reg offsets */
+#define NAND_FLASH_CMD			0x00
+#define NAND_ADDR0			0x04
+#define NAND_ADDR1			0x08
+#define NAND_FLASH_CHIP_SELECT		0x0c
+#define NAND_EXEC_CMD			0x10
+#define NAND_FLASH_STATUS		0x14
+#define NAND_BUFFER_STATUS		0x18
+#define NAND_DEV0_CFG0			0x20
+#define NAND_DEV0_CFG1			0x24
+#define NAND_DEV0_ECC_CFG		0x28
+#define NAND_DEV1_ECC_CFG		0x2c
+#define NAND_DEV1_CFG0			0x30
+#define NAND_DEV1_CFG1			0x34
+#define NAND_READ_ID			0x40
+#define NAND_READ_STATUS		0x44
+#define NAND_DEV_CMD0			0xa0
+#define NAND_DEV_CMD1			0xa4
+#define NAND_DEV_CMD2			0xa8
+#define NAND_DEV_CMD_VLD		0xac
+#define SFLASHC_BURST_CFG		0xe0
+#define NAND_ERASED_CW_DETECT_CFG	0xe8
+#define NAND_ERASED_CW_DETECT_STATUS	0xec
+#define NAND_EBI2_ECC_BUF_CFG		0xf0
+#define FLASH_BUF_ACC			0x100
+
+#define NAND_CTRL			0xf00
+#define NAND_VERSION			0xf08
+#define NAND_READ_LOCATION_0		0xf20
+#define NAND_READ_LOCATION_1		0xf24
+
+/* dummy register offsets, used by write_reg_dma */
+#define NAND_DEV_CMD1_RESTORE		0xdead
+#define NAND_DEV_CMD_VLD_RESTORE	0xbeef
+
+/* NAND_FLASH_CMD bits */
+#define PAGE_ACC			BIT(4)
+#define LAST_PAGE			BIT(5)
+
+/* NAND_FLASH_CHIP_SELECT bits */
+#define NAND_DEV_SEL			0
+#define DM_EN				BIT(2)
+
+/* NAND_FLASH_STATUS bits */
+#define FS_OP_ERR			BIT(4)
+#define FS_READY_BSY_N			BIT(5)
+#define FS_MPU_ERR			BIT(8)
+#define FS_DEVICE_STS_ERR		BIT(16)
+#define FS_DEVICE_WP			BIT(23)
+
+/* NAND_BUFFER_STATUS bits */
+#define BS_UNCORRECTABLE_BIT		BIT(8)
+#define BS_CORRECTABLE_ERR_MSK		0x1f
+
+/* NAND_DEVn_CFG0 bits */
+#define DISABLE_STATUS_AFTER_WRITE	4
+#define CW_PER_PAGE			6
+#define UD_SIZE_BYTES			9
+#define ECC_PARITY_SIZE_BYTES_RS	19
+#define SPARE_SIZE_BYTES		23
+#define NUM_ADDR_CYCLES			27
+#define STATUS_BFR_READ			30
+#define SET_RD_MODE_AFTER_STATUS	31
+
+/* NAND_DEVn_CFG0 bits */
+#define DEV0_CFG1_ECC_DISABLE		0
+#define WIDE_FLASH			1
+#define NAND_RECOVERY_CYCLES		2
+#define CS_ACTIVE_BSY			5
+#define BAD_BLOCK_BYTE_NUM		6
+#define BAD_BLOCK_IN_SPARE_AREA		16
+#define WR_RD_BSY_GAP			17
+#define ENABLE_BCH_ECC			27
+
+/* NAND_DEV0_ECC_CFG bits */
+#define ECC_CFG_ECC_DISABLE		0
+#define ECC_SW_RESET			1
+#define ECC_MODE			4
+#define ECC_PARITY_SIZE_BYTES_BCH	8
+#define ECC_NUM_DATA_BYTES		16
+#define ECC_FORCE_CLK_OPEN		30
+
+/* NAND_DEV_CMD1 bits */
+#define READ_ADDR			0
+
+/* NAND_DEV_CMD_VLD bits */
+#define READ_START_VLD			0
+
+/* NAND_EBI2_ECC_BUF_CFG bits */
+#define NUM_STEPS			0
+
+/* NAND_ERASED_CW_DETECT_CFG bits */
+#define ERASED_CW_ECC_MASK		1
+#define AUTO_DETECT_RES			0
+#define MASK_ECC			(1 << ERASED_CW_ECC_MASK)
+#define RESET_ERASED_DET		(1 << AUTO_DETECT_RES)
+#define ACTIVE_ERASED_DET		(0 << AUTO_DETECT_RES)
+#define CLR_ERASED_PAGE_DET		(RESET_ERASED_DET | MASK_ECC)
+#define SET_ERASED_PAGE_DET		(ACTIVE_ERASED_DET | MASK_ECC)
+
+/* NAND_ERASED_CW_DETECT_STATUS bits */
+#define PAGE_ALL_ERASED			BIT(7)
+#define CODEWORD_ALL_ERASED		BIT(6)
+#define PAGE_ERASED			BIT(5)
+#define CODEWORD_ERASED			BIT(4)
+#define ERASED_PAGE			(PAGE_ALL_ERASED | PAGE_ERASED)
+#define ERASED_CW			(CODEWORD_ALL_ERASED | CODEWORD_ERASED)
+
+/* Version Mask */
+#define NAND_VERSION_MAJOR_MASK		0xf0000000
+#define NAND_VERSION_MAJOR_SHIFT	28
+#define NAND_VERSION_MINOR_MASK		0x0fff0000
+#define NAND_VERSION_MINOR_SHIFT	16
+
+/* NAND OP_CMDs */
+#define PAGE_READ			0x2
+#define PAGE_READ_WITH_ECC		0x3
+#define PAGE_READ_WITH_ECC_SPARE	0x4
+#define PROGRAM_PAGE			0x6
+#define PAGE_PROGRAM_WITH_ECC		0x7

drivers/mtd/nand/qcom_nandc.c line 156

Quoted 10 lines

drivers/mtd/nand/qcom_nandc.c new file +10 -0

+#define PROGRAM_PAGE_SPARE		0x9
+#define BLOCK_ERASE			0xa
+#define FETCH_ID			0xb
+#define RESET_DEVICE			0xd
+
+/*
+ * the NAND controller performs reads/writes with ECC in 516 byte chunks.
+ * the driver calls the chunks 'step' or 'codeword' interchangeably
+ */
+#define NANDC_STEP_SIZE			512

Sometimes you're using spaces...

drivers/mtd/nand/qcom_nandc.c line 162

Quoted 6 lines

drivers/mtd/nand/qcom_nandc.c new file +6 -0

+
+/*
+ * the largest page size we support is 8K, this will have 16 steps/codewords
+ * of 512 bytes each
+ */
+#define	MAX_NUM_STEPS			(SZ_8K / NANDC_STEP_SIZE)

... and other times tabs between #define and the MACRO name. I'd suggest choosing one solution and sticking to it.

drivers/mtd/nand/qcom_nandc.c line 171

Quoted 9 lines

drivers/mtd/nand/qcom_nandc.c new file +9 -0

+
+/* we read at most 3 registers per codeword scan */
+#define MAX_REG_RD			(3 * MAX_NUM_STEPS)
+
+/* ECC modes supported by the controller */
+#define ECC_NONE	BIT(0)
+#define ECC_RS_4BIT	BIT(1)
+#define	ECC_BCH_4BIT	BIT(2)
+#define	ECC_BCH_8BIT	BIT(3)

Ditto.

Quoted 84 lines

drivers/mtd/nand/qcom_nandc.c new file +84 -0

+
+struct desc_info {
+	struct list_head node;
+
+	enum dma_data_direction dir;
+	struct scatterlist sgl;
+	struct dma_async_tx_descriptor *dma_desc;
+};
+
+/*
+ * holds the current register values that we want to write. acts as a contiguous
+ * chunk of memory which we use to write the controller registers through DMA.
+ */
+struct nandc_regs {
+	__le32 cmd;
+	__le32 addr0;
+	__le32 addr1;
+	__le32 chip_sel;
+	__le32 exec;
+
+	__le32 cfg0;
+	__le32 cfg1;
+	__le32 ecc_bch_cfg;
+
+	__le32 clrflashstatus;
+	__le32 clrreadstatus;
+
+	__le32 cmd1;
+	__le32 vld;
+
+	__le32 orig_cmd1;
+	__le32 orig_vld;
+
+	__le32 ecc_buf_cfg;
+};
+
+/*
+ * NAND controller data struct
+ *
+ * @controller:			base controller structure
+ * @host_list:			list containing all the chips attached to the
+ *				controller
+ * @dev:			parent device
+ * @res:			resource pointer for platform device (used to
+ *				retrieve the physical addresses of registers
+ *				for DMA)
+ * @base:			MMIO base
+ * @core_clk:			controller clock
+ * @aon_clk:			another controller clock
+ *
+ * @chan:			dma channel
+ * @cmd_crci:			ADM DMA CRCI for command flow control
+ * @data_crci:			ADM DMA CRCI for data flow control
+ * @desc_list:			DMA descriptor list (list of desc_infos)
+ *
+ * @data_buffer:		our local DMA buffer for page read/writes,
+ *				used when we can't use the buffer provided
+ *				by upper layers directly
+ * @buf_size/count/start:	markers for chip->read_buf/write_buf functions
+ * @reg_read_buf:		local buffer for reading back registers via DMA
+ * @reg_read_pos:		marker for data read in reg_read_buf
+ *
+ * @regs:			a contiguous chunk of memory for DMA register
+ *				writes. contains the register values to be
+ *				written to controller
+ * @cmd1/vld:			some fixed controller register values
+ * @ecc_modes:			supported ECC modes by the current controller,
+ *				initialized via DT match data
+ */
+struct qcom_nand_controller {
+	struct nand_hw_control controller;
+	struct list_head host_list;
+
+	struct device *dev;
+
+	struct resource *res;
+	void __iomem *base;
+
+	struct clk *core_clk;
+	struct clk *aon_clk;
+
+	struct dma_chan *chan;
+	unsigned int cmd_crci;
+	unsigned int data_crci;