On Tue, Aug 26, 2014 at 1:38 PM, Daniel Mack <daniel@xxxxxxxxxx> wrote:
> On 08/25/2014 07:22 PM, Jassi Brar wrote:
>> On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack <zonque@xxxxxxxxx> wrote:
>
>>> @@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request *req)
>>>
>>> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>>> src = prm->dma_area + prm->hw_ptr;
>>> - req->actual = req->length;
>>> + req->length = req->actual = uac2->c_pktsize;
>>>
>> This doesn't seem right.
>> We asked req->length to be transmitted by the udc which shouldn't
>> return until that is done. So at this point setting 'length' doesn't
>> mean much. The original assignment to 'actual' is only because we want
>> to ignore any issue that caused the udc to transmit fewer bytes (we
>> drop that data).
>
> Looking at this again, setting req->length is in fact the right thing to
> do. We want to prepare a new packet of a specific length, so we have to
> let the udc driver know how much data is contains before we call
> usb_ep_queue() again.
>
I was coming from increasing usb requests to number of elements in the
pattern array. In which case we set length of each req just once in
afunc_set_alt() and leave iso_complete() *untouched*.
However, if we do want to keep using only 2 usb requests, then yes we
do have to reset the req->length for the next packet. But still that
should ideally be done at the end of iso_complete(). The
initialization of 2 requests' length in afunc_set_alt() should be done
to first 2 elements of pattern (though nothing observable changes).
> Note that this is for SNDRV_PCM_STREAM_PLAYBACK, so for the IN endpoint
> of the gadget. Reading your description again makes me believe you
> actually wanted to do that for the *capture* side, because this is were
> could possibly 'drop data', right?
>
By 'drop data' I meant we don't retry sending length-actual bytes.
Regards,
Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
