| Message ID | 1446556932-14978-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Superseded |
| Headers | show |
On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: > 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 <thomas@wytron.com.tw> > --- 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 . [...] Best regards, Marek Vasut
On 3 November 2015 at 23:14, Marek Vasut <marex@denx.de> wrote: > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: >> 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. >> >> Signed-off-by: Thomas Chou <thomas@wytron.com.tw> >> --- > > 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 . > thanks!
On Tuesday, November 03, 2015 at 06:49:44 PM, Jagan Teki wrote: > On 3 November 2015 at 23:14, Marek Vasut <marex@denx.de> wrote: > > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: > >> 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. I think your patches are still far away from inclusion, so I'm fine with the current way of doing things ;-) Best regards, Marek Vasut
On 3 November 2015 at 23:22, Marek Vasut <marex@denx.de> wrote: > On Tuesday, November 03, 2015 at 06:49:44 PM, Jagan Teki wrote: >> On 3 November 2015 at 23:14, Marek Vasut <marex@denx.de> wrote: >> > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: >> >> 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. > > 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. thanks!
On Tuesday, November 03, 2015 at 06:56:26 PM, Jagan Teki wrote: > On 3 November 2015 at 23:22, Marek Vasut <marex@denx.de> wrote: > > On Tuesday, November 03, 2015 at 06:49:44 PM, Jagan Teki wrote: > >> On 3 November 2015 at 23:14, Marek Vasut <marex@denx.de> wrote: > >> > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: > >> >> 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. > > > > 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. That's fine all right, but you cannot block other patches only because there are some other unfinished ones which might land in mainline who knows when. Best regards, Marek Vasut
Hi Marek, On 2015年11月04日 01:44, Marek Vasut wrote: > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: >> 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 <thomas@wytron.com.tw> >> --- > > 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. Best regards, Thomas
On Wednesday, November 04, 2015 at 03:36:08 AM, Thomas Chou wrote: > Hi Marek, Hi! > On 2015年11月04日 01:44, Marek Vasut wrote: > > On Tuesday, November 03, 2015 at 02:22:12 PM, Thomas Chou wrote: > >> 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 <thomas@wytron.com.tw> > >> --- > > > > 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. I think I had a stake there as well ;-) > 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 Best regards, Marek Vasut
Hi Marek, On 2015年11月04日 11:45, Marek Vasut wrote: > 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. Regards, Thomas
On Wednesday, November 04, 2015 at 05:45:24 AM, Thomas Chou wrote: > Hi Marek, Hi, > On 2015年11月04日 11:45, Marek Vasut wrote: > > 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. 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. Best regards, Marek Vasut
Hi Marek, On 2015年11月04日 13:15, Marek Vasut wrote: > On Wednesday, November 04, 2015 at 05:45:24 AM, Thomas Chou wrote: >> Hi Marek, > > Hi, > >> On 2015年11月04日 11:45, Marek Vasut wrote: >>> 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. > > 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. > > But now that you mention the erasing, I see why you'd opt for the CFI > framework, yeah. Yes. Best regards, Thomas
On Wednesday, November 04, 2015 at 06:33:00 AM, Thomas Chou wrote: > Hi Marek, Hi, > On 2015年11月04日 13:15, Marek Vasut wrote: > > On Wednesday, November 04, 2015 at 05:45:24 AM, Thomas Chou wrote: > >> Hi Marek, > > > > Hi, > > > >> On 2015年11月04日 11:45, Marek Vasut wrote: > >>> 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. > > > > 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. > > > But now that you mention the erasing, I see why you'd opt for the CFI > > framework, yeah. > > Yes. OK, got it. The CFI approach might be the most sensible one then. Thanks for your patience :) Best regards, Marek Vasut
On Tue, 2015-11-03 at 21:22 +0800, thomas@wytron.com.tw wrote: > 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 <thomas@wytron.com.tw> > --- > 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 > ... > > diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c > new file mode 100644 > index 0000000..06bc53e > --- /dev/null > +++ b/drivers/mtd/altera_qspi.c > @@ -0,0 +1,312 @@ > +/* > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > + * > + * 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, ®s->mem_op); > + stat = readl(®s->isr); > + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { > + /* erase failed, sector might be protected */ > + debug("erase %08x fail %x\n", sect, stat); > + writel(stat, ®s->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(®s->isr); > + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { > + /* write failed, sector might be protected */ > + debug("write %08x fail %x\n", pos, stat); > + writel(stat, ®s->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); > + 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). Thanks Chin Liang
On Wednesday, November 04, 2015 at 04:56:10 PM, Chin Liang See wrote: > On Tue, 2015-11-03 at 21:22 +0800, thomas@wytron.com.tw wrote: > > 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 <thomas@wytron.com.tw> > > --- > > > > 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 > > ... > > > > diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c > > new file mode 100644 > > index 0000000..06bc53e > > --- /dev/null > > +++ b/drivers/mtd/altera_qspi.c > > @@ -0,0 +1,312 @@ > > +/* > > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > > + * > > + * 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, ®s->mem_op); > > + stat = readl(®s->isr); > > + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { > > + /* erase failed, sector might be protected */ > > + debug("erase %08x fail %x\n", sect, stat); > > + writel(stat, ®s->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(®s->isr); > > + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { > > + /* write failed, sector might be protected */ > > + debug("write %08x fail %x\n", pos, stat); > > + writel(stat, ®s->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); > > + 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). You cannot do that, memcpy works on memory while write*() operators work on I/O. You should use readsl() and friends then.
Hi Marek, On Wed, 2015-11-04 at 10:18 +0000, marex@denx.de wrote: > On Wednesday, November 04, 2015 at 04:56:10 PM, Chin Liang See wrote: > > On Tue, 2015-11-03 at 21:22 +0800, thomas@wytron.com.tw wrote: > > > 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 <thomas@wytron.com.tw> > > > --- > > > > > > 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 > > > ... > > > > > > diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c > > > new file mode 100644 > > > index 0000000..06bc53e > > > --- /dev/null > > > +++ b/drivers/mtd/altera_qspi.c > > > @@ -0,0 +1,312 @@ > > > +/* > > > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > > > + * > > > + * 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, ®s->mem_op); > > > + stat = readl(®s->isr); > > > + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { > > > + /* erase failed, sector might be protected */ > > > + debug("erase %08x fail %x\n", sect, stat); > > > + writel(stat, ®s->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(®s->isr); > > > + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { > > > + /* write failed, sector might be protected */ > > > + debug("write %08x fail %x\n", pos, stat); > > > + writel(stat, ®s->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); > > > + 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). > > 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 :) Thanks Chin Liang
On Thursday, November 05, 2015 at 03:49:18 AM, Chin Liang See wrote: > Hi Marek, > > On Wed, 2015-11-04 at 10:18 +0000, marex@denx.de wrote: > > On Wednesday, November 04, 2015 at 04:56:10 PM, Chin Liang See wrote: > > > On Tue, 2015-11-03 at 21:22 +0800, thomas@wytron.com.tw wrote: > > > > 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 <thomas@wytron.com.tw> > > > > --- > > > > > > > > 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 > > > > ... > > > > > > > > diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c > > > > new file mode 100644 > > > > index 0000000..06bc53e > > > > --- /dev/null > > > > +++ b/drivers/mtd/altera_qspi.c > > > > @@ -0,0 +1,312 @@ > > > > +/* > > > > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > > > > + * > > > > + * 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, ®s->mem_op); > > > > + stat = readl(®s->isr); > > > > + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { > > > > + /* erase failed, sector might be protected */ > > > > + debug("erase %08x fail %x\n", sect, stat); > > > > + writel(stat, ®s->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(®s->isr); > > > > + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { > > > > + /* write failed, sector might be protected */ > > > > + debug("write %08x fail %x\n", pos, stat); > > > > + writel(stat, ®s->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); > > > > + 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). > > > > 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. > 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 ?
On Thu, 2015-11-05 at 03:53 +0100, marex@denx.de wrote: > On Thursday, November 05, 2015 at 03:49:18 AM, Chin Liang See wrote: > > Hi Marek, > > > > On Wed, 2015-11-04 at 10:18 +0000, marex@denx.de wrote: > > > On Wednesday, November 04, 2015 at 04:56:10 PM, Chin Liang See wrote: > > > > On Tue, 2015-11-03 at 21:22 +0800, thomas@wytron.com.tw wrote: > > > > > 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 <thomas@wytron.com.tw> > > > > > --- > > > > > > > > > > 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 > > > > > ... > > > > > > > > > > diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c > > > > > new file mode 100644 > > > > > index 0000000..06bc53e > > > > > --- /dev/null > > > > > +++ b/drivers/mtd/altera_qspi.c > > > > > @@ -0,0 +1,312 @@ > > > > > +/* > > > > > + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> > > > > > + * > > > > > + * 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, ®s->mem_op); > > > > > + stat = readl(®s->isr); > > > > > + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { > > > > > + /* erase failed, sector might be protected */ > > > > > + debug("erase %08x fail %x\n", sect, stat); > > > > > + writel(stat, ®s->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(®s->isr); > > > > > + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { > > > > > + /* write failed, sector might be protected */ > > > > > + debug("write %08x fail %x\n", pos, stat); > > > > > + writel(stat, ®s->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); > > > > > + 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). > > > > > > 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. > 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. > > 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 ? 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. Thanks Chin Liang
HI Chin Liang, On 2015年11月05日 11:05, Chin Liang See wrote: >>>>> 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. >>> >>> 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. >> > > 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. > >>> 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 ? > > > 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. Best regards, Thomas
On Thursday, November 05, 2015 at 05:26:25 AM, Thomas Chou wrote: > HI Chin Liang, > > On 2015年11月05日 11:05, Chin Liang See wrote: > >>>>> 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. > >>> > >>> 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. > > > > 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. > > > >>> 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 ? > > > > 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. Oh, you need those memcpy_{from,to}io() for this hardware? In that case, go ahead and either implement them or use them :) Now I understand why you cannot use the reads*() function. Best regards, Marek Vasut
diff --git a/doc/device-tree-bindings/mtd/altera_qspi.txt b/doc/device-tree-bindings/mtd/altera_qspi.txt new file mode 100644 index 0000000..3361ac9 --- /dev/null +++ b/doc/device-tree-bindings/mtd/altera_qspi.txt @@ -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@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@0 { + compatible = "altr,epcq512"; + #address-cells = <1>; + #size-cells = <1>; + }; + }; diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index 367c4fe..c16b1d0 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -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" diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index c23c0c1..7f018a4 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -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 diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c new file mode 100644 index 0000000..06bc53e --- /dev/null +++ b/drivers/mtd/altera_qspi.c @@ -0,0 +1,312 @@ +/* + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw> + * + * 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, ®s->mem_op); + stat = readl(®s->isr); + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) { + /* erase failed, sector might be protected */ + debug("erase %08x fail %x\n", sect, stat); + writel(stat, ®s->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(®s->isr); + if (stat & QUADSPI_ISR_ILLEGAL_WRITE) { + /* write failed, sector might be protected */ + debug("write %08x fail %x\n", pos, stat); + writel(stat, ®s->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); + if (ret) + return ret; + to += sizeof(u32); + shift = 0; + } + *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(®s->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, +};
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 <thomas@wytron.com.tw> --- 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