s3c2410_udc: Add handling for S3C244X dual-packet mode

PATCH New Vasily Khoruzhick 4 participants 9 messages Aug 30, 2010 – Feb 7, 2011

Subsystems (2)

USB Arm Samsung Exynos

lore

Share & tools

lore

Vasily Khoruzhick

author

2010-08-30 14:34 UTC #0 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

From: Fabian Godehardt <[email protected]>

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 <[email protected]>

---
 drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

drivers/usb/gadget/s3c2410_udc.c +15 -1

index ea2b3c7..fed323c 100644
@@ -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 */

Signature · 1 line

1.7.2

Vasily Khoruzhick

author

2010-09-06 10:29 UTC #1 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

? ????????? ?? 30 ??????? 2010 17:34:22 ????? Vasily Khoruzhick ???????:

Quoted 17 lines

From: Fabian Godehardt <[email protected]>
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.

Quoted 8 lines

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>

Vasily Khoruzhick

author

2010-12-06 19:39 UTC #2 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

In reply to Vasily Khoruzhick

Quoted 55 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:

From: Fabian Godehardt <[email protected]>
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 <[email protected]>
---
 drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)
(quoted diff) +0 -0
b/drivers/usb/gadget/s3c2410_udc.c index ea2b3c7..fed323c 100644
drivers/usb/gadget/s3c2410_udc.c +8 -1
@@ -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)

drivers/usb/gadget/s3c2410_udc.c static irqreturn_t s3c2410_udc_irq(int dummy, void · line 1036

Quoted 10 lines

drivers/usb/gadget/s3c2410_udc.c +7 -0

+				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

Ben Dooks

2010-12-08 00:52 UTC #3 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

Quoted 4 lines · Vasily Khoruzhick

Quoted 3 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
From: Fabian Godehardt <[email protected]>
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 <[email protected]>

Vasily Khoruzhick

author

2010-12-13 10:37 UTC #4 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

Quoted 10 lines · On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:

Quoted 4 lines · Vasily Khoruzhick
Quoted 3 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
From: Fabian Godehardt <[email protected]>
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 <[email protected]>

Hi, David

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

Regards
Vasily

Vasily Khoruzhick

author

2011-01-12 10:04 UTC #5 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

Quoted 13 lines · On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:

Quoted 11 lines · On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
Quoted 5 lines · Vasily Khoruzhick
Quoted 4 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
From: Fabian Godehardt <[email protected]>
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 <[email protected]>

Quoted 7 lines

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

Ping once again

Lars-Peter Clausen

2011-02-07 02:07 UTC #6 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

Quoted 16 lines · Vasily Khoruzhick

Quoted 15 lines · On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:
Quoted 11 lines · On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
Quoted 5 lines · Vasily Khoruzhick
Quoted 4 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
From: Fabian Godehardt <[email protected]>
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 <[email protected]>
Hi, David

Quoted 8 lines

Quoted 5 lines
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

Greg KH

2011-02-07 02:32 UTC #7 lore raw

PATCH 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

Quoted 26 lines · Lars-Peter Clausen

Quoted 16 lines · Vasily Khoruzhick
Quoted 15 lines · On Monday 13 December 2010 12:37:11 Vasily Khoruzhick wrote:
Quoted 11 lines · On Wednesday 08 December 2010 02:52:41 Ben Dooks wrote:
Quoted 5 lines · Vasily Khoruzhick
Quoted 4 lines · On Monday 30 August 2010 17:34:22 Vasily Khoruzhick wrote:
From: Fabian Godehardt <[email protected]>
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 <[email protected]>
Hi, David
Quoted 8 lines
Quoted 5 lines
could you please merge this patch? If not, should I try to merge it through mm tree?
Regards
Vasily
Ping once again

Quoted 7 lines

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

Vasily Khoruzhick

author

2011-02-07 10:53 UTC #8 lore raw

PATCH RESEND 1/1 s3c2410_udc: Add handling for S3C244X dual-packet mode

From: Fabian Godehardt <[email protected]>

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.

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

Signed-off-by Vasily Khoruzhick <[email protected]>

---
 drivers/usb/gadget/s3c2410_udc.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

drivers/usb/gadget/s3c2410_udc.c +15 -1

index ea2b3c7..fed323c 100644
@@ -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 */

Signature · 1 line

1.7.2