Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Thomas Chou]

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 
---
  drivers/mtd/altera_qspi.c | 39 +--
  1 file changed, 25 insertions(+), 14 deletions(-)



Applied to u-boot-nios.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Marek Vasut]

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 
> ---
>  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, >mem_op);
> - stat = readl(>isr);
> - if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
> - /* erase failed, sector might be protected */
> - debug("erase %08x fail %x\n", sect, stat);
> - writel(stat, >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) != 0x)
> + 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, >mem_op);
> + stat = readl(>isr);
> + if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
> + /* erase failed, sector might be protected */
> + debug("erase %08x fail %x\n", sect, stat);
> + writel(stat, >isr); /* clear isr */
> + instr->fail_addr = addr;
> + instr->state = MTD_ERASE_FAILED;
> + mtd_erase_callback(instr);
> + return -EIO;
> + }
>   }
>   addr += mtd->erasesize;
>   }

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Marek Vasut]

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 
>  ---
>  
> 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, >mem_op);
>  -stat = readl(>isr);
>  -if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
>  -/* erase failed, sector might be protected */
>  -debug("erase %08x fail %x\n", sect, stat);
>  -writel(stat, >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) != 0x)
>  +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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Thomas Chou]

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 
---
  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, >mem_op);
-   stat = readl(>isr);
-   if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
-   /* erase failed, sector might be protected */
-   debug("erase %08x fail %x\n", sect, stat);
-   writel(stat, >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) != 0x)
+   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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Thomas Chou]

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 
---

   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, >mem_op);
-   stat = readl(>isr);
-   if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
-   /* erase failed, sector might be protected */
-   debug("erase %08x fail %x\n", sect, stat);
-   writel(stat, >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) != 0x)
+   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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/5] altera_qspi: skip erase if the sector is blank

[Marek Vasut]

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 
> >> ---
> >> 
> >>   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, >mem_op);
> >> -  stat = readl(>isr);
> >> -  if (stat & QUADSPI_ISR_ILLEGAL_ERASE) {
> >> -  /* erase failed, sector might be protected */
> >> -  debug("erase %08x fail %x\n", sect, stat);
> >> -  writel(stat, >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) != 0x)
> >> +  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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot