[U-Boot] [PATCH 052/080] cfi_flash: Reduce the scope of some variables

[Mario Six]

Hi Masahiro,

On Fri, Sep 29, 2017 at 7:10 PM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2017-09-29 21:52 GMT+09:00 Mario Six <mario.six at gdsys.cc>:
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>  drivers/mtd/cfi_flash.c | 26 ++++++++++++++------------
>>  1 file changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>> index c47ba416b9..c597c621ca 100644
>> --- a/drivers/mtd/cfi_flash.c
>> +++ b/drivers/mtd/cfi_flash.c
>> @@ -179,10 +179,10 @@ __maybe_weak u64 flash_read64(void *addr)
>>  static flash_info_t *flash_get_info(ulong base)
>>  {
>>         int i;
>> -       flash_info_t *info;
>>
>>         for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
>> -               info = &flash_info[i];
>> +               flash_info_t *info = &flash_info[i];
>> +
>>                 if (info->size && info->start[0] <= base &&
>>                     base <= info->start[0] + info->size - 1)
>>                         return info;
>> @@ -222,8 +222,6 @@ static inline void flash_unmap(flash_info_t *info, flash_sect_t sect,
>>  static void flash_make_cmd(flash_info_t *info, u32 cmd, void *cmdbuf)
>>  {
>>         int i;
>> -       int cword_offset;
>> -       int cp_offset;
>>  #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
>>         u32 cmd_le = cpu_to_le32(cmd);
>>  #endif
>> @@ -231,9 +229,10 @@ static void flash_make_cmd(flash_info_t *info, u32 cmd, void *cmdbuf)
>>         uchar *cp = (uchar *) cmdbuf;
>>
>>         for (i = info->portwidth; i > 0; i--) {
>> -               cword_offset = (info->portwidth - i) % info->chipwidth;
>> +               int cword_offset = (info->portwidth - i) % info->chipwidth;
>> +               int cp_offset = info->portwidth - i;
>>  #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
>> -               cp_offset = info->portwidth - i;
>> +
>>                 val = *((uchar *)&cmd_le + cword_offset);
>>  #else
>>                 cp_offset = i - 1;
>> @@ -2053,16 +2052,10 @@ static void flash_fixup_num(flash_info_t *info, struct cfi_qry *qry)
>>  ulong flash_get_size(phys_addr_t base, int banknum)
>>  {
>>         flash_info_t *info = &flash_info[banknum];
>> -       int i, j;
>>         flash_sect_t sect_cnt;
>>         phys_addr_t sector;
>> -       unsigned long tmp;
>> -       int size_ratio;
>>         uchar num_erase_regions;
>> -       int erase_region_size;
>> -       int erase_region_count;
>>         struct cfi_qry qry;
>> -       unsigned long max_size;
>>
>>         memset(&qry, 0, sizeof(qry));
>>
>> @@ -2075,6 +2068,11 @@ ulong flash_get_size(phys_addr_t base, int banknum)
>>         info->start[0] = (ulong)map_physmem(base, info->portwidth, MAP_NOCACHE);
>>
>>         if (flash_detect_cfi(info, &qry)) {
>> +               int i;
>> +               int size_ratio;
>> +               unsigned long max_size;
>> +               unsigned long tmp;
>> +
>>                 info->vendor = le16_to_cpu(get_unaligned(&qry.p_id));
>>                 info->ext_addr = le16_to_cpu(get_unaligned(&qry.p_adr));
>>                 num_erase_regions = qry.num_erase_regions;
>> @@ -2159,6 +2157,10 @@ ulong flash_get_size(phys_addr_t base, int banknum)
>>                 sect_cnt = 0;
>>                 sector = base;
>>                 for (i = 0; i < num_erase_regions; i++) {
>> +                       int j;
>> +                       int erase_region_size;
>> +                       int erase_region_count;
>> +
>>                         if (i > NUM_ERASE_REGIONS) {
>>                                 printf("%d erase regions found, only %d used\n",
>>                                         num_erase_regions, NUM_ERASE_REGIONS);
>> --
>
>
>
> Are these changes necessary?
>
> Looks like your personal preference to me.
>

Well, scope reduction of variables is a relatively common refactoring method
(see, for example, chapter 10.4 of Code Complete, Second Edition). Also, the
relatively commonly used CppCheck linter automatically finds these instances
very often, which is how I found them.

But no, it is not strictly necessary; I can drop these patches for v2.

Best regards,

Mario