[RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Takashi Sakamoto]

Hi,

This patchset is to update my previous one with mostly full features as an
driver in ALSA firewire stack:

[alsa-devel] [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101576.html

Changes from RFCv1:
 * Support own quirk of packet streaming
 * Add own data block processing layer
 * Support PCM functionality
 * Add proc node to help debugging
 * Add hwdep interface

I note that just powering on, the device is muted. Thus, the device generates no
sound even if packet streaming is successful. You can de-mute it by write
transaction. For example, by using 'firewire-request' command in
linux-firewire-utils(https://github.com/cladisch/linux-firewire-utils):

$ ./firewire-request /dev/fw1 write 0x0000801c0000 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000010000000100000001000000

Currently, libffado gives no way to do it via its mixer interface. Instead, done
in methods to start streaming. I think it better to move de-mute codes to mixer
initialization.

There's an issue of periodical hissing noise. I observed this with Fireface 400.
Interval of the noise differs depending on the situation (10-20 minutes) and
continues around 20 seconds. The cause is not clear but I've experienced similar
issue with TASCAM FireOne. A common point between these two models is
non-blocking transmission. I guess that the cause may be on data block
calculation in AMDTP packet streaming layer (calculate_data_blocks() function
in sound/firewire/amdtp-stream.c). Further investigation is required.

In my previous RFC, I described that zero bits have no effect in write
transactions to 0x00008010051c. But this is wrong. The transaction has
side effect to set the other options. In this patchset, the state of clock
is changed on the way to initialize transaction. This is a bit rude but
unavoidable. Fortunately, it's software's responsibility to read from flash
memory and set initial configuration, thus I think this change is approval
because just powering on the state of device is against user's configuration.
The loading and configuring are still userspace responsibility.

MIDI functionality can be disabled when any userspace applications run with
libffado, as I described. This is a bug of libffado due to node ID handling.
In line 199 of libffado/src/rme/fireface_hw.cpp, node id is handled as
uint16_t value. This is a fashion of libffado to represent Node ID by lower 6
bits of actual Node ID of IEEE 1394 bus. In the other lines, actual Node Id is
constructed with 0xffc0. Just using the raw value returned by
ConfigRom::getNodeId() is invalid.


For this work, I can borrow test device (Fireface 400) from Syntax Japan Inc.
It's my fortune to have an opportunity to contact to the company. I'm graceful
to Takeshi Mitsuhashi, Tahiro Hashizume and their kindness. Thank you.


Takashi Sakamoto (11):
  fireface: add skeleton for RME Fireface series
  fireface: postpone sound card registration
  fireface: add model specific structure
  fireface: add transaction support
  fireface: add support for MIDI functionality
  fireface: add proc node to help debugging
  firewire-lib: add no-header packet processing
  fireface: add data processing layer
  fireface: add stream management functionality
  fireface: add PCM functionality
  fireface: add hwdep interface

 include/uapi/sound/asound.h                    |   3 +-
 include/uapi/sound/firewire.h                  |   1 +
 sound/firewire/Kconfig                         |   7 +
 sound/firewire/Makefile                        |   1 +
 sound/firewire/amdtp-stream.c                  | 104 +++++--
 sound/firewire/amdtp-stream.h                  |   2 +
 sound/firewire/fireface/Makefile               |   4 +
 sound/firewire/fireface/amdtp-ff.c             | 221 ++++++++++++++
 sound/firewire/fireface/fireface-hwdep.c       | 191 ++++++++++++
 sound/firewire/fireface/fireface-midi.c        | 131 ++++++++
 sound/firewire/fireface/fireface-pcm.c         | 390 ++++++++++++++++++++++++
 sound/firewire/fireface/fireface-proc.c        | 237 +++++++++++++++
 sound/firewire/fireface/fireface-stream.c      | 397 +++++++++++++++++++++++++
 sound/firewire/fireface/fireface-transaction.c | 301 +++++++++++++++++++
 sound/firewire/fireface/fireface.c             | 210 +++++++++++++
 sound/firewire/fireface/fireface.h             | 131 ++++++++
 16 files changed, 2313 insertions(+), 18 deletions(-)
 create mode 100644 sound/firewire/fireface/Makefile
 create mode 100644 sound/firewire/fireface/amdtp-ff.c
 create mode 100644 sound/firewire/fireface/fireface-hwdep.c
 create mode 100644 sound/firewire/fireface/fireface-midi.c
 create mode 100644 sound/firewire/fireface/fireface-pcm.c
 create mode 100644 sound/firewire/fireface/fireface-proc.c
 create mode 100644 sound/firewire/fireface/fireface-stream.c
 create mode 100644 sound/firewire/fireface/fireface-transaction.c
 create mode 100644 sound/firewire/fireface/fireface.c
 create mode 100644 sound/firewire/fireface/fireface.h

-- 
2.5.0

[PATCH 01/11] fireface: add skeleton for RME Fireface series

[Takashi Sakamoto]

This commit adds a new driver for RME Fireface series. This commit just
creates/removes card instance according to IEEE 1394 bus event. More
functions will be added in following commits.

Three types of firmware have released by RME GmbH; for Fireface 400, for
Fireface 800 and for UCX/802/UFX. It's reasonable that these models use
different protocol for communication. Currently, I've investigated
Fireface 400 and nothing others.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/Kconfig             |   7 +++
 sound/firewire/Makefile            |   1 +
 sound/firewire/fireface/Makefile   |   2 +
 sound/firewire/fireface/fireface.c | 124 +++++++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.h |  39 ++++++++++++
 5 files changed, 173 insertions(+)
 create mode 100644 sound/firewire/fireface/Makefile
 create mode 100644 sound/firewire/fireface/fireface.c
 create mode 100644 sound/firewire/fireface/fireface.h

diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index e92a6d9..0eef3b4 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -148,4 +148,11 @@ config SND_FIREWIRE_TASCAM
 	 To compile this driver as a module, choose M here: the module
 	 will be called snd-firewire-tascam.
 
+config SND_FIREFACE
+	tristate "RME Fireface series support"
+	select SND_FIREWIRE_LIB
+	help
+	 Say Y here to include support for RME fireface series.
+	  * Fireface 400
+
 endif # SND_FIREWIRE
diff --git a/sound/firewire/Makefile b/sound/firewire/Makefile
index f5fb625..8cd4d2d 100644
--- a/sound/firewire/Makefile
+++ b/sound/firewire/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_SND_FIREWORKS) += fireworks/
 obj-$(CONFIG_SND_BEBOB) += bebob/
 obj-$(CONFIG_SND_FIREWIRE_DIGI00X) += digi00x/
 obj-$(CONFIG_SND_FIREWIRE_TASCAM) += tascam/
+obj-$(CONFIG_SND_FIREFACE) += fireface/
diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
new file mode 100644
index 0000000..f7113ab
--- /dev/null
+++ b/sound/firewire/fireface/Makefile
@@ -0,0 +1,2 @@
+snd-fireface-objs := fireface.o
+obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
new file mode 100644
index 0000000..c8b7fe7
--- /dev/null
+++ b/sound/firewire/fireface/fireface.c
@@ -0,0 +1,124 @@
+/*
+ * fireface.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "fireface.h"
+
+#define OUI_RME	0x000a35
+
+MODULE_DESCRIPTION("RME Fireface series Driver");
+MODULE_AUTHOR("Takashi Sakamoto <o-takashi@sakamocchi.jp>");
+MODULE_LICENSE("GPL v2");
+
+static void name_card(struct snd_ff *ff)
+{
+	struct fw_device *fw_dev = fw_parent_device(ff->unit);
+	const char *const model = "Fireface 400";
+
+	strcpy(ff->card->driver, "Fireface");
+	strcpy(ff->card->shortname, model);
+	strcpy(ff->card->mixername, model);
+	snprintf(ff->card->longname, sizeof(ff->card->longname),
+		 "RME %s, GUID %08x%08x at %s, S%d", model,
+		 fw_dev->config_rom[3], fw_dev->config_rom[4],
+		 dev_name(&ff->unit->device), 100 << fw_dev->max_speed);
+}
+
+static void ff_card_free(struct snd_card *card)
+{
+	struct snd_ff *ff = card->private_data;
+
+	fw_unit_put(ff->unit);
+
+	mutex_destroy(&ff->mutex);
+}
+
+static int snd_ff_probe(struct fw_unit *unit,
+			   const struct ieee1394_device_id *entry)
+{
+	struct snd_card *card;
+	struct snd_ff *ff;
+	int err;
+
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
+			   sizeof(struct snd_ff), &card);
+	if (err < 0)
+		return err;
+	card->private_free = ff_card_free;
+
+	/* initialize myself */
+	ff = card->private_data;
+	ff->card = card;
+	ff->unit = fw_unit_get(unit);
+
+	mutex_init(&ff->mutex);
+	dev_set_drvdata(&unit->device, ff);
+
+	name_card(ff);
+
+	err = snd_card_register(card);
+	if (err < 0) {
+		snd_card_free(card);
+		return err;
+	}
+
+	return 0;
+}
+
+static void snd_ff_update(struct fw_unit *unit)
+{
+	return;
+}
+
+static void snd_ff_remove(struct fw_unit *unit)
+{
+	struct snd_ff *ff = dev_get_drvdata(&unit->device);
+
+	/* No need to wait for releasing card object in this context. */
+	snd_card_free_when_closed(ff->card);
+}
+
+static const struct ieee1394_device_id snd_ff_id_table[] = {
+	/* Fireface 400 */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_SPECIFIER_ID |
+				  IEEE1394_MATCH_VERSION |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_RME,
+		.specifier_id	= 0x000a35,
+		.version	= 0x000002,
+		.model_id	= 0x101800,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(ieee1394, snd_ff_id_table);
+
+static struct fw_driver ff_driver = {
+	.driver = {
+		.owner	= THIS_MODULE,
+		.name	= "snd-fireface",
+		.bus	= &fw_bus_type,
+	},
+	.probe    = snd_ff_probe,
+	.update   = snd_ff_update,
+	.remove   = snd_ff_remove,
+	.id_table = snd_ff_id_table,
+};
+
+static int __init snd_ff_init(void)
+{
+	return driver_register(&ff_driver.driver);
+}
+
+static void __exit snd_ff_exit(void)
+{
+	driver_unregister(&ff_driver.driver);
+}
+
+module_init(snd_ff_init);
+module_exit(snd_ff_exit);
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
new file mode 100644
index 0000000..ab596c7
--- /dev/null
+++ b/sound/firewire/fireface/fireface.h
@@ -0,0 +1,39 @@
+/*
+ * fireface.h - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#ifndef SOUND_FIREFACE_H_INCLUDED
+#define SOUND_FIREFACE_H_INCLUDED
+
+#include <linux/device.h>
+#include <linux/firewire.h>
+#include <linux/firewire-constants.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/compat.h>
+
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/info.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/firewire.h>
+#include <sound/hwdep.h>
+#include <sound/rawmidi.h>
+
+#include "../lib.h"
+#include "../amdtp-stream.h"
+#include "../iso-resources.h"
+
+struct snd_ff {
+	struct snd_card *card;
+	struct fw_unit *unit;
+	struct mutex mutex;
+};
+#endif
-- 
2.5.0

[PATCH 02/11] fireface: postpone sound card registration

[Takashi Sakamoto]

Just after appearing on IEEE 1394 bus, this unit generates several bus
resets. This is due to loading firmware from on-board flash memory and
initialize hardware. It's better to postpone sound card registration.

This commit schedules workqueue to process actual probe processing
1 seconds after the last bus-reset. The card instance is kept at unit
probe callback and released at card free callback. Therefore, when the
actual probe processing fails, the memory block is wasted. This is due to
simplify driver implementation.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/fireface.c | 59 ++++++++++++++++++++++++++++++++++----
 sound/firewire/fireface/fireface.h |  3 ++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index c8b7fe7..cf10e97 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -10,6 +10,8 @@
 
 #define OUI_RME	0x000a35
 
+#define PROBE_DELAY_MS		(1 * MSEC_PER_SEC)
+
 MODULE_DESCRIPTION("RME Fireface series Driver");
 MODULE_AUTHOR("Takashi Sakamoto <o-takashi@sakamocchi.jp>");
 MODULE_LICENSE("GPL v2");
@@ -32,16 +34,47 @@ static void ff_card_free(struct snd_card *card)
 {
 	struct snd_ff *ff = card->private_data;
 
+	/* The workqueue for registration uses the memory block. */
+	cancel_work_sync(&ff->dwork.work);
+
 	fw_unit_put(ff->unit);
 
 	mutex_destroy(&ff->mutex);
 }
 
