* [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
@ 2016-11-02 7:24 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:25 ` [PATCH 3/8] mmc: core: remove BUG_ONs from mmc Shawn Lin
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:24 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open
2016-11-02 7:24 ` [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:24, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> 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
>
>
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/8] mmc: core: remove BUG_ONs from mmc
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
2016-11-02 7:24 ` [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open Shawn Lin
@ 2016-11-02 7:25 ` Shawn Lin
2016-11-02 9:40 ` Venu Byravarasu
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:25 ` [PATCH 4/8] mmc: core: remove BUG_ONs from sd Shawn Lin
` (5 subsequent siblings)
7 siblings, 2 replies; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:25 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-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 | 14 --------------
drivers/mmc/core/mmc_ops.c | 17 -----------------
2 files changed, 31 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e811bd9..4763a35 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1464,7 +1464,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
u32 cid[4];
u32 rocr;
- BUG_ON(!host);
WARN_ON(!host->claimed);
/* Set correct bus mode for MMC before attempting init */
@@ -1854,9 +1853,6 @@ 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);
-
mmc_remove_card(host->card);
host->card = NULL;
}
@@ -1876,9 +1872,6 @@ static void mmc_detect(struct mmc_host *host)
{
int err;
- BUG_ON(!host);
- BUG_ON(!host->card);
-
mmc_get_card(host->card);
/*
@@ -1904,9 +1897,6 @@ 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);
-
mmc_claim_host(host);
if (mmc_card_suspended(host->card))
@@ -1963,9 +1953,6 @@ static int _mmc_resume(struct mmc_host *host)
{
int err = 0;
- BUG_ON(!host);
- BUG_ON(!host->card);
-
mmc_claim_host(host);
if (!mmc_card_suspended(host->card))
@@ -2098,7 +2085,6 @@ int mmc_attach_mmc(struct mmc_host *host)
int err;
u32 ocr, rocr;
- BUG_ON(!host);
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 481bbdb..2bc4780 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,8 +89,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
struct mmc_command cmd = {0};
- BUG_ON(!host);
-
cmd.opcode = MMC_SELECT_CARD;
if (card) {
@@ -109,7 +104,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 +175,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,9 +213,6 @@ int mmc_all_send_cid(struct mmc_host *host, u32 *cid)
int err;
struct mmc_command cmd = {0};
- BUG_ON(!host);
- BUG_ON(!cid);
-
cmd.opcode = MMC_ALL_SEND_CID;
cmd.arg = 0;
cmd.flags = MMC_RSP_R2 | MMC_CMD_BCR;
@@ -241,9 +230,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,9 +243,6 @@ 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);
-
cmd.opcode = opcode;
cmd.arg = arg;
cmd.flags = MMC_RSP_R2 | MMC_CMD_AC;
--
2.3.7
^ permalink raw reply related [flat|nested] 17+ messages in thread* RE: [PATCH 3/8] mmc: core: remove BUG_ONs from mmc
2016-11-02 7:25 ` [PATCH 3/8] mmc: core: remove BUG_ONs from mmc Shawn Lin
@ 2016-11-02 9:40 ` Venu Byravarasu
2016-11-29 12:41 ` Ulf Hansson
1 sibling, 0 replies; 17+ messages in thread
From: Venu Byravarasu @ 2016-11-02 9:40 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson; +Cc: linux-mmc@vger.kernel.org, Venu Byravarasu
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Shawn Lin
> Sent: Wednesday, November 02, 2016 12:56 PM
> To: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org; Shawn Lin <shawn.lin@rock-chips.com>
> Subject: [PATCH 3/8] mmc: core: remove BUG_ONs from mmc
>
> 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 | 14 --------------
> drivers/mmc/core/mmc_ops.c | 17 -----------------
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> e811bd9..4763a35 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1464,7 +1464,6 @@ static int mmc_init_card(struct mmc_host *host, u32
> ocr,
> u32 cid[4];
> u32 rocr;
>
> - BUG_ON(!host);
Instead of just removing BUG_ON, it would be worth adding null check and
return from here graciously as you mentioned in patch commit message,
without which next instruction below may crash if host == NULL.
Similar is the case with other removals below.
> WARN_ON(!host->claimed);
>
> /* Set correct bus mode for MMC before attempting init */ @@ -1854,9
> +1853,6 @@ 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);
> -
> mmc_remove_card(host->card);
> host->card = NULL;
> }
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/8] mmc: core: remove BUG_ONs from mmc
2016-11-02 7:25 ` [PATCH 3/8] mmc: core: remove BUG_ONs from mmc Shawn Lin
2016-11-02 9:40 ` Venu Byravarasu
@ 2016-11-29 12:41 ` Ulf Hansson
1 sibling, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:25, 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/mmc.c | 14 --------------
> drivers/mmc/core/mmc_ops.c | 17 -----------------
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index e811bd9..4763a35 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1464,7 +1464,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> u32 cid[4];
> u32 rocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> /* Set correct bus mode for MMC before attempting init */
> @@ -1854,9 +1853,6 @@ 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);
> -
> mmc_remove_card(host->card);
> host->card = NULL;
> }
> @@ -1876,9 +1872,6 @@ static void mmc_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_get_card(host->card);
>
> /*
> @@ -1904,9 +1897,6 @@ 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);
> -
> mmc_claim_host(host);
>
> if (mmc_card_suspended(host->card))
> @@ -1963,9 +1953,6 @@ static int _mmc_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (!mmc_card_suspended(host->card))
> @@ -2098,7 +2085,6 @@ int mmc_attach_mmc(struct mmc_host *host)
> int err;
> u32 ocr, rocr;
>
> - BUG_ON(!host);
> 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 481bbdb..2bc4780 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,8 +89,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> -
> cmd.opcode = MMC_SELECT_CARD;
>
> if (card) {
> @@ -109,7 +104,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 +175,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,9 +213,6 @@ int mmc_all_send_cid(struct mmc_host *host, u32 *cid)
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> - BUG_ON(!cid);
> -
> cmd.opcode = MMC_ALL_SEND_CID;
> cmd.arg = 0;
> cmd.flags = MMC_RSP_R2 | MMC_CMD_BCR;
> @@ -241,9 +230,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,9 +243,6 @@ 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);
> -
> cmd.opcode = opcode;
> cmd.arg = arg;
> cmd.flags = MMC_RSP_R2 | MMC_CMD_AC;
> --
> 2.3.7
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/8] mmc: core: remove BUG_ONs from sd
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
2016-11-02 7:24 ` [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open Shawn Lin
2016-11-02 7:25 ` [PATCH 3/8] mmc: core: remove BUG_ONs from mmc Shawn Lin
@ 2016-11-02 7:25 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:26 ` [PATCH 5/8] mmc: core: remove BUG_ONs from core.c Shawn Lin
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:25 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-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/sd.c | 14 --------------
drivers/mmc/core/sd_ops.c | 27 ++++-----------------------
2 files changed, 4 insertions(+), 37 deletions(-)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 73c762a..deb90c2 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,9 +1061,6 @@ static void mmc_sd_detect(struct mmc_host *host)
{
int err;
- BUG_ON(!host);
- BUG_ON(!host->card);
-
mmc_get_card(host->card);
/*
@@ -1091,9 +1084,6 @@ static int _mmc_sd_suspend(struct mmc_host *host)
{
int err = 0;
- BUG_ON(!host);
- BUG_ON(!host->card);
-
mmc_claim_host(host);
if (mmc_card_suspended(host->card))
@@ -1136,9 +1126,6 @@ static int _mmc_sd_resume(struct mmc_host *host)
{
int err = 0;
- BUG_ON(!host);
- BUG_ON(!host->card);
-
mmc_claim_host(host);
if (!mmc_card_suspended(host->card))
@@ -1221,7 +1208,6 @@ int mmc_attach_sd(struct mmc_host *host)
int err;
u32 ocr, rocr;
- BUG_ON(!host);
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..de125a4 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,9 +219,6 @@ 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);
-
cmd.opcode = SD_SEND_RELATIVE_ADDR;
cmd.arg = 0;
cmd.flags = MMC_RSP_R6 | MMC_CMD_BCR;
@@ -249,10 +241,6 @@ 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);
-
/* NOTE: caller guarantees scr is heap-allocated */
err = mmc_app_cmd(card->host, card);
@@ -307,9 +295,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,10 +337,6 @@ 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);
-
/* NOTE: caller guarantees ssr is heap-allocated */
err = mmc_app_cmd(card->host, card);
--
2.3.7
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/8] mmc: core: remove BUG_ONs from sd
2016-11-02 7:25 ` [PATCH 4/8] mmc: core: remove BUG_ONs from sd Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:25, 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/sd.c | 14 --------------
> drivers/mmc/core/sd_ops.c | 27 ++++-----------------------
> 2 files changed, 4 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 73c762a..deb90c2 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,9 +1061,6 @@ static void mmc_sd_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_get_card(host->card);
>
> /*
> @@ -1091,9 +1084,6 @@ static int _mmc_sd_suspend(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (mmc_card_suspended(host->card))
> @@ -1136,9 +1126,6 @@ static int _mmc_sd_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (!mmc_card_suspended(host->card))
> @@ -1221,7 +1208,6 @@ int mmc_attach_sd(struct mmc_host *host)
> int err;
> u32 ocr, rocr;
>
> - BUG_ON(!host);
> 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..de125a4 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,9 +219,6 @@ 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);
> -
> cmd.opcode = SD_SEND_RELATIVE_ADDR;
> cmd.arg = 0;
> cmd.flags = MMC_RSP_R6 | MMC_CMD_BCR;
> @@ -249,10 +241,6 @@ 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);
> -
> /* NOTE: caller guarantees scr is heap-allocated */
>
> err = mmc_app_cmd(card->host, card);
> @@ -307,9 +295,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,10 +337,6 @@ 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);
> -
> /* NOTE: caller guarantees ssr is heap-allocated */
>
> err = mmc_app_cmd(card->host, card);
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/8] mmc: core: remove BUG_ONs from core.c
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
` (2 preceding siblings ...)
2016-11-02 7:25 ` [PATCH 4/8] mmc: core: remove BUG_ONs from sd Shawn Lin
@ 2016-11-02 7:26 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:26 ` [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON Shawn Lin
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:26 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-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/core.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 2553d90..7fc4814 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,8 +349,6 @@ 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))
return;
@@ -754,8 +752,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 +846,6 @@ int mmc_stop_bkops(struct mmc_card *card)
{
int err = 0;
- BUG_ON(!card);
err = mmc_interrupt_hpi(card);
/*
@@ -1666,8 +1661,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 +1877,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 +1917,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 +1938,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 +2851,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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/8] mmc: core: remove BUG_ONs from core.c
2016-11-02 7:26 ` [PATCH 5/8] mmc: core: remove BUG_ONs from core.c Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:26, 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/core.c | 34 +++++++++-------------------------
> 1 file changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 2553d90..7fc4814 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,8 +349,6 @@ 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))
> return;
>
> @@ -754,8 +752,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 +846,6 @@ int mmc_stop_bkops(struct mmc_card *card)
> {
> int err = 0;
>
> - BUG_ON(!card);
> err = mmc_interrupt_hpi(card);
>
> /*
> @@ -1666,8 +1661,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 +1877,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 +1917,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 +1938,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 +2851,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
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
` (3 preceding siblings ...)
2016-11-02 7:26 ` [PATCH 5/8] mmc: core: remove BUG_ONs from core.c Shawn Lin
@ 2016-11-02 7:26 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:26 ` [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg Shawn Lin
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:26 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON
2016-11-02 7:26 ` [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> 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
>
>
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
` (4 preceding siblings ...)
2016-11-02 7:26 ` [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON Shawn Lin
@ 2016-11-02 7:26 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-02 7:26 ` [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling Shawn Lin
2016-11-29 12:40 ` [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Ulf Hansson
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:26 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg
2016-11-02 7:26 ` [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> 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
>
>
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
` (5 preceding siblings ...)
2016-11-02 7:26 ` [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg Shawn Lin
@ 2016-11-02 7:26 ` Shawn Lin
2016-11-29 12:41 ` Ulf Hansson
2016-11-29 12:40 ` [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Ulf Hansson
7 siblings, 1 reply; 17+ messages in thread
From: Shawn Lin @ 2016-11-02 7:26 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling
2016-11-02 7:26 ` [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling Shawn Lin
@ 2016-11-29 12:41 ` Ulf Hansson
0 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> 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
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/8] mmc: core: remove BUG_ONs from sdio
2016-11-02 7:24 [PATCH 1/8] mmc: core: remove BUG_ONs from sdio Shawn Lin
` (6 preceding siblings ...)
2016-11-02 7:26 ` [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling Shawn Lin
@ 2016-11-29 12:40 ` Ulf Hansson
7 siblings, 0 replies; 17+ messages in thread
From: Ulf Hansson @ 2016-11-29 12:40 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
On 2 November 2016 at 08:24, 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>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/sdio.c | 17 ++---------------
> drivers/mmc/core/sdio_cis.c | 3 ++-
> drivers/mmc/core/sdio_irq.c | 12 +++++++-----
> 3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index bd44ba8..ecbc529 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,6 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
> u32 rocr = 0;
> u32 ocr_card = ocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> /* to query card if 1.8V signalling is supported */
> @@ -791,9 +791,6 @@ static void mmc_sdio_remove(struct mmc_host *host)
> {
> int i;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> for (i = 0;i < host->card->sdio_funcs;i++) {
> if (host->card->sdio_func[i]) {
> sdio_remove_func(host->card->sdio_func[i]);
> @@ -820,9 +817,6 @@ static void mmc_sdio_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> /* Make sure card is powered before detecting it */
> if (host->caps & MMC_CAP_POWER_OFF_CARD) {
> err = pm_runtime_get_sync(&host->card->dev);
> @@ -916,9 +910,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> /* Basic card reinitialization. */
> mmc_claim_host(host);
>
> @@ -970,9 +961,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 +1051,6 @@ int mmc_attach_sdio(struct mmc_host *host)
> u32 ocr, rocr;
> struct mmc_card *card;
>
> - BUG_ON(!host);
> 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
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread