[PATCH 0/5] ALSA: add helper functions for MIDI message transmission on IEEE 1394 asynchronous transaction

[PATCH 0/5] ALSA: add helper functions for MIDI message transmission on IEEE 1394 asynchronous transaction

[Takashi Sakamoto]

Hi,

This patchset updates a part of my former post (patch 03-07):

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

This patchset adds some helper functions for MIDI transmission via IEEE 1394
asynchronous transaction.

Changes:
 * Use workqueue instead of tasklet for better usage of system resources.
 * Use ktime_t instead of jiffies for better resolution of time counting.
 * Some minor fixes.


Takashi Sakamoto (5):
  ALSA: firewire-lib: add helper functions for asynchronous transactions
    to transfer MIDI messages
  ALSA: firewire-lib: add a restriction for a transaction at once
  ALSA: firewire-lib: schedule work again when MIDI substream has rest
    of MIDI messages
  ALSA: firewire-lib: add throttle for MIDI data rate
  ALSA: firewire-lib: avoid endless loop to transfer MIDI messages at
    fatal error

 sound/firewire/lib.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++
 sound/firewire/lib.h |  56 +++++++++++++++++++++
 2 files changed, 194 insertions(+)

-- 
2.1.4

[PATCH 1/5] ALSA: firewire-lib: add helper functions for asynchronous transactions to transfer MIDI messages

[Takashi Sakamoto]

Some models receive MIDI messages via IEEE 1394 asynchronous transactions.
In this case, MIDI messages are transferred in fixed-length payload. It's
nice that firewire-lib module has common helper functions.

This commit implements this idea. Each driver adds
'struct snd_fw_async_midi_port' in its instance structure. In probing,
it should call snd_fw_async_midi_port_init() to initialize the
structure with some parameters such as target address, the length
of payload in a transaction and a pointer for callback function
to fill the payload buffer. At 'struct snd_rawmidi_ops.trigger()'
callback, it should call 'snd_fw_async_midi_port_run()' to start
transactions. Each driver should ensure that the lifetime of MIDI
substream continues till calling 'snd_fw_async_midi_port_finish()'.

The helper functions support retries to transferring MIDI messages when
transmission errors occur. When transactions are successful, the helper
functions call 'snd_rawmidi_transmit_ack()' internally to consume MIDI
bytes in the buffer. Therefore, Each driver is expected to use
'snd_rawmidi_transmit_peek()' to tell the number of bytes to transfer in
return value of 'fill' callback.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++
 sound/firewire/lib.h |  50 +++++++++++++++++++++++++
 2 files changed, 153 insertions(+)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 7409edb..03ada3f 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/firewire.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 #include "lib.h"
 
 #define ERROR_RETRY_DELAY_MS	20
@@ -66,6 +67,108 @@ int snd_fw_transaction(struct fw_unit *unit, int tcode,
 }
 EXPORT_SYMBOL(snd_fw_transaction);
 
