diff mbox

mmc: sdhci: fix retuning timer wrongly deleted in sdhci_tasklet_finish

Message ID 1310362031-8186-1-git-send-email-Aaron.Lu@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

aaron lu July 11, 2011, 5:27 a.m. UTC
Currently, the retuning timer for retuning mode 1 will be deleted in
function sdhci_tasklet_finish after a mmc request done, which will make
retuning timing never trigger again. This patch fixed this problem.

Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
---
 drivers/mmc/host/sdhci.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

Comments

Arindam Nath July 15, 2011, 6:51 a.m. UTC | #1
Hi Philip, Zhangfei

Do you have any comments on this patch?

Thanks,
Arindam

> -----Original Message-----
> From: Aaron Lu [mailto:Aaron.Lu@amd.com]
> Sent: Monday, July 11, 2011 10:57 AM
> To: Chris Ball
> Cc: Nath, Arindam; linux-mmc@vger.kernel.org; Lu, Aaron
> Subject: [PATCH] mmc: sdhci: fix retuning timer wrongly deleted in
> sdhci_tasklet_finish
> 
> Currently, the retuning timer for retuning mode 1 will be deleted in
> function sdhci_tasklet_finish after a mmc request done, which will make
> retuning timing never trigger again. This patch fixed this problem.
> 
> Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
> ---
>  drivers/mmc/host/sdhci.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 91d9892..6250bac 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
> param)
> 
>  	del_timer(&host->timer);
> 
> -	if (host->version >= SDHCI_SPEC_300)
> -		del_timer(&host->tuning_timer);
> -
>  	mrq = host->mrq;
> 
>  	/*
> --
> 1.7.1


--
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
aaron lu July 21, 2011, 5:23 a.m. UTC | #2
On Fri, Jul 15, 2011 at 02:51:34PM +0800, Nath, Arindam wrote:
> Hi Philip, Zhangfei
> 
> Do you have any comments on this patch?
>

Hi all,

This patch is sent for a while now and didn't seem to receive any
notice yet...So, can you please take a look and give your comments?

This is a bug fix for exsting code, if the re-tuning timer is deleted,
the re-tuning will not happen again, that will cause problems for SDHC
3.0 hosts which utilize re-tuning mode 1.

Thanks.
 
> 
> > -----Original Message-----
> > From: Aaron Lu [mailto:Aaron.Lu@amd.com]
> > Sent: Monday, July 11, 2011 10:57 AM
> > To: Chris Ball
> > Cc: Nath, Arindam; linux-mmc@vger.kernel.org; Lu, Aaron
> > Subject: [PATCH] mmc: sdhci: fix retuning timer wrongly deleted in
> > sdhci_tasklet_finish
> > 
> > Currently, the retuning timer for retuning mode 1 will be deleted in
> > function sdhci_tasklet_finish after a mmc request done, which will
> make
> > retuning timing never trigger again. This patch fixed this problem.
> > 
> > Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
> > ---
> >  drivers/mmc/host/sdhci.c |    3 ---
> >  1 files changed, 0 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 91d9892..6250bac 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
> > param)
> > 
> >  	del_timer(&host->timer);
> > 
> > -	if (host->version >= SDHCI_SPEC_300)
> > -		del_timer(&host->tuning_timer);
> > -
> >  	mrq = host->mrq;
> > 
> >  	/*
> > --
> > 1.7.1
> 
> 

--
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
Zhangfei Gao July 21, 2011, 9:35 a.m. UTC | #3
> This is a bug fix for exsting code, if the re-tuning timer is deleted,
> the re-tuning will not happen again, that will cause problems for SDHC
> 3.0 hosts which utilize re-tuning mode 1.
>
> Thanks.
>
>>
>> > -----Original Message-----
>> > From: Aaron Lu [mailto:Aaron.Lu@amd.com]
>> > Sent: Monday, July 11, 2011 10:57 AM
>> > To: Chris Ball
>> > Cc: Nath, Arindam; linux-mmc@vger.kernel.org; Lu, Aaron
>> > Subject: [PATCH] mmc: sdhci: fix retuning timer wrongly deleted in
>> > sdhci_tasklet_finish
>> >
>> > Currently, the retuning timer for retuning mode 1 will be deleted in
>> > function sdhci_tasklet_finish after a mmc request done, which will
>> make
>> > retuning timing never trigger again. This patch fixed this problem.

Does the execute_tuning is called again?
del_timer is not delete timer really, but deactivate the timer, which
could be re-activated by mod_timer.
So if execute_tuning is called, the mod_timer will tigger the tuning
timer again.

>> >
>> > Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
>> > ---
>> >  drivers/mmc/host/sdhci.c |    3 ---
>> >  1 files changed, 0 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> > index 91d9892..6250bac 100644
>> > --- a/drivers/mmc/host/sdhci.c
>> > +++ b/drivers/mmc/host/sdhci.c
>> > @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
>> > param)
>> >
>> >     del_timer(&host->timer);
>> >
>> > -   if (host->version >= SDHCI_SPEC_300)
>> > -           del_timer(&host->tuning_timer);
>> > -
>> >     mrq = host->mrq;
>> >
>> >     /*
>> > --
>> > 1.7.1
>>
>>
>
>
--
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
aaron lu July 21, 2011, 10:03 a.m. UTC | #4
On Thu, Jul 21, 2011 at 05:35:02PM +0800, zhangfei gao wrote:
> 
> Does the execute_tuning is called again?
> del_timer is not delete timer really, but deactivate the timer, which
> could be re-activated by mod_timer.
> So if execute_tuning is called, the mod_timer will tigger the tuning
> timer again.
>

Hi zhangfei,

Thanks for the comment.

The execute_tuning will be called at two places:
1 In mmc_sd_init_uhs_card, when host is initializing an UHS card,
and the re-tuning timer will be activated for the first time;
2 When re-tuning timer expired

So if the re-tuning timer is deactivated in sdhci_tasklet_finish,
execute_tuning will have no chance of getting called again, and the
host will not be able to do the re-tuning anymore.
 
> >> >
> >> > Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
> >> > ---
> >> >  drivers/mmc/host/sdhci.c |    3 ---
> >> >  1 files changed, 0 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> > index 91d9892..6250bac 100644
> >> > --- a/drivers/mmc/host/sdhci.c
> >> > +++ b/drivers/mmc/host/sdhci.c
> >> > @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
> >> > param)
> >> >
> >> >     del_timer(&host->timer);
> >> >
> >> > -   if (host->version >= SDHCI_SPEC_300)
> >> > -           del_timer(&host->tuning_timer);
> >> > -
> >> >     mrq = host->mrq;
> >> >
> >> >     /*
> >> > --
> >> > 1.7.1
> >>
> >>
> >
> >
> 

--
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
Zhangfei Gao July 22, 2011, 10:21 a.m. UTC | #5
On Thu, Jul 21, 2011 at 6:03 PM, Aaron Lu <aaron.lu@amd.com> wrote:
> On Thu, Jul 21, 2011 at 05:35:02PM +0800, zhangfei gao wrote:
>>
>> Does the execute_tuning is called again?
>> del_timer is not delete timer really, but deactivate the timer, which
>> could be re-activated by mod_timer.
>> So if execute_tuning is called, the mod_timer will tigger the tuning
>> timer again.
>>
>
> Hi zhangfei,
>
> Thanks for the comment.
>
> The execute_tuning will be called at two places:
> 1 In mmc_sd_init_uhs_card, when host is initializing an UHS card,
> and the re-tuning timer will be activated for the first time;
> 2 When re-tuning timer expired
>
> So if the re-tuning timer is deactivated in sdhci_tasklet_finish,
> execute_tuning will have no chance of getting called again, and the
> host will not be able to do the re-tuning anymore.

Thanks for explanation, looks good to me.

>
>> >> >
>> >> > Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
>> >> > ---
>> >> >  drivers/mmc/host/sdhci.c |    3 ---
>> >> >  1 files changed, 0 insertions(+), 3 deletions(-)
>> >> >
>> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> >> > index 91d9892..6250bac 100644
>> >> > --- a/drivers/mmc/host/sdhci.c
>> >> > +++ b/drivers/mmc/host/sdhci.c
>> >> > @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
>> >> > param)
>> >> >
>> >> >     del_timer(&host->timer);
>> >> >
>> >> > -   if (host->version >= SDHCI_SPEC_300)
>> >> > -           del_timer(&host->tuning_timer);
>> >> > -
>> >> >     mrq = host->mrq;
>> >> >
>> >> >     /*
>> >> > --
>> >> > 1.7.1
>> >>
>> >>
>> >
>> >
>>
>
>
--
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
aaron lu July 27, 2011, 9:15 a.m. UTC | #6
On Fri, Jul 22, 2011 at 06:21:11PM +0800, zhangfei gao wrote:
> 
> Thanks for explanation, looks good to me.
> 

Hi Philip & Zhangfei,

Can I have your reviewed-by tag in the patch?

Thanks,
Aaron

> >> >> >
> >> >> > Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
> >> >> > ---
> >> >> >  drivers/mmc/host/sdhci.c |    3 ---
> >> >> >  1 files changed, 0 insertions(+), 3 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> >> > index 91d9892..6250bac 100644
> >> >> > --- a/drivers/mmc/host/sdhci.c
> >> >> > +++ b/drivers/mmc/host/sdhci.c
> >> >> > @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long
> >> >> > param)
> >> >> >
> >> >> >     del_timer(&host->timer);
> >> >> >
> >> >> > -   if (host->version >= SDHCI_SPEC_300)
> >> >> > -           del_timer(&host->tuning_timer);
> >> >> > -
> >> >> >     mrq = host->mrq;
> >> >> >
> >> >> >     /*
> >> >> > --
> >> >> > 1.7.1
> >> >>
> >> >>
> >> >
> >> >
> >>
> >
> >
> 

--
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
Philip Rakity July 27, 2011, 2:56 p.m. UTC | #7
Reviewed-by: Philip Rakity  <prakity@marvell.com>
Chris Ball July 28, 2011, 10:28 p.m. UTC | #8
Hi Aaron,

On Mon, Jul 11 2011, Aaron Lu wrote:
> Currently, the retuning timer for retuning mode 1 will be deleted in
> function sdhci_tasklet_finish after a mmc request done, which will make
> retuning timing never trigger again. This patch fixed this problem.
>
> Signed-off-by: Aaron Lu <Aaron.Lu@amd.com>
> ---
>  drivers/mmc/host/sdhci.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 91d9892..6250bac 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1863,9 +1863,6 @@ static void sdhci_tasklet_finish(unsigned long param)
>  
>  	del_timer(&host->timer);
>  
> -	if (host->version >= SDHCI_SPEC_300)
> -		del_timer(&host->tuning_timer);
> -
>  	mrq = host->mrq;
>  
>  	/*

Pushed to mmc-next for 3.1 with Philip's Reviewed-by, thanks.

- Chris.
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 91d9892..6250bac 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1863,9 +1863,6 @@  static void sdhci_tasklet_finish(unsigned long param)
 
 	del_timer(&host->timer);
 
-	if (host->version >= SDHCI_SPEC_300)
-		del_timer(&host->tuning_timer);
-
 	mrq = host->mrq;
 
 	/*