All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 v5] ALSA: dice: improve card registration processing
@ 2015-12-31  4:58 Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 1/4] ALSA: dice: split subaddress check from category check Takashi Sakamoto
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Takashi Sakamoto @ 2015-12-31  4:58 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stefanr, ffado-devel

Hi,

This patchset updates my previous one:
[alsa-devel] [PATCH 0/5 v4] ALSA: dice: improve card registration processing
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/102504.html

Changes:
 * Merge patch 02 and 03 in v4.
 * Allocate sound card instance in sound card registration work, independently
   of dice instance, allocated in .probe callback and set as driver_data of
   device structure.
 * Use __be32 type for __be32 data.

In this version, sound card instance is not allocated in .probe callback. In
the callback, dice instance is allocated independently of the card instance.
Thus, in .probe callback, this driver reserve no slot of sound card. The
allocation of sound card instance is performed in a scheduled work. When
processing in the work fails, the work can be re-scheduled again according to
bus reset.

Takashi Sakamoto (4):
  ALSA: dice: split subaddress check from category check
  ALSA: dice: postpone card registration
  ALSA: dice: purge transaction initialization at timeout of Dice
    notification
  ALSA: dice: expand timeout to wait for Dice notification

 sound/firewire/dice/dice-transaction.c | 123 +++++++++++-------
 sound/firewire/dice/dice.c             | 227 +++++++++++++++++----------------
 sound/firewire/dice/dice.h             |   3 +
 3 files changed, 202 insertions(+), 151 deletions(-)

-- 
2.5.0

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

* [PATCH 1/4] ALSA: dice: split subaddress check from category check
  2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
@ 2015-12-31  4:58 ` Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 2/4] ALSA: dice: postpone card registration Takashi Sakamoto
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Sakamoto @ 2015-12-31  4:58 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stefanr, ffado-devel

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, while the latter operation sends read transaction.
The latter can be merged into initialization of transaction system.

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

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

diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index aee7461..fdb7841 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -331,39 +331,60 @@ 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;
+	__be32 version;
+	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,
+				 &version, sizeof(version), 0);
+	if (err < 0)
+		goto end;
+
+	if ((version & cpu_to_be32(0xff000000)) != cpu_to_be32(0x01000000)) {
+		dev_err(&dice->unit->device,
+			"unknown DICE version: 0x%08x\n", be32_to_cpu(version));
+		err = -ENODEV;
 		goto end;
 	}
 
@@ -380,3 +401,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

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

* [PATCH 2/4] ALSA: dice: postpone card registration
  2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 1/4] ALSA: dice: split subaddress check from category check Takashi Sakamoto
@ 2015-12-31  4:58 ` Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 3/4] ALSA: dice: purge transaction initialization at timeout of Dice notification Takashi Sakamoto
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Sakamoto @ 2015-12-31  4:58 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stefanr, ffado-devel

Some models based on ASIC for Dice II series (STD, CP) change their
hardware configurations after appearing on IEEE 1394 bus. This is due to
interactions of boot loader (RedBoot), firmwares (eCos) and vendor's
configurations. This causes current ALSA dice driver to get wrong
information about the hardware's capability because its probe function
runs just after detecting unit of the model.

As long as I investigated, it takes a bit time (less than 1 second) to load
the firmware after bootstrap. Just after loaded, the driver can get
information about the unit. Then the hardware is initialized according to
vendor's configurations. After, the got information becomes wrong.
Between bootstrap, firmware loading and post configuration, some bus resets
are observed.

This commit offloads most processing of probe function into workqueue and
schedules the workqueue after successive bus resets. This has an effect to
get correct hardware information and avoid involvement to bus reset storm.

For code simplicity, this change effects all of Dice-based models, i.e.
Dice II, Dice Jr., Dice Mini and Dice III.

I use a loose strategy to manage a race condition between the work and the
bus reset. This is due to a specification of dice transaction. When bus
reset occurs, registered address for the transaction is cleared. Drivers
must re-register their own address again. While, this operation is required
for the work because the work includes to wait for the transaction. This
commit uses no lock primitives for the race condition. Instead, checking
'registered' member of 'struct snd_dice' avoid executing the work again.
If sound card is not registered, the work can be scheduled again by bus
reset handler.

When .remove callback is executed, the sound card is going to be released.
The work should not be pending or executed in the releasing. This commit
uses cancel_delayed_work_sync() in .remove callback and wait till the
pending work finished. After .remove callback, .update callback is not
executed, therefore no works are scheduled again.

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

diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 26271cc..f5e15c4 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);
@@ -175,6 +177,16 @@ static void dice_card_strings(struct snd_dice *dice)
 	strcpy(card->mixername, "DICE");
 }
 
+static void dice_free(struct snd_dice *dice)
+{
+	snd_dice_stream_destroy_duplex(dice);
+	snd_dice_transaction_destroy(dice);
+	fw_unit_put(dice->unit);
+
+	mutex_destroy(&dice->mutex);
+	kfree(dice);
+}
+
 /*
  * This module releases the FireWire unit data after all ALSA character devices
  * are released by applications. This is for releasing stream data or finishing
@@ -183,39 +195,21 @@ static void dice_card_strings(struct snd_dice *dice)
  */
 static void dice_card_free(struct snd_card *card)
 {
-	struct snd_dice *dice = card->private_data;
-
-	snd_dice_stream_destroy_duplex(dice);
-	snd_dice_transaction_destroy(dice);
-	fw_unit_put(dice->unit);
-
-	mutex_destroy(&dice->mutex);
+	dice_free(card->private_data);
 }
 
-static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
+static void do_registration(struct work_struct *work)
 {
-	struct snd_card *card;
-	struct snd_dice *dice;
+	struct snd_dice *dice = container_of(work, struct snd_dice, dwork.work);
 	int err;
 
-	err = check_dice_category(unit);
-	if (err < 0)
-		return -ENODEV;
+	if (dice->registered)
+		return;
 
-	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
-			   sizeof(*dice), &card);
+	err = snd_card_new(&dice->unit->device, -1, NULL, THIS_MODULE, 0,
+			   &dice->card);
 	if (err < 0)
-		goto end;
-
-	dice = card->private_data;
-	dice->card = card;
-	dice->unit = fw_unit_get(unit);
-	card->private_free = dice_card_free;
-
-	spin_lock_init(&dice->lock);
-	mutex_init(&dice->mutex);
-	init_completion(&dice->clock_accepted);
-	init_waitqueue_head(&dice->hwdep_wait);
+		return;
 
 	err = snd_dice_transaction_init(dice);
 	if (err < 0)
@@ -227,56 +221,131 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 
 	dice_card_strings(dice);
 
+	snd_dice_create_proc(dice);
+
 	err = snd_dice_create_pcm(dice);
 	if (err < 0)
 		goto error;
 
-	err = snd_dice_create_hwdep(dice);
+	err = snd_dice_create_midi(dice);
 	if (err < 0)
 		goto error;
 
-	snd_dice_create_proc(dice);
-
-	err = snd_dice_create_midi(dice);
+	err = snd_dice_create_hwdep(dice);
 	if (err < 0)
 		goto error;
 
-	err = snd_dice_stream_init_duplex(dice);
+	err = snd_card_register(dice->card);
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(card);
+	/*
+	 * After registered, dice instance can be released corresponding to
+	 * releasing the sound card instance.
+	 */
+	dice->card->private_free = dice_card_free;
+	dice->card->private_data = dice;
+	dice->registered = true;
+
+	return;
+error:
+	snd_dice_transaction_destroy(dice);
+	snd_card_free(dice->card);
+	dev_info(&dice->unit->device,
+		 "Sound card registration failed: %d\n", err);
+}
+
+static void schedule_registration(struct snd_dice *dice)
+{
+	struct fw_card *fw_card = fw_parent_device(dice->unit)->card;
+	u64 now, delay;
+
+	now = get_jiffies_64();
+	delay = fw_card->reset_jiffies + msecs_to_jiffies(PROBE_DELAY_MS);
+
+	if (time_after64(delay, now))
+		delay -= now;
+	else
+		delay = 0;
+
+	mod_delayed_work(system_wq, &dice->dwork, delay);
+}
+
+static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
+{
+	struct snd_dice *dice;
+	int err;
+
+	err = check_dice_category(unit);
+	if (err < 0)
+		return -ENODEV;
+
+	/* Allocate this independent of sound card instance. */
+	dice = kzalloc(sizeof(struct snd_dice), GFP_KERNEL);
+	if (dice == NULL)
+		return -ENOMEM;
+
+	dice->unit = fw_unit_get(unit);
+	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_stream_init_duplex(dice);
 	if (err < 0) {
-		snd_dice_stream_destroy_duplex(dice);
-		goto error;
+		dice_free(dice);
+		return err;
 	}
 
-	dev_set_drvdata(&unit->device, dice);
-end:
-	return err;
-error:
-	snd_card_free(card);
-	return err;
+	/* Allocate and register this sound card later. */
+	INIT_DEFERRABLE_WORK(&dice->dwork, do_registration);
+	schedule_registration(dice);
+
+	return 0;
 }
 
 static void dice_remove(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
-	/* No need to wait for releasing card object in this context. */
-	snd_card_free_when_closed(dice->card);
+	/*
+	 * Confirm to stop the work for registration before the sound card is
+	 * going to be released. The work is not scheduled again because bus
+	 * reset handler is not called anymore.
+	 */
+	cancel_delayed_work_sync(&dice->dwork);
+
+	if (dice->registered) {
+		/* No need to wait for releasing card object in this context. */
+		snd_card_free_when_closed(dice->card);
+	} else {
+		/* Don't forget this case. */
+		dice_free(dice);
+	}
 }
 
 static void dice_bus_reset(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
+	/* Postpone a workqueue for deferred registration. */
+	if (!dice->registered)
+		schedule_registration(dice);
+
 	/* The handler address register becomes initialized. */
 	snd_dice_transaction_reinit(dice);
 
-	mutex_lock(&dice->mutex);
-	snd_dice_stream_update_duplex(dice);
-	mutex_unlock(&dice->mutex);
+	/*
+	 * After registration, userspace can start packet streaming, then this
+	 * code block works fine.
+	 */
+	if (dice->registered) {
+		mutex_lock(&dice->mutex);
+		snd_dice_stream_update_duplex(dice);
+		mutex_unlock(&dice->mutex);
+	}
 }
 
 #define DICE_INTERFACE	0x000001
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 101550ac..3d5ebeb 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 registered;
+	struct delayed_work dwork;
+
 	/* Offsets for sub-addresses */
 	unsigned int global_offset;
 	unsigned int rx_offset;
-- 
2.5.0

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

* [PATCH 3/4] ALSA: dice: purge transaction initialization at timeout of Dice notification
  2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 1/4] ALSA: dice: split subaddress check from category check Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 2/4] ALSA: dice: postpone card registration Takashi Sakamoto
@ 2015-12-31  4:58 ` Takashi Sakamoto
  2015-12-31  4:58 ` [PATCH 4/4] ALSA: dice: expand timeout to wait for " Takashi Sakamoto
  2016-01-06  9:19 ` [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Iwai
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Sakamoto @ 2015-12-31  4:58 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stefanr, ffado-devel

In previous commit, card registration is processed under situation
with few bus reset. There's no need to add a workaround of transaction
re-initialization at timeout.

This commit purges the re-initialization.

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

diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index fdb7841..55c1fbf 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -65,16 +65,15 @@ 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 i;
 	__be32 info;
 	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 +86,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 +101,13 @@ retry:
 	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
 						&info, 4);
 	if (err < 0)
-		goto end;
+		return err;
 
-	/* 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);
-		if (err < 0)
-			goto end;
+			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0)
+		return -ETIMEDOUT;
 
-		msleep(500);	/* arbitrary */
-		goto retry;
-	}
-end:
-	return err;
+	return 0;
 }
 
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
-- 
2.5.0

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

* [PATCH 4/4] ALSA: dice: expand timeout to wait for Dice notification
  2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2015-12-31  4:58 ` [PATCH 3/4] ALSA: dice: purge transaction initialization at timeout of Dice notification Takashi Sakamoto
@ 2015-12-31  4:58 ` Takashi Sakamoto
  2016-01-06  9:19 ` [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Iwai
  4 siblings, 0 replies; 10+ messages in thread
From: Takashi Sakamoto @ 2015-12-31  4:58 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, stefanr, ffado-devel

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 55c1fbf..a4ff4e0 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

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

* Re: [PATCH 0/4 v5] ALSA: dice: improve card registration processing
  2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2015-12-31  4:58 ` [PATCH 4/4] ALSA: dice: expand timeout to wait for " Takashi Sakamoto
@ 2016-01-06  9:19 ` Takashi Iwai
  2016-01-06 10:50   ` Takashi Sakamoto
  4 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2016-01-06  9:19 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, stefanr, clemens, ffado-devel

On Thu, 31 Dec 2015 05:58:10 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset updates my previous one:
> [alsa-devel] [PATCH 0/5 v4] ALSA: dice: improve card registration processing
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/102504.html
> 
> Changes:
>  * Merge patch 02 and 03 in v4.
>  * Allocate sound card instance in sound card registration work, independently
>    of dice instance, allocated in .probe callback and set as driver_data of
>    device structure.
>  * Use __be32 type for __be32 data.
> 
> In this version, sound card instance is not allocated in .probe callback. In
> the callback, dice instance is allocated independently of the card instance.
> Thus, in .probe callback, this driver reserve no slot of sound card. The
> allocation of sound card instance is performed in a scheduled work. When
> processing in the work fails, the work can be re-scheduled again according to
> bus reset.

Applied now all four patches.  Thanks.


Takashi

> 
> Takashi Sakamoto (4):
>   ALSA: dice: split subaddress check from category check
>   ALSA: dice: postpone card registration
>   ALSA: dice: purge transaction initialization at timeout of Dice
>     notification
>   ALSA: dice: expand timeout to wait for Dice notification
> 
>  sound/firewire/dice/dice-transaction.c | 123 +++++++++++-------
>  sound/firewire/dice/dice.c             | 227 +++++++++++++++++----------------
>  sound/firewire/dice/dice.h             |   3 +
>  3 files changed, 202 insertions(+), 151 deletions(-)
> 
> -- 
> 2.5.0
> 

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

* Re: [PATCH 0/4 v5] ALSA: dice: improve card registration processing
  2016-01-06  9:19 ` [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Iwai
@ 2016-01-06 10:50   ` Takashi Sakamoto
  2016-01-06 13:04     ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Sakamoto @ 2016-01-06 10:50 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, stefanr, clemens, ffado-devel

Hi,

On Jan 06 2016 18:19, Takashi Iwai wrote:
> On Thu, 31 Dec 2015 05:58:10 +0100,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> This patchset updates my previous one:
>> [alsa-devel] [PATCH 0/5 v4] ALSA: dice: improve card registration processing
>> a
>>
>> Changes:
>>  * Merge patch 02 and 03 in v4.
>>  * Allocate sound card instance in sound card registration work, independently
>>    of dice instance, allocated in .probe callback and set as driver_data of
>>    device structure.
>>  * Use __be32 type for __be32 data.
>>
>> In this version, sound card instance is not allocated in .probe callback. In
>> the callback, dice instance is allocated independently of the card instance.
>> Thus, in .probe callback, this driver reserve no slot of sound card. The
>> allocation of sound card instance is performed in a scheduled work. When
>> processing in the work fails, the work can be re-scheduled again according to
>> bus reset.
> 
> Applied now all four patches.  Thanks.

OK. Thanks.

How about the rest of my work for ALSA Dice driver to Linux 4.5?

[alsa-devel] [RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html

It takes almost one month since I posted the RFCv2. We already had a
discussion about the patchset in RFCv1 and in my opinion there's no
thorough disagreement. If possible, I'd like to merge it in the merge
window for Linux 4.5. If we go for it, I'll post them in this night.


Have a nice year.

> Takashi
> 
>>
>> Takashi Sakamoto (4):
>>   ALSA: dice: split subaddress check from category check
>>   ALSA: dice: postpone card registration
>>   ALSA: dice: purge transaction initialization at timeout of Dice
>>     notification
>>   ALSA: dice: expand timeout to wait for Dice notification
>>
>>  sound/firewire/dice/dice-transaction.c | 123 +++++++++++-------
>>  sound/firewire/dice/dice.c             | 227 +++++++++++++++++----------------
>>  sound/firewire/dice/dice.h             |   3 +
>>  3 files changed, 202 insertions(+), 151 deletions(-)
>>
>> -- 
>> 2.5.0
>>

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

* Re: [PATCH 0/4 v5] ALSA: dice: improve card registration processing
  2016-01-06 10:50   ` Takashi Sakamoto
@ 2016-01-06 13:04     ` Takashi Iwai
  2016-01-07  0:21       ` Takashi Sakamoto
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2016-01-06 13:04 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, stefanr, clemens, ffado-devel

On Wed, 06 Jan 2016 11:50:01 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Jan 06 2016 18:19, Takashi Iwai wrote:
> > On Thu, 31 Dec 2015 05:58:10 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> This patchset updates my previous one:
> >> [alsa-devel] [PATCH 0/5 v4] ALSA: dice: improve card registration processing
> >> a
> >>
> >> Changes:
> >>  * Merge patch 02 and 03 in v4.
> >>  * Allocate sound card instance in sound card registration work, independently
> >>    of dice instance, allocated in .probe callback and set as driver_data of
> >>    device structure.
> >>  * Use __be32 type for __be32 data.
> >>
> >> In this version, sound card instance is not allocated in .probe callback. In
> >> the callback, dice instance is allocated independently of the card instance.
> >> Thus, in .probe callback, this driver reserve no slot of sound card. The
> >> allocation of sound card instance is performed in a scheduled work. When
> >> processing in the work fails, the work can be re-scheduled again according to
> >> bus reset.
> > 
> > Applied now all four patches.  Thanks.
> 
> OK. Thanks.
> 
> How about the rest of my work for ALSA Dice driver to Linux 4.5?
> 
> [alsa-devel] [RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html
> 
> It takes almost one month since I posted the RFCv2. We already had a
> discussion about the patchset in RFCv1 and in my opinion there's no
> thorough disagreement. If possible, I'd like to merge it in the merge
> window for Linux 4.5. If we go for it, I'll post them in this night.

Well, RFC is RFC and not for merging.  If it's meant for merging,
let's poke relevant people whether it's OK.


Takashi

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

* Re: [PATCH 0/4 v5] ALSA: dice: improve card registration processing
  2016-01-06 13:04     ` Takashi Iwai
@ 2016-01-07  0:21       ` Takashi Sakamoto
  2016-01-07  7:47         ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Sakamoto @ 2016-01-07  0:21 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, stefanr, clemens, ffado-devel

On Jan 6 2016 22:04, Takashi Iwai wrote:
>> How about the rest of my work for ALSA Dice driver to Linux 4.5?
>>
>> [alsa-devel] [RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html
>>
>> It takes almost one month since I posted the RFCv2. We already had a
>> discussion about the patchset in RFCv1 and in my opinion there's no
>> thorough disagreement. If possible, I'd like to merge it in the merge
>> window for Linux 4.5. If we go for it, I'll post them in this night.
> 
> Well, RFC is RFC and not for merging.  If it's meant for merging,
> let's poke relevant people whether it's OK.

Indeed for merging process.

However, in my opinion, RFC is one of ways to show directions for future
works and establish _loose_ agreement among active developers. In terms
of it, I think it's time to put the patchset to the review process for
merging.

But it seems to be a bit late for 4.5. I'll postpone them to next
developing cycle for 4.6.


For 4.5, I'll finish all of my work, and I currently plan for 4.6:
 * Limit PCM substreams to current sampling transfer frequency for Dice
driver
 * Replace tasklet with workqueue for scs1x functionality of OXFW driver.
 * Some works in userspace, mainly libhinawa and hinawa-utils
 * Development of firewire-transceiver module for testing streaming
engine of firewire-lib.


Regards

Takashi Sakamoto

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

* Re: [PATCH 0/4 v5] ALSA: dice: improve card registration processing
  2016-01-07  0:21       ` Takashi Sakamoto
@ 2016-01-07  7:47         ` Takashi Iwai
  0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2016-01-07  7:47 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, stefanr, clemens, ffado-devel

On Thu, 07 Jan 2016 01:21:44 +0100,
Takashi Sakamoto wrote:
> 
> On Jan 6 2016 22:04, Takashi Iwai wrote:
> >> How about the rest of my work for ALSA Dice driver to Linux 4.5?
> >>
> >> [alsa-devel] [RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
> >> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html
> >>
> >> It takes almost one month since I posted the RFCv2. We already had a
> >> discussion about the patchset in RFCv1 and in my opinion there's no
> >> thorough disagreement. If possible, I'd like to merge it in the merge
> >> window for Linux 4.5. If we go for it, I'll post them in this night.
> > 
> > Well, RFC is RFC and not for merging.  If it's meant for merging,
> > let's poke relevant people whether it's OK.
> 
> Indeed for merging process.
> 
> However, in my opinion, RFC is one of ways to show directions for future
> works and establish _loose_ agreement among active developers. In terms
> of it, I think it's time to put the patchset to the review process for
> merging.
> 
> But it seems to be a bit late for 4.5. I'll postpone them to next
> developing cycle for 4.6.

OK, then please resubmit the whole patchset after the next merge
window is closed.


Takashi

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

end of thread, other threads:[~2016-01-07  7:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-31  4:58 [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Sakamoto
2015-12-31  4:58 ` [PATCH 1/4] ALSA: dice: split subaddress check from category check Takashi Sakamoto
2015-12-31  4:58 ` [PATCH 2/4] ALSA: dice: postpone card registration Takashi Sakamoto
2015-12-31  4:58 ` [PATCH 3/4] ALSA: dice: purge transaction initialization at timeout of Dice notification Takashi Sakamoto
2015-12-31  4:58 ` [PATCH 4/4] ALSA: dice: expand timeout to wait for " Takashi Sakamoto
2016-01-06  9:19 ` [PATCH 0/4 v5] ALSA: dice: improve card registration processing Takashi Iwai
2016-01-06 10:50   ` Takashi Sakamoto
2016-01-06 13:04     ` Takashi Iwai
2016-01-07  0:21       ` Takashi Sakamoto
2016-01-07  7:47         ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.