[U-Boot] [PATCH 1/5] altera_qspi: call callback even if the erase failed
Erase is an asynchronous operation. Device drivers are supposed to call instr->callback() whenever the operation completes, even if it completes with a failure.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/mtd/altera_qspi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index c7e37ad..627a8cc 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -146,6 +146,7 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) debug("erase %08x fail %x\n", sect, stat); writel(stat, ®s->isr); /* clear isr */ instr->state = MTD_ERASE_FAILED; + mtd_erase_callback(instr); return -EIO; } addr += mtd->erasesize;
If the erase fails, fail_addr might indicate exactly which block failed. If fail_addr = MTD_FAIL_ADDR_UNKNOWN, the failure was not at the device level or was not specific to any particular block.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/mtd/altera_qspi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index 627a8cc..b0d4f2c 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -145,6 +145,7 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) /* erase failed, sector might be protected */ debug("erase %08x fail %x\n", sect, stat); writel(stat, ®s->isr); /* clear isr */ + instr->fail_addr = addr; instr->state = MTD_ERASE_FAILED; mtd_erase_callback(instr); return -EIO;
On 2015年12月24日 08:51, Thomas Chou wrote:
If the erase fails, fail_addr might indicate exactly which block failed. If fail_addr = MTD_FAIL_ADDR_UNKNOWN, the failure was not at the device level or was not specific to any particular block.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-nios.
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len; u32 sect; u32 stat; + u32 *flash, *last;
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->fail_addr = addr; - instr->state = MTD_ERASE_FAILED; - mtd_erase_callback(instr); - return -EIO; + flash = pdata->base + addr; + last = pdata->base + addr + mtd->erasesize; + /* skip erase if sector is blank */ + while (flash < last) { + if (readl(flash) != 0xffffffff) + break; + flash++; + } + if (flash < last) { + 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->fail_addr = addr; + instr->state = MTD_ERASE_FAILED; + mtd_erase_callback(instr); + return -EIO; + } } addr += mtd->erasesize; }
On Thursday, December 24, 2015 at 01:51:22 AM, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len; u32 sect; u32 stat;
u32 *flash, *last;
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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;
flash = pdata->base + addr;last = pdata->base + addr + mtd->erasesize;/* skip erase if sector is blank */while (flash < last) {if (readl(flash) != 0xffffffff)break;flash++;
Shouldn't $last be divided by 4 ? $flash is u32 * afterall .
}if (flash < last) {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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;}
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 09:28, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:22 AM, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len; u32 sect; u32 stat;
u32 *flash, *last;
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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;
flash = pdata->base + addr;last = pdata->base + addr + mtd->erasesize;/* skip erase if sector is blank */while (flash < last) {if (readl(flash) != 0xffffffff)break;flash++;Shouldn't $last be divided by 4 ? $flash is u32 * afterall .
No. Both flash and last are assigned with byte addressing from pdata->base, which is void *.
Best regards, Thomas
On Thursday, December 24, 2015 at 03:23:05 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:28, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:22 AM, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len;
u32 sect; u32 stat;
u32 *flash, *last;
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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;
flash = pdata->base + addr;last = pdata->base + addr + mtd->erasesize;/* skip erase if sector is blank */while (flash < last) {if (readl(flash) != 0xffffffff)break;flash++;Shouldn't $last be divided by 4 ? $flash is u32 * afterall .
No. Both flash and last are assigned with byte addressing from pdata->base, which is void *.
The data type of both $flash and $last is u32 * though?
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 11:01, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:23:05 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:28, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:22 AM, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len;
u32 sect; u32 stat;
u32 *flash, *last;
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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;
flash = pdata->base + addr;last = pdata->base + addr + mtd->erasesize;/* skip erase if sector is blank */while (flash < last) {if (readl(flash) != 0xffffffff)break;flash++;Shouldn't $last be divided by 4 ? $flash is u32 * afterall .
No. Both flash and last are assigned with byte addressing from pdata->base, which is void *.
The data type of both $flash and $last is u32 * though?
Yes.
flash = pdata->base + addr;
will be the same as,
flash = pdata->base; flash += addr / 4;
Best regards, Thomas
On Thursday, December 24, 2015 at 04:08:33 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 11:01, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:23:05 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:28, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:22 AM, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index b0d4f2c..8a630a6 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -131,24 +131,35 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) size_t end = addr + len;
u32 sect; u32 stat;
u32 *flash, *last;
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->fail_addr = addr;instr->state = MTD_ERASE_FAILED;mtd_erase_callback(instr);return -EIO;
flash = pdata->base + addr;last = pdata->base + addr + mtd->erasesize;/* skip erase if sector is blank */while (flash < last) {if (readl(flash) != 0xffffffff)break;flash++;Shouldn't $last be divided by 4 ? $flash is u32 * afterall .
No. Both flash and last are assigned with byte addressing from pdata->base, which is void *.
The data type of both $flash and $last is u32 * though?
Yes.
flash = pdata->base + addr;
will be the same as,
flash = pdata->base; flash += addr / 4;
Please ignore what I said, sorry.
On 2015年12月24日 08:51, Thomas Chou wrote:
Skip erase if the sector is blank. The sector erase is slow, and may take 0.7 sec typically or up to 3 sec worst-case.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
Applied to u-boot-nios.
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/mtd/altera_qspi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index 8a630a6..0624ff4 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -52,6 +52,7 @@ struct altera_qspi_platdata { unsigned long size; };
+static uint flash_verbose; flash_info_t flash_info[CONFIG_SYS_MAX_FLASH_BANKS]; /* FLASH chips info */
static void altera_qspi_get_locked_range(struct mtd_info *mtd, loff_t *ofs, @@ -74,6 +75,11 @@ void flash_print_info(flash_info_t *info) putc('\n'); }
+void flash_set_verbose(uint v) +{ + flash_verbose = v; +} + int flash_erase(flash_info_t *info, int s_first, int s_last) { struct mtd_info *mtd = info->mtd; @@ -84,10 +90,13 @@ int flash_erase(flash_info_t *info, int s_first, int s_last) instr.mtd = mtd; instr.addr = mtd->erasesize * s_first; instr.len = mtd->erasesize * (s_last + 1 - s_first); + flash_set_verbose(1); ret = mtd_erase(mtd, &instr); + flash_set_verbose(0); if (ret) return ERR_PROTECTED;
+ puts(" done\n"); return 0; }
@@ -160,6 +169,11 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) mtd_erase_callback(instr); return -EIO; } + if (flash_verbose) + putc('.'); + } else { + if (flash_verbose) + putc(','); } addr += mtd->erasesize; }
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
Alternatively, we might set the flash_verbose directly and remove the flash_set_verbose().
Best regards, Thomas
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
Alternatively, we might set the flash_verbose directly and remove the flash_set_verbose().
Best regards, Thomas
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Best regards, Thomas
On Thursday, December 24, 2015 at 04:26:57 AM, Thomas Chou wrote:
Hi Marek,
Hi Thomas,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
Yeah, that I do understand and I agree this is a good idea :)
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Urm, altera_qspi is CFI, right ? So can we stuff this into common code or not? Sorry if I am confused and off the mark.
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 11:42, Marek Vasut wrote:
On Thursday, December 24, 2015 at 04:26:57 AM, Thomas Chou wrote:
Hi Marek,
Hi Thomas,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
Yeah, that I do understand and I agree this is a good idea :)
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Urm, altera_qspi is CFI, right ? So can we stuff this into common code or not? Sorry if I am confused and off the mark.
No worries. I think the dotting is best done per driver. We can have common behavior. But there is little advantage to make it common code.
Merry Xmas.
Thomas Chou
On Thursday, December 24, 2015 at 06:24:45 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 11:42, Marek Vasut wrote:
On Thursday, December 24, 2015 at 04:26:57 AM, Thomas Chou wrote:
Hi Marek,
Hi Thomas,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote:
On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote: > Show sector erase progress with dot and comma. > > Signed-off-by: Thomas Chou thomas@wytron.com.tw > ---
Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
Yeah, that I do understand and I agree this is a good idea :)
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Urm, altera_qspi is CFI, right ? So can we stuff this into common code or not? Sorry if I am confused and off the mark.
No worries. I think the dotting is best done per driver.
Well why do you think so ?
We can have common behavior. But there is little advantage to make it common code.
Merry Xmas.
Merry Xmas to you too!
Best regards, Marek Vasut
Hi Marek,
On 2015年12月24日 13:37, Marek Vasut wrote:
On Thursday, December 24, 2015 at 06:24:45 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 11:42, Marek Vasut wrote:
On Thursday, December 24, 2015 at 04:26:57 AM, Thomas Chou wrote:
Hi Marek,
Hi Thomas,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 09:29, Marek Vasut wrote: > On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote: >> Show sector erase progress with dot and comma. >> >> Signed-off-by: Thomas Chou thomas@wytron.com.tw >> --- > > Shouldn't this go into common code ?
The code to print a dot is minimal. It will cost more if this go into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
Yeah, that I do understand and I agree this is a good idea :)
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Urm, altera_qspi is CFI, right ? So can we stuff this into common code or not? Sorry if I am confused and off the mark.
No worries. I think the dotting is best done per driver.
Well why do you think so ?
Well, it is only a putc() which is plain and simple.
Best regards, Thomas
On Friday, December 25, 2015 at 01:12:28 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 13:37, Marek Vasut wrote:
On Thursday, December 24, 2015 at 06:24:45 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月24日 11:42, Marek Vasut wrote:
On Thursday, December 24, 2015 at 04:26:57 AM, Thomas Chou wrote:
Hi Marek,
Hi Thomas,
On 2015年12月24日 11:02, Marek Vasut wrote:
On Thursday, December 24, 2015 at 03:50:57 AM, Thomas Chou wrote: > Hi Marek, > > On 2015年12月24日 09:29, Marek Vasut wrote: >> On Thursday, December 24, 2015 at 01:51:23 AM, Thomas Chou wrote: >>> Show sector erase progress with dot and comma. >>> >>> Signed-off-by: Thomas Chou thomas@wytron.com.tw >>> --- >> >> Shouldn't this go into common code ? > > The code to print a dot is minimal. It will cost more if this go > into common code.
But then this driver behaves in a non-standard manner AND noone benefits from this functionality but this driver.
The sector erase might take very long when there are a lot of sectors to erase, say 12 min to 50 min for 1024 sectors on the 10m50 board. Without the display of progress, it will look like the board hangs.
Yeah, that I do understand and I agree this is a good idea :)
The dotting code comes from the old cfi_flash.c. Some other parallel flash have this, but not spi-flash. It is trivial to add though.
Urm, altera_qspi is CFI, right ? So can we stuff this into common code or not? Sorry if I am confused and off the mark.
No worries. I think the dotting is best done per driver.
Well why do you think so ?
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Best regards, Marek Vasut
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
Best regards, Thomas
On Friday, December 25, 2015 at 09:33:52 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
No, I don't, I am not a CFI expert, sorry.
Best regards, Marek Vasut
Hi Marek,
On 2015年12月25日 17:58, Marek Vasut wrote:
On Friday, December 25, 2015 at 09:33:52 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
No, I don't, I am not a CFI expert, sorry.
Then I shall push these first and find way to put it to common code some time later. Thanks.
Best regards, Thomas
On Monday, December 28, 2015 at 02:17:46 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 17:58, Marek Vasut wrote:
On Friday, December 25, 2015 at 09:33:52 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
No, I don't, I am not a CFI expert, sorry.
Then I shall push these first and find way to put it to common code some time later. Thanks.
Is this any way to deal with feedback ? Why don't you put it into flash_erase() for example, in drivers/mtd/cfi_flash.c ?
Best regards, Marek Vasut
Hi Marek,
On 2015年12月28日 13:35, Marek Vasut wrote:
On Monday, December 28, 2015 at 02:17:46 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 17:58, Marek Vasut wrote:
On Friday, December 25, 2015 at 09:33:52 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
No, I don't, I am not a CFI expert, sorry.
Then I shall push these first and find way to put it to common code some time later. Thanks.
Is this any way to deal with feedback ? Why don't you put it into flash_erase() for example, in drivers/mtd/cfi_flash.c ?
Though altera-qspi works like CFI flash, it is not CFI flash as it does not have CFI info. It is a separate driver to cfi_flash.c.
Both drivers and many other parallel flash drivers under the board/arch directories are called by common/cmd_flash.c, which is very old and somewhat nasty. It could be the place for common feedback. But this is too far and deeply involved. I wanted to touch it a month ago, made some patches but withdrew then.
Best regards, Thomas
On Monday, December 28, 2015 at 09:04:55 AM, Thomas Chou wrote:
Hi Marek,
Hi!
On 2015年12月28日 13:35, Marek Vasut wrote:
On Monday, December 28, 2015 at 02:17:46 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 17:58, Marek Vasut wrote:
On Friday, December 25, 2015 at 09:33:52 AM, Thomas Chou wrote:
Hi Marek,
On 2015年12月25日 12:08, Marek Vasut wrote:
> Well, it is only a putc() which is plain and simple.
Sure, but then I still don't understand why this cannot be in the common code.
Yes, it can be in the common code. Do you have an idea how should it look like?
No, I don't, I am not a CFI expert, sorry.
Then I shall push these first and find way to put it to common code some time later. Thanks.
Is this any way to deal with feedback ? Why don't you put it into flash_erase() for example, in drivers/mtd/cfi_flash.c ?
Though altera-qspi works like CFI flash, it is not CFI flash as it does not have CFI info. It is a separate driver to cfi_flash.c.
Both drivers and many other parallel flash drivers under the board/arch directories are called by common/cmd_flash.c, which is very old and somewhat nasty. It could be the place for common feedback. But this is too far and deeply involved. I wanted to touch it a month ago, made some patches but withdrew then.
I see, thanks for the clarification. OK then.
It'd be nice if you managed to clean that part up eventually :)
Best regards, Marek Vasut
On 2015年12月24日 08:51, Thomas Chou wrote:
Show sector erase progress with dot and comma.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Applied to u-boot-nios.
Allow ctrl-c to abort the erase ops.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- drivers/mtd/altera_qspi.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c index 0624ff4..a9148a7 100644 --- a/drivers/mtd/altera_qspi.c +++ b/drivers/mtd/altera_qspi.c @@ -5,6 +5,7 @@ */
#include <common.h> +#include <console.h> #include <dm.h> #include <errno.h> #include <fdt_support.h> @@ -145,6 +146,14 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr) instr->state = MTD_ERASING; addr &= ~(mtd->erasesize - 1); /* get lower aligned address */ while (addr < end) { + if (ctrlc()) { + if (flash_verbose) + putc('\n'); + instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN; + instr->state = MTD_ERASE_FAILED; + mtd_erase_callback(instr); + return -EIO; + } flash = pdata->base + addr; last = pdata->base + addr + mtd->erasesize; /* skip erase if sector is blank */
On 2015年12月24日 08:51, Thomas Chou wrote:
Allow ctrl-c to abort the erase ops.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 9 +++++++++ 1 file changed, 9 insertions(+)
Applied to u-boot-nios.
On 2015年12月24日 08:51, Thomas Chou wrote:
Erase is an asynchronous operation. Device drivers are supposed to call instr->callback() whenever the operation completes, even if it completes with a failure.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
drivers/mtd/altera_qspi.c | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-nios.
participants (2)
-
Marek Vasut -
Thomas Chou