[U-Boot] [PATCH] fdt_support: Use VLA instead of MEMORY_BANKS_MAX

[Ramon Fried]

On Mon, Aug 13, 2018 at 7:22 PM Ramon Fried <ramon.fried at gmail.com> wrote:

> On August 13, 2018 7:15:14 PM GMT+03:00, Tom Rini <trini at konsulko.com>
> wrote:
> >On Mon, Aug 13, 2018 at 07:14:00PM +0300, Ramon Fried wrote:
> >> On August 13, 2018 7:08:22 PM GMT+03:00, Tom Rini
> ><trini at konsulko.com> wrote:
> >> >On Mon, Aug 13, 2018 at 09:54:30PM +0300, Ramon Fried wrote:
> >> >> On Mon, Aug 13, 2018 at 5:52 PM Tom Rini <trini at konsulko.com>
> >wrote:
> >> >>
> >> >> > On Mon, Aug 13, 2018 at 08:20:03AM +0100, Peter Robinson wrote:
> >> >> >
> >> >> > > On Sun, Aug 12, 2018 at 9:37 PM, Ramon Fried
> >> ><ramon.fried at gmail.com>
> >> >> > wrote:
> >> >> > > > From: Ramon Fried <ramon.fried at intel.com>
> >> >> > > >
> >> >> > > > Instead of relaying on user to configure MEMORY_BANKS_MAX
> >> >> > > > correctly, use VLA (variable length array) to accommodate
> >the
> >> >> > > > required banks.
> >> >> > >
> >> >> > > With the kernel actively removing VLAs [1] does it make sense
> >for
> >> >us
> >> >> > > to use them?
> >> >> >
> >> >> > Agreed.
> >> >> >
> >> >> > Also, why is the answer NOT to go back to the way things were
> >with
> >> >> > 5e5745465c94 and increase CONFIG_NR_DRAM_BANKS when needed?  It
> >> >seems
> >> >> >
> >> >> The whole purpose of my patch was to enable to fixup more banks
> >than
> >> >> defined in
> >> >> CONFIG_NR_DRAM_BANKS.
> >> >>
> >> >> Another option would be to add
> >> >> +#ifndef MEMORY_BANKS_MAX
> >> >> #define MEMORY_BANKS_MAX 4
> >> >> +#endif
> >> >> and let the use alter the value in include/configs if necessary.
> >> >
> >> >I think for our purposes it's best to say that, as the code was
> >> >written,
> >> >if we need more banks to be configured at build time, they should
> >be.
> >> >This may also mean that certain platforms need to bump their default
> >up
> >> >in order to support the hardware you're using that shows this issue.
> >> >Thanks!
> >> I'm confused. To which hardware you're referring to? Do you still
> >> think we should revert my patch?
> >
> >Yes, I think we should bring the code back to the way it was for a long
> >while.  And I assume there was a specific piece of hardware that
> >triggered this round of changes?
> Yes. Dragonboards.
> I can implement this fixup function in the snapdragon arch folder.
>
> Tom, a last effort to reduce code duplication. is this acceptable ?
  #if CONFIG_NR_DRAM_BANKS > 4
  #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
  #else
  #define MEMORY_BANKS_MAX 4
  #endif

-- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>