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

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

In message fa2126d60911130006q3d5a1879pb177a51a4544fb6b@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@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