[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

From: Fabian Godehardt <fg@emlix.com>

This is a patch that seems to make the USB hangs on the S3C244X go away.
At least a good amount of ping torture didn't make them come back so far.

The issue is that, if there are several back-to-back packets, sometimes no
interrupt is generated for one of them. This seems to be caused by the
mysterious dual packet mode, which the USB hardware enters automatically
if the endpoint size is half that of the FIFO. (On the 244X, this is the
normal situation for bulk data endpoints.)

There is also a timing factor in this. It seems that what happens is that
the USB hardware automatically sends an acknowledgement if there is only one 
packet in the FIFO (the FIFO has space for two). If another packet arrives
before the host has retrieved and acknowledged the previous one, no interrupt
is generated for that second one.

However, there may be an indication. There is one undocumented bit (none
of the 244x manuals document it), OUT_CRS1_REG[1], that seems to be set
suspiciously often when this condition occurs. There is also
CLR_DATA_TOGGLE, OUT_CRS1_REG[7], which may have a function related to
this. (The Samsung manual is rather terse on that, as usual.)

This needs to be examined further. For now, the patch seems to do the
trick.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
index ea2b3c7..fed323c 100644
--- a/drivers/usb/gadget/s3c2410_udc.c
+++ b/drivers/usb/gadget/s3c2410_udc.c
@@ -902,7 +902,7 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
 	int pwr_reg;
 	int ep0csr;
 	int i;
-	u32 idx;
+	u32 idx, idx2;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->lock, flags);
@@ -1017,6 +1017,20 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
 		}
 	}
 
+	/* what else causes this interrupt? a receive! who is it? */
+	if (!usb_status && !usbd_status && !pwr_reg && !ep0csr) {
+		for (i = 1; i < S3C2410_ENDPOINTS; i++) {
+			idx2 = udc_read(S3C2410_UDC_INDEX_REG);
+			udc_write(i, S3C2410_UDC_INDEX_REG);
+
+			if (udc_read(S3C2410_UDC_OUT_CSR1_REG) & 0x1)
+				s3c2410_udc_handle_ep(&dev->ep[i]);
+
+			/* restore index */
+			udc_write(idx2, S3C2410_UDC_INDEX_REG);
+		}
+	}
+
 	dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", IRQ_USBD);
 
 	/* Restore old index */
-- 
1.7.2

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

? ????????? ?? 30 ??????? 2010 17:34:22 ????? Vasily Khoruzhick ???????:
> From: Fabian Godehardt <fg@emlix.com>
> 
> This is a patch that seems to make the USB hangs on the S3C244X go away.
> At least a good amount of ping torture didn't make them come back so far.
> 
> The issue is that, if there are several back-to-back packets, sometimes no
> interrupt is generated for one of them. This seems to be caused by the
> mysterious dual packet mode, which the USB hardware enters automatically
> if the endpoint size is half that of the FIFO. (On the 244X, this is the
> normal situation for bulk data endpoints.)
> 
> There is also a timing factor in this. It seems that what happens is that
> the USB hardware automatically sends an acknowledgement if there is only
> one packet in the FIFO (the FIFO has space for two). If another packet
> arrives before the host has retrieved and acknowledged the previous one,
> no interrupt is generated for that second one.
> 
> However, there may be an indication. There is one undocumented bit (none
> of the 244x manuals document it), OUT_CRS1_REG[1], that seems to be set
> suspiciously often when this condition occurs. There is also
> CLR_DATA_TOGGLE, OUT_CRS1_REG[7], which may have a function related to
> this. (The Samsung manual is rather terse on that, as usual.)
> 
> This needs to be examined further. For now, the patch seems to do the
> trick.

