usb: gadget: s3c-hsotg: fix core reset timeout failure
PATCH 1/1 usb: gadget: s3c-hsotg: fix core reset timeout failure
The timeout values were 1000 and timeout issue occured many times on my s3c6410 Soc based board (mostly when booting whith USB cable not connected). This patch increase the values to 10000 to guarantee the success of reset.
Having set timeout to 10000, I printed the remained timeout values which can cause timeout issue before this change (tested several times):
the first timeout value remained:
timeout = 8079 timeout = 8079 timeout = 8078 timeout = 8081 the second timeout value remained: timeout = 7940 timeout = 7945 timeout = 7940 timeout = 7938
Seeing from above values, I think 10000 is big enough.
--- drivers/usb/gadget/s3c-hsotg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/s3c-hsotg.c +0 -0
@@ -2215,7 +2215,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg
*hsotg)
/* issue soft reset */ writel(GRSTCTL_CSftRst, hsotg->regs + GRSTCTL);
- timeout = 1000;
+ timeout = 10000;
do {
grstctl = readl(hsotg->regs + GRSTCTL);
} while ((grstctl & GRSTCTL_CSftRst) && timeout-- > 0);
@@ -2225,7 +2225,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
return -EINVAL;
}
- timeout = 1000;
+ timeout = 10000;
while (1) {
u32 grstctl = readl(hsotg->regs + GRSTCTL);
Signature · 1 line
1.7.9.5
Hi,
Quoted 37 lines · Changbin
The timeout values were 1000 and timeout issue occured many times on my s3c6410 Soc based board (mostly when booting whith USB cable not connected). This patch increase the values to 10000 to guarantee the success of reset.
Having set timeout to 10000, I printed the remained timeout values which can cause timeout issue before this change (tested several times):
the first timeout value remained:timeout = 8079 timeout = 8079 timeout = 8078 timeout = 8081 the second timeout value remained: timeout = 7940 timeout = 7945 timeout = 7940 timeout = 7938Seeing from above values, I think 10000 is big enough.
Signed-off-by Du, Changbin <[email protected]>--- drivers/usb/gadget/s3c-hsotg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/usb/gadget/s3c-hsotg.c +1
-1@@ -2215,7 +2215,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)/* issue soft reset */ writel (GRSTCTL_CSftRst, hsotg->regs +GRSTCTL ); - timeout =1000 ; + timeout =10000 ; do { grstctl = readl(hsotg->regs + GRSTCTL);
Quoted 1 line
} while ((grstctl & GRSTCTL_CSftRst) && timeout-- > 0);
how about msleep inside the loop?
Thanks
Kishon
Quoted 23 lines
Hi,
Quoted 19 lines · Changbin
The timeout values were 1000 and timeout issue occured many times on my s3c6410 Soc based board (mostly when booting whith USB cable not connected). This patch increase the values to 10000 to guarantee the success of reset.
Having set timeout to 10000, I printed the remained timeout values which can cause timeout issue before this change (tested several times):
the first timeout value remained:timeout = 8079 timeout = 8079 timeout = 8078 timeout = 8081 the second timeout value remained: timeout = 7940 timeout = 7945 timeout = 7940 timeout = 7938Seeing from above values, I think 10000 is big enough.
Quoted 6 lines
Review
Quoted 6 lines
Signed-off-by Du, Changbin <[email protected]>--- drivers/usb/gadget/s3c-hsotg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/s3c-hsotg.c
Quoted 8 lines
Quoted 8 lines
index f4abb0e..f3e2234 100644
drivers/usb/gadget/s3c-hsotg.c +0
-0@@ -2215,7 +2215,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)/* issue soft reset */ writel (GRSTCTL_CSftRst, hsotg->regs +GRSTCTL );
Quoted 7 lines
Quoted 5 lines
drivers/usb/gadget/s3c-hsotg.c +1
-1- timeout =1000 ; + timeout =10000 ; do { grstctl = readl(hsotg->regs + GRSTCTL);} while ((grstctl & GRSTCTL_CSftRst) && timeout-- > 0);how about msleep inside the loop?
I don't know if that's ok! The datasheet just says:
Once this bit is cleared software must wait at least 3 PHY clocks before doing any access to the PHY domain.
Software must also check that bit 31 of this register is 1 (AHB Master is IDLE) before starting any operation.
But it doesn't say how long to wait. I don't know if it can sleep for a while and then check the register's value.
In fact, sometimes the AHB Master will be IDLE quickly(even only need read once), but sometimes may need
Much more time. To reduce the risk, keeping it in original way may be better.
Thanks!
Du, Changbin
Quoted 3 lines
Thanks
Kishon
Hello.
Quoted 29 lines · Changbin
The timeout values were 1000 and timeout issue occured many times on my s3c6410 Soc based board (mostly when booting whith USB cable not connected). This patch increase the values to 10000 to guarantee the success of reset.
Having set timeout to 10000, I printed the remained timeout values which can cause timeout issue before this change (tested several times):
the first timeout value remained:timeout = 8079 timeout = 8079 timeout = 8078 timeout = 8081 the second timeout value remained: timeout = 7940 timeout = 7945 timeout = 7940 timeout = 7938Seeing from above values, I think 10000 is big enough.
Signed-off-by Du, Changbin <[email protected]>--- drivers/usb/gadget/s3c-hsotg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/usb/gadget/s3c-hsotg.c +0
-0
drivers/usb/gadget/s3c-hsotg.c static int s3c_hsotg_corereset(struct s3c_hsotg · line 2215
Quoted 1 line
drivers/usb/gadget/s3c-hsotg.c +0
-0@@ -2215,7 +2215,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)
Your patch is line wrapped; seems easy to fix manually while applying though...
WBR, Sergei
Quoted 8 lines
Review
Quoted 8 lines
Seeing from above values, I think 10000 is big enough.
Signed-off-by Du, Changbin <[email protected]>--- drivers/usb/gadget/s3c-hsotg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/s3c-hsotg.c
Quoted 8 lines
drivers/usb/gadget/s3c-hsotg.c static int s3c_hsotg_corereset(struct s3c_hsotg · line 2215
Quoted 5 lines
index f4abb0e..f3e2234 100644
drivers/usb/gadget/s3c-hsotg.c +0
-0@@ -2215,7 +2215,7 @@ static int s3c_hsotg_corereset(struct s3c_hsotg *hsotg)Your patch is line wrapped; seems easy to fix manually while applying
though...
Sorry, it's done by my mail client. I will send a new one soon. Thanks for pointing out it.
Du, Changbin
Quoted 2 lines
WBR, Sergei