[PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Adrian Hunter]

Do not access host in omap_mmc_remove() if it is null.  This
occurs when module is removed before its platform data is set.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4bc25c0..f4b7c30 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1128,15 +1128,17 @@ static int omap_mmc_remove(struct platform_device *pdev)
 	struct resource *res;
 	u16 vdd = 0;
 
-	mmc_omap_fclk_state(host, ON);
-	if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
-	/*
-	 * Set the vdd back to 3V,
-	 * applicable for dual volt support.
-	 */
-		vdd = fls(host->mmc->ocr_avail) - 1;
-		if (omap_mmc_switch_opcond(host, vdd) != 0)
-			host->mmc->ios.vdd = vdd;
+	if (host) {
+		mmc_omap_fclk_state(host, ON);
+		if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
+			/*
+			 * Set the vdd back to 3V,
+			 * applicable for dual volt support.
+			 */
+			vdd = fls(host->mmc->ocr_avail) - 1;
+			if (omap_mmc_switch_opcond(host, vdd) != 0)
+				host->mmc->ios.vdd = vdd;
+		}
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.5.4.3

[PATCH 0/9] OMAP: HSMMC: Patches

[Adrian Hunter]

Hi

OMAP HSMMC is not ready for mainline, but development is ongoing.

In the meantime these patches are best posted and applied via
omap mailing list and tree.

Regards
Adrian

Re: [PATCH 0/9] OMAP: HSMMC: Patches

[Adrian Hunter]

Adrian Hunter wrote:
> Hi
> 
> OMAP HSMMC is not ready for mainline, but development is ongoing.
> 
> In the meantime these patches are best posted and applied via
> omap mailing list and tree.
> 
> Regards
> Adrian
> -- 
> 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
> 

Just realised I forgot the from lines in all those patches - will resend.

[PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Adrian Hunter]

From: Jarkko Lavinen <jarkko.lavinen@nokia.com>
Date: Thu, 6 Nov 2008 16:02:29 +0200

Do not access host in omap_mmc_remove() if it is null.  This
occurs when module is removed before its platform data is set.

This fixes the oops part in bug 86677.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4bc25c0..f4b7c30 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1128,15 +1128,17 @@ static int omap_mmc_remove(struct platform_device *pdev)
 	struct resource *res;
 	u16 vdd = 0;
 
-	mmc_omap_fclk_state(host, ON);
-	if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
-	/*
-	 * Set the vdd back to 3V,
-	 * applicable for dual volt support.
-	 */
-		vdd = fls(host->mmc->ocr_avail) - 1;
-		if (omap_mmc_switch_opcond(host, vdd) != 0)
-			host->mmc->ios.vdd = vdd;
+	if (host) {
+		mmc_omap_fclk_state(host, ON);
+		if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
+			/*
+			 * Set the vdd back to 3V,
+			 * applicable for dual volt support.
+			 */
+			vdd = fls(host->mmc->ocr_avail) - 1;
+			if (omap_mmc_switch_opcond(host, vdd) != 0)
+				host->mmc->ios.vdd = vdd;
+		}
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.5.4.3

Re: [PATCH 0/9] OMAP: HSMMC: Patches

[Adrian Hunter]

Adrian Hunter wrote:
> Adrian Hunter wrote:
>> OMAP HSMMC is not ready for mainline, but development is ongoing.
>>
>> In the meantime these patches are best posted and applied via
>> omap mailing list and tree.
>
> Just realised I forgot the from lines in all those patches - will resend.
>

And now 4 and 5 again without the spurious bug references...

[PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Adrian Hunter]

From: Jarkko Lavinen <jarkko.lavinen@nokia.com>
Date: Thu, 6 Nov 2008 16:02:29 +0200

Do not access host in omap_mmc_remove() if it is null.  This
occurs when module is removed before its platform data is set.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@nokia.com>
---
 drivers/mmc/host/omap_hsmmc.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4bc25c0..f4b7c30 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1128,15 +1128,17 @@ static int omap_mmc_remove(struct platform_device *pdev)
 	struct resource *res;
 	u16 vdd = 0;
 
-	mmc_omap_fclk_state(host, ON);
-	if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
-	/*
-	 * Set the vdd back to 3V,
-	 * applicable for dual volt support.
-	 */
-		vdd = fls(host->mmc->ocr_avail) - 1;
-		if (omap_mmc_switch_opcond(host, vdd) != 0)
-			host->mmc->ios.vdd = vdd;
+	if (host) {
+		mmc_omap_fclk_state(host, ON);
+		if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
+			/*
+			 * Set the vdd back to 3V,
+			 * applicable for dual volt support.
+			 */
+			vdd = fls(host->mmc->ocr_avail) - 1;
+			if (omap_mmc_switch_opcond(host, vdd) != 0)
+				host->mmc->ios.vdd = vdd;
+		}
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.5.4.3

Re: [PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Grazvydas Ignotas]

On Fri, Nov 21, 2008 at 11:58 AM, Adrian Hunter
<ext-adrian.hunter@nokia.com> wrote:
<snip>
> @@ -1128,15 +1128,17 @@ static int omap_mmc_remove(struct platform_device
<snip>
> +               mmc_omap_fclk_state(host, ON);
> +               if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
> +                       /*
> +                        * Set the vdd back to 3V,
> +                        * applicable for dual volt support.
> +                        */
> +                       vdd = fls(host->mmc->ocr_avail) - 1;
> +                       if (omap_mmc_switch_opcond(host, vdd) != 0)
> +                               host->mmc->ios.vdd = vdd;
> +               }

I wonder what this block is for? Wouldn't simple
   host->mmc->ios.vdd = vdd;
be enough? It calls omap_mmc_switch_opcond() which toggles clocks and
power without good reason (I think). Clocks will be turned off later
in omap_mmc_remove() anyway, and .set_ios will turn off power. Also,
HCTL SDVSDET bit will never be set for MMC2 and MMC3, causing
mentioned omap_mmc_switch_opcond() to be called unconditionally here.

The same block can be found in mmc_omap_detect() function, where it is
executed on card removal. There it causes MMC2 controller to stop
working completely on pandora board :(

Re: [PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Jarkko Lavinen]

On Fri, Nov 21, 2008 at 04:03:25PM +0200, ext Grazvydas Ignotas wrote:
...
> Also, HCTL SDVSDET bit will never be set for MMC2 and MMC3, causing
> mentioned omap_mmc_switch_opcond() to be called unconditionally here.
> 
> The same block can be found in mmc_omap_detect() function, where it is
> executed on card removal. There it causes MMC2 controller to stop
> working completely on pandora board :(

TI TRM saus HSMMC2 and HSMMC3 works only with SDVS set to 1.8V in HCTL.

If the voltage is set to 3.0V and then SDBP (bus power bit) is set,
it is rejected and SDBP remains zero, the controller does nothing and 
is seemingly frozen.

The wrong voltage for MMC2 SDVS is set in omap_mmc_switch_opcond() if 
vdd different from 1.8V.

Also omap_mmc_suspend() set SDVS to 3.0V even for MMC2 and MMC3.  When
suspending, everything seemed to work and at resume MMC2 was hung
and failed to detect the card since not even CMD0 was sent.

Regards
Jarkko Lavinen

Re: [PATCH 5/9] OMAP: HSMMC: Fix oops in omap_mmc_remove

[Grazvydas Ignotas]

On Fri, Nov 21, 2008 at 5:27 PM, Jarkko Lavinen
<jarkko.lavinen@nokia.com> wrote:
> On Fri, Nov 21, 2008 at 04:03:25PM +0200, ext Grazvydas Ignotas wrote:
> ...
>> Also, HCTL SDVSDET bit will never be set for MMC2 and MMC3, causing
>> mentioned omap_mmc_switch_opcond() to be called unconditionally here.
>>
>> The same block can be found in mmc_omap_detect() function, where it is
>> executed on card removal. There it causes MMC2 controller to stop
>> working completely on pandora board :(
>
> TI TRM saus HSMMC2 and HSMMC3 works only with SDVS set to 1.8V in HCTL.
>
> If the voltage is set to 3.0V and then SDBP (bus power bit) is set,
> it is rejected and SDBP remains zero, the controller does nothing and
> is seemingly frozen.
>
> The wrong voltage for MMC2 SDVS is set in omap_mmc_switch_opcond() if
> vdd different from 1.8V.
>
> Also omap_mmc_suspend() set SDVS to 3.0V even for MMC2 and MMC3.  When
> suspending, everything seemed to work and at resume MMC2 was hung
> and failed to detect the card since not even CMD0 was sent.

ok, but why there are calls to  omap_mmc_switch_opcond() from both
omap_mmc_remove() and mmc_omap_detect() at all? This basically causes
power and clocks to be turned off, then on, then again off whenever
card is or host driver removed. This is probably no big deal, but
still.