[alsa-devel] [PATCH 8/9] ALSA: usx2y: usbusx2yaudio: use usb_fill_int_urb()

[Takashi Iwai]

On Tue, 19 Jun 2018 23:55:20 +0200,
Sebastian Andrzej Siewior wrote:
> 
> Using usb_fill_int_urb() helps to find code which initializes an
> URB. A grep for members of the struct (like ->complete) reveal lots
> of other things, too.
> 
> The "&& !(*purb)->transfer_buffer" check has been removed because the
> URB has been freshly allocated a few lines above so ->transfer_buffer
> has to be NULL here.
> The `dev' and `transfer_size' assignments have been moved from
> usX2Y_urbs_start() to usX2Y_urbs_allocate() because they don't change
> overtime.
> 
> Cc: Jaroslav Kysela <perex at perex.cz>
> Cc: Takashi Iwai <tiwai at suse.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> ---
>  sound/usb/usx2y/usbusx2yaudio.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
> index 2b833054e3b0..9a49bdb07508 100644
> --- a/sound/usb/usx2y/usbusx2yaudio.c
> +++ b/sound/usb/usx2y/usbusx2yaudio.c
> @@ -425,6 +425,9 @@ static int usX2Y_urbs_allocate(struct snd_usX2Y_substream *subs)
>  	/* allocate and initialize data urbs */
>  	for (i = 0; i < NRURBS; i++) {
>  		struct urb **purb = subs->urb + i;
> +		void *buf = NULL;
> +		unsigned int len = 0;
> +
>  		if (*purb) {
>  			usb_kill_urb(*purb);
>  			continue;
> @@ -434,22 +437,18 @@ static int usX2Y_urbs_allocate(struct snd_usX2Y_substream *subs)
>  			usX2Y_urbs_release(subs);
>  			return -ENOMEM;
>  		}
> -		if (!is_playback && !(*purb)->transfer_buffer) {
> +		if (!is_playback) {
>  			/* allocate a capture buffer per urb */
> -			(*purb)->transfer_buffer =
> -				kmalloc_array(subs->maxpacksize,
> -					      nr_of_packs(), GFP_KERNEL);
> -			if (NULL == (*purb)->transfer_buffer) {
> +			len = subs->maxpacksize * nr_of_packs();
> +			buf = kmalloc(len, GFP_KERNEL);

I'd keep kmalloc_array() as is, and just put subs->maxpacksize *
nr_of_packs() in usb_fill_int_urb().  Otherwise it's a step backward.


thanks,

Takashi