Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1407784 > unrolled thread
| Started by | Shawn Lin <shawn.lin@rock-chips.com> |
|---|---|
| First post | 2016-05-27 05:10 +0200 |
| Last post | 2016-05-27 06:20 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 2/2] mmc: dw_mmc: check card present before starting request Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-27 05:10 +0200
Re: [PATCH 2/2] mmc: dw_mmc: check card present before starting request Jaehoon Chung <jh80.chung@samsung.com> - 2016-05-27 05:50 +0200
Re: [PATCH 2/2] mmc: dw_mmc: check card present before starting request Shawn Lin <shawn.lin@rock-chips.com> - 2016-05-27 06:20 +0200
| From | Shawn Lin <shawn.lin@rock-chips.com> |
|---|---|
| Date | 2016-05-27 05:10 +0200 |
| Subject | [PATCH 2/2] mmc: dw_mmc: check card present before starting request |
| Message-ID | <rDgjU-6rz-3@gated-at.bofh.it> |
The main reason to add this check is to avoid unnecessary
mmc_request like the on-going cmd and the corresponding sbc
if the card is removed. Although we have already checked this in
dw_mci_handle_cd for runtime usage of sd card and dw_mci_init_slot
for noremovable devices, but there is a timing gap before it really
calls dw_mci_get_cd as mmc_detect_change needs some delay here.
Another gain here is that we could save some checkings of card status
after sd card been removed.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/mmc/host/dw_mmc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index cb30e91..8eb7898 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -105,6 +105,7 @@ struct idmac_desc {
static bool dw_mci_reset(struct dw_mci *host);
static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
static int dw_mci_card_busy(struct mmc_host *mmc);
+static int dw_mci_get_cd(struct mmc_host *mmc);
#if defined(CONFIG_DEBUG_FS)
static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -1255,7 +1256,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
*/
spin_lock_bh(&host->lock);
- if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
+ if (!dw_mci_get_cd(mmc)) {
spin_unlock_bh(&host->lock);
mrq->cmd->error = -ENOMEDIUM;
mmc_request_done(mmc, mrq);
--
2.3.7
[toc] | [next] | [standalone]
| From | Jaehoon Chung <jh80.chung@samsung.com> |
|---|---|
| Date | 2016-05-27 05:50 +0200 |
| Message-ID | <rDgWC-6Eh-7@gated-at.bofh.it> |
| In reply to | #1407784 |
Hi Shawn,
On 05/27/2016 12:02 PM, Shawn Lin wrote:
> The main reason to add this check is to avoid unnecessary
> mmc_request like the on-going cmd and the corresponding sbc
> if the card is removed. Although we have already checked this in
> dw_mci_handle_cd for runtime usage of sd card and dw_mci_init_slot
> for noremovable devices, but there is a timing gap before it really
> calls dw_mci_get_cd as mmc_detect_change needs some delay here.
>
> Another gain here is that we could save some checkings of card status
> after sd card been removed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> drivers/mmc/host/dw_mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index cb30e91..8eb7898 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -105,6 +105,7 @@ struct idmac_desc {
> static bool dw_mci_reset(struct dw_mci *host);
> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
> static int dw_mci_card_busy(struct mmc_host *mmc);
> +static int dw_mci_get_cd(struct mmc_host *mmc);
>
> #if defined(CONFIG_DEBUG_FS)
> static int dw_mci_req_show(struct seq_file *s, void *v)
> @@ -1255,7 +1256,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
> */
> spin_lock_bh(&host->lock);
>
> - if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
> + if (!dw_mci_get_cd(mmc)) {
> spin_unlock_bh(&host->lock);
> mrq->cmd->error = -ENOMEDIUM;
> mmc_request_done(mmc, mrq);
I think spin_lock/unlock_bh() should be removed at here.
Instead, Can it locate before calling dw_mci_queue_request()?
Anyway, it looks good to me.
Best Regards,
Jaehoon Chung
>
[toc] | [prev] | [next] | [standalone]
| From | Shawn Lin <shawn.lin@rock-chips.com> |
|---|---|
| Date | 2016-05-27 06:20 +0200 |
| Subject | Re: [PATCH 2/2] mmc: dw_mmc: check card present before starting request |
| Message-ID | <rDhpD-74E-1@gated-at.bofh.it> |
| In reply to | #1407794 |
在 2016/5/27 11:46, Jaehoon Chung 写道:
> Hi Shawn,
>
> On 05/27/2016 12:02 PM, Shawn Lin wrote:
>> The main reason to add this check is to avoid unnecessary
>> mmc_request like the on-going cmd and the corresponding sbc
>> if the card is removed. Although we have already checked this in
>> dw_mci_handle_cd for runtime usage of sd card and dw_mci_init_slot
>> for noremovable devices, but there is a timing gap before it really
>> calls dw_mci_get_cd as mmc_detect_change needs some delay here.
>>
>> Another gain here is that we could save some checkings of card status
>> after sd card been removed.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> drivers/mmc/host/dw_mmc.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index cb30e91..8eb7898 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -105,6 +105,7 @@ struct idmac_desc {
>> static bool dw_mci_reset(struct dw_mci *host);
>> static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>> static int dw_mci_card_busy(struct mmc_host *mmc);
>> +static int dw_mci_get_cd(struct mmc_host *mmc);
>>
>> #if defined(CONFIG_DEBUG_FS)
>> static int dw_mci_req_show(struct seq_file *s, void *v)
>> @@ -1255,7 +1256,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>> */
>> spin_lock_bh(&host->lock);
>>
>> - if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
>> + if (!dw_mci_get_cd(mmc)) {
>> spin_unlock_bh(&host->lock);
>> mrq->cmd->error = -ENOMEDIUM;
>> mmc_request_done(mmc, mrq);
>
> I think spin_lock/unlock_bh() should be removed at here.
yes, we should hole the lock before dw_mci_queue_request as
dw_mci_get_cd release it, in which state we do the minmum changes
for achieving what we need here.
I will fix it.
> Instead, Can it locate before calling dw_mci_queue_request()?
>
> Anyway, it looks good to me.
>
> Best Regards,
> Jaehoon Chung
>
>>
>
>
>
>
--
Best Regards
Shawn Lin
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web