[RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming

[RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming

[Takashi Sakamoto]

This patchset updates my previous one:
[alsa-devel] [RFC][PATCH 0/8] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-November/100670.html

I hope more users to test this patchset in this developing cycle.

[Background]
I've frequently got private messages from owners of dice-based models
since below patchset was merged to mainline:

[alsa-devel] [PATCH 00/15 v5] ALSA: Enhancement for existed FireWire drivers
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-December/085142.html

Some of them addressed that ALSA dice driver fails to handle their
models, while their experiences are not the same. The summary:
 * ETIMEOUT at starting streaming
 * detecting packet discontinuity at starting streaming
 * ETIMEOUT at probing snd-dice
 * no sound even if starting streaming

This patchset is my solutions for these issues. It consists of four parts;

[Patch 01]
ETIMEDOUT at starting streaming means that ALSA dice driver has never received
dice notification within current timeout (200 msec).
This patch expands the timeout up to 2 sec. There's some disadvantages of this
change in usage of ALSA PCM API, see commit comment.

[Patch 02]
Packet discontinuity at starting streaming means that the device fail to get
constant clock signals from phase lock loop (PLL) circuit. In this case, ALSA
dice driver successfully starts streaming and receives AMDTP packet from the
device, therefore any dice notifications are already transferred. It's
reasonable to judge that the device has a quirk to transfer the notification
before set its PLL circuit steadily.
This patch continue to check a register so that the device get phase lock after
receiving dice notification.

[Patch 03-04]
When connected to IEEE 1394 bus, some models generate bus reset after appearing
on bus topology. Usually, boot processing of Dice based models consists of
three stages; boot loader (RedBoot) is loaded, it loads eCos, the operating
system executes vendor's configurations for hardware initialization. It's
reasonable to assume that bus reset occurs at each of these stages.
Furthermore, I observed that ImpactTwin (TC Electronic produced) changes some
registers before/after ALSA dice driver is probed with my RFC v1. This means
that the hardware initialization requires a bit time.
These patches postpone unit probe processing after continuous bus resets. These
patches have advantages to start communication to the device after the hardware
initialization. These patches may improve handling Dice II based models such as
ImpactTwin and IO26 (Alesis produces).

[Patch 05-10]
When seeing sound/firewire/dice/dice-interface.h, Dice interface gives no ways
for drivers to get all formats of AMDTP stream. Current ALSA dice driver manage
to generate 'format cache' at unit probing processing, while this requires to
change the state of actual clock on devices. This idea is not good because of
ignoring hardware design. This idea is not good in a case that users save their
favorite configuration to the device. Furthermore, this is not also good in a
case that the stream format is not decided according to sampling transfer
frequency only, i.e. data format of digital interface is also dominant to the
stream format.
These patches add limitation to constrain PCM substream at current sampling
transfer frequency. As a result, userspace applications are allowed to request
according to current sampling transfer frequency. When users change the
frequency, they should use userspace applications with fw character devices to
set it in advance. ALSA dice driver gives no way to achieve it in describes
reasons. Although ALSA dice driver goes backward with these patchset, we don't
beat any actual hardware designs.


[Changes from RFC v1]
 * Patch 03-04 are newly added. This enables to handle bus reset at unit probe
   processing, and drops one of reasons to constrain PCM substreams at current
   sampling transfer frequency.
 * Minor arrangements

Takashi Sakamoto (10):
  ALSA: dice: expand timeout to wait for Dice notification
  ALSA: dice: wait for ensuring phase lock
  ALSA: dice: split subaddress check from category check
  ALSA: dice: postpone probe processing
  ALSA: dice: limit to current sampling transfer frequency
  ALSA: dice: limit stream to current sampling transfer frequency.
  ALSA: dice: add MIDI ports according to current number of MIDI
    substreams
  ALSA: dice: get the number of MBLA data channel at opening PCM
    substream
  ALSA: dice: purge generating channel cache
  ALSA: dice: ensure phase lock before starting streaming

 sound/firewire/dice/dice-midi.c        |  25 +++-
 sound/firewire/dice/dice-pcm.c         | 201 ++++++++-------------------
 sound/firewire/dice/dice-stream.c      |  84 ++++++++----
 sound/firewire/dice/dice-transaction.c | 158 ++++++++++-----------
 sound/firewire/dice/dice.c             | 242 ++++++++++++---------------------
 sound/firewire/dice/dice.h             |  11 +-
 6 files changed, 293 insertions(+), 428 deletions(-)

-- 
2.5.0

[PATCH 01/10] ALSA: dice: expand timeout to wait for Dice notification

[Takashi Sakamoto]

Some users have reported that their Dice based models generate ETIMEDOUT
when starting PCM playback. It means that current timeout (=100msec) is
not enough for their models to transfer notifications.

This commit expands the timeout up to 2 sec. As a result, in a worst case,
any operations to start AMDTP streams takes 2 sec or more. Then, in
userspace, snd_pcm_hw_params(), snd_pcm_prepare(), snd_pcm_recover(),
snd_rawmidi_open(), snd_seq_connect_from() and snd_seq_connect_to() may
take the time.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-transaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index aee7461..9ea223a 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -9,7 +9,7 @@
 
 #include "dice.h"
 
-#define NOTIFICATION_TIMEOUT_MS	100
+#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
 
 static u64 get_subaddr(struct snd_dice *dice, enum snd_dice_addr_type type,
 		       u64 offset)
-- 
2.5.0

[PATCH 02/10] ALSA: dice: wait for ensuring phase lock

[Takashi Sakamoto]

Some users have reported that their Dice based models generate packet
discontinuity at the beginning of streaming. When standing on an
assumption that the value of SYT field in transferred AMDTP packet comes
from phase lock circuit, this comes from phase unlocking with current
clock source. Just waiting for dice notification is not enough to prevent
from packet discontinuity.

This commit checks the register of phase lock after clock state change for
this purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-transaction.c | 54 +++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index 9ea223a..ba0f526 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -65,16 +65,16 @@ static unsigned int get_clock_info(struct snd_dice *dice, __be32 *info)
 static int set_clock_info(struct snd_dice *dice,
 			  unsigned int rate, unsigned int source)
 {
-	unsigned int retries = 3;
+	unsigned int retries = 10;
 	unsigned int i;
-	__be32 info;
+	__be32 info, stat;
 	u32 mask;
 	u32 clock;
 	int err;
-retry:
+
 	err = get_clock_info(dice, &info);
 	if (err < 0)
-		goto end;
+		return err;
 
 	clock = be32_to_cpu(info);
 	if (source != UINT_MAX) {
@@ -87,10 +87,8 @@ retry:
 			if (snd_dice_rates[i] == rate)
 				break;
 		}
-		if (i == ARRAY_SIZE(snd_dice_rates)) {
-			err = -EINVAL;
-			goto end;
-		}
+		if (i == ARRAY_SIZE(snd_dice_rates))
+			return -EINVAL;
 
 		mask = CLOCK_RATE_MASK;
 		clock &= ~mask;
@@ -104,25 +102,33 @@ retry:
 	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
 						&info, 4);
 	if (err < 0)
-		goto end;
-
-	/* Timeout means it's invalid request, probably bus reset occurred. */
-	if (wait_for_completion_timeout(&dice->clock_accepted,
-			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0) {
-		if (retries-- == 0) {
-			err = -ETIMEDOUT;
-			goto end;
-		}
-
-		err = snd_dice_transaction_reinit(dice);
+		return err;
+
+	wait_for_completion_timeout(&dice->clock_accepted,
+			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS));
+
+	/*
+	 * Some models don't perform phase lock yet. In this case, transferred
+	 * packet includes dicsontinuity. Here, wait till one second.
+	 */
+	while (retries-- > 0) {
+		err = snd_dice_transaction_read_global(dice, GLOBAL_STATUS,
+						       &stat, sizeof(stat));
 		if (err < 0)
-			goto end;
+			return err;
+
+		if ((be32_to_cpu(stat) & 0x01) == STATUS_SOURCE_LOCKED &&
+		    ((be32_to_cpu(stat) & STATUS_NOMINAL_RATE_MASK) ==
+		      (be32_to_cpu(info) & STATUS_NOMINAL_RATE_MASK)))
+			break;
 
-		msleep(500);	/* arbitrary */
-		goto retry;
+		msleep(100);
 	}
-end:
-	return err;
+
+	if (retries == 0)
+		return -ETIMEDOUT;
+
+	return 0;
 }
 
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
-- 
2.5.0

[PATCH 03/10] ALSA: dice: split subaddress check from category check

[Takashi Sakamoto]

Before allocating an instance of sound card, ALSA dice driver checks
chip_ID_hi in Bus information block of Config ROM, then also checks
subaddresses. The former operation reads cache of Config ROM in Linux
FireWire subsystem, and the latter operation sends read transaction. The
latter can be merged into initialization of transaction.

This commit splits these two operations to reduce needless transaction in
probe processing.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-transaction.c | 89 ++++++++++++++++++++++++++--------
 sound/firewire/dice/dice.c             | 72 +++------------------------
 2 files changed, 77 insertions(+), 84 deletions(-)

diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index ba0f526..46b3481 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -337,39 +337,59 @@ int snd_dice_transaction_reinit(struct snd_dice *dice)
 	return register_notification_address(dice, false);
 }
 
-int snd_dice_transaction_init(struct snd_dice *dice)
+static int get_subaddrs(struct snd_dice *dice)
 {
-	struct fw_address_handler *handler = &dice->notification_handler;
+	static const int min_values[10] = {
+		10, 0x64 / 4,
+		10, 0x18 / 4,
+		10, 0x18 / 4,
+		0, 0,
+		0, 0,
+	};
 	__be32 *pointers;
+	u32 data;
+	unsigned int i;
 	int err;
 
-	/* Use the same way which dice_interface_check() does. */
-	pointers = kmalloc(sizeof(__be32) * 10, GFP_KERNEL);
+	pointers = kmalloc_array(ARRAY_SIZE(min_values), sizeof(__be32),
+				 GFP_KERNEL);
 	if (pointers == NULL)
 		return -ENOMEM;
 
-	/* Get offsets for sub-addresses */
+	/*
+	 * Check that the sub address spaces exist and are located inside the
+	 * private address space.  The minimum values are chosen so that all
+	 * minimally required registers are included.
+	 */
 	err = snd_fw_transaction(dice->unit, TCODE_READ_BLOCK_REQUEST,
-				 DICE_PRIVATE_SPACE,
-				 pointers, sizeof(__be32) * 10, 0);
+				 DICE_PRIVATE_SPACE, pointers,
+				 sizeof(__be32) * ARRAY_SIZE(min_values), 0);
 	if (err < 0)
 		goto end;
 
-	/* Allocation callback in address space over host controller */
-	handler->length = 4;
-	handler->address_callback = dice_notification;
-	handler->callback_data = dice;
-	err = fw_core_add_address_handler(handler, &fw_high_memory_region);
-	if (err < 0) {
-		handler->callback_data = NULL;
-		goto end;
+	for (i = 0; i < ARRAY_SIZE(min_values); ++i) {
+		data = be32_to_cpu(pointers[i]);
+		if (data < min_values[i] || data >= 0x40000) {
+			err = -ENODEV;
+			goto end;
+		}
 	}
 
-	/* Register the address space */
-	err = register_notification_address(dice, true);
-	if (err < 0) {
-		fw_core_remove_address_handler(handler);
-		handler->callback_data = NULL;
+	/*
+	 * Check that the implemented DICE driver specification major version
+	 * number matches.
+	 */
+	err = snd_fw_transaction(dice->unit, TCODE_READ_QUADLET_REQUEST,
+				 DICE_PRIVATE_SPACE +
+				 be32_to_cpu(pointers[0]) * 4 + GLOBAL_VERSION,
+				 &data, sizeof(data), 0);
+	if (err < 0)
+		goto end;
+
+	if ((data & cpu_to_be32(0xff000000)) != cpu_to_be32(0x01000000)) {
+		dev_err(&dice->unit->device,
+			"unknown DICE version: 0x%08x\n", be32_to_cpu(data));
+		err = -ENODEV;
 		goto end;
 	}
 
@@ -386,3 +406,32 @@ end:
 	kfree(pointers);
 	return err;
 }
+
+int snd_dice_transaction_init(struct snd_dice *dice)
+{
+	struct fw_address_handler *handler = &dice->notification_handler;
+	int err;
+
+	err = get_subaddrs(dice);
+	if (err < 0)
+		return err;
+
+	/* Allocation callback in address space over host controller */
+	handler->length = 4;
+	handler->address_callback = dice_notification;
+	handler->callback_data = dice;
+	err = fw_core_add_address_handler(handler, &fw_high_memory_region);
+	if (err < 0) {
+		handler->callback_data = NULL;
+		return err;
+	}
+
+	/* Register the address space */
+	err = register_notification_address(dice, true);
+	if (err < 0) {
+		fw_core_remove_address_handler(handler);
+		handler->callback_data = NULL;
+	}
+
+	return err;
+}
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 0cda05c..26271cc 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -18,27 +18,12 @@ MODULE_LICENSE("GPL v2");
 #define WEISS_CATEGORY_ID	0x00
 #define LOUD_CATEGORY_ID	0x10
 
-static int dice_interface_check(struct fw_unit *unit)
+static int check_dice_category(struct fw_unit *unit)
 {
-	static const int min_values[10] = {
-		10, 0x64 / 4,
-		10, 0x18 / 4,
-		10, 0x18 / 4,
-		0, 0,
-		0, 0,
-	};
 	struct fw_device *device = fw_parent_device(unit);
 	struct fw_csr_iterator it;
-	int key, val, vendor = -1, model = -1, err;
-	unsigned int category, i;
-	__be32 *pointers;
-	u32 value;
-	__be32 version;
-
-	pointers = kmalloc_array(ARRAY_SIZE(min_values), sizeof(__be32),
-				 GFP_KERNEL);
-	if (pointers == NULL)
-		return -ENOMEM;
+	int key, val, vendor = -1, model = -1;
+	unsigned int category;
 
 	/*
 	 * Check that GUID and unit directory are constructed according to DICE
@@ -64,51 +49,10 @@ static int dice_interface_check(struct fw_unit *unit)
 	else
 		category = DICE_CATEGORY_ID;
 	if (device->config_rom[3] != ((vendor << 8) | category) ||
-	    device->config_rom[4] >> 22 != model) {
-		err = -ENODEV;
-		goto end;
-	}
-
-	/*
-	 * Check that the sub address spaces exist and are located inside the
-	 * private address space.  The minimum values are chosen so that all
-	 * minimally required registers are included.
-	 */
-	err = snd_fw_transaction(unit, TCODE_READ_BLOCK_REQUEST,
-				 DICE_PRIVATE_SPACE, pointers,
-				 sizeof(__be32) * ARRAY_SIZE(min_values), 0);
-	if (err < 0) {
-		err = -ENODEV;
-		goto end;
-	}
-	for (i = 0; i < ARRAY_SIZE(min_values); ++i) {
-		value = be32_to_cpu(pointers[i]);
-		if (value < min_values[i] || value >= 0x40000) {
-			err = -ENODEV;
-			goto end;
-		}
-	}
+	    device->config_rom[4] >> 22 != model)
+		return -ENODEV;
 
-	/*
-	 * Check that the implemented DICE driver specification major version
-	 * number matches.
-	 */
-	err = snd_fw_transaction(unit, TCODE_READ_QUADLET_REQUEST,
-				 DICE_PRIVATE_SPACE +
-				 be32_to_cpu(pointers[0]) * 4 + GLOBAL_VERSION,
-				 &version, 4, 0);
-	if (err < 0) {
-		err = -ENODEV;
-		goto end;
-	}
-	if ((version & cpu_to_be32(0xff000000)) != cpu_to_be32(0x01000000)) {
-		dev_err(&unit->device,
-			"unknown DICE version: 0x%08x\n", be32_to_cpu(version));
-		err = -ENODEV;
-		goto end;
-	}
-end:
-	return err;
+	return 0;
 }
 
 static int highest_supported_mode_rate(struct snd_dice *dice,
@@ -254,9 +198,9 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 	struct snd_dice *dice;
 	int err;
 
-	err = dice_interface_check(unit);
+	err = check_dice_category(unit);
 	if (err < 0)
-		goto end;
+		return -ENODEV;
 
 	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
 			   sizeof(*dice), &card);
-- 
2.5.0

[PATCH 04/10] ALSA: dice: postpone probe processing

[Takashi Sakamoto]

Some models based on ASIC for Dice II (STD, CP) change their hardware
configurations after appearing on IEEE 1394 bus. This is due to
interactions of boot loader (RedBoot), firmwares with eCos, and vendor's
configurations.

It may takes a bit time (less than 1 seconds) to load and execute the
firmware. Then it initializes the hardware according to vendor's
configurations. After configured, it generate bus reset. These operations
finished within 1 to 1.5 seconds. As a result, successive several bus reset
are observed in probe processing of IEEE 1394 unit drivers and it sometimes
causes troubles.

This commit offloads probe processing into workqueue to postpone it after
the hardware initialization and bus resets. For simplicity, this change
effects all of Dice-based models, thus Dice II, Dice Jr., Dice Mini and
Dice III.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice.c | 107 +++++++++++++++++++++++++++++++++------------
 sound/firewire/dice/dice.h |   3 ++
 2 files changed, 82 insertions(+), 28 deletions(-)

diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 26271cc..c7d5e05 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -18,6 +18,8 @@ MODULE_LICENSE("GPL v2");
 #define WEISS_CATEGORY_ID	0x00
 #define LOUD_CATEGORY_ID	0x10
 
+#define PROBE_DELAY_MS		(2 * MSEC_PER_SEC)
+
 static int check_dice_category(struct fw_unit *unit)
 {
 	struct fw_device *device = fw_parent_device(unit);
@@ -185,17 +187,70 @@ static void dice_card_free(struct snd_card *card)
 {
 	struct snd_dice *dice = card->private_data;
 
+	/* The workqueue for registration uses the memory block. */
+	cancel_work_sync(&dice->dwork.work);
+
 	snd_dice_stream_destroy_duplex(dice);
+
 	snd_dice_transaction_destroy(dice);
+
 	fw_unit_put(dice->unit);
 
 	mutex_destroy(&dice->mutex);
 }
 
+static void do_probe(struct work_struct *work)
+{
+	struct snd_dice *dice = container_of(work, struct snd_dice, dwork.work);
+	int err;
+
+	mutex_lock(&dice->mutex);
+
+	if (dice->card->shutdown || dice->probed)
+		goto end;
+
+	err = snd_dice_transaction_init(dice);
+	if (err < 0)
+		goto end;
+
+	err = dice_read_params(dice);
+	if (err < 0)
+		goto end;
+
+	dice_card_strings(dice);
+
+	err = snd_dice_create_pcm(dice);
+	if (err < 0)
+		goto end;
+
+	err = snd_dice_create_midi(dice);
+	if (err < 0)
+		goto end;
+
+	err = snd_card_register(dice->card);
+	if (err < 0) {
+		snd_dice_stream_destroy_duplex(dice);
+		goto end;
+	}
+
+	dice->probed = true;
+end:
+	mutex_unlock(&dice->mutex);
+
+	/*
+	 * It's a difficult work to manage a race condition between workqueue,
+	 * unit event handlers and processes. The memory block for this card
+	 * is released as the same way that usual sound cards are going to be
+	 * released.
+	 */
+}
+
 static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 {
+	struct fw_card *fw_card = fw_parent_device(unit)->card;
 	struct snd_card *card;
 	struct snd_dice *dice;
+	unsigned long delay;
 	int err;
 
 	err = check_dice_category(unit);
@@ -205,29 +260,20 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
 			   sizeof(*dice), &card);
 	if (err < 0)
-		goto end;
+		return err;
 
 	dice = card->private_data;
 	dice->card = card;
 	dice->unit = fw_unit_get(unit);
 	card->private_free = dice_card_free;
+	dev_set_drvdata(&unit->device, dice);
 
 	spin_lock_init(&dice->lock);
 	mutex_init(&dice->mutex);
 	init_completion(&dice->clock_accepted);
 	init_waitqueue_head(&dice->hwdep_wait);
 
-	err = snd_dice_transaction_init(dice);
-	if (err < 0)
-		goto error;
-
-	err = dice_read_params(dice);
-	if (err < 0)
-		goto error;
-
-	dice_card_strings(dice);
-
-	err = snd_dice_create_pcm(dice);
+	err = snd_dice_stream_init_duplex(dice);
 	if (err < 0)
 		goto error;
 
@@ -237,23 +283,13 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 
 	snd_dice_create_proc(dice);
 
-	err = snd_dice_create_midi(dice);
-	if (err < 0)
-		goto error;
-
-	err = snd_dice_stream_init_duplex(dice);
-	if (err < 0)
-		goto error;
-
-	err = snd_card_register(card);
-	if (err < 0) {
-		snd_dice_stream_destroy_duplex(dice);
-		goto error;
-	}
+	/* Register this sound card later. */
+	INIT_DEFERRABLE_WORK(&dice->dwork, do_probe);
+	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
+				fw_card->reset_jiffies - get_jiffies_64();
+	schedule_delayed_work(&dice->dwork, delay);
 
-	dev_set_drvdata(&unit->device, dice);
-end:
-	return err;
+	return 0;
 error:
 	snd_card_free(card);
 	return err;
@@ -263,13 +299,28 @@ static void dice_remove(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
+	/* For a race condition of struct snd_card.shutdown. */
+	mutex_lock(&dice->mutex);
+
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(dice->card);
+
+	mutex_unlock(&dice->mutex);
 }
 
 static void dice_bus_reset(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
+	struct fw_card *fw_card = fw_parent_device(unit)->card;
+	unsigned long delay;
+
+	/* Postpone a workqueue for deferred registration. */
+	if (!dice->probed) {
+		delay = msecs_to_jiffies(PROBE_DELAY_MS) -
+				(get_jiffies_64() - fw_card->reset_jiffies);
+		mod_delayed_work(dice->dwork.wq, &dice->dwork, delay);
+		return;
+	}
 
 	/* The handler address register becomes initialized. */
 	snd_dice_transaction_reinit(dice);
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 101550ac..4741113 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -45,6 +45,9 @@ struct snd_dice {
 	spinlock_t lock;
 	struct mutex mutex;
 
+	bool probed;
+	struct delayed_work dwork;
+
 	/* Offsets for sub-addresses */
 	unsigned int global_offset;
 	unsigned int rx_offset;
-- 
2.5.0

[PATCH 05/10] ALSA: dice: limit to current sampling transfer frequency

[Takashi Sakamoto]

ALSA PCM core has a functionality for rule of parameters for PCM
substream. Typically, when userspace opens PCM character device, each
driver adds its own rules to PCM substream, according to design of
devices. When the userspace executes hw_params ioctl with favorable
parameters, the actual parameters are calculated with the rules and the
given parameters.

Currently, ALSA Dice driver has the rule between channels and rates, while
Dice interface design doesn't allow drivers to retrieve all of their
combinations in advance. Dice devices just allows drivers to get current
sampling transfer frequency, the number of multi bit linear audio data
channels and MIDI conformant data channels in an data block of an AMDTP
packet.

This commit purges the rule and limit PCM substreams to current sampling
transfer frequency, following to the interface design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 172 ++++++++---------------------------------
 1 file changed, 33 insertions(+), 139 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 9b34319..eaaabf0 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -9,99 +9,42 @@
 
 #include "dice.h"
 
-static int dice_rate_constraint(struct snd_pcm_hw_params *params,
-				struct snd_pcm_hw_rule *rule)
-{
-	struct snd_pcm_substream *substream = rule->private;
-	struct snd_dice *dice = substream->private_data;
-
-	const struct snd_interval *c =
-		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
-	struct snd_interval *r =
-		hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
-	struct snd_interval rates = {
-		.min = UINT_MAX, .max = 0, .integer = 1
-	};
-	unsigned int i, rate, mode, *pcm_channels;
-
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-		pcm_channels = dice->tx_channels;
-	else
-		pcm_channels = dice->rx_channels;
-
-	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
-		rate = snd_dice_rates[i];
-		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
-			continue;
-
-		if (!snd_interval_test(c, pcm_channels[mode]))
-			continue;
-
-		rates.min = min(rates.min, rate);
-		rates.max = max(rates.max, rate);
-	}
-
-	return snd_interval_refine(r, &rates);
-}
-
-static int dice_channels_constraint(struct snd_pcm_hw_params *params,
-				    struct snd_pcm_hw_rule *rule)
-{
-	struct snd_pcm_substream *substream = rule->private;
-	struct snd_dice *dice = substream->private_data;
-
-	const struct snd_interval *r =
-		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
-	struct snd_interval *c =
-		hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
-	struct snd_interval channels = {
-		.min = UINT_MAX, .max = 0, .integer = 1
-	};
-	unsigned int i, rate, mode, *pcm_channels;
-
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-		pcm_channels = dice->tx_channels;
-	else
-		pcm_channels = dice->rx_channels;
-
-	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
-		rate = snd_dice_rates[i];
-		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
-			continue;
-
-		if (!snd_interval_test(r, rate))
-			continue;
-
-		channels.min = min(channels.min, pcm_channels[mode]);
-		channels.max = max(channels.max, pcm_channels[mode]);
-	}
-
-	return snd_interval_refine(c, &channels);
-}
-
-static void limit_channels_and_rates(struct snd_dice *dice,
-				     struct snd_pcm_runtime *runtime,
-				     unsigned int *pcm_channels)
+static int limit_channels_and_rates(struct snd_dice *dice,
+				    struct snd_pcm_runtime *runtime,
+				    struct amdtp_stream *stream)
 {
 	struct snd_pcm_hardware *hw = &runtime->hw;
-	unsigned int i, rate, mode;
+	unsigned int rate;
+	__be32 reg[2];
+	int err;
 
-	hw->channels_min = UINT_MAX;
-	hw->channels_max = 0;
+	/*
+	 * Retrieve current Multi Bit Linear Audio data channel and limit to
+	 * it.
+	 */
+	if (stream == &dice->tx_stream) {
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
+	} else {
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
+	}
+	if (err < 0)
+		return err;
 
-	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
-		rate = snd_dice_rates[i];
-		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
-			continue;
-		hw->rates |= snd_pcm_rate_to_rate_bit(rate);
+	hw->channels_min = hw->channels_max = be32_to_cpu(reg[0]);
 
-		if (pcm_channels[mode] == 0)
-			continue;
-		hw->channels_min = min(hw->channels_min, pcm_channels[mode]);
-		hw->channels_max = max(hw->channels_max, pcm_channels[mode]);
-	}
+	/* Retrieve current sampling transfer frequency and limit to it. */
+	err = snd_dice_transaction_get_rate(dice, &rate);
+	if (err < 0)
+		return err;
 
+	hw->rates = snd_pcm_rate_to_rate_bit(rate);
+	hw->rate_min = rate;
+	hw->rate_max = rate;
 	snd_pcm_limit_hw_rates(runtime);
+
+	return 0;
 }
 
 static void limit_period_and_buffer(struct snd_pcm_hardware *hw)
@@ -122,7 +65,6 @@ static int init_hw_info(struct snd_dice *dice,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_pcm_hardware *hw = &runtime->hw;
 	struct amdtp_stream *stream;
-	unsigned int *pcm_channels;
 	int err;
 
 	hw->info = SNDRV_PCM_INFO_MMAP |
@@ -135,37 +77,22 @@ static int init_hw_info(struct snd_dice *dice,
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		hw->formats = AM824_IN_PCM_FORMAT_BITS;
 		stream = &dice->tx_stream;
-		pcm_channels = dice->tx_channels;
 	} else {
 		hw->formats = AM824_OUT_PCM_FORMAT_BITS;
 		stream = &dice->rx_stream;
-		pcm_channels = dice->rx_channels;
 	}
 
-	limit_channels_and_rates(dice, runtime, pcm_channels);
-	limit_period_and_buffer(hw);
-
-	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				  dice_rate_constraint, substream,
-				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	err = limit_channels_and_rates(dice, runtime, stream);
 	if (err < 0)
-		goto end;
-	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
-				  dice_channels_constraint, substream,
-				  SNDRV_PCM_HW_PARAM_RATE, -1);
-	if (err < 0)
-		goto end;
+		return err;
+	limit_period_and_buffer(hw);
 
-	err = amdtp_am824_add_pcm_hw_constraints(stream, runtime);
-end:
-	return err;
+	return amdtp_am824_add_pcm_hw_constraints(stream, runtime);
 }
 
 static int pcm_open(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	unsigned int source, rate;
-	bool internal;
 	int err;
 
 	err = snd_dice_stream_lock_try(dice);
@@ -176,39 +103,6 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	if (err < 0)
 		goto err_locked;
 
-	err = snd_dice_transaction_get_clock_source(dice, &source);
-	if (err < 0)
-		goto err_locked;
-	switch (source) {
-	case CLOCK_SOURCE_AES1:
-	case CLOCK_SOURCE_AES2:
-	case CLOCK_SOURCE_AES3:
-	case CLOCK_SOURCE_AES4:
-	case CLOCK_SOURCE_AES_ANY:
-	case CLOCK_SOURCE_ADAT:
-	case CLOCK_SOURCE_TDIF:
-	case CLOCK_SOURCE_WC:
-		internal = false;
-		break;
-	default:
-		internal = true;
-		break;
-	}
-
-	/*
-	 * When source of clock is not internal or any PCM streams are running,
-	 * available sampling rate is limited at current sampling rate.
-	 */
-	if (!internal ||
-	    amdtp_stream_pcm_running(&dice->tx_stream) ||
-	    amdtp_stream_pcm_running(&dice->rx_stream)) {
-		err = snd_dice_transaction_get_rate(dice, &rate);
-		if (err < 0)
-			goto err_locked;
-		substream->runtime->hw.rate_min = rate;
-		substream->runtime->hw.rate_max = rate;
-	}
-
 	snd_pcm_set_sync(substream);
 end:
 	return err;
-- 
2.5.0

[PATCH 06/10] ALSA: dice: limit stream to current sampling transfer frequency.

[Takashi Sakamoto]

In previous commit, ALSA Dice driver limits PCM substreams at current
sampling transfer frequency and current number of Multi bit linear audio
data channel. Thus, the driver has no need to start AMDTP streams at
the other sampling transfer frequency except for current one. This is due
to Dice interface design.

This commit limits AMDTP stream at current sampling transfer frequency,
according to the design.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index a6a39f7..4f74e3e 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -99,6 +99,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 			unsigned int rate)
 {
 	struct fw_iso_resources *resources;
+	__be32 reg[2];
 	unsigned int i, mode, pcm_chs, midi_ports;
 	bool double_pcm_frames;
 	int err;
@@ -108,14 +109,20 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 		goto end;
 	if (stream == &dice->tx_stream) {
 		resources = &dice->tx_resources;
-		pcm_chs = dice->tx_channels[mode];
-		midi_ports = dice->tx_midi_ports[mode];
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
 	} else {
 		resources = &dice->rx_resources;
-		pcm_chs = dice->rx_channels[mode];
-		midi_ports = dice->rx_midi_ports[mode];
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
 	}
 
+	if (err < 0)
+		goto end;
+
+	pcm_chs = be32_to_cpu(reg[0]);
+	midi_ports = be32_to_cpu(reg[1]);
+
 	/*
 	 * At 176.4/192.0 kHz, Dice has a quirk to transfer two PCM frames in
 	 * one data block of AMDTP packet. Thus sampling transfer frequency is
@@ -224,8 +231,10 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	}
 	if (rate == 0)
 		rate = curr_rate;
-	if (rate != curr_rate)
-		stop_stream(dice, master);
+	if (rate != curr_rate) {
+		err = -EINVAL;
+		goto end;
+	}
 
 	if (!amdtp_stream_running(master)) {
 		stop_stream(dice, slave);
-- 
2.5.0

[PATCH 07/10] ALSA: dice: add MIDI ports according to current number of MIDI substreams

[Takashi Sakamoto]

This commit changes the way to add ALSA MIDI ports. This driver read the
number of multiplexed MIDI substreams from hardware register, then adds the
same number of ALSA MIDI ports. This commit is based on my assumption that
the number is fixed at all of supported sampling transfer frequency.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-midi.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/dice/dice-midi.c b/sound/firewire/dice/dice-midi.c
index 151b09f..59ac5cb 100644
--- a/sound/firewire/dice/dice-midi.c
+++ b/sound/firewire/dice/dice-midi.c
@@ -103,16 +103,29 @@ static void set_midi_substream_names(struct snd_dice *dice,
 
 int snd_dice_create_midi(struct snd_dice *dice)
 {
+	__be32 reg;
 	struct snd_rawmidi *rmidi;
 	struct snd_rawmidi_str *str;
-	unsigned int i, midi_in_ports, midi_out_ports;
+	unsigned int midi_in_ports, midi_out_ports;
 	int err;
 
-	midi_in_ports = midi_out_ports = 0;
-	for (i = 0; i < 3; i++) {
-		midi_in_ports = max(dice->tx_midi_ports[i], midi_in_ports);
-		midi_out_ports = max(dice->rx_midi_ports[i], midi_out_ports);
-	}
+	/*
+	 * Use current number of MIDI conformant data channel.
+	 *
+	 * TODO: in the case that the number of MIDI conformant data channel
+	 * differs depending on current sampling transfer frequency?
+	 */
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_MIDI,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	midi_in_ports = be32_to_cpu(reg);
+
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_MIDI,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	midi_out_ports = be32_to_cpu(reg);
 
 	if (midi_in_ports + midi_out_ports == 0)
 		return 0;
-- 
2.5.0

[PATCH 08/10] ALSA: dice: get the number of MBLA data channel at opening PCM substream

[Takashi Sakamoto]

This commit is a preparation to remove members related to the array
number of channels for multi bit linear audio data and MIDI conformant
data. The number of multi bit linear audio data channel is retrieved by
asynchronous transaction to some registers.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index eaaabf0..c3343b6 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -296,17 +296,30 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 		.page      = snd_pcm_lib_get_vmalloc_page,
 		.mmap      = snd_pcm_lib_mmap_vmalloc,
 	};
+	__be32 reg;
 	struct snd_pcm *pcm;
-	unsigned int i, capture, playback;
+	unsigned int capture, playback;
 	int err;
 
-	capture = playback = 0;
-	for (i = 0; i < 3; i++) {
-		if (dice->tx_channels[i] > 0)
-			capture = 1;
-		if (dice->rx_channels[i] > 0)
-			playback = 1;
-	}
+	/*
+	 * Check whether PCM substreams are required.
+	 *
+	 * TODO: in the case that any PCM substreams are not avail at a certain
+	 * sampling transfer frequency?
+	 */
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	if (be32_to_cpu(reg) > 0)
+		capture = 1;
+
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	if (be32_to_cpu(reg) > 0)
+		playback = 1;
 
 	err = snd_pcm_new(dice->card, "DICE", 0, playback, capture, &pcm);
 	if (err < 0)
-- 
2.5.0

[PATCH 09/10] ALSA: dice: purge generating channel cache

[Takashi Sakamoto]

Dice interface design doesn't allow drivers to read supported combination
between sampling transfer frequencies and the number of Multi bit linear
audio data channels. Due to the design, ALSA dice driver changes current
sampling transfer frequency to generate cache of the combinations at
probing processing.

Although, this idea is worse because ALSA dice driver changes the state of
clock. This is not what users want when they save favorite configuration
to the device in advance.

Furthermore, there's a possibility that the format of data block is decided
not only according to current sampling transfer frequency, but also the
other factors, i.e. data format for digital interface. It's not good to
generate channel cache according to the sampling transfer frequency only.

This commit purges processing cache data and related structure members. As
a result, users must set preferable sampling transfer frequency before
using ALSA PCM applications, as long as they want to start any PCM
substreams at the rate.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 24 ++------------
 sound/firewire/dice/dice.c        | 67 ++-------------------------------------
 sound/firewire/dice/dice.h        |  7 ----
 3 files changed, 5 insertions(+), 93 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 4f74e3e..716db09 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -24,23 +24,6 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	[6] = 192000,
 };
 
-int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
-				  unsigned int *mode)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
-		if (!(dice->clock_caps & BIT(i)))
-			continue;
-		if (snd_dice_rates[i] != rate)
-			continue;
-
-		*mode = (i - 1) / 2;
-		return 0;
-	}
-	return -EINVAL;
-}
-
 static void release_resources(struct snd_dice *dice,
 			      struct fw_iso_resources *resources)
 {
@@ -100,13 +83,10 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 {
 	struct fw_iso_resources *resources;
 	__be32 reg[2];
-	unsigned int i, mode, pcm_chs, midi_ports;
+	unsigned int i, pcm_chs, midi_ports;
 	bool double_pcm_frames;
 	int err;
 
-	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
-	if (err < 0)
-		goto end;
 	if (stream == &dice->tx_stream) {
 		resources = &dice->tx_resources;
 		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
@@ -133,7 +113,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 	 * For this quirk, blocking mode is required and PCM buffer size should
 	 * be aligned to SYT_INTERVAL.
 	 */
-	double_pcm_frames = mode > 1;
+	double_pcm_frames = rate > 96000;
 	if (double_pcm_frames) {
 		rate /= 2;
 		pcm_chs *= 2;
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index c7d5e05..1993c16 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -57,65 +57,10 @@ static int check_dice_category(struct fw_unit *unit)
 	return 0;
 }
 
-static int highest_supported_mode_rate(struct snd_dice *dice,
-				       unsigned int mode, unsigned int *rate)
-{
-	unsigned int i, m;
-
-	for (i = ARRAY_SIZE(snd_dice_rates); i > 0; i--) {
-		*rate = snd_dice_rates[i - 1];
-		if (snd_dice_stream_get_rate_mode(dice, *rate, &m) < 0)
-			continue;
-		if (mode == m)
-			break;
-	}
-	if (i == 0)
-		return -EINVAL;
-
-	return 0;
-}
-
-static int dice_read_mode_params(struct snd_dice *dice, unsigned int mode)
-{
-	__be32 values[2];
-	unsigned int rate;
-	int err;
-
-	if (highest_supported_mode_rate(dice, mode, &rate) < 0) {
-		dice->tx_channels[mode] = 0;
-		dice->tx_midi_ports[mode] = 0;
-		dice->rx_channels[mode] = 0;
-		dice->rx_midi_ports[mode] = 0;
-		return 0;
-	}
-
-	err = snd_dice_transaction_set_rate(dice, rate);
-	if (err < 0)
-		return err;
-
-	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-					   values, sizeof(values));
-	if (err < 0)
-		return err;
-
-	dice->tx_channels[mode]   = be32_to_cpu(values[0]);
-	dice->tx_midi_ports[mode] = be32_to_cpu(values[1]);
-
-	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-					   values, sizeof(values));
-	if (err < 0)
-		return err;
-
-	dice->rx_channels[mode]   = be32_to_cpu(values[0]);
-	dice->rx_midi_ports[mode] = be32_to_cpu(values[1]);
-
-	return 0;
-}
-
-static int dice_read_params(struct snd_dice *dice)
+static int check_clock_caps(struct snd_dice *dice)
 {
 	__be32 value;
-	int mode, err;
+	int err;
 
 	/* some very old firmwares don't tell about their clock support */
 	if (dice->clock_caps > 0) {
@@ -133,12 +78,6 @@ static int dice_read_params(struct snd_dice *dice)
 				   CLOCK_CAP_SOURCE_INTERNAL;
 	}
 
-	for (mode = 2; mode >= 0; --mode) {
-		err = dice_read_mode_params(dice, mode);
-		if (err < 0)
-			return err;
-	}
-
 	return 0;
 }
 
@@ -213,7 +152,7 @@ static void do_probe(struct work_struct *work)
 	if (err < 0)
 		goto end;
 
-	err = dice_read_params(dice);
+	err = check_clock_caps(dice);
 	if (err < 0)
 		goto end;
 
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 4741113..60f9661 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -56,10 +56,6 @@ struct snd_dice {
 	unsigned int rsrv_offset;
 
 	unsigned int clock_caps;
-	unsigned int tx_channels[3];
-	unsigned int rx_channels[3];
-	unsigned int tx_midi_ports[3];
-	unsigned int rx_midi_ports[3];
 
 	struct fw_address_handler notification_handler;
 	int owner_generation;
@@ -169,9 +165,6 @@ void snd_dice_transaction_destroy(struct snd_dice *dice);
 #define SND_DICE_RATES_COUNT	7
 extern const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT];
 
-int snd_dice_stream_get_rate_mode(struct snd_dice *dice,
-				  unsigned int rate, unsigned int *mode);
-
 int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate);
 void snd_dice_stream_stop_duplex(struct snd_dice *dice);
 int snd_dice_stream_init_duplex(struct snd_dice *dice);
-- 
2.5.0

[PATCH 10/10] ALSA: dice: ensure phase lock before starting streaming

[Takashi Sakamoto]

In former commits, probing process has no need to set sampling transfer
frequency. Although it's OK to drop a function to change the frequency
from this module, some models require it before streaming. This seems to
be due to phase lock of clock source.

This commit moves the function from transaction layer to stream layer, and
rename it according to the purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c      | 51 ++++++++++++++++++++++-
 sound/firewire/dice/dice-transaction.c | 75 ----------------------------------
 sound/firewire/dice/dice.h             |  1 -
 3 files changed, 49 insertions(+), 78 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 716db09..2a07cfa 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -10,6 +10,7 @@
 #include "dice.h"
 
 #define	CALLBACK_TIMEOUT	200
+#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
 
 const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	/* mode 0 */
@@ -24,6 +25,52 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	[6] = 192000,
 };
 
+static int ensure_phase_lock(struct snd_dice *dice)
+{
+	unsigned int retries = 10;
+	__be32 reg, stat;
+	int err;
+
+	err = snd_dice_transaction_read_global(dice, GLOBAL_CLOCK_SELECT,
+					       &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+
+	if (completion_done(&dice->clock_accepted))
+		reinit_completion(&dice->clock_accepted);
+
+	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
+						&reg, sizeof(reg));
+	if (err < 0)
+		return err;
+
+	wait_for_completion_timeout(&dice->clock_accepted,
+			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS));
+
+	/*
+	 * Some models don't perform phase lock yet. In this case, transferred
+	 * packet includes dicsontinuity. Here, wait till one second.
+	 */
+	while (retries-- > 0) {
+		err = snd_dice_transaction_read_global(dice, GLOBAL_STATUS,
+						       &stat, sizeof(stat));
+		if (err < 0)
+			return err;
+
+		if ((be32_to_cpu(stat) & 0x01) == STATUS_SOURCE_LOCKED &&
+		    ((be32_to_cpu(stat) & STATUS_NOMINAL_RATE_MASK) ==
+		      (be32_to_cpu(reg) & STATUS_NOMINAL_RATE_MASK)))
+			break;
+
+		msleep(100);
+	}
+
+	if (retries == 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static void release_resources(struct snd_dice *dice,
 			      struct fw_iso_resources *resources)
 {
@@ -222,10 +269,10 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 
 		amdtp_stream_set_sync(sync_mode, master, slave);
 
-		err = snd_dice_transaction_set_rate(dice, rate);
+		err = ensure_phase_lock(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
-				"fail to set sampling rate\n");
+				"fail to ensure phase lock\n");
 			goto end;
 		}
 
diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index 46b3481..cc4ee65 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -9,8 +9,6 @@
 
 #include "dice.h"
 
-#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
-
 static u64 get_subaddr(struct snd_dice *dice, enum snd_dice_addr_type type,
 		       u64 offset)
 {
@@ -62,75 +60,6 @@ static unsigned int get_clock_info(struct snd_dice *dice, __be32 *info)
 						info, 4);
 }
 
-static int set_clock_info(struct snd_dice *dice,
-			  unsigned int rate, unsigned int source)
-{
-	unsigned int retries = 10;
-	unsigned int i;
-	__be32 info, stat;
-	u32 mask;
-	u32 clock;
-	int err;
-
-	err = get_clock_info(dice, &info);
-	if (err < 0)
-		return err;
-
-	clock = be32_to_cpu(info);
-	if (source != UINT_MAX) {
-		mask = CLOCK_SOURCE_MASK;
-		clock &= ~mask;
-		clock |= source;
-	}
-	if (rate != UINT_MAX) {
-		for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
-			if (snd_dice_rates[i] == rate)
-				break;
-		}
-		if (i == ARRAY_SIZE(snd_dice_rates))
-			return -EINVAL;
-
-		mask = CLOCK_RATE_MASK;
-		clock &= ~mask;
-		clock |= i << CLOCK_RATE_SHIFT;
-	}
-	info = cpu_to_be32(clock);
-
-	if (completion_done(&dice->clock_accepted))
-		reinit_completion(&dice->clock_accepted);
-
-	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
-						&info, 4);
-	if (err < 0)
-		return err;
-
-	wait_for_completion_timeout(&dice->clock_accepted,
-			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS));
-
-	/*
-	 * Some models don't perform phase lock yet. In this case, transferred
-	 * packet includes dicsontinuity. Here, wait till one second.
-	 */
-	while (retries-- > 0) {
-		err = snd_dice_transaction_read_global(dice, GLOBAL_STATUS,
-						       &stat, sizeof(stat));
-		if (err < 0)
-			return err;
-
-		if ((be32_to_cpu(stat) & 0x01) == STATUS_SOURCE_LOCKED &&
-		    ((be32_to_cpu(stat) & STATUS_NOMINAL_RATE_MASK) ==
-		      (be32_to_cpu(info) & STATUS_NOMINAL_RATE_MASK)))
-			break;
-
-		msleep(100);
-	}
-
-	if (retries == 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
 					  unsigned int *source)
 {
@@ -164,10 +93,6 @@ int snd_dice_transaction_get_rate(struct snd_dice *dice, unsigned int *rate)
 end:
 	return err;
 }
