* [PATCH v5 0/3] LED triggers for USB host and device
@ 2014-09-17 7:21 Michal Sojka
2014-09-17 7:21 ` [PATCH v5 1/3] usb: gadget: Refactor request completion Michal Sojka
` (2 more replies)
0 siblings, 3 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-17 7:21 UTC (permalink / raw)
To: linux-usb
Cc: Michal Sojka, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
(this is resend of a patch series from about three weeks ago)
This adds LED triggers for USB host and device. First patch refactors
UDC drivers as requested by Felipe Balbi, second is a preparation for
the third, which adds the LED triggers.
Changes from v4:
- Added performance numbers to the commit message of the last patch
(greg k-h).
- Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
- Used proper coding style for switch statement (greg k-h).
- Added comment about NULL argument (greg k-h).
- EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
- Both triggers are now registerd even if host or gagdet subsystem
is not enabled (Bryan Wu, greg k-h).
Changes from v3:
- usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
conditioned block.
- Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
- Removed outdated comment (Alan Stern).
- req->complete == NULL is now a bug. Previously, this was ignored
(Alan Stern).
- File rename moved to a separate commit (greg k-h).
Changes from v2:
- Host/gadget triggers merged to a single file in usb/common/ (Felipe
Balbi).
- UDC drivers refactored so that LED trigger works for all of them.
Changes from v1:
- Moved from drivers/leds/ to drivers/usb/.
- Improved Kconfig help.
- Linked with other modules rather than being standalone modules.
Michal Sojka (3):
usb: gadget: Refactor request completion
usb: Rename usb-common.c
usb: Add LED triggers for USB activity
drivers/usb/Kconfig | 10 +++++
drivers/usb/chipidea/udc.c | 6 +--
drivers/usb/common/Makefile | 5 ++-
drivers/usb/common/{usb-common.c => common.c} | 0
drivers/usb/common/led.c | 57 +++++++++++++++++++++++++++
drivers/usb/core/hcd.c | 2 +
drivers/usb/dwc2/gadget.c | 6 +--
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +-
drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 ++---
drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +--
drivers/usb/gadget/udc/fsl_udc_core.c | 6 +--
drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c | 8 +---
drivers/usb/gadget/udc/mv_udc_core.c | 8 +---
drivers/usb/gadget/udc/net2272.c | 2 +-
drivers/usb/gadget/udc/net2280.c | 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +-
drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
drivers/usb/gadget/udc/udc-core.c | 23 +++++++++++
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb.h | 12 ++++++
include/linux/usb/gadget.h | 8 ++++
37 files changed, 157 insertions(+), 57 deletions(-)
rename drivers/usb/common/{usb-common.c => common.c} (100%)
create mode 100644 drivers/usb/common/led.c
--
2.1.0
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 1/3] usb: gadget: Refactor request completion
2014-09-17 7:21 [PATCH v5 0/3] LED triggers for USB host and device Michal Sojka
@ 2014-09-17 7:21 ` Michal Sojka
2014-09-17 15:28 ` Felipe Balbi
2014-09-17 7:21 ` [PATCH v5 2/3] usb: Rename usb-common.c Michal Sojka
2014-09-17 7:21 ` [PATCH v5 3/3] usb: Add LED triggers for USB activity Michal Sojka
2 siblings, 1 reply; 57+ messages in thread
From: Michal Sojka @ 2014-09-17 7:21 UTC (permalink / raw)
To: linux-usb
Cc: Michal Sojka, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.
All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/dwc2/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
drivers/usb/gadget/udc/net2272.c | 2 +-
drivers/usb/gadget/udc/net2280.c | 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
drivers/usb/gadget/udc/udc-core.c | 19 +++++++++++++++++++
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb/gadget.h | 8 ++++++++
31 files changed, 68 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
hwreq->req.length)
hweptemp = hwep->ci->ep0in;
- hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
hs_req = ep->req;
ep->req = NULL;
list_del_init(&hs_req->queue);
- hs_req->req.complete(&ep->ep,
- &hs_req->req);
+ usb_gadget_giveback_request(&ep->ep,
+ &hs_req->req);
}
/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
if (hs_req->req.complete) {
spin_unlock(&hsotg->lock);
- hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+ usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
spin_lock(&hsotg->lock);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
req->request.length, status);
spin_unlock(&dwc->lock);
- req->request.complete(&dep->endpoint, &req->request);
+ usb_gadget_giveback_request(&dep->endpoint, &req->request);
spin_lock(&dwc->lock);
}
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 41b062e..3b9d138 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
&req->req, req->req.length, ep->ep.name, sts);
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->halted = halted;
}
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..9968f53 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
ep->stopped = 1;
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 906e65f..e96032f 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
list_del_init(&req->queue);
usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
@@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
ep->ep.name, req, req->req.status, req->req.actual);
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index e969eb8..2235b88 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
breq->req.status = -ESHUTDOWN;
spin_unlock_irqrestore(&udc->lock, flags);
- breq->req.complete(&iudma->bep->ep, &breq->req);
+ usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
spin_lock_irqsave(&udc->lock, flags);
}
}
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 2b54955..81dc595 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
req->req.status = -ESHUTDOWN;
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
}
}
@@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
spin_unlock(&dum->lock);
_req->actual = _req->length;
_req->status = 0;
- _req->complete(_ep, _req);
+ usb_gadget_giveback_request(_ep, _req);
spin_lock(&dum->lock);
} else
list_add_tail(&req->queue, &ep->queue);
@@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
dev_dbg(udc_dev(dum),
"dequeued req %p from %s, len %d buf %p\n",
req, _ep->name, _req->length, _req->buf);
- _req->complete(_ep, _req);
+ usb_gadget_giveback_request(_ep, _req);
}
local_irq_restore(flags);
return retval;
@@ -1385,7 +1385,7 @@ top:
list_del_init(&req->queue);
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
/* requests might have been unlinked... */
@@ -1761,7 +1761,7 @@ restart:
req);
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
ep->already_seen = 0;
goto restart;
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
index e143d69..1d31592 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
req->req.status = status;
spin_unlock(&ep->fotg210->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->fotg210->lock);
if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 7324308..dd18ea3 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
ep->stopped = 1;
spin_unlock(&udc->lock);
- /* this complete() should a func implemented by gadget layer,
- * eg fsg->bulk_in_complete() */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
@@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_LICENSE("GPL");
-
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index 75b23ea..c362079 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- /* complete() is from gadget layer,
- * eg fsg->bulk_in_complete() */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
index d40255f..d52687d 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
req->req.status = status;
spin_unlock(&ep->fusb300->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->fusb300->lock);
if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index 6c85839..bf9c5ef 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 08df5c4..924533b 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
} else if (req->req.complete) {
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
}
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 1629ad7..feab0ba 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
ep->req_pending = 0;
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index de88d33..8985656 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -729,7 +729,7 @@ __acquires(m66592->lock)
restart = 1;
spin_unlock(&ep->m66592->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->m66592->lock);
if (restart) {
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 1624871..046a1f8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
}
spin_unlock(&ep->u3d->lock);
- /*
- * complete() is from gadget layer,
- * eg fsg->bulk_in_complete()
- */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->u3d->lock);
}
diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
index 040fb16..3c5db80 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- /*
- * complete() is from gadget layer,
- * eg fsg->bulk_in_complete()
- */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 059cfe5..84d7162 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index f4eac11..2cda166 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index e731373..dcdfea4 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index eb8c3be..0b69b32 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1486,7 +1486,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
spin_unlock(&dev->lock);
if (!ep->in)
pch_udc_ep_clear_rrdy(ep);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->halted = halted;
}
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index 251e4d5..42f7eeb 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..4868369 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
if (pflags)
spin_unlock_irqrestore(&ep->lock, *pflags);
local_irq_save(flags);
- req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
+ usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
local_irq_restore(flags);
if (pflags)
spin_lock_irqsave(&ep->lock, *pflags);
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index 4600842..0ec09a4 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
sudmac_free_channel(ep->r8a66597, ep, req);
spin_unlock(&ep->r8a66597->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->r8a66597->lock);
if (restart) {
diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index 10c6a12..dfbf557 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
hsep->stopped = 1;
spin_unlock(&hsudc->lock);
- if (hsreq->req.complete != NULL)
- hsreq->req.complete(&hsep->ep, &hsreq->req);
+ usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
spin_lock(&hsudc->lock);
hsep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 357b58e..ff423d1 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
status = req->req.status;
ep->halted = 1;
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
ep->halted = halted;
}
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..29789f1 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* ------------------------------------------------------------------------- */
+/**
+ * usb_gadget_giveback_request - give the request back to the gadget layer
+ * Context: in_interrupt()
+ *
+ * This is called by device controller drivers in order to return the
+ * completed request back to the gadget layer.
+ */
+void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req)
+{
+ if (likely(req->complete))
+ req->complete(ep, req);
+ else
+ pr_err("%s : req->complete must not be NULL\n", __func__);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+
+/* ------------------------------------------------------------------------- */
+
static void usb_gadget_state_work(struct work_struct *work)
{
struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..24c8c02 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
ep->end_point.name, request,
req->request.actual, req->request.length,
request->status);
- req->request.complete(&req->ep->end_point, &req->request);
+ usb_gadget_giveback_request(&req->ep->end_point, &req->request);
spin_lock(&musb->lock);
ep->busy = busy;
}
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 04e6505..2d17c10 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
ureq->req.status = status;
- ureq->req.complete(&uep->ep, &ureq->req);
+ usb_gadget_giveback_request(&uep->ep, &ureq->req);
}
static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 2/3] usb: Rename usb-common.c
2014-09-17 7:21 [PATCH v5 0/3] LED triggers for USB host and device Michal Sojka
2014-09-17 7:21 ` [PATCH v5 1/3] usb: gadget: Refactor request completion Michal Sojka
@ 2014-09-17 7:21 ` Michal Sojka
2014-09-17 7:21 ` [PATCH v5 3/3] usb: Add LED triggers for USB activity Michal Sojka
2 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-17 7:21 UTC (permalink / raw)
To: linux-usb
Cc: Michal Sojka, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/common/Makefile | 4 +++-
drivers/usb/common/{usb-common.c => common.c} | 0
2 files changed, 3 insertions(+), 1 deletion(-)
rename drivers/usb/common/{usb-common.c => common.c} (100%)
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
# Makefile for the usb common parts.
#
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON) += usb-common.o
+usb-common-y += common.o
+
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 3/3] usb: Add LED triggers for USB activity
2014-09-17 7:21 [PATCH v5 0/3] LED triggers for USB host and device Michal Sojka
2014-09-17 7:21 ` [PATCH v5 1/3] usb: gadget: Refactor request completion Michal Sojka
2014-09-17 7:21 ` [PATCH v5 2/3] usb: Rename usb-common.c Michal Sojka
@ 2014-09-17 7:21 ` Michal Sojka
2 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-17 7:21 UTC (permalink / raw)
To: linux-usb
Cc: Michal Sojka, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.
Both triggers should work with all host/device controllers. Tested only
with musb.
Performace: I measured performance overheads on ARM Cortex-A8 (TI
AM335x) running on 600 MHz.
Duration of usb_led_activity():
- with no LED attached to the trigger: 2 ± 1 µs
- with one GPIO LED attached to the trigger: 2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)
Duration of functions calling usb_led_activity() (with this patch
applied and no LED attached to the trigger):
- __usb_hcd_giveback_urb(): 10 - 25 µs
- usb_gadget_giveback_request(): 2 - 6 µs
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/Kconfig | 10 +++++++
drivers/usb/common/Makefile | 1 +
drivers/usb/common/led.c | 57 +++++++++++++++++++++++++++++++++++++++
drivers/usb/core/hcd.c | 2 ++
drivers/usb/gadget/udc/udc-core.c | 4 +++
include/linux/usb.h | 12 +++++++++
6 files changed, 86 insertions(+)
create mode 100644 drivers/usb/common/led.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..9d9cc3b 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,14 @@ source "drivers/usb/phy/Kconfig"
source "drivers/usb/gadget/Kconfig"
+config USB_LED_TRIG
+ bool "USB LED Triggers"
+ depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
+ help
+ This option adds LED triggers for USB host and/or gadget activity.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as activity indicators for USB host or
+ gadget.
+
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 0000000..df23da0
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,57 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb.h>
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+ struct led_trigger *trig = NULL;
+
+ switch (ev) {
+ case USB_LED_EVENT_GADGET:
+ trig = ledtrig_usb_gadget;
+ break;
+ case USB_LED_EVENT_HOST:
+ trig = ledtrig_usb_host;
+ break;
+ }
+ /* led_trigger_blink_oneshot() handles trig == NULL gracefully */
+ led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL_GPL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+ led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+ led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+ return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+ led_trigger_unregister_simple(ledtrig_usb_gadget);
+ led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
usbmon_urb_complete(&hcd->self, urb, status);
usb_anchor_suspend_wakeups(anchor);
usb_unanchor_urb(urb);
+ if (likely(status == 0))
+ usb_led_activity(USB_LED_EVENT_HOST);
/* pass ownership to the completion handler */
urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 29789f1..f272cf0 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
+#include <linux/usb.h>
/**
* struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req)
{
+ if (likely(req->status == 0))
+ usb_led_activity(USB_LED_EVENT_GADGET);
+
if (likely(req->complete))
req->complete(ep, req);
else
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
/* debugfs stuff */
extern struct dentry *usb_debug_root;
+/* LED triggers */
+enum usb_led_event {
+ USB_LED_EVENT_HOST = 0,
+ USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
#endif /* __KERNEL__ */
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
2014-09-17 7:21 ` [PATCH v5 1/3] usb: gadget: Refactor request completion Michal Sojka
@ 2014-09-17 15:28 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-17 15:28 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3439 bytes --]
On Wed, Sep 17, 2014 at 09:21:11AM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
> drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
> drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
> drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
> drivers/usb/gadget/udc/net2272.c | 2 +-
> drivers/usb/gadget/udc/net2280.c | 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
> drivers/usb/gadget/udc/udc-core.c | 19 +++++++++++++++++++
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> include/linux/usb/gadget.h | 8 ++++++++
I would rather split this into several patches, btw. With the
introduction of usb_gadget_giveback_request() being the first one in the
series. It's easier to review that way.
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..29789f1 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> /* ------------------------------------------------------------------------- */
>
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + if (likely(req->complete))
> + req->complete(ep, req);
> + else
> + pr_err("%s : req->complete must not be NULL\n", __func__);
let it Oops. We require ->complete to be valid, if there's any gadget
driver not setting ->complete, it deserves to oops so we can the error.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
@ 2014-09-17 15:28 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-17 15:28 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3439 bytes --]
On Wed, Sep 17, 2014 at 09:21:11AM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
> drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
> drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
> drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
> drivers/usb/gadget/udc/net2272.c | 2 +-
> drivers/usb/gadget/udc/net2280.c | 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
> drivers/usb/gadget/udc/udc-core.c | 19 +++++++++++++++++++
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> include/linux/usb/gadget.h | 8 ++++++++
I would rather split this into several patches, btw. With the
introduction of usb_gadget_giveback_request() being the first one in the
series. It's easier to review that way.
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..29789f1 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> /* ------------------------------------------------------------------------- */
>
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + if (likely(req->complete))
> + req->complete(ep, req);
> + else
> + pr_err("%s : req->complete must not be NULL\n", __func__);
let it Oops. We require ->complete to be valid, if there's any gadget
driver not setting ->complete, it deserves to oops so we can the error.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
2014-09-17 15:28 ` Felipe Balbi
@ 2014-09-23 8:09 ` Michal Sojka
-1 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-23 8:09 UTC (permalink / raw)
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
Dear Felipe,
On Wed, Sep 17 2014, Felipe Balbi wrote:
> On Wed, Sep 17, 2014 at 09:21:11AM +0200, Michal Sojka wrote:
>> All USB peripheral controller drivers called completion routines
>> directly. This patch moves the completion call from drivers to
>> usb_gadget_giveback_request(), in order to have a place where common
>> functionality can be added.
>>
>> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
>> call to usb_gadget_giveback_request(). This was compile-tested with all
>> ARM drivers enabled and runtime-tested for musb.
>>
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
>> ---
>> drivers/usb/chipidea/udc.c | 6 +++---
>> drivers/usb/dwc2/gadget.c | 6 +++---
>> drivers/usb/dwc3/gadget.c | 2 +-
>> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
>> drivers/usb/gadget/udc/at91_udc.c | 2 +-
>> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
>> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
>> drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
>> drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
>> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
>> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
>> drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
>> drivers/usb/gadget/udc/goku_udc.c | 2 +-
>> drivers/usb/gadget/udc/gr_udc.c | 2 +-
>> drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
>> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
>> drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
>> drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
>> drivers/usb/gadget/udc/net2272.c | 2 +-
>> drivers/usb/gadget/udc/net2280.c | 2 +-
>> drivers/usb/gadget/udc/omap_udc.c | 2 +-
>> drivers/usb/gadget/udc/pch_udc.c | 2 +-
>> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
>> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
>> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
>> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
>> drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
>> drivers/usb/gadget/udc/udc-core.c | 19 +++++++++++++++++++
>> drivers/usb/musb/musb_gadget.c | 2 +-
>> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
>> include/linux/usb/gadget.h | 8 ++++++++
>
> I would rather split this into several patches, btw. With the
> introduction of usb_gadget_giveback_request() being the first one in the
> series. It's easier to review that way.
This would be no problem.
>> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
>> index b0d9817..29789f1 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>>
>> /* ------------------------------------------------------------------------- */
>>
>> +/**
>> + * usb_gadget_giveback_request - give the request back to the gadget layer
>> + * Context: in_interrupt()
>> + *
>> + * This is called by device controller drivers in order to return the
>> + * completed request back to the gadget layer.
>> + */
>> +void usb_gadget_giveback_request(struct usb_ep *ep,
>> + struct usb_request *req)
>> +{
>> + if (likely(req->complete))
>> + req->complete(ep, req);
>> + else
>> + pr_err("%s : req->complete must not be NULL\n", __func__);
>
> let it Oops. We require ->complete to be valid, if there's any gadget
> driver not setting ->complete, it deserves to oops so we can the
> error.
The Oops was there before, but I removed it because greg k-h didn't want
it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
still want the oops here?
-Michal
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
@ 2014-09-23 8:09 ` Michal Sojka
0 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-23 8:09 UTC (permalink / raw)
To: Felipe Balbi
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
Dear Felipe,
On Wed, Sep 17 2014, Felipe Balbi wrote:
> On Wed, Sep 17, 2014 at 09:21:11AM +0200, Michal Sojka wrote:
>> All USB peripheral controller drivers called completion routines
>> directly. This patch moves the completion call from drivers to
>> usb_gadget_giveback_request(), in order to have a place where common
>> functionality can be added.
>>
>> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
>> call to usb_gadget_giveback_request(). This was compile-tested with all
>> ARM drivers enabled and runtime-tested for musb.
>>
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
>> ---
>> drivers/usb/chipidea/udc.c | 6 +++---
>> drivers/usb/dwc2/gadget.c | 6 +++---
>> drivers/usb/dwc3/gadget.c | 2 +-
>> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
>> drivers/usb/gadget/udc/at91_udc.c | 2 +-
>> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
>> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
>> drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
>> drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
>> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
>> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
>> drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
>> drivers/usb/gadget/udc/goku_udc.c | 2 +-
>> drivers/usb/gadget/udc/gr_udc.c | 2 +-
>> drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
>> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
>> drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
>> drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
>> drivers/usb/gadget/udc/net2272.c | 2 +-
>> drivers/usb/gadget/udc/net2280.c | 2 +-
>> drivers/usb/gadget/udc/omap_udc.c | 2 +-
>> drivers/usb/gadget/udc/pch_udc.c | 2 +-
>> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
>> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
>> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
>> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
>> drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
>> drivers/usb/gadget/udc/udc-core.c | 19 +++++++++++++++++++
>> drivers/usb/musb/musb_gadget.c | 2 +-
>> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
>> include/linux/usb/gadget.h | 8 ++++++++
>
> I would rather split this into several patches, btw. With the
> introduction of usb_gadget_giveback_request() being the first one in the
> series. It's easier to review that way.
This would be no problem.
>> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
>> index b0d9817..29789f1 100644
>> --- a/drivers/usb/gadget/udc/udc-core.c
>> +++ b/drivers/usb/gadget/udc/udc-core.c
>> @@ -106,6 +106,25 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>>
>> /* ------------------------------------------------------------------------- */
>>
>> +/**
>> + * usb_gadget_giveback_request - give the request back to the gadget layer
>> + * Context: in_interrupt()
>> + *
>> + * This is called by device controller drivers in order to return the
>> + * completed request back to the gadget layer.
>> + */
>> +void usb_gadget_giveback_request(struct usb_ep *ep,
>> + struct usb_request *req)
>> +{
>> + if (likely(req->complete))
>> + req->complete(ep, req);
>> + else
>> + pr_err("%s : req->complete must not be NULL\n", __func__);
>
> let it Oops. We require ->complete to be valid, if there's any gadget
> driver not setting ->complete, it deserves to oops so we can the
> error.
The Oops was there before, but I removed it because greg k-h didn't want
it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
still want the oops here?
-Michal
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
2014-09-23 8:09 ` Michal Sojka
@ 2014-09-24 14:48 ` Felipe Balbi
-1 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 14:48 UTC (permalink / raw)
To: Michal Sojka
Cc: Felipe Balbi, linux-usb, Alan Stern, Bryan Wu,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
Hi,
On Tue, Sep 23, 2014 at 10:09:22AM +0200, Michal Sojka wrote:
> >> +/**
> >> + * usb_gadget_giveback_request - give the request back to the gadget layer
> >> + * Context: in_interrupt()
> >> + *
> >> + * This is called by device controller drivers in order to return the
> >> + * completed request back to the gadget layer.
> >> + */
> >> +void usb_gadget_giveback_request(struct usb_ep *ep,
> >> + struct usb_request *req)
> >> +{
> >> + if (likely(req->complete))
> >> + req->complete(ep, req);
> >> + else
> >> + pr_err("%s : req->complete must not be NULL\n", __func__);
> >
> > let it Oops. We require ->complete to be valid, if there's any gadget
> > driver not setting ->complete, it deserves to oops so we can the
> > error.
>
> The Oops was there before, but I removed it because greg k-h didn't want
> it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
> still want the oops here?
you don't need a BUG_ON(), just call req->complete() directly without
any checks, if it's not valid, it _will_ oops. ->complete() is mandatory
and anybody sending requests without complete deserve to oops.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
@ 2014-09-24 14:48 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 14:48 UTC (permalink / raw)
To: Michal Sojka
Cc: Felipe Balbi, linux-usb, Alan Stern, Bryan Wu,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
Hi,
On Tue, Sep 23, 2014 at 10:09:22AM +0200, Michal Sojka wrote:
> >> +/**
> >> + * usb_gadget_giveback_request - give the request back to the gadget layer
> >> + * Context: in_interrupt()
> >> + *
> >> + * This is called by device controller drivers in order to return the
> >> + * completed request back to the gadget layer.
> >> + */
> >> +void usb_gadget_giveback_request(struct usb_ep *ep,
> >> + struct usb_request *req)
> >> +{
> >> + if (likely(req->complete))
> >> + req->complete(ep, req);
> >> + else
> >> + pr_err("%s : req->complete must not be NULL\n", __func__);
> >
> > let it Oops. We require ->complete to be valid, if there's any gadget
> > driver not setting ->complete, it deserves to oops so we can the
> > error.
>
> The Oops was there before, but I removed it because greg k-h didn't want
> it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
> still want the oops here?
you don't need a BUG_ON(), just call req->complete() directly without
any checks, if it's not valid, it _will_ oops. ->complete() is mandatory
and anybody sending requests without complete deserve to oops.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
2014-09-23 8:09 ` Michal Sojka
@ 2014-09-24 15:08 ` Alan Stern
-1 siblings, 0 replies; 57+ messages in thread
From: Alan Stern @ 2014-09-24 15:08 UTC (permalink / raw)
To: Michal Sojka
Cc: Felipe Balbi, linux-usb, Bryan Wu, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac
On Tue, 23 Sep 2014, Michal Sojka wrote:
> >> +/**
> >> + * usb_gadget_giveback_request - give the request back to the gadget layer
> >> + * Context: in_interrupt()
> >> + *
> >> + * This is called by device controller drivers in order to return the
> >> + * completed request back to the gadget layer.
> >> + */
> >> +void usb_gadget_giveback_request(struct usb_ep *ep,
> >> + struct usb_request *req)
> >> +{
> >> + if (likely(req->complete))
> >> + req->complete(ep, req);
> >> + else
> >> + pr_err("%s : req->complete must not be NULL\n", __func__);
> >
> > let it Oops. We require ->complete to be valid, if there's any gadget
> > driver not setting ->complete, it deserves to oops so we can the
> > error.
>
> The Oops was there before, but I removed it because greg k-h didn't want
> it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
> still want the oops here?
Greg didn't want you to add a BUG() statement, whereas Felipe wants you
to leave out the "if" test and pr_err(). Just omit both, and everyone
will be satisfied.
Alan Stern
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 1/3] usb: gadget: Refactor request completion
@ 2014-09-24 15:08 ` Alan Stern
0 siblings, 0 replies; 57+ messages in thread
From: Alan Stern @ 2014-09-24 15:08 UTC (permalink / raw)
To: Michal Sojka
Cc: Felipe Balbi, linux-usb, Bryan Wu, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac
On Tue, 23 Sep 2014, Michal Sojka wrote:
> >> +/**
> >> + * usb_gadget_giveback_request - give the request back to the gadget layer
> >> + * Context: in_interrupt()
> >> + *
> >> + * This is called by device controller drivers in order to return the
> >> + * completed request back to the gadget layer.
> >> + */
> >> +void usb_gadget_giveback_request(struct usb_ep *ep,
> >> + struct usb_request *req)
> >> +{
> >> + if (likely(req->complete))
> >> + req->complete(ep, req);
> >> + else
> >> + pr_err("%s : req->complete must not be NULL\n", __func__);
> >
> > let it Oops. We require ->complete to be valid, if there's any gadget
> > driver not setting ->complete, it deserves to oops so we can the
> > error.
>
> The Oops was there before, but I removed it because greg k-h didn't want
> it. See http://marc.info/?l=linux-usb&m=140917381611947&w=2. Do you
> still want the oops here?
Greg didn't want you to add a BUG() statement, whereas Felipe wants you
to leave out the "if" test and pr_err(). Just omit both, and everyone
will be satisfied.
Alan Stern
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 15:08 ` Alan Stern
(?)
@ 2014-09-24 20:43 ` Michal Sojka
[not found] ` <1411591401-5874-1-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
` (3 more replies)
-1 siblings, 4 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac, Michal Sojka
This adds LED triggers for USB host and device. First two patches
refactor UDC drivers as requested by Felipe Balbi, the next renames a
file and the last is the actual implementation of the LED triggers.
Changes from v5:
- Refactoring of USB gadget completion split into two patches (Filipe
Balbi)
- Removed check for non-NULL req->completion (Filipe Balbi)
Changes from v4:
- Added performance numbers to the commit message of the last patch
(greg k-h).
- Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
- Used proper coding style for switch statement (greg k-h).
- Added comment about NULL argument (greg k-h).
- EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
- Both triggers are now registerd even if host or gagdet subsystem
is not enabled (Bryan Wu, greg k-h).
Changes from v3:
- usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
conditioned block.
- Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
- Removed outdated comment (Alan Stern).
- req->complete == NULL is now a bug. Previously, this was ignored
(Alan Stern).
- File rename moved to a separate commit (greg k-h).
Changes from v2:
- Host/gadget triggers merged to a single file in usb/common/ (Felipe
Balbi).
- UDC drivers refactored so that LED trigger works for all of them.
Changes from v1:
- Moved from drivers/leds/ to drivers/usb/.
- Improved Kconfig help.
- Linked with other modules rather than being standalone modules.
Michal Sojka (4):
usb: gadget: Introduce usb_gadget_giveback_request()
usb: gadget: Refactor request completion
usb: Rename usb-common.c
usb: Add LED triggers for USB activity
drivers/usb/Kconfig | 10 +++
drivers/usb/chipidea/udc.c | 6 +-
drivers/usb/common/Makefile | 5 +-
drivers/usb/common/common.c | 144 ++++++++++++++++++++++++++++++++
drivers/usb/common/led.c | 57 +++++++++++++
drivers/usb/common/usb-common.c | 144 --------------------------------
drivers/usb/core/hcd.c | 2 +
drivers/usb/dwc2/gadget.c | 6 +-
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +-
drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +--
drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
drivers/usb/gadget/udc/fsl_udc_core.c | 6 +-
drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c | 8 +-
drivers/usb/gadget/udc/mv_udc_core.c | 8 +-
drivers/usb/gadget/udc/net2272.c | 2 +-
drivers/usb/gadget/udc/net2280.c | 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +-
drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
drivers/usb/gadget/udc/udc-core.c | 20 +++++
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb.h | 12 +++
include/linux/usb/gadget.h | 8 ++
38 files changed, 298 insertions(+), 201 deletions(-)
create mode 100644 drivers/usb/common/common.c
create mode 100644 drivers/usb/common/led.c
delete mode 100644 drivers/usb/common/usb-common.c
--
2.1.0
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v6 1/4] usb: gadget: Introduce usb_gadget_giveback_request()
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
@ 2014-09-24 20:43 ` Michal Sojka
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
michal.vokac-veUE7cmDK2A, Michal Sojka
All USB peripheral controller drivers call completion routines directly.
This patch adds usb_gadget_giveback_request() which will be used instead
of direct invocation in the next patch. The goal here is to have a place
where common functionality can be added.
Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
---
drivers/usb/gadget/udc/udc-core.c | 16 ++++++++++++++++
include/linux/usb/gadget.h | 8 ++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..f7b3151 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -106,6 +106,22 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* ------------------------------------------------------------------------- */
+/**
+ * usb_gadget_giveback_request - give the request back to the gadget layer
+ * Context: in_interrupt()
+ *
+ * This is called by device controller drivers in order to return the
+ * completed request back to the gadget layer.
+ */
+void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req)
+{
+ req->complete(ep, req);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+
+/* ------------------------------------------------------------------------- */
+
static void usb_gadget_state_work(struct work_struct *work)
{
struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 1/4] usb: gadget: Introduce usb_gadget_giveback_request()
@ 2014-09-24 20:43 ` Michal Sojka
0 siblings, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac, Michal Sojka
All USB peripheral controller drivers call completion routines directly.
This patch adds usb_gadget_giveback_request() which will be used instead
of direct invocation in the next patch. The goal here is to have a place
where common functionality can be added.
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/gadget/udc/udc-core.c | 16 ++++++++++++++++
include/linux/usb/gadget.h | 8 ++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b0d9817..f7b3151 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -106,6 +106,22 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* ------------------------------------------------------------------------- */
+/**
+ * usb_gadget_giveback_request - give the request back to the gadget layer
+ * Context: in_interrupt()
+ *
+ * This is called by device controller drivers in order to return the
+ * completed request back to the gadget layer.
+ */
+void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req)
+{
+ req->complete(ep, req);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+
+/* ------------------------------------------------------------------------- */
+
static void usb_gadget_state_work(struct work_struct *work)
{
struct usb_gadget *gadget = work_to_gadget(work);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c3a6185..f795e95 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
/*-------------------------------------------------------------------------*/
+/* utility to give requests back to the gadget layer */
+
+extern void usb_gadget_giveback_request(struct usb_ep *ep,
+ struct usb_request *req);
+
+
+/*-------------------------------------------------------------------------*/
+
/* utility wrapping a simple endpoint selection policy */
extern struct usb_ep *usb_ep_autoconfig(struct usb_gadget *,
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 2/4] usb: gadget: Refactor request completion
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
[not found] ` <1411591401-5874-1-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
@ 2014-09-24 20:43 ` Michal Sojka
2014-09-24 23:14 ` Felipe Balbi
2014-09-29 8:50 ` Robert Baldyga
2014-09-24 20:43 ` [PATCH v6 3/4] usb: Rename usb-common.c Michal Sojka
2014-09-24 20:43 ` [PATCH v6 4/4] usb: Add LED triggers for USB activity Michal Sojka
3 siblings, 2 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac, Michal Sojka
Use the recently introduced usb_gadget_giveback_request() in favor of
direct invocation of the completion routine.
All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/dwc2/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
drivers/usb/gadget/udc/net2272.c | 2 +-
drivers/usb/gadget/udc/net2280.c | 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
29 files changed, 41 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
hwreq->req.length)
hweptemp = hwep->ci->ep0in;
- hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ce6071d..ada5f30 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
hs_req = ep->req;
ep->req = NULL;
list_del_init(&hs_req->queue);
- hs_req->req.complete(&ep->ep,
- &hs_req->req);
+ usb_gadget_giveback_request(&ep->ep,
+ &hs_req->req);
}
/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
if (hs_req->req.complete) {
spin_unlock(&hsotg->lock);
- hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+ usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
spin_lock(&hsotg->lock);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 490a6ca..45f1dfc 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
req->request.length, status);
spin_unlock(&dwc->lock);
- req->request.complete(&dep->endpoint, &req->request);
+ usb_gadget_giveback_request(&dep->endpoint, &req->request);
spin_lock(&dwc->lock);
}
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 41b062e..3b9d138 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
&req->req, req->req.length, ep->ep.name, sts);
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->halted = halted;
}
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index cfd18bc..9968f53 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
ep->stopped = 1;
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index c9fe67e..1529926 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
list_del_init(&req->queue);
usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
@@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
ep->ep.name, req, req->req.status, req->req.actual);
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index e969eb8..2235b88 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
breq->req.status = -ESHUTDOWN;
spin_unlock_irqrestore(&udc->lock, flags);
- breq->req.complete(&iudma->bep->ep, &breq->req);
+ usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
spin_lock_irqsave(&udc->lock, flags);
}
}
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 2b54955..81dc595 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
req->req.status = -ESHUTDOWN;
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
}
}
@@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
spin_unlock(&dum->lock);
_req->actual = _req->length;
_req->status = 0;
- _req->complete(_ep, _req);
+ usb_gadget_giveback_request(_ep, _req);
spin_lock(&dum->lock);
} else
list_add_tail(&req->queue, &ep->queue);
@@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
dev_dbg(udc_dev(dum),
"dequeued req %p from %s, len %d buf %p\n",
req, _ep->name, _req->length, _req->buf);
- _req->complete(_ep, _req);
+ usb_gadget_giveback_request(_ep, _req);
}
local_irq_restore(flags);
return retval;
@@ -1385,7 +1385,7 @@ top:
list_del_init(&req->queue);
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
/* requests might have been unlinked... */
@@ -1761,7 +1761,7 @@ restart:
req);
spin_unlock(&dum->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dum->lock);
ep->already_seen = 0;
goto restart;
diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
index e143d69..1d31592 100644
--- a/drivers/usb/gadget/udc/fotg210-udc.c
+++ b/drivers/usb/gadget/udc/fotg210-udc.c
@@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
req->req.status = status;
spin_unlock(&ep->fotg210->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->fotg210->lock);
if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 7324308..dd18ea3 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
ep->stopped = 1;
spin_unlock(&udc->lock);
- /* this complete() should a func implemented by gadget layer,
- * eg fsg->bulk_in_complete() */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
@@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_LICENSE("GPL");
-
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index 75b23ea..c362079 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- /* complete() is from gadget layer,
- * eg fsg->bulk_in_complete() */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
index 5c5d1ad..8286df7 100644
--- a/drivers/usb/gadget/udc/fusb300_udc.c
+++ b/drivers/usb/gadget/udc/fusb300_udc.c
@@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
req->req.status = status;
spin_unlock(&ep->fusb300->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->fusb300->lock);
if (ep->epnum) {
diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index 6c85839..bf9c5ef 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
index 08df5c4..924533b 100644
--- a/drivers/usb/gadget/udc/gr_udc.c
+++ b/drivers/usb/gadget/udc/gr_udc.c
@@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
} else if (req->req.complete) {
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
}
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 1629ad7..feab0ba 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
ep->req_pending = 0;
spin_unlock(&udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&udc->lock);
}
diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index de88d33..8985656 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -729,7 +729,7 @@ __acquires(m66592->lock)
restart = 1;
spin_unlock(&ep->m66592->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->m66592->lock);
if (restart) {
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 1624871..046a1f8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
}
spin_unlock(&ep->u3d->lock);
- /*
- * complete() is from gadget layer,
- * eg fsg->bulk_in_complete()
- */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->u3d->lock);
}
diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
index 040fb16..3c5db80 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- /*
- * complete() is from gadget layer,
- * eg fsg->bulk_in_complete()
- */
- if (req->req.complete)
- req->req.complete(&ep->ep, &req->req);
+
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
index 059cfe5..84d7162 100644
--- a/drivers/usb/gadget/udc/net2272.c
+++ b/drivers/usb/gadget/udc/net2272.c
@@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index 2e95715..8d13337 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&dev->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index e731373..dcdfea4 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
spin_unlock(&ep->udc->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->udc->lock);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index 460d953..ccbe3d4 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1490,7 +1490,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
spin_unlock(&dev->lock);
if (!ep->in)
pch_udc_ep_clear_rrdy(ep);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&dev->lock);
ep->halted = halted;
}
diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
index 251e4d5..42f7eeb 100644
--- a/drivers/usb/gadget/udc/pxa25x_udc.c
+++ b/drivers/usb/gadget/udc/pxa25x_udc.c
@@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
/* don't modify queue heads during completion callback */
ep->stopped = 1;
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
ep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 597d39f..4868369 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
if (pflags)
spin_unlock_irqrestore(&ep->lock, *pflags);
local_irq_save(flags);
- req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
+ usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
local_irq_restore(flags);
if (pflags)
spin_lock_irqsave(&ep->lock, *pflags);
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index de2a871..ad7f71e 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
sudmac_free_channel(ep->r8a66597, ep, req);
spin_unlock(&ep->r8a66597->lock);
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
spin_lock(&ep->r8a66597->lock);
if (restart) {
diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
index 10c6a12..dfbf557 100644
--- a/drivers/usb/gadget/udc/s3c-hsudc.c
+++ b/drivers/usb/gadget/udc/s3c-hsudc.c
@@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
hsep->stopped = 1;
spin_unlock(&hsudc->lock);
- if (hsreq->req.complete != NULL)
- hsreq->req.complete(&hsep->ep, &hsreq->req);
+ usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
spin_lock(&hsudc->lock);
hsep->stopped = stopped;
}
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 357b58e..ff423d1 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
status = req->req.status;
ep->halted = 1;
- req->req.complete(&ep->ep, &req->req);
+ usb_gadget_giveback_request(&ep->ep, &req->req);
ep->halted = halted;
}
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index d4aa779..24c8c02 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
ep->end_point.name, request,
req->request.actual, req->request.length,
request->status);
- req->request.complete(&req->ep->end_point, &req->request);
+ usb_gadget_giveback_request(&req->ep->end_point, &req->request);
spin_lock(&musb->lock);
ep->busy = busy;
}
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index 04e6505..2d17c10 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
ureq->req.status = status;
- ureq->req.complete(&uep->ep, &ureq->req);
+ usb_gadget_giveback_request(&uep->ep, &ureq->req);
}
static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 3/4] usb: Rename usb-common.c
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
[not found] ` <1411591401-5874-1-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
@ 2014-09-24 20:43 ` Michal Sojka
[not found] ` <1411591401-5874-4-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
2014-09-25 15:03 ` Greg Kroah-Hartman
2014-09-24 20:43 ` [PATCH v6 4/4] usb: Add LED triggers for USB activity Michal Sojka
3 siblings, 2 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac, Michal Sojka
In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/common/Makefile | 4 +-
drivers/usb/common/common.c | 144 ++++++++++++++++++++++++++++++++++++++++
drivers/usb/common/usb-common.c | 144 ----------------------------------------
3 files changed, 147 insertions(+), 145 deletions(-)
create mode 100644 drivers/usb/common/common.c
delete mode 100644 drivers/usb/common/usb-common.c
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
# Makefile for the usb common parts.
#
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON) += usb-common.o
+usb-common-y += common.o
+
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
new file mode 100644
index 0000000..6dfd30a
--- /dev/null
+++ b/drivers/usb/common/common.c
@@ -0,0 +1,144 @@
+/*
+ * Provides code common for host and device side USB.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ *
+ * If either host side (ie. CONFIG_USB=y) or device side USB stack
+ * (ie. CONFIG_USB_GADGET=y) is compiled in the kernel, this module is
+ * compiled-in as well. Otherwise, if either of the two stacks is
+ * compiled as module, this file is compiled as module as well.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/of.h>
+#include <linux/usb/otg.h>
+
+const char *usb_otg_state_string(enum usb_otg_state state)
+{
+ static const char *const names[] = {
+ [OTG_STATE_A_IDLE] = "a_idle",
+ [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
+ [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
+ [OTG_STATE_A_HOST] = "a_host",
+ [OTG_STATE_A_SUSPEND] = "a_suspend",
+ [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
+ [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
+ [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
+ [OTG_STATE_B_IDLE] = "b_idle",
+ [OTG_STATE_B_SRP_INIT] = "b_srp_init",
+ [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
+ [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
+ [OTG_STATE_B_HOST] = "b_host",
+ };
+
+ if (state < 0 || state >= ARRAY_SIZE(names))
+ return "UNDEFINED";
+
+ return names[state];
+}
+EXPORT_SYMBOL_GPL(usb_otg_state_string);
+
+static const char *const speed_names[] = {
+ [USB_SPEED_UNKNOWN] = "UNKNOWN",
+ [USB_SPEED_LOW] = "low-speed",
+ [USB_SPEED_FULL] = "full-speed",
+ [USB_SPEED_HIGH] = "high-speed",
+ [USB_SPEED_WIRELESS] = "wireless",
+ [USB_SPEED_SUPER] = "super-speed",
+};
+
+const char *usb_speed_string(enum usb_device_speed speed)
+{
+ if (speed < 0 || speed >= ARRAY_SIZE(speed_names))
+ speed = USB_SPEED_UNKNOWN;
+ return speed_names[speed];
+}
+EXPORT_SYMBOL_GPL(usb_speed_string);
+
+const char *usb_state_string(enum usb_device_state state)
+{
+ static const char *const names[] = {
+ [USB_STATE_NOTATTACHED] = "not attached",
+ [USB_STATE_ATTACHED] = "attached",
+ [USB_STATE_POWERED] = "powered",
+ [USB_STATE_RECONNECTING] = "reconnecting",
+ [USB_STATE_UNAUTHENTICATED] = "unauthenticated",
+ [USB_STATE_DEFAULT] = "default",
+ [USB_STATE_ADDRESS] = "addressed",
+ [USB_STATE_CONFIGURED] = "configured",
+ [USB_STATE_SUSPENDED] = "suspended",
+ };
+
+ if (state < 0 || state >= ARRAY_SIZE(names))
+ return "UNKNOWN";
+
+ return names[state];
+}
+EXPORT_SYMBOL_GPL(usb_state_string);
+
+#ifdef CONFIG_OF
+static const char *const usb_dr_modes[] = {
+ [USB_DR_MODE_UNKNOWN] = "",
+ [USB_DR_MODE_HOST] = "host",
+ [USB_DR_MODE_PERIPHERAL] = "peripheral",
+ [USB_DR_MODE_OTG] = "otg",
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np: Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+ const char *dr_mode;
+ int err, i;
+
+ err = of_property_read_string(np, "dr_mode", &dr_mode);
+ if (err < 0)
+ return USB_DR_MODE_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
+ if (!strcmp(dr_mode, usb_dr_modes[i]))
+ return i;
+
+ return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+
+/**
+ * of_usb_get_maximum_speed - Get maximum requested speed for a given USB
+ * controller.
+ * @np: Pointer to the given device_node
+ *
+ * The function gets the maximum speed string from property "maximum-speed",
+ * and returns the corresponding enum usb_device_speed.
+ */
+enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np)
+{
+ const char *maximum_speed;
+ int err;
+ int i;
+
+ err = of_property_read_string(np, "maximum-speed", &maximum_speed);
+ if (err < 0)
+ return USB_SPEED_UNKNOWN;
+
+ for (i = 0; i < ARRAY_SIZE(speed_names); i++)
+ if (strcmp(maximum_speed, speed_names[i]) == 0)
+ return i;
+
+ return USB_SPEED_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_maximum_speed);
+
+#endif
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/usb-common.c
deleted file mode 100644
index 6dfd30a..0000000
--- a/drivers/usb/common/usb-common.c
+++ /dev/null
@@ -1,144 +0,0 @@
-/*
- * Provides code common for host and device side USB.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2.
- *
- * If either host side (ie. CONFIG_USB=y) or device side USB stack
- * (ie. CONFIG_USB_GADGET=y) is compiled in the kernel, this module is
- * compiled-in as well. Otherwise, if either of the two stacks is
- * compiled as module, this file is compiled as module as well.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/usb/ch9.h>
-#include <linux/usb/of.h>
-#include <linux/usb/otg.h>
-
-const char *usb_otg_state_string(enum usb_otg_state state)
-{
- static const char *const names[] = {
- [OTG_STATE_A_IDLE] = "a_idle",
- [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
- [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
- [OTG_STATE_A_HOST] = "a_host",
- [OTG_STATE_A_SUSPEND] = "a_suspend",
- [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
- [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
- [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
- [OTG_STATE_B_IDLE] = "b_idle",
- [OTG_STATE_B_SRP_INIT] = "b_srp_init",
- [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
- [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
- [OTG_STATE_B_HOST] = "b_host",
- };
-
- if (state < 0 || state >= ARRAY_SIZE(names))
- return "UNDEFINED";
-
- return names[state];
-}
-EXPORT_SYMBOL_GPL(usb_otg_state_string);
-
-static const char *const speed_names[] = {
- [USB_SPEED_UNKNOWN] = "UNKNOWN",
- [USB_SPEED_LOW] = "low-speed",
- [USB_SPEED_FULL] = "full-speed",
- [USB_SPEED_HIGH] = "high-speed",
- [USB_SPEED_WIRELESS] = "wireless",
- [USB_SPEED_SUPER] = "super-speed",
-};
-
-const char *usb_speed_string(enum usb_device_speed speed)
-{
- if (speed < 0 || speed >= ARRAY_SIZE(speed_names))
- speed = USB_SPEED_UNKNOWN;
- return speed_names[speed];
-}
-EXPORT_SYMBOL_GPL(usb_speed_string);
-
-const char *usb_state_string(enum usb_device_state state)
-{
- static const char *const names[] = {
- [USB_STATE_NOTATTACHED] = "not attached",
- [USB_STATE_ATTACHED] = "attached",
- [USB_STATE_POWERED] = "powered",
- [USB_STATE_RECONNECTING] = "reconnecting",
- [USB_STATE_UNAUTHENTICATED] = "unauthenticated",
- [USB_STATE_DEFAULT] = "default",
- [USB_STATE_ADDRESS] = "addressed",
- [USB_STATE_CONFIGURED] = "configured",
- [USB_STATE_SUSPENDED] = "suspended",
- };
-
- if (state < 0 || state >= ARRAY_SIZE(names))
- return "UNKNOWN";
-
- return names[state];
-}
-EXPORT_SYMBOL_GPL(usb_state_string);
-
-#ifdef CONFIG_OF
-static const char *const usb_dr_modes[] = {
- [USB_DR_MODE_UNKNOWN] = "",
- [USB_DR_MODE_HOST] = "host",
- [USB_DR_MODE_PERIPHERAL] = "peripheral",
- [USB_DR_MODE_OTG] = "otg",
-};
-
-/**
- * of_usb_get_dr_mode - Get dual role mode for given device_node
- * @np: Pointer to the given device_node
- *
- * The function gets phy interface string from property 'dr_mode',
- * and returns the correspondig enum usb_dr_mode
- */
-enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
-{
- const char *dr_mode;
- int err, i;
-
- err = of_property_read_string(np, "dr_mode", &dr_mode);
- if (err < 0)
- return USB_DR_MODE_UNKNOWN;
-
- for (i = 0; i < ARRAY_SIZE(usb_dr_modes); i++)
- if (!strcmp(dr_mode, usb_dr_modes[i]))
- return i;
-
- return USB_DR_MODE_UNKNOWN;
-}
-EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
-
-/**
- * of_usb_get_maximum_speed - Get maximum requested speed for a given USB
- * controller.
- * @np: Pointer to the given device_node
- *
- * The function gets the maximum speed string from property "maximum-speed",
- * and returns the corresponding enum usb_device_speed.
- */
-enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np)
-{
- const char *maximum_speed;
- int err;
- int i;
-
- err = of_property_read_string(np, "maximum-speed", &maximum_speed);
- if (err < 0)
- return USB_SPEED_UNKNOWN;
-
- for (i = 0; i < ARRAY_SIZE(speed_names); i++)
- if (strcmp(maximum_speed, speed_names[i]) == 0)
- return i;
-
- return USB_SPEED_UNKNOWN;
-}
-EXPORT_SYMBOL_GPL(of_usb_get_maximum_speed);
-
-#endif
-
-MODULE_LICENSE("GPL");
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 4/4] usb: Add LED triggers for USB activity
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
` (2 preceding siblings ...)
2014-09-24 20:43 ` [PATCH v6 3/4] usb: Rename usb-common.c Michal Sojka
@ 2014-09-24 20:43 ` Michal Sojka
2014-09-24 20:56 ` Felipe Balbi
3 siblings, 1 reply; 57+ messages in thread
From: Michal Sojka @ 2014-09-24 20:43 UTC (permalink / raw)
To: linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac, Michal Sojka
With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.
Both triggers should work with all host/device controllers. Tested only
with musb.
Performace: I measured performance overheads on ARM Cortex-A8 (TI
AM335x) running on 600 MHz.
Duration of usb_led_activity():
- with no LED attached to the trigger: 2 ± 1 µs
- with one GPIO LED attached to the trigger: 2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)
Duration of functions calling usb_led_activity() (with this patch
applied and no LED attached to the trigger):
- __usb_hcd_giveback_urb(): 10 - 25 µs
- usb_gadget_giveback_request(): 2 - 6 µs
Signed-off-by: Michal Sojka <sojka@merica.cz>
---
drivers/usb/Kconfig | 10 +++++++
drivers/usb/common/Makefile | 1 +
drivers/usb/common/led.c | 57 +++++++++++++++++++++++++++++++++++++++
drivers/usb/core/hcd.c | 2 ++
drivers/usb/gadget/udc/udc-core.c | 4 +++
include/linux/usb.h | 12 +++++++++
6 files changed, 86 insertions(+)
create mode 100644 drivers/usb/common/led.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index cf1b19b..ae481c3 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -149,4 +149,14 @@ source "drivers/usb/phy/Kconfig"
source "drivers/usb/gadget/Kconfig"
+config USB_LED_TRIG
+ bool "USB LED Triggers"
+ depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
+ help
+ This option adds LED triggers for USB host and/or gadget activity.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as activity indicators for USB host or
+ gadget.
+
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 0000000..df23da0
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,57 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka <sojka@merica.cz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/usb.h>
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+ struct led_trigger *trig = NULL;
+
+ switch (ev) {
+ case USB_LED_EVENT_GADGET:
+ trig = ledtrig_usb_gadget;
+ break;
+ case USB_LED_EVENT_HOST:
+ trig = ledtrig_usb_host;
+ break;
+ }
+ /* led_trigger_blink_oneshot() handles trig == NULL gracefully */
+ led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL_GPL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+ led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+ led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+ return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+ led_trigger_unregister_simple(ledtrig_usb_gadget);
+ led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
usbmon_urb_complete(&hcd->self, urb, status);
usb_anchor_suspend_wakeups(anchor);
usb_unanchor_urb(urb);
+ if (likely(status == 0))
+ usb_led_activity(USB_LED_EVENT_HOST);
/* pass ownership to the completion handler */
urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index f7b3151..931b9f1 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
+#include <linux/usb.h>
/**
* struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req)
{
+ if (likely(req->status == 0))
+ usb_led_activity(USB_LED_EVENT_GADGET);
+
req->complete(ep, req);
}
EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block *nb);
/* debugfs stuff */
extern struct dentry *usb_debug_root;
+/* LED triggers */
+enum usb_led_event {
+ USB_LED_EVENT_HOST = 0,
+ USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
#endif /* __KERNEL__ */
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v6 4/4] usb: Add LED triggers for USB activity
2014-09-24 20:43 ` [PATCH v6 4/4] usb: Add LED triggers for USB activity Michal Sojka
@ 2014-09-24 20:56 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:56 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3796 bytes --]
Hi,
On Wed, Sep 24, 2014 at 10:43:21PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
>
> Both triggers should work with all host/device controllers. Tested only
> with musb.
>
> Performace: I measured performance overheads on ARM Cortex-A8 (TI
> AM335x) running on 600 MHz.
>
> Duration of usb_led_activity():
> - with no LED attached to the trigger: 2 ± 1 µs
> - with one GPIO LED attached to the trigger: 2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)
>
> Duration of functions calling usb_led_activity() (with this patch
> applied and no LED attached to the trigger):
> - __usb_hcd_giveback_urb(): 10 - 25 µs
> - usb_gadget_giveback_request(): 2 - 6 µs
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/Kconfig | 10 +++++++
> drivers/usb/common/Makefile | 1 +
> drivers/usb/common/led.c | 57 +++++++++++++++++++++++++++++++++++++++
> drivers/usb/core/hcd.c | 2 ++
> drivers/usb/gadget/udc/udc-core.c | 4 +++
> include/linux/usb.h | 12 +++++++++
> 6 files changed, 86 insertions(+)
> create mode 100644 drivers/usb/common/led.c
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index cf1b19b..ae481c3 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -149,4 +149,14 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +config USB_LED_TRIG
> + bool "USB LED Triggers"
> + depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
> + help
> + This option adds LED triggers for USB host and/or gadget activity.
> +
> + Say Y here if you are working on a system with led-class supported
> + LEDs and you want to use them as activity indicators for USB host or
> + gadget.
> +
> endif # USB_SUPPORT
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 052c120..ca2f8bd 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,5 +4,6 @@
>
> obj-$(CONFIG_USB_COMMON) += usb-common.o
> usb-common-y += common.o
> +usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> new file mode 100644
> index 0000000..df23da0
> --- /dev/null
> +++ b/drivers/usb/common/led.c
> @@ -0,0 +1,57 @@
> +/*
> + * LED Triggers for USB Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/usb.h>
> +
> +#define BLINK_DELAY 30
> +
> +static unsigned long usb_blink_delay = BLINK_DELAY;
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
> +DEFINE_LED_TRIGGER(ledtrig_usb_host);
> +
> +void usb_led_activity(enum usb_led_event ev)
> +{
> + struct led_trigger *trig = NULL;
> +
> + switch (ev) {
> + case USB_LED_EVENT_GADGET:
> + trig = ledtrig_usb_gadget;
> + break;
> + case USB_LED_EVENT_HOST:
> + trig = ledtrig_usb_host;
> + break;
> + }
> + /* led_trigger_blink_oneshot() handles trig == NULL gracefully */
> + led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
> +}
> +EXPORT_SYMBOL_GPL(usb_led_activity);
> +
> +
minor nit: one blank line only. other than that, this looks pretty good
to me:
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 4/4] usb: Add LED triggers for USB activity
@ 2014-09-24 20:56 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:56 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3796 bytes --]
Hi,
On Wed, Sep 24, 2014 at 10:43:21PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
>
> Both triggers should work with all host/device controllers. Tested only
> with musb.
>
> Performace: I measured performance overheads on ARM Cortex-A8 (TI
> AM335x) running on 600 MHz.
>
> Duration of usb_led_activity():
> - with no LED attached to the trigger: 2 ± 1 µs
> - with one GPIO LED attached to the trigger: 2 ± 1 µs or 8 ± 2 µs (two peaks in histogram)
>
> Duration of functions calling usb_led_activity() (with this patch
> applied and no LED attached to the trigger):
> - __usb_hcd_giveback_urb(): 10 - 25 µs
> - usb_gadget_giveback_request(): 2 - 6 µs
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/Kconfig | 10 +++++++
> drivers/usb/common/Makefile | 1 +
> drivers/usb/common/led.c | 57 +++++++++++++++++++++++++++++++++++++++
> drivers/usb/core/hcd.c | 2 ++
> drivers/usb/gadget/udc/udc-core.c | 4 +++
> include/linux/usb.h | 12 +++++++++
> 6 files changed, 86 insertions(+)
> create mode 100644 drivers/usb/common/led.c
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index cf1b19b..ae481c3 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -149,4 +149,14 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +config USB_LED_TRIG
> + bool "USB LED Triggers"
> + depends on LEDS_CLASS && USB_COMMON && LEDS_TRIGGERS
> + help
> + This option adds LED triggers for USB host and/or gadget activity.
> +
> + Say Y here if you are working on a system with led-class supported
> + LEDs and you want to use them as activity indicators for USB host or
> + gadget.
> +
> endif # USB_SUPPORT
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 052c120..ca2f8bd 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,5 +4,6 @@
>
> obj-$(CONFIG_USB_COMMON) += usb-common.o
> usb-common-y += common.o
> +usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> new file mode 100644
> index 0000000..df23da0
> --- /dev/null
> +++ b/drivers/usb/common/led.c
> @@ -0,0 +1,57 @@
> +/*
> + * LED Triggers for USB Activity
> + *
> + * Copyright 2014 Michal Sojka <sojka@merica.cz>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/usb.h>
> +
> +#define BLINK_DELAY 30
> +
> +static unsigned long usb_blink_delay = BLINK_DELAY;
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
> +DEFINE_LED_TRIGGER(ledtrig_usb_host);
> +
> +void usb_led_activity(enum usb_led_event ev)
> +{
> + struct led_trigger *trig = NULL;
> +
> + switch (ev) {
> + case USB_LED_EVENT_GADGET:
> + trig = ledtrig_usb_gadget;
> + break;
> + case USB_LED_EVENT_HOST:
> + trig = ledtrig_usb_host;
> + break;
> + }
> + /* led_trigger_blink_oneshot() handles trig == NULL gracefully */
> + led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
> +}
> +EXPORT_SYMBOL_GPL(usb_led_activity);
> +
> +
minor nit: one blank line only. other than that, this looks pretty good
to me:
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
@ 2014-09-24 20:59 ` Felipe Balbi
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:59 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alan Stern, Bryan Wu,
Felipe Balbi, Greg Kroah-Hartman, Linux LED Subsystem,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, michal.vokac-veUE7cmDK2A
[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]
On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> This adds LED triggers for USB host and device. First two patches
> refactor UDC drivers as requested by Felipe Balbi, the next renames a
> file and the last is the actual implementation of the LED triggers.
>
> Changes from v5:
> - Refactoring of USB gadget completion split into two patches (Filipe
> Balbi)
> - Removed check for non-NULL req->completion (Filipe Balbi)
>
> Changes from v4:
> - Added performance numbers to the commit message of the last patch
> (greg k-h).
> - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> - Used proper coding style for switch statement (greg k-h).
> - Added comment about NULL argument (greg k-h).
> - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> - Both triggers are now registerd even if host or gagdet subsystem
> is not enabled (Bryan Wu, greg k-h).
>
> Changes from v3:
> - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> conditioned block.
> - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> - Removed outdated comment (Alan Stern).
> - req->complete == NULL is now a bug. Previously, this was ignored
> (Alan Stern).
> - File rename moved to a separate commit (greg k-h).
>
> Changes from v2:
> - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> Balbi).
> - UDC drivers refactored so that LED trigger works for all of them.
>
> Changes from v1:
> - Moved from drivers/leds/ to drivers/usb/.
> - Improved Kconfig help.
> - Linked with other modules rather than being standalone modules.
>
> Michal Sojka (4):
> usb: gadget: Introduce usb_gadget_giveback_request()
> usb: gadget: Refactor request completion
> usb: Rename usb-common.c
> usb: Add LED triggers for USB activity
Greg, how do you want to handle this one as it touched both host and
gadget ? I could wait until you apply this series to your usb-next and
based my pull request on that. That'll work as long as it doesn't take
too long for this to be merged.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
@ 2014-09-24 20:59 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:59 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]
On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> This adds LED triggers for USB host and device. First two patches
> refactor UDC drivers as requested by Felipe Balbi, the next renames a
> file and the last is the actual implementation of the LED triggers.
>
> Changes from v5:
> - Refactoring of USB gadget completion split into two patches (Filipe
> Balbi)
> - Removed check for non-NULL req->completion (Filipe Balbi)
>
> Changes from v4:
> - Added performance numbers to the commit message of the last patch
> (greg k-h).
> - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> - Used proper coding style for switch statement (greg k-h).
> - Added comment about NULL argument (greg k-h).
> - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> - Both triggers are now registerd even if host or gagdet subsystem
> is not enabled (Bryan Wu, greg k-h).
>
> Changes from v3:
> - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> conditioned block.
> - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> - Removed outdated comment (Alan Stern).
> - req->complete == NULL is now a bug. Previously, this was ignored
> (Alan Stern).
> - File rename moved to a separate commit (greg k-h).
>
> Changes from v2:
> - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> Balbi).
> - UDC drivers refactored so that LED trigger works for all of them.
>
> Changes from v1:
> - Moved from drivers/leds/ to drivers/usb/.
> - Improved Kconfig help.
> - Linked with other modules rather than being standalone modules.
>
> Michal Sojka (4):
> usb: gadget: Introduce usb_gadget_giveback_request()
> usb: gadget: Refactor request completion
> usb: Rename usb-common.c
> usb: Add LED triggers for USB activity
Greg, how do you want to handle this one as it touched both host and
gadget ? I could wait until you apply this series to your usb-next and
based my pull request on that. That'll work as long as it doesn't take
too long for this to be merged.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 1/4] usb: gadget: Introduce usb_gadget_giveback_request()
2014-09-24 20:43 ` Michal Sojka
@ 2014-09-24 21:00 ` Felipe Balbi
-1 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 21:00 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alan Stern, Bryan Wu,
Felipe Balbi, Greg Kroah-Hartman, Linux LED Subsystem,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, michal.vokac-veUE7cmDK2A
[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]
On Wed, Sep 24, 2014 at 10:43:18PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers call completion routines directly.
> This patch adds usb_gadget_giveback_request() which will be used instead
> of direct invocation in the next patch. The goal here is to have a place
> where common functionality can be added.
>
> Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
> ---
> drivers/usb/gadget/udc/udc-core.c | 16 ++++++++++++++++
> include/linux/usb/gadget.h | 8 ++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..f7b3151 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -106,6 +106,22 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> /* ------------------------------------------------------------------------- */
>
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> +
> +/* ------------------------------------------------------------------------- */
> +
> static void usb_gadget_state_work(struct work_struct *work)
> {
> struct usb_gadget *gadget = work_to_gadget(work);
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index c3a6185..f795e95 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
>
> /*-------------------------------------------------------------------------*/
>
> +/* utility to give requests back to the gadget layer */
> +
> +extern void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req);
> +
> +
minot nit: one blank line only, other than that
Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 1/4] usb: gadget: Introduce usb_gadget_giveback_request()
@ 2014-09-24 21:00 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 21:00 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]
On Wed, Sep 24, 2014 at 10:43:18PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers call completion routines directly.
> This patch adds usb_gadget_giveback_request() which will be used instead
> of direct invocation in the next patch. The goal here is to have a place
> where common functionality can be added.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/gadget/udc/udc-core.c | 16 ++++++++++++++++
> include/linux/usb/gadget.h | 8 ++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..f7b3151 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -106,6 +106,22 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> /* ------------------------------------------------------------------------- */
>
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> +
> +/* ------------------------------------------------------------------------- */
> +
> static void usb_gadget_state_work(struct work_struct *work)
> {
> struct usb_gadget *gadget = work_to_gadget(work);
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index c3a6185..f795e95 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -1013,6 +1013,14 @@ extern void usb_gadget_set_state(struct usb_gadget *gadget,
>
> /*-------------------------------------------------------------------------*/
>
> +/* utility to give requests back to the gadget layer */
> +
> +extern void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req);
> +
> +
minot nit: one blank line only, other than that
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 20:59 ` Felipe Balbi
(?)
@ 2014-09-24 21:41 ` Greg Kroah-Hartman
[not found] ` <20140924214155.GA30689-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
-1 siblings, 1 reply; 57+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-24 21:41 UTC (permalink / raw)
To: Felipe Balbi
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > This adds LED triggers for USB host and device. First two patches
> > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > file and the last is the actual implementation of the LED triggers.
> >
> > Changes from v5:
> > - Refactoring of USB gadget completion split into two patches (Filipe
> > Balbi)
> > - Removed check for non-NULL req->completion (Filipe Balbi)
> >
> > Changes from v4:
> > - Added performance numbers to the commit message of the last patch
> > (greg k-h).
> > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > - Used proper coding style for switch statement (greg k-h).
> > - Added comment about NULL argument (greg k-h).
> > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > - Both triggers are now registerd even if host or gagdet subsystem
> > is not enabled (Bryan Wu, greg k-h).
> >
> > Changes from v3:
> > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > conditioned block.
> > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > - Removed outdated comment (Alan Stern).
> > - req->complete == NULL is now a bug. Previously, this was ignored
> > (Alan Stern).
> > - File rename moved to a separate commit (greg k-h).
> >
> > Changes from v2:
> > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > Balbi).
> > - UDC drivers refactored so that LED trigger works for all of them.
> >
> > Changes from v1:
> > - Moved from drivers/leds/ to drivers/usb/.
> > - Improved Kconfig help.
> > - Linked with other modules rather than being standalone modules.
> >
> > Michal Sojka (4):
> > usb: gadget: Introduce usb_gadget_giveback_request()
> > usb: gadget: Refactor request completion
> > usb: Rename usb-common.c
> > usb: Add LED triggers for USB activity
>
> Greg, how do you want to handle this one as it touched both host and
> gadget ? I could wait until you apply this series to your usb-next and
> based my pull request on that. That'll work as long as it doesn't take
> too long for this to be merged.
I can just take this now, for 3.18-rc1, you've already sent me your
patches for that release, so there shouldn't be any conflicts, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 21:41 ` Greg Kroah-Hartman
@ 2014-09-24 22:18 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 22:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Felipe Balbi, Michal Sojka, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Alan Stern, Bryan Wu, Linux LED Subsystem,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, michal.vokac-veUE7cmDK2A
[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]
On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > This adds LED triggers for USB host and device. First two patches
> > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > file and the last is the actual implementation of the LED triggers.
> > >
> > > Changes from v5:
> > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > Balbi)
> > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > >
> > > Changes from v4:
> > > - Added performance numbers to the commit message of the last patch
> > > (greg k-h).
> > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > - Used proper coding style for switch statement (greg k-h).
> > > - Added comment about NULL argument (greg k-h).
> > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > - Both triggers are now registerd even if host or gagdet subsystem
> > > is not enabled (Bryan Wu, greg k-h).
> > >
> > > Changes from v3:
> > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > conditioned block.
> > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > - Removed outdated comment (Alan Stern).
> > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > (Alan Stern).
> > > - File rename moved to a separate commit (greg k-h).
> > >
> > > Changes from v2:
> > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > Balbi).
> > > - UDC drivers refactored so that LED trigger works for all of them.
> > >
> > > Changes from v1:
> > > - Moved from drivers/leds/ to drivers/usb/.
> > > - Improved Kconfig help.
> > > - Linked with other modules rather than being standalone modules.
> > >
> > > Michal Sojka (4):
> > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > usb: gadget: Refactor request completion
> > > usb: Rename usb-common.c
> > > usb: Add LED triggers for USB activity
> >
> > Greg, how do you want to handle this one as it touched both host and
> > gadget ? I could wait until you apply this series to your usb-next and
> > based my pull request on that. That'll work as long as it doesn't take
> > too long for this to be merged.
>
> I can just take this now, for 3.18-rc1, you've already sent me your
> patches for that release, so there shouldn't be any conflicts, right?
that's better. Can you wait 1 day though ? I can run these through my
build tests.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
@ 2014-09-24 22:18 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 22:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Felipe Balbi, Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]
On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > This adds LED triggers for USB host and device. First two patches
> > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > file and the last is the actual implementation of the LED triggers.
> > >
> > > Changes from v5:
> > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > Balbi)
> > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > >
> > > Changes from v4:
> > > - Added performance numbers to the commit message of the last patch
> > > (greg k-h).
> > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > - Used proper coding style for switch statement (greg k-h).
> > > - Added comment about NULL argument (greg k-h).
> > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > - Both triggers are now registerd even if host or gagdet subsystem
> > > is not enabled (Bryan Wu, greg k-h).
> > >
> > > Changes from v3:
> > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > conditioned block.
> > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > - Removed outdated comment (Alan Stern).
> > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > (Alan Stern).
> > > - File rename moved to a separate commit (greg k-h).
> > >
> > > Changes from v2:
> > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > Balbi).
> > > - UDC drivers refactored so that LED trigger works for all of them.
> > >
> > > Changes from v1:
> > > - Moved from drivers/leds/ to drivers/usb/.
> > > - Improved Kconfig help.
> > > - Linked with other modules rather than being standalone modules.
> > >
> > > Michal Sojka (4):
> > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > usb: gadget: Refactor request completion
> > > usb: Rename usb-common.c
> > > usb: Add LED triggers for USB activity
> >
> > Greg, how do you want to handle this one as it touched both host and
> > gadget ? I could wait until you apply this series to your usb-next and
> > based my pull request on that. That'll work as long as it doesn't take
> > too long for this to be merged.
>
> I can just take this now, for 3.18-rc1, you've already sent me your
> patches for that release, so there shouldn't be any conflicts, right?
that's better. Can you wait 1 day though ? I can run these through my
build tests.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
@ 2014-09-24 23:14 ` Felipe Balbi
2014-09-29 8:50 ` Robert Baldyga
1 sibling, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:14 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 499 bytes --]
On Wed, Sep 24, 2014 at 10:43:19PM +0200, Michal Sojka wrote:
> Use the recently introduced usb_gadget_giveback_request() in favor of
> direct invocation of the completion routine.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
@ 2014-09-24 23:14 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:14 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 499 bytes --]
On Wed, Sep 24, 2014 at 10:43:19PM +0200, Michal Sojka wrote:
> Use the recently introduced usb_gadget_giveback_request() in favor of
> direct invocation of the completion routine.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 3/4] usb: Rename usb-common.c
2014-09-24 20:43 ` [PATCH v6 3/4] usb: Rename usb-common.c Michal Sojka
@ 2014-09-24 23:15 ` Felipe Balbi
2014-09-25 15:03 ` Greg Kroah-Hartman
1 sibling, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:15 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alan Stern, Bryan Wu,
Felipe Balbi, Greg Kroah-Hartman, Linux LED Subsystem,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, michal.vokac-veUE7cmDK2A
[-- Attachment #1: Type: text/plain, Size: 683 bytes --]
On Wed, Sep 24, 2014 at 10:43:20PM +0200, Michal Sojka wrote:
> In the next commit, we will want the usb-common module to be composed of
> two object files. Since Kbuild cannot "append" another object to an
> existing one, we need to rename usb-common.c to something
> else (common.c) and create usb-common.o by linking the wanted objects
> together. Currently, usb-common.o comprises only common.o.
>
> Signed-off-by: Michal Sojka <sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
for future reference, you can use git format-patch -M -C to detect
renames, the generated diff is far better ;-)
Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 3/4] usb: Rename usb-common.c
@ 2014-09-24 23:15 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:15 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Wed, Sep 24, 2014 at 10:43:20PM +0200, Michal Sojka wrote:
> In the next commit, we will want the usb-common module to be composed of
> two object files. Since Kbuild cannot "append" another object to an
> existing one, we need to rename usb-common.c to something
> else (common.c) and create usb-common.o by linking the wanted objects
> together. Currently, usb-common.o comprises only common.o.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
for future reference, you can use git format-patch -M -C to detect
renames, the generated diff is far better ;-)
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 22:18 ` Felipe Balbi
@ 2014-09-24 23:15 ` Felipe Balbi
-1 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:15 UTC (permalink / raw)
To: Felipe Balbi
Cc: Greg Kroah-Hartman, Michal Sojka, linux-usb, Alan Stern,
Bryan Wu, Linux LED Subsystem, linux-kernel, michal.vokac
[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]
On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > This adds LED triggers for USB host and device. First two patches
> > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > file and the last is the actual implementation of the LED triggers.
> > > >
> > > > Changes from v5:
> > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > Balbi)
> > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > >
> > > > Changes from v4:
> > > > - Added performance numbers to the commit message of the last patch
> > > > (greg k-h).
> > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > - Used proper coding style for switch statement (greg k-h).
> > > > - Added comment about NULL argument (greg k-h).
> > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > is not enabled (Bryan Wu, greg k-h).
> > > >
> > > > Changes from v3:
> > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > conditioned block.
> > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > - Removed outdated comment (Alan Stern).
> > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > (Alan Stern).
> > > > - File rename moved to a separate commit (greg k-h).
> > > >
> > > > Changes from v2:
> > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > Balbi).
> > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > >
> > > > Changes from v1:
> > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > - Improved Kconfig help.
> > > > - Linked with other modules rather than being standalone modules.
> > > >
> > > > Michal Sojka (4):
> > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > usb: gadget: Refactor request completion
> > > > usb: Rename usb-common.c
> > > > usb: Add LED triggers for USB activity
> > >
> > > Greg, how do you want to handle this one as it touched both host and
> > > gadget ? I could wait until you apply this series to your usb-next and
> > > based my pull request on that. That'll work as long as it doesn't take
> > > too long for this to be merged.
> >
> > I can just take this now, for 3.18-rc1, you've already sent me your
> > patches for that release, so there shouldn't be any conflicts, right?
>
> that's better. Can you wait 1 day though ? I can run these through my
> build tests.
running my build tests on top of v3.17-rc6 + these 4 patches. I'll
report back tomorrow. From a code perspective, though, they look
alright.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
@ 2014-09-24 23:15 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-24 23:15 UTC (permalink / raw)
To: Felipe Balbi
Cc: Greg Kroah-Hartman, Michal Sojka, linux-usb, Alan Stern,
Bryan Wu, Linux LED Subsystem, linux-kernel, michal.vokac
[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]
On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > This adds LED triggers for USB host and device. First two patches
> > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > file and the last is the actual implementation of the LED triggers.
> > > >
> > > > Changes from v5:
> > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > Balbi)
> > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > >
> > > > Changes from v4:
> > > > - Added performance numbers to the commit message of the last patch
> > > > (greg k-h).
> > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > - Used proper coding style for switch statement (greg k-h).
> > > > - Added comment about NULL argument (greg k-h).
> > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > is not enabled (Bryan Wu, greg k-h).
> > > >
> > > > Changes from v3:
> > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > conditioned block.
> > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > - Removed outdated comment (Alan Stern).
> > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > (Alan Stern).
> > > > - File rename moved to a separate commit (greg k-h).
> > > >
> > > > Changes from v2:
> > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > Balbi).
> > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > >
> > > > Changes from v1:
> > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > - Improved Kconfig help.
> > > > - Linked with other modules rather than being standalone modules.
> > > >
> > > > Michal Sojka (4):
> > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > usb: gadget: Refactor request completion
> > > > usb: Rename usb-common.c
> > > > usb: Add LED triggers for USB activity
> > >
> > > Greg, how do you want to handle this one as it touched both host and
> > > gadget ? I could wait until you apply this series to your usb-next and
> > > based my pull request on that. That'll work as long as it doesn't take
> > > too long for this to be merged.
> >
> > I can just take this now, for 3.18-rc1, you've already sent me your
> > patches for that release, so there shouldn't be any conflicts, right?
>
> that's better. Can you wait 1 day though ? I can run these through my
> build tests.
running my build tests on top of v3.17-rc6 + these 4 patches. I'll
report back tomorrow. From a code perspective, though, they look
alright.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-24 23:15 ` Felipe Balbi
(?)
@ 2014-09-25 10:36 ` Greg Kroah-Hartman
2014-09-25 13:56 ` Felipe Balbi
-1 siblings, 1 reply; 57+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-25 10:36 UTC (permalink / raw)
To: Felipe Balbi
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > > This adds LED triggers for USB host and device. First two patches
> > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > > file and the last is the actual implementation of the LED triggers.
> > > > >
> > > > > Changes from v5:
> > > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > > Balbi)
> > > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > > >
> > > > > Changes from v4:
> > > > > - Added performance numbers to the commit message of the last patch
> > > > > (greg k-h).
> > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > > - Used proper coding style for switch statement (greg k-h).
> > > > > - Added comment about NULL argument (greg k-h).
> > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > > is not enabled (Bryan Wu, greg k-h).
> > > > >
> > > > > Changes from v3:
> > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > > conditioned block.
> > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > > - Removed outdated comment (Alan Stern).
> > > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > > (Alan Stern).
> > > > > - File rename moved to a separate commit (greg k-h).
> > > > >
> > > > > Changes from v2:
> > > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > > Balbi).
> > > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > > >
> > > > > Changes from v1:
> > > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > > - Improved Kconfig help.
> > > > > - Linked with other modules rather than being standalone modules.
> > > > >
> > > > > Michal Sojka (4):
> > > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > > usb: gadget: Refactor request completion
> > > > > usb: Rename usb-common.c
> > > > > usb: Add LED triggers for USB activity
> > > >
> > > > Greg, how do you want to handle this one as it touched both host and
> > > > gadget ? I could wait until you apply this series to your usb-next and
> > > > based my pull request on that. That'll work as long as it doesn't take
> > > > too long for this to be merged.
> > >
> > > I can just take this now, for 3.18-rc1, you've already sent me your
> > > patches for that release, so there shouldn't be any conflicts, right?
> >
> > that's better. Can you wait 1 day though ? I can run these through my
> > build tests.
>
> running my build tests on top of v3.17-rc6 + these 4 patches. I'll
> report back tomorrow. From a code perspective, though, they look
> alright.
Yes, will wait, thanks.
greg k-h
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-25 10:36 ` Greg Kroah-Hartman
@ 2014-09-25 13:56 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-25 13:56 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Felipe Balbi, Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3627 bytes --]
Hi,
On Thu, Sep 25, 2014 at 12:36:23PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > > > This adds LED triggers for USB host and device. First two patches
> > > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > > > file and the last is the actual implementation of the LED triggers.
> > > > > >
> > > > > > Changes from v5:
> > > > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > > > Balbi)
> > > > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > > > >
> > > > > > Changes from v4:
> > > > > > - Added performance numbers to the commit message of the last patch
> > > > > > (greg k-h).
> > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > > > - Used proper coding style for switch statement (greg k-h).
> > > > > > - Added comment about NULL argument (greg k-h).
> > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > > > is not enabled (Bryan Wu, greg k-h).
> > > > > >
> > > > > > Changes from v3:
> > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > > > conditioned block.
> > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > > > - Removed outdated comment (Alan Stern).
> > > > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > > > (Alan Stern).
> > > > > > - File rename moved to a separate commit (greg k-h).
> > > > > >
> > > > > > Changes from v2:
> > > > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > > > Balbi).
> > > > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > > > >
> > > > > > Changes from v1:
> > > > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > > > - Improved Kconfig help.
> > > > > > - Linked with other modules rather than being standalone modules.
> > > > > >
> > > > > > Michal Sojka (4):
> > > > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > > > usb: gadget: Refactor request completion
> > > > > > usb: Rename usb-common.c
> > > > > > usb: Add LED triggers for USB activity
> > > > >
> > > > > Greg, how do you want to handle this one as it touched both host and
> > > > > gadget ? I could wait until you apply this series to your usb-next and
> > > > > based my pull request on that. That'll work as long as it doesn't take
> > > > > too long for this to be merged.
> > > >
> > > > I can just take this now, for 3.18-rc1, you've already sent me your
> > > > patches for that release, so there shouldn't be any conflicts, right?
> > >
> > > that's better. Can you wait 1 day though ? I can run these through my
> > > build tests.
> >
> > running my build tests on top of v3.17-rc6 + these 4 patches. I'll
> > report back tomorrow. From a code perspective, though, they look
> > alright.
>
> Yes, will wait, thanks.
nothing interesting on my build tests. I have also tested the series
with my AM437x SK and I can see LED blinking with usb-gadget and
usb-host triggers.
For the whole series:
Tested-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
@ 2014-09-25 13:56 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-25 13:56 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Felipe Balbi, Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
[-- Attachment #1: Type: text/plain, Size: 3627 bytes --]
Hi,
On Thu, Sep 25, 2014 at 12:36:23PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > > > This adds LED triggers for USB host and device. First two patches
> > > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > > > file and the last is the actual implementation of the LED triggers.
> > > > > >
> > > > > > Changes from v5:
> > > > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > > > Balbi)
> > > > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > > > >
> > > > > > Changes from v4:
> > > > > > - Added performance numbers to the commit message of the last patch
> > > > > > (greg k-h).
> > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > > > - Used proper coding style for switch statement (greg k-h).
> > > > > > - Added comment about NULL argument (greg k-h).
> > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > > > is not enabled (Bryan Wu, greg k-h).
> > > > > >
> > > > > > Changes from v3:
> > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > > > conditioned block.
> > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > > > - Removed outdated comment (Alan Stern).
> > > > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > > > (Alan Stern).
> > > > > > - File rename moved to a separate commit (greg k-h).
> > > > > >
> > > > > > Changes from v2:
> > > > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > > > Balbi).
> > > > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > > > >
> > > > > > Changes from v1:
> > > > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > > > - Improved Kconfig help.
> > > > > > - Linked with other modules rather than being standalone modules.
> > > > > >
> > > > > > Michal Sojka (4):
> > > > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > > > usb: gadget: Refactor request completion
> > > > > > usb: Rename usb-common.c
> > > > > > usb: Add LED triggers for USB activity
> > > > >
> > > > > Greg, how do you want to handle this one as it touched both host and
> > > > > gadget ? I could wait until you apply this series to your usb-next and
> > > > > based my pull request on that. That'll work as long as it doesn't take
> > > > > too long for this to be merged.
> > > >
> > > > I can just take this now, for 3.18-rc1, you've already sent me your
> > > > patches for that release, so there shouldn't be any conflicts, right?
> > >
> > > that's better. Can you wait 1 day though ? I can run these through my
> > > build tests.
> >
> > running my build tests on top of v3.17-rc6 + these 4 patches. I'll
> > report back tomorrow. From a code perspective, though, they look
> > alright.
>
> Yes, will wait, thanks.
nothing interesting on my build tests. I have also tested the series
with my AM437x SK and I can see LED blinking with usb-gadget and
usb-host triggers.
For the whole series:
Tested-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 0/4] LED triggers for USB host and device
2014-09-25 13:56 ` Felipe Balbi
(?)
@ 2014-09-25 14:56 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 57+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-25 14:56 UTC (permalink / raw)
To: Felipe Balbi
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu,
Linux LED Subsystem, linux-kernel, michal.vokac
On Thu, Sep 25, 2014 at 08:56:32AM -0500, Felipe Balbi wrote:
> Hi,
>
> On Thu, Sep 25, 2014 at 12:36:23PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 24, 2014 at 06:15:50PM -0500, Felipe Balbi wrote:
> > > On Wed, Sep 24, 2014 at 05:18:30PM -0500, Felipe Balbi wrote:
> > > > On Wed, Sep 24, 2014 at 11:41:55PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Wed, Sep 24, 2014 at 03:59:36PM -0500, Felipe Balbi wrote:
> > > > > > On Wed, Sep 24, 2014 at 10:43:17PM +0200, Michal Sojka wrote:
> > > > > > > This adds LED triggers for USB host and device. First two patches
> > > > > > > refactor UDC drivers as requested by Felipe Balbi, the next renames a
> > > > > > > file and the last is the actual implementation of the LED triggers.
> > > > > > >
> > > > > > > Changes from v5:
> > > > > > > - Refactoring of USB gadget completion split into two patches (Filipe
> > > > > > > Balbi)
> > > > > > > - Removed check for non-NULL req->completion (Filipe Balbi)
> > > > > > >
> > > > > > > Changes from v4:
> > > > > > > - Added performance numbers to the commit message of the last patch
> > > > > > > (greg k-h).
> > > > > > > - Replaced BUG_ON with pr_err (Alan Stern, greg k-h).
> > > > > > > - Used proper coding style for switch statement (greg k-h).
> > > > > > > - Added comment about NULL argument (greg k-h).
> > > > > > > - EXPORT_SYMBOL changed to EXPORT_SYMBOL_GPL (greg k-h).
> > > > > > > - Both triggers are now registerd even if host or gagdet subsystem
> > > > > > > is not enabled (Bryan Wu, greg k-h).
> > > > > > >
> > > > > > > Changes from v3:
> > > > > > > - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA
> > > > > > > conditioned block.
> > > > > > > - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi).
> > > > > > > - Removed outdated comment (Alan Stern).
> > > > > > > - req->complete == NULL is now a bug. Previously, this was ignored
> > > > > > > (Alan Stern).
> > > > > > > - File rename moved to a separate commit (greg k-h).
> > > > > > >
> > > > > > > Changes from v2:
> > > > > > > - Host/gadget triggers merged to a single file in usb/common/ (Felipe
> > > > > > > Balbi).
> > > > > > > - UDC drivers refactored so that LED trigger works for all of them.
> > > > > > >
> > > > > > > Changes from v1:
> > > > > > > - Moved from drivers/leds/ to drivers/usb/.
> > > > > > > - Improved Kconfig help.
> > > > > > > - Linked with other modules rather than being standalone modules.
> > > > > > >
> > > > > > > Michal Sojka (4):
> > > > > > > usb: gadget: Introduce usb_gadget_giveback_request()
> > > > > > > usb: gadget: Refactor request completion
> > > > > > > usb: Rename usb-common.c
> > > > > > > usb: Add LED triggers for USB activity
> > > > > >
> > > > > > Greg, how do you want to handle this one as it touched both host and
> > > > > > gadget ? I could wait until you apply this series to your usb-next and
> > > > > > based my pull request on that. That'll work as long as it doesn't take
> > > > > > too long for this to be merged.
> > > > >
> > > > > I can just take this now, for 3.18-rc1, you've already sent me your
> > > > > patches for that release, so there shouldn't be any conflicts, right?
> > > >
> > > > that's better. Can you wait 1 day though ? I can run these through my
> > > > build tests.
> > >
> > > running my build tests on top of v3.17-rc6 + these 4 patches. I'll
> > > report back tomorrow. From a code perspective, though, they look
> > > alright.
> >
> > Yes, will wait, thanks.
>
> nothing interesting on my build tests. I have also tested the series
> with my AM437x SK and I can see LED blinking with usb-gadget and
> usb-host triggers.
>
> For the whole series:
>
> Tested-by: Felipe Balbi <balbi@ti.com>
Thanks for testing and letting me know, will queue it up now.
greg k-h
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 3/4] usb: Rename usb-common.c
2014-09-24 20:43 ` [PATCH v6 3/4] usb: Rename usb-common.c Michal Sojka
[not found] ` <1411591401-5874-4-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
@ 2014-09-25 15:03 ` Greg Kroah-Hartman
2014-09-25 15:48 ` project wide: git config entry for [diff] renames=true Joe Perches
1 sibling, 1 reply; 57+ messages in thread
From: Greg Kroah-Hartman @ 2014-09-25 15:03 UTC (permalink / raw)
To: Michal Sojka
Cc: linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Linux LED Subsystem, linux-kernel, michal.vokac
On Wed, Sep 24, 2014 at 10:43:20PM +0200, Michal Sojka wrote:
> In the next commit, we will want the usb-common module to be composed of
> two object files. Since Kbuild cannot "append" another object to an
> existing one, we need to rename usb-common.c to something
> else (common.c) and create usb-common.o by linking the wanted objects
> together. Currently, usb-common.o comprises only common.o.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> Acked-by: Felipe Balbi <balbi@ti.com>
> Tested-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/usb/common/Makefile | 4 +-
> drivers/usb/common/common.c | 144 ++++++++++++++++++++++++++++++++++++++++
> drivers/usb/common/usb-common.c | 144 ----------------------------------------
> 3 files changed, 147 insertions(+), 145 deletions(-)
> create mode 100644 drivers/usb/common/common.c
> delete mode 100644 drivers/usb/common/usb-common.c
In the future, please generate a git "move" diff, which makes it easier
to review, and prove that nothing really changed. It also helps if the
file is a bit different from what you diffed against, which in my case,
was true.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 57+ messages in thread
* project wide: git config entry for [diff] renames=true
2014-09-25 15:03 ` Greg Kroah-Hartman
@ 2014-09-25 15:48 ` Joe Perches
2014-09-25 18:00 ` Jeff King
0 siblings, 1 reply; 57+ messages in thread
From: Joe Perches @ 2014-09-25 15:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Linux LED Subsystem, linux-kernel, michal.vokac, git
On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote:
> In the future, please generate a git "move" diff, which makes it easier
> to review, and prove that nothing really changed. It also helps if the
> file is a bit different from what you diffed against, which in my case,
> was true.
Maybe it'd be possible to add
[diff]
renames = true
to the .git/config file.
but I don't find a mechanism to add anything to the
.git/config and have it be pulled.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
2014-09-25 15:48 ` project wide: git config entry for [diff] renames=true Joe Perches
@ 2014-09-25 18:00 ` Jeff King
2014-09-25 18:06 ` Joe Perches
[not found] ` <20140925180005.GA11755-AdEPDUrAXsQ@public.gmane.org>
0 siblings, 2 replies; 57+ messages in thread
From: Jeff King @ 2014-09-25 18:00 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Michal Sojka, linux-usb, Alan Stern,
Bryan Wu, Felipe Balbi, Linux LED Subsystem, linux-kernel,
michal.vokac, git
On Thu, Sep 25, 2014 at 08:48:31AM -0700, Joe Perches wrote:
> On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote:
>
> > In the future, please generate a git "move" diff, which makes it easier
> > to review, and prove that nothing really changed. It also helps if the
> > file is a bit different from what you diffed against, which in my case,
> > was true.
>
> Maybe it'd be possible to add
>
> [diff]
> renames = true
>
> to the .git/config file.
>
> but I don't find a mechanism to add anything to the
> .git/config and have it be pulled.
There is no such mechanism within git. We've resisted adding one because
of the danger of something like:
[diff]
external = rm -rf /
diff.renames is probably safe, but any config-sharing mechanism would
have to deal with either whitelisting, or providing some mechanism for
the puller to review changes before blindly following them.
-Peff
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
2014-09-25 18:00 ` Jeff King
@ 2014-09-25 18:06 ` Joe Perches
2014-09-25 18:43 ` Junio C Hamano
[not found] ` <20140925180005.GA11755-AdEPDUrAXsQ@public.gmane.org>
1 sibling, 1 reply; 57+ messages in thread
From: Joe Perches @ 2014-09-25 18:06 UTC (permalink / raw)
To: Jeff King
Cc: Greg Kroah-Hartman, Michal Sojka, linux-usb, Alan Stern,
Bryan Wu, Felipe Balbi, Linux LED Subsystem, linux-kernel,
michal.vokac, git
On Thu, 2014-09-25 at 14:00 -0400, Jeff King wrote:
> On Thu, Sep 25, 2014 at 08:48:31AM -0700, Joe Perches wrote:
>
> > On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote:
> >
> > > In the future, please generate a git "move" diff, which makes it easier
> > > to review, and prove that nothing really changed. It also helps if the
> > > file is a bit different from what you diffed against, which in my case,
> > > was true.
> >
> > Maybe it'd be possible to add
> >
> > [diff]
> > renames = true
> >
> > to the .git/config file.
> >
> > but I don't find a mechanism to add anything to the
> > .git/config and have it be pulled.
>
> There is no such mechanism within git. We've resisted adding one because
> of the danger of something like:
>
> [diff]
> external = rm -rf /
>
> diff.renames is probably safe, but any config-sharing mechanism would
> have to deal with either whitelisting, or providing some mechanism for
> the puller to review changes before blindly following them.
Another mechanism might be to add a repository
top level .gitconfig and add whatever to that.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
2014-09-25 18:06 ` Joe Perches
@ 2014-09-25 18:43 ` Junio C Hamano
0 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-09-25 18:43 UTC (permalink / raw)
To: Joe Perches
Cc: Jeff King, Greg Kroah-Hartman, Michal Sojka, linux-usb,
Alan Stern, Bryan Wu, Felipe Balbi, Linux LED Subsystem,
linux-kernel, michal.vokac, git
Joe Perches <joe@perches.com> writes:
> On Thu, 2014-09-25 at 14:00 -0400, Jeff King wrote:
> ...
>> diff.renames is probably safe, but any config-sharing mechanism would
>> have to deal with either whitelisting, or providing some mechanism for
>> the puller to review changes before blindly following them.
>
> Another mechanism might be to add a repository
> top level .gitconfig and add whatever to that.
That could be smaller half of an implementation detail of one of the
two possibilities Jeff mentioned i.e. "mechanism for the puller to
review changes before blindly following". It gives the transfer
part. You still need a new mechanism to make that file that is
tracked in the repository to be used as part of your configuration
variable set after letting the puller to review and approve.
A puller who blindly trust the project could use the "include"
mechanism from your .git/config to include a file with a well-known
name that is tracked by the project _without_ review or approval. I
doubt we would recommend that in an open source setting, though.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
2014-09-25 18:00 ` Jeff King
@ 2014-09-25 18:53 ` Junio C Hamano
[not found] ` <20140925180005.GA11755-AdEPDUrAXsQ@public.gmane.org>
1 sibling, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-09-25 18:53 UTC (permalink / raw)
To: Jeff King
Cc: Joe Perches, Greg Kroah-Hartman, Michal Sojka,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Alan Stern, Bryan Wu,
Felipe Balbi, Linux LED Subsystem,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, michal.vokac-veUE7cmDK2A,
git
Jeff King <peff-AdEPDUrAXsQ@public.gmane.org> writes:
> There is no such mechanism within git. We've resisted adding one because
> of the danger of something like:
>
> [diff]
> external = rm -rf /
>
> diff.renames is probably safe, but any config-sharing mechanism would
> have to deal with either whitelisting, or providing some mechanism for
> the puller to review changes before blindly following them.
It might be useful to add a "safe include" feature, perhaps? We
ship a small set of hardcoded default whitelist (diff.renames may be
included in there), and allow the user who do not want to be
affected to override it with
[include]
safe = !diff.renames
or even
[config]
safe = !*
at the same time allow them to add what we do not hardcode to it
using the same mechanism, e.g.
[config]
safe = merge.*
Then
[include]
safe
path = ../project.gitconfig
[include]
path = $HOME/.gitconfig-variant1
would only allow the variables include.safe deems safe to affect
us from the in-tree file, and use everything from my personal set in
my home directory.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
@ 2014-09-25 18:53 ` Junio C Hamano
0 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-09-25 18:53 UTC (permalink / raw)
To: Jeff King
Cc: Joe Perches, Greg Kroah-Hartman, Michal Sojka, linux-usb,
Alan Stern, Bryan Wu, Felipe Balbi, Linux LED Subsystem,
linux-kernel, michal.vokac, git
Jeff King <peff@peff.net> writes:
> There is no such mechanism within git. We've resisted adding one because
> of the danger of something like:
>
> [diff]
> external = rm -rf /
>
> diff.renames is probably safe, but any config-sharing mechanism would
> have to deal with either whitelisting, or providing some mechanism for
> the puller to review changes before blindly following them.
It might be useful to add a "safe include" feature, perhaps? We
ship a small set of hardcoded default whitelist (diff.renames may be
included in there), and allow the user who do not want to be
affected to override it with
[include]
safe = !diff.renames
or even
[config]
safe = !*
at the same time allow them to add what we do not hardcode to it
using the same mechanism, e.g.
[config]
safe = merge.*
Then
[include]
safe
path = ../project.gitconfig
[include]
path = $HOME/.gitconfig-variant1
would only allow the variables include.safe deems safe to affect
us from the in-tree file, and use everything from my personal set in
my home directory.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: project wide: git config entry for [diff] renames=true
2014-09-25 18:53 ` Junio C Hamano
(?)
@ 2014-09-25 18:55 ` Junio C Hamano
-1 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-09-25 18:55 UTC (permalink / raw)
To: Jeff King; +Cc: Joe Perches, git
Junio C Hamano <gitster-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
writes:
> or even
>
> [config]
> safe = !*
> ...
Gaah, I meant [include] in all places I spelled [config] in the
previous message.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
2014-09-24 23:14 ` Felipe Balbi
@ 2014-09-29 8:50 ` Robert Baldyga
2014-09-29 9:13 ` Michal Sojka
2014-09-29 14:05 ` Felipe Balbi
1 sibling, 2 replies; 57+ messages in thread
From: Robert Baldyga @ 2014-09-29 8:50 UTC (permalink / raw)
To: Michal Sojka, linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac
Hi,
On 09/24/2014 10:43 PM, Michal Sojka wrote:
> Use the recently introduced usb_gadget_giveback_request() in favor of
> direct invocation of the completion routine.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka <sojka@merica.cz>
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c | 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
> drivers/usb/gadget/udc/fotg210-udc.c | 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-----
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++----
> drivers/usb/gadget/udc/fusb300_udc.c | 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c | 8 ++------
> drivers/usb/gadget/udc/mv_udc_core.c | 8 ++------
> drivers/usb/gadget/udc/net2272.c | 2 +-
> drivers/usb/gadget/udc/net2280.c | 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c | 2 +-
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> 29 files changed, 41 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
> if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
> hwreq->req.length)
> hweptemp = hwep->ci->ep0in;
> - hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index ce6071d..ada5f30 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg,
> hs_req = ep->req;
> ep->req = NULL;
> list_del_init(&hs_req->queue);
> - hs_req->req.complete(&ep->ep,
> - &hs_req->req);
> + usb_gadget_giveback_request(&ep->ep,
> + &hs_req->req);
> }
>
> /* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg *hsotg,
>
> if (hs_req->req.complete) {
> spin_unlock(&hsotg->lock);
> - hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> + usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
> spin_lock(&hsotg->lock);
> }
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 490a6ca..45f1dfc 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
> req->request.length, status);
>
> spin_unlock(&dwc->lock);
> - req->request.complete(&dep->endpoint, &req->request);
> + usb_gadget_giveback_request(&dep->endpoint, &req->request);
> spin_lock(&dwc->lock);
> }
>
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
> index 41b062e..3b9d138 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -841,7 +841,7 @@ __acquires(ep->dev->lock)
> &req->req, req->req.length, ep->ep.name, sts);
>
> spin_unlock(&dev->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dev->lock);
> ep->halted = halted;
> }
> diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
> index cfd18bc..9968f53 100644
> --- a/drivers/usb/gadget/udc/at91_udc.c
> +++ b/drivers/usb/gadget/udc/at91_udc.c
> @@ -267,7 +267,7 @@ static void done(struct at91_ep *ep, struct at91_request *req, int status)
>
> ep->stopped = 1;
> spin_unlock(&udc->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&udc->lock);
> ep->stopped = stopped;
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index c9fe67e..1529926 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -463,7 +463,7 @@ static void receive_data(struct usba_ep *ep)
> list_del_init(&req->queue);
> usba_ep_writel(ep, CTL_DIS, USBA_RX_BK_RDY);
> spin_unlock(&udc->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&udc->lock);
> }
>
> @@ -495,7 +495,7 @@ request_complete(struct usba_ep *ep, struct usba_request *req, int status)
> ep->ep.name, req, req->req.status, req->req.actual);
>
> spin_unlock(&udc->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&udc->lock);
> }
>
> diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
> index e969eb8..2235b88 100644
> --- a/drivers/usb/gadget/udc/bcm63xx_udc.c
> +++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
> @@ -1088,7 +1088,7 @@ static int bcm63xx_ep_disable(struct usb_ep *ep)
> breq->req.status = -ESHUTDOWN;
>
> spin_unlock_irqrestore(&udc->lock, flags);
> - breq->req.complete(&iudma->bep->ep, &breq->req);
> + usb_gadget_giveback_request(&iudma->bep->ep, &breq->req);
> spin_lock_irqsave(&udc->lock, flags);
> }
> }
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 2b54955..81dc595 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -258,7 +258,7 @@ static void nuke(struct dummy *dum, struct dummy_ep *ep)
> req->req.status = -ESHUTDOWN;
>
> spin_unlock(&dum->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dum->lock);
> }
> }
> @@ -658,7 +658,7 @@ static int dummy_queue(struct usb_ep *_ep, struct usb_request *_req,
> spin_unlock(&dum->lock);
> _req->actual = _req->length;
> _req->status = 0;
> - _req->complete(_ep, _req);
> + usb_gadget_giveback_request(_ep, _req);
> spin_lock(&dum->lock);
> } else
> list_add_tail(&req->queue, &ep->queue);
> @@ -702,7 +702,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
> dev_dbg(udc_dev(dum),
> "dequeued req %p from %s, len %d buf %p\n",
> req, _ep->name, _req->length, _req->buf);
> - _req->complete(_ep, _req);
> + usb_gadget_giveback_request(_ep, _req);
> }
> local_irq_restore(flags);
> return retval;
> @@ -1385,7 +1385,7 @@ top:
> list_del_init(&req->queue);
>
> spin_unlock(&dum->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dum->lock);
>
> /* requests might have been unlinked... */
> @@ -1761,7 +1761,7 @@ restart:
> req);
>
> spin_unlock(&dum->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dum->lock);
> ep->already_seen = 0;
> goto restart;
> diff --git a/drivers/usb/gadget/udc/fotg210-udc.c b/drivers/usb/gadget/udc/fotg210-udc.c
> index e143d69..1d31592 100644
> --- a/drivers/usb/gadget/udc/fotg210-udc.c
> +++ b/drivers/usb/gadget/udc/fotg210-udc.c
> @@ -70,7 +70,7 @@ static void fotg210_done(struct fotg210_ep *ep, struct fotg210_request *req,
> req->req.status = status;
>
> spin_unlock(&ep->fotg210->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&ep->fotg210->lock);
>
> if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> index 7324308..dd18ea3 100644
> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
> ep->stopped = 1;
> spin_unlock(&udc->lock);
>
> - /* this complete() should a func implemented by gadget layer,
> - * eg fsg->bulk_in_complete() */
> - if (req->req.complete)
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
It looks like you have omitted if() statement. Are you sure that request
has set complete() callback?
Maybe it's worth to add this if inside usb_gadget_giveback_request()
function?
>
> spin_lock(&udc->lock);
>
> @@ -2728,4 +2725,3 @@ module_platform_driver(udc_driver);
> MODULE_DESCRIPTION(DRIVER_DESC);
> MODULE_AUTHOR(DRIVER_AUTHOR);
> MODULE_LICENSE("GPL");
> -
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index 75b23ea..c362079 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
> ep->stopped = 1;
>
> spin_unlock(&ep->udc->lock);
> - /* complete() is from gadget layer,
> - * eg fsg->bulk_in_complete() */
> - if (req->req.complete)
> - req->req.complete(&ep->ep, &req->req);
> +
> + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> spin_lock(&ep->udc->lock);
> ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
> index 5c5d1ad..8286df7 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -876,7 +876,7 @@ static void done(struct fusb300_ep *ep, struct fusb300_request *req,
> req->req.status = status;
>
> spin_unlock(&ep->fusb300->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&ep->fusb300->lock);
>
> if (ep->epnum) {
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index 6c85839..bf9c5ef 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -320,7 +320,7 @@ done(struct goku_ep *ep, struct goku_request *req, int status)
> /* don't modify queue heads during completion callback */
> ep->stopped = 1;
> spin_unlock(&dev->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dev->lock);
> ep->stopped = stopped;
> }
> diff --git a/drivers/usb/gadget/udc/gr_udc.c b/drivers/usb/gadget/udc/gr_udc.c
> index 08df5c4..924533b 100644
> --- a/drivers/usb/gadget/udc/gr_udc.c
> +++ b/drivers/usb/gadget/udc/gr_udc.c
> @@ -339,7 +339,7 @@ static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
> } else if (req->req.complete) {
> spin_unlock(&dev->lock);
>
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> spin_lock(&dev->lock);
> }
> diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
> index 1629ad7..feab0ba 100644
> --- a/drivers/usb/gadget/udc/lpc32xx_udc.c
> +++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
> @@ -1479,7 +1479,7 @@ static void done(struct lpc32xx_ep *ep, struct lpc32xx_request *req, int status)
>
> ep->req_pending = 0;
> spin_unlock(&udc->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&udc->lock);
> }
>
> diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
> index de88d33..8985656 100644
> --- a/drivers/usb/gadget/udc/m66592-udc.c
> +++ b/drivers/usb/gadget/udc/m66592-udc.c
> @@ -729,7 +729,7 @@ __acquires(m66592->lock)
> restart = 1;
>
> spin_unlock(&ep->m66592->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&ep->m66592->lock);
>
> if (restart) {
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
> index 1624871..046a1f8 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -222,12 +222,8 @@ void mv_u3d_done(struct mv_u3d_ep *ep, struct mv_u3d_req *req, int status)
> }
>
> spin_unlock(&ep->u3d->lock);
> - /*
> - * complete() is from gadget layer,
> - * eg fsg->bulk_in_complete()
> - */
> - if (req->req.complete)
> - req->req.complete(&ep->ep, &req->req);
> +
> + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> spin_lock(&ep->u3d->lock);
> }
> diff --git a/drivers/usb/gadget/udc/mv_udc_core.c b/drivers/usb/gadget/udc/mv_udc_core.c
> index 040fb16..3c5db80 100644
> --- a/drivers/usb/gadget/udc/mv_udc_core.c
> +++ b/drivers/usb/gadget/udc/mv_udc_core.c
> @@ -248,12 +248,8 @@ static void done(struct mv_ep *ep, struct mv_req *req, int status)
> ep->stopped = 1;
>
> spin_unlock(&ep->udc->lock);
> - /*
> - * complete() is from gadget layer,
> - * eg fsg->bulk_in_complete()
> - */
> - if (req->req.complete)
> - req->req.complete(&ep->ep, &req->req);
> +
> + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> spin_lock(&ep->udc->lock);
> ep->stopped = stopped;
> diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c
> index 059cfe5..84d7162 100644
> --- a/drivers/usb/gadget/udc/net2272.c
> +++ b/drivers/usb/gadget/udc/net2272.c
> @@ -394,7 +394,7 @@ net2272_done(struct net2272_ep *ep, struct net2272_request *req, int status)
> /* don't modify queue heads during completion callback */
> ep->stopped = 1;
> spin_unlock(&dev->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dev->lock);
> ep->stopped = stopped;
> }
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index 2e95715..8d13337 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -928,7 +928,7 @@ done(struct net2280_ep *ep, struct net2280_request *req, int status)
> /* don't modify queue heads during completion callback */
> ep->stopped = 1;
> spin_unlock(&dev->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dev->lock);
> ep->stopped = stopped;
> }
> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index e731373..dcdfea4 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -315,7 +315,7 @@ done(struct omap_ep *ep, struct omap_req *req, int status)
> /* don't modify queue heads during completion callback */
> ep->stopped = 1;
> spin_unlock(&ep->udc->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&ep->udc->lock);
> ep->stopped = stopped;
> }
> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index 460d953..ccbe3d4 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -1490,7 +1490,7 @@ static void complete_req(struct pch_udc_ep *ep, struct pch_udc_request *req,
> spin_unlock(&dev->lock);
> if (!ep->in)
> pch_udc_ep_clear_rrdy(ep);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&dev->lock);
> ep->halted = halted;
> }
> diff --git a/drivers/usb/gadget/udc/pxa25x_udc.c b/drivers/usb/gadget/udc/pxa25x_udc.c
> index 251e4d5..42f7eeb 100644
> --- a/drivers/usb/gadget/udc/pxa25x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa25x_udc.c
> @@ -347,7 +347,7 @@ static void done(struct pxa25x_ep *ep, struct pxa25x_request *req, int status)
>
> /* don't modify queue heads during completion callback */
> ep->stopped = 1;
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> ep->stopped = stopped;
> }
>
> diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
> index 597d39f..4868369 100644
> --- a/drivers/usb/gadget/udc/pxa27x_udc.c
> +++ b/drivers/usb/gadget/udc/pxa27x_udc.c
> @@ -758,7 +758,7 @@ static void req_done(struct pxa_ep *ep, struct pxa27x_request *req, int status,
> if (pflags)
> spin_unlock_irqrestore(&ep->lock, *pflags);
> local_irq_save(flags);
> - req->req.complete(&req->udc_usb_ep->usb_ep, &req->req);
> + usb_gadget_giveback_request(&req->udc_usb_ep->usb_ep, &req->req);
> local_irq_restore(flags);
> if (pflags)
> spin_lock_irqsave(&ep->lock, *pflags);
> diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
> index de2a871..ad7f71e 100644
> --- a/drivers/usb/gadget/udc/r8a66597-udc.c
> +++ b/drivers/usb/gadget/udc/r8a66597-udc.c
> @@ -925,7 +925,7 @@ __acquires(r8a66597->lock)
> sudmac_free_channel(ep->r8a66597, ep, req);
>
> spin_unlock(&ep->r8a66597->lock);
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> spin_lock(&ep->r8a66597->lock);
>
> if (restart) {
> diff --git a/drivers/usb/gadget/udc/s3c-hsudc.c b/drivers/usb/gadget/udc/s3c-hsudc.c
> index 10c6a12..dfbf557 100644
> --- a/drivers/usb/gadget/udc/s3c-hsudc.c
> +++ b/drivers/usb/gadget/udc/s3c-hsudc.c
> @@ -258,8 +258,7 @@ static void s3c_hsudc_complete_request(struct s3c_hsudc_ep *hsep,
>
> hsep->stopped = 1;
> spin_unlock(&hsudc->lock);
> - if (hsreq->req.complete != NULL)
> - hsreq->req.complete(&hsep->ep, &hsreq->req);
> + usb_gadget_giveback_request(&hsep->ep, &hsreq->req);
> spin_lock(&hsudc->lock);
> hsep->stopped = stopped;
> }
> diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
> index 357b58e..ff423d1 100644
> --- a/drivers/usb/gadget/udc/s3c2410_udc.c
> +++ b/drivers/usb/gadget/udc/s3c2410_udc.c
> @@ -272,7 +272,7 @@ static void s3c2410_udc_done(struct s3c2410_ep *ep,
> status = req->req.status;
>
> ep->halted = 1;
> - req->req.complete(&ep->ep, &req->req);
> + usb_gadget_giveback_request(&ep->ep, &req->req);
> ep->halted = halted;
> }
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index d4aa779..24c8c02 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -176,7 +176,7 @@ __acquires(ep->musb->lock)
> ep->end_point.name, request,
> req->request.actual, req->request.length,
> request->status);
> - req->request.complete(&req->ep->end_point, &req->request);
> + usb_gadget_giveback_request(&req->ep->end_point, &req->request);
> spin_lock(&musb->lock);
> ep->busy = busy;
> }
> diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
> index 04e6505..2d17c10 100644
> --- a/drivers/usb/renesas_usbhs/mod_gadget.c
> +++ b/drivers/usb/renesas_usbhs/mod_gadget.c
> @@ -129,7 +129,7 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
> dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
>
> ureq->req.status = status;
> - ureq->req.complete(&uep->ep, &ureq->req);
> + usb_gadget_giveback_request(&uep->ep, &ureq->req);
> }
>
> static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
>
Best regards,
Robert Baldyga
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
2014-09-29 8:50 ` Robert Baldyga
@ 2014-09-29 9:13 ` Michal Sojka
2014-09-29 14:05 ` Felipe Balbi
1 sibling, 0 replies; 57+ messages in thread
From: Michal Sojka @ 2014-09-29 9:13 UTC (permalink / raw)
To: Robert Baldyga, linux-usb
Cc: Alan Stern, Bryan Wu, Felipe Balbi, Greg Kroah-Hartman,
Linux LED Subsystem, linux-kernel, michal.vokac
On Mon, Sep 29 2014, Robert Baldyga wrote:
> Hi,
>
> On 09/24/2014 10:43 PM, Michal Sojka wrote:
>> Use the recently introduced usb_gadget_giveback_request() in favor of
>> direct invocation of the completion routine.
>>
>> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
>> call to usb_gadget_giveback_request(). This was compile-tested with all
>> ARM drivers enabled and runtime-tested for musb.
>>
>> Signed-off-by: Michal Sojka <sojka@merica.cz>
[...]
>> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
>> index 7324308..dd18ea3 100644
>> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
>> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
>> @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
>> ep->stopped = 1;
>> spin_unlock(&udc->lock);
>>
>> - /* this complete() should a func implemented by gadget layer,
>> - * eg fsg->bulk_in_complete() */
>> - if (req->req.complete)
>> - req->req.complete(&ep->ep, &req->req);
>> + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> It looks like you have omitted if() statement. Are you sure that request
> has set complete() callback?
Yes, see the rest of the thread. This was suggested by other reviewers.
-Michal
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
2014-09-29 8:50 ` Robert Baldyga
@ 2014-09-29 14:05 ` Felipe Balbi
2014-09-29 14:05 ` Felipe Balbi
1 sibling, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-29 14:05 UTC (permalink / raw)
To: Robert Baldyga
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]
Hi,
On Mon, Sep 29, 2014 at 10:50:02AM +0200, Robert Baldyga wrote:
> > diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> > index 7324308..dd18ea3 100644
> > --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> > +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> > @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
> > ep->stopped = 1;
> > spin_unlock(&udc->lock);
> >
> > - /* this complete() should a func implemented by gadget layer,
> > - * eg fsg->bulk_in_complete() */
> > - if (req->req.complete)
> > - req->req.complete(&ep->ep, &req->req);
> > + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> It looks like you have omitted if() statement. Are you sure that request
> has set complete() callback?
complete is mandatory. If a request comes without a complete, it
deserves to oops.
> Maybe it's worth to add this if inside usb_gadget_giveback_request()
> function?
nope, we want those cases to oops so we catch the broken gadget driver.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v6 2/4] usb: gadget: Refactor request completion
@ 2014-09-29 14:05 ` Felipe Balbi
0 siblings, 0 replies; 57+ messages in thread
From: Felipe Balbi @ 2014-09-29 14:05 UTC (permalink / raw)
To: Robert Baldyga
Cc: Michal Sojka, linux-usb, Alan Stern, Bryan Wu, Felipe Balbi,
Greg Kroah-Hartman, Linux LED Subsystem, linux-kernel,
michal.vokac
[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]
Hi,
On Mon, Sep 29, 2014 at 10:50:02AM +0200, Robert Baldyga wrote:
> > diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> > index 7324308..dd18ea3 100644
> > --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> > +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> > @@ -118,10 +118,7 @@ static void done(struct qe_ep *ep, struct qe_req *req, int status)
> > ep->stopped = 1;
> > spin_unlock(&udc->lock);
> >
> > - /* this complete() should a func implemented by gadget layer,
> > - * eg fsg->bulk_in_complete() */
> > - if (req->req.complete)
> > - req->req.complete(&ep->ep, &req->req);
> > + usb_gadget_giveback_request(&ep->ep, &req->req);
>
> It looks like you have omitted if() statement. Are you sure that request
> has set complete() callback?
complete is mandatory. If a request comes without a complete, it
deserves to oops.
> Maybe it's worth to add this if inside usb_gadget_giveback_request()
> function?
nope, we want those cases to oops so we catch the broken gadget driver.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* [RFC/PATCH 0/2] Introduce safe-include config feature
2014-09-25 18:53 ` Junio C Hamano
(?)
(?)
@ 2014-10-03 1:37 ` Rasmus Villemoes
2014-10-03 1:37 ` [RFC/PATCH 1/2] config: Add safe-include directive Rasmus Villemoes
2014-10-03 1:37 ` [RFC/PATCH 2/2] config: Add test of safe-include feature Rasmus Villemoes
-1 siblings, 2 replies; 57+ messages in thread
From: Rasmus Villemoes @ 2014-10-03 1:37 UTC (permalink / raw)
To: Junio C Hamano, git
Cc: Jeff King, Joe Perches, Greg Kroah-Hartman, Rasmus Villemoes
[trimming Ccs]
This is an attempt at implementing the suggested safe-include config
feature. It mostly has the semantics Junio suggested in the parent
post, but it does not directly extend the current include directive;
instead, it uses a separate safe-include directive. This is done so
that if a repository is used with both old and new versions of git,
the older versions will just silently ignore the safe-include, instead
of ignoring include.safe and then proceeding to processing "path =
../project.gitconfig".
Config variables are whitelisted using safe-include.whitelist; the
value is interpreted as a whitespace-separated list of, possibly
negated, patterns. Later patterns override earlier ones.
If the feature is deemed worthwhile and my approach is acceptable,
I'll go ahead and try to write some documentation. For now, there is
just a small test script.
Rasmus Villemoes (2):
config: Add safe-include directive
config: Add test of safe-include feature
config.c | 91 +++++++++++++++++++++++++++++++++++++--
t/t1309-config-safe-include.sh | 96 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 184 insertions(+), 3 deletions(-)
create mode 100755 t/t1309-config-safe-include.sh
--
2.0.4
^ permalink raw reply [flat|nested] 57+ messages in thread
* [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-03 1:37 ` [RFC/PATCH 0/2] Introduce safe-include config feature Rasmus Villemoes
@ 2014-10-03 1:37 ` Rasmus Villemoes
2014-10-03 5:27 ` Junio C Hamano
2014-10-03 1:37 ` [RFC/PATCH 2/2] config: Add test of safe-include feature Rasmus Villemoes
1 sibling, 1 reply; 57+ messages in thread
From: Rasmus Villemoes @ 2014-10-03 1:37 UTC (permalink / raw)
To: Junio C Hamano, git
Cc: Jeff King, Joe Perches, Greg Kroah-Hartman, Rasmus Villemoes
This adds a variant of the include directive, where only certain
config variables in the included files are honoured. The set of
honoured variables consists of those the user has mentioned in a
safe-include.whitelist directive, along with a small set of git.git
blessed ones.
This can, for example, be used by a project to supply a set of
suggested configuration variables, such as "diff.renames = true". The
project would provide these in e.g project.gitconfig, and the user then
has to explicitly opt-in by putting
[safe-include]
path = ../project.gitconfig
into .git/config, possibly preceding the path directive with a
whitelist directive.
The problem with simply using the ordinary include directive for this
purpose is that certain configuration variables (e.g. diff.external)
can allow arbitrary programs to be run.
Older versions of git do not understand the safe-include directives,
so they will effectively just ignore them.
Obviously, we must ignore safe-include.whitelist directives when we
are processing a safe-included file.
Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
---
config.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 88 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index a677eb6..764cda1 100644
--- a/config.c
+++ b/config.c
@@ -11,6 +11,7 @@
#include "quote.h"
#include "hashmap.h"
#include "string-list.h"
+#include "wildmatch.h"
struct config_source {
struct config_source *prev;
@@ -39,6 +40,79 @@ static struct config_source *cf;
static int zlib_compression_seen;
+struct safe_var {
+ struct safe_var *next;
+ const char *pattern;
+ int blacklisted;
+};
+
+static int safe_include_depth;
+static struct safe_var *safe_var_head;
+
+static const char *builtin_safe_patterns[] = {
+ "diff.renames",
+};
+
+static int config_name_is_safe(const char *var)
+{
+ struct safe_var *sv;
+ unsigned i;
+
+ for (sv = safe_var_head; sv; sv = sv->next) {
+ /* Handle malformed patterns? */
+ if (wildmatch(sv->pattern, var, WM_CASEFOLD, NULL) == WM_MATCH)
+ return !sv->blacklisted;
+ }
+ for (i = 0; i < ARRAY_SIZE(builtin_safe_patterns); ++i) {
+ if (wildmatch(builtin_safe_patterns[i], var, WM_CASEFOLD, NULL) == WM_MATCH)
+ return 1;
+ }
+
+ return 0;
+}
+
+static void config_add_safe_pattern(const char *p)
+{
+ struct safe_var *sv;
+ int blacklist = 0;
+
+ if (*p == '!') {
+ blacklist = 1;
+ ++p;
+ }
+ if (!*p)
+ return;
+ sv = xmalloc(sizeof(*sv));
+ sv->pattern = xstrdup(p);
+ sv->blacklisted = blacklist;
+ sv->next = safe_var_head;
+ safe_var_head = sv;
+}
+
+static void config_add_safe_names(const char *value)
+{
+ char *patterns = xstrdup(value);
+ char *p, *save;
+
+ /*
+ * This allows giving multiple patterns in a single line, e.g.
+ *
+ * whitelist = !* foo.bar squirrel.*
+ *
+ * to override the builtin list of safe vars and only declare
+ * foo.bar and the squirrel section safe. But it has the
+ * obvious drawback that one cannot match subsection names
+ * containing whitespace. The alternative is that the above
+ * would have to be written on three separate whitelist lines.
+ */
+ for (p = strtok_r(patterns, " \t", &save); p; p = strtok_r(NULL, " \t", &save)) {
+ config_add_safe_pattern(p);
+ }
+
+ free(patterns);
+}
+
+
/*
* Default config_set that contains key-value pairs from the usual set of config
* config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
@@ -142,12 +216,23 @@ int git_config_include(const char *var, const char *value, void *data)
* Pass along all values, including "include" directives; this makes it
* possible to query information on the includes themselves.
*/
- ret = inc->fn(var, value, inc->data);
- if (ret < 0)
- return ret;
+ if (safe_include_depth == 0 || config_name_is_safe(var)) {
+ ret = inc->fn(var, value, inc->data);
+ if (ret < 0)
+ return ret;
+ }
if (!strcmp(var, "include.path"))
ret = handle_path_include(value, inc);
+ else if (safe_include_depth == 0
+ && !strcmp(var, "safe-include.whitelist")) {
+ config_add_safe_names(value);
+ }
+ else if (!strcmp(var, "safe-include.path")) {
+ safe_include_depth++;
+ ret = handle_path_include(value, inc);
+ safe_include_depth--;
+ }
return ret;
}
--
2.0.4
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [RFC/PATCH 2/2] config: Add test of safe-include feature
2014-10-03 1:37 ` [RFC/PATCH 0/2] Introduce safe-include config feature Rasmus Villemoes
2014-10-03 1:37 ` [RFC/PATCH 1/2] config: Add safe-include directive Rasmus Villemoes
@ 2014-10-03 1:37 ` Rasmus Villemoes
1 sibling, 0 replies; 57+ messages in thread
From: Rasmus Villemoes @ 2014-10-03 1:37 UTC (permalink / raw)
To: Junio C Hamano, git
Cc: Jeff King, Joe Perches, Greg Kroah-Hartman, Rasmus Villemoes
This adds a script for testing various aspects of the safe-include feature.
Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>
---
t/t1309-config-safe-include.sh | 96 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
create mode 100755 t/t1309-config-safe-include.sh
diff --git a/t/t1309-config-safe-include.sh b/t/t1309-config-safe-include.sh
new file mode 100755
index 0000000..b8ccc94
--- /dev/null
+++ b/t/t1309-config-safe-include.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+test_description='test config file safe-include directives'
+. ./test-lib.sh
+
+
+test_expect_success 'blacklist by default' '
+ echo "[diff]external = badprog" >project &&
+ echo "[safe-include]path = project" >.gitconfig &&
+ test_must_fail git config diff.external
+'
+
+
+test_expect_success 'builtin safe rules' '
+ echo "[diff]renames = true" >project &&
+ echo "[safe-include]path = project" >.gitconfig &&
+ echo true >expect &&
+ git config diff.renames >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'user blacklist taking precedence' '
+ echo "[diff]renames = true" >project &&
+ cat >.gitconfig <<-\EOF &&
+ [diff]renames = false
+ [safe-include]whitelist = !diff.renames
+ [safe-include]path = project
+ EOF
+ echo false >expect &&
+ git config diff.renames >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'wildcard matching' '
+ cat >project <<-\EOF &&
+ [test]beer = true
+ [test]bar = true
+ [test]foo = true
+ EOF
+ cat >.gitconfig <<-\EOF &&
+ [safe-include]whitelist = test.b*r
+ [safe-include]path = project
+ EOF
+ printf "test.bar true\ntest.beer true\n" | sort >expect &&
+ git config --get-regexp "^test" | sort >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ignore whitelist directives in safe-included files' '
+ cat >project <<-\EOF &&
+ [safe-include]whitelist = *
+ [diff]external = badprog
+ EOF
+ echo "[safe-include]path = project" >.gitconfig &&
+ test_must_fail git config diff.external
+'
+
+test_expect_success 'multiple whitelist/blacklist patterns in one line' '
+ cat >.gitconfig <<-\EOF &&
+ [safe-include]whitelist = !* foo.bar squirrel.* !squirrel.xyz
+ [safe-include]path = project
+ EOF
+ cat >project <<-\EOF &&
+ [diff]renames = true
+ [foo]bar = bar
+ [squirrel]abc = abc
+ [squirrel]xyz = xyz
+ EOF
+ test_must_fail git config diff.renames &&
+ test_must_fail git config squirrel.xyz &&
+ echo bar >expect &&
+ git config foo.bar >actual &&
+ test_cmp expect actual
+ echo abc >expect &&
+ git config squirrel.abc >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'case insensitivity' '
+ cat >.gitconfig <<-\EOF &&
+ [safe-include]whitelist = Test.Abc test.xyz
+ [safe-include]path = project
+ EOF
+ cat >project <<-\EOF &&
+ [test]abc = abc
+ [TeST]XyZ = XyZ
+ EOF
+ echo abc >expect &&
+ git config test.abc >actual &&
+ test_cmp expect actual &&
+ echo XyZ >expect &&
+ git config test.xyz >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
2.0.4
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-03 1:37 ` [RFC/PATCH 1/2] config: Add safe-include directive Rasmus Villemoes
@ 2014-10-03 5:27 ` Junio C Hamano
2014-10-03 5:34 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-10-03 5:27 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Jeff King, Joe Perches, Greg Kroah-Hartman
On Thu, Oct 2, 2014 at 6:37 PM, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> This adds a variant of the include directive, where only certain
> config variables in the included files are honoured. The set of
> honoured variables consists of those the user has mentioned in a
> safe-include.whitelist directive, along with a small set of git.git
> blessed ones.
>
> This can, for example, be used by a project to supply a set of
> suggested configuration variables, such as "diff.renames = true". The
> project would provide these in e.g project.gitconfig, and the user then
> has to explicitly opt-in by putting
>
> [safe-include]
> path = ../project.gitconfig
>
> into .git/config, possibly preceding the path directive with a
> whitelist directive.
Good thinking to protect against accidental inclusion by older versions of Git.
Even though I did allude to ../project.gitconfig in the original message, I
think there should probably be an explicit syntax to name a path that is
relative to the root level of the working tree. People do funky things using
$GIT_DIR and $GIT_WORK_TREE to break the ".. relative to the config
file is the root level of the working tree" assumption, and also a repository
can have a regular file ".git" that points at the real location of the directory
that has "config" in it, in which case its parent directory is very unlikely to
be the root level of the working tree.
That syntax _could_ be just a relative path (e.g. project.gitconfig names
the file with that name at the top-level of the working tree), and if we are
to do so, we should forbid any relative path that escapes from the working
tree (e.g. ../project.gitconfig is forbidden, but down/down/../../.gitconfig
could be OK as it is the same as .gitconfig). For that matter, anything with
/./ and /../ in it can safely be forbidden without losing functionality.
The reason why I think it is sufficient to take a relative path as relative
to the working tree is primarily because I do not see a reason why we
would want to do a safer inclusion of anything inside $GIT_DIR (which
would be the natural interpretation if the relatigve path is taken as relative
to the including config file, in the same way as how the regular include
is processed). But I could be missing some other useful use cases.
And we can allow absolute path, e.g. /etc/gitconfig, of course, but I'd
prefer to at least initially forbid an absolute path, due to the same worries
I have against the "unset some variables defined in /etc/gitconfig" topic
we discussed earlier today in a separate thread.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-03 5:27 ` Junio C Hamano
@ 2014-10-03 5:34 ` Junio C Hamano
2014-10-03 18:52 ` Junio C Hamano
2014-10-06 9:28 ` Rasmus Villemoes
2 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-10-03 5:34 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Jeff King, Joe Perches, Greg Kroah-Hartman
On Thu, Oct 2, 2014 at 10:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> On Thu, Oct 2, 2014 at 6:37 PM, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
>> This adds a variant of the include directive, where only certain
>> config variables in the included files are honoured. The set of
>> honoured variables consists of those the user has mentioned in a
>> safe-include.whitelist directive, along with a small set of git.git
>> blessed ones.
Another design decision we would need to make is if it should be
allowed for a safe-included file to use safe-include directive to
include other files. Offhand I do not think of a reason we absolutely
need to support it, but there may be an interesting workflow enabled
if we did so. I dunno.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-03 5:27 ` Junio C Hamano
2014-10-03 5:34 ` Junio C Hamano
@ 2014-10-03 18:52 ` Junio C Hamano
2014-10-06 9:28 ` Rasmus Villemoes
2 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-10-03 18:52 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Jeff King, Joe Perches, Greg Kroah-Hartman
Junio C Hamano <gitster@pobox.com> writes:
> Even though I did allude to ../project.gitconfig in the original message, I
> think there should probably be an explicit syntax to name a path that is
> relative to the root level of the working tree. People do funky things using
> $GIT_DIR and $GIT_WORK_TREE to break the ".. relative to the config
> file is the root level of the working tree" assumption, and also a repository
> can have a regular file ".git" that points at the real location of the directory
> that has "config" in it, in which case its parent directory is very unlikely to
> be the root level of the working tree.
There is another reason why I suspect that it may make the resulting
system more useful if we had a way to explicitly mark the path you
used to safeInclude (by the way, we do not do dashes in names for
configuration by convention) as referring to something inside the
project's working tree. In a bare repository, we might want to grab
the blob at that path in HEAD (i.e. the project's primary branch)
and include its contents.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-03 5:27 ` Junio C Hamano
2014-10-03 5:34 ` Junio C Hamano
2014-10-03 18:52 ` Junio C Hamano
@ 2014-10-06 9:28 ` Rasmus Villemoes
2014-10-06 17:58 ` Junio C Hamano
2 siblings, 1 reply; 57+ messages in thread
From: Rasmus Villemoes @ 2014-10-06 9:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Joe Perches, Greg Kroah-Hartman
Junio C Hamano <gitster@pobox.com> wrote:
> (by the way, we do not do dashes in names for configuration by
> convention)
OK. Actually, I now think I'd prefer a subsection [include "safe"], but
I don't have any strong preferences regarding the names.
> That syntax _could_ be just a relative path (e.g. project.gitconfig names
> the file with that name at the top-level of the working tree), and if we are
> to do so, we should forbid any relative path that escapes from the working
> tree (e.g. ../project.gitconfig is forbidden, but down/down/../../.gitconfig
> could be OK as it is the same as .gitconfig). For that matter, anything with
> /./ and /../ in it can safely be forbidden without losing functionality.
I agree that it would be most useful to interpret relative paths as
being relative to the working tree. I'm not sure what would be gained by
checking for ./ and ../ components, a symlink could easily be used to
circumvent that.
> And we can allow absolute path, e.g. /etc/gitconfig, of course, but I'd
> prefer to at least initially forbid an absolute path, due to the same worries
> I have against the "unset some variables defined in /etc/gitconfig" topic
> we discussed earlier today in a separate thread.
One might (ab)use the feature to only use some settings from a global
file, e.g.
[include "safe"]
whitelist = !foo.*
path = ~/extra.gitconfig
But I'm fine with forbidding absolute paths until someone actually comes
with such a use case.
> Another design decision we would need to make is if it should be
> allowed for a safe-included file to use safe-include directive to
> include other files. Offhand I do not think of a reason we absolutely
> need to support it, but there may be an interesting workflow enabled
> if we did so. I dunno.
After one level of safe-include, any safe-include can also be done as a
normal include (but one may need to spell the path differently if the
two included files are not both at the top of the working tree). One
could imagine a project supplying lots of defaults and splitting those
into separate files, each included from a single project.gitconfig.
Anyway, my proposal allows nesting includes and safe-includes inside
safe-includes; forbidding it would just be a matter of adding a
safe_include_depth == 0 check in two places. (Then safe_include_depth
probably could/should be renamed in_safe_include.) I think I have a
slight preference to allowing nested includes, but if absolute paths are
forbidden for safe-includes, they should also be forbidden for
include-inside-safe-include.
Rasmus
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [RFC/PATCH 1/2] config: Add safe-include directive
2014-10-06 9:28 ` Rasmus Villemoes
@ 2014-10-06 17:58 ` Junio C Hamano
0 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2014-10-06 17:58 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, Jeff King, Joe Perches, Greg Kroah-Hartman
Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>
>> (by the way, we do not do dashes in names for configuration by
>> convention)
>
> OK. Actually, I now think I'd prefer a subsection [include "safe"], but
> I don't have any strong preferences regarding the names.
I think Peff mentioned something about having the second level
between include and path, so I'll defer it to him.
>> That syntax _could_ be just a relative path (e.g. project.gitconfig names
>> the file with that name at the top-level of the working tree), and if we are
>> to do so, we should forbid any relative path that escapes from the working
>> tree (e.g. ../project.gitconfig is forbidden, but down/down/../../.gitconfig
>> could be OK as it is the same as .gitconfig). For that matter, anything with
>> /./ and /../ in it can safely be forbidden without losing functionality.
>
> I agree that it would be most useful to interpret relative paths as
> being relative to the working tree. I'm not sure what would be gained by
> checking for ./ and ../ components, a symlink could easily be used to
> circumvent that.
If the "limit to the the working tree" is the reason to suggest a
relative path to be taken as relative to the working tree, which my
suggestion clearly was, the reader should be intelligent enough to
infer that an implementation working in that mode should make sure
symlinks and any other means do not step outside it.
And as you noticed that, you apparently are ;-)
> One might (ab)use the feature to only use some settings from a global
> file, e.g.
>
> [include "safe"]
> whitelist = !foo.*
> path = ~/extra.gitconfig
You do not have to write something you do not want to use in your
own ~/extra.gitconfig that is under your $HOME/, so I'd prefer to
explicitly forbidding such a use case at least in the beginning.
^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2014-10-06 17:58 UTC | newest]
Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-17 7:21 [PATCH v5 0/3] LED triggers for USB host and device Michal Sojka
2014-09-17 7:21 ` [PATCH v5 1/3] usb: gadget: Refactor request completion Michal Sojka
2014-09-17 15:28 ` Felipe Balbi
2014-09-17 15:28 ` Felipe Balbi
2014-09-23 8:09 ` Michal Sojka
2014-09-23 8:09 ` Michal Sojka
2014-09-24 14:48 ` Felipe Balbi
2014-09-24 14:48 ` Felipe Balbi
2014-09-24 15:08 ` Alan Stern
2014-09-24 15:08 ` Alan Stern
2014-09-24 20:43 ` [PATCH v6 0/4] LED triggers for USB host and device Michal Sojka
[not found] ` <1411591401-5874-1-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
2014-09-24 20:43 ` [PATCH v6 1/4] usb: gadget: Introduce usb_gadget_giveback_request() Michal Sojka
2014-09-24 20:43 ` Michal Sojka
[not found] ` <1411591401-5874-2-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
2014-09-24 21:00 ` Felipe Balbi
2014-09-24 21:00 ` Felipe Balbi
2014-09-24 20:59 ` [PATCH v6 0/4] LED triggers for USB host and device Felipe Balbi
2014-09-24 20:59 ` Felipe Balbi
2014-09-24 21:41 ` Greg Kroah-Hartman
[not found] ` <20140924214155.GA30689-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2014-09-24 22:18 ` Felipe Balbi
2014-09-24 22:18 ` Felipe Balbi
2014-09-24 23:15 ` Felipe Balbi
2014-09-24 23:15 ` Felipe Balbi
2014-09-25 10:36 ` Greg Kroah-Hartman
2014-09-25 13:56 ` Felipe Balbi
2014-09-25 13:56 ` Felipe Balbi
2014-09-25 14:56 ` Greg Kroah-Hartman
2014-09-24 20:43 ` [PATCH v6 2/4] usb: gadget: Refactor request completion Michal Sojka
2014-09-24 23:14 ` Felipe Balbi
2014-09-24 23:14 ` Felipe Balbi
2014-09-29 8:50 ` Robert Baldyga
2014-09-29 9:13 ` Michal Sojka
2014-09-29 14:05 ` Felipe Balbi
2014-09-29 14:05 ` Felipe Balbi
2014-09-24 20:43 ` [PATCH v6 3/4] usb: Rename usb-common.c Michal Sojka
[not found] ` <1411591401-5874-4-git-send-email-sojka-Knnw/vAvyUalVyrhU4qvOw@public.gmane.org>
2014-09-24 23:15 ` Felipe Balbi
2014-09-24 23:15 ` Felipe Balbi
2014-09-25 15:03 ` Greg Kroah-Hartman
2014-09-25 15:48 ` project wide: git config entry for [diff] renames=true Joe Perches
2014-09-25 18:00 ` Jeff King
2014-09-25 18:06 ` Joe Perches
2014-09-25 18:43 ` Junio C Hamano
[not found] ` <20140925180005.GA11755-AdEPDUrAXsQ@public.gmane.org>
2014-09-25 18:53 ` Junio C Hamano
2014-09-25 18:53 ` Junio C Hamano
2014-09-25 18:55 ` Junio C Hamano
2014-10-03 1:37 ` [RFC/PATCH 0/2] Introduce safe-include config feature Rasmus Villemoes
2014-10-03 1:37 ` [RFC/PATCH 1/2] config: Add safe-include directive Rasmus Villemoes
2014-10-03 5:27 ` Junio C Hamano
2014-10-03 5:34 ` Junio C Hamano
2014-10-03 18:52 ` Junio C Hamano
2014-10-06 9:28 ` Rasmus Villemoes
2014-10-06 17:58 ` Junio C Hamano
2014-10-03 1:37 ` [RFC/PATCH 2/2] config: Add test of safe-include feature Rasmus Villemoes
2014-09-24 20:43 ` [PATCH v6 4/4] usb: Add LED triggers for USB activity Michal Sojka
2014-09-24 20:56 ` Felipe Balbi
2014-09-24 20:56 ` Felipe Balbi
2014-09-17 7:21 ` [PATCH v5 2/3] usb: Rename usb-common.c Michal Sojka
2014-09-17 7:21 ` [PATCH v5 3/3] usb: Add LED triggers for USB activity Michal Sojka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.