+static void do_probe(struct work_struct *work)
+{
+	struct snd_ff *ff = container_of(work, struct snd_ff, dwork.work);
+	int err;
+
+	mutex_lock(&ff->mutex);
+
+	if (ff->card->shutdown || ff->probed)
+		goto end;
+
+	err = snd_card_register(ff->card);
+	if (err < 0)
+		goto end;
+
+	ff->probed = true;
+end:
+	mutex_unlock(&ff->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 snd_ff_probe(struct fw_unit *unit,
 			   const struct ieee1394_device_id *entry)
 {
+	struct fw_card *fw_card = fw_parent_device(unit)->card;
 	struct snd_card *card;
 	struct snd_ff *ff;
+	unsigned long delay;
 	int err;
 
 	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
@@ -60,26 +93,40 @@ static int snd_ff_probe(struct fw_unit *unit,
 
 	name_card(ff);
 
-	err = snd_card_register(card);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	/* Register this sound card later. */
+	INIT_DEFERRABLE_WORK(&ff->dwork, do_probe);
+	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
+				fw_card->reset_jiffies - get_jiffies_64();
+	schedule_delayed_work(&ff->dwork, delay);
 
 	return 0;
 }
 
 static void snd_ff_update(struct fw_unit *unit)
 {
-	return;
+	struct snd_ff *ff = 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 (!ff->probed) {
+		delay = msecs_to_jiffies(PROBE_DELAY_MS) -
+				(get_jiffies_64() - fw_card->reset_jiffies);
+		mod_delayed_work(ff->dwork.wq, &ff->dwork, delay);
+	}
 }
 
 static void snd_ff_remove(struct fw_unit *unit)
 {
 	struct snd_ff *ff = dev_get_drvdata(&unit->device);
 
+	/* For a race condition of struct snd_card.shutdown. */
+	mutex_lock(&ff->mutex);
+
 	/* No need to wait for releasing card object in this context. */
 	snd_card_free_when_closed(ff->card);
+
+	mutex_unlock(&ff->mutex);
 }
 
 static const struct ieee1394_device_id snd_ff_id_table[] = {
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index ab596c7..74877ef 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -35,5 +35,8 @@ struct snd_ff {
 	struct snd_card *card;
 	struct fw_unit *unit;
 	struct mutex mutex;
+
+	bool probed;
+	struct delayed_work dwork;
 };
 #endif
-- 
2.5.0

[PATCH 03/11] fireface: add model specific structure

[Takashi Sakamoto]

RME Fireface series has several models and their specifications are
different. Currently, we find no way to retrieve the specification
from actual devices.

This commit adds a structure to describe model specific data.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/fireface.c | 13 +++++++++----
 sound/firewire/fireface/fireface.h |  6 ++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index cf10e97..f96913b 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -16,16 +16,19 @@ MODULE_DESCRIPTION("RME Fireface series Driver");
 MODULE_AUTHOR("Takashi Sakamoto <o-takashi@sakamocchi.jp>");
 MODULE_LICENSE("GPL v2");
 
+struct snd_ff_spec spec_ff400 = {
+	.name = "Fireface400",
+};
+
 static void name_card(struct snd_ff *ff)
 {
 	struct fw_device *fw_dev = fw_parent_device(ff->unit);
-	const char *const model = "Fireface 400";
 
 	strcpy(ff->card->driver, "Fireface");
-	strcpy(ff->card->shortname, model);
-	strcpy(ff->card->mixername, model);
+	strcpy(ff->card->shortname, ff->spec->name);
+	strcpy(ff->card->mixername, ff->spec->name);
 	snprintf(ff->card->longname, sizeof(ff->card->longname),
-		 "RME %s, GUID %08x%08x at %s, S%d", model,
+		 "RME %s, GUID %08x%08x at %s, S%d", ff->spec->name,
 		 fw_dev->config_rom[3], fw_dev->config_rom[4],
 		 dev_name(&ff->unit->device), 100 << fw_dev->max_speed);
 }
@@ -91,6 +94,7 @@ static int snd_ff_probe(struct fw_unit *unit,
 	mutex_init(&ff->mutex);
 	dev_set_drvdata(&unit->device, ff);
 
+	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
 	name_card(ff);
 
 	/* Register this sound card later. */
@@ -140,6 +144,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= 0x000a35,
 		.version	= 0x000002,
 		.model_id	= 0x101800,
+		.driver_data	= (kernel_ulong_t)&spec_ff400,
 	},
 	{}
 };
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 74877ef..91e924c 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -31,6 +31,10 @@
 #include "../amdtp-stream.h"
 #include "../iso-resources.h"
 
+struct snd_ff_spec {
+	const char *const name;
+};
+
 struct snd_ff {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -38,5 +42,7 @@ struct snd_ff {
 
 	bool probed;
 	struct delayed_work dwork;
+
+	const struct snd_ff_spec *spec;
 };
 #endif
-- 
2.5.0

[PATCH 04/11] fireface: add transaction support

[Takashi Sakamoto]

RME Fireface series doesn't transfer MIDI messages on isochronous packets.
The messages are transferred in asynchronous transactions.

The unit handles MIDI messages in asynchronous transactions transmitted in
two addresses; 0x000008018000 and 0x000008019000. These two are
corresponding to physical MIDI port 1 and 2.

The unit transfers MIDI messages by asynchronous transactions to certain
addresses. Drivers can decide upper 4 byte of the addresses by a write
transaction to 0x0000801003f4. Drivers have four options to the rest;
0x000000000, 0x00000080, 0x00000100, 0x00000180. Drivers can enable them
by write transactions to 0x00008010051c. This register may consist of
bit flags. In this mechanism, drivers cannot use Private space of IEEE 1212
in which they allow to add some side effects to read/write transactions.

This commit adds transaction support for MIDI messaging. To receive
asynchronous transactions, the driver allocates a range of address in
Memory space. The driver selects 0x00000000 as lower 4 byte of the
address, thus the driver retries to allocate when gaining unusable range.

Writing non-zero value to enable the transaction has side effects. This
commit write 0x0400001f for decide divice's initial state.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile               |   2 +-
 sound/firewire/fireface/fireface-transaction.c | 301 +++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c             |   8 +
 sound/firewire/fireface/fireface.h             |  30 +++
 4 files changed, 340 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/fireface-transaction.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index f7113ab..aa52e41 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,2 +1,2 @@
-snd-fireface-objs := fireface.o
+snd-fireface-objs := fireface.o fireface-transaction.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-transaction.c b/sound/firewire/fireface/fireface-transaction.c
new file mode 100644
index 0000000..8365cb4
--- /dev/null
+++ b/sound/firewire/fireface/fireface-transaction.c
@@ -0,0 +1,301 @@
+/*
+ * fireface-transaction.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "fireface.h"
+
+static void finish_transmit_midi_msg(struct snd_ff *ff, unsigned int port,
+				     int rcode)
+{
+	struct snd_rawmidi_substream *substream =
+				ACCESS_ONCE(ff->rx_midi_substreams[port]);
+
+	if (rcode_is_permanent_error(rcode)) {
+		ff->rx_midi_error[port] = true;
+		return;
+	}
+
+	if (rcode != RCODE_COMPLETE) {
+		/* Transfer the message again, immediately. */
+		ff->next_ktime[port] = ktime_set(0, 0);
+		schedule_work(&ff->rx_midi_work[port]);
+		return;
+	}
+
+	snd_rawmidi_transmit_ack(substream, ff->rx_bytes[port]);
+	ff->rx_bytes[port] = 0;
+
+	if (!snd_rawmidi_transmit_empty(substream))
+		schedule_work(&ff->rx_midi_work[port]);
+}
+
+static void finish_transmit_midi0_msg(struct fw_card *card, int rcode,
+				      void *data, size_t length,
+				      void *callback_data)
+{
+	struct snd_ff *ff =
+		container_of(callback_data, struct snd_ff, transactions[0]);
+	finish_transmit_midi_msg(ff, 0, rcode);
+}
+
+static void finish_transmit_midi1_msg(struct fw_card *card, int rcode,
+				      void *data, size_t length,
+				      void *callback_data)
+{
+	struct snd_ff *ff =
+		container_of(callback_data, struct snd_ff, transactions[1]);
+	finish_transmit_midi_msg(ff, 1, rcode);
+}
+
+static inline void fill_midi_buf(struct snd_ff *ff, unsigned int port,
+				 unsigned int index, u8 byte)
+{
+	ff->msg_buf[port][index] = cpu_to_le32(byte);
+}
+
+static void transmit_midi_msg(struct snd_ff *ff, unsigned int port)
+{
+	struct snd_rawmidi_substream *substream =
+			ACCESS_ONCE(ff->rx_midi_substreams[port]);
+	u8 *buf = (u8 *)ff->msg_buf[port];
+	int i, len;
+
+	struct fw_device *fw_dev = fw_parent_device(ff->unit);
+	unsigned long long addr;
+	int generation;
+	fw_transaction_callback_t callback;
+
+	if (substream == NULL || snd_rawmidi_transmit_empty(substream))
+		return;
+
+	if (ff->rx_bytes[port] > 0 || ff->rx_midi_error[port])
+		return;
+
+	/* Do it in next chance. */
+	if (ktime_after(ff->next_ktime[port], ktime_get())) {
+		schedule_work(&ff->rx_midi_work[port]);
+		return;
+	}
+
+	len = snd_rawmidi_transmit_peek(substream, buf,
+					SND_FF_MAXIMIM_MIDI_QUADS);
+	if (len <= 0)
+		return;
+
+	for (i = len - 1; i >= 0; i--)
+		fill_midi_buf(ff, port, i, buf[i]);
+
+	if (port == 0) {
+		addr = SND_FF_ADDR_MIDI_RX_PORT_0;
+		callback = finish_transmit_midi0_msg;
+	} else {
+		addr = SND_FF_ADDR_MIDI_RX_PORT_1;
+		callback = finish_transmit_midi1_msg;
+	}
+
+	/* Set interval to next transaction. */
+	ff->next_ktime[port] = ktime_add_ns(ktime_get(),
+					    len * 8 * NSEC_PER_SEC / 31250);
+	ff->rx_bytes[port] = len;
+
+	/*
+	 * In Linux FireWire core, when generation is updated with memory
+	 * barrier, node id has already been updated. In this module, After
+	 * this smp_rmb(), load/store instructions to memory are completed.
+	 * Thus, both of generation and node id are available with recent
+	 * values. This is a light-serialization solution to handle bus reset
+	 * events on IEEE 1394 bus.
+	 */
+	generation = fw_dev->generation;
+	smp_rmb();
+	fw_send_request(fw_dev->card, &ff->transactions[port],
+			TCODE_WRITE_BLOCK_REQUEST,
+			fw_dev->node_id, generation, fw_dev->max_speed,
+			addr, &ff->msg_buf[port], len * 4,
+			callback, &ff->transactions[port]);
+}
+
+static void transmit_midi0_msg(struct work_struct *work)
+{
+	struct snd_ff *ff = container_of(work, struct snd_ff, rx_midi_work[0]);
+
+	transmit_midi_msg(ff, 0);
+}
+
+static void transmit_midi1_msg(struct work_struct *work)
+{
+	struct snd_ff *ff = container_of(work, struct snd_ff, rx_midi_work[1]);
+
+	transmit_midi_msg(ff, 1);
+}
+
+static void handle_midi_msg(struct fw_card *card, struct fw_request *request,
+			    int tcode, int destination, int source,
+			    int generation, unsigned long long offset,
+			    void *data, size_t length, void *callback_data)
+{
+	struct snd_ff *ff = callback_data;
+	__le32 *buf = data;
+	u32 quad;
+	u8 byte;
+	unsigned int index;
+	struct snd_rawmidi_substream *substream;
+	int i;
+
+	fw_send_response(card, request, RCODE_COMPLETE);
+
+	for (i = 0; i < length / 4; i++) {
+		quad = le32_to_cpu(buf[i]);
+
+		/* Message in first port. */
+		/*
+		 * This value may represent the index of this unit when the
+		 * same units are on the same IEEE 1394 bus. This driver don't
+		 * use it.
+		 */
+		index = (quad >> 8) & 0xff;
+		if (index > 0) {
+			substream = ACCESS_ONCE(ff->tx_midi_substreams[0]);
+			if (substream != NULL) {
+				byte = quad & 0xff;
+				snd_rawmidi_receive(substream, &byte, 1);
+			}
+		}
+
+		/* Message in second port. */
+		index = (quad >> 24) & 0xff;
+		if (index > 0) {
+			substream = ACCESS_ONCE(ff->tx_midi_substreams[1]);
+			if (substream != NULL) {
+				byte = (quad >> 16) & 0xff;
+				snd_rawmidi_receive(substream, &byte, 1);
+			}
+		}
+	}
+}
+
+static int allocate_own_address(struct snd_ff *ff, int i)
+{
+	struct fw_address_region midi_msg_region;
+	int err;
+
+	ff->async_handler.length = SND_FF_MAXIMIM_MIDI_QUADS * 4;
+	ff->async_handler.address_callback = handle_midi_msg;
+	ff->async_handler.callback_data = ff;
+
+	midi_msg_region.start = 0x000100000000 * i;
+	midi_msg_region.end = midi_msg_region.start + ff->async_handler.length;
+
+	err = fw_core_add_address_handler(&ff->async_handler, &midi_msg_region);
+	if (err >= 0) {
+		/* Controllers register this region of address. */
+		if (ff->async_handler.offset & 0x0000ffffffff) {
+			fw_core_remove_address_handler(&ff->async_handler);
+			err = -EAGAIN;
+		}
+	}
+
+	return err;
+}
+
+int snd_ff_transaction_reregister(struct snd_ff *ff)
+{
+	struct fw_card *fw_card = fw_parent_device(ff->unit)->card;
+	u32 addr;
+	__le32 reg;
+	int err;
+
+	/*
+	 * Controllers are allowed to register its node ID and upper 2 byte of
+	 * local address to listen asynchronous transactions. Rest of the local
+	 * address is decided later. This is the reason we should pay attension
+	 * to allocating the address.
+	 */
+	addr = (fw_card->node_id << 16) | (ff->async_handler.offset >> 32);
+	reg = cpu_to_le32(addr);
+	err = snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 SND_FF_ADDR_CONTROLLER_ADDR_HI,
+				 &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	/*
+	 * The 3rd-6th bits in MSB of this register are used to indicate fixed
+	 * offset of address to transfer asynchronous transaction.
+	 *  - 6th: 0x00000180
+	 *  - 5th: 0x00000100
+	 *  - 4th: 0x00000080
+	 *  - 3rd: 0x00000000
+	 *
+	 * Here, we use 3rd bit because controllers are not allowed to register
+	 * arbitrary address for this purpose.
+	 *
+	 * The 7th and 8th bits in MSB of this register are used to the other
+	 * purpose.
+	 *
+	 * The 1st and 2nd bits in LSB of this register are used to cancel
+	 * transferring asynchronous transactions. These two bits have the same
+	 * effect.
+	 *  - 1st/2nd: cancel transferring
+	 *
+	 * This transaction has a side effect to configure the other options.
+	 * The LSM of this value means:
+	 *  - 5th: control quadlet speed
+	 *  - 4th: control double speed
+	 *  - 3rd: control base frequency bit 1
+	 *  - 2nd: control base frequency bit 0
+	 *  - 1st: use internal clock source
+	 */
+	reg = cpu_to_le32(0x0400001f);
+	return snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				  SND_FF_ADDR_GENERAL_PARAMS,
+				  &reg, sizeof(reg), 0);
+}
+
+int snd_ff_transaction_register(struct snd_ff *ff)
+{
+	int i, err;
+
+	/*
+	 * Allocate in Memory Space of IEC 13213, but 8 byte in LSB should be
+	 * zero due to device specification.
+	 */
+	for (i = 0; i < 0xffff; i++) {
+		err = allocate_own_address(ff, i);
+		if (err != -EBUSY && err != -EAGAIN)
+			break;
+	}
+	if (err < 0)
+		return err;
+
+	err = snd_ff_transaction_reregister(ff);
+	if (err < 0)
+		return err;
+
+	INIT_WORK(&ff->rx_midi_work[0], transmit_midi0_msg);
+	INIT_WORK(&ff->rx_midi_work[1], transmit_midi1_msg);
+
+	return 0;
+}
+
+void snd_ff_transaction_unregister(struct snd_ff *ff)
+{
+	__le32 reg;
+
+	/* Stop transferring and listening. */
+	reg = cpu_to_le32(0x03000000);
+	snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+			  SND_FF_ADDR_GENERAL_PARAMS,
+			  &reg, sizeof(reg), 0);
+
+	reg = cpu_to_le32(0x00000000);
+	snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   SND_FF_ADDR_CONTROLLER_ADDR_HI,
+			   &reg, sizeof(reg), 0);
+
+	fw_core_remove_address_handler(&ff->async_handler);
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index f96913b..b3ee869 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -40,6 +40,8 @@ static void ff_card_free(struct snd_card *card)
 	/* The workqueue for registration uses the memory block. */
 	cancel_work_sync(&ff->dwork.work);
 
+	snd_ff_transaction_unregister(ff);
+
 	fw_unit_put(ff->unit);
 
 	mutex_destroy(&ff->mutex);
@@ -55,6 +57,10 @@ static void do_probe(struct work_struct *work)
 	if (ff->card->shutdown || ff->probed)
 		goto end;
 
+	err = snd_ff_transaction_register(ff);
+	if (err < 0)
+		goto end;
+
 	err = snd_card_register(ff->card);
 	if (err < 0)
 		goto end;
@@ -118,6 +124,8 @@ static void snd_ff_update(struct fw_unit *unit)
 				(get_jiffies_64() - fw_card->reset_jiffies);
 		mod_delayed_work(ff->dwork.wq, &ff->dwork, delay);
 	}
+
+	snd_ff_transaction_reregister(ff);
 }
 
 static void snd_ff_remove(struct fw_unit *unit)
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 91e924c..6d132f9 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -31,6 +31,10 @@
 #include "../amdtp-stream.h"
 #include "../iso-resources.h"
 
+#define SND_FF_MAXIMIM_MIDI_QUADS	9
+#define SND_FF_IN_MIDI_PORTS		2
+#define SND_FF_OUT_MIDI_PORTS		2
+
 struct snd_ff_spec {
 	const char *const name;
 };
@@ -44,5 +48,31 @@ struct snd_ff {
 	struct delayed_work dwork;
 
 	const struct snd_ff_spec *spec;
+
+	/* To handle MIDI tx. */
+	struct snd_rawmidi_substream *tx_midi_substreams[SND_FF_IN_MIDI_PORTS];
+	struct fw_address_handler async_handler;
+
+	/* TO handle MIDI rx. */
+	struct snd_rawmidi_substream *rx_midi_substreams[SND_FF_OUT_MIDI_PORTS];
+	u8 running_status[SND_FF_OUT_MIDI_PORTS];
+	__le32 msg_buf[SND_FF_OUT_MIDI_PORTS][SND_FF_MAXIMIM_MIDI_QUADS];
+	struct work_struct rx_midi_work[SND_FF_OUT_MIDI_PORTS];
+	struct fw_transaction transactions[SND_FF_OUT_MIDI_PORTS];
+	ktime_t next_ktime[SND_FF_OUT_MIDI_PORTS];
+	bool rx_midi_error[SND_FF_OUT_MIDI_PORTS];
+	unsigned int rx_bytes[SND_FF_OUT_MIDI_PORTS];
 };
+
+#define SND_FF_ADDR_CONTROLLER_ADDR_HI	0x0000801003f4
+#define SND_FF_ADDR_GENERAL_PARAMS	0x00008010051c
+#define SND_FF_ADDR_MIDI_RX_PORT_0	0x000080180000
+#define SND_FF_ADDR_MIDI_RX_PORT_1	0x000080190000
+
+#define SND_FF_ADDR_MIDI_TX		0x000100000000
+
+int snd_ff_transaction_register(struct snd_ff *ff);
+int snd_ff_transaction_reregister(struct snd_ff *ff);
+void snd_ff_transaction_unregister(struct snd_ff *ff);
+
 #endif
-- 
2.5.0

[PATCH 05/11] fireface: add support for MIDI functionality

[Takashi Sakamoto]

In previous commit, fireface driver supports transaction functionality.
This commit adds MIDI functionality for userspace.

Userspace applications can always disable this functionality by sending
invalid values to 0x0000801003f4 and 0x00008010051c by write transactions.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile        |   2 +-
 sound/firewire/fireface/fireface-midi.c | 131 ++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c      |   5 ++
 sound/firewire/fireface/fireface.h      |   3 +
 4 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/fireface-midi.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index aa52e41..2174b4b 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,2 +1,2 @@
-snd-fireface-objs := fireface.o fireface-transaction.o
+snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-midi.c b/sound/firewire/fireface/fireface-midi.c
new file mode 100644
index 0000000..d324f4e7
--- /dev/null
+++ b/sound/firewire/fireface/fireface-midi.c
@@ -0,0 +1,131 @@
+/*
+ * fireface-midi.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "fireface.h"
+
+static int midi_capture_open(struct snd_rawmidi_substream *substream)
+{
+	/* Do nothing. */
+	return 0;
+}
+
+static int midi_playback_open(struct snd_rawmidi_substream *substream)
+{
+	struct snd_ff *ff = substream->rmidi->private_data;
+
+	/* Initialize internal status. */
+	ff->running_status[substream->number] = 0;
+	ff->rx_midi_error[substream->number] = false;
+
+	ACCESS_ONCE(ff->rx_midi_substreams[substream->number]) = substream;
+
+	return 0;
+}
+
+static int midi_capture_close(struct snd_rawmidi_substream *substream)
+{
+	/* Do nothing. */
+	return 0;
+}
+
+static int midi_playback_close(struct snd_rawmidi_substream *substream)
+{
+	struct snd_ff *ff = substream->rmidi->private_data;
+
+	cancel_work_sync(&ff->rx_midi_work[substream->number]);
+	ACCESS_ONCE(ff->rx_midi_substreams[substream->number]) = NULL;
+
+	return 0;
+}
+
+static void midi_capture_trigger(struct snd_rawmidi_substream *substream,
+				 int up)
+{
+	struct snd_ff *ff = substream->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ff->lock, flags);
+
+	if (up)
+		ACCESS_ONCE(ff->tx_midi_substreams[substream->number]) =
+								substream;
+	else
+		ACCESS_ONCE(ff->tx_midi_substreams[substream->number]) = NULL;
+
+	spin_unlock_irqrestore(&ff->lock, flags);
+}
+
+static void midi_playback_trigger(struct snd_rawmidi_substream *substream,
+				  int up)
+{
+	struct snd_ff *ff = substream->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ff->lock, flags);
+
+	if (up || !ff->rx_midi_error[substream->number])
+		schedule_work(&ff->rx_midi_work[substream->number]);
+
+	spin_unlock_irqrestore(&ff->lock, flags);
+}
+
+static struct snd_rawmidi_ops midi_capture_ops = {
+	.open		= midi_capture_open,
+	.close		= midi_capture_close,
+	.trigger	= midi_capture_trigger,
+};
+
+static struct snd_rawmidi_ops midi_playback_ops = {
+	.open		= midi_playback_open,
+	.close		= midi_playback_close,
+	.trigger	= midi_playback_trigger,
+};
+
+static void set_midi_substream_names(struct snd_rawmidi_str *stream,
+				     const char *const name)
+{
+	struct snd_rawmidi_substream *substream;
+
+	list_for_each_entry(substream, &stream->substreams, list) {
+		snprintf(substream->name, sizeof(substream->name),
+			 "%s MIDI %d", name, substream->number + 1);
+	}
+}
+
+int snd_ff_create_midi_devices(struct snd_ff *ff)
+{
+	struct snd_rawmidi *rmidi;
+	struct snd_rawmidi_str *stream;
+	int err;
+
+	err = snd_rawmidi_new(ff->card, ff->card->driver, 0,
+			      SND_FF_OUT_MIDI_PORTS, SND_FF_IN_MIDI_PORTS,
+			      &rmidi);
+	if (err < 0)
+		return err;
+
+	snprintf(rmidi->name, sizeof(rmidi->name),
+		 "%s MIDI", ff->card->shortname);
+	rmidi->private_data = ff;
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_INPUT;
+	snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_INPUT,
+			    &midi_capture_ops);
+	stream = &rmidi->streams[SNDRV_RAWMIDI_STREAM_INPUT];
+	set_midi_substream_names(stream, ff->card->shortname);
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_OUTPUT;
+	snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT,
+			    &midi_playback_ops);
+	stream = &rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT];
+	set_midi_substream_names(stream, ff->card->shortname);
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_DUPLEX;
+
+	return 0;
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index b3ee869..0883cda 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -61,6 +61,10 @@ static void do_probe(struct work_struct *work)
 	if (err < 0)
 		goto end;
 
+	err = snd_ff_create_midi_devices(ff);
+	if (err < 0)
+		goto end;
+
 	err = snd_card_register(ff->card);
 	if (err < 0)
 		goto end;
@@ -98,6 +102,7 @@ static int snd_ff_probe(struct fw_unit *unit,
 	ff->unit = fw_unit_get(unit);
 
 	mutex_init(&ff->mutex);
+	spin_lock_init(&ff->lock);
 	dev_set_drvdata(&unit->device, ff);
 
 	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 6d132f9..33d94b8 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -43,6 +43,7 @@ struct snd_ff {
 	struct snd_card *card;
 	struct fw_unit *unit;
 	struct mutex mutex;
+	spinlock_t lock;
 
 	bool probed;
 	struct delayed_work dwork;
@@ -75,4 +76,6 @@ int snd_ff_transaction_register(struct snd_ff *ff);
 int snd_ff_transaction_reregister(struct snd_ff *ff);
 void snd_ff_transaction_unregister(struct snd_ff *ff);
 
+int snd_ff_create_midi_devices(struct snd_ff *ff);
+
 #endif
-- 
2.5.0

[PATCH 06/11] fireface: add proc node to help debugging

[Takashi Sakamoto]

Drivers retrieve the state and configuration of clock by read transactions.

This commit adds proc node to dump the information for debugging.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile        |   3 +-
 sound/firewire/fireface/fireface-proc.c | 237 ++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c      |   2 +
 sound/firewire/fireface/fireface.h      |   2 +
 4 files changed, 243 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/fireface-proc.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index 2174b4b..c9d1f4e 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,2 +1,3 @@
-snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o
+snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o \
+		     fireface-proc.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-proc.c b/sound/firewire/fireface/fireface-proc.c
new file mode 100644
index 0000000..d642cba
--- /dev/null
+++ b/sound/firewire/fireface/fireface-proc.c
@@ -0,0 +1,237 @@
+/*
+ * fireface-proc.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "./fireface.h"
+
+static void proc_read_clock_status(struct snd_info_entry *entry,
+				   struct snd_info_buffer *buffer)
+{
+	struct snd_ff *ff = entry->private_data;
+	__le32 reg;
+	u32 data;
+	int err;
+
+	err = snd_fw_transaction(ff->unit, TCODE_READ_QUADLET_REQUEST,
+				 0x0000801c0000, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return;
+
+	data = le32_to_cpu(reg);
+
+	snd_iprintf(buffer, "External source detection:\n");
+
+	snd_iprintf(buffer, "Word Clock:");
+	if ((data >> 24) & 0x20) {
+		if ((data >> 24) & 0x40)
+			snd_iprintf(buffer, "sync\n");
+		else
+			snd_iprintf(buffer, "lock\n");
+	} else {
+		snd_iprintf(buffer, "none\n");
+	}
+
+	snd_iprintf(buffer, "S/PDIF:");
+	if ((data >> 16) & 0x10) {
+		if ((data >> 16) & 0x04)
+			snd_iprintf(buffer, "sync\n");
+		else
+			snd_iprintf(buffer, "lock\n");
+	} else {
+		snd_iprintf(buffer, "none\n");
+	}
+
+	snd_iprintf(buffer, "ADAT:");
+	if ((data >> 8) & 0x04) {
+		if ((data >> 8) & 0x10)
+			snd_iprintf(buffer, "sync\n");
+		else
+			snd_iprintf(buffer, "lock\n");
+	} else {
+		snd_iprintf(buffer, "none\n");
+	}
+
+	snd_iprintf(buffer, "\nUsed external source:\n");
+
+	if (((data >> 22) & 0x07) == 0x07) {
+		snd_iprintf(buffer, "None\n");
+	} else {
+		switch ((data >> 22) & 0x07) {
+		case 0x00:
+			snd_iprintf(buffer, "ADAT:");
+			break;
+		case 0x03:
+			snd_iprintf(buffer, "S/PDIF:");
+			break;
+		case 0x04:
+			snd_iprintf(buffer, "Word:");
+			break;
+		case 0x07:
+			snd_iprintf(buffer, "Nothing:");
+			break;
+		case 0x01:
+		case 0x02:
+		case 0x05:
+		case 0x06:
+		default:
+			snd_iprintf(buffer, "unknown:");
+			break;
+		}
+
+		if ((data >> 25) & 0x07) {
+			switch ((data >> 25) & 0x07) {
+			case 0x01:
+				snd_iprintf(buffer, "32000\n");
+				break;
+			case 0x02:
+				snd_iprintf(buffer, "44100\n");
+				break;
+			case 0x03:
+				snd_iprintf(buffer, "48000\n");
+				break;
+			case 0x04:
+				snd_iprintf(buffer, "64000\n");
+				break;
+			case 0x05:
+				snd_iprintf(buffer, "88200\n");
+				break;
+			case 0x06:
+				snd_iprintf(buffer, "96000\n");
+				break;
+			case 0x07:
+				snd_iprintf(buffer, "128000\n");
+				break;
+			case 0x08:
+				snd_iprintf(buffer, "176400\n");
+				break;
+			case 0x09:
+				snd_iprintf(buffer, "192000\n");
+				break;
+			case 0x00:
+				snd_iprintf(buffer, "unknown\n");
+				break;
+			}
+		}
+	}
+
+	snd_iprintf(buffer, "Multiplied:");
+	snd_iprintf(buffer, "%d\n", (data & 0x3ff) * 250);
+}
+
+static void proc_read_clock_config(struct snd_info_entry *entry,
+				   struct snd_info_buffer *buffer)
+{
+	struct snd_ff *ff = entry->private_data;
+	__le32 reg;
+	u32 data;
+	unsigned int rate;
+	const char *src;
+	int err;
+
+	err = snd_fw_transaction(ff->unit, TCODE_READ_BLOCK_REQUEST,
+				 0x0000801c0004, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return;
+
+	data = le32_to_cpu(reg);
+
+	snd_iprintf(buffer, "Output S/PDIF format: %s (Emphasis: %s)\n",
+		    (data & 0x20) ? "Professional" : "Consumer",
+		    (data & 0x40) ? "on" : "off");
+
+	snd_iprintf(buffer, "Optical output interface format: %s\n",
+		    ((data >> 8) & 0x01) ? "S/PDIF" : "ADAT");
+
+	snd_iprintf(buffer, "Word output single speed: %s\n",
+		    ((data >> 8) & 0x20) ? "on" : "off");
+
+	snd_iprintf(buffer, "S/PDIF input interface: %s\n",
+		    ((data >> 8) & 0x02) ? "Optical" : "Coaxial");
+
+	switch ((data >> 1) & 0x03) {
+	case 0x01:
+		rate = 32000;
+		break;
+	case 0x00:
+		rate = 44100;
+		break;
+	case 0x03:
+		rate = 48000;
+		break;
+	case 0x02:
+	default:
+		return;
+	}
+
+	if (data & 0x08)
+		rate *= 2;
+	else if (data & 0x10)
+		rate *= 4;
+
+	snd_iprintf(buffer, "Sampling rate: %d\n", rate);
+
+	if (data & 0x01) {
+		src = "Internal";
+	} else {
+		switch ((data >> 10) & 0x07) {
+		case 0x00:
+			src = "ADAT";
+			break;
+		case 0x03:
+			src = "S/PDIF";
+			break;
+		case 0x04:
+			src = "Word";
+			break;
+		case 0x05:
+			src = "LTC";
+			break;
+		default:
+			return;
+		}
+	}
+
+	snd_iprintf(buffer, "Sync to clock source: %s\n", src);
+}
+
+static void add_node(struct snd_ff *ff, struct snd_info_entry *root,
+		     const char *name,
+		     void (*op)(struct snd_info_entry *e,
+				struct snd_info_buffer *b))
+{
+	struct snd_info_entry *entry;
+
+	entry = snd_info_create_card_entry(ff->card, name, root);
+	if (entry == NULL)
+		return;
+
+	snd_info_set_text_ops(entry, ff, op);
+	if (snd_info_register(entry) < 0)
+		snd_info_free_entry(entry);
+}
+
+void snd_ff_proc_init(struct snd_ff *ff)
+{
+	struct snd_info_entry *root;
+
+	/*
+	 * All nodes are automatically removed at snd_card_disconnect(),
+	 * by following to link list.
+	 */
+	root = snd_info_create_card_entry(ff->card, "firewire",
+					  ff->card->proc_root);
+	if (root == NULL)
+		return;
+	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	if (snd_info_register(root) < 0) {
+		snd_info_free_entry(root);
+		return;
+	}
+
+	add_node(ff, root, "clock-config", proc_read_clock_config);
+	add_node(ff, root, "clock-status", proc_read_clock_status);
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index 0883cda..2ea84ce 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -108,6 +108,8 @@ static int snd_ff_probe(struct fw_unit *unit,
 	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
 	name_card(ff);
 
+	snd_ff_proc_init(ff);
+
 	/* Register this sound card later. */
 	INIT_DEFERRABLE_WORK(&ff->dwork, do_probe);
 	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 33d94b8..38a200e 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -76,6 +76,8 @@ int snd_ff_transaction_register(struct snd_ff *ff);
 int snd_ff_transaction_reregister(struct snd_ff *ff);
 void snd_ff_transaction_unregister(struct snd_ff *ff);
 
+void snd_ff_proc_init(struct snd_ff *ff);
+
 int snd_ff_create_midi_devices(struct snd_ff *ff);
 
 #endif
-- 
2.5.0

[PATCH 07/11] firewire-lib: add no-header packet processing

[Takashi Sakamoto]

RME Fireface series doesn't apply IEC 61883-1/6. Its packet includes
no CIP headers. 64,0 and 128,0 kHz are supported. The device doesn't
transmit 8,000 packets per second. 0, 2, 3 are used as tag of received
isochronous packets.

On the other hand, there's a common feature. The number of data blocks
transferred in a second is the same as sampling transfer frequency.
Current AMDTP stream layer already has a method to calculate it.

This commit adds support for the transferring protocol. CIP_NO_HEADERS
flag is newly added. When this flag is set:
 * 0 and 1 are used as the tag.
 * Skip header evaluation.
 * Use another way to calculate the quadlets of isochronous packet payload.

In ALSA userspace interface, 128.0 kHz is not supported. And the AMDTP
stream layer doesn't support 64.0 kHz. These modes are dropped.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 104 +++++++++++++++++++++++++++++++++++-------
 sound/firewire/amdtp-stream.h |   2 +
 2 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index ed29026..e2c13f0 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -225,11 +225,15 @@ EXPORT_SYMBOL(amdtp_stream_set_parameters);
 unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s)
 {
 	unsigned int multiplier = 1;
+	unsigned int header_size = 0;
 
 	if (s->flags & CIP_JUMBO_PAYLOAD)
 		multiplier = 5;
+	if (!(s->flags & CIP_NO_HEADERS))
+		header_size = 8;
 
-	return 8 + s->syt_interval * s->data_block_quadlets * 4 * multiplier;
+	return header_size +
+		s->syt_interval * s->data_block_quadlets * 4 * multiplier;
 }
 EXPORT_SYMBOL(amdtp_stream_get_max_payload);
 
@@ -374,7 +378,10 @@ static int queue_packet(struct amdtp_stream *s,
 		goto end;
 
 	p.interrupt = IS_ALIGNED(s->packet_index + 1, INTERRUPT_INTERVAL);
-	p.tag = TAG_CIP;
+	if (s->flags & CIP_NO_HEADERS)
+		p.tag = 0;
+	else
+		p.tag = TAG_CIP;
 	p.header_length = header_length;
 	p.payload_length = (!skip) ? payload_length : 0;
 	p.skip = skip;
@@ -404,6 +411,52 @@ static inline int queue_in_packet(struct amdtp_stream *s)
 			    amdtp_stream_get_max_payload(s), false);
 }
 
+static int handle_out_packet_without_header(struct amdtp_stream *s,
+					    unsigned int data_blocks)
+{
+	__be32 *buffer;
+	unsigned int payload_length;
+	unsigned int pcm_frames;
+	struct snd_pcm_substream *pcm;
+
+	buffer = s->buffer.packets[s->packet_index].buffer;
+	pcm_frames = s->process_data_blocks(s, buffer, data_blocks, 0);
+
+	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
+
+	payload_length = data_blocks * 4 * s->data_block_quadlets;
+	if (queue_out_packet(s, payload_length, false) < 0)
+		return -EIO;
+
+	pcm = ACCESS_ONCE(s->pcm);
+	if (pcm && pcm_frames > 0)
+		update_pcm_pointers(s, pcm, pcm_frames);
+
+	/* No need to return the number of handled data blocks. */
+	return 0;
+}
+
+static int handle_in_packet_without_header(struct amdtp_stream *s,
+				unsigned int payload_quadlets, __be32 *buffer,
+				unsigned int *data_blocks)
+{
+	struct snd_pcm_substream *pcm;
+	unsigned int pcm_frames;
+
+	*data_blocks = payload_quadlets / s->data_block_quadlets;
+
+	pcm_frames = s->process_data_blocks(s, buffer, *data_blocks, 0);
+
+	if (queue_in_packet(s) < 0)
+		return -EIO;
+
+	pcm = ACCESS_ONCE(s->pcm);
+	if (pcm && pcm_frames > 0)
+		update_pcm_pointers(s, pcm, pcm_frames);
+
+	return 0;
+}
+
 static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks,
 			     unsigned int syt)
 {
@@ -566,10 +619,19 @@ static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
 		syt = calculate_syt(s, ++cycle);
 		data_blocks = calculate_data_blocks(s, syt);
 
-		if (handle_out_packet(s, data_blocks, syt) < 0) {
-			s->packet_index = -1;
-			amdtp_stream_pcm_abort(s);
-			return;
+		if (s->flags & CIP_NO_HEADERS) {
+			if (handle_out_packet_without_header(s,
+							data_blocks) < 0) {
+				s->packet_index = -1;
+				amdtp_stream_pcm_abort(s);
+				return;
+			}
+		} else {
+			if (handle_out_packet(s, data_blocks, syt) < 0) {
+				s->packet_index = -1;
+				amdtp_stream_pcm_abort(s);
+				return;
+			}
 		}
 	}
 
@@ -609,20 +671,28 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
 			break;
 		}
 
