[RFC] Misc cleanups

[RFC] Misc cleanups

[Felipe Balbi]

Hi all,

The following series implements some one-liner changes to TWL4030,
musb Kconfig and omap34xx.h.

Comments are welcome.

[RFC patch] I2C: TWL4030: Remove unneded pr_err define

[Felipe Balbi]

pr_err definition was moved to include/linux/kernel.h
it's unecessary keeping that in twl4030_core.c

Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
---
 drivers/i2c/chips/twl4030_core.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/chips/twl4030_core.c b/drivers/i2c/chips/twl4030_core.c
index 991ebf4..5b77d99 100644
--- a/drivers/i2c/chips/twl4030_core.c
+++ b/drivers/i2c/chips/twl4030_core.c
@@ -51,8 +51,6 @@
 
 #define DRIVER_NAME			"twl4030"
 
-#define pr_err(fmt, arg...)	printk(KERN_ERR DRIVER_NAME ": " fmt, ##arg);
-
 /**** Macro Definitions */
 #define TWL_CLIENT_STRING		"TWL4030-ID"
 #define TWL_CLIENT_USED			1
-- 
1.5.3.5.666.gfb5f

[RFC patch] ARM: OMAP: Fix typo in drivers/usb/musb/Kconfig

[Felipe Balbi]

We don't have ARCH_OMAP343X. Change to ARCH_OMAP34XX

Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
---
 drivers/usb/musb/Kconfig |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 2e030b9..c795b01 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -30,7 +30,7 @@ config USB_MUSB_SOC
 	depends on USB_MUSB_HDRC
 	default y if ARCH_DAVINCI
 	default y if ARCH_OMAP2430
-	default y if ARCH_OMAP343X
+	default y if ARCH_OMAP34XX
 	help
 	  Use a static <asm/arch/hdrc_cnf.h> file to describe how the
 	  controller is configured (endpoints, mechanisms, etc) on the
@@ -43,7 +43,7 @@ comment "OMAP 243x high speed USB support"
 	depends on USB_MUSB_HDRC && ARCH_OMAP2430
 
 comment "OMAP 343x high speed USB support"
-	depends on USB_MUSB_HDRC && ARCH_OMAP343X
+	depends on USB_MUSB_HDRC && ARCH_OMAP34XX
 
 config USB_TUSB6010
 	boolean "TUSB 6010 support"
@@ -145,7 +145,7 @@ config MUSB_PIO_ONLY
 config USB_INVENTRA_DMA
 	bool
 	depends on USB_MUSB_HDRC && !MUSB_PIO_ONLY
-	default ARCH_OMAP2430 || ARCH_OMAP343X
+	default ARCH_OMAP2430 || ARCH_OMAP34XX
 	help
 	  Enable DMA transfers using Mentor's engine.
 
-- 
1.5.3.5.666.gfb5f

[RFC patch] ARM: OMAP: Define OMAP34XX_HS_BASE

[Felipe Balbi]

Following other omap archs, define OMAP34XX_HS_BASE macro
to be used later, for example in musb driver.

Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
---
 include/asm-arm/arch-omap/omap34xx.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/asm-arm/arch-omap/omap34xx.h b/include/asm-arm/arch-omap/omap34xx.h
index 0265e3b..1650937 100644
--- a/include/asm-arm/arch-omap/omap34xx.h
+++ b/include/asm-arm/arch-omap/omap34xx.h
@@ -71,6 +71,7 @@
 #define OMAP2_CTRL_BASE			OMAP3430_CTRL_BASE
 #define OMAP34XX_CONTROL_DEVCONF0	(L4_34XX_BASE + 0x2274)
 #define OMAP34XX_CONTROL_DEVCONF1	(L4_34XX_BASE + 0x22D8)
+#define OMAP34XX_HS_BASE		(L4_34XX_BASE + 0x65000)
 #define OMAP2_CONTROL_STATUS		(OMAP2_CTRL_BASE + 0x2f0)
 
 #endif
-- 
1.5.3.5.666.gfb5f

RE: [RFC patch] ARM: OMAP: Fix typo in drivers/usb/musb/Kconfig

[Gadiyar, Anand]

Hi, 

> We don't have ARCH_OMAP343X. Change to ARCH_OMAP34XX
> 
> Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
> ---
>  drivers/usb/musb/Kconfig |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 2e030b9..c795b01 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -30,7 +30,7 @@ config USB_MUSB_SOC
>  	depends on USB_MUSB_HDRC
>  	default y if ARCH_DAVINCI
>  	default y if ARCH_OMAP2430
> -	default y if ARCH_OMAP343X
> +	default y if ARCH_OMAP34XX
>  	help
>  	  Use a static <asm/arch/hdrc_cnf.h> file to describe how the
>  	  controller is configured (endpoints, mechanisms, etc) on the
> @@ -43,7 +43,7 @@ comment "OMAP 243x high speed USB support"
>  	depends on USB_MUSB_HDRC && ARCH_OMAP2430
>  
>  comment "OMAP 343x high speed USB support"
> -	depends on USB_MUSB_HDRC && ARCH_OMAP343X
> +	depends on USB_MUSB_HDRC && ARCH_OMAP34XX
>  
>  config USB_TUSB6010
>  	boolean "TUSB 6010 support"
> @@ -145,7 +145,7 @@ config MUSB_PIO_ONLY
>  config USB_INVENTRA_DMA
>  	bool
>  	depends on USB_MUSB_HDRC && !MUSB_PIO_ONLY
> -	default ARCH_OMAP2430 || ARCH_OMAP343X
> +	default ARCH_OMAP2430 || ARCH_OMAP34XX
>  	help
>  	  Enable DMA transfers using Mentor's engine.
>  
> -- 

I made the exact same changes as part of getting MUSB running on 3430 ES2.

You beat me to the submission though. :)
Acked by me.

--
Anand Gadiyar
gadiyar@ti.com

RE: [RFC patch] ARM: OMAP: Fix typo in drivers/usb/musb/Kconfig

[Felipe Balbi]

Hi,

On Mon, 19 Nov 2007 18:23:38 +0530, "Gadiyar, Anand" <gadiyar@ti.com>
wrote:
> Hi, 
> 
>> We don't have ARCH_OMAP343X. Change to ARCH_OMAP34XX
>> 
>> Signed-off-by: Felipe Balbi <felipe.lima@indt.org.br>
>> ---
>>  drivers/usb/musb/Kconfig |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
>> index 2e030b9..c795b01 100644
>> --- a/drivers/usb/musb/Kconfig
>> +++ b/drivers/usb/musb/Kconfig
>> @@ -30,7 +30,7 @@ config USB_MUSB_SOC
>>  	depends on USB_MUSB_HDRC
>>  	default y if ARCH_DAVINCI
>>  	default y if ARCH_OMAP2430
>> -	default y if ARCH_OMAP343X
>> +	default y if ARCH_OMAP34XX
>>  	help
>>  	  Use a static <asm/arch/hdrc_cnf.h> file to describe how the
>>  	  controller is configured (endpoints, mechanisms, etc) on the
>> @@ -43,7 +43,7 @@ comment "OMAP 243x high speed USB support"
>>  	depends on USB_MUSB_HDRC && ARCH_OMAP2430
>>  
>>  comment "OMAP 343x high speed USB support"
>> -	depends on USB_MUSB_HDRC && ARCH_OMAP343X
>> +	depends on USB_MUSB_HDRC && ARCH_OMAP34XX
>>  
>>  config USB_TUSB6010
>>  	boolean "TUSB 6010 support"
>> @@ -145,7 +145,7 @@ config MUSB_PIO_ONLY
>>  config USB_INVENTRA_DMA
>>  	bool
>>  	depends on USB_MUSB_HDRC && !MUSB_PIO_ONLY
>> -	default ARCH_OMAP2430 || ARCH_OMAP343X
>> +	default ARCH_OMAP2430 || ARCH_OMAP34XX
>>  	help
>>  	  Enable DMA transfers using Mentor's engine.
>>  
>>

you're also working on OMAP3's USB??
can we share code?? I have an almost working board-3430sdp-usb.c

-- 
Best Regards,

Felipe Balbi
http://felipebalbi.com
me@felipebalbi.com

RE: [RFC patch] ARM: OMAP: Fix typo in drivers/usb/musb/Kconfig

[Gadiyar, Anand]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

Hi, 

> you're also working on OMAP3's USB??
> can we share code?? I have an almost working board-3430sdp-usb.c

Yes. I'm working on OMAP3's USB. I will be pushing out a few patches over
the next couple of weeks. Would greatly appreciate any feedback you can give.

Warm Regards,
Anand

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

RE: [RFC patch] ARM: OMAP: Fix typo in drivers/usb/musb/Kconfig

[Felipe Balbi]

