[alsa-devel] [PATCH 0/5 v4] ALSA: add helper functions for MIDI message transmission on IEEE 1394 asynchronous transaction
Hi,
This patchset updates a part of my previous post:
[alsa-devel] [PATCH 0/5] ALSA: add helper functions for MIDI message transmission on IEEE 1394 asynchronous transaction http://mailman.alsa-project.org/pipermail/alsa-devel/2015-October/098536.htm...
(Besides, the patchset should have been version 3, thus this patchset is v4.)
This patchset adds some helper functions for MIDI transmission via IEEE 1394 asynchronous transaction.
Changes: * fix compile error due to missing member of structure. * improve comments
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(+)
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 to 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
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;
Currently, when two MIDI trigger callbacks can be called immediately, transactions for the second MIDI messages can be 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)
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 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 | 18 +++++++++++++++++- sound/firewire/lib.h | 1 + 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c index 3e9afd7..9a98c7c 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,7 @@ 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);
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;
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
On Fri, 09 Oct 2015 01:10:24 +0200, Takashi Sakamoto wrote:
Hi,
This patchset updates a part of my previous post:
[alsa-devel] [PATCH 0/5] ALSA: add helper functions for MIDI message transmission on IEEE 1394 asynchronous transaction http://mailman.alsa-project.org/pipermail/alsa-devel/2015-October/098536.htm...
(Besides, the patchset should have been version 3, thus this patchset is v4.)
This patchset adds some helper functions for MIDI transmission via IEEE 1394 asynchronous transaction.
Changes:
- fix compile error due to missing member of structure.
- improve comments
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
Applied, thanks.
Takashi
sound/firewire/lib.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++ sound/firewire/lib.h | 56 +++++++++++++++++++++ 2 files changed, 194 insertions(+)
-- 2.1.4
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto