Re: [PATCH v6 3/3] usb: gadget: f_midi: pre-allocate IN requests
On 01/12/15 18:31, Felipe F. Tonello wrote:
> This patch introduces pre-allocation of IN endpoint USB requests. This
> improves on latency (requires no usb request allocation on transmit) and avoid
> several potential probles on allocating too many usb requests (which involves
> DMA pool allocation problems).
>
> This implementation also handles better multiple MIDI Gadget ports, always
> processing the last processed MIDI substream if the last USB request wasn't
> enought to handle the whole stream.
>
> Signed-off-by: Felipe F. Tonello
> ---
> drivers/usb/gadget/function/f_midi.c | 166
> +++
> drivers/usb/gadget/legacy/gmidi.c| 2 +-
> 2 files changed, 129 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c
> b/drivers/usb/gadget/function/f_midi.c
> index 79dc611..fb1fe96d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -88,6 +89,9 @@ struct f_midi {
> int index;
> char *id;
> unsigned int buflen, qlen;
> + /* This fifo is used as a buffer ring for pre-allocated IN usb_requests
> */
> + DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
> + unsigned int in_last_port;
> };
>
> static inline struct f_midi *func_to_midi(struct usb_function *f)
> @@ -95,7 +99,7 @@ static inline struct f_midi *func_to_midi(struct
> usb_function *f)
> return container_of(f, struct f_midi, func);
> }
>
> -static void f_midi_transmit(struct f_midi *midi, struct usb_request *req);
> +static void f_midi_transmit(struct f_midi *midi);
>
> DECLARE_UAC_AC_HEADER_DESCRIPTOR(1);
> DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1);
> @@ -253,7 +257,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request
> *req)
> } else if (ep == midi->in_ep) {
> /* Our transmit completed. See if there's more to go.
>* f_midi_transmit eats req, don't queue it again. */
> - f_midi_transmit(midi, req);
> + req->length = 0;
> + f_midi_transmit(midi);
> return;
> }
> break;
> @@ -264,10 +269,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request
> *req)
> case -ESHUTDOWN:/* disconnect from host */
> VDBG(cdev, "%s gone (%d), %d/%d\n", ep->name, status,
> req->actual, req->length);
> - if (ep == midi->out_ep)
> + if (ep == midi->out_ep) {
> f_midi_handle_out_data(ep, req);
> -
> - free_ep_req(ep, req);
> + /* We don't need to free IN requests because it's
> handled
> + * by the midi->in_req_fifo. */
> + free_ep_req(ep, req);
> + }
> return;
>
> case -EOVERFLOW:/* buffer overrun on read means that
> @@ -334,6 +341,20 @@ static int f_midi_set_alt(struct usb_function *f,
> unsigned intf, unsigned alt)
> if (err)
> return err;
>
> + /* pre-allocate write usb requests to use on f_midi_transmit. */
> + while (kfifo_avail(&midi->in_req_fifo)) {
> + struct usb_request *req =
> + midi_alloc_ep_req(midi->in_ep, midi->buflen);
> +
> + if (req == NULL)
> + return -ENOMEM;
> +
> + req->length = 0;
> + req->complete = f_midi_complete;
> +
> + kfifo_put(&midi->in_req_fifo, req);
> + }
> +
> /* allocate a bunch of read buffers and queue them all at once. */
> for (i = 0; i < midi->qlen && err == 0; i++) {
> struct usb_request *req =
> @@ -358,6 +379,7 @@ static void f_midi_disable(struct usb_function *f)
> {
> struct f_midi *midi = func_to_midi(f);
> struct usb_composite_dev *cdev = f->config->cdev;
> + struct usb_request *req = NULL;
>
> DBG(cdev, "disable\n");
>
> @@ -367,6 +389,10 @@ static void f_midi_disable(struct usb_function *f)
>*/
> usb_ep_disable(midi->in_ep);
> usb_ep_disable(midi->out_ep);
> +
> + /* release IN requests */
> + while (kfifo_get(&midi->in_req_fifo, &req))
> + free_ep_req(midi->in_ep, req);
> }
>
> static int f_midi_snd_free(struct snd_device *device)
> @@ -488,57 +514,113 @@ static void f_midi_transmit_byte(struct usb_request
> *req,
> }
> }
>
> -static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> +static void f_midi_drop_out_substreams(struct f_midi *midi)
> {
> - struct usb_ep *ep = midi->in_ep;
> - int i;
> -
> - if (!ep)
> - return;
> -
> - if (!req)
> - req = midi_alloc_ep_req(ep, midi->buflen);
> -
> - if (!req) {
> - ERROR(midi, "%s: alloc_ep_request failed\n"
[PATCH v6 3/3] usb: gadget: f_midi: pre-allocate IN requests
This patch introduces pre-allocation of IN endpoint USB requests. This
improves on latency (requires no usb request allocation on transmit) and avoid
several potential probles on allocating too many usb requests (which involves
DMA pool allocation problems).
This implementation also handles better multiple MIDI Gadget ports, always
processing the last processed MIDI substream if the last USB request wasn't
enought to handle the whole stream.
Signed-off-by: Felipe F. Tonello
---
drivers/usb/gadget/function/f_midi.c | 166 +++
drivers/usb/gadget/legacy/gmidi.c| 2 +-
2 files changed, 129 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c
b/drivers/usb/gadget/function/f_midi.c
index 79dc611..fb1fe96d 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
@@ -88,6 +89,9 @@ struct f_midi {
int index;
char *id;
unsigned int buflen, qlen;
+ /* This fifo is used as a buffer ring for pre-allocated IN usb_requests
*/
+ DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
+ unsigned int in_last_port;
};
static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -95,7 +99,7 @@ static inline struct f_midi *func_to_midi(struct usb_function
*f)
return container_of(f, struct f_midi, func);
}
-static void f_midi_transmit(struct f_midi *midi, struct usb_request *req);
+static void f_midi_transmit(struct f_midi *midi);
DECLARE_UAC_AC_HEADER_DESCRIPTOR(1);
DECLARE_USB_MIDI_OUT_JACK_DESCRIPTOR(1);
@@ -253,7 +257,8 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
} else if (ep == midi->in_ep) {
/* Our transmit completed. See if there's more to go.
* f_midi_transmit eats req, don't queue it again. */
- f_midi_transmit(midi, req);
+ req->length = 0;
+ f_midi_transmit(midi);
return;
}
break;
@@ -264,10 +269,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request
*req)
case -ESHUTDOWN:/* disconnect from host */
VDBG(cdev, "%s gone (%d), %d/%d\n", ep->name, status,
req->actual, req->length);
- if (ep == midi->out_ep)
+ if (ep == midi->out_ep) {
f_midi_handle_out_data(ep, req);
-
- free_ep_req(ep, req);
+ /* We don't need to free IN requests because it's
handled
+* by the midi->in_req_fifo. */
+ free_ep_req(ep, req);
+ }
return;
case -EOVERFLOW:/* buffer overrun on read means that
@@ -334,6 +341,20 @@ static int f_midi_set_alt(struct usb_function *f, unsigned
intf, unsigned alt)
if (err)
return err;
+ /* pre-allocate write usb requests to use on f_midi_transmit. */
+ while (kfifo_avail(&midi->in_req_fifo)) {
+ struct usb_request *req =
+ midi_alloc_ep_req(midi->in_ep, midi->buflen);
+
+ if (req == NULL)
+ return -ENOMEM;
+
+ req->length = 0;
+ req->complete = f_midi_complete;
+
+ kfifo_put(&midi->in_req_fifo, req);
+ }
+
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
@@ -358,6 +379,7 @@ static void f_midi_disable(struct usb_function *f)
{
struct f_midi *midi = func_to_midi(f);
struct usb_composite_dev *cdev = f->config->cdev;
+ struct usb_request *req = NULL;
DBG(cdev, "disable\n");
@@ -367,6 +389,10 @@ static void f_midi_disable(struct usb_function *f)
*/
usb_ep_disable(midi->in_ep);
usb_ep_disable(midi->out_ep);
+
+ /* release IN requests */
+ while (kfifo_get(&midi->in_req_fifo, &req))
+ free_ep_req(midi->in_ep, req);
}
static int f_midi_snd_free(struct snd_device *device)
@@ -488,57 +514,113 @@ static void f_midi_transmit_byte(struct usb_request *req,
}
}
-static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
+static void f_midi_drop_out_substreams(struct f_midi *midi)
{
- struct usb_ep *ep = midi->in_ep;
- int i;
-
- if (!ep)
- return;
-
- if (!req)
- req = midi_alloc_ep_req(ep, midi->buflen);
-
- if (!req) {
- ERROR(midi, "%s: alloc_ep_request failed\n", __func__);
- return;
- }
- req->length = 0;
- req->complete = f_midi_complete;
+ unsigned int i;
for (i = 0; i < MAX_PORTS; i++) {
st
