From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Wed, 07 Jun 2023 10:55:13 +0200 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by lore.white.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1q6owk-00E6i9-Gq for lore@lore.pengutronix.de; Wed, 07 Jun 2023 10:55:13 +0200 Received: from localhost ([127.0.0.1] helo=metis.ext.pengutronix.de) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1q6owh-0007p7-3T; Wed, 07 Jun 2023 10:55:11 +0200 Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1q6owf-0007nF-DE; Wed, 07 Jun 2023 10:55:09 +0200 Received: from [2a0a:edc0:0:900:1d::77] (helo=ptz.office.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from ) id 1q6owe-005hWu-Oz; Wed, 07 Jun 2023 10:55:08 +0200 Received: from ukl by ptz.office.stw.pengutronix.de with local (Exim 4.94.2) (envelope-from ) id 1q6owe-00BxXN-4o; Wed, 07 Jun 2023 10:55:08 +0200 Date: Wed, 7 Jun 2023 10:55:08 +0200 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Ahmad Fatoum Message-ID: <20230607085508.xl6nsx2ajidfnghp@pengutronix.de> References: <20230607080818.715192-1-a.fatoum@pengutronix.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="77y4lhuomljjtjix" Content-Disposition: inline In-Reply-To: <20230607080818.715192-1-a.fatoum@pengutronix.de> Subject: Re: [OSS-Tools] [PATCH] libdt: fix of_get_devicepath looking up sibling if device unavailable X-BeenThere: oss-tools@pengutronix.de X-Mailman-Version: 2.1.29 Precedence: list List-Id: Pengutronix Public Open-Source-Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: oss-tools@pengutronix.de Sender: "OSS-Tools" X-SA-Exim-Connect-IP: 127.0.0.1 X-SA-Exim-Mail-From: oss-tools-bounces@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false --77y4lhuomljjtjix Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Ahmad, On Wed, Jun 07, 2023 at 10:08:18AM +0200, Ahmad Fatoum wrote: > of_get_devicepath code flow is split into two: >=20 > A) Either the device tree node in question has a direct udev_device > associated with it >=20 > B) Or we assume it's a partition and lookup udev_device for the parent > first, before finding a child udev_device or setting a partition > offset within the parent udev_device. >=20 > Since v2017.03.0, we have had a fallthrough from case A into case B: > If we have a udev_device, but it's neither a EEPROMs, MTDs or block > device, we just consider it a partition. This is problematic, because > this may result in us pointing at a very different device: >=20 > - backend points at a SD-Card host. Host is enabled, but SD-Card > is not inserted, so no block device >=20 > - case A fails, so it's assumed it's a partition and case B > uses parent SoC bus to lookup appropriate device >=20 > - We fall through into the second device_find_block_device, which > will take the first matching block device across the SoC. So > we could end up with the eMMC: a completely different device > than what was pointed at. So another surprise is that device_find_block_device() recurses to find a device when starting on /soc, isn't it? Is this worth addressing? > Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robu= st") > Signed-off-by: Ahmad Fatoum > --- > src/libdt.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) >=20 > diff --git a/src/libdt.c b/src/libdt.c > index e54d7fb5649d..7b99efe5b2de 100644 > --- a/src/libdt.c > +++ b/src/libdt.c > @@ -2492,9 +2492,11 @@ int of_get_devicepath(struct device_node *partitio= n_node, char **devpath, off_t > } > =20 > /* > - * If we found a device but couldn't classify it above, we fall > - * through. > + * If we find a udev_device but couldn't classify it above, > + * it's an error. Falling through would mean to handle it as a > + * partition and could lead us to return an arbitrary sibling device > */ > + return -ENODEV; I don't remember the details of 929ed64cb42f any more, but probably I didn't have a specific case that were fixed by that commit. Your rationale makes sense. Reviewed-by: Uwe Kleine-K=F6nig Best regards Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | https://www.pengutronix.de/ | --77y4lhuomljjtjix Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmSAResACgkQj4D7WH0S /k4htwgAuS1z34UxfZCNZEegxRcEKft2O74LeEDf636RIhJLB0UttZzsXdXexFkF 57DF1SMFNH/Q4yuqU15FspKZIOtGVg3tBro6otOTEHpuFF1Dcqh7BK+50ethC48X n0vvVwOEs3Wm17XjvftIFjoe14jJZgjIS6Wz+xrKYaEj+u2o+j9zXy//oDp20xeE cwszhWQsjgrXaXFarDaeG6Qz0NoGs4U6JmszuFQeypHCuln/cLUjGjooKlUyusLH DGRNt2WC3/z7rtyyorjHpm0InfWk7dK8W5m8A7eGiQU798IFBKXRKqWTY54tbFq2 HaIbmj8hN2zq9xbv3ZPeYaQfuY+p6w== =y5SY -----END PGP SIGNATURE----- --77y4lhuomljjtjix--