On Mon, 19 Nov 2007 19:18:13 +0530, "Gadiyar, Anand" <gadiyar@ti.com>
wrote:
> Hi, 
> 
>> you're also working on OMAP3's USB??
>> can we share code?? I have an almost working board-3430sdp-usb.c
> 
> Yes. I'm working on OMAP3's USB. I will be pushing out a few patches over
> the next couple of weeks. Would greatly appreciate any feedback you can
> give.

That'll be nice. I'll at least, try out your patches and make mine better
:-p

> 
> Warm Regards,
> Anand
> 
-- 
Best Regards,

Felipe Balbi
http://felipebalbi.com
me@felipebalbi.com

[PATCH] OMAP: MUSB: MUSB gadget fix

[Pandita, Vikram]

From: Vikram Pandita <vikram.pandita@ti.com>

This fix gets MUSB Gadget Isochronous IN transfers working for File Storage Gadget

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---

Index: linux-omap/drivers/usb/musb/musb_gadget.c
===================================================================
--- linux-omap.orig/drivers/usb/musb/musb_gadget.c	2007-11-19 17:49:41.000000000 +0530
+++ linux-omap/drivers/usb/musb/musb_gadget.c	2007-11-19 17:50:31.000000000 +0530
@@ -499,7 +499,7 @@
 
 					DBG(4, "sending zero pkt\n");
 					musb_writew(epio, MUSB_TXCSR,
-							MUSB_TXCSR_MODE
+							csr | MUSB_TXCSR_MODE
 							| MUSB_TXCSR_TXPKTRDY);
 					request->zero = 0;
 				}

[PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[Pandita, Vikram]

From: Vikram Pandita <vikram.pandita@ti.com>

Fix to get MUSB working as Module for 2430 and Davinci platform

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---

Index: linux-omap/drivers/usb/musb/musb_core.c
===================================================================
--- linux-omap.orig/drivers/usb/musb/musb_core.c	2007-11-20 14:24:03.000000000 +0530
+++ linux-omap/drivers/usb/musb/musb_core.c	2007-11-20 14:26:22.000000000 +0530
@@ -1336,7 +1336,7 @@
 
 	/* log core options (read using indexed model) */
 	musb_ep_select(mbase, 0);
-	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
+	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
 
 	strcpy(aInfo, (reg & MUSB_CONFIGDATA_UTMIDW) ? "UTMI-16" : "UTMI-8");
 	if (reg & MUSB_CONFIGDATA_DYNFIFO) {

Re: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[David Brownell]

On Tuesday 20 November 2007, Pandita, Vikram wrote:
> From: Vikram Pandita <vikram.pandita@ti.com>
> 
> Fix to get MUSB working as Module for 2430 and Davinci platform

I don't understand the point of this patch.  It certainly used to
work as a module on DaVinci ... as recently as a few months ago.

And this particular register is at a fixed location, not subject
to the usual register banking stuff.  Kicking in that banking
logic would be incorrect.


> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> ---
> 
> Index: linux-omap/drivers/usb/musb/musb_core.c
> ===================================================================
> --- linux-omap.orig/drivers/usb/musb/musb_core.c	2007-11-20 14:24:03.000000000 +0530
> +++ linux-omap/drivers/usb/musb/musb_core.c	2007-11-20 14:26:22.000000000 +0530
> @@ -1336,7 +1336,7 @@
>  
>  	/* log core options (read using indexed model) */
>  	musb_ep_select(mbase, 0);
> -	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
> +	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
>  
>  	strcpy(aInfo, (reg & MUSB_CONFIGDATA_UTMIDW) ? "UTMI-16" : "UTMI-8");
>  	if (reg & MUSB_CONFIGDATA_DYNFIFO) {

RE: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[Pandita, Vikram]

Hi David

>I don't understand the point of this patch.  It certainly used to
>work as a module on DaVinci ... as recently as a few months ago.

For 24xx, MUSB as module was not working since one year I started looking into it.
Since CONFIGDATA register needs the index register to be zero. This is not mentioned in the register description but in the Table in the beginning of the Mentor Spec PG.

"Reference Mentor Spec 2 . 1 . MUSBMHDRC REGISTER MAP
ConfigData Returns details of core configuration. (Index register set to select Endpoint 0.) 2.3.4"

So on a reload of module, the index register is some value and the CONFIGDATA value is not got correctly and module load fails.

After this fix, the module loading/un-loading works.

>
>And this particular register is at a fixed location, not subject
>to the usual register banking stuff.  Kicking in that banking
>logic would be incorrect.

"Reference Mentor Spec 2 . 1 . MUSBMHDRC REGISTER MAP
ConfigData Returns details of core configuration. (Index register set to select Endpoint 0.) 2.3.4"

>
>
>> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
>> ---
>>
>> Index: linux-omap/drivers/usb/musb/musb_core.c
>> ===================================================================
>> --- linux-omap.orig/drivers/usb/musb/musb_core.c	2007-11-20 14:24:03.000000000 +0530
>> +++ linux-omap/drivers/usb/musb/musb_core.c	2007-11-20 14:26:22.000000000 +0530
>> @@ -1336,7 +1336,7 @@
>>
>>  	/* log core options (read using indexed model) */
>>  	musb_ep_select(mbase, 0);
>> -	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
>> +	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
>>
>>  	strcpy(aInfo, (reg & MUSB_CONFIGDATA_UTMIDW) ? "UTMI-16" : "UTMI-8");
>>  	if (reg & MUSB_CONFIGDATA_DYNFIFO) {

Re: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[David Brownell]

> >I don't understand the point of this patch.  It certainly used to
> >work as a module on DaVinci ... as recently as a few months ago.
>
> For 24xx, MUSB as module was not working since one year I started looking into it.
> Since CONFIGDATA register needs the index register to be zero. This is not
> mentioned in the register description but in the Table in the beginning of
> the Mentor Spec PG.

Then the correct patch would be to set that register to zero,
instead of what your patch does...


> >And this particular register is at a fixed location, not subject
> >to the usual register banking stuff.  Kicking in that banking
> >logic would be incorrect.
>
> "Reference Mentor Spec 2 . 1 . MUSBMHDRC REGISTER MAP
> ConfigData Returns details of core configuration. (Index register set to
> select Endpoint 0.) 2.3.4"

... again arguing that the correct fix would be to set the index
register to zero, even when a non-indexed addressing mode is used.


> >> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> >> ---
> >>
> >> Index: linux-omap/drivers/usb/musb/musb_core.c
> >> ===================================================================
> >> --- linux-omap.orig/drivers/usb/musb/musb_core.c	2007-11-20 14:24:03.000000000 +0530
> >> +++ linux-omap/drivers/usb/musb/musb_core.c	2007-11-20 14:26:22.000000000 +0530
> >> @@ -1336,7 +1336,7 @@
> >>
> >>  	/* log core options (read using indexed model) */
> >>  	musb_ep_select(mbase, 0);
> >> -	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
> >> +	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
> >>
> >>  	strcpy(aInfo, (reg & MUSB_CONFIGDATA_UTMIDW) ? "UTMI-16" : "UTMI-8");
> >>  	if (reg & MUSB_CONFIGDATA_DYNFIFO) {
>

RE: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[Pandita, Vikram]

>> >I don't understand the point of this patch.  It certainly used to
>> >work as a module on DaVinci ... as recently as a few months ago.
>>
>> For 24xx, MUSB as module was not working since one year I started looking into it.
>> Since CONFIGDATA register needs the index register to be zero. This is not
>> mentioned in the register description but in the Table in the beginning of
>> the Mentor Spec PG.
>
>Then the correct patch would be to set that register to zero,
>instead of what your patch does...

24XX uses the flat model as opposed to indexed and 
so the call musb_ep_select(mbase, 0); in following code has no affect for 24XX/Davinci platforms.

[snip]
>> >>  	musb_ep_select(mbase, 0);
>> >> -	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
>> >> +	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));


As such to continue to use the FLAT model, we should access the correct offset of MUSB_CONFIGDATA register using: MUSB_EP_OFFSET(0, MUSB_CONFIGDATA)

For TUSB6010, the two lines of code evaluate to: 
>> musb_ep_select(mbase, 0);
   musb_writeb((mbase), MUSB_INDEX, (0)); //SET INDEX

>> reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
   reg = musb_readb(mbase, (0x10 + MUSB_CONFIGDATA)); //ACCESS register 

For 24xx/Davince, the two lines of code evaluate to:
>> musb_ep_select(mbase, 0);
	(((void)(mbase)),((void)(0))); //NO CODE

>> reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
	Reg = musb_readb(mbase, (0x100 + (0x10*(0)) + (MUSB_CONFIGDATA))

>
>
>> >And this particular register is at a fixed location, not subject
>> >to the usual register banking stuff.  Kicking in that banking
>> >logic would be incorrect.
>>
>> "Reference Mentor Spec 2 . 1 . MUSBMHDRC REGISTER MAP
>> ConfigData Returns details of core configuration. (Index register set to
>> select Endpoint 0.) 2.3.4"
>
>... again arguing that the correct fix would be to set the index
>register to zero, even when a non-indexed addressing mode is used.
>
>
>> >> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
>> >> ---
>> >>
>> >> Index: linux-omap/drivers/usb/musb/musb_core.c
>> >> ===================================================================
>> >> --- linux-omap.orig/drivers/usb/musb/musb_core.c	2007-11-20 14:24:03.000000000 +0530
>> >> +++ linux-omap/drivers/usb/musb/musb_core.c	2007-11-20 14:26:22.000000000 +0530
>> >> @@ -1336,7 +1336,7 @@
>> >>
>> >>  	/* log core options (read using indexed model) */
>> >>  	musb_ep_select(mbase, 0);
>> >> -	reg = musb_readb(mbase, 0x10 + MUSB_CONFIGDATA);
>> >> +	reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));
>> >>
>> >>  	strcpy(aInfo, (reg & MUSB_CONFIGDATA_UTMIDW) ? "UTMI-16" : "UTMI-8");
>> >>  	if (reg & MUSB_CONFIGDATA_DYNFIFO) {
>>

Re: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[David Brownell]

> >Then the correct patch would be to set that register to zero,
> >instead of what your patch does...
>
> 24XX uses the flat model as opposed to indexed and 
> so the call musb_ep_select(mbase, 0); in following code has no affect
> for 24XX/Davinci platforms.

So you're agreeing with my comment then.

RE: [PATCH] OMAP: MUSB: MUSB fix to work as Module for 24xx/Davinci

[Pandita, Vikram]

>> >Then the correct patch would be to set that register to zero,
>> >instead of what your patch does...
>>
>> 24XX uses the flat model as opposed to indexed and
>> so the call musb_ep_select(mbase, 0); in following code has no affect
>> for 24XX/Davinci platforms.
>
>So you're agreeing with my comment then.
Not completely

[Snip] Your Comment:  
>> ... again arguing that the correct fix would be to set the index
>> register to zero, even when a non-indexed addressing mode is used.

CONFIGDATA is no different from any other indexed mode registers and for FLAT model we can access the same way as any other indexed register which is as follows: 
reg = musb_readb(mbase, MUSB_EP_OFFSET(0, MUSB_CONFIGDATA));

[PATCH-REWORKED] OMAP: MUSB: MUSB gadget fix

[Pandita, Vikram]

From: Vikram Pandita <vikram.pandita@ti.com>

REWORKED: 
This fix gets MUSB Gadget Isochronous IN/OUT transfers working

1) Isochronous IN transfers working for File Storage Gadget
   TXCSR register write was wrongly erasing all bits
	
2) Isochronous OUT transfers working for File Storage Gadget
   Minor fix:
   ./testusb -a -t15 -c1 -s512 -g8
   Testing with -g8 option will always case OVERFLOW condition which should not be a problem for ISO mode. So check for that condition and not report it as error in gadgetfs test application


Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>

Index: linux-omap/drivers/usb/musb/musb_gadget.c
===================================================================
--- linux-omap.orig/drivers/usb/musb/musb_gadget.c	2007-11-19 18:04:25.000000000 +0530
+++ linux-omap/drivers/usb/musb/musb_gadget.c	2007-11-21 16:15:50.000000000 +0530
@@ -111,7 +111,8 @@
 	req = to_musb_request(request);
 
 	list_del(&request->list);
-	if (req->request.status == -EINPROGRESS)
+	if (req->request.status == -EINPROGRESS ||
+		(ep->type == USB_ENDPOINT_XFER_ISOC && req->request.status == -EOVERFLOW))
 		req->request.status = status;
 	musb = req->musb;
 
@@ -499,9 +500,10 @@
 
 					DBG(4, "sending zero pkt\n");
 					musb_writew(epio, MUSB_TXCSR,
-							MUSB_TXCSR_MODE
+							csr | MUSB_TXCSR_MODE
 							| MUSB_TXCSR_TXPKTRDY);
 					request->zero = 0;
+					break;
 				}
 
 				/* ... or if not, then complete it */

---

>-----Original Message-----
>From: linux-omap-open-source-bounces+vikram.pandita=ti.com@linux.omap.com [mailto:linux-omap-open-
>source-bounces+vikram.pandita=ti.com@linux.omap.com] On Behalf Of Pandita, Vikram
>Sent: Monday, November 19, 2007 6:33 PM
>To: linux-omap-open-source@linux.omap.com
>Subject: [PATCH] OMAP: MUSB: MUSB gadget fix
>
>From: Vikram Pandita <vikram.pandita@ti.com>
>
>This fix gets MUSB Gadget Isochronous IN transfers working for File Storage Gadget
>
>Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
>---
>
>Index: linux-omap/drivers/usb/musb/musb_gadget.c
>===================================================================
>--- linux-omap.orig/drivers/usb/musb/musb_gadget.c	2007-11-19 17:49:41.000000000 +0530
>+++ linux-omap/drivers/usb/musb/musb_gadget.c	2007-11-19 17:50:31.000000000 +0530
>@@ -499,7 +499,7 @@
>
> 					DBG(4, "sending zero pkt\n");
> 					musb_writew(epio, MUSB_TXCSR,
>-							MUSB_TXCSR_MODE
>+							csr | MUSB_TXCSR_MODE
> 							| MUSB_TXCSR_TXPKTRDY);
> 					request->zero = 0;
> 				}
>_______________________________________________
>Linux-omap-open-source mailing list
>Linux-omap-open-source@linux.omap.com
>http://linux.omap.com/mailman/listinfo/linux-omap-open-source

