[PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[Felipe F. Tonello]

req->actual == req->length means that there is no data left to enqueue,
so free the request.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---

Changes in v2:
 * Re enqueue not fully completed requests, instead of read ALSA buffers.

 drivers/usb/gadget/function/f_midi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index edb84ca..62356cf 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -256,10 +256,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
 			/* We received stuff. req is queued again, below */
 			f_midi_handle_out_data(ep, 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);
-			return;
+			/* Our transmit completed. If there is no more to go,
+			   don't queue it again. */
+			if (req->actual == req->length) {
+				free_ep_req(ep, req);
+				return;
+			}
 		}
 		break;
 
-- 
2.1.4

Re: [PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[Sergei Shtylyov]

Hello.

On 9/23/2015 3:01 PM, Felipe F. Tonello wrote:

> req->actual == req->length means that there is no data left to enqueue,
> so free the request.
>
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
>
> Changes in v2:
>   * Re enqueue not fully completed requests, instead of read ALSA buffers.
>
>   drivers/usb/gadget/function/f_midi.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index edb84ca..62356cf 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -256,10 +256,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
>   			/* We received stuff. req is queued again, below */
>   			f_midi_handle_out_data(ep, 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);
> -			return;
> +			/* Our transmit completed. If there is no more to go,
> +			   don't queue it again. */

    The preferred multi-line comment style is this:

/*
  * bla
  * bla
  */

MBR, Sergei

Re: [PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[Peter Chen]

On Wed, Sep 23, 2015 at 01:01:44PM +0100, Felipe F. Tonello wrote:
> req->actual == req->length means that there is no data left to enqueue,
> so free the request.
> 
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> 
> Changes in v2:
>  * Re enqueue not fully completed requests, instead of read ALSA buffers.
> 
>  drivers/usb/gadget/function/f_midi.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index edb84ca..62356cf 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -256,10 +256,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
>  			/* We received stuff. req is queued again, below */
>  			f_midi_handle_out_data(ep, 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);
> -			return;
> +			/* Our transmit completed. If there is no more to go,
> +			   don't queue it again. */
> +			if (req->actual == req->length) {
> +				free_ep_req(ep, req);
> +				return;
> +			}

I find f_midi_transmit will judge if there are more data, if without
data, it will free the request.

Besides, does one trigger only do one transfer? Sorry, I can't make my
aplaymidi to receive data, probably due to hardware without midi
support?

root@imx6qpsabreauto:~# aplaymidi
ALSA lib
/var/lib/jenkins/workspace/fido-3.14.28-X11-consolidated_new/temp_build_dir/build_all/tmp/work/cortexa9hf-vfp-neon-mx6qdl-poky-linux-gnueabi/alsa-lib/1.0.28-r0/alsa-lib-1.0.28/src/seq/seq_hw.c:457:(snd_seq_hw_open)
open /dev/snd/seq failed: No such file or directory
Cannot open sequencer - No such file or directory

-- 

Best Regards,
Peter Chen

Re: [PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[Clemens Ladisch]

Peter Chen wrote:
> I can't make my aplaymidi to receive data

> # aplaymidi
> open /dev/snd/seq failed: No such file or directory

modprobe snd-seq

There are mechanisms to load it automatically, but your embedded system
might not bother about any of them.  Or CONFIG_SND_SEQUENCER isn't
enabled at all.

In any case, raw MIDI (with the amidi tool) should work.


Regards,
Clemens

Re: [PATCH v2 2/3] usb: gadget: f_midi: free usb request when done

[Felipe Tonello]

Hi Peter,

On Thu, Sep 24, 2015 at 2:38 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Wed, Sep 23, 2015 at 01:01:44PM +0100, Felipe F. Tonello wrote:
>> req->actual == req->length means that there is no data left to enqueue,
>> so free the request.
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>> ---
>>
>> Changes in v2:
>>  * Re enqueue not fully completed requests, instead of read ALSA buffers.
>>
>>  drivers/usb/gadget/function/f_midi.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
>> index edb84ca..62356cf 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -256,10 +256,12 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
>>                       /* We received stuff. req is queued again, below */
>>                       f_midi_handle_out_data(ep, 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);
>> -                     return;
>> +                     /* Our transmit completed. If there is no more to go,
>> +                        don't queue it again. */
>> +                     if (req->actual == req->length) {
>> +                             free_ep_req(ep, req);
>> +                             return;
>> +                     }
>
> I find f_midi_transmit will judge if there are more data, if without
> data, it will free the request.

No. Transmit is called when ALSA trigger is called (whenever
snd_*_read() snd_*_write() is called).

The current code is not crashing anything, but I believe it is just
wrong. It can cause potential problems in future if one is not careful
when doing changes to it.

I would like to send a v3 with free_ep_req().

Felipe

>
> Besides, does one trigger only do one transfer? Sorry, I can't make my
> aplaymidi to receive data, probably due to hardware without midi
> support?
>
> root@imx6qpsabreauto:~# aplaymidi
> ALSA lib
> /var/lib/jenkins/workspace/fido-3.14.28-X11-consolidated_new/temp_build_dir/build_all/tmp/work/cortexa9hf-vfp-neon-mx6qdl-poky-linux-gnueabi/alsa-lib/1.0.28-r0/alsa-lib-1.0.28/src/seq/seq_hw.c:457:(snd_seq_hw_open)
> open /dev/snd/seq failed: No such file or directory
> Cannot open sequencer - No such file or directory
>
> --
>
> Best Regards,
> Peter Chen