[PATCH 0/6] Move debugfs to block layer, take 3

[PATCH 0/6] Move debugfs to block layer, take 3

[Linus Walleij]

Changes since last revision:

- Added a necessary EXPORT patch

- Added a patch on top that moves the debugfs into the block
  module.

Linus Walleij (6):
  mmc: block: Anonymize the drv op data pointer
  mmc: debugfs: No blocklayer = no card status and extcsd
  mmc: ops: export mmc_get_status()
  mmc: debugfs: Move card status retrieveal into the block layer
  mmc: debugfs: Move EXT CSD debugfs access to block layer
  mmc: debugfs: Move block debugfs into block module

 drivers/mmc/core/block.c   | 151 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/core/debugfs.c |  90 +--------------------------
 drivers/mmc/core/mmc_ops.c |   1 +
 drivers/mmc/core/queue.h   |   6 +-
 4 files changed, 155 insertions(+), 93 deletions(-)

-- 
2.9.4

[PATCH 1/6 v3] mmc: block: Anonymize the drv op data pointer

[Linus Walleij]

We have a data pointer for the ioctl() data, but we need to
pass other data along with the DRV_OP:s, so make this a
void * so it can be reused.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- No changes just resending
---
 drivers/mmc/core/block.c | 8 +++++---
 drivers/mmc/core/queue.h | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 64f9fda92229..7a365d7641b5 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -598,7 +598,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
 		__GFP_RECLAIM);
 	idatas[0] = idata;
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_IOCTL;
-	req_to_mmc_queue_req(req)->idata = idatas;
+	req_to_mmc_queue_req(req)->drv_op_data = idatas;
 	req_to_mmc_queue_req(req)->ioc_count = 1;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ioc_err = req_to_mmc_queue_req(req)->drv_op_result;
@@ -674,7 +674,7 @@ static int mmc_blk_ioctl_multi_cmd(struct block_device *bdev,
 		idata[0]->ic.write_flag ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN,
 		__GFP_RECLAIM);
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_IOCTL;
-	req_to_mmc_queue_req(req)->idata = idata;
+	req_to_mmc_queue_req(req)->drv_op_data = idata;
 	req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ioc_err = req_to_mmc_queue_req(req)->drv_op_result;
@@ -1175,6 +1175,7 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 	struct mmc_queue_req *mq_rq;
 	struct mmc_card *card = mq->card;
 	struct mmc_blk_data *md = mq->blkdata;
+	struct mmc_blk_ioc_data **idata;
 	int ret;
 	int i;
 
@@ -1182,8 +1183,9 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 
 	switch (mq_rq->drv_op) {
 	case MMC_DRV_OP_IOCTL:
+		idata = mq_rq->drv_op_data;
 		for (i = 0; i < mq_rq->ioc_count; i++) {
-			ret = __mmc_blk_ioctl_cmd(card, md, mq_rq->idata[i]);
+			ret = __mmc_blk_ioctl_cmd(card, md, idata[i]);
 			if (ret)
 				break;
 		}
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index 361b46408e0f..cf26a15a64bf 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -51,7 +51,7 @@ struct mmc_queue_req {
 	struct mmc_async_req	areq;
 	enum mmc_drv_op		drv_op;
 	int			drv_op_result;
-	struct mmc_blk_ioc_data	**idata;
+	void			*drv_op_data;
 	unsigned int		ioc_count;
 };
 
-- 
2.9.4

[PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Linus Walleij]

If we don't have the block layer enabled, we do not present card
status and extcsd in the debugfs.

Debugfs is not ABI, and maintaining files of no relevance for
non-block devices comes at a high maintenance cost if we shall
support it with the block layer compiled out.

The expected number of debugfs users utilizing these two
debugfs files is already low as there is an ioctl() to get the
same information using the mmc-tools, and of these few users
the expected number of people using it on SDIO or combo cards
are expected to be zero.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- No changes just resending
---
 drivers/mmc/core/debugfs.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index a1fba5732d66..b176932b8092 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -281,6 +281,8 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
 	debugfs_remove_recursive(host->debugfs_root);
 }
 
+#if IS_ENABLED(CONFIG_MMC_BLOCK)
+
 static int mmc_dbg_card_status_get(void *data, u64 *val)
 {
 	struct mmc_card	*card = data;
@@ -360,6 +362,32 @@ static const struct file_operations mmc_dbg_ext_csd_fops = {
 	.llseek		= default_llseek,
 };
 
+static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
+{
+	if (mmc_card_mmc(card) || mmc_card_sd(card)) {
+		if (!debugfs_create_file("status", S_IRUSR, root, card,
+					 &mmc_dbg_card_status_fops))
+			return -EIO;
+	}
+
+	if (mmc_card_mmc(card)) {
+		if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
+					 &mmc_dbg_ext_csd_fops))
+			return -EIO;
+	}
+
+	return 0;
+}
+
+#else /* !IS_ENABLED(CONFIG_MMC_BLOCK) */
+
+static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
+{
+	return 0;
+}
+
+#endif
+
 void mmc_add_card_debugfs(struct mmc_card *card)
 {
 	struct mmc_host	*host = card->host;
@@ -382,15 +410,8 @@ void mmc_add_card_debugfs(struct mmc_card *card)
 	if (!debugfs_create_x32("state", S_IRUSR, root, &card->state))
 		goto err;
 
-	if (mmc_card_mmc(card) || mmc_card_sd(card))
-		if (!debugfs_create_file("status", S_IRUSR, root, card,
-					&mmc_dbg_card_status_fops))
-			goto err;
-
-	if (mmc_card_mmc(card))
-		if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
-					&mmc_dbg_ext_csd_fops))
-			goto err;
+	if (mmc_add_block_debugfs(card, root))
+		goto err;
 
 	return;
 
-- 
2.9.4

[PATCH 3/6 v3] mmc: ops: export mmc_get_status()

[Linus Walleij]

This function retrieves the status of the card with the default
number of retries. Since the block layer wants to use this, and
since the block layer is a loadable kernel module, we need to
export this symbol.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- New patch to fix a build error, enumerating v3 to keep it
  together with the other patches.
---
 drivers/mmc/core/mmc_ops.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index ae1fc4818240..e6e47aef3849 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -82,6 +82,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
 {
 	return __mmc_send_status(card, status, MMC_CMD_RETRIES);
 }
+EXPORT_SYMBOL_GPL(mmc_send_status);
 
 static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
 {
-- 
2.9.4

[PATCH 4/6 v3] mmc: debugfs: Move card status retrieveal into the block layer

[Linus Walleij]

This debugfs entry suffers from all the same starvation
issues as the other userspace things, under e.g. a heavy
dd operation.

It is therefore logical to move this over to the block layer
when it is enabled, using the new custom requests and issue
it using the block request queue.

This makes this debugfs card access land under the request
queue host lock instead of orthogonally taking the lock.

Tested during heavy dd load by cat:in the status file.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- No changes just resending
---
 drivers/mmc/core/block.c   | 28 ++++++++++++++++++++++++++++
 drivers/mmc/core/block.h   |  1 +
 drivers/mmc/core/debugfs.c | 15 ++-------------
 drivers/mmc/core/queue.h   |  2 ++
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 7a365d7641b5..96fe0640c480 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1176,6 +1176,7 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 	struct mmc_card *card = mq->card;
 	struct mmc_blk_data *md = mq->blkdata;
 	struct mmc_blk_ioc_data **idata;
+	u32 status;
 	int ret;
 	int i;
 
@@ -1205,6 +1206,11 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 			card->ext_csd.boot_ro_lock |=
 				EXT_CSD_BOOT_WP_B_PWR_WP_EN;
 		break;
+	case MMC_DRV_OP_GET_CARD_STATUS:
+		ret = mmc_send_status(card, &status);
+		if (!ret)
+			ret = status;
+		break;
 	default:
 		pr_err("%s: unknown driver specific operation\n",
 		       md->disk->disk_name);
@@ -1954,6 +1960,28 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 		mmc_put_card(card);
 }
 
+/* Called from debugfs for MMC/SD cards */
+int mmc_blk_card_status_get(struct mmc_card *card, u64 *val)
+{
+	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
+	struct mmc_queue *mq = &md->queue;
+	struct request *req;
+	int ret;
+
+	/* Ask the block layer about the card status */
+	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
+	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	blk_execute_rq(mq->queue, NULL, req, 0);
+	ret = req_to_mmc_queue_req(req)->drv_op_result;
+	if (ret >= 0) {
+		*val = ret;
+		ret = 0;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(mmc_blk_card_status_get);
+
 static inline int mmc_blk_readonly(struct mmc_card *card)
 {
 	return mmc_card_readonly(card) ||
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index 860ca7c8df86..70861f3a059a 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -5,5 +5,6 @@ struct mmc_queue;
 struct request;
 
 void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
+int mmc_blk_card_status_get(struct mmc_card *card, u64 *val);
 
 #endif
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index b176932b8092..dca5717c437b 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -22,6 +22,7 @@
 #include "core.h"
 #include "card.h"
 #include "host.h"
+#include "block.h"
 #include "mmc_ops.h"
 
 #ifdef CONFIG_FAIL_MMC_REQUEST
@@ -285,19 +286,7 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
 
 static int mmc_dbg_card_status_get(void *data, u64 *val)
 {
-	struct mmc_card	*card = data;
-	u32		status;
-	int		ret;
-
-	mmc_get_card(card);
-
-	ret = mmc_send_status(data, &status);
-	if (!ret)
-		*val = status;
-
-	mmc_put_card(card);
-
-	return ret;
+	return mmc_blk_card_status_get(data, val);
 }
 DEFINE_SIMPLE_ATTRIBUTE(mmc_dbg_card_status_fops, mmc_dbg_card_status_get,
 		NULL, "%08llx\n");
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index cf26a15a64bf..c2325c6659f5 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -36,10 +36,12 @@ struct mmc_blk_request {
  * enum mmc_drv_op - enumerates the operations in the mmc_queue_req
  * @MMC_DRV_OP_IOCTL: ioctl operation
  * @MMC_DRV_OP_BOOT_WP: write protect boot partitions
+ * @MMC_DRV_OP_GET_CARD_STATUS: get card status
  */
 enum mmc_drv_op {
 	MMC_DRV_OP_IOCTL,
 	MMC_DRV_OP_BOOT_WP,
+	MMC_DRV_OP_GET_CARD_STATUS,
 };
 
 struct mmc_queue_req {
-- 
2.9.4

[PATCH 5/6 v3] mmc: debugfs: Move EXT CSD debugfs access to block layer

[Linus Walleij]

Just like the previous commit moving status retriveal for
MMC and SD cards into the block layer (when active), this
moves the retrieveal of the EXT CSD from the card from
the special ext_csd file into the block stack as well.

It has been tested with and without the block layer and
during heavy load from dd.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- No changes just resending
---
 drivers/mmc/core/block.c   | 21 +++++++++++++++++++++
 drivers/mmc/core/block.h   |  1 +
 drivers/mmc/core/debugfs.c |  4 +---
 drivers/mmc/core/queue.h   |  2 ++
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 96fe0640c480..4708f95ebdd0 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1176,6 +1176,7 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 	struct mmc_card *card = mq->card;
 	struct mmc_blk_data *md = mq->blkdata;
 	struct mmc_blk_ioc_data **idata;
+	u8 **ext_csd;
 	u32 status;
 	int ret;
 	int i;
@@ -1211,6 +1212,10 @@ static void mmc_blk_issue_drv_op(struct mmc_queue *mq, struct request *req)
 		if (!ret)
 			ret = status;
 		break;
+	case MMC_DRV_OP_GET_EXT_CSD:
+		ext_csd = mq_rq->drv_op_data;
+		ret = mmc_get_ext_csd(card, ext_csd);
+		break;
 	default:
 		pr_err("%s: unknown driver specific operation\n",
 		       md->disk->disk_name);
@@ -1982,6 +1987,22 @@ int mmc_blk_card_status_get(struct mmc_card *card, u64 *val)
 }
 EXPORT_SYMBOL(mmc_blk_card_status_get);
 
+/* Called from debugfs for MMC cards */
+int mmc_blk_get_ext_csd(struct mmc_card *card, u8 **ext_csd)
+{
+	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
+	struct mmc_queue *mq = &md->queue;
+	struct request *req;
+
+	/* Ask the block layer about the EXT CSD */
+	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
+	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_data = ext_csd;
+	blk_execute_rq(mq->queue, NULL, req, 0);
+	return req_to_mmc_queue_req(req)->drv_op_result;
+}
+EXPORT_SYMBOL(mmc_blk_get_ext_csd);
+
 static inline int mmc_blk_readonly(struct mmc_card *card)
 {
 	return mmc_card_readonly(card) ||
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index 70861f3a059a..377ebbf6a978 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -6,5 +6,6 @@ struct request;
 
 void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
 int mmc_blk_card_status_get(struct mmc_card *card, u64 *val);
+int mmc_blk_get_ext_csd(struct mmc_card *card, u8 **ext_csd);
 
 #endif
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index dca5717c437b..cc1f7085111c 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -305,9 +305,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 	if (!buf)
 		return -ENOMEM;
 
-	mmc_get_card(card);
-	err = mmc_get_ext_csd(card, &ext_csd);
-	mmc_put_card(card);
+	err = mmc_blk_get_ext_csd(card, &ext_csd);
 	if (err)
 		goto out_free;
 
diff --git a/drivers/mmc/core/queue.h b/drivers/mmc/core/queue.h
index c2325c6659f5..04fc89360a7a 100644
--- a/drivers/mmc/core/queue.h
+++ b/drivers/mmc/core/queue.h
@@ -37,11 +37,13 @@ struct mmc_blk_request {
  * @MMC_DRV_OP_IOCTL: ioctl operation
  * @MMC_DRV_OP_BOOT_WP: write protect boot partitions
  * @MMC_DRV_OP_GET_CARD_STATUS: get card status
+ * @MMC_DRV_OP_GET_EXT_CSD: get the EXT CSD from an eMMC card
  */
 enum mmc_drv_op {
 	MMC_DRV_OP_IOCTL,
 	MMC_DRV_OP_BOOT_WP,
 	MMC_DRV_OP_GET_CARD_STATUS,
+	MMC_DRV_OP_GET_EXT_CSD,
 };
 
 struct mmc_queue_req {
-- 
2.9.4

[PATCH 6/6 v3] mmc: debugfs: Move block debugfs into block module

[Linus Walleij]

The if #IS_ENABLED() clauses inside the debugfs code isn't
very nice.

And alternative is to move the block layer debugfs file
handling into the block module, so they live and die with
the block initialization of the card.

This makes the code smaller since we do not need cross-calls
between debugfs.c and block.c.

On the other hand it moves some debugfs code from debugfs.c
and into block.c.

So both approaches have their merits.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- New patch after discussion with Ulf, numbering v3 to keep
  the series together.

Ulf: I leave it to you to decide if you prefer to centralize
debugfs code in debugfs.c or to localize the block debugfs
code inside block.c.
---
 drivers/mmc/core/block.c   | 170 +++++++++++++++++++++++++++++++++++----------
 drivers/mmc/core/block.h   |   2 -
 drivers/mmc/core/debugfs.c |  96 -------------------------
 3 files changed, 132 insertions(+), 136 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 4708f95ebdd0..1ce6012ce3c1 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -36,6 +36,7 @@
 #include <linux/compat.h>
 #include <linux/pm_runtime.h>
 #include <linux/idr.h>
+#include <linux/debugfs.h>
 
 #include <linux/mmc/ioctl.h>
 #include <linux/mmc/card.h>
@@ -1965,44 +1966,6 @@ void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 		mmc_put_card(card);
 }
 
-/* Called from debugfs for MMC/SD cards */
-int mmc_blk_card_status_get(struct mmc_card *card, u64 *val)
-{
-	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
-	struct mmc_queue *mq = &md->queue;
-	struct request *req;
-	int ret;
-
-	/* Ask the block layer about the card status */
-	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
-	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
-	blk_execute_rq(mq->queue, NULL, req, 0);
-	ret = req_to_mmc_queue_req(req)->drv_op_result;
-	if (ret >= 0) {
-		*val = ret;
-		ret = 0;
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL(mmc_blk_card_status_get);
-
-/* Called from debugfs for MMC cards */
-int mmc_blk_get_ext_csd(struct mmc_card *card, u8 **ext_csd)
-{
-	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
-	struct mmc_queue *mq = &md->queue;
-	struct request *req;
-
-	/* Ask the block layer about the EXT CSD */
-	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
-	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
-	req_to_mmc_queue_req(req)->drv_op_data = ext_csd;
-	blk_execute_rq(mq->queue, NULL, req, 0);
-	return req_to_mmc_queue_req(req)->drv_op_result;
-}
-EXPORT_SYMBOL(mmc_blk_get_ext_csd);
-
 static inline int mmc_blk_readonly(struct mmc_card *card)
 {
 	return mmc_card_readonly(card) ||
@@ -2275,6 +2238,134 @@ static int mmc_add_disk(struct mmc_blk_data *md)
 	return ret;
 }
 
+#ifdef CONFIG_DEBUG_FS
+
+static int mmc_dbg_card_status_get(void *data, u64 *val)
+{
+	struct mmc_card *card = data;
+	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
+	struct mmc_queue *mq = &md->queue;
+	struct request *req;
+	int ret;
+
+	/* Ask the block layer about the card status */
+	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
+	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	blk_execute_rq(mq->queue, NULL, req, 0);
+	ret = req_to_mmc_queue_req(req)->drv_op_result;
+	if (ret >= 0) {
+		*val = ret;
+		ret = 0;
+	}
+
+	return ret;
+}
+DEFINE_SIMPLE_ATTRIBUTE(mmc_dbg_card_status_fops, mmc_dbg_card_status_get,
+		NULL, "%08llx\n");
+
+/* That is two digits * 512 + 1 for newline */
+#define EXT_CSD_STR_LEN 1025
+
+static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
+{
+	struct mmc_card *card = inode->i_private;
+	struct mmc_blk_data *md = dev_get_drvdata(&card->dev);
+	struct mmc_queue *mq = &md->queue;
+	struct request *req;
+	char *buf;
+	ssize_t n = 0;
+	u8 *ext_csd;
+	int err, i;
+
+	buf = kmalloc(EXT_CSD_STR_LEN + 1, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Ask the block layer for the EXT CSD */
+	req = blk_get_request(mq->queue, REQ_OP_DRV_IN, __GFP_RECLAIM);
+	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
+	blk_execute_rq(mq->queue, NULL, req, 0);
+	err = req_to_mmc_queue_req(req)->drv_op_result;
+	if (err) {
+		pr_err("FAILED %d\n", err);
+		goto out_free;
+	}
+
+	for (i = 0; i < 512; i++)
+		n += sprintf(buf + n, "%02x", ext_csd[i]);
+	n += sprintf(buf + n, "\n");
+
+	if (n != EXT_CSD_STR_LEN) {
+		err = -EINVAL;
+		goto out_free;
+	}
+
+	filp->private_data = buf;
+	kfree(ext_csd);
+	return 0;
+
+out_free:
+	kfree(buf);
+	return err;
+}
+
+static ssize_t mmc_ext_csd_read(struct file *filp, char __user *ubuf,
+				size_t cnt, loff_t *ppos)
+{
+	char *buf = filp->private_data;
+
+	return simple_read_from_buffer(ubuf, cnt, ppos,
+				       buf, EXT_CSD_STR_LEN);
+}
+
+static int mmc_ext_csd_release(struct inode *inode, struct file *file)
+{
+	kfree(file->private_data);
+	return 0;
+}
+
+static const struct file_operations mmc_dbg_ext_csd_fops = {
+	.open		= mmc_ext_csd_open,
+	.read		= mmc_ext_csd_read,
+	.release	= mmc_ext_csd_release,
+	.llseek		= default_llseek,
+};
+
+static int mmc_blk_add_debugfs(struct mmc_card *card)
+{
+	struct dentry *root;
+
+	if (!card->debugfs_root)
+		return 0;
+
+	root = card->debugfs_root;
+
+	if (mmc_card_mmc(card) || mmc_card_sd(card)) {
+		if (!debugfs_create_file("status", S_IRUSR, root, card,
+					 &mmc_dbg_card_status_fops))
+			return -EIO;
+	}
+
+	if (mmc_card_mmc(card)) {
+		if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
+					 &mmc_dbg_ext_csd_fops))
+			return -EIO;
+	}
+
+	return 0;
+}
+
+
+#else
+
+static int mmc_blk_add_debugfs(struct mmc_card *card)
+{
+	return 0;
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
 static int mmc_blk_probe(struct mmc_card *card)
 {
 	struct mmc_blk_data *md, *part_md;
@@ -2311,6 +2402,9 @@ static int mmc_blk_probe(struct mmc_card *card)
 			goto out;
 	}
 
+	/* Add two debugfs entries */
+	mmc_blk_add_debugfs(card);
+
 	pm_runtime_set_autosuspend_delay(&card->dev, 3000);
 	pm_runtime_use_autosuspend(&card->dev);
 
diff --git a/drivers/mmc/core/block.h b/drivers/mmc/core/block.h
index 377ebbf6a978..860ca7c8df86 100644
--- a/drivers/mmc/core/block.h
+++ b/drivers/mmc/core/block.h
@@ -5,7 +5,5 @@ struct mmc_queue;
 struct request;
 
 void mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req);
-int mmc_blk_card_status_get(struct mmc_card *card, u64 *val);
-int mmc_blk_get_ext_csd(struct mmc_card *card, u8 **ext_csd);
 
 #endif
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index cc1f7085111c..8583c2dc879e 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -282,99 +282,6 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
 	debugfs_remove_recursive(host->debugfs_root);
 }
 
-#if IS_ENABLED(CONFIG_MMC_BLOCK)
-
-static int mmc_dbg_card_status_get(void *data, u64 *val)
-{
-	return mmc_blk_card_status_get(data, val);
-}
-DEFINE_SIMPLE_ATTRIBUTE(mmc_dbg_card_status_fops, mmc_dbg_card_status_get,
-		NULL, "%08llx\n");
-
-#define EXT_CSD_STR_LEN 1025
-
-static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
-{
-	struct mmc_card *card = inode->i_private;
-	char *buf;
-	ssize_t n = 0;
-	u8 *ext_csd;
-	int err, i;
-
-	buf = kmalloc(EXT_CSD_STR_LEN + 1, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	err = mmc_blk_get_ext_csd(card, &ext_csd);
-	if (err)
-		goto out_free;
-
-	for (i = 0; i < 512; i++)
-		n += sprintf(buf + n, "%02x", ext_csd[i]);
-	n += sprintf(buf + n, "\n");
-
-	if (n != EXT_CSD_STR_LEN) {
-		err = -EINVAL;
-		goto out_free;
-	}
-
-	filp->private_data = buf;
-	kfree(ext_csd);
-	return 0;
-
-out_free:
-	kfree(buf);
-	return err;
-}
-
-static ssize_t mmc_ext_csd_read(struct file *filp, char __user *ubuf,
-				size_t cnt, loff_t *ppos)
-{
-	char *buf = filp->private_data;
-
-	return simple_read_from_buffer(ubuf, cnt, ppos,
-				       buf, EXT_CSD_STR_LEN);
-}
-
-static int mmc_ext_csd_release(struct inode *inode, struct file *file)
-{
-	kfree(file->private_data);
-	return 0;
-}
-
-static const struct file_operations mmc_dbg_ext_csd_fops = {
-	.open		= mmc_ext_csd_open,
-	.read		= mmc_ext_csd_read,
-	.release	= mmc_ext_csd_release,
-	.llseek		= default_llseek,
-};
-
-static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
-{
-	if (mmc_card_mmc(card) || mmc_card_sd(card)) {
-		if (!debugfs_create_file("status", S_IRUSR, root, card,
-					 &mmc_dbg_card_status_fops))
-			return -EIO;
-	}
-
-	if (mmc_card_mmc(card)) {
-		if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
-					 &mmc_dbg_ext_csd_fops))
-			return -EIO;
-	}
-
-	return 0;
-}
-
-#else /* !IS_ENABLED(CONFIG_MMC_BLOCK) */
-
-static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
-{
-	return 0;
-}
-
-#endif
-
 void mmc_add_card_debugfs(struct mmc_card *card)
 {
 	struct mmc_host	*host = card->host;
@@ -397,9 +304,6 @@ void mmc_add_card_debugfs(struct mmc_card *card)
 	if (!debugfs_create_x32("state", S_IRUSR, root, &card->state))
 		goto err;
 
-	if (mmc_add_block_debugfs(card, root))
-		goto err;
-
 	return;
 
 err:
-- 
2.9.4

Re: [PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Ulf Hansson]

On 8 June 2017 at 10:53, Linus Walleij <linus.walleij@linaro.org> wrote:
> If we don't have the block layer enabled, we do not present card
> status and extcsd in the debugfs.
>
> Debugfs is not ABI, and maintaining files of no relevance for
> non-block devices comes at a high maintenance cost if we shall
> support it with the block layer compiled out.

Well, as a matter of fact the code is still there after these changes,
just in a different form.

I would rather set the arguments to why these changes are good, that
we want to minimize the number of users of the bif fat mmc lock, but
also to deal with mmc request through the regular I/O path as to avoid
starvation.

>
> The expected number of debugfs users utilizing these two
> debugfs files is already low as there is an ioctl() to get the
> same information using the mmc-tools, and of these few users
> the expected number of people using it on SDIO or combo cards
> are expected to be zero.

Yeah. I can have been thinking a bit more about this. So, perhaps this
is good first step, to not entirely remove the current debugfs nodes
for cards.

However, from maintenance point of view, this series doesn't really
make me more happy, rather the opposite. Some more comments below.

>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - No changes just resending
> ---
>  drivers/mmc/core/debugfs.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index a1fba5732d66..b176932b8092 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -281,6 +281,8 @@ void mmc_remove_host_debugfs(struct mmc_host *host)
>         debugfs_remove_recursive(host->debugfs_root);
>  }
>
> +#if IS_ENABLED(CONFIG_MMC_BLOCK)
> +
>  static int mmc_dbg_card_status_get(void *data, u64 *val)
>  {
>         struct mmc_card *card = data;
> @@ -360,6 +362,32 @@ static const struct file_operations mmc_dbg_ext_csd_fops = {
>         .llseek         = default_llseek,
>  };
>
> +static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
> +{
> +       if (mmc_card_mmc(card) || mmc_card_sd(card)) {
> +               if (!debugfs_create_file("status", S_IRUSR, root, card,
> +                                        &mmc_dbg_card_status_fops))
> +                       return -EIO;
> +       }
> +
> +       if (mmc_card_mmc(card)) {
> +               if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
> +                                        &mmc_dbg_ext_csd_fops))
> +                       return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
> +#else /* !IS_ENABLED(CONFIG_MMC_BLOCK) */
> +
> +static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
> +{

So does this really work as expected?

Currently one can build the mmc block as a separate module. If that
module is inserted after the mmc core module attempts to register the
debugfs nodes, we would end up in this stub function, right?

In other words, those system that for some odd reason decide to insmod
the mmc block module at a later point, won't be given the debugfs
nodes?

> +       return 0;
> +}
> +
> +#endif
> +
>  void mmc_add_card_debugfs(struct mmc_card *card)
>  {
>         struct mmc_host *host = card->host;
> @@ -382,15 +410,8 @@ void mmc_add_card_debugfs(struct mmc_card *card)
>         if (!debugfs_create_x32("state", S_IRUSR, root, &card->state))
>                 goto err;
>
> -       if (mmc_card_mmc(card) || mmc_card_sd(card))
> -               if (!debugfs_create_file("status", S_IRUSR, root, card,
> -                                       &mmc_dbg_card_status_fops))
> -                       goto err;
> -
> -       if (mmc_card_mmc(card))
> -               if (!debugfs_create_file("ext_csd", S_IRUSR, root, card,
> -                                       &mmc_dbg_ext_csd_fops))
> -                       goto err;
> +       if (mmc_add_block_debugfs(card, root))
> +               goto err;
>
>         return;
>
> --
> 2.9.4
>

Kind regards
Uffe

Re: [PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Linus Walleij]

On Fri, Jun 16, 2017 at 11:32 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 8 June 2017 at 10:53, Linus Walleij <linus.walleij@linaro.org> wrote:

>> +#else /* !IS_ENABLED(CONFIG_MMC_BLOCK) */
>> +
>> +static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
>> +{
>
> So does this really work as expected?
>
> Currently one can build the mmc block as a separate module. If that
> module is inserted after the mmc core module attempts to register the
> debugfs nodes, we would end up in this stub function, right?

Not really. IS_ENABLED() is a compile-time flag. The stub is not even
compiled in if the block layer is selected as module.

I.e. if CONFIG_MMC_BLOCK is "y" or "m" then the real function gets
compiled in. If it is "n" the stub gets compiled in.

> In other words, those system that for some odd reason decide to insmod
> the mmc block module at a later point, won't be given the debugfs
> nodes?

This is not really related, since I have all the debugfs files for the
block layer as static functions in this patch, i.e. if the block layer
is inserted, the debugfs files come up.

On a broader note, AFAIK it is already impossible to insert the block
module after the core module (if it was selected as "m" at compile time),
as the block module must be inserted first, or the core module cannot
resolve the symbols it needs from the block module.

insmod will fail, and modprobe will load the required modules first,
so it orders the block layer to load before the core so that the symbols
are resolved.

Yours,
Linus Walleij

Re: [PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Ulf Hansson]

On 19 June 2017 at 11:25, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jun 16, 2017 at 11:32 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 8 June 2017 at 10:53, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>> +#else /* !IS_ENABLED(CONFIG_MMC_BLOCK) */
>>> +
>>> +static int mmc_add_block_debugfs(struct mmc_card *card, struct dentry *root)
>>> +{
>>
>> So does this really work as expected?
>>
>> Currently one can build the mmc block as a separate module. If that
>> module is inserted after the mmc core module attempts to register the
>> debugfs nodes, we would end up in this stub function, right?
>
> Not really. IS_ENABLED() is a compile-time flag. The stub is not even
> compiled in if the block layer is selected as module.
>
> I.e. if CONFIG_MMC_BLOCK is "y" or "m" then the real function gets
> compiled in. If it is "n" the stub gets compiled in.

Okay.

>
>> In other words, those system that for some odd reason decide to insmod
>> the mmc block module at a later point, won't be given the debugfs
>> nodes?
>
> This is not really related, since I have all the debugfs files for the
> block layer as static functions in this patch, i.e. if the block layer
> is inserted, the debugfs files come up.

If that would be the case, the I am fine. But, I am not sure. See comment below.

>
> On a broader note, AFAIK it is already impossible to insert the block
> module after the core module (if it was selected as "m" at compile time),
> as the block module must be inserted first, or the core module cannot
> resolve the symbols it needs from the block module.

No, this is wrong. It's actually the opposite.

Currently there are no dependency from the core module on the block
module. However, the block module uses exported functions from the
core module, so the dependency is the opposite.

This is exactly what I was trying to say, we must not create a
circular dependency of the modules.

To solve this, you must move the entire creation of the debugfs nodes
to be done when the block device driver probes, in other words from
mmc_blk_probe().

>
> insmod will fail, and modprobe will load the required modules first,
> so it orders the block layer to load before the core so that the symbols
> are resolved.
>
> Yours,
> Linus Walleij

Kind regards
Uffe

Re: [PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Linus Walleij]

On Mon, Jun 19, 2017 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 19 June 2017 at 11:25, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This is not really related, since I have all the debugfs files for the
>> block layer as static functions in this patch, i.e. if the block layer
>> is inserted, the debugfs files come up.
>
> If that would be the case, the I am fine. But, I am not sure. See comment below.
>
>> On a broader note, AFAIK it is already impossible to insert the block
>> module after the core module (if it was selected as "m" at compile time),
>> as the block module must be inserted first, or the core module cannot
>> resolve the symbols it needs from the block module.
>
> No, this is wrong. It's actually the opposite.
>
> Currently there are no dependency from the core module on the block
> module. However, the block module uses exported functions from the
> core module, so the dependency is the opposite.

Aha, I see how you mean.

> This is exactly what I was trying to say, we must not create a
> circular dependency of the modules.

I think modprobe resolves circular dependencies by inserting
both modules at the same time, actually. insmod will not work.

> To solve this, you must move the entire creation of the debugfs nodes
> to be done when the block device driver probes, in other words from
> mmc_blk_probe().

Yes and this is done in patch 6/6.

If you think it is dangerous to have this as an intermediary step, we
can just squash the patches.

Yours,
Linus Walleij

Re: [PATCH 2/6 v3] mmc: debugfs: No blocklayer = no card status and extcsd

[Ulf Hansson]

On 19 June 2017 at 13:14, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jun 19, 2017 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 19 June 2017 at 11:25, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>>> This is not really related, since I have all the debugfs files for the
>>> block layer as static functions in this patch, i.e. if the block layer
>>> is inserted, the debugfs files come up.
>>
>> If that would be the case, the I am fine. But, I am not sure. See comment below.
>>
>>> On a broader note, AFAIK it is already impossible to insert the block
>>> module after the core module (if it was selected as "m" at compile time),
>>> as the block module must be inserted first, or the core module cannot
>>> resolve the symbols it needs from the block module.
>>
>> No, this is wrong. It's actually the opposite.
>>
>> Currently there are no dependency from the core module on the block
>> module. However, the block module uses exported functions from the
>> core module, so the dependency is the opposite.
>
> Aha, I see how you mean.
>
>> This is exactly what I was trying to say, we must not create a
>> circular dependency of the modules.
>
> I think modprobe resolves circular dependencies by inserting
> both modules at the same time, actually. insmod will not work.
>
>> To solve this, you must move the entire creation of the debugfs nodes
>> to be done when the block device driver probes, in other words from
>> mmc_blk_probe().
>
> Yes and this is done in patch 6/6.
>
> If you think it is dangerous to have this as an intermediary step, we
> can just squash the patches.

Yes, please do that!

Kind regards
Uffe