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@xxxxxxxxxxxxx>
> ---
> 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
