From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fdt_support: Use VLA instead of MEMORY_BANKS_MAX
Date: Mon, 13 Aug 2018 12:15:14 -0400 [thread overview]
Message-ID: <20180813161514.GC29229@bill-the-cat> (raw)
In-Reply-To: <89946243-B5E3-450F-9D8E-942B4165D83D@gmail.com>
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?
--
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/3f121c67/attachment.sig>
next prev parent reply other threads:[~2018-08-13 16:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-12 20:37 [U-Boot] [PATCH] fdt_support: Use VLA instead of MEMORY_BANKS_MAX Ramon Fried
2018-08-13 7:20 ` Peter Robinson
2018-08-13 14:52 ` Tom Rini
2018-08-13 18:54 ` Ramon Fried
2018-08-13 16:08 ` Tom Rini
2018-08-13 16:14 ` Ramon Fried
2018-08-13 16:15 ` Tom Rini [this message]
2018-08-13 16:22 ` Ramon Fried
2018-08-13 19:55 ` Ramon Fried
2018-08-13 16:59 ` Tom Rini
2018-08-13 17:19 ` Ramon Fried
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180813161514.GC29229@bill-the-cat \
--to=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox