[PATCH] firewire: debug AT, AR, and selfID-complete events

[PATCH] firewire: debug AT, AR, and selfID-complete events

[Stefan Richter]

This adds debug printks for asynchronous transmission and reception and
for self ID reception.  The debug code is per default deactivated by #if
blocks.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

I am undecided whether this is OK for mainline, and whether the debug
options should become a Kconfig option or runtime options.

It may be temporarily useful to the few driver developers.  Do we want
end users to provide us with debug logs of this kind?  Then it should
be one or two runtime options.


 drivers/firewire/fw-ohci.c |  113 +++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -40,6 +40,9 @@
 #include "fw-ohci.h"
 #include "fw-transaction.h"
 
+#define FIREWIRE_OHCI_DEBUG_AT_AR	0
+#define FIREWIRE_OHCI_DEBUG_SELFIDS	0
+
 #define DESCRIPTOR_OUTPUT_MORE		0
 #define DESCRIPTOR_OUTPUT_LAST		(1 << 12)
 #define DESCRIPTOR_INPUT_MORE		(2 << 12)
@@ -316,6 +319,71 @@ static int ar_context_add_page(struct ar
 	return 0;
 }
 
+#if FIREWIRE_OHCI_DEBUG_AT_AR
+static void debug_ar_at_event(char dir, int speed, u32 *header, int evt)
+{
+	static const char *evts[] = {
+		[0x00] = "evt_no_status",	[0x01] = "-reserved-",
+		[0x02] = "evt_long_packet",	[0x03] = "evt_missing_ack",
+		[0x04] = "evt_underrun",	[0x05] = "evt_overrun",
+		[0x06] = "evt_descriptor_read",	[0x07] = "evt_data_read",
+		[0x08] = "evt_data_write",	[0x09] = "evt_bus_reset",
+		[0x0a] = "evt_timeout",		[0x0b] = "evt_tcode_err",
+		[0x0c] = "-reserved-",		[0x0d] = "-reserved-",
+		[0x0e] = "evt_unknown",		[0x0f] = "evt_flushed",
+		[0x10] = "-reserved-",		[0x11] = "ack_complete",
+		[0x12] = "ack_pending ",	[0x13] = "-reserved-",
+		[0x14] = "ack_busy_X",		[0x15] = "ack_busy_A",
+		[0x16] = "ack_busy_B",		[0x17] = "-reserved-",
+		[0x18] = "-reserved-",		[0x19] = "-reserved-",
+		[0x1a] = "-reserved-",		[0x1b] = "ack_tardy",
+		[0x1c] = "-reserved-",		[0x1d] = "ack_data_error",
+		[0x1e] = "ack_type_error",	[0x1f] = "-reserved-",
+	};
+	static const char *tcodes[] = {
+		[0x0] = "quadlet write request", [0x1] = "block write request",
+		[0x2] = "write response",	 [0x3] = "-reserved-",
+		[0x4] = "quadlet read request",	 [0x5] = "block read request",
+		[0x6] = "quadlet read response", [0x7] = "block read response",
+		[0x8] = "cycle start",		 [0x9] = "lock request",
+		[0xa] = "async stream packet",	 [0xb] = "lock response",
+		[0xc] = "-reserved-",		 [0xd] = "-reserved-",
+		[0xe] = "link internal",	 [0xf] = "reserved",
+	};
+	static const char *phys[] = {
+		[0x0] = "phy config packet",	 [0x1] = "link-on packet",
+		[0x2] = "self-id packet",	 [0x3] = "-reserved-",
+	};
+	int tcode = header[0] >> 4 & 0xf;
+
+	evt &= 0x1f;
+	if (header[0] == ~header[1])
+		printk(KERN_DEBUG "A%c evt %02x: %s, %s, %08x\n",
+		       dir, evt, evts[evt], phys[header[0] >> 30 & 0x3],
+		       header[0]);
+	else if (tcode == 0xe)
+		printk(KERN_DEBUG "A%c evt %02x: %s, %s\n",
+		       dir, evt, evts[evt], tcodes[tcode]);
+	else if (tcode == 0x0 || tcode == 0x1 ||
+		 tcode == 0x4 || tcode == 0x5 || tcode == 0x9)
+		printk(KERN_DEBUG "A%c evt %02x: tcode %x spd %x tl %02x "
+		       "src %04x dest %04x: "
+		       "%s, %s, %04x%08x\n",
+		       dir, evt, tcode, speed, header[0] >> 10 & 0x3f,
+		       header[1] >> 16, header[0] >> 16,
+		       evts[evt], tcodes[tcode], header[1] & 0xffff, header[2]);
+	else
+		printk(KERN_DEBUG "A%c evt %02x: tcode %x spd %x tl %02x "
+		       "src %04x dest %04x: "
+		       "%s, %s\n",
+		       dir, evt, tcode, speed, header[0] >> 10 & 0x3f,
+		       header[1] >> 16, header[0] >> 16,
+		       evts[evt], tcodes[tcode]);
+}
+#else
+#define debug_ar_at_event(dir, speed, header, evt)
+#endif /* FIREWIRE_OHCI_DEBUG_AT_AR */
+
 #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
 #define cond_le32_to_cpu(v) \
 	(ohci->old_uninorth ? (__force __u32)(v) : le32_to_cpu(v))
