[PATCH v2 0/4] SD card bug fixes

[PATCH v2 0/4] SD card bug fixes

[Bao D. Nguyen]

I tried to make some changes earlier in the patch
[<PATCH v1> 9/9] mmc: sd: Fix trivial SD card issues].
There were a lot of valid comments asking to clarify
and separate the fixes into logical patches. With this
patch series, I am trying to address those comments.

Changes since v1:
- Addressed Ulf Hansson's comment regarding unnecessary NULL pointer check in the mmc_bus_shutdown(). 

Bao D. Nguyen (1):
  mmc: core: Add check for NULL pointer access

Ritesh Harjani (1):
  mmc: core: Make host->card as NULL when card is removed

Sahitya Tummala (1):
  mmc: core: update host->card after getting RCA for SD card

Subhash Jadavani (1):
  mmc: core: Attribute the IO wait time properly in
    mmc_wait_for_req_done()

 drivers/mmc/core/bus.c  | 3 +++
 drivers/mmc/core/core.c | 5 ++++-
 drivers/mmc/core/sd.c   | 6 ++++--
 3 files changed, 11 insertions(+), 3 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/4] mmc: core: Add check for NULL pointer access

[Bao D. Nguyen]

If the SD card is removed, the mmc_card pointer can be set to NULL
by the mmc_sd_remove() function. Check mmc_card pointer to avoid NULL
pointer access.

Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/mmc/core/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6b38c19..94441a0 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -666,6 +666,9 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 {
 	unsigned int mult;
 
+	if (!card)
+		return;
+
 	/*
 	 * SDIO cards only define an upper 1 s limit on access.
 	 */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/4] mmc: core: Add check for NULL pointer access

[Ulf Hansson]

On Fri, 6 Mar 2020 at 05:59, Bao D. Nguyen <nguyenb@codeaurora.org> wrote:
>
> If the SD card is removed, the mmc_card pointer can be set to NULL
> by the mmc_sd_remove() function. Check mmc_card pointer to avoid NULL
> pointer access.

As stated in the other replies, this is just a vague explanation to a
*potential* problem.

Please explain the details for how this problem can occur - or a way
to reproduce the problem.

Kind regards
Uffe

>
> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/mmc/core/core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 6b38c19..94441a0 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -666,6 +666,9 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
>  {
>         unsigned int mult;
>
> +       if (!card)
> +               return;
> +
>         /*
>          * SDIO cards only define an upper 1 s limit on access.
>          */
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

[PATCH v2 2/4] mmc: core: Attribute the IO wait time properly in mmc_wait_for_req_done()

[Bao D. Nguyen]

From: Subhash Jadavani <subhashj@codeaurora.org>

In mmc_wait_for_req_done() function, change the call wait_for_completion()
to wait_for_compltion_io(). This change makes the kernel account for
wait time as I/O wait and through another configuration, this io wait
is treated as busy which makes the acpu clock to scale up.

Signed-off-by: Murali Palnati <palnatim@codeaurora.org>
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/mmc/core/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 94441a0..97a384a 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -399,7 +399,7 @@ void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq)
 	struct mmc_command *cmd;
 
 	while (1) {
-		wait_for_completion(&mrq->completion);
+		wait_for_completion_io(&mrq->completion);
 
 		cmd = mrq->cmd;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/4] mmc: core: Make host->card as NULL when card is removed

[Bao D. Nguyen]

From: Ritesh Harjani <riteshh@codeaurora.org>

Make host->card as NULL when card is removed otherwise
we do see some kernel crashes in async contexts,
where host->card is referred (as dangling pointer).

Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/mmc/core/bus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 74de3f2..e83821c 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -247,12 +247,15 @@ void mmc_unregister_driver(struct mmc_driver *drv)
 static void mmc_release_card(struct device *dev)
 {
 	struct mmc_card *card = mmc_dev_to_card(dev);
+	struct mmc_host *host = card->host;
 
 	sdio_free_common_cis(card);
 
 	kfree(card->info);
 
 	kfree(card);
+	if (host)
+		host->card = NULL;
 }
 
 /*
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 4/4] mmc: core: update host->card after getting RCA for SD card

[Bao D. Nguyen]

From: Sahitya Tummala <stummala@codeaurora.org>

Make the host->card available before tuning is invoked for SD card.
In the sdhci_msm_execute_tuning(), we will send CMD13 only if
host->card is present because it needs the card->rca as its
argument to be sent. For emmc, host->card is already updated
immediately after the mmc_alloc_card(). In the similar way,
this change is for SD card. Without this change, tuning functionality
will not be able to send CMD13 to make sure the card is ready
for next data command. If the last tuning command failed
and we did not send CMD13 to ensure card is in transfer state,
the next read/write command will fail.

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/mmc/core/sd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 76c7add..f0872e3 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -989,6 +989,7 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 		err = mmc_send_relative_addr(host, &card->rca);
 		if (err)
 			goto free_card;
+		host->card = card;
 	}
 
 	if (!oldcard) {
@@ -1100,12 +1101,13 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
 		goto free_card;
 	}
 done:
-	host->card = card;
 	return 0;
 
 free_card:
-	if (!oldcard)
+	if (!oldcard) {
+		host->card = NULL;
 		mmc_remove_card(card);
+	}
 
 	return err;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project