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

[Ramon Fried]

On August 13, 2018 7:59:05 PM GMT+03:00, Tom Rini <trini at konsulko.com> wrote:
>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 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
>
>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!
Sure. Np

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