[RFC PATCH 0/8] Attempt to clean up BUG_ONs for mmc-tree

[RFC PATCH 0/8] Attempt to clean up BUG_ONs for mmc-tree

[Shawn Lin]

We have already removed some BUG_ONs recently[0][1], however there
are still many pointless BUG_ONs for mmc core. So this patchset is
going on removing these BUG_ONs from drivers/mmc/core/* and drivers/mmc/card/*.
It is also possible for us to remove it from drivers/mmc/host/* in the future.

No functional changes intented in theory, except for adding some error
handle, but I held it for a while in order to test it fully and decided
to send it after the merge window of 4.9.

Please review and comment. :)

[0]: https://patchwork.kernel.org/patch/9300739/
[1]: https://patchwork.kernel.org/patch/9300741/



Shawn Lin (8):
  mmc: core: remove BUG_ONs from sdio
  mmc: debugfs: remove BUG_ON from mmc_ext_csd_open
  mmc: core: remove BUG_ONs from mmc
  mmc: core: remove BUG_ONs from sd
  mmc: core: remove BUG_ONs from core.c
  mmc: sdio_uart: remove meaningless BUG_ON
  mmc: queue: remove BUG_ON for bounce_sg
  mmc: mmc_test: remove BUG_ONs and deploy error handling

 drivers/mmc/card/mmc_test.c  | 12 ++++++++----
 drivers/mmc/card/queue.c     |  2 --
 drivers/mmc/card/sdio_uart.c |  2 --
 drivers/mmc/core/core.c      | 40 ++++++++++++++--------------------------
 drivers/mmc/core/debugfs.c   |  6 +++++-
 drivers/mmc/core/mmc.c       | 24 ++++++++++++++----------
 drivers/mmc/core/mmc_ops.c   | 20 ++++++--------------
 drivers/mmc/core/sd.c        | 20 +++++++++-----------
 drivers/mmc/core/sd_ops.c    | 30 ++++++++++--------------------
 drivers/mmc/core/sdio.c      | 26 ++++++++++++++------------
 drivers/mmc/core/sdio_cis.c  |  3 ++-
 drivers/mmc/core/sdio_irq.c  | 12 +++++++-----
 12 files changed, 89 insertions(+), 108 deletions(-)

-- 
2.3.7

[RFC PATCH 1/8] mmc: core: remove BUG_ONs from sdio

[Shawn Lin]

BUG_ONs doesn't help anything except for stop the system from
running. If it occurs, it implies we should deploy proper error
handling for that. So this patch is gonna discard these meaningless
BUG_ONs and deploy error handling if needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/core/sdio.c     | 26 ++++++++++++++------------
 drivers/mmc/core/sdio_cis.c |  3 ++-
 drivers/mmc/core/sdio_irq.c | 12 +++++++-----
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index bd44ba8..80883c6 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -63,7 +63,8 @@ static int sdio_init_func(struct mmc_card *card, unsigned int fn)
 	int ret;
 	struct sdio_func *func;
 
-	BUG_ON(fn > SDIO_MAX_FUNCS);
+	if (WARN_ON(fn > SDIO_MAX_FUNCS))
+		return -EINVAL;
 
 	func = sdio_alloc_func(card);
 	if (IS_ERR(func))
@@ -555,7 +556,9 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	u32 rocr = 0;
 	u32 ocr_card = ocr;
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
+
 	WARN_ON(!host->claimed);
 
 	/* to query card if 1.8V signalling is supported */
