From mboxrd@z Thu Jan 1 00:00:00 1970 Delivery-date: Mon, 05 Jun 2023 10:37:41 +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 1q65ih-00BYnV-3X for lore@lore.pengutronix.de; Mon, 05 Jun 2023 10:37:41 +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 1q65id-0006TA-Ge; Mon, 05 Jun 2023 10:37:39 +0200 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1q65ia-0006Sv-Pc; Mon, 05 Jun 2023 10:37:36 +0200 Received: from rhi by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1q65ia-0004ac-In; Mon, 05 Jun 2023 10:37:36 +0200 Date: Mon, 5 Jun 2023 10:37:36 +0200 From: Roland Hieber To: Ahmad Fatoum Message-ID: <20230605083736.zgz67shnl7eew6te@pengutronix.de> References: <20230531152253.1407395-1-a.fatoum@pengutronix.de> <20230531152253.1407395-6-a.fatoum@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230531152253.1407395-6-a.fatoum@pengutronix.de> User-Agent: NeoMutt/20180716 Subject: Re: [OSS-Tools] [PATCH 5/8] libdt: use block device partition instead of parent if found 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 On Wed, May 31, 2023 at 05:22:50PM +0200, Ahmad Fatoum wrote: > Linux doesn't parse MTD fixed-partitions described in the device tree > for block devices and EEPROMs. For block devices, The dt-utils work around > this by doing the lookup in userspace and then arrive at the parent block > device along with an offset that is passed along. We can do better > though: If there's a partition block device that contains the region > we are interested in, we should just be using that. > > This avoids the issue of triggering a reread of the partition table > after writing barebox state because udev is notified that we had > been opening the whole block device writable. > > Tested-by: Ulrich Ölmann > Signed-off-by: Ahmad Fatoum > --- > src/libdt.c | 60 +++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 45 insertions(+), 15 deletions(-) > > diff --git a/src/libdt.c b/src/libdt.c > index 440fcbd32fb4..e90ac5e26273 100644 > --- a/src/libdt.c > +++ b/src/libdt.c > @@ -2212,23 +2212,29 @@ static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *at > return 0; > } > > +/* True if A completely contains B */ > +static bool region_contains(loff_t starta, loff_t enda, > + loff_t startb, loff_t endb) > +{ > + return starta <= startb && enda >= endb; > +} > + > /* > - * device_find_block_device - extract device path from udev block device > + * cdev_from_block_device - Populate cdev from udev block device > * > * @dev: the udev_device to extract information from > - * @devpath: returns the devicepath under which the block device is accessible > + * @cdev: the cdev output parameter to populate > * > * returns 0 for success or negative error value on failure. > */ > -int device_find_block_device(struct udev_device *dev, > - char **devpath) > +static int cdev_from_block_device(struct udev_device *dev, > + struct cdev *cdev) > { > > struct udev *udev; > struct udev_enumerate *enumerate; > struct udev_list_entry *devices, *dev_list_entry; > - struct udev_device *part; > - const char *outpath; > + struct udev_device *part, *best_match = NULL; > int ret; > > udev = udev_new(); > @@ -2252,19 +2258,43 @@ int device_find_block_device(struct udev_device *dev, > if (!devtype) > continue; > if (!strcmp(devtype, "disk")) { > - outpath = udev_device_get_devnode(part); > - *devpath = strdup(outpath); > - ret = 0; > - goto out; > + best_match = part; > + > + /* Should we try to find a matching partition first? */ Nitpick: double space ;-) - Roland > + if (!cdev->size) > + break; > + } else if (cdev->size && !strcmp(devtype, "partition")) { > + u64 partstart, partsize; > + > + ret = udev_device_parse_sysattr_u64(part, "start", &partstart); > + if (ret) > + continue; > + > + ret = udev_device_parse_sysattr_u64(part, "size", &partsize); > + if (ret) > + continue; > + > + /* start/size sys attributes are always in 512-byte units */ > + partstart *= 512; > + partsize *= 512; > + > + if (!region_contains(partstart, partstart + partsize, > + cdev->offset, cdev->offset + cdev->size)) > + continue; > + > + best_match = part; > + cdev->offset -= partstart; > + break; > } > } > - ret = -ENODEV; > > -out: > + if (best_match) > + cdev->devpath = strdup(udev_device_get_devnode(best_match)); > + > udev_enumerate_unref(enumerate); > udev_unref(udev); > > - return ret; > + return best_match ? 0 : -ENODEV; > } > > /* > @@ -2604,10 +2634,10 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev) > > return of_parse_partition(partition_node, &cdev->offset, &cdev->size); > } else { > - ret = device_find_block_device(dev, &cdev->devpath); > + ret = of_parse_partition(partition_node, &cdev->offset, &cdev->size); > if (ret) > return ret; > - return of_parse_partition(partition_node, &cdev->offset, &cdev->size); > + return cdev_from_block_device(dev, cdev); > } > > return -EINVAL; > -- > 2.39.2 > > > -- Roland Hieber, Pengutronix e.K. | r.hieber@pengutronix.de | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |