[PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[Takashi Sakamoto]

Hi,

In IR context of Linux FireWire subsystem, some quadlets of packet
payload can be handled as a part of context header. As a result context
payload can just include the rest of packet payload.

This patchset uses the mechanism to unify handlers of incoming packet
for with-CIP and without-CIP headers.

Takashi Sakamoto (6):
  ALSA: firewire-lib: use clear name for variable of CIP header
  ALSA: firewire-lib: calculate the length of packet payload in packet
    handler
  ALSA: firewire-lib: compute pointer to payload buffer in context
    handler
  ALSA: firewire-lib: split helper function to check incoming CIP header
  ALSA: firewire-lib: use 16 bytes IR context header to separate CIP
    header
  ALSA: firewire-lib: unify packet handler for IR context

 sound/firewire/amdtp-stream.c | 180 ++++++++++++++++++----------------
 sound/firewire/amdtp-stream.h |   8 +-
 2 files changed, 97 insertions(+), 91 deletions(-)

-- 
2.20.1

[PATCH 1/6] ALSA: firewire-lib: use clear name for variable of CIP header

[Takashi Sakamoto]

This commit is to distinguish variable of CIP header from variable of
isochronous context header.

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

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index f43943fd962d..020edf2b1726 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -286,15 +286,15 @@ EXPORT_SYMBOL(amdtp_stream_set_parameters);
 unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s)
 {
 	unsigned int multiplier = 1;
-	unsigned int header_size = 0;
+	unsigned int cip_header_size = 0;
 
 	if (s->flags & CIP_JUMBO_PAYLOAD)
 		multiplier = 5;
 	if (!(s->flags & CIP_NO_HEADER))
-		header_size = 8;
+		cip_header_size = sizeof(__be32) * 2;
 
-	return header_size +
-		s->syt_interval * s->data_block_quadlets * 4 * multiplier;
+	return cip_header_size +
+		s->syt_interval * s->data_block_quadlets * sizeof(__be32) * multiplier;
 }
 EXPORT_SYMBOL(amdtp_stream_get_max_payload);
 
-- 
2.20.1

[PATCH 2/6] ALSA: firewire-lib: calculate the length of packet payload in packet handler

[Takashi Sakamoto]

In current packet handler, the length of payload is given as an argument
of callback function, however this value is just required to process
payload of transferred isoc packet, thus just for IR context.

This commit replaces the argument for payload of packet with the
argument of context header. As a result, the length of payload is
computed in packet handler for IR context.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 49 ++++++++++++++++-------------------
 sound/firewire/amdtp-stream.h |  5 ++--
 2 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 020edf2b1726..4584525a7f30 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -474,14 +474,14 @@ static inline int queue_in_packet(struct amdtp_stream *s)
 	return queue_packet(s, s->ctx_data.tx.max_payload_length);
 }
 
