[PATCH v5 0/3] Mediatek SPI-NOR flash driver

[PATCH v5 0/3] Mediatek SPI-NOR flash driver

[Bayi Cheng]

This series is based on v4.3-rc1 and l2-mtd.git
(git://git.infradead.org/l2-mtd.git)

Change in v5:
1: add "status = "disable"" to device tree
2: add document "flash" node
3: fix some statement error in Kconfig file
4: fix alphabetical order error in makefile
5: delete the parament "mtd_info *mtd" in mt8173_nor structure
6: delete SPINOR_OP_WREN repeated calls
7: add mt8173_nor_do_tx & mt8173_nor_do_rx for them full potential
8: use a subnode to represent the flash

Change in v4:
1: delete the parament "write_enable" for mt8173_nor_write_reg
2: fix the build warning for calling mt8173_nor_write_single_byte

Change in v3:
1: use switch() to replace some if-else statement
2: use shifts to replace endianness statement
3: delete some unused macros
4: use auto-increment mechanism for single write
5: write address added to 32bytes

Change in v2:
1. Rebase to 4.3-rc1
2. propagate error code
3. delete mux clock and axi clock in dts file
4. descripts more exactly for binding file
5. change file names from mtk-nor.c to mtk_quadspi.c
6. delete some functions witch were used once time

Bayi Cheng (3):
  doc: dt: add documentation for Mediatek spi-nor controller
  mtd: mtk-nor: mtk serial flash controller driver
  arm64: dts: mt8173: Add nor flash node

 .../devicetree/bindings/mtd/mtk-quadspi.txt        |  41 ++
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  16 +
 drivers/mtd/spi-nor/Kconfig                        |   7 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/mtk-quadspi.c                  | 491 +++++++++++++++++++++
 5 files changed, 556 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
 create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c

--
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[Bayi Cheng]

Add device tree binding documentation for serial flash with
Mediatek serial flash controller

Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
---
 .../devicetree/bindings/mtd/mtk-quadspi.txt        | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
new file mode 100644
index 0000000..866b492
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
@@ -0,0 +1,41 @@
+* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
+
+Required properties:
+- compatible: 	  should be "mediatek,mt8173-nor";
+- reg: 		  physical base address and length of the controller's register
+- clocks: 	  the phandle of the clock needed by the nor controller
+- clock-names: 	  the name of the clocks
+		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
+		  and "sf" is used for controller, these are the clocks witch
+		  hardware needs to enabling nor flash and nor flash controller.
+		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+- #address-cells: should be <1>
+- #size-cells:	  should be <0>
+
+The SPI Flash must be a child of the nor_flash node and must have a
+compatible property.
+
+Required properties:
+- compatible:	  May include a device-specific string consisting of the manufacturer
+		  and name of the chip. Must also include "jedec,spi-nor" for any
+		  SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F).
+- reg :		  Chip-Select number
+
+Example:
+
+nor_flash: spi@1100d000 {
+	compatible = "mediatek,mt8173-nor";
+	reg = <0 0x1100d000 0 0xe0>;
+	clocks = <&pericfg CLK_PERI_SPI>,
+		 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+	clock-names = "spi", "sf";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "disabled";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+	};
+};
+
-- 
1.8.1.1.dirty

[PATCH v5 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Bayi Cheng]

add spi nor flash driver for mediatek controller

Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/mtd/spi-nor/Kconfig       |   7 +
 drivers/mtd/spi-nor/Makefile      |   1 +
 drivers/mtd/spi-nor/mtk-quadspi.c | 491 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 499 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 89bf4c1..387396d 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -7,6 +7,13 @@ menuconfig MTD_SPI_NOR
 
 if MTD_SPI_NOR
 
