All of lore.kernel.org
 help / color / mirror / Atom feed
* s3c-hsotg fixes
@ 2010-06-11  8:20 Ben Dooks
  2010-06-11  8:20 ` [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock Ben Dooks
                   ` (11 more replies)
  0 siblings, 12 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel


Some further fixes for the s3c-hsotg block, as well as a repost of
the missed USB phy clock setting patch which either got missed or
passed over from last time.

As a note, this series has a patch adding support for dedicated FIFO
mode, this is in my view a fix, as when the hsotg block is compiled
with dedicated fifos then each USB IN non-periodic pipe needs to be
alloacted a unique TX FIFO. We have no actual data on how well the
block performs when all IN TX NP FIFOs are all set to the same one,
but it really should be fixed.

I am not sure if I have been subscribed to linux-usb, please CC: me
just in case.

-- 
Ben

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  9:35   ` Marek Szyprowski
  2010-06-11  8:20 ` [PATCH 02/11] USB: s3c-hsotg: Increase TX fifo limit Ben Dooks
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

From: Maurus Cuelenaere <mcuelenaere@gmail.com>

The PLL that drives the USB clock supports 3 input clocks: 12, 24 and 48Mhz.
This patch adds support to the USB driver for setting the correct register bit
according to the given clock.

This depends on the following patch:
[PATCH] ARM: S3C64XX: Add USB external clock definition

Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 26193ec..6a303ce 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
@@ -2753,6 +2754,7 @@ static void __devinit s3c_hsotg_initep(struct s3c_hsotg *hsotg,
  */
 static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
 {
+	struct clk *xusbxti;
 	u32 osc;
 
 	writel(0, S3C_PHYPWR);
@@ -2760,6 +2762,23 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
 
 	osc = hsotg->plat->is_osc ? S3C_PHYCLK_EXT_OSC : 0;
 
+	xusbxti = clk_get(hsotg->dev, "xusbxti");
+	if (xusbxti && !IS_ERR(xusbxti)) {
+		switch (clk_get_rate(xusbxti)) {
+		case 12000000:
+		    osc |= S3C_PHYCLK_CLKSEL_12M;
+		    break;
+		case 24000000:
+		    osc |= S3C_PHYCLK_CLKSEL_24M;
+		    break;
+		default:
+		case 48000000:
+		    /* default reference clock */
+		    break;
+		}
+		clk_put(xusbxti);
+	}
+
 	writel(osc | 0x10, S3C_PHYCLK);
 
 	/* issue a full set of resets to the otg and core */
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/11] USB: s3c-hsotg: Increase TX fifo limit
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
  2010-06-11  8:20 ` [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 03/11] USB: s3c-hsotg: The NPTX/PTX FIFO sizes are in words, not bytes Ben Dooks
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

Up the FIFO size for the TX to 1024 entries, as this now seems to work
with all the cores. This fixes a problem when using large packets on
a core with MPS set to 512 can hang due to insufficient space for the
writes.

The hang arises due to getting the non-periodic FIFO empty IRQ but
not being able to satisfy any requests since there is never enough
space to write 512 bytes into the buffer. This means we end up with
a stream of interrupt requests.

It is easier to up the TX FIFO to fill the space we left for it
than to try and fix the positions in the code where we should have
limited the max-packet size to < TXFIFOSIZE, since the TXFIFOSIZE
depends on how the TX FIFOs have been setup.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 6a303ce..ba00e6e 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -311,11 +311,11 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
 		hsotg->regs + S3C_GNPTXFSIZ);
 	*/
 
-	/* set FIFO sizes to 2048/0x1C0 */
+	/* set FIFO sizes to 2048/1024 */
 
 	writel(2048, hsotg->regs + S3C_GRXFSIZ);
 	writel(S3C_GNPTXFSIZ_NPTxFStAddr(2048) |
-	       S3C_GNPTXFSIZ_NPTxFDep(0x1C0),
+	       S3C_GNPTXFSIZ_NPTxFDep(1024),
 	       hsotg->regs + S3C_GNPTXFSIZ);
 
 	/* arange all the rest of the TX FIFOs, as some versions of this
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/11] USB: s3c-hsotg: The NPTX/PTX FIFO sizes are in words, not bytes
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
  2010-06-11  8:20 ` [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock Ben Dooks
  2010-06-11  8:20 ` [PATCH 02/11] USB: s3c-hsotg: Increase TX fifo limit Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 04/11] USB: s3c-hsotg: Avoid overwriting contents of perodic in 'fifo' Ben Dooks
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

Fix a problem where we have been underestimating the space available in
the IN PTX/NPTX FIFOs by assuming that they where simply word aligned
instead of in number-of-words. This means all length calculations need
to be multiplied-by-4.

Note, we do not change the information about fifo size or start addresses
available to userspace as we assume the user can multiply by four easily
and is already knows these values are in words.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index ba00e6e..a018b6a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -506,6 +506,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
 		}
 
 		can_write = S3C_GNPTXSTS_NPTxFSpcAvail_GET(gnptxsts);
+		can_write *= 4;	/* fifo size is in 32bit quantities. */
 	}
 
 	dev_dbg(hsotg->dev, "%s: GNPTXSTS=%08x, can=%d, to=%d, mps %d\n",
@@ -2733,7 +2734,7 @@ static void __devinit s3c_hsotg_initep(struct s3c_hsotg *hsotg,
 	 */
 
 	ptxfifo = readl(hsotg->regs + S3C_DPTXFSIZn(epnum));
-	hs_ep->fifo_size = S3C_DPTXFSIZn_DPTxFSize_GET(ptxfifo);
+	hs_ep->fifo_size = S3C_DPTXFSIZn_DPTxFSize_GET(ptxfifo) * 4;
 
 	/* if we're using dma, we need to set the next-endpoint pointer
 	 * to be something valid.
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/11] USB: s3c-hsotg: Avoid overwriting contents of perodic in 'fifo'
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (2 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 03/11] USB: s3c-hsotg: The NPTX/PTX FIFO sizes are in words, not bytes Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 05/11] USB: s3c-hsotg: Re-initialise all FIFOs on USB bus reset Ben Dooks
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

In shared fifo mode (used on older SoCs) the periodic in fifo beahves
much more like a packet buffer, discarding old data when writing new
data. Avoid this by ensuring that we do not load new transactions in
when there is data sitting already in the FIFO.

Note, this may not be an observed bug, we are fixing the case that this
may happen.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index a018b6a..df3e8d1 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -92,7 +92,9 @@ struct s3c_hsotg_req;
  * For periodic IN endpoints, we have fifo_size and fifo_load to try
  * and keep track of the amount of data in the periodic FIFO for each
  * of these as we don't have a status register that tells us how much
- * is in each of them.
+ * is in each of them. (note, this may actually be useless information
+ * as in shared-fifo mode periodic in acts like a single-frame packet
+ * buffer than a fifo)
  */
 struct s3c_hsotg_ep {
 	struct usb_ep		ep;
@@ -475,6 +477,14 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
 
 		size_left = S3C_DxEPTSIZ_XferSize_GET(epsize);
 
+		/* if shared fifo, we cannot write anything until the
+		 * previous data has been completely sent.
+		 */
+		if (hs_ep->fifo_load != 0) {
+			s3c_hsotg_en_gsint(hsotg, S3C_GINTSTS_PTxFEmp);
+			return -ENOSPC;
+		}
+
 		dev_dbg(hsotg->dev, "%s: left=%d, load=%d, fifo=%d, size %d\n",
 			__func__, size_left,
 			hs_ep->size_loaded, hs_ep->fifo_load, hs_ep->fifo_size);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/11] USB: s3c-hsotg: Re-initialise all FIFOs on USB bus reset
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (3 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 04/11] USB: s3c-hsotg: Avoid overwriting contents of perodic in 'fifo' Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode Ben Dooks
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

The USB documentation suggest that the FIFOs should be reset when a
bus reset event happens. Use the s3c_hsotg_init_fifo() to ensure that
the FIFO layout is correct and that the FIFOs are flushed before
acknowledging the reset.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index df3e8d1..5b61bcf 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2083,17 +2083,12 @@ irq_retry:
 		kill_all_requests(hsotg, &hsotg->eps[0], -ECONNRESET, true);
 
 		/* it seems after a reset we can end up with a situation
-		 * where the TXFIFO still has data in it... try flushing
-		 * it to remove anything that may still be in it.
+		 * where the TXFIFO still has data in it... the docs
+		 * suggest resetting all the fifos, so use the init_fifo
+		 * code to relayout and flush the fifos.
 		 */
 
-		if (1) {
-			writel(S3C_GRSTCTL_TxFNum(0) | S3C_GRSTCTL_TxFFlsh,
-			       hsotg->regs + S3C_GRSTCTL);
-
-			dev_info(hsotg->dev, "GNPTXSTS=%08x\n",
-				 readl(hsotg->regs + S3C_GNPTXSTS));
-		}
+		s3c_hsotg_init_fifo(hsotg);
 
 		s3c_hsotg_enqueue_setup(hsotg);
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (4 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 05/11] USB: s3c-hsotg: Re-initialise all FIFOs on USB bus reset Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:54     ` Maurus Cuelenaere
  2010-06-11  8:20 ` [PATCH 07/11] USB: s3c-hsotg: Only load packet per fifo write Ben Dooks
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

Add support for the dedicated FIFO mode on newer SoCs such as the S5PV210
partly to improve support and to fix the bug where any non-EP0 IN endpoint
requires its own FIFO allocation.

To fix this, we ensure that any non-zero IN endpoint is given a TXFIFO
using the same allocation method as the periodic case (all our current
hardware has enough FIFOs and FIFO memory for a 1:1 mapping) and ensure
that the necessary transmission done interrupt is enabled.

The default settings from reset for the core point all EPs at FIFO0,
used for the control endpoint. However, the controller documentation
states that all IN endpoints _must_ have a unique FIFO to avoid any
contention during transmission.

Note, this leaves us with a large IN FIFO for EP0 (which re-uses the
old NPTXFIFO) for an endpoint which cannot shift more than a pair of
packets at a time... this is a waste, but it looks like we cannot
re-allocate space to the individual IN FIFOs as they are already
maxed out (to be confirmed).

Signed-off-by: Ben Dooks <ben-linux@fluff.org?
---
 .../arm/plat-samsung/include/plat/regs-usb-hsotg.h |    2 +
 drivers/usb/gadget/s3c-hsotg.c                     |   40 +++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
index 8d18d9d..dc90f5e 100644
--- a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
+++ b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
@@ -226,6 +226,7 @@
 
 #define S3C_DIEPMSK				S3C_HSOTG_REG(0x810)
 
+#define S3C_DIEPMSK_TxFIFOEmpty			(1 << 7)
 #define S3C_DIEPMSK_INEPNakEffMsk		(1 << 6)
 #define S3C_DIEPMSK_INTknEPMisMsk		(1 << 5)
 #define S3C_DIEPMSK_INTknTXFEmpMsk		(1 << 4)
@@ -371,6 +372,7 @@
 
 #define S3C_DIEPDMA(_a)				S3C_HSOTG_REG(0x914 + ((_a) * 0x20))
 #define S3C_DOEPDMA(_a)				S3C_HSOTG_REG(0xB14 + ((_a) * 0x20))
+#define S3C_DTXFSTS(_a)				S3C_HSOTG_REG(0x918 + ((_a) * 0x20))
 
 #define S3C_EPFIFO(_a)				S3C_HSOTG_REG(0x1000 + ((_a) * 0x1000))
 
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 5b61bcf..819e75a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -12,6 +12,8 @@
  * published by the Free Software Foundation.
 */
 
+#define DEBUG
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -131,6 +133,7 @@ struct s3c_hsotg_ep {
  * @regs: The memory area mapped for accessing registers.
  * @regs_res: The resource that was allocated when claiming register space.
  * @irq: The IRQ number we are using
+ * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
  * @debug_root: root directrory for debugfs.
  * @debug_file: main status file for debugfs.
  * @debug_fifo: FIFO status file for debugfs.
@@ -149,6 +152,8 @@ struct s3c_hsotg {
 	struct resource		*regs_res;
 	int			irq;
 
+	unsigned int		dedicated_fifos:1;
+
 	struct dentry		*debug_root;
 	struct dentry		*debug_file;
 	struct dentry		*debug_fifo;
@@ -467,7 +472,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
 	if (to_write == 0)
 		return 0;
 
-	if (periodic) {
+	if (periodic && !hsotg->dedicated_fifos) {
 		u32 epsize = readl(hsotg->regs + S3C_DIEPTSIZ(hs_ep->index));
 		int size_left;
 		int size_done;
@@ -505,6 +510,11 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
 			s3c_hsotg_en_gsint(hsotg, S3C_GINTSTS_PTxFEmp);
 			return -ENOSPC;
 		}
+	} else if (hsotg->dedicated_fifos && hs_ep->index != 0) {
+		can_write = readl(hsotg->regs + S3C_DTXFSTS(hs_ep->index));
+
+		can_write &= 0xffff;
+		can_write *= 4;
 	} else {
 		if (S3C_GNPTXSTS_NPTxQSpcAvail_GET(gnptxsts) == 0) {
 			dev_dbg(hsotg->dev,
@@ -1830,6 +1840,15 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
 				 __func__, idx);
 			clear |= S3C_DIEPMSK_INTknEPMisMsk;
 		}
+
+		/* FIFO has space or is empty (see GAHBCFG) */
+		if (hsotg->dedicated_fifos &&
+		    ints & S3C_DIEPMSK_TxFIFOEmpty) {
+			dev_dbg(hsotg->dev, "%s: ep%d: TxFIFOEmpty\n",
+				__func__, idx);
+			s3c_hsotg_trytx(hsotg, hs_ep);
+			clear |= S3C_DIEPMSK_TxFIFOEmpty;
+		}
 	}
 
 	writel(clear, hsotg->regs + epint_reg);
@@ -2281,6 +2300,12 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 		break;
 	}
 
+	/* if the hardware has dedicated fifos, we must give each IN EP
+	 * a unique tx-fifo even if it is non-periodic.
+	 */
+	if (dir_in && hsotg->dedicated_fifos)
+		epctrl |= S3C_DxEPCTL_TxFNum(index);
+
 	/* for non control endpoints, set PID to D0 */
 	if (index)
 		epctrl |= S3C_DxEPCTL_SetD0PID;
@@ -2570,7 +2595,8 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 
 	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
 	       S3C_DIEPMSK_INTknEPMisMsk |
-	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk,
+	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk |
+	       ((hsotg->dedicated_fifos) ? S3C_DIEPMSK_TxFIFOEmpty : 0),
 	       hsotg->regs + S3C_DIEPMSK);
 
 	/* don't need XferCompl, we get that from RXFIFO in slave mode. In
@@ -2797,6 +2823,8 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
 
 static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
 {
+	u32 cfg4;
+
 	/* unmask subset of endpoint interrupts */
 
 	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
@@ -2832,6 +2860,14 @@ static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
 
 	writel(using_dma(hsotg) ? S3C_GAHBCFG_DMAEn : 0x0,
 	       hsotg->regs + S3C_GAHBCFG);
+
+	/* check hardware configuration */
+
+	cfg4 = readl(hsotg->regs + 0x50);
+	hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
+
+	dev_info(hsotg->dev, "%s fifos\n",
+		 hsotg->dedicated_fifos ? "dedicated" : "shared");
 }
 
 static void s3c_hsotg_dump(struct s3c_hsotg *hsotg)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/11] USB: s3c-hsotg: Only load packet per fifo write
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (5 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 08/11] USB: s3c-hsotg: Check for new request before enqueing new setup Ben Dooks
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

Limit the IN FIFO write to a single packet per attempt at writing,
as per the specifications and ensure that we don't return fifo-full
so that we can continue writing packets if we have the space.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 819e75a..7c35bfe 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -539,6 +539,17 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
 	if (can_write > 512)
 		can_write = 512;
 
+	/* limit the write to one max-packet size worth of data, but allow
+	 * the transfer to return that it did not run out of fifo space
+	 * doing it. */
+	if (to_write > hs_ep->ep.maxpacket) {
+		to_write = hs_ep->ep.maxpacket;
+
+		s3c_hsotg_en_gsint(hsotg,
+				   periodic ? S3C_GINTSTS_PTxFEmp :
+				   S3C_GINTSTS_NPTxFEmp);
+	}
+
 	/* see if we can write data */
 
 	if (to_write > can_write) {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/11] USB: s3c-hsotg: Check for new request before enqueing new setup
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (6 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 07/11] USB: s3c-hsotg: Only load packet per fifo write Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 09/11] USB: s3c-hsotg: Fix max EP0 IN request length Ben Dooks
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

Before trying a new setup transaction after getting an EP0 in complete
interrupt, check that the driver did not try and send more EP0 IN data
before enqueing a new setup transaction.

This fixes a bug where we cannot send all of the IN data in one go
so split the transfer, but then fail to send all the data as we start
waiting for a new OUT transaction

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 7c35bfe..2555f27 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -1791,7 +1791,7 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
 		if (dir_in) {
 			s3c_hsotg_complete_in(hsotg, hs_ep);
 
-			if (idx == 0)
+			if (idx == 0 && !hs_ep->req)
 				s3c_hsotg_enqueue_setup(hsotg);
 		} else if (using_dma(hsotg)) {
 			/* We're using DMA, we need to fire an OutDone here
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/11] USB: s3c-hsotg: Fix max EP0 IN request length
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (7 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 08/11] USB: s3c-hsotg: Check for new request before enqueing new setup Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  8:20 ` [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit Ben Dooks
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

The maximum length for any EP0 IN request on EP0 is 127 bytes, not 128
as the driver currently has it.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 2555f27..17b0779 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -613,8 +613,7 @@ static unsigned get_ep_limit(struct s3c_hsotg_ep *hs_ep)
 		maxpkt = S3C_DxEPTSIZ_PktCnt_LIMIT + 1;
 	} else {
 		if (hs_ep->dir_in) {
-			/* maxsize = S3C_DIEPTSIZ0_XferSize_LIMIT + 1; */
-			maxsize = 64+64+1;
+			maxsize = 64+64;
 			maxpkt = S3C_DIEPTSIZ0_PktCnt_LIMIT + 1;
 		} else {
 			maxsize = 0x3f;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (8 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 09/11] USB: s3c-hsotg: Fix max EP0 IN request length Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11  9:01   ` Maurus Cuelenaere
  2010-06-11  9:07   ` Marek Szyprowski
  2010-06-11  8:20 ` [PATCH 11/11] USB: s3c-hsotg: Fix OUT packet request retry Ben Dooks
  2010-06-11 10:25 ` s3c-hsotg fixes Marek Szyprowski
  11 siblings, 2 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

The EP0 out limit is the same as the IN limit, so make them the same.

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

tmp-fix
---
 drivers/usb/gadget/s3c-hsotg.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 17b0779..b34a05d 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -612,11 +612,10 @@ static unsigned get_ep_limit(struct s3c_hsotg_ep *hs_ep)
 		maxsize = S3C_DxEPTSIZ_XferSize_LIMIT + 1;
 		maxpkt = S3C_DxEPTSIZ_PktCnt_LIMIT + 1;
 	} else {
+		maxsize = 64+64;
 		if (hs_ep->dir_in) {
-			maxsize = 64+64;
 			maxpkt = S3C_DIEPTSIZ0_PktCnt_LIMIT + 1;
 		} else {
-			maxsize = 0x3f;
 			maxpkt = 2;
 		}
 	}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/11] USB: s3c-hsotg: Fix OUT packet request retry
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (9 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit Ben Dooks
@ 2010-06-11  8:20 ` Ben Dooks
  2010-06-11 10:25 ` s3c-hsotg fixes Marek Szyprowski
  11 siblings, 0 replies; 21+ messages in thread
From: Ben Dooks @ 2010-06-11  8:20 UTC (permalink / raw)
  To: linux-arm-kernel

If there is more data in the request than we could fit into a single
hardware request, then check when the OutDone event is received if
we have more data, and if so, schedule the new data instead of trying
to complete the request (and in the case of EP0, sending a 0 packet
in the middle of a transfer).

Also, move the debug message about the current transfer state before
the warning about a bad transfer.

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index b34a05d..3da45f5 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -1384,6 +1384,9 @@ static void s3c_hsotg_rx_data(struct s3c_hsotg *hsotg, int ep_idx, int size)
 	read_ptr = hs_req->req.actual;
 	max_req = hs_req->req.length - read_ptr;
 
+	dev_dbg(hsotg->dev, "%s: read %d/%d, done %d/%d\n",
+		__func__, to_read, max_req, read_ptr, hs_req->req.length);
+
 	if (to_read > max_req) {
 		/* more data appeared than we where willing
 		 * to deal with in this request.
@@ -1393,9 +1396,6 @@ static void s3c_hsotg_rx_data(struct s3c_hsotg *hsotg, int ep_idx, int size)
 		WARN_ON_ONCE(1);
 	}
 
-	dev_dbg(hsotg->dev, "%s: read %d/%d, done %d/%d\n",
-		__func__, to_read, max_req, read_ptr, hs_req->req.length);
-
 	hs_ep->total_data += to_read;
 	hs_req->req.actual += to_read;
 	to_read = DIV_ROUND_UP(to_read, 4);
@@ -1464,9 +1464,11 @@ static void s3c_hsotg_send_zlp(struct s3c_hsotg *hsotg,
 static void s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
 				     int epnum, bool was_setup)
 {
+	u32 epsize = readl(hsotg->regs + S3C_DOEPTSIZ(epnum));
 	struct s3c_hsotg_ep *hs_ep = &hsotg->eps[epnum];
 	struct s3c_hsotg_req *hs_req = hs_ep->req;
 	struct usb_request *req = &hs_req->req;
+	unsigned size_left = S3C_DxEPTSIZ_XferSize_GET(epsize);
 	int result = 0;
 
 	if (!hs_req) {
@@ -1475,9 +1477,7 @@ static void s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
 	}
 
 	if (using_dma(hsotg)) {
-		u32 epsize = readl(hsotg->regs + S3C_DOEPTSIZ(epnum));
 		unsigned size_done;
-		unsigned size_left;
 
 		/* Calculate the size of the transfer by checking how much
 		 * is left in the endpoint size register and then working it
@@ -1487,14 +1487,18 @@ static void s3c_hsotg_handle_outdone(struct s3c_hsotg *hsotg,
 		 * so may overshoot/undershoot the transfer.
 		 */
 
-		size_left = S3C_DxEPTSIZ_XferSize_GET(epsize);
-
 		size_done = hs_ep->size_loaded - size_left;
 		size_done += hs_ep->last_load;
 
 		req->actual = size_done;
 	}
 
+	/* if there is more request to do, schedule new transfer */
+	if (req->actual < req->length && size_left == 0) {
+		s3c_hsotg_start_req(hsotg, hs_ep, hs_req, true);
+		return;
+	}
+
 	if (req->actual < req->length && req->short_not_ok) {
 		dev_dbg(hsotg->dev, "%s: got %d/%d (short not ok) => error\n",
 			__func__, req->actual, req->length);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode
  2010-06-11  8:20 ` [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode Ben Dooks
@ 2010-06-11  8:54     ` Maurus Cuelenaere
  0 siblings, 0 replies; 21+ messages in thread
From: Maurus Cuelenaere @ 2010-06-11  8:54 UTC (permalink / raw)
  To: linux-samsung-soc; +Cc: Ben Dooks

Op 11-06-10 10:20, Ben Dooks schreef:
> Add support for the dedicated FIFO mode on newer SoCs such as the S5PV210
> partly to improve support and to fix the bug where any non-EP0 IN endpoint
> requires its own FIFO allocation.
> 
> To fix this, we ensure that any non-zero IN endpoint is given a TXFIFO
> using the same allocation method as the periodic case (all our current
> hardware has enough FIFOs and FIFO memory for a 1:1 mapping) and ensure
> that the necessary transmission done interrupt is enabled.
> 
> The default settings from reset for the core point all EPs at FIFO0,
> used for the control endpoint. However, the controller documentation
> states that all IN endpoints _must_ have a unique FIFO to avoid any
> contention during transmission.
> 
> Note, this leaves us with a large IN FIFO for EP0 (which re-uses the
> old NPTXFIFO) for an endpoint which cannot shift more than a pair of
> packets at a time... this is a waste, but it looks like we cannot
> re-allocate space to the individual IN FIFOs as they are already
> maxed out (to be confirmed).
> 
> Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org?

Shouldn't this be s/\?/>/ ?

> ---
>  .../arm/plat-samsung/include/plat/regs-usb-hsotg.h |    2 +
>  drivers/usb/gadget/s3c-hsotg.c                     |   40 +++++++++++++++++++-
>  2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> index 8d18d9d..dc90f5e 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> @@ -226,6 +226,7 @@
>  
>  #define S3C_DIEPMSK				S3C_HSOTG_REG(0x810)
>  
> +#define S3C_DIEPMSK_TxFIFOEmpty			(1 << 7)
>  #define S3C_DIEPMSK_INEPNakEffMsk		(1 << 6)
>  #define S3C_DIEPMSK_INTknEPMisMsk		(1 << 5)
>  #define S3C_DIEPMSK_INTknTXFEmpMsk		(1 << 4)
> @@ -371,6 +372,7 @@
>  
>  #define S3C_DIEPDMA(_a)				S3C_HSOTG_REG(0x914 + ((_a) * 0x20))
>  #define S3C_DOEPDMA(_a)				S3C_HSOTG_REG(0xB14 + ((_a) * 0x20))
> +#define S3C_DTXFSTS(_a)				S3C_HSOTG_REG(0x918 + ((_a) * 0x20))
>  
>  #define S3C_EPFIFO(_a)				S3C_HSOTG_REG(0x1000 + ((_a) * 0x1000))
>  
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index 5b61bcf..819e75a 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -12,6 +12,8 @@
>   * published by the Free Software Foundation.
>  */
>  
> +#define DEBUG

I don't think this is an intended change?

> +
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
> @@ -131,6 +133,7 @@ struct s3c_hsotg_ep {
>   * @regs: The memory area mapped for accessing registers.
>   * @regs_res: The resource that was allocated when claiming register space.
>   * @irq: The IRQ number we are using
> + * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
>   * @debug_root: root directrory for debugfs.
>   * @debug_file: main status file for debugfs.
>   * @debug_fifo: FIFO status file for debugfs.
> @@ -149,6 +152,8 @@ struct s3c_hsotg {
>  	struct resource		*regs_res;
>  	int			irq;
>  
> +	unsigned int		dedicated_fifos:1;
> +
>  	struct dentry		*debug_root;
>  	struct dentry		*debug_file;
>  	struct dentry		*debug_fifo;
> @@ -467,7 +472,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>  	if (to_write == 0)
>  		return 0;
>  
> -	if (periodic) {
> +	if (periodic && !hsotg->dedicated_fifos) {
>  		u32 epsize = readl(hsotg->regs + S3C_DIEPTSIZ(hs_ep->index));
>  		int size_left;
>  		int size_done;
> @@ -505,6 +510,11 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>  			s3c_hsotg_en_gsint(hsotg, S3C_GINTSTS_PTxFEmp);
>  			return -ENOSPC;
>  		}
> +	} else if (hsotg->dedicated_fifos && hs_ep->index != 0) {
> +		can_write = readl(hsotg->regs + S3C_DTXFSTS(hs_ep->index));
> +
> +		can_write &= 0xffff;
> +		can_write *= 4;
>  	} else {
>  		if (S3C_GNPTXSTS_NPTxQSpcAvail_GET(gnptxsts) == 0) {
>  			dev_dbg(hsotg->dev,
> @@ -1830,6 +1840,15 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
>  				 __func__, idx);
>  			clear |= S3C_DIEPMSK_INTknEPMisMsk;
>  		}
> +
> +		/* FIFO has space or is empty (see GAHBCFG) */
> +		if (hsotg->dedicated_fifos &&
> +		    ints & S3C_DIEPMSK_TxFIFOEmpty) {
> +			dev_dbg(hsotg->dev, "%s: ep%d: TxFIFOEmpty\n",
> +				__func__, idx);
> +			s3c_hsotg_trytx(hsotg, hs_ep);
> +			clear |= S3C_DIEPMSK_TxFIFOEmpty;
> +		}
>  	}
>  
>  	writel(clear, hsotg->regs + epint_reg);
> @@ -2281,6 +2300,12 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>  		break;
>  	}
>  
> +	/* if the hardware has dedicated fifos, we must give each IN EP
> +	 * a unique tx-fifo even if it is non-periodic.
> +	 */
> +	if (dir_in && hsotg->dedicated_fifos)
> +		epctrl |= S3C_DxEPCTL_TxFNum(index);
> +
>  	/* for non control endpoints, set PID to D0 */
>  	if (index)
>  		epctrl |= S3C_DxEPCTL_SetD0PID;
> @@ -2570,7 +2595,8 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>  
>  	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
>  	       S3C_DIEPMSK_INTknEPMisMsk |
> -	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk,
> +	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk |
> +	       ((hsotg->dedicated_fifos) ? S3C_DIEPMSK_TxFIFOEmpty : 0),
>  	       hsotg->regs + S3C_DIEPMSK);
>  
>  	/* don't need XferCompl, we get that from RXFIFO in slave mode. In
> @@ -2797,6 +2823,8 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
>  
>  static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
>  {
> +	u32 cfg4;
> +
>  	/* unmask subset of endpoint interrupts */
>  
>  	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
> @@ -2832,6 +2860,14 @@ static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
>  
>  	writel(using_dma(hsotg) ? S3C_GAHBCFG_DMAEn : 0x0,
>  	       hsotg->regs + S3C_GAHBCFG);
> +
> +	/* check hardware configuration */
> +
> +	cfg4 = readl(hsotg->regs + 0x50);
> +	hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
> +
> +	dev_info(hsotg->dev, "%s fifos\n",
> +		 hsotg->dedicated_fifos ? "dedicated" : "shared");
>  }
>  
>  static void s3c_hsotg_dump(struct s3c_hsotg *hsotg)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode
@ 2010-06-11  8:54     ` Maurus Cuelenaere
  0 siblings, 0 replies; 21+ messages in thread
From: Maurus Cuelenaere @ 2010-06-11  8:54 UTC (permalink / raw)
  To: linux-arm-kernel

Op 11-06-10 10:20, Ben Dooks schreef:
> Add support for the dedicated FIFO mode on newer SoCs such as the S5PV210
> partly to improve support and to fix the bug where any non-EP0 IN endpoint
> requires its own FIFO allocation.
> 
> To fix this, we ensure that any non-zero IN endpoint is given a TXFIFO
> using the same allocation method as the periodic case (all our current
> hardware has enough FIFOs and FIFO memory for a 1:1 mapping) and ensure
> that the necessary transmission done interrupt is enabled.
> 
> The default settings from reset for the core point all EPs at FIFO0,
> used for the control endpoint. However, the controller documentation
> states that all IN endpoints _must_ have a unique FIFO to avoid any
> contention during transmission.
> 
> Note, this leaves us with a large IN FIFO for EP0 (which re-uses the
> old NPTXFIFO) for an endpoint which cannot shift more than a pair of
> packets at a time... this is a waste, but it looks like we cannot
> re-allocate space to the individual IN FIFOs as they are already
> maxed out (to be confirmed).
> 
> Signed-off-by: Ben Dooks <ben-linux@fluff.org?

Shouldn't this be s/\?/>/ ?

> ---
>  .../arm/plat-samsung/include/plat/regs-usb-hsotg.h |    2 +
>  drivers/usb/gadget/s3c-hsotg.c                     |   40 +++++++++++++++++++-
>  2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> index 8d18d9d..dc90f5e 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-usb-hsotg.h
> @@ -226,6 +226,7 @@
>  
>  #define S3C_DIEPMSK				S3C_HSOTG_REG(0x810)
>  
> +#define S3C_DIEPMSK_TxFIFOEmpty			(1 << 7)
>  #define S3C_DIEPMSK_INEPNakEffMsk		(1 << 6)
>  #define S3C_DIEPMSK_INTknEPMisMsk		(1 << 5)
>  #define S3C_DIEPMSK_INTknTXFEmpMsk		(1 << 4)
> @@ -371,6 +372,7 @@
>  
>  #define S3C_DIEPDMA(_a)				S3C_HSOTG_REG(0x914 + ((_a) * 0x20))
>  #define S3C_DOEPDMA(_a)				S3C_HSOTG_REG(0xB14 + ((_a) * 0x20))
> +#define S3C_DTXFSTS(_a)				S3C_HSOTG_REG(0x918 + ((_a) * 0x20))
>  
>  #define S3C_EPFIFO(_a)				S3C_HSOTG_REG(0x1000 + ((_a) * 0x1000))
>  
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index 5b61bcf..819e75a 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -12,6 +12,8 @@
>   * published by the Free Software Foundation.
>  */
>  
> +#define DEBUG

I don't think this is an intended change?

> +
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
> @@ -131,6 +133,7 @@ struct s3c_hsotg_ep {
>   * @regs: The memory area mapped for accessing registers.
>   * @regs_res: The resource that was allocated when claiming register space.
>   * @irq: The IRQ number we are using
> + * @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
>   * @debug_root: root directrory for debugfs.
>   * @debug_file: main status file for debugfs.
>   * @debug_fifo: FIFO status file for debugfs.
> @@ -149,6 +152,8 @@ struct s3c_hsotg {
>  	struct resource		*regs_res;
>  	int			irq;
>  
> +	unsigned int		dedicated_fifos:1;
> +
>  	struct dentry		*debug_root;
>  	struct dentry		*debug_file;
>  	struct dentry		*debug_fifo;
> @@ -467,7 +472,7 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>  	if (to_write == 0)
>  		return 0;
>  
> -	if (periodic) {
> +	if (periodic && !hsotg->dedicated_fifos) {
>  		u32 epsize = readl(hsotg->regs + S3C_DIEPTSIZ(hs_ep->index));
>  		int size_left;
>  		int size_done;
> @@ -505,6 +510,11 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
>  			s3c_hsotg_en_gsint(hsotg, S3C_GINTSTS_PTxFEmp);
>  			return -ENOSPC;
>  		}
> +	} else if (hsotg->dedicated_fifos && hs_ep->index != 0) {
> +		can_write = readl(hsotg->regs + S3C_DTXFSTS(hs_ep->index));
> +
> +		can_write &= 0xffff;
> +		can_write *= 4;
>  	} else {
>  		if (S3C_GNPTXSTS_NPTxQSpcAvail_GET(gnptxsts) == 0) {
>  			dev_dbg(hsotg->dev,
> @@ -1830,6 +1840,15 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
>  				 __func__, idx);
>  			clear |= S3C_DIEPMSK_INTknEPMisMsk;
>  		}
> +
> +		/* FIFO has space or is empty (see GAHBCFG) */
> +		if (hsotg->dedicated_fifos &&
> +		    ints & S3C_DIEPMSK_TxFIFOEmpty) {
> +			dev_dbg(hsotg->dev, "%s: ep%d: TxFIFOEmpty\n",
> +				__func__, idx);
> +			s3c_hsotg_trytx(hsotg, hs_ep);
> +			clear |= S3C_DIEPMSK_TxFIFOEmpty;
> +		}
>  	}
>  
>  	writel(clear, hsotg->regs + epint_reg);
> @@ -2281,6 +2300,12 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>  		break;
>  	}
>  
> +	/* if the hardware has dedicated fifos, we must give each IN EP
> +	 * a unique tx-fifo even if it is non-periodic.
> +	 */
> +	if (dir_in && hsotg->dedicated_fifos)
> +		epctrl |= S3C_DxEPCTL_TxFNum(index);
> +
>  	/* for non control endpoints, set PID to D0 */
>  	if (index)
>  		epctrl |= S3C_DxEPCTL_SetD0PID;
> @@ -2570,7 +2595,8 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
>  
>  	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
>  	       S3C_DIEPMSK_INTknEPMisMsk |
> -	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk,
> +	       S3C_DIEPMSK_EPDisbldMsk | S3C_DIEPMSK_XferComplMsk |
> +	       ((hsotg->dedicated_fifos) ? S3C_DIEPMSK_TxFIFOEmpty : 0),
>  	       hsotg->regs + S3C_DIEPMSK);
>  
>  	/* don't need XferCompl, we get that from RXFIFO in slave mode. In
> @@ -2797,6 +2823,8 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
>  
>  static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
>  {
> +	u32 cfg4;
> +
>  	/* unmask subset of endpoint interrupts */
>  
>  	writel(S3C_DIEPMSK_TimeOUTMsk | S3C_DIEPMSK_AHBErrMsk |
> @@ -2832,6 +2860,14 @@ static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
>  
>  	writel(using_dma(hsotg) ? S3C_GAHBCFG_DMAEn : 0x0,
>  	       hsotg->regs + S3C_GAHBCFG);
> +
> +	/* check hardware configuration */
> +
> +	cfg4 = readl(hsotg->regs + 0x50);
> +	hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
> +
> +	dev_info(hsotg->dev, "%s fifos\n",
> +		 hsotg->dedicated_fifos ? "dedicated" : "shared");
>  }
>  
>  static void s3c_hsotg_dump(struct s3c_hsotg *hsotg)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit
  2010-06-11  8:20 ` [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit Ben Dooks
@ 2010-06-11  9:01   ` Maurus Cuelenaere
  2010-06-11  9:07   ` Marek Szyprowski
  1 sibling, 0 replies; 21+ messages in thread
From: Maurus Cuelenaere @ 2010-06-11  9:01 UTC (permalink / raw)
  To: linux-arm-kernel




Op 11-06-10 10:20, Ben Dooks schreef:
> The EP0 out limit is the same as the IN limit, so make them the same.
> 
> Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> 
> tmp-fix

Shouldn't this line be removed?

> ---
>  drivers/usb/gadget/s3c-hsotg.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index 17b0779..b34a05d 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -612,11 +612,10 @@ static unsigned get_ep_limit(struct s3c_hsotg_ep *hs_ep)
>  		maxsize = S3C_DxEPTSIZ_XferSize_LIMIT + 1;
>  		maxpkt = S3C_DxEPTSIZ_PktCnt_LIMIT + 1;
>  	} else {
> +		maxsize = 64+64;
>  		if (hs_ep->dir_in) {
> -			maxsize = 64+64;
>  			maxpkt = S3C_DIEPTSIZ0_PktCnt_LIMIT + 1;
>  		} else {
> -			maxsize = 0x3f;
>  			maxpkt = 2;
>  		}
>  	}

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit
  2010-06-11  8:20 ` [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit Ben Dooks
  2010-06-11  9:01   ` Maurus Cuelenaere
@ 2010-06-11  9:07   ` Marek Szyprowski
  1 sibling, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2010-06-11  9:07 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Friday, June 11, 2010 10:21 AM Ben Dooks wrote:

> The EP0 out limit is the same as the IN limit, so make them the same.
> 
> Signed-off-by: Ben Dooks <ben-linux@fluff.org>
> 
> tmp-fix

^ looks like this temporary fix managed to get into the final patch series
without proper cleanup.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock
  2010-06-11  8:20 ` [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock Ben Dooks
@ 2010-06-11  9:35   ` Marek Szyprowski
  2010-06-11  9:46     ` Maurus Cuelenaere
  0 siblings, 1 reply; 21+ messages in thread
From: Marek Szyprowski @ 2010-06-11  9:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Friday, June 11, 2010 10:21 AM Ben Dooks wrote:

> From: Maurus Cuelenaere <mcuelenaere@gmail.com>
> 
> The PLL that drives the USB clock supports 3 input clocks: 12, 24 and 48Mhz.
> This patch adds support to the USB driver for setting the correct register
> bit
> according to the given clock.
> 
> This depends on the following patch:
> [PATCH] ARM: S3C64XX: Add USB external clock definition
> 
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
> Signed-off-by: Ben Dooks <ben-linux@fluff.org>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-
> hsotg.c
> index 26193ec..6a303ce 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -23,6 +23,7 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
> +#include <linux/clk.h>
> 
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> @@ -2753,6 +2754,7 @@ static void __devinit s3c_hsotg_initep(struct
> s3c_hsotg *hsotg,
>   */
>  static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
>  {
> +	struct clk *xusbxti;
>  	u32 osc;
> 
>  	writel(0, S3C_PHYPWR);
> @@ -2760,6 +2762,23 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg
> *hsotg)
> 
>  	osc = hsotg->plat->is_osc ? S3C_PHYCLK_EXT_OSC : 0;
> 
> +	xusbxti = clk_get(hsotg->dev, "xusbxti");
> +	if (xusbxti && !IS_ERR(xusbxti)) {
> +		switch (clk_get_rate(xusbxti)) {
> +		case 12000000:
> +		    osc |= S3C_PHYCLK_CLKSEL_12M;
> +		    break;
> +		case 24000000:
> +		    osc |= S3C_PHYCLK_CLKSEL_24M;
> +		    break;
> +		default:
> +		case 48000000:
> +		    /* default reference clock */
> +		    break;

One quick style issue: there is some tabs-vs-spaces mess in the above
code.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock
  2010-06-11  9:35   ` Marek Szyprowski
@ 2010-06-11  9:46     ` Maurus Cuelenaere
  0 siblings, 0 replies; 21+ messages in thread
From: Maurus Cuelenaere @ 2010-06-11  9:46 UTC (permalink / raw)
  To: linux-arm-kernel

The PLL that drives the USB clock supports 3 input clocks: 12, 24 and 48Mhz.
This patch adds support to the USB driver for setting the correct register bit
according to the given clock.

This depends on the following patch:
[PATCH] ARM: S3C64XX: Add USB external clock definition

Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---

v2: - fixes space issues
    - use MHZ define

---
 drivers/usb/gadget/s3c-hsotg.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 1f73b48..40ee63a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
@@ -2699,6 +2700,7 @@ static void __devinit s3c_hsotg_initep(struct s3c_hsotg *hsotg,
  */
 static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
 {
+	struct clk *xusbxti;
 	u32 osc;
 
 	writel(0, S3C_PHYPWR);
@@ -2706,6 +2708,23 @@ static void s3c_hsotg_otgreset(struct s3c_hsotg *hsotg)
 
 	osc = hsotg->plat->is_osc ? S3C_PHYCLK_EXT_OSC : 0;
 
+	xusbxti = clk_get(hsotg->dev, "xusbxti");
+	if (xusbxti && !IS_ERR(xusbxti)) {
+		switch (clk_get_rate(xusbxti)) {
+		case 12*MHZ:
+			osc |= S3C_PHYCLK_CLKSEL_12M;
+			break;
+		case 24*MHZ:
+			osc |= S3C_PHYCLK_CLKSEL_24M;
+			break;
+		default:
+		case 48*MHZ:
+			/* default reference clock */
+			break;
+		}
+		clk_put(xusbxti);
+	}
+
 	writel(osc | 0x10, S3C_PHYCLK);
 
 	/* issue a full set of resets to the otg and core */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* s3c-hsotg fixes
  2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
                   ` (10 preceding siblings ...)
  2010-06-11  8:20 ` [PATCH 11/11] USB: s3c-hsotg: Fix OUT packet request retry Ben Dooks
@ 2010-06-11 10:25 ` Marek Szyprowski
  2010-06-12  3:34   ` Ben Dooks
  11 siblings, 1 reply; 21+ messages in thread
From: Marek Szyprowski @ 2010-06-11 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Friday, June 11, 2010 10:21 AM Ben Dooks wrote:

> Some further fixes for the s3c-hsotg block, as well as a repost of
> the missed USB phy clock setting patch which either got missed or
> passed over from last time.
> 
> As a note, this series has a patch adding support for dedicated FIFO
> mode, this is in my view a fix, as when the hsotg block is compiled
> with dedicated fifos then each USB IN non-periodic pipe needs to be
> alloacted a unique TX FIFO. We have no actual data on how well the
> block performs when all IN TX NP FIFOs are all set to the same one,
> but it really should be fixed.

This patch series improves the driver a lot. Now it is possible to get
CDC Ethernet gadget working on S5PV210 SoC (Samsung Aquila machine).
However there are still some issues left to be resolved. I've noticed
the following things:

1. CDC Ethernet gadget: there are problems after unplugging and 
plugging the usb cable again while data is being transferred. To trigger
this it is enough to run 'ping host' command on the device and replug
the usb cable. After that no data packets are sent do the usb host
(checked with hardware usb analyser), although the ep0 control transfers
are performed correctly (device has been enumerated correctly).

2. RNDIS Ethernet gadget: after some stress tests I get an error, which
causes RNDIS session to hang (what is probably a direct result of the
otg block reinit/reset). Here is an example log:

[   69.155000] s3c-hsotg s3c-hsotg: s3c_hsotg_handle_rx: unknown status 00000000
[   69.155000] s3c-hsotg s3c-hsotg: DCFG=0x00040020, DCTL=0x00000000,
DIEPMSK=000000af
[   69.155000] s3c-hsotg s3c-hsotg: GAHBCFG=0x00000001, 0x44=0x00000000
[   69.155000] s3c-hsotg s3c-hsotg: GRXFSIZ=0x00000800, GNPTXFSIZ=0x04000800
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[1] FSize=768, StAddr=0x00000f00
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[2] FSize=768, StAddr=0x00001200
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[3] FSize=768, StAddr=0x00001500
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[4] FSize=768, StAddr=0x00001800
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[5] FSize=768, StAddr=0x00001b00
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[6] FSize=768, StAddr=0x00001e00
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[7] FSize=768, StAddr=0x00002100
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[8] FSize=768, StAddr=0x00002400
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[9] FSize=768, StAddr=0x00002700
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[10] FSize=768, StAddr=0x00002a00
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[11] FSize=768, StAddr=0x00002d00
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[12] FSize=768, StAddr=0x00003000
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[13] FSize=768, StAddr=0x00003300
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[14] FSize=768, StAddr=0x00003600
[   69.155000] s3c-hsotg s3c-hsotg: DPTx[15] FSize=768, StAddr=0x00003900
[   69.155000] s3c-hsotg s3c-hsotg: ep0-in: EPCTL=0x00028000, SIZ=0x00000000,
DMA=0x085290ed
[   69.155000] s3c-hsotg s3c-hsotg: ep0-out: EPCTL=0x80028000, SIZ=0x60000000,
DMA=0xca625c65
[   69.155000] s3c-hsotg s3c-hsotg: ep1-in: EPCTL=0x00488200, SIZ=0x20000000,
DMA=0x5f9184b4
[   69.155000] s3c-hsotg s3c-hsotg: ep1-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x318b408d
[   69.155000] s3c-hsotg s3c-hsotg: ep2-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x50d8cf8b
[   69.155000] s3c-hsotg s3c-hsotg: ep2-out: EPCTL=0x800b8200, SIZ=0x00180800,
DMA=0x5792d85e
[   69.155000] s3c-hsotg s3c-hsotg: ep3-in: EPCTL=0x00cc8008, SIZ=0x00000000,
DMA=0x5dee1680
[   69.155000] s3c-hsotg s3c-hsotg: ep3-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0xa96c912b
[   69.155000] s3c-hsotg s3c-hsotg: ep4-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x5e84ae7a
[   69.155000] s3c-hsotg s3c-hsotg: ep4-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0xef2e9c78
[   69.155000] s3c-hsotg s3c-hsotg: ep5-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x9d8f4d72
[   69.155000] s3c-hsotg s3c-hsotg: ep5-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x93e82a21
[   69.155000] s3c-hsotg s3c-hsotg: ep6-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x202acadf
[   69.155000] s3c-hsotg s3c-hsotg: ep6-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x5520c3c9
[   69.155000] s3c-hsotg s3c-hsotg: ep7-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x782200c2
[   69.155000] s3c-hsotg s3c-hsotg: ep7-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0xeb0b8035
[   69.155000] s3c-hsotg s3c-hsotg: ep8-in: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x002de535
[   69.155000] s3c-hsotg s3c-hsotg: ep8-out: EPCTL=0x00000200, SIZ=0x00000000,
DMA=0x8c0b77f7
[   69.155000] s3c-hsotg s3c-hsotg: ep9-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xa59e32a0
[   69.155000] s3c-hsotg s3c-hsotg: ep9-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0x07aa36a2
[   69.155000] s3c-hsotg s3c-hsotg: ep10-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xc6732fd5
[   69.155000] s3c-hsotg s3c-hsotg: ep10-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xfbf3f43d
[   69.155000] s3c-hsotg s3c-hsotg: ep11-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xb1d750a4
[   69.155000] s3c-hsotg s3c-hsotg: ep11-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0x30024c06
[   69.155000] s3c-hsotg s3c-hsotg: ep12-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0x154162af
[   69.155000] s3c-hsotg s3c-hsotg: ep12-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0x60f9ef07
[   69.155000] s3c-hsotg s3c-hsotg: ep13-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xbea16bb2
[   69.155000] s3c-hsotg s3c-hsotg: ep13-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xd1723f0c
[   69.155000] s3c-hsotg s3c-hsotg: ep14-in: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0x40d1795b
[   69.155000] s3c-hsotg s3c-hsotg: ep14-out: EPCTL=0x00000000, SIZ=0x00000000,
DMA=0xc0b155d6
[   69.155000] s3c-hsotg s3c-hsotg: DVBUSDIS=0x00000b8f, DVBUSPULSE=000002c6

I will try to investigate these issues further.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* s3c-hsotg fixes
  2010-06-11 10:25 ` s3c-hsotg fixes Marek Szyprowski
@ 2010-06-12  3:34   ` Ben Dooks
  2010-06-14  5:40     ` Marek Szyprowski
  0 siblings, 1 reply; 21+ messages in thread
From: Ben Dooks @ 2010-06-12  3:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 11, 2010 at 12:25:26PM +0200, Marek Szyprowski wrote:
> Hello,
> 
> On Friday, June 11, 2010 10:21 AM Ben Dooks wrote:
> 
> > Some further fixes for the s3c-hsotg block, as well as a repost of
> > the missed USB phy clock setting patch which either got missed or
> > passed over from last time.
> > 
> > As a note, this series has a patch adding support for dedicated FIFO
> > mode, this is in my view a fix, as when the hsotg block is compiled
> > with dedicated fifos then each USB IN non-periodic pipe needs to be
> > alloacted a unique TX FIFO. We have no actual data on how well the
> > block performs when all IN TX NP FIFOs are all set to the same one,
> > but it really should be fixed.
> 
> This patch series improves the driver a lot. Now it is possible to get
> CDC Ethernet gadget working on S5PV210 SoC (Samsung Aquila machine).
> However there are still some issues left to be resolved. I've noticed
> the following things:
> 
> 1. CDC Ethernet gadget: there are problems after unplugging and 
> plugging the usb cable again while data is being transferred. To trigger
> this it is enough to run 'ping host' command on the device and replug
> the usb cable. After that no data packets are sent do the usb host
> (checked with hardware usb analyser), although the ep0 control transfers
> are performed correctly (device has been enumerated correctly).

Hmm, that is intereting. Maybe we broke non-ep0 fifos somehow
 
> 2. RNDIS Ethernet gadget: after some stress tests I get an error, which
> causes RNDIS session to hang (what is probably a direct result of the
> otg block reinit/reset). Here is an example log:

any particular kind of stress tests, or are you just telling it nasty
things to upset it? will try and look into this too.

 
> [   69.155000] s3c-hsotg s3c-hsotg: s3c_hsotg_handle_rx: unknown status 00000000
> [   69.155000] s3c-hsotg s3c-hsotg: DCFG=0x00040020, DCTL=0x00000000,
> DIEPMSK=000000af
> [   69.155000] s3c-hsotg s3c-hsotg: GAHBCFG=0x00000001, 0x44=0x00000000
> [   69.155000] s3c-hsotg s3c-hsotg: GRXFSIZ=0x00000800, GNPTXFSIZ=0x04000800
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[1] FSize=768, StAddr=0x00000f00
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[2] FSize=768, StAddr=0x00001200
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[3] FSize=768, StAddr=0x00001500
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[4] FSize=768, StAddr=0x00001800
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[5] FSize=768, StAddr=0x00001b00
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[6] FSize=768, StAddr=0x00001e00
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[7] FSize=768, StAddr=0x00002100
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[8] FSize=768, StAddr=0x00002400
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[9] FSize=768, StAddr=0x00002700
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[10] FSize=768, StAddr=0x00002a00
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[11] FSize=768, StAddr=0x00002d00
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[12] FSize=768, StAddr=0x00003000
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[13] FSize=768, StAddr=0x00003300
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[14] FSize=768, StAddr=0x00003600
> [   69.155000] s3c-hsotg s3c-hsotg: DPTx[15] FSize=768, StAddr=0x00003900
> [   69.155000] s3c-hsotg s3c-hsotg: ep0-in: EPCTL=0x00028000, SIZ=0x00000000,
> DMA=0x085290ed
> [   69.155000] s3c-hsotg s3c-hsotg: ep0-out: EPCTL=0x80028000, SIZ=0x60000000,
> DMA=0xca625c65
> [   69.155000] s3c-hsotg s3c-hsotg: ep1-in: EPCTL=0x00488200, SIZ=0x20000000,
> DMA=0x5f9184b4
> [   69.155000] s3c-hsotg s3c-hsotg: ep1-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x318b408d
> [   69.155000] s3c-hsotg s3c-hsotg: ep2-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x50d8cf8b
> [   69.155000] s3c-hsotg s3c-hsotg: ep2-out: EPCTL=0x800b8200, SIZ=0x00180800,
> DMA=0x5792d85e
> [   69.155000] s3c-hsotg s3c-hsotg: ep3-in: EPCTL=0x00cc8008, SIZ=0x00000000,
> DMA=0x5dee1680
> [   69.155000] s3c-hsotg s3c-hsotg: ep3-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0xa96c912b
> [   69.155000] s3c-hsotg s3c-hsotg: ep4-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x5e84ae7a
> [   69.155000] s3c-hsotg s3c-hsotg: ep4-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0xef2e9c78
> [   69.155000] s3c-hsotg s3c-hsotg: ep5-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x9d8f4d72
> [   69.155000] s3c-hsotg s3c-hsotg: ep5-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x93e82a21
> [   69.155000] s3c-hsotg s3c-hsotg: ep6-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x202acadf
> [   69.155000] s3c-hsotg s3c-hsotg: ep6-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x5520c3c9
> [   69.155000] s3c-hsotg s3c-hsotg: ep7-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x782200c2
> [   69.155000] s3c-hsotg s3c-hsotg: ep7-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0xeb0b8035
> [   69.155000] s3c-hsotg s3c-hsotg: ep8-in: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x002de535
> [   69.155000] s3c-hsotg s3c-hsotg: ep8-out: EPCTL=0x00000200, SIZ=0x00000000,
> DMA=0x8c0b77f7
> [   69.155000] s3c-hsotg s3c-hsotg: ep9-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xa59e32a0
> [   69.155000] s3c-hsotg s3c-hsotg: ep9-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0x07aa36a2
> [   69.155000] s3c-hsotg s3c-hsotg: ep10-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xc6732fd5
> [   69.155000] s3c-hsotg s3c-hsotg: ep10-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xfbf3f43d
> [   69.155000] s3c-hsotg s3c-hsotg: ep11-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xb1d750a4
> [   69.155000] s3c-hsotg s3c-hsotg: ep11-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0x30024c06
> [   69.155000] s3c-hsotg s3c-hsotg: ep12-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0x154162af
> [   69.155000] s3c-hsotg s3c-hsotg: ep12-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0x60f9ef07
> [   69.155000] s3c-hsotg s3c-hsotg: ep13-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xbea16bb2
> [   69.155000] s3c-hsotg s3c-hsotg: ep13-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xd1723f0c
> [   69.155000] s3c-hsotg s3c-hsotg: ep14-in: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0x40d1795b
> [   69.155000] s3c-hsotg s3c-hsotg: ep14-out: EPCTL=0x00000000, SIZ=0x00000000,
> DMA=0xc0b155d6
> [   69.155000] s3c-hsotg s3c-hsotg: DVBUSDIS=0x00000b8f, DVBUSPULSE=000002c6
> 
> I will try to investigate these issues further.
> 
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* s3c-hsotg fixes
  2010-06-12  3:34   ` Ben Dooks
@ 2010-06-14  5:40     ` Marek Szyprowski
  0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2010-06-14  5:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Saturday, June 12, 2010 5:34 AM Ben Dooks wrote:

> On Fri, Jun 11, 2010 at 12:25:26PM +0200, Marek Szyprowski wrote:
> > Hello,
> >
> > On Friday, June 11, 2010 10:21 AM Ben Dooks wrote:
> >
> > > Some further fixes for the s3c-hsotg block, as well as a repost of
> > > the missed USB phy clock setting patch which either got missed or
> > > passed over from last time.
> > >
> > > As a note, this series has a patch adding support for dedicated FIFO
> > > mode, this is in my view a fix, as when the hsotg block is compiled
> > > with dedicated fifos then each USB IN non-periodic pipe needs to be
> > > alloacted a unique TX FIFO. We have no actual data on how well the
> > > block performs when all IN TX NP FIFOs are all set to the same one,
> > > but it really should be fixed.
> >
> > This patch series improves the driver a lot. Now it is possible to get
> > CDC Ethernet gadget working on S5PV210 SoC (Samsung Aquila machine).
> > However there are still some issues left to be resolved. I've noticed
> > the following things:
> >
> > 1. CDC Ethernet gadget: there are problems after unplugging and
> > plugging the usb cable again while data is being transferred. To trigger
> > this it is enough to run 'ping host' command on the device and replug
> > the usb cable. After that no data packets are sent do the usb host
> > (checked with hardware usb analyser), although the ep0 control transfers
> > are performed correctly (device has been enumerated correctly).
> 
> Hmm, that is intereting. Maybe we broke non-ep0 fifos somehow
> 
> > 2. RNDIS Ethernet gadget: after some stress tests I get an error, which
> > causes RNDIS session to hang (what is probably a direct result of the
> > otg block reinit/reset). Here is an example log:
> 
> any particular kind of stress tests, or are you just telling it nasty
> things to upset it? will try and look into this too.

I've tested it with Windows XP SP3 host (Pentium4 2.8GHz, VIA USB 2.0 EHCI
controller) and the following script:

--->8--[killer.bat]---
:foo
ping 192.168.129.3
goto foo
--->8-----------------

192.168.129.3 is an IP address of the client system (Linux on S5PV210).

Four instances of this script was running simultaneously and a Task Manager
with Network Status tab was opened. Each time when Windows runs ping command
the RNDIS transaction is performed. So this test basically checked how the
driver would act on a flood of RNDIS transactions. Each NDIS transactions
consists of the following transfers: RNDIS Command (ep0-control in), RNDIS
notification (ep3-interrupt in) and RNDIS Response (ep0-control out). All 3
parts of RNDIS transaction must be performed correctly, otherwise Windows
host locks (it is quite easy to notice this, Network Status graph stops
scrolling). The test took about 5 minutes to trigger the issue.

The flood of RNDIS transactions happens also during normal usage, some
firewalls or anti-virus scanning tools also can cause it, so it shouldn't
be considered something really unusual.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2010-06-14  5:40 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-11  8:20 s3c-hsotg fixes Ben Dooks
2010-06-11  8:20 ` [PATCH 01/11] USB: s3c_hsotg: Add support for external USB clock Ben Dooks
2010-06-11  9:35   ` Marek Szyprowski
2010-06-11  9:46     ` Maurus Cuelenaere
2010-06-11  8:20 ` [PATCH 02/11] USB: s3c-hsotg: Increase TX fifo limit Ben Dooks
2010-06-11  8:20 ` [PATCH 03/11] USB: s3c-hsotg: The NPTX/PTX FIFO sizes are in words, not bytes Ben Dooks
2010-06-11  8:20 ` [PATCH 04/11] USB: s3c-hsotg: Avoid overwriting contents of perodic in 'fifo' Ben Dooks
2010-06-11  8:20 ` [PATCH 05/11] USB: s3c-hsotg: Re-initialise all FIFOs on USB bus reset Ben Dooks
2010-06-11  8:20 ` [PATCH 06/11] USB: s3c-hsotg: Add initial detection and setup for dedicated FIFO mode Ben Dooks
2010-06-11  8:54   ` Maurus Cuelenaere
2010-06-11  8:54     ` Maurus Cuelenaere
2010-06-11  8:20 ` [PATCH 07/11] USB: s3c-hsotg: Only load packet per fifo write Ben Dooks
2010-06-11  8:20 ` [PATCH 08/11] USB: s3c-hsotg: Check for new request before enqueing new setup Ben Dooks
2010-06-11  8:20 ` [PATCH 09/11] USB: s3c-hsotg: Fix max EP0 IN request length Ben Dooks
2010-06-11  8:20 ` [PATCH 10/11] USB: s3c-hsotg: Fix the OUT EP0 limit Ben Dooks
2010-06-11  9:01   ` Maurus Cuelenaere
2010-06-11  9:07   ` Marek Szyprowski
2010-06-11  8:20 ` [PATCH 11/11] USB: s3c-hsotg: Fix OUT packet request retry Ben Dooks
2010-06-11 10:25 ` s3c-hsotg fixes Marek Szyprowski
2010-06-12  3:34   ` Ben Dooks
2010-06-14  5:40     ` Marek Szyprowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.