[PATCH 0/8] ALSA: firewire-lib/firewire-tascam: localize async midi port

[PATCH 0/8] ALSA: firewire-lib/firewire-tascam: localize async midi port

[Takashi Sakamoto]

Hi,

At a development period for Linux v4.4, firewire-lib module got an
abstraction of asynchronous transaction for transmission of MIDI message.
I did this with an assumption that this is required for Digi00x series, as
well as TASCAM FireWire series. However, my recent work reveal it's
actually not[0].

This patchset localizes the abstraction just for TASCAM FireWire series.

[0] [alsa-devel] [PATCH 0/4] ALSA: firewire-digi00x: integration to support console models
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-April/119397.html

Takashi Sakamoto (8):
  firewire-lib/firewire-tascam: localize async midi port
  firewire-tascam: remove callback function from async midi port
  firewire-tascam: send fixed-length transaction for async midi port
  firewire-tascam: use the same address for asynchronous transaction for
    MIDI message
  firewire-tascam: use fixed-length array for message cache to async
    midi port
  firewire-tascam: initialize parameters at open of rawmidi character
    devices
  firewire-tascam: move message parameters for async midi port
  firewire-tascam: support drain callback for MIDI playback substream

 sound/firewire/lib.c                       | 141 ----------------------------
 sound/firewire/lib.h                       |  54 -----------
 sound/firewire/tascam/tascam-midi.c        |  13 ++-
 sound/firewire/tascam/tascam-transaction.c | 142 ++++++++++++++++++++++++-----
 sound/firewire/tascam/tascam.h             |  39 +++++++-
 5 files changed, 162 insertions(+), 227 deletions(-)

-- 
2.9.3

[PATCH 1/8] firewire-lib/firewire-tascam: localize async midi port

[Takashi Sakamoto]

In Linux kernel 4.4, firewire-lib got a feature called as 'async midi port'
for transmission of MIDI message via IEEE 1394 asynchronous communication,
however actual consumer of this feature is ALSA driver for TASCAM FireWire
series only. When adding this feature, I assumed that ALSA driver for
Digi00x might also be a consumer, actually it's not.

This commit moves the feature from firewire-lib to firewire-tascam module.
Two minor kernel APIs are removed.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c                       | 141 -----------------------------
 sound/firewire/lib.h                       |  54 -----------
 sound/firewire/tascam/tascam-transaction.c | 125 +++++++++++++++++++++++++
 sound/firewire/tascam/tascam.h             |  45 +++++++++
 4 files changed, 170 insertions(+), 195 deletions(-)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 7683238..39dfa74 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -99,147 +99,6 @@ void snd_fw_schedule_registration(struct fw_unit *unit,
 }
 EXPORT_SYMBOL(snd_fw_schedule_registration);
 
-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);
-
-	/* This port is closed. */
-	if (substream == NULL)
-		return;
-
-	if (rcode == RCODE_COMPLETE)
-		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 = 0;
-	else
-		/* Don't continue processing. */
-		port->error = true;
-
-	port->idling = true;
-
-	if (!snd_rawmidi_transmit_empty(substream))
-		schedule_work(&port->work);
-}
-
-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;
-
-	/* Under transacting or error state. */
-	if (!port->idling || port->error)
-		return;
-
-	/* Nothing to do. */
-	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.
-	 */
-	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) {
-			port->next_ktime = 0;
-			schedule_work(&port->work);
-		} else {
-			/* Fatal error. */
-			port->error = true;
-		}
-		return;
-	}
-
-	/* Calculate type of transaction. */
-	if (port->len == 4)
-		type = TCODE_WRITE_QUADLET_REQUEST;
-	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;
-
-	/*
-	 * 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;
-	port->idling = true;
-	port->next_ktime = 0;
-	port->error = false;
-
-	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 f676931..eef7092 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -25,58 +25,4 @@ static inline bool rcode_is_permanent_error(int rcode)
 void snd_fw_schedule_registration(struct fw_unit *unit,
 				  struct delayed_work *dwork);
 
-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;
-	bool idling;
-	ktime_t next_ktime;
-	bool error;
-
-	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)
-{
-	if (!port->error) {
-		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;
-	port->error = false;
-}
-
 #endif
diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 040a96d..8ba006e 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -144,6 +144,131 @@ static int fill_message(struct snd_rawmidi_substream *substream, u8 *buf)
 	return consume;
 }
 
+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);
+
+	/* This port is closed. */
+	if (substream == NULL)
+		return;
+
+	if (rcode == RCODE_COMPLETE)
+		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 = 0;
+	else
+		/* Don't continue processing. */
+		port->error = true;
+
+	port->idling = true;
+
+	if (!snd_rawmidi_transmit_empty(substream))
+		schedule_work(&port->work);
+}
+
+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;
+
+	/* Under transacting or error state. */
+	if (!port->idling || port->error)
+		return;
+
+	/* Nothing to do. */
+	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.
+	 */
+	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) {
+			port->next_ktime = 0;
+			schedule_work(&port->work);
+		} else {
+			/* Fatal error. */
+			port->error = true;
+		}
+		return;
+	}
+
+	/* Calculate type of transaction. */
+	if (port->len == 4)
+		type = TCODE_WRITE_QUADLET_REQUEST;
+	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;
+
+	/*
+	 * 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);
+}
+
+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;
+	port->idling = true;
+	port->next_ktime = 0;
+	port->error = false;
+
+	INIT_WORK(&port->work, midi_port_work);
+
+	return 0;
+}
+
+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);
+}
+
 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,
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index d3cd406..d50adec 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -45,6 +45,29 @@ struct snd_tscm_spec {
 #define TSCM_MIDI_IN_PORT_MAX	4
 #define TSCM_MIDI_OUT_PORT_MAX	4
 
+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;
+	bool idling;
+	ktime_t next_ktime;
+	bool error;
+
+	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;
+};
+
 struct snd_tscm {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -131,6 +154,28 @@ 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_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);
+
+static inline void
+snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
+			   struct snd_rawmidi_substream *substream)
+{
+	if (!port->error) {
+		port->substream = substream;
+		schedule_work(&port->work);
+	}
+}
+
+static inline void
+snd_fw_async_midi_port_finish(struct snd_fw_async_midi_port *port)
+{
+	port->substream = NULL;
+	port->error = false;
+}
+
 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);
-- 
2.9.3

[PATCH 2/8] firewire-tascam: remove callback function from async midi port

[Takashi Sakamoto]

As a result of localization of async midi port, ALSA driver for TASCAM
FireWire series can call helper function directly instead of callback
registration.

This commit removes the redundant design.

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

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 8ba006e..dcbc003 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -197,7 +197,7 @@ static void midi_port_work(struct work_struct *work)
 	 * Later, snd_rawmidi_transmit_ack() is called.
 	 */
 	memset(port->buf, 0, port->len);
-	port->consume_bytes = port->fill(substream, port->buf);
+	port->consume_bytes = fill_message(substream, port->buf);
 	if (port->consume_bytes <= 0) {
 		/* Do it in next chance, immediately. */
 		if (port->consume_bytes == 0) {
@@ -242,8 +242,7 @@ static void midi_port_work(struct work_struct *work)
 }
 
 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)
+		struct fw_unit *unit, u64 addr, unsigned int len)
 {
 	port->len = DIV_ROUND_UP(len, 4) * 4;
 	port->buf = kzalloc(port->len, GFP_KERNEL);
@@ -252,7 +251,6 @@ 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;
 	port->next_ktime = 0;
 	port->error = false;
@@ -347,7 +345,7 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 		err = snd_fw_async_midi_port_init(
 				&tscm->out_ports[i], tscm->unit,
 				TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_RX_QUAD,
-				4, fill_message);
+				4);
 		if (err < 0)
 			goto error;
 	}
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index d50adec..08e626e 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -45,11 +45,6 @@ struct snd_tscm_spec {
 #define TSCM_MIDI_IN_PORT_MAX	4
 #define TSCM_MIDI_OUT_PORT_MAX	4
 
-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;
@@ -64,7 +59,6 @@ struct snd_fw_async_midi_port {
 	unsigned int len;
 
 	struct snd_rawmidi_substream *substream;
-	snd_fw_async_midi_port_fill fill;
 	unsigned int consume_bytes;
 };
 
@@ -155,8 +149,7 @@ int snd_tscm_stream_lock_try(struct snd_tscm *tscm);
 void snd_tscm_stream_lock_release(struct snd_tscm *tscm);
 
 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);
+		struct fw_unit *unit, u64 addr, unsigned int len);
 void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port);
 
 static inline void
-- 
2.9.3

[PATCH 3/8] firewire-tascam: send fixed-length transaction for async midi port

[Takashi Sakamoto]

TASCAM FireWire series uses asynchronous transactions with fixed length
payload for MIDI messaging. On the other hand, ALSA driver for the series
has a redundant design to handle different length of payload.

This commit removes the redundant abstraction.

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

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index dcbc003..2f5e20c 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -176,7 +176,6 @@ static void midi_port_work(struct work_struct *work)
 			container_of(work, struct snd_fw_async_midi_port, work);
 	struct snd_rawmidi_substream *substream = ACCESS_ONCE(port->substream);
 	int generation;
-	int type;
 
 	/* Under transacting or error state. */
 	if (!port->idling || port->error)
@@ -196,7 +195,7 @@ static void midi_port_work(struct work_struct *work)
 	 * Fill the buffer. The callee must use snd_rawmidi_transmit_peek().
 	 * Later, snd_rawmidi_transmit_ack() is called.
 	 */
-	memset(port->buf, 0, port->len);
+	memset(port->buf, 0, 4);
 	port->consume_bytes = fill_message(substream, port->buf);
 	if (port->consume_bytes <= 0) {
 		/* Do it in next chance, immediately. */
@@ -210,12 +209,6 @@ static void midi_port_work(struct work_struct *work)
 		return;
 	}
 
-	/* Calculate type of transaction. */
-	if (port->len == 4)
-		type = TCODE_WRITE_QUADLET_REQUEST;
-	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);
@@ -234,18 +227,18 @@ static void midi_port_work(struct work_struct *work)
 	generation = port->parent->generation;
 	smp_rmb();
 
-	fw_send_request(port->parent->card, &port->transaction, type,
+	fw_send_request(port->parent->card, &port->transaction,
+			TCODE_WRITE_QUADLET_REQUEST,
 			port->parent->node_id, generation,
 			port->parent->max_speed, port->addr,
-			port->buf, port->len, async_midi_port_callback,
+			port->buf, 4, async_midi_port_callback,
 			port);
 }
 
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit, u64 addr, unsigned int len)
+		struct fw_unit *unit, u64 addr)
 {
-	port->len = DIV_ROUND_UP(len, 4) * 4;
-	port->buf = kzalloc(port->len, GFP_KERNEL);
+	port->buf = kzalloc(4, GFP_KERNEL);
 	if (port->buf == NULL)
 		return -ENOMEM;
 
@@ -344,8 +337,7 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 	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);
+				TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_RX_QUAD);
 		if (err < 0)
 			goto error;
 	}
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index 08e626e..c1becc6 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -56,7 +56,6 @@ struct snd_fw_async_midi_port {
 	struct fw_transaction transaction;
 
 	u8 *buf;
-	unsigned int len;
 
 	struct snd_rawmidi_substream *substream;
 	unsigned int consume_bytes;
@@ -149,7 +148,7 @@ int snd_tscm_stream_lock_try(struct snd_tscm *tscm);
 void snd_tscm_stream_lock_release(struct snd_tscm *tscm);
 
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit, u64 addr, unsigned int len);
+		struct fw_unit *unit, u64 addr);
 void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port);
 
 static inline void
-- 
2.9.3

[PATCH 4/8] firewire-tascam: use the same address for asynchronous transaction for MIDI message

[Takashi Sakamoto]

Units on TASCAM FireWire series receive MIDI messages by asynchronous
transactions on IEEE 1394 bus. Although the transaction is sent to a
certain register, current ALSA driver for this series has a redundant design.

This commit use the same address for the transaction.

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

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 2f5e20c..4e362b83 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -230,20 +230,20 @@ static void midi_port_work(struct work_struct *work)
 	fw_send_request(port->parent->card, &port->transaction,
 			TCODE_WRITE_QUADLET_REQUEST,
 			port->parent->node_id, generation,
-			port->parent->max_speed, port->addr,
+			port->parent->max_speed,
+			TSCM_ADDR_BASE + TSCM_OFFSET_MIDI_RX_QUAD,
 			port->buf, 4, async_midi_port_callback,
 			port);
 }
 
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit, u64 addr)
+		struct fw_unit *unit)
 {
 	port->buf = kzalloc(4, GFP_KERNEL);
 	if (port->buf == NULL)
 		return -ENOMEM;
 
 	port->parent = fw_parent_device(unit);
-	port->addr = addr;
 	port->idling = true;
 	port->next_ktime = 0;
 	port->error = false;
@@ -336,8 +336,7 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 
 	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);
+				&tscm->out_ports[i], tscm->unit);
 		if (err < 0)
 			goto error;
 	}
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index c1becc6..a120d74 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -52,7 +52,6 @@ struct snd_fw_async_midi_port {
 	ktime_t next_ktime;
 	bool error;
 
-	u64 addr;
 	struct fw_transaction transaction;
 
 	u8 *buf;
@@ -148,7 +147,7 @@ int snd_tscm_stream_lock_try(struct snd_tscm *tscm);
 void snd_tscm_stream_lock_release(struct snd_tscm *tscm);
 
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit, u64 addr);
+		struct fw_unit *unit);
 void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port);
 
 static inline void
-- 
2.9.3

[PATCH 5/8] firewire-tascam: use fixed-length array for message cache to async midi port

[Takashi Sakamoto]

ALSA driver for TASCAM FireWire series internally allocates 4 byte buffer
for asynchronous transaction to transfer MIDI messages. However, the buffer
can be allocated with memory object of parent structure.

This commit adds 4 byte array as a member of the structure and obsoletes
the redundant allocation. This is deallocated with memory object of parent
structure.

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

diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 4e362b83..248afe6 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -239,10 +239,6 @@ static void midi_port_work(struct work_struct *work)
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 		struct fw_unit *unit)
 {
-	port->buf = kzalloc(4, GFP_KERNEL);
-	if (port->buf == NULL)
-		return -ENOMEM;
-
 	port->parent = fw_parent_device(unit);
 	port->idling = true;
 	port->next_ktime = 0;
@@ -253,13 +249,6 @@ int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 	return 0;
 }
 
-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);
-}
-
 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,
@@ -389,7 +378,6 @@ int snd_tscm_transaction_reregister(struct snd_tscm *tscm)
 void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
 {
 	__be32 reg;
-	unsigned int i;
 
 	if (tscm->async_handler.callback_data == NULL)
 		return;
@@ -416,7 +404,4 @@ void snd_tscm_transaction_unregister(struct snd_tscm *tscm)
 
 	fw_core_remove_address_handler(&tscm->async_handler);
 	tscm->async_handler.callback_data = NULL;
-
-	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 a120d74..05884ae 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -54,7 +54,7 @@ struct snd_fw_async_midi_port {
 
 	struct fw_transaction transaction;
 
-	u8 *buf;
+	u8 buf[4];
 
 	struct snd_rawmidi_substream *substream;
 	unsigned int consume_bytes;
@@ -148,7 +148,6 @@ void snd_tscm_stream_lock_release(struct snd_tscm *tscm);
 
 int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
 		struct fw_unit *unit);
-void snd_fw_async_midi_port_destroy(struct snd_fw_async_midi_port *port);
 
 static inline void
 snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
@@ -164,6 +163,7 @@ static inline void
 snd_fw_async_midi_port_finish(struct snd_fw_async_midi_port *port)
 {
 	port->substream = NULL;
+	cancel_work_sync(&port->work);
 	port->error = false;
 }
 
-- 
2.9.3

[PATCH 6/8] firewire-tascam: initialize parameters at open of rawmidi character devices

[Takashi Sakamoto]

In current design of ALSA driver for TASCAM FireWire series, initialization
of members in asymc midi port structure is done at device probing. Some of
the members should be initialized every time to use rawmidi devices because
they're changed in sequence of transmission for MIDI messages.

This commit adds a new function to initialize them. Invariant parameters
during object lifetime are kept as is.

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

diff --git a/sound/firewire/tascam/tascam-midi.c b/sound/firewire/tascam/tascam-midi.c
index df4f95d..901df81 100644
--- a/sound/firewire/tascam/tascam-midi.c
+++ b/sound/firewire/tascam/tascam-midi.c
@@ -18,6 +18,8 @@ static int midi_playback_open(struct snd_rawmidi_substream *substream)
 {
 	struct snd_tscm *tscm = substream->rmidi->private_data;
 
+	snd_fw_async_midi_port_init(&tscm->out_ports[substream->number]);
+
 	/* Initialize internal status. */
 	tscm->running_status[substream->number] = 0;
 	tscm->on_sysex[substream->number] = 0;
diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index 248afe6..a248a4a 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -236,17 +236,10 @@ static void midi_port_work(struct work_struct *work)
 			port);
 }
 
-int snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit)
+void snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port)
 {
-	port->parent = fw_parent_device(unit);
 	port->idling = true;
-	port->next_ktime = 0;
 	port->error = false;
-
-	INIT_WORK(&port->work, midi_port_work);
-
-	return 0;
 }
 
 static void handle_midi_tx(struct fw_card *card, struct fw_request *request,
@@ -324,10 +317,9 @@ int snd_tscm_transaction_register(struct snd_tscm *tscm)
 		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);
-		if (err < 0)
-			goto error;
+		tscm->out_ports[i].parent = fw_parent_device(tscm->unit);
+		tscm->out_ports[i].next_ktime = 0;
+		INIT_WORK(&tscm->out_ports[i].work, midi_port_work);
 	}
 
 	return err;
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index 05884ae..a5bbe02 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -146,8 +146,7 @@ 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_fw_async_midi_port_init(struct snd_fw_async_midi_port *port,
-		struct fw_unit *unit);
+void snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port);
 
 static inline void
 snd_fw_async_midi_port_run(struct snd_fw_async_midi_port *port,
-- 
2.9.3

[PATCH 7/8] firewire-tascam: move message parameters for async midi port

[Takashi Sakamoto]

Units on TASCAM FireWire series handle MIDI messages with support for
running status. Drivers for the series should remember current running
status and transfer valid MIDI messages. For this purpose, current
ALSA driver for the series has some members in its top-level structure.
This is due to better abstraction of async midi port. Nowadays, the
abstraction was localized just for the driver.

This commit moves the members to structure for async midi port.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-midi.c        |  3 ---
 sound/firewire/tascam/tascam-transaction.c | 33 +++++++++++++++---------------
 sound/firewire/tascam/tascam.h             |  4 ++--
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/sound/firewire/tascam/tascam-midi.c b/sound/firewire/tascam/tascam-midi.c
index 901df81..760f72a 100644
--- a/sound/firewire/tascam/tascam-midi.c
+++ b/sound/firewire/tascam/tascam-midi.c
@@ -20,9 +20,6 @@ static int midi_playback_open(struct snd_rawmidi_substream *substream)
 
 	snd_fw_async_midi_port_init(&tscm->out_ports[substream->number]);
 
-	/* Initialize internal status. */
-	tscm->running_status[substream->number] = 0;
-	tscm->on_sysex[substream->number] = 0;
 	return 0;
 }
 
diff --git a/sound/firewire/tascam/tascam-transaction.c b/sound/firewire/tascam/tascam-transaction.c
index a248a4a..8967c52 100644
--- a/sound/firewire/tascam/tascam-transaction.c
+++ b/sound/firewire/tascam/tascam-transaction.c
@@ -58,39 +58,38 @@ static inline int calculate_message_bytes(u8 status)
 	return -EINVAL;
 }
 
-static int fill_message(struct snd_rawmidi_substream *substream, u8 *buf)
+static int fill_message(struct snd_fw_async_midi_port *port,
+			struct snd_rawmidi_substream *substream)
 {
-	struct snd_tscm *tscm = substream->rmidi->private_data;
-	unsigned int port = substream->number;
 	int i, len, consume;
 	u8 *label, *msg;
 	u8 status;
 
 	/* The first byte is used for label, the rest for MIDI bytes. */
-	label = buf;
-	msg = buf + 1;
+	label = port->buf;
+	msg = port->buf + 1;
 
 	consume = snd_rawmidi_transmit_peek(substream, msg, 3);
 	if (consume == 0)
 		return 0;
 
 	/* On exclusive message. */
-	if (tscm->on_sysex[port]) {
+	if (port->on_sysex) {
 		/* Seek the end of exclusives. */
 		for (i = 0; i < consume; ++i) {
 			if (msg[i] == 0xf7) {
-				tscm->on_sysex[port] = false;
+				port->on_sysex = false;
 				break;
 			}
 		}
 
 		/* At the end of exclusive message, use label 0x07. */
-		if (!tscm->on_sysex[port]) {
+		if (!port->on_sysex) {
 			consume = i + 1;
-			*label = (port << 4) | 0x07;
+			*label = (substream->number << 4) | 0x07;
 		/* During exclusive message, use label 0x04. */
 		} else if (consume == 3) {
-			*label = (port << 4) | 0x04;
+			*label = (substream->number << 4) | 0x04;
 		/* We need to fill whole 3 bytes. Go to next change. */
 		} else {
 			return 0;
@@ -101,12 +100,12 @@ static int fill_message(struct snd_rawmidi_substream *substream, u8 *buf)
 		/* The beginning of exclusives. */
 		if (msg[0] == 0xf0) {
 			/* Transfer it in next chance in another condition. */
-			tscm->on_sysex[port] = true;
+			port->on_sysex = true;
 			return 0;
 		} else {
 			/* On running-status. */
 			if ((msg[0] & 0x80) != 0x80)
-				status = tscm->running_status[port];
+				status = port->running_status;
 			else
 				status = msg[0];
 
@@ -124,18 +123,18 @@ static int fill_message(struct snd_rawmidi_substream *substream, u8 *buf)
 
 				msg[2] = msg[1];
 				msg[1] = msg[0];
-				msg[0] = tscm->running_status[port];
+				msg[0] = port->running_status;
 			} else {
 				/* Enough MIDI bytes were not retrieved. */
 				if (consume < len)
 					return 0;
 				consume = len;
 
-				tscm->running_status[port] = msg[0];
+				port->running_status = msg[0];
 			}
 		}
 
-		*label = (port << 4) | (msg[0] >> 4);
+		*label = (substream->number << 4) | (msg[0] >> 4);
 	}
 
 	if (len > 0 && len < 3)
@@ -196,7 +195,7 @@ static void midi_port_work(struct work_struct *work)
 	 * Later, snd_rawmidi_transmit_ack() is called.
 	 */
 	memset(port->buf, 0, 4);
-	port->consume_bytes = fill_message(substream, port->buf);
+	port->consume_bytes = fill_message(port, substream);
 	if (port->consume_bytes <= 0) {
 		/* Do it in next chance, immediately. */
 		if (port->consume_bytes == 0) {
@@ -240,6 +239,8 @@ void snd_fw_async_midi_port_init(struct snd_fw_async_midi_port *port)
 {
 	port->idling = true;
 	port->error = false;
+	port->running_status = 0;
+	port->on_sysex = false;
 }
 
 static void handle_midi_tx(struct fw_card *card, struct fw_request *request,
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index a5bbe02..35c3a23 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -55,6 +55,8 @@ struct snd_fw_async_midi_port {
 	struct fw_transaction transaction;
 
 	u8 buf[4];
+	u8 running_status;
+	bool on_sysex;
 
 	struct snd_rawmidi_substream *substream;
 	unsigned int consume_bytes;
@@ -87,8 +89,6 @@ struct snd_tscm {
 
 	/* 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];
 };
 
 #define TSCM_ADDR_BASE			0xffff00000000ull
-- 
2.9.3

[PATCH 8/8] firewire-tascam: support drain callback for MIDI playback substream

[Takashi Sakamoto]

ALSA driver for TASCAM FireWire series transfers MIDI messages in system
workqueue. In current design of the driver, applications should wait for
sequence of transmission when they close ALSA rawmidi character devices.
However, when considering design of rawmidi interface, it's preferable
to wait in drain ioctl.

This commit adds support for the drain ioctl to wait for the end of
the transmission.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam-midi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/firewire/tascam/tascam-midi.c b/sound/firewire/tascam/tascam-midi.c
index 760f72a..4a741570 100644
--- a/sound/firewire/tascam/tascam-midi.c
+++ b/sound/firewire/tascam/tascam-midi.c
@@ -31,11 +31,14 @@ static int midi_capture_close(struct snd_rawmidi_substream *substream)
 
 static int midi_playback_close(struct snd_rawmidi_substream *substream)
 {
+	return 0;
+}
+
+static void midi_playback_drain(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)
@@ -77,6 +80,7 @@ int snd_tscm_create_midi_devices(struct snd_tscm *tscm)
 	static const struct snd_rawmidi_ops playback_ops = {
 		.open		= midi_playback_open,
 		.close		= midi_playback_close,
+		.drain		= midi_playback_drain,
 		.trigger	= midi_playback_trigger,
 	};
 	struct snd_rawmidi *rmidi;
-- 
2.9.3

Re: [PATCH 1/8] firewire-lib/firewire-tascam: localize async midi port

[Takashi Iwai]

On Thu, 13 Apr 2017 07:15:20 +0200,
Takashi Sakamoto wrote:
> 
> In Linux kernel 4.4, firewire-lib got a feature called as 'async midi port'
> for transmission of MIDI message via IEEE 1394 asynchronous communication,
> however actual consumer of this feature is ALSA driver for TASCAM FireWire
> series only. When adding this feature, I assumed that ALSA driver for
> Digi00x might also be a consumer, actually it's not.
> 
> This commit moves the feature from firewire-lib to firewire-tascam module.
> Two minor kernel APIs are removed.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Now this one conflicts with your latest fix.  Please resubmit the
fixed version with unsigned -> signed change.


thanks,

Takashi