From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adam Ford Date: Mon, 8 Oct 2018 13:26:14 -0500 Subject: [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command In-Reply-To: <20181008194627.71352d73@bbrezillon> References: <20181001134331.9756-1-miquel.raynal@bootlin.com> <20181001134331.9756-6-miquel.raynal@bootlin.com> <20181008194627.71352d73@bbrezillon> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, Oct 8, 2018 at 12:46 PM Boris Brezillon wrote: > > On Mon, 1 Oct 2018 15:43:29 +0200 > Miquel Raynal wrote: > > > +#if defined(CONFIG_MTD_PARTITIONS) > > +int mtd_probe_devices(void) > > +{ > > + static char *old_mtdparts; > > + static char *old_mtdids; > > + const char *mtdparts = env_get("mtdparts"); > > + const char *mtdids = env_get("mtdids"); > > + bool remaining_partitions = true; > > + struct mtd_info *mtd; > > + > > + mtd_probe_uclass_mtd_devs(); > > + > > + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */ > > + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids)) > > + return 0; > > Should be: > > if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) || > (mtdparts && old_mtdparts && mtdids && old_mtdids && > !strcmp(mtdparts, old_mtdparts) && > !strcmp(mtdids, old_mtdids))) > return 0; > Thanks for the suggestion. I created a patch to change this. I forgot to add the Suggested-by note. If whomever the maintainer wants to edit the patch, comments, etc. go ahead. Just know it now works on omap3_logic. > > + > > + /* Update the local copy of mtdparts */ > > + free(old_mtdparts); > > + free(old_mtdids); > > + old_mtdparts = strdup(mtdparts); > > + old_mtdids = strdup(mtdids); > > + > > + /* If at least one partition is still in use, do not delete anything */ > > + mtd_for_each_device(mtd) { > > + if (mtd->usecount) { > > + printf("Partition \"%s\" already in use, aborting\n", > > + mtd->name); > > + return -EACCES; > > + } > > + } > > + > > + /* > > + * Everything looks clear, remove all partitions. It is not safe to > > + * remove entries from the mtd_for_each_device loop as it uses idr > > + * indexes and the partitions removal is done in bulk (all partitions of > > + * one device at the same time), so break and iterate from start each > > + * time a new partition is found and deleted. > > + */ > > + while (remaining_partitions) { > > + remaining_partitions = false; > > + mtd_for_each_device(mtd) { > > + if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) { > > + del_mtd_partitions(mtd); > > + remaining_partitions = true; > > + break; > > + } > > + } > > + } > > + > > We should bail out if either mtdparts or mtdids is NULL: > > if (!mtdparts || !mtdids) > return 0; And add this in the patch as well. > > > + /* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */ > > + if (strstr(mtdparts, "mtdparts=")) > > + mtdparts += 9; > > + > > + /* For each MTD device in mtdparts */ > > + while (mtdparts[0] != '\0') { > > + char mtd_name[MTD_NAME_MAX_LEN], *colon; > > + struct mtd_partition *parts; > > + int mtd_name_len, nparts; > > + int ret; > > + > > + colon = strchr(mtdparts, ':'); > > + if (!colon) { > > + printf("Wrong mtdparts: %s\n", mtdparts); > > + return -EINVAL; > > + } > > + > > + mtd_name_len = colon - mtdparts; > > + strncpy(mtd_name, mtdparts, mtd_name_len); > > + mtd_name[mtd_name_len] = '\0'; > > + /* Move the pointer forward (including the ':') */ > > + mtdparts += mtd_name_len + 1; > > + mtd = get_mtd_device_nm(mtd_name); > > + if (IS_ERR_OR_NULL(mtd)) { > > + char linux_name[MTD_NAME_MAX_LEN]; > > + > > + /* > > + * The MTD device named "mtd_name" does not exist. Try > > + * to find a correspondance with an MTD device having > > + * the same type and number as defined in the mtdids. > > + */ > > + debug("No device named %s\n", mtd_name); > > + ret = mtd_search_alternate_name(mtd_name, linux_name, > > + MTD_NAME_MAX_LEN); > > + if (!ret) > > + mtd = get_mtd_device_nm(linux_name); > > + > > + /* > > + * If no device could be found, move the mtdparts > > + * pointer forward until the next set of partitions. > > + */ > > + if (ret || IS_ERR_OR_NULL(mtd)) { > > + printf("Could not find a valid device for %s\n", > > + mtd_name); > > + mtdparts = strchr(mtdparts, ';'); > > + if (mtdparts) > > + mtdparts++; > > + > > + continue; > > + } > > + } > > + > > + /* > > + * Parse the MTD device partitions. It will update the mtdparts > > + * pointer, create an array of parts (that must be freed), and > > + * return the number of partition structures in the array. > > + */ > > + ret = mtd_parse_partitions(mtd, &mtdparts, &parts, &nparts); > > + if (ret) { > > + printf("Could not parse device %s\n", mtd->name); > > + put_mtd_device(mtd); > > + return -EINVAL; > > + } > > + > > + if (!nparts) > > + continue; > > + > > + /* Create the new MTD partitions */ > > + add_mtd_partitions(mtd, parts, nparts); > > + > > + /* Free the structures allocated during the parsing */ > > + mtd_free_parsed_partitions(parts, nparts); > > + > > + put_mtd_device(mtd); > > + } > > + > > + return 0; > > +} > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot