Message ID | 1455380910-30604-5-git-send-email-drjones@redhat.com |
---|---|
State | Superseded |
Headers | show |
On Sat, Feb 13, 2016 at 05:28:17PM +0100, Andrew Jones wrote: > Index nr was overreaching with its indexing. It should only > index the caller's regs array. We need a different index for > the memory nodes' reg properties. > > Signed-off-by: Andrew Jones <drjones@redhat.com> I think I'm missing something, I don't see how reg_idx will ever take a different value from nr. > > --- > The next patch needs this fix. > --- > lib/devicetree.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/lib/devicetree.c b/lib/devicetree.c > index 36cb28610ff41..a5c7f7c69ddfd 100644 > --- a/lib/devicetree.c > +++ b/lib/devicetree.c > @@ -165,22 +165,24 @@ int dt_pbus_get_base_compatible(const char *compatible, > int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs) > { > const char *pn = "device_type", *pv = "memory"; > - int node, ret, pl = strlen(pv) + 1, nr = 0; > + int node, ret, reg_idx, pl = strlen(pv) + 1, nr = 0; > struct dt_pbus_reg reg; > > node = fdt_node_offset_by_prop_value(fdt, -1, pn, pv, pl); > > while (node >= 0) { > > + reg_idx = 0; > + > while (nr < nr_regs) { > - ret = dt_pbus_translate_node(node, nr, ®); > + ret = dt_pbus_translate_node(node, reg_idx, ®); > if (ret == -FDT_ERR_NOTFOUND) > break; > if (ret < 0) > return ret; > regs[nr].addr = reg.addr; > regs[nr].size = reg.size; > - ++nr; > + ++nr, ++reg_idx; > } > > node = fdt_node_offset_by_prop_value(fdt, node, pn, pv, pl);
On 15.02.2016 00:00, David Gibson wrote: > On Sat, Feb 13, 2016 at 05:28:17PM +0100, Andrew Jones wrote: >> Index nr was overreaching with its indexing. It should only >> index the caller's regs array. We need a different index for >> the memory nodes' reg properties. >> >> Signed-off-by: Andrew Jones <drjones@redhat.com> > > I think I'm missing something, I don't see how reg_idx will ever take > a different value from nr. There is one difference ... >> --- >> The next patch needs this fix. >> --- >> lib/devicetree.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/lib/devicetree.c b/lib/devicetree.c >> index 36cb28610ff41..a5c7f7c69ddfd 100644 >> --- a/lib/devicetree.c >> +++ b/lib/devicetree.c >> @@ -165,22 +165,24 @@ int dt_pbus_get_base_compatible(const char *compatible, >> int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs) >> { >> const char *pn = "device_type", *pv = "memory"; >> - int node, ret, pl = strlen(pv) + 1, nr = 0; >> + int node, ret, reg_idx, pl = strlen(pv) + 1, nr = 0; >> struct dt_pbus_reg reg; >> >> node = fdt_node_offset_by_prop_value(fdt, -1, pn, pv, pl); >> >> while (node >= 0) { >> >> + reg_idx = 0; ... here: reg_idx is reset to zero, but nr is not. >> while (nr < nr_regs) { >> - ret = dt_pbus_translate_node(node, nr, ®); >> + ret = dt_pbus_translate_node(node, reg_idx, ®); >> if (ret == -FDT_ERR_NOTFOUND) >> break; >> if (ret < 0) >> return ret; >> regs[nr].addr = reg.addr; >> regs[nr].size = reg.size; >> - ++nr; >> + ++nr, ++reg_idx; >> } >> >> node = fdt_node_offset_by_prop_value(fdt, node, pn, pv, pl); Sounds reasonable to me. Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/lib/devicetree.c b/lib/devicetree.c index 36cb28610ff41..a5c7f7c69ddfd 100644 --- a/lib/devicetree.c +++ b/lib/devicetree.c @@ -165,22 +165,24 @@ int dt_pbus_get_base_compatible(const char *compatible, int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs) { const char *pn = "device_type", *pv = "memory"; - int node, ret, pl = strlen(pv) + 1, nr = 0; + int node, ret, reg_idx, pl = strlen(pv) + 1, nr = 0; struct dt_pbus_reg reg; node = fdt_node_offset_by_prop_value(fdt, -1, pn, pv, pl); while (node >= 0) { + reg_idx = 0; + while (nr < nr_regs) { - ret = dt_pbus_translate_node(node, nr, ®); + ret = dt_pbus_translate_node(node, reg_idx, ®); if (ret == -FDT_ERR_NOTFOUND) break; if (ret < 0) return ret; regs[nr].addr = reg.addr; regs[nr].size = reg.size; - ++nr; + ++nr, ++reg_idx; } node = fdt_node_offset_by_prop_value(fdt, node, pn, pv, pl);
Index nr was overreaching with its indexing. It should only index the caller's regs array. We need a different index for the memory nodes' reg properties. Signed-off-by: Andrew Jones <drjones@redhat.com> --- The next patch needs this fix. --- lib/devicetree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)