| Message ID | 20200912104252.80602-1-marek.vasut+renesas@gmail.com |
|---|---|
| State | Accepted |
| Commit | 81d0cef3b268ccc4f1061a3e29850fbd23166d20 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | lib: fdt: Fix fdtdec_setup_mem..() conversion to livetree API | expand |
Hi Marek, Thanks for the patch. > -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Marek Vasut > Sent: 12 September 2020 11:43 > To: u-boot@lists.denx.de > Cc: Marek Vasut <marek.vasut+renesas@gmail.com>; Michal Simek > <michal.simek@xilinx.com>; Simon Glass <sjg@chromium.org>; Tom Rini > <trini@konsulko.com> > Subject: [PATCH] lib: fdt: Fix fdtdec_setup_mem..() conversion to livetree > API > > Repair incorrectly negated condition in the original patch which broke DT > memory node parsing on everything which has more than one DT memory > node, e.g. R-Car3. > > In case multiple valid memory nodes are present in the DT, the original patch > would complete parsing cycle for the first memory node, then move on to > the next one, identify it as a valid, and end the parsing. The fix is to invert the > condition, to make the code behave as it did before the livetree conversion, > so it would continue parsing the subsequent memory nodes as well. > > Fixes: c2f0950c33 ("lib: fdt: Convert fdtdes_setup_mem..() to livetree API") > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@konsulko.com> Tested this patch on following RZ/G2 Boards and it reports correct DRAM size with 2020.10 uboot master branch. 1) RZ/G2H (a.k.a r8a774e1) 2) RZ/G2M (a.k.a r8a774a1) 3) RZ/G2N (a.k.a r8a774b1) Tested-by: Biju Das <biju.das.jz@bp.renesas.com> Cheers, Biju > --- > lib/fdtdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c index d3b22ec323..5f41f58a63 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -1101,7 +1101,7 @@ int fdtdec_setup_memory_banksize(void) > if (ret < 0) { > reg = 0; > mem = get_next_memory_node(mem); > -if (ofnode_valid(mem)) > +if (!ofnode_valid(mem)) > break; > > ret = ofnode_read_resource(mem, reg++, &res); > @@ -1146,7 +1146,7 @@ int fdtdec_setup_mem_size_base_lowest(void) > if (ret < 0) { > reg = 0; > mem = get_next_memory_node(mem); > -if (ofnode_valid(mem)) > +if (!ofnode_valid(mem)) > break; > > ret = ofnode_read_resource(mem, reg++, &res); > -- > 2.28.0 Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
On 12. 09. 20 12:42, Marek Vasut wrote: > Repair incorrectly negated condition in the original patch which broke > DT memory node parsing on everything which has more than one DT memory > node, e.g. R-Car3. > > In case multiple valid memory nodes are present in the DT, the original > patch would complete parsing cycle for the first memory node, then move > on to the next one, identify it as a valid, and end the parsing. The fix > is to invert the condition, to make the code behave as it did before the > livetree conversion, so it would continue parsing the subsequent memory > nodes as well. > > Fixes: c2f0950c33 ("lib: fdt: Convert fdtdes_setup_mem..() to livetree API") > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@konsulko.com> > --- > lib/fdtdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index d3b22ec323..5f41f58a63 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -1101,7 +1101,7 @@ int fdtdec_setup_memory_banksize(void) > if (ret < 0) { > reg = 0; > mem = get_next_memory_node(mem); > - if (ofnode_valid(mem)) > + if (!ofnode_valid(mem)) > break; > > ret = ofnode_read_resource(mem, reg++, &res); > @@ -1146,7 +1146,7 @@ int fdtdec_setup_mem_size_base_lowest(void) > if (ret < 0) { > reg = 0; > mem = get_next_memory_node(mem); > - if (ofnode_valid(mem)) > + if (!ofnode_valid(mem)) > break; > > ret = ofnode_read_resource(mem, reg++, &res); > We are not using multiple memory node. We use multiple tuples inside one memory node that's why it didn't come up. Good catch. Reviewed-by: Michal Simek <michal.simek@xilinx.com> Tom: Can you please take it to your tree? Thanks, Michal
On Tue, Sep 15, 2020 at 03:58:43PM +0200, Michal Simek wrote: > > > On 12. 09. 20 12:42, Marek Vasut wrote: > > Repair incorrectly negated condition in the original patch which broke > > DT memory node parsing on everything which has more than one DT memory > > node, e.g. R-Car3. > > > > In case multiple valid memory nodes are present in the DT, the original > > patch would complete parsing cycle for the first memory node, then move > > on to the next one, identify it as a valid, and end the parsing. The fix > > is to invert the condition, to make the code behave as it did before the > > livetree conversion, so it would continue parsing the subsequent memory > > nodes as well. > > > > Fixes: c2f0950c33 ("lib: fdt: Convert fdtdes_setup_mem..() to livetree API") > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > > Cc: Michal Simek <michal.simek@xilinx.com> > > Cc: Simon Glass <sjg@chromium.org> > > Cc: Tom Rini <trini@konsulko.com> > > --- > > lib/fdtdec.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > index d3b22ec323..5f41f58a63 100644 > > --- a/lib/fdtdec.c > > +++ b/lib/fdtdec.c > > @@ -1101,7 +1101,7 @@ int fdtdec_setup_memory_banksize(void) > > if (ret < 0) { > > reg = 0; > > mem = get_next_memory_node(mem); > > - if (ofnode_valid(mem)) > > + if (!ofnode_valid(mem)) > > break; > > > > ret = ofnode_read_resource(mem, reg++, &res); > > @@ -1146,7 +1146,7 @@ int fdtdec_setup_mem_size_base_lowest(void) > > if (ret < 0) { > > reg = 0; > > mem = get_next_memory_node(mem); > > - if (ofnode_valid(mem)) > > + if (!ofnode_valid(mem)) > > break; > > > > ret = ofnode_read_resource(mem, reg++, &res); > > > > We are not using multiple memory node. We use multiple tuples inside one > memory node that's why it didn't come up. Good catch. > > Reviewed-by: Michal Simek <michal.simek@xilinx.com> > > Tom: Can you please take it to your tree? Will do, thanks!
On Sat, Sep 12, 2020 at 12:42:52PM +0200, Marek Vasut wrote: > Repair incorrectly negated condition in the original patch which broke > DT memory node parsing on everything which has more than one DT memory > node, e.g. R-Car3. > > In case multiple valid memory nodes are present in the DT, the original > patch would complete parsing cycle for the first memory node, then move > on to the next one, identify it as a valid, and end the parsing. The fix > is to invert the condition, to make the code behave as it did before the > livetree conversion, so it would continue parsing the subsequent memory > nodes as well. > > Fixes: c2f0950c33 ("lib: fdt: Convert fdtdes_setup_mem..() to livetree API") > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Michal Simek <michal.simek@xilinx.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@konsulko.com> > Tested-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Michal Simek <michal.simek@xilinx.com> Applied to u-boot/master, thanks!
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index d3b22ec323..5f41f58a63 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1101,7 +1101,7 @@ int fdtdec_setup_memory_banksize(void) if (ret < 0) { reg = 0; mem = get_next_memory_node(mem); - if (ofnode_valid(mem)) + if (!ofnode_valid(mem)) break; ret = ofnode_read_resource(mem, reg++, &res); @@ -1146,7 +1146,7 @@ int fdtdec_setup_mem_size_base_lowest(void) if (ret < 0) { reg = 0; mem = get_next_memory_node(mem); - if (ofnode_valid(mem)) + if (!ofnode_valid(mem)) break; ret = ofnode_read_resource(mem, reg++, &res);
Repair incorrectly negated condition in the original patch which broke DT memory node parsing on everything which has more than one DT memory node, e.g. R-Car3. In case multiple valid memory nodes are present in the DT, the original patch would complete parsing cycle for the first memory node, then move on to the next one, identify it as a valid, and end the parsing. The fix is to invert the condition, to make the code behave as it did before the livetree conversion, so it would continue parsing the subsequent memory nodes as well. Fixes: c2f0950c33 ("lib: fdt: Convert fdtdes_setup_mem..() to livetree API") Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Cc: Michal Simek <michal.simek@xilinx.com> Cc: Simon Glass <sjg@chromium.org> Cc: Tom Rini <trini@konsulko.com> --- lib/fdtdec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)