[PATCH 0/6] USB: Fixes for fsl_qe_udc driver

Hi all,

Just resending some fixes that seem to be lost...

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

In case of probing errors the driver kfrees the udc_controller, but it
doesn't set the pointer to NULL.

When usb_gadget_register_driver is called, it checks for udc_controller
!= NULL, the check passes and the driver accesses nonexistent memory.
Fix this by setting udc_controller to NULL in case of errors.

While at it, also implement irq_of_parse_and_map()'s failure and cleanup
cases.

Signed-off-by: Anton Vorontsov <redacted>
Acked-by: David Brownell <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index d15be3d..db6fe04 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -2604,6 +2604,10 @@ static int __devinit qe_udc_probe(struct of_device *ofdev,
 			(unsigned long)udc_controller);
 	/* request irq and disable DR  */
 	udc_controller->usb_irq = irq_of_parse_and_map(np, 0);
+	if (!udc_controller->usb_irq) {
+		ret = -EINVAL;
+		goto err_noirq;
+	}
 
 	ret = request_irq(udc_controller->usb_irq, qe_udc_irq, 0,
 				driver_name, udc_controller);

@@ -2625,6 +2629,8 @@ static int __devinit qe_udc_probe(struct of_device *ofdev,
 err6:
 	free_irq(udc_controller->usb_irq, udc_controller);
 err5:
+	irq_dispose_mapping(udc_controller->usb_irq);
+err_noirq:
 	if (udc_controller->nullmap) {
 		dma_unmap_single(udc_controller->gadget.dev.parent,
 			udc_controller->nullp, 256,

@@ -2648,7 +2654,7 @@ err2:
 	iounmap(udc_controller->usb_regs);
 err1:
 	kfree(udc_controller);
-
+	udc_controller = NULL;
 	return ret;
 }
 

@@ -2710,6 +2716,7 @@ static int __devexit qe_udc_remove(struct of_device *ofdev)
 	kfree(ep->txframe);
 
 	free_irq(udc_controller->usb_irq, udc_controller);
+	irq_dispose_mapping(udc_controller->usb_irq);
 
 	tasklet_kill(&udc_controller->rx_tasklet);
 

-- 
1.5.6.5

The call chain is this:

qe_udc_irq() <- grabs the udc->lock spinlock
rx_irq()
qe_ep0_rx()
ep0_setup_handle()
setup_received_handle()
ch9getstatus()
qe_ep_queue() <- tries to grab the udc->lock again

It seems unsafe to temporarily drop the lock in the ch9getstatus(),
so to fix that bug the lock-less __qe_ep_queue() function
implemented and used by the ch9getstatus().

Signed-off-by: Anton Vorontsov <redacted>
Acked-by: David Brownell <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index db6fe04..2677c9e 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -1681,14 +1681,11 @@ static void qe_free_request(struct usb_ep *_ep, struct usb_request *_req)
 		kfree(req);
 }
 
-/* queues (submits) an I/O request to an endpoint */
-static int qe_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
-				gfp_t gfp_flags)
+static int __qe_ep_queue(struct usb_ep *_ep, struct usb_request *_req)
 {
 	struct qe_ep *ep = container_of(_ep, struct qe_ep, ep);
 	struct qe_req *req = container_of(_req, struct qe_req, req);
 	struct qe_udc *udc;
-	unsigned long flags;
 	int reval;
 
 	udc = ep->udc;

@@ -1732,7 +1729,7 @@ static int qe_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
 	list_add_tail(&req->queue, &ep->queue);
 	dev_vdbg(udc->dev, "gadget have request in %s! %d\n",
 			ep->name, req->req.length);
-	spin_lock_irqsave(&udc->lock, flags);
+
 	/* push the request to device */
 	if (ep_is_in(ep))
 		reval = ep_req_send(ep, req);

@@ -1748,11 +1745,24 @@ static int qe_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
 	if (ep->dir == USB_DIR_OUT)
 		reval = ep_req_receive(ep, req);
 
-	spin_unlock_irqrestore(&udc->lock, flags);
-
 	return 0;
 }
 
+/* queues (submits) an I/O request to an endpoint */
+static int qe_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
+		       gfp_t gfp_flags)
+{
+	struct qe_ep *ep = container_of(_ep, struct qe_ep, ep);
+	struct qe_udc *udc = ep->udc;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&udc->lock, flags);
+	ret = __qe_ep_queue(_ep, _req);
+	spin_unlock_irqrestore(&udc->lock, flags);
+	return ret;
+}
+
 /* dequeues (cancels, unlinks) an I/O request from an endpoint */
 static int qe_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
 {

@@ -2008,7 +2018,7 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value,
 	udc->ep0_dir = USB_DIR_IN;
 
 	/* data phase */
-	status = qe_ep_queue(&ep->ep, &req->req, GFP_ATOMIC);
+	status = __qe_ep_queue(&ep->ep, &req->req);
 
 	if (status == 0)
 		return;

-- 
1.5.6.5

qe_udc_reg_init() leaves the USB controller enabled before muram memory
initialized. Sometimes the uninitialized muram memory confuses the
controller, and it start sending the busy interrupts.

Fix this by disabling the controller, it will be enabled later by
the gadget driver, at bind time.

Signed-off-by: Anton Vorontsov <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index 2677c9e..b460c6d 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -2452,8 +2452,12 @@ static int __devinit qe_udc_reg_init(struct qe_udc *udc)
 	struct usb_ctlr __iomem *qe_usbregs;
 	qe_usbregs = udc->usb_regs;
 
-	/* Init the usb register */
+	/* Spec says that we must enable the USB controller to change mode. */
 	out_8(&qe_usbregs->usb_usmod, 0x01);
+	/* Mode changed, now disable it, since muram isn't initialized yet. */
+	out_8(&qe_usbregs->usb_usmod, 0x00);
+
+	/* Initialize the rest. */
 	out_be16(&qe_usbregs->usb_usbmr, 0);
 	out_8(&qe_usbregs->usb_uscom, 0);
 	out_be16(&qe_usbregs->usb_usber, USBER_ALL_CLEAR);

-- 
1.5.6.5

Freescale QE UDC controllers can't report the "port change" states,
so the only way to handle disconnects is to process bus reset
interrupts. The bus reset can take some time, that is, few irqs.
Gadgets may print the disconnection events, and this causes few
repetitive messages in the kernel log.

This patch fixes the issue by using the usb_state machine, if the
usb controller has been already reset, just quit the reset irq
early.

Signed-off-by: Anton Vorontsov <redacted>
Acked-by: David Brownell <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index b460c6d..4726582 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -2161,6 +2161,9 @@ static int reset_irq(struct qe_udc *udc)
 {
 	unsigned char i;
 
+	if (udc->usb_state == USB_STATE_DEFAULT)
+		return 0;
+
 	qe_usb_disable();
 	out_8(&udc->usb_regs->usb_usadr, 0);
 

-- 
1.5.6.5

Before freeing an endpoint's muram memory, we should stop all activity
of the endpoint, otherwise the QE UDC controller might do nasty things
with the muram memory that isn't belong to that endpoint anymore.

The qe_ep_reset() effectively flushes the hardware fifos, finishes all
late transaction and thus prevents the corruption.

Signed-off-by: Anton Vorontsov <redacted>
Acked-by: David Brownell <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index 4726582..ea42088 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -1622,6 +1622,7 @@ static int qe_ep_disable(struct usb_ep *_ep)
 	nuke(ep, -ESHUTDOWN);
 	ep->desc = NULL;
 	ep->stopped = 1;
+	qe_ep_reset(udc, ep->epnum);
 	spin_unlock_irqrestore(&udc->lock, flags);
 
 	cpm_muram_free(cpm_muram_offset(ep->rxbase));

-- 
1.5.6.5

While disabling an endpoint the driver nuking any pending requests,
thus completing them with -ESHUTDOWN status. But the driver doesn't
clear the tx_req, which means that a next TX request (after
ep_enable), might get stalled, since the driver won't queue the new
reqests.

This patch fixes a bug I'm observing with ethernet gadget while
playing with ifconfig usb0 up/down (the up/down sequence disables
and enables `in' and `out' endpoints).

Signed-off-by: Anton Vorontsov <redacted>
Acked-by: David Brownell <redacted>
---
 drivers/usb/gadget/fsl_qe_udc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index ea42088..69ccbeb 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c

@@ -1622,6 +1622,7 @@ static int qe_ep_disable(struct usb_ep *_ep)
 	nuke(ep, -ESHUTDOWN);
 	ep->desc = NULL;
 	ep->stopped = 1;
+	ep->tx_req = NULL;
 	qe_ep_reset(udc, ep->epnum);
 	spin_unlock_irqrestore(&udc->lock, flags);
 

-- 
1.5.6.5

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

- k

On Tue, Jan 06, 2009 at 10:44:13PM -0600, Kumar Gala wrote:

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

quoted

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

thanks,

greg k-h

On Jan 6, 2009, at 10:53 PM, Greg KH wrote:

On Tue, Jan 06, 2009 at 10:44:13PM -0600, Kumar Gala wrote:

quoted

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

quoted

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

Any update on the review of these 6 patches?

- k

On Tue, Jan 13, 2009 at 09:49:39AM -0600, Kumar Gala wrote:

On Jan 6, 2009, at 10:53 PM, Greg KH wrote:

quoted

On Tue, Jan 06, 2009 at 10:44:13PM -0600, Kumar Gala wrote:

quoted

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

quoted

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

Any update on the review of these 6 patches?

It's in my queue, been busy with "real work" for a bit right now,
sorry...

thanks,

greg k-h

quoted

quoted

quoted

quoted

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

Any update on the review of these 6 patches?

It's in my queue, been busy with "real work" for a bit right now,
sorry...

thanks,

greg k-h

np. Just going through my patchworks queue and hadn't seen these show  
up in any tree.

- k

On Tue, Jan 13, 2009 at 08:40:01AM -0800, Greg KH wrote:

On Tue, Jan 13, 2009 at 09:49:39AM -0600, Kumar Gala wrote:

quoted

On Jan 6, 2009, at 10:53 PM, Greg KH wrote:

quoted

On Tue, Jan 06, 2009 at 10:44:13PM -0600, Kumar Gala wrote:

quoted

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

quoted

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

Any update on the review of these 6 patches?

It's in my queue, been busy with "real work" for a bit right now,
sorry...

Hello Greg,

Can you please look into these patches? I think they all are 2.6.29
material, since the QE UDC driver is barely usable w/o these fixes.

Thanks,

p.s. And thank you for merging the FHCI driver!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

On Mon, Feb 02, 2009 at 06:50:04PM +0300, Anton Vorontsov wrote:

On Tue, Jan 13, 2009 at 08:40:01AM -0800, Greg KH wrote:

quoted

On Tue, Jan 13, 2009 at 09:49:39AM -0600, Kumar Gala wrote:

quoted

On Jan 6, 2009, at 10:53 PM, Greg KH wrote:

quoted

On Tue, Jan 06, 2009 at 10:44:13PM -0600, Kumar Gala wrote:

quoted

On Dec 25, 2008, at 8:14 AM, Anton Vorontsov wrote:

quoted

Hi all,

Just resending some fixes that seem to be lost...

Greg,

What happened w/this patch set?

As it was sent during the hollidays (on Christmas day at that), it's
still in my "to-review" queue.  I'll get to it in a few days after I
resync the USB tree with Linus.

Any update on the review of these 6 patches?

It's in my queue, been busy with "real work" for a bit right now,
sorry...

Hello Greg,

Can you please look into these patches? I think they all are 2.6.29
material, since the QE UDC driver is barely usable w/o these fixes.

My appologies, I've now applied them all and will queue them up to Linus
before .29 is out.

thanks,

greg k-h