-static int handle_out_packet(struct amdtp_stream *s,
-			     unsigned int payload_length, unsigned int cycle,
-			     unsigned int index)
+static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle,
+			     const __be32 *ctx_header, unsigned int index)
 {
 	__be32 *buffer;
 	unsigned int syt;
 	unsigned int data_blocks;
 	unsigned int pcm_frames;
+	unsigned int payload_length;
 	struct snd_pcm_substream *pcm;
 
 	buffer = s->buffer.packets[s->packet_index].buffer;
@@ -521,13 +521,14 @@ static int handle_out_packet(struct amdtp_stream *s,
 }
 
 static int handle_out_packet_without_header(struct amdtp_stream *s,
-			unsigned int payload_length, unsigned int cycle,
-			unsigned int index)
+				unsigned int cycle, const __be32 *ctx_header,
+				unsigned int index)
 {
 	__be32 *buffer;
 	unsigned int syt;
 	unsigned int data_blocks;
 	unsigned int pcm_frames;
+	unsigned int payload_length;
 	struct snd_pcm_substream *pcm;
 
 	buffer = s->buffer.packets[s->packet_index].buffer;
@@ -551,11 +552,11 @@ static int handle_out_packet_without_header(struct amdtp_stream *s,
 	return 0;
 }
 
-static int handle_in_packet(struct amdtp_stream *s,
-			    unsigned int payload_length, unsigned int cycle,
-			    unsigned int index)
+static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
+			    const __be32 *ctx_header, unsigned int index)
 {
 	__be32 *buffer;
+	unsigned int payload_length;
 	u32 cip_header[2];
 	unsigned int sph, fmt, fdf, syt;
 	unsigned int data_block_quadlets, data_block_counter, dbc_interval;
@@ -564,6 +565,14 @@ static int handle_in_packet(struct amdtp_stream *s,
 	unsigned int pcm_frames;
 	bool lost;
 
+	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
+	if (payload_length > s->ctx_data.tx.max_payload_length) {
+		dev_err(&s->unit->device,
+			"Detect jumbo payload: %04x %04x\n",
+			payload_length, s->ctx_data.tx.max_payload_length);
+		return -EIO;
+	}
+
 	buffer = s->buffer.packets[s->packet_index].buffer;
 	cip_header[0] = be32_to_cpu(buffer[0]);
 	cip_header[1] = be32_to_cpu(buffer[1]);
@@ -668,14 +677,16 @@ static int handle_in_packet(struct amdtp_stream *s,
 }
 
 static int handle_in_packet_without_header(struct amdtp_stream *s,
-			unsigned int payload_length, unsigned int cycle,
-			unsigned int index)
+				unsigned int cycle, const __be32 *ctx_header,
+				unsigned int index)
 {
 	__be32 *buffer;
+	unsigned int payload_length;
 	unsigned int data_blocks;
 	struct snd_pcm_substream *pcm;
 	unsigned int pcm_frames;
 
+	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
 	buffer = s->buffer.packets[s->packet_index].buffer;
 	data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets;
 
@@ -745,7 +756,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,
 
 		cycle = compute_it_cycle(*ctx_header);
 
-		if (s->handle_packet(s, 0, cycle, i) < 0) {
+		if (s->handle_packet(s, cycle, ctx_header, i) < 0) {
 			cancel_stream(s);
 			return;
 		}
@@ -762,7 +773,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,
 {
 	struct amdtp_stream *s = private_data;
 	unsigned int i, packets;
-	unsigned int payload_length, max_payload_length;
 	__be32 *ctx_header = header;
 
 	if (s->packet_index < 0)
@@ -771,25 +781,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,
 	// The number of packets in buffer.
 	packets = header_length / s->ctx_data.tx.ctx_header_size;
 
-	/* For buffer-over-run prevention. */
-	max_payload_length = s->ctx_data.tx.max_payload_length;
-
 	for (i = 0; i < packets; i++) {
-		u32 iso_header = be32_to_cpu(ctx_header[0]);
 		u32 cycle;
 
 		cycle = compute_cycle_count(ctx_header[1]);
 
-		/* The number of bytes in this packet */
-		payload_length = iso_header >> ISO_DATA_LENGTH_SHIFT;
-		if (payload_length > max_payload_length) {
-			dev_err(&s->unit->device,
-				"Detect jumbo payload: %04x %04x\n",
-				payload_length, max_payload_length);
-			break;
-		}
-
-		if (s->handle_packet(s, payload_length, cycle, i) < 0)
+		if (s->handle_packet(s, cycle, ctx_header, i) < 0)
 			break;
 
 		ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header);
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index 1945ef59ab92..d317fdc6ab5f 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -108,9 +108,8 @@ struct amdtp_stream {
 	struct iso_packets_buffer buffer;
 	int packet_index;
 	int tag;
-	int (*handle_packet)(struct amdtp_stream *s,
-			unsigned int payload_quadlets, unsigned int cycle,
-			unsigned int index);
+	int (*handle_packet)(struct amdtp_stream *s, unsigned int cycle,
+			     const __be32 *ctx_header, unsigned int index);
 	union {
 		struct {
 			unsigned int ctx_header_size;
-- 
2.20.1

[PATCH 3/6] ALSA: firewire-lib: compute pointer to payload buffer in context handler

[Takashi Sakamoto]

The value of pointer to payload buffer is computed in each packet
handler, however the pointer can be decided before call of packet
handler.

This commit adds an argument for the pointer to the packet handler to
reduce codes to compute for the pointer.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 28 +++++++++++++---------------
 sound/firewire/amdtp-stream.h |  3 ++-
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 4584525a7f30..ab9dc7e9ffa4 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -475,16 +475,15 @@ static inline int queue_in_packet(struct amdtp_stream *s)
 }
 
 static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle,
-			     const __be32 *ctx_header, unsigned int index)
+			     const __be32 *ctx_header, __be32 *buffer,
+			     unsigned int index)
 {
-	__be32 *buffer;
 	unsigned int syt;
 	unsigned int data_blocks;
 	unsigned int pcm_frames;
 	unsigned int payload_length;
 	struct snd_pcm_substream *pcm;
 
-	buffer = s->buffer.packets[s->packet_index].buffer;
 	syt = calculate_syt(s, cycle);
 	data_blocks = calculate_data_blocks(s, syt);
 	pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);
@@ -522,16 +521,14 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle,
 
 static int handle_out_packet_without_header(struct amdtp_stream *s,
 				unsigned int cycle, const __be32 *ctx_header,
-				unsigned int index)
+				__be32 *buffer, unsigned int index)
 {
-	__be32 *buffer;
 	unsigned int syt;
 	unsigned int data_blocks;
 	unsigned int pcm_frames;
 	unsigned int payload_length;
 	struct snd_pcm_substream *pcm;
 
-	buffer = s->buffer.packets[s->packet_index].buffer;
 	syt = calculate_syt(s, cycle);
 	data_blocks = calculate_data_blocks(s, syt);
 	pcm_frames = s->process_data_blocks(s, buffer, data_blocks, &syt);
@@ -553,9 +550,9 @@ static int handle_out_packet_without_header(struct amdtp_stream *s,
 }
 
 static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
-			    const __be32 *ctx_header, unsigned int index)
+			    const __be32 *ctx_header, __be32 *buffer,
+			    unsigned int index)
 {
-	__be32 *buffer;
 	unsigned int payload_length;
 	u32 cip_header[2];
 	unsigned int sph, fmt, fdf, syt;
@@ -573,7 +570,6 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		return -EIO;
 	}
 
-	buffer = s->buffer.packets[s->packet_index].buffer;
 	cip_header[0] = be32_to_cpu(buffer[0]);
 	cip_header[1] = be32_to_cpu(buffer[1]);
 
@@ -678,17 +674,15 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 
 static int handle_in_packet_without_header(struct amdtp_stream *s,
 				unsigned int cycle, const __be32 *ctx_header,
-				unsigned int index)
+				__be32 *buffer, unsigned int index)
 {
-	__be32 *buffer;
 	unsigned int payload_length;
 	unsigned int data_blocks;
 	struct snd_pcm_substream *pcm;
 	unsigned int pcm_frames;
 
 	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
-	buffer = s->buffer.packets[s->packet_index].buffer;
-	data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets;
+	data_blocks = payload_length / 4 / s->data_block_quadlets;
 
 	trace_amdtp_packet(s, cycle, NULL, payload_length, data_blocks, index);
 
@@ -753,10 +747,12 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,
 
 	for (i = 0; i < packets; ++i) {
 		u32 cycle;
+		__be32 *buffer;
 
 		cycle = compute_it_cycle(*ctx_header);
+		buffer = s->buffer.packets[s->packet_index].buffer;
 
-		if (s->handle_packet(s, cycle, ctx_header, i) < 0) {
+		if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0) {
 			cancel_stream(s);
 			return;
 		}
@@ -783,10 +779,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,
 
 	for (i = 0; i < packets; i++) {
 		u32 cycle;
+		__be32 *buffer;
 
 		cycle = compute_cycle_count(ctx_header[1]);
+		buffer = s->buffer.packets[s->packet_index].buffer;
 
-		if (s->handle_packet(s, cycle, ctx_header, i) < 0)
+		if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0)
 			break;
 
 		ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header);
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index d317fdc6ab5f..5aa9683593d2 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -109,7 +109,8 @@ struct amdtp_stream {
 	int packet_index;
 	int tag;
 	int (*handle_packet)(struct amdtp_stream *s, unsigned int cycle,
-			     const __be32 *ctx_header, unsigned int index);
+			     const __be32 *ctx_header, __be32 *buffer,
+			     unsigned int index);
 	union {
 		struct {
 			unsigned int ctx_header_size;
-- 
2.20.1

[PATCH 4/6] ALSA: firewire-lib: split helper function to check incoming CIP header

[Takashi Sakamoto]

A parser for CIP header in incoming packet is enough large.

This commit splits it into a helper function to better looks of packet
handler.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 100 ++++++++++++++++++++--------------
 1 file changed, 60 insertions(+), 40 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index ab9dc7e9ffa4..e9976a877944 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -549,29 +549,19 @@ static int handle_out_packet_without_header(struct amdtp_stream *s,
 	return 0;
 }
 
-static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
-			    const __be32 *ctx_header, __be32 *buffer,
-			    unsigned int index)
+static int check_cip_header(struct amdtp_stream *s, const __be32 *buf,
+			    unsigned int payload_length,
+			    unsigned int *data_blocks, unsigned int *syt)
 {
-	unsigned int payload_length;
 	u32 cip_header[2];
-	unsigned int sph, fmt, fdf, syt;
-	unsigned int data_block_quadlets, data_block_counter, dbc_interval;
-	unsigned int data_blocks;
-	struct snd_pcm_substream *pcm;
-	unsigned int pcm_frames;
+	unsigned int sph;
+	unsigned int fmt;
+	unsigned int fdf;
+	unsigned int data_block_counter;
 	bool lost;
 
-	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
-	if (payload_length > s->ctx_data.tx.max_payload_length) {
-		dev_err(&s->unit->device,
-			"Detect jumbo payload: %04x %04x\n",
-			payload_length, s->ctx_data.tx.max_payload_length);
-		return -EIO;
-	}
-
-	cip_header[0] = be32_to_cpu(buffer[0]);
-	cip_header[1] = be32_to_cpu(buffer[1]);
+	cip_header[0] = be32_to_cpu(buf[0]);
+	cip_header[1] = be32_to_cpu(buf[1]);
 
 	/*
 	 * This module supports 'Two-quadlet CIP header with SYT field'.
@@ -583,9 +573,7 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		dev_info_ratelimited(&s->unit->device,
 				"Invalid CIP header for AMDTP: %08X:%08X\n",
 				cip_header[0], cip_header[1]);
-		data_blocks = 0;
-		pcm_frames = 0;
-		goto end;
+		return -EAGAIN;
 	}
 
 	/* Check valid protocol or not. */
@@ -595,19 +583,17 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		dev_info_ratelimited(&s->unit->device,
 				     "Detect unexpected protocol: %08x %08x\n",
 				     cip_header[0], cip_header[1]);
-		data_blocks = 0;
-		pcm_frames = 0;
-		goto end;
+		return -EAGAIN;
 	}
 
 	/* Calculate data blocks */
 	fdf = (cip_header[1] & CIP_FDF_MASK) >> CIP_FDF_SHIFT;
-	if (payload_length < 12 ||
+	if (payload_length < sizeof(__be32) * 2 ||
 	    (fmt == CIP_FMT_AM && fdf == AMDTP_FDF_NO_DATA)) {
-		data_blocks = 0;
+		*data_blocks = 0;
 	} else {
-		data_block_quadlets =
-			(cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT;
+		unsigned int data_block_quadlets =
+				(cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT;
 		/* avoid division by zero */
 		if (data_block_quadlets == 0) {
 			dev_err(&s->unit->device,
@@ -618,13 +604,13 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		if (s->flags & CIP_WRONG_DBS)
 			data_block_quadlets = s->data_block_quadlets;
 
-		data_blocks = (payload_length / 4 - 2) /
+		*data_blocks = (payload_length / sizeof(__be32) - 2) /
 							data_block_quadlets;
 	}
 
 	/* Check data block counter continuity */
 	data_block_counter = cip_header[0] & CIP_DBC_MASK;
-	if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) &&
+	if (*data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) &&
 	    s->data_block_counter != UINT_MAX)
 		data_block_counter = s->data_block_counter;
 
@@ -635,10 +621,12 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 	} else if (!(s->flags & CIP_DBC_IS_END_EVENT)) {
 		lost = data_block_counter != s->data_block_counter;
 	} else {
-		if (data_blocks > 0 && s->ctx_data.tx.dbc_interval > 0)
+		unsigned int dbc_interval;
+
+		if (*data_blocks > 0 && s->ctx_data.tx.dbc_interval > 0)
 			dbc_interval = s->ctx_data.tx.dbc_interval;
 		else
-			dbc_interval = data_blocks;
+			dbc_interval = *data_blocks;
 
 		lost = data_block_counter !=
 		       ((s->data_block_counter + dbc_interval) & 0xff);
@@ -651,16 +639,48 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		return -EIO;
 	}
 
-	trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index);
-
-	syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK;
-	pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);
+	*syt = cip_header[1] & CIP_SYT_MASK;
 
-	if (s->flags & CIP_DBC_IS_END_EVENT)
+	if (s->flags & CIP_DBC_IS_END_EVENT) {
 		s->data_block_counter = data_block_counter;
-	else
+	} else {
 		s->data_block_counter =
-				(data_block_counter + data_blocks) & 0xff;
+				(data_block_counter + *data_blocks) & 0xff;
+	}
+
+	return 0;
+}
+
+static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
+			    const __be32 *ctx_header, __be32 *buffer,
+			    unsigned int index)
+{
+	unsigned int payload_length;
+	unsigned int syt;
+	unsigned int data_blocks;
+	struct snd_pcm_substream *pcm;
+	unsigned int pcm_frames;
+	int err;
+
+	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
+	if (payload_length > s->ctx_data.tx.max_payload_length) {
+		dev_err(&s->unit->device,
+			"Detect jumbo payload: %04x %04x\n",
+			payload_length, s->ctx_data.tx.max_payload_length);
+		return -EIO;
+	}
+
+	err = check_cip_header(s, buffer, payload_length, &data_blocks, &syt);
+	if (err < 0) {
+		if (err != -EAGAIN)
+			return err;
+		pcm_frames = 0;
+		goto end;
+	}
+
+	trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index);
+
+	pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);
 end:
 	if (queue_in_packet(s) < 0)
 		return -EIO;
