* [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context. @ 2013-08-01 2:18 ` majianpeng 0 siblings, 0 replies; 5+ messages in thread From: majianpeng @ 2013-08-01 2:18 UTC (permalink / raw) To: balajitk, cjb; +Cc: mayuzheng, linux-mmc, linux-omap, linux-kernel We found a problem when we removed a working sd card that the irqaction of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 transition.It used loops_per_jiffy as the timer. The code is: > while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > && (i++ < limit)) > cpu_relax(); But the loops_per_jiffy is: > while(i++ < limit) > cpu_relax(); It add some codes so the time became long. Becasue those codes in ISR context, it can't use timer_before/after. I divived the time into 1ms and used udelay(1) to instead. It will cause do additional udelay(1).But from my test,it looks good. Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> --- drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 1865321..96daca1 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, unsigned long limit = (loops_per_jiffy * msecs_to_jiffies(MMC_TIMEOUT_MS)); + /*Divided time into us for unit 1,we can use udelay(1)*/ + i = limit / (MMC_TIMEOUT_MS * 1000); OMAP_HSMMC_WRITE(host->base, SYSCTL, OMAP_HSMMC_READ(host->base, SYSCTL) | bit); @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, * Monitor a 0->1 transition first */ if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) - && (i++ < limit)) - cpu_relax(); + while (i--) { + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) + break; + udelay(1); + } } - i = 0; - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && - (i++ < limit)) - cpu_relax(); + i = limit / (MMC_TIMEOUT_MS * 1000); + while (i--) { + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) + break; + udealy(1); + } if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) dev_err(mmc_dev(host->mmc), -- 1.8.1.2 Thanks! Jianpeng Ma ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context. @ 2013-08-01 2:18 ` majianpeng 0 siblings, 0 replies; 5+ messages in thread From: majianpeng @ 2013-08-01 2:18 UTC (permalink / raw) To: balajitk, cjb; +Cc: mayuzheng, linux-mmc, linux-omap, linux-kernel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="gb2312", Size: 2429 bytes --] We found a problem when we removed a working sd card that the irqaction of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 transition.It used loops_per_jiffy as the timer. The code is: > while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > && (i++ < limit)) > cpu_relax(); But the loops_per_jiffy is: > while(i++ < limit) > cpu_relax(); It add some codes so the time became long. Becasue those codes in ISR context, it can't use timer_before/after. I divived the time into 1ms and used udelay(1) to instead. It will cause do additional udelay(1).But from my test,it looks good. Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> --- drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 1865321..96daca1 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, unsigned long limit = (loops_per_jiffy * msecs_to_jiffies(MMC_TIMEOUT_MS)); + /*Divided time into us for unit 1,we can use udelay(1)*/ + i = limit / (MMC_TIMEOUT_MS * 1000); OMAP_HSMMC_WRITE(host->base, SYSCTL, OMAP_HSMMC_READ(host->base, SYSCTL) | bit); @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, * Monitor a 0->1 transition first */ if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) - && (i++ < limit)) - cpu_relax(); + while (i--) { + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) + break; + udelay(1); + } } - i = 0; - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && - (i++ < limit)) - cpu_relax(); + i = limit / (MMC_TIMEOUT_MS * 1000); + while (i--) { + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) + break; + udealy(1); + } if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) dev_err(mmc_dev(host->mmc), -- 1.8.1.2 Thanks! Jianpeng Maÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context. 2013-08-01 2:18 ` majianpeng (?) @ 2013-08-01 8:22 ` Hein Tibosch 2013-08-01 8:40 ` majianpeng -1 siblings, 1 reply; 5+ messages in thread From: Hein Tibosch @ 2013-08-01 8:22 UTC (permalink / raw) To: majianpeng Cc: balajitk, cjb, mayuzheng, linux-mmc, linux-omap, linux-kernel, Felipe Balbi Hi Jianpeng Ma, On 8/1/2013 10:18 AM, majianpeng wrote: > We found a problem when we removed a working sd card that the irqaction > of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. > In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 > transition.It used loops_per_jiffy as the timer. > The code is: >> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> && (i++ < limit)) >> cpu_relax(); > But the loops_per_jiffy is: >> while(i++ < limit) >> cpu_relax(); > It add some codes so the time became long. > Becasue those codes in ISR context, it can't use timer_before/after. > I divived the time into 1ms and used udelay(1) to instead. > It will cause do additional udelay(1).But from my test,it looks good. > > Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> > Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> > Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> > --- > drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > index 1865321..96daca1 100644 > --- a/drivers/mmc/host/omap_hsmmc.c > +++ b/drivers/mmc/host/omap_hsmmc.c > @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, > unsigned long limit = (loops_per_jiffy * > msecs_to_jiffies(MMC_TIMEOUT_MS)); > > + /*Divided time into us for unit 1,we can use udelay(1)*/ > + i = limit / (MMC_TIMEOUT_MS * 1000); 'limit' is a number of loops, which you now divide by 20,000? To get uS, you could just change: - unsigned long limit = (loops_per_jiffy * - msecs_to_jiffies(MMC_TIMEOUT_MS)); + unsigned long limit = 1000 * MMC_TIMEOUT_MS; and make this amount of loops using udelay(). > OMAP_HSMMC_WRITE(host->base, SYSCTL, > OMAP_HSMMC_READ(host->base, SYSCTL) | bit); > > @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, > * Monitor a 0->1 transition first > */ > if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { > - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > - && (i++ < limit)) > - cpu_relax(); I still don't see why any of these loops could last 3.6 seconds? Yes the __raw_readl() will add some time, but so much? I'd like to see which value you get for 'limit' on your machine Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated on time? > + while (i--) { > + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > + break; > + udelay(1); In earlier threads, the use of udelay was disliked because it's a waste of cpu cycles. The desired bit in SYSCTL will change, while udelay() is still making many useless loops. > + } > } > - i = 0; > > - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && > - (i++ < limit)) > - cpu_relax(); > + i = limit / (MMC_TIMEOUT_MS * 1000); > + while (i--) { > + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > + break; > + udealy(1); > + } > > if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) > dev_err(mmc_dev(host->mmc), Hein ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context. 2013-08-01 8:22 ` Hein Tibosch @ 2013-08-01 8:40 ` majianpeng 0 siblings, 0 replies; 5+ messages in thread From: majianpeng @ 2013-08-01 8:40 UTC (permalink / raw) To: Hein Tibosch Cc: balajitk, cjb, mayuzheng, linux-mmc, linux-omap, linux-kernel, Felipe Balbi >Hi Jianpeng Ma, > >On 8/1/2013 10:18 AM, majianpeng wrote: >> We found a problem when we removed a working sd card that the irqaction >> of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. >> In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 >> transition.It used loops_per_jiffy as the timer. >> The code is: >>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>> && (i++ < limit)) >>> cpu_relax(); >> But the loops_per_jiffy is: >>> while(i++ < limit) >>> cpu_relax(); >> It add some codes so the time became long. >> Becasue those codes in ISR context, it can't use timer_before/after. >> I divived the time into 1ms and used udelay(1) to instead. >> It will cause do additional udelay(1).But from my test,it looks good. >> >> Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >> --- >> drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- >> 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 1865321..96daca1 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> unsigned long limit = (loops_per_jiffy * >> msecs_to_jiffies(MMC_TIMEOUT_MS)); >> >> + /*Divided time into us for unit 1,we can use udelay(1)*/ >> + i = limit / (MMC_TIMEOUT_MS * 1000); > >'limit' is a number of loops, which you now divide by 20,000? > >To get uS, you could just change: > >- unsigned long limit = (loops_per_jiffy * >- msecs_to_jiffies(MMC_TIMEOUT_MS)); >+ unsigned long limit = 1000 * MMC_TIMEOUT_MS; > Yes, you are right. >and make this amount of loops using udelay(). > >> OMAP_HSMMC_WRITE(host->base, SYSCTL, >> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); >> >> @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> * Monitor a 0->1 transition first >> */ >> if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { >> - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> - && (i++ < limit)) >> - cpu_relax(); > >I still don't see why any of these loops could last 3.6 seconds? >Yes the __raw_readl() will add some time, but so much? >I'd like to see which value you get for 'limit' on your machine >Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated >on time? From my test, i found it don't monitor a 0->1 transtion.That is the last result is 'i = limit'. The later while opertion stop also because 'i = limit'. The basic reason is the hardware. It write bit and monitor 1-->0---1>---0.But we start monitor we only got 0 and the value don't change. Maybe the transition is 1-->0.So the monitor can't work but still waste cpu. > >> + while (i--) { >> + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udelay(1); > >In earlier threads, the use of udelay was disliked because it's a waste >of cpu cycles. The desired bit in SYSCTL will change, while udelay() >is still making many useless loops. Yes, but it at most wast 1us. Jianpeng Ma > >> + } >> } >> - i = 0; >> >> - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && >> - (i++ < limit)) >> - cpu_relax(); >> + i = limit / (MMC_TIMEOUT_MS * 1000); >> + while (i--) { >> + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udealy(1); >> + } >> >> if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> dev_err(mmc_dev(host->mmc), >Hein Thanks! Jianpeng Ma >Hi Jianpeng Ma, > >On 8/1/2013 10:18 AM, majianpeng wrote: >> We found a problem when we removed a working sd card that the irqaction >> of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. >> In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 >> transition.It used loops_per_jiffy as the timer. >> The code is: >>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>> && (i++ < limit)) >>> cpu_relax(); >> But the loops_per_jiffy is: >>> while(i++ < limit) >>> cpu_relax(); >> It add some codes so the time became long. >> Becasue those codes in ISR context, it can't use timer_before/after. >> I divived the time into 1ms and used udelay(1) to instead. >> It will cause do additional udelay(1).But from my test,it looks good. >> >> Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >> --- >> drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- >> 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 1865321..96daca1 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> unsigned long limit = (loops_per_jiffy * >> msecs_to_jiffies(MMC_TIMEOUT_MS)); >> >> + /*Divided time into us for unit 1,we can use udelay(1)*/ >> + i = limit / (MMC_TIMEOUT_MS * 1000); > >'limit' is a number of loops, which you now divide by 20,000? > >To get uS, you could just change: > >- unsigned long limit = (loops_per_jiffy * >- msecs_to_jiffies(MMC_TIMEOUT_MS)); >+ unsigned long limit = 1000 * MMC_TIMEOUT_MS; > >and make this amount of loops using udelay(). > >> OMAP_HSMMC_WRITE(host->base, SYSCTL, >> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); >> >> @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> * Monitor a 0->1 transition first >> */ >> if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { >> - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> - && (i++ < limit)) >> - cpu_relax(); > >I still don't see why any of these loops could last 3.6 seconds? >Yes the __raw_readl() will add some time, but so much? >I'd like to see which value you get for 'limit' on your machine >Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated >on time? > >> + while (i--) { >> + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udelay(1); > >In earlier threads, the use of udelay was disliked because it's a waste >of cpu cycles. The desired bit in SYSCTL will change, while udelay() >is still making many useless loops. > >> + } >> } >> - i = 0; >> >> - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && >> - (i++ < limit)) >> - cpu_relax(); >> + i = limit / (MMC_TIMEOUT_MS * 1000); >> + while (i--) { >> + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udealy(1); >> + } >> >> if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> dev_err(mmc_dev(host->mmc), >Hein ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context. @ 2013-08-01 8:40 ` majianpeng 0 siblings, 0 replies; 5+ messages in thread From: majianpeng @ 2013-08-01 8:40 UTC (permalink / raw) To: Hein Tibosch Cc: balajitk, cjb, mayuzheng, linux-mmc, linux-omap, linux-kernel, Felipe Balbi [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="gb2312", Size: 7071 bytes --] >Hi Jianpeng Ma, > >On 8/1/2013 10:18 AM, majianpeng wrote: >> We found a problem when we removed a working sd card that the irqaction >> of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. >> In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 >> transition.It used loops_per_jiffy as the timer. >> The code is: >>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>> && (i++ < limit)) >>> cpu_relax(); >> But the loops_per_jiffy is: >>> while(i++ < limit) >>> cpu_relax(); >> It add some codes so the time became long. >> Becasue those codes in ISR context, it can't use timer_before/after. >> I divived the time into 1ms and used udelay(1) to instead. >> It will cause do additional udelay(1).But from my test,it looks good. >> >> Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >> --- >> drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- >> 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 1865321..96daca1 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> unsigned long limit = (loops_per_jiffy * >> msecs_to_jiffies(MMC_TIMEOUT_MS)); >> >> + /*Divided time into us for unit 1,we can use udelay(1)*/ >> + i = limit / (MMC_TIMEOUT_MS * 1000); > >'limit' is a number of loops, which you now divide by 20,000? > >To get uS, you could just change: > >- unsigned long limit = (loops_per_jiffy * >- msecs_to_jiffies(MMC_TIMEOUT_MS)); >+ unsigned long limit = 1000 * MMC_TIMEOUT_MS; > Yes, you are right. >and make this amount of loops using udelay(). > >> OMAP_HSMMC_WRITE(host->base, SYSCTL, >> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); >> >> @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> * Monitor a 0->1 transition first >> */ >> if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { >> - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> - && (i++ < limit)) >> - cpu_relax(); > >I still don't see why any of these loops could last 3.6 seconds? >Yes the __raw_readl() will add some time, but so much? >I'd like to see which value you get for 'limit' on your machine >Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated >on time? >From my test, i found it don't monitor a 0->1 transtion.That is the last result is 'i = limit'. The later while opertion stop also because 'i = limit'. The basic reason is the hardware. It write bit and monitor 1-->0---1>---0.But we start monitor we only got 0 and the value don't change. Maybe the transition is 1-->0.So the monitor can't work but still waste cpu. > >> + while (i--) { >> + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udelay(1); > >In earlier threads, the use of udelay was disliked because it's a waste >of cpu cycles. The desired bit in SYSCTL will change, while udelay() >is still making many useless loops. Yes, but it at most wast 1us. Jianpeng Ma > >> + } >> } >> - i = 0; >> >> - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && >> - (i++ < limit)) >> - cpu_relax(); >> + i = limit / (MMC_TIMEOUT_MS * 1000); >> + while (i--) { >> + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udealy(1); >> + } >> >> if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> dev_err(mmc_dev(host->mmc), >Hein Thanks! Jianpeng Ma >Hi Jianpeng Ma, > >On 8/1/2013 10:18 AM, majianpeng wrote: >> We found a problem when we removed a working sd card that the irqaction >> of omap_hsmmc can sleep to 3.6s. This cause our watchdog to work. >> In func omap_hsmmc_reset_controller_fsm, it should watch a 0->1 >> transition.It used loops_per_jiffy as the timer. >> The code is: >>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>> && (i++ < limit)) >>> cpu_relax(); >> But the loops_per_jiffy is: >>> while(i++ < limit) >>> cpu_relax(); >> It add some codes so the time became long. >> Becasue those codes in ISR context, it can't use timer_before/after. >> I divived the time into 1ms and used udelay(1) to instead. >> It will cause do additional udelay(1).But from my test,it looks good. >> >> Reported-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Tested-by: Yuzheng Ma <mayuzheng@kedacom.com> >> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >> --- >> drivers/mmc/host/omap_hsmmc.c | 20 +++++++++++++------- >> 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 1865321..96daca1 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -977,6 +977,8 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> unsigned long limit = (loops_per_jiffy * >> msecs_to_jiffies(MMC_TIMEOUT_MS)); >> >> + /*Divided time into us for unit 1,we can use udelay(1)*/ >> + i = limit / (MMC_TIMEOUT_MS * 1000); > >'limit' is a number of loops, which you now divide by 20,000? > >To get uS, you could just change: > >- unsigned long limit = (loops_per_jiffy * >- msecs_to_jiffies(MMC_TIMEOUT_MS)); >+ unsigned long limit = 1000 * MMC_TIMEOUT_MS; > >and make this amount of loops using udelay(). > >> OMAP_HSMMC_WRITE(host->base, SYSCTL, >> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); >> >> @@ -985,15 +987,19 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> * Monitor a 0->1 transition first >> */ >> if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) { >> - while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> - && (i++ < limit)) >> - cpu_relax(); > >I still don't see why any of these loops could last 3.6 seconds? >Yes the __raw_readl() will add some time, but so much? >I'd like to see which value you get for 'limit' on your machine >Would PM play a role? Or cpu-freq, and 'loops_per_jiffy' isn't updated >on time? > >> + while (i--) { >> + if ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udelay(1); > >In earlier threads, the use of udelay was disliked because it's a waste >of cpu cycles. The desired bit in SYSCTL will change, while udelay() >is still making many useless loops. > >> + } >> } >> - i = 0; >> >> - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && >> - (i++ < limit)) >> - cpu_relax(); >> + i = limit / (MMC_TIMEOUT_MS * 1000); >> + while (i--) { >> + if (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> + break; >> + udealy(1); >> + } >> >> if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> dev_err(mmc_dev(host->mmc), >Heinÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-08-01 9:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-01 2:18 [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context majianpeng 2013-08-01 2:18 ` majianpeng 2013-08-01 8:22 ` Hein Tibosch 2013-08-01 8:40 ` majianpeng 2013-08-01 8:40 ` majianpeng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.