From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Subject: Re: [kvm-unit-tests PATCH v3 04/17] devicetree: fix dt_get_memory_params Date: Mon, 15 Feb 2016 08:22:07 +0100 Message-ID: <56C17C9F.2040106@redhat.com> References: <1455380910-30604-1-git-send-email-drjones@redhat.com> <1455380910-30604-5-git-send-email-drjones@redhat.com> <20160214230033.GI2732@voom.fritz.box> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="gAJROFQMnion9DW4GrXH9QtrfdjLASkMi" Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, dgibson@redhat.com, agraf@suse.de, lvivier@redhat.com, pbonzini@redhat.com To: David Gibson , Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56249 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751998AbcBOHWS (ORCPT ); Mon, 15 Feb 2016 02:22:18 -0500 In-Reply-To: <20160214230033.GI2732@voom.fritz.box> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gAJROFQMnion9DW4GrXH9QtrfdjLASkMi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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 >=20 > 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 *comp= atible, >> int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs) >> { >> const char *pn =3D "device_type", *pv =3D "memory"; >> - int node, ret, pl =3D strlen(pv) + 1, nr =3D 0; >> + int node, ret, reg_idx, pl =3D strlen(pv) + 1, nr =3D 0; >> struct dt_pbus_reg reg; >> =20 >> node =3D fdt_node_offset_by_prop_value(fdt, -1, pn, pv, pl); >> =20 >> while (node >=3D 0) { >> =20 >> + reg_idx =3D 0; =2E.. here: reg_idx is reset to zero, but nr is not. >> while (nr < nr_regs) { >> - ret =3D dt_pbus_translate_node(node, nr, ®); >> + ret =3D dt_pbus_translate_node(node, reg_idx, ®); >> if (ret =3D=3D -FDT_ERR_NOTFOUND) >> break; >> if (ret < 0) >> return ret; >> regs[nr].addr =3D reg.addr; >> regs[nr].size =3D reg.size; >> - ++nr; >> + ++nr, ++reg_idx; >> } >> =20 >> node =3D fdt_node_offset_by_prop_value(fdt, node, pn, pv, pl); Sounds reasonable to me. Reviewed-by: Thomas Huth --gAJROFQMnion9DW4GrXH9QtrfdjLASkMi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJWwXylAAoJEC7Z13T+cC211tsP/iQR01RB/6vH86IG2rq7wL67 AB7KvJG2yCapPYjM+yjPFRDsSccdPk5e21Jft3AXXwvWoOnDOqmmp7nHlndvRcra FLC2yC28kHPTehKjuCZ7gW8hDrapo2KY7ZSn+hpzCyxQ+U3xs+qSpl7yU1YTPC+Y ZOmcoB1NGK3v1phFi3dj+ofdTpb0u0XaQ5HU3C8kZIj+WItx17i7Y/ALZPfizydn RxPKI38hQo+6WTfAcbiNqLKk2B/OoJLuR7n28Up5TR7pjSD4z+vuZ1n8YRBJVe8R 9uK20kwtofQs5CVCwbdgDylSo3Hn3aedEfXVOtBWcmRU4jGF0/ffFvLd2fllq3oN FBKqZyed0ty/WsCjkzzsH4VkaddHaD1T/ZEZJZ3IqGr9R/4GGNIyaH8rEks1E5RM 1z/8QJLccPFwxQQyCETWR5aj1stSnCU01B3JOhN6Wdc4Mn4FpP+o6sD2KTRp2cot 6artfLsjyNp3OL7RHUszTteTqpL4iFQA0pkhilzfMk9HcaHoYE7aXhd4qMikhV6L ogemHQws1ZJAWCeNJ/Xnnm40thXRnoxxIjI0RILDBcJUs3W4v8NKf8r+Evv6iTte 8WGJGgUn9sBhLxYj1aRxbf5xpXcaWz9YOBoI7378l8PID//3nIN4przU+sMB0oLd TQGlelQH/Bmzj+SYHawg =IeGj -----END PGP SIGNATURE----- --gAJROFQMnion9DW4GrXH9QtrfdjLASkMi--