SPI NOR has equivalent sysfs device attributes, thus let's remove the
duplicated debugfs entries. There's no one else using partid and partname
debugfs files, thus remove them from the mtd core as well.
If you find the idea acceptable, I would like to queue these through
spi-nor/next, I'll have some other patches that will depend on these.
Tudor Ambarus (2):
mtd: spi-nor: Remove debugfs entries that duplicate sysfs entries
mtd: core: Remove partid and partname debugfs files
drivers/mtd/mtdcore.c | 35 +----------------------------------
drivers/mtd/spi-nor/core.c | 12 ------------
include/linux/mtd/mtd.h | 3 ---
3 files changed, 1 insertion(+), 49 deletions(-)
--
2.25.1
SPI NOR sysfs defines partname and jedec_id device attributes, which
duplicate the information from debugfs. Since the sysfs directory
structure and the attributes in each directory define an ABI between the
kernel and user space, thus it can never be removed, remove the debugfs
entries so that we don't duplicate the information.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/core.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 2e21d5ac0e2d..435bd17af0b4 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3126,16 +3126,6 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
return NULL;
}
-static void spi_nor_debugfs_init(struct spi_nor *nor,
- const struct flash_info *info)
-{
- struct mtd_info *mtd = &nor->mtd;
-
- mtd->dbg.partname = info->name;
- mtd->dbg.partid = devm_kasprintf(nor->dev, GFP_KERNEL, "spi-nor:%*phN",
- info->id_len, info->id);
-}
-
static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
const char *name)
{
@@ -3243,8 +3233,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->info = info;
- spi_nor_debugfs_init(nor, info);
-
mutex_init(&nor->lock);
/* Init flash parameters based on flash_info struct and SFDP */
--
2.25.1
partid and partname debugfs file are no longer used in mtd, remove
dead code.
Signed-off-by: Tudor Ambarus <[email protected]>
---
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);
}
#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 17/12/21 02:26PM, Tudor Ambarus wrote:
> SPI NOR sysfs defines partname and jedec_id device attributes, which
> duplicate the information from debugfs. Since the sysfs directory
> structure and the attributes in each directory define an ABI between the
> kernel and user space, thus it can never be removed, remove the debugfs
> entries so that we don't duplicate the information.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
Am 2021-12-17 13:26, schrieb Tudor Ambarus:
> SPI NOR has equivalent sysfs device attributes, thus let's remove the
> duplicated debugfs entries. There's no one else using partid and
> partname
> debugfs files, thus remove them from the mtd core as well.
>
> If you find the idea acceptable, I would like to queue these through
> spi-nor/next, I'll have some other patches that will depend on these.
I'm fine with it.
-michael
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 <[email protected]>
> ---
> 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
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
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 <[email protected]>
>> ---
>> 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, [email protected] 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
On Fri, 17 Dec 2021 14:26:34 +0200, Tudor Ambarus wrote:
> SPI NOR has equivalent sysfs device attributes, thus let's remove the
> duplicated debugfs entries. There's no one else using partid and partname
> debugfs files, thus remove them from the mtd core as well.
>
> If you find the idea acceptable, I would like to queue these through
> spi-nor/next, I'll have some other patches that will depend on these.
>
> [...]
Applied to spi-nor/next, thanks!
[1/2] mtd: spi-nor: Remove debugfs entries that duplicate sysfs entries
https://git.kernel.org/mtd/c/5f340402bbfc
Best regards,
--
Tudor Ambarus <[email protected]>