Re: [patch 2.6.23-omap] misc musb tweaks

Re: [patch 2.6.23-omap] misc musb tweaks

[David Brownell]

On Sunday 14 October 2007, Alan Stern wrote:
> On Sun, 14 Oct 2007, David Brownell wrote:
> 
> > > What else should we still fix before submitting to linux-usb-devel?
> > 
> > I don't see much of a reason why it shouldn't go upstream "now"...
> 
> The host part will need a bunch of changes to put it in line with the 
> new urb->status stuff.  Without that it won't even compile.  Not 
> difficult to do, just something to keep in the back of your mind...

And that's all in the current kernel.org GIT tree, ISTR ...

- Dave

Re: [patch 2.6.23-omap] misc musb tweaks

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [071014 18:45]:
> On Sunday 14 October 2007, Alan Stern wrote:
> > On Sun, 14 Oct 2007, David Brownell wrote:
> > 
> > > > What else should we still fix before submitting to linux-usb-devel?
> > > 
> > > I don't see much of a reason why it shouldn't go upstream "now"...
> > 
> > The host part will need a bunch of changes to put it in line with the 
> > new urb->status stuff.  Without that it won't even compile.  Not 
> > difficult to do, just something to keep in the back of your mind...
> 
> And that's all in the current kernel.org GIT tree, ISTR ...

OK, I'll look into it and prepare the patches for review within next
few days hopefully.

Regards,

Tony

[patch 2.6.23-omap] misc musb tweaks

[David Brownell]

From: Alan Stern <stern@rowland.harvard.edu>

The patch below fixes a few minor infelicities.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/usb/musb/musb_host.c    |    6 +++---
 drivers/usb/musb/musb_host.h    |    9 ++++-----
 drivers/usb/musb/musb_virthub.c |   14 +++++++++-----
 3 files changed, 16 insertions(+), 13 deletions(-)

--- o26.orig/drivers/usb/musb/musb_host.c	2007-08-27 17:24:01.000000000 -0700
+++ o26/drivers/usb/musb/musb_host.c	2007-08-28 14:54:08.000000000 -0700
@@ -1513,9 +1513,9 @@ void musb_host_rx(struct musb *musb, u8 
 
 #ifdef CONFIG_USB_INVENTRA_DMA
 		/* done if urb buffer is full or short packet is recd */
-		done = ((urb->actual_length + xfer_len) >=
-				urb->transfer_buffer_length)
-			|| (dma->actual_len & (qh->maxpacket - 1));
+		done = (urb->actual_length + xfer_len >=
+				urb->transfer_buffer_length
+			|| dma->actual_len < qh->maxpacket);
 
 		/* send IN token for next packet, without AUTOREQ */
 		if (!done) {
--- o26.orig/drivers/usb/musb/musb_virthub.c	2007-08-27 17:24:01.000000000 -0700
+++ o26/drivers/usb/musb/musb_virthub.c	2007-08-28 14:54:08.000000000 -0700
@@ -202,6 +202,7 @@ void musb_root_disconnect(struct musb *m
 
 /*---------------------------------------------------------------------*/
 
+/* Caller may or may not hold musb->lock */
 int musb_hub_status_data(struct usb_hcd *hcd, char *buf)
 {
 	struct musb	*musb = hcd_to_musb(hcd);
@@ -228,14 +229,17 @@ int musb_hub_control(
 	int		retval = 0;
 	unsigned long	flags;
 
-	if (unlikely(!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)))
+	spin_lock_irqsave(&musb->lock, flags);
+
+	if (unlikely(!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags))) {
+		spin_unlock_irqrestore(&musb->lock, flags);
 		return -ESHUTDOWN;
+	}
 
 	/* hub features:  always zero, setting is a NOP
 	 * port features: reported, sometimes updated when host is active
 	 * no indicators
 	 */
-	spin_lock_irqsave(&musb->lock, flags);
 	switch (typeReq) {
 	case ClearHubFeature:
 	case SetHubFeature:
@@ -248,7 +252,7 @@ int musb_hub_control(
 		}
 		break;
 	case ClearPortFeature:
-		if (wIndex != 1)
+		if ((wIndex & 0xff) != 1)
 			goto error;
 
 		switch (wValue) {
@@ -302,12 +306,12 @@ int musb_hub_control(
 
 		/* finish RESET signaling? */
 		if ((musb->port1_status & USB_PORT_STAT_RESET)
-				&& time_after(jiffies, musb->rh_timer))
+				&& time_after_eq(jiffies, musb->rh_timer))
 			musb_port_reset(musb, false);
 
 		/* finish RESUME signaling? */
 		if ((musb->port1_status & MUSB_PORT_STAT_RESUME)
-				&& time_after(jiffies, musb->rh_timer)) {
+				&& time_after_eq(jiffies, musb->rh_timer)) {
 			u8		power;
 
 			power = musb_readb(musb->mregs, MUSB_POWER);
--- o26.orig/drivers/usb/musb/musb_host.h	2007-08-27 17:24:01.000000000 -0700
+++ o26/drivers/usb/musb/musb_host.h	2007-08-28 14:54:08.000000000 -0700
@@ -37,13 +37,12 @@
 
 static inline struct usb_hcd *musb_to_hcd(struct musb *musb)
 {
-	return (struct usb_hcd *) (((void *)musb)
-			- offsetof(struct usb_hcd, hcd_priv));
+	return container_of((void *) musb, struct usb_hcd, hcd_priv);
 }
 
 static inline struct musb *hcd_to_musb(struct usb_hcd *hcd)
 {
-	return (void *) hcd->hcd_priv;
+	return (struct musb *) (hcd->hcd_priv);
 }
 
 /* stored in "usb_host_endpoint.hcpriv" for scheduled endpoints */
@@ -77,7 +76,7 @@ static inline struct musb_qh *first_qh(s
 {
 	if (list_empty(q))
 		return NULL;
-	return container_of(q->next, struct musb_qh, ring);
+	return list_entry(q->next, struct musb_qh, ring);
 }
 
 
@@ -102,7 +101,7 @@ static inline struct urb *next_urb(struc
 	queue = &qh->hep->urb_list;
 	if (list_empty(queue))
 		return NULL;
-	return container_of(queue->next, struct urb, urb_list);
+	return list_entry(queue->next, struct urb, urb_list);
 #else
 	return NULL;
 #endif

Re: [patch 2.6.23-omap] misc musb tweaks

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [071012 14:29]:
> From: Alan Stern <stern@rowland.harvard.edu>
> 
> The patch below fixes a few minor infelicities.

Pushed, glad to see _the_ USB experts working on this thing :)

I guess we should still at least clean up the CONFIG_USB_INVENTRA_DMA
sections in the common DMA code before submitting to linux-usb-devel.

What else should we still fix before submitting to linux-usb-devel?

Regards,

Tony

Re: [patch 2.6.23-omap] misc musb tweaks

[David Brownell]

On Sunday 14 October 2007, Tony Lindgren wrote:
> I guess we should still at least clean up the CONFIG_USB_INVENTRA_DMA
> sections in the common DMA code before submitting to linux-usb-devel.

That's ugly as all get-out, but not what I'd call a blocking issue.

It *should* be essentially an overhaul to the guts of the DMA glue:
there are {RX,TX}_{submit,complete}_{gadget,host} paths, which should
get unified, then seriously retested on several platforms.  That's
not straightforward... and there's no reason not to do that *AFTER*
the code goes upstream.

Similarly with the Blackfin (525?) support:  that can be done after
a upstream merge, too.


> What else should we still fix before submitting to linux-usb-devel?

I don't see much of a reason why it shouldn't go upstream "now"...

What we need is someone to create the patches.  When I asked a while
back, nobody really objected to "one huge patch" ... though I think
breaking it down into

 (a) drivers/usb/musb/*.[hc]
 (b) the various platform hooks, in arch/ and include/asm-*/*
 (c) kbuild stuff (Kconfig and Makefile updates)

would be best.  That is, no need to split out the gadget side
from the host side, or TUSB6010 from OMAP2430 from DaVinci, etc.
(It'd also be good to have a MAINTAINERS entry...)

If someone did that part, and made sure the TUSB6010 stuff would
build on some non-OMAP hardware (without DMA), I could sanity
check it and sign off before sending it upstream.  Dunno if it'd
make 2.6.24, but I'd not rule it out.

- Dave