Re: [PATCH v7 1/2] mtd: mtdcore: add debugfs nodes for querying the flash name and id

[Zhuohao Lee <zhuohao@xxxxxxxxxxxx>]

Hi all, May i have your comment for this patch? Thanks.

On Mon, Jun 3, 2019 at 8:38 PM Zhuohao Lee <zhuohao@xxxxxxxxxxxx> wrote:
>
> Currently, we don't have vfs nodes for querying the underlying flash name
> and flash id. This information is important especially when we want to
> know the flash detail of the defective system. In order to support the
> query, we add mtd_debugfs_populate() to create two debugfs nodes
> (ie. partname and partid). The upper driver can assign the pointer to
> partname and partid before calling mtd_device_register().
>
> Signed-off-by: Zhuohao Lee <zhuohao@xxxxxxxxxxxx>
> ---
> Changes in V7:
> - Remove unused check for partname/partid
> - Previous discussion: https://patchwork.ozlabs.org/patch/1109200/
> Changes in V6:
> - Create the debugfs only when the partname/partid is not NULL
> - Previous discussion: https://patchwork.ozlabs.org/patch/1109041/
> Changes in V5:
> - Move debugfs_create_dir() to mtd_debugfs_populate()
> - Previous discussion: https://patchwork.ozlabs.org/patch/1107810/
> Changes in V4:
> - Separate the change to two patches. The first patch is adding the general
>   handling for the partname and partid in the mtdcore.c. The second patch
>   is enabling the two debugfs nodes for spi-nor.
> - Previous discussion: https://patchwork.ozlabs.org/patch/1097377/
> Changes in v3:
> - Add partname and partid to mtd.h and create debugfs inside mtdcore.c
> - Previous discussion: https://patchwork.ozlabs.org/patch/1095731/
> Changes in v2:
> - Change to use debugfs to output flash name and id
> - Previous discussion: https://patchwork.ozlabs.org/patch/1067763/
> ---
>  drivers/mtd/mtdcore.c   | 84 ++++++++++++++++++++++++++++++++++++-----
>  include/linux/mtd/mtd.h |  3 ++
>  2 files changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 3ef01baef9b6..d4603bc1d4eb 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -357,6 +357,80 @@ static const struct device_type mtd_devtype = {
>         .release        = mtd_release,
>  };
>
> +static int mtd_partid_show(struct seq_file *s, void *p)
> +{
> +       struct mtd_info *mtd = s->private;
> +
> +       seq_printf(s, "%s\n", mtd->dbg.partid);
> +
> +       return 0;
> +}
> +
> +static int mtd_partid_debugfs_open(struct inode *inode, struct file *file)
> +{
> +       return single_open(file, mtd_partid_show, inode->i_private);
> +}
> +
> +static const struct file_operations mtd_partid_debug_fops = {
> +       .open           = mtd_partid_debugfs_open,
> +       .read           = seq_read,
> +       .llseek         = seq_lseek,
> +       .release        = single_release,
> +};
> +
> +static int mtd_partname_show(struct seq_file *s, void *p)
> +{
> +       struct mtd_info *mtd = s->private;
> +
> +       seq_printf(s, "%s\n", mtd->dbg.partname);
> +
> +       return 0;
> +}
> +
> +static int mtd_partname_debugfs_open(struct inode *inode, struct file *file)
> +{
> +       return single_open(file, mtd_partname_show, inode->i_private);
> +}
> +
> +static const struct file_operations mtd_partname_debug_fops = {
> +       .open           = mtd_partname_debugfs_open,
> +       .read           = seq_read,
> +       .llseek         = seq_lseek,
> +       .release        = single_release,
> +};
> +
> +static struct dentry *dfs_dir_mtd;
> +
> +static void mtd_debugfs_populate(struct mtd_info *mtd)
> +{
> +       struct device *dev = &mtd->dev;
> +       struct dentry *root, *dent;
> +
> +       if (IS_ERR_OR_NULL(dfs_dir_mtd))
> +               return;
> +
> +       root = mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(dev),
> +                                                    dfs_dir_mtd);
> +       if (IS_ERR_OR_NULL(root)) {
> +               pr_debug("mtd device %s won't show data in debugfs\n",
> +                        dev_name(dev));
> +               return;
> +       }
> +
> +       if (mtd->dbg.partid) {
> +               dent = debugfs_create_file("partid", S_IRUSR, root, mtd,
> +                                          &mtd_partid_debug_fops);
> +               if (IS_ERR_OR_NULL(dent))
> +                       pr_err("cannot create debugfs entry for partid\n");
> +       }
> +       if (mtd->dbg.partname) {
> +               dent = debugfs_create_file("partname", S_IRUSR, root, mtd,
> +                                          &mtd_partname_debug_fops);
> +               if (IS_ERR_OR_NULL(dent))
> +                       pr_err("cannot create debugfs entry for partname\n");
> +       }
> +}
> +
>  #ifndef CONFIG_MMU
>  unsigned mtd_mmap_capabilities(struct mtd_info *mtd)
>  {
> @@ -534,8 +608,6 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
>         return 0;
>  }
>
> -static struct dentry *dfs_dir_mtd;
> -
>  /**
>   *     add_mtd_device - register an MTD device
>   *     @mtd: pointer to new MTD device info structure
> @@ -621,13 +693,7 @@ int add_mtd_device(struct mtd_info *mtd)
>         if (error)
>                 goto fail_nvmem_add;
>
> -       if (!IS_ERR_OR_NULL(dfs_dir_mtd)) {
> -               mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(&mtd->dev), dfs_dir_mtd);
> -               if (IS_ERR_OR_NULL(mtd->dbg.dfs_dir)) {
> -                       pr_debug("mtd device %s won't show data in debugfs\n",
> -                                dev_name(&mtd->dev));
> -               }
> -       }
> +       mtd_debugfs_populate(mtd);
>
>         device_create(&mtd_class, mtd->dev.parent, MTD_DEVT(i) + 1, NULL,
>                       "mtd%dro", i);
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 677768b21a1d..b11afb85d962 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -203,6 +203,9 @@ struct module;      /* only needed for owner field in mtd_info */
>   */
>  struct mtd_debug_info {
>         struct dentry *dfs_dir;
> +
> +       const char *partname;
> +       const char *partid;
>  };
>
>  struct mtd_info {
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/