s3c-hsotg driver updates / fixes

s3c-hsotg driver updates / fixes

[Ben Dooks]

This series includes the usb-xtal rate addition/fix from Marus which
has been submitted (so please ignore if merge is already pending) and
fixes for some of the problems that have been seen.

This is not the full set of fixes, as there are still some more complicated
issues to sort out with things such as stall handling on non-ep0 endpoints
which are being investiagted.

Thanks to Thomas Abraham and Kukjin Kim at Samsung for providing help and
further information to help track down these problems.

[PATCH 1/6] USB: s3c_hsotg: Add support for external USB clock

[Ben Dooks]

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 1f73b48..dce9366 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 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.6.3.3

[PATCH 2/6] USB: s3c_hsotg: define USB_GADGET_DUALSPEED in Kconfig

[Ben Dooks]

From: Maurus Cuelenaere <mcuelenaere@gmail.com>

The s3c_hsotg driver sets usb_gadget->is_dualspeed to 1, yet it doesn't define
USB_GADGET_DUALSPEED in Kconfig. This triggers a NULL pointer dereference in
the composite driver (which is fixed in another patch).

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

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 649c0c5..591ae9f 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -295,6 +295,7 @@ config USB_GADGET_S3C_HSOTG
 	boolean "S3C HS/OtG USB Device controller"
 	depends on S3C_DEV_USB_HSOTG
 	select USB_GADGET_S3C_HSOTG_PIO
+	select USB_GADGET_DUALSPEED
 	help
 	  The Samsung S3C64XX USB2.0 high-speed gadget controller
 	  integrated into the S3C64XX series SoC.
-- 
1.6.3.3

[PATCH 3/6] USB: s3c-hsotg: Ensure TX FIFO addresses setup when initialising FIFOs

[Ben Dooks]

Some versions of the S3C HS OtG block startup with overlapping TX FIFO
information, so change the fifo_init code to ensure that known values
are set into the FIFO registers at initialisation/reset time.

This also ensures that the FIFO RAM pointers are in a known state
before use.

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

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index dce9366..2ee75a7 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -298,6 +298,11 @@ static void s3c_hsotg_ctrl_epint(struct s3c_hsotg *hsotg,
  */
 static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
 {
+	unsigned int ep;
+	unsigned int addr;
+	unsigned int size;
+	u32 val;
+
 	/* the ryu 2.6.24 release ahs
 	   writel(0x1C0, hsotg->regs + S3C_GRXFSIZ);
 	   writel(S3C_GNPTXFSIZ_NPTxFStAddr(0x200) |
@@ -311,6 +316,26 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
 	writel(S3C_GNPTXFSIZ_NPTxFStAddr(2048) |
 	       S3C_GNPTXFSIZ_NPTxFDep(0x1C0),
 	       hsotg->regs + S3C_GNPTXFSIZ);
+
+	/* arange all the rest of the TX FIFOs, as some versions of this
+	 * block have overlapping default addresses. This also ensures
+	 * that if the settings have been changed, then they are set to
+	 * known values. */
+
+	/* start at the end of the GNPTXFSIZ, rounded up */
+	addr = 2048 + 1024;
+	size = 768;
+
+	/* currently we allocate TX FIFOs for all possible endpoints,
+	 * and assume that they are all the same size. */
+
+	for (ep = 0; ep <= 15; ep++) {
+		val = addr;
+		val |= size << S3C_DPTXFSIZn_DPTxFSize_SHIFT;
+		addr += size;
+
+		writel(val, hsotg->regs + S3C_DPTXFSIZn(ep));
+	}
 }
 
 /**
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] USB: s3c-hsotg: SoftDisconnect minimum 3ms

[Ben Dooks]

The shortest period SoftDisconnect can be asserted for is 3 milliseconds
according to the V210 datasheet, so ensure that we add an msleep() to
the registration code to enforce this.

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

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 2ee75a7..8ed4f8a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2600,6 +2600,9 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 	writel(S3C_DCTL_CGOUTNak | S3C_DCTL_CGNPInNAK,
 	       hsotg->regs + S3C_DCTL);
 
+	/* must be at-least 3ms to allow bus to see disconnect */
+	msleep(3);
+
 	/* remove the soft-disconnect and let's go */
 	__bic32(hsotg->regs + S3C_DCTL, S3C_DCTL_SftDiscon);
 
-- 
1.6.3.3

[PATCH 5/6] USB: s3c-hsotg: Ensure FIFOs are fully flushed after layout

[Ben Dooks]

According to the design guide, if the FIFO layout is changed, then the
FIFOs must be flushed to ensure all FIFO pointers are correct.

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

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 8ed4f8a..0cb0462 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -301,6 +301,7 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
 	unsigned int ep;
 	unsigned int addr;
 	unsigned int size;
+	int timeout;
 	u32 val;
 
 	/* the ryu 2.6.24 release ahs
@@ -336,6 +337,31 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
 
 		writel(val, hsotg->regs + S3C_DPTXFSIZn(ep));
 	}
+
+	/* according to p428 of the design guide, we need to ensure that
+	 * all fifos are flushed before continuing */
+
+	writel(S3C_GRSTCTL_TxFNum(0x10) | S3C_GRSTCTL_TxFFlsh |
+	       S3C_GRSTCTL_RxFFlsh, hsotg->regs + S3C_GRSTCTL);
+
+	/* wait until the fifos are both flushed */
+	timeout = 100;
+	while (1) {
+		val = readl(hsotg->regs + S3C_GRSTCTL);
+
+		if ((val & (S3C_GRSTCTL_TxFFlsh | S3C_GRSTCTL_RxFFlsh)) == 0)
+			break;
+
+		if (--timeout == 0) {
+			dev_err(hsotg->dev,
+				"%s: timeout flushing fifos (GRSTCTL=%08x)\n",
+				__func__, val);
+		}
+
+		udelay(1);
+	}
+
+	dev_dbg(hsotg->dev, "FIFOs reset, timeout at %d\n", timeout);
 }
 
 /**
-- 
1.6.3.3

[PATCH 6/6] USB: s3c-hsotg: Enable soft disconnect during initialization

[Ben Dooks]

From: Thomas Abraham <thomas.ab@samsung.com>

Enable soft disconnect bit the OTG core during initialization. Without this,
the host sees that a gadget is connected and tries to enumerate. The
soft disconnect should be enabled until the USB gadget driver is
registered with this otg driver.

Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
---
 drivers/usb/gadget/s3c-hsotg.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 0cb0462..6a303ce 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2803,6 +2803,9 @@ static void s3c_hsotg_init(struct s3c_hsotg *hsotg)
 
 	writel(0, hsotg->regs + S3C_DAINTMSK);
 
+	/* Be in disconnected state until gadget is registered */
+	__orr32(hsotg->regs + S3C_DCTL, S3C_DCTL_SftDiscon);
+
 	if (0) {
 		/* post global nak until we're ready */
 		writel(S3C_DCTL_SGNPInNAK | S3C_DCTL_SGOUTNak,
-- 
1.6.3.3

Re: [PATCH 3/6] USB: s3c-hsotg: Ensure TX FIFO addresses setup when initialising FIFOs

[Maulik Mankad]

On Tue, May 25, 2010 at 10:06 AM, Ben Dooks <ben-linux@fluff.org> wrote:
> Some versions of the S3C HS OtG block startup with overlapping TX FIFO
> information, so change the fifo_init code to ensure that known values
> are set into the FIFO registers at initialisation/reset time.
>
> This also ensures that the FIFO RAM pointers are in a known state
> before use.
>
> Signed-off-by: Ben Dooks <ben-linux@fluff.org>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index dce9366..2ee75a7 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -298,6 +298,11 @@ static void s3c_hsotg_ctrl_epint(struct s3c_hsotg *hsotg,
>  */
>  static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>  {
> +       unsigned int ep;
> +       unsigned int addr;
> +       unsigned int size;
> +       u32 val;
> +
>        /* the ryu 2.6.24 release ahs
>           writel(0x1C0, hsotg->regs + S3C_GRXFSIZ);
>           writel(S3C_GNPTXFSIZ_NPTxFStAddr(0x200) |
> @@ -311,6 +316,26 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>        writel(S3C_GNPTXFSIZ_NPTxFStAddr(2048) |
>               S3C_GNPTXFSIZ_NPTxFDep(0x1C0),
>               hsotg->regs + S3C_GNPTXFSIZ);
> +
> +       /* arange all the rest of the TX FIFOs, as some versions of this
> +        * block have overlapping default addresses. This also ensures
> +        * that if the settings have been changed, then they are set to
> +        * known values. */

Please see Documentation/CodingStyle for multi-line comments.

> +
> +       /* start at the end of the GNPTXFSIZ, rounded up */
> +       addr = 2048 + 1024;
> +       size = 768;
> +
> +       /* currently we allocate TX FIFOs for all possible endpoints,
> +        * and assume that they are all the same size. */

Same here.

> +
> +       for (ep = 0; ep <= 15; ep++) {
> +               val = addr;
> +               val |= size << S3C_DPTXFSIZn_DPTxFSize_SHIFT;
> +               addr += size;
> +
> +               writel(val, hsotg->regs + S3C_DPTXFSIZn(ep));
> +       }
>  }

Regards,
Maulik

Re: [PATCH 5/6] USB: s3c-hsotg: Ensure FIFOs are fully flushed after layout

[Maulik Mankad]

On Tue, May 25, 2010 at 10:06 AM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> According to the design guide, if the FIFO layout is changed, then the
> FIFOs must be flushed to ensure all FIFO pointers are correct.
>
> Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> ---
>  drivers/usb/gadget/s3c-hsotg.c |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index 8ed4f8a..0cb0462 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -301,6 +301,7 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>        unsigned int ep;
>        unsigned int addr;
>        unsigned int size;
> +       int timeout;
>        u32 val;
>
>        /* the ryu 2.6.24 release ahs
> @@ -336,6 +337,31 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>
>                writel(val, hsotg->regs + S3C_DPTXFSIZn(ep));
>        }
> +
> +       /* according to p428 of the design guide, we need to ensure that
> +        * all fifos are flushed before continuing */

Please see Documentation/CodingStyle for multi-line comments.

> +
> +       writel(S3C_GRSTCTL_TxFNum(0x10) | S3C_GRSTCTL_TxFFlsh |
> +              S3C_GRSTCTL_RxFFlsh, hsotg->regs + S3C_GRSTCTL);
> +
> +       /* wait until the fifos are both flushed */
> +       timeout = 100;
> +       while (1) {
> +               val = readl(hsotg->regs + S3C_GRSTCTL);
> +
> +               if ((val & (S3C_GRSTCTL_TxFFlsh | S3C_GRSTCTL_RxFFlsh)) == 0)
> +                       break;
> +
> +               if (--timeout == 0) {
> +                       dev_err(hsotg->dev,
> +                               "%s: timeout flushing fifos (GRSTCTL=%08x)\n",
> +                               __func__, val);
> +               }
> +
> +               udelay(1);
> +       }
> +
> +       dev_dbg(hsotg->dev, "FIFOs reset, timeout at %d\n", timeout);
>  }

Regards,
Maulik
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] USB: s3c-hsotg: Ensure TX FIFO addresses setup when initialising FIFOs

[Ben Dooks]

On Tue, May 25, 2010 at 11:00:39AM +0530, Maulik Mankad wrote:
> On Tue, May 25, 2010 at 10:06 AM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> > Some versions of the S3C HS OtG block startup with overlapping TX FIFO
> > information, so change the fifo_init code to ensure that known values
> > are set into the FIFO registers at initialisation/reset time.
> >
> > This also ensures that the FIFO RAM pointers are in a known state
> > before use.
> >
> > Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> > ---
> >  drivers/usb/gadget/s3c-hsotg.c |   25 +++++++++++++++++++++++++
> >  1 files changed, 25 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> > index dce9366..2ee75a7 100644
> > --- a/drivers/usb/gadget/s3c-hsotg.c
> > +++ b/drivers/usb/gadget/s3c-hsotg.c
> > @@ -298,6 +298,11 @@ static void s3c_hsotg_ctrl_epint(struct s3c_hsotg *hsotg,
> >  */
> >  static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
> >  {
> > +       unsigned int ep;
> > +       unsigned int addr;
> > +       unsigned int size;
> > +       u32 val;
> > +
> >        /* the ryu 2.6.24 release ahs
> >           writel(0x1C0, hsotg->regs + S3C_GRXFSIZ);
> >           writel(S3C_GNPTXFSIZ_NPTxFStAddr(0x200) |
> > @@ -311,6 +316,26 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
> >        writel(S3C_GNPTXFSIZ_NPTxFStAddr(2048) |
> >               S3C_GNPTXFSIZ_NPTxFDep(0x1C0),
> >               hsotg->regs + S3C_GNPTXFSIZ);
> > +
> > +       /* arange all the rest of the TX FIFOs, as some versions of this
> > +        * block have overlapping default addresses. This also ensures
> > +        * that if the settings have been changed, then they are set to
> > +        * known values. */
> 
> Please see Documentation/CodingStyle for multi-line comments.
> 
> > +
> > +       /* start at the end of the GNPTXFSIZ, rounded up */
> > +       addr = 2048 + 1024;
> > +       size = 768;
> > +
> > +       /* currently we allocate TX FIFOs for all possible endpoints,
> > +        * and assume that they are all the same size. */
> 
> Same here.
> 
> > +
> > +       for (ep = 0; ep <= 15; ep++) {
> > +               val = addr;
> > +               val |= size << S3C_DPTXFSIZn_DPTxFSize_SHIFT;
> > +               addr += size;
> > +
> > +               writel(val, hsotg->regs + S3C_DPTXFSIZn(ep));
> > +       }
> >  }
> 
> Regards,
> Maulik

ok, fixed both,

-- 
Ben

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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] USB: s3c_hsotg: Add support for external USB clock

[Greg KH]

On Tue, May 25, 2010 at 05:36:48AM +0100, 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

Care to take this through the arm tree then?

thanks,

greg k-h

Re: [PATCH 1/6] USB: s3c_hsotg: Add support for external USB clock

[Maurus Cuelenaere]

Op 01-06-10 22:41, Greg KH schreef:
> On Tue, May 25, 2010 at 05:36:48AM +0100, 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
>>     
> Care to take this through the arm tree then?
>
> thanks,
>
> greg k-h
>   

If I'm not mistaken, this patch is already in Ben's tree (the
USB_GADGET_DUALSPEED one too)

-- 
Maurus Cuelenaere