+config MTD_MT81xx_NOR
+	tristate "Mediatek MT81xx SPI NOR flash controller"
+	help
+	  This enables access to SPI NOR flash, using MT81xx SPI NOR flash
+	  controller. This controller does not support generic SPI BUS, It only
+	  supports SPI NOR Flash.
+
 config MTD_SPI_NOR_USE_4K_SECTORS
 	bool "Use small 4096 B erase sectors"
 	default y
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index e53333e..0bf3a7f8 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
+obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
new file mode 100644
index 0000000..33a8dc5
--- /dev/null
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -0,0 +1,491 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+
+#define MTK_NOR_CMD_REG			0x00
+#define MTK_NOR_CNT_REG			0x04
+#define MTK_NOR_RDSR_REG		0x08
+#define MTK_NOR_RDATA_REG		0x0c
+#define MTK_NOR_RADR0_REG		0x10
+#define MTK_NOR_RADR1_REG		0x14
+#define MTK_NOR_RADR2_REG		0x18
+#define MTK_NOR_WDATA_REG		0x1c
+#define MTK_NOR_PRGDATA0_REG		0x20
+#define MTK_NOR_PRGDATA1_REG		0x24
+#define MTK_NOR_PRGDATA2_REG		0x28
+#define MTK_NOR_PRGDATA3_REG		0x2c
+#define MTK_NOR_PRGDATA4_REG		0x30
+#define MTK_NOR_PRGDATA5_REG		0x34
+#define MTK_NOR_SHREG0_REG		0x38
+#define MTK_NOR_SHREG1_REG		0x3c
+#define MTK_NOR_SHREG2_REG		0x40
+#define MTK_NOR_SHREG3_REG		0x44
+#define MTK_NOR_SHREG4_REG		0x48
+#define MTK_NOR_SHREG5_REG		0x4c
+#define MTK_NOR_SHREG6_REG		0x50
+#define MTK_NOR_SHREG7_REG		0x54
+#define MTK_NOR_SHREG8_REG		0x58
+#define MTK_NOR_SHREG9_REG		0x5c
+#define MTK_NOR_CFG1_REG		0x60
+#define MTK_NOR_CFG2_REG		0x64
+#define MTK_NOR_CFG3_REG		0x68
+#define MTK_NOR_STATUS0_REG		0x70
+#define MTK_NOR_STATUS1_REG		0x74
+#define MTK_NOR_STATUS2_REG		0x78
+#define MTK_NOR_STATUS3_REG		0x7c
+#define MTK_NOR_FLHCFG_REG		0x84
+#define MTK_NOR_TIME_REG		0x94
+#define MTK_NOR_PP_DATA_REG		0x98
+#define MTK_NOR_PREBUF_STUS_REG		0x9c
+#define MTK_NOR_DELSEL0_REG		0xa0
+#define MTK_NOR_DELSEL1_REG		0xa4
+#define MTK_NOR_INTRSTUS_REG		0xa8
+#define MTK_NOR_INTREN_REG		0xac
+#define MTK_NOR_CHKSUM_CTL_REG		0xb8
+#define MTK_NOR_CHKSUM_REG		0xbc
+#define MTK_NOR_CMD2_REG		0xc0
+#define MTK_NOR_WRPROT_REG		0xc4
+#define MTK_NOR_RADR3_REG		0xc8
+#define MTK_NOR_DUAL_REG		0xcc
+#define MTK_NOR_DELSEL2_REG		0xd0
+#define MTK_NOR_DELSEL3_REG		0xd4
+#define MTK_NOR_DELSEL4_REG		0xd8
+
+/* commands for mtk nor controller */
+#define MTK_NOR_READ_CMD		0x0
+#define MTK_NOR_RDSR_CMD		0x2
+#define MTK_NOR_PRG_CMD			0x4
+#define MTK_NOR_WR_CMD			0x10
+#define MTK_NOR_PIO_WR_CMD		0x90
+#define MTK_NOR_WRSR_CMD		0x20
+#define MTK_NOR_PIO_READ_CMD		0x81
+#define MTK_NOR_WR_BUF_ENABLE		0x1
+#define MTK_NOR_WR_BUF_DISABLE		0x0
+#define MTK_NOR_ENABLE_SF_CMD		0x30
+#define MTK_NOR_DUAD_ADDR_EN		0x8
+#define MTK_NOR_QUAD_READ_EN		0x4
+#define MTK_NOR_DUAL_ADDR_EN		0x2
+#define MTK_NOR_DUAL_READ_EN		0x1
+#define MTK_NOR_DUAL_DISABLE		0x0
+#define MTK_NOR_FAST_READ		0x1
+
+#define SFLASH_WRBUF_SIZE		128
+#define get_nth_byte(d, n)		((d >> (8 * n)) & 0xff)
+
+struct mt8173_nor {
+	struct spi_nor nor;
+	struct device *dev;
+	void __iomem *base;	/* nor flash base address */
+	struct clk *spi_clk;
+	struct clk *nor_clk;
+};
+
+static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
+{
+	struct spi_nor *nor = &mt8173_nor->nor;
+
+	writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG);
+
+	switch (nor->flash_read) {
+	case SPI_NOR_FAST:
+		writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
+		       MTK_NOR_CFG1_REG);
+		break;
+	case SPI_NOR_DUAL:
+		writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	case SPI_NOR_QUAD:
+		writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	default:
+		writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	}
+}
+
+static int mt8173_nor_execute_cmd(struct mt8173_nor *mt8173_nor, u8 cmdval)
+{
+	int reg;
+	u8 val = cmdval & 0x1f;
+
+	writeb(cmdval, mt8173_nor->base + MTK_NOR_CMD_REG);
+	return readl_poll_timeout(mt8173_nor->base + MTK_NOR_CMD_REG, reg,
+				  !(reg & val), 100, 10000);
+}
+
+static int mt8173_nor_do_tx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
+			    int len)
+{
+	int i;
+
+	if (len > 5)
+		return -EINVAL;
+
+	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+
+	for (i = 0; i < len; i++)
+		writeb(buf[len - 1 - i], mt8173_nor->base +
+			       MTK_NOR_PRGDATA0_REG + 4 * (4 - i));
+	writeb((len + 1) * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
+}
+
+/*
+ * this function is used to execute special read commands.
+ * such as SPINOR_OP_RDFSR, SPINOR_OP_RDCR, SPINOR_OP_RD_EVCR and so on.
+ * len is no more than 1.
+ */
+static int mt8173_nor_do_rx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
+			    int len)
+{
+	if (len > 1)
+		return -EINVAL;
+
+	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+
+	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
+}
+
+/* cmd1 sent to nor flash, cmd2 write to nor controller */
+static int mt8173_nor_set_para(struct mt8173_nor *mt8173_nor, int cmd1,
+			       int cmd2)
+{
+	writeb(cmd1, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
+	return mt8173_nor_execute_cmd(mt8173_nor, cmd2);
+}
+
+static int mt8173_nor_write_buffer_enable(struct mt8173_nor *mt8173_nor)
+{
+	u8 reg;
+
+	/* the bit0 of MTK_NOR_CFG2_REG is pre-fetch buffer
+	 * 0: pre-fetch buffer use for read
+	 * 1: pre-fetch buffer use for page program
+	 */
+	writel(MTK_NOR_WR_BUF_ENABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
+	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
+				  0x01 == (reg & 0x01), 100, 10000);
+}
+
+static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
+{
+	u8 reg;
+
+	writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
+	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
+				  MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
+				  10000);
+}
+
+static int mt8173_nor_erase_sector(struct spi_nor *nor, loff_t offset)
+{
+	u8 buf[4], i = 0;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	while (i < 4) {
+		buf[i] = get_nth_byte(offset, i);
+		i++;
+	}
+	if (nor->mtd.size <= 0x1000000)
+		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 3);
+	else
+		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 4);
+}
+
+static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
+			   size_t *retlen, u_char *buffer)
+{
+	int i, ret;
+	int addr = (int)from;
+	u8 *buf = (u8 *)buffer;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	/* set mode for fast read mode ,dual mode or quad mode */
+	mt8173_nor_set_read_mode(mt8173_nor);
+	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
+	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
+	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
+	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+
+	for (i = 0; i < length; i++, (*retlen)++) {
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
+		if (ret < 0)
+			return ret;
+		buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
+	}
+	return 0;
+}
+
+static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
+					int addr, int length, u8 *data)
+{
+	int i, ret;
+
+	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
+	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
+	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
+	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+
+	for (i = 0; i < length; i++) {
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
+		if (ret < 0)
+			return ret;
+		writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
+	}
+	return 0;
+}
+
+static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
+				   const u8 *buf)
+{
+	int i, bufidx, data;
+
+	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
+	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
+	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
+	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+
+	bufidx = 0;
+	for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
+		data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
+		       buf[bufidx + 1]<<8 | buf[bufidx];
+		bufidx += 4;
+		writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
+	}
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
+}
+
+static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
+			     size_t *retlen, const u_char *buf)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	ret = mt8173_nor_write_buffer_enable(mt8173_nor);
+	if (ret < 0)
+		dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
+
+	while (len >= SFLASH_WRBUF_SIZE) {
+		ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
+		if (ret < 0)
+			dev_err(mt8173_nor->dev, "write buffer failed!\n");
+		len -= SFLASH_WRBUF_SIZE;
+		to += SFLASH_WRBUF_SIZE;
+		buf += SFLASH_WRBUF_SIZE;
+		(*retlen) += SFLASH_WRBUF_SIZE;
+	}
+	ret = mt8173_nor_write_buffer_disable(mt8173_nor);
+	if (ret < 0)
+		dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
+
+	if (len) {
+		ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
+						   (u8 *)buf);
+		if (ret < 0)
+			dev_err(mt8173_nor->dev, "write single byte failed!\n");
+		(*retlen) += len;
+	}
+}
+
+static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	/* mtk nor controller doesn't supoort SPINOR_OP_RDCR */
+	switch (opcode) {
+	case SPINOR_OP_RDID:
+		/* read JEDEC ID need 4 bytes commands */
+		memset(buf, 0x0, 4);
+		ret = mt8173_nor_do_tx(mt8173_nor, opcode, buf, 3);
+		if (ret < 0)
+			return ret;
+
+		/* mtk nor flash controller only support 3 bytes IDs */
+		buf[2] = readb(mt8173_nor->base + MTK_NOR_SHREG0_REG);
+		buf[1] = readb(mt8173_nor->base + MTK_NOR_SHREG1_REG);
+		buf[0] = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);
+		break;
+	case SPINOR_OP_RDSR:
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
+		if (ret < 0)
+			return ret;
+		*buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
+		break;
+	default:
+		/* read other register of nor flash */
+		ret = mt8173_nor_do_rx(mt8173_nor, opcode, buf, len);
+		*buf = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);
+		break;
+	}
+	return ret;
+}
+
+static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+				int len)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	switch (opcode) {
+	case SPINOR_OP_WRSR:
+		ret = mt8173_nor_set_para(mt8173_nor, *buf,
+					  MTK_NOR_WRSR_CMD);
+		break;
+	case SPINOR_OP_CHIP_ERASE:
+		ret = mt8173_nor_set_para(mt8173_nor, opcode,
+					  MTK_NOR_PRG_CMD);
+		break;
+	default:
+		ret = mt8173_nor_do_tx(mt8173_nor, opcode, NULL, 0);
+		if (ret)
+			dev_warn(mt8173_nor->dev, "set write enable fail!\n");
+		break;
+	}
+	return ret;
+}
+
+static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
+			       struct mtd_part_parser_data *ppdata)
+{
+	int ret;
+	struct spi_nor *nor;
+
+	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
+
+	nor = &mt8173_nor->nor;
+	nor->dev = mt8173_nor->dev;
+	nor->priv = mt8173_nor;
+	nor->flash_node = ppdata->of_node;
+
+	/* fill the hooks to spi nor */
+	nor->read = mt8173_nor_read;
+	nor->read_reg = mt8173_nor_read_reg;
+	nor->write = mt8173_nor_write;
+	nor->write_reg = mt8173_nor_write_reg;
+	nor->erase = mt8173_nor_erase_sector;
+	nor->mtd.owner = THIS_MODULE;
+	nor->mtd.name = "mtk_nor";
+	/* initialized with NULL */
+	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
+	if (ret)
+		return ret;
+
+	return mtd_device_parse_register(&nor->mtd, NULL, ppdata, NULL, 0);
+}
+
+static int mtk_nor_drv_probe(struct platform_device *pdev)
+{
+	struct device_node *flash_np;
+	struct mtd_part_parser_data ppdata;
+	struct resource *res;
+	int ret;
+	struct mt8173_nor *mt8173_nor;
+
+	if (!pdev->dev.of_node) {
+		dev_err(&pdev->dev, "No DT found\n");
+		return -EINVAL;
+	}
+
+	mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
+	if (!mt8173_nor)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, mt8173_nor);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mt8173_nor->base))
+		return PTR_ERR(mt8173_nor->base);
+
+	mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
+	if (IS_ERR(mt8173_nor->spi_clk)) {
+		ret = PTR_ERR(mt8173_nor->spi_clk);
+		goto nor_free;
+	}
+
+	mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
+	if (IS_ERR(mt8173_nor->nor_clk)) {
+		ret = PTR_ERR(mt8173_nor->nor_clk);
+		goto nor_free;
+	}
+
+	mt8173_nor->dev = &pdev->dev;
+	clk_prepare_enable(mt8173_nor->spi_clk);
+	clk_prepare_enable(mt8173_nor->nor_clk);
+
+	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
+	if (!flash_np) {
+		dev_err(&pdev->dev, "no SPI flash device to configure\n");
+		ret = -ENODEV;
+		goto nor_free;
+	}
+	ppdata.of_node = flash_np;
+	ret = mtk_nor_init(mt8173_nor, &ppdata);
+
+nor_free:
+	return ret;
+}
+
+static int mtk_nor_drv_remove(struct platform_device *pdev)
+{
+	struct mt8173_nor *mt8173_nor = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(mt8173_nor->spi_clk);
+	clk_disable_unprepare(mt8173_nor->nor_clk);
+	return 0;
+}
+
+static const struct of_device_id mtk_nor_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-nor"},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_nor_of_ids);
+
+static struct platform_driver mtk_nor_driver = {
+	.probe = mtk_nor_drv_probe,
+	.remove = mtk_nor_drv_remove,
+	.driver = {
+		.name = "mtk-nor",
+		.of_match_table = mtk_nor_of_ids,
+	},
+};
+
+module_platform_driver(mtk_nor_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MediaTek SPI NOR Flash Driver");
-- 
1.8.1.1.dirty

[PATCH v5 3/3] arm64: dts: mt8173: Add nor flash node

[Bayi Cheng]

Add Mediatek nor flash node

Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..f5f08eb 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -365,6 +365,22 @@
 			status = "disabled";
 		};
 
