Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [alsa-devel] [PATCH 0/2] ALSA: firewire-tascam: fix to handle error code to read status of sampling clock
@ 2019-09-10 13:51 Takashi Sakamoto
  2019-09-10 13:51 ` [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock Takashi Sakamoto
  2019-09-10 13:51 ` [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry Takashi Sakamoto
  0 siblings, 2 replies; 5+ messages in thread
From: Takashi Sakamoto @ 2019-09-10 13:51 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel

Hi,

ALSA firewire-tascam driver has a bug not to check return value from
a call of snd_tscm_stream_get_clock() since its first commit. This
patchset fixes the bug.

In models of TASCAM FIreWire series, the status of sampling clock is
represented in 2 bytes in MSB of a certain register. These bits are
zero in intermediate state after configuring the sampling clock. This
behaviour sometimes takes ALSA PCM applications to receive error when
starting both playback/capture PCM substreams because the operation to
start one PCM substream can change the sampling clock. In this case,
next operation for another PCM substream possible fails.

This patchset adds check and retry for the above case. Although the
change is a bit intrusive for stable releases, it's required to avoid
superficial regression due to the additional error check.

Takashi Sakamoto (2):
  ALSA: firewire-tascam: handle error code when getting current source
    of clock
  ALSA: firewire-tascam: check intermediate state of clock status and
    retry

 sound/firewire/tascam/tascam-pcm.c    |  3 ++
 sound/firewire/tascam/tascam-stream.c | 42 ++++++++++++++++++---------
 2 files changed, 31 insertions(+), 14 deletions(-)

-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock
  2019-09-10 13:51 [alsa-devel] [PATCH 0/2] ALSA: firewire-tascam: fix to handle error code to read status of sampling clock Takashi Sakamoto
@ 2019-09-10 13:51 ` Takashi Sakamoto
  2019-09-10 14:17   ` Takashi Iwai
  2019-09-10 13:51 ` [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry Takashi Sakamoto
  1 sibling, 1 reply; 5+ messages in thread
From: Takashi Sakamoto @ 2019-09-10 13:51 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stable

The return value of snd_tscm_stream_get_clock() is ignored. This commit
checks the value and handle error.

Fixes: e453df44f0d6 ("ALSA: firewire-tascam: add PCM functionality")
Cc: <stable@vger.kernel.org> # v4.4+
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c
index b5ced5415e40..2377732caa52 100644
--- a/sound/firewire/tascam/tascam-pcm.c
+++ b/sound/firewire/tascam/tascam-pcm.c
@@ -56,6 +56,9 @@ static int pcm_open(struct snd_pcm_substream *substream)
 		goto err_locked;
 
 	err = snd_tscm_stream_get_clock(tscm, &clock);
+	if (err < 0)
+		goto err_locked;
+
 	if (clock != SND_TSCM_CLOCK_INTERNAL ||
 	    amdtp_stream_pcm_running(&tscm->rx_stream) ||
 	    amdtp_stream_pcm_running(&tscm->tx_stream)) {
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry
  2019-09-10 13:51 [alsa-devel] [PATCH 0/2] ALSA: firewire-tascam: fix to handle error code to read status of sampling clock Takashi Sakamoto
  2019-09-10 13:51 ` [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock Takashi Sakamoto
@ 2019-09-10 13:51 ` Takashi Sakamoto
  2019-09-10 14:17   ` Takashi Iwai
  1 sibling, 1 reply; 5+ messages in thread
From: Takashi Sakamoto @ 2019-09-10 13:51 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stable

2 bytes in MSB of register for clock status is zero during intermediate
state after changing status of sampling clock in models of TASCAM FireWire
series. The duration of this state differs depending on cases. During the
state, it's better to retry reading the register for current status of
the clock.

In current implementation, the intermediate state is checked only when
getting current sampling transmission frequency, then retry reading.
This care is required for the other operations to read the register.

This commit moves the codes of check and retry into helper function
commonly used for operations to read the register.

Fixes: e453df44f0d6 ("ALSA: firewire-tascam: add PCM functionality")
Cc: <stable@vger.kernel.org> # v4.4+
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-stream.c | 42 ++++++++++++++++++---------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c
index 9e2dc2fe3271..adf69a520b80 100644
--- a/sound/firewire/tascam/tascam-stream.c
+++ b/sound/firewire/tascam/tascam-stream.c
@@ -8,20 +8,37 @@
 #include <linux/delay.h>
 #include "tascam.h"
 
+#define CLOCK_STATUS_MASK      0xffff0000
+#define CLOCK_CONFIG_MASK      0x0000ffff
+
 #define CALLBACK_TIMEOUT 500
 
 static int get_clock(struct snd_tscm *tscm, u32 *data)
 {
+	int trial = 0;
 	__be32 reg;
 	int err;
 
-	err = snd_fw_transaction(tscm->unit, TCODE_READ_QUADLET_REQUEST,
-				 TSCM_ADDR_BASE + TSCM_OFFSET_CLOCK_STATUS,
-				 &reg, sizeof(reg), 0);
-	if (err >= 0)
+	while (trial++ < 5) {
+		err = snd_fw_transaction(tscm->unit, TCODE_READ_QUADLET_REQUEST,
+				TSCM_ADDR_BASE + TSCM_OFFSET_CLOCK_STATUS,
+				&reg, sizeof(reg), 0);
+		if (err < 0)
+			return err;
+
 		*data = be32_to_cpu(reg);
+		if (*data & CLOCK_STATUS_MASK)
+			break;
 
-	return err;
+		// In intermediate state after changing clock status.
+		msleep(50);
+	}
+
+	// Still in the intermediate state.
+	if (trial >= 5)
+		return -EAGAIN;
+
+	return 0;
 }
 
 static int set_clock(struct snd_tscm *tscm, unsigned int rate,
@@ -34,7 +51,7 @@ static int set_clock(struct snd_tscm *tscm, unsigned int rate,
 	err = get_clock(tscm, &data);
 	if (err < 0)
 		return err;
-	data &= 0x0000ffff;
+	data &= CLOCK_CONFIG_MASK;
 
 	if (rate > 0) {
 		data &= 0x000000ff;
@@ -79,17 +96,14 @@ static int set_clock(struct snd_tscm *tscm, unsigned int rate,
 
 int snd_tscm_stream_get_rate(struct snd_tscm *tscm, unsigned int *rate)
 {
-	u32 data = 0x0;
-	unsigned int trials = 0;
+	u32 data;
 	int err;
 
-	while (data == 0x0 || trials++ < 5) {
-		err = get_clock(tscm, &data);
-		if (err < 0)
-			return err;
+	err = get_clock(tscm, &data);
+	if (err < 0)
+		return err;
 
-		data = (data & 0xff000000) >> 24;
-	}
+	data = (data & 0xff000000) >> 24;
 
 	/* Check base rate. */
 	if ((data & 0x0f) == 0x01)
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock
  2019-09-10 13:51 ` [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock Takashi Sakamoto
@ 2019-09-10 14:17   ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2019-09-10 14:17 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens, stable

On Tue, 10 Sep 2019 15:51:51 +0200,
Takashi Sakamoto wrote:
> 
> The return value of snd_tscm_stream_get_clock() is ignored. This commit
> checks the value and handle error.
> 
> Fixes: e453df44f0d6 ("ALSA: firewire-tascam: add PCM functionality")
> Cc: <stable@vger.kernel.org> # v4.4+
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Applied, thanks.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry
  2019-09-10 13:51 ` [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry Takashi Sakamoto
@ 2019-09-10 14:17   ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2019-09-10 14:17 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens, stable

On Tue, 10 Sep 2019 15:51:52 +0200,
Takashi Sakamoto wrote:
> 
> 2 bytes in MSB of register for clock status is zero during intermediate
> state after changing status of sampling clock in models of TASCAM FireWire
> series. The duration of this state differs depending on cases. During the
> state, it's better to retry reading the register for current status of
> the clock.
> 
> In current implementation, the intermediate state is checked only when
> getting current sampling transmission frequency, then retry reading.
> This care is required for the other operations to read the register.
> 
> This commit moves the codes of check and retry into helper function
> commonly used for operations to read the register.
> 
> Fixes: e453df44f0d6 ("ALSA: firewire-tascam: add PCM functionality")
> Cc: <stable@vger.kernel.org> # v4.4+
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Applied, thanks.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10 13:51 [alsa-devel] [PATCH 0/2] ALSA: firewire-tascam: fix to handle error code to read status of sampling clock Takashi Sakamoto
2019-09-10 13:51 ` [alsa-devel] [PATCH 1/2] ALSA: firewire-tascam: handle error code when getting current source of clock Takashi Sakamoto
2019-09-10 14:17   ` Takashi Iwai
2019-09-10 13:51 ` [alsa-devel] [PATCH 2/2] ALSA: firewire-tascam: check intermediate state of clock status and retry Takashi Sakamoto
2019-09-10 14:17   ` Takashi Iwai

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org alsa-devel@archiver.kernel.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox