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

Dear Vipin Kumar,

In message 83d1d72b0912182244yb303c6ai3b61d7b896020c00@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