-		syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK;
-		if (handle_in_packet(s, payload_quadlets, buffer,
+		if (s->flags & CIP_NO_HEADERS) {
+			if (handle_in_packet_without_header(s, payload_quadlets,
+						buffer, &data_blocks) < 0) {
+				s->packet_index = -1;
+				break;
+			}
+		} else {
+			syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK;
+			if (handle_in_packet(s, payload_quadlets, buffer,
 						&data_blocks, syt) < 0) {
-			s->packet_index = -1;
-			break;
-		}
-
-		/* Process sync slave stream */
-		if (s->sync_slave && s->sync_slave->callbacked) {
-			if (handle_out_packet(s->sync_slave,
-					      data_blocks, syt) < 0) {
 				s->packet_index = -1;
 				break;
 			}
+
+			/* Process sync slave stream */
+			if (s->sync_slave && s->sync_slave->callbacked) {
+				if (handle_out_packet(s->sync_slave,
+						      data_blocks, syt) < 0) {
+					s->packet_index = -1;
+					break;
+				}
+			}
 		}
 	}
 
@@ -762,7 +832,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 
 	/* NOTE: TAG1 matches CIP. This just affects in stream. */
 	tag = FW_ISO_CONTEXT_MATCH_TAG1;
-	if (s->flags & CIP_EMPTY_WITH_TAG0)
+	if ((s->flags & CIP_EMPTY_WITH_TAG0) || (s->flags & CIP_NO_HEADERS))
 		tag |= FW_ISO_CONTEXT_MATCH_TAG0;
 
 	s->callbacked = false;
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index 8775704..5a3a613 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -33,6 +33,7 @@
  * @CIP_JUMBO_PAYLOAD: Only for in-stream. The number of data blocks in an
  *	packet is larger than IEC 61883-6 defines. Current implementation
  *	allows 5 times as large as IEC 61883-6 defines.
+ * @CIP_NO_HEADERS: a lack of headers in packets
  */
 enum cip_flags {
 	CIP_NONBLOCKING		= 0x00,
@@ -45,6 +46,7 @@ enum cip_flags {
 	CIP_SKIP_INIT_DBC_CHECK	= 0x40,
 	CIP_EMPTY_HAS_WRONG_DBC	= 0x80,
 	CIP_JUMBO_PAYLOAD	= 0x100,
+	CIP_NO_HEADERS		= 0x200,
 };
 
 /**
-- 
2.5.0

[PATCH 08/11] fireface: add data processing layer

[Takashi Sakamoto]

RME Fireface series doesn't apply IEC 61883-6 for its format of data block.
Its data channel includes no labels. The PCM sample is aligned to little
endian. The upper 1 byte of each channel includes information of device's
status.

This commit adds own data processing layer. The device's status is not
supported yet.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile   |   2 +-
 sound/firewire/fireface/amdtp-ff.c | 221 +++++++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.h |   8 ++
 3 files changed, 230 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/amdtp-ff.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index c9d1f4e..f22f0b5 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,3 +1,3 @@
 snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o \
-		     fireface-proc.o
+		     fireface-proc.o amdtp-ff.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/amdtp-ff.c b/sound/firewire/fireface/amdtp-ff.c
new file mode 100644
index 0000000..ab2a99d
--- /dev/null
+++ b/sound/firewire/fireface/amdtp-ff.c
@@ -0,0 +1,221 @@
+/*
+ * amdtp-ff.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include <sound/pcm.h>
+#include "fireface.h"
+
+struct amdtp_ff {
+	unsigned int pcm_channels;
+
+	void (*transfer_samples)(struct amdtp_stream *s,
+				 struct snd_pcm_substream *pcm,
+				 __be32 *buffer, unsigned int frames);
+};
+
+int amdtp_ff_set_parameters(struct amdtp_stream *s, unsigned int rate,
+			    unsigned int pcm_channels)
+{
+	struct amdtp_ff *p = s->protocol;
+	unsigned int data_channels;
+
+	if (amdtp_stream_running(s))
+		return -EBUSY;
+
+	p->pcm_channels = pcm_channels;
+	data_channels = pcm_channels;
+
+	return amdtp_stream_set_parameters(s, rate, data_channels);
+}
+
+static void write_pcm_s32(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames)
+{
+	struct amdtp_ff *p = s->protocol;
+	struct snd_pcm_runtime *runtime = pcm->runtime;
+	unsigned int channels, remaining_frames, i, c;
+	const u32 *src;
+
+	channels = p->pcm_channels;
+	src = (void *)runtime->dma_area +
+			frames_to_bytes(runtime, s->pcm_buffer_pointer);
+	remaining_frames = runtime->buffer_size - s->pcm_buffer_pointer;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < channels; ++c) {
+			/* TODO: fill upper 1 byte for the other aim. */
+			buffer[c] = cpu_to_le32(*src);
+			src++;
+		}
+		buffer += s->data_block_quadlets;
+		if (--remaining_frames == 0)
+			src = (void *)runtime->dma_area;
+	}
+}
+
+static void write_pcm_s16(struct amdtp_stream *s,
+			  struct snd_pcm_substream *pcm,
+			  __be32 *buffer, unsigned int frames)
+{
+	struct amdtp_ff *p = s->protocol;
+	struct snd_pcm_runtime *runtime = pcm->runtime;
+	unsigned int channels, remaining_frames, i, c;
+	const u16 *src;
+
+	channels = p->pcm_channels;
+	src = (void *)runtime->dma_area +
+			frames_to_bytes(runtime, s->pcm_buffer_pointer);
+	remaining_frames = runtime->buffer_size - s->pcm_buffer_pointer;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < channels; ++c) {
+			/* TODO: fill upper 1 byte for the other aim. */
+			buffer[c] = cpu_to_le32(*src << 16);
+			src++;
+		}
+		buffer += s->data_block_quadlets;
+		if (--remaining_frames == 0)
+			src = (void *)runtime->dma_area;
+	}
+}
+
+static void read_pcm_s32(struct amdtp_stream *s,
+			 struct snd_pcm_substream *pcm,
+			 __be32 *buffer, unsigned int frames)
+{
+	struct amdtp_ff *p = s->protocol;
+	struct snd_pcm_runtime *runtime = pcm->runtime;
+	unsigned int channels, remaining_frames, i, c;
+	u32 *dst;
+
+	channels = p->pcm_channels;
+	dst  = (void *)runtime->dma_area +
+			frames_to_bytes(runtime, s->pcm_buffer_pointer);
+	remaining_frames = runtime->buffer_size - s->pcm_buffer_pointer;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < channels; ++c) {
+			/* TODO: read upper 1 byte for the other aim. */
+			*dst = cpu_to_le32(buffer[c]) & 0xffffff00;
+			dst++;
+		}
+		buffer += s->data_block_quadlets;
+		if (--remaining_frames == 0)
+			dst = (void *)runtime->dma_area;
+	}
+}
+
+static void write_pcm_silence(struct amdtp_stream *s,
+			      __be32 *buffer, unsigned int frames)
+{
+	struct amdtp_ff *p = s->protocol;
+	unsigned int i, c, channels = p->pcm_channels;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < channels; ++c)
+			/* TODO: write upper 1 byte for the other aim. */
+			buffer[c] = cpu_to_le32(0x00000000);
+		buffer += s->data_block_quadlets;
+	}
+}
+
+void amdtp_ff_set_pcm_format(struct amdtp_stream *s, snd_pcm_format_t format)
+{
+	struct amdtp_ff *p = s->protocol;
+
+	if (WARN_ON(amdtp_stream_pcm_running(s)))
+		return;
+
+	switch (format) {
+	default:
+		WARN_ON(1);
+		/* fail through */
+	case SNDRV_PCM_FORMAT_S16:
+		if (s->direction == AMDTP_OUT_STREAM) {
+			p->transfer_samples = write_pcm_s16;
+			break;
+		}
+		WARN_ON(1);
+		/* fail through */
+	case SNDRV_PCM_FORMAT_S32:
+		if (s->direction == AMDTP_OUT_STREAM)
+			p->transfer_samples = write_pcm_s32;
+		else
+			p->transfer_samples = read_pcm_s32;
+		break;
+	}
+}
+
+int amdtp_ff_add_pcm_hw_constraints(struct amdtp_stream *s,
+				    struct snd_pcm_runtime *runtime)
+{
+	int err;
+
+	/*
+	 * Our implementation allows this protocol to deliver 24 bit sample in
+	 * 32bit data channel.
+	 */
+	err = snd_pcm_hw_constraint_msbits(runtime, 0, 32, 24);
+	if (err < 0)
+		return err;
+
+	return amdtp_stream_add_pcm_hw_constraints(s, runtime);
+}
+
+static unsigned int process_rx_data_blocks(struct amdtp_stream *s,
+					   __be32 *buffer,
+					   unsigned int data_blocks,
+					   unsigned int *syt)
+{
+	struct amdtp_ff *p = s->protocol;
+	struct snd_pcm_substream *pcm = ACCESS_ONCE(s->pcm);
+	unsigned int pcm_frames;
+
+	if (pcm) {
+		p->transfer_samples(s, pcm, buffer, data_blocks);
+		pcm_frames = data_blocks;
+	} else {
+		write_pcm_silence(s, buffer, data_blocks);
+		pcm_frames = 0;
+	}
+
+	return pcm_frames;
+}
+
+static unsigned int process_tx_data_blocks(struct amdtp_stream *s,
+					   __be32 *buffer,
+					   unsigned int data_blocks,
+					   unsigned int *syt)
+{
+	struct amdtp_ff *p = s->protocol;
+	struct snd_pcm_substream *pcm = ACCESS_ONCE(s->pcm);
+	unsigned int pcm_frames;
+
+	if (pcm) {
+		p->transfer_samples(s, pcm, buffer, data_blocks);
+		pcm_frames = data_blocks;
+	} else {
+		pcm_frames = 0;
+	}
+
+	return pcm_frames;
+}
+
+int amdtp_ff_init(struct amdtp_stream *s, struct fw_unit *unit,
+		  enum amdtp_stream_direction dir)
+{
+	amdtp_stream_process_data_blocks_t process_data_blocks;
+
+	if (dir == AMDTP_IN_STREAM)
+		process_data_blocks = process_tx_data_blocks;
+	else
+		process_data_blocks = process_rx_data_blocks;
+
+	return amdtp_stream_init(s, unit, dir, CIP_NO_HEADERS, 0,
+				 process_data_blocks, sizeof(struct amdtp_ff));
+}
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 38a200e..dd50261 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -76,6 +76,14 @@ int snd_ff_transaction_register(struct snd_ff *ff);
 int snd_ff_transaction_reregister(struct snd_ff *ff);
 void snd_ff_transaction_unregister(struct snd_ff *ff);
 
+int amdtp_ff_set_parameters(struct amdtp_stream *s, unsigned int rate,
+			    unsigned int pcm_channels);
+void amdtp_ff_set_pcm_format(struct amdtp_stream *s, snd_pcm_format_t format);
+int amdtp_ff_add_pcm_hw_constraints(struct amdtp_stream *s,
+				    struct snd_pcm_runtime *runtime);
+int amdtp_ff_init(struct amdtp_stream *s, struct fw_unit *unit,
+		  enum amdtp_stream_direction dir);
+
 void snd_ff_proc_init(struct snd_ff *ff);
 
 int snd_ff_create_midi_devices(struct snd_ff *ff);
-- 
2.5.0

[PATCH 09/11] fireface: add stream management functionality

[Takashi Sakamoto]

This commit adds support of stream management functionality.

The device has three modes of streaming according to sampling transfer
frequency. Each mode has own number of data channels in an packet. The
set of the numbers may be model-specific, therefore this commit adds
some members into model-specific structure to describe it.

The length of registers for the number of isochronous channel is just
three bits, therefore 0-7ch are available.

When bus reset occurs on IEEE 1394 bus, the device discontinues to
transmit packets. This commit aborts PCM substreams at bus reset handler.

The device manages its sampling clock independently of sampling transfer
frequency, against IEC 61883-6. Thus, it's a lower cost to change the
sampling transfer frequency, while data fetch between streaming layer and
DSP require bigger buffer for resampling. As a result, device latency may
tend to be larger than ASICs for IEC 61883-1/6 such as DM1000/DM1100/DM1500
(BeBoB) and OXFW970/971.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile          |   2 +-
 sound/firewire/fireface/fireface-stream.c | 358 ++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c        |  10 +
 sound/firewire/fireface/fireface.h        |  28 +++
 4 files changed, 397 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/fireface-stream.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index f22f0b5..5b4bb68 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,3 +1,3 @@
 snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o \
-		     fireface-proc.o amdtp-ff.o
+		     fireface-proc.o amdtp-ff.o fireface-stream.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-stream.c b/sound/firewire/fireface/fireface-stream.c
new file mode 100644
index 0000000..c7743a8
--- /dev/null
+++ b/sound/firewire/fireface/fireface-stream.c
@@ -0,0 +1,358 @@
+/*
+ * fireface-stream.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include <linux/delay.h>
+#include "fireface.h"
+
+#define CALLBACK_TIMEOUT_MS	200
+
+static int get_rate_mode(unsigned int rate, unsigned int *mode)
+{
+	int i;
+
+	for (i = 0; i < CIP_SFC_COUNT; i++) {
+		if (amdtp_rate_table[i] == rate)
+			break;
+	}
+
+	if (i == CIP_SFC_COUNT)
+		return -EINVAL;
+
+	*mode = ((int)i - 1) / 2;
+
+	return 0;
+}
+
+int snd_ff_stream_get_clock(struct snd_ff *ff, unsigned int *rate,
+			    enum snd_ff_clock_src *src)
+{
+	__le32 reg;
+	u32 data;
+	int err;
+
+	err = snd_fw_transaction(ff->unit, TCODE_READ_QUADLET_REQUEST,
+				 0x0000801C0004, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+	data = le32_to_cpu(reg);
+
+	/* Calculate sampling rate. */
+	switch ((data >> 1) & 0x03) {
+	case 0x01:
+		*rate = 32000;
+		break;
+	case 0x00:
+		*rate = 44100;
+		break;
+	case 0x03:
+		*rate = 48000;
+		break;
+	case 0x02:
+	default:
+		return -EIO;
+	}
+
+	if (data & 0x08)
+		*rate *= 2;
+	else if (data & 0x10)
+		*rate *= 4;
+
+	/* Calculate source of clock. */
+	if (data & 0x01) {
+		*src = SND_FF_CLOCK_SRC_INTERNAL;
+	} else {
+		/* TODO: 0x00, 0x01, 0x02, 0x06, 0x07? */
+		switch ((data >> 10) & 0x07) {
+		case 0x03:
+			*src = SND_FF_CLOCK_SRC_SPDIF;
+			break;
+		case 0x04:
+			*src = SND_FF_CLOCK_SRC_WORD;
+			break;
+		case 0x05:
+			*src = SND_FF_CLOCK_SRC_LTC;
+			break;
+		case 0x00:
+		default:
+			*src = SND_FF_CLOCK_SRC_ADAT;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * In this device, the length of register for isochronous channels is just
+ * three bits. Therefore, we can allocate between 0 and 7 channel.
+ */
+static int keep_resources(struct snd_ff *ff, unsigned int rate)
+{
+	int mode;
+	int err;
+
+	err = get_rate_mode(rate, &mode);
+	if (err < 0)
+		return err;
+
+	/* Keep resources for in-stream. */
+	err = amdtp_ff_set_parameters(&ff->tx_stream, rate,
+				      ff->spec->pcm_capture_channels[mode]);
+	if (err < 0)
+		return err;
+	ff->tx_resources.channels_mask = 0x00000000000000ffuLL;
+	err = fw_iso_resources_allocate(&ff->tx_resources,
+			amdtp_stream_get_max_payload(&ff->tx_stream),
+			fw_parent_device(ff->unit)->max_speed);
+	if (err < 0)
+		return err;
+
+	/* Keep resources for out-stream. */
+	err = amdtp_ff_set_parameters(&ff->rx_stream, rate,
+				      ff->spec->pcm_playback_channels[mode]);
+	if (err < 0)
+		return err;
+	ff->rx_resources.channels_mask = 0x00000000000000ffuLL;
+	err = fw_iso_resources_allocate(&ff->rx_resources,
+			amdtp_stream_get_max_payload(&ff->rx_stream),
+			fw_parent_device(ff->unit)->max_speed);
+	if (err < 0)
+		fw_iso_resources_free(&ff->tx_resources);
+
+	return err;
+}
+
+static void release_resources(struct snd_ff *ff)
+{
+	fw_iso_resources_free(&ff->tx_resources);
+	fw_iso_resources_free(&ff->rx_resources);
+}
+
+static int begin_session(struct snd_ff *ff, unsigned int rate)
+{
+	__le32 reg;
+	int i, err;
+
+	/* Check whether the given value is supported or not. */
+	for (i = 0; i < CIP_SFC_COUNT; i++) {
+		if (amdtp_rate_table[i] == rate)
+			break;
+	}
+	if (i == CIP_SFC_COUNT)
+		return -EINVAL;
+
+	/* Set the number of data blocks transferred in a second. */
+	reg = cpu_to_le32(rate);
+	err = snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 0x000080100500, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	msleep(100);
+
+	/*
+	 * Set isochronous channel and the number of quadlets of received
+	 * packets.
+	 */
+	reg = cpu_to_le32(((ff->rx_stream.data_block_quadlets << 3) << 8) |
+			  ff->rx_resources.channel);
+	err = snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 0x000080100504, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	/*
+	 * Set isochronous channel and the number of quadlets of transmitted
+	 * packet.
+	 */
+	/* TODO: investigate the purpose of this 0x80. */
+	reg = cpu_to_le32((0x80 << 24) |
+			  (ff->tx_resources.channel << 5) |
+			  (ff->tx_stream.data_block_quadlets));
+	err = snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 0x00008010050c, &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	/* Allow to transmit packets. */
+	reg = cpu_to_le32(0x00000001);
+	return snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 0x000080100508, &reg, sizeof(reg), 0);
+}
+
+static void finish_session(struct snd_ff *ff)
+{
+	__le32 reg;
+
+	reg = cpu_to_le32(0x80000000);
+	snd_fw_transaction(ff->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   0x000080100510, &reg, sizeof(reg), 0);
+}
+
+static int init_stream(struct snd_ff *ff, enum amdtp_stream_direction dir)
+{
+	int err;
+	struct fw_iso_resources *resources;
+	struct amdtp_stream *stream;
+
+	if (dir == AMDTP_IN_STREAM) {
+		resources = &ff->tx_resources;
+		stream = &ff->tx_stream;
+	} else {
+		resources = &ff->rx_resources;
+		stream = &ff->rx_stream;
+	}
+
+	err = fw_iso_resources_init(resources, ff->unit);
+	if (err < 0)
+		return err;
+
+	err = amdtp_ff_init(stream, ff->unit, dir);
+	if (err < 0)
+		fw_iso_resources_destroy(resources);
+
+	return err;
+}
+
+static void destroy_stream(struct snd_ff *ff, enum amdtp_stream_direction dir)
+{
+	if (dir == AMDTP_IN_STREAM) {
+		amdtp_stream_destroy(&ff->tx_stream);
+		fw_iso_resources_destroy(&ff->tx_resources);
+	} else {
+		amdtp_stream_destroy(&ff->rx_stream);
+		fw_iso_resources_destroy(&ff->rx_resources);
+	}
+}
+
+int snd_ff_stream_init_duplex(struct snd_ff *ff)
+{
+	int err;
+
+	err = init_stream(ff, AMDTP_OUT_STREAM);
+	if (err < 0)
+		goto end;
+
+	err = init_stream(ff, AMDTP_IN_STREAM);
+	if (err < 0)
+		destroy_stream(ff, AMDTP_OUT_STREAM);
+end:
+	return err;
+}
+
+/*
+ * This function should be called before starting streams or after stopping
+ * streams.
+ */
+void snd_ff_stream_destroy_duplex(struct snd_ff *ff)
+{
+	destroy_stream(ff, AMDTP_IN_STREAM);
+	destroy_stream(ff, AMDTP_OUT_STREAM);
+}
+
+int snd_ff_stream_start_duplex(struct snd_ff *ff, unsigned int rate)
+{
+	unsigned int curr_rate;
+	enum snd_ff_clock_src src;
+	int err;
+
+	if (ff->substreams_counter == 0)
+		return 0;
+
+	err = snd_ff_stream_get_clock(ff, &curr_rate, &src);
+	if (err < 0)
+		return err;
+	if (curr_rate != rate ||
+	    amdtp_streaming_error(&ff->tx_stream) ||
+	    amdtp_streaming_error(&ff->rx_stream)) {
+		finish_session(ff);
+
+		amdtp_stream_stop(&ff->tx_stream);
+		amdtp_stream_stop(&ff->rx_stream);
+
+		release_resources(ff);
+	}
+
+	/*
+	 * Regardless of current source of clock signal, drivers transfer some
+	 * packets. Then, the device transfers packets.
+	 */
+	if (!amdtp_stream_running(&ff->rx_stream)) {
+		err = keep_resources(ff, rate);
+		if (err < 0)
+			goto error;
+
+		err = begin_session(ff, rate);
+		if (err < 0)
+			goto error;
+
+		err = amdtp_stream_start(&ff->rx_stream,
+					 ff->rx_resources.channel,
+					 fw_parent_device(ff->unit)->max_speed);
+		if (err < 0)
+			goto error;
+
+		if (!amdtp_stream_wait_callback(&ff->rx_stream,
+						CALLBACK_TIMEOUT_MS)) {
+			err = -ETIMEDOUT;
+			goto error;
+		}
+	}
+
+	/*
+	 * The incoming packets has no timestamp, thus no afraid of detecting
+	 * packet discontinuity.
+	 */
+	if (!amdtp_stream_running(&ff->tx_stream)) {
+		err = amdtp_stream_start(&ff->tx_stream,
+					 ff->tx_resources.channel,
+					 fw_parent_device(ff->unit)->max_speed);
+		if (err < 0)
+			goto error;
+
+		if (!amdtp_stream_wait_callback(&ff->tx_stream,
+						CALLBACK_TIMEOUT_MS)) {
+			err = -ETIMEDOUT;
+			goto error;
+		}
+	}
+
+	return 0;
+error:
+	amdtp_stream_stop(&ff->tx_stream);
+	amdtp_stream_stop(&ff->rx_stream);
+
+	finish_session(ff);
+	release_resources(ff);
+
+	return err;
+}
+
+void snd_ff_stream_stop_duplex(struct snd_ff *ff)
+{
+	if (ff->substreams_counter > 0)
+		return;
+
+	amdtp_stream_stop(&ff->tx_stream);
+	amdtp_stream_stop(&ff->rx_stream);
+	finish_session(ff);
+	release_resources(ff);
+}
+
+void snd_ff_stream_update_duplex(struct snd_ff *ff)
+{
+	/* The device discontinue to transfer packets.  */
+	amdtp_stream_pcm_abort(&ff->tx_stream);
+	amdtp_stream_stop(&ff->tx_stream);
+
+	amdtp_stream_pcm_abort(&ff->rx_stream);
+	amdtp_stream_stop(&ff->rx_stream);
+
+	fw_iso_resources_update(&ff->tx_resources);
+	fw_iso_resources_update(&ff->rx_resources);
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index 2ea84ce..52efbd0 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -18,6 +18,8 @@ MODULE_LICENSE("GPL v2");
 
 struct snd_ff_spec spec_ff400 = {
 	.name = "Fireface400",
+	.pcm_capture_channels = {18, 14, 10},
+	.pcm_playback_channels = {18, 14, 10},
 };
 
 static void name_card(struct snd_ff *ff)
@@ -110,6 +112,12 @@ static int snd_ff_probe(struct fw_unit *unit,
 
 	snd_ff_proc_init(ff);
 
+	err = snd_ff_stream_init_duplex(ff);
+	if (err < 0) {
+		snd_card_free(card);
+		return err;
+	}
+
 	/* Register this sound card later. */
 	INIT_DEFERRABLE_WORK(&ff->dwork, do_probe);
 	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
@@ -133,6 +141,8 @@ static void snd_ff_update(struct fw_unit *unit)
 	}
 
 	snd_ff_transaction_reregister(ff);
+
+	snd_ff_stream_update_duplex(ff);
 }
 
 static void snd_ff_remove(struct fw_unit *unit)
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index dd50261..991f9d4 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -35,8 +35,12 @@
 #define SND_FF_IN_MIDI_PORTS		2
 #define SND_FF_OUT_MIDI_PORTS		2
 
+#define SND_FF_STREAM_MODES		3
+
 struct snd_ff_spec {
 	const char *const name;
+	const unsigned int pcm_capture_channels[SND_FF_STREAM_MODES];
+	const unsigned int pcm_playback_channels[SND_FF_STREAM_MODES];
 };
 
 struct snd_ff {
@@ -63,6 +67,12 @@ struct snd_ff {
 	ktime_t next_ktime[SND_FF_OUT_MIDI_PORTS];
 	bool rx_midi_error[SND_FF_OUT_MIDI_PORTS];
 	unsigned int rx_bytes[SND_FF_OUT_MIDI_PORTS];
+
+	unsigned int substreams_counter;
+	struct amdtp_stream tx_stream;
+	struct amdtp_stream rx_stream;
+	struct fw_iso_resources tx_resources;
+	struct fw_iso_resources rx_resources;
 };
 
 #define SND_FF_ADDR_CONTROLLER_ADDR_HI	0x0000801003f4
@@ -72,6 +82,15 @@ struct snd_ff {
 
 #define SND_FF_ADDR_MIDI_TX		0x000100000000
 
+enum snd_ff_clock_src {
+	SND_FF_CLOCK_SRC_INTERNAL,
+	SND_FF_CLOCK_SRC_SPDIF,
+	SND_FF_CLOCK_SRC_ADAT,
+	SND_FF_CLOCK_SRC_WORD,
+	SND_FF_CLOCK_SRC_LTC,
+	/* TODO: perhaps ADAT2 and TCO exists. */
+};
+
 int snd_ff_transaction_register(struct snd_ff *ff);
 int snd_ff_transaction_reregister(struct snd_ff *ff);
 void snd_ff_transaction_unregister(struct snd_ff *ff);
@@ -84,6 +103,15 @@ int amdtp_ff_add_pcm_hw_constraints(struct amdtp_stream *s,
 int amdtp_ff_init(struct amdtp_stream *s, struct fw_unit *unit,
 		  enum amdtp_stream_direction dir);
 
+int snd_ff_stream_init_duplex(struct snd_ff *ff);
+void snd_ff_stream_destroy_duplex(struct snd_ff *ff);
+int snd_ff_stream_start_duplex(struct snd_ff *ff, unsigned int rate);
+void snd_ff_stream_stop_duplex(struct snd_ff *ff);
+void snd_ff_stream_update_duplex(struct snd_ff *ff);
+
+int snd_ff_stream_get_clock(struct snd_ff *ff, unsigned int *rate,
+			    enum snd_ff_clock_src *src);
+
 void snd_ff_proc_init(struct snd_ff *ff);
 
 int snd_ff_create_midi_devices(struct snd_ff *ff);
-- 
2.5.0

[PATCH 10/11] fireface: add PCM functionality

[Takashi Sakamoto]

This commit adds PCM functionality to transmit/receive PCM samples on
isochronous packet streaming. This commit enables userspace applications
to start/stop packet streaming via ALSA PCM interface.

The requested sampling rate is basically used as sampling transfer
frequency of AMDTP packet streaming. This is restricted when the device is
synchronized to external clock source. In this case, input clock rate
is used as the sampling transfer frequency, except for 64.0/128.0 kHz.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile       |   3 +-
 sound/firewire/fireface/fireface-pcm.c | 381 +++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c     |   4 +
 sound/firewire/fireface/fireface.h     |   2 +
 4 files changed, 389 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/fireface/fireface-pcm.c

diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index 5b4bb68..4b6b9a9 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,3 +1,4 @@
 snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o \
-		     fireface-proc.o amdtp-ff.o fireface-stream.o
+		     fireface-proc.o amdtp-ff.o fireface-stream.o \
+		     fireface-pcm.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-pcm.c b/sound/firewire/fireface/fireface-pcm.c
new file mode 100644
index 0000000..705df28
--- /dev/null
+++ b/sound/firewire/fireface/fireface-pcm.c
@@ -0,0 +1,381 @@
+/*
+ * fireface-pcm.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "fireface.h"
+
+static inline unsigned int get_multiplier_mode_with_index(unsigned int index)
+{
+	return ((int)index - 1) / 2;
+}
+
+static int hw_rule_rate(struct snd_pcm_hw_params *params,
+			struct snd_pcm_hw_rule *rule)
+{
+	const unsigned int *pcm_channels = rule->private;
+	struct snd_interval *r =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+	const struct snd_interval *c =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int i, mode;
+
+	for (i = 0; i < ARRAY_SIZE(amdtp_rate_table); i++) {
+		mode = get_multiplier_mode_with_index(i);
+		if (!snd_interval_test(c, pcm_channels[mode]))
+			continue;
+
+		t.min = min(t.min, amdtp_rate_table[i]);
+		t.max = max(t.max, amdtp_rate_table[i]);
+	}
+
+	return snd_interval_refine(r, &t);
+}
+
+static int hw_rule_channels(struct snd_pcm_hw_params *params,
+			    struct snd_pcm_hw_rule *rule)
+{
+	const unsigned int *pcm_channels = rule->private;
+	struct snd_interval *c =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	const struct snd_interval *r =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int i, mode;
+
+	for (i = 0; i < ARRAY_SIZE(amdtp_rate_table); i++) {
+		mode = get_multiplier_mode_with_index(i);
+		if (!snd_interval_test(r, amdtp_rate_table[i]))
+			continue;
+
+		t.min = min(t.min, pcm_channels[mode]);
+		t.max = max(t.max, pcm_channels[mode]);
+	}
+
+	return snd_interval_refine(c, &t);
+}
+
+static void limit_channels_and_rates(struct snd_pcm_hardware *hw,
+				     const unsigned int *pcm_channels)
+{
+	unsigned int mode;
+	unsigned int rate, channels;
+	int i;
+
+	hw->channels_min = UINT_MAX;
+	hw->channels_max = 0;
+	hw->rate_min = UINT_MAX;
+	hw->rate_max = 0;
+
+	for (i = 0; i < ARRAY_SIZE(amdtp_rate_table); i++) {
+		mode = get_multiplier_mode_with_index(i);
+
+		channels = pcm_channels[mode];
+		if (pcm_channels[mode] == 0)
+			continue;
+		hw->channels_min = min(hw->channels_min, channels);
+		hw->channels_max = max(hw->channels_max, channels);
+
+		rate = amdtp_rate_table[i];
+		hw->rates |= snd_pcm_rate_to_rate_bit(rate);
+		hw->rate_min = min(hw->rate_min, rate);
+		hw->rate_max = max(hw->rate_max, rate);
+	}
+}
+
+static void limit_period_and_buffer(struct snd_pcm_hardware *hw)
+{
+	hw->periods_min = 2;		/* SNDRV_PCM_INFO_BATCH */
+	hw->periods_max = UINT_MAX;
+
+	hw->period_bytes_min = 4 * hw->channels_max;	/* bytes for a frame */
+
+	/* Just to prevent from allocating much pages. */
+	hw->period_bytes_max = hw->period_bytes_min * 2048;
+	hw->buffer_bytes_max = hw->period_bytes_max * hw->periods_min;
+}
+
+static int pcm_init_hw_params(struct snd_ff *ff,
+			      struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct amdtp_stream *s;
+	const unsigned int *pcm_channels;
+	int err;
+
+	runtime->hw.info = SNDRV_PCM_INFO_BATCH |
+			   SNDRV_PCM_INFO_BLOCK_TRANSFER |
+			   SNDRV_PCM_INFO_INTERLEAVED |
+			   SNDRV_PCM_INFO_JOINT_DUPLEX |
+			   SNDRV_PCM_INFO_MMAP |
+			   SNDRV_PCM_INFO_MMAP_VALID;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		runtime->hw.formats = SNDRV_PCM_FMTBIT_S32;
+		s = &ff->tx_stream;
+		pcm_channels = ff->spec->pcm_capture_channels;
+	} else {
+		runtime->hw.formats = SNDRV_PCM_FMTBIT_S32 |
+				      SNDRV_PCM_FMTBIT_S16;
+		s = &ff->rx_stream;
+		pcm_channels = ff->spec->pcm_playback_channels;
+	}
+
+	/* limit rates */
+	limit_channels_and_rates(&runtime->hw, pcm_channels);
+	limit_period_and_buffer(&runtime->hw);
+
+	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+				  hw_rule_channels, (void *)pcm_channels,
+				  SNDRV_PCM_HW_PARAM_RATE, -1);
+	if (err < 0)
+		return err;
+
+	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+				  hw_rule_rate, (void *)pcm_channels,
+				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	if (err < 0)
+		return err;
+
+	return amdtp_ff_add_pcm_hw_constraints(s, runtime);
+}
+
+static int pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_ff *ff = substream->private_data;
+	unsigned int rate;
+	enum snd_ff_clock_src src;
+	int err;
+
+	err = pcm_init_hw_params(ff, substream);
+	if (err < 0)
+		return err;
+
+	err = snd_ff_stream_get_clock(ff, &rate, &src);
+	if (src != SND_FF_CLOCK_SRC_INTERNAL ||
+	    amdtp_stream_pcm_running(&ff->rx_stream) ||
+	    amdtp_stream_pcm_running(&ff->tx_stream)) {
+		substream->runtime->hw.rate_min = rate;
+		substream->runtime->hw.rate_max = rate;
+	}
+
+	snd_pcm_set_sync(substream);
+
+	return err;
+}
+
+static int pcm_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_ff *ff = substream->private_data;
+	int err;
+
+	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
+					       params_buffer_bytes(hw_params));
+	if (err < 0)
+		return err;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		mutex_lock(&ff->mutex);
+		ff->substreams_counter++;
+		mutex_unlock(&ff->mutex);
+	}
+
+	amdtp_ff_set_pcm_format(&ff->tx_stream, params_format(hw_params));
+
+	return 0;
+}
+
+static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_ff *ff = substream->private_data;
+	int err;
+
+	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
+					       params_buffer_bytes(hw_params));
+	if (err < 0)
+		return err;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		mutex_lock(&ff->mutex);
+		ff->substreams_counter++;
+		mutex_unlock(&ff->mutex);
+	}
+
+	amdtp_ff_set_pcm_format(&ff->rx_stream, params_format(hw_params));
+
+	return 0;
+}
+
+static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_ff *ff = substream->private_data;
+
+	mutex_lock(&ff->mutex);
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		ff->substreams_counter--;
+
+	snd_ff_stream_stop_duplex(ff);
+
+	mutex_unlock(&ff->mutex);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_ff *ff = substream->private_data;
+
+	mutex_lock(&ff->mutex);
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		ff->substreams_counter--;
+
+	snd_ff_stream_stop_duplex(ff);
+
+	mutex_unlock(&ff->mutex);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int pcm_capture_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_ff *ff = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	mutex_lock(&ff->mutex);
+
+	err = snd_ff_stream_start_duplex(ff, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&ff->tx_stream);
+
+	mutex_unlock(&ff->mutex);
+
+	return err;
+}
+
+static int pcm_playback_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_ff *ff = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	mutex_lock(&ff->mutex);
+
+	err = snd_ff_stream_start_duplex(ff, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&ff->rx_stream);
+
+	mutex_unlock(&ff->mutex);
+
+	return err;
+}
+
+static int pcm_capture_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_ff *ff = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&ff->tx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&ff->tx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_ff *ff = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&ff->rx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&ff->rx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_ff *ff = sbstrm->private_data;
+
+	return amdtp_stream_pcm_pointer(&ff->tx_stream);
+}
+
+static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_ff *ff = sbstrm->private_data;
+
+	return amdtp_stream_pcm_pointer(&ff->rx_stream);
+}
+
+static struct snd_pcm_ops pcm_capture_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_capture_hw_params,
+	.hw_free	= pcm_capture_hw_free,
+	.prepare	= pcm_capture_prepare,
+	.trigger	= pcm_capture_trigger,
+	.pointer	= pcm_capture_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+};
+
+static struct snd_pcm_ops pcm_playback_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_playback_hw_params,
+	.hw_free	= pcm_playback_hw_free,
+	.prepare	= pcm_playback_prepare,
+	.trigger	= pcm_playback_trigger,
+	.pointer	= pcm_playback_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+	.mmap		= snd_pcm_lib_mmap_vmalloc,
+};
+
+int snd_ff_create_pcm_devices(struct snd_ff *ff)
+{
+	struct snd_pcm *pcm;
+	int err;
+
+	err = snd_pcm_new(ff->card, ff->card->driver, 0, 1, 1, &pcm);
+	if (err < 0)
+		return err;
+
+	pcm->private_data = ff;
+	snprintf(pcm->name, sizeof(pcm->name),
+		 "%s PCM", ff->card->shortname);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &pcm_playback_ops);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_capture_ops);
+
+	return 0;
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index 52efbd0..59f0441 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -67,6 +67,10 @@ static void do_probe(struct work_struct *work)
 	if (err < 0)
 		goto end;
 
