From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Mon, 13 Aug 2018 12:59:05 -0400 Subject: [U-Boot] [PATCH] fdt_support: Use VLA instead of MEMORY_BANKS_MAX In-Reply-To: References: <20180812203749.24241-1-ramon.fried@gmail.com> <20180813145205.GZ29229@bill-the-cat> <20180813160822.GB29229@bill-the-cat> <89946243-B5E3-450F-9D8E-942B4165D83D@gmail.com> <20180813161514.GC29229@bill-the-cat> Message-ID: <20180813165905.GG29229@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, Aug 13, 2018 at 10:55:03PM +0300, Ramon Fried wrote: > On Mon, Aug 13, 2018 at 7:22 PM Ramon Fried wrote: > > > On August 13, 2018 7:15:14 PM GMT+03:00, Tom Rini > > 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 > > > 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 > > >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 > > >> > > > >> >> > wrote: > > >> >> > > > From: Ramon Fried > > >> >> > > > > > >> >> > > > 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: