On 6/10/2015 6:27 AM, Kaukab, Yousaf wrote: >> -----Original Message----- >> From: John Youn [mailto:john.y...@synopsys.com] >> Sent: Wednesday, June 10, 2015 1:06 AM >> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com; >> john.y...@synopsys.com >> Cc: Herrero, Gregory; he...@sntech.de; Holmberg, Hans >> Subject: Re: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue >> >> On 6/5/2015 7:23 AM, Mian Yousaf Kaukab wrote: >>> To avoid sleep while atomic bugs, allocate qh before calling >>> dwc2_hcd_urb_enqueue. qh pointer can be used directly now instead of >>> passing ep->hcpriv as double pointer. >>> >>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kau...@intel.com> >>> --- >>> drivers/usb/dwc2/hcd.c | 31 ++++++++++++++++++++++++++---- >>> drivers/usb/dwc2/hcd.h | 5 ++++- >>> drivers/usb/dwc2/hcd_queue.c | 45 >>> ++++++++++++-------------------------------- >>> 3 files changed, 43 insertions(+), 38 deletions(-) >>> >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index >>> b10377c..80bce71 100644 >>> --- a/drivers/usb/dwc2/hcd.c >>> +++ b/drivers/usb/dwc2/hcd.c >>> @@ -359,7 +359,7 @@ void dwc2_hcd_stop(struct dwc2_hsotg *hsotg) >>> >>> /* Caller must hold driver lock */ >>> static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg, >>> - struct >> dwc2_hcd_urb *urb, void **ep_handle, >>> + struct >> dwc2_hcd_urb *urb, struct dwc2_qh *qh, >>> gfp_t mem_flags) >>> { >>> struct dwc2_qtd *qtd; >>> @@ -391,8 +391,7 @@ static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg >> *hsotg, >>> return -ENOMEM; >>> >>> dwc2_hcd_qtd_init(qtd, urb); >>> - retval = dwc2_hcd_qtd_add(hsotg, qtd, (struct dwc2_qh >> **)ep_handle, >>> - mem_flags); >>> + retval = dwc2_hcd_qtd_add(hsotg, qtd, qh); >>> if (retval) { >>> dev_err(hsotg->dev, >>> "DWC OTG HCD URB Enqueue >> failed adding QTD. Error status %d\n", @@ >>> -2445,6 +2444,8 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, >> struct urb *urb, >>> u32 tflags = 0; >>> void *buf; >>> unsigned long flags; >>> + struct dwc2_qh *qh; >>> + bool qh_allocated = false; >>> >>> if (dbg_urb(urb)) { >>> dev_vdbg(hsotg->dev, "DWC OTG HCD URB >> Enqueue\n"); @@ -2523,13 >>> +2524,24 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct >> urb *urb, >>> >> urb->iso_frame_desc[i].length); >>> >>> urb->hcpriv = dwc2_urb; >>> + qh = (struct dwc2_qh *) ep->hcpriv; >>> + /* Create QH for the endpoint if it doesn't exist */ >>> + if (!qh) { >>> + qh = dwc2_hcd_qh_create(hsotg, dwc2_urb, >> mem_flags); >>> + if (!qh) { >>> + retval = -ENOMEM; >>> + goto fail0; >>> + } >>> + ep->hcpriv = qh; >>> + qh_allocated = true; >>> + } >>> >>> spin_lock_irqsave(&hsotg->lock, flags); >>> retval = usb_hcd_link_urb_to_ep(hcd, urb); >>> if (retval) >>> goto fail1; >>> >>> - retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, &ep->hcpriv, >> mem_flags); >>> + retval = dwc2_hcd_urb_enqueue(hsotg, dwc2_urb, qh, >> mem_flags); >>> if (retval) >>> goto fail2; >>> >>> @@ -2549,6 +2561,17 @@ fail2: >>> fail1: >>> spin_unlock_irqrestore(&hsotg->lock, flags); >>> urb->hcpriv = NULL; >>> + if (qh_allocated) { >>> + struct dwc2_qtd *qtd2, *qtd2_tmp; >>> + >>> + ep->hcpriv = NULL; >>> + dwc2_hcd_qh_unlink(hsotg, qh); >>> + /* Free each QTD in the QH's QTD list */ >>> + list_for_each_entry_safe(qtd2, qtd2_tmp, &qh- >>> qtd_list, >>> + >> qtd_list_entry) >>> + >> dwc2_hcd_qtd_unlink_and_free(hsotg, qtd2, qh); >>> + dwc2_hcd_qh_free(hsotg, qh); >>> + } >>> fail0: >>> kfree(dwc2_urb); >>> >>> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h index >>> 7b5841c..fc10549 100644 >>> --- a/drivers/usb/dwc2/hcd.h >>> +++ b/drivers/usb/dwc2/hcd.h >>> @@ -463,6 +463,9 @@ extern void dwc2_hcd_queue_transactions(struct >>> dwc2_hsotg *hsotg, >>> /* Schedule Queue Functions */ >>> /* Implemented in hcd_queue.c */ >>> extern void dwc2_hcd_init_usecs(struct dwc2_hsotg *hsotg); >>> +extern struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg, >>> + >> struct dwc2_hcd_urb *urb, >>> + >> gfp_t mem_flags); >>> extern void dwc2_hcd_qh_free(struct dwc2_hsotg *hsotg, struct dwc2_qh >>> *qh); extern int dwc2_hcd_qh_add(struct dwc2_hsotg *hsotg, struct >>> dwc2_qh *qh); extern void dwc2_hcd_qh_unlink(struct dwc2_hsotg >>> *hsotg, struct dwc2_qh *qh); @@ -471,7 +474,7 @@ extern void >>> dwc2_hcd_qh_deactivate(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh, >>> >>> extern void dwc2_hcd_qtd_init(struct dwc2_qtd *qtd, struct >>> dwc2_hcd_urb *urb); extern int dwc2_hcd_qtd_add(struct dwc2_hsotg >> *hsotg, struct dwc2_qtd *qtd, >>> - struct dwc2_qh **qh, gfp_t >> mem_flags); >>> + struct dwc2_qh *qh); >>> >>> /* Unlinks and frees a QTD */ >>> static inline void dwc2_hcd_qtd_unlink_and_free(struct dwc2_hsotg >>> *hsotg, diff --git a/drivers/usb/dwc2/hcd_queue.c >>> b/drivers/usb/dwc2/hcd_queue.c index 9b5c362..e573a4f 100644 >>> --- a/drivers/usb/dwc2/hcd_queue.c >>> +++ b/drivers/usb/dwc2/hcd_queue.c >>> @@ -191,7 +191,7 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, >> struct dwc2_qh *qh, >>> * >>> * Return: Pointer to the newly allocated QH, or NULL on error >>> */ >>> -static struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg, >>> +struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg, >>> >> struct dwc2_hcd_urb *urb, >>> >> gfp_t mem_flags) >>> { >>> @@ -767,57 +767,36 @@ void dwc2_hcd_qtd_init(struct dwc2_qtd *qtd, >> struct dwc2_hcd_urb *urb) >>> * >>> * @hsotg: The DWC HCD structure >>> * @qtd: The QTD to add >>> - * @qh: Out parameter to return queue head >>> - * @atomic_alloc: Flag to do atomic alloc if needed >>> + * @qh: Queue head to add qtd to >>> * >>> * Return: 0 if successful, negative error code otherwise >>> * >>> - * Finds the correct QH to place the QTD into. If it does not find a >>> QH, it >>> - * will create a new QH. If the QH to which the QTD is added is not >>> currently >>> - * scheduled, it is placed into the proper schedule based on its EP type. >>> + * If the QH to which the QTD is added is not currently scheduled, it >>> + is placed >>> + * into the proper schedule based on its EP type. >>> */ >>> int dwc2_hcd_qtd_add(struct dwc2_hsotg *hsotg, struct dwc2_qtd *qtd, >>> - struct dwc2_qh **qh, gfp_t mem_flags) >>> + struct dwc2_qh *qh) >>> { >>> - struct dwc2_hcd_urb *urb = qtd->urb; >>> - int allocated = 0; >>> int retval; >>> >>> /* >>> * Get the QH which holds the QTD-list to insert to. Create QH if >> it >>> * doesn't exist. >>> */ >> >> This comment doesn't apply anymore. Otherwise the series looks ok. > > Applies cleanly to testing/next at my end. I just noticed that the patch is > truncated on gmane > (http://permalink.gmane.org/gmane.linux.usb.general/126493). Looks OK on > other sites. I will resend the whole series anyway. > >> >> Acked-by: John Youn <johny...@synopsys.com> >> >> >> John >> > /Yousaf >
Sorry, I meant that the comment is no longer valid. John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
