| Message ID | 20171111144614.5272-1-boris.brezillon@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: Fix debugfs file creation when mtd->->dbg.dfs_dir is invalid | expand |
Hi Richard, Hm, the subject is not accurate. Should be something like that instead: mtd: Avoid probe failures when mtd->dbg.dfs_dir is invalid I can resend a version fixing that if you want. Regards, Boris On Sat, 11 Nov 2017 15:46:14 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > Commit e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs > entries") tried to make MTD related debugfs stuff consistent across the > MTD framework by creating a root <debugfs>/mtd/ directory containing > one directory per MTD device. > > The problem is that, by default, the MTD layer only registers the > master device if no partitions are defined for this master. This > behavior breaks all drivers that expect mtd->dbg.dfs_dir to be filled > correctly after calling mtd_device_register() in order to add their own > debugfs entries. > > The only way we can force all MTD masters to be registered no matter if > they expose partitions or not is by enabling the > CONFIG_MTD_PARTITIONED_MASTER option. > > In such situations, there's no other solution but to accept skipping > debugfs initialization when dbg.dfs_dir is invalid, and when this > happens, inform the user that he should consider enabling > CONFIG_MTD_PARTITIONED_MASTER. > > Fixes: e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries") > Cc: <stable@vger.kernel.org> > Cc: Mario J. Rugiero <mrugiero@gmail.com> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/mtd/devices/docg3.c | 7 ++++++- > drivers/mtd/nand/nandsim.c | 12 +++++++++--- > 2 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c > index 84b16133554b..0806f72102c0 100644 > --- a/drivers/mtd/devices/docg3.c > +++ b/drivers/mtd/devices/docg3.c > @@ -1814,8 +1814,13 @@ static void __init doc_dbg_register(struct mtd_info *floor) > struct dentry *root = floor->dbg.dfs_dir; > struct docg3 *docg3 = floor->priv; > > - if (IS_ERR_OR_NULL(root)) > + if (IS_ERR_OR_NULL(root)) { > + if (IS_ENABLED(CONFIG_DEBUG_FS) && > + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) > + dev_warn(floor->dev.parent, > + "CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); > return; > + } > > debugfs_create_file("docg3_flashcontrol", S_IRUSR, root, docg3, > &flashcontrol_fops); > diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c > index 246b4393118e..a22f4d7ca1cb 100644 > --- a/drivers/mtd/nand/nandsim.c > +++ b/drivers/mtd/nand/nandsim.c > @@ -520,11 +520,17 @@ static int nandsim_debugfs_create(struct nandsim *dev) > struct dentry *root = nsmtd->dbg.dfs_dir; > struct dentry *dent; > > - if (!IS_ENABLED(CONFIG_DEBUG_FS)) > - return 0; > + if (IS_ENABLED(CONFIG_DEBUG_FS) && > + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) && > + dev->nbparts) > + NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); > > + /* > + * Just skip debugfs initialization when the debugfs directory is > + * missing. > + */ > if (IS_ERR_OR_NULL(root)) > - return -1; > + return 0; > > dent = debugfs_create_file("nandsim_wear_report", S_IRUSR, > root, dev, &dfs_fops);
On Sat, 11 Nov 2017 15:46:14 +0100 Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > Commit e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs > entries") tried to make MTD related debugfs stuff consistent across the > MTD framework by creating a root <debugfs>/mtd/ directory containing > one directory per MTD device. > > The problem is that, by default, the MTD layer only registers the > master device if no partitions are defined for this master. This > behavior breaks all drivers that expect mtd->dbg.dfs_dir to be filled > correctly after calling mtd_device_register() in order to add their own > debugfs entries. > > The only way we can force all MTD masters to be registered no matter if > they expose partitions or not is by enabling the > CONFIG_MTD_PARTITIONED_MASTER option. > > In such situations, there's no other solution but to accept skipping > debugfs initialization when dbg.dfs_dir is invalid, and when this > happens, inform the user that he should consider enabling > CONFIG_MTD_PARTITIONED_MASTER. > > Fixes: e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries") > Cc: <stable@vger.kernel.org> > Cc: Mario J. Rugiero <mrugiero@gmail.com> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > drivers/mtd/devices/docg3.c | 7 ++++++- > drivers/mtd/nand/nandsim.c | 12 +++++++++--- > 2 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c > index 84b16133554b..0806f72102c0 100644 > --- a/drivers/mtd/devices/docg3.c > +++ b/drivers/mtd/devices/docg3.c > @@ -1814,8 +1814,13 @@ static void __init doc_dbg_register(struct mtd_info *floor) > struct dentry *root = floor->dbg.dfs_dir; > struct docg3 *docg3 = floor->priv; > > - if (IS_ERR_OR_NULL(root)) > + if (IS_ERR_OR_NULL(root)) { > + if (IS_ENABLED(CONFIG_DEBUG_FS) && > + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) > + dev_warn(floor->dev.parent, > + "CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); > return; > + } > > debugfs_create_file("docg3_flashcontrol", S_IRUSR, root, docg3, > &flashcontrol_fops); > diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c > index 246b4393118e..a22f4d7ca1cb 100644 > --- a/drivers/mtd/nand/nandsim.c > +++ b/drivers/mtd/nand/nandsim.c > @@ -520,11 +520,17 @@ static int nandsim_debugfs_create(struct nandsim *dev) > struct dentry *root = nsmtd->dbg.dfs_dir; > struct dentry *dent; > > - if (!IS_ENABLED(CONFIG_DEBUG_FS)) > - return 0; > + if (IS_ENABLED(CONFIG_DEBUG_FS) && > + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) && > + dev->nbparts) > + NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); > Actually we can't rely on dev->nbparts here because partitions can be defined through the cmdline (using mtdpard=). I'll fix that with something similar to what is done in the docg3 driver: /* * Just skip debugfs initialization when the debugfs directory is * missing. */ if (IS_ERR_OR_NULL(root)) { if (IS_ENABLED(CONFIG_DEBUG_FS) && !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); return 0; } > + /* > + * Just skip debugfs initialization when the debugfs directory is > + * missing. > + */ > if (IS_ERR_OR_NULL(root)) > - return -1; > + return 0; > > dent = debugfs_create_file("nandsim_wear_report", S_IRUSR, > root, dev, &dfs_fops);
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c index 84b16133554b..0806f72102c0 100644 --- a/drivers/mtd/devices/docg3.c +++ b/drivers/mtd/devices/docg3.c @@ -1814,8 +1814,13 @@ static void __init doc_dbg_register(struct mtd_info *floor) struct dentry *root = floor->dbg.dfs_dir; struct docg3 *docg3 = floor->priv; - if (IS_ERR_OR_NULL(root)) + if (IS_ERR_OR_NULL(root)) { + if (IS_ENABLED(CONFIG_DEBUG_FS) && + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER)) + dev_warn(floor->dev.parent, + "CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); return; + } debugfs_create_file("docg3_flashcontrol", S_IRUSR, root, docg3, &flashcontrol_fops); diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c index 246b4393118e..a22f4d7ca1cb 100644 --- a/drivers/mtd/nand/nandsim.c +++ b/drivers/mtd/nand/nandsim.c @@ -520,11 +520,17 @@ static int nandsim_debugfs_create(struct nandsim *dev) struct dentry *root = nsmtd->dbg.dfs_dir; struct dentry *dent; - if (!IS_ENABLED(CONFIG_DEBUG_FS)) - return 0; + if (IS_ENABLED(CONFIG_DEBUG_FS) && + !IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) && + dev->nbparts) + NS_WARN("CONFIG_MTD_PARTITIONED_MASTER must be enabled to expose debugfs stuff\n"); + /* + * Just skip debugfs initialization when the debugfs directory is + * missing. + */ if (IS_ERR_OR_NULL(root)) - return -1; + return 0; dent = debugfs_create_file("nandsim_wear_report", S_IRUSR, root, dev, &dfs_fops);
Commit e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries") tried to make MTD related debugfs stuff consistent across the MTD framework by creating a root <debugfs>/mtd/ directory containing one directory per MTD device. The problem is that, by default, the MTD layer only registers the master device if no partitions are defined for this master. This behavior breaks all drivers that expect mtd->dbg.dfs_dir to be filled correctly after calling mtd_device_register() in order to add their own debugfs entries. The only way we can force all MTD masters to be registered no matter if they expose partitions or not is by enabling the CONFIG_MTD_PARTITIONED_MASTER option. In such situations, there's no other solution but to accept skipping debugfs initialization when dbg.dfs_dir is invalid, and when this happens, inform the user that he should consider enabling CONFIG_MTD_PARTITIONED_MASTER. Fixes: e8e3edb95ce6 ("mtd: create per-device and module-scope debugfs entries") Cc: <stable@vger.kernel.org> Cc: Mario J. Rugiero <mrugiero@gmail.com> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- drivers/mtd/devices/docg3.c | 7 ++++++- drivers/mtd/nand/nandsim.c | 12 +++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-)