+static void async_midi_port_callback(struct fw_card *card, int rcode,
+				     void *data, size_t length,
+				     void *callback_data)
+{
+	struct snd_fw_async_midi_port *port = callback_data;
+	struct snd_rawmidi_substream *substream = ACCESS_ONCE(port->substream);
+
+	if (rcode == RCODE_COMPLETE && substream != NULL)
+		snd_rawmidi_transmit_ack(substream, port->consume_bytes);
+}
+
+static void midi_port_work(struct work_struct *work)
+{
+	struct snd_fw_async_midi_port *port =
+			container_of(work, struct snd_fw_async_midi_port, work);
+	struct snd_rawmidi_substream *substream = ACCESS_ONCE(port->substream);
+	int generation;
+	int type;
+
+	/* Nothing to do. */
+	if (substream == NULL || snd_rawmidi_transmit_empty(substream))
+		return;
+
+	/*
+	 * Fill the buffer. The callee must use snd_rawmidi_transmit_peek().
+	 * Later, snd_rawmidi_transmit_ack() is called.
+	 */
+	memset(port->buf, 0, port->len);
+	port->consume_bytes = port->fill(substream, port->buf);
+	if (port->consume_bytes <= 0) {
+		/* Do it in next chance, immediately. */
+		if (port->consume_bytes == 0)
+			schedule_work(&port->work);
+		return;
+	}
+
+	/* Calculate type of transaction. */
+	if (port->len == 4)
+		type = TCODE_WRITE_QUADLET_REQUEST;
+	else
+		type = TCODE_WRITE_BLOCK_REQUEST;
+
+	/* Start this transaction. */
+	/*
+	 * In Linux FireWire core, when generation is updated with memory
+	 * barrier, node id has already been updated. In this module, After
+	 * this smp_rmb(), load/store instructions to memory are completed.
+	 * Thus, both of generation and node id are available with recent
+	 * values. This is a light-serialization solution to handle bus reset
+	 * events on IEEE 1394 bus.
+	 */
+	generation = port->parent->generation;
+	smp_rmb();
+
+	fw_send_request(port->parent->card, &port->transaction, type,
+			port->parent->node_id, generation,
+			port->parent->max_speed, port->addr,
+			port->buf, port->len, async_midi_port_callback,
+			port);
+}
+
+/**
+ * snd_fw_async_midi_port_init - initialize asynchronous MIDI port structure
+ * @port: the asynchronous MIDI port to initialize
+ * @unit: the target of the asynchronous transaction
+ * @addr: the address to which transactions are transferred
+ * @len: the length of transaction
+ * @fill: the callback function to fill given buffer, and returns the
+ *	       number of consumed bytes for MIDI message.
+ *
+ */
+int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
+		struct fw_unit *unit, u64 addr, unsigned int len,
+		snd_fw_async_midi_port_fill fill)
+{
+	port->len = DIV_ROUND_UP(len, 4) * 4;
+	port->buf = kzalloc(port->len, GFP_KERNEL);
+	if (port->buf == NULL)
+		return -ENOMEM;
+
+	port->parent = fw_parent_device(unit);
+	port->addr = addr;
+	port->fill = fill;
+
+	INIT_WORK(&port->work, midi_port_work);
+
+	return 0;
+}
+EXPORT_SYMBOL(snd_fw_async_midi_port_init);
+
+/**
+ * snd_fw_async_midi_port_destroy - free asynchronous MIDI port structure
+ * @port: the asynchronous MIDI port structure
+ */
+void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port)
+{
+	snd_fw_async_midi_port_finish(port);
+	cancel_work_sync(&port->work);
+	kfree(port->buf);
+}
+EXPORT_SYMBOL(snd_fw_async_midi_port_destroy);
+
 MODULE_DESCRIPTION("FireWire audio helper functions");
 MODULE_AUTHOR("Clemens Ladisch <clemens@ladisch.de>");
 MODULE_LICENSE("GPL v2");
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 02cfabc..37a7fe4 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -3,6 +3,8 @@
 
 #include <linux/firewire-constants.h>
 #include <linux/types.h>
+#include <linux/sched.h>
+#include <sound/rawmidi.h>
 
 struct fw_unit;
 
@@ -20,4 +22,52 @@ static inline bool rcode_is_permanent_error(int rcode)
 	return rcode == RCODE_TYPE_ERROR || rcode == RCODE_ADDRESS_ERROR;
 }
 
+struct snd_fw_async_midi_port;
+typedef int (*snd_fw_async_midi_port_fill)(
+				struct snd_rawmidi_substream *substream,
+				u8 *buf);
+
+struct snd_fw_async_midi_port {
+	struct fw_device *parent;
+	struct work_struct work;
+
+	u64 addr;
+	struct fw_transaction transaction;
+
+	u8 *buf;
+	unsigned int len;
+
+	struct snd_rawmidi_substream *substream;
+	snd_fw_async_midi_port_fill fill;
+	unsigned int consume_bytes;
+};
+
+int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
+		struct fw_unit *unit, u64 addr, unsigned int len,
+		snd_fw_async_midi_port_fill fill);
+void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port);
+
+/**
+ * snd_fw_async_midi_port_run - run transactions for the async MIDI port
+ * @port: the asynchronous MIDI port
+ * @substream: the MIDI substream
+ */
+static inline void
+snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
+			   struct snd_rawmidi_substream *substream)
+{
+	port->substream = substream;
+	schedule_work(&port->work);
+}
+
+/**
+ * snd_fw_async_midi_port_finish - finish the asynchronous MIDI port
+ * @port: the asynchronous MIDI port
+ */
+static inline void
+snd_fw_async_midi_port_finish(struct snd_fw_async_midi_port *port)
+{
+	port->substream = NULL;
+}
+
 #endif
-- 
2.1.4

[PATCH 2/5] ALSA: firewire-lib: add a restriction for a transaction at once

[Takashi Sakamoto]

Currently, when waiting for a response, callers can start another
transaction by scheduling another work. This is not good for error
processing of transaction, especially the first response is too late.

This commit serialize request/response transactions, by adding one
boolean member to represent idling state.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 9 +++++++++
 sound/firewire/lib.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 03ada3f..ddc3e88 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -76,6 +76,8 @@ static void async_midi_port_callback(struct fw_card *card, int rcode,
 
 	if (rcode == RCODE_COMPLETE && substream != NULL)
 		snd_rawmidi_transmit_ack(substream, port->consume_bytes);
+
+	port->idling = true;
 }
 
 static void midi_port_work(struct work_struct *work)
@@ -86,6 +88,10 @@ static void midi_port_work(struct work_struct *work)
 	int generation;
 	int type;
 
+	/* Under transacting. */
+	if (!port->idling)
+		return;
+
 	/* Nothing to do. */
 	if (substream == NULL || snd_rawmidi_transmit_empty(substream))
 		return;
@@ -110,6 +116,8 @@ static void midi_port_work(struct work_struct *work)
 		type = TCODE_WRITE_BLOCK_REQUEST;
 
 	/* Start this transaction. */
+	port->idling = false;
+
 	/*
 	 * In Linux FireWire core, when generation is updated with memory
 	 * barrier, node id has already been updated. In this module, After
@@ -150,6 +158,7 @@ int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 	port->parent = fw_parent_device(unit);
 	port->addr = addr;
 	port->fill = fill;
+	port->idling = true;
 
 	INIT_WORK(&port->work, midi_port_work);
 
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 37a7fe4..0af06f4 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -30,6 +30,7 @@ typedef int (*snd_fw_async_midi_port_fill)(
 struct snd_fw_async_midi_port {
 	struct fw_device *parent;
 	struct work_struct work;
+	bool idling;
 
 	u64 addr;
 	struct fw_transaction transaction;
-- 
2.1.4

[PATCH 3/5] ALSA: firewire-lib: schedule work again when MIDI substream has rest of MIDI messages

[Takashi Sakamoto]

Currently, when two MIDI trigger callbacks are called immediately,
transactions for the second MIDI messages are postpone till next trigger
callback. This is not good for real-time message transmission.

This commit schedules work again at response handling callback if the
MIDI substream still includes untransferred MIDI messages.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index ddc3e88..3e9afd7 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -78,6 +78,9 @@ static void async_midi_port_callback(struct fw_card *card, int rcode,
 		snd_rawmidi_transmit_ack(substream, port->consume_bytes);
 
 	port->idling = true;
+
+	if (!snd_rawmidi_transmit_empty(substream))
+		schedule_work(&port->work);
 }
 
 static void midi_port_work(struct work_struct *work)
-- 
2.1.4

[PATCH 4/5] ALSA: firewire-lib: add throttle for MIDI data rate

[Takashi Sakamoto]

Typically, the target devices have internal buffer to adjust output of
received MIDI messages for MIDI serial bus, while the capacity of the
buffer is limited. IEEE 1394 transactions can transfer more MIDI messages
than MIDI serial bus can. This can cause buffer over flow in device side.

This commit adds throttle to limit MIDI data rate by counting intervals
by jiffies between two MIDI messages. Usual MIDI messages consists of two
or three bytes. This requires 1.302 to 1.953 mili-seconds interval between
these messages. This commit uses kernel monotonic time service to calculate
the time of next transaction.

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

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 3e9afd7..fd620e8 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -76,6 +76,9 @@ static void async_midi_port_callback(struct fw_card *card, int rcode,
 
 	if (rcode == RCODE_COMPLETE && substream != NULL)
 		snd_rawmidi_transmit_ack(substream, port->consume_bytes);
+	else if (!rcode_is_permanent_error(rcode))
+		/* To start next transaction immediately for recovery. */
+		port->next_ktime = ktime_set(0, 0);
 
 	port->idling = true;
 
@@ -99,6 +102,12 @@ static void midi_port_work(struct work_struct *work)
 	if (substream == NULL || snd_rawmidi_transmit_empty(substream))
 		return;
 
+	/* Do it in next chance. */
+	if (ktime_after(port->next_ktime, ktime_get())) {
+		schedule_work(&port->work);
+		return;
+	}
+
 	/*
 	 * Fill the buffer. The callee must use snd_rawmidi_transmit_peek().
 	 * Later, snd_rawmidi_transmit_ack() is called.
@@ -107,8 +116,10 @@ static void midi_port_work(struct work_struct *work)
 	port->consume_bytes = port->fill(substream, port->buf);
 	if (port->consume_bytes <= 0) {
 		/* Do it in next chance, immediately. */
-		if (port->consume_bytes == 0)
+		if (port->consume_bytes == 0) {
+			port->next_ktime = ktime_set(0, 0);
 			schedule_work(&port->work);
+		}
 		return;
 	}
 
@@ -118,6 +129,10 @@ static void midi_port_work(struct work_struct *work)
 	else
 		type = TCODE_WRITE_BLOCK_REQUEST;
 
+	/* Set interval to next transaction. */
+	port->next_ktime = ktime_add_ns(ktime_get(),
+				port->consume_bytes * 8 * NSEC_PER_SEC / 31250);
+
 	/* Start this transaction. */
 	port->idling = false;
 
@@ -162,6 +177,8 @@ int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 	port->addr = addr;
 	port->fill = fill;
 	port->idling = true;
+	port->next_ktime = ktime_set(0, 0);
+	port->error = false;
 
 	INIT_WORK(&port->work, midi_port_work);
 
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 0af06f4..59e0865 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -31,6 +31,7 @@ struct snd_fw_async_midi_port {
 	struct fw_device *parent;
 	struct work_struct work;
 	bool idling;
+	ktime_t next_ktime;
 
 	u64 addr;
 	struct fw_transaction transaction;
-- 
2.1.4

[PATCH 5/5] ALSA: firewire-lib: avoid endless loop to transfer MIDI messages at fatal error

[Takashi Sakamoto]

Currently, when asynchronous transactions finish in error state and
retries, work scheduling and work running also continues. This
should be canceled at fatal error because it can cause endless loop.

This commit enables to cancel transferring MIDI messages when transactions
encounter fatal errors. This is achieved by setting error state.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 10 ++++++++--
 sound/firewire/lib.h |  8 ++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index fd620e8..edf1c8b 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -79,6 +79,9 @@ static void async_midi_port_callback(struct fw_card *card, int rcode,
 	else if (!rcode_is_permanent_error(rcode))
 		/* To start next transaction immediately for recovery. */
 		port->next_ktime = ktime_set(0, 0);
+	else
+		/* Don't continue processing. */
+		port->error = true;
 
 	port->idling = true;
 
@@ -94,8 +97,8 @@ static void midi_port_work(struct work_struct *work)
 	int generation;
 	int type;
 
-	/* Under transacting. */
-	if (!port->idling)
+	/* Under transacting or error state. */
+	if (!port->idling || port->error)
 		return;
 
 	/* Nothing to do. */
@@ -119,6 +122,9 @@ static void midi_port_work(struct work_struct *work)
 		if (port->consume_bytes == 0) {
 			port->next_ktime = ktime_set(0, 0);
 			schedule_work(&port->work);
+		} else {
+			/* Fatal error. */
+			port->error = true;
 		}
 		return;
 	}
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 59e0865..f3f6f84 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -32,6 +32,7 @@ struct snd_fw_async_midi_port {
 	struct work_struct work;
 	bool idling;
 	ktime_t next_ktime;
+	bool error;
 
 	u64 addr;
 	struct fw_transaction transaction;
@@ -58,8 +59,10 @@ static inline void
 snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
 			   struct snd_rawmidi_substream *substream)
 {
-	port->substream = substream;
-	schedule_work(&port->work);
+	if (!port->error) {
+		port->substream = substream;
+		schedule_work(&port->work);
+	}
 }
 
 /**
@@ -70,6 +73,7 @@ static inline void
 snd_fw_async_midi_port_finish(struct snd_fw_async_midi_port *port)
 {
 	port->substream = NULL;
+	port->error = false;
 }
 
 #endif
-- 
2.1.4

[PATCH 5/5] ALSA: firewire-lib: avoid endless loop to transfer MIDI messages at fatal error

[Takashi Sakamoto]

Currently, when asynchronous transactions finish in error state and
retries, work scheduling and work running also continues. This
should be canceled at fatal error because it can cause endless loop.

This commit enables to cancel transferring MIDI messages when transactions
encounter fatal errors. This is achieved by setting error state.

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

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 9a98c7c..edf1c8b 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -79,6 +79,9 @@ static void async_midi_port_callback(struct fw_card *card, int rcode,
 	else if (!rcode_is_permanent_error(rcode))
 		/* To start next transaction immediately for recovery. */
 		port->next_ktime = ktime_set(0, 0);
+	else
+		/* Don't continue processing. */
+		port->error = true;
 
 	port->idling = true;
 
@@ -94,8 +97,8 @@ static void midi_port_work(struct work_struct *work)
 	int generation;
 	int type;
 
-	/* Under transacting. */
-	if (!port->idling)
+	/* Under transacting or error state. */
+	if (!port->idling || port->error)
 		return;
 
 	/* Nothing to do. */
@@ -119,6 +122,9 @@ static void midi_port_work(struct work_struct *work)
 		if (port->consume_bytes == 0) {
 			port->next_ktime = ktime_set(0, 0);
 			schedule_work(&port->work);
+		} else {
+			/* Fatal error. */
+			port->error = true;
 		}
 		return;
 	}
@@ -178,6 +184,7 @@ int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 	port->fill = fill;
 	port->idling = true;
 	port->next_ktime = ktime_set(0, 0);
+	port->error = false;
 
 	INIT_WORK(&port->work, midi_port_work);
 
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index 59e0865..f3f6f84 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -32,6 +32,7 @@ struct snd_fw_async_midi_port {
 	struct work_struct work;
 	bool idling;
 	ktime_t next_ktime;
+	bool error;
 
 	u64 addr;
 	struct fw_transaction transaction;
@@ -58,8 +59,10 @@ static inline void
 snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
 			   struct snd_rawmidi_substream *substream)
 {
-	port->substream = substream;
-	schedule_work(&port->work);
+	if (!port->error) {
+		port->substream = substream;
+		schedule_work(&port->work);
+	}
 }
 
 /**
@@ -70,6 +73,7 @@ static inline void
 snd_fw_async_midi_port_finish(struct snd_fw_async_midi_port *port)
 {
 	port->substream = NULL;
+	port->error = false;
 }
 
 #endif
-- 
2.1.4