-int snd_dice_transaction_set_rate(struct snd_dice *dice, unsigned int rate)
-{
-	return set_clock_info(dice, rate, UINT_MAX);
-}
 
 int snd_dice_transaction_set_enable(struct snd_dice *dice)
 {
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 60f9661..3726fd4 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -154,7 +154,6 @@ static inline int snd_dice_transaction_read_sync(struct snd_dice *dice,
 
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
 					  unsigned int *source);
-int snd_dice_transaction_set_rate(struct snd_dice *dice, unsigned int rate);
 int snd_dice_transaction_get_rate(struct snd_dice *dice, unsigned int *rate);
 int snd_dice_transaction_set_enable(struct snd_dice *dice);
 void snd_dice_transaction_clear_enable(struct snd_dice *dice);
-- 
2.5.0

Re: [PATCH 02/10] ALSA: dice: wait for ensuring phase lock

[Clemens Ladisch]

Takashi Sakamoto wrote:
> Some users have reported that their Dice based models generate packet
> discontinuity at the beginning of streaming. When standing on an
> assumption that the value of SYT field in transferred AMDTP packet comes
> from phase lock circuit, this comes from phase unlocking with current
> clock source. Just waiting for dice notification is not enough to prevent
> from packet discontinuity.
>
> This commit checks the register of phase lock after clock state change for
> this purpose.

Is this patch actually known to help?

This discontinuity could also come from an unexpected initial DBC value.
I think it would be a good idea to unconditionally enable
CIP_SKIP_INIT_DBC_CHECK for all devices.


Regards,
Clemens

Re: [PATCH 02/10] ALSA: dice: wait for ensuring phase lock

[Takashi Sakamoto]

Hi Clemens,

On Dec 12 2015 20:42, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> Some users have reported that their Dice based models generate packet
>> discontinuity at the beginning of streaming. When standing on an
>> assumption that the value of SYT field in transferred AMDTP packet comes
>> from phase lock circuit, this comes from phase unlocking with current
>> clock source. Just waiting for dice notification is not enough to prevent
>> from packet discontinuity.
>>
>> This commit checks the register of phase lock after clock state change for
>> this purpose.
> 
> Is this patch actually known to help?
> 
> This discontinuity could also come from an unexpected initial DBC value.
> I think it would be a good idea to unconditionally enable
> CIP_SKIP_INIT_DBC_CHECK for all devices.

This issue also occurs on my ImpactTwin. Today, I dropped this patch and
tried re-generate this issue, then I cannot regenerate it.

As long as I tested, the deferred registration has an effect for this
issue. I guess that the beginning of packet streaming is envolved to the
end of hardware initialization, then discontinuity occurs.

I decide to drop this patch from next post, thanks to address it.


Regards

Takashi Sakamoto