lkml.org 
[lkml]   [2018]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC/RFT PATCH 1/6] uvcvideo: Refactor URB descriptors
Hi Kieran,

Just minor suggestions below:

On Wed, 3 Jan 2018, Kieran Bingham wrote:

> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> We currently store three separate arrays for each URB reference we hold.
>
> Objectify the data needed to track URBs into a single uvc_urb structure,
> allowing better object management and tracking of the URB.
>
> All accesses to the data pointers through stream, are converted to use a
> uvc_urb pointer for consistency.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_video.c | 46 ++++++++++++++++++++------------
> drivers/media/usb/uvc/uvcvideo.h | 18 ++++++++++---
> 2 files changed, 44 insertions(+), 20 deletions(-)

[snip]

> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 19e725e2bda5..4afa8ce13ea7 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -479,6 +479,20 @@ struct uvc_stats_stream {
> unsigned int max_sof; /* Maximum STC.SOF value */
> };
>
> +/**
> + * struct uvc_urb - URB context management structure
> + *
> + * @urb: described URB. Must be allocated with usb_alloc_urb()

Didn't you mean "describes?"

> + * @urb_buffer: memory storage for the URB
> + * @urb_dma: DMA coherent addressing for the urb_buffer

The whole struct describes URBs, so, I wouldn't repeat that in these two
field names, I'd just call them "buffer" and "dma." OTOH, later you add
more fields like "stream," which aren't per-URB, so, maybe you want to
keep these prefixes.

Thanks
Guennadi

> + */
> +struct uvc_urb {
> + struct urb *urb;
> +
> + char *urb_buffer;
> + dma_addr_t urb_dma;
> +};
> +
> struct uvc_streaming {
> struct list_head list;
> struct uvc_device *dev;
> @@ -521,9 +535,7 @@ struct uvc_streaming {
> __u32 max_payload_size;
> } bulk;
>
> - struct urb *urb[UVC_URBS];
> - char *urb_buffer[UVC_URBS];
> - dma_addr_t urb_dma[UVC_URBS];
> + struct uvc_urb uvc_urb[UVC_URBS];
> unsigned int urb_size;
>
> __u32 sequence;
> --
> git-series 0.9.1
>

\
 
 \ /
  Last update: 2018-01-04 19:26    [W:0.116 / U:0.624 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site