mtd: add altera quadspi driver

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Quoted 3 lines

Signed-off-by Thomas Chou <[email protected]>

---

You might want to look at https://lwn.net/Articles/636882/ , it is the driver for the same hardware, but for Linux. But keep in mind that the driver had some difficulties getting in, you might want to check the discussions in linux-mtd .

Quoted 4 lines · Marek Vasut

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

This should be part of drivers/mtd/spi-nor which I'm working currently, might take couple of days to push the patches to ML.

Quoted 6 lines · Jagan Teki

Quoted 4 lines · Marek Vasut

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Quoted 2 lines

This should be part of drivers/mtd/spi-nor which I'm working currently, might take couple of days to push the patches to ML.

I think your patches are still far away from inclusion, so I'm fine with the current way of doing things ;-)

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Jagan Teki

Quoted 4 lines · Marek Vasut

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Quoted 5 lines

Quoted 2 lines

This should be part of drivers/mtd/spi-nor which I'm working currently, might take couple of days to push the patches to ML.

I think your patches are still far away from inclusion, so I'm fine with the current way of doing things ;-)

I think it's better to wait for main things to be done first, and if possible I'm open for anyone to help me to move forward.

Quoted 4 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Quoted 2 lines

Signed-off-by Thomas Chou <[email protected]>

---

You might want to look at https://lwn.net/Articles/636882/ , it is the driver for the same hardware, but for Linux. But keep in mind that the driver had some difficulties getting in, you might want to check the discussions in linux-mtd .

Hi Marek,

Quoted 8 lines · Thomas Chou

Quoted 6 lines

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

Quoted 4 lines

You might want to look at https://lwn.net/Articles/636882/ , it is the driver for the same hardware, but for Linux. But keep in mind that the driver had some difficulties getting in, you might want to check the discussions in linux-mtd .

I did check and follow the threads for a while since you pointed me about it in earlier communication. It is v5 last month. But the author decided to wait for hardware fix on rdid.

Yet I have a different point as I stated in the patch message. It is NOT a spi-nor since the hardware converted it to parallel interface. It should be treated more like cfi flash. I think it might be a mistake to take it as spi-nor. And this might be the hidden cause to prevent the linux driver getting in. So I wrote it my way.

Let me just put an idea here, it might be wrong -- but doesn't this seem like some sort of NVMEM device? See for example:

https://lkml.org/lkml/2015/5/21/643

Hi Marek,

Quoted 4 lines

Let me just put an idea here, it might be wrong -- but doesn't this seem like some sort of NVMEM device? See for example:

https://lkml.org/lkml/2015/5/21/643

Thanks for the pointer. However, it needs erasing before writing.

Quoted 1 line · Thomas Chou

Hi Marek,

Hi,

Quoted 4 lines · Marek Vasut

Quoted 3 lines

Let me just put an idea here, it might be wrong -- but doesn't this seem like some sort of NVMEM device? See for example:

Quoted 3 lines

Quoted 1 line

https://lkml.org/lkml/2015/5/21/643

Thanks for the pointer. However, it needs erasing before writing.

But does it operate in some sort of memory-mapped fashion or is this more of a SPI controller ? I think it's the former, right ?

But now that you mention the erasing, I see why you'd opt for the CFI framework, yeah.

Hi Marek,

Quoted 9 lines · Thomas Chou

Quoted 1 line

Hi Marek,

Hi,

Quoted 4 lines · Marek Vasut

Quoted 3 lines

Let me just put an idea here, it might be wrong -- but doesn't this seem like some sort of NVMEM device? See for example:

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

https://lkml.org/lkml/2015/5/21/643

Thanks for the pointer. However, it needs erasing before writing.

But does it operate in some sort of memory-mapped fashion or is this more of a SPI controller ? I think it's the former, right ?

Right. The former.

Quoted 2 lines

But now that you mention the erasing, I see why you'd opt for the CFI framework, yeah.

Yes.

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---
 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 +++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 312 +++++++++++++++++++++++++++
 4 files changed, 357 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

...

drivers/mtd/altera_qspi.c new file +195 -0

new file mode 100644
index 0000000..06bc53e
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and erase operations
+ */
+#define QUADSPI_MEM_OP_BULK_ERASE		0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE		0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT		0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid write or
+ * erase operation trigerred an interrupt
+ */
+#define QUADSPI_ISR_ILLEGAL_ERASE		BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE		BIT(1)
+
+struct altera_qspi_regs {
+	u32	rd_status;
+	u32	rd_sid;
+	u32	rd_rdid;
+	u32	mem_op;
+	u32	isr;
+	u32	imr;
+	u32	chip_select;
+};
+
+struct altera_qspi_platdata {
+	struct altera_qspi_regs *regs;
+	void *base;
+	unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];	/* FLASH chips info */
+
+void flash_print_info(flash_info_t *info)
+{
+	printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+	       info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct erase_info instr;
+	int ret;
+
+	memset(&instr, 0, sizeof(instr));
+	instr.addr = mtd->erasesize * s_first;
+	instr.len = mtd->erasesize * (s_last + 1 - s_first);
+	ret = mtd_erase(mtd, &instr);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong cnt)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	ulong base = (ulong)pdata->base;
+	loff_t to = addr - base;
+	size_t retlen;
+	int ret;
+
+	ret = mtd_write(mtd, to, cnt, &retlen, src);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+unsigned long flash_init(void)
+{
+	struct udevice *dev;
+
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+
+	return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	size_t addr = instr->addr;
+	size_t len = instr->len;
+	size_t end = addr + len;
+	u32 sect;
+	u32 stat;
+
+	instr->state = MTD_ERASING;
+	addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+	while (addr < end) {
+		sect = addr / mtd->erasesize;
+		sect <<= 8;
+		sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+		debug("erase %08x\n", sect);
+		writel(sect, &regs->mem_op);
+		stat = readl(&regs->isr);
+		if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+			/* erase failed, sector might be protected */
+			debug("erase %08x fail %x\n", sect, stat);
+			writel(stat, &regs->isr); /* clear isr */
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		addr += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+	mtd_erase_callback(instr);
+
+	return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from, size_t len,
+			    size_t *retlen, u_char *buf)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+	memcpy(buf, pdata->base + from, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static inline u32 add_byte(u32 data, u8 byte, int shift)
+{
+	data &= ~(0xff << shift);
+	data |= byte << shift;
+	return data;
+}
+
+static int altera_qspi_write_word(struct mtd_info *mtd, loff_t to,
+				  u32 data)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	u32 pos = (u32)to;
+	u32 stat;
+
+	/* write to flash 32 bits at a time */
+	writel(data, pdata->base + pos);
+	/* check whether write triggered a illegal write interrupt */
+	stat = readl(&regs->isr);
+	if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+		/* write failed, sector might be protected */
+		debug("write %08x fail %x\n", pos, stat);
+		writel(stat, &regs->isr); /* clear isr */
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t len,
+			     size_t *retlen, const u_char *buf)
+{
+	const u_char *end = buf + len;
+	unsigned shift;
+	u32 data;
+	int ret;
+
+	shift = (to & (sizeof(u32) - 1)) * 8; /* first shift to add byte */
+	to &= ~(sizeof(u32) - 1); /* get lower aligned address */
+	while (buf < end) {
+		data = 0xffffffff; /* pad data */
+		while (buf < end && shift < 32) {
+			/* add byte from buf */
+			data = add_byte(data, *buf++, shift);
+			shift += 8;
+		}
+		ret = altera_qspi_write_word(mtd, to, data);

Quoted 61 lines · thomas at wytron.com.tw

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 +++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 312
 +++++++++++++++++++++++++++ 4 files changed, 357 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

...

drivers/mtd/altera_qspi.c new file +18 -0

new file mode 100644
index 0000000..06bc53e
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and erase

operations + */

drivers/mtd/altera_qspi.c new file +6 -0

+#define QUADSPI_MEM_OP_BULK_ERASE		0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE		0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT		0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid

write or + * erase operation trigerred an interrupt
+ */

drivers/mtd/altera_qspi.c new file +12 -0

+#define QUADSPI_ISR_ILLEGAL_ERASE		BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE		BIT(1)
+
+struct altera_qspi_regs {
+	u32	rd_status;
+	u32	rd_sid;
+	u32	rd_rdid;
+	u32	mem_op;
+	u32	isr;
+	u32	imr;
+	u32	chip_select;
+};

drivers/mtd/altera_qspi.c

Quoted 8 lines

drivers/mtd/altera_qspi.c new file +8 -0

+
+struct altera_qspi_platdata {
+	struct altera_qspi_regs *regs;
+	void *base;
+	unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];	/* FLASH chips 

Quoted 148 lines

*/ +

drivers/mtd/altera_qspi.c new file +53 -0

+void flash_print_info(flash_info_t *info)
+{
+	printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+	       info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct erase_info instr;
+	int ret;
+
+	memset(&instr, 0, sizeof(instr));
+	instr.addr = mtd->erasesize * s_first;
+	instr.len = mtd->erasesize * (s_last + 1 - s_first);
+	ret = mtd_erase(mtd, &instr);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong cnt)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	ulong base = (ulong)pdata->base;
+	loff_t to = addr - base;
+	size_t retlen;
+	int ret;
+
+	ret = mtd_write(mtd, to, cnt, &retlen, src);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+unsigned long flash_init(void)
+{
+	struct udevice *dev;
+
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+
+	return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info

*instr) +{

drivers/mtd/altera_qspi.c new file +34 -0

+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	size_t addr = instr->addr;
+	size_t len = instr->len;
+	size_t end = addr + len;
+	u32 sect;
+	u32 stat;
+
+	instr->state = MTD_ERASING;
+	addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+	while (addr < end) {
+		sect = addr / mtd->erasesize;
+		sect <<= 8;
+		sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+		debug("erase %08x\n", sect);
+		writel(sect, &regs->mem_op);
+		stat = readl(&regs->isr);
+		if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+			/* erase failed, sector might be protected */
+			debug("erase %08x fail %x\n", sect, stat);
+			writel(stat, &regs->isr); /* clear isr */
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		addr += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+	mtd_erase_callback(instr);
+
+	return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from, size_t

len, + size_t *retlen, u_char *buf)

drivers/mtd/altera_qspi.c new file +41 -0

+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+	memcpy(buf, pdata->base + from, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static inline u32 add_byte(u32 data, u8 byte, int shift)
+{
+	data &= ~(0xff << shift);
+	data |= byte << shift;
+	return data;
+}
+
+static int altera_qspi_write_word(struct mtd_info *mtd, loff_t to,
+				  u32 data)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	u32 pos = (u32)to;
+	u32 stat;
+
+	/* write to flash 32 bits at a time */
+	writel(data, pdata->base + pos);
+	/* check whether write triggered a illegal write interrupt */
+	stat = readl(&regs->isr);
+	if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+		/* write failed, sector might be protected */
+		debug("write %08x fail %x\n", pos, stat);
+		writel(stat, &regs->isr); /* clear isr */
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t

len, + size_t *retlen, const u_char *buf)

drivers/mtd/altera_qspi.c new file +16 -0

+{
+	const u_char *end = buf + len;
+	unsigned shift;
+	u32 data;
+	int ret;
+
+	shift = (to & (sizeof(u32) - 1)) * 8; /* first shift to add byte */
+	to &= ~(sizeof(u32) - 1); /* get lower aligned address */
+	while (buf < end) {
+		data = 0xffffffff; /* pad data */
+		while (buf < end && shift < 32) {
+			/* add byte from buf */
+			data = add_byte(data, *buf++, shift);
+			shift += 8;
+		}
+		ret = altera_qspi_write_word(mtd, to, data);

drivers/mtd/altera_qspi.c line 205

Quoted 10 lines

drivers/mtd/altera_qspi.c new file +10 -0

+		if (ret)
+			return ret;
+		to += sizeof(u32);
+		shift = 0;
+	}
+	*retlen = len;
+
+	return 0;
+}
+

Hi Thomas,

Thanks for the patch.

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode + address + dummy).

Quoted 62 lines · Chin Liang See

Quoted 61 lines · thomas at wytron.com.tw

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 +++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 312
 +++++++++++++++++++++++++++ 4 files changed, 357 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

...

drivers/mtd/altera_qspi.c new file +18 -0

new file mode 100644
index 0000000..06bc53e
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and erase

operations + */

drivers/mtd/altera_qspi.c new file +6 -0

+#define QUADSPI_MEM_OP_BULK_ERASE		0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE		0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT		0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid

write or + * erase operation trigerred an interrupt
+ */

drivers/mtd/altera_qspi.c new file +12 -0

+#define QUADSPI_ISR_ILLEGAL_ERASE		BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE		BIT(1)
+
+struct altera_qspi_regs {
+	u32	rd_status;
+	u32	rd_sid;
+	u32	rd_rdid;
+	u32	mem_op;
+	u32	isr;
+	u32	imr;
+	u32	chip_select;
+};

Quoted 8 lines

drivers/mtd/altera_qspi.c

Quoted 8 lines

drivers/mtd/altera_qspi.c new file +8 -0

+
+struct altera_qspi_platdata {
+	struct altera_qspi_regs *regs;
+	void *base;
+	unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];	/* FLASH chips 

info

Quoted 163 lines

Quoted 148 lines

*/ +

drivers/mtd/altera_qspi.c new file +53 -0

+void flash_print_info(flash_info_t *info)
+{
+	printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+	       info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct erase_info instr;
+	int ret;
+
+	memset(&instr, 0, sizeof(instr));
+	instr.addr = mtd->erasesize * s_first;
+	instr.len = mtd->erasesize * (s_last + 1 - s_first);
+	ret = mtd_erase(mtd, &instr);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong cnt)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	ulong base = (ulong)pdata->base;
+	loff_t to = addr - base;
+	size_t retlen;
+	int ret;
+
+	ret = mtd_write(mtd, to, cnt, &retlen, src);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+unsigned long flash_init(void)
+{
+	struct udevice *dev;
+
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+
+	return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info

*instr) +{

drivers/mtd/altera_qspi.c new file +34 -0

+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	size_t addr = instr->addr;
+	size_t len = instr->len;
+	size_t end = addr + len;
+	u32 sect;
+	u32 stat;
+
+	instr->state = MTD_ERASING;
+	addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+	while (addr < end) {
+		sect = addr / mtd->erasesize;
+		sect <<= 8;
+		sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+		debug("erase %08x\n", sect);
+		writel(sect, &regs->mem_op);
+		stat = readl(&regs->isr);
+		if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+			/* erase failed, sector might be protected */
+			debug("erase %08x fail %x\n", sect, stat);
+			writel(stat, &regs->isr); /* clear isr */
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		addr += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+	mtd_erase_callback(instr);
+
+	return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from, size_t

len, + size_t *retlen, u_char *buf)

drivers/mtd/altera_qspi.c new file +41 -0

+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+	memcpy(buf, pdata->base + from, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static inline u32 add_byte(u32 data, u8 byte, int shift)
+{
+	data &= ~(0xff << shift);
+	data |= byte << shift;
+	return data;
+}
+
+static int altera_qspi_write_word(struct mtd_info *mtd, loff_t to,
+				  u32 data)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	u32 pos = (u32)to;
+	u32 stat;
+
+	/* write to flash 32 bits at a time */
+	writel(data, pdata->base + pos);
+	/* check whether write triggered a illegal write interrupt */
+	stat = readl(&regs->isr);
+	if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+		/* write failed, sector might be protected */
+		debug("write %08x fail %x\n", pos, stat);
+		writel(stat, &regs->isr); /* clear isr */
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t

len, + size_t *retlen, const u_char *buf)

drivers/mtd/altera_qspi.c new file +16 -0

+{
+	const u_char *end = buf + len;
+	unsigned shift;
+	u32 data;
+	int ret;
+
+	shift = (to & (sizeof(u32) - 1)) * 8; /* first shift to add byte */
+	to &= ~(sizeof(u32) - 1); /* get lower aligned address */
+	while (buf < end) {
+		data = 0xffffffff; /* pad data */
+		while (buf < end && shift < 32) {
+			/* add byte from buf */
+			data = add_byte(data, *buf++, shift);
+			shift += 8;
+		}
+		ret = altera_qspi_write_word(mtd, to, data);

drivers/mtd/altera_qspi.c line 205

Quoted 10 lines

drivers/mtd/altera_qspi.c new file +10 -0

+		if (ret)
+			return ret;
+		to += sizeof(u32);
+		shift = 0;
+	}
+	*retlen = len;
+
+	return 0;
+}
+

Hi Thomas,

Thanks for the patch.

Quoted 4 lines

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode + address + dummy).

You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.

Hi Marek,

Quoted 205 lines · marex at denx.de

Quoted 62 lines · Chin Liang See

Quoted 61 lines · thomas at wytron.com.tw

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 +++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 312
 +++++++++++++++++++++++++++ 4 files changed, 357 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

...

drivers/mtd/altera_qspi.c new file +18 -0

new file mode 100644
index 0000000..06bc53e
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and

erase operations + */

drivers/mtd/altera_qspi.c new file +6 -0

+#define QUADSPI_MEM_OP_BULK_ERASE		0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE		0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT		0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid

write or + * erase operation trigerred an interrupt
+ */

drivers/mtd/altera_qspi.c new file +12 -0

+#define QUADSPI_ISR_ILLEGAL_ERASE		BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE		BIT(1)
+
+struct altera_qspi_regs {
+	u32	rd_status;
+	u32	rd_sid;
+	u32	rd_rdid;
+	u32	mem_op;
+	u32	isr;
+	u32	imr;
+	u32	chip_select;
+};

Quoted 8 lines

drivers/mtd/altera_qspi.c

Quoted 8 lines

drivers/mtd/altera_qspi.c new file +8 -0

+
+struct altera_qspi_platdata {
+	struct altera_qspi_regs *regs;
+	void *base;
+	unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];	/* FLASH chips

info

Quoted 131 lines

Quoted 131 lines

*/ +

drivers/mtd/altera_qspi.c new file +23 -0

+void flash_print_info(flash_info_t *info)
+{
+	printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+	       info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct erase_info instr;
+	int ret;
+
+	memset(&instr, 0, sizeof(instr));
+	instr.addr = mtd->erasesize * s_first;
+	instr.len = mtd->erasesize * (s_last + 1 - s_first);
+	ret = mtd_erase(mtd, &instr);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong

cnt) +{

drivers/mtd/altera_qspi.c new file +29 -0

+	struct mtd_info *mtd = info->mtd;
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	ulong base = (ulong)pdata->base;
+	loff_t to = addr - base;
+	size_t retlen;
+	int ret;
+
+	ret = mtd_write(mtd, to, cnt, &retlen, src);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+unsigned long flash_init(void)
+{
+	struct udevice *dev;
+
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+
+	return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info

*instr) +{

drivers/mtd/altera_qspi.c new file +34 -0

+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	size_t addr = instr->addr;
+	size_t len = instr->len;
+	size_t end = addr + len;
+	u32 sect;
+	u32 stat;
+
+	instr->state = MTD_ERASING;
+	addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+	while (addr < end) {
+		sect = addr / mtd->erasesize;
+		sect <<= 8;
+		sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+		debug("erase %08x\n", sect);
+		writel(sect, &regs->mem_op);
+		stat = readl(&regs->isr);
+		if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+			/* erase failed, sector might be protected */
+			debug("erase %08x fail %x\n", sect, stat);
+			writel(stat, &regs->isr); /* clear isr */
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		addr += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+	mtd_erase_callback(instr);
+
+	return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from,

size_t len, + size_t *retlen, u_char *buf)

drivers/mtd/altera_qspi.c new file +41 -0

+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+	memcpy(buf, pdata->base + from, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static inline u32 add_byte(u32 data, u8 byte, int shift)
+{
+	data &= ~(0xff << shift);
+	data |= byte << shift;
+	return data;
+}
+
+static int altera_qspi_write_word(struct mtd_info *mtd, loff_t to,
+				  u32 data)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	u32 pos = (u32)to;
+	u32 stat;
+
+	/* write to flash 32 bits at a time */
+	writel(data, pdata->base + pos);
+	/* check whether write triggered a illegal write interrupt */
+	stat = readl(&regs->isr);
+	if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+		/* write failed, sector might be protected */
+		debug("write %08x fail %x\n", pos, stat);
+		writel(stat, &regs->isr); /* clear isr */
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t

Quoted 8 lines

Quoted 8 lines

drivers/mtd/altera_qspi.c

Quoted 8 lines

len, + size_t *retlen, const u_char *buf)

drivers/mtd/altera_qspi.c new file +7 -0

+{
+	const u_char *end = buf + len;
+	unsigned shift;
+	u32 data;
+	int ret;
+
+	shift = (to & (sizeof(u32) - 1)) * 8; /* first shift to add byte 

Quoted 29 lines

Quoted 28 lines

Quoted 9 lines

drivers/mtd/altera_qspi.c new file +9 -0

+	to &= ~(sizeof(u32) - 1); /* get lower aligned address */
+	while (buf < end) {
+		data = 0xffffffff; /* pad data */
+		while (buf < end && shift < 32) {
+			/* add byte from buf */
+			data = add_byte(data, *buf++, shift);
+			shift += 8;
+		}
+		ret = altera_qspi_write_word(mtd, to, data);

drivers/mtd/altera_qspi.c line 205

Quoted 10 lines

drivers/mtd/altera_qspi.c new file +10 -0

+		if (ret)
+			return ret;
+		to += sizeof(u32);
+		shift = 0;
+	}
+	*retlen = len;
+
+	return 0;
+}
+

Hi Thomas,

Thanks for the patch.

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode
+ address + dummy).

Quoted 2 lines

You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.

Actually I am thinking to take advantage the cache fill and dump. But after rethinking, this might limit some of the use case as we want the driver to support NIOS II without cache. With that, just ignore this comment for now.

But your comment lead to the fact that the read part is now using memcpy. Thomas needs to fix that to use the readl :)

Quoted 208 lines · Chin Liang See

Hi Marek,

Quoted 205 lines · marex at denx.de

Quoted 62 lines · Chin Liang See

Quoted 61 lines · thomas at wytron.com.tw

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---

 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 +++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 312
 +++++++++++++++++++++++++++ 4 files changed, 357 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

...

drivers/mtd/altera_qspi.c new file +18 -0

new file mode 100644
index 0000000..06bc53e
@@ -0,0 +1,312 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and

erase operations + */

drivers/mtd/altera_qspi.c new file +6 -0

+#define QUADSPI_MEM_OP_BULK_ERASE		0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE		0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT		0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid

write or + * erase operation trigerred an interrupt
+ */

drivers/mtd/altera_qspi.c new file +12 -0

+#define QUADSPI_ISR_ILLEGAL_ERASE		BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE		BIT(1)
+
+struct altera_qspi_regs {
+	u32	rd_status;
+	u32	rd_sid;
+	u32	rd_rdid;
+	u32	mem_op;
+	u32	isr;
+	u32	imr;
+	u32	chip_select;
+};

Quoted 8 lines

drivers/mtd/altera_qspi.c

Quoted 8 lines

drivers/mtd/altera_qspi.c new file +8 -0

+
+struct altera_qspi_platdata {
+	struct altera_qspi_regs *regs;
+	void *base;
+	unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];	/* FLASH chips

info

Quoted 131 lines

Quoted 131 lines

*/ +

drivers/mtd/altera_qspi.c new file +23 -0

+void flash_print_info(flash_info_t *info)
+{
+	printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+	       info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+	struct mtd_info *mtd = info->mtd;
+	struct erase_info instr;
+	int ret;
+
+	memset(&instr, 0, sizeof(instr));
+	instr.addr = mtd->erasesize * s_first;
+	instr.len = mtd->erasesize * (s_last + 1 - s_first);
+	ret = mtd_erase(mtd, &instr);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong

cnt) +{

drivers/mtd/altera_qspi.c new file +29 -0

+	struct mtd_info *mtd = info->mtd;
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	ulong base = (ulong)pdata->base;
+	loff_t to = addr - base;
+	size_t retlen;
+	int ret;
+
+	ret = mtd_write(mtd, to, cnt, &retlen, src);
+	if (ret)
+		return ERR_NOT_ERASED;
+
+	return 0;
+}
+
+unsigned long flash_init(void)
+{
+	struct udevice *dev;
+
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+
+	return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info

*instr) +{

drivers/mtd/altera_qspi.c new file +34 -0

+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	size_t addr = instr->addr;
+	size_t len = instr->len;
+	size_t end = addr + len;
+	u32 sect;
+	u32 stat;
+
+	instr->state = MTD_ERASING;
+	addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+	while (addr < end) {
+		sect = addr / mtd->erasesize;
+		sect <<= 8;
+		sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+		debug("erase %08x\n", sect);
+		writel(sect, &regs->mem_op);
+		stat = readl(&regs->isr);
+		if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+			/* erase failed, sector might be protected */
+			debug("erase %08x fail %x\n", sect, stat);
+			writel(stat, &regs->isr); /* clear isr */
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		addr += mtd->erasesize;
+	}
+	instr->state = MTD_ERASE_DONE;
+	mtd_erase_callback(instr);
+
+	return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from,

size_t len, + size_t *retlen, u_char *buf)

drivers/mtd/altera_qspi.c new file +41 -0

+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+	memcpy(buf, pdata->base + from, len);
+	*retlen = len;
+
+	return 0;
+}
+
+static inline u32 add_byte(u32 data, u8 byte, int shift)
+{
+	data &= ~(0xff << shift);
+	data |= byte << shift;
+	return data;
+}
+
+static int altera_qspi_write_word(struct mtd_info *mtd, loff_t to,
+				  u32 data)
+{
+	struct udevice *dev = mtd->dev;
+	struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+	struct altera_qspi_regs *regs = pdata->regs;
+	u32 pos = (u32)to;
+	u32 stat;
+
+	/* write to flash 32 bits at a time */
+	writel(data, pdata->base + pos);
+	/* check whether write triggered a illegal write interrupt */
+	stat = readl(&regs->isr);
+	if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+		/* write failed, sector might be protected */
+		debug("write %08x fail %x\n", pos, stat);
+		writel(stat, &regs->isr); /* clear isr */
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/mtd/altera_qspi.c

Quoted 8 lines

len, + size_t *retlen, const u_char *buf)

drivers/mtd/altera_qspi.c new file +7 -0

+{
+	const u_char *end = buf + len;
+	unsigned shift;
+	u32 data;
+	int ret;
+
+	shift = (to & (sizeof(u32) - 1)) * 8; /* first shift to add byte 

*/

Quoted 29 lines

Quoted 29 lines

Quoted 28 lines

Quoted 9 lines

drivers/mtd/altera_qspi.c new file +9 -0

+	to &= ~(sizeof(u32) - 1); /* get lower aligned address */
+	while (buf < end) {
+		data = 0xffffffff; /* pad data */
+		while (buf < end && shift < 32) {
+			/* add byte from buf */
+			data = add_byte(data, *buf++, shift);
+			shift += 8;
+		}
+		ret = altera_qspi_write_word(mtd, to, data);

drivers/mtd/altera_qspi.c line 205

Quoted 10 lines

drivers/mtd/altera_qspi.c new file +10 -0

+		if (ret)
+			return ret;
+		to += sizeof(u32);
+		shift = 0;
+	}
+	*retlen = len;
+
+	return 0;
+}
+

Hi Thomas,

Thanks for the patch.

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode
+ address + dummy).

Quoted 7 lines

Quoted 2 lines

You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.

Actually I am thinking to take advantage the cache fill and dump. But after rethinking, this might limit some of the use case as we want the driver to support NIOS II without cache. With that, just ignore this comment for now.

I'm not sure I want to ask for details here. I think we're reading data from some sort of IO device, so we should just use readl() or readsl() to read them out (and write*() for the other direction). I don't think cache operations can be involved in any way. Correct me if I'm wrong please.

Quoted 2 lines

But your comment lead to the fact that the read part is now using memcpy. Thomas needs to fix that to use the readl :)