-- 
2.20.1

[PATCH 5/6] ALSA: firewire-lib: use 16 bytes IR context header to separate CIP header

[Takashi Sakamoto]

In IR context, some quadlets of packet payload can be included into
context header. This is good for packet with CIP header because the
context payload buffer can includes data blocks only for with-CIP and
without-CIP pakets.

This commit uses 16 bytes IR context header for this purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 37 ++++++++++++++++++++++++-----------
 sound/firewire/amdtp-stream.h |  2 +-
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index e9976a877944..fa99210f5a48 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -56,7 +56,10 @@
 #define INTERRUPT_INTERVAL	16
 #define QUEUE_LENGTH		48
 
-#define IR_HEADER_SIZE		8	// For header and timestamp.
+// For iso header, tstamp and 2 CIP header.
+#define IR_CTX_HEADER_SIZE_CIP		16
+// For iso header and tstamp.
+#define IR_CTX_HEADER_SIZE_NO_CIP	8
 #define HEADER_TSTAMP_MASK	0x0000ffff
 
 static void pcm_period_tasklet(unsigned long data);
@@ -471,7 +474,7 @@ static inline int queue_out_packet(struct amdtp_stream *s,
 
 static inline int queue_in_packet(struct amdtp_stream *s)
 {
-	return queue_packet(s, s->ctx_data.tx.max_payload_length);
+	return queue_packet(s, s->ctx_data.tx.max_ctx_payload_length);
 }
 
 static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle,
@@ -656,6 +659,7 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 			    unsigned int index)
 {
 	unsigned int payload_length;
+	const __be32 *cip_header;
 	unsigned int syt;
 	unsigned int data_blocks;
 	struct snd_pcm_substream *pcm;
@@ -663,14 +667,17 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 	int err;
 
 	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
-	if (payload_length > s->ctx_data.tx.max_payload_length) {
+	if (payload_length > s->ctx_data.tx.ctx_header_size +
+					s->ctx_data.tx.max_ctx_payload_length) {
 		dev_err(&s->unit->device,
 			"Detect jumbo payload: %04x %04x\n",
-			payload_length, s->ctx_data.tx.max_payload_length);
+			payload_length, s->ctx_data.tx.max_ctx_payload_length);
 		return -EIO;
 	}
 
-	err = check_cip_header(s, buffer, payload_length, &data_blocks, &syt);
+	cip_header = ctx_header + 2;
+	err = check_cip_header(s, cip_header, payload_length, &data_blocks,
+			       &syt);
 	if (err < 0) {
 		if (err != -EAGAIN)
 			return err;
@@ -678,9 +685,10 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 		goto end;
 	}
 
-	trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index);
+	trace_amdtp_packet(s, cycle, cip_header, payload_length, data_blocks,
+			   index);
 
