On Tue, Sep 22, 2015 at 08:40:23PM +0200, Krzysztof Opasiak wrote:
> Since:
>
> commit e0857ce58e8658657f5f12fe25272b93cfeb16aa
this should be something like:
Since commit e0857ce58e86 (" .... ")
> ("usb: gadget: loopback: don't queue requests to bogus endpoints")
>
> Loopback function is not realy working as that commit removed
> all looping back logic. After that commit ep-out works like
> /dev/null and ep-in works like /dev/zero.
>
> This commit fix this issue by allocating set of out requests
> and set of in requests but each out req shares buffer with
> one in req:
>
> out_req->buf ---> buf <--- in_req.buf
> out_req->context <---> in_req.context
>
> The completion routine simply enqueue the suitable req in
> an oposite direction.
>
> Cc: <stable@xxxxxxxxxxxxxxx> # 3.18+
missing Fixes: e0857ce58e86 ("...")
I'll fix both while applying, but make sure to make this proper
next time.
> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
> ---
> Changes since v2:
> - fix requests context assignment
>
> Changes since v1:
> - add missing usb_ep_free_request() in complete() callback
> ---
> drivers/usb/gadget/function/f_loopback.c | 131 +++++++++++++++++++++---------
> 1 file changed, 92 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
> index e4bfed4..c369554 100644
> --- a/drivers/usb/gadget/function/f_loopback.c
> +++ b/drivers/usb/gadget/function/f_loopback.c
> @@ -245,22 +245,38 @@ static void loopback_complete(struct usb_ep *ep, struct usb_request *req)
> int status = req->status;
>
> switch (status) {
> -
> case 0: /* normal completion? */
> if (ep == loop->out_ep) {
> - req->zero = (req->actual < req->length);
> - req->length = req->actual;
> + /*
> + * We received some data from the host so let's
> + * queue it so host can read the from our in ep
> + */
> + struct usb_request *in_req = req->context;
> +
> + in_req->zero = (req->actual < req->length);
> + in_req->length = req->actual;
> + ep = loop->in_ep;
> + req = in_req;
> + } else {
> + /*
> + * We have just looped back a bunch of data
> + * to host. Now let's wait for some more data.
> + */
> + req = req->context;
> + ep = loop->out_ep;
> }
>
> - /* queue the buffer for some later OUT packet */
> - req->length = loop->buflen;
> + /* queue the buffer back to host or for next bunch of data */
> status = usb_ep_queue(ep, req, GFP_ATOMIC);
> - if (status == 0)
> + if (status == 0) {
> return;
> + } else {
> + ERROR(cdev, "Unable to loop back buffer to %s: %d\n",
> + ep->name, status);
> + goto free_req;
> + }
>
> /* "should never get here" */
> - /* FALLTHROUGH */
> -
> default:
> ERROR(cdev, "%s loop complete --> %d, %d/%d\n", ep->name,
> status, req->actual, req->length);
> @@ -274,6 +290,10 @@ static void loopback_complete(struct usb_ep *ep, struct usb_request *req)
> case -ECONNABORTED: /* hardware forced ep reset */
> case -ECONNRESET: /* request dequeued */
> case -ESHUTDOWN: /* disconnect from host */
> +free_req:
> + usb_ep_free_request(ep == loop->in_ep ?
> + loop->out_ep : loop->in_ep,
> + req->context);
> free_ep_req(ep, req);
> return;
> }
> @@ -295,50 +315,72 @@ static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
> return alloc_ep_req(ep, len, loop->buflen);
> }
>
> -static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback *loop,
> - struct usb_ep *ep)
> +static int alloc_requests(struct usb_composite_dev *cdev,
> + struct f_loopback *loop)
> {
> - struct usb_request *req;
> - unsigned i;
> - int result;
> -
> - /*
> - * one endpoint writes data back IN to the host while another endpoint
> - * just reads OUT packets
> - */
> - result = config_ep_by_speed(cdev->gadget, &(loop->function), ep);
> - if (result)
> - goto fail0;
> - result = usb_ep_enable(ep);
> - if (result < 0)
> - goto fail0;
> - ep->driver_data = loop;
> + struct usb_request *in_req, *out_req;
> + int i;
> + int result = 0;
>
> /*
> * allocate a bunch of read buffers and queue them all at once.
> - * we buffer at most 'qlen' transfers; fewer if any need more
> - * than 'buflen' bytes each.
> + * we buffer at most 'qlen' transfers; We allocate buffers only
> + * for out transfer and reuse them in IN transfers to implement
> + * our loopback functionality
> */
> for (i = 0; i < loop->qlen && result == 0; i++) {
> - req = lb_alloc_ep_req(ep, 0);
> - if (!req)
> - goto fail1;
> + result = -ENOMEM;
> +
> + in_req = usb_ep_alloc_request(loop->in_ep, GFP_KERNEL);
> + if (!in_req)
> + goto fail;
> +
> + out_req = lb_alloc_ep_req(loop->out_ep, 0);
> + if (!out_req)
> + goto fail_in;
> +
> + in_req->complete = loopback_complete;
> + out_req->complete = loopback_complete;
> +
> + in_req->buf = out_req->buf;
> + /* length will be set in complete routine */
> + in_req->context = out_req;
> + out_req->context = in_req;
>
> - req->complete = loopback_complete;
> - result = usb_ep_queue(ep, req, GFP_ATOMIC);
> + result = usb_ep_queue(loop->out_ep, out_req, GFP_ATOMIC);
> if (result) {
> ERROR(cdev, "%s queue req --> %d\n",
> - ep->name, result);
> - goto fail1;
> + loop->out_ep->name, result);
> + goto fail_out;
> }
> }
>
> return 0;
>
> -fail1:
> - usb_ep_disable(ep);
> +fail_out:
> + free_ep_req(loop->out_ep, out_req);
> +fail_in:
> + usb_ep_free_request(loop->in_ep, in_req);
> +fail:
> + return result;
> +}
> +
> +static int enable_endpoint(struct usb_composite_dev *cdev,
> + struct f_loopback *loop, struct usb_ep *ep)
> +{
> + int result;
> +
> + result = config_ep_by_speed(cdev->gadget, &(loop->function), ep);
> + if (result)
> + goto out;
>
> -fail0:
> + result = usb_ep_enable(ep);
> + if (result < 0)
> + goto out;
> + ep->driver_data = loop;
> + result = 0;
> +
> +out:
> return result;
> }
>
> @@ -349,13 +391,24 @@ enable_loopback(struct usb_composite_dev *cdev, struct f_loopback *loop)
>
> result = enable_endpoint(cdev, loop, loop->in_ep);
> if (result)
> - return result;
> + goto out;
>
> result = enable_endpoint(cdev, loop, loop->out_ep);
> if (result)
> - return result;
> + goto disable_in;
> +
> + result = alloc_requests(cdev, loop);
> + if (result)
> + goto disable_out;
>
> DBG(cdev, "%s enabled\n", loop->function.name);
> + return 0;
> +
> +disable_out:
> + usb_ep_disable(loop->out_ep);
> +disable_in:
> + usb_ep_disable(loop->in_ep);
> +out:
> return result;
> }
>
> --
> 1.7.9.5
>
--
balbi
Attachment:
signature.asc
Description: PGP signature