[PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[Du, 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 = 7938
Seeing from above values, I think 10000 is big enough.

Signed-off-by: Du, Changbin <changbin.du@gmail.com>
---
 drivers/usb/gadget/s3c-hsotg.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index f4abb0e..f3e2234 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -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);
-- 
1.7.9.5

Re: [PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[ABRAHAM, KISHON VIJAY]

Hi,

On Tue, Jul 3, 2012 at 7:33 AM, Du, Changbin <changbin.du@gmail.com> wrote:
> 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.
>
> Signed-off-by: Du, Changbin <changbin.du@gmail.com>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index f4abb0e..f3e2234 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -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);

how about msleep inside the loop?

Thanks
Kishon

RE: [PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[Du, Changbin]

> Hi,
> 
> On Tue, Jul 3, 2012 at 7:33 AM, Du, Changbin <changbin.du@gmail.com>
> wrote:
> > 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.
> >
> > Signed-off-by: Du, Changbin <changbin.du@gmail.com>
> > ---
> >  drivers/usb/gadget/s3c-hsotg.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/s3c-hsotg.c
b/drivers/usb/gadget/s3c-hsotg.c
> > index f4abb0e..f3e2234 100644
> > --- a/drivers/usb/gadget/s3c-hsotg.c
> > +++ b/drivers/usb/gadget/s3c-hsotg.c
> > @@ -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);
> 
> 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

> 
> Thanks
> Kishon

Re: [PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[Sergei Shtylyov]

Hello.

On 07/03/2012 06:03 AM, Du, Changbin wrote:

> 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.

> Signed-off-by: Du, Changbin <changbin.du@gmail.com>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index f4abb0e..f3e2234 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -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

RE: [PATCH 1/1] usb: gadget: s3c-hsotg: fix core reset timeout failure

[Du, Changbin]

> > Seeing from above values, I think 10000 is big enough.
> 
> > Signed-off-by: Du, Changbin <changbin.du@gmail.com>
> > ---
> >  drivers/usb/gadget/s3c-hsotg.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> > diff --git a/drivers/usb/gadget/s3c-hsotg.c
b/drivers/usb/gadget/s3c-hsotg.c
> > index f4abb0e..f3e2234 100644
> > --- a/drivers/usb/gadget/s3c-hsotg.c
> > +++ b/drivers/usb/gadget/s3c-hsotg.c
> > @@ -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

> 
> WBR, Sergei