[RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Takashi Sakamoto]

Hi,

This patchset is to add a new driver for RME Fireface series. This is still
working in progress but currently the new driver support MIDI functionality.

Unfortunately, ffado library can disturb this functionality.
In RME::Device::init_hardware() function, the library sends a write transaction
to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
ISO/IEC 13213. This is a worst case I describe in patch 03.

A workaround is to load this module after running ffado library. This module
sends write transactions with valid values and regain MIDI functionality.

I think it better that FFADO developers fixes the bug as long as they doesn't
support MIDI functionality.

Takashi Sakamoto (3):
  fireface: add skeleton for RME Fireface series
  fireface: add transaction support
  fireface: add support for MIDI functionality

 sound/firewire/Kconfig                         |   7 +
 sound/firewire/Makefile                        |   1 +
 sound/firewire/fireface/Makefile               |   2 +
 sound/firewire/fireface/fireface-midi.c        | 129 ++++++++
 sound/firewire/fireface/fireface-transaction.c | 388 +++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c             | 151 ++++++++++
 sound/firewire/fireface/fireface.h             |  73 +++++
 7 files changed, 751 insertions(+)
 create mode 100644 sound/firewire/fireface/Makefile
 create mode 100644 sound/firewire/fireface/fireface-midi.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 1/3] 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.

Just after appearing on IEEE 1394 bus, this unit generates several bus
resets. This is due to loading and enabling firmware from on-board flash
memory. Therefore, it's better to postopone calling snd_card_register()
(not yet).

Three types of firmware have been 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 | 139 +++++++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.h |  41 +++++++++++
 5 files changed, 190 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..7e21667
