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 <tr...@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 <tr...@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.fr...@gmail.com> > >> > wrote: > >> > > > From: Ramon Fried <ramon.fr...@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? -- Tom
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot