USB: Fixes for fsl_qe_udc driver
cover USB: Fixes for fsl_qe_udc driver
Hi all,
Just resending some fixes that seem to be lost...
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
PATCH 1/6 USB: fsl_qe_udc: Fix oops on QE UDC probe failure
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.
--- drivers/usb/gadget/fsl_qe_udc.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +8 -1
@@ -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);
Signature · 1 line
1.5.6.5
PATCH 2/6 USB: fsl_qe_udc: Fix recursive locking bug in ch9getstatus()
In reply to Anton VorontsovThe 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().
--- drivers/usb/gadget/fsl_qe_udc.c | 26 ++++++++++++++++++-------- 1 files changed, 18 insertions(+), 8 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +18 -8
@@ -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 ;
Signature · 1 line
1.5.6.5
PATCH 3/6 USB: fsl_qe_udc: Fix QE USB controller initialization
In reply to Anton Vorontsovqe_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.
--- drivers/usb/gadget/fsl_qe_udc.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +5 -1
@@ -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 );
Signature · 1 line
1.5.6.5
PATCH 4/6 USB: fsl_qe_udc: Fix disconnects reporting during bus reset
In reply to Anton VorontsovFreescale 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.
--- drivers/usb/gadget/fsl_qe_udc.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +3 -0
@@ -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 );
Signature · 1 line
1.5.6.5
PATCH 5/6 USB: fsl_qe_udc: Fix muram corruption by disabled endpoints
In reply to Anton VorontsovBefore 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.
--- drivers/usb/gadget/fsl_qe_udc.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +1 -0
@@ -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));
Signature · 1 line
1.5.6.5
PATCH 6/6 USB: fsl_qe_udc: Fix stalled TX requests bug
In reply to Anton VorontsovWhile 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).
--- drivers/usb/gadget/fsl_qe_udc.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/usb/gadget/fsl_qe_udc.c +1 -0
@@ -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);
Signature · 1 line
1.5.6.5
Quoted 3 lines · Anton Vorontsov
Hi all,
Just resending some fixes that seem to be lost...
Greg,
What happened w/this patch set?
- k
Quoted 10 lines · Kumar Gala
Quoted 3 lines · Anton Vorontsov
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
Quoted 8 lines · Greg KH
Quoted 7 lines · Kumar Gala
Quoted 3 lines · Anton Vorontsov
Hi all,
Just resending some fixes that seem to be lost...
Quoted 7 lines
Quoted 3 lines
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
Quoted 13 lines · Kumar Gala
Quoted 10 lines · Greg KH
Quoted 9 lines · Kumar Gala
Quoted 3 lines · Anton Vorontsov
Hi all,
Just resending some fixes that seem to be lost...
Greg,
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
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 10 lines
Quoted 10 lines
Quoted 9 lines
Quoted 5 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 1 line
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
Quoted 12 lines · Greg KH
Quoted 11 lines · Kumar Gala
Quoted 10 lines · Greg KH
Quoted 8 lines · Kumar Gala
Quoted 3 lines · Anton Vorontsov
Hi all,
Just resending some fixes that seem to be lost...
Greg,
What happened w/this patch set?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
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!
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
Quoted 19 lines · Anton Vorontsov
Quoted 18 lines · Greg KH
Quoted 16 lines · Kumar Gala
Quoted 6 lines · Greg KH
Quoted 5 lines · Kumar Gala
Quoted 3 lines · Anton Vorontsov
Hi all,
Just resending some fixes that seem to be lost...
Quoted 7 lines
Quoted 3 lines
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?
Quoted 7 lines
Quoted 2 lines
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