--- /dev/null
+++ b/sound/firewire/fireface/fireface.c
@@ -0,0 +1,139 @@
+/*
+ * fireface.c - a part of driver for RMW 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 int identify_model(struct snd_ff *ff,
+			  const struct ieee1394_device_id *entry)
+{
+	struct fw_device *fw_dev = fw_parent_device(ff->unit);
+	const char *model;
+
+	/* TODO: how to detect all of models? */
+	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);
+
+	return 0;
+}
+
+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);
+	spin_lock_init(&ff->lock);
+	dev_set_drvdata(&unit->device, ff);
+
+	err = identify_model(ff, entry);
+	if (err < 0)
+		goto error;
+
+	/*
+	 * TODO: the rest of work should be done in workqueue because of some
+	 * bus resets.
+	 */
+
+	err = snd_card_register(card);
+	if (err < 0)
+		goto error;
+
+	return err;
+error:
+	snd_card_free(card);
+	return err;
+}
+
+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..e72d53f
--- /dev/null
+++ b/sound/firewire/fireface/fireface.h
@@ -0,0 +1,41 @@
+/*
+ * 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;
+	spinlock_t lock;
+};
+#endif
-- 
2.5.0

[PATCH 2/3] 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 4 byte in LSB 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 and writing zero bit has no effect. Reading this register
always returns 0x00000000. 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 the 8 byte of LSB of the
address, thus the driver retries to allocate when gaining unusable range.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile               |   2 +-
 sound/firewire/fireface/fireface-transaction.c | 388 +++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c             |  10 +-
 sound/firewire/fireface/fireface.h             |  30 ++
 4 files changed, 428 insertions(+), 2 deletions(-)
 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..07a2b9c
--- /dev/null
+++ b/sound/firewire/fireface/fireface-transaction.c
@@ -0,0 +1,388 @@
+/*
+ * fireface-transaction.c - a part of driver for RMW Fireface series
+ *
+ * Copyright (c) 2015-2016 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "fireface.h"
+
+/*
+ * When return minus value, given argument is not MIDI status.
+ * When return 0, given argument is a beginning of system exclusive.
+ * When return the others, given argument is MIDI data.
+ */
+static inline int calculate_message_bytes(u8 status)
+{
+	switch (status) {
+	case 0xf6:	/* Tune request. */
+	case 0xf8:	/* Timing clock. */
+	case 0xfa:	/* Start. */
+	case 0xfb:	/* Continue. */
+	case 0xfc:	/* Stop. */
+	case 0xfe:	/* Active sensing. */
+	case 0xff:	/* System reset. */
+		return 1;
+	case 0xf1:	/* MIDI time code quarter frame. */
+	case 0xf3:	/* Song select. */
+		return 2;
+	case 0xf2:	/* Song position pointer. */
+		return 3;
+	case 0xf0:	/* Exclusive. */
+		return 0;
+	case 0xf7:	/* End of exclusive. */
+		break;
+	case 0xf4:	/* Undefined. */
+	case 0xf5:	/* Undefined. */
+	case 0xf9:	/* Undefined. */
+	case 0xfd:	/* Undefined. */
+		break;
+	default:
+		switch (status & 0xf0) {
+		case 0x80:	/* Note on. */
+		case 0x90:	/* Note off. */
+		case 0xa0:	/* Polyphonic key pressure. */
+		case 0xb0:	/* Control change and Mode change. */
+		case 0xe0:	/* Pitch bend change. */
+			return 3;
+		case 0xc0:	/* Program change. */
+		case 0xd0:	/* Channel pressure. */
+			return 2;
+		default:
+		break;
+		}
+	break;
+	}
+
+	return -EINVAL;
+}
+
+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_be32((byte << 8) << 16);
+}
+
+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];
+	u8 status;
+	int i, consume, 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;
+	}
+
+	/* Retrieve one MIDI byte to calculate length of the message. */
+	len = snd_rawmidi_transmit_peek(substream, buf,
+					SND_FF_MAXIMIM_MIDI_QUADS);
+	if (len <= 0)
+		return;
+
+	/* Not on system exclusives. */
+	if (ff->running_status[port] != 0xf0 && *buf != 0xf0) {
+		/* On running-status. */
+		if ((*buf & 0x80) != 0x80)
+			status = ff->running_status[port];
+		else
+			status = *buf;
+
+		/* Calculate consume bytes. */
+		consume = calculate_message_bytes(status);
+		if (consume <= 0)
+			return;
+
+		/* On running-status. */
+		if ((*buf & 0x80) != 0x80) {
+			if (len < consume - 1)
+				return;
+			consume -= 1;
+		} else {
+			if (len < consume)
+				return;
+			ff->running_status[port] = *buf;
+		}
+	} else {
+		/* On system exclusives. */
+		ff->running_status[port] = 0xf0;
+
+		/* Seek the end of exclusives. */
+		consume = 1;
+		for (i = 0; i < len; i++) {
+			if (buf[i] == 0xf7) {
+				ff->running_status[port] = 0x00;
+				break;
+			}
+			consume++;
+		}
+	}
+
+	for (i = consume - 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(),
+					    consume * NSEC_PER_SEC / 3125);
+	ff->rx_bytes[port] = consume;
+
+	/*
+	 * 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();
+
+	/* Wait response. */
+	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], consume * 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;
+	__be32 *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 = be32_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 >> 16) & 0xff;
+		if (index > 0) {
+			substream = ACCESS_ONCE(ff->tx_midi_substreams[0]);
+			if (substream != NULL) {
+				byte = (quad >> 24) & 0xff;
+				snd_rawmidi_receive(substream, &byte, 1);
+			}
+		}
+
+		/* Message in second port. */
+		index = quad & 0xff;
+		if (index > 0) {
+			substream = ACCESS_ONCE(ff->tx_midi_substreams[1]);
+			if (substream != NULL) {
+				byte = (quad >> 8) & 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;
+	__be32 reg;
+	int err;
+
+	/*
+	 * Controllers are allowed to register its node ID and 2 byte in MSB of
+	 * address to listen asynchronous transactions. This is the reason we
+	 * should pay attension to allocating the address.
+	 *
+	 * And the value of this register is aligned to little endian.
+	 */
+	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 LSB of this register are used to indicate fixed
+	 * offset of address to transfer asynchronous transaction.
+	 *  - 6th: 0x00000180
+	 *  - 5th: 0x00000100
+	 *  - 4th: 0x00000080
+	 *  - 3rd: 0x00000000
+	 *
+	 * Here, 3rd bit is used because controllers are not allowed to register
+	 * arbitrary address for this purpose.
+	 *
+	 * The 7th-8th bits in LSB of this register are used to the other
+	 * purpose.
+	 *
+	 * The 1st-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
+	 *
+	 * the value of zero for each bits has no meaning in this register.
+	 */
+	reg = cpu_to_be32(0x00000004);
+	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)
+{
+	__be32 reg;
+
+	/* Stop transferring and listening. */
+	reg = cpu_to_be32(0x00000003);
+	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 7e21667..d9cfac2 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -38,6 +38,8 @@ static void ff_card_free(struct snd_card *card)
 {
 	struct snd_ff *ff = card->private_data;
 
+	snd_ff_transaction_unregister(ff);
+
 	fw_unit_put(ff->unit);
 
 	mutex_destroy(&ff->mutex);
@@ -74,6 +76,10 @@ static int snd_ff_probe(struct fw_unit *unit,
 	 * bus resets.
 	 */
 
+	err = snd_ff_transaction_register(ff);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
@@ -86,7 +92,9 @@ error:
 
 static void snd_ff_update(struct fw_unit *unit)
 {
-	return;
+	struct snd_ff *ff = dev_get_drvdata(&unit->device);
+
+	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 e72d53f..914472a 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -31,11 +31,41 @@
 #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 {
 	struct snd_card *card;
 	struct fw_unit *unit;
 
 	struct mutex mutex;
 	spinlock_t lock;
+
+	/* 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];
+	__be32 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 3/3] 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 in write transactions.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/Makefile        |   2 +-
 sound/firewire/fireface/fireface-midi.c | 129 ++++++++++++++++++++++++++++++++
 sound/firewire/fireface/fireface.c      |   4 +
 sound/firewire/fireface/fireface.h      |   2 +
 4 files changed, 136 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..0c0f99f
--- /dev/null
+++ b/sound/firewire/fireface/fireface-midi.c
@@ -0,0 +1,129 @@
+/*
+ * 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;
+
+	spin_lock(&ff->lock);
+
+	if (up)
+		ACCESS_ONCE(ff->tx_midi_substreams[substream->number]) =
+								substream;
+	else
+		ACCESS_ONCE(ff->tx_midi_substreams[substream->number]) = NULL;
+
+	spin_unlock(&ff->lock);
+}
+
+static void midi_playback_trigger(struct snd_rawmidi_substream *substream,
+				  int up)
+{
+	struct snd_ff *ff = substream->rmidi->private_data;
+
+	spin_lock(&ff->lock);
+
+	if (up || !ff->rx_midi_error[substream->number])
+		schedule_work(&ff->rx_midi_work[substream->number]);
+
+	spin_unlock(&ff->lock);
+}
+
+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 d9cfac2..1d0eb86f 100644
--- a/sound/firewire/fireface/fireface.c
+++ b/sound/firewire/fireface/fireface.c
@@ -80,6 +80,10 @@ static int snd_ff_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_ff_create_midi_devices(ff);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/fireface/fireface.h b/sound/firewire/fireface/fireface.h
index 914472a..017bbb5 100644
--- a/sound/firewire/fireface/fireface.h
+++ b/sound/firewire/fireface/fireface.h
@@ -68,4 +68,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

Re: [FFADO-devel] [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Илья]

Thanks a lot! I was waiting for this. And I'll observe an ALSA-FW development (I mainly intersted in RME driver).  


--
Пожалуйста, цитируйте ВСЮ переписку!

С уважением,
Илья .

>Воскресенье,  6 декабря 2015, 22:23 +09:00 от Takashi Sakamoto <o-takashi@sakamocchi.jp>:
>
>Hi,
>
>This patchset is to add a new driver for RME Fireface series. This is still
>working in progress but currently the new driver support MIDI functionality.
>
>Unfortunately, ffado library can disturb this functionality.
>In RME::Device::init_hardware() function, the library sends a write transaction
>to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
>ISO/IEC 13213. This is a worst case I describe in patch 03.
>
>A workaround is to load this module after running ffado library. This module
>sends write transactions with valid values and regain MIDI functionality.
>
>I think it better that FFADO developers fixes the bug as long as they doesn't
>support MIDI functionality.
>
>Takashi Sakamoto (3):
>  fireface: add skeleton for RME Fireface series
>  fireface: add transaction support
>  fireface: add support for MIDI functionality
>
> sound/firewire/Kconfig                         |   7 +
> sound/firewire/Makefile                        |   1 +
> sound/firewire/fireface/Makefile               |   2 +
> sound/firewire/fireface/fireface-midi.c        | 129 ++++++++
> sound/firewire/fireface/fireface-transaction.c | 388 +++++++++++++++++++++++++
> sound/firewire/fireface/fireface.c             | 151 ++++++++++
> sound/firewire/fireface/fireface.h             |  73 +++++
> 7 files changed, 751 insertions(+)
> create mode 100644 sound/firewire/fireface/Makefile
> create mode 100644 sound/firewire/fireface/fireface-midi.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
>
>
>------------------------------------------------------------------------------
>Go from Idea to Many App Stores Faster with Intel(R) XDK
>Give your users amazing mobile app experiences with Intel(R) XDK.
>Use one codebase in this all-in-one HTML5 development environment.
>Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
>http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
>_______________________________________________
>FFADO-devel mailing list
>FFADO-devel@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/ffado-devel

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

Re: [FFADO-devel] [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Jonathan Woithe]

On Sun, Dec 06, 2015 at 10:23:41PM +0900, Takashi Sakamoto wrote:
> Unfortunately, ffado library can disturb this functionality.
> In RME::Device::init_hardware() function, the library sends a write transaction
> to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
> ISO/IEC 13213. This is a worst case I describe in patch 03.

The number which FFADO writes to this register is not invalid: it is in fact
the same number which is used in drivers on other operating systems
(obtained from protocol analysis).

> I think it better that FFADO developers fixes the bug as long as they doesn't
> support MIDI functionality.

As above, this is not exactly a bug because other systems set that register
to the value which FFADO uses.  In the interests of interoperability I'm
willing to remove manipulation of this register from FFADO, but I suggest
that in time the ALSA driver should consider setting this register as is
done under other systems or else we could introduce subtle behavioural
differences down the track - at least until such time as we understand what
the high part of that register does.

jonathan

Re: [FFADO-devel] [PATCH 1/3] fireface: add skeleton for RME Fireface series

[Jonathan Woithe]

On Sun, Dec 06, 2015 at 10:23:42PM +0900, Takashi Sakamoto wrote:
> Three types of firmware have been released by RME GmbH; for Fireface 400,
> for Fireface 800 and for UCX/802/UFX.

The FF400 and FF800 firmwares are pretty much the same.  The control
protocol does differ between the two but at a fundamental level they are the
same - especially where streaming is concerned.  They are similar enough
that it would not make sense to implement a separate driver for the FF800.

You are correct in concluding that the latter is a totally new firmware.  In
addition to the UFX/UCX/802 RME have also ported as much as is relevant to
the FF800.  The mixer interface (which doesn't directly concern ALSA) is
completely different.  I have not yet investigated the streaming part in
full but my initial investigations suggest that it may not be significantly
different.

> It's reasonable that these models use different protocol for
> communication.

For FF400 and FF800 the streaming protocol is very similar.  I expect the
same to hold for the newer devices too, but will be able to say more once I
get a chance to fully probe the UFX.  In other words, at this stage it is
fair to say that the streaming component for all devices will be similar,
and there is probably no reason to consider there being three completely
different firmwares.  There are obviously some differences which will need
to be allowed for in places, but the same is true for almost every device
family.

In the patch:

> diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
> new file mode 100644
> index 0000000..7e21667
> --- /dev/null
> +++ b/sound/firewire/fireface/fireface.c
> @@ -0,0 +1,139 @@
> +/*
> + * fireface.c - a part of driver for RMW Fireface series

Typo: it should be "RME", obviously.

> +	/* TODO: how to detect all of models? */
> +	model = "Fireface 400";

Though the unit version field of the ConfigROM.

Once I get my head around git and how best to keep track of these patch
series I can provide a patch for this.  Despite several attempts to truly
understand git over the last few years I have so far failed abysmally.  :-(

jonathan

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Takashi Sakamoto]

Hi,

On Dec 07 2015 06:57, Jonathan Woithe wrote:
> On Sun, Dec 06, 2015 at 10:23:41PM +0900, Takashi Sakamoto wrote:
>> Unfortunately, ffado library can disturb this functionality.
>> In RME::Device::init_hardware() function, the library sends a write transaction
>> to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
>> ISO/IEC 13213. This is a worst case I describe in patch 03.
>
> The number which FFADO writes to this register is not invalid: it is in fact
> the same number which is used in drivers on other operating systems
> (obtained from protocol analysis).

No.

As long as I tested with a debug option to firewire-ohci module, it 
sends write transaction with '01000000'. This value includes invalid 
node ID.

ConfigRom::getNodeId() returns the invalid value. I think there's 
something wrong coding, such like forgetting initialization or update of 
the instance.

>> I think it better that FFADO developers fixes the bug as long as they doesn't
>> support MIDI functionality.
>
> As above, this is not exactly a bug because other systems set that register
> to the value which FFADO uses.  In the interests of interoperability I'm
> willing to remove manipulation of this register from FFADO, but I suggest
> that in time the ALSA driver should consider setting this register as is
> done under other systems or else we could introduce subtle behavioural
> differences down the track - at least until such time as we understand what
> the high part of that register does.


Regards

Takashi Sakamoto

Re: [PATCH 1/3] fireface: add skeleton for RME Fireface series

[Takashi Sakamoto]

Hi,

On Dec 07 2015 07:13, Jonathan Woithe wrote:
> On Sun, Dec 06, 2015 at 10:23:42PM +0900, Takashi Sakamoto wrote:
>> Three types of firmware have been released by RME GmbH; for Fireface 400,
>> for Fireface 800 and for UCX/802/UFX.
>
> The FF400 and FF800 firmwares are pretty much the same.  The control
> protocol does differ between the two but at a fundamental level they are the
> same - especially where streaming is concerned.  They are similar enough
> that it would not make sense to implement a separate driver for the FF800.
>
> You are correct in concluding that the latter is a totally new firmware.  In
> addition to the UFX/UCX/802 RME have also ported as much as is relevant to
> the FF800.  The mixer interface (which doesn't directly concern ALSA) is
> completely different.  I have not yet investigated the streaming part in
> full but my initial investigations suggest that it may not be significantly
> different.
>
>> It's reasonable that these models use different protocol for
>> communication.
>
> For FF400 and FF800 the streaming protocol is very similar.  I expect the
> same to hold for the newer devices too, but will be able to say more once I
> get a chance to fully probe the UFX.  In other words, at this stage it is
> fair to say that the streaming component for all devices will be similar,
> and there is probably no reason to consider there being three completely
> different firmwares.  There are obviously some differences which will need
> to be allowed for in places, but the same is true for almost every device
> family.

If so, why the vendor (RME GmbH) produces these different firmwares? 
Could I request your opinions about it?

> In the patch:
>
>> diff --git a/sound/firewire/fireface/fireface.c b/sound/firewire/fireface/fireface.c
>> new file mode 100644
>> index 0000000..7e21667
>> --- /dev/null
>> +++ b/sound/firewire/fireface/fireface.c
>> @@ -0,0 +1,139 @@
>> +/*
>> + * fireface.c - a part of driver for RMW Fireface series
>
> Typo: it should be "RME", obviously.
>
>> +	/* TODO: how to detect all of models? */
>> +	model = "Fireface 400";
>
> Though the unit version field of the ConfigROM.
>
> Once I get my head around git and how best to keep track of these patch
> series I can provide a patch for this.  Despite several attempts to truly
> understand git over the last few years I have so far failed abysmally.  :-(

Currently, I have no direction to support models except for FF400 
because I've never seen their packets. Even if you insists that they are 
similar, actually they're somewhat different (and I guess that the 
difference is larger than you expect, at least, isochronous packet 
streaming management).


Regards

Takashi Sakamoto

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Jonathan Woithe]

On Mon, Dec 07, 2015 at 10:27:08AM +0900, Takashi Sakamoto wrote:
> On Dec 07 2015 06:57, Jonathan Woithe wrote:
> >On Sun, Dec 06, 2015 at 10:23:41PM +0900, Takashi Sakamoto wrote:
> >>Unfortunately, ffado library can disturb this functionality.
> >>In RME::Device::init_hardware() function, the library sends a write transaction
> >>to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
> >>ISO/IEC 13213. This is a worst case I describe in patch 03.
> >
> >The number which FFADO writes to this register is not invalid: it is in fact
> >the same number which is used in drivers on other operating systems
> >(obtained from protocol analysis).
> 
> No.
> 
> As long as I tested with a debug option to firewire-ohci module, it sends
> write transaction with '01000000'. This value includes invalid node ID.

Curious.  Fair enough (although I was confused by the reference to the
"higher part" in the commit message: it implied the higher part of the
address in my mind, but in fact the issue with the node ID relates to the
lower part.  Whatever.  I'll look into it at some point.

Regards
  jonathan

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Takashi Sakamoto]

Hi,

On Dec 07 2015 10:37, Jonathan Woithe wrote:
> On Mon, Dec 07, 2015 at 10:27:08AM +0900, Takashi Sakamoto wrote:
>> On Dec 07 2015 06:57, Jonathan Woithe wrote:
>>> On Sun, Dec 06, 2015 at 10:23:41PM +0900, Takashi Sakamoto wrote:
>>>> Unfortunately, ffado library can disturb this functionality.
>>>> In RME::Device::init_hardware() function, the library sends a write transaction
>>>> to 0x0000801003f4 with invalid value as higher part of address in IEEE 1212 or
>>>> ISO/IEC 13213. This is a worst case I describe in patch 03.
>>>
>>> The number which FFADO writes to this register is not invalid: it is in fact
>>> the same number which is used in drivers on other operating systems
>>> (obtained from protocol analysis).
>>
>> No.
>>
>> As long as I tested with a debug option to firewire-ohci module, it sends
>> write transaction with '01000000'. This value includes invalid node ID.
>
> Curious.  Fair enough (although I was confused by the reference to the
> "higher part" in the commit message: it implied the higher part of the
> address in my mind, but in fact the issue with the node ID relates to the
> lower part.  Whatever.  I'll look into it at some point.

In IEEE 1212 (or ISO/IEC 13213), 64 bit addressing is defined. IEEE 1394 
utilize the specification.

Here is a actual example of the 64 bit address:
  * FFC1'FFFF'F000'0904 (oPCR[0] in node FFC1)
  * FFC2'FFFE'0000'0000 (the first address of Private space in node FFC2)
  * FFC3'ECC0'8000'0000 (the address of Echo Fireworks Command in node FFC3)

I mean the 'higher part' is 4 byte in MSB of the address. Drivers 
register the 4 byte to 0x'0000'8010'03f4, then Fireface 400 transfers 
asynchronous transactions to an address including the 4 byte in its MSB.


Regards

Takashi Sakamoto

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Takashi Sakamoto]

On Dec 07 2015 10:52, Takashi Sakamoto wrote:
> Hi,
>
> On Dec 07 2015 10:37, Jonathan Woithe wrote:
>> On Mon, Dec 07, 2015 at 10:27:08AM +0900, Takashi Sakamoto wrote:
>>> On Dec 07 2015 06:57, Jonathan Woithe wrote:
>>>> On Sun, Dec 06, 2015 at 10:23:41PM +0900, Takashi Sakamoto wrote:
>>>>> Unfortunately, ffado library can disturb this functionality.
>>>>> In RME::Device::init_hardware() function, the library sends a write
>>>>> transaction
>>>>> to 0x0000801003f4 with invalid value as higher part of address in
>>>>> IEEE 1212 or
>>>>> ISO/IEC 13213. This is a worst case I describe in patch 03.
>>>>
>>>> The number which FFADO writes to this register is not invalid: it is
>>>> in fact
>>>> the same number which is used in drivers on other operating systems
>>>> (obtained from protocol analysis).
>>>
>>> No.
>>>
>>> As long as I tested with a debug option to firewire-ohci module, it
>>> sends
>>> write transaction with '01000000'. This value includes invalid node ID.
>>
>> Curious.  Fair enough (although I was confused by the reference to the
>> "higher part" in the commit message: it implied the higher part of the
>> address in my mind, but in fact the issue with the node ID relates to the
>> lower part.  Whatever.  I'll look into it at some point.
>
> In IEEE 1212 (or ISO/IEC 13213), 64 bit addressing is defined. IEEE 1394
> utilize the specification.
>
> Here is a actual example of the 64 bit address:
>   * FFC1'FFFF'F000'0904 (oPCR[0] in node FFC1)
>   * FFC2'FFFE'0000'0000 (the first address of Private space in node FFC2)
>   * FFC3'ECC0'8000'0000 (the address of Echo Fireworks Command in node
> FFC3)
>
> I mean the 'higher part' is 4 byte in MSB of the address. Drivers
> register the 4 byte to 0x'0000'8010'03f4, then Fireface 400 transfers
> asynchronous transactions to an address including the 4 byte in its MSB.

More explaination.

If OHCI 1394 host controller gets node ID of FFC7 on the IEEE 1394 bus 
and ALSA Fireface driver allocates local address of 0x'4567'0000'0000 of 
the controller, then the driver must register the value of '0xFFC74567' 
to Fireface 400.

Actually, for Fireface 400, the value should be aligned to little 
endiann. Therefore, the driver sends write transaction with value of 
'0x6745C7FF'.

In my understanding, 0x0000 for node ID is invalid. Furthermore, 0x0010 
is not always true because the address range which drivers allocate on 
the controller is different depending on situation.


Regards

Takashi Sakamoto

Re: [PATCH 1/3] fireface: add skeleton for RME Fireface series

[Jonathan Woithe]

On Mon, Dec 07, 2015 at 10:32:26AM +0900, Takashi Sakamoto wrote:
> On Dec 07 2015 07:13, Jonathan Woithe wrote:
> >>It's reasonable that these models use different protocol for
> >>communication.
> >
> >For FF400 and FF800 the streaming protocol is very similar.  I expect the
> >same to hold for the newer devices too, but will be able to say more once I
> >get a chance to fully probe the UFX.  In other words, at this stage it is
> >fair to say that the streaming component for all devices will be similar,
> >and there is probably no reason to consider there being three completely
> >different firmwares.  There are obviously some differences which will need
> >to be allowed for in places, but the same is true for almost every device
> >family.
> 
> If so, why the vendor (RME GmbH) produces these different firmwares? Could
> I request your opinions about it?

The hardware used is different in each model.  The Fireface product line is
FPGA-based and as such the "firmware" (which is as much to do with the FPGA
bitstream as it is to do with software) is very tightly bound to the exact
nature of the componentry which the FPGA needs to interface with.  This is
why there are separate firmwares for each model.  However, the programming
interface exposed to the bus by the different firmwares have a high degree
of similarity.  There are some differences which are mostly evolutionary,
corresponding to improved ways to implement things in the FPGA, better PCB
layour, and so on.  However, the basic ideas remain the same.  This applies
to the FF400 and FF800 and I expect the streaming portion of the newer units
as well.

RME has revised the signal routing and control portions of their interfaces
in the last few years which has, I expect, changed the way mixer and DSP
control is implemented.  I expect this doesn't impact the streaming
components significantly, although as noted earlier I need to confirm this. 
This newer implementation is known as TotalMixFX while the earlier one was
referred to as TotalMix.

To summarise, the firmware which goes into an RME Fireface interface is
predominantly an FPGA configuration bitstream and is therefore tightly bound
to the underlying hardware.  Therefore there is per-device firmware. 
However, the exposed interface remains consistent across devices.

Does that address your concerns, or have I misinterpreted what you are
asking or referring to?

> >>+	/* TODO: how to detect all of models? */
> >>+	model = "Fireface 400";
> >
> >Though the unit version field of the ConfigROM.
> >
> >Once I get my head around git and how best to keep track of these patch
> >series I can provide a patch for this.  Despite several attempts to truly
> >understand git over the last few years I have so far failed abysmally.  :-(
> 
> Currently, I have no direction to support models except for FF400 because
> I've never seen their packets. Even if you insists that they are similar,
> actually they're somewhat different (and I guess that the difference is
> larger than you expect, at least, isochronous packet streaming management).

I have been working with both the FF400 and the FF800 at the driver level
since 2009.  I am intimately familiar with the isochronous streaming systems
of both and don't have to guess what the differences might be.  I know what
the differences are, and as I eluded to earlier they are not much more than
superficial.

Regards
  jonathan

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Jonathan Woithe]

On Mon, Dec 07, 2015 at 11:05:21AM +0900, Takashi Sakamoto wrote:
> More explaination.
> :

Thanks for posting the extended explanation.

> Furthermore, 0x0010 is not always true because the address range which
> drivers allocate on the controller is different depending on situation.

In general that's true.  For the FF400 at least though the non-nodeID
component is effectively hard coded on other systems.  If this is used
solely as an address then we wouldn't have to follow that convention
obviously, but this assertion would have to be confirmed valid in practice.

Regards
  jonathan

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Takashi Sakamoto]

Hi,

On Dec 07 2015 11:21, Jonathan Woithe wrote:
> In general that's true.  For the FF400 at least though the non-nodeID
> component is effectively hard coded on other systems.  If this is used
> solely as an address then we wouldn't have to follow that convention
> obviously, but this assertion would have to be confirmed valid in practice.

I think you mention about a part of value except for node ID. In my 
former example (0xFFC74567), it's 0x4547.

You mean that Fireface 400 ignores the part and sends transactions to 
the fixed local address (i.e. 0x'0001'0000'0000).

Although, this is against the fact that I got in my experiences with 
Fireface 400. The model can reasonably send transactions to the address 
which the driver registered.


Regards

Takashi Sakamoto

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Jonathan Woithe]

On Mon, Dec 07, 2015 at 11:42:23AM +0900, Takashi Sakamoto wrote:
> On Dec 07 2015 11:21, Jonathan Woithe wrote:
> >In general that's true.  For the FF400 at least though the non-nodeID
> >component is effectively hard coded on other systems.  If this is used
> >solely as an address then we wouldn't have to follow that convention
> >obviously, but this assertion would have to be confirmed valid in practice.
> 
> I think you mention about a part of value except for node ID.

Yes, that's what I was referring to.  Sorry for not being clearer.

> In my former example (0xFFC74567), it's 0x4547.

Right.  Node ID is in the upper 16 bits, and another value (0x4547 in the
above example) in the lower 16 bits.

> You mean that Fireface 400 ignores the part and sends transactions to the
> fixed local address (i.e. 0x'0001'0000'0000).

No, what I meant was that the low part (the 0x4547 in your example) is
always 0x0001 on other systems for the FF400.  Apologies for the confusion.

> The model can reasonably send transactions to the address which the driver
> registered.

Ok, that's good.  It means we're free to choose exactly what suits us.

jonathan

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Clemens Ladisch]

Jonathan Woithe wrote:
> On Mon, Dec 07, 2015 at 11:05:21AM +0900, Takashi Sakamoto wrote:
>> Furthermore, 0x0010 is not always true because the address range which
>> drivers allocate on the controller is different depending on situation.
>
> In general that's true.  For the FF400 at least though the non-nodeID
> component is effectively hard coded on other systems.

That's just because this value happens to be the first free address in
the FireWire address space.  If you had other FireWire devices whose
drivers allocated addresses, you would see different values.  (Maybe
even for multiple FF devices, but the driver might be able to share the
range for all devices.)


Regards,
Clemens

Re: [RFC][PATCH 0/3] ALSA: fireface: new driver for RME Fireface series (MIDI only)

[Jonathan Woithe]

On Mon, Dec 07, 2015 at 08:37:22AM +0100, Clemens Ladisch wrote:
> Jonathan Woithe wrote:
> > On Mon, Dec 07, 2015 at 11:05:21AM +0900, Takashi Sakamoto wrote:
> >> Furthermore, 0x0010 is not always true because the address range which
> >> drivers allocate on the controller is different depending on situation.
> >
> > In general that's true.  For the FF400 at least though the non-nodeID
> > component is effectively hard coded on other systems.
> 
> That's just because this value happens to be the first free address in
> the FireWire address space.

It's hardcoded (in the way I subsequently detailed) on at least one other
(non-Linux) system: when that particular FF400 register is written the upper
16 bits are a node ID and the lower 16 bits are unconditionally set to
0x0001.  That's not to say that this is the best way, but it's what's done
on that system, with all the caveats that this implies.

In the above system there is no explicit consideration of what addresses
happen to be free.  The address which is chosen does not appear to be the
lowest free address: there would be plenty unused addresses below it in
general.

> If you had other FireWire devices whose drivers allocated addresses, you
> would see different values.

For this reason (that is, other drivers/devices grabbing the resulting
address range) the methodology I detailed above for choosing the target
address is perhaps not the most appropriate for Linux.  Takashi S's tests
indicate that there is no hidden functionality controlled by the lower 16
bits of this register so we are free to adopt our own address selection
philosphy.

> (Maybe even for multiple FF devices, but the driver might be able to share
> the range for all devices.)

The multiple FF400 case would, I expect, be disambiguated by the inclusion
of the node ID in the register value (and hense the address listened to).

Regards
  jonathan

Re: [PATCH 2/3] fireface: add transaction support

[Clemens Ladisch]

Takashi Sakamoto wrote:
> +static void transmit_midi_msg(struct snd_ff *ff, unsigned int port)
> ...
> +	/* Retrieve one MIDI byte to calculate length of the message. */
> +	len = snd_rawmidi_transmit_peek(substream, buf,
> +					SND_FF_MAXIMIM_MIDI_QUADS);

SND_FF_MAXIMIM_MIDI_QUADS does not have the value 1.

> +		/* Calculate consume bytes. */
> +		consume = calculate_message_bytes(status);
> +		if (consume <= 0)
> +			return;

As far as I can see, sending one of the "undefined" bytes can stop the
stream permanently.  Invalid bytes need to be acked to ignore/remove
them.


Regards,
Clemens

Re: [PATCH 2/3] fireface: add transaction support

[Takashi Sakamoto]

On Dec 08 2015 19:22, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> +static void transmit_midi_msg(struct snd_ff *ff, unsigned int port)
>> ...
>> +	/* Retrieve one MIDI byte to calculate length of the message. */
>> +	len = snd_rawmidi_transmit_peek(substream, buf,
>> +					SND_FF_MAXIMIM_MIDI_QUADS);
> 
> SND_FF_MAXIMIM_MIDI_QUADS does not have the value 1.

Oops. It's my mistake to left the message as what it had been.
Originally, I programmed to retrieve one byte. Later, I investigated the
maximum message size and changed the code.

>> +		/* Calculate consume bytes. */
>> +		consume = calculate_message_bytes(status);
>> +		if (consume <= 0)
>> +			return;
> 
> As far as I can see, sending one of the "undefined" bytes can stop the
> stream permanently.  Invalid bytes need to be acked to ignore/remove
> them.

Exactly. We should find better way to handle such messages. Do you have
any good ideas?


Thanks

Takashi Sakamoto

Re: [PATCH 2/3] fireface: add transaction support

[Clemens Ladisch]

Takashi Sakamoto wrote:
> On Dec 08 2015 19:22, Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> +		/* Calculate consume bytes. */
>>> +		consume = calculate_message_bytes(status);
>>> +		if (consume <= 0)
>>> +			return;
>>
>> As far as I can see, sending one of the "undefined" bytes can stop the
>> stream permanently.  Invalid bytes need to be acked to ignore/remove
>> them.
>
> Exactly. We should find better way to handle such messages. Do you have
> any good ideas?

Call snd_rawmidi_transmit_ack(, 1) and continue.


Regards,
Clemens

Re: [PATCH 2/3] fireface: add transaction support

[Takashi Sakamoto]

On Dec 08 2015 20:29, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> On Dec 08 2015 19:22, Clemens Ladisch wrote:
>>> Takashi Sakamoto wrote:
>>>> +		/* Calculate consume bytes. */
>>>> +		consume = calculate_message_bytes(status);
>>>> +		if (consume <= 0)
>>>> +			return;
>>>
>>> As far as I can see, sending one of the "undefined" bytes can stop the
>>> stream permanently.  Invalid bytes need to be acked to ignore/remove
>>> them.
>>
>> Exactly. We should find better way to handle such messages. Do you have
>> any good ideas?
> 
> Call snd_rawmidi_transmit_ack(, 1) and continue.

$ git diff
diff --git a/sound/firewire/fireface/fireface-transaction.c
b/sound/firewire/fireface/fireface-transaction.c
index 07a2b9c..6b8c7a8 100644
--- a/sound/firewire/fireface/fireface-transaction.c
+++ b/sound/firewire/fireface/fireface-transaction.c
@@ -148,8 +148,10 @@ static void transmit_midi_msg(struct snd_ff *ff,
unsigned int port)

                /* Calculate consume bytes. */
                consume = calculate_message_bytes(status);
-               if (consume <= 0)
+               if (consume <= 0) {
+                       snd_rawmidi_transmit_ack(substream, 1);
                        return;
+               }

                /* On running-status. */
                if ((*buf & 0x80) != 0x80) {

Hm. This looks simple and works better, while I suspect that this is
appropriate to device driver, because this idea drops the message from
userspace. This is against a principle that device drivers just pass
data from a side to another side without censoring and modification.

I think it better to transfer the message to the device, even if it's
invalid in MIDI spec. It's what the userspace wants.


Thanks

Takashi Sakamoto

Re: [PATCH 2/3] fireface: add transaction support

[Clemens Ladisch]

Takashi Sakamoto wrote:
> On Dec 08 2015 20:29, Clemens Ladisch wrote:
>> Takashi Sakamoto wrote:
>>> On Dec 08 2015 19:22, Clemens Ladisch wrote:
>>>> Takashi Sakamoto wrote:
>>>>> +		/* Calculate consume bytes. */
>>>>> +		consume = calculate_message_bytes(status);
>>>>> +		if (consume <= 0)
>>>>> +			return;
>>>>
>>>> As far as I can see, sending one of the "undefined" bytes can stop the
>>>> stream permanently.  Invalid bytes need to be acked to ignore/remove
>>>> them.
>>>
>>> Exactly. We should find better way to handle such messages. Do you have
>>> any good ideas?
>>
>> Call snd_rawmidi_transmit_ack(, 1) and continue.
>
> $ git diff
> diff --git a/sound/firewire/fireface/fireface-transaction.c
> b/sound/firewire/fireface/fireface-transaction.c
> index 07a2b9c..6b8c7a8 100644
> --- a/sound/firewire/fireface/fireface-transaction.c
> +++ b/sound/firewire/fireface/fireface-transaction.c
> @@ -148,8 +148,10 @@ static void transmit_midi_msg(struct snd_ff *ff,
> unsigned int port)
>
>                 /* Calculate consume bytes. */
>                 consume = calculate_message_bytes(status);
> -               if (consume <= 0)
> +               if (consume <= 0) {
> +                       snd_rawmidi_transmit_ack(substream, 1);
>                         return;
> +               }
>
>                 /* On running-status. */
>                 if ((*buf & 0x80) != 0x80) {
>
> Hm. This looks simple and works better, while I suspect that this is
> appropriate to device driver, because this idea drops the message from
> userspace. This is against a principle that device drivers just pass
> data from a side to another side without censoring and modification.
>
> I think it better to transfer the message to the device, even if it's
> invalid in MIDI spec. It's what the userspace wants.

The code takes care to send entire MIDI messages.
Is that actually required by the FF?  (Windows does not have a raw MIDI
interface, so this problem could not happen there.)


Regards,
Clemens

Re: [PATCH 2/3] fireface: add transaction support

[Takashi Sakamoto]

On Dec 08 2015 21:36, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> On Dec 08 2015 20:29, Clemens Ladisch wrote:
>>> Takashi Sakamoto wrote:
>>>> On Dec 08 2015 19:22, Clemens Ladisch wrote:
>>>>> Takashi Sakamoto wrote:
>>>>>> +		/* Calculate consume bytes. */
>>>>>> +		consume = calculate_message_bytes(status);
>>>>>> +		if (consume <= 0)
>>>>>> +			return;
>>>>>
>>>>> As far as I can see, sending one of the "undefined" bytes can stop the
>>>>> stream permanently.  Invalid bytes need to be acked to ignore/remove
>>>>> them.
>>>>
>>>> Exactly. We should find better way to handle such messages. Do you have
>>>> any good ideas?
>>>
>>> Call snd_rawmidi_transmit_ack(, 1) and continue.
>>
>> $ git diff
>> diff --git a/sound/firewire/fireface/fireface-transaction.c
>> b/sound/firewire/fireface/fireface-transaction.c
>> index 07a2b9c..6b8c7a8 100644
>> --- a/sound/firewire/fireface/fireface-transaction.c
>> +++ b/sound/firewire/fireface/fireface-transaction.c
>> @@ -148,8 +148,10 @@ static void transmit_midi_msg(struct snd_ff *ff,
>> unsigned int port)
>>
>>                 /* Calculate consume bytes. */
>>                 consume = calculate_message_bytes(status);
>> -               if (consume <= 0)
>> +               if (consume <= 0) {
>> +                       snd_rawmidi_transmit_ack(substream, 1);
>>                         return;
>> +               }
>>
>>                 /* On running-status. */
>>                 if ((*buf & 0x80) != 0x80) {
>>
>> Hm. This looks simple and works better, while I suspect that this is
>> appropriate to device driver, because this idea drops the message from
>> userspace. This is against a principle that device drivers just pass
>> data from a side to another side without censoring and modification.
>>
>> I think it better to transfer the message to the device, even if it's
>> invalid in MIDI spec. It's what the userspace wants.
> 
> The code takes care to send entire MIDI messages.
> Is that actually required by the FF?  (Windows does not have a raw MIDI
> interface, so this problem could not happen there.)

Windows driver performs it so I immitate it.

I confirmed that bytes of one MIDI message in continuous two
asynchronous transactions are handled correctly. So we can purge the
codes for calculation of the number of bytes in the MIDI message.


Thanks

Takashi Sakamoto