[PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

The documentation for the dw_mmc part says that the low power
mode should normally only be set for MMC and SD memory and should
be turned off for SDIO cards that need interrupts detected.

The best place I could find to do this is when the SDIO interrupt
was first enabled.  I rely on the fact that dw_mci_setup_bus()
will be called when it's time to reenable.

Change-Id: Id0e33a4e3a0a77ce8a5053b6e73197d53a5d46bb
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/mmc/host/dw_mmc.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 72dc3cd..0cb2756 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -862,6 +862,31 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	return present;
 }
 
+/*
+ * Disable lower power mode.
+ *
+ * Low power mode will stop the card clock when idle.  According to
+ * documentation we should disable low power mode for SDIO cards if we
+ * need interrupts to work.
+ *
+ * This function is fast if the power mode is already disabled.
+ */
+static void dw_mci_disable_low_power(struct mmc_host *mmc)
+{
+	struct dw_mci_slot *slot = mmc_priv(mmc);
+	struct dw_mci *host = slot->host;
+	u32 clk_en_a;
+	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
+
+	clk_en_a = mci_readl(host, CLKENA);
+
+	if (clk_en_a & clken_low_pwr) {
+		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
+		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+			     SDMMC_CMD_PRV_DAT_WAIT, 0);
+	}
+}
+
 static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -871,6 +896,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 	/* Enable/disable Slot Specific SDIO interrupt */
 	int_mask = mci_readl(host, INTMASK);
 	if (enb) {
+		/*
+		 * Turn off low power mode if it was enabled.  This is a bit of
+		 * a heavy operation and we disable / enable IRQs a lot, so
+		 * we'll leave them disabled; they will get re-enabled again in
+		 * dw_mci_setup_bus().
+		 */
+		dw_mci_disable_low_power(mmc);
+
 		mci_writel(host, INTMASK,
 			   (int_mask | SDMMC_INT_SDIO(slot->id)));
 	} else {
-- 
1.7.7.3

Re: [PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Will Newton]

On Fri, Jul 20, 2012 at 7:13 PM, Doug Anderson <dianders@chromium.org> wrote:
> The documentation for the dw_mmc part says that the low power
> mode should normally only be set for MMC and SD memory and should
> be turned off for SDIO cards that need interrupts detected.
>
> The best place I could find to do this is when the SDIO interrupt
> was first enabled.  I rely on the fact that dw_mci_setup_bus()
> will be called when it's time to reenable.
>
> Change-Id: Id0e33a4e3a0a77ce8a5053b6e73197d53a5d46bb
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mmc/host/dw_mmc.c |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 72dc3cd..0cb2756 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -862,6 +862,31 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>         return present;
>  }
>
> +/*
> + * Disable lower power mode.
> + *
> + * Low power mode will stop the card clock when idle.  According to
> + * documentation we should disable low power mode for SDIO cards if we
> + * need interrupts to work.
> + *
> + * This function is fast if the power mode is already disabled.
> + */
> +static void dw_mci_disable_low_power(struct mmc_host *mmc)
> +{
> +       struct dw_mci_slot *slot = mmc_priv(mmc);
> +       struct dw_mci *host = slot->host;
> +       u32 clk_en_a;
> +       const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> +
> +       clk_en_a = mci_readl(host, CLKENA);
> +
> +       if (clk_en_a & clken_low_pwr) {
> +               mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
> +               mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> +                            SDMMC_CMD_PRV_DAT_WAIT, 0);
> +       }
> +}
> +
>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  {
>         struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -871,6 +896,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>         /* Enable/disable Slot Specific SDIO interrupt */
>         int_mask = mci_readl(host, INTMASK);
>         if (enb) {
> +               /*
> +                * Turn off low power mode if it was enabled.  This is a bit of
> +                * a heavy operation and we disable / enable IRQs a lot, so
> +                * we'll leave them disabled; they will get re-enabled again in
> +                * dw_mci_setup_bus().
> +                */
> +               dw_mci_disable_low_power(mmc);
> +
>                 mci_writel(host, INTMASK,
>                            (int_mask | SDMMC_INT_SDIO(slot->id)));
>         } else {
> --
> 1.7.7.3

Is it safe to just disable low power here or could the setting be
overwritten in setup_bus?

Re: [PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

On Sat, Jul 21, 2012 at 3:40 AM, Will Newton <will.newton@gmail.com> wrote:
>>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>  {
>>         struct dw_mci_slot *slot = mmc_priv(mmc);
>> @@ -871,6 +896,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>         /* Enable/disable Slot Specific SDIO interrupt */
>>         int_mask = mci_readl(host, INTMASK);
>>         if (enb) {
>> +               /*
>> +                * Turn off low power mode if it was enabled.  This is a bit of
>> +                * a heavy operation and we disable / enable IRQs a lot, so
>> +                * we'll leave them disabled; they will get re-enabled again in
>> +                * dw_mci_setup_bus().
>> +                */
>> +               dw_mci_disable_low_power(mmc);
>> +
>
> Is it safe to just disable low power here or could the setting be
> overwritten in setup_bus?

Very good question.  In my current setup I don't see setup_bus()
called during normal operation.  If it were, my kernel messages would
be constantly spammed with messages like:
    Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)

...and they're not.  Things may be different with different SDIO cards perhaps?

In any case, it's pretty easy for me to spin the patch so that we
don't clobber the low power bit in setup_bus() if SDIO interrupts are
enabled.  That makes a lot of sense, though I'd need to make sure that
low power mode does eventually get set again if someone ejects the
SDIO card and puts in a non-SDIO card.

I'll spin the patch tomorrow when I can test it properly and also
address some commenting concerns another engineer at chromium.org had.

It still feels to me like there ought to be a better place to put this
code.  I'd rather disable low power mode as soon as we detect an SDIO
card.  I spent time searching and the best I could find was
dw_mci_enable_sdio_irq(), but I'm all ears if someone has a better
idea!  :)  Certainly this code needs to go somewhere if we want SDIO
interrupts to be reliable.

Thanks for your feeback!  :)

-Doug

Re: [PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Will Newton]

On Mon, Jul 23, 2012 at 3:48 AM, Doug Anderson <dianders@chromium.org> wrote:
> On Sat, Jul 21, 2012 at 3:40 AM, Will Newton <will.newton@gmail.com> wrote:
>>>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>>  {
>>>         struct dw_mci_slot *slot = mmc_priv(mmc);
>>> @@ -871,6 +896,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>>         /* Enable/disable Slot Specific SDIO interrupt */
>>>         int_mask = mci_readl(host, INTMASK);
>>>         if (enb) {
>>> +               /*
>>> +                * Turn off low power mode if it was enabled.  This is a bit of
>>> +                * a heavy operation and we disable / enable IRQs a lot, so
>>> +                * we'll leave them disabled; they will get re-enabled again in
>>> +                * dw_mci_setup_bus().
>>> +                */
>>> +               dw_mci_disable_low_power(mmc);
>>> +
>>
>> Is it safe to just disable low power here or could the setting be
>> overwritten in setup_bus?
>
> Very good question.  In my current setup I don't see setup_bus()
> called during normal operation.  If it were, my kernel messages would
> be constantly spammed with messages like:
>     Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)
>
> ...and they're not.  Things may be different with different SDIO cards perhaps?

Yeah I think setup_bus should only setup the card clock once at
startup but it may also be required on resume?

I should probably mention I have not tested this driver with any SDIO
devices, although I believe there are other people out there who do!

Re: [PATCH] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

On Mon, Jul 23, 2012 at 2:19 AM, Will Newton <will.newton@gmail.com> wrote:
>> Very good question.  In my current setup I don't see setup_bus()
>> called during normal operation.  If it were, my kernel messages would
>> be constantly spammed with messages like:
>>     Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)
>>
>> ...and they're not.  Things may be different with different SDIO cards perhaps?
>
> Yeah I think setup_bus should only setup the card clock once at
> startup but it may also be required on resume?

We just got suspend/resume working yesterday, so I can now test this!  :)

With our current driver (which had some modifications to allow for
MMC_PM_KEEP_POWER that I assume will be posted before too long), I did
some testing with printk.  On my system I found that
dw_mci_setup_bus() is always called with SDIO interrupts turned off,
even during the resume path.  That means my previous posted patch is
OK.

I also looked more closely at the resume path.  I see this in the
current upstream code in the resume function:

	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);

This will clobber SDIO interrupts.  That means that if we have any
hope of SDIO interrupts working, someone will need to call
dw_mci_enable_sdio_irq() which will re-disable low power mode.  This
also points to my previous patch being OK.


...but putting the extra check in setup_bus() still doesn't hurt,
though, so I'll post that shortly.  I have looked into the SDIO code
and see that when the sdio_irq_thread exits it always disables SDIO
interrupts.  That means that I can still rely on setup_bus to properly
re-enable low power mode when it's called after an SDIO module is
removed.  :)


> I should probably mention I have not tested this driver with any SDIO
> devices, although I believe there are other people out there who do!

Agreed.  Given that I've seen recent patches (authored May 14th 2012,
for instance) fixing major SDIO issues with this driver, I'd conclude:

* Use of this driver for SDIO is very new and there may still be bugs.

* If others are using SDIO interrupts and haven't seen this issue,
they've got something different about their system.  Perhaps the SDIO
module they're using behaves in a way that SDIO interrupts always come
in at the same time as some other source?  ...or maybe they do some
type of periodic polling and are thus OK with missing some interrupts?
 The exynos manual that includes the dw mmc controller is very clear
that you can't use low power mode and SDIO interrupts and it was
definitely failing for us.

I've added the author of the most recent SDIO patch to this email
thread.  Sorry for missing that before.  Kyoungil: do you have any
comments on this?


-Doug

[PATCH v2] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

The documentation for the dw_mmc part says that the low power
mode should normally only be set for MMC and SD memory and should
be turned off for SDIO cards that need interrupts detected.

The best place I could find to do this is when the SDIO interrupt
was first enabled.  I rely on the fact that dw_mci_setup_bus()
will be called when it's time to reenable.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Commenting fixes requested by Grant Grundler.
- Be extra certain that we don't re-turn on the low power mode in
  CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
  There are no known instances of this happening but it's good to be safe.

 drivers/mmc/host/dw_mmc.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 72dc3cd..0ab1771 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 {
 	struct dw_mci *host = slot->host;
 	u32 div;
+	u32 clk_en_a;
 
 	if (slot->clock != host->current_speed) {
 		div = host->bus_hz / slot->clock;
@@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 		mci_send_cmd(slot,
 			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
 
-		/* enable clock */
-		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
-			   SDMMC_CLKEN_LOW_PWR) << slot->id));
+		/* enable clock; only low power if no SDIO */
+		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
+		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
+		mci_writel(host, CLKENA, clk_en_a);
 
 		/* inform CIU */
 		mci_send_cmd(slot,
@@ -862,6 +865,32 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	return present;
 }
 
+/*
+ * Disable lower power mode.
+ *
+ * Low power mode will stop the card clock when idle.  According to
+ * documentation (Exynos 5250 User's Manual 0.04, description of
+ * CLKENA register) we should disable low power mode for SDIO cards
+ * if we need interrupts to work.
+ *
+ * This function is fast if the power mode is already disabled.
+ */
+static void dw_mci_disable_low_power(struct mmc_host *mmc)
+{
+	struct dw_mci_slot *slot = mmc_priv(mmc);
+	struct dw_mci *host = slot->host;
+	u32 clk_en_a;
+	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
+
+	clk_en_a = mci_readl(host, CLKENA);
+
+	if (clk_en_a & clken_low_pwr) {
+		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
+		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+			     SDMMC_CMD_PRV_DAT_WAIT, 0);
+	}
+}
+
 static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -871,6 +900,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 	/* Enable/disable Slot Specific SDIO interrupt */
 	int_mask = mci_readl(host, INTMASK);
 	if (enb) {
+		/*
+		 * Turn off low power mode if it was enabled.  This is a bit of
+		 * a heavy operation and we disable / enable IRQs a lot, so
+		 * we'll leave low power mode disabled and it will get
+		 * re-enabled again in dw_mci_setup_bus().
+		 */
+		dw_mci_disable_low_power(mmc);
+
 		mci_writel(host, INTMASK,
 			   (int_mask | SDMMC_INT_SDIO(slot->id)));
 	} else {
-- 
1.7.7.3

RE: [PATCH v2] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Seungwon Jeon]

July 24, 2012, Doug Anderson <dianders@chromium.org> wrote:
> The documentation for the dw_mmc part says that the low power
> mode should normally only be set for MMC and SD memory and should
> be turned off for SDIO cards that need interrupts detected.
> 
> The best place I could find to do this is when the SDIO interrupt
> was first enabled.  I rely on the fact that dw_mci_setup_bus()
> will be called when it's time to reenable.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Commenting fixes requested by Grant Grundler.
> - Be extra certain that we don't re-turn on the low power mode in
>   CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
>   There are no known instances of this happening but it's good to be safe.
> 
>  drivers/mmc/host/dw_mmc.c |   43 ++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 72dc3cd..0ab1771 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  {
>  	struct dw_mci *host = slot->host;
>  	u32 div;
> +	u32 clk_en_a;
> 
>  	if (slot->clock != host->current_speed) {
>  		div = host->bus_hz / slot->clock;
> @@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  		mci_send_cmd(slot,
>  			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
> 
> -		/* enable clock */
> -		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
> -			   SDMMC_CLKEN_LOW_PWR) << slot->id));
> +		/* enable clock; only low power if no SDIO */
> +		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> +		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> +			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> +		mci_writel(host, CLKENA, clk_en_a);
I have followed this patch from v1.
I think it's a good point.
Looks good to me.

> 
>  		/* inform CIU */
>  		mci_send_cmd(slot,
> @@ -862,6 +865,32 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>  	return present;
>  }
> 
> +/*
> + * Disable lower power mode.
> + *
> + * Low power mode will stop the card clock when idle.  According to
> + * documentation (Exynos 5250 User's Manual 0.04, description of
> + * CLKENA register) we should disable low power mode for SDIO cards
> + * if we need interrupts to work.
Above comment is correct, but this is not specific for Exynos5250.
Exynos5250 is just one of host controllers base on Synopsys's.
It'd be better to remove the part related to Exynos you mentioned .


> + *
> + * This function is fast if the power mode is already disabled.
Definitely, low power mode not power mode, right?

Best regards,
Seungwon Jeon

> + */
> +static void dw_mci_disable_low_power(struct mmc_host *mmc)
> +{
> +	struct dw_mci_slot *slot = mmc_priv(mmc);
> +	struct dw_mci *host = slot->host;
> +	u32 clk_en_a;
> +	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> +
> +	clk_en_a = mci_readl(host, CLKENA);
> +
> +	if (clk_en_a & clken_low_pwr) {
> +		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
> +		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> +			     SDMMC_CMD_PRV_DAT_WAIT, 0);
> +	}
> +}
> +
>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  {
>  	struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -871,6 +900,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  	/* Enable/disable Slot Specific SDIO interrupt */
>  	int_mask = mci_readl(host, INTMASK);
>  	if (enb) {
> +		/*
> +		 * Turn off low power mode if it was enabled.  This is a bit of
> +		 * a heavy operation and we disable / enable IRQs a lot, so
> +		 * we'll leave low power mode disabled and it will get
> +		 * re-enabled again in dw_mci_setup_bus().
> +		 */
> +		dw_mci_disable_low_power(mmc);
> +
>  		mci_writel(host, INTMASK,
>  			   (int_mask | SDMMC_INT_SDIO(slot->id)));
>  	} else {
> --
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Jaehoon Chung]

On 07/24/2012 02:02 AM, Doug Anderson wrote:
> The documentation for the dw_mmc part says that the low power
> mode should normally only be set for MMC and SD memory and should
> be turned off for SDIO cards that need interrupts detected.
> 
> The best place I could find to do this is when the SDIO interrupt
> was first enabled.  I rely on the fact that dw_mci_setup_bus()
> will be called when it's time to reenable.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Commenting fixes requested by Grant Grundler.
> - Be extra certain that we don't re-turn on the low power mode in
>   CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
>   There are no known instances of this happening but it's good to be safe.
> 
>  drivers/mmc/host/dw_mmc.c |   43 ++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 72dc3cd..0ab1771 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  {
>  	struct dw_mci *host = slot->host;
>  	u32 div;
> +	u32 clk_en_a;
>  
>  	if (slot->clock != host->current_speed) {
>  		div = host->bus_hz / slot->clock;
> @@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  		mci_send_cmd(slot,
>  			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>  
> -		/* enable clock */
> -		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
> -			   SDMMC_CLKEN_LOW_PWR) << slot->id));
> +		/* enable clock; only low power if no SDIO */
> +		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> +		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> +			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> +		mci_writel(host, CLKENA, clk_en_a);
>  
>  		/* inform CIU */
>  		mci_send_cmd(slot,
> @@ -862,6 +865,32 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>  	return present;
>  }
>  
> +/*
> + * Disable lower power mode.
> + *
> + * Low power mode will stop the card clock when idle.  According to
> + * documentation (Exynos 5250 User's Manual 0.04, description of
> + * CLKENA register) we should disable low power mode for SDIO cards
> + * if we need interrupts to work.
> + *
As Seungwon is mentioned, that is not exynos5250 specific.
> + * This function is fast if the power mode is already disabled.
> + */
> +static void dw_mci_disable_low_power(struct mmc_host *mmc)
> +{
> +	struct dw_mci_slot *slot = mmc_priv(mmc);
> +	struct dw_mci *host = slot->host;
> +	u32 clk_en_a;
> +	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> +
> +	clk_en_a = mci_readl(host, CLKENA);
> +
> +	if (clk_en_a & clken_low_pwr) {
> +		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
> +		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> +			     SDMMC_CMD_PRV_DAT_WAIT, 0);
> +	}
> +}
> +
>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  {
>  	struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -871,6 +900,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  	/* Enable/disable Slot Specific SDIO interrupt */
>  	int_mask = mci_readl(host, INTMASK);
>  	if (enb) {
> +		/*
> +		 * Turn off low power mode if it was enabled.  This is a bit of
> +		 * a heavy operation and we disable / enable IRQs a lot, so
> +		 * we'll leave low power mode disabled and it will get
> +		 * re-enabled again in dw_mci_setup_bus().
> +		 */
> +		dw_mci_disable_low_power(mmc);
How about using "slot" instead of "mmc"?
> +
>  		mci_writel(host, INTMASK,
>  			   (int_mask | SDMMC_INT_SDIO(slot->id)));
>  	} else {
> 

Re: [PATCH v2] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

On Mon, Jul 23, 2012 at 6:17 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> I have followed this patch from v1.
> I think it's a good point.
> Looks good to me.

Thanks for the review!  Looking forward to your ack with the next version.  :)

> Above comment is correct, but this is not specific for Exynos5250.
> Exynos5250 is just one of host controllers base on Synopsys's.
> It'd be better to remove the part related to Exynos you mentioned .

Done.  Grant had requested me to quote exactly which manual I found
the requirement in, but I think describing just that it was found in
the description of the CLKENA register is enough.  I've changed it.

> Definitely, low power mode not power mode, right?

Done.  Good catch!


-Doug

Re: [PATCH v2] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

On Mon, Jul 23, 2012 at 6:36 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> How about using "slot" instead of "mmc"?

Done.

[PATCH v3] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

The documentation for the dw_mmc part says that the low power
mode should normally only be set for MMC and SD memory and should
be turned off for SDIO cards that need interrupts detected.

The best place I could find to do this is when the SDIO interrupt
was first enabled.  I rely on the fact that dw_mci_setup_bus()
will be called when it's time to reenable.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v3:
- Commenting fixes requested by Seungwoon Jeon and Jaehoon Chung.
- Only pass 'slot' to the low power disable function since whole mmc
  structure wasn't needed.

Changes in v2:
- Commenting fixes requested by Grant Grundler.
- Be extra certain that we don't re-turn on the low power mode in
  CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
  There are no known instances of this happening but it's good to be safe.


 drivers/mmc/host/dw_mmc.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 72dc3cd..fbe5be3 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 {
 	struct dw_mci *host = slot->host;
 	u32 div;
+	u32 clk_en_a;
 
 	if (slot->clock != host->current_speed) {
 		div = host->bus_hz / slot->clock;
@@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 		mci_send_cmd(slot,
 			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
 
-		/* enable clock */
-		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
-			   SDMMC_CLKEN_LOW_PWR) << slot->id));
+		/* enable clock; only low power if no SDIO */
+		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
+		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
+		mci_writel(host, CLKENA, clk_en_a);
 
 		/* inform CIU */
 		mci_send_cmd(slot,
@@ -862,6 +865,30 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	return present;
 }
 
+/*
+ * Disable lower power mode.
+ *
+ * Low power mode will stop the card clock when idle.  According to the
+ * description of the CLKENA register we should disable low power mode
+ * for SDIO cards if we need SDIO interrupts to work.
+ *
+ * This function is fast if low power mode is already disabled.
+ */
+static void dw_mci_disable_low_power(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	u32 clk_en_a;
+	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
+
+	clk_en_a = mci_readl(host, CLKENA);
+
+	if (clk_en_a & clken_low_pwr) {
+		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
+		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+			     SDMMC_CMD_PRV_DAT_WAIT, 0);
+	}
+}
+
 static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -871,6 +898,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 	/* Enable/disable Slot Specific SDIO interrupt */
 	int_mask = mci_readl(host, INTMASK);
 	if (enb) {
+		/*
+		 * Turn off low power mode if it was enabled.  This is a bit of
+		 * a heavy operation and we disable / enable IRQs a lot, so
+		 * we'll leave low power mode disabled and it will get
+		 * re-enabled again in dw_mci_setup_bus().
+		 */
+		dw_mci_disable_low_power(mmc_priv(mmc));
+
 		mci_writel(host, INTMASK,
 			   (int_mask | SDMMC_INT_SDIO(slot->id)));
 	} else {
-- 
1.7.7.3

Re: [PATCH v3] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Will Newton]

On Tue, Jul 24, 2012 at 5:59 PM, Doug Anderson <dianders@chromium.org> wrote:
> The documentation for the dw_mmc part says that the low power
> mode should normally only be set for MMC and SD memory and should
> be turned off for SDIO cards that need interrupts detected.
>
> The best place I could find to do this is when the SDIO interrupt
> was first enabled.  I rely on the fact that dw_mci_setup_bus()
> will be called when it's time to reenable.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v3:
> - Commenting fixes requested by Seungwoon Jeon and Jaehoon Chung.
> - Only pass 'slot' to the low power disable function since whole mmc
>   structure wasn't needed.
>
> Changes in v2:
> - Commenting fixes requested by Grant Grundler.
> - Be extra certain that we don't re-turn on the low power mode in
>   CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
>   There are no known instances of this happening but it's good to be safe.
>
>
>  drivers/mmc/host/dw_mmc.c |   41 ++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 38 insertions(+), 3 deletions(-)

Acked-by: Will Newton <will.newton@imgtec.com>

Re: [PATCH v3] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Jaehoon Chung]

> +
>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  {
>  	struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -871,6 +898,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  	/* Enable/disable Slot Specific SDIO interrupt */
>  	int_mask = mci_readl(host, INTMASK);
>  	if (enb) {
> +		/*
> +		 * Turn off low power mode if it was enabled.  This is a bit of
> +		 * a heavy operation and we disable / enable IRQs a lot, so
> +		 * we'll leave low power mode disabled and it will get
> +		 * re-enabled again in dw_mci_setup_bus().
> +		 */
> +		dw_mci_disable_low_power(mmc_priv(mmc));
Just use the slot. slot is already assigned to mmc_priv(mmc)

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

Re: [PATCH v3] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

On Wed, Jul 25, 2012 at 3:02 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Just use the slot. slot is already assigned to mmc_priv(mmc)

Oops.  Now I feel sheepish.  Done.  :)

[PATCH v4] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Doug Anderson]

The documentation for the dw_mmc part says that the low power
mode should normally only be set for MMC and SD memory and should
be turned off for SDIO cards that need interrupts detected.

The best place I could find to do this is when the SDIO interrupt
was first enabled.  I rely on the fact that dw_mci_setup_bus()
will be called when it's time to reenable.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v4:
- Don't regenerate slot variable when we already had it.

Changes in v3:
- Commenting fixes requested by Seungwoon Jeon and Jaehoon Chung.
- Only pass 'slot' to the low power disable function since whole mmc
  structure wasn't needed.

Changes in v2:
- Commenting fixes requested by Grant Grundler.
- Be extra certain that we don't re-turn on the low power mode in
  CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
  There are no known instances of this happening but it's good to be safe.

 drivers/mmc/host/dw_mmc.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 72dc3cd..882748b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 {
 	struct dw_mci *host = slot->host;
 	u32 div;
+	u32 clk_en_a;
 
 	if (slot->clock != host->current_speed) {
 		div = host->bus_hz / slot->clock;
@@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
 		mci_send_cmd(slot,
 			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
 
-		/* enable clock */
-		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
-			   SDMMC_CLKEN_LOW_PWR) << slot->id));
+		/* enable clock; only low power if no SDIO */
+		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
+		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
+		mci_writel(host, CLKENA, clk_en_a);
 
 		/* inform CIU */
 		mci_send_cmd(slot,
@@ -862,6 +865,30 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
 	return present;
 }
 
+/*
+ * Disable lower power mode.
+ *
+ * Low power mode will stop the card clock when idle.  According to the
+ * description of the CLKENA register we should disable low power mode
+ * for SDIO cards if we need SDIO interrupts to work.
+ *
+ * This function is fast if low power mode is already disabled.
+ */
+static void dw_mci_disable_low_power(struct dw_mci_slot *slot)
+{
+	struct dw_mci *host = slot->host;
+	u32 clk_en_a;
+	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
+
+	clk_en_a = mci_readl(host, CLKENA);
+
+	if (clk_en_a & clken_low_pwr) {
+		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
+		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+			     SDMMC_CMD_PRV_DAT_WAIT, 0);
+	}
+}
+
 static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 {
 	struct dw_mci_slot *slot = mmc_priv(mmc);
@@ -871,6 +898,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
 	/* Enable/disable Slot Specific SDIO interrupt */
 	int_mask = mci_readl(host, INTMASK);
 	if (enb) {
+		/*
+		 * Turn off low power mode if it was enabled.  This is a bit of
+		 * a heavy operation and we disable / enable IRQs a lot, so
+		 * we'll leave low power mode disabled and it will get
+		 * re-enabled again in dw_mci_setup_bus().
+		 */
+		dw_mci_disable_low_power(slot);
+
 		mci_writel(host, INTMASK,
 			   (int_mask | SDMMC_INT_SDIO(slot->id)));
 	} else {
-- 
1.7.7.3

RE: [PATCH v4] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Seungwon Jeon]

July 26, 2012, Doug Anderson <dianders@chromium.org> wrote:
> The documentation for the dw_mmc part says that the low power
> mode should normally only be set for MMC and SD memory and should
> be turned off for SDIO cards that need interrupts detected.
> 
> The best place I could find to do this is when the SDIO interrupt
> was first enabled.  I rely on the fact that dw_mci_setup_bus()
> will be called when it's time to reenable.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v4:
> - Don't regenerate slot variable when we already had it.
> 
> Changes in v3:
> - Commenting fixes requested by Seungwoon Jeon and Jaehoon Chung.
> - Only pass 'slot' to the low power disable function since whole mmc
>   structure wasn't needed.
> 
> Changes in v2:
> - Commenting fixes requested by Grant Grundler.
> - Be extra certain that we don't re-turn on the low power mode in
>   CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
>   There are no known instances of this happening but it's good to be safe.
> 
>  drivers/mmc/host/dw_mmc.c |   41 ++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 72dc3cd..882748b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  {
>  	struct dw_mci *host = slot->host;
>  	u32 div;
> +	u32 clk_en_a;
> 
>  	if (slot->clock != host->current_speed) {
>  		div = host->bus_hz / slot->clock;
> @@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>  		mci_send_cmd(slot,
>  			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
> 
> -		/* enable clock */
> -		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
> -			   SDMMC_CLKEN_LOW_PWR) << slot->id));
> +		/* enable clock; only low power if no SDIO */
> +		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> +		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> +			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> +		mci_writel(host, CLKENA, clk_en_a);
> 
>  		/* inform CIU */
>  		mci_send_cmd(slot,
> @@ -862,6 +865,30 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>  	return present;
>  }
> 
> +/*
> + * Disable lower power mode.
> + *
> + * Low power mode will stop the card clock when idle.  According to the
> + * description of the CLKENA register we should disable low power mode
> + * for SDIO cards if we need SDIO interrupts to work.
> + *
> + * This function is fast if low power mode is already disabled.
> + */
> +static void dw_mci_disable_low_power(struct dw_mci_slot *slot)
> +{
> +	struct dw_mci *host = slot->host;
> +	u32 clk_en_a;
> +	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> +
> +	clk_en_a = mci_readl(host, CLKENA);
> +
> +	if (clk_en_a & clken_low_pwr) {
> +		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
> +		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> +			     SDMMC_CMD_PRV_DAT_WAIT, 0);
> +	}
> +}
> +
>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  {
>  	struct dw_mci_slot *slot = mmc_priv(mmc);
> @@ -871,6 +898,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>  	/* Enable/disable Slot Specific SDIO interrupt */
>  	int_mask = mci_readl(host, INTMASK);
>  	if (enb) {
> +		/*
> +		 * Turn off low power mode if it was enabled.  This is a bit of
> +		 * a heavy operation and we disable / enable IRQs a lot, so
> +		 * we'll leave low power mode disabled and it will get
> +		 * re-enabled again in dw_mci_setup_bus().
> +		 */
> +		dw_mci_disable_low_power(slot);
> +
>  		mci_writel(host, INTMASK,
>  			   (int_mask | SDMMC_INT_SDIO(slot->id)));
>  	} else {

Basically, disabling low power mode doesn't affect origin working.
This patch will guarantees to supply the clock while sdio interrupt is activated.
Looks good to me.
Chris, feel free to add my ack.

Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

> --
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] mmc: dw_mmc: Disable low power mode if SDIO interrupts are used

[Chris Ball]

Hi,

On Thu, Jul 26 2012, Seungwon Jeon wrote:
> July 26, 2012, Doug Anderson <dianders@chromium.org> wrote:
>> The documentation for the dw_mmc part says that the low power
>> mode should normally only be set for MMC and SD memory and should
>> be turned off for SDIO cards that need interrupts detected.
>> 
>> The best place I could find to do this is when the SDIO interrupt
>> was first enabled.  I rely on the fact that dw_mci_setup_bus()
>> will be called when it's time to reenable.
>> 
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>> Changes in v4:
>> - Don't regenerate slot variable when we already had it.
>> 
>> Changes in v3:
>> - Commenting fixes requested by Seungwoon Jeon and Jaehoon Chung.
>> - Only pass 'slot' to the low power disable function since whole mmc
>>   structure wasn't needed.
>> 
>> Changes in v2:
>> - Commenting fixes requested by Grant Grundler.
>> - Be extra certain that we don't re-turn on the low power mode in
>>   CLKENA in dw_mci_setup_bus() if SDIO interrupts are enabled.
>>   There are no known instances of this happening but it's good to be safe.
>> 
>>  drivers/mmc/host/dw_mmc.c |   41 ++++++++++++++++++++++++++++++++++++++---
>>  1 files changed, 38 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 72dc3cd..882748b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -627,6 +627,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>>  {
>>  	struct dw_mci *host = slot->host;
>>  	u32 div;
>> +	u32 clk_en_a;
>> 
>>  	if (slot->clock != host->current_speed) {
>>  		div = host->bus_hz / slot->clock;
>> @@ -659,9 +660,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>>  		mci_send_cmd(slot,
>>  			     SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> 
>> -		/* enable clock */
>> -		mci_writel(host, CLKENA, ((SDMMC_CLKEN_ENABLE |
>> -			   SDMMC_CLKEN_LOW_PWR) << slot->id));
>> +		/* enable clock; only low power if no SDIO */
>> +		clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> +		if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>> +			clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>> +		mci_writel(host, CLKENA, clk_en_a);
>> 
>>  		/* inform CIU */
>>  		mci_send_cmd(slot,
>> @@ -862,6 +865,30 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
>>  	return present;
>>  }
>> 
>> +/*
>> + * Disable lower power mode.
>> + *
>> + * Low power mode will stop the card clock when idle.  According to the
>> + * description of the CLKENA register we should disable low power mode
>> + * for SDIO cards if we need SDIO interrupts to work.
>> + *
>> + * This function is fast if low power mode is already disabled.
>> + */
>> +static void dw_mci_disable_low_power(struct dw_mci_slot *slot)
>> +{
>> +	struct dw_mci *host = slot->host;
>> +	u32 clk_en_a;
>> +	const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
>> +
>> +	clk_en_a = mci_readl(host, CLKENA);
>> +
>> +	if (clk_en_a & clken_low_pwr) {
>> +		mci_writel(host, CLKENA, clk_en_a & ~clken_low_pwr);
>> +		mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
>> +			     SDMMC_CMD_PRV_DAT_WAIT, 0);
>> +	}
>> +}
>> +
>>  static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>  {
>>  	struct dw_mci_slot *slot = mmc_priv(mmc);
>> @@ -871,6 +898,14 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>  	/* Enable/disable Slot Specific SDIO interrupt */
>>  	int_mask = mci_readl(host, INTMASK);
>>  	if (enb) {
>> +		/*
>> +		 * Turn off low power mode if it was enabled.  This is a bit of
>> +		 * a heavy operation and we disable / enable IRQs a lot, so
>> +		 * we'll leave low power mode disabled and it will get
>> +		 * re-enabled again in dw_mci_setup_bus().
>> +		 */
>> +		dw_mci_disable_low_power(slot);
>> +
>>  		mci_writel(host, INTMASK,
>>  			   (int_mask | SDMMC_INT_SDIO(slot->id)));
>>  	} else {
>
> Basically, disabling low power mode doesn't affect origin working.
> This patch will guarantees to supply the clock while sdio interrupt is activated.
> Looks good to me.
> Chris, feel free to add my ack.
>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Thanks, pushed to mmc-next for 3.7.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child