Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1244570

Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done

Show all headers | View raw

Felipe Tonello wrote:
> On Fri, Oct 9, 2015 at 10:23 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>> Felipe Tonello wrote:
>>>               } 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);
>>> +                     /* Our transmit completed. Don't queue it again. */
>>> +                     free_ep_req(ep, req);
>>>                       return;
>>>               }
>>>               break;
>>
>> The ALSA framework will give you a notification _once_.  If the
>> resulting data is larger than midi->buflen, then you have to continue
>> sending packets.  This is exactly what the call to f_midi_transmit()
>> does.
>
> That's true. But what it will also happen is that f_midi_transmit()
> will potentially eat up data from an unrelated ALSA trigger.

The triggers are for some MIDI port of the _same_ USB endpoint, so
they're not unrelated.  f_midi_transmit() collects data from all ports
anyway; separating the data from different ports into different USB
packets would just needlessly introduce additional latency.

> In the end it is all fine, because the function will realise the
> request->len == 0 so it will free the request. But logically speaking
> it is incorrect and error prone.

It is _not_ incorrect if you realize that f_midi_transmit() applies to
the endpoint, not to any particular port.

>> (To decrease latency, it might be a good idea to queue multiple requests,
>> but you wouldn't want to continue piling up requests if the host isn't
>> listening.  sound/usb/midi.c just allocates a fixed number of requests,
>> and always reuses them.)
>
> I believe that is the best way to implement. Create multiple requests
> until the ALSA substreams buffer are empty and free the request on
> completion.

I believe a better way to implement this is to allocate a fixed number
of requests, and to always reuse them.

> The problem of having requests when host isn't listening will happen
> anyway because there is no way to know that until completion.

But if you have no upper limit on the number of queues requests, you
will eventually run out of (DMA) memory.


Regards,
Clemens
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v3 0/4] USB MIDI Gadget bug fixes and improvements Felipe Tonello <eu@felipetonello.com> - 2015-10-08 10:40 +0200
  Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Clemens Ladisch <clemens@ladisch.de> - 2015-10-09 11:30 +0200
    Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Felipe Balbi <balbi@ti.com> - 2015-10-09 23:00 +0200
      Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Clemens Ladisch <clemens@ladisch.de> - 2015-10-11 21:20 +0200
        Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Felipe Tonello <eu@felipetonello.com> - 2015-10-12 12:20 +0200
    Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Felipe Tonello <eu@felipetonello.com> - 2015-10-12 11:50 +0200
      Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Clemens Ladisch <clemens@ladisch.de> - 2015-10-12 12:20 +0200
        Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Felipe Tonello <eu@felipetonello.com> - 2015-10-12 14:00 +0200
          Re: [PATCH v3 1/4] usb: gadget: f_midi: free usb request when done Clemens Ladisch <clemens@ladisch.de> - 2015-10-12 14:20 +0200

csiph-web