+	err = snd_ff_create_pcm_devices(ff);
+	if (err < 0)
+		goto end;
+
 	err = snd_card_register(ff->card);
 	if (err < 0)
 		goto end;
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 991f9d4..63834e7 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -116,4 +116,6 @@ void snd_ff_proc_init(struct snd_ff *ff);
 
 int snd_ff_create_midi_devices(struct snd_ff *ff);
 
+int snd_ff_create_pcm_devices(struct snd_ff *ff);
+
 #endif
-- 
2.5.0

[PATCH 11/11] fireface: add hwdep interface

[Takashi Sakamoto]

This commit adds hwdep interface so as the other drivers for audio and
music units on IEEE 1394 have.

This interface is designed for mixer/control applications. By using this
interface, an application can get information about firewire node, can
lock/unlock kernel streaming and can get notification at starting/stopping
kernel streaming.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/uapi/sound/asound.h               |   3 +-
 include/uapi/sound/firewire.h             |   1 +
 sound/firewire/fireface/Makefile          |   2 +-
 sound/firewire/fireface/fireface-hwdep.c  | 191 ++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface-pcm.c    |  13 +-
 sound/firewire/fireface/fireface-stream.c |  39 ++++++
 sound/firewire/fireface/fireface.c        |   5 +
 sound/firewire/fireface/fireface.h        |  10 ++
 8 files changed, 260 insertions(+), 4 deletions(-)
 create mode 100644 sound/firewire/fireface/fireface-hwdep.c

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index a82108e..f1cd38e 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -102,9 +102,10 @@ enum {
 	SNDRV_HWDEP_IFACE_FW_OXFW,	/* Oxford OXFW970/971 based device */
 	SNDRV_HWDEP_IFACE_FW_DIGI00X,	/* Digidesign Digi 002/003 family */
 	SNDRV_HWDEP_IFACE_FW_TASCAM,	/* TASCAM FireWire series */
+	SNDRV_HWDEP_IFACE_FW_FIREFACE,	/* RME Fireface series */
 
 	/* Don't forget to change the following: */
-	SNDRV_HWDEP_IFACE_LAST = SNDRV_HWDEP_IFACE_FW_TASCAM
+	SNDRV_HWDEP_IFACE_LAST = SNDRV_HWDEP_IFACE_FW_FIREFACE
 };
 
 struct snd_hwdep_info {
diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h
index db79a12..f6e58ae 100644
--- a/include/uapi/sound/firewire.h
+++ b/include/uapi/sound/firewire.h
@@ -65,6 +65,7 @@ union snd_firewire_event {
 #define SNDRV_FIREWIRE_TYPE_OXFW	4
 #define SNDRV_FIREWIRE_TYPE_DIGI00X	5
 #define SNDRV_FIREWIRE_TYPE_TASCAM	6
+#define SNDRV_FIREWIRE_TYPE_FIREFACE	7
 /* RME, MOTU, ... */
 
 struct snd_firewire_get_info {
diff --git a/sound/firewire/fireface/Makefile b/sound/firewire/fireface/Makefile
index 4b6b9a9..9eea168 100644
--- a/sound/firewire/fireface/Makefile
+++ b/sound/firewire/fireface/Makefile
@@ -1,4 +1,4 @@
 snd-fireface-objs := fireface.o fireface-transaction.o fireface-midi.o \
 		     fireface-proc.o amdtp-ff.o fireface-stream.o \
-		     fireface-pcm.o
+		     fireface-pcm.o fireface-hwdep.o
 obj-$(CONFIG_SND_FIREFACE) += snd-fireface.o
diff --git a/sound/firewire/fireface/fireface-hwdep.c b/sound/firewire/fireface/fireface-hwdep.c
new file mode 100644
index 0000000..bc86927
--- /dev/null
+++ b/sound/firewire/fireface/fireface-hwdep.c
@@ -0,0 +1,191 @@
+/*
+ * fireface-hwdep.c - a part of driver for RME Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+/*
+ * This codes give three functionality.
+ *
+ * 1.get firewire node information
+ * 2.get notification about starting/stopping stream
+ * 3.lock/unlock stream
+ */
+
+#include "fireface.h"
+
+static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
+		       loff_t *offset)
+{
+	struct snd_ff *ff = hwdep->private_data;
+	DEFINE_WAIT(wait);
+	union snd_firewire_event event;
+
+	spin_lock_irq(&ff->lock);
+
+	while (!ff->dev_lock_changed) {
+		prepare_to_wait(&ff->hwdep_wait, &wait, TASK_INTERRUPTIBLE);
+		spin_unlock_irq(&ff->lock);
+		schedule();
+		finish_wait(&ff->hwdep_wait, &wait);
+		if (signal_pending(current))
+			return -ERESTARTSYS;
+		spin_lock_irq(&ff->lock);
+	}
+
+	memset(&event, 0, sizeof(event));
+	if (ff->dev_lock_changed) {
+		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
+		event.lock_status.status = (ff->dev_lock_count > 0);
+		ff->dev_lock_changed = false;
+
+		count = min_t(long, count, sizeof(event.lock_status));
+	}
+
+	spin_unlock_irq(&ff->lock);
+
+	if (copy_to_user(buf, &event, count))
+		return -EFAULT;
+
+	return count;
+}
+
+static unsigned int hwdep_poll(struct snd_hwdep *hwdep, struct file *file,
+			       poll_table *wait)
+{
+	struct snd_ff *ff = hwdep->private_data;
+	unsigned int events;
+
+	poll_wait(file, &ff->hwdep_wait, wait);
+
+	spin_lock_irq(&ff->lock);
+	if (ff->dev_lock_changed)
+		events = POLLIN | POLLRDNORM;
+	else
+		events = 0;
+	spin_unlock_irq(&ff->lock);
+
+	return events;
+}
+
+static int hwdep_get_info(struct snd_ff *ff, void __user *arg)
+{
+	struct fw_device *dev = fw_parent_device(ff->unit);
+	struct snd_firewire_get_info info;
+
+	memset(&info, 0, sizeof(info));
+	info.type = SNDRV_FIREWIRE_TYPE_FIREFACE;
+	info.card = dev->card->index;
+	*(__be32 *)&info.guid[0] = cpu_to_be32(dev->config_rom[3]);
+	*(__be32 *)&info.guid[4] = cpu_to_be32(dev->config_rom[4]);
+	strlcpy(info.device_name, dev_name(&dev->device),
+		sizeof(info.device_name));
+
+	if (copy_to_user(arg, &info, sizeof(info)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int hwdep_lock(struct snd_ff *ff)
+{
+	int err;
+
+	spin_lock_irq(&ff->lock);
+
+	if (ff->dev_lock_count == 0) {
+		ff->dev_lock_count = -1;
+		err = 0;
+	} else {
+		err = -EBUSY;
+	}
+
+	spin_unlock_irq(&ff->lock);
+
+	return err;
+}
+
+static int hwdep_unlock(struct snd_ff *ff)
+{
+	int err;
+
+	spin_lock_irq(&ff->lock);
+
+	if (ff->dev_lock_count == -1) {
+		ff->dev_lock_count = 0;
+		err = 0;
+	} else {
+		err = -EBADFD;
+	}
+
+	spin_unlock_irq(&ff->lock);
+
+	return err;
+}
+
+static int hwdep_release(struct snd_hwdep *hwdep, struct file *file)
+{
+	struct snd_ff *ff = hwdep->private_data;
+
+	spin_lock_irq(&ff->lock);
+	if (ff->dev_lock_count == -1)
+		ff->dev_lock_count = 0;
+	spin_unlock_irq(&ff->lock);
+
+	return 0;
+}
+
+static int hwdep_ioctl(struct snd_hwdep *hwdep, struct file *file,
+		       unsigned int cmd, unsigned long arg)
+{
+	struct snd_ff *ff = hwdep->private_data;
+
+	switch (cmd) {
+	case SNDRV_FIREWIRE_IOCTL_GET_INFO:
+		return hwdep_get_info(ff, (void __user *)arg);
+	case SNDRV_FIREWIRE_IOCTL_LOCK:
+		return hwdep_lock(ff);
+	case SNDRV_FIREWIRE_IOCTL_UNLOCK:
+		return hwdep_unlock(ff);
+	default:
+		return -ENOIOCTLCMD;
+	}
+}
+
+#ifdef CONFIG_COMPAT
+static int hwdep_compat_ioctl(struct snd_hwdep *hwdep, struct file *file,
+			      unsigned int cmd, unsigned long arg)
+{
+	return hwdep_ioctl(hwdep, file, cmd,
+			   (unsigned long)compat_ptr(arg));
+}
+#else
+#define hwdep_compat_ioctl NULL
+#endif
+
+int snd_ff_create_hwdep_devices(struct snd_ff *ff)
+{
+	static const struct snd_hwdep_ops hwdep_ops = {
+		.read		= hwdep_read,
+		.release	= hwdep_release,
+		.poll		= hwdep_poll,
+		.ioctl		= hwdep_ioctl,
+		.ioctl_compat	= hwdep_compat_ioctl,
+	};
+	struct snd_hwdep *hwdep;
+	int err;
+
+	err = snd_hwdep_new(ff->card, ff->card->driver, 0, &hwdep);
+	if (err < 0)
+		return err;
+
+	strcpy(hwdep->name, ff->card->driver);
+	hwdep->iface = SNDRV_HWDEP_IFACE_FW_FIREFACE;
+	hwdep->ops = hwdep_ops;
+	hwdep->private_data = ff;
+	hwdep->exclusive = true;
+
+	return 0;
+}
diff --git a/sound/firewire/fireface/fireface-pcm.c b/sound/firewire/fireface/fireface-pcm.c
index 705df28..2ddd248 100644
--- a/sound/firewire/fireface/fireface-pcm.c
+++ b/sound/firewire/fireface/fireface-pcm.c
@@ -155,10 +155,16 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	enum snd_ff_clock_src src;
 	int err;
 
-	err = pcm_init_hw_params(ff, substream);
+	err = snd_ff_stream_lock_try(ff);
 	if (err < 0)
 		return err;
 
+	err = pcm_init_hw_params(ff, substream);
+	if (err < 0) {
+		snd_ff_stream_lock_release(ff);
+		return err;
+	}
+
 	err = snd_ff_stream_get_clock(ff, &rate, &src);
 	if (src != SND_FF_CLOCK_SRC_INTERNAL ||
 	    amdtp_stream_pcm_running(&ff->rx_stream) ||
@@ -169,11 +175,14 @@ static int pcm_open(struct snd_pcm_substream *substream)
 
 	snd_pcm_set_sync(substream);
 
-	return err;
+	return 0;
 }
 
 static int pcm_close(struct snd_pcm_substream *substream)
 {
+	struct snd_ff *ff = substream->private_data;
+
+	snd_ff_stream_lock_release(ff);
 	return 0;
 }
 
diff --git a/sound/firewire/fireface/fireface-stream.c b/sound/firewire/fireface/fireface-stream.c
index c7743a8..252533a 100644
--- a/sound/firewire/fireface/fireface-stream.c
+++ b/sound/firewire/fireface/fireface-stream.c
@@ -356,3 +356,42 @@ void snd_ff_stream_update_duplex(struct snd_ff *ff)
 	fw_iso_resources_update(&ff->tx_resources);
 	fw_iso_resources_update(&ff->rx_resources);
 }
+
+void snd_ff_stream_lock_changed(struct snd_ff *ff)
+{
+	ff->dev_lock_changed = true;
+	wake_up(&ff->hwdep_wait);
+}
+
+int snd_ff_stream_lock_try(struct snd_ff *ff)
+{
+	int err;
+
+	spin_lock_irq(&ff->lock);
+
+	/* user land lock this */
+	if (ff->dev_lock_count < 0) {
+		err = -EBUSY;
+		goto end;
+	}
+
+	/* this is the first time */
+	if (ff->dev_lock_count++ == 0)
+		snd_ff_stream_lock_changed(ff);
+	err = 0;
+end:
+	spin_unlock_irq(&ff->lock);
+	return err;
+}
+
+void snd_ff_stream_lock_release(struct snd_ff *ff)
+{
+	spin_lock_irq(&ff->lock);
+
+	if (WARN_ON(ff->dev_lock_count <= 0))
+		goto end;
+	if (--ff->dev_lock_count == 0)
+		snd_ff_stream_lock_changed(ff);
+end:
+	spin_unlock_irq(&ff->lock);
+}
diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
index 59f0441..f8f42cc 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -71,6 +71,10 @@ static void do_probe(struct work_struct *work)
 	if (err < 0)
 		goto end;
 
+	err = snd_ff_create_hwdep_devices(ff);
+	if (err < 0)
+		goto end;
+
 	err = snd_card_register(ff->card);
 	if (err < 0)
 		goto end;
@@ -109,6 +113,7 @@ static int snd_ff_probe(struct fw_unit *unit,
 
 	mutex_init(&ff->mutex);
 	spin_lock_init(&ff->lock);
+	init_waitqueue_head(&ff->hwdep_wait);
 	dev_set_drvdata(&unit->device, ff);
 
 	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 63834e7..4860bab 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -73,6 +73,10 @@ struct snd_ff {
 	struct amdtp_stream rx_stream;
 	struct fw_iso_resources tx_resources;
 	struct fw_iso_resources rx_resources;
+
+	int dev_lock_count;
+	bool dev_lock_changed;
+	wait_queue_head_t hwdep_wait;
 };
 
 #define SND_FF_ADDR_CONTROLLER_ADDR_HI	0x0000801003f4
@@ -112,10 +116,16 @@ void snd_ff_stream_update_duplex(struct snd_ff *ff);
 int snd_ff_stream_get_clock(struct snd_ff *ff, unsigned int *rate,
 			    enum snd_ff_clock_src *src);
 
+void snd_ff_stream_lock_changed(struct snd_ff *ff);
+int snd_ff_stream_lock_try(struct snd_ff *ff);
+void snd_ff_stream_lock_release(struct snd_ff *ff);
+
 void snd_ff_proc_init(struct snd_ff *ff);
 
 int snd_ff_create_midi_devices(struct snd_ff *ff);
 
 int snd_ff_create_pcm_devices(struct snd_ff *ff);
 
+int snd_ff_create_hwdep_devices(struct snd_ff *ff);
+
 #endif
-- 
2.5.0

Re: [FFADO-devel] [RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Jonathan Woithe]

On Sun, Dec 20, 2015 at 09:28:32PM +0900, Takashi Sakamoto wrote:
> I note that just powering on, the device is muted.

Only with respect to the audio streams from the PC.  All other
hardware-based routing is set up as per the saved device configuration.

> You can de-mute it by write
> transaction. For example, by using 'firewire-request' command in
> linux-firewire-utils(https://github.com/cladisch/linux-firewire-utils):
> 
> $ ./firewire-request /dev/fw1 write 0x0000801c0000 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000010000000100000001000000
> 
> Currently, libffado gives no way to do it via its mixer interface. Instead, done
> in methods to start streaming. I think it better to move de-mute codes to mixer
> initialization.

The mutes controlled here apply only to the software audio streams.  I
suspect they are utilised to eliminate noise during streaming setup and when
the device is idle with respect to the computer.  When other systems mute
individual playback channels under user control they do not use these mutes
and instead do it in other ways.  The above mutes are only ever turned off
and on as part of streaming setup and teardown.  As a result, I think Linux
should do the same thing because this is the mode of operation used on other
systems and is therefore the "supported" approach.

In other words, I think we should do what all other systems do with these
devices and treat this mute register as a part of the streaming setup
procedure.

Note in passing that the above firewire-request will only enable output for
the first four audio streams.  Others (such as ADAT) will remain muted.  The
correct initialisation of the 28 quadlets at 0x0000801c0000 is to set every
quadlet to 1.

> There's an issue of periodical hissing noise. I observed this with Fireface 400.
> Interval of the noise differs depending on the situation (10-20 minutes) and
> continues around 20 seconds. The cause is not clear but I've experienced similar
> issue with TASCAM FireOne. A common point between these two models is
> non-blocking transmission. I guess that the cause may be on data block
> calculation in AMDTP packet streaming layer (calculate_data_blocks() function
> in sound/firewire/amdtp-stream.c). Further investigation is required.

I've just got back from an week's off-line break and will look at the code
once I've caught up on things.  I suspect the audio packets are not being
timed correctly so over time things get a little out of sync.  The timing
mentioned above certainly suggests a slow beating.

I haven't looked at calculate_data_blocks() yet so I don't know how it
works.  I suspect that the method of timing data blocks doesn't quite match
up with what the Fireface interfaces really expect.  There are two "proper"
ways to determine whether an audio packet should be sent:

1) Monitor the buffer status bits returned within the incoming audio data
   stream and send at an appropriate time.

2) Keep track of the time between incoming audio packets and simply mirror
   that in the output stream.

Option 1 is rather difficult in reality and in practice isn't really
required.  Option 2 is sufficient and is guaranteed to keep everything in
sync.

As a side comment, FFADO does neither because of the way the RME driver must
interface with the rest of the FFADO infrastructure.

> In my previous RFC, I described that zero bits have no effect in write
> transactions to 0x00008010051c. But this is wrong. The transaction has
> side effect to set the other options.

That would be correct.  0x00008010051c is the start of the FF400's
configuration block.  Fields within this set all manner of things including
phantom power status, input pads, optical port mode and so on.  Blindly
writing zeros will force the device into the physical state represented by
that zero byte pattern.

> In this patchset, the state of clock is changed on the way to initialize
> transaction.  This is a bit rude but unavoidable.  Fortunately, it's
> software's responsibility to read from flash memory and set initial
> configuration,

Not quite.  The device powers up with the configuration that is stored in
the internal flash memory and does not require any interaction with a
computer in order to be functional.  Software needs to synchronise itself
with this configuration so as to maintain it.  Forcing a particular
configuration is fine for an initial version of a driver, but in time there
is a need to be more sophisticated about how this is handled.  The setup of
these interfaces can be highly complex and many users rely on the flash
memory configuration to avoid time consuming setup at every power on. 
Forcing a configuration reset like this can certainly be worked around
during initial development, but in the long term the driver needs to
preserve device status.

I have several ideas about how this might look but need to wait to see what
framework we have to work within before any of that can be solidified.  The
primary difficulty is that the configuration register needs to be known by
all software components which interact with the device: not just the
streaming system.  A way to make the configuration available to usespace
will be needed.  In FFADO this was handled with a shared memory segment so
it didn't matter which component (streaming, mixer) was started first.  I
imagine in FFADO the configuration will be made available via hwdep, /proc/
or something similar, ideally in a manner which is transparent to the type
of Fireface device in use.

In summary: zeroing out the configuration block is fine for now, but in time
this will need to be revisited.

Regards
  jonathan

Re: [RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Takashi Sakamoto]

On Dec 21 2015 13:14, Jonathan Woithe wrote:
> On Sun, Dec 20, 2015 at 09:28:32PM +0900, Takashi Sakamoto wrote:
>> I note that just powering on, the device is muted.
> 
> Only with respect to the audio streams from the PC.  All other
> hardware-based routing is set up as per the saved device configuration.
> 
>> You can de-mute it by write
>> transaction. For example, by using 'firewire-request' command in
>> linux-firewire-utils(https://github.com/cladisch/linux-firewire-utils):
>>
>> $ ./firewire-request /dev/fw1 write 0x0000801c0000 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000010000000100000001000000
>>
>> Currently, libffado gives no way to do it via its mixer interface. Instead, done
>> in methods to start streaming. I think it better to move de-mute codes to mixer
>> initialization.
> 
> The mutes controlled here apply only to the software audio streams.  I
> suspect they are utilised to eliminate noise during streaming setup and when
> the device is idle with respect to the computer.  When other systems mute
> individual playback channels under user control they do not use these mutes
> and instead do it in other ways.  The above mutes are only ever turned off
> and on as part of streaming setup and teardown.  As a result, I think Linux
> should do the same thing because this is the mode of operation used on other
> systems and is therefore the "supported" approach.
> 
> In other words, I think we should do what all other systems do with these
> devices and treat this mute register as a part of the streaming setup
> procedure.
> 
> Note in passing that the above firewire-request will only enable output for
> the first four audio streams.  Others (such as ADAT) will remain muted.  The
> correct initialisation of the 28 quadlets at 0x0000801c0000 is to set every
> quadlet to 1.

Could I request your opinion about the fact that the device keeps unmute
state once unmuted? If your insistent is reasonable, the device should
get mute every time to start to receive packets.

>> There's an issue of periodical hissing noise. I observed this with Fireface 400.
>> Interval of the noise differs depending on the situation (10-20 minutes) and
>> continues around 20 seconds. The cause is not clear but I've experienced similar
>> issue with TASCAM FireOne. A common point between these two models is
>> non-blocking transmission. I guess that the cause may be on data block
>> calculation in AMDTP packet streaming layer (calculate_data_blocks() function
>> in sound/firewire/amdtp-stream.c). Further investigation is required.
> 
> I've just got back from an week's off-line break and will look at the code
> once I've caught up on things.  I suspect the audio packets are not being
> timed correctly so over time things get a little out of sync.  The timing
> mentioned above certainly suggests a slow beating.
> 
> I haven't looked at calculate_data_blocks() yet so I don't know how it
> works.  I suspect that the method of timing data blocks doesn't quite match
> up with what the Fireface interfaces really expect.  There are two "proper"
> ways to determine whether an audio packet should be sent:
> 
> 1) Monitor the buffer status bits returned within the incoming audio data
>    stream and send at an appropriate time.
> 
> 2) Keep track of the time between incoming audio packets and simply mirror
>    that in the output stream.
> 
> Option 1 is rather difficult in reality and in practice isn't really
> required.  Option 2 is sufficient and is guaranteed to keep everything in
> sync.
> 
> As a side comment, FFADO does neither because of the way the RME driver must
> interface with the rest of the FFADO infrastructure.
> 
>> In my previous RFC, I described that zero bits have no effect in write
>> transactions to 0x00008010051c. But this is wrong. The transaction has
>> side effect to set the other options.
> 
> That would be correct.  0x00008010051c is the start of the FF400's
> configuration block.  Fields within this set all manner of things including
> phantom power status, input pads, optical port mode and so on.  Blindly
> writing zeros will force the device into the physical state represented by
> that zero byte pattern.
> 
>> In this patchset, the state of clock is changed on the way to initialize
>> transaction.  This is a bit rude but unavoidable.  Fortunately, it's
>> software's responsibility to read from flash memory and set initial
>> configuration,
> 
> Not quite.  The device powers up with the configuration that is stored in
> the internal flash memory and does not require any interaction with a
> computer in order to be functional.  Software needs to synchronise itself
> with this configuration so as to maintain it.  Forcing a particular
> configuration is fine for an initial version of a driver, but in time there
> is a need to be more sophisticated about how this is handled.  The setup of
> these interfaces can be highly complex and many users rely on the flash
> memory configuration to avoid time consuming setup at every power on. 
> Forcing a configuration reset like this can certainly be worked around
> during initial development, but in the long term the driver needs to
> preserve device status.
> 
> I have several ideas about how this might look but need to wait to see what
> framework we have to work within before any of that can be solidified.  The
> primary difficulty is that the configuration register needs to be known by
> all software components which interact with the device: not just the
> streaming system.  A way to make the configuration available to usespace
> will be needed.  In FFADO this was handled with a shared memory segment so
> it didn't matter which component (streaming, mixer) was started first.  I
> imagine in FFADO the configuration will be made available via hwdep, /proc/
> or something similar, ideally in a manner which is transparent to the type
> of Fireface device in use.
> 
> In summary: zeroing out the configuration block is fine for now, but in time
> this will need to be revisited.

I'd like to drop the code to send write transactions to the address, and
leave the decision to userspace. As a result, userspace applications
cannot receive MIDI messages from the device via ALSA rawmidi character
devices, unless any userspace applications performs the configuration.

The main work of device drivers is to give ways to transfer data between
userspace applications and devices. Nothing others. Currently. userspace
applications are given ways to communicate to the device via fw
character device. Thus, no need to implement such surplus work to
snd-fireface.

Instead, I hope libffado to drop the code to register node address
regardless of whether the address is valid or invalid. Of cource, if
userspace applications want to disallow kernel modules to handle the
messages, it can do it, it's what userspace want.


Regards

Takashi Sakamoto

Re: [RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Jonathan Woithe]

On Mon, Dec 21, 2015 at 10:00:38PM +0900, Takashi Sakamoto wrote:
> On Dec 21 2015 13:14, Jonathan Woithe wrote:
> > On Sun, Dec 20, 2015 at 09:28:32PM +0900, Takashi Sakamoto wrote:
> >> I note that just powering on, the device is muted.
> > 
> > Only with respect to the audio streams from the PC.  All other
> > hardware-based routing is set up as per the saved device configuration.
> > 
> >> You can de-mute it by write
> >> transaction. For example, by using 'firewire-request' command in
> >> linux-firewire-utils(https://github.com/cladisch/linux-firewire-utils):
> >>
> >> $ ./firewire-request /dev/fw1 write 0x0000801c0000 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000010000000100000001000000
> >>
> >> Currently, libffado gives no way to do it via its mixer interface. Instead, done
> >> in methods to start streaming. I think it better to move de-mute codes to mixer
> >> initialization.
> > 
> > The mutes controlled here apply only to the software audio streams.  I
> > suspect they are utilised to eliminate noise during streaming setup and when
> > the device is idle with respect to the computer.  When other systems mute
> > individual playback channels under user control they do not use these mutes
> > and instead do it in other ways.  The above mutes are only ever turned off
> > and on as part of streaming setup and teardown.  As a result, I think Linux
> > should do the same thing because this is the mode of operation used on other
> > systems and is therefore the "supported" approach.
> > 
> > In other words, I think we should do what all other systems do with these
> > devices and treat this mute register as a part of the streaming setup
> > procedure.
> > 
> > Note in passing that the above firewire-request will only enable output for
> > the first four audio streams.  Others (such as ADAT) will remain muted.  The
> > correct initialisation of the 28 quadlets at 0x0000801c0000 is to set every
> > quadlet to 1.

I made a typo at the end of that last paragraph: the statement should have
read "set every quadlet to 0".  It's a mute state, so the unmuted state is
obviously 0.  The above firewire-request will leave some higher-numbered
channels muted which may be in the unused portion of the FF400 map (I
haven't counted the zeros to know where the 0x1's start).  Apologies for any
confusion caused.

> Could I request your opinion about the fact that the device keeps unmute
> state once unmuted? If your insistent is reasonable, the device should
> get mute every time to start to receive packets.

I'm not quite sure what you mean here.  Are you saying that on your device,
once the block at 0x801c0000 is set to "unmute", that setting it back to
mute has no effect?  Or was this asked as a result of confusion caused by
my typo?

> >> In my previous RFC, I described that zero bits have no effect in write
> >> transactions to 0x00008010051c. But this is wrong. The transaction has
> >> side effect to set the other options.
> > 
> > That would be correct.  0x00008010051c is the start of the FF400's
> > configuration block.  Fields within this set all manner of things including
> > phantom power status, input pads, optical port mode and so on.  Blindly
> > writing zeros will force the device into the physical state represented by
> > that zero byte pattern.
> > 
> >> In this patchset, the state of clock is changed on the way to initialize
> >> transaction.  This is a bit rude but unavoidable.  Fortunately, it's
> >> software's responsibility to read from flash memory and set initial
> >> configuration,
> > 
> > Not quite.  The device powers up with the configuration that is stored in
> > the internal flash memory and does not require any interaction with a
> > computer in order to be functional.  Software needs to synchronise itself
> > with this configuration so as to maintain it.  Forcing a particular
> > configuration is fine for an initial version of a driver, but in time there
> > is a need to be more sophisticated about how this is handled.  The setup of
> > these interfaces can be highly complex and many users rely on the flash
> > memory configuration to avoid time consuming setup at every power on. 
> > Forcing a configuration reset like this can certainly be worked around
> > during initial development, but in the long term the driver needs to
> > preserve device status.
> > 
> > I have several ideas about how this might look but need to wait to see what
> > framework we have to work within before any of that can be solidified.  The
> > primary difficulty is that the configuration register needs to be known by
> > all software components which interact with the device: not just the
> > streaming system.  A way to make the configuration available to usespace
> > will be needed.  In FFADO this was handled with a shared memory segment so
> > it didn't matter which component (streaming, mixer) was started first.  I
> > imagine in FFADO the configuration will be made available via hwdep, /proc/
> > or something similar, ideally in a manner which is transparent to the type
> > of Fireface device in use.
> > 
> > In summary: zeroing out the configuration block is fine for now, but in time
> > this will need to be revisited.
> 
> I'd like to drop the code to send write transactions to the address, and
> leave the decision to userspace. ...

That would be ideal as it would keep the kernel code as clean and
light-weight as possible.  However, the streaming system needs to know some
of the details as set by the configuration block because some of those
settings affect the number of channels being streamed.  The configuration
block is write-only (reading that address returns different information) so
it's not possible to interrogate the device for its current setting.  This
is why software needs to maintain a persistent copy of the device's
configuration that's accessible to both device configuration programs
(running in userspace) and streaming code (running in the kernel).  At the
very least the kernel will need to provide a pass-through interface to the
configuration block so it can keep track of the device's settings.  However,
rather than a non-descript binary block it would be better imho if a more
descriptive interface was used.

Regards
  jonathan

Re: [RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Takashi Sakamoto]

On Dec 22 2015 15:45, Jonathan Woithe wrote:
> On Mon, Dec 21, 2015 at 10:00:38PM +0900, Takashi Sakamoto wrote:
>> On Dec 21 2015 13:14, Jonathan Woithe wrote:
>>> On Sun, Dec 20, 2015 at 09:28:32PM +0900, Takashi Sakamoto wrote:
>>>> I note that just powering on, the device is muted.
>>>
>>> Only with respect to the audio streams from the PC.  All other
>>> hardware-based routing is set up as per the saved device configuration.
>>>
>>>> You can de-mute it by write
>>>> transaction. For example, by using 'firewire-request' command in
>>>> linux-firewire-utils(https://github.com/cladisch/linux-firewire-utils):
>>>>
>>>> $ ./firewire-request /dev/fw1 write 0x0000801c0000 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001000000010000000100000001000000
>>>>
>>>> Currently, libffado gives no way to do it via its mixer interface. Instead, done
>>>> in methods to start streaming. I think it better to move de-mute codes to mixer
>>>> initialization.
>>>
>>> The mutes controlled here apply only to the software audio streams.  I
>>> suspect they are utilised to eliminate noise during streaming setup and when
>>> the device is idle with respect to the computer.  When other systems mute
>>> individual playback channels under user control they do not use these mutes
>>> and instead do it in other ways.  The above mutes are only ever turned off
>>> and on as part of streaming setup and teardown.  As a result, I think Linux
>>> should do the same thing because this is the mode of operation used on other
>>> systems and is therefore the "supported" approach.
>>>
>>> In other words, I think we should do what all other systems do with these
>>> devices and treat this mute register as a part of the streaming setup
>>> procedure.
>>>
>>> Note in passing that the above firewire-request will only enable output for
>>> the first four audio streams.  Others (such as ADAT) will remain muted.  The
>>> correct initialisation of the 28 quadlets at 0x0000801c0000 is to set every
>>> quadlet to 1.
> 
> I made a typo at the end of that last paragraph: the statement should have
> read "set every quadlet to 0".  It's a mute state, so the unmuted state is
> obviously 0.  The above firewire-request will leave some higher-numbered
> channels muted which may be in the unused portion of the FF400 map (I
> haven't counted the zeros to know where the 0x1's start).  Apologies for any
> confusion caused.
> 
>> Could I request your opinion about the fact that the device keeps unmute
>> state once unmuted? If your insistent is reasonable, the device should
>> get mute every time to start to receive packets.
> 
> I'm not quite sure what you mean here.  Are you saying that on your device,
> once the block at 0x801c0000 is set to "unmute", that setting it back to
> mute has no effect?  Or was this asked as a result of confusion caused by
> my typo?

Regardless of your typo.

Once the device is unmute, then it keeps the state regardless of
starting/stopping streams. If the vendor uses the unmute state to avoid
any noises at starting packet streaming, the device should automatically
be muted when starting streaming or after stopping streaming.

I realize it means that unmute/mute are not related to streaming
functionality. Therefore, no reasons to implement it in streaming driver.

>>>> In my previous RFC, I described that zero bits have no effect in write
>>>> transactions to 0x00008010051c. But this is wrong. The transaction has
>>>> side effect to set the other options.
>>>
>>> That would be correct.  0x00008010051c is the start of the FF400's
>>> configuration block.  Fields within this set all manner of things including
>>> phantom power status, input pads, optical port mode and so on.  Blindly
>>> writing zeros will force the device into the physical state represented by
>>> that zero byte pattern.
>>>
>>>> In this patchset, the state of clock is changed on the way to initialize
>>>> transaction.  This is a bit rude but unavoidable.  Fortunately, it's
>>>> software's responsibility to read from flash memory and set initial
>>>> configuration,
>>>
>>> Not quite.  The device powers up with the configuration that is stored in
>>> the internal flash memory and does not require any interaction with a
>>> computer in order to be functional.  Software needs to synchronise itself
>>> with this configuration so as to maintain it.  Forcing a particular
>>> configuration is fine for an initial version of a driver, but in time there
>>> is a need to be more sophisticated about how this is handled.  The setup of
>>> these interfaces can be highly complex and many users rely on the flash
>>> memory configuration to avoid time consuming setup at every power on. 
>>> Forcing a configuration reset like this can certainly be worked around
>>> during initial development, but in the long term the driver needs to
>>> preserve device status.
>>>
>>> I have several ideas about how this might look but need to wait to see what
>>> framework we have to work within before any of that can be solidified.  The
>>> primary difficulty is that the configuration register needs to be known by
>>> all software components which interact with the device: not just the
>>> streaming system.  A way to make the configuration available to usespace
>>> will be needed.  In FFADO this was handled with a shared memory segment so
>>> it didn't matter which component (streaming, mixer) was started first.  I
>>> imagine in FFADO the configuration will be made available via hwdep, /proc/
>>> or something similar, ideally in a manner which is transparent to the type
>>> of Fireface device in use.
>>>
>>> In summary: zeroing out the configuration block is fine for now, but in time
>>> this will need to be revisited.
>>
>> I'd like to drop the code to send write transactions to the address, and
>> leave the decision to userspace. ...
> 
> That would be ideal as it would keep the kernel code as clean and
> light-weight as possible.  However, the streaming system needs to know some
> of the details as set by the configuration block because some of those
> settings affect the number of channels being streamed.  The configuration
> block is write-only (reading that address returns different information) so
> it's not possible to interrogate the device for its current setting.  This
> is why software needs to maintain a persistent copy of the device's
> configuration that's accessible to both device configuration programs
> (running in userspace) and streaming code (running in the kernel).  At the
> very least the kernel will need to provide a pass-through interface to the
> configuration block so it can keep track of the device's settings.  However,
> rather than a non-descript binary block it would be better imho if a more
> descriptive interface was used.

You have the wrong.

Please read fireface-proc.c or snd_ff_stream_get_clock() in
fireface-stream.c, then describe your opinion about the issue.


Regards

Takashi Sakamoto

Re: [RFC v2][PATCH 00/11] ALSA: fireface: new driver for RME Fireface series

[Jonathan Woithe]

On Tue, Dec 22, 2015 at 07:54:19PM +0900, Takashi Sakamoto wrote:
> Once the device is unmute, then it keeps the state regardless of
> starting/stopping streams. If the vendor uses the unmute state to avoid
> any noises at starting packet streaming, the device should automatically
> be muted when starting streaming or after stopping streaming.

The vendor unmutes this register when streaming is started for the first
time after device initialisation.  This is possibly due to noise suppression
during the initial device setup although I have not verified this
electrically.  In my view we should do the same since this is the way the
device is used (and therefore supported) on other operating systems.  If we
deviate from the way the vendor drives the interface we could introduce
subtle side effects for no good reason.

> I realize it means that unmute/mute are not related to streaming
> functionality. Therefore, no reasons to implement it in streaming driver.

We may have to agree to disagree on this point.  Where possible we should,
imho, drive the hardware according to the vendor's procedures.

> >> I'd like to drop the code to send write transactions to the address, and
> >> leave the decision to userspace. ...
> :
> Please read fireface-proc.c or snd_ff_stream_get_clock() in
> fireface-stream.c, then describe your opinion about the issue.

snd_ff_stream_get_clock() is exclusively about identifying the clock source
and rate.  Some of this information is available in the read-only status
registers starting at 0x801c0000 as you have found.  However, deducing the
rate from bits returned by a single quadlet read of 0x801c0004 is
undocumented and in any case will not be accurate for some sync modes and
clock sources.  This means that some configuration needs persistent storage
and the kernel driver is the logical place for this.  As an aside, the
status register content changes depending on the block read origin and
request size.

fireface-proc.c provides userspace with access to the status register, with
the same access caveats as for snd_ff_stream_get_clock().

At this point I am inclined to just wait to see what lands and fix up the
usability and interfacing issues later.

Regards
  jonathan

Re: [PATCH 02/11] fireface: postpone sound card registration

[Stefan Richter]

On Dec 20 Takashi Sakamoto wrote:
> Just after appearing on IEEE 1394 bus, this unit generates several bus
> resets. This is due to loading firmware from on-board flash memory and
> initialize hardware. It's better to postpone sound card registration.
> 
> This commit schedules workqueue to process actual probe processing
> 1 seconds after the last bus-reset. The card instance is kept at unit
> probe callback and released at card free callback. Therefore, when the
> actual probe processing fails, the memory block is wasted. This is due to
> simplify driver implementation.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/fireface/fireface.c | 59 ++++++++++++++++++++++++++++++++++----
>  sound/firewire/fireface/fireface.h |  3 ++
>  2 files changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
> index c8b7fe7..cf10e97 100644
> --- a/sound/firewire/fireface/fireface.c
> +++ b/sound/firewire/fireface/fireface.c
> @@ -10,6 +10,8 @@
>  
>  #define OUI_RME	0x000a35
>  
> +#define PROBE_DELAY_MS		(1 * MSEC_PER_SEC)
> +
>  MODULE_DESCRIPTION("RME Fireface series Driver");
>  MODULE_AUTHOR("Takashi Sakamoto <o-takashi@sakamocchi.jp>");
>  MODULE_LICENSE("GPL v2");
> @@ -32,16 +34,47 @@ static void ff_card_free(struct snd_card *card)
>  {
>  	struct snd_ff *ff = card->private_data;
>  
> +	/* The workqueue for registration uses the memory block. */
> +	cancel_work_sync(&ff->dwork.work);
> +
>  	fw_unit_put(ff->unit);
>  
>  	mutex_destroy(&ff->mutex);
>  }

The same comments apply as to patch "ALSA: dice: postpone card
registration":

  - use cancel_delayed_work_sync(&ff->dwork);

  - ff_card_free() is called via snd_card_free_when_closed() at
    snd_ff_remove() while &ff->mutex is held.  On the other hand, the
    worker do_probe() attempts to take the mutex too.  Hence this can
    deadlock.

    cancel_delayed_work_sync(&ff->dwork) needs to be called outside a
    ff->mutex protected section, certainly at the beginning of
    snd_ff_remove().

  - mutex_destroy(&ff->mutex); cannot be called here.
    snd_ff_remove() is still holding the mutex.

[No further comments below, only quoting the patch.]

> +static void do_probe(struct work_struct *work)
> +{
> +	struct snd_ff *ff = container_of(work, struct snd_ff, dwork.work);
> +	int err;
> +
> +	mutex_lock(&ff->mutex);
> +
> +	if (ff->card->shutdown || ff->probed)
> +		goto end;
> +
> +	err = snd_card_register(ff->card);
> +	if (err < 0)
> +		goto end;
> +
> +	ff->probed = true;
> +end:
> +	mutex_unlock(&ff->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 snd_ff_probe(struct fw_unit *unit,
>  			   const struct ieee1394_device_id *entry)
>  {
> +	struct fw_card *fw_card = fw_parent_device(unit)->card;
>  	struct snd_card *card;
>  	struct snd_ff *ff;
> +	unsigned long delay;
>  	int err;
>  
>  	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
> @@ -60,26 +93,40 @@ static int snd_ff_probe(struct fw_unit *unit,
>  
>  	name_card(ff);
>  
> -	err = snd_card_register(card);
> -	if (err < 0) {
> -		snd_card_free(card);
> -		return err;
> -	}
> +	/* Register this sound card later. */
> +	INIT_DEFERRABLE_WORK(&ff->dwork, do_probe);
> +	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
> +				fw_card->reset_jiffies - get_jiffies_64();
> +	schedule_delayed_work(&ff->dwork, delay);
>  
>  	return 0;
>  }
>  
>  static void snd_ff_update(struct fw_unit *unit)
>  {
> -	return;
> +	struct snd_ff *ff = 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 (!ff->probed) {
> +		delay = msecs_to_jiffies(PROBE_DELAY_MS) -
> +				(get_jiffies_64() - fw_card->reset_jiffies);
> +		mod_delayed_work(ff->dwork.wq, &ff->dwork, delay);
> +	}
>  }
>  
>  static void snd_ff_remove(struct fw_unit *unit)
>  {
>  	struct snd_ff *ff = dev_get_drvdata(&unit->device);
>  
> +	/* For a race condition of struct snd_card.shutdown. */
> +	mutex_lock(&ff->mutex);
> +
>  	/* No need to wait for releasing card object in this context. */
>  	snd_card_free_when_closed(ff->card);
> +
> +	mutex_unlock(&ff->mutex);
>  }
>  
>  static const struct ieee1394_device_id snd_ff_id_table[] = {
> diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
> index ab596c7..74877ef 100644
> --- a/sound/firewire/fireface/fireface.h
> +++ b/sound/firewire/fireface/fireface.h
> @@ -35,5 +35,8 @@ struct snd_ff {
>  	struct snd_card *card;
>  	struct fw_unit *unit;
>  	struct mutex mutex;
> +
> +	bool probed;
> +	struct delayed_work dwork;
>  };
>  #endif

-- 
Stefan Richter
-=====-===== ==-- ==---
http://arcgraph.de/sr/

Re: [PATCH 02/11] fireface: postpone sound card registration

[Stefan Richter]

On Dec 20 Takashi Sakamoto wrote:
> --- a/sound/firewire/fireface/fireface.c
> +++ b/sound/firewire/fireface/fireface.c
> @@ -10,6 +10,8 @@
>  
>  #define OUI_RME	0x000a35
>  
> +#define PROBE_DELAY_MS		(1 * MSEC_PER_SEC)
[...]
>  static int snd_ff_probe(struct fw_unit *unit,
>  			   const struct ieee1394_device_id *entry)
>  {
> +	struct fw_card *fw_card = fw_parent_device(unit)->card;
>  	struct snd_card *card;
>  	struct snd_ff *ff;
> +	unsigned long delay;
>  	int err;
>  
>  	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
> @@ -60,26 +93,40 @@ static int snd_ff_probe(struct fw_unit *unit,
>  
>  	name_card(ff);
>  
> -	err = snd_card_register(card);
> -	if (err < 0) {
> -		snd_card_free(card);
> -		return err;
> -	}
> +	/* Register this sound card later. */
> +	INIT_DEFERRABLE_WORK(&ff->dwork, do_probe);
> +	delay = msecs_to_jiffies(PROBE_DELAY_MS) +
> +				fw_card->reset_jiffies - get_jiffies_64();
> +	schedule_delayed_work(&ff->dwork, delay);
>  
>  	return 0;
>  }
>  
>  static void snd_ff_update(struct fw_unit *unit)
>  {
> -	return;
> +	struct snd_ff *ff = 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 (!ff->probed) {
> +		delay = msecs_to_jiffies(PROBE_DELAY_MS) -
> +				(get_jiffies_64() - fw_card->reset_jiffies);
> +		mod_delayed_work(ff->dwork.wq, &ff->dwork, delay);
> +	}
>  }

Obviously the same concerns about overflow as in Takashi Iwai's comment on
patch "ALSA: dice: postpone card registration" apply here too.

This makes me wonder:  What if you simply use a fixed delay here without
subtraction of the time between "now" and "last reset"?  This would make
the code simpler, but would of course result in varying and less
predictable scheduling of your deferred function.
-- 
Stefan Richter
-=====-===== ==-- ==---
http://arcgraph.de/sr/