@@ -791,8 +794,8 @@ static void mmc_sdio_remove(struct mmc_host *host)
 {
 	int i;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return;
 
 	for (i = 0;i < host->card->sdio_funcs;i++) {
 		if (host->card->sdio_func[i]) {
@@ -820,8 +823,8 @@ static void mmc_sdio_detect(struct mmc_host *host)
 {
 	int err;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (WARN_ON(!host))
+		goto out;
 
 	/* Make sure card is powered before detecting it */
 	if (host->caps & MMC_CAP_POWER_OFF_CARD) {
@@ -916,8 +919,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
 {
 	int err = 0;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return -EINVAL;
 
 	/* Basic card reinitialization. */
 	mmc_claim_host(host);
@@ -970,9 +973,6 @@ static int mmc_sdio_power_restore(struct mmc_host *host)
 {
 	int ret;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
-
 	mmc_claim_host(host);
 
 	/*
@@ -1063,7 +1063,9 @@ int mmc_attach_sdio(struct mmc_host *host)
 	u32 ocr, rocr;
 	struct mmc_card *card;
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
+
 	WARN_ON(!host->claimed);
 
 	err = mmc_send_io_op_cond(host, 0, &ocr);
diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
index dcb3dee..f8c3728 100644
--- a/drivers/mmc/core/sdio_cis.c
+++ b/drivers/mmc/core/sdio_cis.c
@@ -262,7 +262,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func)
 	else
 		prev = &card->tuples;
 
-	BUG_ON(*prev);
+	if (*prev)
+		return -EINVAL;
 
 	do {
 		unsigned char tpl_code, tpl_link;
diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
index 91bbbfb..f1faf9a 100644
--- a/drivers/mmc/core/sdio_irq.c
+++ b/drivers/mmc/core/sdio_irq.c
@@ -214,7 +214,9 @@ static int sdio_card_irq_put(struct mmc_card *card)
 	struct mmc_host *host = card->host;
 
 	WARN_ON(!host->claimed);
-	BUG_ON(host->sdio_irqs < 1);
+
+	if (host->sdio_irqs < 1)
+		return -EINVAL;
 
 	if (!--host->sdio_irqs) {
 		if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
@@ -261,8 +263,8 @@ int sdio_claim_irq(struct sdio_func *func, sdio_irq_handler_t *handler)
 	int ret;
 	unsigned char reg;
 
-	BUG_ON(!func);
-	BUG_ON(!func->card);
+	if (!func)
+		return -EINVAL;
 
 	pr_debug("SDIO: Enabling IRQ for %s...\n", sdio_func_id(func));
 
@@ -304,8 +306,8 @@ int sdio_release_irq(struct sdio_func *func)
 	int ret;
 	unsigned char reg;
 
-	BUG_ON(!func);
-	BUG_ON(!func->card);
+	if (!func)
+		return -EINVAL;
 
 	pr_debug("SDIO: Disabling IRQ for %s...\n", sdio_func_id(func));
 
-- 
2.3.7

[RFC PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open

[Shawn Lin]

Return error value for file_operations callback instead
of triggering BUG_ON which is meaningless. Personally I
don't believe n != EXT_CSD_STR_LEN could happen. Anyway,
propagate the error to the caller.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/debugfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index c8451ce..30623b8 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -321,7 +321,11 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 	for (i = 0; i < 512; i++)
 		n += sprintf(buf + n, "%02x", ext_csd[i]);
 	n += sprintf(buf + n, "\n");
-	BUG_ON(n != EXT_CSD_STR_LEN);
+
+	if (n != EXT_CSD_STR_LEN) {
+		err = -EINVAL;
+		goto out_free;
+	}
 
 	filp->private_data = buf;
 	kfree(ext_csd);
-- 
2.3.7

[RFC PATCH 3/8] mmc: core: remove BUG_ONs from mmc

[Shawn Lin]

BUG_ONs doesn't help anything except for stop the system from
running. If it occurs, it implies we should deploy proper error
handling for that. So this patch is gonna discard these meaningless
BUG_ONs and deploy error handling if needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/mmc.c     | 24 ++++++++++++++----------
 drivers/mmc/core/mmc_ops.c | 20 ++++++--------------
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 39fc5b2..8f63b59 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1477,7 +1477,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	u32 cid[4];
 	u32 rocr;
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
+
 	WARN_ON(!host->claimed);
 
 	/* Set correct bus mode for MMC before attempting init */
@@ -1867,8 +1869,8 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
  */
 static void mmc_remove(struct mmc_host *host)
 {
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return;
 
 	mmc_remove_card(host->card);
 	host->card = NULL;
@@ -1889,8 +1891,8 @@ static void mmc_detect(struct mmc_host *host)
 {
 	int err;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (WARN_ON(!host))
+		return;
 
 	mmc_get_card(host->card);
 
@@ -1917,8 +1919,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 	unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
 					EXT_CSD_POWER_OFF_LONG;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (WARN_ON(!host))
+		return -EINVAL;
 
 	mmc_claim_host(host);
 
@@ -1976,8 +1978,8 @@ static int _mmc_resume(struct mmc_host *host)
 {
 	int err = 0;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (WARN_ON(!host))
+		return -EINVAL;
 
 	mmc_claim_host(host);
 
@@ -2111,7 +2113,9 @@ int mmc_attach_mmc(struct mmc_host *host)
 	int err;
 	u32 ocr, rocr;
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
+
 	WARN_ON(!host->claimed);
 
 	/* Set correct bus mode for MMC before attempting attach */
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index ad6e979..ceb41eb 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -60,9 +60,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = MMC_SEND_STATUS;
 	if (!mmc_host_is_spi(card->host))
 		cmd.arg = card->rca << 16;
@@ -92,7 +89,8 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
 {
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
 
 	cmd.opcode = MMC_SELECT_CARD;
 
@@ -109,7 +107,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
 
 int mmc_select_card(struct mmc_card *card)
 {
-	BUG_ON(!card);
 
 	return _mmc_select_card(card->host, card);
 }
@@ -181,8 +178,6 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 	struct mmc_command cmd = {0};
 	int i, err = 0;
 
-	BUG_ON(!host);
-
 	cmd.opcode = MMC_SEND_OP_COND;
 	cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
 	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;
@@ -221,8 +216,8 @@ int mmc_all_send_cid(struct mmc_host *host, u32 *cid)
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!host);
-	BUG_ON(!cid);
+	if (!cid)
+		return -ENOMEM;
 
 	cmd.opcode = MMC_ALL_SEND_CID;
 	cmd.arg = 0;
@@ -241,9 +236,6 @@ int mmc_set_relative_addr(struct mmc_card *card)
 {
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = MMC_SET_RELATIVE_ADDR;
 	cmd.arg = card->rca << 16;
 	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
@@ -257,8 +249,8 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode)
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!host);
-	BUG_ON(!cxd);
+	if (!cxd)
+		return -ENOMEM;
 
 	cmd.opcode = opcode;
 	cmd.arg = arg;
-- 
2.3.7

[RFC PATCH 4/8] mmc: core: remove BUG_ONs from sd

[Shawn Lin]

BUG_ONs doesn't help anything except for stop the system from
running. If it occurs, it implies we should deploy proper error
handling for that. So this patch is gonna discard these meaningless
BUG_ONs and deploy error handling if needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/sd.c     | 20 +++++++++-----------
 drivers/mmc/core/sd_ops.c | 30 ++++++++++--------------------
 2 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 73c762a..bd3ddcc 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -927,7 +927,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 	u32 cid[4];
 	u32 rocr = 0;
 
-	BUG_ON(!host);
 	WARN_ON(!host->claimed);
 
 	err = mmc_sd_get_cid(host, ocr, cid, &rocr);
@@ -1043,9 +1042,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
  */
 static void mmc_sd_remove(struct mmc_host *host)
 {
-	BUG_ON(!host);
-	BUG_ON(!host->card);
-
 	mmc_remove_card(host->card);
 	host->card = NULL;
 }
@@ -1065,8 +1061,8 @@ static void mmc_sd_detect(struct mmc_host *host)
 {
 	int err;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return;
 
 	mmc_get_card(host->card);
 
@@ -1091,8 +1087,8 @@ static int _mmc_sd_suspend(struct mmc_host *host)
 {
 	int err = 0;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return -EINVAL;
 
 	mmc_claim_host(host);
 
@@ -1136,8 +1132,8 @@ static int _mmc_sd_resume(struct mmc_host *host)
 {
 	int err = 0;
 
-	BUG_ON(!host);
-	BUG_ON(!host->card);
+	if (!host)
+		return -EINVAL;
 
 	mmc_claim_host(host);
 
@@ -1221,7 +1217,9 @@ int mmc_attach_sd(struct mmc_host *host)
 	int err;
 	u32 ocr, rocr;
 
-	BUG_ON(!host);
+	if (!host)
+		return -EINVAL;
+
 	WARN_ON(!host->claimed);
 
 	err = mmc_send_app_op_cond(host, 0, &ocr);
diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
index 16b774c..a2cc157 100644
--- a/drivers/mmc/core/sd_ops.c
+++ b/drivers/mmc/core/sd_ops.c
@@ -27,8 +27,8 @@ int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card)
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!host);
-	BUG_ON(card && (card->host != host));
+	if (WARN_ON(card && card->host != host))
+		return -EINVAL;
 
 	cmd.opcode = MMC_APP_CMD;
 
@@ -72,8 +72,8 @@ int mmc_wait_for_app_cmd(struct mmc_host *host, struct mmc_card *card,
 
 	int i, err;
 
-	BUG_ON(!cmd);
-	BUG_ON(retries < 0);
+	if (retries < 0)
+		retries = MMC_CMD_RETRIES;
 
 	err = -EIO;
 
@@ -122,9 +122,6 @@ int mmc_app_set_bus_width(struct mmc_card *card, int width)
 {
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	cmd.opcode = SD_APP_SET_BUS_WIDTH;
 	cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
 
@@ -147,8 +144,6 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
 	struct mmc_command cmd = {0};
 	int i, err = 0;
 
-	BUG_ON(!host);
-
 	cmd.opcode = SD_APP_OP_COND;
 	if (mmc_host_is_spi(host))
 		cmd.arg = ocr & (1 << 30); /* SPI only defines one bit */
@@ -224,8 +219,8 @@ int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca)
 	int err;
 	struct mmc_command cmd = {0};
 
-	BUG_ON(!host);
-	BUG_ON(!rca);
+	if (!rca)
+		return -ENOMEM;
 
 	cmd.opcode = SD_SEND_RELATIVE_ADDR;
 	cmd.arg = 0;
@@ -249,9 +244,8 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
 	struct scatterlist sg;
 	void *data_buf;
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-	BUG_ON(!scr);
+	if (!scr)
+		return -ENOMEM;
 
 	/* NOTE: caller guarantees scr is heap-allocated */
 
@@ -307,9 +301,6 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int group,
 	struct mmc_data data = {0};
 	struct scatterlist sg;
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-
 	/* NOTE: caller guarantees resp is heap-allocated */
 
 	mode = !!mode;
@@ -352,9 +343,8 @@ int mmc_app_sd_status(struct mmc_card *card, void *ssr)
 	struct mmc_data data = {0};
 	struct scatterlist sg;
 
-	BUG_ON(!card);
-	BUG_ON(!card->host);
-	BUG_ON(!ssr);
+	if (!ssr)
+		return -ENOMEM;
 
 	/* NOTE: caller guarantees ssr is heap-allocated */
 
-- 
2.3.7

[RFC PATCH 5/8] mmc: core: remove BUG_ONs from core.c

[Shawn Lin]

BUG_ONs doesn't help anything except for stop the system from
running. If it occurs, it implies we should deploy proper error
handling for that. So this patch is gonna discard these meaningless
BUG_ONs and deploy error handling if needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/core.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2553d90..d8b166b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -306,16 +306,16 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 		mrq->sbc->mrq = mrq;
 	}
 	if (mrq->data) {
-		BUG_ON(mrq->data->blksz > host->max_blk_size);
-		BUG_ON(mrq->data->blocks > host->max_blk_count);
-		BUG_ON(mrq->data->blocks * mrq->data->blksz >
-			host->max_req_size);
-
+		if (mrq->data->blksz > host->max_blk_size ||
+		    mrq->data->blocks > host->max_blk_count ||
+		    mrq->data->blocks * mrq->data->blksz > host->max_req_size)
+			return -EINVAL;
 #ifdef CONFIG_MMC_DEBUG
 		sz = 0;
 		for_each_sg(mrq->data->sg, sg, mrq->data->sg_len, i)
 			sz += sg->length;
-		BUG_ON(sz != mrq->data->blocks * mrq->data->blksz);
+		if (sz != mrq->data->blocks * mrq->data->blksz)
+			return -EINVAL;
 #endif
 
 		mrq->cmd->data = mrq->data;
@@ -349,9 +349,8 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
 	int timeout;
 	bool use_busy_signal;
 
-	BUG_ON(!card);
-
-	if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card))
+	if (!card || !card->ext_csd.man_bkops_en ||
+	    mmc_card_doing_bkops(card))
 		return;
 
 	err = mmc_read_bkops_status(card);
@@ -754,8 +753,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
 	u32 status;
 	unsigned long prg_wait;
 
-	BUG_ON(!card);
-
 	if (!card->ext_csd.hpi_en) {
 		pr_info("%s: HPI enable bit unset\n", mmc_hostname(card->host));
 		return 1;
@@ -850,7 +847,9 @@ int mmc_stop_bkops(struct mmc_card *card)
 {
 	int err = 0;
 
-	BUG_ON(!card);
+	if (!card)
+		return -EINVAL;
+
 	err = mmc_interrupt_hpi(card);
 
 	/*
@@ -1666,8 +1665,6 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, u32 ocr)
 	int err = 0;
 	u32 clock;
 
-	BUG_ON(!host);
-
 	/*
 	 * Send CMD11 only if the request is to switch the card to
 	 * 1.8V signalling.
@@ -1884,9 +1881,7 @@ void mmc_power_cycle(struct mmc_host *host, u32 ocr)
  */
 static void __mmc_release_bus(struct mmc_host *host)
 {
-	BUG_ON(!host);
-	BUG_ON(host->bus_refs);
-	BUG_ON(!host->bus_dead);
+	WARN_ON(!host->bus_dead);
 
 	host->bus_ops = NULL;
 }
@@ -1926,15 +1921,12 @@ void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops)
 {
 	unsigned long flags;
 
-	BUG_ON(!host);
-	BUG_ON(!ops);
-
 	WARN_ON(!host->claimed);
 
 	spin_lock_irqsave(&host->lock, flags);
 
-	BUG_ON(host->bus_ops);
-	BUG_ON(host->bus_refs);
+	WARN_ON(host->bus_ops);
+	WARN_ON(host->bus_refs);
 
 	host->bus_ops = ops;
 	host->bus_refs = 1;
@@ -1950,8 +1942,6 @@ void mmc_detach_bus(struct mmc_host *host)
 {
 	unsigned long flags;
 
-	BUG_ON(!host);
-
 	WARN_ON(!host->claimed);
 	WARN_ON(!host->bus_ops);
 
@@ -2865,8 +2855,6 @@ void mmc_stop_host(struct mmc_host *host)
 	}
 	mmc_bus_put(host);
 
-	BUG_ON(host->card);
-
 	mmc_claim_host(host);
 	mmc_power_off(host);
 	mmc_release_host(host);
-- 
2.3.7

[RFC PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON

[Shawn Lin]

The code seems quite simple to maintain the sdio_uart_table,
and the insert/remove port from the table are symmetric. If
the BUG_ON occurs, which means serial_core modify the index
or mess up the port sequence anyway.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/card/sdio_uart.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/card/sdio_uart.c b/drivers/mmc/card/sdio_uart.c
index 5af6fb9..491c187 100644
--- a/drivers/mmc/card/sdio_uart.c
+++ b/drivers/mmc/card/sdio_uart.c
@@ -135,8 +135,6 @@ static void sdio_uart_port_remove(struct sdio_uart_port *port)
 {
 	struct sdio_func *func;
 
-	BUG_ON(sdio_uart_table[port->index] != port);
-
 	spin_lock(&sdio_uart_table_lock);
 	sdio_uart_table[port->index] = NULL;
 	spin_unlock(&sdio_uart_table_lock);
-- 
2.3.7

[RFC PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg

[Shawn Lin]

bounce_sg for mqrq_cur and mqrq_pre are proper
allocated when initializing the queue and will not
be freed before explicitly cleaning the queue. So from
the code itself it should be quite confident to remove
this check. If that BUG_ON take effects, it is mostly
likely the memory is randomly oopsing.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/card/queue.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 8037f73..6c8978a 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -505,8 +505,6 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
 			return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
 	}
 
-	BUG_ON(!mqrq->bounce_sg);
-
 	if (mmc_packed_cmd(cmd_type))
 		sg_len = mmc_queue_packed_map_sg(mq, mqrq->packed,
 						 mqrq->bounce_sg, cmd_type);
-- 
2.3.7

[RFC PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling

[Shawn Lin]

It is unnecessary to panic the kernel when testing mmc. Instead,
cast a warning for folkz to debug and return the error code to
the caller to indicate the failure of this test should be enough.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/card/mmc_test.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index 5a8dc5a..db1a7ac 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -214,7 +214,8 @@ static void mmc_test_prepare_mrq(struct mmc_test_card *test,
 	struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
 	unsigned dev_addr, unsigned blocks, unsigned blksz, int write)
 {
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop))
+		return;
 
 	if (blocks > 1) {
 		mrq->cmd->opcode = write ?
@@ -694,7 +695,8 @@ static int mmc_test_cleanup(struct mmc_test_card *test)
 static void mmc_test_prepare_broken_mrq(struct mmc_test_card *test,
 	struct mmc_request *mrq, int write)
 {
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return;
 
 	if (mrq->data->blocks > 1) {
 		mrq->cmd->opcode = write ?
@@ -714,7 +716,8 @@ static int mmc_test_check_result(struct mmc_test_card *test,
 {
 	int ret;
 
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return -EINVAL;
 
 	ret = 0;
 
@@ -755,7 +758,8 @@ static int mmc_test_check_broken_result(struct mmc_test_card *test,
 {
 	int ret;
 
-	BUG_ON(!mrq || !mrq->cmd || !mrq->data);
+	if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
+		return -EINVAL;
 
 	ret = 0;
 
-- 
2.3.7

Re: [RFC PATCH 1/8] mmc: core: remove BUG_ONs from sdio

[Ulf Hansson]

On 18 October 2016 at 14:03, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
>  drivers/mmc/core/sdio.c     | 26 ++++++++++++++------------
>  drivers/mmc/core/sdio_cis.c |  3 ++-
>  drivers/mmc/core/sdio_irq.c | 12 +++++++-----
>  3 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index bd44ba8..80883c6 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -63,7 +63,8 @@ static int sdio_init_func(struct mmc_card *card, unsigned int fn)
>         int ret;
>         struct sdio_func *func;
>
> -       BUG_ON(fn > SDIO_MAX_FUNCS);
> +       if (WARN_ON(fn > SDIO_MAX_FUNCS))
> +               return -EINVAL;
>
>         func = sdio_alloc_func(card);
>         if (IS_ERR(func))
> @@ -555,7 +556,9 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>         u32 rocr = 0;
>         u32 ocr_card = ocr;
>
> -       BUG_ON(!host);
> +       if (!host)
> +               return -EINVAL;
> +

Just remove this altogether. We always have a host here and the
function is internal to sdio.c

>         WARN_ON(!host->claimed);
>
>         /* to query card if 1.8V signalling is supported */
> @@ -791,8 +794,8 @@ static void mmc_sdio_remove(struct mmc_host *host)
>  {
>         int i;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (!host)
> +               return;
>

Ditto.

>         for (i = 0;i < host->card->sdio_funcs;i++) {
>                 if (host->card->sdio_func[i]) {
> @@ -820,8 +823,8 @@ static void mmc_sdio_detect(struct mmc_host *host)
>  {
>         int err;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (WARN_ON(!host))
> +               goto out;

Ditto.

>
>         /* Make sure card is powered before detecting it */
>         if (host->caps & MMC_CAP_POWER_OFF_CARD) {
> @@ -916,8 +919,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
>  {
>         int err = 0;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (!host)
> +               return -EINVAL;
>

Ditto.

>         /* Basic card reinitialization. */
>         mmc_claim_host(host);
> @@ -970,9 +973,6 @@ static int mmc_sdio_power_restore(struct mmc_host *host)
>  {
>         int ret;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> -
>         mmc_claim_host(host);
>
>         /*
> @@ -1063,7 +1063,9 @@ int mmc_attach_sdio(struct mmc_host *host)
>         u32 ocr, rocr;
>         struct mmc_card *card;
>
> -       BUG_ON(!host);
> +       if (!host)
> +               return -EINVAL;
> +

Ditto.

>         WARN_ON(!host->claimed);
>
>         err = mmc_send_io_op_cond(host, 0, &ocr);
> diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> index dcb3dee..f8c3728 100644
> --- a/drivers/mmc/core/sdio_cis.c
> +++ b/drivers/mmc/core/sdio_cis.c
> @@ -262,7 +262,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func)
>         else
>                 prev = &card->tuples;
>
> -       BUG_ON(*prev);
> +       if (*prev)
> +               return -EINVAL;
>
>         do {
>                 unsigned char tpl_code, tpl_link;
> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> index 91bbbfb..f1faf9a 100644
> --- a/drivers/mmc/core/sdio_irq.c
> +++ b/drivers/mmc/core/sdio_irq.c
> @@ -214,7 +214,9 @@ static int sdio_card_irq_put(struct mmc_card *card)
>         struct mmc_host *host = card->host;
>
>         WARN_ON(!host->claimed);
> -       BUG_ON(host->sdio_irqs < 1);
> +
> +       if (host->sdio_irqs < 1)
> +               return -EINVAL;
>
>         if (!--host->sdio_irqs) {
>                 if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
> @@ -261,8 +263,8 @@ int sdio_claim_irq(struct sdio_func *func, sdio_irq_handler_t *handler)
>         int ret;
>         unsigned char reg;
>
> -       BUG_ON(!func);
> -       BUG_ON(!func->card);
> +       if (!func)
> +               return -EINVAL;
>
>         pr_debug("SDIO: Enabling IRQ for %s...\n", sdio_func_id(func));
>
> @@ -304,8 +306,8 @@ int sdio_release_irq(struct sdio_func *func)
>         int ret;
>         unsigned char reg;
>
> -       BUG_ON(!func);
> -       BUG_ON(!func->card);
> +       if (!func)
> +               return -EINVAL;
>
>         pr_debug("SDIO: Disabling IRQ for %s...\n", sdio_func_id(func));
>
> --
> 2.3.7

Kind regards
Uffe

Re: [RFC PATCH 3/8] mmc: core: remove BUG_ONs from mmc

[Ulf Hansson]

On 18 October 2016 at 14:04, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
>  drivers/mmc/core/mmc.c     | 24 ++++++++++++++----------
>  drivers/mmc/core/mmc_ops.c | 20 ++++++--------------
>  2 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 39fc5b2..8f63b59 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1477,7 +1477,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>         u32 cid[4];
>         u32 rocr;
>
> -       BUG_ON(!host);
> +       if (!host)
> +               return -EINVAL;
> +

Not needed, just remove.

>         WARN_ON(!host->claimed);
>
>         /* Set correct bus mode for MMC before attempting init */
> @@ -1867,8 +1869,8 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
>   */
>  static void mmc_remove(struct mmc_host *host)
>  {
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (!host)
> +               return;
>

Ditto.

>         mmc_remove_card(host->card);
>         host->card = NULL;
> @@ -1889,8 +1891,8 @@ static void mmc_detect(struct mmc_host *host)
>  {
>         int err;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (WARN_ON(!host))
> +               return;
>

Ditto.

>         mmc_get_card(host->card);
>
> @@ -1917,8 +1919,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>         unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
>                                         EXT_CSD_POWER_OFF_LONG;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (WARN_ON(!host))
> +               return -EINVAL;
>

Ditto.

>         mmc_claim_host(host);
>
> @@ -1976,8 +1978,8 @@ static int _mmc_resume(struct mmc_host *host)
>  {
>         int err = 0;
>
> -       BUG_ON(!host);
> -       BUG_ON(!host->card);
> +       if (WARN_ON(!host))
> +               return -EINVAL;
>

Ditto.

>         mmc_claim_host(host);
>
> @@ -2111,7 +2113,9 @@ int mmc_attach_mmc(struct mmc_host *host)
>         int err;
>         u32 ocr, rocr;
>
> -       BUG_ON(!host);
> +       if (!host)
> +               return -EINVAL;
> +

Ditto.

>         WARN_ON(!host->claimed);
>
>         /* Set correct bus mode for MMC before attempting attach */
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index ad6e979..ceb41eb 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -60,9 +60,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
>         int err;
>         struct mmc_command cmd = {0};
>
> -       BUG_ON(!card);
> -       BUG_ON(!card->host);
> -
>         cmd.opcode = MMC_SEND_STATUS;
>         if (!mmc_host_is_spi(card->host))
>                 cmd.arg = card->rca << 16;
> @@ -92,7 +89,8 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
>  {
>         struct mmc_command cmd = {0};
>
> -       BUG_ON(!host);
> +       if (!host)
> +               return -EINVAL;
>

Ditto.

>         cmd.opcode = MMC_SELECT_CARD;
>
> @@ -109,7 +107,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
>
>  int mmc_select_card(struct mmc_card *card)
>  {
> -       BUG_ON(!card);
>
>         return _mmc_select_card(card->host, card);
>  }
> @@ -181,8 +178,6 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
>         struct mmc_command cmd = {0};
>         int i, err = 0;
>
> -       BUG_ON(!host);
> -
>         cmd.opcode = MMC_SEND_OP_COND;
>         cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
>         cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;
> @@ -221,8 +216,8 @@ int mmc_all_send_cid(struct mmc_host *host, u32 *cid)
>         int err;
>         struct mmc_command cmd = {0};
>
> -       BUG_ON(!host);
> -       BUG_ON(!cid);
> +       if (!cid)
> +               return -ENOMEM;

Remove as not needed, as mmc_all_send_cid() is internal to the mmc
core. We know all callers already provide the cid.

>
>         cmd.opcode = MMC_ALL_SEND_CID;
>         cmd.arg = 0;
> @@ -241,9 +236,6 @@ int mmc_set_relative_addr(struct mmc_card *card)
>  {
>         struct mmc_command cmd = {0};
>
> -       BUG_ON(!card);
> -       BUG_ON(!card->host);
> -
>         cmd.opcode = MMC_SET_RELATIVE_ADDR;
>         cmd.arg = card->rca << 16;
>         cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> @@ -257,8 +249,8 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode)
>         int err;
>         struct mmc_command cmd = {0};
>
> -       BUG_ON(!host);
> -       BUG_ON(!cxd);
> +       if (!cxd)
> +               return -ENOMEM;

For the same reasons as above, let's remove this.

>
>         cmd.opcode = opcode;
>         cmd.arg = arg;
> --

Kind regards
Uffe

Re: [RFC PATCH 0/8] Attempt to clean up BUG_ONs for mmc-tree

[Ulf Hansson]

On 18 October 2016 at 14:03, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> We have already removed some BUG_ONs recently[0][1], however there
> are still many pointless BUG_ONs for mmc core. So this patchset is
> going on removing these BUG_ONs from drivers/mmc/core/* and drivers/mmc/card/*.
> It is also possible for us to remove it from drivers/mmc/host/* in the future.
>
> No functional changes intented in theory, except for adding some error
> handle, but I held it for a while in order to test it fully and decided
> to send it after the merge window of 4.9.
>
> Please review and comment. :)

I have reviewed patch 1, patch 2 and patch 3. I think you can follow a
pattern for my review comment, which means the similar comments are
applicable the rest of series as well. Please have a look to see what
you think and perhaps re-spin this.

Thanks for working on this clean-up!

Kind regards
Uffe

>
> [0]: https://patchwork.kernel.org/patch/9300739/
> [1]: https://patchwork.kernel.org/patch/9300741/
>
>
>
> Shawn Lin (8):
>   mmc: core: remove BUG_ONs from sdio
>   mmc: debugfs: remove BUG_ON from mmc_ext_csd_open
>   mmc: core: remove BUG_ONs from mmc
>   mmc: core: remove BUG_ONs from sd
>   mmc: core: remove BUG_ONs from core.c
>   mmc: sdio_uart: remove meaningless BUG_ON
>   mmc: queue: remove BUG_ON for bounce_sg
>   mmc: mmc_test: remove BUG_ONs and deploy error handling
>
>  drivers/mmc/card/mmc_test.c  | 12 ++++++++----
>  drivers/mmc/card/queue.c     |  2 --
>  drivers/mmc/card/sdio_uart.c |  2 --
>  drivers/mmc/core/core.c      | 40 ++++++++++++++--------------------------
>  drivers/mmc/core/debugfs.c   |  6 +++++-
>  drivers/mmc/core/mmc.c       | 24 ++++++++++++++----------
>  drivers/mmc/core/mmc_ops.c   | 20 ++++++--------------
>  drivers/mmc/core/sd.c        | 20 +++++++++-----------
>  drivers/mmc/core/sd_ops.c    | 30 ++++++++++--------------------
>  drivers/mmc/core/sdio.c      | 26 ++++++++++++++------------
>  drivers/mmc/core/sdio_cis.c  |  3 ++-
>  drivers/mmc/core/sdio_irq.c  | 12 +++++++-----
>  12 files changed, 89 insertions(+), 108 deletions(-)
>
> --
> 2.3.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html