From: Doug Anderson <dianders@chromium.org>
To: Addy Ke <addy.ke@rock-chips.com>
Cc: "Jaehoon Chung" <jh80.chung@samsung.com>,
"Ulf Hansson" <ulf.hansson@linaro.org>,
"Olof Johansson" <olof@lixom.net>,
"Alim Akhtar" <alim.akhtar@gmail.com>,
"Andrzej Hajda" <a.hajda@samsung.com>,
"Heiko Stübner" <heiko@sntech.de>,
"Eddie Cai" <cf@rock-chips.com>, lintao <lintao@rock-chips.com>,
"Tao Huang" <huangtao@rock-chips.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
"Javier Martinez Canillas" <javier.martinez@collabora.co.uk>
Subject: Re: [PATCH v4 3/3] mmc: dw_mmc: Don't start command while data busy
Date: Thu, 19 Feb 2015 16:21:31 -0800 [thread overview]
Message-ID: <CAD=FV=WS_KG2XjQEtKPuBy__mtWDD3ja58rccEHxV=1XmBTCuw@mail.gmail.com> (raw)
In-Reply-To: <1423894668-8886-4-git-send-email-addy.ke@rock-chips.com>
Addy,
On Fri, Feb 13, 2015 at 10:17 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> We should wait until unbusy before the next request.
> But this does't need if the command is CMD13, which can access
> SD Status register regardless of data busy.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
> Changes in v4:
> - CMD13 doesn't need wait until unbusy.
>
> drivers/mmc/host/dw_mmc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ac21863..692d97a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1076,6 +1076,10 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
> WARN_ON(slot->mrq);
>
> + /* Wait until unbusy if the command isn't CMD13 */
> + if (mrq->cmd->opcode != MMC_SEND_STATUS)
> + dw_mci_wait_busy(slot);
> +
I think you need to be more general. You really should be checking
"cmd_flags & SDMMC_CMD_PRV_DAT_WAIT". That leverages dw_mmc's
knowledge about whether it needs to wait. Right now you'll be waiting
for CMD52 (SDIO) which I don't think is a good idea.
It also seems like this would be better in dw_mci_start_command() so
that we have the least chance of hitting the case. The downside is
that you can't sleep there, though... Hrm.
I updated a version of my take on this at
<https://chromium-review.googlesource.com/#/c/244850/>. I'll put a
timeout on it soon-ish. Any extra testing that folks can do would be
appreciated...
-Doug
WARNING: multiple messages have this Message-ID (diff)
From: dianders@chromium.org (Doug Anderson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 3/3] mmc: dw_mmc: Don't start command while data busy
Date: Thu, 19 Feb 2015 16:21:31 -0800 [thread overview]
Message-ID: <CAD=FV=WS_KG2XjQEtKPuBy__mtWDD3ja58rccEHxV=1XmBTCuw@mail.gmail.com> (raw)
In-Reply-To: <1423894668-8886-4-git-send-email-addy.ke@rock-chips.com>
Addy,
On Fri, Feb 13, 2015 at 10:17 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> We should wait until unbusy before the next request.
> But this does't need if the command is CMD13, which can access
> SD Status register regardless of data busy.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
> Changes in v4:
> - CMD13 doesn't need wait until unbusy.
>
> drivers/mmc/host/dw_mmc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ac21863..692d97a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1076,6 +1076,10 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
> WARN_ON(slot->mrq);
>
> + /* Wait until unbusy if the command isn't CMD13 */
> + if (mrq->cmd->opcode != MMC_SEND_STATUS)
> + dw_mci_wait_busy(slot);
> +
I think you need to be more general. You really should be checking
"cmd_flags & SDMMC_CMD_PRV_DAT_WAIT". That leverages dw_mmc's
knowledge about whether it needs to wait. Right now you'll be waiting
for CMD52 (SDIO) which I don't think is a good idea.
It also seems like this would be better in dw_mci_start_command() so
that we have the least chance of hitting the case. The downside is
that you can't sleep there, though... Hrm.
I updated a version of my take on this at
<https://chromium-review.googlesource.com/#/c/244850/>. I'll put a
timeout on it soon-ish. Any extra testing that folks can do would be
appreciated...
-Doug
next prev parent reply other threads:[~2015-02-20 0:21 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-05 11:13 [PATCH] mmc: dw_mmc: fix bug that cause 'Timeout sending command' Addy Ke
2015-02-05 11:13 ` Addy Ke
2015-02-09 4:48 ` Ulf Hansson
2015-02-09 4:51 ` Ulf Hansson
2015-02-09 4:51 ` Ulf Hansson
2015-02-09 4:51 ` Ulf Hansson
2015-02-09 6:56 ` Addy
2015-02-09 6:56 ` Addy
2015-02-09 6:56 ` Addy
2015-02-09 7:04 ` Jaehoon Chung
2015-02-09 7:04 ` Jaehoon Chung
2015-02-09 7:04 ` Jaehoon Chung
2015-02-09 9:17 ` addy ke
2015-02-09 9:17 ` addy ke
2015-02-09 9:17 ` addy ke
2015-02-09 7:25 ` [PATCH v2 0/2] about data busy Addy Ke
2015-02-09 7:25 ` Addy Ke
2015-02-09 7:25 ` Addy Ke
2015-02-09 7:25 ` [PATCH v2 1/2] mmc: dw_mmc: fix bug that cause 'Timeout sending command' Addy Ke
2015-02-09 7:25 ` Addy Ke
2015-02-09 10:01 ` Jaehoon Chung
2015-02-09 10:01 ` Jaehoon Chung
2015-02-11 3:07 ` Addy
2015-02-11 3:07 ` Addy
2015-02-10 15:22 ` Alim Akhtar
2015-02-10 15:22 ` Alim Akhtar
2015-02-10 15:22 ` Alim Akhtar
2015-02-11 2:57 ` Addy
2015-02-11 2:57 ` Addy
2015-02-11 2:57 ` Addy
2015-02-11 3:25 ` Ulf Hansson
2015-02-11 3:46 ` Jaehoon Chung
2015-02-11 3:52 ` Ulf Hansson
2015-02-11 11:58 ` Andrzej Hajda
2015-02-11 23:20 ` Alim Akhtar
2015-02-11 23:20 ` Alim Akhtar
2015-02-12 2:28 ` addy ke
2015-02-12 11:10 ` Andrzej Hajda
2015-02-12 13:59 ` Alim Akhtar
2015-02-12 13:59 ` Alim Akhtar
2015-02-13 8:15 ` addy ke
2015-02-12 11:13 ` Andrzej Hajda
2015-02-12 11:13 ` Andrzej Hajda
2015-02-12 13:53 ` Alim Akhtar
2015-02-12 13:53 ` Alim Akhtar
2015-02-09 7:25 ` [PATCH v2 2/2] mmc: dw_mmc: Don't start command while data busy Addy Ke
2015-02-09 7:25 ` Addy Ke
2015-02-13 11:52 ` [PATCH v3 0/3] about " Addy Ke
2015-02-13 11:52 ` Addy Ke
2015-02-13 11:52 ` [PATCH v3 1/3] mmc: dw_mmc: update clock after host reach a stable voltage Addy Ke
2015-02-13 11:52 ` Addy Ke
2015-02-13 11:52 ` [PATCH v3 2/3] mmc: dw_mmc: fix bug that cause 'Timeout sending command' Addy Ke
2015-02-13 11:52 ` Addy Ke
2015-02-13 11:52 ` [PATCH v3 3/3] mmc: dw_mmc: Don't start command while data busy Addy Ke
2015-02-13 11:52 ` Addy Ke
2015-02-14 6:17 ` [PATCH v4 0/3] about " Addy Ke
2015-02-14 6:17 ` Addy Ke
2015-02-14 6:17 ` [PATCH v4 1/3] mmc: dw_mmc: update clock after host reach a stable voltage Addy Ke
2015-02-14 6:17 ` Addy Ke
2015-02-15 23:28 ` Alim Akhtar
2015-02-15 23:28 ` Alim Akhtar
2015-02-19 10:30 ` addy ke
2015-02-19 10:30 ` addy ke
2015-02-19 23:49 ` Doug Anderson
2015-02-19 23:49 ` Doug Anderson
2015-02-20 0:02 ` Russell King - ARM Linux
2015-02-20 0:02 ` Russell King - ARM Linux
2015-02-20 1:04 ` Doug Anderson
2015-02-20 1:04 ` Doug Anderson
2015-02-20 19:05 ` Doug Anderson
2015-02-20 19:05 ` Doug Anderson
2015-02-25 7:52 ` Alim Akhtar
2015-02-25 7:52 ` Alim Akhtar
2015-02-25 9:56 ` Jaehoon Chung
2015-02-25 9:56 ` Jaehoon Chung
2015-02-25 21:05 ` Doug Anderson
2015-02-25 21:05 ` Doug Anderson
2015-02-14 6:17 ` [PATCH v4 2/3] mmc: dw_mmc: fix bug that cause 'Timeout sending command' Addy Ke
2015-02-14 6:17 ` Addy Ke
2015-02-14 6:17 ` [PATCH v4 3/3] mmc: dw_mmc: Don't start command while data busy Addy Ke
2015-02-14 6:17 ` Addy Ke
2015-02-20 0:21 ` Doug Anderson [this message]
2015-02-20 0:21 ` Doug Anderson
2015-02-15 11:41 ` [PATCH v4 0/3] about " Javier Martinez Canillas
2015-02-15 11:41 ` Javier Martinez Canillas
2015-02-16 5:48 ` Jaehoon Chung
2015-02-16 5:48 ` Jaehoon Chung
2015-02-16 11:09 ` Javier Martinez Canillas
2015-02-16 11:09 ` Javier Martinez Canillas
2015-02-19 10:55 ` addy ke
2015-02-19 10:55 ` addy ke
2015-02-20 19:03 ` Doug Anderson
2015-02-20 19:03 ` Doug Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAD=FV=WS_KG2XjQEtKPuBy__mtWDD3ja58rccEHxV=1XmBTCuw@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=a.hajda@samsung.com \
--cc=addy.ke@rock-chips.com \
--cc=alim.akhtar@gmail.com \
--cc=cf@rock-chips.com \
--cc=heiko@sntech.de \
--cc=huangtao@rock-chips.com \
--cc=javier.martinez@collabora.co.uk \
--cc=jh80.chung@samsung.com \
--cc=lintao@rock-chips.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=olof@lixom.net \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.