Uhm, I don't think I understand this remark, sorry. I never suggested to use memcpy() in this entire thread, did I ?

Quoted 13 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode
+ address + dummy).

Quoted 7 lines

Quoted 2 lines

You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.

Actually I am thinking to take advantage the cache fill and dump. But after rethinking, this might limit some of the use case as we want the driver to support NIOS II without cache. With that, just ignore this comment for now.

Quoted 5 lines

I'm not sure I want to ask for details here. I think we're reading data from some sort of IO device, so we should just use readl() or readsl() to read them out (and write*() for the other direction). I don't think cache operations can be involved in any way. Correct me if I'm wrong please.

Sure, I can share more. Since the read can support up to burst of 64 byte, we can use the a cache fill method which eventually trigger a read of 32 byte (which is size of a cache line) to the Quad SPI controller. To ensure we don't read from old data, we need to invalidate that cache line (through address). By doing this, we can gain better performance as we are reading 32 bytes of data instead 4 per transaction.

Quoted 3 lines

Quoted 2 lines

But your comment lead to the fact that the read part is now using memcpy. Thomas needs to fix that to use the readl :)

Quoted 2 lines

Uhm, I don't think I understand this remark, sorry. I never suggested to use memcpy() in this entire thread, did I ?

Nope, but this trigger me that we need to do the same for read. The memcpy might lead to the driver reading old data that stay on cache instead from controller. Another way to get rid of this is invalidate the cache.

