diff mbox

OMAP: HSMMC: Initialize hsmmc controller registers when resuming

Message ID 49A3EF96.3090603@nokia.com (mailing list archive)
State Awaiting Upstream, archived
Headers show

Commit Message

Adrian Hunter Feb. 24, 2009, 1:01 p.m. UTC
David Brownell wrote:
> On Monday 23 February 2009, Adrian Hunter wrote:
>> Also, you will need the following patch if you actually want the power
>> to go off.
> 
> Current GIT already has a patch supporting power-off for
> MMC2; tested on SDP and some custom hardware.  So this
> patch won't apply.

Sorry, hadn't noticed that.

> Are you sure that's needed for MMC1?  The led showing MMC1
> power did go off correctly (when using MMC2 for root), and
> the MMC1 regulator entry in sysfs agreed MMC1 was off.  So
> I thought this was only an issue for MMC2 (and presumably
> MMC3, though I don't have a board using it.)

Yes, if you never switch to 1.8V, then MMC1 is not affected.

However, for dual-voltage cards, if the MMC1 voltage is
switched to 1.8V, then the offending code will leave the
power on, when it should be off i.e. omap_mmc_switch_opcond()
turns the power on, but we are trying to power off.

If you initialise correctly (as Kim Kyuwon's patch does)
then the offending code is not necessary, so it should
just be removed.

I would still like the other two issues I raised considered.
They were:
	1.  'host' can be NULL in omap_mmc_suspend() / omap_mmc_resume()
	2. wait for SDBP bit

> I agree that code removed by this patch is ugly and worth
> removing if it's not actually needed for MMC1.

Here is a patch against current OMAP tree.

From: Adrian Hunter <ext-adrian.hunter@nokia.com>
Date: Tue, 24 Feb 2009 14:48:16 +0200
Subject: [PATCH] OMAP: HSMMC: do not re-power when powering off MMC

Remove code that turns MMC1 power back on after it
has been powered off (when the voltage is 1.8V).

The offending code is not necessary because the
host controller bus voltage is initialized to
3V when probing or resuming.  Note that MMC powers up
with the highest voltage available (see mmc_power_up())
which will be 3V also.

Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   17 -----------------
 1 files changed, 0 insertions(+), 17 deletions(-)

Comments

David Brownell Feb. 24, 2009, 10:10 p.m. UTC | #1
On Tuesday 24 February 2009, Adrian Hunter wrote:
> > I agree that code removed by this patch is ugly and worth
> > removing if it's not actually needed for MMC1.
> 
> Here is a patch against current OMAP tree.
> 
> From: Adrian Hunter <ext-adrian.hunter@nokia.com>
> Date: Tue, 24 Feb 2009 14:48:16 +0200
> Subject: [PATCH] OMAP: HSMMC: do not re-power when powering off MMC
> 
> Remove code that turns MMC1 power back on after it
> has been powered off (when the voltage is 1.8V).
> 
> The offending code is not necessary because the
> host controller bus voltage is initialized to
> 3V when probing or resuming.  Note that MMC powers up
> with the highest voltage available (see mmc_power_up())
> which will be 3V also.
> 
> Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>

Looks OK to me -- ack.  Safe to merge through the MMC
tree right away, but it'll be a NOP until the glue
actually supports 1.8V correctly for MMC1 ... so IMO
no rush to merge for 2.6.29-final.


> ---
>  drivers/mmc/host/omap_hsmmc.c |   17 -----------------
>  1 files changed, 0 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index c0d5420..43cec98 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -486,9 +486,6 @@ static int omap_mmc_switch_opcond(struct mmc_omap_host *host, int vdd)
>         u32 reg_val = 0;
>         int ret;
>  
> -       if (host->id != OMAP_MMC1_DEVID)
> -               return 0;
> -
>         /* Disable the clocks */
>         clk_disable(host->fclk);
>         clk_disable(host->iclk);
> @@ -787,20 +784,6 @@ static void omap_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>         switch (ios->power_mode) {
>         case MMC_POWER_OFF:
>                 mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
> -               /*
> -                * Reset interface voltage to 3V if it's 1.8V now;
> -                * only relevant on MMC-1, the others always use 1.8V.
> -                *
> -                * REVISIT: If we are able to detect cards after unplugging
> -                * a 1.8V card, this code should not be needed.
> -                */
> -               if (host->id != OMAP_MMC1_DEVID)
> -                       break;
> -               if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
> -                       int vdd = fls(host->mmc->ocr_avail) - 1;
> -                       if (omap_mmc_switch_opcond(host, vdd) != 0)
> -                               host->mmc->ios.vdd = vdd;
> -               }
>                 break;
>         case MMC_POWER_UP:
>                 mmc_slot(host).set_power(host->dev, host->slot_id, 1, ios->vdd);



--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Brownell Feb. 24, 2009, 10:12 p.m. UTC | #2
On Tuesday 24 February 2009, Adrian Hunter wrote:
> I would still like the other two issues I raised considered.
> They were:
>         1.  'host' can be NULL in omap_mmc_suspend() / omap_mmc_resume()
>         2. wait for SDBP bit

Someone from Nokia was going to be shepherding HSMMC patches,
as I recall... that person should deal with these issues.  :)

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Feb. 27, 2009, 10:08 p.m. UTC | #3
* David Brownell <david-b@pacbell.net> [090224 14:14]:
> On Tuesday 24 February 2009, Adrian Hunter wrote:
> > > I agree that code removed by this patch is ugly and worth
> > > removing if it's not actually needed for MMC1.
> > 
> > Here is a patch against current OMAP tree.
> > 
> > From: Adrian Hunter <ext-adrian.hunter@nokia.com>
> > Date: Tue, 24 Feb 2009 14:48:16 +0200
> > Subject: [PATCH] OMAP: HSMMC: do not re-power when powering off MMC
> > 
> > Remove code that turns MMC1 power back on after it
> > has been powered off (when the voltage is 1.8V).
> > 
> > The offending code is not necessary because the
> > host controller bus voltage is initialized to
> > 3V when probing or resuming.  Note that MMC powers up
> > with the highest voltage available (see mmc_power_up())
> > which will be 3V also.
> > 
> > Signed-off-by: Adrian Hunter <ext-adrian.hunter@nokia.com>
> 
> Looks OK to me -- ack.  Safe to merge through the MMC
> tree right away, but it'll be a NOP until the glue
> actually supports 1.8V correctly for MMC1 ... so IMO
> no rush to merge for 2.6.29-final.

Good to hear we get rid of that REVISIT part.. My ack here too.

Acked-by: Tony Lindgren <tony@atomide.com>
 
> > ---
> >  drivers/mmc/host/omap_hsmmc.c |   17 -----------------
> >  1 files changed, 0 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > index c0d5420..43cec98 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -486,9 +486,6 @@ static int omap_mmc_switch_opcond(struct mmc_omap_host *host, int vdd)
> >         u32 reg_val = 0;
> >         int ret;
> >  
> > -       if (host->id != OMAP_MMC1_DEVID)
> > -               return 0;
> > -
> >         /* Disable the clocks */
> >         clk_disable(host->fclk);
> >         clk_disable(host->iclk);
> > @@ -787,20 +784,6 @@ static void omap_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >         switch (ios->power_mode) {
> >         case MMC_POWER_OFF:
> >                 mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
> > -               /*
> > -                * Reset interface voltage to 3V if it's 1.8V now;
> > -                * only relevant on MMC-1, the others always use 1.8V.
> > -                *
> > -                * REVISIT: If we are able to detect cards after unplugging
> > -                * a 1.8V card, this code should not be needed.
> > -                */
> > -               if (host->id != OMAP_MMC1_DEVID)
> > -                       break;
> > -               if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
> > -                       int vdd = fls(host->mmc->ocr_avail) - 1;
> > -                       if (omap_mmc_switch_opcond(host, vdd) != 0)
> > -                               host->mmc->ios.vdd = vdd;
> > -               }
> >                 break;
> >         case MMC_POWER_UP:
> >                 mmc_slot(host).set_power(host->dev, host->slot_id, 1, ios->vdd);
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index c0d5420..43cec98 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -486,9 +486,6 @@  static int omap_mmc_switch_opcond(struct mmc_omap_host *host, int vdd)
 	u32 reg_val = 0;
 	int ret;
 
-	if (host->id != OMAP_MMC1_DEVID)
-		return 0;
-
 	/* Disable the clocks */
 	clk_disable(host->fclk);
 	clk_disable(host->iclk);
@@ -787,20 +784,6 @@  static void omap_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	switch (ios->power_mode) {
 	case MMC_POWER_OFF:
 		mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
-		/*
-		 * Reset interface voltage to 3V if it's 1.8V now;
-		 * only relevant on MMC-1, the others always use 1.8V.
-		 *
-		 * REVISIT: If we are able to detect cards after unplugging
-		 * a 1.8V card, this code should not be needed.
-		 */
-		if (host->id != OMAP_MMC1_DEVID)
-			break;
-		if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
-			int vdd = fls(host->mmc->ocr_avail) - 1;
-			if (omap_mmc_switch_opcond(host, vdd) != 0)
-				host->mmc->ios.vdd = vdd;
-		}
 		break;
 	case MMC_POWER_UP:
 		mmc_slot(host).set_power(host->dev, host->slot_id, 1, ios->vdd);