[U-Boot] [PATCH v8 4/9] x86: slimbootloader: Add memory configuration

[Park, Aiden]

Hi Andy,

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko at gmail.com]
> Sent: Friday, August 2, 2019 4:51 PM
> To: Park, Aiden <aiden.park at intel.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Simon Glass <sjg at chromium.org>
> Subject: Re: [PATCH v8 4/9] x86: slimbootloader: Add memory
> configuration
> 
> On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko
> <andy.shevchenko at gmail.com> wrote:
> >
> > On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden <aiden.park at intel.com>
> wrote:
> > >
> > > Slim Bootloader provides memory map info thru its HOB list pointer.
> > > Configure memory size and relocation memory from the HOB data, and
> > > provide e820 entries as well.
> > > - Get memory size from the memory map info HOB
> > > - Set available top memory lower than 4GB for U-Boot relocation
> > > - Provide e820 entries from the memory map info HOB
> >
> > > +#define for_each_memory_map_entry_reversed(iter, entries) \
> > > +       for (iter = entries->count - 1; iter >= 0; iter--) \
> > > +               if (entries->entry[iter].type == E820_RAM)
> > > +
> >
> > It seems you missed my answer to Bin.
> > This is simple incorrect. Checkpatch sometimes is wrong.
> 
> Let me elaborate why.
> 
> The idea of having
> 
>  if (foo) {} else
> 
> pattern is to avoid weirndess like
> 
>   for_each_...() {
>     ...
>   } else {
>     ...WTF!..
>   }
> 
Sorry for missing your comment. I understand '{} else' is better way.
It looks checkpatch issue is bigger scope than this one.
What about this in this series?

#define for_each_if(condition) if (!(condition)) {} else
#define for_each_memory_map_entry_reversed(iter, entries) \
        for (iter = entries->count - 1; iter >= 0; iter--) \
               for_each_if(entries->entry[iter].type == E820_RAM)

tricky, but checkpatch does not report ERROR. 

> 
> --
> With Best Regards,
> Andy Shevchenko

Best Regards,
Aiden