Hi Minas,

在 2017年11月06日 17:28, Minas Harutyunyan 写道:
Hi,

On 11/6/2017 12:46 PM, William Wu wrote:
The actual_length in dwc2_hcd_urb structure is used
to indicate the total data length transferred so far,
but in dwc2_update_isoc_urb_state(), it just updates
the actual_length of isoc frame, and don't update the
urb actual_length at the same time, this will cause
device drivers working error which depend on the urb
actual_length.

we can easily find this issue if use an USB camera,
the userspace use libusb to get USB data from kernel
via devio driver.In usb devio driver, processcompl()
function will process urb complete and copy data to
userspace depending on urb actual_length.

Let's update the urb actual_length if the isoc frame
is valid.

Signed-off-by: William Wu <william...@rock-chips.com>
---
   drivers/usb/dwc2/hcd_intr.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 28a8210..01b1e13 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -580,6 +580,7 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
                frame_desc->status = 0;
                frame_desc->actual_length = dwc2_get_actual_xfer_length(hsotg,
                                        chan, chnum, qtd, halt_status, NULL);
+               urb->actual_length += frame_desc->actual_length;
                break;
        case DWC2_HC_XFER_FRAME_OVERRUN:
                urb->error_count++;
@@ -599,6 +600,7 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
                frame_desc->status = -EPROTO;
                frame_desc->actual_length = dwc2_get_actual_xfer_length(hsotg,
                                        chan, chnum, qtd, halt_status, NULL);
+               urb->actual_length += frame_desc->actual_length;
/* Skip whole frame */
                if (chan->qh->do_split &&

According urb description in usb.h urb->actual_length used for non-iso
transfers:

"@actual_length: This is read in non-iso completion functions, and ...

   * ISO transfer status is reported in the status and actual_length fields
   * of the iso_frame_desc array, ...."
Yes, urb->actual_length is used for non-iso transfers. And for ISO transfer, the most usb class drivers can only use iso frame status and actual_length to handle usb data,
like: kernel v4.4 drivers/media/usb/uvc/uvc_video.c

But the usb devio driver really need the urb actual_length in the processcompl() if
handle ISO transfer, just as I mentioned in the commit message.

And I also found the same issue on raspberrypi board:
https://github.com/raspberrypi/linux/issues/903

So do you think we need to fix the usb devio driver, rather than fix dwc2?
I think maybe we can use urb actual length for ISO transfer, it seems that
don't cause any side-effect.


Thanks,
Minas







--
吴良峰 William.Wu
福建省福州市铜盘路软件大道89号软件园A区21号楼
No.21 Building, A District, No.89,software Boulevard Fuzhou,Fujian, PRC
手机: 13685012275
座机: 0591-83991906-8520
邮件:w...@rock-chips.com


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to