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

[Zhuohao Lee <zhuohao@xxxxxxxxxxxx>]

Hi Boris and all,

May i have your review for this patch?
I have changed the code based on the previous discussion?
Thank you for your time and effort :)

On Mon, Jun 17, 2019 at 10:13 AM Zhuohao Lee <zhuohao@xxxxxxxxxxxx> wrote:
>
> 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/