Re: Re: [PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context.

2013-08-22 Thread majianpeng 
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 mayuzh...@kedacom.com
 Tested-by: Yuzheng Ma mayuzh...@kedacom.com
 Reviewed-by: Hein Tibosch hein_tibo...@yahoo.es
 Signed-off-by: Jianpeng Ma majianp...@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!

[PATCH V2] mmc: omap_hsmmc: Fix sleep too long in ISR context.

2013-08-07 Thread majianpeng 
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 mayuzh...@kedacom.com
Tested-by: Yuzheng Ma mayuzh...@kedacom.com
Reviewed-by: Hein Tibosch hein_tibo...@yahoo.es
Signed-off-by: Jianpeng Ma majianp...@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

Re: Re: [PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context.

2013-08-01 Thread majianpeng 
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 mayuzh...@kedacom.com
 Tested-by: Yuzheng Ma mayuzh...@kedacom.com
 Signed-off-by: Jianpeng Ma majianp...@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 mayuzh...@kedacom.com
 Tested-by: Yuzheng Ma mayuzh...@kedacom.com
 Signed-off-by: Jianpeng Ma majianp...@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

[PATCH] mmc: omap_hsmmc: Fix sleep too long in ISR context.

2013-07-31 Thread majianpeng 
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 mayuzh...@kedacom.com
Tested-by: Yuzheng Ma mayuzh...@kedacom.com
Signed-off-by: Jianpeng Ma majianp...@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 MaN�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{雹f��{ay��,j��f＂�h���z��wア�
⒎�j:+v���w�j�m��赙zZ+�茛j��!�i

About omap2 mmc host close too long irq in irqaction.

2013-07-17 Thread majianpeng 
Hi all,
Now i worked on omp2 and met a probelm which someplace close_irq for 
3.6second. 
The kernel version is 2.6.37. I used trace to find in irq_action:omap_hsmmc_irq.
This problem occured by removed the sdcard when there are io operations.

I found the read problem is in omap_hsmmc_reset_controller_fsm.
In omap_hsmmc_reset_controller_fsm:
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));

OMAP_HSMMC_WRITE(host-base, SYSCTL,
 OMAP_HSMMC_READ(host-base, SYSCTL) | bit);

/*   
 * 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();
}
 In func, it used loops_per_jiffy  in order to avoid do many time exceed 
MMC_TIMEOUT_MS.
In face oops_per_jiify is like:
while(i++  loops_per_jiffy)
cpu_relax();
But actually, it used as follow:
while ((!(OMAP_HSMMC_READ(host-base, SYSCTL)  bit))
   (i++  limit))
it add some operations like: read regisger, , .
So the time may exceed MMC_TIMEOUT_MS.
I used those code to test and found it's ok:
for (i = 0 ; i  10; i++){
if (!(OMAP_HSMMC_READ(host-base, SYSCTL)  bit))
  break;
j = limit/10;
while(j--)
cpu_relax();
}

Using this the problom can't occur.
Am i missing something?

Thanks!
Jianpeng Ma