Ping
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100906/45ff392a/attachment.sig>

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
> From: Fabian Godehardt <fg@emlix.com>
> 
> This is a patch that seems to make the USB hangs on the S3C244X go away.
> At least a good amount of ping torture didn't make them come back so far.
> 
> The issue is that, if there are several back-to-back packets, sometimes no
> interrupt is generated for one of them. This seems to be caused by the
> mysterious dual packet mode, which the USB hardware enters automatically
> if the endpoint size is half that of the FIFO. (On the 244X, this is the
> normal situation for bulk data endpoints.)
> 
> There is also a timing factor in this. It seems that what happens is that
> the USB hardware automatically sends an acknowledgement if there is only
> one packet in the FIFO (the FIFO has space for two). If another packet
> arrives before the host has retrieved and acknowledged the previous one,
> no interrupt is generated for that second one.
> 
> However, there may be an indication. There is one undocumented bit (none
> of the 244x manuals document it), OUT_CRS1_REG[1], that seems to be set
> suspiciously often when this condition occurs. There is also
> CLR_DATA_TOGGLE, OUT_CRS1_REG[7], which may have a function related to
> this. (The Samsung manual is rather terse on that, as usual.)
> 
> This needs to be examined further. For now, the patch seems to do the
> trick.
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/gadget/s3c2410_udc.c
> b/drivers/usb/gadget/s3c2410_udc.c index ea2b3c7..fed323c 100644
> --- a/drivers/usb/gadget/s3c2410_udc.c
> +++ b/drivers/usb/gadget/s3c2410_udc.c
> @@ -902,7 +902,7 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void
> *_dev) int pwr_reg;
>  	int ep0csr;
>  	int i;
> -	u32 idx;
> +	u32 idx, idx2;
>  	unsigned long flags;
> 
>  	spin_lock_irqsave(&dev->lock, flags);
> @@ -1017,6 +1017,20 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void
> *_dev) }
>  	}
> 
> +	/* what else causes this interrupt? a receive! who is it? */
> +	if (!usb_status && !usbd_status && !pwr_reg && !ep0csr) {
> +		for (i = 1; i < S3C2410_ENDPOINTS; i++) {
> +			idx2 = udc_read(S3C2410_UDC_INDEX_REG);
> +			udc_write(i, S3C2410_UDC_INDEX_REG);
> +
> +			if (udc_read(S3C2410_UDC_OUT_CSR1_REG) & 0x1)
> +				s3c2410_udc_handle_ep(&dev->ep[i]);
> +
> +			/* restore index */
> +			udc_write(idx2, S3C2410_UDC_INDEX_REG);
> +		}
> +	}
> +
>  	dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", IRQ_USBD);
> 
>  	/* Restore old index */

Ping? s3c2410_udc is broken for s3c244x for ages, fix is available, why not to 
apply it?

Regards
Vasily

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Ben Dooks]

On 06/12/10 19:39, Vasily Khoruzhick wrote:
> On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
>> From: Fabian Godehardt <fg@emlix.com>
>>
>> This is a patch that seems to make the USB hangs on the S3C244X go away.

Hmm who's job is it to apply it?
I'd certainly ack it

Acked-by: Ben Dooks <ben-linux@fluff.org>

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
> On 06/12/10 19:39, Vasily Khoruzhick wrote:
> > On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
> >> From: Fabian Godehardt <fg@emlix.com>
> >> 
> >> This is a patch that seems to make the USB hangs on the S3C244X go away.
> 
> Hmm who's job is it to apply it?
> I'd certainly ack it
> 
> Acked-by: Ben Dooks <ben-linux@fluff.org>

Hi, David

could you please merge this patch? If not, should I try to merge it through mm 
tree?

Regards
Vasily

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:
> On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
> > On 06/12/10 19:39, Vasily Khoruzhick wrote:
> > > On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
> > >> From: Fabian Godehardt <fg@emlix.com>
> > >> 
> > >> This is a patch that seems to make the USB hangs on the S3C244X go
> > >> away.
> > 
> > Hmm who's job is it to apply it?
> > I'd certainly ack it
> > 
> > Acked-by: Ben Dooks <ben-linux@fluff.org>
> 
> Hi, David
> 
> could you please merge this patch? If not, should I try to merge it through
> mm tree?
> 
> Regards
> Vasily

Ping once again

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Lars-Peter Clausen]

On 01/12/2011 11:04 AM, Vasily Khoruzhick wrote:
> On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:
>> On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
>>> On 06/12/10 19:39, Vasily Khoruzhick wrote:
>>>> On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
>>>>> From: Fabian Godehardt <fg@emlix.com>
>>>>>
>>>>> This is a patch that seems to make the USB hangs on the S3C244X go
>>>>> away.
>>>
>>> Hmm who's job is it to apply it?
>>> I'd certainly ack it
>>>
>>> Acked-by: Ben Dooks <ben-linux@fluff.org>
>>
>> Hi, David
>>
>> could you please merge this patch? If not, should I try to merge it through
>> mm tree?
>>
>> Regards
>> Vasily
> 
> Ping once again
> 