-	pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);
+	pcm_frames = s->process_data_blocks(s, buffer, data_blocks, &syt);
 end:
 	if (queue_in_packet(s) < 0)
 		return -EIO;
@@ -883,6 +891,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 		[CIP_SFC_176400] = {  0,   67 },
 	};
 	unsigned int ctx_header_size;
+	unsigned int max_ctx_payload_size;
 	enum dma_data_direction dir;
 	int type, tag, err;
 
@@ -909,14 +918,21 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 	if (s->direction == AMDTP_IN_STREAM) {
 		dir = DMA_FROM_DEVICE;
 		type = FW_ISO_CONTEXT_RECEIVE;
-		ctx_header_size = IR_HEADER_SIZE;
+		if (!(s->flags & CIP_NO_HEADER))
+			ctx_header_size = IR_CTX_HEADER_SIZE_CIP;
+		else
+			ctx_header_size = IR_CTX_HEADER_SIZE_NO_CIP;
 	} else {
 		dir = DMA_TO_DEVICE;
 		type = FW_ISO_CONTEXT_TRANSMIT;
 		ctx_header_size = 0;	// No effect for IT context.
 	}
+
+	max_ctx_payload_size = amdtp_stream_get_max_payload(s) -
+			       ctx_header_size;
+
 	err = iso_packets_buffer_init(&s->buffer, s->unit, QUEUE_LENGTH,
-				      amdtp_stream_get_max_payload(s), dir);
+				      max_ctx_payload_size, dir);
 	if (err < 0)
 		goto err_unlock;
 
