All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
@ 2015-10-12 10:10 Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 1/5] ALSA: firewire-tascam: add support for incoming MIDI messages by asynchronous transaction Takashi Sakamoto
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

Hi,

TASCAM FireWire series driver may be newly available in Linux 4.4. For
the driver, this patchset adds ALSA MIDI ports to communicate to physical
MIDI ports.

This patchset includes some fixes, and updates my former post:

[alsa-devel] [PATCH 00/25 v2] ALSA: support AMDTP variants
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-August/096739.html

Changes:
 * support system exclusive messages
 * drop virtual MIDI ports
 * drop MMAP support via hwdep interface for status and control message

Although TASCAM FireWire series has physical controls, this patchset
doesn't support them. I think some facilities are required to enable this
functionality:
 * handling MIDI messages on virtual MIDI ports by this driver
 * handling status and control messages in isochronous packet by this
   driver
 * parsing the status and control messages by userspace application
 * supporting assignment of physical controls to MIDI ports by userspace
   application
 * turn on/off LEDs on device by userspace application

These work are apparently beyond my current effort.

Takashi Sakamoto (5):
  ALSA: firewire-tascam: add support for incoming MIDI messages by
    asynchronous transaction
  ALSA: firewire-tascam: add support for outgoing MIDI messages by
    asynchronous transaction
  ALSA: firewire-tascam: add support for MIDI functionality
  ALSA: firewire-tascam: Turn on/off FireWire LED
  ALSA: firewire-tascam: change device probing processing

 sound/firewire/tascam/Makefile             |   3 +-
 sound/firewire/tascam/tascam-midi.c        | 135 +++++++++++++
 sound/firewire/tascam/tascam-transaction.c | 293 +++++++++++++++++++++++++++++
 sound/firewire/tascam/tascam.c             |  89 +++++----
 sound/firewire/tascam/tascam.h             |  30 +++
 5 files changed, 503 insertions(+), 47 deletions(-)
 create mode 100644 sound/firewire/tascam/tascam-midi.c
 create mode 100644 sound/firewire/tascam/tascam-transaction.c

-- 
2.1.4

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

* [PATCH 1/5] ALSA: firewire-tascam: add support for incoming MIDI messages by asynchronous transaction
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
@ 2015-10-12 10:10 ` Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 2/5] ALSA: firewire-tascam: add support for outgoing " Takashi Sakamoto
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

TASCAM FireWire series use asynchronous transaction to transfer MIDI
messages. The transaction is sent to a registered address.

This commit supports the incoming MIDI messages. The messages in the
transaction include some quirks:
 * Two quadlets are used for one MIDI message and one timestamp.
 * Usually, the first byte of the first quadlet includes MIDI port and MSB
   4 bit of MIDI status. For system exclusive message, the first byte
   includes MIDI port and 0x04, or 0x07 in the end of the message.
 * The rest of the first quadlet includes MIDI bytes up to 3.
 * Several set of MIDI messages and timestamp can be transferred in one
   block transaction, up to 8 sets.

I note that TASCAM FireWire series ignores ID bytes of system exclusive
message. When receiving system exclusive messages with ID bytes on physical
MIDI bus, the series transfers the messages without ID bytes on IEEE 1394
bus, and vice versa.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/Makefile             |   3 +-
 sound/firewire/tascam/tascam-transaction.c | 186 +++++++++++++++++++++++++++++
 sound/firewire/tascam/tascam.c             |   7 ++
 sound/firewire/tascam/tascam.h             |  18 +++
 4 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/tascam/tascam-transaction.c

diff --git a/sound/firewire/tascam/Makefile b/sound/firewire/tascam/Makefile
index 6beefc2..a1f9fe7 100644
--- a/sound/firewire/tascam/Makefile
+++ b/sound/firewire/tascam/Makefile
@@ -1,3 +1,4 @@
 snd-firewire-tascam-objs := tascam-proc.o amdtp-tascam.o tascam-stream.o \
-			    tascam-pcm.o tascam-hwdep.o tascam.o
+			    tascam-pcm.o tascam-hwdep.o tascam-transaction.o \
+			    tascam.o
 obj-$(CONFIG_SND_FIREWIRE_TASCAM) += snd-firewire-tascam.o
diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
new file mode 100644
index 0000000..853438d
--- /dev/null
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -0,0 +1,186 @@
+/*
+ * tascam-transaction.c - a part of driver for TASCAM FireWire series
+ *
+ * Copyright (c) 2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "tascam.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 handle_midi_tx(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_tscm *tscm = callback_data;
+	u32 *buf = (u32 *)data;
+	unsigned int messages;
+	unsigned int i;
+	unsigned int port;
+	struct snd_rawmidi_substream *substream;
+	u8 *b;
+	int bytes;
+
+	if (offset != tscm->async_handler.offset)
+		goto end;
+
+	messages = length / 8;
+	for (i = 0; i < messages; i++) {
+		b = (u8 *)(buf + i * 2);
+
+		port = b[0] >> 4;
+		/* TODO: support virtual MIDI ports. */
+		if (port > tscm->spec->midi_capture_ports)
+			goto end;
+
+		/* Assume the message length. */
+		bytes = calculate_message_bytes(b[1]);
+		/* On MIDI data or exclusives. */
+		if (bytes <= 0) {
+			/* Seek the end of exclusives. */
+			for (bytes = 1; bytes < 4; bytes++) {
+				if (b[bytes] == 0xf7)
+					break;
+			}
+			if (bytes == 4)
+				bytes = 3;
+		}
+
+		substream = ACCESS_ONCE(tscm->tx_midi_substreams[port]);
+		if (substream != NULL)
+			snd_rawmidi_receive(substream, b + 1, bytes);
+	}
+end:
+	fw_send_response(card, request, RCODE_COMPLETE);
+}
+
+int snd_tscm_transaction_register(struct snd_tscm *tscm)
+{
+	static const struct fw_address_region resp_register_region = {
+		.start	= 0xffffe0000000ull,
+		.end	= 0xffffe000ffffull,
+	};
+	int err;
+
+	/*
+	 * Usually, two quadlets are transferred by one transaction. The first
+	 * quadlet has MIDI messages, the rest includes timestamp.
+	 * Sometimes, 8 set of the data is transferred by a block transaction.
+	 */
+	tscm->async_handler.length = 8 * 8;
+	tscm->async_handler.address_callback = handle_midi_tx;
+	tscm->async_handler.callback_data = tscm;
+
+	err = fw_core_add_address_handler(&tscm->async_handler,
+					  &resp_register_region);
+	if (err < 0)
+		return err;
+
+	err = snd_tscm_transaction_reregister(tscm);
+	if (err < 0)
+		fw_core_remove_address_handler(&tscm->async_handler);
+
+	return err;
+}
+
+/* At bus reset, these registers are cleared. */
+int snd_tscm_transaction_reregister(struct snd_tscm *tscm)
+{
+	struct fw_device *device = fw_parent_device(tscm->unit);
+	__be32 reg;
+	int err;
+
+	/* Register messaging address. Block transaction is not allowed. */
+	reg = cpu_to_be32((device->card->node_id << 16) |
+			  (tscm->async_handler.offset >> 32));
+	err = snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ADDR_HI,
+				 &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	reg = cpu_to_be32(tscm->async_handler.offset);
+	err = snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ADDR_LO,
+				 &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	/* Turn on messaging. */
+	reg = cpu_to_be32(0x00000001);
+	return snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+				  TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ON,
+				  &reg, sizeof(reg), 0);
+}
+
+void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
+{
+	__be32 reg;
+
+	/* Turn off messaging. */
+	reg = cpu_to_be32(0x00000000);
+	snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ON,
+			   &reg, sizeof(reg), 0);
+
+	/* Unregister the address. */
+	snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ADDR_HI,
+			   &reg, sizeof(reg), 0);
+	snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ADDR_LO,
+			   &reg, sizeof(reg), 0);
+
+	fw_core_remove_address_handler(&tscm->async_handler);
+}
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index ee2f498..de9e8df 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -82,6 +82,7 @@ static void tscm_card_free(struct snd_card *card)
 {
 	struct snd_tscm *tscm = card->private_data;
 
+	snd_tscm_transaction_unregister(tscm);
 	snd_tscm_stream_destroy_duplex(tscm);
 
 	fw_unit_put(tscm->unit);
@@ -127,6 +128,10 @@ static int snd_tscm_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_tscm_transaction_register(tscm);
+	if (err < 0)
+		goto error;
+
 	err = snd_tscm_create_hwdep_device(tscm);
 	if (err < 0)
 		goto error;
@@ -147,6 +152,8 @@ static void snd_tscm_update(struct fw_unit *unit)
 {
 	struct snd_tscm *tscm = dev_get_drvdata(&unit->device);
 
+	snd_tscm_transaction_reregister(tscm);
+
 	mutex_lock(&tscm->mutex);
 	snd_tscm_stream_update_duplex(tscm);
 	mutex_unlock(&tscm->mutex);
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index 75a3b9a..b0e602b 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -25,6 +25,7 @@
 #include <sound/pcm_params.h>
 #include <sound/firewire.h>
 #include <sound/hwdep.h>
+#include <sound/rawmidi.h>
 
 #include "../lib.h"
 #include "../amdtp-stream.h"
@@ -41,6 +42,9 @@ struct snd_tscm_spec {
 	bool is_controller;
 };
 
+#define TSCM_MIDI_IN_PORT_MAX	4
+#define TSCM_MIDI_OUT_PORT_MAX	4
+
 struct snd_tscm {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -59,6 +63,10 @@ struct snd_tscm {
 	int dev_lock_count;
 	bool dev_lock_changed;
 	wait_queue_head_t hwdep_wait;
+
+	/* For MIDI message incoming transactions. */
+	struct fw_address_handler async_handler;
+	struct snd_rawmidi_substream *tx_midi_substreams[TSCM_MIDI_IN_PORT_MAX];
 };
 
 #define TSCM_ADDR_BASE			0xffff00000000ull
@@ -81,6 +89,12 @@ struct snd_tscm {
 #define TSCM_OFFSET_CLOCK_STATUS	0x0228
 #define TSCM_OFFSET_SET_OPTION		0x022c
 
+#define TSCM_OFFSET_MIDI_TX_ON		0x0300
+#define TSCM_OFFSET_MIDI_TX_ADDR_HI	0x0304
+#define TSCM_OFFSET_MIDI_TX_ADDR_LO	0x0308
+
+#define TSCM_OFFSET_MIDI_RX_QUAD	0x4000
+
 enum snd_tscm_clock {
 	SND_TSCM_CLOCK_INTERNAL = 0,
 	SND_TSCM_CLOCK_WORD	= 1,
@@ -108,6 +122,10 @@ void snd_tscm_stream_lock_changed(struct snd_tscm *tscm);
 int snd_tscm_stream_lock_try(struct snd_tscm *tscm);
 void snd_tscm_stream_lock_release(struct snd_tscm *tscm);
 
+int snd_tscm_transaction_register(struct snd_tscm *tscm);
+int snd_tscm_transaction_reregister(struct snd_tscm *tscm);
+void snd_tscm_transaction_unregister(struct snd_tscm *tscm);
+
 void snd_tscm_proc_init(struct snd_tscm *tscm);
 
 int snd_tscm_create_pcm_devices(struct snd_tscm *tscm);
-- 
2.1.4

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

* [PATCH 2/5] ALSA: firewire-tascam: add support for outgoing MIDI messages by asynchronous transaction
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 1/5] ALSA: firewire-tascam: add support for incoming MIDI messages by asynchronous transaction Takashi Sakamoto
@ 2015-10-12 10:10 ` Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 3/5] ALSA: firewire-tascam: add support for MIDI functionality Takashi Sakamoto
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

TASCAM FireWire series use asynchronous transaction to receive MIDI
messages. The transaction should be sent to a certain address.

This commit supports the outgoing MIDI messages. The messages in the
transaction includes some quirks:
 * One MIDI message is transferred in one quadlet transaction, except for
   system exclusives.
 * MIDI running status is not allowed, thus transactions always include
   status byte.
 * The basic data format is the same as transferring MIDI messages
   supported in previous commit.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-transaction.c | 95 +++++++++++++++++++++++++++++-
 sound/firewire/tascam/tascam.h             |  8 +++
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 853438d..6b74fb5 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -58,6 +58,83 @@ static inline int calculate_message_bytes(u8 status)
 	return -EINVAL;
 }
 
+static int fill_message(struct snd_rawmidi_substream *substream, u8 *buf)
+{
+	struct snd_tscm *tscm = substream->rmidi->private_data;
+	unsigned int port = substream->number;
+	unsigned int len;
+	unsigned int i;
+	u8 status;
+	int consume;
+
+	buf[0] = buf[1] = buf[2] = buf[3] = 0x00;
+
+	len = snd_rawmidi_transmit_peek(substream, buf + 1, 3);
+	if (len == 0)
+		return 0;
+
+	/* On exclusive message. */
+	if (tscm->on_sysex[port]) {
+		/* Seek the end of exclusives. */
+		for (i = 1; i < 4 || i < len; ++i) {
+			if (buf[i] == 0xf7) {
+				tscm->on_sysex[port] = false;
+				break;
+			}
+		}
+
+		/* At the end of exclusive message, use label 0x07. */
+		if (!tscm->on_sysex[port]) {
+			len = i;
+			buf[0] = (port << 4) | 0x07;
+		/* During exclusive message, use label 0x04. */
+		} else if (len == 3) {
+			buf[0] = (port << 4) | 0x04;
+		/* We need to fill whole 3 bytes. Go to next change. */
+		} else {
+			len = 0;
+		}
+	} else {
+		/* The beginning of exclusives. */
+		if (buf[1] == 0xf0) {
+			/* Transfer it in next chance in another condition. */
+			tscm->on_sysex[port] = true;
+			return 0;
+		} else {
+			/* On running-status. */
+			if ((buf[1] & 0x80) != 0x80)
+				status = tscm->running_status[port];
+			else
+				status = buf[1];
+
+			/* Calculate consume bytes. */
+			consume = calculate_message_bytes(status);
+			if (consume <= 0)
+				return 0;
+
+			/* On running-status. */
+			if ((buf[1] & 0x80) != 0x80) {
+				buf[3] = buf[2];
+				buf[2] = buf[1];
+				buf[1] = tscm->running_status[port];
+				consume--;
+			} else {
+				tscm->running_status[port] = buf[1];
+			}
+
+			/* Confirm length. */
+			if (len < consume)
+				return 0;
+			if (len > consume)
+				len = consume;
+		}
+
+		buf[0] = (port << 4) | (buf[1] >> 4);
+	}
+
+	return len;
+}
+
 static void handle_midi_tx(struct fw_card *card, struct fw_request *request,
 			   int tcode, int destination, int source,
 			   int generation, unsigned long long offset,
@@ -111,6 +188,7 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 		.start	= 0xffffe0000000ull,
 		.end	= 0xffffe000ffffull,
 	};
+	unsigned int i;
 	int err;
 
 	/*
@@ -129,9 +207,21 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 
 	err = snd_tscm_transaction_reregister(tscm);
 	if (err < 0)
-		fw_core_remove_address_handler(&tscm->async_handler);
+		goto error;
+
+	for (i = 0; i < TSCM_MIDI_OUT_PORT_MAX; i++) {
+		err = snd_fw_async_midi_port_init(
+				&tscm->out_ports[i], tscm->unit,
+				TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_RX_QUAD,
+				4, fill_message);
+		if (err < 0)
+			goto error;
+	}
 
 	return err;
+error:
+	fw_core_remove_address_handler(&tscm->async_handler);
+	return err;
 }
 
 /* At bus reset, these registers are cleared. */
@@ -167,6 +257,7 @@ int snd_tscm_transaction_reregister(struct snd_tscm *tscm)
 void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
 {
 	__be32 reg;
+	unsigned int i;
 
 	/* Turn off messaging. */
 	reg = cpu_to_be32(0x00000000);
@@ -183,4 +274,6 @@ void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
 			   &reg, sizeof(reg), 0);
 
 	fw_core_remove_address_handler(&tscm->async_handler);
+	for (i = 0; i < TSCM_MIDI_OUT_PORT_MAX; i++)
+		snd_fw_async_midi_port_destroy(&tscm->out_ports[i]);
 }
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index b0e602b..c2f0c74 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -67,6 +67,14 @@ struct snd_tscm {
 	/* For MIDI message incoming transactions. */
 	struct fw_address_handler async_handler;
 	struct snd_rawmidi_substream *tx_midi_substreams[TSCM_MIDI_IN_PORT_MAX];
+
+	/* For MIDI message outgoing transactions. */
+	struct snd_fw_async_midi_port out_ports[TSCM_MIDI_OUT_PORT_MAX];
+	u8 running_status[TSCM_MIDI_OUT_PORT_MAX];
+	bool on_sysex[TSCM_MIDI_OUT_PORT_MAX];
+
+	/* For control messages. */
+	struct snd_firewire_tascam_status *status;
 };
 
 #define TSCM_ADDR_BASE			0xffff00000000ull
-- 
2.1.4

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

* [PATCH 3/5] ALSA: firewire-tascam: add support for MIDI functionality
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 1/5] ALSA: firewire-tascam: add support for incoming MIDI messages by asynchronous transaction Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 2/5] ALSA: firewire-tascam: add support for outgoing " Takashi Sakamoto
@ 2015-10-12 10:10 ` Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 4/5] ALSA: firewire-tascam: Turn on/off FireWire LED Takashi Sakamoto
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

In former commits, this driver got functionalities to transfer/receive
MIDI messages to/from TASCAM FireWire series.

This commit adds some ALSA MIDI ports to enable userspace applications
to use the functionalities.

I note that this commit doesn't support virtual MIDI ports which console
models support. A physical controls can be assigned to a certain MIDI
ports including physical and virtual. But the way is not clear.

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

diff --git a/sound/firewire/tascam/Makefile b/sound/firewire/tascam/Makefile
index a1f9fe7..0fc955d 100644
--- a/sound/firewire/tascam/Makefile
+++ b/sound/firewire/tascam/Makefile
@@ -1,4 +1,4 @@
 snd-firewire-tascam-objs := tascam-proc.o amdtp-tascam.o tascam-stream.o \
 			    tascam-pcm.o tascam-hwdep.o tascam-transaction.o \
-			    tascam.o
+			    tascam-midi.o tascam.o
 obj-$(CONFIG_SND_FIREWIRE_TASCAM) += snd-firewire-tascam.o
diff --git a/sound/firewire/tascam/tascam-midi.c b/sound/firewire/tascam/tascam-midi.c
new file mode 100644
index 0000000..41f8420
--- /dev/null
+++ b/sound/firewire/tascam/tascam-midi.c
@@ -0,0 +1,135 @@
+/*
+ * tascam-midi.c - a part of driver for TASCAM FireWire series
+ *
+ * Copyright (c) 2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "tascam.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_tscm *tscm = substream->rmidi->private_data;
+
+	/* Initialize internal status. */
+	tscm->running_status[substream->number] = 0;
+	tscm->on_sysex[substream->number] = 0;
+	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_tscm *tscm = substream->rmidi->private_data;
+
+	snd_fw_async_midi_port_finish(&tscm->out_ports[substream->number]);
+
+	return 0;
+}
+
+static void midi_capture_trigger(struct snd_rawmidi_substream *substrm, int up)
+{
+	struct snd_tscm *tscm = substrm->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tscm->lock, flags);
+
+	if (up)
+		tscm->tx_midi_substreams[substrm->number] = substrm;
+	else
+		tscm->tx_midi_substreams[substrm->number] = NULL;
+
+	spin_unlock_irqrestore(&tscm->lock, flags);
+}
+
+static void midi_playback_trigger(struct snd_rawmidi_substream *substrm, int up)
+{
+	struct snd_tscm *tscm = substrm->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tscm->lock, flags);
+
+	if (up)
+		snd_fw_async_midi_port_run(&tscm->out_ports[substrm->number],
+					   substrm);
+
+	spin_unlock_irqrestore(&tscm->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,
+};
+
+int snd_tscm_create_midi_devices(struct snd_tscm *tscm)
+{
+	struct snd_rawmidi *rmidi;
+	struct snd_rawmidi_str *stream;
+	struct snd_rawmidi_substream *subs;
+	int err;
+
+	err = snd_rawmidi_new(tscm->card, tscm->card->driver, 0,
+			      tscm->spec->midi_playback_ports,
+			      tscm->spec->midi_capture_ports,
+			      &rmidi);
+	if (err < 0)
+		return err;
+
+	snprintf(rmidi->name, sizeof(rmidi->name),
+		 "%s MIDI", tscm->card->shortname);
+	rmidi->private_data = tscm;
+
+	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 port names for MIDI input. */
+	list_for_each_entry(subs, &stream->substreams, list) {
+		/* TODO: support virtual MIDI ports. */
+		if (subs->number < tscm->spec->midi_capture_ports) {
+			/* Hardware MIDI ports. */
+			snprintf(subs->name, sizeof(subs->name),
+				 "%s MIDI %d",
+				 tscm->card->shortname, subs->number + 1);
+		}
+	}
+
+	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 port names for MIDI ourput. */
+	list_for_each_entry(subs, &stream->substreams, list) {
+		if (subs->number < tscm->spec->midi_playback_ports) {
+			/* Hardware MIDI ports only. */
+			snprintf(subs->name, sizeof(subs->name),
+				 "%s MIDI %d",
+				 tscm->card->shortname, subs->number + 1);
+		}
+	}
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_DUPLEX;
+
+	return 0;
+}
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index de9e8df..dc07e3e 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -132,6 +132,10 @@ static int snd_tscm_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_tscm_create_midi_devices(tscm);
+	if (err < 0)
+		goto error;
+
 	err = snd_tscm_create_hwdep_device(tscm);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index c2f0c74..b86bb7f 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -138,6 +138,8 @@ void snd_tscm_proc_init(struct snd_tscm *tscm);
 
 int snd_tscm_create_pcm_devices(struct snd_tscm *tscm);
 
+int snd_tscm_create_midi_devices(struct snd_tscm *tscm);
+
 int snd_tscm_create_hwdep_device(struct snd_tscm *tscm);
 
 #endif
-- 
2.1.4

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

* [PATCH 4/5] ALSA: firewire-tascam: Turn on/off FireWire LED
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2015-10-12 10:10 ` [PATCH 3/5] ALSA: firewire-tascam: add support for MIDI functionality Takashi Sakamoto
@ 2015-10-12 10:10 ` Takashi Sakamoto
  2015-10-12 10:10 ` [PATCH 5/5] ALSA: firewire-tascam: change device probing processing Takashi Sakamoto
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

TASCAM FireWire series has some LEDs on its surface. These LEDs can be
turned on/off by receiving asynchronous transactions to a certain
address. One of the LEDs is labels as 'FireWire'. It's better to light it
up when this driver starts to work. Besides, the LED for 'FireWire' is
turned off at bus reset.

This commit implements this idea.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-transaction.c | 14 ++++++++++++++
 sound/firewire/tascam/tascam.h             |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 6b74fb5..1c9a88b 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -252,6 +252,14 @@ int snd_tscm_transaction_reregister(struct snd_tscm *tscm)
 	return snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
 				  TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_TX_ON,
 				  &reg, sizeof(reg), 0);
+	if (err < 0)
+		return err;
+
+	/* Turn on FireWire LED. */
+	reg = cpu_to_be32(0x0001008e);
+	return snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+				  TSCM_ADDR_BASE + TSCM_OFFSET_LED_POWER,
+				  &reg, sizeof(reg), 0);
 }
 
 void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
@@ -259,6 +267,12 @@ void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
 	__be32 reg;
 	unsigned int i;
 
