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

Dear Vipin KUMAR,

In message 1260955110-5656-3-git-send-email-vipin.kumar@st.com you wrote:

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.

• 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?

• /* 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?

+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.

Best regards,

Wolfgang Denk