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

[Tom Rini <trini@konsulko.com>]

On Mon, Aug 13, 2018 at 10:55:03PM +0300, Ramon Fried wrote:
> On Mon, Aug 13, 2018 at 7:22 PM Ramon Fried <ramon.fried@gmail.com> wrote:
> 
> > On August 13, 2018 7:15:14 PM GMT+03:00, Tom Rini <trini@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@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@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@gmail.com>
> > >> >> > wrote:
> > >> >> > > > From: Ramon Fried <ramon.fried@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

If you've got some time, can you add CONFIG_NR_DRAM_BANKS to Kconfig and
set the default to 4 ?  I'll take care of re-running moveconfig.py if
there's conflicts.  This could probably go in the top-level Kconfig file
near the malloc options.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180813/0400d4d1/attachment.sig>