HI Chin Liang,

Quoted 29 lines · Chin Liang See

Quoted 13 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I notice you are writing in word style which might have concern in performance. As the burst count can go up to 64, we can write larger data through memcpy. This will avoid redundancy of data header (opcode + address + dummy).

Quoted 7 lines

Quoted 2 lines

You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.

Actually I am thinking to take advantage the cache fill and dump. But after rethinking, this might limit some of the use case as we want the driver to support NIOS II without cache. With that, just ignore this comment for now.

Quoted 5 lines

I'm not sure I want to ask for details here. I think we're reading data from some sort of IO device, so we should just use readl() or readsl() to read them out (and write*() for the other direction). I don't think cache operations can be involved in any way. Correct me if I'm wrong please.

Sure, I can share more. Since the read can support up to burst of 64 byte, we can use the a cache fill method which eventually trigger a read of 32 byte (which is size of a cache line) to the Quad SPI controller. To ensure we don't read from old data, we need to invalidate that cache line (through address). By doing this, we can gain better performance as we are reading 32 bytes of data instead 4 per transaction.

Quoted 3 lines

Quoted 2 lines

But your comment lead to the fact that the read part is now using memcpy. Thomas needs to fix that to use the readl :)

Quoted 7 lines

Quoted 2 lines

Uhm, I don't think I understand this remark, sorry. I never suggested to use memcpy() in this entire thread, did I ?

