[PATCH 0/2 v2] ALSA: oxfw: implement retries for scs1x at transaction failure

[PATCH 0/2 v2] ALSA: oxfw: implement retries for scs1x at transaction failure

[Takashi Sakamoto]

Hi,

This patchset updates my former post below. Changes are just adding my sign
(I forgot it...) and improvements of commit messages.

[alsa-devel] [PATCH 0/2] ALSA: oxfw: implement retries for scs1x at transaction failure
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104579.html

In thread of the post, I got some comments about work to extend ALSA
rawmidi core. I've investigated it in a few days ago and realized that it
may not so light work, mainly because of a lack of unified state
management[0].

In this development cycle, I'd like to give it up and use my time for the
other work within my plan for the cycle. Please just apply these two patches.

[0] In short, the state management is understandable for developers who
can understand it, and I'm not.

Regards

Takashi Sakamoto (2):
  ALSA: oxfw: retry MIDI transferring for scs1x at transaction failure
  ALSA: oxfw: discontinue MIDI substream for scs1x at transaction
    failure

 sound/firewire/oxfw/oxfw-scs1x.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

-- 
2.5.0

[PATCH 1/2] ALSA: oxfw: retry MIDI transferring for scs1x at transaction failure

[Takashi Sakamoto]

Currently, ALSA oxfw driver has a TODO to retry MIDI transferring
at transaction failure.

This commit achieves it. Current implementation uses snd_rawmidi_transmit()
to transfer messages, thus the target MIDI messages are not in buffer when
transaction failure is detected. Although we cannot use a pair of
snd_rawmidi_transmit_peek() and snd_ramwidi_transmit_ack(), the
messages are still in scs1x specific structure and the data is available
for retries.

This commit adds a member to the structure for the length of buffered
messages, and uses the value again at retries.

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

diff --git a/sound/firewire/oxfw/oxfw-scs1x.c b/sound/firewire/oxfw/oxfw-scs1x.c
index f7ac124..72446ac 100644
--- a/sound/firewire/oxfw/oxfw-scs1x.c
+++ b/sound/firewire/oxfw/oxfw-scs1x.c
@@ -31,6 +31,7 @@ struct fw_scs1x {
 	u8 buffer[HSS1394_MAX_PACKET_SIZE];
 	bool transaction_running;
 	struct fw_transaction transaction;
+	unsigned int transaction_bytes;
 	struct fw_device *fw_dev;
 };
 
@@ -125,8 +126,8 @@ static void scs_write_callback(struct fw_card *card, int rcode,
 {
 	struct fw_scs1x *scs = callback_data;
 
-	if (rcode == RCODE_GENERATION)
-		;	/* TODO: retry this packet */
+	if (rcode != RCODE_GENERATION)
+		scs->transaction_bytes = 0;
 
 	scs->transaction_running = false;
 	schedule_work(&scs->work);
@@ -183,6 +184,9 @@ static void scs_output_work(struct work_struct *work)
 		return;
 	}
 
+	if (scs->transaction_bytes > 0)
+		goto retry;
+
 	i = scs->output_bytes;
 	for (;;) {
 		if (snd_rawmidi_transmit(stream, &byte, 1) != 1) {
@@ -253,13 +257,16 @@ static void scs_output_work(struct work_struct *work)
 	scs->output_bytes = 1;
 	scs->output_escaped = false;
 
+	scs->transaction_bytes = i;
+retry:
 	scs->transaction_running = true;
 	generation = scs->fw_dev->generation;
 	smp_rmb(); /* node_id vs. generation */
 	fw_send_request(scs->fw_dev->card, &scs->transaction,
 			TCODE_WRITE_BLOCK_REQUEST, scs->fw_dev->node_id,
 			generation, scs->fw_dev->max_speed, HSS1394_ADDRESS,
-			scs->buffer, i, scs_write_callback, scs);
+			scs->buffer, scs->transaction_bytes,
+			scs_write_callback, scs);
 }
 
 static int midi_capture_open(struct snd_rawmidi_substream *stream)
@@ -309,6 +316,7 @@ static void midi_playback_trigger(struct snd_rawmidi_substream *stream, int up)
 		scs->output_bytes = 1;
 		scs->output_escaped = false;
 		scs->output_idle = false;
+		scs->transaction_bytes = 0;
 
 		ACCESS_ONCE(scs->output) = stream;
 		schedule_work(&scs->work);
-- 
2.5.0

[PATCH 2/2] ALSA: oxfw: discontinue MIDI substream for scs1x at transaction failure

[Takashi Sakamoto]

With a previous commit, ALSA oxfw driver retries transferring MIDI
messages at transaction failure for scs1x. On the other hand, there're
fatal transaction error. Then, no MIDI messages reach to the unit anymore.
In this case, MIDI substream should be terminated.

This commit stops MIDI transmission after the fatal error occurs.
Unfortunately, unlike ALSA PCM functionality, ALSA rawmidi core has no
feature to discontinue MIDI substream runtime in kernel side, thus this
commit just stops MIDI transmission without notifying it to userspace.

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

diff --git a/sound/firewire/oxfw/oxfw-scs1x.c b/sound/firewire/oxfw/oxfw-scs1x.c
index 72446ac..f897c98 100644
--- a/sound/firewire/oxfw/oxfw-scs1x.c
+++ b/sound/firewire/oxfw/oxfw-scs1x.c
@@ -32,6 +32,7 @@ struct fw_scs1x {
 	bool transaction_running;
 	struct fw_transaction transaction;
 	unsigned int transaction_bytes;
+	bool error;
 	struct fw_device *fw_dev;
 };
 
@@ -126,8 +127,13 @@ static void scs_write_callback(struct fw_card *card, int rcode,
 {
 	struct fw_scs1x *scs = callback_data;
 
-	if (rcode != RCODE_GENERATION)
-		scs->transaction_bytes = 0;
+	if (!rcode_is_permanent_error(rcode)) {
+		/* Don't retry for this data. */
+		if (rcode == RCODE_COMPLETE)
+			scs->transaction_bytes = 0;
+	} else {
+		scs->error = true;
+	}
 
 	scs->transaction_running = false;
 	schedule_work(&scs->work);
@@ -178,7 +184,7 @@ static void scs_output_work(struct work_struct *work)
 		return;
 
 	stream = ACCESS_ONCE(scs->output);
-	if (!stream) {
+	if (!stream || scs->error) {
 		scs->output_idle = true;
 		wake_up(&scs->idle_wait);
 		return;
@@ -317,6 +323,7 @@ static void midi_playback_trigger(struct snd_rawmidi_substream *stream, int up)
 		scs->output_escaped = false;
 		scs->output_idle = false;
 		scs->transaction_bytes = 0;
+		scs->error = false;
 
 		ACCESS_ONCE(scs->output) = stream;
 		schedule_work(&scs->work);
-- 
2.5.0

Re: [PATCH 0/2 v2] ALSA: oxfw: implement retries for scs1x at transaction failure

[Takashi Iwai]

On Wed, 24 Feb 2016 01:26:31 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset updates my former post below. Changes are just adding my sign
> (I forgot it...) and improvements of commit messages.
> 
> [alsa-devel] [PATCH 0/2] ALSA: oxfw: implement retries for scs1x at transaction failure
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104579.html
> 
> In thread of the post, I got some comments about work to extend ALSA
> rawmidi core. I've investigated it in a few days ago and realized that it
> may not so light work, mainly because of a lack of unified state
> management[0].
> 
> In this development cycle, I'd like to give it up and use my time for the
> other work within my plan for the cycle. Please just apply these two patches.
> 
> [0] In short, the state management is understandable for developers who
> can understand it, and I'm not.
> 
> Regards
> 
> Takashi Sakamoto (2):
>   ALSA: oxfw: retry MIDI transferring for scs1x at transaction failure
>   ALSA: oxfw: discontinue MIDI substream for scs1x at transaction
>     failure

Applied both now.  Thanks.


Takashi

Re: [PATCH 0/2 v2] ALSA: oxfw: implement retries for scs1x at transaction failure

[Takashi Sakamoto]

On Feb 25 2016 00:33, Takashi Iwai wrote:
> On Wed, 24 Feb 2016 01:26:31 +0100,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> This patchset updates my former post below. Changes are just adding my sign
>> (I forgot it...) and improvements of commit messages.
>>
>> [alsa-devel] [PATCH 0/2] ALSA: oxfw: implement retries for scs1x at transaction failure
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104579.html
>>
>> In thread of the post, I got some comments about work to extend ALSA
>> rawmidi core. I've investigated it in a few days ago and realized that it
>> may not so light work, mainly because of a lack of unified state
>> management[0].
>>
>> In this development cycle, I'd like to give it up and use my time for the
>> other work within my plan for the cycle. Please just apply these two patches.
>>
>> [0] In short, the state management is understandable for developers who
>> can understand it, and I'm not.
>>
>> Regards
>>
>> Takashi Sakamoto (2):
>>    ALSA: oxfw: retry MIDI transferring for scs1x at transaction failure
>>    ALSA: oxfw: discontinue MIDI substream for scs1x at transaction
>>      failure
>
> Applied both now.  Thanks.

Thanks.

ALSA rawmidi core handles substreams for both directions, therefore 
state management may not so simple as we expected (at least, for driver 
based on packet streaming such as IEC 61883-1/6, there're more items to 
be considered. So as USB MIDI Device class, too, I think). I'd like to 
schedule the work to next developing cycle for Linux 4.7 or later.


Regards

Takashi Sakamoto