mmc: dw_mmc: fix the checking whether card is present

PATCH In review Jaehoon Chung 2 participants 3 messages Feb 3, 2014 – Feb 7, 2014

Subsystems (2)

MMC, SD and SDIO Synopsys DesignWare MMC/SD/SDIO driver

lore

Share & tools

lore

Jaehoon Chung

author maintainer

2014-02-03 11:15 UTC #0 lore raw

PATCH v3 8/8 mmc: dw_mmc: fix the checking whether card is present

Broken card detection is assumed "the card is present". Non-removable card didn't call get_cd().
dw-mmc controller didn't check whether card is present or not. Because DW_MMC_CARD_PRESENT flag is set into dw_mci_get_cd(). If cd-pin is used, then dw_mci_get_cd() is called.

Signed-off-by Jaehoon Chung <[email protected]>

---
 drivers/mmc/host/dw_mmc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/mmc/host/dw_mmc.c +3 -1

index ac2907d..0d6ab8f 100644
@@ -910,6 +910,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci *host = slot->host;
+	struct dw_mci_board *brd = host->pdata;
 
 	WARN_ON(slot->mrq);

@@ -920,7 +921,8 @@ 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 (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+			!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
 		spin_unlock_bh(&host->lock);
 		mrq->cmd->error = -ENOMEDIUM;
 		mmc_request_done(mmc, mrq);

Signature · 1 line

1.7.9.5

Seungwon Jeon

2014-02-06 11:29 UTC #1 lore raw

Quoted 24 lines · Jaehoon Chung

Broken card detection is assumed "the card is present". Non-removable card didn't call get_cd().
dw-mmc controller didn't check whether card is present or not. Because DW_MMC_CARD_PRESENT flag is set into dw_mci_get_cd(). If cd-pin is used, then dw_mci_get_cd() is called.
Signed-off-by Jaehoon Chung <[email protected]>
---
 drivers/mmc/host/dw_mmc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc.c +1 -0
index ac2907d..0d6ab8f 100644
@@ -910,6 +910,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci *host = slot->host;
+	struct dw_mci_board *brd = host->pdata;
 
 	WARN_ON(slot->mrq);
 

drivers/mmc/host/dw_mmc.c static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) · line 925

Quoted 6 lines

drivers/mmc/host/dw_mmc.c +2 -1

@@ -920,7 +921,8 @@ 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 (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+			!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {

I think 'non-removable' property doesn't indicate DW_MCI_QUIRK_BROKEN_CARD_DETECTION. Actually DW_MCI_QUIRK_BROKEN_CARD_DETECTION is set with 'broken-cd' property as quirk. Still, DW_MMC_CARD_PRESENT flag won't be set with 'non-removable' property.

The following part was removed from commit bf626e5(mmc: dw_mmc: use slot-gpio to handle cd pin). It would be better to restore that to resolved this problem.

<Quoted>
static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

 #endif /* CONFIG_MMC_DW_IDMAC */
        }

- if (dw_mci_get_cd(mmc))
- set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
- else
- clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-
</Quoted>

Then, Needed to check 'non-removable' case in dw_mci_get_cd(). I think MMC_CAP_NONREMOVABLE is useful.

Thanks,
Seungwon Jeon

Jaehoon Chung

author maintainer

2014-02-07 02:45 UTC #2 lore raw

Quoted 28 lines · Seungwon Jeon

Quoted 27 lines · Jaehoon Chung
Broken card detection is assumed "the card is present". Non-removable card didn't call get_cd().
dw-mmc controller didn't check whether card is present or not. Because DW_MMC_CARD_PRESENT flag is set into dw_mci_get_cd(). If cd-pin is used, then dw_mci_get_cd() is called.
Signed-off-by Jaehoon Chung <[email protected]>
---
 drivers/mmc/host/dw_mmc.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/mmc/host/dw_mmc.c +1 -0
index ac2907d..0d6ab8f 100644
@@ -910,6 +910,7 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
 	struct dw_mci *host = slot->host;
+	struct dw_mci_board *brd = host->pdata;
 
 	WARN_ON(slot->mrq);
 
@@ -920,7 +921,8 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	 */
 	spin_lock_bh(&host->lock);
 

Quoted 7 lines

drivers/mmc/host/dw_mmc.c static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) · line 925
Quoted 3 lines
drivers/mmc/host/dw_mmc.c +2 -1
-	if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
+	if (!(brd->quirks & DW_MCI_QUIRK_BROKEN_CARD_DETECTION) &&
+			!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
I think 'non-removable' property doesn't indicate DW_MCI_QUIRK_BROKEN_CARD_DETECTION. Actually DW_MCI_QUIRK_BROKEN_CARD_DETECTION is set with 'broken-cd' property as quirk. Still, DW_MMC_CARD_PRESENT flag won't be set with 'non-removable' property.

Right, it's didn't indicate DW_MCI_QUIRK_BROKEN_CARD_DETECTION. I think that the below approach is more reasonable.

Quoted 15 lines

The following part was removed from commit bf626e5(mmc: dw_mmc: use slot-gpio to handle cd pin). It would be better to restore that to resolved this problem.
<Quoted>
static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
(quoted diff) +0 -5
@@ -2197,12 +2223,8 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 #endif /* CONFIG_MMC_DW_IDMAC */
        }
 
-       if (dw_mci_get_cd(mmc))
-               set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-       else
-               clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
-

Quoted 1 line

</Quoted>

It's good. I will update after testing.

Quoted 3 lines

Then, Needed to check 'non-removable' case in dw_mci_get_cd(). I think MMC_CAP_NONREMOVABLE is useful.

Right. MMC_CAP_NONREMOVABLE is useful. When it's set, it needs not to check the card-status with polling system. It's related with increasing a little performance.

Do you have any opinion for other patches?

Best Regards,
Jaehoon Chung

Quoted 5 lines

Thanks,
Seungwon Jeon