| Message ID | 1395400229-22957-43-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
I realize this patch includes a bug to cause wrong detection of packet discontinuity. > @@ -745,6 +745,8 @@ static void handle_in_packet(struct amdtp_stream *s, > > /* Check data block counter continuity */ > data_block_counter = cip_header[0] & AMDTP_DBC_MASK; > + if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC)) > + data_block_counter = s->data_block_counter; > if (!(s->flags & CIP_DBC_IS_END_EVENT)) { > lost = data_block_counter != s->data_block_counter; > } else { These lines should be: > + if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) && > + s->data_block_counter != UINT_MAX) > + data_block_counter = s->data_block_counter; When CIP_SKIP_INIT_DBC_CHECK is enabled, initial value of s->data_block_counter is set to UINT_MAX. An intention of this is to skip detecting discontinuity for a first packet [31/44]. This initial value has a bad effect when CIP_EMPTY_HAS_WRONG_DBC is enabled. A first packet is an empty packet. When this packet is handled, data_block_counter is set to initial value of s->data_block_counter (UINT_MAX). Later, s->data_block_counter is set to 0xff with bitmask in below lines. if (s->flags & CIP_DBC_IS_END_EVENT) s->data_block_counter = data_block_counter; else s->data_block_counter = (data_block_counter + data_blocks) & 0xff; When a packet with data blocks is handled later, s->data_block_counter is 0xff, not UINT_MAX. This causes discontinuity detection because this packet has 0x00 in itx dbc in below lines. if (lost && s->data_block_counter != UINT_MAX) { dev_info(&s->unit->device, "Detect discontinuity of CIP: %02X %02X\n", s->data_block_counter, data_block_counter); See this log: [ 2933.102892] 01020000 9002FFFF 02 [ 2933.102894] 01020000 9002FFFF 02 [ 2933.102896] 01020000 9002FFFF 02 [ 2933.102899] 01020000 9002FFFF 02 [ 2933.102901] 01020000 9002FFFF 02 [ 2933.102903] 01020000 9002FFFF 02 [ 2933.104871] 01110000 900258B5 8A [ 2933.104877] snd-bebob fw1.0: Detect discontinuity of CIP: FF 00 I'm sorry to post a patch including a bug. Regards Takashi Sakamoto o-takashi@sakamocchi.jp
Hi Euan, Thanks for your testing. (Mar 24 2014 10:41), Euan de Kock wrote: > Thanks for the update. This bit me last night, and I just commented out > the check to keep working. > > I'll try and retest it tonight with this version. (On a Echo Audiofire > Pre8 with firmware < 5) > > Regards, > > Euan deKock. When reading this message, I wondered why this bug affects driving on Echo AudioFirePre8. As describing the title, this affects a part of BeBoB based devices. Did you test with M-Audio Firewire 1814 or M-Audio ProjectMix I/O? If not, it may be a bug for Fireworks driver. Then please send me output from: /proc/asound/cardX/firewire/*. (The 'cardX' should be for your device.) Thanks Takashi Sakamoto o-takashi@sakamocchi.jp
diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index 820d790..c443a3a 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -745,6 +745,8 @@ static void handle_in_packet(struct amdtp_stream *s, /* Check data block counter continuity */ data_block_counter = cip_header[0] & AMDTP_DBC_MASK; + if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC)) + data_block_counter = s->data_block_counter; if (!(s->flags & CIP_DBC_IS_END_EVENT)) { lost = data_block_counter != s->data_block_counter; } else { diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h index e3ee739..7981f4d 100644 --- a/sound/firewire/amdtp.h +++ b/sound/firewire/amdtp.h @@ -29,6 +29,8 @@ * The value of data_block_quadlets is used instead of reported value. * @CIP_SKIP_INIT_DBC_CHECK: Only for in-stream. The value of dbc in first * packet is not continuous from an initial value. + * @CIP_EMPTY_HAS_WRONG_DBC: Only for in-stream. The value of dbc in empty + * packet is wrong but the others are correct. */ enum cip_flags { CIP_NONBLOCKING = 0x00, @@ -39,6 +41,7 @@ enum cip_flags { CIP_DBC_IS_END_EVENT = 0x10, CIP_WRONG_DBS = 0x20, CIP_SKIP_INIT_DBC_CHECK = 0x40, + CIP_EMPTY_HAS_WRONG_DBC = 0x80, }; /** diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index 95d82f4..5be82f1 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -436,6 +436,13 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob) } /* See comments in next function */ bebob->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK; + /* + * At high sampling rate, M-Audio special firmware transmits empty + * packet with the value of dbc incremented by 8 but the others are + * valid to IEC 61883-1. + */ + if (bebob->maudio_special_quirk) + bebob->tx_stream.flags |= CIP_EMPTY_HAS_WRONG_DBC; err = amdtp_stream_init(&bebob->rx_stream, bebob->unit, AMDTP_OUT_STREAM, CIP_BLOCKING);
M-Audio Firewire 1814 has a quirk, ProjectMix I/O also has. They transmit empty packet with wrong value of dbc incremented by 8 at high sampling rate. According to IEC 61883-1, this value should be the same as the one in previous packet. This commit adds a flag named as CIP_EMPTY_HAS_WRONG_DBC. With flag, the value of dbc in empty packet is overwittern by the one in expected value. This is an example of this quirk: CIP Header 0 CIP Header 1 Payload size 010D0000 9004F759 210 010D0010 90040B59 210 010D0020 90042359 210 01020028 9004FFFF 2 <- 010D0030 90043759 210 010D0040 90044B59 210 010D0050 90046359 210 01020058 9004FFFF 2 <- 010D0060 90047759 210 010D0070 90048B59 210 010D0080 9004A359 210 01020088 9004FFFF 2 <- 010D0090 9004B759 210 010D00A0 9004CB59 210 010D00B0 9004E359 210 010200B8 9004FFFF 2 <- 010D00C0 9004F759 210 010D00D0 90040B59 210 010D00E0 90042359 210 Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/firewire/amdtp.c | 2 ++ sound/firewire/amdtp.h | 3 +++ sound/firewire/bebob/bebob_stream.c | 7 +++++++ 3 files changed, 12 insertions(+)