Re: [PATCH-REWORKED] OMAP: MUSB: MUSB gadget fix

[David Brownell]

On Wednesday 21 November 2007, Pandita, Vikram wrote:
> From: Vikram Pandita <vikram.pandita@ti.com>
> 
> REWORKED: 
> This fix gets MUSB Gadget Isochronous IN/OUT transfers working
> 
> 1) Isochronous IN transfers working for File Storage Gadget
>    TXCSR register write was wrongly erasing all bits

Could you elaborate a bit?  File storage is only specified to work
with bulk transfers.  Were you just hacking that to become a source
of ISO data?

Normally one just configures the gadgetfs example program to set
up ISO endpoints with known/testable data patterns.


> 2) Isochronous OUT transfers working for File Storage Gadget
>    Minor fix:
>    ./testusb -a -t15 -c1 -s512 -g8
>    Testing with -g8 option will always case OVERFLOW condition
>    which should not be a problem for ISO mode. So check for that
>    condition and not report it as error in gadgetfs test application  

That's wrong.  The reason any fault would "not" be a problem is
because the software which *issues* the transfer request knows
how to handle that fault code.  And an OVERFLOW condition is very
clearly an error that might need attention ... the host goofed up
badly, sending more data than was allowed, in most cases.

 
> 
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> 
> Index: linux-omap/drivers/usb/musb/musb_gadget.c
> ===================================================================
> --- linux-omap.orig/drivers/usb/musb/musb_gadget.c	2007-11-19 18:04:25.000000000 +0530
> +++ linux-omap/drivers/usb/musb/musb_gadget.c	2007-11-21 16:15:50.000000000 +0530
> @@ -111,7 +111,8 @@
>  	req = to_musb_request(request);
>  
>  	list_del(&request->list);
> -	if (req->request.status == -EINPROGRESS)
> +	if (req->request.status == -EINPROGRESS ||
> +		(ep->type == USB_ENDPOINT_XFER_ISOC && req->request.status == -EOVERFLOW))

... so NAK on this part ...

>  		req->request.status = status;
>  	musb = req->musb;
>  
> @@ -499,9 +500,10 @@
>  
>  					DBG(4, "sending zero pkt\n");
>  					musb_writew(epio, MUSB_TXCSR,
> -							MUSB_TXCSR_MODE
> +							csr | MUSB_TXCSR_MODE
>  							| MUSB_TXCSR_TXPKTRDY);
>  					request->zero = 0;
> +					break;

This might be correct, I'd have to look in more detail at that
logic.  I do seem to recall distrusting that code path, in part
because it was awkward to test.

However, your explanation of what's going on doesn't wash.
Every bit in MUSB_TXCSR_P_WZC_BITS is a write-zero-to-clear,
and you're writing zero there.  So ... which bits outside of
that group needed to stay set?

Also, why the "break"?  You added that in this iteration of
the patch, with no comments...

>  				}
>  
>  				/* ... or if not, then complete it */
> 
> ---
>