From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eDXIv-0002mv-HZ for linux-mtd@lists.infradead.org; Sat, 11 Nov 2017 15:02:43 +0000 Date: Sat, 11 Nov 2017 16:02:19 +0100 From: Boris Brezillon To: David Woodhouse , Brian Norris , Boris Brezillon , Marek Vasut , Richard Weinberger , Cyrille Pitchen , linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org, "Mario J . Rugiero" Subject: Re: [PATCH] mtd: Fix debugfs file creation when mtd->->dbg.dfs_dir is invalid Message-ID: <20171111160219.467842e6@bbrezillon> In-Reply-To: <20171111144614.5272-1-boris.brezillon@free-electrons.com> References: <20171111144614.5272-1-boris.brezillon@free-electrons.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sat, 11 Nov 2017 15:46:14 +0100 Boris Brezillon 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 /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: > Cc: Mario J. Rugiero > Signed-off-by: Boris Brezillon > --- > 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);