[PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[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 <james.hogan@imgtec.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 60063cc..1a42c7c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -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);
 		}
 
-- 
1.8.1.2

Re: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[Jaehoon Chung]

Looks good to me.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

On 03/12/2013 07:43 PM, James Hogan wrote:
> 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 <james.hogan@imgtec.com>
> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Chris Ball <cjb@laptop.org>
> ---
>  drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 60063cc..1a42c7c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -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);
>  		}
>  
> 

Re: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[James Hogan]

On 12/03/13 11:26, Jaehoon Chung wrote:
> Looks good to me.
> 
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Thanks Jaehoon

Cheers
James

> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/12/2013 07:43 PM, James Hogan wrote:
>> 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 <james.hogan@imgtec.com>
>> Cc: Seungwon Jeon <tgih.jun@samsung.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Chris Ball <cjb@laptop.org>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 60063cc..1a42c7c 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -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);
>>  		}
>>  
>>
> 

RE: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[Seungwon Jeon]

Hi James,

On Tuesday, March 12, 2013, James Hogan wrote:
> 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 <james.hogan@imgtec.com>
> Cc: Seungwon Jeon <tgih.jun@samsung.com>

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

Re: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[James Hogan]

On 13/03/13 14:20, Seungwon Jeon wrote:
> Hi James,
> 
> On Tuesday, March 12, 2013, James Hogan wrote:
>> 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 <james.hogan@imgtec.com>
>> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> 
> 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

RE: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[Seungwon Jeon]

On Wednesday, March 13, 2013, James Hogan wrote:
> On 13/03/13 14:20, Seungwon Jeon wrote:
> > Hi James,
> >
> > On Tuesday, March 12, 2013, James Hogan wrote:
> >> 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 <james.hogan@imgtec.com>
> >> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> >
> > 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.
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 <tgih.jun@samsung.com>

Thanks,
Seungwon Jeon

Re: [PATCH] mmc: dw_mmc: setpower on MMC_POWER_{UP,OFF}

[Chris Ball]

Hi,

On Tue, Mar 12 2013, James Hogan wrote:
> 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.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child