@@ -934,8 +950,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 	amdtp_stream_update(s);
 
 	if (s->direction == AMDTP_IN_STREAM) {
-		s->ctx_data.tx.max_payload_length =
-						amdtp_stream_get_max_payload(s);
+		s->ctx_data.tx.max_ctx_payload_length = max_ctx_payload_size;
 		s->ctx_data.tx.ctx_header_size = ctx_header_size;
 	}
 
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index 5aa9683593d2..234483a31df5 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -116,7 +116,7 @@ struct amdtp_stream {
 			unsigned int ctx_header_size;
 
 			// limit for payload of iso packet.
-			unsigned int max_payload_length;
+			unsigned int max_ctx_payload_length;
 
 			// For quirks of CIP headers.
 			// Fixed interval of dbc between previos/current
-- 
2.20.1

[PATCH 6/6] ALSA: firewire-lib: unify packet handler for IR context

[Takashi Sakamoto]

Usage of 16 bytes IR context header allows to handle context payload by
the same code for with-CIP and without-CIP packets.

This commit unifies both handlers of with-CIP and without-CIP packets.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 54 ++++++++++-------------------------
 1 file changed, 15 insertions(+), 39 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index fa99210f5a48..2d9c764061d1 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -676,13 +676,20 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 	}
 
 	cip_header = ctx_header + 2;
