* [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context. @ 2013-08-07 10:05 majianpeng 2013-08-20 18:52 ` Hein Tibosch 0 siblings, 1 reply; 4+ messages in thread From: majianpeng @ 2013-08-07 10:05 UTC (permalink / raw) To: balajitk, cjb; +Cc: linux-mmc, linux-omap, mayuzheng, Hein Tibosch V2: clean up code. V1: www.mail-archive.com/linux-omap@vger.../msg93239.html 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.But avoiding endless waiting, it used loops_per_jiffy as the timer. The code is: > while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) > && (i++ < limit)) > cpu_relax(); But generanly loops_per_jiffy as a timer,it should like: > while(i++ < limit) > cpu_relax(); I found for the long time case, the while-opeation stoped because 'i == limit'. Because added some code, so the duration became too longer than MMC_TIMEOU_US(20ms). The software can't monitor the transition of hardware for thi case. 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> Reviewed-by: Hein Tibosch <hein_tibosch@yahoo.es> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> --- drivers/mmc/host/omap_hsmmc.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 1865321..bbda5ed 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -973,9 +973,8 @@ static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host, static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, unsigned long bit) { - unsigned long i = 0; - unsigned long limit = (loops_per_jiffy * - msecs_to_jiffies(MMC_TIMEOUT_MS)); + /*change to 1ms,so we can use udelay(1)*/ + unsigned long limit = MMC_TIMEOUT_MS * 1000; OMAP_HSMMC_WRITE(host->base, SYSCTL, OMAP_HSMMC_READ(host->base, SYSCTL) | bit); @@ -984,17 +983,15 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, * OMAP4 ES2 and greater has an updated reset logic. * 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 = 0; - - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && - (i++ < limit)) - cpu_relax(); - + if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) + while (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit) + && limit--) + udelay(1); + + limit = MMC_TIMEOUT_MS * 1000; + while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && limit--) + udelay(1); + if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) dev_err(mmc_dev(host->mmc), "Timeout waiting on controller reset in %s\n", -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context. 2013-08-07 10:05 [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context majianpeng @ 2013-08-20 18:52 ` Hein Tibosch 2013-08-21 17:14 ` Balaji T K 0 siblings, 1 reply; 4+ messages in thread From: Hein Tibosch @ 2013-08-20 18:52 UTC (permalink / raw) To: cjb, Felipe Balbi Cc: majianpeng, balajitk, linux-mmc, linux-omap, mayuzheng, Matt Porter, Santosh Shilimkar, Tony Lindgren, Syed Mohammed Khasim, Madhusudhan, Mohit Jalori Hi, [ added some people from TI ] On 8/7/2013 6:05 PM, majianpeng wrote: > V2: > clean up code. > V1: > www.mail-archive.com/linux-omap@vger.../msg93239.html > > > 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.But avoiding endless waiting, it used loops_per_jiffy as the timer. Tried on a OMAP4460: This can easily be replicated: just withdraw an SD-card and the kernel will get blocked during more than 3 seconds. Calling OMAP_HSMMC_READ() in this loop makes it last so long. The function waits for a 0=>1, followed by a 1=>0 transition. The value of 1 always comes, but in most cases the code is just too slow to detect it. The first loop will only stop when (i == limit) > The code is: >> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >> && (i++ < limit)) >> cpu_relax(); > But generanly loops_per_jiffy as a timer,it should like: >> while(i++ < limit) >> cpu_relax(); > I found for the long time case, the while-opeation stoped because 'i == limit'. > Because added some code, so the duration became too longer than > MMC_TIMEOU_US(20ms). > > The software can't monitor the transition of hardware for thi case. > > 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> > Reviewed-by: Hein Tibosch <hein_tibosch@yahoo.es> > Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> > --- > drivers/mmc/host/omap_hsmmc.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c > index 1865321..bbda5ed 100644 > --- a/drivers/mmc/host/omap_hsmmc.c > +++ b/drivers/mmc/host/omap_hsmmc.c > @@ -973,9 +973,8 @@ static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host, > static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, > unsigned long bit) > { > - unsigned long i = 0; > - unsigned long limit = (loops_per_jiffy * > - msecs_to_jiffies(MMC_TIMEOUT_MS)); > + /*change to 1ms,so we can use udelay(1)*/ > + unsigned long limit = MMC_TIMEOUT_MS * 1000; > > OMAP_HSMMC_WRITE(host->base, SYSCTL, > OMAP_HSMMC_READ(host->base, SYSCTL) | bit); Checked here: the SRC-bit indeed becomes high. After the test 'features & HSMMC_HAS_UPDATED_RESET', the bit has become low again already. Changing to order of statements worked for me, but for Jianpeng Ma this didn't work (timings are 'on the edge'). This reset function is also called while mounting a card and then 'SRC' will be high long enough (more than 100 loops). It is after withdrawing the card that the reset is performed extremely fast. > @@ -984,17 +983,15 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, > * OMAP4 ES2 and greater has an updated reset logic. > * 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 = 0; > - > - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && > - (i++ < limit)) > - cpu_relax(); > - > + if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) > + while (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit) > + && limit--) > + udelay(1); In this way, the loop will only last about 'MMC_TIMEOUT_MS' ms and my WDT doesn't get triggered :-) > + limit = MMC_TIMEOUT_MS * 1000; > + while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && limit--) > + udelay(1); > + > if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) > dev_err(mmc_dev(host->mmc), > "Timeout waiting on controller reset in %s\n", Anybody an opinion on this? Thanks, Hein ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context. 2013-08-20 18:52 ` Hein Tibosch @ 2013-08-21 17:14 ` Balaji T K 2013-08-22 12:03 ` majianpeng 0 siblings, 1 reply; 4+ messages in thread From: Balaji T K @ 2013-08-21 17:14 UTC (permalink / raw) To: Hein Tibosch Cc: cjb, Felipe Balbi, majianpeng, linux-mmc, linux-omap, mayuzheng, Santosh Shilimkar, Tony Lindgren On Wednesday 21 August 2013 12:22 AM, Hein Tibosch wrote: > Hi, > > [ added some people from TI ] > > On 8/7/2013 6:05 PM, majianpeng wrote: >> V2: >> clean up code. >> V1: >> www.mail-archive.com/linux-omap@vger.../msg93239.html >> >> >> 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.But avoiding endless waiting, it used loops_per_jiffy as the timer. > > Tried on a OMAP4460: > This can easily be replicated: just withdraw an SD-card and the kernel > will get blocked during more than 3 seconds. > Calling OMAP_HSMMC_READ() in this loop makes it last so long. > > The function waits for a 0=>1, followed by a 1=>0 transition. > The value of 1 always comes, but in most cases the code is just too > slow to detect it. The first loop will only stop when (i == limit) > >> The code is: >>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>> && (i++ < limit)) >>> cpu_relax(); >> But generanly loops_per_jiffy as a timer,it should like: >>> while(i++ < limit) >>> cpu_relax(); >> I found for the long time case, the while-opeation stoped because 'i == limit'. >> Because added some code, so the duration became too longer than >> MMC_TIMEOU_US(20ms). >> >> The software can't monitor the transition of hardware for thi case. >> >> 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> >> Reviewed-by: Hein Tibosch <hein_tibosch@yahoo.es> >> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >> --- >> drivers/mmc/host/omap_hsmmc.c | 25 +++++++++++-------------- >> 1 file changed, 11 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index 1865321..bbda5ed 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -973,9 +973,8 @@ static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host, >> static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> unsigned long bit) >> { >> - unsigned long i = 0; >> - unsigned long limit = (loops_per_jiffy * >> - msecs_to_jiffies(MMC_TIMEOUT_MS)); >> + /*change to 1ms,so we can use udelay(1)*/ >> + unsigned long limit = MMC_TIMEOUT_MS * 1000; >> >> OMAP_HSMMC_WRITE(host->base, SYSCTL, >> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); > > Checked here: the SRC-bit indeed becomes high. > After the test 'features & HSMMC_HAS_UPDATED_RESET', the bit has > become low again already. > Changing to order of statements worked for me, but for Jianpeng Ma > this didn't work (timings are 'on the edge'). I think 1->0 transition is missed sometimes and waiting until timeout, Jianpeng Ma, which SoC are you testing ? > > This reset function is also called while mounting a card and then 'SRC' > will be high long enough (more than 100 loops). It is after withdrawing > the card that the reset is performed extremely fast. > >> @@ -984,17 +983,15 @@ static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >> * OMAP4 ES2 and greater has an updated reset logic. >> * 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 = 0; >> - >> - while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && >> - (i++ < limit)) >> - cpu_relax(); >> - >> + if (mmc_slot(host).features & HSMMC_HAS_UPDATED_RESET) >> + while (!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> + && limit--) >> + udelay(1); > > In this way, the loop will only last about 'MMC_TIMEOUT_MS' ms > and my WDT doesn't get triggered :-) > >> + limit = MMC_TIMEOUT_MS * 1000; >> + while ((OMAP_HSMMC_READ(host->base, SYSCTL) & bit) && limit--) >> + udelay(1); >> + >> if (OMAP_HSMMC_READ(host->base, SYSCTL) & bit) >> dev_err(mmc_dev(host->mmc), >> "Timeout waiting on controller reset in %s\n", > > Anybody an opinion on this? > > Thanks, Hein > -- 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context. 2013-08-21 17:14 ` Balaji T K @ 2013-08-22 12:03 ` majianpeng 0 siblings, 0 replies; 4+ messages in thread From: majianpeng @ 2013-08-22 12:03 UTC (permalink / raw) To: balajitk, Hein Tibosch Cc: cjb, Felipe Balbi, linux-mmc, linux-omap, mayuzheng, Santosh Shilimkar, Tony Lindgren >On Wednesday 21 August 2013 12:22 AM, Hein Tibosch wrote: >> Hi, >> >> [ added some people from TI ] >> >> On 8/7/2013 6:05 PM, majianpeng wrote: >>> V2: >>> clean up code. >>> V1: >>> www.mail-archive.com/linux-omap@vger.../msg93239.html >>> >>> >>> 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.But avoiding endless waiting, it used loops_per_jiffy as the timer. >> >> Tried on a OMAP4460: >> This can easily be replicated: just withdraw an SD-card and the kernel >> will get blocked during more than 3 seconds. >> Calling OMAP_HSMMC_READ() in this loop makes it last so long. >> >> The function waits for a 0=>1, followed by a 1=>0 transition. >> The value of 1 always comes, but in most cases the code is just too >> slow to detect it. The first loop will only stop when (i == limit) >> >>> The code is: >>>> while ((!(OMAP_HSMMC_READ(host->base, SYSCTL) & bit)) >>>> && (i++ < limit)) >>>> cpu_relax(); >>> But generanly loops_per_jiffy as a timer,it should like: >>>> while(i++ < limit) >>>> cpu_relax(); >>> I found for the long time case, the while-opeation stoped because 'i == limit'. >>> Because added some code, so the duration became too longer than >>> MMC_TIMEOU_US(20ms). >>> >>> The software can't monitor the transition of hardware for thi case. >>> >>> 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> >>> Reviewed-by: Hein Tibosch <hein_tibosch@yahoo.es> >>> Signed-off-by: Jianpeng Ma <majianpeng@gmail.com> >>> --- >>> drivers/mmc/host/omap_hsmmc.c | 25 +++++++++++-------------- >>> 1 file changed, 11 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >>> index 1865321..bbda5ed 100644 >>> --- a/drivers/mmc/host/omap_hsmmc.c >>> +++ b/drivers/mmc/host/omap_hsmmc.c >>> @@ -973,9 +973,8 @@ static inline void omap_hsmmc_dbg_report_irq(struct omap_hsmmc_host *host, >>> static inline void omap_hsmmc_reset_controller_fsm(struct omap_hsmmc_host *host, >>> unsigned long bit) >>> { >>> - unsigned long i = 0; >>> - unsigned long limit = (loops_per_jiffy * >>> - msecs_to_jiffies(MMC_TIMEOUT_MS)); >>> + /*change to 1ms,so we can use udelay(1)*/ >>> + unsigned long limit = MMC_TIMEOUT_MS * 1000; >>> >>> OMAP_HSMMC_WRITE(host->base, SYSCTL, >>> OMAP_HSMMC_READ(host->base, SYSCTL) | bit); >> >> Checked here: the SRC-bit indeed becomes high. >> After the test 'features & HSMMC_HAS_UPDATED_RESET', the bit has >> become low again already. >> Changing to order of statements worked for me, but for Jianpeng Ma >> this didn't work (timings are 'on the edge'). > >I think 1->0 transition is missed sometimes and waiting until timeout, >Jianpeng Ma, which SoC are you testing ? > Hi, my sos is DM8107. Thanks! Jianpeng Ma! ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-22 12:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-07 10:05 [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context majianpeng 2013-08-20 18:52 ` Hein Tibosch 2013-08-21 17:14 ` Balaji T K 2013-08-22 12:03 ` majianpeng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).