Nope, but this trigger me that we need to do the same for read. The memcpy might lead to the driver reading old data that stay on cache instead from controller. Another way to get rid of this is invalidate the cache.

Thank for the reminding about the read part. I should not use memcpy() indeed.

Maybe we could pull the memcpy_fromio() and memcpy_toio() or the asm-generic/io.h from Linux kernel?

For i/o access, we bypass the cache for u-boot nios2 with ioremap() or map_physaddr(uncached). So no worries or advantage about cache.

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Signed-off-by Thomas Chou <[email protected]>

---
v2
use memcpy_toio() for mtd_write() as suggested by Chin Liang and Marek.

 doc/device-tree-bindings/mtd/altera_qspi.txt |  35 ++++
 drivers/mtd/Kconfig                          |   9 +
 drivers/mtd/Makefile                         |   1 +
 drivers/mtd/altera_qspi.c                    | 276 +++++++++++++++++++++++++++
 4 files changed, 321 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/altera_qspi.txt
 create mode 100644 drivers/mtd/altera_qspi.c

doc/device-tree-bindings/mtd/altera_qspi.txt new file +35 -0

new file mode 100644
index 0000000..3361ac9
@@ -0,0 +1,35 @@
+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
+       - #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 at 0x180014a0 {
+               compatible = "altr,quadspi-1.0";
+               reg = <0x180014a0 0x00000020>,
+                     <0x14000000 0x04000000>;
+               reg-names = "avl_csr", "avl_mem";
+               #address-cells = <1>;
+               #size-cells = <0>;
+               flash0: epcq512 at 0 {
+                       compatible = "altr,epcq512";
+                       #address-cells = <1>;
+                       #size-cells = <1>;
+               };
+       };

drivers/mtd/Kconfig +9 -0

index 367c4fe..c16b1d0 100644
@@ -19,6 +19,15 @@ config CFI_FLASH
          option. Visit <http://www.amd.com/products/nvd/overview/cfi.html>
          for more information on CFI.
 
