> -----Original Message----- > From: John Youn [mailto:John.Youn@xxxxxxxxxxxx] > Sent: Wednesday, June 10, 2015 7:33 PM > To: Kaukab, Yousaf; John Youn; linux-usb@xxxxxxxxxxxxxxx; balbi@xxxxxx > Cc: Herrero, Gregory; heiko@xxxxxxxxx; Holmberg, Hans > Subject: Re: [PATCH 1/3] usb: dwc2: host: allocate qh before atomic enqueue > > On 6/10/2015 6:27 AM, Kaukab, Yousaf wrote: > >> -----Original Message----- > >> From: John Youn [mailto:John.Youn@xxxxxxxxxxxx] > >> Sent: Wednesday, June 10, 2015 1:06 AM > >> To: Kaukab, Yousaf; linux-usb@xxxxxxxxxxxxxxx; balbi@xxxxxx; > >> john.youn@xxxxxxxxxxxx > >> Cc: Herrero, Gregory; heiko@xxxxxxxxx; 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.kaukab@xxxxxxxxx> > >>> --- > >>> 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 <johnyoun@xxxxxxxxxxxx> > >> > >> > >> John > >> > > /Yousaf > > > > Sorry, I meant that the comment is no longer valid. My bad. I will fix it. > > John > /Yousaf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