+	/* Turn off FireWire LED. */
+	reg = cpu_to_be32(0x0000008e);
+	snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   TSCM_ADDR_BASE + TSCM_OFFSET_LED_POWER,
+			   &reg, sizeof(reg), 0);
+
 	/* Turn off messaging. */
 	reg = cpu_to_be32(0x00000000);
 	snd_fw_transaction(tscm->unit, TCODE_WRITE_QUADLET_REQUEST,
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index b86bb7f..2d028d2 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -101,6 +101,8 @@ struct snd_tscm {
 #define TSCM_OFFSET_MIDI_TX_ADDR_HI	0x0304
 #define TSCM_OFFSET_MIDI_TX_ADDR_LO	0x0308
 
+#define TSCM_OFFSET_LED_POWER		0x0404
+
 #define TSCM_OFFSET_MIDI_RX_QUAD	0x4000
 
 enum snd_tscm_clock {
-- 
2.1.4

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

* [PATCH 5/5] ALSA: firewire-tascam: change device probing processing
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2015-10-12 10:10 ` [PATCH 4/5] ALSA: firewire-tascam: Turn on/off FireWire LED Takashi Sakamoto
@ 2015-10-12 10:10 ` Takashi Sakamoto
  2015-10-12 12:42   ` Stefan Richter
  2015-10-12 12:21 ` [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Iwai
  2015-10-12 12:48 ` Stefan Richter
  6 siblings, 1 reply; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-12 10:10 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, Stefan Richter, ffado-devel

Currently, this driver picks up model name with be32_to_cpu() macro
to align characters. This is wrong operation because the result is
different depending on CPU endiannness.

Additionally, vendor released several versions of firmware for this
series. It's not better to assign model-dependent information to
device entry according to the version field.

This commit fixes these bugs. The name of model is picked up correctly
and used to identify model-dependent information.

Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Fixes: c0949b278515('ALSA: firewire-tascam: add skeleton for TASCAM FireWire series')
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam.c | 78 +++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 46 deletions(-)

diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index dc07e3e..3baf1b2 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -24,16 +24,6 @@ static struct snd_tscm_spec model_specs[] = {
 		.is_controller = true,
 	},
 	{
-		.name = "FW-1804",
-		.has_adat = true,
-		.has_spdif = true,
-		.pcm_capture_analog_channels = 8,
-		.pcm_playback_analog_channels = 2,
-		.midi_capture_ports = 2,
-		.midi_playback_ports = 4,
-		.is_controller = false,
-	},
-	{
 		.name = "FW-1082",
 		.has_adat = false,
 		.has_spdif = true,
@@ -43,34 +33,46 @@ static struct snd_tscm_spec model_specs[] = {
 		.midi_playback_ports = 2,
 		.is_controller = true,
 	},
+	/* FW-1804 mey be supported. */
 };
 
-static int check_name(struct snd_tscm *tscm)
+static int identify_model(struct snd_tscm *tscm)
 {
 	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
-	char vendor[8];
+	const u32 *config_rom = fw_dev->config_rom;
 	char model[8];
-	__u32 data;
-
-	/* Retrieve model name. */
-	data = be32_to_cpu(fw_dev->config_rom[28]);
-	memcpy(model, &data, 4);
-	data = be32_to_cpu(fw_dev->config_rom[29]);
-	memcpy(model + 4, &data, 4);
-	model[7] = '\0';
-
-	/* Retrieve vendor name. */
-	data = be32_to_cpu(fw_dev->config_rom[23]);
-	memcpy(vendor, &data, 4);
-	data = be32_to_cpu(fw_dev->config_rom[24]);
-	memcpy(vendor + 4, &data, 4);
-	vendor[7] = '\0';
+	unsigned int i;
+	u8 c;
+
+	if (fw_dev->config_rom_length < 30) {
+		dev_err(&tscm->unit->device,
+			"Configuration ROM is too short.\n");
+		return -ENODEV;
+	}
+
+	/* Pick up model name from certain addresses. */
+	for (i = 0; i < 8; i++) {
+		c = config_rom[28 + i / 4] >> (24 - 8 * (i % 4));
+		if (c == '\0')
+			break;
+		model[i] = c;
+	}
+	model[i] = '\0';
+
+	for (i = 0; i < ARRAY_SIZE(model_specs); i++) {
+		if (strcmp(model, model_specs[i].name) == 0) {
+			tscm->spec = &model_specs[i];
+			break;
+		}
+	}
+	if (tscm->spec == NULL)
+		return -ENODEV;
 
 	strcpy(tscm->card->driver, "FW-TASCAM");
 	strcpy(tscm->card->shortname, model);
 	strcpy(tscm->card->mixername, model);
 	snprintf(tscm->card->longname, sizeof(tscm->card->longname),
-		 "%s %s, GUID %08x%08x at %s, S%d", vendor, model,
+		 "TASCAM %s, GUID %08x%08x at %s, S%d", model,
 		 cpu_to_be32(fw_dev->config_rom[3]),
 		 cpu_to_be32(fw_dev->config_rom[4]),
 		 dev_name(&tscm->unit->device), 100 << fw_dev->max_speed);
@@ -108,13 +110,12 @@ static int snd_tscm_probe(struct fw_unit *unit,
 	tscm = card->private_data;
 	tscm->card = card;
 	tscm->unit = fw_unit_get(unit);
-	tscm->spec = (const struct snd_tscm_spec *)entry->driver_data;
 
 	mutex_init(&tscm->mutex);
 	spin_lock_init(&tscm->lock);
 	init_waitqueue_head(&tscm->hwdep_wait);
 
-	err = check_name(tscm);
+	err = identify_model(tscm);
 	if (err < 0)
 		goto error;
 
@@ -172,27 +173,12 @@ static void snd_tscm_remove(struct fw_unit *unit)
 }
 
 static const struct ieee1394_device_id snd_tscm_id_table[] = {
-	/* FW-1082 */
-	{
-		.match_flags = IEEE1394_MATCH_VENDOR_ID |
-			       IEEE1394_MATCH_SPECIFIER_ID |
-			       IEEE1394_MATCH_VERSION,
-		.vendor_id = 0x00022e,
-		.specifier_id = 0x00022e,
-		.version = 0x800003,
-		.driver_data = (kernel_ulong_t)&model_specs[2],
-	},
-	/* FW-1884 */
 	{
 		.match_flags = IEEE1394_MATCH_VENDOR_ID |
-			       IEEE1394_MATCH_SPECIFIER_ID |
-			       IEEE1394_MATCH_VERSION,
+			       IEEE1394_MATCH_SPECIFIER_ID,
 		.vendor_id = 0x00022e,
 		.specifier_id = 0x00022e,
-		.version = 0x800000,
-		.driver_data = (kernel_ulong_t)&model_specs[0],
 	},
-	/* FW-1804 mey be supported if IDs are clear. */
 	/* FE-08 requires reverse-engineering because it just has faders. */
 	{}
 };
-- 
2.1.4

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2015-10-12 10:10 ` [PATCH 5/5] ALSA: firewire-tascam: change device probing processing Takashi Sakamoto
@ 2015-10-12 12:21 ` Takashi Iwai
  2015-10-12 12:48 ` Stefan Richter
  6 siblings, 0 replies; 26+ messages in thread
From: Takashi Iwai @ 2015-10-12 12:21 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens, ffado-devel

On Mon, 12 Oct 2015 12:10:20 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> TASCAM FireWire series driver may be newly available in Linux 4.4. For
> the driver, this patchset adds ALSA MIDI ports to communicate to physical
> MIDI ports.
> 
> This patchset includes some fixes, and updates my former post:
> 
> [alsa-devel] [PATCH 00/25 v2] ALSA: support AMDTP variants
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-August/096739.html
> 
> Changes:
>  * support system exclusive messages
>  * drop virtual MIDI ports
>  * drop MMAP support via hwdep interface for status and control message
> 
> Although TASCAM FireWire series has physical controls, this patchset
> doesn't support them. I think some facilities are required to enable this
> functionality:
>  * handling MIDI messages on virtual MIDI ports by this driver
>  * handling status and control messages in isochronous packet by this
>    driver
>  * parsing the status and control messages by userspace application
>  * supporting assignment of physical controls to MIDI ports by userspace
>    application
>  * turn on/off LEDs on device by userspace application
> 
> These work are apparently beyond my current effort.
> 
> Takashi Sakamoto (5):
>   ALSA: firewire-tascam: add support for incoming MIDI messages by
>     asynchronous transaction
>   ALSA: firewire-tascam: add support for outgoing MIDI messages by
>     asynchronous transaction
>   ALSA: firewire-tascam: add support for MIDI functionality
>   ALSA: firewire-tascam: Turn on/off FireWire LED
>   ALSA: firewire-tascam: change device probing processing

Applied now.  Thanks.


Takashi


>  sound/firewire/tascam/Makefile             |   3 +-
>  sound/firewire/tascam/tascam-midi.c        | 135 +++++++++++++
>  sound/firewire/tascam/tascam-transaction.c | 293 +++++++++++++++++++++++++++++
>  sound/firewire/tascam/tascam.c             |  89 +++++----
>  sound/firewire/tascam/tascam.h             |  30 +++
>  5 files changed, 503 insertions(+), 47 deletions(-)
>  create mode 100644 sound/firewire/tascam/tascam-midi.c
>  create mode 100644 sound/firewire/tascam/tascam-transaction.c
> 
> -- 
> 2.1.4
> 

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

* Re: [PATCH 5/5] ALSA: firewire-tascam: change device probing processing
  2015-10-12 10:10 ` [PATCH 5/5] ALSA: firewire-tascam: change device probing processing Takashi Sakamoto
@ 2015-10-12 12:42   ` Stefan Richter
  2015-10-13 14:12     ` Takashi Sakamoto
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2015-10-12 12:42 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, clemens, ffado-devel

On Oct 12 Takashi Sakamoto wrote:
[...]
> Fixes: c0949b278515('ALSA: firewire-tascam: add skeleton for TASCAM FireWire series')

I have some trivial comments, and one off-by-one index error in case
of unexpected Config ROM contents.

[...]
> --- a/sound/firewire/tascam/tascam.c
> +++ b/sound/firewire/tascam/tascam.c
> @@ -24,16 +24,6 @@ static struct snd_tscm_spec model_specs[] = {
>  		.is_controller = true,
>  	},
>  	{
> -		.name = "FW-1804",
> -		.has_adat = true,
> -		.has_spdif = true,
> -		.pcm_capture_analog_channels = 8,
> -		.pcm_playback_analog_channels = 2,
> -		.midi_capture_ports = 2,
> -		.midi_playback_ports = 4,
> -		.is_controller = false,
> -	},
> -	{
>  		.name = "FW-1082",
>  		.has_adat = false,
>  		.has_spdif = true,
> @@ -43,34 +33,46 @@ static struct snd_tscm_spec model_specs[] = {
>  		.midi_playback_ports = 2,
>  		.is_controller = true,
>  	},
> +	/* FW-1804 mey be supported. */

mey -> may  (already fixed in tiwai/sound.git)

>  };
>  
> -static int check_name(struct snd_tscm *tscm)
> +static int identify_model(struct snd_tscm *tscm)
>  {
>  	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
> -	char vendor[8];
> +	const u32 *config_rom = fw_dev->config_rom;
>  	char model[8];
> -	__u32 data;
> -
> -	/* Retrieve model name. */
> -	data = be32_to_cpu(fw_dev->config_rom[28]);
> -	memcpy(model, &data, 4);
> -	data = be32_to_cpu(fw_dev->config_rom[29]);
> -	memcpy(model + 4, &data, 4);
> -	model[7] = '\0';
> -
> -	/* Retrieve vendor name. */
> -	data = be32_to_cpu(fw_dev->config_rom[23]);
> -	memcpy(vendor, &data, 4);
> -	data = be32_to_cpu(fw_dev->config_rom[24]);
> -	memcpy(vendor + 4, &data, 4);
> -	vendor[7] = '\0';
> +	unsigned int i;
> +	u8 c;
> +
> +	if (fw_dev->config_rom_length < 30) {
> +		dev_err(&tscm->unit->device,
> +			"Configuration ROM is too short.\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Pick up model name from certain addresses. */
> +	for (i = 0; i < 8; i++) {
> +		c = config_rom[28 + i / 4] >> (24 - 8 * (i % 4));
> +		if (c == '\0')
> +			break;
> +		model[i] = c;
> +	}
> +	model[i] = '\0';

You could get a buffer overrun here.  Perhaps only go to i < 7:

	for (i = 0; i < 7; i++) {
		[...]
	}
	model[i] = '\0';

> +	for (i = 0; i < ARRAY_SIZE(model_specs); i++) {
> +		if (strcmp(model, model_specs[i].name) == 0) {
> +			tscm->spec = &model_specs[i];
> +			break;
> +		}
> +	}
> +	if (tscm->spec == NULL)
> +		return -ENODEV;
>  
>  	strcpy(tscm->card->driver, "FW-TASCAM");
>  	strcpy(tscm->card->shortname, model);
>  	strcpy(tscm->card->mixername, model);
>  	snprintf(tscm->card->longname, sizeof(tscm->card->longname),
> -		 "%s %s, GUID %08x%08x at %s, S%d", vendor, model,
> +		 "TASCAM %s, GUID %08x%08x at %s, S%d", model,
>  		 cpu_to_be32(fw_dev->config_rom[3]),
>  		 cpu_to_be32(fw_dev->config_rom[4]),
>  		 dev_name(&tscm->unit->device), 100 << fw_dev->max_speed);

Should be
		fw_dev->config_rom[3],
		fw_dev->config_rom[4],

since snprintf wants CPU-endian values.

> @@ -108,13 +110,12 @@ static int snd_tscm_probe(struct fw_unit *unit,
>  	tscm = card->private_data;
>  	tscm->card = card;
>  	tscm->unit = fw_unit_get(unit);
> -	tscm->spec = (const struct snd_tscm_spec *)entry->driver_data;
>  
>  	mutex_init(&tscm->mutex);
>  	spin_lock_init(&tscm->lock);
>  	init_waitqueue_head(&tscm->hwdep_wait);
>  
> -	err = check_name(tscm);
> +	err = identify_model(tscm);
>  	if (err < 0)
>  		goto error;
>  
> @@ -172,27 +173,12 @@ static void snd_tscm_remove(struct fw_unit *unit)
>  }
>  
>  static const struct ieee1394_device_id snd_tscm_id_table[] = {
> -	/* FW-1082 */
> -	{
> -		.match_flags = IEEE1394_MATCH_VENDOR_ID |
> -			       IEEE1394_MATCH_SPECIFIER_ID |
> -			       IEEE1394_MATCH_VERSION,
> -		.vendor_id = 0x00022e,
> -		.specifier_id = 0x00022e,
> -		.version = 0x800003,
> -		.driver_data = (kernel_ulong_t)&model_specs[2],
> -	},
> -	/* FW-1884 */
>  	{
>  		.match_flags = IEEE1394_MATCH_VENDOR_ID |
> -			       IEEE1394_MATCH_SPECIFIER_ID |
> -			       IEEE1394_MATCH_VERSION,
> +			       IEEE1394_MATCH_SPECIFIER_ID,
>  		.vendor_id = 0x00022e,
>  		.specifier_id = 0x00022e,
> -		.version = 0x800000,
> -		.driver_data = (kernel_ulong_t)&model_specs[0],
>  	},
> -	/* FW-1804 mey be supported if IDs are clear. */
>  	/* FE-08 requires reverse-engineering because it just has faders. */
>  	{}
>  };

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

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2015-10-12 12:21 ` [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Iwai
@ 2015-10-12 12:48 ` Stefan Richter
  2015-10-12 22:20   ` Jonathan Woithe
  6 siblings, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2015-10-12 12:48 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

On Oct 12 Takashi Sakamoto wrote:
> Although TASCAM FireWire series has physical controls, this patchset
> doesn't support them. I think some facilities are required to enable this
> functionality:
>  * handling MIDI messages on virtual MIDI ports by this driver
>  * handling status and control messages in isochronous packet by this
>    driver

As a side note, AFAIR, RME devices encapsulate status in their isochronous
stream too... or was it MOTU?

>  * parsing the status and control messages by userspace application
>  * supporting assignment of physical controls to MIDI ports by userspace
>    application
>  * turn on/off LEDs on device by userspace application
> 
> These work are apparently beyond my current effort.
-- 
Stefan Richter
-=====-===== =-=- -==--
http://arcgraph.de/sr/

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-12 12:48 ` Stefan Richter
@ 2015-10-12 22:20   ` Jonathan Woithe
  2015-10-13  9:36     ` Takashi Sakamoto
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-12 22:20 UTC (permalink / raw)
  To: Stefan Richter; +Cc: tiwai, alsa-devel, clemens, ffado-devel, Takashi Sakamoto

On Mon, Oct 12, 2015 at 02:48:16PM +0200, Stefan Richter wrote:
> On Oct 12 Takashi Sakamoto wrote:
> > Although TASCAM FireWire series has physical controls, this patchset
> > doesn't support them. I think some facilities are required to enable this
> > functionality:
> >  * handling MIDI messages on virtual MIDI ports by this driver
> >  * handling status and control messages in isochronous packet by this
> >    driver
> 
> As a side note, AFAIR, RME devices encapsulate status in their isochronous
> stream too... or was it MOTU?

Both do, but in completely different ways for different purposes.  In the
case of RME devices the information communicated can be used to help control
and monitor the streaming system but it is not essential: there are other
arguably easier ways to maintain the streaming rates, and these alternatives
are what we use in FFADO now.

In MOTU devices the information relates mainly to device control changes
which are made via the front panel controls.  This is used to keep software
mixer/control programs in sync with the hardware when changes are made on
the hardware while the software control application is running.  It sounds
like this is close to what the Tascam units do based on Takashi's comments
(although obviously with a very different protocol).

jonathan

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-12 22:20   ` Jonathan Woithe
@ 2015-10-13  9:36     ` Takashi Sakamoto
  2015-10-13 10:02       ` Jonathan Woithe
  2015-10-13 14:15       ` Stefan Richter
  0 siblings, 2 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-13  9:36 UTC (permalink / raw)
  To: Jonathan Woithe, Stefan Richter; +Cc: tiwai, alsa-devel, clemens, ffado-devel

HI,

On Oct 13 2015 07:20, Jonathan Woithe wrote:
> On Mon, Oct 12, 2015 at 02:48:16PM +0200, Stefan Richter wrote:
>> On Oct 12 Takashi Sakamoto wrote:
>>> Although TASCAM FireWire series has physical controls, this patchset
>>> doesn't support them. I think some facilities are required to enable this
>>> functionality:
>>>   * handling MIDI messages on virtual MIDI ports by this driver
>>>   * handling status and control messages in isochronous packet by this
>>>     driver
>>
>> As a side note, AFAIR, RME devices encapsulate status in their isochronous
>> stream too... or was it MOTU?
>
> Both do, but in completely different ways for different purposes.  In the
> case of RME devices the information communicated can be used to help control
> and monitor the streaming system but it is not essential: there are other
> arguably easier ways to maintain the streaming rates, and these alternatives
> are what we use in FFADO now.
>
> In MOTU devices the information relates mainly to device control changes
> which are made via the front panel controls.  This is used to keep software
> mixer/control programs in sync with the hardware when changes are made on
> the hardware while the software control application is running.  It sounds
> like this is close to what the Tascam units do based on Takashi's comments
> (although obviously with a very different protocol).

(In this message, I mention about the other issues because of its 
priority, sorry.)

I don't know the reason that you're interested in it, while I'm not 
currently interested in it.

When the status and control messages are handled by the drivers, 
consumers are userspace applications. Userspace applications perform 
some reactions against the status and control messages. Most of the 
reactions are archieved via fw character devices.

(Of cource, I know there're some requirements for drivers to perform 
such responsibilities, instead of userspace applications. But it's a 
rare case, I think.)

When considering about userspace applications, permissions of fw 
character devices are important. Thus, 'udevd' in systemd project has 
enough rules for userspace applications to handle IEEE 1394 units for 
audio and sound.

In the series of the patchset, I committed for some models which 
Digidesign and TASCAM produced. Unfortunately, there're no rules for 
these models. Userspace applications are disabled to access these units. 
I believe this issue is prior to handling status and control messages.

Besides, I think this is a main issue which Damien Zammit faced in this 
post:

[alsa-devel] firewire mixer interface -- was Re: [PATCH 11/11] ALSA: 
digi00x: apply double-oh-three algorism to multiplex PCM samples
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-March/089381.html

If possible, I'd like to add enough rules to udevd, including RME and 
MOTU models before working for them.


Well, for udevd, current ALSA firewire stack has another issue. It's 
'device database' for IEEE 1394 units.

Currently, the database has entries with vendor/model name of OHCI 1394 
host controller for any IEEE 1394 units. It's preferrable to use 
vendor/model name of each units, at least for audio and sound.

PulseAudio includes a patch for this issue. It has additional conditions 
to select correct strings for IEEE 1394 units.
http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=3ac73598c67cb59a318c8baaf33fe97eed1e0b3e

If possible, I'd like to remove this ugly patch by working in udevd 
upstream.



I'm glad to get your help for these issues, Stefan and Jonathan.


Regards

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-13  9:36     ` Takashi Sakamoto
@ 2015-10-13 10:02       ` Jonathan Woithe
  2015-10-13 22:20         ` Stefan Richter
  2015-10-19 14:21         ` Takashi Sakamoto
  2015-10-13 14:15       ` Stefan Richter
  1 sibling, 2 replies; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-13 10:02 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

On Tue, Oct 13, 2015 at 06:36:32PM +0900, Takashi Sakamoto wrote:
> On Oct 13 2015 07:20, Jonathan Woithe wrote:
> >On Mon, Oct 12, 2015 at 02:48:16PM +0200, Stefan Richter wrote:
> >>On Oct 12 Takashi Sakamoto wrote:
> >>>Although TASCAM FireWire series has physical controls, this patchset
> >>>doesn't support them. I think some facilities are required to enable this
> >>>functionality:
> >>>  * handling MIDI messages on virtual MIDI ports by this driver
> >>>  * handling status and control messages in isochronous packet by this
> >>>    driver
> >>
> >>As a side note, AFAIR, RME devices encapsulate status in their isochronous
> >>stream too... or was it MOTU?
> >
> >Both do, but in completely different ways for different purposes. ...
> 
> I don't know the reason that you're interested in it, while I'm not
> currently interested in it.

My comment was to clarify things for Stefan.  Stefan was simply making the
observation that Tascam are not the only devices which send non-audio data
through iso packets.

> When the status and control messages are handled by the drivers, consumers
> are userspace applications. Userspace applications perform some reactions
> against the status and control messages. Most of the reactions are archieved
> via fw character devices.

That depends on what those reactions are.  But yes, in *most* cases it's
userspace that will be most interested in these data.

> When considering about userspace applications, permissions of fw character
> devices are important.

For sure.  However, for cases like MOTU, where mixer control setting updates
are pushed to the computer by the device in response to front panel actions,
there will be a need for userspace to somehow tap into this flow.  Userspace
could not gain access to the iso stream because the kernel will have control
over that.  Or can userspace tap into iso streams that have been initiated
by the kernel (that is surprising to me if it's true).

> If possible, I'd like to add enough rules to udevd, including RME and MOTU
> models before working for them.

I have some ideas about how both of these device families might fit in.  No
code yet, unfortunately, due to an unexpectedly busy year.  Note that the
issue of non-audio data extraction from the iso stream to user space isn't
relevant for RME: it's only MOTU which will require this.  In any case, it
goes beyond udevd permissions.

> I'm glad to get your help for these issues, Stefan and Jonathan.

Now that there is functional infrastructure within ALSA to build on I am
hoping to start experimenting with RME and MOTU devices over the next few
months by porting my FFADO streaming drivers to ALSA.

Regards
  jonathan

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

* Re: [PATCH 5/5] ALSA: firewire-tascam: change device probing processing
  2015-10-12 12:42   ` Stefan Richter
@ 2015-10-13 14:12     ` Takashi Sakamoto
  0 siblings, 0 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-13 14:12 UTC (permalink / raw)
  To: Stefan Richter; +Cc: tiwai, alsa-devel, clemens, ffado-devel

On Oct 12 2015 21:42, Stefan Richter wrote:
>> -static int check_name(struct snd_tscm *tscm)
>> +static int identify_model(struct snd_tscm *tscm)
>>  {
>>  	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
>> -	char vendor[8];
>> +	const u32 *config_rom = fw_dev->config_rom;
>>  	char model[8];
>> -	__u32 data;
>> -
>> -	/* Retrieve model name. */
>> -	data = be32_to_cpu(fw_dev->config_rom[28]);
>> -	memcpy(model, &data, 4);
>> -	data = be32_to_cpu(fw_dev->config_rom[29]);
>> -	memcpy(model + 4, &data, 4);
>> -	model[7] = '\0';
>> -
>> -	/* Retrieve vendor name. */
>> -	data = be32_to_cpu(fw_dev->config_rom[23]);
>> -	memcpy(vendor, &data, 4);
>> -	data = be32_to_cpu(fw_dev->config_rom[24]);
>> -	memcpy(vendor + 4, &data, 4);
>> -	vendor[7] = '\0';
>> +	unsigned int i;
>> +	u8 c;
>> +
>> +	if (fw_dev->config_rom_length < 30) {
>> +		dev_err(&tscm->unit->device,
>> +			"Configuration ROM is too short.\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* Pick up model name from certain addresses. */
>> +	for (i = 0; i < 8; i++) {
>> +		c = config_rom[28 + i / 4] >> (24 - 8 * (i % 4));
>> +		if (c == '\0')
>> +			break;
>> +		model[i] = c;
>> +	}
>> +	model[i] = '\0';
> 
> You could get a buffer overrun here.  Perhaps only go to i < 7:

Indeed, thanks.

> 	for (i = 0; i < 7; i++) {
> 		[...]
> 	}
> 	model[i] = '\0';
> 
>> +	for (i = 0; i < ARRAY_SIZE(model_specs); i++) {
>> +		if (strcmp(model, model_specs[i].name) == 0) {
>> +			tscm->spec = &model_specs[i];
>> +			break;
>> +		}
>> +	}
>> +	if (tscm->spec == NULL)
>> +		return -ENODEV;
>>  
>>  	strcpy(tscm->card->driver, "FW-TASCAM");
>>  	strcpy(tscm->card->shortname, model);
>>  	strcpy(tscm->card->mixername, model);
>>  	snprintf(tscm->card->longname, sizeof(tscm->card->longname),
>> -		 "%s %s, GUID %08x%08x at %s, S%d", vendor, model,
>> +		 "TASCAM %s, GUID %08x%08x at %s, S%d", model,
>>  		 cpu_to_be32(fw_dev->config_rom[3]),
>>  		 cpu_to_be32(fw_dev->config_rom[4]),
>>  		 dev_name(&tscm->unit->device), 100 << fw_dev->max_speed);
> 
> Should be
> 		fw_dev->config_rom[3],
> 		fw_dev->config_rom[4],
> 
> since snprintf wants CPU-endian values.

Firewire-digi00x also includes the same bug.

I found some endianness bug in the other modules. I'll fixed these bugs
in the same series of patches later.


Thanks

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-13  9:36     ` Takashi Sakamoto
  2015-10-13 10:02       ` Jonathan Woithe
@ 2015-10-13 14:15       ` Stefan Richter
  2015-10-19 14:13         ` Takashi Sakamoto
  1 sibling, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2015-10-13 14:15 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

On Oct 13 Takashi Sakamoto wrote:
> HI,
> 
> On Oct 13 2015 07:20, Jonathan Woithe wrote:
> > On Mon, Oct 12, 2015 at 02:48:16PM +0200, Stefan Richter wrote:
> >> On Oct 12 Takashi Sakamoto wrote:
> >>> Although TASCAM FireWire series has physical controls, this patchset
> >>> doesn't support them. I think some facilities are required to enable this
> >>> functionality:
> >>>   * handling MIDI messages on virtual MIDI ports by this driver
> >>>   * handling status and control messages in isochronous packet by this
> >>>     driver
> >>
> >> As a side note, AFAIR, RME devices encapsulate status in their isochronous
> >> stream too... or was it MOTU?
> >
> > Both do, but in completely different ways for different purposes.  In the
> > case of RME devices the information communicated can be used to help control
> > and monitor the streaming system but it is not essential: there are other
> > arguably easier ways to maintain the streaming rates, and these alternatives
> > are what we use in FFADO now.
> >
> > In MOTU devices the information relates mainly to device control changes
> > which are made via the front panel controls.  This is used to keep software
> > mixer/control programs in sync with the hardware when changes are made on
> > the hardware while the software control application is running.  It sounds
> > like this is close to what the Tascam units do based on Takashi's comments
> > (although obviously with a very different protocol).
> 
> (In this message, I mention about the other issues because of its 
> priority, sorry.)
> 
> I don't know the reason that you're interested in it, while I'm not 
> currently interested in it.

I was just generally wondering what the extent of protocol similarities
are.  Personally I don't own a Digidesign, TASCAM, RME or MOTU at this
time.  Thanks to you and to Jonathan for explaining.

> When the status and control messages are handled by the drivers, 
> consumers are userspace applications. Userspace applications perform 
> some reactions against the status and control messages. Most of the 
> reactions are archieved via fw character devices.
> 
> (Of cource, I know there're some requirements for drivers to perform 
> such responsibilities, instead of userspace applications. But it's a 
> rare case, I think.)
> 
> When considering about userspace applications, permissions of fw 
> character devices are important. Thus, 'udevd' in systemd project has 
> enough rules for userspace applications to handle IEEE 1394 units for 
> audio and sound.

Many of the udev rules which concern audio devices are actually maintained
in the FFADO project and thus installed by the FFADO package (or by
installing FFADO from source), in contrast to the usual way of keeping
udev rules in the udev/systemd project and installing them via an udev
package.

FFADO is of course not unique in this regard.  As an example, the following
packages are providing files in /lib/udev/rules.d/ on my current desktop
installation:  alsa-utils, consolekit, fuse, kino, libffado, libgphoto2,
libwacom, lvm2, media-player-info, netifrc, ntfs3g, sane-backends, udev,
udisks, upower-pm-utils.  And of course I have site-specific rules in
/etc/udev/rules.d/ too.

> In the series of the patchset, I committed for some models which 
> Digidesign and TASCAM produced. Unfortunately, there're no rules for 
> these models. Userspace applications are disabled to access these units. 
> I believe this issue is prior to handling status and control messages.
> 
> Besides, I think this is a main issue which Damien Zammit faced in this 
> post:
> 
> [alsa-devel] firewire mixer interface -- was Re: [PATCH 11/11] ALSA: 
> digi00x: apply double-oh-three algorism to multiplex PCM samples
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-March/089381.html

A developer of a mixer program which requires access to /dev/fw* has a few
options:
  - During early development, he could ask his testers to put a tentative
    rule into /etc/udev/rules.d/ themselves.
  - If the program is being developed as part of the FFADO project,
    necessary rules should obviously be added to 60-ffado.rules.
  - Otherwise the rules could be maintained and packaged together with the
    mixer program.
  - Or the rules could be submitted to the udev/systemd project.

Going the mainline udev route will naturally ensure that the rules conform
to standard practice, and that may make the lives of distributors, admins,
or users easier.

> If possible, I'd like to add enough rules to udevd, including RME and 
> MOTU models before working for them.

For those RME and MOTU models whose mixer program continues to be
developed in the FFADO project, the FFADO-maintained ruleset is obviously
sufficient.

> Well, for udevd, current ALSA firewire stack has another issue. It's 
> 'device database' for IEEE 1394 units.
> 
> Currently, the database has entries with vendor/model name of OHCI 1394 
> host controller for any IEEE 1394 units. It's preferrable to use 
> vendor/model name of each units, at least for audio and sound.
> 
> PulseAudio includes a patch for this issue. It has additional conditions 
> to select correct strings for IEEE 1394 units.
> http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=3ac73598c67cb59a318c8baaf33fe97eed1e0b3e
> 
> If possible, I'd like to remove this ugly patch by working in udevd 
> upstream.

I saw the message which David Henningsson copied to linux1394-devel on
February 16 but did not react---and have not noticed the issue before---,
as I don't have pulseaudio installed on my own PCs yet.  It's probably
the wrong place to ask, but:  Why do they generally prefer
ID_{VENDOR,MODEL}_FROM_DATABASE over ID_{VENDOR,MODEL}?  I guess the
former is more reliable or nicer on some other buses.

I will have a look at the libudev sources whether the _FROM_DATABASE
results can be improved for firewire.
-- 
Stefan Richter
-=====-===== =-=- -==-=
http://arcgraph.de/sr/

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-13 10:02       ` Jonathan Woithe
@ 2015-10-13 22:20         ` Stefan Richter
  2015-10-19 14:21         ` Takashi Sakamoto
  1 sibling, 0 replies; 26+ messages in thread
From: Stefan Richter @ 2015-10-13 22:20 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: tiwai, alsa-devel, clemens, ffado-devel, Takashi Sakamoto

On Oct 13 Jonathan Woithe wrote:
> for cases like MOTU, where mixer control setting updates
> are pushed to the computer by the device in response to front panel actions,
> there will be a need for userspace to somehow tap into this flow.  Userspace
> could not gain access to the iso stream because the kernel will have control
> over that.  Or can userspace tap into iso streams that have been initiated
> by the kernel (that is surprising to me if it's true).

Indeed we do not have support for multiple local listeners on the same
stream.  (firewire-core supports multiple listeners for FCP reception, but
this is a simpler case in several respects.)

I guess getting MOTU status out to userspace could work by having the
audio driver extract the data from the stream and push it as a new type of
event of the <sound/firewire.h> API through the ALSA hwdep interface.

(Apologies for going somewhat offtopic.)
-- 
Stefan Richter
-=====-===== =-=- -==-=
http://arcgraph.de/sr/

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-13 14:15       ` Stefan Richter
@ 2015-10-19 14:13         ` Takashi Sakamoto
  2015-10-19 23:36           ` Jonathan Woithe
  0 siblings, 1 reply; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-19 14:13 UTC (permalink / raw)
  To: Stefan Richter; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

Hi Stefan,

Sorry to be late for reply, but I have some works to ALSA firewire
stack. Furthermore, it takes me a bit time to write messages because I
always start to write after enough consideration, investigation and
arrangement about the point of issues.

On Oct 13 2015 23:15, Stefan Richter wrote:
> I was just generally wondering what the extent of protocol similarities
> are.  Personally I don't own a Digidesign, TASCAM, RME or MOTU at this
> time.  Thanks to you and to Jonathan for explaining.

These protocols use different format of data block, while I guess that
the number of data blocks transferred per second and the number of data
blocks in packets follow to IEC 61883-6. In this developing period, I
re-design AMDTP functionality in firewire-lib according to this assumption.

>> When the status and control messages are handled by the drivers, 
>> consumers are userspace applications. Userspace applications perform 
>> some reactions against the status and control messages. Most of the 
>> reactions are archieved via fw character devices.
>>
>> (Of cource, I know there're some requirements for drivers to perform 
>> such responsibilities, instead of userspace applications. But it's a 
>> rare case, I think.)
>>
>> When considering about userspace applications, permissions of fw 
>> character devices are important. Thus, 'udevd' in systemd project has 
>> enough rules for userspace applications to handle IEEE 1394 units for 
>> audio and sound.
> 
> Many of the udev rules which concern audio devices are actually maintained
> in the FFADO project and thus installed by the FFADO package (or by
> installing FFADO from source), in contrast to the usual way of keeping
> udev rules in the udev/systemd project and installing them via an udev
> package.
>
> FFADO is of course not unique in this regard.  As an example, the following
> packages are providing files in /lib/udev/rules.d/ on my current desktop
> installation:  alsa-utils, consolekit, fuse, kino, libffado, libgphoto2,
> libwacom, lvm2, media-player-info, netifrc, ntfs3g, sane-backends, udev,
> udisks, upower-pm-utils.  And of course I have site-specific rules in
> /etc/udev/rules.d/ too.

It's just for units supported by FFADO project, while udev has its rule
for ALSA sound cards.

https://github.com/systemd/systemd/blob/master/rules/78-sound-card.rules

When considering about the requirement of permissions for fw character
devices for the units, adding more rules for them is valuable. At least,
it's good to discuss about it, I think.

>> In the series of the patchset, I committed for some models which 
>> Digidesign and TASCAM produced. Unfortunately, there're no rules for 
>> these models. Userspace applications are disabled to access these units. 
>> I believe this issue is prior to handling status and control messages.
>>
>> Besides, I think this is a main issue which Damien Zammit faced in this 
>> post:
>>
>> [alsa-devel] firewire mixer interface -- was Re: [PATCH 11/11] ALSA: 
>> digi00x: apply double-oh-three algorism to multiplex PCM samples
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-March/089381.html
> 
> A developer of a mixer program which requires access to /dev/fw* has a few
> options:
>   - During early development, he could ask his testers to put a tentative
>     rule into /etc/udev/rules.d/ themselves.
>   - If the program is being developed as part of the FFADO project,
>     necessary rules should obviously be added to 60-ffado.rules.
>   - Otherwise the rules could be maintained and packaged together with the
>     mixer program.
>   - Or the rules could be submitted to the udev/systemd project.
> 
> Going the mainline udev route will naturally ensure that the rules conform
> to standard practice, and that may make the lives of distributors, admins,
> or users easier.

I'll continue to have cooperative relationship to FFADO project, while,
to tell the truth, I'm not willing to add my mixer/control programs to
the project anymore for several reasons:

 * code base is too old.
  * Especially for python3 support. Many distributions start to suppress
    python2 supports and near future ffado packages may be grouped
    as 'deprecated'.
 * over-specification
  * For most of mixer/control applications, it's just enough to
    send/receive asynchronous transactions. On the other hand, FFADO
    framework force developers to implement more codes unrelated to it.
    For example, D-Bus programming.
  * Libffado also includes streaming functionality and this is used to
    jackd. It's not so easy to modify without regressions, especially
    discover routine.
 * defectives as service application for IEEE 1394 bus
  * Libffado cannot handle bus-reset correctly, thus libffado cannot
    handle device hot-plugging. Although, ffado-dbus-server performs as
    system service. This is due to wrong usage of libraw1394 API.

But this is in my case. In this thread, it's better to continue to
discuss with ignoring it.

>> If possible, I'd like to add enough rules to udevd, including RME and 
>> MOTU models before working for them.
> 
> For those RME and MOTU models whose mixer program continues to be
> developed in the FFADO project, the FFADO-maintained ruleset is obviously
> sufficient.

As long as mixer/controls programs are developed by FFADO project only.

Here, I don't mean that I will do it. But it's disadvantage to persons
who like simpler applications. For such users, FFADO stuffs are not
always needed, especially for no-GUI users. I've got several messages
from such users and reccomend to use 'firewire-request' in
linux-firewire-tools package. For such users, it's better to add the
rules for fw character devices.

>> Well, for udevd, current ALSA firewire stack has another issue. It's 
>> 'device database' for IEEE 1394 units.
>>
>> Currently, the database has entries with vendor/model name of OHCI 1394 
>> host controller for any IEEE 1394 units. It's preferrable to use 
>> vendor/model name of each units, at least for audio and sound.
>>
>> PulseAudio includes a patch for this issue. It has additional conditions 
>> to select correct strings for IEEE 1394 units.
>> http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=3ac73598c67cb59a318c8baaf33fe97eed1e0b3e
>>
>> If possible, I'd like to remove this ugly patch by working in udevd 
>> upstream.
> 
> I saw the message which David Henningsson copied to linux1394-devel on
> February 16 but did not react---and have not noticed the issue before---,
> as I don't have pulseaudio installed on my own PCs yet.  It's probably
> the wrong place to ask, but:  Why do they generally prefer
> ID_{VENDOR,MODEL}_FROM_DATABASE over ID_{VENDOR,MODEL}?  I guess the
> former is more reliable or nicer on some other buses.

Yep. I also have the same opinion about PulseAudio implementation. I
guess there're some reasons shared to PulseAudio/udev/systemd
developers, not to us.

> I will have a look at the libudev sources whether the _FROM_DATABASE
> results can be improved for firewire.

Which lines?


Thanks

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-13 10:02       ` Jonathan Woithe
  2015-10-13 22:20         ` Stefan Richter
@ 2015-10-19 14:21         ` Takashi Sakamoto
  2015-10-19 23:45           ` Jonathan Woithe
  1 sibling, 1 reply; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-19 14:21 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

Hi Jonathan,

On Oct 13 2015 19:02, Jonathan Woithe wrote:
>> I'm glad to get your help for these issues, Stefan and Jonathan.
> 
> Now that there is functional infrastructure within ALSA to build on I am
> hoping to start experimenting with RME and MOTU devices over the next few
> months by porting my FFADO streaming drivers to ALSA.

This year I got some supports (not commercial) from Japan office of
Synthax Inc. They lent some RME models to me. At the beginning of audio
and music unit on IEEE 1394 bus, I'll borrow FireFace 400 and start to
develop for ALSA RME driver, next month.

Currently, ALSA MOTU driver is under working in my repository, so it's
better to start your development for it.


Thanks

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-19 14:13         ` Takashi Sakamoto
@ 2015-10-19 23:36           ` Jonathan Woithe
  2015-10-20  0:50             ` Takashi Sakamoto
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-19 23:36 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

Hi Takashi

On Mon, Oct 19, 2015 at 11:13:24PM +0900, Takashi Sakamoto wrote:
> On Oct 13 2015 23:15, Stefan Richter wrote:
> > I was just generally wondering what the extent of protocol similarities
> > are.  Personally I don't own a Digidesign, TASCAM, RME or MOTU at this
> > time.  Thanks to you and to Jonathan for explaining.
> 
> These protocols use different format of data block, while I guess that
> the number of data blocks transferred per second and the number of data
> blocks in packets follow to IEC 61883-6. In this developing period, I
> re-design AMDTP functionality in firewire-lib according to this assumption.

As far as I remember IEC 61883-6 uses a fixed number of frames per packet
(is this what you meant by "blocks in packets"?): 8, 16 or 32 depending on
sample rate.  My recollection could be wrong here though.  In any case:

 * For RME the number of frames per packet is not fixed which allows the
   software to utilise whichever number it needs in order to effect an
   efficient data transfer.  Within a given running stream this number can
   vary by +/- 1 as required to maintain audio sync with the interface.

 * Neither RME or the MOTU protocols sync to the firewire clock.  They have
   a separate audio clock which is not locked to the firewire clock under
   any circumstances, nor does the firewire clock ever lock to the audio
   clock (or the master oscillator from which it is derived).  As a result
   the "number of data blocks transferred per second" is not fixed and
   depends on the relationship between the audio clock and the firewire bus
   clock.  It's a long time since I read the relevant portions of IEC
   61883-6 so I can't recall exactly how this fits with the reality of these
   interfaces.

It should be noted that neither of these devices make claims about using IEC
61883-6 in any way, although it happens that there are some similarities.

> ... while, to tell the truth, I'm not willing to add my mixer/control
> programs to the project anymore for several reasons:

I'm disappointed to hear this, but each to their own.  I think it would be
good to evolve FFADO such that ultimately the control/mixer applications for
firewire devices were all developed under this one project so users had a
one-stop shop whenever they needed a control/mixer application for firewire
devices.

There are also other reasons to seriously consider a common project for
these control/mixer programs.  Perhaps most significantly, while the layout
and capabilities of each device vary markedly the low level controls
required are often common.  For example, most devices require a matrix mixer
of some sort.  If a new generation of control programs were to live under a
common project then there is the opportunity to share these elements between
all such programs.  If each is developed in isolation this can't happen,
resulting in many implementations of the same thing.

Of course one option would be to incorporate such programs into alsa-tools
or alsa-utils.  However, due to the divergent nature of the programs the
collection would quickly become quite sizable which warrants consideration
keeping them separate.  FFADO is already established and could easily act as
the repository for these new tools.

>  * code base is too old.
>   * Especially for python3 support. Many distributions start to suppress
>     python2 supports and near future ffado packages may be grouped
>     as 'deprecated'.

There is no reason at all that new control/mixer applications within FFADO
have to use the existing infrastructure.  If someone were to submit a pure
python3 mixer/control application for inclusion within FFADO which did not
utilise the existing FFADO infrastructure I would have no problem with that
and would be happy to include it in the project.

As an aside, it would be great if the existing python mixer code could be
migrated to python3 (with or without reliance on libffado as it exists
presently) since there is a lot of useful things in there.  As always the
issue is time and developer resources (and the fact that none of the
currently active FFADO developers are python3 experts).  In the meantime,
any new program that's written could make use of python3 from the start if
that's what the developer thought was best.

Another option to consider would be the creation of a ffado3 development
which is designed from the outset to work only in conjunction with the
kernel streaming drivers.  This makes the break from the old infastructure
explicit while still being able to provide a common framework to support
mixer/control programs.

>  * over-specification
>   * For most of mixer/control applications, it's just enough to
>     send/receive asynchronous transactions. On the other hand, FFADO
>     framework force developers to implement more codes unrelated to it.
>     For example, D-Bus programming.

I think there is a misunderstanding here.  Firstly, as above, there is
nothing forcing developers to use any of the existing framework.  If someone
were to write a new mixer application for some device which utilised native
async send/receive without any reliance on the rest of the FFADO
infrastructure I would have no problem with that.

Secondly, it should be said that the existing FFADO framework does not force
developers to deal with D-Bus programming directly.  The details are taken
care of in the lower layers of libffado.  I have personally written two
drivers and the corresponding mixer modules and have never once done what I
would consider to be "D-Bus programming".

Finally, I agree completely that the existing FFADO device infrastructure is
overkill if control/mixer applications are the only target.  However, it's
not compulsary to utilise it, as explained previously.  For control/mixer
programs it would make sense to implement a simple direct usage of async
commands.

>   * Libffado also includes streaming functionality and this is used to
>     jackd. It's not so easy to modify without regressions, especially
>     discover routine.

The streaming functionality is part of the existing infrastructure which
does not need to be used in new support programs if it is not needed.  In
any case, the streaming code remains completely dormant unless explicitly
called.  Therefore a control/mixer program can - if it wishes - make use of
the existing driver layer without those drivers ever attempting to do any
streaming.  As streaming functionality is moved into the kernel and debugged
against relevant interfaces there will clearly be no need to update the
FFADO streaming components, and in time I see that they will in fact be
removed.

>  * defectives as service application for IEEE 1394 bus
>   * Libffado cannot handle bus-reset correctly, thus libffado cannot
>     handle device hot-plugging. Although, ffado-dbus-server performs as
>     system service. This is due to wrong usage of libraw1394 API.

This is not something I've had any personal problem with.  Perhaps it
depends on the device being used.  FFADO does include bus reset handling. 
While it may not work reliably during streaming, it seems to work at least
for some devices in the context of control/mixer programs.  If there is an
issue during streaming it could be argued that it's irrelevant since the
streaming portion of FFADO will be deprecated once all drivers are present
in the kernel and in any case, if streaming audio and a device goes away you
clearly have other issues to solve.

All that aside, new control/mixer programs would not need to use libffado if
the author didn't want to.

> But this is in my case. In this thread, it's better to continue to
> discuss with ignoring it.

I think the discussion about where future control/mixer programs will live
should happen sooner rather than later.  As stated, I think they ought to be
collected under a single coordinated project to facilitate the sharing of
basic building blocks and I can see no reason why this can't be FFADO. 
However, obviously I'm open to other suggestions.  Having the coordinated
project for all such programs also provides a natural place to have and
maintain any udev rules which are required, much like FFADO already does.

> >> If possible, I'd like to add enough rules to udevd, including RME and 
> >> MOTU models before working for them.
> > 
> > For those RME and MOTU models whose mixer program continues to be
> > developed in the FFADO project, the FFADO-maintained ruleset is obviously
> > sufficient.
> 
> As long as mixer/controls programs are developed by FFADO project only.

No, not necessarily.  Yes, FFADO (in whatever incarnation) would need to be
installed to pick up the udev rules.  But once this was done there is no
requirement to actually use the programs provided by FFADO if you don't want
to, and it's not as if FFADO takes up hundreds of megabytes of storage
space.  If the rules are present then any user in the "audio" group can
access the fw device node of the audio interface in whatever way they
choose: they don't have to use libffado.

Having said that, I think it is a natural evolution for the next generation
of control/mixer programs to be developed under the FFADO umbrella.

Regards
  jonathan

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-19 14:21         ` Takashi Sakamoto
@ 2015-10-19 23:45           ` Jonathan Woithe
  0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-19 23:45 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

Hi Takashi

On Mon, Oct 19, 2015 at 11:21:09PM +0900, Takashi Sakamoto wrote:
> > Now that there is functional infrastructure within ALSA to build on I am
> > hoping to start experimenting with RME and MOTU devices over the next few
> > months by porting my FFADO streaming drivers to ALSA.
> 
> This year I got some supports (not commercial) from Japan office of
> Synthax Inc. They lent some RME models to me. At the beginning of audio
> and music unit on IEEE 1394 bus, I'll borrow FireFace 400 and start to
> develop for ALSA RME driver, next month.
> 
> Currently, ALSA MOTU driver is under working in my repository, so it's
> better to start your development for it.

To be honest I would rather see the RME functionality in place first because
MOTU are hostile to Linux and have never provided any assistance to Linux
development efforts.  While I definitely want to migrate the MOTU FFADO
streaming driver into the kernel, RME is definitely my priority at this time
and what I want to spend my very limited development time on.

You obviously have far more time to work on this than me at the present
moment so I suspect you'd get something working before me.  That's fine. 
However, when you are ready to start working on the RME driver I would very
much like to talk to you about the infrastructure required in order to make
it possible to add support for the rest of the family beyond the FF400 along
with other features which may not be immediately obvious by a reading of the
FFADO driver code.  I have ideas about how this might come together.

Regards
  jonathan

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-19 23:36           ` Jonathan Woithe
@ 2015-10-20  0:50             ` Takashi Sakamoto
  2015-10-20  2:09               ` Takashi Sakamoto
                                 ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-20  0:50 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

On Oct 20 2015 08:36, Jonathan Woithe wrote:
> Hi Takashi
> 
> On Mon, Oct 19, 2015 at 11:13:24PM +0900, Takashi Sakamoto wrote:
>> On Oct 13 2015 23:15, Stefan Richter wrote:
>>> I was just generally wondering what the extent of protocol similarities
>>> are.  Personally I don't own a Digidesign, TASCAM, RME or MOTU at this
>>> time.  Thanks to you and to Jonathan for explaining.
>>
>> These protocols use different format of data block, while I guess that
>> the number of data blocks transferred per second and the number of data
>> blocks in packets follow to IEC 61883-6. In this developing period, I
>> re-design AMDTP functionality in firewire-lib according to this assumption.
> 
> As far as I remember IEC 61883-6 uses a fixed number of frames per packet
> (is this what you meant by "blocks in packets"?): 8, 16 or 32 depending on
> sample rate.

No. It's so called as 'blocking transmission', while IEC 61883-6:2000
describes 'non-blocking transmission' in its clause 6.4. The blocking
transmission is in its Annex A, so optional.

> My recollection could be wrong here though.  In any case:
> 
>  * For RME the number of frames per packet is not fixed which allows the
>    software to utilise whichever number it needs in order to effect an
>    efficient data transfer.  Within a given running stream this number can
>    vary by +/- 1 as required to maintain audio sync with the interface.
>
>  * Neither RME or the MOTU protocols sync to the firewire clock.  They have
>    a separate audio clock which is not locked to the firewire clock under
>    any circumstances, nor does the firewire clock ever lock to the audio
>    clock (or the master oscillator from which it is derived).  As a result
>    the "number of data blocks transferred per second" is not fixed and
>    depends on the relationship between the audio clock and the firewire bus
>    clock.  It's a long time since I read the relevant portions of IEC
>    61883-6 so I can't recall exactly how this fits with the reality of these
>    interfaces.

This is described in IEC 61883-6, not RME/MOTU specific. I reccomend you
to find oppoturnity to read it again for better understanding.

> It should be noted that neither of these devices make claims about using IEC
> 61883-6 in any way, although it happens that there are some similarities.
> 
>> ... while, to tell the truth, I'm not willing to add my mixer/control
>> programs to the project anymore for several reasons:
> 
> I'm disappointed to hear this, but each to their own.  I think it would be
> good to evolve FFADO such that ultimately the control/mixer applications for
> firewire devices were all developed under this one project so users had a
> one-stop shop whenever they needed a control/mixer application for firewire
> devices.
> 
> There are also other reasons to seriously consider a common project for
> these control/mixer programs.  Perhaps most significantly, while the layout
> and capabilities of each device vary markedly the low level controls
> required are often common.  For example, most devices require a matrix mixer
> of some sort.  If a new generation of control programs were to live under a
> common project then there is the opportunity to share these elements between
> all such programs.  If each is developed in isolation this can't happen,
> resulting in many implementations of the same thing.
> 
> Of course one option would be to incorporate such programs into alsa-tools
> or alsa-utils.  However, due to the divergent nature of the programs the
> collection would quickly become quite sizable which warrants consideration
> keeping them separate.  FFADO is already established and could easily act as
> the repository for these new tools.
> 
>>  * code base is too old.
>>   * Especially for python3 support. Many distributions start to suppress
>>     python2 supports and near future ffado packages may be grouped
>>     as 'deprecated'.
> 
> There is no reason at all that new control/mixer applications within FFADO
> have to use the existing infrastructure.  If someone were to submit a pure
> python3 mixer/control application for inclusion within FFADO which did not
> utilise the existing FFADO infrastructure I would have no problem with that
> and would be happy to include it in the project.
> 
> As an aside, it would be great if the existing python mixer code could be
> migrated to python3 (with or without reliance on libffado as it exists
> presently) since there is a lot of useful things in there.  As always the
> issue is time and developer resources (and the fact that none of the
> currently active FFADO developers are python3 experts).  In the meantime,
> any new program that's written could make use of python3 from the start if
> that's what the developer thought was best.
> 
> Another option to consider would be the creation of a ffado3 development
> which is designed from the outset to work only in conjunction with the
> kernel streaming drivers.  This makes the break from the old infastructure
> explicit while still being able to provide a common framework to support
> mixer/control programs.
> 
>>  * over-specification
>>   * For most of mixer/control applications, it's just enough to
>>     send/receive asynchronous transactions. On the other hand, FFADO
>>     framework force developers to implement more codes unrelated to it.
>>     For example, D-Bus programming.
> 
> I think there is a misunderstanding here.  Firstly, as above, there is
> nothing forcing developers to use any of the existing framework.  If someone
> were to write a new mixer application for some device which utilised native
> async send/receive without any reliance on the rest of the FFADO
> infrastructure I would have no problem with that.
> 
> Secondly, it should be said that the existing FFADO framework does not force
> developers to deal with D-Bus programming directly.  The details are taken
> care of in the lower layers of libffado.  I have personally written two
> drivers and the corresponding mixer modules and have never once done what I
> would consider to be "D-Bus programming".
> 
> Finally, I agree completely that the existing FFADO device infrastructure is
> overkill if control/mixer applications are the only target.  However, it's
> not compulsary to utilise it, as explained previously.  For control/mixer
> programs it would make sense to implement a simple direct usage of async
> commands.
> 
>>   * Libffado also includes streaming functionality and this is used to
>>     jackd. It's not so easy to modify without regressions, especially
>>     discover routine.
> 
> The streaming functionality is part of the existing infrastructure which
> does not need to be used in new support programs if it is not needed.  In
> any case, the streaming code remains completely dormant unless explicitly
> called.  Therefore a control/mixer program can - if it wishes - make use of
> the existing driver layer without those drivers ever attempting to do any
> streaming.  As streaming functionality is moved into the kernel and debugged
> against relevant interfaces there will clearly be no need to update the
> FFADO streaming components, and in time I see that they will in fact be
> removed.
> 
>>  * defectives as service application for IEEE 1394 bus
>>   * Libffado cannot handle bus-reset correctly, thus libffado cannot
>>     handle device hot-plugging. Although, ffado-dbus-server performs as
>>     system service. This is due to wrong usage of libraw1394 API.
> 
> This is not something I've had any personal problem with.  Perhaps it
> depends on the device being used.  FFADO does include bus reset handling. 
> While it may not work reliably during streaming, it seems to work at least
> for some devices in the context of control/mixer programs.  If there is an
> issue during streaming it could be argued that it's irrelevant since the
> streaming portion of FFADO will be deprecated once all drivers are present
> in the kernel and in any case, if streaming audio and a device goes away you
> clearly have other issues to solve.
> 
> All that aside, new control/mixer programs would not need to use libffado if
> the author didn't want to.
> 
>> But this is in my case. In this thread, it's better to continue to
>> discuss with ignoring it.
> 
> I think the discussion about where future control/mixer programs will live
> should happen sooner rather than later.  As stated, I think they ought to be
> collected under a single coordinated project to facilitate the sharing of
> basic building blocks and I can see no reason why this can't be FFADO. 
> However, obviously I'm open to other suggestions.  Having the coordinated
> project for all such programs also provides a natural place to have and
> maintain any udev rules which are required, much like FFADO already does.

...It seems to be my mistake to have described current FFADO status.
Just ignoring it, sorry but I'm not willing to take my time for such
discussion. It's waste of time due to differences of our stance on this
softwares and a lack of understanding about Linux system.

>>>> If possible, I'd like to add enough rules to udevd, including RME and 
>>>> MOTU models before working for them.
>>>
>>> For those RME and MOTU models whose mixer program continues to be
>>> developed in the FFADO project, the FFADO-maintained ruleset is obviously
>>> sufficient.
>>
>> As long as mixer/controls programs are developed by FFADO project only.
> 
> No, not necessarily.  Yes, FFADO (in whatever incarnation) would need to be
> installed to pick up the udev rules.  But once this was done there is no
> requirement to actually use the programs provided by FFADO if you don't want
> to, and it's not as if FFADO takes up hundreds of megabytes of storage
> space.  If the rules are present then any user in the "audio" group can
> access the fw device node of the audio interface in whatever way they
> choose: they don't have to use libffado.

Exactly. I don't want to install ffado packages just for udev rules.
When using polkit correctly, I guess users doesn't need to join in
'audio' group, so as PulseAudio achieved with polkit.

> Having said that, I think it is a natural evolution for the next generation
> of control/mixer programs to be developed under the FFADO umbrella.

I prefer imagination to subsistence for discussion. Especially, 'A is
better but actually where A is and who works for A' is meaningless for
discussion.


Thanks

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-20  0:50             ` Takashi Sakamoto
@ 2015-10-20  2:09               ` Takashi Sakamoto
  2015-10-20  2:57                 ` Jonathan Woithe
  2015-10-20  2:52               ` Jonathan Woithe
  2015-10-20  7:39               ` Stefan Richter
  2 siblings, 1 reply; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-20  2:09 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

On Oct 20 2015 09:50, Takashi Sakamoto wrote:
>> Having said that, I think it is a natural evolution for the next generation
>> of control/mixer programs to be developed under the FFADO umbrella.
>
> I prefer imagination to subsistence for discussion. Especially, 'A is
> better but actually where A is and who works for A' is meaningless for
> discussion.

Oops. I mistook to write English text.

'I prefer substance to imagination for discussion'.


Regards

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-20  0:50             ` Takashi Sakamoto
  2015-10-20  2:09               ` Takashi Sakamoto
@ 2015-10-20  2:52               ` Jonathan Woithe
  2015-10-20  7:39               ` Stefan Richter
  2 siblings, 0 replies; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-20  2:52 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

On Tue, Oct 20, 2015 at 09:50:46AM +0900, Takashi Sakamoto wrote:
> > As far as I remember IEC 61883-6 uses a fixed number of frames per packet
> > (is this what you meant by "blocks in packets"?): 8, 16 or 32 depending on
> > sample rate.  My recollection could be wrong here though.
> 
> No. It's so called as 'blocking transmission', while IEC 61883-6:2000
> describes 'non-blocking transmission' in its clause 6.4. The blocking
> transmission is in its Annex A, so optional.

Ok.

> > I think the discussion about where future control/mixer programs will live
> > should happen sooner rather than later.  As stated, I think they ought to be
> > collected under a single coordinated project to facilitate the sharing of
> > basic building blocks and I can see no reason why this can't be FFADO. 
> > However, obviously I'm open to other suggestions.  Having the coordinated
> > project for all such programs also provides a natural place to have and
> > maintain any udev rules which are required, much like FFADO already does.
> 
> ...It seems to be my mistake to have described current FFADO status.
> Just ignoring it, sorry but I'm not willing to take my time for such
> discussion. It's waste of time due to differences of our stance on this
> softwares and a lack of understanding about Linux system.

I am not sure it was intended, but this response unfortunately comes across
to me as abrasive and confrontational.  Misleading and incorrect claims were
made about FFADO and what would be possible within the confines of that
project, and these were then used to justify a particular view.  Since these
were made in a public forum I felt that the record should be corrected.

It was not a mistake for you to describe what you thought to be the current
status of FFADO because it gives a much clearer picture of what your views
are with regard to FFADO.  My response was an attempt to continue the
discussion so we could better understand where the other was coming from,
and come up with a coordinated plan to migrate some remaining firewire audio
device streaming support into the kernel.  I am disappointed that you view
such a discussion as a "waste of time".

It remains my view that control and mixer programs for firewire audio
interfaces should be developed under a single project due to the opportunity
to share control widgets and other features.  Unfortunately I do not know
the details of your "stance on this softwares" (sic) which has lead to you
the conclusion that this is not a good idea and could not be done under the
FFADO umbrella (perhaps as a ffado3 development, starting with a clean
slate).  As a result, all I can really see at this point is you prefer to go
it alone with the development of your control programs for reasons unknown
to me.

It is unclear to me who "a lack of understanding about Linux system" is
directed to.  If it was towards me I dispute the assertion, having been
using Linux since 1992, contributing to various projects since the early
2000s and writing Linux firewire audio drivers for over 8 years.

> > No, not necessarily.  Yes, FFADO (in whatever incarnation) would need to be
> > installed to pick up the udev rules.  But once this was done there is no
> > requirement to actually use the programs provided by FFADO if you don't want
> > to, and it's not as if FFADO takes up hundreds of megabytes of storage
> > space.  If the rules are present then any user in the "audio" group can
> > access the fw device node of the audio interface in whatever way they
> > choose: they don't have to use libffado.
> 
> Exactly. I don't want to install ffado packages just for udev rules.

Under the scenario I envisaged (which was looking to a future time when the
transistion to kernel streaming drivers was complete), a "ffado" install
would consist of the necessary udev rules plus the control/mixer programs
for the interfaces.  In other words, it would be the things that most people
would be using.  FFADO would no longer need or include any of the existing
driver infrastructure because streaming would be handled in the kernel.  But
whatever, the location of udev rules isn't something I intend to loose sleep
over.  Besides, if the control utilities are maintained in isolation from
each other then it's a moot point anyway.

> > Having said that, I think it is a natural evolution for the next generation
> > of control/mixer programs to be developed under the FFADO umbrella.
> 
> I prefer imagination to subsistence for discussion. Especially, 'A is
> better but actually where A is and who works for A' is meaningless for
> discussion.

I don't quite follow this statement but it seems to be dismissive of the
usefulness of ongoing discussion.

If I have understood the tone of your post correctly it seems you don't
really wish to collaborate on the streaming driver work for either RME or
MOTU, nor engage in a constructive discussion about the finer details of
these interfaces which I have deduced, observed and worked around over the
past 8 years.  Instead your preference seems to be to work on this yourself
without external input.  If this is the case, then while it disappoints me
I'm fine with that; life is too short to get upset about things like this.

As I have stated publicly on various occasions, it has been my intent to
start porting the RME and MOTU streaming drivers to the kernel for the last
12-18 months.  Unfortunately, while I have developed some ideas about how
this might be done, real life and family responsibilities have prevented me
from doing the coding.  Clearly you have far more time than I and can make a
start earlier than me so it makes sense for you to move on this - that's how
it works in the FOSS world.  However, I am sadened that you don't wish to
discuss the development with people who have been working with these
interfaces for many years.

When writing these drivers, please include acknowledgements to the people
who are responsible for the community's knowledge about these interfaces, as
currently embodied in the FFADO source and documentation.  This knowledge is
the result of many hundreds of hours work over eight years, and without the
FFADO source and documentation it would be impossible or very difficult to
write these remaining kernel streaming drivers.  The MOTU protocol discovery
was done entirely by myself on the Traveler, with adjustments for the other
devices being subsequently deduced between myself and owners of the
respective interfaces.  Determination of the RME programming details was
done by myself with some input from RME themselves.

I am pleased that there is ongoing work to move the FFADO audio streaming
functionality into the kernel by new developers who are able to spend
significant amounts of time on the effort over a relatively short period of
time.  It is just unfortunate that the knowledge, experience and views of
existing developers are being abruptly dismissed.

Regards
  jonathan

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-20  2:09               ` Takashi Sakamoto
@ 2015-10-20  2:57                 ` Jonathan Woithe
  0 siblings, 0 replies; 26+ messages in thread
From: Jonathan Woithe @ 2015-10-20  2:57 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Stefan Richter, clemens, ffado-devel

On Tue, Oct 20, 2015 at 11:09:16AM +0900, Takashi Sakamoto wrote:
> On Oct 20 2015 09:50, Takashi Sakamoto wrote:
> >>Having said that, I think it is a natural evolution for the next generation
> >>of control/mixer programs to be developed under the FFADO umbrella.
> >
> >I prefer imagination to subsistence for discussion. Especially, 'A is
> >better but actually where A is and who works for A' is meaningless for
> >discussion.
> 
> Oops. I mistook to write English text.
> 
> 'I prefer substance to imagination for discussion'.

Thanks, that makes sense.

I appreciate how difficult it is to communicate coherently in a second
language, given that doing so is a skill that I was not able to develop
despite studying Japanese at school for four years.  By all accounts,
English is one of the hardest languages to pick up, so you're doing very
well.

Regards
  jonathan

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-20  0:50             ` Takashi Sakamoto
  2015-10-20  2:09               ` Takashi Sakamoto
  2015-10-20  2:52               ` Jonathan Woithe
@ 2015-10-20  7:39               ` Stefan Richter
  2015-10-26 15:18                 ` Takashi Sakamoto
  2 siblings, 1 reply; 26+ messages in thread
From: Stefan Richter @ 2015-10-20  7:39 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

On Oct 20 Takashi Sakamoto wrote:
> When using polkit correctly, I guess users doesn't need to join in
> 'audio' group, so as PulseAudio achieved with polkit.

With regard to access to /dev/fw* files, this is true with the existing
FFADO rules too.  60-ffado.rules sets ENV{ID_FFADO}="1", and
consolekit's 70-udev-acl.rules recognizes ID_FFADO and runs udev-acl on
the device.

(I.e. the current "console" owner is granted access to the character
device file via access control list (ACL), which is a mechanism in
parallel to Unix permission flags.)

The console owner policy and ACL mechanism are not a complete replacement
for the group mechanism though:
  - There may be headless systems and other occasions at which the audio
    user is not console owner.
  - Processes involved in capture or playback, i.e. applications beyond
    mixers, may require realtime scheduling class privilege and memlocking
    privilege, which are traditionally configured for Unix groups and
    users (typically for a group).  Not sure whether a mechanism exists
    which can implement a console owner policy for realtime and memlock
    privileges.
-- 
Stefan Richter
-=====-===== =-=- =-=--
http://arcgraph.de/sr/

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-20  7:39               ` Stefan Richter
@ 2015-10-26 15:18                 ` Takashi Sakamoto
  2015-10-27  1:38                   ` Stefan Richter
  0 siblings, 1 reply; 26+ messages in thread
From: Takashi Sakamoto @ 2015-10-26 15:18 UTC (permalink / raw)
  To: Stefan Richter; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

Hi Stefan,

On Oct 20 2015 16:39, Stefan Richter wrote:
> On Oct 20 Takashi Sakamoto wrote:
>> When using polkit correctly, I guess users doesn't need to join in
>> 'audio' group, so as PulseAudio achieved with polkit.

(PulseAudio did remove usage of polkit at 2008. Currently, its access to
sound character devices is granted according to permissions added by ACL.)

> With regard to access to /dev/fw* files, this is true with the existing
> FFADO rules too.  60-ffado.rules sets ENV{ID_FFADO}="1", and
> consolekit's 70-udev-acl.rules recognizes ID_FFADO and runs udev-acl on
> the device.
> 
> (I.e. the current "console" owner is granted access to the character
> device file via access control list (ACL), which is a mechanism in
> parallel to Unix permission flags.)

In systemd era, consolekit is obsoleted by systemd-logind, and udevd
calls a functionality of systemd-logind to add enough ACL for uaccess entry.

> The console owner policy and ACL mechanism are not a complete replacement
> for the group mechanism though:
>   - There may be headless systems and other occasions at which the audio
>     user is not console owner.

I think usual user doesn't use such systems. Here, I imagine the user
works with major Linux discribution such as Ubuntu.

>   - Processes involved in capture or playback, i.e. applications beyond
>     mixers, may require realtime scheduling class privilege and memlocking
>     privilege, which are traditionally configured for Unix groups and
>     users (typically for a group).  Not sure whether a mechanism exists
>     which can implement a console owner policy for realtime and memlock
>     privileges.

Hm. About such applications which changes scheduling attributes of
tasks, the user intentionally run for his or her purpose. I think this
is not so usual cases which the system should support as a default.

I think it better to continue discussion about enough ACL, regardless of
'audio' group. In short, when connecting audio and music units on IEEE
1394 bus, login users have enough permission to corresponding firewire
character devices, then they can execute any I/O operatins to the
devices. This is not achieved yet.

For example, in this developing period, I added new module to support
TASCAM FW1082/1884. When connecting these models, currently, login users
are not grant. Root user is just granted.

There's another issue. In this time, I also added TASCAM FireOne support
to ALSA OXFW driver. Any login users are granted to access the
corresponding character device, while the device node belongs to 'video'
user. I think this line causes this issue:
https://github.com/systemd/systemd/blob/c379f143a5ccdbc94a87cfca0174e7f21fa05f26/rules/50-udev-default.rules#L43

The model has 0x00a02d for 'specifier_id' in its unit directory of
CONFIG ROM. This may match the line.


Well, I think this is a better start point for this discussion:
 * Any login users should be granted to access firewire character
   devices for audio and music units on IEEE 1394 bus, with helps of
   consolekit or systemd-logind. The users can run any apprications
   to execute IO operation to these devices without additional
   configurations.
 * When users want to start applications requiring special privillege,
   the users should add configurations, i.e. for PAM. In this case,
   typically, users join in 'audio' group and add the configurations to
   'audio' group.



Thanks

Takashi Sakamoto

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

* Re: [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality
  2015-10-26 15:18                 ` Takashi Sakamoto
@ 2015-10-27  1:38                   ` Stefan Richter
  0 siblings, 0 replies; 26+ messages in thread
From: Stefan Richter @ 2015-10-27  1:38 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, Jonathan Woithe, clemens, ffado-devel

On Oct 27 Takashi Sakamoto wrote:
> On Oct 20 2015 16:39, Stefan Richter wrote:
> > The console owner policy and ACL mechanism are not a complete replacement
> > for the group mechanism though:
> >   - There may be headless systems and other occasions at which the audio
> >     user is not console owner.
> 
> I think usual user doesn't use such systems. Here, I imagine the user
> works with major Linux discribution such as Ubuntu.
> 
> >   - Processes involved in capture or playback, i.e. applications beyond
> >     mixers, may require realtime scheduling class privilege and memlocking
> >     privilege, which are traditionally configured for Unix groups and
> >     users (typically for a group).  Not sure whether a mechanism exists
> >     which can implement a console owner policy for realtime and memlock
> >     privileges.
> 
> Hm. About such applications which changes scheduling attributes of
> tasks, the user intentionally run for his or her purpose. I think this
> is not so usual cases which the system should support as a default.

I mentioned the two points mostly as an illustration for pros and cons of
Unix groups mechanism/ policy in comparison to ACL mechanism/ console
owner policy.

If one of us submits new rules for inclusion into mainline systemd git, I
assume that the udev rules maintainers of the systemd project will direct
us as necessary, so that good default mechanisms and policies are
implemented.

[...]
> There's another issue. In this time, I also added TASCAM FireOne support
> to ALSA OXFW driver. Any login users are granted to access the
> corresponding character device, while the device node belongs to 'video'
> user. I think this line causes this issue:
> https://github.com/systemd/systemd/blob/c379f143a5ccdbc94a87cfca0174e7f21fa05f26/rules/50-udev-default.rules#L43
> 
> The model has 0x00a02d for 'specifier_id' in its unit directory of
> CONFIG ROM. This may match the line.

In earlier times, some or all of these lines had comments that explained
what is what. The comments were removed eventually.  Here is what each
Specifier and Version (i.e. protocol) identifiers means (see also bottom of
http://1394ta.org/1394-test-resources/):

SUBSYSTEM=="firewire", ATTR{units}=="*0x00a02d:0x00010*", GROUP="video"
	specifier: 1394 TA
	protocol: IIDC (Instrumentation & Industrial Digital Camera)

SUBSYSTEM=="firewire", ATTR{units}=="*0x00b09d:0x00010*", GROUP="video"
	specifier: Point Grey Research, Inc.
	protocol: IIDC
	(actually plain IIDC as specified by 1394 TA, but AFAIK the
	nonstandard specifier ID is there to get the vendor's driver
	loaded on other OSs)

SUBSYSTEM=="firewire", ATTR{units}=="*0x00a02d:0x010001*", GROUP="video"
	specifier: 1394 TA
	protocol: AV/C

SUBSYSTEM=="firewire", ATTR{units}=="*0x00a02d:0x014001*", GROUP="video"
	specifier: 1394 TA
	protocol: AV/C and vendor-unique

Note, the "*" asterisks in these patterns serve two purposes:  First, a
node with several units will expose a space-separated list of
specifier:version tuples in the units attribute.  Second, the version ID
is always 24 bits wide, but we may want to match just the uppermost 20 or
fewer bits of the version.

BTW, the "units" attribute and the others are described in
Documentation/ABI/stable/sysfs-bus-firewire.  But here is an example from
a device with four units:
$ cat /sys/bus/firewire/devices/fw13/*{name,units}
iSight
Apple Computer, Inc.
0x00a02d:0x000102 0x000a27:0x000010 0x000a27:0x000011 0x000a27:0x000012
(IIDC v1.30, iSight audio, iSight factory, iSight iris units)

As you know, firewire-core merely discovers nodes and units (including AV/C
units), but not subdevices below them, such as SBP logical units or AV/C
subunits.  Consequently, firewire-core does not expose a sysfs attribute
which describes the AV/C subunit type (monitor, audio, disc, tape, tuner,
camera, music, and more).  The first udev rules for FireWire devices were
written in times when camcorders and other DV devices were considered more
numerous in use than audio AV/C devices.  Hence the choice of GROUP="video"
which has been carried over from that time.  In addition, many
distributions
at the time added new user accounts to a "video" group by default, but
fewer distributions did the same with an "audio" group.

Do we need to add AV/C subunit discovery to firewire-core, in order to add
AV/C subunit type attributes to sysfs?  Perhaps not, if we can come up with
udev rules for audio devices which work off existing attributes like those
which the audio kernel drivers themselves match by their mod-alias.  We
need to restrict ourselves to "node device attributes"
in /sys/bus/firewire/devices/fw[0-9]+/ though.  The "unit device
attributes" in /sys/bus/firewire/devices/fw[0-9]+[.][0-9]+/ cannot be used
because they are populated in later uevents than the one which
triggers creation of the /dev/fw* file.
-- 
Stefan Richter
-=====-===== =-=- ==-==
http://arcgraph.de/sr/

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

end of thread, other threads:[~2015-10-27  1:38 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 10:10 [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Sakamoto
2015-10-12 10:10 ` [PATCH 1/5] ALSA: firewire-tascam: add support for incoming MIDI messages by asynchronous transaction Takashi Sakamoto
2015-10-12 10:10 ` [PATCH 2/5] ALSA: firewire-tascam: add support for outgoing " Takashi Sakamoto
2015-10-12 10:10 ` [PATCH 3/5] ALSA: firewire-tascam: add support for MIDI functionality Takashi Sakamoto
2015-10-12 10:10 ` [PATCH 4/5] ALSA: firewire-tascam: Turn on/off FireWire LED Takashi Sakamoto
2015-10-12 10:10 ` [PATCH 5/5] ALSA: firewire-tascam: change device probing processing Takashi Sakamoto
2015-10-12 12:42   ` Stefan Richter
2015-10-13 14:12     ` Takashi Sakamoto
2015-10-12 12:21 ` [PATCH 0/5] ALSA: firewire-tascam: add MIDI functionality Takashi Iwai
2015-10-12 12:48 ` Stefan Richter
2015-10-12 22:20   ` Jonathan Woithe
2015-10-13  9:36     ` Takashi Sakamoto
2015-10-13 10:02       ` Jonathan Woithe
2015-10-13 22:20         ` Stefan Richter
2015-10-19 14:21         ` Takashi Sakamoto
2015-10-19 23:45           ` Jonathan Woithe
2015-10-13 14:15       ` Stefan Richter
2015-10-19 14:13         ` Takashi Sakamoto
2015-10-19 23:36           ` Jonathan Woithe
2015-10-20  0:50             ` Takashi Sakamoto
2015-10-20  2:09               ` Takashi Sakamoto
2015-10-20  2:57                 ` Jonathan Woithe
2015-10-20  2:52               ` Jonathan Woithe
2015-10-20  7:39               ` Stefan Richter
2015-10-26 15:18                 ` Takashi Sakamoto
2015-10-27  1:38                   ` Stefan Richter

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