@@ -376,6 +444,8 @@ static __le32 *handle_ar_packet(struct a
 	p.timestamp  = status & 0xffff;
 	p.generation = ohci->request_generation;
 
+	debug_ar_at_event('R', p.speed, p.header, status >> 16 & 0x1f);
+
 	/*
 	 * The OHCI bus reset handler synthesizes a phy packet with
 	 * the new generation number when a bus reset happens (see
@@ -824,6 +894,8 @@ static int handle_at_packet(struct conte
 	evt = le16_to_cpu(last->transfer_status) & 0x1f;
 	packet->timestamp = le16_to_cpu(last->res_count);
 
+	debug_ar_at_event('T', packet->speed, packet->header, evt);
+
 	switch (evt) {
 	case OHCI1394_evt_timeout:
 		/* Async response transmit timed out. */
@@ -1005,6 +1077,45 @@ at_context_transmit(struct context *ctx,
 
 }
 
+#if FIREWIRE_OHCI_DEBUG_SELFIDS
+static char _p(u32 *s, int shift)
+{
+	static const char port_connected[] = { '.', '-', 'p', 'c', };
+
+	return port_connected[*s >> shift & 3];
+}
+
+static void debug_selfid_complete_event(int self_id_count, u32 *s)
+{
+	static const char *speed[] = {
+		[0] = "S100", [1] = "S200", [2] = "S400",    [3] = "beta",
+	};
+	static const char *power[] = {
+		[0] = "+0W",  [1] = "+15W", [2] = "+30W",    [3] = "+45W",
+		[4] = "-3W",  [5] = " ?W",  [6] = "-3..-6W", [7] = "-3..-10W",
+	};
+
+	for (; self_id_count--; ++s)
+		if ((*s & 1 << 23) == 0)
+			printk(KERN_DEBUG "selfID 0: %08x, phy %d [%c%c%c] "
+			       "%s gc=%d %s %s%s%s%s\n",
+			       *s, *s >> 24 & 63, _p(s, 6), _p(s, 4), _p(s, 2),
+			       speed[*s >> 14 & 3], *s >> 16 & 63,
+			       power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "",
+			       *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : "",
+			       *s & 1 ? "..." : "");
+		else
+			printk(KERN_DEBUG "selfID n: %08x, phy %d "
+			       "[%c%c%c%c%c%c%c%c]%s\n",
+			       *s, *s >> 24 & 63,
+			       _p(s, 16), _p(s, 14), _p(s, 12), _p(s, 10),
+			       _p(s,  8), _p(s,  6), _p(s,  4), _p(s,  2),
+			       *s & 1 ? "..." : "");
+}
+#else
+#define debug_selfid_complete_event(self_id_count, s)
+#endif /* FIREWIRE_OHCI_DEBUG_SELFIDS */
+
 static void bus_reset_tasklet(unsigned long data)
 {
 	struct fw_ohci *ohci = (struct fw_ohci *)data;
@@ -1115,6 +1226,8 @@ static void bus_reset_tasklet(unsigned l
 		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
 				  free_rom, free_rom_bus);
 
+	debug_selfid_complete_event(self_id_count, ohci->self_id_buffer);
+
 	fw_core_handle_bus_reset(&ohci->card, ohci->node_id, generation,
 				 self_id_count, ohci->self_id_buffer);
 }

-- 
Stefan Richter
-=====-==--- --== -=--=
http://arcgraph.de/sr/

Re: [PATCH] firewire: debug AT, AR, and selfID-complete events

[Jarod Wilson]

On Sunday 09 March 2008 04:49:34 pm Stefan Richter wrote:
> This adds debug printks for asynchronous transmission and reception and
> for self ID reception.  The debug code is per default deactivated by #if
> blocks.
>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
>
> I am undecided whether this is OK for mainline, and whether the debug
> options should become a Kconfig option or runtime options.
>
> It may be temporarily useful to the few driver developers.  Do we want
> end users to provide us with debug logs of this kind?  Then it should
> be one or two runtime options.


My vote would be to include this sort of thing with a run-time option to 
enable the extra spew, since its a perfectly accepted standard for other 
drivers. It ought to be extremely useful in determining whats going wrong 
with end-user setups, particularly in cases where we can't reproduce the 
problem (typically for lack of having the specific hardware in-house).


-- 
Jarod Wilson
jwilson@redhat.com

[PATCH update] firewire: debug AT, AR, and selfID-complete events

[Stefan Richter]

On 10 Mar, Jarod Wilson wrote:
> My vote would be to include this sort of thing with a run-time option to 
> enable the extra spew, since its a perfectly accepted standard for other 
> drivers. It ought to be extremely useful in determining whats going wrong 
> with end-user setups, particularly in cases where we can't reproduce the 
> problem (typically for lack of having the specific hardware in-house).


From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: firewire: debug AT, AR, and selfID-complete events

This adds debug printks for asynchronous transmission and reception and
for self ID reception.  They can be enabled at module load time, and at
runtime via /sys/module/firewire_ohci/parameters/debug.

This code inflates firewire-ohci.ko by 6 kB = 24% on x86-64 and
by 4 kB = 20% on i686.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

Update:  Converted into runtime option, shortened some strings, added
payload dump of quadlet transactions and of selfID generation.

 drivers/firewire/fw-ohci.c |  139 +++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -27,6 +27,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
 
@@ -40,6 +41,16 @@
 #include "fw-ohci.h"
 #include "fw-transaction.h"
 
+#define OHCI_PARAM_DEBUG_SELFIDS     1
+#define OHCI_PARAM_DEBUG_AT_AR       2
+
+static int param_debug;
+module_param_named(debug, param_debug, int, 0644);
+MODULE_PARM_DESC(debug, "Verbose logging (default = 0"
+	", self-IDs = "		__stringify(OHCI_PARAM_DEBUG_SELFIDS)
+	", AT and AR events = "	__stringify(OHCI_PARAM_DEBUG_AT_AR)
+	", or a combination)");
+
 #define DESCRIPTOR_OUTPUT_MORE		0
 #define DESCRIPTOR_OUTPUT_LAST		(1 << 12)
 #define DESCRIPTOR_INPUT_MORE		(2 << 12)
@@ -316,6 +327,88 @@ static int ar_context_add_page(struct ar
 	return 0;
 }
 
+static const char *evts[] = {
+	[0x00] = "evt_no_status",	[0x01] = "-reserved-",
+	[0x02] = "evt_long_packet",	[0x03] = "evt_missing_ack",
+	[0x04] = "evt_underrun",	[0x05] = "evt_overrun",
+	[0x06] = "evt_descriptor_read",	[0x07] = "evt_data_read",
+	[0x08] = "evt_data_write",	[0x09] = "evt_bus_reset",
+	[0x0a] = "evt_timeout",		[0x0b] = "evt_tcode_err",
+	[0x0c] = "-reserved-",		[0x0d] = "-reserved-",
+	[0x0e] = "evt_unknown",		[0x0f] = "evt_flushed",
+	[0x10] = "-reserved-",		[0x11] = "ack_complete",
+	[0x12] = "ack_pending ",	[0x13] = "-reserved-",
+	[0x14] = "ack_busy_X",		[0x15] = "ack_busy_A",
+	[0x16] = "ack_busy_B",		[0x17] = "-reserved-",
+	[0x18] = "-reserved-",		[0x19] = "-reserved-",
+	[0x1a] = "-reserved-",		[0x1b] = "ack_tardy",
+	[0x1c] = "-reserved-",		[0x1d] = "ack_data_error",
+	[0x1e] = "ack_type_error",	[0x1f] = "-reserved-",
+};
+static const char *tcodes[] = {
+	[0x0] = "QW req",		[0x1] = "BW req",
+	[0x2] = "W resp",		[0x3] = "-reserved-",
+	[0x4] = "QR req",		[0x5] = "BR req",
+	[0x6] = "QR resp",		[0x7] = "BR resp",
+	[0x8] = "cycle start",		[0x9] = "Lk req",
+	[0xa] = "async stream packet",	[0xb] = "Lk resp",
+	[0xc] = "-reserved-",		[0xd] = "-reserved-",
+	[0xe] = "link internal",	[0xf] = "-reserved-",
+};
+static const char *phys[] = {
+	[0x0] = "phy config packet",	[0x1] = "link-on packet",
+	[0x2] = "self-id packet",	[0x3] = "-reserved-",
+};
+
+static void debug_ar_at_event(char dir, int speed, u32 *header, int evt)
+{
+	int tcode = header[0] >> 4 & 0xf;
+	char specific[12];
+
+	evt &= 0x1f;
+	if (header[0] == ~header[1]) {
+		printk(KERN_DEBUG "A%c %s, %s, %08x\n",
+		       dir, evts[evt], phys[header[0] >> 30 & 0x3],
+		       header[0]);
+		return;
+	}
+
+	switch (tcode) {
+	case 0x0: case 0x6: case 0x8:
+		snprintf(specific, sizeof(specific), " = %08x",
+			 be32_to_cpu((__force __be32)header[3]));
+		break;
+	case 0x1: case 0x5: case 0x7: case 0x9: case 0xb:
+		snprintf(specific, sizeof(specific), " %x,%x",
+			 header[3] >> 16, header[3] & 0xffff);
+		break;
+	default:
+		specific[0] = '\0';
+	}
+
+	switch (tcode) {
+	case 0xe: case 0xa:
+		printk(KERN_DEBUG "A%c %s, %s\n",
+		       dir, evts[evt], tcodes[tcode]);
+		break;
+	case 0x0: case 0x1: case 0x4: case 0x5: case 0x9:
+		printk(KERN_DEBUG "A%c spd %x tl %02x, "
+		       "%04x -> %04x, %s, "
+		       "%s, %04x%08x%s\n",
+		       dir, speed, header[0] >> 10 & 0x3f,
+		       header[1] >> 16, header[0] >> 16, evts[evt],
+		       tcodes[tcode], header[1] & 0xffff, header[2], specific);
+		break;
+	default:
+		printk(KERN_DEBUG "A%c spd %x tl %02x, "
+		       "%04x -> %04x, %s, "
+		       "%s%s\n",
+		       dir, speed, header[0] >> 10 & 0x3f,
+		       header[1] >> 16, header[0] >> 16, evts[evt],
+		       tcodes[tcode], specific);
+	}
+}
+
 #if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
 #define cond_le32_to_cpu(v) \
 	(ohci->old_uninorth ? (__force __u32)(v) : le32_to_cpu(v))
@@ -376,6 +469,9 @@ static __le32 *handle_ar_packet(struct a
 	p.timestamp  = status & 0xffff;
 	p.generation = ohci->request_generation;
 
+	if (unlikely(param_debug & OHCI_PARAM_DEBUG_AT_AR))
+		debug_ar_at_event('R', p.speed, p.header, status >> 16 & 0x1f);
+
 	/*
 	 * The OHCI bus reset handler synthesizes a phy packet with
 	 * the new generation number when a bus reset happens (see
@@ -824,6 +920,9 @@ static int handle_at_packet(struct conte
 	evt = le16_to_cpu(last->transfer_status) & 0x1f;
 	packet->timestamp = le16_to_cpu(last->res_count);
 
+	if (unlikely(param_debug & OHCI_PARAM_DEBUG_AT_AR))
+		debug_ar_at_event('T', packet->speed, packet->header, evt);
+
 	switch (evt) {
 	case OHCI1394_evt_timeout:
 		/* Async response transmit timed out. */
@@ -1005,6 +1104,43 @@ at_context_transmit(struct context *ctx,
 
 }
 
+static const char *speed[] = {
+	[0] = "S100", [1] = "S200", [2] = "S400",    [3] = "beta",
+};
+static const char *power[] = {
+	[0] = "+0W",  [1] = "+15W", [2] = "+30W",    [3] = "+45W",
+	[4] = "-3W",  [5] = " ?W",  [6] = "-3..-6W", [7] = "-3..-10W",
+};
+static const char port[] = { '.', '-', 'p', 'c', };
+
+static char _p(u32 *s, int shift)
+{
+	return port[*s >> shift & 3];
+}
+
+static void debug_selfids(int generation, int self_id_count, u32 *s)
+{
+	printk(KERN_DEBUG KBUILD_MODNAME ": %d selfIDs, generation %d\n",
+	       self_id_count, generation);
+
+	for (; self_id_count--; ++s)
+		if ((*s & 1 << 23) == 0)
+			printk(KERN_DEBUG "selfID 0: %08x, phy %d [%c%c%c] "
+			       "%s gc=%d %s %s%s%s%s\n",
+			       *s, *s >> 24 & 63, _p(s, 6), _p(s, 4), _p(s, 2),
+			       speed[*s >> 14 & 3], *s >> 16 & 63,
+			       power[*s >> 8 & 7], *s >> 22 & 1 ? "L" : "",
+			       *s >> 11 & 1 ? "c" : "", *s & 2 ? "i" : "",
+			       *s & 1 ? "..." : "");
+		else
+			printk(KERN_DEBUG "selfID n: %08x, phy %d "
+			       "[%c%c%c%c%c%c%c%c]%s\n",
+			       *s, *s >> 24 & 63,
+			       _p(s, 16), _p(s, 14), _p(s, 12), _p(s, 10),
+			       _p(s,  8), _p(s,  6), _p(s,  4), _p(s,  2),
+			       *s & 1 ? "..." : "");
+}
+
 static void bus_reset_tasklet(unsigned long data)
 {
 	struct fw_ohci *ohci = (struct fw_ohci *)data;
@@ -1115,6 +1251,9 @@ static void bus_reset_tasklet(unsigned l
 		dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
 				  free_rom, free_rom_bus);
 
+	if (unlikely(param_debug & OHCI_PARAM_DEBUG_SELFIDS))
+		debug_selfids(generation, self_id_count, ohci->self_id_buffer);
+
 	fw_core_handle_bus_reset(&ohci->card, ohci->node_id, generation,
 				 self_id_count, ohci->self_id_buffer);
 }


-- 
Stefan Richter
-=====-==--- --== -=-=-
http://arcgraph.de/sr/

[PATCH] firewire: fw-ohci: untangle a mixed unsigned/signed expression

[Stefan Richter]

My old C textbook says that the result of casting an unsigned to signed
is undefined if the former's value can't be represented in the latter's
type.  Maybe that book is too old...?

Anyway, this change makes it more explicit what is going on, i.e. that
p.ack is meant to become negative on some occasions.  (p.ack is an int.)
fw-ohci.s does actually not change due to 1st and 2nd hunk of this patch
(with gcc 4.1.2 i686 -O2).

The 3rd hunk makes an expression more readable.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---

applies after patch "firewire: debug AT, AR, and selfID-complete events"

 drivers/firewire/fw-ohci.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -421,6 +421,7 @@ static __le32 *handle_ar_packet(struct a
 	struct fw_ohci *ohci = ctx->ohci;
 	struct fw_packet p;
 	u32 status, length, tcode;
+	int evt;
 
 	p.header[0] = cond_le32_to_cpu(buffer[0]);
 	p.header[1] = cond_le32_to_cpu(buffer[1]);
@@ -464,13 +465,14 @@ static __le32 *handle_ar_packet(struct a
 	length = (p.header_length + p.payload_length + 3) / 4;
 	status = cond_le32_to_cpu(buffer[length]);
 
-	p.ack        = ((status >> 16) & 0x1f) - 16;
+	evt          = (status >> 16) & 0x1f;
+	p.ack        = evt - 16;
 	p.speed      = (status >> 21) & 0x7;
 	p.timestamp  = status & 0xffff;
 	p.generation = ohci->request_generation;
 
 	if (unlikely(param_debug & OHCI_PARAM_DEBUG_AT_AR))
-		debug_ar_at_event('R', p.speed, p.header, status >> 16 & 0x1f);
+		debug_ar_at_event('R', p.speed, p.header, evt);
 
 	/*
 	 * The OHCI bus reset handler synthesizes a phy packet with
@@ -482,7 +484,7 @@ static __le32 *handle_ar_packet(struct a
 	 * request.
 	 */
 
-	if (p.ack + 16 == 0x09)
+	if (evt == OHCI1394_evt_bus_reset)
 		ohci->request_generation = (p.header[2] >> 16) & 0xff;
 	else if (ctx == &ohci->ar_request_ctx)
 		fw_core_handle_request(&ohci->card, &p);

-- 
Stefan Richter
-=====-==--- --== -=-==
http://arcgraph.de/sr/

Re: [PATCH update] firewire: debug AT, AR, and selfID-complete events

[Stefan Richter]

I wrote:
> +MODULE_PARM_DESC(debug, "Verbose logging (default = 0"
> +	", self-IDs = "		__stringify(OHCI_PARAM_DEBUG_SELFIDS)
> +	", AT and AR events = "	__stringify(OHCI_PARAM_DEBUG_AT_AR)
> +	", or a combination)");

Interrupt event logging needs to be added as well.
-- 
Stefan Richter
-=====-==--- --== -==--
http://arcgraph.de/sr/

Re: [PATCH update] firewire: debug AT, AR, and selfID-complete events

[Jarod Wilson]

On Wednesday 12 March 2008 05:15:42 am Stefan Richter wrote:
> I wrote:
> > +MODULE_PARM_DESC(debug, "Verbose logging (default = 0"
> > +	", self-IDs = "		__stringify(OHCI_PARAM_DEBUG_SELFIDS)
> > +	", AT and AR events = "	__stringify(OHCI_PARAM_DEBUG_AT_AR)
> > +	", or a combination)");
>
> Interrupt event logging needs to be added as well.

Want to do that as a later follow-up patch, or roll it into this one and post 
another update? (I'm fully prepared to sign off on either way after having 
extensively used these patches the past several days...)

-- 
Jarod Wilson
jwilson@redhat.com

Re: [PATCH update] firewire: debug AT, AR, and selfID-complete events

[Stefan Richter]

Jarod Wilson wrote:
> Want to do that as a later follow-up patch, or roll it into this one and post 
> another update?

I haven't committed to linux1394-2.6.git, hence I prefer to post an
update rather than incremental patch.

> (I'm fully prepared to sign off on either way after having 
> extensively used these patches the past several days...)

BTW, if I get sign-offs/ acks/ test feedback on a patch before another
patch update (but not before committing the final version to the
upstream submission branch), I add those sign-offs etc. like this

    S-o-b: Someone
    S-o-b: Me (modified whitespace or something else trivial)

or this

    Initial changelog

    S-o-b: Someone

    Changelog post scriptum

    S-o-b: Me

provided that the update only marginally changes the patch or can be
clearly distinguished from the formerly signed off parts.  Of course if
I get sign-offs etc. on the final version of the patch
-- 
Stefan Richter
-=====-==--- --== -==--
http://arcgraph.de/sr/

Re: [PATCH update] firewire: debug AT, AR, and selfID-complete events

[Jarod Wilson]

On Wednesday 12 March 2008 12:01:17 pm Stefan Richter wrote:
> Jarod Wilson wrote:
> > Want to do that as a later follow-up patch, or roll it into this one and
> > post another update?
>
> I haven't committed to linux1394-2.6.git, hence I prefer to post an
> update rather than incremental patch.
>
> > (I'm fully prepared to sign off on either way after having
> > extensively used these patches the past several days...)
>
> BTW, if I get sign-offs/ acks/ test feedback on a patch before another
> patch update (but not before committing the final version to the
> upstream submission branch), I add those sign-offs etc. like this
>
>     S-o-b: Someone
>     S-o-b: Me (modified whitespace or something else trivial)
>
> or this
>
>     Initial changelog
>
>     S-o-b: Someone
>
>     Changelog post scriptum
>
>     S-o-b: Me
>
> provided that the update only marginally changes the patch or can be
> clearly distinguished from the formerly signed off parts.  Of course if
> I get sign-offs etc. on the final version of the patch

In that case, for the version with module params to enable debugging:

Signed-off-by: Jarod Wilson <jwilson@redhat.com>


-- 
Jarod Wilson
jwilson@redhat.com