This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
implementation patch [1]
[0]: git://git.infradead.org/l2-mtd.git
[1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
Change in v6:
1: delete mt8173_nor_do_rx
2: delete mt8173_nor_do_rx
3: add mt8173_nor_do_tx_rx for general usage
4: support nor flash with 6 IDs
5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
6: add mt8173_nor_set_addr to programming the address register
7: initialize the ppdata in mtk_nor_init
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 | 475 +++++++++++++++++++++
5 files changed, 540 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
Add device tree binding documentation for serial flash with
Mediatek serial flash controller
Signed-off-by: Bayi Cheng <[email protected]>
---
.../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
add spi nor flash driver for mediatek controller
Signed-off-by: Bayi Cheng <[email protected]>
---
drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
3 files changed, 483 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..f544bbe 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..6582811
--- /dev/null
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -0,0 +1,475 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Bayi Cheng <[email protected]>
+ *
+ * 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/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
+
+/* 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;
+ 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_rx(struct mt8173_nor *mt8173_nor, u8 op,
+ u8 *tx, int txlen, u8 *rx, int rxlen)
+{
+ int len = 1 + txlen + rxlen;
+ int i, ret, idx;
+
+ if (len > MTK_NOR_MAX_SHIFT + 1)
+ return -EINVAL;
+
+ writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
+
+ /* start at PRGDATA5, go down to PRGDATA0 */
+ idx = MTK_NOR_MAX_SHIFT - 1;
+
+ /* opcode */
+ writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+ idx--;
+
+ /* program TX data */
+ for (i = 0; i < txlen; i++, idx--)
+ writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+
+ /* 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 = rxlen - 1;
+
+ /* read out RX data */
+ for (i = 0; i < rxlen; i++, idx--)
+ rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
+
+ return 0;
+}
+
+/* Do a WRSR (Write Status Register) command */
+static int mt8173_nor_wr_sr(struct mt8173_nor *mt8173_nor, u8 sr)
+{
+ 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, MTK_NOR_WRSR_CMD);
+}
+
+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 void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
+{
+ int i;
+
+ for (i = 0; i < 3; i++) {
+ writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
+ addr >>= 8;
+ }
+ /* 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,
+ 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);
+ 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);
+ 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;
+
+ 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);
+ 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;
+
+ mt8173_nor_set_addr(mt8173_nor, addr);
+
+ 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;
+
+ switch (opcode) {
+ case SPINOR_OP_RDSR:
+ ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
+ if (ret < 0)
+ return ret;
+ if (len == 1)
+ *buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
+ else
+ dev_err(mt8173_nor->dev, "len should be 1 for read status!\n");
+ break;
+ default:
+ ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
+ 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:
+ /* We only handle 1 byte */
+ ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
+ break;
+ default:
+ ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
+ if (ret)
+ dev_warn(mt8173_nor->dev, "write reg failure!\n");
+ break;
+ }
+ return ret;
+}
+
+static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
+ 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 = 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->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 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);
+
+ /* 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;
+ }
+ ret = mtk_nor_init(mt8173_nor, flash_np);
+
+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
Add Mediatek nor flash node
Signed-off-by: Bayi Cheng <[email protected]>
---
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
drivers/mtd/spi-nor/mtk-quadspi.c:223:6-28: Move constant to right.
drivers/mtd/spi-nor/mtk-quadspi.c:214:6-10: Move constant to right.
Move constants to the right of binary operators.
Semantic patch information:
Depends on personal taste in some cases.
Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
CC: Bayi Cheng <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---
Please take the patch only if it's a positive warning. Thanks!
mtk-quadspi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/mtd/spi-nor/mtk-quadspi.c
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -211,7 +211,7 @@ static int mt8173_nor_write_buffer_enabl
*/
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);
+ (reg & 0x01) == 0x01, 100, 10000);
}
static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
@@ -220,7 +220,7 @@ static int mt8173_nor_write_buffer_disab
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,
+ (reg & 0x1) == MTK_NOR_WR_BUF_DISABLE, 100,
10000);
}
Hi Bayi,
[auto build test ERROR on mtd/master]
[also build test ERROR on v4.3 next-20151106]
url: https://github.com/0day-ci/linux/commits/Bayi-Cheng/Mediatek-SPI-NOR-flash-driver/20151106-235157
base: git://git.infradead.org/linux-mtd.git master
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/mtd/spi-nor/mtk-quadspi.c: In function 'mtk_nor_init':
>> drivers/mtd/spi-nor/mtk-quadspi.c:381:5: error: 'struct spi_nor' has no member named 'flash_node'
nor->flash_node = flash_node;
^
drivers/mtd/spi-nor/mtk-quadspi.c:387:17: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
nor->write_reg = mt8173_nor_write_reg;
^
>> drivers/mtd/spi-nor/mtk-quadspi.c:388:10: error: request for member 'owner' in something not a structure or union
nor->mtd.owner = THIS_MODULE;
^
>> drivers/mtd/spi-nor/mtk-quadspi.c:389:10: error: request for member 'name' in something not a structure or union
nor->mtd.name = "mtk_nor";
^
drivers/mtd/spi-nor/mtk-quadspi.c:395:35: warning: passing argument 1 of 'mtd_device_parse_register' from incompatible pointer type [-Wincompatible-pointer-types]
return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
^
In file included from drivers/mtd/spi-nor/mtk-quadspi.c:24:0:
include/linux/mtd/mtd.h:372:12: note: expected 'struct mtd_info *' but argument is of type 'struct mtd_info **'
extern int mtd_device_parse_register(struct mtd_info *mtd,
^
coccinelle warnings: (new ones prefixed by >>)
>> drivers/mtd/spi-nor/mtk-quadspi.c:223:6-28: Move constant to right.
drivers/mtd/spi-nor/mtk-quadspi.c:214:6-10: Move constant to right.
Please review and possibly fold the followup patch.
vim +381 drivers/mtd/spi-nor/mtk-quadspi.c
217 static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
218 {
219 u8 reg;
220
221 writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
222 return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
> 223 MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
224 10000);
225 }
226
227 static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
228 {
229 int i;
230
231 for (i = 0; i < 3; i++) {
232 writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
233 addr >>= 8;
234 }
235 /* Last register is non-contiguous */
236 writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
237 }
238
239 static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
240 size_t *retlen, u_char *buffer)
241 {
242 int i, ret;
243 int addr = (int)from;
244 u8 *buf = (u8 *)buffer;
245 struct mt8173_nor *mt8173_nor = nor->priv;
246
247 /* set mode for fast read mode ,dual mode or quad mode */
248 mt8173_nor_set_read_mode(mt8173_nor);
249 mt8173_nor_set_addr(mt8173_nor, addr);
250
251 for (i = 0; i < length; i++, (*retlen)++) {
252 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
253 if (ret < 0)
254 return ret;
255 buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
256 }
257 return 0;
258 }
259
260 static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
261 int addr, int length, u8 *data)
262 {
263 int i, ret;
264
265 mt8173_nor_set_addr(mt8173_nor, addr);
266
267 for (i = 0; i < length; i++) {
268 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
269 if (ret < 0)
270 return ret;
271 writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
272 }
273 return 0;
274 }
275
276 static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
277 const u8 *buf)
278 {
279 int i, bufidx, data;
280
281 mt8173_nor_set_addr(mt8173_nor, addr);
282
283 bufidx = 0;
284 for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
285 data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
286 buf[bufidx + 1]<<8 | buf[bufidx];
287 bufidx += 4;
288 writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
289 }
290 return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
291 }
292
293 static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
294 size_t *retlen, const u_char *buf)
295 {
296 int ret;
297 struct mt8173_nor *mt8173_nor = nor->priv;
298
299 ret = mt8173_nor_write_buffer_enable(mt8173_nor);
300 if (ret < 0)
301 dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
302
303 while (len >= SFLASH_WRBUF_SIZE) {
304 ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
305 if (ret < 0)
306 dev_err(mt8173_nor->dev, "write buffer failed!\n");
307 len -= SFLASH_WRBUF_SIZE;
308 to += SFLASH_WRBUF_SIZE;
309 buf += SFLASH_WRBUF_SIZE;
310 (*retlen) += SFLASH_WRBUF_SIZE;
311 }
312 ret = mt8173_nor_write_buffer_disable(mt8173_nor);
313 if (ret < 0)
314 dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
315
316 if (len) {
317 ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
318 (u8 *)buf);
319 if (ret < 0)
320 dev_err(mt8173_nor->dev, "write single byte failed!\n");
321 (*retlen) += len;
322 }
323 }
324
325 static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
326 {
327 int ret;
328 struct mt8173_nor *mt8173_nor = nor->priv;
329
330 switch (opcode) {
331 case SPINOR_OP_RDSR:
332 ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
333 if (ret < 0)
334 return ret;
335 if (len == 1)
336 *buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
337 else
338 dev_err(mt8173_nor->dev, "len should be 1 for read status!\n");
339 break;
340 default:
341 ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
342 break;
343 }
344 return ret;
345 }
346
347 static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
348 int len)
349 {
350 int ret;
351 struct mt8173_nor *mt8173_nor = nor->priv;
352
353 switch (opcode) {
354 case SPINOR_OP_WRSR:
355 /* We only handle 1 byte */
356 ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
357 break;
358 default:
359 ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
360 if (ret)
361 dev_warn(mt8173_nor->dev, "write reg failure!\n");
362 break;
363 }
364 return ret;
365 }
366
367 static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
368 struct device_node *flash_node)
369 {
370 struct mtd_part_parser_data ppdata = {
371 .of_node = flash_node,
372 };
373 int ret;
374 struct spi_nor *nor;
375 /* initialize controller to accept commands */
376 writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
377
378 nor = &mt8173_nor->nor;
379 nor->dev = mt8173_nor->dev;
380 nor->priv = mt8173_nor;
> 381 nor->flash_node = flash_node;
382
383 /* fill the hooks to spi nor */
384 nor->read = mt8173_nor_read;
385 nor->read_reg = mt8173_nor_read_reg;
386 nor->write = mt8173_nor_write;
387 nor->write_reg = mt8173_nor_write_reg;
> 388 nor->mtd.owner = THIS_MODULE;
> 389 nor->mtd.name = "mtk_nor";
390 /* initialized with NULL */
391 ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
392 if (ret)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng <[email protected]>
Acked-by: Rob Herring <[email protected]>
> ---
> .../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
>
Hi Bayi,
On Fri, Nov 06, 2015 at 11:48:06PM +0800, Bayi Cheng wrote:
> This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
> implementation patch [1]
>
> [0]: git://git.infradead.org/l2-mtd.git
> [1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
>
> Change in v6:
> 1: delete mt8173_nor_do_rx
> 2: delete mt8173_nor_do_rx
> 3: add mt8173_nor_do_tx_rx for general usage
> 4: support nor flash with 6 IDs
> 5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
> 6: add mt8173_nor_set_addr to programming the address register
> 7: initialize the ppdata in mtk_nor_init
This series is looking a lot better to me. Thanks for incorporating (and
I hope fully reviewing and testing!) my suggested changes. I have a just
a few small comments that I might post to the driver patch, and if
that's all that's outstanding, I can fix them up myself before applying.
I believe you didn't completely answer all my questions from v5 though.
I'll repeat a bit here. Particularly, refer to [1].
I'll summarize; I understand that your common transmit/receive operation
works something like this:
Quoting from [1]:
> (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
My questions were:
(a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
mentioned in the SoC manual, and it doesn't really match any of the
steps above. Perhaps it's just a quirk of the controller's
programming model?
(b) How do you determine X from step (5)?
Right now, your code seems to answer that X is "rxlen - 1". Correct?
If that's correct and if I put all of my understanding together
correctly, this means that you can actually shift out (in PRGDATA) up to
6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
7 bytes, except that the first byte is received during the opcode cycle,
and so it is discarded, and we effectively receive only 6 bytes.
Is that all correct? If so, then I think you still need to adjust the
boundary conditions in your do_tx_rx() function. (I'll comment on the
driver to point out the specifics.)
Regards,
Brian
[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
Hi Bayi,
A few small comments.
On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> add spi nor flash driver for mediatek controller
>
> Signed-off-by: Bayi Cheng <[email protected]>
> ---
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 483 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c
> 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..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@
...
> +/* Can shift up to 48 bits (6 bytes) of TX/RX */
> +#define MTK_NOR_MAX_SHIFT 6
...
> +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
> + u8 *tx, int txlen, u8 *rx, int rxlen)
> +{
> + int len = 1 + txlen + rxlen;
> + int i, ret, idx;
> +
> + if (len > MTK_NOR_MAX_SHIFT + 1)
> + return -EINVAL;
So I see you adjusted these bounds to add 1, which inspired one of my
questions on the cover letter.
Why do you allow len=7, which means you'd program 7*8 to the COUNT
register, when the SoC manual says it has a max of 48? Is the manual
wrong?
I notice you added the '+ 1' to your driver, so it allows:
do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */
but that means your driver also allows:
do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of
bounds write on PRGDATA
register -1 */
If I understand correctly, the following constraints are more correct:
/* Can only transmit 1 opcode and 5 other bytes */
if (1 + txlen > MTK_NOR_MAX_SHIFT)
return -EINVAL;
/* Can only receive 6 bytes after the opcode */
if (rxlen > MTK_NOR_MAX_SHIFT)
return -EINVAL;
/* Can only handle XXX bytes total */
/* How many bytes is the max for register MTK_NOR_CNT_REG ? */
if (len > XXX)
return -EINVAL;
> +
> + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +
> + /* start at PRGDATA5, go down to PRGDATA0 */
> + idx = MTK_NOR_MAX_SHIFT - 1;
> +
> + /* opcode */
> + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> + idx--;
> +
> + /* program TX data */
> + for (i = 0; i < txlen; i++, idx--)
> + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> +
> + /* 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 = rxlen - 1;
> +
> + /* read out RX data */
> + for (i = 0; i < rxlen; i++, idx--)
> + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
> +
> + return 0;
> +}
> +
...
> +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> + struct device_node *flash_node)
> +{
> + struct mtd_part_parser_data ppdata = {
> + .of_node = flash_node,
> + };
> + int ret;
> + struct spi_nor *nor;
Normally we'd have a blank line here.
> + /* 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 = 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->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);
> +}
...
Brian
On Mon, 2015-11-09 at 19:01 -0800, Brian Norris wrote:
> Hi Bayi,
>
> A few small comments.
>
> On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> > add spi nor flash driver for mediatek controller
> >
> > Signed-off-by: Bayi Cheng <[email protected]>
> > ---
> > drivers/mtd/spi-nor/Kconfig | 7 +
> > drivers/mtd/spi-nor/Makefile | 1 +
> > drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 483 insertions(+)
> > create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c
>
>
> > 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..6582811
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > @@ -0,0 +1,475 @@
>
> ...
>
> > +/* Can shift up to 48 bits (6 bytes) of TX/RX */
> > +#define MTK_NOR_MAX_SHIFT 6
>
> ...
>
> > +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
> > + u8 *tx, int txlen, u8 *rx, int rxlen)
> > +{
> > + int len = 1 + txlen + rxlen;
> > + int i, ret, idx;
> > +
> > + if (len > MTK_NOR_MAX_SHIFT + 1)
> > + return -EINVAL;
>
> So I see you adjusted these bounds to add 1, which inspired one of my
> questions on the cover letter.
>
> Why do you allow len=7, which means you'd program 7*8 to the COUNT
> register, when the SoC manual says it has a max of 48? Is the manual
> wrong?
>
Hi Brian, you are right, the manual is wrong here, Actually, it has a
max of 56. when we want to read 6 IDs, we need transfer 1 byte command
and 6 bytes null address to nor flash, then we can read the six IDs from
SHREGx register.
> I notice you added the '+ 1' to your driver, so it allows:
>
> do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */
>
> but that means your driver also allows:
>
> do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of
> bounds write on PRGDATA
> register -1 */
>
> If I understand correctly, the following constraints are more correct:
>
> /* Can only transmit 1 opcode and 5 other bytes */
> if (1 + txlen > MTK_NOR_MAX_SHIFT)
> return -EINVAL;
>
> /* Can only receive 6 bytes after the opcode */
> if (rxlen > MTK_NOR_MAX_SHIFT)
> return -EINVAL;
>
> /* Can only handle XXX bytes total */
> /* How many bytes is the max for register MTK_NOR_CNT_REG ? */
> if (len > XXX)
> return -EINVAL;
>
yes, your constraints seems more correct, and I will adapt these lines
to next patch.
> > +
> > + writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> > +
> > + /* start at PRGDATA5, go down to PRGDATA0 */
> > + idx = MTK_NOR_MAX_SHIFT - 1;
> > +
> > + /* opcode */
> > + writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> > + idx--;
> > +
> > + /* program TX data */
> > + for (i = 0; i < txlen; i++, idx--)
> > + writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> > +
> > + /* 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 = rxlen - 1;
> > +
> > + /* read out RX data */
> > + for (i = 0; i < rxlen; i++, idx--)
> > + rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
> > +
> > + return 0;
> > +}
> > +
>
> ...
>
> > +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> > + struct device_node *flash_node)
> > +{
> > + struct mtd_part_parser_data ppdata = {
> > + .of_node = flash_node,
> > + };
> > + int ret;
> > + struct spi_nor *nor;
>
> Normally we'd have a blank line here.
>
Ok, I will fix it, and thanks for your advice.
> > + /* 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 = 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->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);
> > +}
>
> ...
>
> Brian
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> Hi Bayi,
>
> On Fri, Nov 06, 2015 at 11:48:06PM +0800, Bayi Cheng wrote:
> > This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
> > implementation patch [1]
> >
> > [0]: git://git.infradead.org/l2-mtd.git
> > [1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
> >
> > Change in v6:
> > 1: delete mt8173_nor_do_rx
> > 2: delete mt8173_nor_do_rx
> > 3: add mt8173_nor_do_tx_rx for general usage
> > 4: support nor flash with 6 IDs
> > 5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
> > 6: add mt8173_nor_set_addr to programming the address register
> > 7: initialize the ppdata in mtk_nor_init
>
> This series is looking a lot better to me. Thanks for incorporating (and
> I hope fully reviewing and testing!) my suggested changes. I have a just
> a few small comments that I might post to the driver patch, and if
> that's all that's outstanding, I can fix them up myself before applying.
>
> I believe you didn't completely answer all my questions from v5 though.
> I'll repeat a bit here. Particularly, refer to [1].
> I'll summarize; I understand that your common transmit/receive operation
> works something like this:
>
> Quoting from [1]:
> > (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
>
> My questions were:
>
> (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> mentioned in the SoC manual, and it doesn't really match any of the
> steps above. Perhaps it's just a quirk of the controller's
> programming model?
>
yes, for this question, I have done some testes, If I change the
PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
functions, then the controller will be hanged, and I have asked our
designer for double confirm.
> (b) How do you determine X from step (5)?
>
> Right now, your code seems to answer that X is "rxlen - 1". Correct?
>
yes, I have used "rxlen -1", because the first of nor flash output is
located at SHREG[0], in the other words, the output data starts at
SHREG[0] and go up to SHREG[relen -1]
> If that's correct and if I put all of my understanding together
> correctly, this means that you can actually shift out (in PRGDATA) up to
> 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> 7 bytes, except that the first byte is received during the opcode cycle,
> and so it is discarded, and we effectively receive only 6 bytes.
>
> Is that all correct? If so, then I think you still need to adjust the
> boundary conditions in your do_tx_rx() function. (I'll comment on the
> driver to point out the specifics.)
Yes, you are right! and I will adjust the boundary conditions in
do_tx_rx() function.
By the way, could you tell me whether I need to publish a new patch? or
you can fix them up directly?
>
> Regards,
> Brian
>
> [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
On Wed, Nov 11, 2015 at 10:04:14PM +0800, Bayi Cheng wrote:
> On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> > I believe you didn't completely answer all my questions from v5 though.
> > I'll repeat a bit here. Particularly, refer to [1].
> > I'll summarize; I understand that your common transmit/receive operation
> > works something like this:
> >
> > Quoting from [1]:
> > > (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
> >
> > My questions were:
> >
> > (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> > mentioned in the SoC manual, and it doesn't really match any of the
> > steps above. Perhaps it's just a quirk of the controller's
> > programming model?
> >
> yes, for this question, I have done some testes, If I change the
> PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
> functions, then the controller will be hanged, and I have asked our
> designer for double confirm.
I wasn't suggesting to change this to PRGDATA5. I just was wondering why
the difference. It's not documented. (I suppose an acceptable answer is
just "because that's how the HW works.")
> > (b) How do you determine X from step (5)?
> >
> > Right now, your code seems to answer that X is "rxlen - 1". Correct?
> >
> yes, I have used "rxlen -1", because the first of nor flash output is
> located at SHREG[0], in the other words, the output data starts at
> SHREG[0] and go up to SHREG[relen -1]
But, we aren't reading from SHREG[0] first; we're reading backwards from
SHREG[rxlen - 1] down to SHREG[0]. It seems that's correct, right?
> > If that's correct and if I put all of my understanding together
> > correctly, this means that you can actually shift out (in PRGDATA) up to
> > 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> > 7 bytes, except that the first byte is received during the opcode cycle,
> > and so it is discarded, and we effectively receive only 6 bytes.
> >
> > Is that all correct? If so, then I think you still need to adjust the
> > boundary conditions in your do_tx_rx() function. (I'll comment on the
> > driver to point out the specifics.)
>
> Yes, you are right! and I will adjust the boundary conditions in
> do_tx_rx() function.
OK, good. BTW, can you make sure to rewrite the appropriate MAX macro(s)
to reflect the right values? It seems like maybe you'll want separate
macros for the maximum TX and RX -- and total (?), or is this just the
same as RX? -- since they seem to have different limits.
> By the way, could you tell me whether I need to publish a new patch? or
> you can fix them up directly?
I think there are a few more adjustments to make, so please just post a
new version of the driver only. The DT binding and DTS changes look good
to go now.
Regards,
Brian
> > [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
>
> Signed-off-by: Bayi Cheng <[email protected]>
> ---
Applied to l2-mtd.git/next (for 4.5). This will show up in
linux-next.git after the merge window.
Also squashed in a small diff (below), to fix up some language issues
and to refer the reader to the jedec,spi-nor.txt document.
> .../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
The DT binding document shouldn't be talking about software (i.e.,
shouldn't be talking about "drivers").
> +
> +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>;
> + };
> +};
> +
diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
index 866b492c38d2..fb314f09861b 100644
--- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
@@ -1,19 +1,19 @@
-* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
+* Serial NOR flash controller for MTK MT81xx (and similar)
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,
+- clocks: the phandle of the clocks needed by the nor controller
+- clock-names: the names of the clocks
+ the clocks should be named "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.
+The SPI flash must be a child of the nor_flash node and must have a
+compatible property. Also see jedec,spi-nor.txt.
Required properties:
- compatible: May include a device-specific string consisting of the manufacturer
On Fri, Nov 06, 2015 at 11:48:09PM +0800, Bayi Cheng wrote:
> Add Mediatek nor flash node
>
> Signed-off-by: Bayi Cheng <[email protected]>
Acked-by: Brian Norris <[email protected]>
> ---
> 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
>
One more small comment, since you're respinning this:
On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 0000000..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@
...
> +static int mtk_nor_drv_probe(struct platform_device *pdev)
> +{
> + struct device_node *flash_np;
> + 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);
You enable the clocks here, but...
> +
> + /* only support one attached flash */
> + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> + if (!flash_np) {
... you might bail out here if there is no available flash node, and you
never disable the clocks. (Same is true if we fail to detect the flash;
you leave the no-longer-needed clocks enabled.) Seems like maybe you
should disable clocks on failure.
> + dev_err(&pdev->dev, "no SPI flash device to configure\n");
> + ret = -ENODEV;
> + goto nor_free;
> + }
> + ret = mtk_nor_init(mt8173_nor, flash_np);
> +
> +nor_free:
> + return ret;
> +}
Brian
On Wed, 2015-11-11 at 12:26 -0800, Brian Norris wrote:
> On Wed, Nov 11, 2015 at 10:04:14PM +0800, Bayi Cheng wrote:
> > On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> > > I believe you didn't completely answer all my questions from v5 though.
> > > I'll repeat a bit here. Particularly, refer to [1].
> > > I'll summarize; I understand that your common transmit/receive operation
> > > works something like this:
> > >
> > > Quoting from [1]:
> > > > (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
> > >
> > > My questions were:
> > >
> > > (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> > > mentioned in the SoC manual, and it doesn't really match any of the
> > > steps above. Perhaps it's just a quirk of the controller's
> > > programming model?
> > >
> > yes, for this question, I have done some testes, If I change the
> > PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
> > functions, then the controller will be hanged, and I have asked our
> > designer for double confirm.
>
> I wasn't suggesting to change this to PRGDATA5. I just was wondering why
> the difference. It's not documented. (I suppose an acceptable answer is
> just "because that's how the HW works.")
>
I have synced with our designer, and just as you said, that's how the HW
works, and the read operation is different is different from other
operation. By the way, I have double confirm our driver code, and I have
made a mistake for quad read operation, the quad command need use
PRGDATA4 Instead PRGDATA3.
PS: write PRGDATA4 0xEB(4bit I/O read mode) or 0x6B(4 bit output mode),
So I will modify mt8173_nor_set_read_mode function in next patch.
> > > (b) How do you determine X from step (5)?
> > >
> > > Right now, your code seems to answer that X is "rxlen - 1". Correct?
> > >
> > yes, I have used "rxlen -1", because the first of nor flash output is
> > located at SHREG[0], in the other words, the output data starts at
> > SHREG[0] and go up to SHREG[relen -1]
>
> But, we aren't reading from SHREG[0] first; we're reading backwards from
> SHREG[rxlen - 1] down to SHREG[0]. It seems that's correct, right?
Yes, You are right!
>
> > > If that's correct and if I put all of my understanding together
> > > correctly, this means that you can actually shift out (in PRGDATA) up to
> > > 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> > > 7 bytes, except that the first byte is received during the opcode cycle,
> > > and so it is discarded, and we effectively receive only 6 bytes.
> > >
> > > Is that all correct? If so, then I think you still need to adjust the
> > > boundary conditions in your do_tx_rx() function. (I'll comment on the
> > > driver to point out the specifics.)
> >
> > Yes, you are right! and I will adjust the boundary conditions in
> > do_tx_rx() function.
>
> OK, good. BTW, can you make sure to rewrite the appropriate MAX macro(s)
> to reflect the right values? It seems like maybe you'll want separate
> macros for the maximum TX and RX -- and total (?), or is this just the
> same as RX? -- since they seem to have different limits.
>
OK, I will use two MAX macros, one is for TX&RX, and the other is for
the total.
> > By the way, could you tell me whether I need to publish a new patch? or
> > you can fix them up directly?
>
> I think there are a few more adjustments to make, so please just post a
> new version of the driver only. The DT binding and DTS changes look good
> to go now.
Yes, I will publish a new patch next week, and Thanks again for your
grate help and support!
>
> Regards,
> Brian
>
> > > [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Wed, 2015-11-11 at 12:38 -0800, Brian Norris wrote:
> On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> > Add device tree binding documentation for serial flash with
> > Mediatek serial flash controller
> >
> > Signed-off-by: Bayi Cheng <[email protected]>
> > ---
>
> Applied to l2-mtd.git/next (for 4.5). This will show up in
> linux-next.git after the merge window.
>
> Also squashed in a small diff (below), to fix up some language issues
> and to refer the reader to the jedec,spi-nor.txt document.
>
OK, I will fix it in next patch!
> > .../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
>
> The DT binding document shouldn't be talking about software (i.e.,
> shouldn't be talking about "drivers").
>
OK, I will fix it.
> > +
> > +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>;
> > + };
> > +};
> > +
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> index 866b492c38d2..fb314f09861b 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -1,19 +1,19 @@
> -* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +* Serial NOR flash controller for MTK MT81xx (and similar)
>
> 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,
> +- clocks: the phandle of the clocks needed by the nor controller
> +- clock-names: the names of the clocks
> + the clocks should be named "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.
> +The SPI flash must be a child of the nor_flash node and must have a
> +compatible property. Also see jedec,spi-nor.txt.
>
> Required properties:
> - compatible: May include a device-specific string consisting of the manufacturer
Thanks for your instruction! and I will fix it in the next patch!
On Wed, 2015-11-11 at 13:41 -0800, Brian Norris wrote:
> One more small comment, since you're respinning this:
>
> On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> > new file mode 100644
> > index 0000000..6582811
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > @@ -0,0 +1,475 @@
>
> ...
>
> > +static int mtk_nor_drv_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *flash_np;
> > + 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);
>
> You enable the clocks here, but...
>
> > +
> > + /* only support one attached flash */
> > + flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> > + if (!flash_np) {
>
> ... you might bail out here if there is no available flash node, and you
> never disable the clocks. (Same is true if we fail to detect the flash;
> you leave the no-longer-needed clocks enabled.) Seems like maybe you
> should disable clocks on failure.
Yes, I have forgot to disable these clocks on failure. and I will fix it
in the next patch! Thanks!
>
> > + dev_err(&pdev->dev, "no SPI flash device to configure\n");
> > + ret = -ENODEV;
> > + goto nor_free;
> > + }
> > + ret = mtk_nor_init(mt8173_nor, flash_np);
> > +
> > +nor_free:
> > + return ret;
> > +}
>
> Brian
On Fri, Nov 13, 2015 at 03:20:45PM +0800, Bayi Cheng wrote:
> On Wed, 2015-11-11 at 12:38 -0800, Brian Norris wrote:
> > Applied to l2-mtd.git/next (for 4.5). This will show up in
> > linux-next.git after the merge window.
> >
> > Also squashed in a small diff (below), to fix up some language issues
> > and to refer the reader to the jedec,spi-nor.txt document.
> >
>
> OK, I will fix it in next patch!
No, you don't need to send another patch. I already merged just this one
(the DT documentation) with my own small fixes, with a plan to go into
4.5. You only need to send another version of patch 2 (the driver).
During the merge window, I'm stashing changes in l2-mtd.git's 'next'
branch, so we can keep stuff that's going to 4.5 separate from stuff
thats getting merged to 4.4 right now. You can see it here for now:
http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next
After the merge window closes (probably on Sunday), I'll bring this into
+master.
Regards,
Brian