[U-Boot] [PATCH] fix flash_sect_erase() to display correct message

[Wolfgang Denk]

Dear =?UTF-8?B?7J207Iq57ZiE?=,

In message <fa2126d60911130006q3d5a1879pb177a51a4544fb6b at mail.gmail.com> you wrote:
> flash_sect_erase() displays message "Erased #N sectors" even when
> there are some protected sectors found and command "erase" fail.
> 
> Signed-off-by: Seunghyeon Rhee <seunghyeon at lpmtec.com>
> ---
>  common/cmd_flash.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/common/cmd_flash.c b/common/cmd_flash.c
> index 3773412..b3d982f 100644
> --- a/common/cmd_flash.c
> +++ b/common/cmd_flash.c
> @@ -451,7 +451,10 @@ int flash_sect_erase (ulong addr_first, ulong addr_last)
>  				rcode = flash_erase (info, s_first[bank], s_last[bank]);
>  			}
>  		}
> -		printf ("Erased %d sectors\n", erased);
> +		if (rcode == ERR_PROTECTED)
> +			printf ("Not erased - protected sector(s) found\n");
> +		else
> +			printf ("Erased %d sectors\n", erased);
>  	} else if (rcode == 0) {
>  		puts ("Error: start and/or end address"
>  			" not on sector boundary\n");

I think this patch is not an improvement. Now it prints "Not erased"
even when sectors _have_ successfully been earased, which is at least
as wrong als the old behaviour.


Just to see what we are talking about:

Preparation:
============

=> fli 2

Bank # 2: CFI conformant FLASH (32 x 16)  Size: 4 MB in 35 Sectors
  AMD Standard command set, Manufacturer ID: 0x04, Device ID: 0x2249
  Erase timeout: 16384 ms, write timeout: 1 ms

  Sector Start Addresses:
  40400000        40408000        4040C000        40410000        40420000      
  40440000        40460000        40480000        404A0000        404C0000      
  404E0000        40500000        40520000        40540000        40560000      
  40580000        405A0000        405C0000        405E0000        40600000      
  40620000        40640000        40660000        40680000        406A0000      
  406C0000        406E0000        40700000        40720000        40740000      
  40760000        40780000        407A0000        407C0000        407E0000      

=> protect on 2:2-4
Protect Flash Sectors 2-4 in Bank # 2
=> fli 2           

Bank # 2: CFI conformant FLASH (32 x 16)  Size: 4 MB in 35 Sectors
  AMD Standard command set, Manufacturer ID: 0x04, Device ID: 0x2249
  Erase timeout: 16384 ms, write timeout: 1 ms

  Sector Start Addresses:
  40400000        40408000        4040C000   RO   40410000   RO   40420000   RO 
  40440000        40460000        40480000        404A0000        404C0000      
  404E0000        40500000        40520000        40540000        40560000      
  40580000        405A0000        405C0000        405E0000        40600000      
  40620000        40640000        40660000        40680000        406A0000      
  406C0000        406E0000        40700000        40720000        40740000      
  40760000        40780000        407A0000        407C0000        407E0000      

Case 1:
=======

=> erase 40400000 4047FFFF
- Warning: 3 protected sectors will not be erased!
.... done
Erased 7 sectors

Case 2:
=======

=> erase 40400000 +7FFFF
- Warning: 3 protected sectors will not be erased!
.... done
Erased 7 sectors

Case 3:
=======

=> erase 2:0-6
Erase Flash Sectors 0-6 in Bank # 2 - Warning: 3 protected sectors will not be erased!
.... done

Case 4:
=======

=> erase bank 2
Erase Flash Bank # 2 - Warning: 3 protected sectors will not be erased!
................................ done


As you can see, we _always_ print a warning message.

You can argument that it is incorrect to print "Erased 7 sectors"  in
cases  1  and  2,  as  actually  only 7 - 3 = 4 have been erased, but
printing "Not erased" would definitely be worse.

If you want, and if you can find a clean way to implement it, it
might make sense to change the output into something like "Erased 4
(instead of 7 requested) sectors" or the like.


NAK for the patch as is.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The manager will be continually amazed that policies he took for com-
mon knowledge are totally unknown by some member of his  team.  Since
his fundamental job is to keep everybody going in the same direction,
his chief daily task will be communication, not decision-making.
                              - Fred Brooks, "The Mythical Man Month"