mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

PATCH In review James Hogan 4 participants 7 messages Mar 12, 2013 – Mar 22, 2013

Subsystems (2)

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

lore

Share & tools

lore

James Hogan

author

2013-03-12 10:43 UTC #0 lore raw

PATCH 1/1 mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.

This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:

mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)

Signed-off-by James Hogan <[email protected]>

Cc Seungwon Jeon <[email protected]>

Cc Jaehoon Chung <[email protected]>

Cc Chris Ball <[email protected]>

---
 drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

drivers/mmc/host/dw_mmc.c +8 -8

index 60063cc..1a42c7c 100644
@@ -818,6 +818,14 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
+		/* Power up slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, mmc->ocr_avail);
+		break;
+	case MMC_POWER_OFF:
+		/* Power down slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, 0);
 		break;
 	default:
 		break;

@@ -1674,10 +1682,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			dev_dbg(&slot->mmc->class_dev, "card %s\n",
 				present ? "inserted" : "removed");
 
-			/* Power up slot (before spin_lock, may sleep) */
-			if (present != 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, mmc->ocr_avail);
-
 			spin_lock_bh(&host->lock);
 
 			/* Card change detected */

@@ -1760,10 +1764,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			spin_unlock_bh(&host->lock);
 
-			/* Power down slot (after spin_unlock, may sleep) */
-			if (present == 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, 0);
-
 			present = dw_mci_get_cd(mmc);
 		}

Signature · 1 line

1.8.1.2

Jaehoon Chung

maintainer Acked-by on v1 1/1

2013-03-12 11:26 UTC #1 lore raw

Looks good to me.

Acked-by Jaehoon Chung <[email protected]>

Best Regards,
Jaehoon Chung

Quoted 70 lines · James Hogan

Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
Signed-off-by James Hogan <[email protected]>
Cc Seungwon Jeon <[email protected]>
Cc Jaehoon Chung <[email protected]>
Cc Chris Ball <[email protected]>
---
 drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/mmc/host/dw_mmc.c +8 -8
index 60063cc..1a42c7c 100644
@@ -818,6 +818,14 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
+		/* Power up slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, mmc->ocr_avail);
+		break;
+	case MMC_POWER_OFF:
+		/* Power down slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, 0);
 		break;
 	default:
 		break;
@@ -1674,10 +1682,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			dev_dbg(&slot->mmc->class_dev, "card %s\n",
 				present ? "inserted" : "removed");
 
-			/* Power up slot (before spin_lock, may sleep) */
-			if (present != 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, mmc->ocr_avail);
-
 			spin_lock_bh(&host->lock);
 
 			/* Card change detected */
@@ -1760,10 +1764,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			spin_unlock_bh(&host->lock);
 
-			/* Power down slot (after spin_unlock, may sleep) */
-			if (present == 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, 0);
-
 			present = dw_mci_get_cd(mmc);
 		}
 

James Hogan

author

2013-03-12 16:32 UTC #2 lore raw

Quoted 3 lines · Jaehoon Chung

Looks good to me.
Acked-by Jaehoon Chung <[email protected]>

Thanks Jaehoon

Cheers
James

Quoted 76 lines

Best Regards,
Jaehoon Chung
Quoted 70 lines · James Hogan
Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
Signed-off-by James Hogan <[email protected]>
Cc Seungwon Jeon <[email protected]>
Cc Jaehoon Chung <[email protected]>
Cc Chris Ball <[email protected]>
---
 drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/mmc/host/dw_mmc.c +8 -8
index 60063cc..1a42c7c 100644
@@ -818,6 +818,14 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	switch (ios->power_mode) {
 	case MMC_POWER_UP:
 		set_bit(DW_MMC_CARD_NEED_INIT, &slot->flags);
+		/* Power up slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, mmc->ocr_avail);
+		break;
+	case MMC_POWER_OFF:
+		/* Power down slot */
+		if (slot->host->pdata->setpower)
+			slot->host->pdata->setpower(slot->id, 0);
 		break;
 	default:
 		break;
@@ -1674,10 +1682,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 			dev_dbg(&slot->mmc->class_dev, "card %s\n",
 				present ? "inserted" : "removed");
 
-			/* Power up slot (before spin_lock, may sleep) */
-			if (present != 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, mmc->ocr_avail);
-
 			spin_lock_bh(&host->lock);
 
 			/* Card change detected */
@@ -1760,10 +1764,6 @@ static void dw_mci_work_routine_card(struct work_struct *work)
 
 			spin_unlock_bh(&host->lock);
 
-			/* Power down slot (after spin_unlock, may sleep) */
-			if (present == 0 && host->pdata->setpower)
-				host->pdata->setpower(slot->id, 0);
-
 			present = dw_mci_get_cd(mmc);
 		}
 

Seungwon Jeon

2013-03-13 14:20 UTC #3 lore raw

In reply to James Hogan

Hi James,

Quoted 20 lines · James Hogan

Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)

Quoted 2 lines

Signed-off-by James Hogan <[email protected]>
Cc Seungwon Jeon <[email protected]>

This patch is reasonable.
I just want to know though.
I guess this problem is happened when card is inserted as soon as card is removed. If not, could you explain your situation more?

Thanks,
Seungwon Jeon

James Hogan

author

2013-03-13 14:37 UTC #4 lore raw

Quoted 23 lines · Seungwon Jeon

Hi James,
Quoted 20 lines · James Hogan
Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)

Quoted 7 lines

Quoted 2 lines
Signed-off-by James Hogan <[email protected]>
Cc Seungwon Jeon <[email protected]>
This patch is reasonable.
I just want to know though.
I guess this problem is happened when card is inserted as soon as card is removed. If not, could you explain your situation more?

Hi,

It initially started happening after the patch by Yuvaraj CD ("mmc:
dw_mmc: enable controller interrupt before calling mmc_start_host"). Basically after hard resetting the board the card would be in a dodgy state, and I'd get these elusive errors after boot (see the above patch thread for details). It was specifically the position of the prink that it was sensitive to (I have a JTAG console turned on which adds big latencies to prinks, so it's clearly some kind of race). Unplugging the card cleared the problem, as did cutting the power to the card for a sufficient amount of time. While trying to debug the messages I saw these power callbacks and thought I'd try them as they seem like a much better place to control the card power, and seem to match other host drivers too, and it fixed the problem, I presume due to the intentional delays around those calls. It is of course possible that the race is just hiding again, but I think the patch is an improvement regardless.

Cheers
James

Seungwon Jeon

Acked-by

2013-03-14 10:34 UTC #5 lore raw

Quoted 34 lines · James Hogan

Quoted 23 lines · Seungwon Jeon
Hi James,
Quoted 20 lines · James Hogan
Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 200000Hz, actual 199680HZ div = 250)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 195765Hz, actual 195764HZ div = 255)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 400000Hz, actual 399360HZ div = 125)
mmc0: error -110 whilst initialising SD card
mmc_host mmc0: Bus speed (slot 0) = 99840000Hz (slot req 300000Hz, actual 298922HZ div = 167)
Quoted 7 lines
Quoted 2 lines
Signed-off-by James Hogan <[email protected]>
Cc Seungwon Jeon <[email protected]>
This patch is reasonable.
I just want to know though.
I guess this problem is happened when card is inserted as soon as card is removed. If not, could you explain your situation more?
Hi,

Quoted 14 lines

It initially started happening after the patch by Yuvaraj CD ("mmc:
dw_mmc: enable controller interrupt before calling mmc_start_host"). Basically after hard resetting the board the card would be in a dodgy state, and I'd get these elusive errors after boot (see the above patch thread for details). It was specifically the position of the prink that it was sensitive to (I have a JTAG console turned on which adds big latencies to prinks, so it's clearly some kind of race). Unplugging the card cleared the problem, as did cutting the power to the card for a sufficient amount of time. While trying to debug the messages I saw these power callbacks and thought I'd try them as they seem like a much better place to control the card power, and seem to match other host drivers too, and it fixed the problem, I presume due to the intentional delays around those calls. It is of course possible that the race is just hiding again, but I think the patch is an improvement regardless.

Thank you for your description.
I think the reason is insufficiency of power-on time.
Voltage level shall be below 0.5V at least 1ms when card power-on (from SD spec). I'm adding...

Acked-by Seungwon Jeon <[email protected]>

Thanks,
Seungwon Jeon

Chris Ball

2013-03-22 16:42 UTC #6 lore raw

In reply to James Hogan

Hi,

Quoted 8 lines · James Hogan

Call the setpower platform callback in response to set_ios with ios->power_mode == MMC_POWER_UP or MMC_POWER_OFF, instead of from the card detect work function.
This appears to fix a problem I have where a card stuck in a funny state doesn't get properly cleared by the power being turned off, presumably due to lack of power sequencing. This resulted in the following log messages after boot:

Thanks, pushed to mmc-next for 3.10.

- Chris.

Signature · 2 lines

Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child