Re: [PATCH 2/3] media: stk1160: move transfer_buffer and urb to same struct 'stk1160_urb'

[Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>]

Hi Dafna,

On Tue, Jan 11, 2022 at 08:55:04AM +0200, Dafna Hirschfeld wrote:
> Instead of having two separated arrays, one for the urbs and
> one for their buffers, have one array of a struct containing both.
> In addition, the array is just 16 pointers, no need to dynamically
> allocate it.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/media/usb/stk1160/stk1160-v4l.c   |  2 +-
>  drivers/media/usb/stk1160/stk1160-video.c | 50 ++++++++---------------
>  drivers/media/usb/stk1160/stk1160.h       | 11 ++---
>  3 files changed, 23 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
> index 6a4eb616d516..a06030451db4 100644
> --- a/drivers/media/usb/stk1160/stk1160-v4l.c
> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c
> @@ -232,7 +232,7 @@ static int stk1160_start_streaming(struct stk1160 *dev)
>  
>  	/* submit urbs and enables IRQ */
>  	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
> -		rc = usb_submit_urb(dev->isoc_ctl.urb[i], GFP_KERNEL);
> +		rc = usb_submit_urb(dev->isoc_ctl.stk1160_urb[i].urb, GFP_KERNEL);

The "stk1160_urb" field is a member of struct stk1160_isoc_ctl,
so it's not really needed to name it "stk1160_urb", you could just
name it "urb_ctl" or something like that.

I think the result should be saner:

		rc = usb_submit_urb(dev->isoc_ctl.urb_ctl[i].urb,
				    GFP_KERNEL);

Also, please make sure you run some regression tests with this patch alone
applied (without 3/3). It's easy to make a mistake on this kind of cleanups.

Thanks,
Ezequiel