Re: [PATCH 14/41] ALSA: usb-audio: Create endpoint objects at parsing phase - Pioneer DJ DJM-250MK2 stopped working

From: Takashi Iwai <tiwai@suse.de>
To: "František Kučera" <konference@frantovo.cz>
Cc: alsa-devel@alsa-project.org,
	Matwey Kornilov <matwey.kornilov@gmail.com>,
	Dylan Robinson <dylan_robinson@motu.com>,
	Keith Milner <kamilner@superlative.org>
Subject: Re: [PATCH 14/41] ALSA: usb-audio: Create endpoint objects at parsing phase - Pioneer DJ DJM-250MK2 stopped working
Date: Wed, 06 Jan 2021 10:03:07 +0100	[thread overview]
Message-ID: <s5ha6tm30ac.wl-tiwai@suse.de> (raw)
In-Reply-To: <153e3b6e-0481-1d2e-fd00-4a2391a8035b@frantovo.cz>

On Tue, 05 Jan 2021 23:27:14 +0100,
František Kučera wrote:
> 
> Dne 05. 01. 21 v 14:20 Takashi Iwai napsal(a):
> > The call of snd_usb_add_endpoint() is needed explicitly
> > now at the quirk itself.
> >
> > Below is the v2 patch.  Please give it a try.
> 
> I applied your v2 patch to f6e7a024bfe5 version and tested with DJM-250MK2. It seems working (playback, recording).
> 
> The original error messages (usb 1-3: Cannot find EP 0x1 to open, usb 1-3: Cannot find EP 0x82 to open) disappeared from the log. And new one occurred (Incompatible EP setup for 0x82 - repeats many times). However DJM-250MK2 works again.

OK, then could you try the v3 patch below?


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v3] ALSA: usb-audio: Fix the missing endpoints creations for
 quirks

The recent change in the endpoint management moved the endpoint object
creation from the stream open time to the parser of the audio
descriptor.  It works fine for the standard audio, but it overlooked
the other places that create audio streams via quirks
(QUIRK_AUDIO_FIXED_ENDPOINT) like the reported Pioneer devices;
those call snd_usb_add_audio_stream() manually, hence they miss the
endpoints, eventually resulting in the error at opening streams.
Moreover, now the sync EP setup was moved to the explicit call of
snd_usb_audioformat_set_sync_ep(), and this needs to be added for
those places, too.

This patch addresses those regressions for quirks.  It adds a local
helper function add_audio_stream_from_fixed_fmt(), which does the all
needed tasks, and replaces the calls of snd_usb_add_audio_stream()
with this new function.

Also, since the implicit feedback can be handled generically, drop
the implicit feedback quirk entries for those devices.
This fixes the missing sync_ep_idx entry, too.

Fixes: 54cb31901b83 ("ALSA: usb-audio: Create endpoint objects at parsing phase")
Reported-by: František Kučera <konference@frantovo.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/implicit.c |  2 --
 sound/usb/quirks.c   | 54 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c
index 931042a6a051..3e0d8a59dddb 100644
--- a/sound/usb/implicit.c
+++ b/sound/usb/implicit.c
@@ -58,8 +58,6 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = {
 	IMPLICIT_FB_FIXED_DEV(0x0499, 0x172f, 0x81, 2), /* Steinberg UR22C */
 	IMPLICIT_FB_FIXED_DEV(0x0d9a, 0x00df, 0x81, 2), /* RTX6001 */
 	IMPLICIT_FB_FIXED_DEV(0x22f0, 0x0006, 0x81, 3), /* Allen&Heath Qu-16 */
-	IMPLICIT_FB_FIXED_DEV(0x2b73, 0x000a, 0x82, 0), /* Pioneer DJ DJM-900NXS2 */
-	IMPLICIT_FB_FIXED_DEV(0x2b73, 0x0017, 0x82, 0), /* Pioneer DJ DJM-250MK2 */
 	IMPLICIT_FB_FIXED_DEV(0x1686, 0xf029, 0x82, 2), /* Zoom UAC-2 */
 	IMPLICIT_FB_FIXED_DEV(0x2466, 0x8003, 0x86, 2), /* Fractal Audio Axe-Fx II */
 	IMPLICIT_FB_FIXED_DEV(0x0499, 0x172a, 0x86, 2), /* Yamaha MODX */
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index e4a690bb4c99..b70e2ebc3e29 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -120,6 +120,40 @@ static int create_standard_audio_quirk(struct snd_usb_audio *chip,
 	return 0;
 }
 
+/* create the audio stream and the corresponding endpoints from the fixed
+ * audioformat object; this is used for quirks with the fixed EPs
+ */
+static int add_audio_stream_from_fixed_fmt(struct snd_usb_audio *chip,
+					   struct audioformat *fp)
+{
+	int stream, err;
+
+	stream = (fp->endpoint & USB_DIR_IN) ?
+		SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
+
+	snd_usb_audioformat_set_sync_ep(chip, fp);
+
+	err = snd_usb_add_audio_stream(chip, stream, fp);
+	if (err < 0)
+		return err;
+
+	err = snd_usb_add_endpoint(chip, fp->endpoint,
+				   SND_USB_ENDPOINT_TYPE_DATA);
+	if (err < 0)
+		return err;
+
+	if (fp->sync_ep) {
+		err = snd_usb_add_endpoint(chip, fp->sync_ep,
+					   fp->implicit_fb ?
+					   SND_USB_ENDPOINT_TYPE_DATA :
+					   SND_USB_ENDPOINT_TYPE_SYNC);
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+
 /*
  * create a stream for an endpoint/altsetting without proper descriptors
  */
@@ -131,8 +165,8 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
 	struct audioformat *fp;
 	struct usb_host_interface *alts;
 	struct usb_interface_descriptor *altsd;
-	int stream, err;
 	unsigned *rate_table = NULL;
+	int err;
 
 	fp = kmemdup(quirk->data, sizeof(*fp), GFP_KERNEL);
 	if (!fp)
@@ -153,11 +187,6 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
 		fp->rate_table = rate_table;
 	}
 
-	stream = (fp->endpoint & USB_DIR_IN)
-		? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
-	err = snd_usb_add_audio_stream(chip, stream, fp);
-	if (err < 0)
-		goto error;
 	if (fp->iface != get_iface_desc(&iface->altsetting[0])->bInterfaceNumber ||
 	    fp->altset_idx >= iface->num_altsetting) {
 		err = -EINVAL;
@@ -176,6 +205,13 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip,
 		fp->datainterval = snd_usb_parse_datainterval(chip, alts);
 	if (fp->maxpacksize == 0)
 		fp->maxpacksize = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize);
+	if (!fp->fmt_type)
+		fp->fmt_type = UAC_FORMAT_TYPE_I;
+
+	err = add_audio_stream_from_fixed_fmt(chip, fp);
+	if (err < 0)
+		goto error;
+
 	usb_set_interface(chip->dev, fp->iface, 0);
 	snd_usb_init_pitch(chip, fp);
 	snd_usb_init_sample_rate(chip, fp, fp->rate_max);
@@ -417,7 +453,7 @@ static int create_uaxx_quirk(struct snd_usb_audio *chip,
 	struct usb_host_interface *alts;
 	struct usb_interface_descriptor *altsd;
 	struct audioformat *fp;
-	int stream, err;
+	int err;
 
 	/* both PCM and MIDI interfaces have 2 or more altsettings */
 	if (iface->num_altsetting < 2)
@@ -482,9 +518,7 @@ static int create_uaxx_quirk(struct snd_usb_audio *chip,
 		return -ENXIO;
 	}
 
-	stream = (fp->endpoint & USB_DIR_IN)
-		? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK;
-	err = snd_usb_add_audio_stream(chip, stream, fp);
+	err = add_audio_stream_from_fixed_fmt(chip, fp);
 	if (err < 0) {
 		list_del(&fp->list); /* unlink for avoiding double-free */
 		kfree(fp);
-- 
2.26.2

