[PATCH 0/3] ALSA: bebob: remove workarounds for suppressed quirks

[PATCH 0/3] ALSA: bebob: remove workarounds for suppressed quirks

[Takashi Sakamoto]

Hi,

As a result to apply the commit 1bd1b3be8655 ("ALSA: bebob: perform
sequence replay for media clock recovery"), some quirks seem to be
suppressed.

This patchset removes workarounds for the suppressed quirks and includes
code cleanup.

Takashi Sakamoto (3):
  ALSA: bebob: dismiss sleep after breaking connections
  ALSA: bebob: delete workaround for protocol version 3
  ALSA: bebob: code refactoring for model-dependent quirks

 sound/firewire/bebob/bebob.c        | 41 ++++++++++++++++++++---------
 sound/firewire/bebob/bebob.h        | 10 ++++---
 sound/firewire/bebob/bebob_stream.c | 32 ++++++----------------
 3 files changed, 42 insertions(+), 41 deletions(-)

-- 
2.27.0

[PATCH 1/3] ALSA: bebob: dismiss sleep after breaking connections

[Takashi Sakamoto]

In a commit d3eabe939aee ("ALSA: bebob: expand sleep just after breaking
connections for protocol version 1"), a workaround was added for a quirk
of freeze in BeBoB protocol version 1. As long as seeing with sequence
replay for media clock recovery, the quirk disappears.

This commit removes the workaround.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob_stream.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 470c2b70cbfa..6d47c25654e6 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -401,12 +401,6 @@ static void break_both_connections(struct snd_bebob *bebob)
 {
 	cmp_connection_break(&bebob->in_conn);
 	cmp_connection_break(&bebob->out_conn);
-
-	// These models seem to be in transition state for a longer time. When
-	// accessing in the state, any transactions is corrupted. In the worst
-	// case, the device is going to reboot.
-	if (bebob->version < 2)
-		msleep(600);
 }
 
 static int start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
-- 
2.27.0

[PATCH 2/3] ALSA: bebob: delete workaround for protocol version 3

[Takashi Sakamoto]

In a commit c4d860a0d256 ("ALSA: bebob: loosen up severity of checking
continuity for BeBoB v3 quirk"), a workaround was added for the quirk in
BeBoB protocol version 3 against the discontinuity of data block counter.
As long as seeing with sequence replay for media clock recovery, such
quirk disappears.

This commit deletes the workaround.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c        | 7 -------
 sound/firewire/bebob/bebob.h        | 2 --
 sound/firewire/bebob/bebob_stream.c | 8 --------
 3 files changed, 17 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index e7dd112c31c5..25222cc27e43 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -75,7 +75,6 @@ name_device(struct snd_bebob *bebob)
 	u32 hw_id;
 	u32 data[2] = {0};
 	u32 revision;
-	u32 version;
 	int err;
 
 	/* get vendor name from root directory */
@@ -108,12 +107,6 @@ name_device(struct snd_bebob *bebob)
 	if (err < 0)
 		goto end;
 
-	err = snd_bebob_read_quad(bebob->unit, INFO_OFFSET_BEBOB_VERSION,
-				  &version);
-	if (err < 0)
-		goto end;
-	bebob->version = version;
-
 	strcpy(bebob->card->driver, "BeBoB");
 	strcpy(bebob->card->shortname, model);
 	strcpy(bebob->card->mixername, model);
diff --git a/sound/firewire/bebob/bebob.h b/sound/firewire/bebob/bebob.h
index edd93699ce1a..fc2b9b36159c 100644
--- a/sound/firewire/bebob/bebob.h
+++ b/sound/firewire/bebob/bebob.h
@@ -109,8 +109,6 @@ struct snd_bebob {
 	/* for M-Audio special devices */
 	void *maudio_special_quirk;
 
-	/* For BeBoB version quirk. */
-	unsigned int version;
 	bool discontinuity_quirk;
 
 	struct amdtp_domain domain;
diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 6d47c25654e6..02972b32e170 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -456,14 +456,6 @@ static int init_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 	}
 
 	if (stream == &bebob->tx_stream) {
-		// BeBoB v3 transfers packets with these qurks:
-		//  - In the beginning of streaming, the value of dbc is
-		//    incremented even if no data blocks are transferred.
-		//  - The value of dbc is reset suddenly.
-		if (bebob->version > 2)
-			bebob->tx_stream.flags |= CIP_EMPTY_HAS_WRONG_DBC |
-						  CIP_SKIP_DBC_ZERO_CHECK;
-
 		// At high sampling rate, M-Audio special firmware transmits
 		// empty packet with the value of dbc incremented by 8 but the
 		// others are valid to IEC 61883-1.
-- 
2.27.0

[PATCH 3/3] ALSA: bebob: code refactoring for model-dependent quirks

[Takashi Sakamoto]

This commit adds enumeration and structure member as code refactoring
regarding to model-dependent quirks.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c        | 34 ++++++++++++++++++++++++-----
 sound/firewire/bebob/bebob.h        |  8 +++++--
 sound/firewire/bebob/bebob_stream.c | 18 +++++++--------
 3 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 25222cc27e43..452317e53565 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -159,6 +159,30 @@ check_audiophile_booted(struct fw_unit *unit)
 	return strncmp(name, "FW Audiophile Bootloader", 24) != 0;
 }
 
+static int detect_quirks(struct snd_bebob *bebob, const struct ieee1394_device_id *entry)
+{
+	if (entry->vendor_id == VEN_MAUDIO1) {
+		switch (entry->model_id) {
+		case MODEL_MAUDIO_PROFIRELIGHTBRIDGE:
+			// M-Audio ProFire Lightbridge has a quirk to transfer packets with
+			// discontinuous cycle or data block counter in early stage of packet
+			// streaming. The cycle span from the first packet with event is variable.
+			bebob->quirks |= SND_BEBOB_QUIRK_INITIAL_DISCONTINUOUS_DBC;
+			break;
+		case MODEL_MAUDIO_FW1814:
+		case MODEL_MAUDIO_PROJECTMIX:
+			// At high sampling rate, M-Audio special firmware transmits empty packet
+			// with the value of dbc incremented by 8.
+			bebob->quirks |= SND_BEBOB_QUIRK_WRONG_DBC;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return 0;
+}
+
 static int bebob_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
 	unsigned int card_index;
@@ -219,6 +243,10 @@ static int bebob_probe(struct fw_unit *unit, const struct ieee1394_device_id *en
 	if (err < 0)
 		goto error;
 
+	err = detect_quirks(bebob, entry);
+	if (err < 0)
+		goto error;
+
 	if (bebob->spec == &maudio_special_spec) {
 		if (entry->model_id == MODEL_MAUDIO_FW1814)
 			err = snd_bebob_maudio_special_discover(bebob, true);
@@ -230,12 +258,6 @@ static int bebob_probe(struct fw_unit *unit, const struct ieee1394_device_id *en
 	if (err < 0)
 		goto error;
 
-	// M-Audio ProFire Lightbridge has a quirk to transfer packets with discontinuous cycle or
-	// data block counter in early stage of packet streaming. The cycle span from the first
-	// packet with event is variable.
-	if (entry->vendor_id == VEN_MAUDIO1 && entry->model_id == MODEL_MAUDIO_PROFIRELIGHTBRIDGE)
-		bebob->discontinuity_quirk = true;
-
 	err = snd_bebob_stream_init_duplex(bebob);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/bebob/bebob.h b/sound/firewire/bebob/bebob.h
index fc2b9b36159c..dff8e25c6ca3 100644
--- a/sound/firewire/bebob/bebob.h
+++ b/sound/firewire/bebob/bebob.h
@@ -75,6 +75,11 @@ struct snd_bebob_spec {
 	const struct snd_bebob_meter_spec *meter;
 };
 
+enum snd_bebob_quirk {
+	SND_BEBOB_QUIRK_INITIAL_DISCONTINUOUS_DBC,
+	SND_BEBOB_QUIRK_WRONG_DBC,
+};
+
 struct snd_bebob {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -84,6 +89,7 @@ struct snd_bebob {
 	spinlock_t lock;
 
 	const struct snd_bebob_spec *spec;
+	unsigned int quirks;	// Combination of snd_bebob_quirk enumerations.
 
 	unsigned int midi_input_ports;
 	unsigned int midi_output_ports;
@@ -109,8 +115,6 @@ struct snd_bebob {
 	/* for M-Audio special devices */
 	void *maudio_special_quirk;
 
-	bool discontinuity_quirk;
-
 	struct amdtp_domain domain;
 };
 
diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 02972b32e170..e3e23e42add3 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -430,6 +430,7 @@ static int start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 
 static int init_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 {
+	unsigned int flags = CIP_BLOCKING;
 	enum amdtp_stream_direction dir_stream;
 	struct cmp_connection *conn;
 	enum cmp_direction dir_conn;
@@ -445,24 +446,21 @@ static int init_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 		dir_conn = CMP_INPUT;
 	}
 
+	if (stream == &bebob->tx_stream) {
+		if (bebob->quirks & SND_BEBOB_QUIRK_WRONG_DBC)
+			flags |= CIP_EMPTY_HAS_WRONG_DBC;
+	}
+
 	err = cmp_connection_init(conn, bebob->unit, dir_conn, 0);
 	if (err < 0)
 		return err;
 
-	err = amdtp_am824_init(stream, bebob->unit, dir_stream, CIP_BLOCKING);
+	err = amdtp_am824_init(stream, bebob->unit, dir_stream, flags);
 	if (err < 0) {
 		cmp_connection_destroy(conn);
 		return err;
 	}
 
-	if (stream == &bebob->tx_stream) {
-		// At high sampling rate, M-Audio special firmware transmits
-		// empty packet with the value of dbc incremented by 8 but the
-		// others are valid to IEC 61883-1.
-		if (bebob->maudio_special_quirk)
-			bebob->tx_stream.flags |= CIP_EMPTY_HAS_WRONG_DBC;
-	}
-
 	return 0;
 }
 
@@ -630,7 +628,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 		if (err < 0)
 			goto error;
 
-		if (!bebob->discontinuity_quirk)
+		if (!(bebob->quirks & SND_BEBOB_QUIRK_INITIAL_DISCONTINUOUS_DBC))
 			tx_init_skip_cycles = 0;
 		else
 			tx_init_skip_cycles = 16000;
-- 
2.27.0

Re: [PATCH 0/3] ALSA: bebob: remove workarounds for suppressed quirks

[Takashi Iwai]

On Fri, 11 Jun 2021 05:50:00 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> As a result to apply the commit 1bd1b3be8655 ("ALSA: bebob: perform
> sequence replay for media clock recovery"), some quirks seem to be
> suppressed.
> 
> This patchset removes workarounds for the suppressed quirks and includes
> code cleanup.
> 
> Takashi Sakamoto (3):
>   ALSA: bebob: dismiss sleep after breaking connections
>   ALSA: bebob: delete workaround for protocol version 3
>   ALSA: bebob: code refactoring for model-dependent quirks

Applied all three patches now.  Thanks.


Takashi