| Message ID | 20211217122636.474976-3-tudor.ambarus@microchip.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mtd: core: Remove partid and partname debugfs files | expand |
On 17/12/21 02:26PM, Tudor Ambarus wrote: > partid and partname debugfs file are no longer used in mtd, remove > dead code. Hmm, spi-nor was the only user? Quick grepping does confirm that. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/mtdcore.c | 35 +---------------------------------- > include/linux/mtd/mtd.h | 3 --- > 2 files changed, 1 insertion(+), 37 deletions(-) > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index 9186268d361b..1977755fc955 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -336,49 +336,16 @@ static const struct device_type mtd_devtype = { > .release = mtd_release, > }; > > -static int mtd_partid_debug_show(struct seq_file *s, void *p) > -{ > - struct mtd_info *mtd = s->private; > - > - seq_printf(s, "%s\n", mtd->dbg.partid); > - > - return 0; > -} > - > -DEFINE_SHOW_ATTRIBUTE(mtd_partid_debug); > - > -static int mtd_partname_debug_show(struct seq_file *s, void *p) > -{ > - struct mtd_info *mtd = s->private; > - > - seq_printf(s, "%s\n", mtd->dbg.partname); > - > - return 0; > -} > - > -DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug); > - > static struct dentry *dfs_dir_mtd; > > static void mtd_debugfs_populate(struct mtd_info *mtd) > { > - struct mtd_info *master = mtd_get_master(mtd); > struct device *dev = &mtd->dev; > - struct dentry *root; > > if (IS_ERR_OR_NULL(dfs_dir_mtd)) > return; > > - root = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); > - mtd->dbg.dfs_dir = root; > - > - if (master->dbg.partid) > - debugfs_create_file("partid", 0400, root, master, > - &mtd_partid_debug_fops); > - > - if (master->dbg.partname) > - debugfs_create_file("partname", 0400, root, master, > - &mtd_partname_debug_fops); > + mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); What is the point of having creating the directory if there is not going to be anything in it at all? I think we should drop the entire thing if there is no intention of using it. > } > > #ifndef CONFIG_MMU > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index f5e7dfc2e4e9..00cb269d0261 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -188,9 +188,6 @@ struct module; /* only needed for owner field in mtd_info */ > */ > struct mtd_debug_info { > struct dentry *dfs_dir; > - > - const char *partname; > - const char *partid; > }; > > /** > -- > 2.25.1 >
On 12/20/21 3:07 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 17/12/21 02:26PM, Tudor Ambarus wrote: >> partid and partname debugfs file are no longer used in mtd, remove s/file/files >> dead code. > > Hmm, spi-nor was the only user? Quick grepping does confirm that. Yes. > >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> drivers/mtd/mtdcore.c | 35 +---------------------------------- >> include/linux/mtd/mtd.h | 3 --- >> 2 files changed, 1 insertion(+), 37 deletions(-) >> >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c >> index 9186268d361b..1977755fc955 100644 >> --- a/drivers/mtd/mtdcore.c >> +++ b/drivers/mtd/mtdcore.c >> @@ -336,49 +336,16 @@ static const struct device_type mtd_devtype = { >> .release = mtd_release, >> }; >> >> -static int mtd_partid_debug_show(struct seq_file *s, void *p) >> -{ >> - struct mtd_info *mtd = s->private; >> - >> - seq_printf(s, "%s\n", mtd->dbg.partid); >> - >> - return 0; >> -} >> - >> -DEFINE_SHOW_ATTRIBUTE(mtd_partid_debug); >> - >> -static int mtd_partname_debug_show(struct seq_file *s, void *p) >> -{ >> - struct mtd_info *mtd = s->private; >> - >> - seq_printf(s, "%s\n", mtd->dbg.partname); >> - >> - return 0; >> -} >> - >> -DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug); >> - >> static struct dentry *dfs_dir_mtd; >> >> static void mtd_debugfs_populate(struct mtd_info *mtd) >> { >> - struct mtd_info *master = mtd_get_master(mtd); >> struct device *dev = &mtd->dev; >> - struct dentry *root; >> >> if (IS_ERR_OR_NULL(dfs_dir_mtd)) >> return; >> >> - root = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); >> - mtd->dbg.dfs_dir = root; >> - >> - if (master->dbg.partid) >> - debugfs_create_file("partid", 0400, root, master, >> - &mtd_partid_debug_fops); >> - >> - if (master->dbg.partname) >> - debugfs_create_file("partname", 0400, root, master, >> - &mtd_partname_debug_fops); >> + mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); > > What is the point of having creating the directory if there is not going > to be anything in it at all? I think we should drop the entire thing if There will be files in it: mtdswap_stats, nandsim_wear_report, docg3 files. $ git grep dbg.dfs_dir drivers/mtd/ drivers/mtd/devices/docg3.c: struct dentry *root = floor->dbg.dfs_dir; drivers/mtd/mtdcore.c: mtd->dbg.dfs_dir = root; drivers/mtd/mtdcore.c: debugfs_remove_recursive(mtd->dbg.dfs_dir); drivers/mtd/mtdswap.c: struct dentry *root = d->mtd->dbg.dfs_dir; drivers/mtd/nand/raw/nandsim.c: struct dentry *root = nsmtd->dbg.dfs_dir; drivers/mtd/ubi/debug.c: debugfs_remove_recursive(ubi->dbg.dfs_dir);
On 20/12/21 01:23PM, Tudor.Ambarus@microchip.com wrote: > On 12/20/21 3:07 PM, Pratyush Yadav wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On 17/12/21 02:26PM, Tudor Ambarus wrote: > >> partid and partname debugfs file are no longer used in mtd, remove > > s/file/files > > >> dead code. > > > > Hmm, spi-nor was the only user? Quick grepping does confirm that. > > Yes. > > > > >> > >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > >> --- > >> drivers/mtd/mtdcore.c | 35 +---------------------------------- > >> include/linux/mtd/mtd.h | 3 --- > >> 2 files changed, 1 insertion(+), 37 deletions(-) > >> > >> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > >> index 9186268d361b..1977755fc955 100644 > >> --- a/drivers/mtd/mtdcore.c > >> +++ b/drivers/mtd/mtdcore.c > >> @@ -336,49 +336,16 @@ static const struct device_type mtd_devtype = { > >> .release = mtd_release, > >> }; > >> > >> -static int mtd_partid_debug_show(struct seq_file *s, void *p) > >> -{ > >> - struct mtd_info *mtd = s->private; > >> - > >> - seq_printf(s, "%s\n", mtd->dbg.partid); > >> - > >> - return 0; > >> -} > >> - > >> -DEFINE_SHOW_ATTRIBUTE(mtd_partid_debug); > >> - > >> -static int mtd_partname_debug_show(struct seq_file *s, void *p) > >> -{ > >> - struct mtd_info *mtd = s->private; > >> - > >> - seq_printf(s, "%s\n", mtd->dbg.partname); > >> - > >> - return 0; > >> -} > >> - > >> -DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug); > >> - > >> static struct dentry *dfs_dir_mtd; > >> > >> static void mtd_debugfs_populate(struct mtd_info *mtd) > >> { > >> - struct mtd_info *master = mtd_get_master(mtd); > >> struct device *dev = &mtd->dev; > >> - struct dentry *root; > >> > >> if (IS_ERR_OR_NULL(dfs_dir_mtd)) > >> return; > >> > >> - root = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); > >> - mtd->dbg.dfs_dir = root; > >> - > >> - if (master->dbg.partid) > >> - debugfs_create_file("partid", 0400, root, master, > >> - &mtd_partid_debug_fops); > >> - > >> - if (master->dbg.partname) > >> - debugfs_create_file("partname", 0400, root, master, > >> - &mtd_partname_debug_fops); > >> + mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); > > > > What is the point of having creating the directory if there is not going > > to be anything in it at all? I think we should drop the entire thing if > There will be files in it: mtdswap_stats, nandsim_wear_report, docg3 files. > > $ git grep dbg.dfs_dir drivers/mtd/ > drivers/mtd/devices/docg3.c: struct dentry *root = floor->dbg.dfs_dir; > drivers/mtd/mtdcore.c: mtd->dbg.dfs_dir = root; > drivers/mtd/mtdcore.c: debugfs_remove_recursive(mtd->dbg.dfs_dir); > drivers/mtd/mtdswap.c: struct dentry *root = d->mtd->dbg.dfs_dir; > drivers/mtd/nand/raw/nandsim.c: struct dentry *root = nsmtd->dbg.dfs_dir; > drivers/mtd/ubi/debug.c: debugfs_remove_recursive(ubi->dbg.dfs_dir); Ok, I didn't see that. In that case, Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 9186268d361b..1977755fc955 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -336,49 +336,16 @@ static const struct device_type mtd_devtype = { .release = mtd_release, }; -static int mtd_partid_debug_show(struct seq_file *s, void *p) -{ - struct mtd_info *mtd = s->private; - - seq_printf(s, "%s\n", mtd->dbg.partid); - - return 0; -} - -DEFINE_SHOW_ATTRIBUTE(mtd_partid_debug); - -static int mtd_partname_debug_show(struct seq_file *s, void *p) -{ - struct mtd_info *mtd = s->private; - - seq_printf(s, "%s\n", mtd->dbg.partname); - - return 0; -} - -DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug); - static struct dentry *dfs_dir_mtd; static void mtd_debugfs_populate(struct mtd_info *mtd) { - struct mtd_info *master = mtd_get_master(mtd); struct device *dev = &mtd->dev; - struct dentry *root; if (IS_ERR_OR_NULL(dfs_dir_mtd)) return; - root = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); - mtd->dbg.dfs_dir = root; - - if (master->dbg.partid) - debugfs_create_file("partid", 0400, root, master, - &mtd_partid_debug_fops); - - if (master->dbg.partname) - debugfs_create_file("partname", 0400, root, master, - &mtd_partname_debug_fops); + mtd->dbg.dfs_dir = debugfs_create_dir(dev_name(dev), dfs_dir_mtd); } #ifndef CONFIG_MMU diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index f5e7dfc2e4e9..00cb269d0261 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -188,9 +188,6 @@ struct module; /* only needed for owner field in mtd_info */ */ struct mtd_debug_info { struct dentry *dfs_dir; - - const char *partname; - const char *partid; }; /**
partid and partname debugfs file are no longer used in mtd, remove dead code. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/mtdcore.c | 35 +---------------------------------- include/linux/mtd/mtd.h | 3 --- 2 files changed, 1 insertion(+), 37 deletions(-)