I'd really like to see that patch merged as well.

Added Greg KH to Cc.

Greg could you take the patch?
The original mail can be found at [1], but maybe it would be a good idea if Vasily
resend it.

- Lars

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/024404.html

[PATCH] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Greg KH]

On Mon, Feb 07, 2011 at 03:07:55AM +0100, Lars-Peter Clausen wrote:
> On 01/12/2011 11:04 AM, Vasily Khoruzhick wrote:
> > On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:
> >> On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
> >>> On 06/12/10 19:39, Vasily Khoruzhick wrote:
> >>>> On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
> >>>>> From: Fabian Godehardt <fg@emlix.com>
> >>>>>
> >>>>> This is a patch that seems to make the USB hangs on the S3C244X go
> >>>>> away.
> >>>
> >>> Hmm who's job is it to apply it?
> >>> I'd certainly ack it
> >>>
> >>> Acked-by: Ben Dooks <ben-linux@fluff.org>
> >>
> >> Hi, David
> >>
> >> could you please merge this patch? If not, should I try to merge it through
> >> mm tree?
> >>
> >> Regards
> >> Vasily
> > 
> > Ping once again
> > 
> 
> I'd really like to see that patch merged as well.
> 
> Added Greg KH to Cc.
> 
> Greg could you take the patch?
> The original mail can be found at [1], but maybe it would be a good idea if Vasily
> resend it.

Yes, please resend it, I need it in email form.

thanks,

greg k-h

[PATCH RESEND] s3c2410_udc: Add handling for S3C244X dual-packet mode

[Vasily Khoruzhick]

From: Fabian Godehardt <fg@emlix.com>

This is a patch that seems to make the USB hangs on the S3C244X go away.
At least a good amount of ping torture didn't make them come back so far.

The issue is that, if there are several back-to-back packets, sometimes no
interrupt is generated for one of them. This seems to be caused by the
mysterious dual packet mode, which the USB hardware enters automatically
if the endpoint size is half that of the FIFO. (On the 244X, this is the
normal situation for bulk data endpoints.)

There is also a timing factor in this. It seems that what happens is that
the USB hardware automatically sends an acknowledgement if there is only one 
packet in the FIFO (the FIFO has space for two). If another packet arrives
before the host has retrieved and acknowledged the previous one, no interrupt
is generated for that second one.

However, there may be an indication. There is one undocumented bit (none
of the 244x manuals document it), OUT_CRS1_REG[1], that seems to be set
suspiciously often when this condition occurs. There is also
CLR_DATA_TOGGLE, OUT_CRS1_REG[7], which may have a function related to
this. (The Samsung manual is rather terse on that, as usual.)

This needs to be examined further. For now, the patch seems to do the
trick.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
index ea2b3c7..fed323c 100644
--- a/drivers/usb/gadget/s3c2410_udc.c
+++ b/drivers/usb/gadget/s3c2410_udc.c
@@ -902,7 +902,7 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
 	int pwr_reg;
 	int ep0csr;
 	int i;
-	u32 idx;
+	u32 idx, idx2;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->lock, flags);
@@ -1017,6 +1017,20 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
 		}
 	}
 
+	/* what else causes this interrupt? a receive! who is it? */
+	if (!usb_status && !usbd_status && !pwr_reg && !ep0csr) {
+		for (i = 1; i < S3C2410_ENDPOINTS; i++) {
+			idx2 = udc_read(S3C2410_UDC_INDEX_REG);
+			udc_write(i, S3C2410_UDC_INDEX_REG);
+
+			if (udc_read(S3C2410_UDC_OUT_CSR1_REG) & 0x1)
+				s3c2410_udc_handle_ep(&dev->ep[i]);
+
+			/* restore index */
+			udc_write(idx2, S3C2410_UDC_INDEX_REG);
+		}
+	}
+
 	dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", IRQ_USBD);
 
 	/* Restore old index */
-- 
1.7.2