19 Dec
2009
19 Dec
'09
7:44 a.m.
Dear Wolfgang,
Signed-off-by: Vipin vipin.kumar@st.com
...
+static ulong flash_get_size(ulong base, int banknum) +{
- flash_info_t *info = &flash_info[banknum];
- unsigned int value = 0;
- unsigned int density = 0;
remove useless initialization.
Ok I will send out a fresh patchset v2 with your review comments incorporated. Please let me know if it is ok
- int i;
- value = smi_read_id(info, banknum);
- density = (value >> 16) & 0xff;
- switch (density) {
- case 0x10:
- info->size = 64 * 1024;
- info->sector_count = 2;
- break;
- case 0x11:
- info->size = 128 * 1024;
- info->sector_count = 4;
- break;
- case 0x12:
- info->size = 256 * 1024;
- info->sector_count = 4;
- break;
- case 0x13:
- info->size = 512 * 1024;
- info->sector_count = 8;
- break;
- case 0x14:
- info->size = 1 * 1024 * 1024;
- info->sector_count = 16;
- break;
- case 0x15:
- info->size = 2 * 1024 * 1024;
- info->sector_count = 32;
- break;
- case 0x16:
- info->size = 4 * 1024 * 1024;
- info->sector_count = 64;
- break;
- case 0x17:
- info->size = 8 * 1024 * 1024;
- info->sector_count = 128;
- break;
- case 0x18:
- info->size = 16 * 1024 * 1024;
- info->sector_count = 64;
- break;
- default:
- return 0x0;
- }
Consider using lookup tables?
Currently supported flashes have consequent values of density. It may have random values supported in future. That's the reason I feel it's better to keep the code this way
- /* Assume that all sectors are unprotected by default */
- for (i = 0; i < CONFIG_SYS_MAX_FLASH_SECT; i++)
- info->protect[i] = 0;
Um... is this assumption correct?
It is intentional
+static int smi_wait_till_ready(int bank, int timeout) +{
- int count;
- int sr;
- /* One chip guarantees max 5 msec wait here after page writes,
- but potentially three seconds (!) after page erase. */
- for (count = 0; count < timeout; count++) {
- sr = smi_read_sr(bank);
- if (sr < 0)
- break;
- else if (!(sr & WIP_BIT))
- return 0;
Use braces here.
Ok. braces added
Best Regards Vipin