+		nor_flash: spi@1100d000 {
+			compatible = "mediatek,mt8173-nor";
+			reg = <0 0x1100d000 0 0xe0>;
+			clocks = <&pericfg CLK_PERI_SPI>,
+				 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+			clock-names = "spi", "sf";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			flash@0 {
+				compatible = "jedec,spi-nor";
+				reg = <0>;
+			};
+		};
+
 		i2c3: i2c3@11010000 {
 			compatible = "mediatek,mt8173-i2c";
 			reg = <0 0x11010000 0 0x70>,
-- 
1.8.1.1.dirty

Re: [PATCH v5 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Brian Norris]

Hi Bayi,

(I drafted most of this before you clarified on how to read out 6 bytes
of ID. So a bit of this is slightly out-of-date. Still, I think most of
the comments and much of the appended patch should be useful to you.)

On Mon, Oct 26, 2015 at 09:40:38PM +0800, Bayi Cheng wrote:
> add spi nor flash driver for mediatek controller
> 
> Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/mtd/spi-nor/Kconfig       |   7 +
>  drivers/mtd/spi-nor/Makefile      |   1 +
>  drivers/mtd/spi-nor/mtk-quadspi.c | 491 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89bf4c1..387396d 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -7,6 +7,13 @@ menuconfig MTD_SPI_NOR
>  
>  if MTD_SPI_NOR
>  
> +config MTD_MT81xx_NOR
> +	tristate "Mediatek MT81xx SPI NOR flash controller"
> +	help
> +	  This enables access to SPI NOR flash, using MT81xx SPI NOR flash
> +	  controller. This controller does not support generic SPI BUS, It only

Nit: s/It/it/

> +	  supports SPI NOR Flash.
> +
>  config MTD_SPI_NOR_USE_4K_SECTORS
>  	bool "Use small 4096 B erase sectors"
>  	default y
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e53333e..0bf3a7f8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
>  obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
> +obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 0000000..33a8dc5
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,491 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/ioport.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>

You're not using GPIOs. You don't need this header.

> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +
> +#define MTK_NOR_CMD_REG			0x00
> +#define MTK_NOR_CNT_REG			0x04
> +#define MTK_NOR_RDSR_REG		0x08
> +#define MTK_NOR_RDATA_REG		0x0c
> +#define MTK_NOR_RADR0_REG		0x10
> +#define MTK_NOR_RADR1_REG		0x14
> +#define MTK_NOR_RADR2_REG		0x18
> +#define MTK_NOR_WDATA_REG		0x1c
> +#define MTK_NOR_PRGDATA0_REG		0x20
> +#define MTK_NOR_PRGDATA1_REG		0x24
> +#define MTK_NOR_PRGDATA2_REG		0x28
> +#define MTK_NOR_PRGDATA3_REG		0x2c
> +#define MTK_NOR_PRGDATA4_REG		0x30
> +#define MTK_NOR_PRGDATA5_REG		0x34
> +#define MTK_NOR_SHREG0_REG		0x38
> +#define MTK_NOR_SHREG1_REG		0x3c
> +#define MTK_NOR_SHREG2_REG		0x40
> +#define MTK_NOR_SHREG3_REG		0x44
> +#define MTK_NOR_SHREG4_REG		0x48
> +#define MTK_NOR_SHREG5_REG		0x4c
> +#define MTK_NOR_SHREG6_REG		0x50
> +#define MTK_NOR_SHREG7_REG		0x54
> +#define MTK_NOR_SHREG8_REG		0x58
> +#define MTK_NOR_SHREG9_REG		0x5c
> +#define MTK_NOR_CFG1_REG		0x60
> +#define MTK_NOR_CFG2_REG		0x64
> +#define MTK_NOR_CFG3_REG		0x68
> +#define MTK_NOR_STATUS0_REG		0x70
> +#define MTK_NOR_STATUS1_REG		0x74
> +#define MTK_NOR_STATUS2_REG		0x78
> +#define MTK_NOR_STATUS3_REG		0x7c
> +#define MTK_NOR_FLHCFG_REG		0x84
> +#define MTK_NOR_TIME_REG		0x94
> +#define MTK_NOR_PP_DATA_REG		0x98
> +#define MTK_NOR_PREBUF_STUS_REG		0x9c
> +#define MTK_NOR_DELSEL0_REG		0xa0
> +#define MTK_NOR_DELSEL1_REG		0xa4
> +#define MTK_NOR_INTRSTUS_REG		0xa8
> +#define MTK_NOR_INTREN_REG		0xac
> +#define MTK_NOR_CHKSUM_CTL_REG		0xb8
> +#define MTK_NOR_CHKSUM_REG		0xbc
> +#define MTK_NOR_CMD2_REG		0xc0
> +#define MTK_NOR_WRPROT_REG		0xc4
> +#define MTK_NOR_RADR3_REG		0xc8
> +#define MTK_NOR_DUAL_REG		0xcc
> +#define MTK_NOR_DELSEL2_REG		0xd0
> +#define MTK_NOR_DELSEL3_REG		0xd4
> +#define MTK_NOR_DELSEL4_REG		0xd8
> +
> +/* commands for mtk nor controller */
> +#define MTK_NOR_READ_CMD		0x0
> +#define MTK_NOR_RDSR_CMD		0x2
> +#define MTK_NOR_PRG_CMD			0x4
> +#define MTK_NOR_WR_CMD			0x10
> +#define MTK_NOR_PIO_WR_CMD		0x90
> +#define MTK_NOR_WRSR_CMD		0x20
> +#define MTK_NOR_PIO_READ_CMD		0x81
> +#define MTK_NOR_WR_BUF_ENABLE		0x1
> +#define MTK_NOR_WR_BUF_DISABLE		0x0
> +#define MTK_NOR_ENABLE_SF_CMD		0x30
> +#define MTK_NOR_DUAD_ADDR_EN		0x8
> +#define MTK_NOR_QUAD_READ_EN		0x4
> +#define MTK_NOR_DUAL_ADDR_EN		0x2
> +#define MTK_NOR_DUAL_READ_EN		0x1
> +#define MTK_NOR_DUAL_DISABLE		0x0
> +#define MTK_NOR_FAST_READ		0x1
> +
> +#define SFLASH_WRBUF_SIZE		128
> +#define get_nth_byte(d, n)		((d >> (8 * n)) & 0xff)
> +
> +struct mt8173_nor {
> +	struct spi_nor nor;
> +	struct device *dev;
> +	void __iomem *base;	/* nor flash base address */
> +	struct clk *spi_clk;
> +	struct clk *nor_clk;
> +};
> +
> +static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
> +{
> +	struct spi_nor *nor = &mt8173_nor->nor;
> +
> +	writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG);
> +
> +	switch (nor->flash_read) {
> +	case SPI_NOR_FAST:
> +		writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
> +		       MTK_NOR_CFG1_REG);
> +		break;
> +	case SPI_NOR_DUAL:
> +		writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	case SPI_NOR_QUAD:
> +		writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	default:
> +		writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	}
> +}
> +
> +static int mt8173_nor_execute_cmd(struct mt8173_nor *mt8173_nor, u8 cmdval)
> +{
> +	int reg;
> +	u8 val = cmdval & 0x1f;
> +
> +	writeb(cmdval, mt8173_nor->base + MTK_NOR_CMD_REG);
> +	return readl_poll_timeout(mt8173_nor->base + MTK_NOR_CMD_REG, reg,
> +				  !(reg & val), 100, 10000);
> +}
> +
> +static int mt8173_nor_do_tx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
> +			    int len)

I realized this code should probably be shared with some of the "read
register" path too, so I'd suggest you make this a combined "tx_rx"
function. I've written and tested a version myself, and I'll paste the
(large) diff against your driver at the end of this email.

> +{
> +	int i;
> +
> +	if (len > 5)
> +		return -EINVAL;
> +
> +	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
> +
> +	for (i = 0; i < len; i++)
> +		writeb(buf[len - 1 - i], mt8173_nor->base +

I'm pretty sure this is wrong. You don't want to iterate over buf[]
*and* PRGDATAx registers backwards; you want to both in the same
direction. e.g.:

  buf[4] => PRGDATA0
  buf[3] => PRGDATA1
  buf[2] => PRGDATA2
  ...

or vice versa. I think this shows a bug in your address calculations for
the erase command, and you "fixed" it by reversing the loop here. More
below.

> +			       MTK_NOR_PRGDATA0_REG + 4 * (4 - i));
> +	writeb((len + 1) * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);

If you extend this function to (optionall) read from SHREGx after the
execute_cmd(), then you could share more code.

> +}
> +
> +/*
> + * this function is used to execute special read commands.
> + * such as SPINOR_OP_RDFSR, SPINOR_OP_RDCR, SPINOR_OP_RD_EVCR and so on.
> + * len is no more than 1.
> + */
> +static int mt8173_nor_do_rx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
> +			    int len)

This is not a "do_rx" command; it doesn't even use the buf or len
fields.

> +{
> +	if (len > 1)
> +		return -EINVAL;

This should definitely be able to handle more than 1 byte.

> +
> +	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
> +
> +	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
> +}
> +
> +/* cmd1 sent to nor flash, cmd2 write to nor controller */
> +static int mt8173_nor_set_para(struct mt8173_nor *mt8173_nor, int cmd1,
> +			       int cmd2)

This function will only actually be needed for the WRSR (write status
register) command, so I'd restructure it / rename it. Again, see my
patch below.

> +{
> +	writeb(cmd1, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
> +	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +	return mt8173_nor_execute_cmd(mt8173_nor, cmd2);
> +}
> +
> +static int mt8173_nor_write_buffer_enable(struct mt8173_nor *mt8173_nor)
> +{
> +	u8 reg;
> +
> +	/* the bit0 of MTK_NOR_CFG2_REG is pre-fetch buffer
> +	 * 0: pre-fetch buffer use for read
> +	 * 1: pre-fetch buffer use for page program
> +	 */
> +	writel(MTK_NOR_WR_BUF_ENABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
> +	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
> +				  0x01 == (reg & 0x01), 100, 10000);
> +}
> +
> +static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
> +{
> +	u8 reg;
> +
> +	writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
> +	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
> +				  MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
> +				  10000);
> +}
> +
> +static int mt8173_nor_erase_sector(struct spi_nor *nor, loff_t offset)
> +{
> +	u8 buf[4], i = 0;
> +	struct mt8173_nor *mt8173_nor = nor->priv;
> +
> +	while (i < 4) {
> +		buf[i] = get_nth_byte(offset, i);
> +		i++;
> +	}

This loop is wrong. Address bytes should not be sent in little endian
order; the *highest* byte should be first. That's why your do_tx()
function is all wrong.

> +	if (nor->mtd.size <= 0x1000000)
> +		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 3);
> +	else
> +		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 4);

You've ignored some of my comments... do not hardcode the 4K sector
command, because spi-nor could be using something else. You should use
'nor->erase_opcode'. Also, 4 vs. 3 should just be using
'nor->addr_width'.

But this can be even easier: I noticed that you really are just
open-coding a write_reg() call, just like m25p80.c was. So I refactored
the code there and shared it in spi-nor for you [1]. With that patch, I
don't think you'll even need to provide an erase() callback at all. Just
use the default.

> +}
> +
> +static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
> +			   size_t *retlen, u_char *buffer)
> +{
> +	int i, ret;
> +	int addr = (int)from;
> +	u8 *buf = (u8 *)buffer;
> +	struct mt8173_nor *mt8173_nor = nor->priv;
> +
> +	/* set mode for fast read mode ,dual mode or quad mode */
> +	mt8173_nor_set_read_mode(mt8173_nor);
> +	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
> +	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
> +	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
> +	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);

Programming the address registers is repeated in several places. Make
that into a helper function.

> +
> +	for (i = 0; i < length; i++, (*retlen)++) {
> +		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
> +		if (ret < 0)
> +			return ret;
> +		buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
> +	}
> +	return 0;
> +}
> +
> +static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
> +					int addr, int length, u8 *data)
> +{
> +	int i, ret;
> +
> +	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
> +	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
> +	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
> +	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);

Helper function.

> +
> +	for (i = 0; i < length; i++) {
> +		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
> +		if (ret < 0)
> +			return ret;
> +		writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
> +	}
> +	return 0;
> +}
> +
> +static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
> +				   const u8 *buf)
> +{
> +	int i, bufidx, data;
> +
> +	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
> +	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
> +	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
> +	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);

Helper function.

> +
> +	bufidx = 0;
> +	for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
> +		data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
> +		       buf[bufidx + 1]<<8 | buf[bufidx];
> +		bufidx += 4;
> +		writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
> +	}
> +	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
> +}
> +
> +static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
> +			     size_t *retlen, const u_char *buf)
> +{
> +	int ret;
> +	struct mt8173_nor *mt8173_nor = nor->priv;
> +
> +	ret = mt8173_nor_write_buffer_enable(mt8173_nor);
> +	if (ret < 0)
> +		dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
> +
> +	while (len >= SFLASH_WRBUF_SIZE) {
> +		ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
> +		if (ret < 0)
> +			dev_err(mt8173_nor->dev, "write buffer failed!\n");
> +		len -= SFLASH_WRBUF_SIZE;
> +		to += SFLASH_WRBUF_SIZE;
> +		buf += SFLASH_WRBUF_SIZE;
> +		(*retlen) += SFLASH_WRBUF_SIZE;
> +	}
> +	ret = mt8173_nor_write_buffer_disable(mt8173_nor);
> +	if (ret < 0)
> +		dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
> +
> +	if (len) {
> +		ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
> +						   (u8 *)buf);
> +		if (ret < 0)
> +			dev_err(mt8173_nor->dev, "write single byte failed!\n");
> +		(*retlen) += len;
> +	}
> +}
> +
> +static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> +{
> +	int ret;
> +	struct mt8173_nor *mt8173_nor = nor->priv;
> +
> +	/* mtk nor controller doesn't supoort SPINOR_OP_RDCR */

What's this comment about? I think your 'default' case below should
handle that, right?

> +	switch (opcode) {
> +	case SPINOR_OP_RDID:
> +		/* read JEDEC ID need 4 bytes commands */
> +		memset(buf, 0x0, 4);
> +		ret = mt8173_nor_do_tx(mt8173_nor, opcode, buf, 3);

You should not be doing a "tx" here ("tx" meaning "transmit" [2] and "rx"
meaning "receive"), because read ID is a "read" function. What your
doing here is kind of hacky.

Instead, you should have a better "do_rx" function (or, as I'm figuring
out) a "do_tx_rx" function that can handle both cases in
straightforward, logical code. See my patch below.

> +		if (ret < 0)
> +			return ret;
> +
> +		/* mtk nor flash controller only support 3 bytes IDs */
> +		buf[2] = readb(mt8173_nor->base + MTK_NOR_SHREG0_REG);
> +		buf[1] = readb(mt8173_nor->base + MTK_NOR_SHREG1_REG);
> +		buf[0] = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);

As we discussed, you can support 5 bytes, not just 3. Now, we'll have to
figure out for sure what to do about the remaining byte(s) that
spi-nor.c wants (right now, it requests only 1 more). When hacking
around myself, I figured if we see a RDID command of more than 5 bytes,
we can just zero out the last byte(s) and run do_rx() with len==5.

> +		break;
> +	case SPINOR_OP_RDSR:
> +		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
> +		if (ret < 0)
> +			return ret;
> +		*buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);

The caller asked for 'len' bytes. Usually that's 1, but we should be
safe and check for 'len == 1'.

> +		break;
> +	default:
> +		/* read other register of nor flash */
> +		ret = mt8173_nor_do_rx(mt8173_nor, opcode, buf, len);
> +		*buf = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);

The caller asked for 'len' bytes, so why are you only reading 1?

> +		break;
> +	}
> +	return ret;
> +}
> +
> +static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
> +				int len)
> +{
> +	int ret;
> +	struct mt8173_nor *mt8173_nor = nor->priv;
> +
> +	switch (opcode) {
> +	case SPINOR_OP_WRSR:
> +		ret = mt8173_nor_set_para(mt8173_nor, *buf,
> +					  MTK_NOR_WRSR_CMD);
> +		break;
> +	case SPINOR_OP_CHIP_ERASE:
> +		ret = mt8173_nor_set_para(mt8173_nor, opcode,
> +					  MTK_NOR_PRG_CMD);

This case should be able to fall under the default case. (If your
do_tx() function actually worked right.)

> +		break;
> +	default:
> +		ret = mt8173_nor_do_tx(mt8173_nor, opcode, NULL, 0);

Why are you passing NULL and 0?? That should be buf and len.

> +		if (ret)
> +			dev_warn(mt8173_nor->dev, "set write enable fail!\n");
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> +			       struct mtd_part_parser_data *ppdata)
> +{
> +	int ret;
> +	struct spi_nor *nor;
> +
> +	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);

This deserves a comment about what it does.

> +
> +	nor = &mt8173_nor->nor;
> +	nor->dev = mt8173_nor->dev;
> +	nor->priv = mt8173_nor;
> +	nor->flash_node = ppdata->of_node;
> +
> +	/* fill the hooks to spi nor */
> +	nor->read = mt8173_nor_read;
> +	nor->read_reg = mt8173_nor_read_reg;
> +	nor->write = mt8173_nor_write;
> +	nor->write_reg = mt8173_nor_write_reg;
> +	nor->erase = mt8173_nor_erase_sector;
> +	nor->mtd.owner = THIS_MODULE;
> +	nor->mtd.name = "mtk_nor";
> +	/* initialized with NULL */
> +	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
> +	if (ret)
> +		return ret;
> +
> +	return mtd_device_parse_register(&nor->mtd, NULL, ppdata, NULL, 0);
> +}
> +
> +static int mtk_nor_drv_probe(struct platform_device *pdev)
> +{
> +	struct device_node *flash_np;
> +	struct mtd_part_parser_data ppdata;

You never initialize this struct. So the other field(s) might contain
garbage.

> +	struct resource *res;
> +	int ret;
> +	struct mt8173_nor *mt8173_nor;
> +
> +	if (!pdev->dev.of_node) {
> +		dev_err(&pdev->dev, "No DT found\n");
> +		return -EINVAL;
> +	}
> +
> +	mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
> +	if (!mt8173_nor)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, mt8173_nor);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(mt8173_nor->base))
> +		return PTR_ERR(mt8173_nor->base);
> +
> +	mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
> +	if (IS_ERR(mt8173_nor->spi_clk)) {
> +		ret = PTR_ERR(mt8173_nor->spi_clk);
> +		goto nor_free;
> +	}
> +
> +	mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
> +	if (IS_ERR(mt8173_nor->nor_clk)) {
> +		ret = PTR_ERR(mt8173_nor->nor_clk);
> +		goto nor_free;
> +	}
> +
> +	mt8173_nor->dev = &pdev->dev;
> +	clk_prepare_enable(mt8173_nor->spi_clk);
> +	clk_prepare_enable(mt8173_nor->nor_clk);
> +
> +	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> +	if (!flash_np) {
> +		dev_err(&pdev->dev, "no SPI flash device to configure\n");
> +		ret = -ENODEV;
> +		goto nor_free;
> +	}
> +	ppdata.of_node = flash_np;
> +	ret = mtk_nor_init(mt8173_nor, &ppdata);
> +
> +nor_free:
> +	return ret;
> +}
> +
> +static int mtk_nor_drv_remove(struct platform_device *pdev)
> +{
> +	struct mt8173_nor *mt8173_nor = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(mt8173_nor->spi_clk);
> +	clk_disable_unprepare(mt8173_nor->nor_clk);
> +	return 0;
> +}
> +
> +static const struct of_device_id mtk_nor_of_ids[] = {
> +	{ .compatible = "mediatek,mt8173-nor"},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_nor_of_ids);
> +
> +static struct platform_driver mtk_nor_driver = {
> +	.probe = mtk_nor_drv_probe,
> +	.remove = mtk_nor_drv_remove,
> +	.driver = {
> +		.name = "mtk-nor",
> +		.of_match_table = mtk_nor_of_ids,
> +	},
> +};
> +
> +module_platform_driver(mtk_nor_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MediaTek SPI NOR Flash Driver");

I will paste a big diff below. If you'd like, I can just email you the
whole driver, since I redid significant portions of it. I tested all of
it, and all the basic functions seem to work well.

Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062916.html
[2] https://en.wikipedia.org/wiki/Transmission_(telecommunications)

---

diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
index 33a8dc56c20f..70dd62c7f989 100644
--- a/drivers/mtd/spi-nor/mtk-quadspi.c
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -101,7 +101,12 @@
 #define MTK_NOR_FAST_READ		0x1
 
 #define SFLASH_WRBUF_SIZE		128
-#define get_nth_byte(d, n)		((d >> (8 * n)) & 0xff)
+
+/* Can shift up to 48 bits (6 bytes) of TX/RX */
+#define MTK_NOR_MAX_SHIFT		6
+/* Helpers for accessing the program data / shift data registers */
+#define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
+#define MTK_NOR_SHREG(n)		(MTK_NOR_SHREG0_REG + 4 * (n))
 
 struct mt8173_nor {
 	struct spi_nor nor;
@@ -147,47 +152,54 @@ static int mt8173_nor_execute_cmd(struct mt8173_nor *mt8173_nor, u8 cmdval)
 				  !(reg & val), 100, 10000);
 }
 
-static int mt8173_nor_do_tx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
-			    int len)
+static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
+			       u8 *tx, int txlen, u8 *rx, int rxlen)
 {
-	int i;
+	int len = 1 + txlen + rxlen;
+	int i, ret, idx;
 
-	if (len > 5)
+	if (len > MTK_NOR_MAX_SHIFT)
 		return -EINVAL;
 
-	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+	writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
 
-	for (i = 0; i < len; i++)
-		writeb(buf[len - 1 - i], mt8173_nor->base +
-			       MTK_NOR_PRGDATA0_REG + 4 * (4 - i));
-	writeb((len + 1) * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
-	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
-}
+	/* start at PRGDATA5, go down to PRGDATA0 */
+	idx = MTK_NOR_MAX_SHIFT - 1;
 
-/*
- * this function is used to execute special read commands.
- * such as SPINOR_OP_RDFSR, SPINOR_OP_RDCR, SPINOR_OP_RD_EVCR and so on.
- * len is no more than 1.
- */
-static int mt8173_nor_do_rx(struct mt8173_nor *mt8173_nor, u8 op, u8 *buf,
-			    int len)
-{
-	if (len > 1)
-		return -EINVAL;
+	/* opcode */
+	writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+	idx--;
 
-	writeb(op, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+	/* program TX data */
+	for (i = 0; i < txlen; i++, idx--)
+		writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
 
-	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
-	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
+	/* clear out rest of TX registers */
+	while (idx >= 0) {
+		writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+		idx--;
+	}
+
+	ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
+	if (ret)
+		return ret;
+
+	/* restart at first RX byte */
+	idx = MTK_NOR_MAX_SHIFT - 2 - txlen;
+
+	/* read out RX data */
+	for (i = 0; i < rxlen; i++, idx--)
+		rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
+
+	return 0;
 }
 
-/* cmd1 sent to nor flash, cmd2 write to nor controller */
-static int mt8173_nor_set_para(struct mt8173_nor *mt8173_nor, int cmd1,
-			       int cmd2)
+/* Do a WRSR (Write Status Register) command */
+static int mt8173_nor_wr_sr(struct mt8173_nor *mt8173_nor, u8 sr)
 {
-	writeb(cmd1, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+	writeb(sr, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
 	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
-	return mt8173_nor_execute_cmd(mt8173_nor, cmd2);
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WRSR_CMD);
 }
 
 static int mt8173_nor_write_buffer_enable(struct mt8173_nor *mt8173_nor)
@@ -213,19 +225,16 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
 				  10000);
 }
 
-static int mt8173_nor_erase_sector(struct spi_nor *nor, loff_t offset)
+static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
 {
-	u8 buf[4], i = 0;
-	struct mt8173_nor *mt8173_nor = nor->priv;
+	int i;
 
-	while (i < 4) {
-		buf[i] = get_nth_byte(offset, i);
-		i++;
+	for (i = 0; i < 3; i++) {
+		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
+		addr >>= 8;
 	}
-	if (nor->mtd.size <= 0x1000000)
-		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 3);
-	else
-		return mt8173_nor_do_tx(mt8173_nor, SPINOR_OP_BE_4K, buf, 4);
+	/* Last register is non-contiguous */
+	writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
 }
 
 static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
@@ -238,10 +247,7 @@ static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
 
 	/* set mode for fast read mode ,dual mode or quad mode */
 	mt8173_nor_set_read_mode(mt8173_nor);
-	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
-	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
-	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
-	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+	mt8173_nor_set_addr(mt8173_nor, addr);
 
 	for (i = 0; i < length; i++, (*retlen)++) {
 		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
@@ -257,10 +263,7 @@ static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
 {
 	int i, ret;
 
-	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
-	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
-	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
-	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+	mt8173_nor_set_addr(mt8173_nor, addr);
 
 	for (i = 0; i < length; i++) {
 		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
@@ -276,10 +279,7 @@ static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
 {
 	int i, bufidx, data;
 
-	writeb((addr >> 24), mt8173_nor->base + MTK_NOR_RADR3_REG);
-	writeb((addr >> 16), mt8173_nor->base + MTK_NOR_RADR2_REG);
-	writeb((addr >> 8), mt8173_nor->base + MTK_NOR_RADR1_REG);
-	writeb(addr, mt8173_nor->base + MTK_NOR_RADR0_REG);
+	mt8173_nor_set_addr(mt8173_nor, addr);
 
 	bufidx = 0;
 	for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
@@ -330,28 +330,23 @@ static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 
 	/* mtk nor controller doesn't supoort SPINOR_OP_RDCR */
 	switch (opcode) {
-	case SPINOR_OP_RDID:
-		/* read JEDEC ID need 4 bytes commands */
-		memset(buf, 0x0, 4);
-		ret = mt8173_nor_do_tx(mt8173_nor, opcode, buf, 3);
-		if (ret < 0)
-			return ret;
-
-		/* mtk nor flash controller only support 3 bytes IDs */
-		buf[2] = readb(mt8173_nor->base + MTK_NOR_SHREG0_REG);
-		buf[1] = readb(mt8173_nor->base + MTK_NOR_SHREG1_REG);
-		buf[0] = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);
-		break;
 	case SPINOR_OP_RDSR:
 		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
 		if (ret < 0)
 			return ret;
 		*buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
 		break;
+	case SPINOR_OP_RDID:
+		if (len > MTK_NOR_MAX_SHIFT - 1) {
+			int i;
+			/* HACK */
+			for (i = MTK_NOR_MAX_SHIFT - 1; i < len; i++)
+				buf[i] = 0;
+			len = MTK_NOR_MAX_SHIFT - 1;
+		}
+		/* fall through */
 	default:
-		/* read other register of nor flash */
-		ret = mt8173_nor_do_rx(mt8173_nor, opcode, buf, len);
-		*buf = readb(mt8173_nor->base + MTK_NOR_SHREG2_REG);
+		ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
 		break;
 	}
 	return ret;
@@ -365,41 +360,40 @@ static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
 
 	switch (opcode) {
 	case SPINOR_OP_WRSR:
-		ret = mt8173_nor_set_para(mt8173_nor, *buf,
-					  MTK_NOR_WRSR_CMD);
-		break;
-	case SPINOR_OP_CHIP_ERASE:
-		ret = mt8173_nor_set_para(mt8173_nor, opcode,
-					  MTK_NOR_PRG_CMD);
+		/* We only handle 1 byte */
+		ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
 		break;
 	default:
-		ret = mt8173_nor_do_tx(mt8173_nor, opcode, NULL, 0);
+		ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
 		if (ret)
-			dev_warn(mt8173_nor->dev, "set write enable fail!\n");
+			dev_warn(mt8173_nor->dev, "write reg failure!\n");
 		break;
 	}
 	return ret;
 }
 
 static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
-			       struct mtd_part_parser_data *ppdata)
+			       struct device_node *flash_node)
 {
+	struct mtd_part_parser_data ppdata = {
+		.of_node = flash_node,
+	};
 	int ret;
 	struct spi_nor *nor;
 
+	/* initialize controller to accept commands */
 	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
 
 	nor = &mt8173_nor->nor;
 	nor->dev = mt8173_nor->dev;
 	nor->priv = mt8173_nor;
-	nor->flash_node = ppdata->of_node;
+	nor->flash_node = flash_node;
 
 	/* fill the hooks to spi nor */
 	nor->read = mt8173_nor_read;
 	nor->read_reg = mt8173_nor_read_reg;
 	nor->write = mt8173_nor_write;
 	nor->write_reg = mt8173_nor_write_reg;
-	nor->erase = mt8173_nor_erase_sector;
 	nor->mtd.owner = THIS_MODULE;
 	nor->mtd.name = "mtk_nor";
 	/* initialized with NULL */
@@ -407,13 +401,12 @@ static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
 	if (ret)
 		return ret;
 
-	return mtd_device_parse_register(&nor->mtd, NULL, ppdata, NULL, 0);
+	return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
 }
 
 static int mtk_nor_drv_probe(struct platform_device *pdev)
 {
 	struct device_node *flash_np;
-	struct mtd_part_parser_data ppdata;
 	struct resource *res;
 	int ret;
 	struct mt8173_nor *mt8173_nor;
@@ -449,14 +442,14 @@ static int mtk_nor_drv_probe(struct platform_device *pdev)
 	clk_prepare_enable(mt8173_nor->spi_clk);
 	clk_prepare_enable(mt8173_nor->nor_clk);
 
+	/* only support one attached flash */
 	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
 	if (!flash_np) {
 		dev_err(&pdev->dev, "no SPI flash device to configure\n");
 		ret = -ENODEV;
 		goto nor_free;
 	}
-	ppdata.of_node = flash_np;
-	ret = mtk_nor_init(mt8173_nor, &ppdata);
+	ret = mtk_nor_init(mt8173_nor, flash_np);
 
 nor_free:
 	return ret;
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Brian Norris]

Hi Bayi,

In reviewing your updated instructions on how to read 6 bytes of ID, I
have one more question about how the PRGDATA and SHREG registers are
supposed to work.

On Mon, Oct 26, 2015 at 09:40:38PM +0800, Bayi Cheng wrote:
[...]
> +static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
> +{
> +	struct spi_nor *nor = &mt8173_nor->nor;
> +
> +	writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG);

So far, I've generalized your code to say that except for a few commands
that are treated specially by your controller, all other opcodes are
queued up in the program/shift register this:

(1) total number of bits to send/receive goes in the COUNT register (so
    far, as many as 7*8=56?)
(2) opcode is written to PRGDATA5
(3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
(4) command is sent (execute_cmd())
(5) data is read back in SHREG{X..0}, if needed

However, I see that

(a) here in mt8173_nor_set_read_mode(), you use only PRGDATA3 (i.e.,
this is different than step (2) above). Why is that different? Is this
just a quirk of the read mode, which is different than the other
arbitrary opcodes (like READ ID)?

(b) it's really unclear how to determine 'X' in step (5). It seems like
it might just be the number of bytes minus 1, since the first byte
aligns with the "opcode" cycle. But I'm not really sure, since in one
case (the 'default' in mt8173_nor_read_reg()) you seemingly randomly
chose to read *only* from SHREG2. That looks wrong to me, but I don't
actually know, because your SoC is very unclear.

My patch at the end of this [1] tries to encapsulate (1)-(5) as best as
I could, but it's really missing out on the answer to (b).

Anyway, I'd really appreciate a good answer to these questions. We
really need to get this stuff right, so your driver can handle NOR flash
opcodes as generically as possible. Right now, your driver mostly looks
like a series of cobbled together switch/case statements to handle the
couple of opcodes you've tested.

Regard,
Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062919.html

> +	switch (nor->flash_read) {
> +	case SPI_NOR_FAST:
> +		writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
> +		       MTK_NOR_CFG1_REG);
> +		break;
> +	case SPI_NOR_DUAL:
> +		writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	case SPI_NOR_QUAD:
> +		writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	default:
> +		writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base +
> +		       MTK_NOR_DUAL_REG);
> +		break;
> +	}
> +}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html