Re: Pioneer DJ DJM-900NXS2 support patches

[Takashi Iwai <tiwai@suse.de>]

On Wed, 29 Apr 2020 11:56:37 +0200,
Dmitry Panchenko | d-Systems wrote:
> 
> Pioneer DJ DJM-900NXS2 is a widely used DJ mixer with 2 audio USB
> interfaces. Both have a MIDI controller, 10 playback and 12 capture
> channels. Audio endpoints are vendor-specific and 3 files need to be
> patched. All playback and capture channels work fine with all
> supported sample rates (44.1k, 48k, 96k). Patches are attached.

Thanks for the patch.
The code changes look good in general.

Some nitpicking:

> --- /tmp/linux-5.4.0/sound/usb/pcm.c	2020-04-29 11:56:36.000000000 +0300
> +++ linux-5.4.0/sound/usb/pcm.c	2020-04-29 12:01:31.916954625 +0300
> @@ -355,6 +355,10 @@
>  	case USB_ID(0x0582, 0x01d8): /* BOSS Katana */
>  		/* BOSS Katana amplifiers do not need quirks */
>  		return 0;
> +	case USB_ID(0x2b73, 0x000a): /* Pioneer DJ DJM-900NXS2 */
> +		ep = 0x82;
> +		ifnum = 0;
> +		goto add_sync_ep_from_ifnum;

I'd put this entry before BOSS Katana, right after other
sync_ep_from_ifnum entries.

> --- /tmp/linux-5.4.0/sound/usb/quirks.c	2020-04-29 11:56:36.000000000 +0300
> +++ linux-5.4.0/sound/usb/quirks.c	2020-04-29 12:00:04.232521294 +0300
> @@ -1414,6 +1414,29 @@
>  	subs->pkt_offset_adj = (emu_samplerate_id >= EMU_QUIRK_SR_176400HZ) ? 4 : 0;
>  }
>  
> +
> +/* 
> + * Pioneer DJ DJM-900NXS2
> + * Device needs to know the sample rate when substream is started
> + */
> +static int pioneer_djm_set_format_quirk (struct snd_usb_substream *subs)
> +{
> +
> +    /* Convert sample rate to little endian */
> +	u8 sr[3];
> +	sr[0] = subs->cur_rate&0xff;
> +	sr[1] = (subs->cur_rate>>8)&0xff;
> +	sr[2] = (subs->cur_rate>>16)&0xff;

Some minor coding style issues (need space, remove space, use tab,
etc).  Please check once scripts/checkpatch.pl and try to fix the
errors.  The error in the quirk table format is known and can be left
as is, though.

> +	
> +	/* Configure device */
> +	usb_set_interface(subs->stream->chip->dev, 0, 1);

You can use subs->dev.

> +	snd_usb_ctl_msg(subs->stream->chip->dev,
> +	    usb_rcvctrlpipe(subs->stream->chip->dev, 0),
> +	    0x01, 0x22, 0x0100, 0x0082, &sr, 0x0003);
> +
> +	return 0;
> +}
> +
>  void snd_usb_set_format_quirk(struct snd_usb_substream *subs,
>  			      struct audioformat *fmt)
>  {
> @@ -1424,6 +1447,8 @@
>  	case USB_ID(0x041e, 0x3f19): /* E-Mu 0204 USB */
>  		set_format_emu_quirk(subs, fmt);
>  		break;
> +    case USB_ID(0x2b73, 0x000a): /* Pioneer DJ DJM-900NXS2 */
> +        pioneer_djm_set_format_quirk(subs);
break is missing here.

Could you address the problems above, and most importantly, give your
Signed-off-by line and resumit?  The SOB is mandatory for merging to
upstream.  See Documentation/process/submitting-patches.rst for
details.


thanks,

Takashi