[U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command

[Adam Ford]

On Mon, Oct 8, 2018 at 12:46 PM Boris Brezillon
<boris.brezillon at bootlin.com> wrote:
>
> On Mon,  1 Oct 2018 15:43:29 +0200
> Miquel Raynal <miquel.raynal at bootlin.com> 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