Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752641AbeADSZP (ORCPT + 1 other); Thu, 4 Jan 2018 13:25:15 -0500 Received: from mout.gmx.net ([212.227.17.20]:64891 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752562AbeADSZN (ORCPT ); Thu, 4 Jan 2018 13:25:13 -0500 Date: Thu, 4 Jan 2018 19:24:04 +0100 (CET) From: Guennadi Liakhovetski X-X-Sender: lyakh@axis700.grange To: Kieran Bingham cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, Olivier BRAUN , kieran.bingham@ideasonboard.com, Mauro Carvalho Chehab , Jaejoong Kim , Baoyou Xie , Hans Verkuil , Aviv Greenberg , Nicolas Dufresne , Greg Kroah-Hartman , Daniel Patrick Johnson , Jim Lin Subject: Re: [RFC/RFT PATCH 1/6] uvcvideo: Refactor URB descriptors In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Provags-ID: V03:K0:Hzj1ZF8YjXDnXLE1mqztCDXenQOizWE6P7tdYT07oGmZhJ/ACuc gZ/UcTKefBIYmuh4U/fhco0psjCeasOvqoeVSX68zsQnz+2V6icGHR2iERO5d+fUkoZHNNZ jRha/5dgURH0YPuFAEF3RyQyuUXqvFVhRsanLjrKsUEYVCGXdkqRzNrXIyT0+gDtpLPlTmW SSsUlF8dhzqOMq7tZrWkQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:5EFSgAM0PB8=:7rMHE/4kFZyxWI8siLU5uS C0b2k+g6OT+CwnZrQv71DVyy5KMiGRGn3kYl3CeFQfhIG/exDX4x3ELhwAgXb7MYpZHwzq06x QpOVnedVHe/NJnQaET3z9zXwwVm0u62mP2F7S/IEnFhzJQLZ4HAxvPmf4m0bFpu4sOoF3g729 b850ZtIQOQ/r7lHoNk/3bQxPk/p/2YXrkWNy3R2JxLwRRt81xhleiSra97uzvwwrAx3wF9Y3Z uU2l4RNir8UpZIKRpD5YHx5cYhZdZ+MVPk82bzl7z2iCHSEHOdjUHVQyXo6VY83IUDr9PIHO/ I7kX1mNt+hlpoOEUjTuAmm7YDICnNoW9ndra3jmvVYrAyj1UbdOivtMRYB6uP8e/YdR37iFm5 W7JJ17iaUmfk3EX1HApCsPcK4+JApyz4Uc1S4IIRZju21zhHhtuatsnn8KWEa7Uu/tiC50kYD AkzpCJnPaPitaXqYuBrr1agdqDw2XQJ9/lCiY0D9bTCtFlbp0LVz+bZCpST44P0tTKTqDhIgf 7RPZjCpnU+USFZV6pgxeLeGyKumUjeTDWFnP1csqw+PTl8pNUE0iYdGB3BRqKrfQ2by5g1gFm eyUGTSmG75c5qdaqZVbfOKhr/hIPP+hS4zWdbheLoGu/G9NWnJM7ic3JsTSGxnuWRBpUe8MJm tvs6avIcAbsEU5k+/ks17tm/mcFMvH16LM+pcwgzTHvtMReEy0mZoIfu5UhMESh614eXlZVMU Sc0V3V6cN6cCuCtyRRHKqIr7gwMaHDd6QItGRKWNol2VaWB8y7qVwAwigNWKxewAwXRAey2IL FjTsTi8UJAnUIGP6nNrqnNE1Dlc73RSRl3EcuJjO3VZf+CjduE= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Hi Kieran, Just minor suggestions below: On Wed, 3 Jan 2018, Kieran Bingham wrote: > From: Kieran Bingham > > 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 > Reviewed-by: Laurent Pinchart > --- > 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 >