Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

On 2015年12月24日 08:51, Thomas Chou wrote:

Show sector erase progress with dot and comma.

Signed-off-by: Thomas Chou 
---
  drivers/mtd/altera_qspi.c | 14 ++
  1 file changed, 14 insertions(+)



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 4/5] altera_qspi: show erase progress

[Marek Vasut]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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 
>  ---
> >>> 
> >>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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


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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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


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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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

Shouldn't this go into common code ?

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 4/5] altera_qspi: show erase progress

[Marek Vasut]

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 
>  ---
> >>> 
> >>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

Show sector erase progress with dot and comma.

Signed-off-by: Thomas Chou 
---
 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, );
+   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;
}
-- 
2.5.0

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

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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 
> >> ---
> > 
> > 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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


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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Marek Vasut]

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 
> >> ---
> > 
> > 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/5] altera_qspi: show erase progress

[Thomas Chou]

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


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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot