Re: firewire-lib: an issue to generate packet with 'no data' in blocking mode

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]

Hi Clemens,

 > I guess I just didn't notice that empty packets don't need to change
 > their FDF field.  This is a bug.

I also paied no attension to empty packets. Then BeBoB chipset teach me.

 > Empty packets should be fine for all devices.  (NO-DATA packets would
 > waste DMA bandwidth.)

I completely agree with you. Would you review an attached patch?


Thanks

Takashi Sakamoto

(Nov 22 2013 20:47), Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> I have a question about generate packet with 'no data' in blocking mode.
>> I think there is out of specification in current firewire-lib.
>>
>> In my understanding of IEC 61883-6, there are two ways:
>>
>> 1. generate 'empty packet' defined in IEC 61883-1
>>     - size of packet is 2 quadlets
>>     - FDF = sfc
>>     - packet includes just CIP headers
>>
>> 2. generate 'special non-empty packet' defined in IEC 61883-6
>>     - size of packet is following to blocking mode
>>     - FDF = 0xff ('NO-DATA' code)
>>     - packet includes dummy data
>>
>> But current implementation is a strange combination of them.
>>   - size of packet is 2 (way 1)
>>   - FDF = 0xff (way 2)
>
> It's an empty NO-DATA packet.  ;-)
>
> I guess I just didn't notice that empty packets don't need to change
> their FDF field.  This is a bug.
>
>> If this is a qurk for some devices, I'll prepare patches to switch
>> generating mode because BeBoB cannot sound with current firewire-lib. If
>> this is a bug, then I want to discuss which is better for firewire-lib.
>
> Empty packets should be fine for all devices.  (NO-DATA packets would
> waste DMA bandwidth.)
>
>
> Regards,
> Clemens

[-- Attachment #2: 0001-firewire-lib-fix-wrong-value-for-FDF-field-in-out-pa.patch --]
[-- Type: text/x-patch, Size: 1989 bytes --]

>From 52312ed0989669a4f15c33e3a3c64da79519480f Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Thu, 21 Nov 2013 14:21:06 +0900
Subject: [PATCH 1/1] firewire-lib: fix wrong value for FDF field as an empty packet

This commit fix out of specification about the value of FDF field in out packet
with 'no data'. This affects blocking mode.

According to IEC 61883-6, there is two way to generate AMDTP packets include no
data in blocking mode.

Way 1. an empty packet defined in IEC 61883-1
 - Size of packet is 2 quadlets.
 - The value of FDF is sfc.
 - The packet includes only CIP headers

Way 2. a special non-empty packet defined in IEC 61883-6
 - Size of packet is following to blocking mode
 - The value of FDF is 0xff. This value is 'NO-DATA'. This means 'The receiver'
   must ignore all the data in a CIP with this FDF code'.
 - The packet includes dummy data.

But current implementation is a combination of them.
 - Size of packet is 2 (way 1)
 - FDF = 0xff (way 2)
This causes BeBoB chipset cannot sound.

This patch applies Way 1.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 amdtp.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/amdtp.c b/amdtp.c
index d322689..a998983 100644
--- a/amdtp.c
+++ b/amdtp.c
@@ -434,17 +434,14 @@ static void queue_out_packet(struct amdtp_out_stream *s, unsigned int cycle)
 		return;
 	index = s->packet_index;
 
+	/* this module generate empty packet for 'no data' */
 	syt = calculate_syt(s, cycle);
-	if (!(s->flags & CIP_BLOCKING)) {
+	if (!(s->flags & CIP_BLOCKING))
 		data_blocks = calculate_data_blocks(s);
-	} else {
-		if (syt != 0xffff) {
-			data_blocks = s->syt_interval;
-		} else {
-			data_blocks = 0;
-			syt = 0xffffff;
-		}
-	}
+	else if (syt != 0xffff)
+		data_blocks = s->syt_interval;
+	else
+		data_blocks = 0;
 
 	buffer = s->buffer.packets[index].buffer;
 	buffer[0] = cpu_to_be32(ACCESS_ONCE(s->source_node_id_field) |
-- 
1.8.3.2


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]