From: Jiancheng Xue <[email protected]>
Add hisilicon spi-nor flash controller driver
Signed-off-by: Binquan Peng <[email protected]>
Signed-off-by: Jiancheng Xue <[email protected]>
Acked-by: Rob Herring <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
---
change log
v10:
Fixed issues pointed by Marek Vasut.
1)Droped the underscores in the argument names of some macros' definition.
2)Changed some varibles to correct type.
3)Rewrote hisi_spi_nor_read/write for readability.
4)Added new functions hisi_spi_nor_register/unregister_all
5)Changed to dynamically allocation for spi_nor embeded in hifmc_host.
Fixed issues pointed by Brian Norris.
1)Replaced some headers with more accurate ones.
v9:
Fixed issues pointed by Jagan Teki.
v8:
Fixed issues pointed by Ezequiel Garcia and Brian Norris.
Moved dts binding file to mtd directory.
Changed the compatible string more specific.
v7:
Rebased to v4.5-rc3.
Fixed issues pointed by Ezequiel Garcia.
v6:
Based on v4.5-rc2
Fixed issues pointed by Ezequiel Garcia.
v5:
Fixed a compile error.
v4:
Rebased to v4.5-rc1
v3:
Added a compatible string "hisilicon,hi3519-sfc".
v2:
Fixed some compiling warings.
.../bindings/mtd/hisilicon,fmc-spi-nor.txt | 24 +
drivers/mtd/spi-nor/Kconfig | 7 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/hisi-sfc.c | 539 +++++++++++++++++++++
4 files changed, 571 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
create mode 100644 drivers/mtd/spi-nor/hisi-sfc.c
diff --git a/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt b/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
new file mode 100644
index 0000000..7498152
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
@@ -0,0 +1,24 @@
+HiSilicon SPI-NOR Flash Controller
+
+Required properties:
+- compatible : Should be "hisilicon,fmc-spi-nor" and one of the following strings:
+ "hisilicon,hi3519-spi-nor"
+- address-cells : Should be 1.
+- size-cells : Should be 0.
+- reg : Offset and length of the register set for the controller device.
+- reg-names : Must include the following two entries: "control", "memory".
+- clocks : handle to spi-nor flash controller clock.
+
+Example:
+spi-nor-controller@10000000 {
+ compatible = "hisilicon,hi3519-spi-nor", "hisilicon,fmc-spi-nor";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10000000 0x1000>, <0x14000000 0x1000000>;
+ reg-names = "control", "memory";
+ clocks = <&clock HI3519_FMC_CLK>;
+ spi-nor@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ };
+};
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index d42c98e..b9ff675 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -38,6 +38,13 @@ config SPI_FSL_QUADSPI
This controller does not support generic SPI. It only supports
SPI NOR.
+config SPI_HISI_SFC
+ tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
+ depends on ARCH_HISI || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ This enables support for hisilicon SPI-NOR flash controller.
+
config SPI_NXP_SPIFI
tristate "NXP SPI Flash Interface (SPIFI)"
depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 0bf3a7f8..8a6fa69 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
+obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c
new file mode 100644
index 0000000..942dafd
--- /dev/null
+++ b/drivers/mtd/spi-nor/hisi-sfc.c
@@ -0,0 +1,539 @@
+/*
+ * HiSilicon SPI Nor Flash Controller Driver
+ *
+ * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Hardware register offsets and field definitions */
+#define FMC_CFG 0x00
+#define SPI_NOR_ADDR_MODE BIT(10)
+#define FMC_GLOBAL_CFG 0x04
+#define FMC_GLOBAL_CFG_WP_ENABLE BIT(6)
+#define FMC_SPI_TIMING_CFG 0x08
+#define TIMING_CFG_TCSH(nr) (((nr) & 0xf) << 8)
+#define TIMING_CFG_TCSS(nr) (((nr) & 0xf) << 4)
+#define TIMING_CFG_TSHSL(nr) ((nr) & 0xf)
+#define CS_HOLD_TIME 0x6
+#define CS_SETUP_TIME 0x6
+#define CS_DESELECT_TIME 0xf
+#define FMC_INT 0x18
+#define FMC_INT_OP_DONE BIT(0)
+#define FMC_INT_CLR 0x20
+#define FMC_CMD 0x24
+#define FMC_CMD_CMD1(cmd) ((cmd) & 0xff)
+#define FMC_ADDRL 0x2c
+#define FMC_OP_CFG 0x30
+#define OP_CFG_FM_CS(cs) ((cs) << 11)
+#define OP_CFG_MEM_IF_TYPE(type) (((type) & 0x7) << 7)
+#define OP_CFG_ADDR_NUM(addr) (((addr) & 0x7) << 4)
+#define OP_CFG_DUMMY_NUM(dummy) ((dummy) & 0xf)
+#define FMC_DATA_NUM 0x38
+#define FMC_DATA_NUM_CNT(cnt) ((cnt) & 0x3fff)
+#define FMC_OP 0x3c
+#define FMC_OP_DUMMY_EN BIT(8)
+#define FMC_OP_CMD1_EN BIT(7)
+#define FMC_OP_ADDR_EN BIT(6)
+#define FMC_OP_WRITE_DATA_EN BIT(5)
+#define FMC_OP_READ_DATA_EN BIT(2)
+#define FMC_OP_READ_STATUS_EN BIT(1)
+#define FMC_OP_REG_OP_START BIT(0)
+#define FMC_DMA_LEN 0x40
+#define FMC_DMA_LEN_SET(len) ((len) & 0x0fffffff)
+#define FMC_DMA_SADDR_D0 0x4c
+#define HIFMC_DMA_MAX_LEN (4096)
+#define HIFMC_DMA_MASK (HIFMC_DMA_MAX_LEN - 1)
+#define FMC_OP_DMA 0x68
+#define OP_CTRL_RD_OPCODE(code) (((code) & 0xff) << 16)
+#define OP_CTRL_WR_OPCODE(code) (((code) & 0xff) << 8)
+#define OP_CTRL_RW_OP(op) ((op) << 1)
+#define OP_CTRL_DMA_OP_READY BIT(0)
+#define FMC_OP_READ 0x0
+#define FMC_OP_WRITE 0x1
+#define FMC_WAIT_TIMEOUT 1000000
+
+enum hifmc_iftype {
+ IF_TYPE_STD,
+ IF_TYPE_DUAL,
+ IF_TYPE_DIO,
+ IF_TYPE_QUAD,
+ IF_TYPE_QIO,
+};
+
+struct hifmc_priv {
+ u32 chipselect;
+ u32 clkrate;
+ struct hifmc_host *host;
+};
+
+#define HIFMC_MAX_CHIP_NUM 2
+struct hifmc_host {
+ struct device *dev;
+ struct mutex lock;
+
+ void __iomem *regbase;
+ void __iomem *iobase;
+ struct clk *clk;
+ void *buffer;
+ dma_addr_t dma_buffer;
+
+ struct spi_nor *nor[HIFMC_MAX_CHIP_NUM];
+ u32 num_chip;
+};
+
+static inline int wait_op_finish(struct hifmc_host *host)
+{
+ u32 reg;
+
+ return readl_poll_timeout(host->regbase + FMC_INT, reg,
+ (reg & FMC_INT_OP_DONE), 0, FMC_WAIT_TIMEOUT);
+}
+
+static int get_if_type(enum read_mode flash_read)
+{
+ enum hifmc_iftype if_type;
+
+ switch (flash_read) {
+ case SPI_NOR_DUAL:
+ if_type = IF_TYPE_DUAL;
+ break;
+ case SPI_NOR_QUAD:
+ if_type = IF_TYPE_QUAD;
+ break;
+ case SPI_NOR_NORMAL:
+ case SPI_NOR_FAST:
+ default:
+ if_type = IF_TYPE_STD;
+ break;
+ }
+
+ return if_type;
+}
+
+static void hisi_spi_nor_init(struct hifmc_host *host)
+{
+ u32 reg;
+
+ reg = TIMING_CFG_TCSH(CS_HOLD_TIME)
+ | TIMING_CFG_TCSS(CS_SETUP_TIME)
+ | TIMING_CFG_TSHSL(CS_DESELECT_TIME);
+ writel(reg, host->regbase + FMC_SPI_TIMING_CFG);
+}
+
+static int hisi_spi_nor_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+ int ret;
+
+ mutex_lock(&host->lock);
+
+ ret = clk_set_rate(host->clk, priv->clkrate);
+ if (ret)
+ goto out;
+
+ ret = clk_prepare_enable(host->clk);
+ if (ret)
+ goto out;
+
+ return 0;
+
+out:
+ mutex_unlock(&host->lock);
+ return ret;
+}
+
+static void hisi_spi_nor_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+
+ clk_disable_unprepare(host->clk);
+ mutex_unlock(&host->lock);
+}
+
+static void hisi_spi_nor_cmd_prepare(struct hifmc_host *host, u8 cmd,
+ u32 *opcfg)
+{
+ u32 reg;
+
+ *opcfg |= FMC_OP_CMD1_EN;
+ switch (cmd) {
+ case SPINOR_OP_RDID:
+ case SPINOR_OP_RDSR:
+ case SPINOR_OP_RDCR:
+ *opcfg |= FMC_OP_READ_DATA_EN;
+ break;
+ case SPINOR_OP_WREN:
+ reg = readl(host->regbase + FMC_GLOBAL_CFG);
+ if (reg & FMC_GLOBAL_CFG_WP_ENABLE) {
+ reg &= ~FMC_GLOBAL_CFG_WP_ENABLE;
+ writel(reg, host->regbase + FMC_GLOBAL_CFG);
+ }
+ break;
+ case SPINOR_OP_WRSR:
+ *opcfg |= FMC_OP_WRITE_DATA_EN;
+ break;
+ case SPINOR_OP_BE_4K:
+ case SPINOR_OP_BE_4K_PMC:
+ case SPINOR_OP_SE_4B:
+ case SPINOR_OP_SE:
+ *opcfg |= FMC_OP_ADDR_EN;
+ break;
+ case SPINOR_OP_EN4B:
+ reg = readl(host->regbase + FMC_CFG);
+ reg |= SPI_NOR_ADDR_MODE;
+ writel(reg, host->regbase + FMC_CFG);
+ break;
+ case SPINOR_OP_EX4B:
+ reg = readl(host->regbase + FMC_CFG);
+ reg &= ~SPI_NOR_ADDR_MODE;
+ writel(reg, host->regbase + FMC_CFG);
+ break;
+ case SPINOR_OP_CHIP_ERASE:
+ default:
+ break;
+ }
+}
+
+static int hisi_spi_nor_send_cmd(struct spi_nor *nor, u8 cmd, int len)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+ u32 reg, op_cfg = 0;
+
+ hisi_spi_nor_cmd_prepare(host, cmd, &op_cfg);
+
+ reg = FMC_CMD_CMD1(cmd);
+ writel(reg, host->regbase + FMC_CMD);
+
+ reg = OP_CFG_FM_CS(priv->chipselect);
+ if (op_cfg & FMC_OP_ADDR_EN)
+ reg |= OP_CFG_ADDR_NUM(nor->addr_width);
+ writel(reg, host->regbase + FMC_OP_CFG);
+
+ reg = FMC_DATA_NUM_CNT(len);
+ writel(reg, host->regbase + FMC_DATA_NUM);
+
+ writel(0xff, host->regbase + FMC_INT_CLR);
+ reg = op_cfg | FMC_OP_REG_OP_START;
+ writel(reg, host->regbase + FMC_OP);
+
+ return wait_op_finish(host);
+}
+
+static int hisi_spi_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+ int len)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+ int ret;
+
+ ret = hisi_spi_nor_send_cmd(nor, opcode, len);
+ if (ret)
+ return ret;
+
+ memcpy_fromio(buf, host->iobase, len);
+
+ return 0;
+}
+
+static int hisi_spi_nor_write_reg(struct spi_nor *nor, u8 opcode,
+ u8 *buf, int len)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+
+ if (len)
+ memcpy_toio(host->iobase, buf, len);
+
+ return hisi_spi_nor_send_cmd(nor, opcode, len);
+}
+
+static void hisi_spi_nor_dma_transfer(struct spi_nor *nor, loff_t start_off,
+ dma_addr_t dma_buf, size_t len, u8 op_type)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+ u8 if_type = 0, dummy = 0;
+ u8 w_cmd = 0, r_cmd = 0;
+ u32 reg;
+
+ writel(start_off, host->regbase + FMC_ADDRL);
+
+ if (op_type == FMC_OP_READ) {
+ if_type = get_if_type(nor->flash_read);
+ dummy = nor->read_dummy >> 3;
+ r_cmd = nor->read_opcode;
+ } else {
+ w_cmd = nor->program_opcode;
+ }
+
+ reg = OP_CFG_FM_CS(priv->chipselect)
+ | OP_CFG_MEM_IF_TYPE(if_type)
+ | OP_CFG_ADDR_NUM(nor->addr_width)
+ | OP_CFG_DUMMY_NUM(dummy);
+ writel(reg, host->regbase + FMC_OP_CFG);
+
+ reg = FMC_DMA_LEN_SET(len);
+ writel(reg, host->regbase + FMC_DMA_LEN);
+ writel(dma_buf, host->regbase + FMC_DMA_SADDR_D0);
+
+ reg = OP_CTRL_RD_OPCODE(r_cmd)
+ | OP_CTRL_WR_OPCODE(w_cmd)
+ | OP_CTRL_RW_OP(op_type)
+ | OP_CTRL_DMA_OP_READY;
+ writel(0xff, host->regbase + FMC_INT_CLR);
+ writel(reg, host->regbase + FMC_OP_DMA);
+ wait_op_finish(host);
+}
+
+static int hisi_spi_nor_read(struct spi_nor *nor, loff_t from, size_t len,
+ size_t *retlen, u_char *read_buf)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+ int offset;
+
+ /* read all bytes in only one read */
+ if (len <= HIFMC_DMA_MAX_LEN) {
+ hisi_spi_nor_dma_transfer(nor, from, host->dma_buffer,
+ len, FMC_OP_READ);
+ memcpy(read_buf, host->buffer, len);
+ } else {
+ /* read HIFMC_DMA_MAX_LEN bytes at a time */
+ for (offset = 0; offset < len; offset += HIFMC_DMA_MAX_LEN) {
+ hisi_spi_nor_dma_transfer(nor, from + offset,
+ host->dma_buffer,
+ HIFMC_DMA_MAX_LEN, FMC_OP_READ);
+ memcpy(read_buf + offset,
+ host->buffer, HIFMC_DMA_MAX_LEN);
+ }
+ /* read remaining bytes */
+ offset -= HIFMC_DMA_MAX_LEN;
+ hisi_spi_nor_dma_transfer(nor, from + offset, host->dma_buffer,
+ len - offset, FMC_OP_READ);
+ memcpy(read_buf + offset, host->buffer, len - offset);
+ }
+ *retlen = len;
+
+ return 0;
+}
+
+static void hisi_spi_nor_write(struct spi_nor *nor, loff_t to,
+ size_t len, size_t *retlen, const u_char *write_buf)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+
+ /* len is smaller than HIFMC_DMA_MAX_LEN*/
+ memcpy(host->buffer, write_buf, len);
+ hisi_spi_nor_dma_transfer(nor, to, host->dma_buffer, len,
+ FMC_OP_WRITE);
+
+ *retlen = len;
+}
+
+static int hisi_spi_nor_erase(struct spi_nor *nor, loff_t offs)
+{
+ struct hifmc_priv *priv = nor->priv;
+ struct hifmc_host *host = priv->host;
+
+ writel(offs, host->regbase + FMC_ADDRL);
+
+ return hisi_spi_nor_send_cmd(nor, nor->erase_opcode, 0);
+}
+
+/**
+ * Get spi flash device information and register it as a mtd device.
+ */
+static int hisi_spi_nor_register(struct device_node *np,
+ struct hifmc_host *host)
+{
+ struct device *dev = host->dev;
+ struct spi_nor *nor;
+ struct hifmc_priv *priv;
+ struct mtd_info *mtd;
+ int ret;
+
+ nor = devm_kzalloc(dev, sizeof(*nor), GFP_KERNEL);
+ if (!nor)
+ return -ENOMEM;
+
+ nor->dev = dev;
+ spi_nor_set_flash_node(nor, np);
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "reg", &priv->chipselect);
+ if (ret) {
+ dev_err(dev, "There's no reg property for %s\n",
+ np->full_name);
+ return ret;
+ }
+
+ ret = of_property_read_u32(np, "spi-max-frequency",
+ &priv->clkrate);
+ if (ret) {
+ dev_err(dev, "There's no spi-max-frequency property for %s\n",
+ np->full_name);
+ return ret;
+ }
+ priv->host = host;
+ nor->priv = priv;
+
+ nor->prepare = hisi_spi_nor_prep;
+ nor->unprepare = hisi_spi_nor_unprep;
+ nor->read_reg = hisi_spi_nor_read_reg;
+ nor->write_reg = hisi_spi_nor_write_reg;
+ nor->read = hisi_spi_nor_read;
+ nor->write = hisi_spi_nor_write;
+ nor->erase = hisi_spi_nor_erase;
+ ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+ if (ret)
+ return ret;
+
+ mtd = &nor->mtd;
+ mtd->name = np->name;
+ ret = mtd_device_register(mtd, NULL, 0);
+ if (ret)
+ return ret;
+
+ host->nor[host->num_chip] = nor;
+ host->num_chip++;
+ return 0;
+}
+
+static void hisi_spi_nor_unregister_all(struct hifmc_host *host)
+{
+ int i;
+
+ for (i = 0; i < host->num_chip; i++)
+ mtd_device_unregister(&host->nor[i]->mtd);
+}
+
+static int hisi_spi_nor_register_all(struct hifmc_host *host)
+{
+ struct device *dev = host->dev;
+ struct device_node *np;
+ int ret;
+
+ for_each_available_child_of_node(dev->of_node, np) {
+ ret = hisi_spi_nor_register(np, host);
+ if (ret)
+ goto fail;
+
+ if (host->num_chip == HIFMC_MAX_CHIP_NUM) {
+ dev_warn(dev, "Flash device number exceeds the maximum chipselect number\n");
+ break;
+ }
+ }
+
+ return 0;
+
+fail:
+ hisi_spi_nor_unregister_all(host);
+ return ret;
+}
+
+static int hisi_spi_nor_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct hifmc_host *host;
+ int ret;
+
+ host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+ if (!host)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, host);
+ host->dev = dev;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
+ host->regbase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(host->regbase))
+ return PTR_ERR(host->regbase);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "memory");
+ host->iobase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(host->iobase))
+ return PTR_ERR(host->iobase);
+
+ host->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(host->clk))
+ return PTR_ERR(host->clk);
+
+ ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_warn(dev, "Unable to set dma mask\n");
+ return ret;
+ }
+
+ host->buffer = dmam_alloc_coherent(dev, HIFMC_DMA_MAX_LEN,
+ &host->dma_buffer, GFP_KERNEL);
+ if (!host->buffer)
+ return -ENOMEM;
+
+ mutex_init(&host->lock);
+ clk_prepare_enable(host->clk);
+ hisi_spi_nor_init(host);
+ ret = hisi_spi_nor_register_all(host);
+ if (ret)
+ mutex_destroy(&host->lock);
+
+ clk_disable_unprepare(host->clk);
+ return ret;
+}
+
+static int hisi_spi_nor_remove(struct platform_device *pdev)
+{
+ struct hifmc_host *host = platform_get_drvdata(pdev);
+
+ hisi_spi_nor_unregister_all(host);
+ mutex_destroy(&host->lock);
+ clk_disable_unprepare(host->clk);
+ return 0;
+}
+
+static const struct of_device_id hisi_spi_nor_dt_ids[] = {
+ { .compatible = "hisilicon,fmc-spi-nor"},
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, hisi_spi_nor_dt_ids);
+
+static struct platform_driver hisi_spi_nor_driver = {
+ .driver = {
+ .name = "hisi-sfc",
+ .of_match_table = hisi_spi_nor_dt_ids,
+ },
+ .probe = hisi_spi_nor_probe,
+ .remove = hisi_spi_nor_remove,
+};
+module_platform_driver(hisi_spi_nor_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("HiSilicon SPI Nor Flash Controller Driver");
--
1.9.1
Hi Jiancheng,
Le 19/04/2016 09:27, Jiancheng Xue a ?crit :
> From: Jiancheng Xue <[email protected]>
>
> Add hisilicon spi-nor flash controller driver
[...]
> +enum hifmc_iftype {
> + IF_TYPE_STD,
> + IF_TYPE_DUAL,
> + IF_TYPE_DIO,
> + IF_TYPE_QUAD,
> + IF_TYPE_QIO,
> +};
Just for my own information, the hisilicon controller supports:
- SPI 1-1-1 : IF_TYPE_STD
- SPI 1-1-2 : IF_TYPE_DUAL
- SPI 1-2-2 : IF_TYPE_DIO
- SPI 1-1-4 : IF_TYPE_QUAD
- SPI 1-4-4 : IF_TYPE_QIO
but not the SPI protocols 2-2-2 or 4-4-4, does it?
If I ask you this question, it's because I wonder how to make the difference
between SPI controllers supporting both SPI 1-4-4 and 4-4-4 and those
supporting only SPI 1-4-4 in the case they rely on the m25p80 driver as an
adaptation layer between the spi-nor framework et the spi framework.
I guess the "spi-tx-bus-width" and "spi-rx-bus-width" DT properties are not
enough to make the difference between these two kinds of SPI controller.
I understand your driver doesn't use the m25p80 driver as it directly calls
spi_nor_scan() and implements the .read_reg, .write_reg, .read, .write and
.erase hooks, but its a general question :)
> +static int get_if_type(enum read_mode flash_read)
> +{
> + enum hifmc_iftype if_type;
> +
> + switch (flash_read) {
> + case SPI_NOR_DUAL:
> + if_type = IF_TYPE_DUAL;
> + break;
> + case SPI_NOR_QUAD:
> + if_type = IF_TYPE_QUAD;
> + break;
> + case SPI_NOR_NORMAL:
> + case SPI_NOR_FAST:
> + default:
> + if_type = IF_TYPE_STD;
> + break;
> + }
> +
> + return if_type;
> +}
Here I understand your QSPI controller could support SPI 1-4-4 and SPI 1-2-2
but you limit it to SPI 1-1-4 and SPI 1-1-2 because the spi-nor framework
doesn't support those protocols yet.
[...]
> +static void hisi_spi_nor_cmd_prepare(struct hifmc_host *host, u8 cmd,
> + u32 *opcfg)
> +{
> + u32 reg;
> +
> + *opcfg |= FMC_OP_CMD1_EN;
> + switch (cmd) {
> + case SPINOR_OP_RDID:
> + case SPINOR_OP_RDSR:
> + case SPINOR_OP_RDCR:
> + *opcfg |= FMC_OP_READ_DATA_EN;
> + break;
> + case SPINOR_OP_WREN:
> + reg = readl(host->regbase + FMC_GLOBAL_CFG);
> + if (reg & FMC_GLOBAL_CFG_WP_ENABLE) {
> + reg &= ~FMC_GLOBAL_CFG_WP_ENABLE;
> + writel(reg, host->regbase + FMC_GLOBAL_CFG);
> + }
> + break;
> + case SPINOR_OP_WRSR:
> + *opcfg |= FMC_OP_WRITE_DATA_EN;
> + break;
> + case SPINOR_OP_BE_4K:
> + case SPINOR_OP_BE_4K_PMC:
> + case SPINOR_OP_SE_4B:
> + case SPINOR_OP_SE:
> + *opcfg |= FMC_OP_ADDR_EN;
> + break;
> + case SPINOR_OP_EN4B:
> + reg = readl(host->regbase + FMC_CFG);
> + reg |= SPI_NOR_ADDR_MODE;
> + writel(reg, host->regbase + FMC_CFG);
> + break;
> + case SPINOR_OP_EX4B:
> + reg = readl(host->regbase + FMC_CFG);
> + reg &= ~SPI_NOR_ADDR_MODE;
> + writel(reg, host->regbase + FMC_CFG);
> + break;
> + case SPINOR_OP_CHIP_ERASE:
> + default:
> + break;
> + }
> +}
IMHO, this is exactly what should be avoided in (Q)SPI controller drivers:
they should not analyse the command op code to guess some settings such as
the SPI protocol to be used or in your case whether some address or data
cycles are included inside the command.
Why? Just because your driver won't know what to do when we introduce new
op codes. Right now, hisi_spi_nor_cmd_prepare() is not aware of op code 0x15
use by Macronix to read its Configuration Register or Micron op codes to
read/write their Volatile Configuration Register and Enhanced Volatile
Configuration Register. So your driver won't support those memories any longer
when new features using those registers will be added in the spi-nor framework.
Since your driver implements struct spi_nor hooks, here is what you could do
instead:
1 - hisi_spi_nor_read_reg(..., u8 *buf, int len)
if buf != NULL then you should set FMC_OP_READ_DATA_EN, don't set it otherwise.
2 - hisi_spi_nor_write_reg(..., u8 *buf, int len)
buf should always be != NULL so I guess you can always set FMC_OP_WRITE_DATA_EN
For the special case of SPINOR_OP_WREN (Write Enable), your driver seems
to clear the FMC_GLOBAL_CFG_WP_ENABLE bit from the FMC_GLOBAL_CFG register, if
set, but never set it again... So why not clear this bit once for all?
Reading the current source code, the support of the hardware write protect
feature is a little bit odd, isn't it?
3 - hisi_spi_nor_read(struct spi_nor *nor, ...)
your driver doesn't seem to call hisi_spi_nor_send_cmd() /
hisi_spi_nor_cmd_prepare() from _read() so I don't know whether you have to
set the FMC_OP_READ_DATA_EN bit here.
Also you can check nor->addr_width to know whether you need SPI_NOR_ADDR_MODE.
4 - hisi_spi_nor_write(struct spi_nor *nor, ...)
Almost the same comment as for _read(): just replace FMC_OP_READ_DATA_EN by
FMC_OP_WRITE_DATA_EN I guess.
5 - hisi_spi_nor_erase(struct spi_nor *nor, ...)
Here again you should check nor->addr_width to know when to set the
SPI_NOR_ADDR_MODE bit in the FMC_CFG register.
The FMC_OP_ADDR_EN bit should always be set for erase operations.
> +static int hisi_spi_nor_send_cmd(struct spi_nor *nor, u8 cmd, int len)
> +{
> + struct hifmc_priv *priv = nor->priv;
> + struct hifmc_host *host = priv->host;
> + u32 reg, op_cfg = 0;
> +
> + hisi_spi_nor_cmd_prepare(host, cmd, &op_cfg);
[...]
Just get rid of hisi_spi_nor_cmd_prepare(), it would simplify the support of
your driver a lot. For sure, I don't know the hisilicon spi-nor flash
controller as much as you do but I'm pretty sure its driver doesn't need to
analyse the op code to guess some hardware settings. There are other means to
find out these settings.
Anyway, I hope these few comments will help you.
Best regards,
Cyrille
Hi Jiancheng,
Cyrille hit on one of my concerns; if posible we'd like not to have you
sniffing the opcodes in read_reg()/write_reg(). But let's keep the
discussion on that thread.
Two other comments below.
On Tue, Apr 19, 2016 at 03:27:19PM +0800, Jiancheng Xue wrote:
> From: Jiancheng Xue <[email protected]>
>
> Add hisilicon spi-nor flash controller driver
>
> Signed-off-by: Binquan Peng <[email protected]>
> Signed-off-by: Jiancheng Xue <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> change log
> v10:
> Fixed issues pointed by Marek Vasut.
> 1)Droped the underscores in the argument names of some macros' definition.
> 2)Changed some varibles to correct type.
> 3)Rewrote hisi_spi_nor_read/write for readability.
> 4)Added new functions hisi_spi_nor_register/unregister_all
> 5)Changed to dynamically allocation for spi_nor embeded in hifmc_host.
> Fixed issues pointed by Brian Norris.
> 1)Replaced some headers with more accurate ones.
> v9:
> Fixed issues pointed by Jagan Teki.
> v8:
> Fixed issues pointed by Ezequiel Garcia and Brian Norris.
> Moved dts binding file to mtd directory.
> Changed the compatible string more specific.
> v7:
> Rebased to v4.5-rc3.
> Fixed issues pointed by Ezequiel Garcia.
> v6:
> Based on v4.5-rc2
> Fixed issues pointed by Ezequiel Garcia.
> v5:
> Fixed a compile error.
> v4:
> Rebased to v4.5-rc1
> v3:
> Added a compatible string "hisilicon,hi3519-sfc".
> v2:
> Fixed some compiling warings.
>
> .../bindings/mtd/hisilicon,fmc-spi-nor.txt | 24 +
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/hisi-sfc.c | 539 +++++++++++++++++++++
> 4 files changed, 571 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
> create mode 100644 drivers/mtd/spi-nor/hisi-sfc.c
>
> diff --git a/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt b/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
> new file mode 100644
> index 0000000..7498152
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/hisilicon,fmc-spi-nor.txt
> @@ -0,0 +1,24 @@
> +HiSilicon SPI-NOR Flash Controller
> +
> +Required properties:
> +- compatible : Should be "hisilicon,fmc-spi-nor" and one of the following strings:
> + "hisilicon,hi3519-spi-nor"
> +- address-cells : Should be 1.
> +- size-cells : Should be 0.
> +- reg : Offset and length of the register set for the controller device.
> +- reg-names : Must include the following two entries: "control", "memory".
> +- clocks : handle to spi-nor flash controller clock.
> +
> +Example:
> +spi-nor-controller@10000000 {
> + compatible = "hisilicon,hi3519-spi-nor", "hisilicon,fmc-spi-nor";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x10000000 0x1000>, <0x14000000 0x1000000>;
> + reg-names = "control", "memory";
> + clocks = <&clock HI3519_FMC_CLK>;
> + spi-nor@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + };
> +};
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index d42c98e..b9ff675 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -38,6 +38,13 @@ config SPI_FSL_QUADSPI
> This controller does not support generic SPI. It only supports
> SPI NOR.
>
> +config SPI_HISI_SFC
> + tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
> + depends on ARCH_HISI || COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> + This enables support for hisilicon SPI-NOR flash controller.
> +
> config SPI_NXP_SPIFI
> tristate "NXP SPI Flash Interface (SPIFI)"
> depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 0bf3a7f8..8a6fa69 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> +obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
> obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c
> new file mode 100644
> index 0000000..942dafd
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/hisi-sfc.c
> @@ -0,0 +1,539 @@
[...]
> +static void hisi_spi_nor_dma_transfer(struct spi_nor *nor, loff_t start_off,
> + dma_addr_t dma_buf, size_t len, u8 op_type)
> +{
> + struct hifmc_priv *priv = nor->priv;
> + struct hifmc_host *host = priv->host;
> + u8 if_type = 0, dummy = 0;
> + u8 w_cmd = 0, r_cmd = 0;
> + u32 reg;
> +
> + writel(start_off, host->regbase + FMC_ADDRL);
> +
> + if (op_type == FMC_OP_READ) {
> + if_type = get_if_type(nor->flash_read);
> + dummy = nor->read_dummy >> 3;
> + r_cmd = nor->read_opcode;
> + } else {
> + w_cmd = nor->program_opcode;
> + }
> +
> + reg = OP_CFG_FM_CS(priv->chipselect)
> + | OP_CFG_MEM_IF_TYPE(if_type)
> + | OP_CFG_ADDR_NUM(nor->addr_width)
> + | OP_CFG_DUMMY_NUM(dummy);
> + writel(reg, host->regbase + FMC_OP_CFG);
> +
> + reg = FMC_DMA_LEN_SET(len);
> + writel(reg, host->regbase + FMC_DMA_LEN);
> + writel(dma_buf, host->regbase + FMC_DMA_SADDR_D0);
> +
> + reg = OP_CTRL_RD_OPCODE(r_cmd)
> + | OP_CTRL_WR_OPCODE(w_cmd)
> + | OP_CTRL_RW_OP(op_type)
> + | OP_CTRL_DMA_OP_READY;
> + writel(0xff, host->regbase + FMC_INT_CLR);
> + writel(reg, host->regbase + FMC_OP_DMA);
> + wait_op_finish(host);
Do you want to return the status here, so we can handle errors?
> +}
> +
> +static int hisi_spi_nor_read(struct spi_nor *nor, loff_t from, size_t len,
> + size_t *retlen, u_char *read_buf)
> +{
> + struct hifmc_priv *priv = nor->priv;
> + struct hifmc_host *host = priv->host;
> + int offset;
> +
> + /* read all bytes in only one read */
> + if (len <= HIFMC_DMA_MAX_LEN) {
> + hisi_spi_nor_dma_transfer(nor, from, host->dma_buffer,
> + len, FMC_OP_READ);
> + memcpy(read_buf, host->buffer, len);
Why do you have a special case for "all in one read"? This is just a
basic loop...
> + } else {
> + /* read HIFMC_DMA_MAX_LEN bytes at a time */
> + for (offset = 0; offset < len; offset += HIFMC_DMA_MAX_LEN) {
> + hisi_spi_nor_dma_transfer(nor, from + offset,
> + host->dma_buffer,
> + HIFMC_DMA_MAX_LEN, FMC_OP_READ);
> + memcpy(read_buf + offset,
> + host->buffer, HIFMC_DMA_MAX_LEN);
> + }
> + /* read remaining bytes */
> + offset -= HIFMC_DMA_MAX_LEN;
> + hisi_spi_nor_dma_transfer(nor, from + offset, host->dma_buffer,
> + len - offset, FMC_OP_READ);
> + memcpy(read_buf + offset, host->buffer, len - offset);
> + }
I think the entire if/else can be rewritten as:
for (offset = 0; offset < len; offset += HIFMC_DMA_MAX_LEN) {
size_t trans = min(HIFMC_DMA_MAX_LEN, len - offset);
hisi_spi_nor_dma_transfer(nor, from + offset, host->dma_buffer,
trans, FMC_OP_READ);
memcpy(read_buf + offset, host->buffer, trans);
}
> + *retlen = len;
> +
> + return 0;
> +}
> +
> +static void hisi_spi_nor_write(struct spi_nor *nor, loff_t to,
> + size_t len, size_t *retlen, const u_char *write_buf)
> +{
> + struct hifmc_priv *priv = nor->priv;
> + struct hifmc_host *host = priv->host;
> +
> + /* len is smaller than HIFMC_DMA_MAX_LEN*/
Where do you get that assumption? This function must handle whatever
'len' is passed to it, and that may be large. (I suspect your error is
inspired by the fact that mtd-utils *usually* does smaller write
transfers.)
By the way, you might be helped by this series:
http://lists.infradead.org/pipermail/linux-mtd/2015-December/063865.html
It got dropped on the floor, but I'm rewriting it to resend soon.
> + memcpy(host->buffer, write_buf, len);
> + hisi_spi_nor_dma_transfer(nor, to, host->dma_buffer, len,
> + FMC_OP_WRITE);
> +
> + *retlen = len;
> +}
[...]
Brian
Hi Brian,
On 2016/4/27 23:38, Brian Norris wrote:
> Hi Jiancheng,
>
> Cyrille hit on one of my concerns; if posible we'd like not to have you
> sniffing the opcodes in read_reg()/write_reg(). But let's keep the
> discussion on that thread.
>
OK. I'll look into this issue seriously and reply on that thread later.
> Two other comments below.
>
> On Tue, Apr 19, 2016 at 03:27:19PM +0800, Jiancheng Xue wrote:
>> From: Jiancheng Xue <[email protected]>
>>
>> Add hisilicon spi-nor flash controller driver
>>
>> Signed-off-by: Binquan Peng <[email protected]>
>> Signed-off-by: Jiancheng Xue <[email protected]>
>> Acked-by: Rob Herring <[email protected]>
>> Reviewed-by: Ezequiel Garcia <[email protected]>
>> ---
[...]
>> diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c
>> new file mode 100644
>> index 0000000..942dafd
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/hisi-sfc.c
>> @@ -0,0 +1,539 @@
>
> [...]
>
>> +static void hisi_spi_nor_dma_transfer(struct spi_nor *nor, loff_t start_off,
>> + dma_addr_t dma_buf, size_t len, u8 op_type)
>> +{
>> + struct hifmc_priv *priv = nor->priv;
>> + struct hifmc_host *host = priv->host;
>> + u8 if_type = 0, dummy = 0;
>> + u8 w_cmd = 0, r_cmd = 0;
>> + u32 reg;
>> +
>> + writel(start_off, host->regbase + FMC_ADDRL);
>> +
>> + if (op_type == FMC_OP_READ) {
>> + if_type = get_if_type(nor->flash_read);
>> + dummy = nor->read_dummy >> 3;
>> + r_cmd = nor->read_opcode;
>> + } else {
>> + w_cmd = nor->program_opcode;
>> + }
>> +
>> + reg = OP_CFG_FM_CS(priv->chipselect)
>> + | OP_CFG_MEM_IF_TYPE(if_type)
>> + | OP_CFG_ADDR_NUM(nor->addr_width)
>> + | OP_CFG_DUMMY_NUM(dummy);
>> + writel(reg, host->regbase + FMC_OP_CFG);
>> +
>> + reg = FMC_DMA_LEN_SET(len);
>> + writel(reg, host->regbase + FMC_DMA_LEN);
>> + writel(dma_buf, host->regbase + FMC_DMA_SADDR_D0);
>> +
>> + reg = OP_CTRL_RD_OPCODE(r_cmd)
>> + | OP_CTRL_WR_OPCODE(w_cmd)
>> + | OP_CTRL_RW_OP(op_type)
>> + | OP_CTRL_DMA_OP_READY;
>> + writel(0xff, host->regbase + FMC_INT_CLR);
>> + writel(reg, host->regbase + FMC_OP_DMA);
>> + wait_op_finish(host);
>
> Do you want to return the status here, so we can handle errors?
>
OK. I'll fix it in v11.
>> +}
>> +
>> +static int hisi_spi_nor_read(struct spi_nor *nor, loff_t from, size_t len,
>> + size_t *retlen, u_char *read_buf)
>> +{
>> + struct hifmc_priv *priv = nor->priv;
>> + struct hifmc_host *host = priv->host;
>> + int offset;
>> +
>> + /* read all bytes in only one read */
>> + if (len <= HIFMC_DMA_MAX_LEN) {
>> + hisi_spi_nor_dma_transfer(nor, from, host->dma_buffer,
>> + len, FMC_OP_READ);
>> + memcpy(read_buf, host->buffer, len);
>
> Why do you have a special case for "all in one read"? This is just a
> basic loop...
>
See below.
>> + } else {
>> + /* read HIFMC_DMA_MAX_LEN bytes at a time */
>> + for (offset = 0; offset < len; offset += HIFMC_DMA_MAX_LEN) {
>> + hisi_spi_nor_dma_transfer(nor, from + offset,
>> + host->dma_buffer,
>> + HIFMC_DMA_MAX_LEN, FMC_OP_READ);
>> + memcpy(read_buf + offset,
>> + host->buffer, HIFMC_DMA_MAX_LEN);
>> + }
>> + /* read remaining bytes */
>> + offset -= HIFMC_DMA_MAX_LEN;
>> + hisi_spi_nor_dma_transfer(nor, from + offset, host->dma_buffer,
>> + len - offset, FMC_OP_READ);
>> + memcpy(read_buf + offset, host->buffer, len - offset);
>> + }
>
> I think the entire if/else can be rewritten as:
>
> for (offset = 0; offset < len; offset += HIFMC_DMA_MAX_LEN) {
> size_t trans = min(HIFMC_DMA_MAX_LEN, len - offset);
>
> hisi_spi_nor_dma_transfer(nor, from + offset, host->dma_buffer,
> trans, FMC_OP_READ);
> memcpy(read_buf + offset, host->buffer, trans);
> }
>
I had referred to the implementation of spi_nor_write. But now it seems much
simpler and clearer above. I'll apply this code in v11. Thank you very much.
>> + *retlen = len;
>> +
>> + return 0;
>> +}
>> +
>> +static void hisi_spi_nor_write(struct spi_nor *nor, loff_t to,
>> + size_t len, size_t *retlen, const u_char *write_buf)
>> +{
>> + struct hifmc_priv *priv = nor->priv;
>> + struct hifmc_host *host = priv->host;
>> +
>> + /* len is smaller than HIFMC_DMA_MAX_LEN*/
>
> Where do you get that assumption? This function must handle whatever
> 'len' is passed to it, and that may be large. (I suspect your error is
> inspired by the fact that mtd-utils *usually* does smaller write
> transfers.)
>
In spi_nor_write, nor->write is called with the len not bigger than nor->page_size.
I wrongly thought nor->page_size was always smaller than HIFMC_DMA_MAX_LEN.
I'll rewrite this function referring to _read function. Thank you!
Regards,
Jiancheng