[U-Boot] [PATCH 2/9] smi driver support for SPEAr SoCs

[Wolfgang Denk]

Dear Vipin Kumar,

In message <83d1d72b0912182244yb303c6ai3b61d7b896020c00 at mail.gmail.com> you wrote:
> 
> >> +     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

That's why I did not recommend to calculate this witha simple
formula, (which would be trivially possible for "size", but not so
for "sector_count"), but using a lookup table - this is independent
of the actual values.

> >> +     /* 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

Why don;t you protect sectors where the U-Bootimage and environment
are stored?


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
If you fail to plan, plan to fail.