From: VIET NGA DAO <[email protected]>
Altera Quad SPI Controller is a soft IP which enables access to
Altera EPCS, EPCQ and Mircon flash chips. This patch adds driver
for these devices.
Signed-off-by: VIET NGA DAO <[email protected]>
---
v4:
- Add more flash devices support ( EPCQL and Micron)
- Remove redundant messages
- Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
- Replace get_flash_name to altera_quadspi_scan
- Remove clk related parts
- Remove altera_quadspi_plat
- Change device tree reg name and remove opcode-id
v3:
- Change altera_epcq driver name to altera_quadspi for more generic name
- Implement flash name searching in altera_quadspi.c instead of spi-nor
- Edit the altra quadspi info table in spi-nor
- Remove wait_til_ready in all read,write, erase, lock, unlock functions
- Merge .h and .c into 1 file
v2:
- Change to spi_nor structure
- Add lock and unlock functions for spi_nor
- Simplify the altera_epcq_lock function
- Replace reg by compatible in device tree
---
.../devicetree/bindings/mtd/altera-quadspi.txt | 49 ++
drivers/mtd/spi-nor/Kconfig | 8 +
drivers/mtd/spi-nor/Makefile | 1 +
drivers/mtd/spi-nor/altera-quadspi.c | 568 ++++++++++++++++++++
drivers/mtd/spi-nor/spi-nor.c | 30 +
5 files changed, 656 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/altera-quadspi.txt
create mode 100644 drivers/mtd/spi-nor/altera-quadspi.c
diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
new file mode 100644
index 0000000..2873319
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
@@ -0,0 +1,49 @@
+* MTD Altera QUADSPI driver
+
+Required properties:
+- compatible: Should be "altr,quadspi-1.0"
+- reg: Address and length of the register set for the device. It contains
+ the information of registers in the same order as described by reg-names
+- reg-names: Should contain the reg names
+ "avl_csr": Should contain the register configuration base address
+ "avl_mem": Should contain the data base address
+- #address-cells: Must be <1>.
+- #size-cells: Must be <0>.
+- flash device tree subnode, there must be a node with the following fields:
+ - compatible: Should contain the flash name:
+ 1. EPCS: epcs16, epcs64, epcs128
+ 2. EPCQ: epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
+ 3. EPCQ-L: epcql256, epcql512, epcql1024
+ 4. Mircon: n25q016-nonjedec, n25q032-nonjedec, n25q064-nonjedec,
+ n25q128a13-nonjedec, n25q128a11-nonjedec, n25q256a-nonjedec,
+ n25q256a11-nonjedec, n25q512a-nonjedec, n25q512ax3-nonjedec,
+ mt25ql512-nonjedec, n25q00-nonjedec, n25q00a11-nonjedec
+ - #address-cells: please refer to /mtd/partition.txt
+ - #size-cells: please refer to /mtd/partition.txt
+ For partitions inside each flash, please refer to /mtd/partition.txt
+
+Example:
+
+ quadspi_controller_0: quadspi@0x180014a0 {
+ compatible = "altr,quadspi-1.0";
+ reg = <0x180014a0 0x00000020>,
+ <0x14000000 0x04000000>;
+ reg-names = "avl_csr", "avl_mem";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ flash0: epcq256@0 {
+ compatible = "altr,epcq256";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ partition@0 {
+ /* 16 MB for raw data. */
+ label = "EPCQ Flash 0 raw data";
+ reg = <0x0 0x1000000>;
+ };
+ partition@1000000 {
+ /* 16 MB for jffs2 data. */
+ label = "EPCQ Flash 0 JFFS 2";
+ reg = <0x1000000 0x1000000>;
+ };
+ };
+ }; //end quadspi@0x180014a0 (quadspi_controller_0)
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 64a4f0e..678dbe3 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -28,4 +28,12 @@ config SPI_FSL_QUADSPI
This enables support for the Quad SPI controller in master mode.
We only connect the NOR to this controller now.
+config SPI_ALTERA_QUADSPI
+ tristate "Altera Generic Quad SPI Controller"
+ depends on OF
+ help
+ This enables access to Altera EPCQ/EPCS/Micron flash chips,
+ used for data storage. See the driver source for the current list,
+ or to add other chips.
+
endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 6a7ce14..4e700df 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
+obj-$(CONFIG_SPI_ALTERA_QUADSPI) += altera-quadspi.o
diff --git a/drivers/mtd/spi-nor/altera-quadspi.c b/drivers/mtd/spi-nor/altera-quadspi.c
new file mode 100644
index 0000000..8b113df
--- /dev/null
+++ b/drivers/mtd/spi-nor/altera-quadspi.c
@@ -0,0 +1,568 @@
+/*
+ * Copyright (C) 2014 Altera Corporation. All rights reserved
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+
+#define ALTERA_QUADSPI_RESOURCE_NAME "altera_quadspi"
+
+/* max possible slots for serial flash chip in the QUADSPI controller */
+#define MAX_NUM_FLASH_CHIP 3
+
+#define EPCS_OPCODE_ID 1
+#define NON_EPCS_OPCODE_ID 2
+
+#define WRITE_CHECK 1
+#define ERASE_CHECK 0
+
+/* Define max times to check status register before we give up. */
+#define QUADSPI_MAX_TIME_OUT (40 * HZ)
+
+/* defines for status register */
+#define QUADSPI_SR_REG 0x0
+#define QUADSPI_SR_WIP_MASK 0x00000001
+#define QUADSPI_SR_WIP 0x1
+#define QUADSPI_SR_WEL 0x2
+#define QUADSPI_SR_BP0 0x4
+#define QUADSPI_SR_BP1 0x8
+#define QUADSPI_SR_BP2 0x10
+#define QUADSPI_SR_BP3 0x40
+#define QUADSPI_SR_TB 0x20
+#define QUADSPI_SR_MASK 0x0000000F
+
+/* defines for device id register */
+#define QUADSPI_SID_REG 0x4
+#define QUADSPI_RDID_REG 0x8
+#define QUADSPI_ID_MASK 0x000000FF
+
+/*
+ * QUADSPI_MEM_OP register offset
+ *
+ * The QUADSPI_MEM_OP register is used to do memory protect and erase operations
+ *
+ */
+#define QUADSPI_MEM_OP_REG 0xC
+
+#define QUADSPI_MEM_OP_CMD_MASK 0x00000003
+#define QUADSPI_MEM_OP_BULK_ERASE_CMD 0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE_CMD 0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT_CMD 0x00000003
+#define QUADSPI_MEM_OP_SECTOR_VALUE_MASK 0x0003FF00
+#define QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK 0x00001F00
+#define QUADSPI_MEM_OP_SECTOR_PROTECT_SHIFT 8
+/*
+ * QUADSPI_ISR register offset
+ *
+ * The QUADSPI_ISR register is used to determine whether an invalid write or
+ * erase operation trigerred an interrupt
+ *
+ */
+#define QUADSPI_ISR_REG 0x10
+
+#define QUADSPI_ISR_ILLEGAL_ERASE_MASK 0x00000001
+#define QUADSPI_ISR_ILLEGAL_WRITE_MASK 0x00000002
+
+/*
+ * QUADSPI_IMR register offset
+ *
+ * The QUADSPI_IMR register is used to mask the invalid erase or the invalid
+ * write interrupts.
+ *
+ */
+#define QUADSPI_IMR_REG 0x14
+#define QUADSPI_IMR_ILLEGAL_ERASE_MASK 0x00000001
+
+#define QUADSPI_IMR_ILLEGAL_WRITE_MASK 0x00000002
+
+#define QUADSPI_CHIP_SELECT_REG 0x18
+#define QUADSPI_CHIP_SELECT_MASK 0x00000007
+#define QUADSPI_CHIP_SELECT_0 0x00000001
+#define QUADSPI_CHIP_SELECT_1 0x00000002
+#define QUADSPI_CHIP_SELECT_2 0x00000004
+
+struct altera_quadspi {
+ u32 opcode_id;
+ void __iomem *csr_base;
+ void __iomem *data_base;
+ u32 num_flashes;
+ struct device *dev;
+ struct altera_quadspi_flash *flash[MAX_NUM_FLASH_CHIP];
+ struct device_node *np[MAX_NUM_FLASH_CHIP];
+};
+
+struct altera_quadspi_flash {
+ struct mtd_info mtd;
+ struct spi_nor nor;
+ struct altera_quadspi *q;
+};
+
+struct flash_device {
+ char *name;
+ u32 opcode_id;
+ u32 device_id;
+};
+
+#define FLASH_ID(_n, _opcode_id, _id) \
+{ \
+ .name = (_n), \
+ .opcode_id = (_opcode_id), \
+ .device_id = (_id), \
+}
+
+static struct flash_device flash_devices[] = {
+ FLASH_ID("epcs16", EPCS_OPCODE_ID, 0x14),
+ FLASH_ID("epcs64", EPCS_OPCODE_ID, 0x16),
+ FLASH_ID("epcs128", EPCS_OPCODE_ID, 0x18),
+
+ FLASH_ID("epcq16", NON_EPCS_OPCODE_ID, 0x15),
+ FLASH_ID("epcq32", NON_EPCS_OPCODE_ID, 0x16),
+ FLASH_ID("epcq64", NON_EPCS_OPCODE_ID, 0x17),
+ FLASH_ID("epcq128", NON_EPCS_OPCODE_ID, 0x18),
+ FLASH_ID("epcq256", NON_EPCS_OPCODE_ID, 0x19),
+ FLASH_ID("epcq512", NON_EPCS_OPCODE_ID, 0x20),
+ FLASH_ID("epcq1024", NON_EPCS_OPCODE_ID, 0x21),
+
+ FLASH_ID("epcql256", NON_EPCS_OPCODE_ID, 0x19),
+ FLASH_ID("epcql512", NON_EPCS_OPCODE_ID, 0x20),
+ FLASH_ID("epcql1024", NON_EPCS_OPCODE_ID, 0x21),
+
+ FLASH_ID("n25q016-nonjedec", NON_EPCS_OPCODE_ID, 0x15),
+ FLASH_ID("n25q032-nonjedec", NON_EPCS_OPCODE_ID, 0x16),
+ FLASH_ID("n25q064-nonjedec", NON_EPCS_OPCODE_ID, 0x17),
+ FLASH_ID("n25q128a13-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
+ FLASH_ID("n25q128a11-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
+ FLASH_ID("n25q256a-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
+ FLASH_ID("n25q256a11-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
+ FLASH_ID("n25q512a-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
+ FLASH_ID("n25q512ax3-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
+ FLASH_ID("mt25ql512-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
+ FLASH_ID("n25q00-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
+ FLASH_ID("n25q00a11-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
+
+};
+
+static int altera_quadspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
+ int len, int wr_en)
+{
+ return 0;
+}
+
+static int altera_quadspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
+ int len)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ u32 data = 0;
+
+ memset(val, 0, len);
+
+ switch (opcode) {
+ case SPINOR_OP_RDSR:
+ data = readl(q->csr_base + QUADSPI_SR_REG);
+ *val = (u8)data & QUADSPI_SR_MASK;
+ break;
+ case SPINOR_OP_RDID:
+ if (q->opcode_id == EPCS_OPCODE_ID)
+ data = readl(q->csr_base + QUADSPI_SID_REG);
+ else
+ data = readl(q->csr_base + QUADSPI_RDID_REG);
+
+ *val = (u8)data & QUADSPI_ID_MASK; /* device id */
+ break;
+ default:
+ *val = 0;
+ break;
+ }
+ return 0;
+}
+
+static int altera_quadspi_write_erase_check(struct spi_nor *nor,
+ bool write_erase)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ u32 val;
+ u32 mask;
+
+ if (write_erase)
+ mask = QUADSPI_ISR_ILLEGAL_WRITE_MASK;
+ else
+ mask = QUADSPI_ISR_ILLEGAL_ERASE_MASK;
+
+ val = readl(q->csr_base + QUADSPI_ISR_REG);
+ if (val & mask) {
+ dev_err(nor->dev,
+ "write/erase failed, sector might be protected\n");
+ /* clear this status for next use */
+ writel(val, q->csr_base + QUADSPI_ISR_REG);
+ return -EIO;
+ }
+ return 0;
+}
+
+static int altera_quadspi_addr_to_sector(struct mtd_info *mtd, uint64_t offset)
+{
+ if (mtd->erasesize_shift)
+ return offset >> mtd->erasesize_shift;
+ do_div(offset, mtd->erasesize);
+ return offset;
+}
+
+static int altera_quadspi_erase(struct spi_nor *nor, loff_t offset)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ struct mtd_info *mtd = &flash->mtd;
+ u32 val;
+ int sector_value;
+
+ sector_value = altera_quadspi_addr_to_sector(mtd, offset);
+ /* sanity check that block_offset is a valid sector number */
+ if (sector_value < 0)
+ return -EINVAL;
+
+ /* sector value should occupy bits 17:8 */
+ val = (sector_value << 8) & QUADSPI_MEM_OP_SECTOR_VALUE_MASK;
+
+ /* sector erase commands occupies lower 2 bits */
+ val |= QUADSPI_MEM_OP_SECTOR_ERASE_CMD;
+
+ /* write sector erase command to QUADSPI_MEM_OP register */
+ writel(val, q->csr_base + QUADSPI_MEM_OP_REG);
+
+ return altera_quadspi_write_erase_check(nor, ERASE_CHECK);
+}
+
+static int altera_quadspi_read(struct spi_nor *nor, loff_t from, size_t len,
+ size_t *retlen, u_char *buf)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+
+ memcpy_fromio(buf, q->data_base + from, len);
+ *retlen = len;
+
+ return 0;
+}
+
+static void altera_quadspi_write(struct spi_nor *nor, loff_t to, size_t len,
+ size_t *retlen, const u_char *buf)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+
+ memcpy_toio(q->data_base + to, buf, len);
+ *retlen += len;
+
+ /* check whether write triggered a illegal write interrupt */
+ altera_quadspi_write_erase_check(nor, WRITE_CHECK);
+}
+
+static int altera_quadspi_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ struct mtd_info *mtd = &flash->mtd;
+ uint32_t offset = ofs;
+ u32 sector_start, sector_end;
+ uint64_t num_sectors;
+ u32 mem_op;
+ u32 sr_bp;
+ u32 sr_tb;
+
+ sector_start = offset;
+ sector_end = altera_quadspi_addr_to_sector(mtd, offset + len);
+ num_sectors = mtd->size;
+ do_div(num_sectors, mtd->erasesize);
+
+ dev_dbg(nor->dev, "%s: sector start is %u,sector end is %u\n",
+ __func__, sector_start, sector_end);
+
+ if (sector_start >= num_sectors / 2) {
+ sr_bp = fls(num_sectors - 1 - sector_start) + 1;
+ sr_tb = 0;
+ } else if ((sector_end < num_sectors / 2) &&
+ (q->opcode_id != EPCS_OPCODE_ID)) {
+ sr_bp = fls(sector_end) + 1;
+ sr_tb = 1;
+ } else {
+ sr_bp = 16;
+ sr_tb = 0;
+ }
+
+ mem_op = (sr_tb << 12) | (sr_bp << 8);
+ mem_op &= QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK;
+ mem_op |= QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
+ writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
+
+ return 0;
+}
+
+static int altera_quadspi_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ u32 mem_op;
+
+ dev_dbg(nor->dev, "Unlock all protected area\n");
+ mem_op = QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
+ writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
+
+ return 0;
+}
+
+static void altera_quadspi_chip_select(struct altera_quadspi *q, u32 bank)
+{
+ u32 val = 0;
+
+ switch (bank) {
+ case 0:
+ val = QUADSPI_CHIP_SELECT_0;
+ break;
+ case 1:
+ val = QUADSPI_CHIP_SELECT_1;
+ break;
+ case 2:
+ val = QUADSPI_CHIP_SELECT_2;
+ break;
+ default:
+ dev_err(q->dev, "invalid bank\n");
+ return;
+ }
+ writel(val, q->csr_base + QUADSPI_CHIP_SELECT_REG);
+}
+
+static int altera_quadspi_probe_config_dt(struct platform_device *pdev,
+ struct device_node *np,
+ struct altera_quadspi *q)
+{
+ struct device_node *pp;
+ struct resource *quadspi_res;
+ int i = 0;
+
+ quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "avl_csr");
+ q->csr_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
+ if (IS_ERR(q->csr_base)) {
+ dev_err(&pdev->dev, "%s: ERROR: failed to map csr base\n",
+ __func__);
+ return PTR_ERR(q->csr_base);
+ }
+
+ quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+ "avl_mem");
+ q->data_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
+ if (IS_ERR(q->data_base)) {
+ dev_err(&pdev->dev, "%s: ERROR: failed to map data base\n",
+ __func__);
+ return PTR_ERR(q->data_base);
+ }
+
+ /* Fill structs for each subnode (flash device) */
+ for_each_available_child_of_node(np, pp) {
+ /* Read bank id from DT */
+ q->np[i] = pp;
+ i++;
+ }
+ q->num_flashes = i;
+ return 0;
+}
+
+static int altera_quadspi_scan(struct spi_nor *nor, const char *name)
+{
+ struct altera_quadspi_flash *flash = nor->priv;
+ struct altera_quadspi *q = flash->q;
+ int index;
+ int ret;
+ u8 id = 0;
+
+ ret = nor->read_reg(nor, SPINOR_OP_RDID, &id, 1);
+ if (ret)
+ return ret;
+
+ for (index = 0; index < ARRAY_SIZE(flash_devices); index++) {
+ if (flash_devices[index].device_id == id &&
+ strcmp(name, flash_devices[index].name) == 0) {
+ q->opcode_id = flash_devices[index].opcode_id;
+ return 0;
+ }
+ }
+
+ /* Memory chip is not listed and not supported */
+ return -EINVAL;
+}
+
+static int altera_quadspi_setup_banks(struct platform_device *pdev,
+ u32 bank, struct device_node *np)
+{
+ struct altera_quadspi *q = platform_get_drvdata(pdev);
+ struct mtd_part_parser_data ppdata = {};
+ struct altera_quadspi_flash *flash;
+ struct spi_nor *nor;
+ int ret = 0;
+ char modalias[40];
+
+ if (bank > q->num_flashes - 1)
+ return -EINVAL;
+
+ altera_quadspi_chip_select(q, bank);
+
+ flash = devm_kzalloc(q->dev, sizeof(*flash), GFP_KERNEL);
+ if (!flash)
+ return -ENOMEM;
+
+ q->flash[bank] = flash;
+ nor = &flash->nor;
+ nor->mtd = &flash->mtd;
+ nor->dev = &pdev->dev;
+ nor->priv = flash;
+ flash->mtd.priv = nor;
+ flash->q = q;
+
+ /* spi nor framework*/
+ nor->read_reg = altera_quadspi_read_reg;
+ nor->write_reg = altera_quadspi_write_reg;
+ nor->read = altera_quadspi_read;
+ nor->write = altera_quadspi_write;
+ nor->erase = altera_quadspi_erase;
+ nor->flash_lock = altera_quadspi_lock;
+ nor->flash_unlock = altera_quadspi_unlock;
+
+ /* scanning flash and checking dev id */
+ if (of_modalias_node(np, modalias, sizeof(modalias)) < 0)
+ return -EINVAL;
+
+ ret = altera_quadspi_scan(nor, modalias);
+ if (ret) {
+ dev_err(nor->dev, "flash not found\n");
+ return ret;
+ }
+
+ ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
+ if (ret)
+ return ret;
+
+ ppdata.of_node = np;
+
+ return mtd_device_parse_register(&flash->mtd, NULL, &ppdata, NULL, 0);
+}
+
+static int altera_quadspi_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct altera_quadspi *q;
+ int ret = 0;
+ int i;
+
+ if (!np) {
+ dev_err(dev, "no device found\n");
+ return -ENODEV;
+ }
+
+ q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
+ if (!q)
+ return -ENOMEM;
+
+ ret = altera_quadspi_probe_config_dt(pdev, np, q);
+ if (ret) {
+ dev_err(dev, "probe device tree failed\n");
+ return -ENODEV;
+ }
+
+ q->dev = dev;
+
+ /* check number of flashes */
+ if (q->num_flashes > MAX_NUM_FLASH_CHIP) {
+ dev_err(dev, "exceeding max number of flashes\n");
+ q->num_flashes = MAX_NUM_FLASH_CHIP;
+ }
+
+ platform_set_drvdata(pdev, q);
+
+ /* loop for each serial flash which is connected to quadspi */
+ for (i = 0; i < q->num_flashes; i++) {
+ ret = altera_quadspi_setup_banks(pdev, i, q->np[i]);
+ if (ret) {
+ dev_err(dev, "bank setup failed\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int altera_quadspi_remove(struct platform_device *pdev)
+{
+ struct altera_quadspi *q = platform_get_drvdata(pdev);
+ struct altera_quadspi_flash *flash;
+ int i;
+ int ret = 0;
+
+ /* clean up for all nor flash */
+ for (i = 0; i < q->num_flashes; i++) {
+ flash = q->flash[i];
+ if (!flash)
+ continue;
+
+ /* clean up mtd stuff */
+ ret = mtd_device_unregister(&flash->mtd);
+ if (ret) {
+ dev_err(&pdev->dev, "error removing mtd\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct of_device_id altera_quadspi_id_table[] = {
+ { .compatible = "altr,quadspi-1.0" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, altera_quadspi_id_table);
+
+static struct platform_driver altera_quadspi_driver = {
+ .driver = {
+ .name = ALTERA_QUADSPI_RESOURCE_NAME,
+ .of_match_table = altera_quadspi_id_table,
+ },
+ .probe = altera_quadspi_probe,
+ .remove = altera_quadspi_remove,
+};
+module_platform_driver(altera_quadspi_driver);
+
+MODULE_AUTHOR("Viet Nga Dao <[email protected]>");
+MODULE_DESCRIPTION("Altera QuadSPI Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 14a5d23..818ac01 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -687,6 +687,36 @@ static const struct spi_device_id spi_nor_ids[] = {
{ "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
{ "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
{ "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
+
+ /* Altera EPCQ/EPCS Flashes are non-JEDEC */
+ { "epcs16", INFO(0, 0, 0x10000, 32, 0) },
+ { "epcs64", INFO(0, 0, 0x10000, 128, 0) },
+ { "epcs128", INFO(0, 0, 0x40000, 256, 0) },
+
+ { "epcq16", INFO(0, 0, 0x10000, 32, 0) },
+ { "epcq32", INFO(0, 0, 0x10000, 64, 0) },
+ { "epcq64", INFO(0, 0, 0x10000, 128, 0) },
+ { "epcq128", INFO(0, 0, 0x10000, 256, 0) },
+ { "epcq256", INFO(0, 0, 0x10000, 512, 0) },
+ { "epcq512", INFO(0, 0, 0x10000, 1024, 0) },
+ { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
+
+ { "epcql256", INFO(0, 0, 0x10000, 512, 0) },
+ { "epcql512", INFO(0, 0, 0x10000, 1024, 0) },
+ { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
+
+ /* Micron non-JEDEC supported by Altera Quad SPI Controller */
+ { "n25q016-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
+ { "n25q032-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
+ { "n25q064-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
+ { "n25q128a13-nonjedec", INFO(0, 0, 64 * 1024, 256, 0) },
+ { "n25q256a-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
+ { "n25q256a11-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
+ { "n25q512a-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
+ { "n25q512ax3-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
+ { "mt25ql512-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
+ { "n25q00a11-nonjedec", INFO(0, 0, 64 * 1024, 2048, 0) },
+
{ },
};
--
1.7.7.4
On Wed, Jun 03, 2015 at 12:30:44AM -0700, [email protected] wrote:
> From: VIET NGA DAO <[email protected]>
>
> Altera Quad SPI Controller is a soft IP which enables access to
> Altera EPCS, EPCQ and Mircon flash chips. This patch adds driver
> for these devices.
>
> Signed-off-by: VIET NGA DAO <[email protected]>
>
> ---
> v4:
> - Add more flash devices support ( EPCQL and Micron)
^^ Unfortunately, I think you've added yourself another burden with this
one. Most of the rest actually is looking pretty good, so it's sad to
see this hold your driver back. Comments below.
> - Remove redundant messages
> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
> - Replace get_flash_name to altera_quadspi_scan
> - Remove clk related parts
> - Remove altera_quadspi_plat
> - Change device tree reg name and remove opcode-id
>
> v3:
> - Change altera_epcq driver name to altera_quadspi for more generic name
> - Implement flash name searching in altera_quadspi.c instead of spi-nor
> - Edit the altra quadspi info table in spi-nor
> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
> - Merge .h and .c into 1 file
>
> v2:
> - Change to spi_nor structure
> - Add lock and unlock functions for spi_nor
> - Simplify the altera_epcq_lock function
> - Replace reg by compatible in device tree
> ---
> .../devicetree/bindings/mtd/altera-quadspi.txt | 49 ++
> drivers/mtd/spi-nor/Kconfig | 8 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/altera-quadspi.c | 568 ++++++++++++++++++++
> drivers/mtd/spi-nor/spi-nor.c | 30 +
> 5 files changed, 656 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> create mode 100644 drivers/mtd/spi-nor/altera-quadspi.c
>
> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> new file mode 100644
> index 0000000..2873319
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> @@ -0,0 +1,49 @@
> +* MTD Altera QUADSPI driver
> +
> +Required properties:
> +- compatible: Should be "altr,quadspi-1.0"
> +- reg: Address and length of the register set for the device. It contains
> + the information of registers in the same order as described by reg-names
> +- reg-names: Should contain the reg names
> + "avl_csr": Should contain the register configuration base address
> + "avl_mem": Should contain the data base address
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>.
> +- flash device tree subnode, there must be a node with the following fields:
> + - compatible: Should contain the flash name:
> + 1. EPCS: epcs16, epcs64, epcs128
> + 2. EPCQ: epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
> + 3. EPCQ-L: epcql256, epcql512, epcql1024
> + 4. Mircon: n25q016-nonjedec, n25q032-nonjedec, n25q064-nonjedec,
> + n25q128a13-nonjedec, n25q128a11-nonjedec, n25q256a-nonjedec,
> + n25q256a11-nonjedec, n25q512a-nonjedec, n25q512ax3-nonjedec,
> + mt25ql512-nonjedec, n25q00-nonjedec, n25q00a11-nonjedec
OK, so you're adding a bunch of Micron flashes which already have
support via standard DT bindings and spi-nor library code, except now
you're adding "-nonjedec" to all of them. You better have a *really*
good reason for this. Are these flash not compatible with the JEDEC READ
ID opcode, by which every other system identifies these parts? Or are
you adding these names because of limitations in your controller? For
the former, I might be able to understand the need, but for the latter,
I'm much disinclined to support this. There's got to be a better way.
> + - #address-cells: please refer to /mtd/partition.txt
> + - #size-cells: please refer to /mtd/partition.txt
> + For partitions inside each flash, please refer to /mtd/partition.txt
> +
> +Example:
> +
> + quadspi_controller_0: quadspi@0x180014a0 {
> + compatible = "altr,quadspi-1.0";
> + reg = <0x180014a0 0x00000020>,
> + <0x14000000 0x04000000>;
> + reg-names = "avl_csr", "avl_mem";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + flash0: epcq256@0 {
> + compatible = "altr,epcq256";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + partition@0 {
> + /* 16 MB for raw data. */
> + label = "EPCQ Flash 0 raw data";
> + reg = <0x0 0x1000000>;
> + };
> + partition@1000000 {
> + /* 16 MB for jffs2 data. */
> + label = "EPCQ Flash 0 JFFS 2";
> + reg = <0x1000000 0x1000000>;
> + };
> + };
> + }; //end quadspi@0x180014a0 (quadspi_controller_0)
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 64a4f0e..678dbe3 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -28,4 +28,12 @@ config SPI_FSL_QUADSPI
> This enables support for the Quad SPI controller in master mode.
> We only connect the NOR to this controller now.
>
> +config SPI_ALTERA_QUADSPI
> + tristate "Altera Generic Quad SPI Controller"
> + depends on OF
> + help
> + This enables access to Altera EPCQ/EPCS/Micron flash chips,
> + used for data storage. See the driver source for the current list,
> + or to add other chips.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 6a7ce14..4e700df 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> +obj-$(CONFIG_SPI_ALTERA_QUADSPI) += altera-quadspi.o
> diff --git a/drivers/mtd/spi-nor/altera-quadspi.c b/drivers/mtd/spi-nor/altera-quadspi.c
> new file mode 100644
> index 0000000..8b113df
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/altera-quadspi.c
> @@ -0,0 +1,568 @@
> +/*
> + * Copyright (C) 2014 Altera Corporation. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +
> +#define ALTERA_QUADSPI_RESOURCE_NAME "altera_quadspi"
> +
> +/* max possible slots for serial flash chip in the QUADSPI controller */
> +#define MAX_NUM_FLASH_CHIP 3
> +
> +#define EPCS_OPCODE_ID 1
> +#define NON_EPCS_OPCODE_ID 2
> +
> +#define WRITE_CHECK 1
> +#define ERASE_CHECK 0
> +
> +/* Define max times to check status register before we give up. */
> +#define QUADSPI_MAX_TIME_OUT (40 * HZ)
> +
> +/* defines for status register */
> +#define QUADSPI_SR_REG 0x0
> +#define QUADSPI_SR_WIP_MASK 0x00000001
> +#define QUADSPI_SR_WIP 0x1
> +#define QUADSPI_SR_WEL 0x2
> +#define QUADSPI_SR_BP0 0x4
> +#define QUADSPI_SR_BP1 0x8
> +#define QUADSPI_SR_BP2 0x10
> +#define QUADSPI_SR_BP3 0x40
> +#define QUADSPI_SR_TB 0x20
> +#define QUADSPI_SR_MASK 0x0000000F
> +
> +/* defines for device id register */
> +#define QUADSPI_SID_REG 0x4
> +#define QUADSPI_RDID_REG 0x8
> +#define QUADSPI_ID_MASK 0x000000FF
> +
> +/*
> + * QUADSPI_MEM_OP register offset
> + *
> + * The QUADSPI_MEM_OP register is used to do memory protect and erase operations
> + *
> + */
> +#define QUADSPI_MEM_OP_REG 0xC
> +
> +#define QUADSPI_MEM_OP_CMD_MASK 0x00000003
> +#define QUADSPI_MEM_OP_BULK_ERASE_CMD 0x00000001
> +#define QUADSPI_MEM_OP_SECTOR_ERASE_CMD 0x00000002
> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_CMD 0x00000003
> +#define QUADSPI_MEM_OP_SECTOR_VALUE_MASK 0x0003FF00
> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK 0x00001F00
> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_SHIFT 8
> +/*
> + * QUADSPI_ISR register offset
> + *
> + * The QUADSPI_ISR register is used to determine whether an invalid write or
> + * erase operation trigerred an interrupt
> + *
> + */
> +#define QUADSPI_ISR_REG 0x10
> +
> +#define QUADSPI_ISR_ILLEGAL_ERASE_MASK 0x00000001
> +#define QUADSPI_ISR_ILLEGAL_WRITE_MASK 0x00000002
> +
> +/*
> + * QUADSPI_IMR register offset
> + *
> + * The QUADSPI_IMR register is used to mask the invalid erase or the invalid
> + * write interrupts.
> + *
> + */
> +#define QUADSPI_IMR_REG 0x14
> +#define QUADSPI_IMR_ILLEGAL_ERASE_MASK 0x00000001
> +
> +#define QUADSPI_IMR_ILLEGAL_WRITE_MASK 0x00000002
> +
> +#define QUADSPI_CHIP_SELECT_REG 0x18
> +#define QUADSPI_CHIP_SELECT_MASK 0x00000007
> +#define QUADSPI_CHIP_SELECT_0 0x00000001
> +#define QUADSPI_CHIP_SELECT_1 0x00000002
> +#define QUADSPI_CHIP_SELECT_2 0x00000004
> +
> +struct altera_quadspi {
> + u32 opcode_id;
> + void __iomem *csr_base;
> + void __iomem *data_base;
> + u32 num_flashes;
> + struct device *dev;
> + struct altera_quadspi_flash *flash[MAX_NUM_FLASH_CHIP];
> + struct device_node *np[MAX_NUM_FLASH_CHIP];
> +};
> +
> +struct altera_quadspi_flash {
> + struct mtd_info mtd;
> + struct spi_nor nor;
> + struct altera_quadspi *q;
> +};
> +
> +struct flash_device {
> + char *name;
> + u32 opcode_id;
> + u32 device_id;
> +};
> +
> +#define FLASH_ID(_n, _opcode_id, _id) \
> +{ \
> + .name = (_n), \
> + .opcode_id = (_opcode_id), \
> + .device_id = (_id), \
> +}
> +
> +static struct flash_device flash_devices[] = {
> + FLASH_ID("epcs16", EPCS_OPCODE_ID, 0x14),
> + FLASH_ID("epcs64", EPCS_OPCODE_ID, 0x16),
> + FLASH_ID("epcs128", EPCS_OPCODE_ID, 0x18),
> +
> + FLASH_ID("epcq16", NON_EPCS_OPCODE_ID, 0x15),
> + FLASH_ID("epcq32", NON_EPCS_OPCODE_ID, 0x16),
> + FLASH_ID("epcq64", NON_EPCS_OPCODE_ID, 0x17),
> + FLASH_ID("epcq128", NON_EPCS_OPCODE_ID, 0x18),
> + FLASH_ID("epcq256", NON_EPCS_OPCODE_ID, 0x19),
> + FLASH_ID("epcq512", NON_EPCS_OPCODE_ID, 0x20),
> + FLASH_ID("epcq1024", NON_EPCS_OPCODE_ID, 0x21),
> +
> + FLASH_ID("epcql256", NON_EPCS_OPCODE_ID, 0x19),
> + FLASH_ID("epcql512", NON_EPCS_OPCODE_ID, 0x20),
> + FLASH_ID("epcql1024", NON_EPCS_OPCODE_ID, 0x21),
> +
My complaints for this table start here:
> + FLASH_ID("n25q016-nonjedec", NON_EPCS_OPCODE_ID, 0x15),
> + FLASH_ID("n25q032-nonjedec", NON_EPCS_OPCODE_ID, 0x16),
> + FLASH_ID("n25q064-nonjedec", NON_EPCS_OPCODE_ID, 0x17),
> + FLASH_ID("n25q128a13-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
> + FLASH_ID("n25q128a11-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
> + FLASH_ID("n25q256a-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
> + FLASH_ID("n25q256a11-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
> + FLASH_ID("n25q512a-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
> + FLASH_ID("n25q512ax3-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
> + FLASH_ID("mt25ql512-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
> + FLASH_ID("n25q00-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
> + FLASH_ID("n25q00a11-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
> +
There's next to zero chance that I'll let you duplicate the spi-nor.c
table here locally. For Altera-specific parts that have funky
identification schemes and (from what I can tell) are limited to use by
this Altera SPI controller, maybe, but you've got to figure out
something better for other commodity parts (e.g., from Micron).
Anyway, it looks like you're just stealing the 3rd byte from the actual
JEDEC ID string already found in spi-nor.c and the datasheets. Can your
controller really not read the full ID string?
> +};
> +
> +static int altera_quadspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
> + int len, int wr_en)
> +{
> + return 0;
> +}
> +
> +static int altera_quadspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
> + int len)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + u32 data = 0;
> +
> + memset(val, 0, len);
> +
> + switch (opcode) {
> + case SPINOR_OP_RDSR:
> + data = readl(q->csr_base + QUADSPI_SR_REG);
> + *val = (u8)data & QUADSPI_SR_MASK;
> + break;
> + case SPINOR_OP_RDID:
> + if (q->opcode_id == EPCS_OPCODE_ID)
> + data = readl(q->csr_base + QUADSPI_SID_REG);
> + else
> + data = readl(q->csr_base + QUADSPI_RDID_REG);
> +
> + *val = (u8)data & QUADSPI_ID_MASK; /* device id */
> + break;
> + default:
> + *val = 0;
> + break;
> + }
> + return 0;
> +}
> +
> +static int altera_quadspi_write_erase_check(struct spi_nor *nor,
> + bool write_erase)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + u32 val;
> + u32 mask;
> +
> + if (write_erase)
> + mask = QUADSPI_ISR_ILLEGAL_WRITE_MASK;
> + else
> + mask = QUADSPI_ISR_ILLEGAL_ERASE_MASK;
> +
> + val = readl(q->csr_base + QUADSPI_ISR_REG);
> + if (val & mask) {
> + dev_err(nor->dev,
> + "write/erase failed, sector might be protected\n");
> + /* clear this status for next use */
> + writel(val, q->csr_base + QUADSPI_ISR_REG);
> + return -EIO;
> + }
> + return 0;
> +}
> +
> +static int altera_quadspi_addr_to_sector(struct mtd_info *mtd, uint64_t offset)
> +{
> + if (mtd->erasesize_shift)
> + return offset >> mtd->erasesize_shift;
> + do_div(offset, mtd->erasesize);
> + return offset;
> +}
> +
> +static int altera_quadspi_erase(struct spi_nor *nor, loff_t offset)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + struct mtd_info *mtd = &flash->mtd;
> + u32 val;
> + int sector_value;
> +
> + sector_value = altera_quadspi_addr_to_sector(mtd, offset);
> + /* sanity check that block_offset is a valid sector number */
> + if (sector_value < 0)
> + return -EINVAL;
> +
> + /* sector value should occupy bits 17:8 */
> + val = (sector_value << 8) & QUADSPI_MEM_OP_SECTOR_VALUE_MASK;
> +
> + /* sector erase commands occupies lower 2 bits */
> + val |= QUADSPI_MEM_OP_SECTOR_ERASE_CMD;
> +
> + /* write sector erase command to QUADSPI_MEM_OP register */
> + writel(val, q->csr_base + QUADSPI_MEM_OP_REG);
> +
> + return altera_quadspi_write_erase_check(nor, ERASE_CHECK);
> +}
> +
> +static int altera_quadspi_read(struct spi_nor *nor, loff_t from, size_t len,
> + size_t *retlen, u_char *buf)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> +
> + memcpy_fromio(buf, q->data_base + from, len);
> + *retlen = len;
> +
> + return 0;
> +}
> +
> +static void altera_quadspi_write(struct spi_nor *nor, loff_t to, size_t len,
> + size_t *retlen, const u_char *buf)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> +
> + memcpy_toio(q->data_base + to, buf, len);
> + *retlen += len;
> +
> + /* check whether write triggered a illegal write interrupt */
> + altera_quadspi_write_erase_check(nor, WRITE_CHECK);
> +}
> +
> +static int altera_quadspi_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + struct mtd_info *mtd = &flash->mtd;
> + uint32_t offset = ofs;
> + u32 sector_start, sector_end;
> + uint64_t num_sectors;
> + u32 mem_op;
> + u32 sr_bp;
> + u32 sr_tb;
> +
> + sector_start = offset;
> + sector_end = altera_quadspi_addr_to_sector(mtd, offset + len);
> + num_sectors = mtd->size;
> + do_div(num_sectors, mtd->erasesize);
> +
> + dev_dbg(nor->dev, "%s: sector start is %u,sector end is %u\n",
> + __func__, sector_start, sector_end);
> +
> + if (sector_start >= num_sectors / 2) {
> + sr_bp = fls(num_sectors - 1 - sector_start) + 1;
> + sr_tb = 0;
> + } else if ((sector_end < num_sectors / 2) &&
> + (q->opcode_id != EPCS_OPCODE_ID)) {
> + sr_bp = fls(sector_end) + 1;
> + sr_tb = 1;
> + } else {
> + sr_bp = 16;
> + sr_tb = 0;
> + }
> +
> + mem_op = (sr_tb << 12) | (sr_bp << 8);
> + mem_op &= QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK;
> + mem_op |= QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
> + writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
> +
> + return 0;
> +}
> +
> +static int altera_quadspi_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + u32 mem_op;
> +
> + dev_dbg(nor->dev, "Unlock all protected area\n");
> + mem_op = QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
> + writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
> +
> + return 0;
> +}
> +
> +static void altera_quadspi_chip_select(struct altera_quadspi *q, u32 bank)
> +{
> + u32 val = 0;
> +
> + switch (bank) {
> + case 0:
> + val = QUADSPI_CHIP_SELECT_0;
> + break;
> + case 1:
> + val = QUADSPI_CHIP_SELECT_1;
> + break;
> + case 2:
> + val = QUADSPI_CHIP_SELECT_2;
> + break;
> + default:
> + dev_err(q->dev, "invalid bank\n");
> + return;
> + }
> + writel(val, q->csr_base + QUADSPI_CHIP_SELECT_REG);
> +}
> +
> +static int altera_quadspi_probe_config_dt(struct platform_device *pdev,
> + struct device_node *np,
> + struct altera_quadspi *q)
> +{
> + struct device_node *pp;
> + struct resource *quadspi_res;
> + int i = 0;
> +
> + quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "avl_csr");
> + q->csr_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
> + if (IS_ERR(q->csr_base)) {
> + dev_err(&pdev->dev, "%s: ERROR: failed to map csr base\n",
> + __func__);
> + return PTR_ERR(q->csr_base);
> + }
> +
> + quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "avl_mem");
> + q->data_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
> + if (IS_ERR(q->data_base)) {
> + dev_err(&pdev->dev, "%s: ERROR: failed to map data base\n",
> + __func__);
> + return PTR_ERR(q->data_base);
> + }
> +
> + /* Fill structs for each subnode (flash device) */
> + for_each_available_child_of_node(np, pp) {
> + /* Read bank id from DT */
> + q->np[i] = pp;
> + i++;
> + }
> + q->num_flashes = i;
> + return 0;
> +}
> +
> +static int altera_quadspi_scan(struct spi_nor *nor, const char *name)
> +{
> + struct altera_quadspi_flash *flash = nor->priv;
> + struct altera_quadspi *q = flash->q;
> + int index;
> + int ret;
> + u8 id = 0;
> +
> + ret = nor->read_reg(nor, SPINOR_OP_RDID, &id, 1);
> + if (ret)
> + return ret;
> +
> + for (index = 0; index < ARRAY_SIZE(flash_devices); index++) {
> + if (flash_devices[index].device_id == id &&
> + strcmp(name, flash_devices[index].name) == 0) {
> + q->opcode_id = flash_devices[index].opcode_id;
> + return 0;
> + }
> + }
> +
> + /* Memory chip is not listed and not supported */
> + return -EINVAL;
> +}
> +
> +static int altera_quadspi_setup_banks(struct platform_device *pdev,
> + u32 bank, struct device_node *np)
> +{
> + struct altera_quadspi *q = platform_get_drvdata(pdev);
> + struct mtd_part_parser_data ppdata = {};
> + struct altera_quadspi_flash *flash;
> + struct spi_nor *nor;
> + int ret = 0;
> + char modalias[40];
> +
> + if (bank > q->num_flashes - 1)
> + return -EINVAL;
> +
> + altera_quadspi_chip_select(q, bank);
> +
> + flash = devm_kzalloc(q->dev, sizeof(*flash), GFP_KERNEL);
> + if (!flash)
> + return -ENOMEM;
> +
> + q->flash[bank] = flash;
> + nor = &flash->nor;
> + nor->mtd = &flash->mtd;
> + nor->dev = &pdev->dev;
> + nor->priv = flash;
> + flash->mtd.priv = nor;
> + flash->q = q;
> +
> + /* spi nor framework*/
> + nor->read_reg = altera_quadspi_read_reg;
> + nor->write_reg = altera_quadspi_write_reg;
> + nor->read = altera_quadspi_read;
> + nor->write = altera_quadspi_write;
> + nor->erase = altera_quadspi_erase;
> + nor->flash_lock = altera_quadspi_lock;
> + nor->flash_unlock = altera_quadspi_unlock;
> +
> + /* scanning flash and checking dev id */
> + if (of_modalias_node(np, modalias, sizeof(modalias)) < 0)
> + return -EINVAL;
> +
> + ret = altera_quadspi_scan(nor, modalias);
> + if (ret) {
> + dev_err(nor->dev, "flash not found\n");
> + return ret;
> + }
> +
> + ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
> + if (ret)
> + return ret;
> +
> + ppdata.of_node = np;
> +
> + return mtd_device_parse_register(&flash->mtd, NULL, &ppdata, NULL, 0);
> +}
> +
> +static int altera_quadspi_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct altera_quadspi *q;
> + int ret = 0;
> + int i;
> +
> + if (!np) {
> + dev_err(dev, "no device found\n");
> + return -ENODEV;
> + }
> +
> + q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
> + if (!q)
> + return -ENOMEM;
> +
> + ret = altera_quadspi_probe_config_dt(pdev, np, q);
> + if (ret) {
> + dev_err(dev, "probe device tree failed\n");
> + return -ENODEV;
> + }
> +
> + q->dev = dev;
> +
> + /* check number of flashes */
> + if (q->num_flashes > MAX_NUM_FLASH_CHIP) {
> + dev_err(dev, "exceeding max number of flashes\n");
> + q->num_flashes = MAX_NUM_FLASH_CHIP;
> + }
> +
> + platform_set_drvdata(pdev, q);
> +
> + /* loop for each serial flash which is connected to quadspi */
> + for (i = 0; i < q->num_flashes; i++) {
> + ret = altera_quadspi_setup_banks(pdev, i, q->np[i]);
> + if (ret) {
> + dev_err(dev, "bank setup failed\n");
> + return ret;
Your error handling still really sucks here. If you succeed at probing
the first flash, but fail at another, the driver probe fails, but you've
still left one or more MTD registered. Is that intended?
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int altera_quadspi_remove(struct platform_device *pdev)
> +{
> + struct altera_quadspi *q = platform_get_drvdata(pdev);
> + struct altera_quadspi_flash *flash;
> + int i;
> + int ret = 0;
> +
> + /* clean up for all nor flash */
> + for (i = 0; i < q->num_flashes; i++) {
> + flash = q->flash[i];
> + if (!flash)
> + continue;
> +
> + /* clean up mtd stuff */
> + ret = mtd_device_unregister(&flash->mtd);
> + if (ret) {
> + dev_err(&pdev->dev, "error removing mtd\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id altera_quadspi_id_table[] = {
> + { .compatible = "altr,quadspi-1.0" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, altera_quadspi_id_table);
> +
> +static struct platform_driver altera_quadspi_driver = {
> + .driver = {
> + .name = ALTERA_QUADSPI_RESOURCE_NAME,
> + .of_match_table = altera_quadspi_id_table,
> + },
> + .probe = altera_quadspi_probe,
> + .remove = altera_quadspi_remove,
> +};
> +module_platform_driver(altera_quadspi_driver);
> +
> +MODULE_AUTHOR("Viet Nga Dao <[email protected]>");
> +MODULE_DESCRIPTION("Altera QuadSPI Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 14a5d23..818ac01 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -687,6 +687,36 @@ static const struct spi_device_id spi_nor_ids[] = {
> { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
> { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
> { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
> +
> + /* Altera EPCQ/EPCS Flashes are non-JEDEC */
> + { "epcs16", INFO(0, 0, 0x10000, 32, 0) },
> + { "epcs64", INFO(0, 0, 0x10000, 128, 0) },
> + { "epcs128", INFO(0, 0, 0x40000, 256, 0) },
> +
> + { "epcq16", INFO(0, 0, 0x10000, 32, 0) },
> + { "epcq32", INFO(0, 0, 0x10000, 64, 0) },
> + { "epcq64", INFO(0, 0, 0x10000, 128, 0) },
> + { "epcq128", INFO(0, 0, 0x10000, 256, 0) },
> + { "epcq256", INFO(0, 0, 0x10000, 512, 0) },
> + { "epcq512", INFO(0, 0, 0x10000, 1024, 0) },
> + { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
> +
> + { "epcql256", INFO(0, 0, 0x10000, 512, 0) },
> + { "epcql512", INFO(0, 0, 0x10000, 1024, 0) },
> + { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
> +
Starting here:
> + /* Micron non-JEDEC supported by Altera Quad SPI Controller */
> + { "n25q016-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
> + { "n25q032-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
> + { "n25q064-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
> + { "n25q128a13-nonjedec", INFO(0, 0, 64 * 1024, 256, 0) },
> + { "n25q256a-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
> + { "n25q256a11-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
> + { "n25q512a-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
> + { "n25q512ax3-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
> + { "mt25ql512-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
> + { "n25q00a11-nonjedec", INFO(0, 0, 64 * 1024, 2048, 0) },
Ending here: NAK, unless you can give a really good explanation of why.
> +
> { },
> };
>
Brian
Thanks Brian for your reply!
On Sat, Jul 25, 2015 at 2:37 AM, Brian Norris
<[email protected]> wrote:
> On Wed, Jun 03, 2015 at 12:30:44AM -0700, [email protected] wrote:
>> From: VIET NGA DAO <[email protected]>
>>
>> Altera Quad SPI Controller is a soft IP which enables access to
>> Altera EPCS, EPCQ and Mircon flash chips. This patch adds driver
>> for these devices.
>>
>> Signed-off-by: VIET NGA DAO <[email protected]>
>>
>> ---
>> v4:
>> - Add more flash devices support ( EPCQL and Micron)
>
> ^^ Unfortunately, I think you've added yourself another burden with this
> one. Most of the rest actually is looking pretty good, so it's sad to
> see this hold your driver back. Comments below.
>
>> - Remove redundant messages
>> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
>> - Replace get_flash_name to altera_quadspi_scan
>> - Remove clk related parts
>> - Remove altera_quadspi_plat
>> - Change device tree reg name and remove opcode-id
>>
>> v3:
>> - Change altera_epcq driver name to altera_quadspi for more generic name
>> - Implement flash name searching in altera_quadspi.c instead of spi-nor
>> - Edit the altra quadspi info table in spi-nor
>> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
>> - Merge .h and .c into 1 file
>>
>> v2:
>> - Change to spi_nor structure
>> - Add lock and unlock functions for spi_nor
>> - Simplify the altera_epcq_lock function
>> - Replace reg by compatible in device tree
>> ---
>> .../devicetree/bindings/mtd/altera-quadspi.txt | 49 ++
>> drivers/mtd/spi-nor/Kconfig | 8 +
>> drivers/mtd/spi-nor/Makefile | 1 +
>> drivers/mtd/spi-nor/altera-quadspi.c | 568 ++++++++++++++++++++
>> drivers/mtd/spi-nor/spi-nor.c | 30 +
>> 5 files changed, 656 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> create mode 100644 drivers/mtd/spi-nor/altera-quadspi.c
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> new file mode 100644
>> index 0000000..2873319
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
>> @@ -0,0 +1,49 @@
>> +* MTD Altera QUADSPI driver
>> +
>> +Required properties:
>> +- compatible: Should be "altr,quadspi-1.0"
>> +- reg: Address and length of the register set for the device. It contains
>> + the information of registers in the same order as described by reg-names
>> +- reg-names: Should contain the reg names
>> + "avl_csr": Should contain the register configuration base address
>> + "avl_mem": Should contain the data base address
>> +- #address-cells: Must be <1>.
>> +- #size-cells: Must be <0>.
>> +- flash device tree subnode, there must be a node with the following fields:
>> + - compatible: Should contain the flash name:
>> + 1. EPCS: epcs16, epcs64, epcs128
>> + 2. EPCQ: epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
>> + 3. EPCQ-L: epcql256, epcql512, epcql1024
>> + 4. Mircon: n25q016-nonjedec, n25q032-nonjedec, n25q064-nonjedec,
>> + n25q128a13-nonjedec, n25q128a11-nonjedec, n25q256a-nonjedec,
>> + n25q256a11-nonjedec, n25q512a-nonjedec, n25q512ax3-nonjedec,
>> + mt25ql512-nonjedec, n25q00-nonjedec, n25q00a11-nonjedec
>
> OK, so you're adding a bunch of Micron flashes which already have
> support via standard DT bindings and spi-nor library code, except now
> you're adding "-nonjedec" to all of them. You better have a *really*
> good reason for this. Are these flash not compatible with the JEDEC READ
> ID opcode, by which every other system identifies these parts? Or are
> you adding these names because of limitations in your controller? For
> the former, I might be able to understand the need, but for the latter,
> I'm much disinclined to support this. There's got to be a better way.
>
Hi Brian,
It is really unfortunate that this controller is not able to read full
JEDEC ID. It only can provide 1 byte ID. I did discuss with IP
designer about this, but it is really unfortunate that they are not
able to fix that issue. Hence it requires software to make changes.
>> + - #address-cells: please refer to /mtd/partition.txt
>> + - #size-cells: please refer to /mtd/partition.txt
>> + For partitions inside each flash, please refer to /mtd/partition.txt
>> +
>> +Example:
>> +
>> + quadspi_controller_0: quadspi@0x180014a0 {
>> + compatible = "altr,quadspi-1.0";
>> + reg = <0x180014a0 0x00000020>,
>> + <0x14000000 0x04000000>;
>> + reg-names = "avl_csr", "avl_mem";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + flash0: epcq256@0 {
>> + compatible = "altr,epcq256";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + partition@0 {
>> + /* 16 MB for raw data. */
>> + label = "EPCQ Flash 0 raw data";
>> + reg = <0x0 0x1000000>;
>> + };
>> + partition@1000000 {
>> + /* 16 MB for jffs2 data. */
>> + label = "EPCQ Flash 0 JFFS 2";
>> + reg = <0x1000000 0x1000000>;
>> + };
>> + };
>> + }; //end quadspi@0x180014a0 (quadspi_controller_0)
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 64a4f0e..678dbe3 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -28,4 +28,12 @@ config SPI_FSL_QUADSPI
>> This enables support for the Quad SPI controller in master mode.
>> We only connect the NOR to this controller now.
>>
>> +config SPI_ALTERA_QUADSPI
>> + tristate "Altera Generic Quad SPI Controller"
>> + depends on OF
>> + help
>> + This enables access to Altera EPCQ/EPCS/Micron flash chips,
>> + used for data storage. See the driver source for the current list,
>> + or to add other chips.
>> +
>> endif # MTD_SPI_NOR
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 6a7ce14..4e700df 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,2 +1,3 @@
>> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
>> +obj-$(CONFIG_SPI_ALTERA_QUADSPI) += altera-quadspi.o
>> diff --git a/drivers/mtd/spi-nor/altera-quadspi.c b/drivers/mtd/spi-nor/altera-quadspi.c
>> new file mode 100644
>> index 0000000..8b113df
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/altera-quadspi.c
>> @@ -0,0 +1,568 @@
>> +/*
>> + * Copyright (C) 2014 Altera Corporation. All rights reserved
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/ioport.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/kernel.h>
>> +#include <linux/log2.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/partitions.h>
>> +#include <linux/mtd/spi-nor.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/sched.h>
>> +
>> +#define ALTERA_QUADSPI_RESOURCE_NAME "altera_quadspi"
>> +
>> +/* max possible slots for serial flash chip in the QUADSPI controller */
>> +#define MAX_NUM_FLASH_CHIP 3
>> +
>> +#define EPCS_OPCODE_ID 1
>> +#define NON_EPCS_OPCODE_ID 2
>> +
>> +#define WRITE_CHECK 1
>> +#define ERASE_CHECK 0
>> +
>> +/* Define max times to check status register before we give up. */
>> +#define QUADSPI_MAX_TIME_OUT (40 * HZ)
>> +
>> +/* defines for status register */
>> +#define QUADSPI_SR_REG 0x0
>> +#define QUADSPI_SR_WIP_MASK 0x00000001
>> +#define QUADSPI_SR_WIP 0x1
>> +#define QUADSPI_SR_WEL 0x2
>> +#define QUADSPI_SR_BP0 0x4
>> +#define QUADSPI_SR_BP1 0x8
>> +#define QUADSPI_SR_BP2 0x10
>> +#define QUADSPI_SR_BP3 0x40
>> +#define QUADSPI_SR_TB 0x20
>> +#define QUADSPI_SR_MASK 0x0000000F
>> +
>> +/* defines for device id register */
>> +#define QUADSPI_SID_REG 0x4
>> +#define QUADSPI_RDID_REG 0x8
>> +#define QUADSPI_ID_MASK 0x000000FF
>> +
>> +/*
>> + * QUADSPI_MEM_OP register offset
>> + *
>> + * The QUADSPI_MEM_OP register is used to do memory protect and erase operations
>> + *
>> + */
>> +#define QUADSPI_MEM_OP_REG 0xC
>> +
>> +#define QUADSPI_MEM_OP_CMD_MASK 0x00000003
>> +#define QUADSPI_MEM_OP_BULK_ERASE_CMD 0x00000001
>> +#define QUADSPI_MEM_OP_SECTOR_ERASE_CMD 0x00000002
>> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_CMD 0x00000003
>> +#define QUADSPI_MEM_OP_SECTOR_VALUE_MASK 0x0003FF00
>> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK 0x00001F00
>> +#define QUADSPI_MEM_OP_SECTOR_PROTECT_SHIFT 8
>> +/*
>> + * QUADSPI_ISR register offset
>> + *
>> + * The QUADSPI_ISR register is used to determine whether an invalid write or
>> + * erase operation trigerred an interrupt
>> + *
>> + */
>> +#define QUADSPI_ISR_REG 0x10
>> +
>> +#define QUADSPI_ISR_ILLEGAL_ERASE_MASK 0x00000001
>> +#define QUADSPI_ISR_ILLEGAL_WRITE_MASK 0x00000002
>> +
>> +/*
>> + * QUADSPI_IMR register offset
>> + *
>> + * The QUADSPI_IMR register is used to mask the invalid erase or the invalid
>> + * write interrupts.
>> + *
>> + */
>> +#define QUADSPI_IMR_REG 0x14
>> +#define QUADSPI_IMR_ILLEGAL_ERASE_MASK 0x00000001
>> +
>> +#define QUADSPI_IMR_ILLEGAL_WRITE_MASK 0x00000002
>> +
>> +#define QUADSPI_CHIP_SELECT_REG 0x18
>> +#define QUADSPI_CHIP_SELECT_MASK 0x00000007
>> +#define QUADSPI_CHIP_SELECT_0 0x00000001
>> +#define QUADSPI_CHIP_SELECT_1 0x00000002
>> +#define QUADSPI_CHIP_SELECT_2 0x00000004
>> +
>> +struct altera_quadspi {
>> + u32 opcode_id;
>> + void __iomem *csr_base;
>> + void __iomem *data_base;
>> + u32 num_flashes;
>> + struct device *dev;
>> + struct altera_quadspi_flash *flash[MAX_NUM_FLASH_CHIP];
>> + struct device_node *np[MAX_NUM_FLASH_CHIP];
>> +};
>> +
>> +struct altera_quadspi_flash {
>> + struct mtd_info mtd;
>> + struct spi_nor nor;
>> + struct altera_quadspi *q;
>> +};
>> +
>> +struct flash_device {
>> + char *name;
>> + u32 opcode_id;
>> + u32 device_id;
>> +};
>> +
>> +#define FLASH_ID(_n, _opcode_id, _id) \
>> +{ \
>> + .name = (_n), \
>> + .opcode_id = (_opcode_id), \
>> + .device_id = (_id), \
>> +}
>> +
>> +static struct flash_device flash_devices[] = {
>> + FLASH_ID("epcs16", EPCS_OPCODE_ID, 0x14),
>> + FLASH_ID("epcs64", EPCS_OPCODE_ID, 0x16),
>> + FLASH_ID("epcs128", EPCS_OPCODE_ID, 0x18),
>> +
>> + FLASH_ID("epcq16", NON_EPCS_OPCODE_ID, 0x15),
>> + FLASH_ID("epcq32", NON_EPCS_OPCODE_ID, 0x16),
>> + FLASH_ID("epcq64", NON_EPCS_OPCODE_ID, 0x17),
>> + FLASH_ID("epcq128", NON_EPCS_OPCODE_ID, 0x18),
>> + FLASH_ID("epcq256", NON_EPCS_OPCODE_ID, 0x19),
>> + FLASH_ID("epcq512", NON_EPCS_OPCODE_ID, 0x20),
>> + FLASH_ID("epcq1024", NON_EPCS_OPCODE_ID, 0x21),
>> +
>> + FLASH_ID("epcql256", NON_EPCS_OPCODE_ID, 0x19),
>> + FLASH_ID("epcql512", NON_EPCS_OPCODE_ID, 0x20),
>> + FLASH_ID("epcql1024", NON_EPCS_OPCODE_ID, 0x21),
>> +
>
> My complaints for this table start here:
>
>> + FLASH_ID("n25q016-nonjedec", NON_EPCS_OPCODE_ID, 0x15),
>> + FLASH_ID("n25q032-nonjedec", NON_EPCS_OPCODE_ID, 0x16),
>> + FLASH_ID("n25q064-nonjedec", NON_EPCS_OPCODE_ID, 0x17),
>> + FLASH_ID("n25q128a13-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
>> + FLASH_ID("n25q128a11-nonjedec", NON_EPCS_OPCODE_ID, 0x18),
>> + FLASH_ID("n25q256a-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
>> + FLASH_ID("n25q256a11-nonjedec", NON_EPCS_OPCODE_ID, 0x19),
>> + FLASH_ID("n25q512a-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
>> + FLASH_ID("n25q512ax3-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
>> + FLASH_ID("mt25ql512-nonjedec", NON_EPCS_OPCODE_ID, 0x20),
>> + FLASH_ID("n25q00-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
>> + FLASH_ID("n25q00a11-nonjedec", NON_EPCS_OPCODE_ID, 0x21),
>> +
>
> There's next to zero chance that I'll let you duplicate the spi-nor.c
> table here locally. For Altera-specific parts that have funky
> identification schemes and (from what I can tell) are limited to use by
> this Altera SPI controller, maybe, but you've got to figure out
> something better for other commodity parts (e.g., from Micron).
>
> Anyway, it looks like you're just stealing the 3rd byte from the actual
> JEDEC ID string already found in spi-nor.c and the datasheets. Can your
> controller really not read the full ID string?
>
Yes, this controller really cant read the full ID string. I was very
unhappy when i know about this additional device support and its
limitation.
>> +};
>> +
>> +static int altera_quadspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len, int wr_en)
>> +{
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *val,
>> + int len)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + u32 data = 0;
>> +
>> + memset(val, 0, len);
>> +
>> + switch (opcode) {
>> + case SPINOR_OP_RDSR:
>> + data = readl(q->csr_base + QUADSPI_SR_REG);
>> + *val = (u8)data & QUADSPI_SR_MASK;
>> + break;
>> + case SPINOR_OP_RDID:
>> + if (q->opcode_id == EPCS_OPCODE_ID)
>> + data = readl(q->csr_base + QUADSPI_SID_REG);
>> + else
>> + data = readl(q->csr_base + QUADSPI_RDID_REG);
>> +
>> + *val = (u8)data & QUADSPI_ID_MASK; /* device id */
>> + break;
>> + default:
>> + *val = 0;
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_write_erase_check(struct spi_nor *nor,
>> + bool write_erase)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + u32 val;
>> + u32 mask;
>> +
>> + if (write_erase)
>> + mask = QUADSPI_ISR_ILLEGAL_WRITE_MASK;
>> + else
>> + mask = QUADSPI_ISR_ILLEGAL_ERASE_MASK;
>> +
>> + val = readl(q->csr_base + QUADSPI_ISR_REG);
>> + if (val & mask) {
>> + dev_err(nor->dev,
>> + "write/erase failed, sector might be protected\n");
>> + /* clear this status for next use */
>> + writel(val, q->csr_base + QUADSPI_ISR_REG);
>> + return -EIO;
>> + }
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_addr_to_sector(struct mtd_info *mtd, uint64_t offset)
>> +{
>> + if (mtd->erasesize_shift)
>> + return offset >> mtd->erasesize_shift;
>> + do_div(offset, mtd->erasesize);
>> + return offset;
>> +}
>> +
>> +static int altera_quadspi_erase(struct spi_nor *nor, loff_t offset)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + struct mtd_info *mtd = &flash->mtd;
>> + u32 val;
>> + int sector_value;
>> +
>> + sector_value = altera_quadspi_addr_to_sector(mtd, offset);
>> + /* sanity check that block_offset is a valid sector number */
>> + if (sector_value < 0)
>> + return -EINVAL;
>> +
>> + /* sector value should occupy bits 17:8 */
>> + val = (sector_value << 8) & QUADSPI_MEM_OP_SECTOR_VALUE_MASK;
>> +
>> + /* sector erase commands occupies lower 2 bits */
>> + val |= QUADSPI_MEM_OP_SECTOR_ERASE_CMD;
>> +
>> + /* write sector erase command to QUADSPI_MEM_OP register */
>> + writel(val, q->csr_base + QUADSPI_MEM_OP_REG);
>> +
>> + return altera_quadspi_write_erase_check(nor, ERASE_CHECK);
>> +}
>> +
>> +static int altera_quadspi_read(struct spi_nor *nor, loff_t from, size_t len,
>> + size_t *retlen, u_char *buf)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> +
>> + memcpy_fromio(buf, q->data_base + from, len);
>> + *retlen = len;
>> +
>> + return 0;
>> +}
>> +
>> +static void altera_quadspi_write(struct spi_nor *nor, loff_t to, size_t len,
>> + size_t *retlen, const u_char *buf)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> +
>> + memcpy_toio(q->data_base + to, buf, len);
>> + *retlen += len;
>> +
>> + /* check whether write triggered a illegal write interrupt */
>> + altera_quadspi_write_erase_check(nor, WRITE_CHECK);
>> +}
>> +
>> +static int altera_quadspi_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + struct mtd_info *mtd = &flash->mtd;
>> + uint32_t offset = ofs;
>> + u32 sector_start, sector_end;
>> + uint64_t num_sectors;
>> + u32 mem_op;
>> + u32 sr_bp;
>> + u32 sr_tb;
>> +
>> + sector_start = offset;
>> + sector_end = altera_quadspi_addr_to_sector(mtd, offset + len);
>> + num_sectors = mtd->size;
>> + do_div(num_sectors, mtd->erasesize);
>> +
>> + dev_dbg(nor->dev, "%s: sector start is %u,sector end is %u\n",
>> + __func__, sector_start, sector_end);
>> +
>> + if (sector_start >= num_sectors / 2) {
>> + sr_bp = fls(num_sectors - 1 - sector_start) + 1;
>> + sr_tb = 0;
>> + } else if ((sector_end < num_sectors / 2) &&
>> + (q->opcode_id != EPCS_OPCODE_ID)) {
>> + sr_bp = fls(sector_end) + 1;
>> + sr_tb = 1;
>> + } else {
>> + sr_bp = 16;
>> + sr_tb = 0;
>> + }
>> +
>> + mem_op = (sr_tb << 12) | (sr_bp << 8);
>> + mem_op &= QUADSPI_MEM_OP_SECTOR_PROTECT_VALUE_MASK;
>> + mem_op |= QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
>> + writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
>> +
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + u32 mem_op;
>> +
>> + dev_dbg(nor->dev, "Unlock all protected area\n");
>> + mem_op = QUADSPI_MEM_OP_SECTOR_PROTECT_CMD;
>> + writel(mem_op, q->csr_base + QUADSPI_MEM_OP_REG);
>> +
>> + return 0;
>> +}
>> +
>> +static void altera_quadspi_chip_select(struct altera_quadspi *q, u32 bank)
>> +{
>> + u32 val = 0;
>> +
>> + switch (bank) {
>> + case 0:
>> + val = QUADSPI_CHIP_SELECT_0;
>> + break;
>> + case 1:
>> + val = QUADSPI_CHIP_SELECT_1;
>> + break;
>> + case 2:
>> + val = QUADSPI_CHIP_SELECT_2;
>> + break;
>> + default:
>> + dev_err(q->dev, "invalid bank\n");
>> + return;
>> + }
>> + writel(val, q->csr_base + QUADSPI_CHIP_SELECT_REG);
>> +}
>> +
>> +static int altera_quadspi_probe_config_dt(struct platform_device *pdev,
>> + struct device_node *np,
>> + struct altera_quadspi *q)
>> +{
>> + struct device_node *pp;
>> + struct resource *quadspi_res;
>> + int i = 0;
>> +
>> + quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> + "avl_csr");
>> + q->csr_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
>> + if (IS_ERR(q->csr_base)) {
>> + dev_err(&pdev->dev, "%s: ERROR: failed to map csr base\n",
>> + __func__);
>> + return PTR_ERR(q->csr_base);
>> + }
>> +
>> + quadspi_res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> + "avl_mem");
>> + q->data_base = devm_ioremap_resource(&pdev->dev, quadspi_res);
>> + if (IS_ERR(q->data_base)) {
>> + dev_err(&pdev->dev, "%s: ERROR: failed to map data base\n",
>> + __func__);
>> + return PTR_ERR(q->data_base);
>> + }
>> +
>> + /* Fill structs for each subnode (flash device) */
>> + for_each_available_child_of_node(np, pp) {
>> + /* Read bank id from DT */
>> + q->np[i] = pp;
>> + i++;
>> + }
>> + q->num_flashes = i;
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_scan(struct spi_nor *nor, const char *name)
>> +{
>> + struct altera_quadspi_flash *flash = nor->priv;
>> + struct altera_quadspi *q = flash->q;
>> + int index;
>> + int ret;
>> + u8 id = 0;
>> +
>> + ret = nor->read_reg(nor, SPINOR_OP_RDID, &id, 1);
>> + if (ret)
>> + return ret;
>> +
>> + for (index = 0; index < ARRAY_SIZE(flash_devices); index++) {
>> + if (flash_devices[index].device_id == id &&
>> + strcmp(name, flash_devices[index].name) == 0) {
>> + q->opcode_id = flash_devices[index].opcode_id;
>> + return 0;
>> + }
>> + }
>> +
>> + /* Memory chip is not listed and not supported */
>> + return -EINVAL;
>> +}
>> +
>> +static int altera_quadspi_setup_banks(struct platform_device *pdev,
>> + u32 bank, struct device_node *np)
>> +{
>> + struct altera_quadspi *q = platform_get_drvdata(pdev);
>> + struct mtd_part_parser_data ppdata = {};
>> + struct altera_quadspi_flash *flash;
>> + struct spi_nor *nor;
>> + int ret = 0;
>> + char modalias[40];
>> +
>> + if (bank > q->num_flashes - 1)
>> + return -EINVAL;
>> +
>> + altera_quadspi_chip_select(q, bank);
>> +
>> + flash = devm_kzalloc(q->dev, sizeof(*flash), GFP_KERNEL);
>> + if (!flash)
>> + return -ENOMEM;
>> +
>> + q->flash[bank] = flash;
>> + nor = &flash->nor;
>> + nor->mtd = &flash->mtd;
>> + nor->dev = &pdev->dev;
>> + nor->priv = flash;
>> + flash->mtd.priv = nor;
>> + flash->q = q;
>> +
>> + /* spi nor framework*/
>> + nor->read_reg = altera_quadspi_read_reg;
>> + nor->write_reg = altera_quadspi_write_reg;
>> + nor->read = altera_quadspi_read;
>> + nor->write = altera_quadspi_write;
>> + nor->erase = altera_quadspi_erase;
>> + nor->flash_lock = altera_quadspi_lock;
>> + nor->flash_unlock = altera_quadspi_unlock;
>> +
>> + /* scanning flash and checking dev id */
>> + if (of_modalias_node(np, modalias, sizeof(modalias)) < 0)
>> + return -EINVAL;
>> +
>> + ret = altera_quadspi_scan(nor, modalias);
>> + if (ret) {
>> + dev_err(nor->dev, "flash not found\n");
>> + return ret;
>> + }
>> +
>> + ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>> + if (ret)
>> + return ret;
>> +
>> + ppdata.of_node = np;
>> +
>> + return mtd_device_parse_register(&flash->mtd, NULL, &ppdata, NULL, 0);
>> +}
>> +
>> +static int altera_quadspi_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct device *dev = &pdev->dev;
>> + struct altera_quadspi *q;
>> + int ret = 0;
>> + int i;
>> +
>> + if (!np) {
>> + dev_err(dev, "no device found\n");
>> + return -ENODEV;
>> + }
>> +
>> + q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
>> + if (!q)
>> + return -ENOMEM;
>> +
>> + ret = altera_quadspi_probe_config_dt(pdev, np, q);
>> + if (ret) {
>> + dev_err(dev, "probe device tree failed\n");
>> + return -ENODEV;
>> + }
>> +
>> + q->dev = dev;
>> +
>> + /* check number of flashes */
>> + if (q->num_flashes > MAX_NUM_FLASH_CHIP) {
>> + dev_err(dev, "exceeding max number of flashes\n");
>> + q->num_flashes = MAX_NUM_FLASH_CHIP;
>> + }
>> +
>> + platform_set_drvdata(pdev, q);
>> +
>> + /* loop for each serial flash which is connected to quadspi */
>> + for (i = 0; i < q->num_flashes; i++) {
>> + ret = altera_quadspi_setup_banks(pdev, i, q->np[i]);
>> + if (ret) {
>> + dev_err(dev, "bank setup failed\n");
>> + return ret;
>
> Your error handling still really sucks here. If you succeed at probing
> the first flash, but fail at another, the driver probe fails, but you've
> still left one or more MTD registered. Is that intended?
>
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int altera_quadspi_remove(struct platform_device *pdev)
>> +{
>> + struct altera_quadspi *q = platform_get_drvdata(pdev);
>> + struct altera_quadspi_flash *flash;
>> + int i;
>> + int ret = 0;
>> +
>> + /* clean up for all nor flash */
>> + for (i = 0; i < q->num_flashes; i++) {
>> + flash = q->flash[i];
>> + if (!flash)
>> + continue;
>> +
>> + /* clean up mtd stuff */
>> + ret = mtd_device_unregister(&flash->mtd);
>> + if (ret) {
>> + dev_err(&pdev->dev, "error removing mtd\n");
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id altera_quadspi_id_table[] = {
>> + { .compatible = "altr,quadspi-1.0" },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, altera_quadspi_id_table);
>> +
>> +static struct platform_driver altera_quadspi_driver = {
>> + .driver = {
>> + .name = ALTERA_QUADSPI_RESOURCE_NAME,
>> + .of_match_table = altera_quadspi_id_table,
>> + },
>> + .probe = altera_quadspi_probe,
>> + .remove = altera_quadspi_remove,
>> +};
>> +module_platform_driver(altera_quadspi_driver);
>> +
>> +MODULE_AUTHOR("Viet Nga Dao <[email protected]>");
>> +MODULE_DESCRIPTION("Altera QuadSPI Driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 14a5d23..818ac01 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -687,6 +687,36 @@ static const struct spi_device_id spi_nor_ids[] = {
>> { "cat25c09", CAT25_INFO( 128, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>> { "cat25c17", CAT25_INFO( 256, 8, 32, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>> { "cat25128", CAT25_INFO(2048, 8, 64, 2, SPI_NOR_NO_ERASE | SPI_NOR_NO_FR) },
>> +
>> + /* Altera EPCQ/EPCS Flashes are non-JEDEC */
>> + { "epcs16", INFO(0, 0, 0x10000, 32, 0) },
>> + { "epcs64", INFO(0, 0, 0x10000, 128, 0) },
>> + { "epcs128", INFO(0, 0, 0x40000, 256, 0) },
>> +
>> + { "epcq16", INFO(0, 0, 0x10000, 32, 0) },
>> + { "epcq32", INFO(0, 0, 0x10000, 64, 0) },
>> + { "epcq64", INFO(0, 0, 0x10000, 128, 0) },
>> + { "epcq128", INFO(0, 0, 0x10000, 256, 0) },
>> + { "epcq256", INFO(0, 0, 0x10000, 512, 0) },
>> + { "epcq512", INFO(0, 0, 0x10000, 1024, 0) },
>> + { "epcq1024", INFO(0, 0, 0x10000, 2048, 0) },
>> +
>> + { "epcql256", INFO(0, 0, 0x10000, 512, 0) },
>> + { "epcql512", INFO(0, 0, 0x10000, 1024, 0) },
>> + { "epcql1024", INFO(0, 0, 0x10000, 2048, 0) },
>> +
>
> Starting here:
>
>> + /* Micron non-JEDEC supported by Altera Quad SPI Controller */
>> + { "n25q016-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
>> + { "n25q032-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
>> + { "n25q064-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
>> + { "n25q128a13-nonjedec", INFO(0, 0, 64 * 1024, 256, 0) },
>> + { "n25q256a-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
>> + { "n25q256a11-nonjedec", INFO(0, 0, 64 * 1024, 512, 0) },
>> + { "n25q512a-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
>> + { "n25q512ax3-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
>> + { "mt25ql512-nonjedec", INFO(0, 0, 64 * 1024, 1024, 0) },
>> + { "n25q00a11-nonjedec", INFO(0, 0, 64 * 1024, 2048, 0) },
>
> Ending here: NAK, unless you can give a really good explanation of why.
>
>> +
>> { },
>> };
>>
>
> Brian
On Mon, Jul 27, 2015 at 03:10:23PM +0800, Viet Nga Dao wrote:
> On Sat, Jul 25, 2015 at 2:37 AM, Brian Norris
> <[email protected]> wrote:
> > On Wed, Jun 03, 2015 at 12:30:44AM -0700, [email protected] wrote:
> >> From: VIET NGA DAO <[email protected]>
> >>
> >> Altera Quad SPI Controller is a soft IP which enables access to
> >> Altera EPCS, EPCQ and Mircon flash chips. This patch adds driver
> >> for these devices.
> >>
> >> Signed-off-by: VIET NGA DAO <[email protected]>
> >>
> >> ---
> >> v4:
> >> - Add more flash devices support ( EPCQL and Micron)
> >
> > ^^ Unfortunately, I think you've added yourself another burden with this
> > one. Most of the rest actually is looking pretty good, so it's sad to
> > see this hold your driver back. Comments below.
FWIW, this comment still stands. I cannot take your patch as-is.
> >> - Remove redundant messages
> >> - Change EPCQ_OPCODE_ID to NON_EPCS_OPCODE_ID
> >> - Replace get_flash_name to altera_quadspi_scan
> >> - Remove clk related parts
> >> - Remove altera_quadspi_plat
> >> - Change device tree reg name and remove opcode-id
> >>
> >> v3:
> >> - Change altera_epcq driver name to altera_quadspi for more generic name
> >> - Implement flash name searching in altera_quadspi.c instead of spi-nor
> >> - Edit the altra quadspi info table in spi-nor
> >> - Remove wait_til_ready in all read,write, erase, lock, unlock functions
> >> - Merge .h and .c into 1 file
> >>
> >> v2:
> >> - Change to spi_nor structure
> >> - Add lock and unlock functions for spi_nor
> >> - Simplify the altera_epcq_lock function
> >> - Replace reg by compatible in device tree
> >> ---
> >> .../devicetree/bindings/mtd/altera-quadspi.txt | 49 ++
> >> drivers/mtd/spi-nor/Kconfig | 8 +
> >> drivers/mtd/spi-nor/Makefile | 1 +
> >> drivers/mtd/spi-nor/altera-quadspi.c | 568 ++++++++++++++++++++
> >> drivers/mtd/spi-nor/spi-nor.c | 30 +
> >> 5 files changed, 656 insertions(+), 0 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> >> create mode 100644 drivers/mtd/spi-nor/altera-quadspi.c
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi.txt b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> >> new file mode 100644
> >> index 0000000..2873319
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi.txt
> >> @@ -0,0 +1,49 @@
> >> +* MTD Altera QUADSPI driver
> >> +
> >> +Required properties:
> >> +- compatible: Should be "altr,quadspi-1.0"
> >> +- reg: Address and length of the register set for the device. It contains
> >> + the information of registers in the same order as described by reg-names
> >> +- reg-names: Should contain the reg names
> >> + "avl_csr": Should contain the register configuration base address
> >> + "avl_mem": Should contain the data base address
> >> +- #address-cells: Must be <1>.
> >> +- #size-cells: Must be <0>.
> >> +- flash device tree subnode, there must be a node with the following fields:
> >> + - compatible: Should contain the flash name:
> >> + 1. EPCS: epcs16, epcs64, epcs128
> >> + 2. EPCQ: epcq16, epcq32, epcq64, epcq128, epcq256, epcq512, epcq1024
> >> + 3. EPCQ-L: epcql256, epcql512, epcql1024
> >> + 4. Mircon: n25q016-nonjedec, n25q032-nonjedec, n25q064-nonjedec,
> >> + n25q128a13-nonjedec, n25q128a11-nonjedec, n25q256a-nonjedec,
> >> + n25q256a11-nonjedec, n25q512a-nonjedec, n25q512ax3-nonjedec,
> >> + mt25ql512-nonjedec, n25q00-nonjedec, n25q00a11-nonjedec
> >
> > OK, so you're adding a bunch of Micron flashes which already have
> > support via standard DT bindings and spi-nor library code, except now
> > you're adding "-nonjedec" to all of them. You better have a *really*
> > good reason for this. Are these flash not compatible with the JEDEC READ
> > ID opcode, by which every other system identifies these parts? Or are
> > you adding these names because of limitations in your controller? For
> > the former, I might be able to understand the need, but for the latter,
> > I'm much disinclined to support this. There's got to be a better way.
> >
>
> Hi Brian,
> It is really unfortunate that this controller is not able to read full
> JEDEC ID. It only can provide 1 byte ID. I did discuss with IP
> designer about this, but it is really unfortunate that they are not
> able to fix that issue. Hence it requires software to make changes.
Wow, what a joke. (Please quote me to your IP "designer.")
In any case, there's no way I'm going to support a ton of new
"*-nonjedec" strings just to support your completely broken controller.
These aren't actually "nonjedec" flash devices, and AFAICT, there's
absolutely nothing wrong with the flash that requires this description.
It's purely your controller's fault.
Thus, I'm not changing the DT binding for all the flash you want to use.
Instead, I think we need to assume that your controller is completely
incapable of identifying the flash that's attached to it, and instead
just rely on specifying this entirely in the device tree. Then, we need
a flag for your driver to notify spi-nor.c that it can't trust the READ
ID command at all, and it must instead rely strictly on flash string
matching. e.g. something like:
ret = of_modalias_node(np, name, sizeof(name)); /* e.g., name = "n25q016" */
if (ret < 0)
return ret;
nor->controller_flags |= SPI_NOR_MY_CONTROLLER_SUCKS_AND_CANT_DO_READID;
ret = spi_nor_scan(nor, name, mode);
along with something like this in spi-nor.c:
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 47df4b5eae2f..04d3aa7ec641 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -709,6 +709,11 @@ static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor)
u8 id[SPI_NOR_MAX_ID_LEN];
struct flash_info *info;
+ if (nor->my_foo_flags & SPI_NOR_MY_CONTROLLER_SUCKS_AND_CANT_DO_READID) {
+ dev_err(nor->dev, "READ ID not supported\n");
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+
tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
if (tmp < 0) {
dev_dbg(nor->dev, " error %d reading JEDEC ID\n", tmp);
@@ -1018,6 +1023,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
if (name)
id = spi_nor_match_id(name);
+ if (!id && (nor->my_foo_flags & SPI_NOR_MY_CONTROLLER_SUCKS_AND_CANT_DO_READID)) {
+ dev_err(dev, "no matching device found, and your controller sucks\n");
+ return -ENODEV;
+ }
/* Try to auto-detect if chip name wasn't specified or not found */
if (!id)
id = spi_nor_read_id(nor);
Brian
On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
Hi!
[...]
> > Hi Brian,
> > It is really unfortunate that this controller is not able to read full
> > JEDEC ID. It only can provide 1 byte ID. I did discuss with IP
> > designer about this, but it is really unfortunate that they are not
> > able to fix that issue. Hence it requires software to make changes.
Thanks for CCing me, I assume this is a driver for that "altera_epcq_controller"
which you can synthesise into your FPGA, is that correct? Is there an Hard IP
variant of this controller in the public or is this purely Soft IP?
Also, I cannot find any documentation for this IP block even if I search through
Quartus/QSys, is there any proper documentation available anywhere?
[...]
Best regards,
Marek Vasut
I'm not very helpful here, so hopefully Viet can be of more use:
On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
> On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
> Also, I cannot find any documentation for this IP block even if I search through
> Quartus/QSys, is there any proper documentation available anywhere?
I never found proper documentation, but I didn't look too hard. I've
mostly been going off of Viet's comments and code.
But FWIW, I did find some relevant info for the peculiar Altera EPCQ
flash here:
https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cfg/cfg_cf52012.pdf
Brian
On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
> I'm not very helpful here, so hopefully Viet can be of more use:
Yup :)
> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
> > Also, I cannot find any documentation for this IP block even if I search
> > through Quartus/QSys, is there any proper documentation available
> > anywhere?
>
> I never found proper documentation, but I didn't look too hard. I've
> mostly been going off of Viet's comments and code.
Me neither, and I looked through the altera stuff in fact. I'm trying
to learn whether this is just an Soft IP, in which case it certainly
can be fixed ; or if there is actually some chip shipping with this
crap synthesised into actual silicon.
> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
> flash here:
>
> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/
> hb/cfg/cfg_cf52012.pdf
Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have
different JEDEC ID and are a bit more expensive.
Best regards,
Marek Vasut
On Tue, Aug 18, 2015 at 2:55 PM, Viet Nga Dao <[email protected]> wrote:
>
>
> -----Original Message-----
> From: [email protected]
> Sent: Tuesday, August 18, 2015 9:33 AM
> To: Brian Norris
> Cc: [email protected]; Viet Nga Dao; [email protected]; Rafa?? Mi??ecki; [email protected]; David Woodhouse; Graham Moore
> Subject: Re: [PATCH] [PATCH v4] mtd:spi-nor: Add Altera Quad SPI Driver
>
> On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
>> I'm not very helpful here, so hopefully Viet can be of more use:
>
> Yup :)
>
>> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
>> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
>> > Also, I cannot find any documentation for this IP block even if I
>> > search through Quartus/QSys, is there any proper documentation
>> > available anywhere?
>>
>> I never found proper documentation, but I didn't look too hard. I've
>> mostly been going off of Viet's comments and code.
>
> Me neither, and I looked through the altera stuff in fact. I'm trying to learn whether this is just an Soft IP, in which case it certainly can be fixed ; or if there is actually some chip shipping with this crap synthesised into actual silicon.
>
>> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
>> flash here:
>>
>> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera
>> ture/
>> hb/cfg/cfg_cf52012.pdf
>
> Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have different JEDEC >ID and are a bit more expensive.
You can find the document at here
(https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_embedded_ip.pdf)
Chapter 42.Page 407.
For the soft IP issue, i've requested hardware engineer to come out
the solution. So in the mean way, our driver will NOT support Micron
flashes until hardware fix is completed.
Hence, i just submitted version 5 for this driver with eliminating
micron device support. Hope this version will get ACKed by you.
Thanks,
Viet Nga
Hi,
>> On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
>>> I'm not very helpful here, so hopefully Viet can be of more use:
>>
>> Yup :)
>>
>>> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
>>> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
>>> > Also, I cannot find any documentation for this IP block even if I
>>> > search through Quartus/QSys, is there any proper documentation
>>> > available anywhere?
>>>
>>> I never found proper documentation, but I didn't look too hard. I've
>>> mostly been going off of Viet's comments and code.
>>
>> Me neither, and I looked through the altera stuff in fact. I'm trying to learn whether this is just an Soft IP, in which case it certainly can be fixed ; or if there is actually some chip shipping with this crap synthesised into actual silicon.
>>
>>> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
>>> flash here:
>>>
>>> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera
>>> ture/
>>> hb/cfg/cfg_cf52012.pdf
>>
>> Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just have different JEDEC >ID and are a bit more expensive.
>
> You can find the document at here
> (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/ug/ug_embedded_ip.pdf)
> Chapter 42.Page 407.
>
> For the soft IP issue, i've requested hardware engineer to come out
> the solution. So in the mean way, our driver will NOT support Micron
> flashes until hardware fix is completed.
>
> Hence, i just submitted version 5 for this driver with eliminating
> micron device support. Hope this version will get ACKed by you.
>
> Thanks,
> Viet Nga
On Thursday, August 20, 2015 at 09:37:33 AM, Viet Nga Dao wrote:
> Hi,
Hi,
> >> On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
> >>> I'm not very helpful here, so hopefully Viet can be of more use:
> >> Yup :)
> >>
> >>> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
> >>> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
> >>> > Also, I cannot find any documentation for this IP block even if I
> >>> > search through Quartus/QSys, is there any proper documentation
> >>> > available anywhere?
> >>>
> >>> I never found proper documentation, but I didn't look too hard. I've
> >>> mostly been going off of Viet's comments and code.
> >>
> >> Me neither, and I looked through the altera stuff in fact. I'm trying to
> >> learn whether this is just an Soft IP, in which case it certainly can
> >> be fixed ; or if there is actually some chip shipping with this crap
> >> synthesised into actual silicon.
> >>
> >>> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
> >>> flash here:
> >>>
> >>> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera
> >>> ture/
> >>> hb/cfg/cfg_cf52012.pdf
> >>
> >> Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just
> >> have different JEDEC >ID and are a bit more expensive.
> >
> > You can find the document at here
> > (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literatu
> > re/ug/ug_embedded_ip.pdf)
> >
> > Chapter 42.Page 407.
> >
> > For the soft IP issue, i've requested hardware engineer to come out
> > the solution.
That's good :)
> > So in the mean way, our driver will NOT support Micron
> > flashes until hardware fix is completed.
This doesn't answer my question, so let me reiterate. Is this controller
only Soft IP (as in, FPGA core) or is this controller shipping in some
chip as Hard IP (as in, piece of silicon) ?
> > Hence, i just submitted version 5 for this driver with eliminating
> > micron device support. Hope this version will get ACKed by you.
We'll see.
Best regards,
Marek Vasut
On Thu, Aug 20, 2015 at 3:55 PM, Marek Vasut <[email protected]> wrote:
> On Thursday, August 20, 2015 at 09:37:33 AM, Viet Nga Dao wrote:
>> Hi,
>
> Hi,
>
>> >> On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
>> >>> I'm not very helpful here, so hopefully Viet can be of more use:
>> >> Yup :)
>> >>
>> >>> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
>> >>> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
>> >>> > Also, I cannot find any documentation for this IP block even if I
>> >>> > search through Quartus/QSys, is there any proper documentation
>> >>> > available anywhere?
>> >>>
>> >>> I never found proper documentation, but I didn't look too hard. I've
>> >>> mostly been going off of Viet's comments and code.
>> >>
>> >> Me neither, and I looked through the altera stuff in fact. I'm trying to
>> >> learn whether this is just an Soft IP, in which case it certainly can
>> >> be fixed ; or if there is actually some chip shipping with this crap
>> >> synthesised into actual silicon.
>> >>
>> >>> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
>> >>> flash here:
>> >>>
>> >>> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/litera
>> >>> ture/
>> >>> hb/cfg/cfg_cf52012.pdf
>> >>
>> >> Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just
>> >> have different JEDEC >ID and are a bit more expensive.
>> >
>> > You can find the document at here
>> > (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literatu
>> > re/ug/ug_embedded_ip.pdf)
>> >
>> > Chapter 42.Page 407.
>> >
>> > For the soft IP issue, i've requested hardware engineer to come out
>> > the solution.
>
> That's good :)
>
>> > So in the mean way, our driver will NOT support Micron
>> > flashes until hardware fix is completed.
>
> This doesn't answer my question, so let me reiterate. Is this controller
> only Soft IP (as in, FPGA core) or is this controller shipping in some
> chip as Hard IP (as in, piece of silicon) ?
>
This is new soft IP.
>> > Hence, i just submitted version 5 for this driver with eliminating
>> > micron device support. Hope this version will get ACKed by you.
>
> We'll see.
>
> Best regards,
> Marek Vasut
On Thursday, August 20, 2015 at 10:06:29 AM, Viet Nga Dao wrote:
> On Thu, Aug 20, 2015 at 3:55 PM, Marek Vasut <[email protected]> wrote:
> > On Thursday, August 20, 2015 at 09:37:33 AM, Viet Nga Dao wrote:
> >> Hi,
> >
> > Hi,
> >
> >> >> On Tuesday, August 18, 2015 at 03:24:44 AM, Brian Norris wrote:
> >> >>> I'm not very helpful here, so hopefully Viet can be of more use:
> >> >> Yup :)
> >> >>
> >> >>> On Mon, Aug 17, 2015 at 07:53:23PM +0200, Marek Vasut wrote:
> >> >>> > On Monday, August 17, 2015 at 06:03:38 PM, Brian Norris wrote:
> >> >>> > Also, I cannot find any documentation for this IP block even if I
> >> >>> > search through Quartus/QSys, is there any proper documentation
> >> >>> > available anywhere?
> >> >>>
> >> >>> I never found proper documentation, but I didn't look too hard. I've
> >> >>> mostly been going off of Viet's comments and code.
> >> >>
> >> >> Me neither, and I looked through the altera stuff in fact. I'm trying
> >> >> to learn whether this is just an Soft IP, in which case it certainly
> >> >> can be fixed ; or if there is actually some chip shipping with this
> >> >> crap synthesised into actual silicon.
> >> >>
> >> >>> But FWIW, I did find some relevant info for the peculiar Altera EPCQ
> >> >>> flash here:
> >> >>>
> >> >>> https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/lite
> >> >>> ra ture/
> >> >>> hb/cfg/cfg_cf52012.pdf
> >> >>
> >> >> Altera EPCS/EPCQ flashes are just rebranded micron flashes, they just
> >> >> have different JEDEC >ID and are a bit more expensive.
> >> >
> >> > You can find the document at here
> >> > (https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/liter
> >> > atu re/ug/ug_embedded_ip.pdf)
> >> >
> >> > Chapter 42.Page 407.
> >> >
> >> > For the soft IP issue, i've requested hardware engineer to come out
> >> > the solution.
> >
> > That's good :)
> >
> >> > So in the mean way, our driver will NOT support Micron
> >> > flashes until hardware fix is completed.
> >
> > This doesn't answer my question, so let me reiterate. Is this controller
> > only Soft IP (as in, FPGA core) or is this controller shipping in some
> > chip as Hard IP (as in, piece of silicon) ?
>
> This is new soft IP.
I see, thanks !
Best regards,
Marek Vasut