[PATCH] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[PATCH] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[Ethan Du]

If suspend called when kmmcd is doing host->ops->disable,
as kmmcd already increased host->en_dis_recurs to 1, the mmc_host_enable
in suspend function will return directly without increase the nesting_cnt,
which will cause the followed register access carried out to the disabled host.

mmc_suspend_host will enable host itself. No need to enable host before it.
Also works on kmmcd will get flushed in mmc_suspend_host, enable host after
it will be safe. So make the mmc_host_enable after it.

Signed-off-by: Ethan <ethan.too@gmail.com>
---
 drivers/mmc/host/omap_hsmmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index b032828..fc74263 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2291,8 +2291,8 @@ static int omap_hsmmc_suspend(struct device *dev)
                        }
                }
                cancel_work_sync(&host->mmc_carddetect_work);
-               mmc_host_enable(host->mmc);
                ret = mmc_suspend_host(host->mmc);
+               mmc_host_enable(host->mmc);
                if (ret == 0) {
                        omap_hsmmc_disable_irq(host);
                        OMAP_HSMMC_WRITE(host->base, HCTL,
--
1.5.6.5

Re: [PATCH] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[Adrian Hunter]

Ethan Du wrote:
> If suspend called when kmmcd is doing host->ops->disable,
> as kmmcd already increased host->en_dis_recurs to 1, the mmc_host_enable
> in suspend function will return directly without increase the nesting_cnt,
> which will cause the followed register access carried out to the disabled host.
> 
> mmc_suspend_host will enable host itself. No need to enable host before it.
> Also works on kmmcd will get flushed in mmc_suspend_host, enable host after
> it will be safe. So make the mmc_host_enable after it.
> 
> Signed-off-by: Ethan <ethan.too@gmail.com>

Looks good to me

Acked-by: Adrian Hunter <adrian.hunter@nokia.com>

> ---
>  drivers/mmc/host/omap_hsmmc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index b032828..fc74263 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -2291,8 +2291,8 @@ static int omap_hsmmc_suspend(struct device *dev)
>                         }
>                 }
>                 cancel_work_sync(&host->mmc_carddetect_work);
> -               mmc_host_enable(host->mmc);
>                 ret = mmc_suspend_host(host->mmc);
> +               mmc_host_enable(host->mmc);
>                 if (ret == 0) {
>                         omap_hsmmc_disable_irq(host);
>                         OMAP_HSMMC_WRITE(host->base, HCTL,
> --
> 1.5.6.5
> --
> 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] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[Adrian Hunter]

Hunter Adrian (Nokia-D/Helsinki) wrote:
> Ethan Du wrote:
>> If suspend called when kmmcd is doing host->ops->disable,
>> as kmmcd already increased host->en_dis_recurs to 1, the mmc_host_enable
>> in suspend function will return directly without increase the nesting_cnt,
>> which will cause the followed register access carried out to the disabled host.
>>
>> mmc_suspend_host will enable host itself. No need to enable host before it.
>> Also works on kmmcd will get flushed in mmc_suspend_host, enable host after
>> it will be safe. So make the mmc_host_enable after it.
>>
>> Signed-off-by: Ethan <ethan.too@gmail.com>
> 
> Looks good to me
> 
> Acked-by: Adrian Hunter <adrian.hunter@nokia.com>
> 

Ping?

Ethan asked me what is happening to this.

Is is OK Madhu?

Andrew can you take it?

>> ---
>>  drivers/mmc/host/omap_hsmmc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index b032828..fc74263 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -2291,8 +2291,8 @@ static int omap_hsmmc_suspend(struct device *dev)
>>                         }
>>                 }
>>                 cancel_work_sync(&host->mmc_carddetect_work);
>> -               mmc_host_enable(host->mmc);
>>                 ret = mmc_suspend_host(host->mmc);
>> +               mmc_host_enable(host->mmc);
>>                 if (ret == 0) {
>>                         omap_hsmmc_disable_irq(host);
>>                         OMAP_HSMMC_WRITE(host->base, HCTL,
>> --
>> 1.5.6.5
>> --
>> 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
>>
> 
> --
> 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] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[Madhusudhan]

> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@nokia.com]
> Sent: Tuesday, July 06, 2010 1:11 PM
> To: Madhusudhan Chikkature
> Cc: Hunter Adrian (Nokia-D/Helsinki); Ethan Du; linux-mmc; Andrew Morton
> Subject: Re: [PATCH] OMAP HSMMC: fix a racing case between kmmcd and
> omap_hsmmc_suspend
> 
> Hunter Adrian (Nokia-D/Helsinki) wrote:
> > Ethan Du wrote:
> >> If suspend called when kmmcd is doing host->ops->disable,
> >> as kmmcd already increased host->en_dis_recurs to 1, the
> mmc_host_enable
> >> in suspend function will return directly without increase the
> nesting_cnt,
> >> which will cause the followed register access carried out to the
> disabled host.
> >>
> >> mmc_suspend_host will enable host itself. No need to enable host before
> it.
> >> Also works on kmmcd will get flushed in mmc_suspend_host, enable host
> after
> >> it will be safe. So make the mmc_host_enable after it.
> >>
> >> Signed-off-by: Ethan <ethan.too@gmail.com>
> >
> > Looks good to me
> >
> > Acked-by: Adrian Hunter <adrian.hunter@nokia.com>
> >
> 
> Ping?
> 
> Ethan asked me what is happening to this.
> 
> Is is OK Madhu?
> 
> Andrew can you take it?
> 
Sorry, I missed to look at this patch. It looks good.

Acked-by: Madhusudhan Chikkature <madhu.cr@ti.com>

> >> ---
> >>  drivers/mmc/host/omap_hsmmc.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/mmc/host/omap_hsmmc.c
> b/drivers/mmc/host/omap_hsmmc.c
> >> index b032828..fc74263 100644
> >> --- a/drivers/mmc/host/omap_hsmmc.c
> >> +++ b/drivers/mmc/host/omap_hsmmc.c
> >> @@ -2291,8 +2291,8 @@ static int omap_hsmmc_suspend(struct device *dev)
> >>                         }
> >>                 }
> >>                 cancel_work_sync(&host->mmc_carddetect_work);
> >> -               mmc_host_enable(host->mmc);
> >>                 ret = mmc_suspend_host(host->mmc);
> >> +               mmc_host_enable(host->mmc);
> >>                 if (ret == 0) {
> >>                         omap_hsmmc_disable_irq(host);
> >>                         OMAP_HSMMC_WRITE(host->base, HCTL,
> >> --
> >> 1.5.6.5
> >> --
> >> 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
> >>
> >
> > --
> > 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
> >

[PATCH RESEND] OMAP HSMMC: fix a racing case between kmmcd and omap_hsmmc_suspend

[Chris Ball]

Hi,

This doesn't seem to have been merged -- Andrew, could you take it?

From: Ethan Du <ethan.too@gmail.com>
Date: Mon, 21 Jun 2010 19:20:41 +0800

If suspend called when kmmcd is doing host->ops->disable,
as kmmcd already increased host->en_dis_recurs to 1, the mmc_host_enable
in suspend function will return directly without increase the nesting_cnt,
which will cause the followed register access carried out to the disabled host.

mmc_suspend_host will enable host itself. No need to enable host before it.
Also works on kmmcd will get flushed in mmc_suspend_host, enable host after
it will be safe. So make the mmc_host_enable after it.

Signed-off-by: Ethan <ethan.too@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@nokia.com>
Acked-by: Madhusudhan Chikkature <madhu.cr@ti.com>
[cjb: rebase against current Linus]
---
 drivers/mmc/host/omap_hsmmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index ba623d6..1f7d01a 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2323,8 +2323,8 @@ static int omap_hsmmc_suspend(struct device *dev)
 			}
 		}
 		cancel_work_sync(&host->mmc_carddetect_work);
-		mmc_host_enable(host->mmc);
 		ret = mmc_suspend_host(host->mmc);
+		mmc_host_enable(host->mmc);
 		if (ret == 0) {
 			omap_hsmmc_disable_irq(host);
 			OMAP_HSMMC_WRITE(host->base, HCTL,
--
1.7.0.1

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