-	err = check_cip_header(s, cip_header, payload_length, &data_blocks,
-			       &syt);
-	if (err < 0) {
-		if (err != -EAGAIN)
-			return err;
-		pcm_frames = 0;
-		goto end;
+	if (!(s->flags & CIP_NO_HEADER)) {
+		cip_header = &ctx_header[2];
+		err = check_cip_header(s, cip_header, payload_length,
+				       &data_blocks, &syt);
+		if (err < 0) {
+			if (err != -EAGAIN)
+				return err;
+			pcm_frames = 0;
+			goto end;
+		}
+	} else {
+		cip_header = NULL;
+		data_blocks = payload_length / 4 / s->data_block_quadlets;
+		syt = 0;
 	}
 
 	trace_amdtp_packet(s, cycle, cip_header, payload_length, data_blocks,
@@ -700,33 +707,6 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
 	return 0;
 }
 
-static int handle_in_packet_without_header(struct amdtp_stream *s,
-				unsigned int cycle, const __be32 *ctx_header,
-				__be32 *buffer, unsigned int index)
-{
-	unsigned int payload_length;
-	unsigned int data_blocks;
-	struct snd_pcm_substream *pcm;
-	unsigned int pcm_frames;
-
-	payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT;
-	data_blocks = payload_length / 4 / s->data_block_quadlets;
-
-	trace_amdtp_packet(s, cycle, NULL, payload_length, data_blocks, index);
-
-	pcm_frames = s->process_data_blocks(s, buffer, data_blocks, NULL);
-	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
-
-	if (queue_in_packet(s) < 0)
-		return -EIO;
-
-	pcm = READ_ONCE(s->pcm);
-	if (pcm && pcm_frames > 0)
-		update_pcm_pointers(s, pcm, pcm_frames);
-
-	return 0;
-}
-
 // In CYCLE_TIMER register of IEEE 1394, 7 bits are used to represent second. On
 // the other hand, in DMA descriptors of 1394 OHCI, 3 bits are used to represent
 // it. Thus, via Linux firewire subsystem, we can get the 3 bits for second.
@@ -812,7 +792,7 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,
 		cycle = compute_cycle_count(ctx_header[1]);
 		buffer = s->buffer.packets[s->packet_index].buffer;
 
-		if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0)
+		if (handle_in_packet(s, cycle, ctx_header, buffer, i) < 0)
 			break;
 
 		ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header);
@@ -847,10 +827,6 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context,
 		cycle = compute_cycle_count(ctx_header[1]);
 
 		context->callback.sc = in_stream_callback;
-		if (s->flags & CIP_NO_HEADER)
-			s->handle_packet = handle_in_packet_without_header;
-		else
-			s->handle_packet = handle_in_packet;
 	} else {
 		cycle = compute_it_cycle(*ctx_header);
 
-- 
2.20.1

Re: [PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[Takashi Iwai]

On Wed, 22 May 2019 16:17:02 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> In IR context of Linux FireWire subsystem, some quadlets of packet
> payload can be handled as a part of context header. As a result context
> payload can just include the rest of packet payload.
> 
> This patchset uses the mechanism to unify handlers of incoming packet
> for with-CIP and without-CIP headers.
> 
> Takashi Sakamoto (6):
>   ALSA: firewire-lib: use clear name for variable of CIP header
>   ALSA: firewire-lib: calculate the length of packet payload in packet
>     handler
>   ALSA: firewire-lib: compute pointer to payload buffer in context
>     handler
>   ALSA: firewire-lib: split helper function to check incoming CIP header
>   ALSA: firewire-lib: use 16 bytes IR context header to separate CIP
>     header
>   ALSA: firewire-lib: unify packet handler for IR context

Applied all six patches now.  Thanks.


Takashi

Re: [PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[Takashi Iwai]

On Wed, 22 May 2019 16:17:02 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> In IR context of Linux FireWire subsystem, some quadlets of packet
> payload can be handled as a part of context header. As a result context
> payload can just include the rest of packet payload.
> 
> This patchset uses the mechanism to unify handlers of incoming packet
> for with-CIP and without-CIP headers.
> 
> Takashi Sakamoto (6):
>   ALSA: firewire-lib: use clear name for variable of CIP header
>   ALSA: firewire-lib: calculate the length of packet payload in packet
>     handler
>   ALSA: firewire-lib: compute pointer to payload buffer in context
>     handler
>   ALSA: firewire-lib: split helper function to check incoming CIP header
>   ALSA: firewire-lib: use 16 bytes IR context header to separate CIP
>     header
>   ALSA: firewire-lib: unify packet handler for IR context

I already applied the previous patchset that had been submitted on
Tuesday, and this seems conflicting.

Could you rebase and resubmit the additional fixes?


thanks,

Takashi

Re: [PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[Takashi Iwai]

On Fri, 24 May 2019 07:58:47 +0200,
Takashi Iwai wrote:
> 
> On Wed, 22 May 2019 16:17:02 +0200,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > In IR context of Linux FireWire subsystem, some quadlets of packet
> > payload can be handled as a part of context header. As a result context
> > payload can just include the rest of packet payload.
> > 
> > This patchset uses the mechanism to unify handlers of incoming packet
> > for with-CIP and without-CIP headers.
> > 
> > Takashi Sakamoto (6):
> >   ALSA: firewire-lib: use clear name for variable of CIP header
> >   ALSA: firewire-lib: calculate the length of packet payload in packet
> >     handler
> >   ALSA: firewire-lib: compute pointer to payload buffer in context
> >     handler
> >   ALSA: firewire-lib: split helper function to check incoming CIP header
> >   ALSA: firewire-lib: use 16 bytes IR context header to separate CIP
> >     header
> >   ALSA: firewire-lib: unify packet handler for IR context
> 
> I already applied the previous patchset that had been submitted on
> Tuesday, and this seems conflicting.

Sorry, I replied to the wrong thread.  I meant the new one
  <20190523151440.5127-1-o-takashi@sakamocchi.jp>
conflicting with the current for-next branch.


Takashi

> 
> Could you rebase and resubmit the additional fixes?
> 
> 
> thanks,
> 
> Takashi

Re: [PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet

[Takashi Sakamoto]

On Fri, May 24, 2019 at 08:04:06AM +0200, Takashi Iwai wrote:
> > I already applied the previous patchset that had been submitted on
> > Tuesday, and this seems conflicting.
> 
> Sorry, I replied to the wrong thread.  I meant the new one
>   <20190523151440.5127-1-o-takashi@sakamocchi.jp>
> conflicting with the current for-next branch.

It's the latest one:

[alsa-devel] [PATCH 0/4] ALSA: firewire-lib: unify handlers for outgoing packet
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-May/149708.html

I'll check them and resubmit after rebasing to current for-next.


Thanks

Takashi Sakamoto