+config ALTERA_QSPI
+       bool "Altera Generic Quad SPI Controller"
+       depends on MTD
+       help
+         This enables access to Altera EPCQ/EPCS flash chips using the
+         Altera Generic Quad SPI Controller. The controller converts SPI
+         NOR flash to parallel flash interface. Please find details on the
+         "Embedded Peripherals IP User Guide" of Altera.
+
 endmenu
 
 source "drivers/mtd/nand/Kconfig"

drivers/mtd/Makefile +1 -0

index c23c0c1..7f018a4 100644
@@ -11,6 +11,7 @@ endif
 obj-$(CONFIG_MTD) += mtd-uclass.o
 obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
 obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
+obj-$(CONFIG_ALTERA_QSPI) += altera_qspi.o
 obj-$(CONFIG_HAS_DATAFLASH) += at45.o
 obj-$(CONFIG_FLASH_CFI_DRIVER) += cfi_flash.o
 obj-$(CONFIG_FLASH_CFI_MTD) += cfi_mtd.o

drivers/mtd/altera_qspi.c new file +276 -1

new file mode 100644
index 0000000..c32b1d3
@@ -0,0 +1,276 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdt_support.h>
+#include <flash.h>
+#include <mtd.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * The QUADSPI_MEM_OP register is used to do memory protect and erase operations
+ */
+#define QUADSPI_MEM_OP_BULK_ERASE              0x00000001
+#define QUADSPI_MEM_OP_SECTOR_ERASE            0x00000002
+#define QUADSPI_MEM_OP_SECTOR_PROTECT          0x00000003
+
+/*
+ * The QUADSPI_ISR register is used to determine whether an invalid write or
+ * erase operation trigerred an interrupt
+ */
+#define QUADSPI_ISR_ILLEGAL_ERASE              BIT(0)
+#define QUADSPI_ISR_ILLEGAL_WRITE              BIT(1)
+
+struct altera_qspi_regs {
+       u32     rd_status;
+       u32     rd_sid;
+       u32     rd_rdid;
+       u32     mem_op;
+       u32     isr;
+       u32     imr;
+       u32     chip_select;
+};
+
+struct altera_qspi_platdata {
+       struct altera_qspi_regs *regs;
+       void *base;
+       unsigned long size;
+};
+
+flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS];   /* FLASH chips info */
+
+void flash_print_info(flash_info_t *info)
+{
+       printf("Altera QSPI flash  Size: %ld MB in %d Sectors\n",
+              info->size >> 20, info->sector_count);
+}
+
+int flash_erase(flash_info_t *info, int s_first, int s_last)
+{
+       struct mtd_info *mtd = info->mtd;
+       struct erase_info instr;
+       int ret;
+
+       memset(&instr, 0, sizeof(instr));
+       instr.addr = mtd->erasesize * s_first;
+       instr.len = mtd->erasesize * (s_last + 1 - s_first);
+       ret = mtd_erase(mtd, &instr);
+       if (ret)
+               return ERR_NOT_ERASED;
+
+       return 0;
+}
+
+int write_buff(flash_info_t *info, uchar *src, ulong addr, ulong cnt)
+{
+       struct mtd_info *mtd = info->mtd;
+       struct udevice *dev = mtd->dev;
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+       ulong base = (ulong)pdata->base;
+       loff_t to = addr - base;
+       size_t retlen;
+       int ret;
+
+       ret = mtd_write(mtd, to, cnt, &retlen, src);
+       if (ret)
+               return ERR_NOT_ERASED;
+
+       return 0;
+}
+
+unsigned long flash_init(void)
+{
+       struct udevice *dev;
+
+       /* probe every MTD device */
+       for (uclass_first_device(UCLASS_MTD, &dev);
+            dev;
+            uclass_next_device(&dev)) {
+       }
+
+       return flash_info[0].size;
+}
+
+static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+       struct udevice *dev = mtd->dev;
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+       struct altera_qspi_regs *regs = pdata->regs;
+       size_t addr = instr->addr;
+       size_t len = instr->len;
+       size_t end = addr + len;
+       u32 sect;
+       u32 stat;
+
+       instr->state = MTD_ERASING;
+       addr &= ~(mtd->erasesize - 1); /* get lower aligned address */
+       while (addr < end) {
+               sect = addr / mtd->erasesize;
+               sect <<= 8;
+               sect |= QUADSPI_MEM_OP_SECTOR_ERASE;
+               debug("erase %08x\n", sect);
+               writel(sect, &regs->mem_op);
+               stat = readl(&regs->isr);
+               if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
+                       /* erase failed, sector might be protected */
+                       debug("erase %08x fail %x\n", sect, stat);
+                       writel(stat, &regs->isr); /* clear isr */
+                       instr->state = MTD_ERASE_FAILED;
+                       return -EIO;
+               }
+               addr += mtd->erasesize;
+       }
+       instr->state = MTD_ERASE_DONE;
+       mtd_erase_callback(instr);
+
+       return 0;
+}
+
+static int altera_qspi_read(struct mtd_info *mtd, loff_t from, size_t len,
+                           size_t *retlen, u_char *buf)
+{
+       struct udevice *dev = mtd->dev;
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+
+       memcpy_fromio(buf, pdata->base + from, len);
+       *retlen = len;
+
+       return 0;
+}
+
+static int altera_qspi_write(struct mtd_info *mtd, loff_t to, size_t len,
+                            size_t *retlen, const u_char *buf)
+{
+       struct udevice *dev = mtd->dev;
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+       struct altera_qspi_regs *regs = pdata->regs;
+       u32 stat;
+
+       memcpy_toio(pdata->base + to, buf, len);
+       /* check whether write triggered a illegal write interrupt */
+       stat = readl(&regs->isr);
+       if (stat & QUADSPI_ISR_ILLEGAL_WRITE) {
+               /* write failed, sector might be protected */
+               debug("write fail %x\n", stat);
+               writel(stat, &regs->isr); /* clear isr */
+               return -EIO;
+       }
+       *retlen = len;
+
+       return 0;
+}
+
+static void altera_qspi_sync(struct mtd_info *mtd)
+{
+}
+
+static int altera_qspi_probe(struct udevice *dev)
+{
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+       struct altera_qspi_regs *regs = pdata->regs;
+       unsigned long base = (unsigned long)pdata->base;
+       struct mtd_info *mtd;
+       flash_info_t *flash = &flash_info[0];
+       u32 rdid;
+       int i;
+
+       rdid = readl(&regs->rd_rdid);
+       debug("rdid %x\n", rdid);
+
+       mtd = calloc(1, sizeof(struct mtd_info));
+       if (!mtd)
+               return -ENOMEM;
+       dev->uclass_priv = mtd;
+       mtd->dev = dev;
+       mtd->name               = "nor0";
+       mtd->type               = MTD_NORFLASH;
+       mtd->flags              = MTD_CAP_NORFLASH;
+       mtd->size               = 1 << ((rdid & 0xff) - 6);
+       mtd->writesize          = 1;
+       mtd->writebufsize       = mtd->writesize;
+       mtd->_erase             = altera_qspi_erase;
+       mtd->_read              = altera_qspi_read;
+       mtd->_write             = altera_qspi_write;
+       mtd->_sync              = altera_qspi_sync;
+       mtd->numeraseregions = 0;
+       mtd->erasesize = 0x10000;
+       if (add_mtd_device(mtd))
+               return -ENOMEM;
+
+       flash->mtd = mtd;
+       flash->size = mtd->size;
+       flash->sector_count = mtd->size / mtd->erasesize;
+       flash->flash_id = rdid;
+       flash->start[0] = base;
+       for (i = 1; i < flash->sector_count; i++)
+               flash->start[i] = flash->start[i - 1] + mtd->erasesize;
+       gd->bd->bi_flashstart = base;
+
+       return 0;
+}
+
+static int altera_qspi_ofdata_to_platdata(struct udevice *dev)
+{
+       struct altera_qspi_platdata *pdata = dev_get_platdata(dev);
+       void *blob = (void *)gd->fdt_blob;
+       int node = dev->of_offset;
+       const char *list, *end;
+       const fdt32_t *cell;
+       void *base;
+       unsigned long addr, size;
+       int parent, addrc, sizec;
+       int len, idx;
+
+       /*
+        * decode regs. there are multiple reg tuples, and they need to
+        * match with reg-names.
+        */
+       parent = fdt_parent_offset(blob, node);
+       of_bus_default_count_cells(blob, parent, &addrc, &sizec);
+       list = fdt_getprop(blob, node, "reg-names", &len);
+       if (!list)
+               return -ENOENT;
+       end = list + len;
+       cell = fdt_getprop(blob, node, "reg", &len);
+       if (!cell)
+               return -ENOENT;
+       idx = 0;
+       while (list < end) {
+               addr = fdt_translate_address((void *)blob,
+                                            node, cell + idx);
+               size = fdt_addr_to_cpu(cell[idx + addrc]);
+               base = ioremap(addr, size);
+               len = strlen(list);
+               if (strcmp(list, "avl_csr") == 0) {
+                       pdata->regs = base;
+               } else if (strcmp(list, "avl_mem") == 0) {
+                       pdata->base = base;
+                       pdata->size = size;
+               }
+               idx += addrc + sizec;
+               list += (len + 1);
+       }
+
+       return 0;
+}
+
+static const struct udevice_id altera_qspi_ids[] = {
+       { .compatible = "altr,quadspi-1.0" },
+       {}
+};
+
+U_BOOT_DRIVER(altera_qspi) = {
+       .name   = "altera_qspi",
+       .id     = UCLASS_MTD,
+       .of_match = altera_qspi_ids,
+       .ofdata_to_platdata = altera_qspi_ofdata_to_platdata,
+       .platdata_auto_alloc_size = sizeof(struct altera_qspi_platdata),
+       .probe  = altera_qspi_probe,
+};
--

2.5.0

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Can you wait till spi-nor ready? it doesn't make sense to have Serial NOR to created as parallel NOR.

Hi Jagan,

Quoted 10 lines · Jagan Teki

Quoted 4 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Can you wait till spi-nor ready? it doesn't make sense to have Serial NOR to created as parallel NOR.

The altera quad spi core is very special that the hardware handle the spi-nor protocol. The core is designed to replace the CFI flash interface. So there is nothing to do with SPI from the parallel flash interface. It is memory mapped. There is no SPI interface. There is nothing related to SPI programming. So please don't worry about the progress on spi-nor. The core should belong to parallel flash, but not serial flash.

Hi Thomas,

Quoted 11 lines · Thomas Chou

Hi Jagan,

Quoted 7 lines · Jagan Teki

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Can you wait till spi-nor ready? it doesn't make sense to have Serial NOR to created as parallel NOR.

Quoted 7 lines

The altera quad spi core is very special that the hardware handle the spi-nor protocol. The core is designed to replace the CFI flash interface. So there is nothing to do with SPI from the parallel flash interface. It is memory mapped. There is no SPI interface. There is nothing related to SPI programming. So please don't worry about the progress on spi-nor. The core should belong to parallel flash, but not serial flash.

Agreed that this is not doing any generic spi things, but it's a spi-nor controller all spi-nor controller should be part of spi-nor subsystem Linux agreed and have a framework for that.

drivers/mtd/spi-nor/fsl-quadspi.c
drivers/mtd/spi-nor/nxp-spifi.c

all these are spi-nor controller which doesn't do any generic spi things but should be in spi-nor subsystem. Even Marek send altera_qspi as spi-nor controller [1]

Since I'm working on similar spi-nor subsystem what Linux have + driver model little worried about this because once we have spi-nor again it should be a re-work.

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

Quoted 22 lines · Jagan Teki

Hi Thomas,

Quoted 11 lines · Thomas Chou

Hi Jagan,

Quoted 7 lines · Jagan Teki

Quoted 3 lines · Thomas Chou

Add Altera Generic Quad SPI Controller support. The controller converts SPI NOR flash to parallel flash interface. So it is not like other SPI flash, but rather like CFI flash.

Can you wait till spi-nor ready? it doesn't make sense to have Serial NOR to created as parallel NOR.

Quoted 7 lines

The altera quad spi core is very special that the hardware handle the spi-nor protocol. The core is designed to replace the CFI flash interface. So there is nothing to do with SPI from the parallel flash interface. It is memory mapped. There is no SPI interface. There is nothing related to SPI programming. So please don't worry about the progress on spi-nor. The core should belong to parallel flash, but not serial flash.

Quoted 3 lines

Agreed that this is not doing any generic spi things, but it's a spi-nor controller all spi-nor controller should be part of spi-nor subsystem Linux agreed and have a framework for that.

The underlying technology is not exposed to the programmer, so this argument is moot. The behavior of this device is closer to CFI flash.

Quoted 6 lines

drivers/mtd/spi-nor/fsl-quadspi.c
drivers/mtd/spi-nor/nxp-spifi.c

all these are spi-nor controller which doesn't do any generic spi things but should be in spi-nor subsystem. Even Marek send altera_qspi as spi-nor controller [1]

This is because that thing communicates like a SPI controller.

Quoted 4 lines

Since I'm working on similar spi-nor subsystem what Linux have + driver model little worried about this because once we have spi-nor again it should be a re-work.

Your work and this driver are orthogonal things.

Quoted 2 lines

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

Quoted 5 lines

Quoted 3 lines

all these are spi-nor controller which doesn't do any generic spi things but should be in spi-nor subsystem. Even Marek send altera_qspi as spi-nor controller [1]

This is because that thing communicates like a SPI controller.

OK, the driver you sent to Linux is not same as this - means not same controller?

Quoted 5 lines

Quoted 3 lines

Since I'm working on similar spi-nor subsystem what Linux have + driver model little worried about this because once we have spi-nor again it should be a re-work.

Your work and this driver are orthogonal things.

OK, let's park my work a side - do you agree with me like this driver should be part of spi-nor?

The altera quad spi core is very special that the hardware handle the

Quoted 5 lines

spi-nor protocol. The core is designed to replace the CFI flash interface. So there is nothing to do with SPI from the parallel flash interface. It is memory mapped. There is no SPI interface. There is nothing related to SPI programming. So please don't worry about the progress on spi-nor. The core should belong to parallel flash, but not serial flash.

Agreed that this is not doing any generic spi things, but it's a spi-nor controller all spi-nor controller should be part of spi-nor subsystem Linux agreed and have a framework for that.

drivers/mtd/spi-nor/fsl-quadspi.c
drivers/mtd/spi-nor/nxp-spifi.c

all these are spi-nor controller which doesn't do any generic spi things but should be in spi-nor subsystem. Even Marek send altera_qspi as spi-nor controller [1]

Since I'm working on similar spi-nor subsystem what Linux have + driver model little worried about this because once we have spi-nor again it should be a re-work.

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

Hi Jagan,

Quoted 24 lines · Jagan Teki

The altera quad spi core is very special that the hardware handle the

Quoted 9 lines

spi-nor protocol. The core is designed to replace the CFI flash interface.
So there is nothing to do with SPI from the parallel flash interface. It is
memory mapped. There is no SPI interface. There is nothing related to SPI programming. So please don't worry about the progress on spi-nor. The core
should belong to parallel flash, but not serial flash.

Agreed that this is not doing any generic spi things, but it's a spi-nor controller all spi-nor controller should be part of spi-nor subsystem Linux agreed and have a framework for that.

drivers/mtd/spi-nor/fsl-quadspi.c
drivers/mtd/spi-nor/nxp-spifi.c

all these are spi-nor controller which doesn't do any generic spi things but should be in spi-nor subsystem. Even Marek send altera_qspi as spi-nor controller [1]

Quoted 5 lines

Since I'm working on similar spi-nor subsystem what Linux have + driver model little worried about this because once we have spi-nor again it should be a re-work.

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

I had been a hardware engineer. I designed chips and boards. I believe this should depend on the programming interface. Eg, there are SSDs of SATA interfaces which build with NAND or EMMC, and it dose not make sense to classify them as NAND or MMC. The altera quadspi core is a similar case, because the hardware converts the programming interface. There is no way to send spi-nor commands that the spi frame work does. It is ridiculous to insist it to use under the spi-nor framework.

The altera quadspi hides details about the underlying spi-nor. We may even choose to build interface that convert EMMC to like cfi flash. The users can run code directly on the flash or use memory command like "cp.b" of u-boot to program the flash. This can be a advantage over the "sf" command.

I appreciate your hardware expertise and am not questioning about that as well. I do agree with the hw logic about altera qspi controller and I don't have any questions with hw either.

But my main intention here was about the software support since Linux discussed about this subject and finally moved to spi-nor controllers (though they don't use spi bus at-all) to spi-nor framework and Marek knows this topic very well.

I believe discussion about this topic much more clear if we send this driver to Linux.

Hi Jagan,

Quoted 10 lines · Jagan Teki

I appreciate your hardware expertise and am not questioning about that as well. I do agree with the hw logic about altera qspi controller and I don't have any questions with hw either.

But my main intention here was about the software support since Linux discussed about this subject and finally moved to spi-nor controllers (though they don't use spi bus at-all) to spi-nor framework and Marek knows this topic very well.

Quoted 2 lines

I believe discussion about this topic much more clear if we send this driver to Linux.

Sorry that I was absent and didn't know about the topic discussed on Linux. The purpose of u-boot and Linux is quite different thought they may share some code. The drivers are different, too. Whatever they chose on Linux is not related here.

Marek did agree that "The behavior of this device is closer to CFI flash.". This is what the core designed for, to replace the CFI flash. I am more concerned with users. Users can use memory commands to display and modify the content directly. This is very point where the driver should stand.

Hi Thomas,

Quoted 13 lines · Thomas Chou

Hi Jagan,

Quoted 10 lines · Jagan Teki

I appreciate your hardware expertise and am not questioning about that as well. I do agree with the hw logic about altera qspi controller and I don't have any questions with hw either.

But my main intention here was about the software support since Linux discussed about this subject and finally moved to spi-nor controllers (though they don't use spi bus at-all) to spi-nor framework and Marek knows this topic very well.

Quoted 8 lines

Quoted 2 lines

I believe discussion about this topic much more clear if we send this driver to Linux.

Sorry that I was absent and didn't know about the topic discussed on Linux. The purpose of u-boot and Linux is quite different thought they may share some code. The drivers are different, too. Whatever they chose on Linux is not related here.

I'm trying to interface the same standard wrt Linux stack, So adding new features is reasonable and easy for developers to send the patches if we have same stand-point as what Linux or vice-versa. Yes drivers are bit different wrt Linux ie OK, anyway we need to implement with dm-driven but the functionality flow will be same like

Linux --> u-boot
mtd_utils --> cmd_sf
mtd_info -->mtd_info
spi-nor --> spi-nor
m25p80.c --> spi-nor-flash (dm-driven)
fsl-quadspi --> fsl-qspi (dm-driven)

Finally, what is the main issue if we move to spi-nor instead of cfi? agreed that it never used generic spi but even if it there in spi-nor also it never used generic spi, spi-nor should also use mtd as cfi and also it's serial flash it should be part of spi-nor that make more sense to me.

Quoted 1 line

Quoted 4 lines

Marek did agree that "The behavior of this device is closer to CFI flash.". This is what the core designed for, to replace the CFI flash. I am more concerned with users. Users can use memory commands to display and modify the content directly. This is very point where the driver should stand.

Yes from uses point of view, it's a cfi flash but generally all cfi's are parallel in nature so it's shouldn't treated as serial. If we use sf user at least thought he would doing serial flash operations.

My intention is not to hold this, but once spi-nor is ready will you able to move it from here?