[GIT PULL] kdb / early debug (2 of 2)

[GIT PULL] kdb / early debug (2 of 2)

[Jason Wessel]

Linus please pull the dbg-early-merge tree which merges in the
capability for early debugging for the x86 architecture.

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git dbg-early-merge

--- Notes for Linus ---

I did not receive acks or feedback from any x86 maintainers at this
point for the early trap changes.  These changes came from the
archives from 2 years back, before the time the mainline kernel I/O
drivers supported early debugger access.

I did not receive acks or feedback on the changes to
arch/x86/kernel/cpu/common.c which covers the initial setup of the
debug registers.  This change is a fix vs a feature.

--- End Notes for Linus ---

This patch set stacks on top of the kgdb / kdb patches and focuses on
allowing the kernel debugger to activate at the same time earlyprintk
becomes available.  Right now early debugging is only available on
x86, but the API in the debug core is generic.  The only thing lacking
for other architecture support is appropriate atomic safe polled I/O
drivers.

The first few patches address the problems in kdb and the architecture
specific x86 kgdb implementation which rely on services which are not
available before the kernel memory allocators are setup.

The patch set includes early debug support for vga + non-USB keyboard
and dbgp devices.  For the vga case, it only works with kdb. For
example, you could boot your kernel and optionally include kgdbwait as
follows:

     earlyprintk=vga ekgboc=kbd kgdbwait

That will stop the kernel somewhere inside setup_arch() and you can
plant breakpoints, modify memory and backtrace.  If you omit the
kgdbwait, the kgdb shell prompt will appear on the vga console only if
there is a kernel fault.

The final 2 patches in the series add support for the EHCI usb debug
port when using a dbgp device.  You have the choice of using
earlyprintk= or using only the kernel debugger on the debug port.  You
can also use either kgdb or kdb with this connection.  Here are two
example kernel boot argument lines:

1) Used in conjunction with earlyprintk on usb debug port 1

   	earlyprintk=dbgp1,keep kgdbdbgp=1 kgdbwait

2) Allow for the capability to break into the kernel with control-c on
   usb debug port 0

        kgdbdbp=0,1 kgdbwait

A patch to add formal documentation to the kgdb docbook for these new
features will be available for the 2.6.34 cycle, because there are a
number of other ways you can make use of these features not described
here.

Thanks,
Jason.

---

The following changes since commit 1bcdae1065c6f01cf2d24a99a6d47b154bab4dfe:
  Jason Wessel (1):
        debug_core: Turn off tracing while in the debugger

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git dbg-early-merge

Jan Kiszka (1):
      x86, kgdb: early trap init for early debug

Jason Wessel (7):
      x86, kgdb, init: Add early and late debug states
      x86,kgdb: Implement early hardware breakpoint debugging
      x86,early dr regs,kgdb: Allow kernel debugger early dr register access
      kgdboc: Add ekgdboc for early use of the kernel debugger
      earlyprintk,vga,kdb: Fix \b and \r for earlyprintk=vga with kdb
      ehci-dbgp: split PID register updates for IN and OUT pipes
      echi-dbgp: Add kernel debugger support for the usb debug port

 Documentation/kernel-parameters.txt |   17 +++
 arch/x86/include/asm/processor.h    |    2 +
 arch/x86/kernel/cpu/common.c        |   29 +++---
 arch/x86/kernel/early_printk.c      |    8 ++
 arch/x86/kernel/kgdb.c              |   47 +++++++--
 arch/x86/kernel/setup.c             |    1 +
 arch/x86/kernel/traps.c             |   14 ++-
 drivers/serial/kgdboc.c             |   19 ++++
 drivers/usb/early/ehci-dbgp.c       |  184 ++++++++++++++++++++++++++---------
 include/linux/kgdb.h                |   14 +++
 init/main.c                         |    4 +-
 kernel/debug/debug_core.c           |   16 +++
 12 files changed, 281 insertions(+), 74 deletions(-)

[PATCH 1/8] x86, kgdb: early trap init for early debug

[Jason Wessel]

From: Jan Kiszka <jan.kiszka@web.de>

Allow the x86 arch to have early exception processing for the purpose
of debugging via the kgdb.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/include/asm/processor.h |    2 ++
 arch/x86/kernel/setup.c          |    1 +
 arch/x86/kernel/traps.c          |   14 ++++++++++----
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index b753ea5..543f48a 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -793,6 +793,8 @@ static inline void wbinvd_halt(void)
 extern void enable_sep_cpu(void);
 extern int sysenter_setup(void);
 
+extern void early_trap_init(void);
+
 /* Defined in head.S */
 extern struct desc_ptr		early_gdt_descr;
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 5d9e40c..63fff98 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -695,6 +695,7 @@ void __init setup_arch(char **cmdline_p)
 	/* VMI may relocate the fixmap; do this before touching ioremap area */
 	vmi_init();
 
+	early_trap_init();
 	early_cpu_init();
 	early_ioremap_init();
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 6ae2122..22468df 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -882,6 +882,16 @@ dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code)
 }
 #endif
 
+/* Set of traps needed for early debugging. */
+void __init early_trap_init(void)
+{
+	set_intr_gate_ist(1, &debug, DEBUG_STACK);
+	/* int3 can be called from all */
+	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
+	set_intr_gate(14, &page_fault);
+	load_idt(&idt_descr);
+}
+
 void __init trap_init(void)
 {
 	int i;
@@ -895,10 +905,7 @@ void __init trap_init(void)
 #endif
 
 	set_intr_gate(0, &divide_error);
-	set_intr_gate_ist(1, &debug, DEBUG_STACK);
 	set_intr_gate_ist(2, &nmi, NMI_STACK);
-	/* int3 can be called from all */
-	set_system_intr_gate_ist(3, &int3, DEBUG_STACK);
 	/* int4 can be called from all */
 	set_system_intr_gate(4, &overflow);
 	set_intr_gate(5, &bounds);
@@ -914,7 +921,6 @@ void __init trap_init(void)
 	set_intr_gate(11, &segment_not_present);
 	set_intr_gate_ist(12, &stack_segment, STACKFAULT_STACK);
 	set_intr_gate(13, &general_protection);
-	set_intr_gate(14, &page_fault);
 	set_intr_gate(15, &spurious_interrupt_bug);
 	set_intr_gate(16, &coprocessor_error);
 	set_intr_gate(17, &alignment_check);
-- 
1.6.4.rc1

[PATCH 2/8] x86, kgdb, init: Add early and late debug states

[Jason Wessel]

The kernel debugger can operate well before mm_init(), but the x86
hardware breakpoint code which uses the perf api requires that the
kernel allocators are initialized.

This means the kernel debug core needs to provide an optional arch
specific call back to allow the initialization functions to run after
the kernel has been further initialized.

The kdb shell already had a similar restriction with an early
initialization and late initialization.  The kdb_init() was moved into
the debug core's version of the late init which is called
dbg_late_init();

CC: kgdb-bugreport@lists.sourceforge.net
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/kernel/kgdb.c    |   17 ++++++++++-------
 include/linux/kgdb.h      |   14 ++++++++++++++
 init/main.c               |    4 ++--
 kernel/debug/debug_core.c |   16 ++++++++++++++++
 4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 3b762bc..896874a 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -596,14 +596,15 @@ static struct notifier_block kgdb_notifier = {
  */
 int kgdb_arch_init(void)
 {
+	return register_die_notifier(&kgdb_notifier);
+}
+
+void kgdb_arch_late(void)
+{
 	int i, cpu;
-	int ret;
 	struct perf_event_attr attr;
 	struct perf_event **pevent;
 
-	ret = register_die_notifier(&kgdb_notifier);
-	if (ret != 0)
-		return ret;
 	/*
 	 * Pre-allocate the hw breakpoint structions in the non-atomic
 	 * portion of kgdb because this operation requires mutexs to
@@ -615,12 +616,15 @@ int kgdb_arch_init(void)
 	attr.bp_type = HW_BREAKPOINT_W;
 	attr.disabled = 1;
 	for (i = 0; i < 4; i++) {
+		if (breakinfo[i].pev)
+			continue;
 		breakinfo[i].pev = register_wide_hw_breakpoint(&attr, NULL);
 		if (IS_ERR(breakinfo[i].pev)) {
-			printk(KERN_ERR "kgdb: Could not allocate hw breakpoints\n");
+			printk(KERN_ERR "kgdb: Could not allocate hw"
+			       "breakpoints\nDisabling the kernel debugger\n");
 			breakinfo[i].pev = NULL;
 			kgdb_arch_exit();
-			return -1;
+			return;
 		}
 		for_each_online_cpu(cpu) {
 			pevent = per_cpu_ptr(breakinfo[i].pev, cpu);
@@ -631,7 +635,6 @@ int kgdb_arch_init(void)
 			}
 		}
 	}
-	return ret;
 }
 
 /**
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 6c784ab..9340f34 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -208,6 +208,17 @@ extern int kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr);
 extern int kgdb_arch_remove_breakpoint(unsigned long addr, char *bundle);
 
 /**
+ *	kgdb_arch_late - Perform any architecture specific initalization.
+ *
+ *	This function will handle the late initalization of any
+ *	architecture specific callbacks.  This is an optional function for
+ *	handling things like late initialization of hw breakpoints.  The
+ *	default implementation does nothing.
+ */
+extern void kgdb_arch_late(void);
+
+
+/**
  * struct kgdb_arch - Describe architecture specific values.
  * @gdb_bpt_instr: The instruction to trigger a breakpoint.
  * @flags: Flags for the breakpoint, currently just %KGDB_HW_BREAKPOINT.
@@ -285,7 +296,10 @@ extern int			kgdb_single_step;
 extern atomic_t			kgdb_active;
 #define in_dbg_master() \
 	(raw_smp_processor_id() == atomic_read(&kgdb_active))
+extern bool dbg_is_early;
+extern void __init dbg_late_init(void);
 #else /* ! CONFIG_KGDB */
 #define in_dbg_master() (0)
+#define dbg_late_init()
 #endif /* ! CONFIG_KGDB */
 #endif /* _KGDB_H_ */
diff --git a/init/main.c b/init/main.c
index 9d415a1..1839e3f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -63,7 +63,7 @@
 #include <linux/sched.h>
 #include <linux/signal.h>
 #include <linux/idr.h>
-#include <linux/kdb.h>
+#include <linux/kgdb.h>
 #include <linux/ftrace.h>
 #include <linux/async.h>
 #include <linux/kmemcheck.h>
@@ -660,7 +660,7 @@ asmlinkage void __init start_kernel(void)
 	key_init();
 	radix_tree_init();
 	security_init();
-	kdb_init(KDB_INIT_FULL);
+	dbg_late_init();
 	vfs_caches_init(totalram_pages);
 	signals_init();
 	/* rootfs populating might need page-writeback */
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 3d36d9e..e7a4d9b 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -78,6 +78,8 @@ static DEFINE_SPINLOCK(kgdb_registration_lock);
 static int kgdb_con_registered;
 /* determine if kgdb console output should be used */
 static int kgdb_use_con;
+/* Flag for alternate operations for early debugging */
+bool dbg_is_early = true;
 /* Next cpu to become the master debug core */
 int dbg_switch_cpu;
 
@@ -777,11 +779,25 @@ static struct notifier_block kgdb_panic_event_nb = {
        .priority	= INT_MAX,
 };
 
+void __weak kgdb_arch_late(void)
+{
+}
+
+void __init dbg_late_init(void)
+{
+	dbg_is_early = false;
+	if (kgdb_io_module_registered)
+		kgdb_arch_late();
+	kdb_init(KDB_INIT_FULL);
+}
+
 static void kgdb_register_callbacks(void)
 {
 	if (!kgdb_io_module_registered) {
 		kgdb_io_module_registered = 1;
 		kgdb_arch_init();
+		if (!dbg_is_early)
+			kgdb_arch_late();
 		atomic_notifier_chain_register(&panic_notifier_list,
 					       &kgdb_panic_event_nb);
 #ifdef CONFIG_MAGIC_SYSRQ
-- 
1.6.4.rc1

[PATCH 3/8] x86,kgdb: Implement early hardware breakpoint debugging

[Jason Wessel]

It is not possible to use the hw_breakpoint.c API prior to mm_init(),
but it is possible to use hardware breakpoints with the kernel
debugger.

Prior to smp_init() it is possible to simply write to the dr registers
of the boot cpu directly.  This can be used up until the
kgdb_arch_late() is invoked, at which point the standard hw_breakpoint.c
API will get used.

CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/kernel/kgdb.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 896874a..8e79fea 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -199,6 +199,8 @@ static struct hw_breakpoint {
 	struct perf_event	**pev;
 } breakinfo[4];
 
+static unsigned long early_dr7;
+
 static void kgdb_correct_hw_break(void)
 {
 	int breakno;
@@ -210,6 +212,14 @@ static void kgdb_correct_hw_break(void)
 		int cpu = raw_smp_processor_id();
 		if (!breakinfo[breakno].enabled)
 			continue;
+		if (dbg_is_early) {
+			set_debugreg(breakinfo[breakno].addr, breakno);
+			early_dr7 |= encode_dr7(breakno,
+						breakinfo[breakno].len,
+						breakinfo[breakno].type);
+			set_debugreg(early_dr7, 7);
+			continue;
+		}
 		bp = *per_cpu_ptr(breakinfo[breakno].pev, cpu);
 		info = counter_arch_bp(bp);
 		if (bp->attr.disabled != 1)
@@ -224,7 +234,8 @@ static void kgdb_correct_hw_break(void)
 		if (!val)
 			bp->attr.disabled = 0;
 	}
-	hw_breakpoint_restore();
+	if (!dbg_is_early)
+		hw_breakpoint_restore();
 }
 
 static int hw_break_reserve_slot(int breakno)
@@ -233,6 +244,9 @@ static int hw_break_reserve_slot(int breakno)
 	int cnt = 0;
 	struct perf_event **pevent;
 
+	if (dbg_is_early)
+		return 0;
+
 	for_each_online_cpu(cpu) {
 		cnt++;
 		pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
@@ -258,6 +272,9 @@ static int hw_break_release_slot(int breakno)
 	struct perf_event **pevent;
 	int cpu;
 
+	if (dbg_is_early)
+		return 0;
+
 	for_each_online_cpu(cpu) {
 		pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
 		if (dbg_release_bp_slot(*pevent))
@@ -302,7 +319,11 @@ static void kgdb_remove_all_hw_break(void)
 		bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
 		if (bp->attr.disabled == 1)
 			continue;
-		arch_uninstall_hw_breakpoint(bp);
+		if (dbg_is_early)
+			early_dr7 &= ~encode_dr7(i, breakinfo[i].len,
+						 breakinfo[i].type);
+		else
+			arch_uninstall_hw_breakpoint(bp);
 		bp->attr.disabled = 1;
 	}
 }
@@ -379,6 +400,11 @@ void kgdb_disable_hw_debug(struct pt_regs *regs)
 	for (i = 0; i < 4; i++) {
 		if (!breakinfo[i].enabled)
 			continue;
+		if (dbg_is_early) {
+			early_dr7 &= ~encode_dr7(i, breakinfo[i].len,
+						 breakinfo[i].type);
+			continue;
+		}
 		bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
 		if (bp->attr.disabled == 1)
 			continue;
-- 
1.6.4.rc1

[PATCH 4/8] x86,early dr regs,kgdb: Allow kernel debugger early dr register access

[Jason Wessel]

If the kernel debugger was configured, attached and started with
kgdbwait, the hardware breakpoint registers should get restored by the
kgdb code which is managing the dr registers.

CC: x86@kernel.org
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/kernel/cpu/common.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4868e4a..712c1eb 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1084,6 +1084,20 @@ static void clear_all_debug_regs(void)
 	}
 }
 
+#ifdef CONFIG_KGDB
+/*
+ * Restore debug regs if using kgdbwait and you have a kernel debugger
+ * connection established.
+ */
+static void dbg_restore_debug_regs(void)
+{
+	if (unlikely(kgdb_connected && arch_kgdb_ops.correct_hw_break))
+		arch_kgdb_ops.correct_hw_break();
+}
+#else /* ! CONFIG_KGDB */
+#define dbg_restore_debug_regs()
+#endif /* ! CONFIG_KGDB */
+
 /*
  * cpu_init() initializes state that is per-CPU. Some data is already
  * initialized (naturally) in the bootstrap process, such as the GDT
@@ -1174,18 +1188,8 @@ void __cpuinit cpu_init(void)
 	load_TR_desc();
 	load_LDT(&init_mm.context);
 
-#ifdef CONFIG_KGDB
-	/*
-	 * If the kgdb is connected no debug regs should be altered.  This
-	 * is only applicable when KGDB and a KGDB I/O module are built
-	 * into the kernel and you are using early debugging with
-	 * kgdbwait. KGDB will control the kernel HW breakpoint registers.
-	 */
-	if (kgdb_connected && arch_kgdb_ops.correct_hw_break)
-		arch_kgdb_ops.correct_hw_break();
-	else
-#endif
-		clear_all_debug_regs();
+	clear_all_debug_regs();
+	dbg_restore_debug_regs();
 
 	fpu_init();
 
@@ -1239,6 +1243,7 @@ void __cpuinit cpu_init(void)
 #endif
 
 	clear_all_debug_regs();
+	dbg_restore_debug_regs();
 
 	/*
 	 * Force FPU initialization:
-- 
1.6.4.rc1

[PATCH 5/8] kgdboc: Add ekgdboc for early use of the kernel debugger

[Jason Wessel]

The ekgdboc= differs from kgdboc= in that you can begin debuggin as
soon as the exceptions are setup and the kgdb I/O driver is available,
instead of waiting until the tty subsystem is available.

CC: kgdb-bugreport@lists.sourceforge.net
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 Documentation/kernel-parameters.txt |    6 ++++++
 drivers/serial/kgdboc.c             |   19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c6936b2..aaeb992 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -708,6 +708,12 @@ and is between 256 and 4096 characters. It is defined in the file
 			The VGA output is eventually overwritten by the real
 			console.
 
+	ekgdboc=	[X86,KGDB] Allow early kernel console debugging
+			ekgdboc=kbd
+
+			This is desgined to be used in conjunction with
+			the boot argument: earlyprintk=vga
+
 	eata=		[HW,SCSI]
 
 	edd=		[EDD]
diff --git a/drivers/serial/kgdboc.c b/drivers/serial/kgdboc.c
index 201cdf5..e8557a5 100644
--- a/drivers/serial/kgdboc.c
+++ b/drivers/serial/kgdboc.c
@@ -236,6 +236,25 @@ static struct kgdb_io kgdboc_io_ops = {
 	.post_exception		= kgdboc_post_exp_handler,
 };
 
+#ifdef CONFIG_KGDB_SERIAL_CONSOLE
+/* This is only available if kgdboc is a built in for early debugging */
+int __init kgdboc_early_init(char *opt)
+{
+	/* save the first character of the config string because the
+	 * init routine can destroy it.
+	 */
+	char save_ch;
+
+	kgdboc_option_setup(opt);
+	save_ch = config[0];
+	init_kgdboc();
+	config[0] = save_ch;
+	return 0;
+}
+
+early_param("ekgdboc", kgdboc_early_init);
+#endif /* CONFIG_KGDB_SERIAL_CONSOLE */
+
 module_init(init_kgdboc);
 module_exit(cleanup_kgdboc);
 module_param_call(kgdboc, param_set_kgdboc_var, param_get_string, &kps, 0644);
-- 
1.6.4.rc1

[PATCH 6/8] earlyprintk,vga,kdb: Fix \b and \r for earlyprintk=vga with kdb

[Jason Wessel]

Allow kdb to work properly with with earlyprintk=vga by interpreting
the backspace and carriage return output characters.  These
interpretation of these characters is used for simple line editing
provided in the kdb shell.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: H. Peter Anvin <hpa@zytor.com>
CC: x86@kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 arch/x86/kernel/early_printk.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index b9c830c..fa99bae 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -41,6 +41,14 @@ static void early_vga_write(struct console *con, const char *str, unsigned n)
 				writew(0x720, VGABASE + 2*(max_xpos*j + i));
 			current_ypos = max_ypos-1;
 		}
+#ifdef CONFIG_KGDB_KDB
+		if (c == '\b') {
+			if (current_xpos > 0)
+				current_xpos--;
+		} else if (c == '\r') {
+			current_xpos = 0;
+		} else
+#endif
 		if (c == '\n') {
 			current_xpos = 0;
 			current_ypos++;
-- 
1.6.4.rc1

[PATCH 7/8] ehci-dbgp: split PID register updates for IN and OUT pipes

[Jason Wessel]

This patch addresses two problems:

1) Bulk reads should always use the DATA0 for the pid, and the write
   PID should toggle between DATA0 and DATA1.  The fix is using
   dbgp_pid_write_update() and dbgp_pid_read_update().

2) The delay loop for waiting for a transaction was not long enough to
   always complete the initial handshake inside dbgp_wait_until_done().
   After the initial handshake the maximum delay length is never reached.

The combined result of these two changes allows for the removal of the
forced resynchronization where a bulk write was issued with a dummy
data payload only to get the device to start accepting data writes
again.

CC: Eric Biederman <ebiederm@xmission.com>
CC: Yinghai Lu <yhlu.kernel@gmail.com>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/usb/early/ehci-dbgp.c |   68 +++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index 2958a12..6e98a36 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -66,8 +66,6 @@ static struct ehci_dev ehci_dev;
 
 #define USB_DEBUG_DEVNUM 127
 
-#define DBGP_DATA_TOGGLE	0x8800
-
 #ifdef DBGP_DEBUG
 #define dbgp_printk printk
 static void dbgp_ehci_status(char *str)
@@ -88,11 +86,6 @@ static inline void dbgp_ehci_status(char *str) { }
 static inline void dbgp_printk(const char *fmt, ...) { }
 #endif
 
-static inline u32 dbgp_pid_update(u32 x, u32 tok)
-{
-	return ((x ^ DBGP_DATA_TOGGLE) & 0xffff00) | (tok & 0xff);
-}
-
 static inline u32 dbgp_len_update(u32 x, u32 len)
 {
 	return (x & ~0x0f) | (len & 0x0f);
@@ -136,6 +129,19 @@ static inline u32 dbgp_len_update(u32 x, u32 len)
 
 #define DBGP_MAX_PACKET		8
 #define DBGP_TIMEOUT		(250 * 1000)
+#define DBGP_LOOPS		1000
+
+static inline u32 dbgp_pid_write_update(u32 x, u32 tok)
+{
+	static int data0 = USB_PID_DATA1;
+	data0 ^= USB_PID_DATA_TOGGLE;
+	return (x & 0xffff0000) | (data0 << 8) | (tok & 0xff);
+}
+
+static inline u32 dbgp_pid_read_update(u32 x, u32 tok)
+{
+	return (x & 0xffff0000) | (USB_PID_DATA0 << 8) | (tok & 0xff);
+}
 
 static int dbgp_wait_until_complete(void)
 {
@@ -180,7 +186,7 @@ static int dbgp_wait_until_done(unsigned ctrl)
 {
 	u32 pids, lpid;
 	int ret;
-	int loop = 3;
+	int loop = DBGP_LOOPS;
 
 retry:
 	writel(ctrl | DBGP_GO, &ehci_debug->control);
@@ -197,6 +203,8 @@ retry:
 		 */
 		if (ret == -DBGP_TIMEOUT && !dbgp_not_safe)
 			dbgp_not_safe = 1;
+		if (ret == -DBGP_ERR_BAD && --loop > 0)
+			goto retry;
 		return ret;
 	}
 
@@ -245,12 +253,20 @@ static inline void dbgp_get_data(void *buf, int size)
 		bytes[i] = (hi >> (8*(i - 4))) & 0xff;
 }
 
-static int dbgp_out(u32 addr, const char *bytes, int size)
+static int dbgp_bulk_write(unsigned devnum, unsigned endpoint,
+			 const char *bytes, int size)
 {
+	int ret;
+	u32 addr;
 	u32 pids, ctrl;
 
+	if (size > DBGP_MAX_PACKET)
+		return -1;
+
+	addr = DBGP_EPADDR(devnum, endpoint);
+
 	pids = readl(&ehci_debug->pids);
-	pids = dbgp_pid_update(pids, USB_PID_OUT);
+	pids = dbgp_pid_write_update(pids, USB_PID_OUT);
 
 	ctrl = readl(&ehci_debug->control);
 	ctrl = dbgp_len_update(ctrl, size);
@@ -260,34 +276,7 @@ static int dbgp_out(u32 addr, const char *bytes, int size)
 	dbgp_set_data(bytes, size);
 	writel(addr, &ehci_debug->address);
 	writel(pids, &ehci_debug->pids);
-	return dbgp_wait_until_done(ctrl);
-}
-
-static int dbgp_bulk_write(unsigned devnum, unsigned endpoint,
-			 const char *bytes, int size)
-{
-	int ret;
-	int loops = 5;
-	u32 addr;
-	if (size > DBGP_MAX_PACKET)
-		return -1;
-
-	addr = DBGP_EPADDR(devnum, endpoint);
-try_again:
-	if (loops--) {
-		ret = dbgp_out(addr, bytes, size);
-		if (ret == -DBGP_ERR_BAD) {
-			int try_loops = 3;
-			do {
-				/* Emit a dummy packet to re-sync communication
-				 * with the debug device */
-				if (dbgp_out(addr, "12345678", 8) >= 0) {
-					udelay(2);
-					goto try_again;
-				}
-			} while (try_loops--);
-		}
-	}
+	ret = dbgp_wait_until_done(ctrl);
 
 	return ret;
 }
@@ -304,7 +293,7 @@ static int dbgp_bulk_read(unsigned devnum, unsigned endpoint, void *data,
 	addr = DBGP_EPADDR(devnum, endpoint);
 
 	pids = readl(&ehci_debug->pids);
-	pids = dbgp_pid_update(pids, USB_PID_IN);
+	pids = dbgp_pid_read_update(pids, USB_PID_IN);
 
 	ctrl = readl(&ehci_debug->control);
 	ctrl = dbgp_len_update(ctrl, size);
@@ -362,7 +351,6 @@ static int dbgp_control_msg(unsigned devnum, int requesttype,
 	return dbgp_bulk_read(devnum, 0, data, size);
 }
 
-
 /* Find a PCI capability */
 static u32 __init find_cap(u32 num, u32 slot, u32 func, int cap)
 {
-- 
1.6.4.rc1

[PATCH 8/8] echi-dbgp: Add kernel debugger support for the usb debug port

[Jason Wessel]

This patch adds the capability to use the usb debug port with the
kernel debugger.  It is also still possible to use this functionality
with or without the earlyprintk=dbgpX.  It is possible to use the
kgdbwait boot argument to debug very early in the kernel start up code.

There are two ways to use this driver extension with a kernel boot argument.

1) kgdbdbgp=#   -- Where # is the number of the usb debug controller

   You must use sysrq-g to break into the kernel debugger on another
   connection type other than the dbgp.

2) kgdbdbgp=#debugControlNum#,#Seconds#

   In this mode, the usb debug port is polled every #Seconds# for
   character input.  It is possible to use gdb or press control-c to
   break into the kernel debugger.

>From the implementation perspective there are 3 high level changes.

1) Allow variable retries for the the hardware via dbgp_bulk_read().

   The amount of retries for the dbgp_bulk_read() needed to be
   variable instead of fixed.  We do not want to poll at all when the
   kernel is operating in interrupt driven mode.  The polling only
   occurs if the kernel was booted when specifying some number of
   seconds via the kgdbdbgp boot argument (IE kgdbdbgp=0,1).  In this
   case the loop count is reduced to 1 so as introduce the smallest
   amount of latency as possible.

2) Save the bulk IN endpoint address for use by the kgdb code.

3) The addition of the kgdb interface code.

   This consisted of adding in a character read function for the dbgp
   as well as a polling thread to allow the dbgp to interrupt the
   kernel execution.  The rest is the typical kgdb I/O api.

CC: Eric Biederman <ebiederm@xmission.com>
CC: Yinghai Lu <yhlu.kernel@gmail.com>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 Documentation/kernel-parameters.txt |   11 +++
 drivers/usb/early/ehci-dbgp.c       |  120 ++++++++++++++++++++++++++++++++---
 2 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index aaeb992..7616c88 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1118,6 +1118,17 @@ and is between 256 and 4096 characters. It is defined in the file
 			use the HighMem zone if it exists, and the Normal
 			zone if it does not.
 
+	kgdbdbgp=	[KGDB,HW] kgdb over EHCI usb debug port.
+			Format: <Controller#>[,poll interval]
+			The controller # is the number of the ehci usb debug
+			port as it is probed via PCI.  The poll interval is
+			optional and is the number seconds in between
+			each poll cycle to the debug port in case you need
+			the functionality for interrupting the kernel with
+			gdb or control-c on the dbgp connection.  When
+			not using this parameter you use sysrq-g to break into
+			the kernel debugger.
+
 	kgdboc=		[KGDB,HW] kgdb over consoles.
 			Requires a tty driver that supports console polling,
 			or a supported polling keyboard driver (non-usb).
diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index 6e98a36..94ecdbc 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -19,6 +19,9 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/ehci_def.h>
 #include <linux/delay.h>
+#include <linux/serial_core.h>
+#include <linux/kgdb.h>
+#include <linux/kthread.h>
 #include <asm/io.h>
 #include <asm/pci-direct.h>
 #include <asm/fixmap.h>
@@ -55,6 +58,7 @@ static struct ehci_regs __iomem *ehci_regs;
 static struct ehci_dbg_port __iomem *ehci_debug;
 static int dbgp_not_safe; /* Cannot use debug device during ehci reset */
 static unsigned int dbgp_endpoint_out;
+static unsigned int dbgp_endpoint_in;
 
 struct ehci_dev {
 	u32 bus;
@@ -91,6 +95,13 @@ static inline u32 dbgp_len_update(u32 x, u32 len)
 	return (x & ~0x0f) | (len & 0x0f);
 }
 
+#ifdef CONFIG_KGDB
+static struct kgdb_io kgdbdbgp_io_ops;
+#define dbgp_kgdb_mode (dbg_io_ops == &kgdbdbgp_io_ops)
+#else
+#define dbgp_kgdb_mode (0)
+#endif
+
 /*
  * USB Packet IDs (PIDs)
  */
@@ -182,11 +193,10 @@ static void dbgp_breath(void)
 	/* Sleep to give the debug port a chance to breathe */
 }
 
-static int dbgp_wait_until_done(unsigned ctrl)
+static int dbgp_wait_until_done(unsigned ctrl, int loop)
 {
 	u32 pids, lpid;
 	int ret;
-	int loop = DBGP_LOOPS;
 
 retry:
 	writel(ctrl | DBGP_GO, &ehci_debug->control);
@@ -276,13 +286,13 @@ static int dbgp_bulk_write(unsigned devnum, unsigned endpoint,
 	dbgp_set_data(bytes, size);
 	writel(addr, &ehci_debug->address);
 	writel(pids, &ehci_debug->pids);
-	ret = dbgp_wait_until_done(ctrl);
+	ret = dbgp_wait_until_done(ctrl, DBGP_LOOPS);
 
 	return ret;
 }
 
 static int dbgp_bulk_read(unsigned devnum, unsigned endpoint, void *data,
-				 int size)
+			  int size, int loops)
 {
 	u32 pids, addr, ctrl;
 	int ret;
@@ -302,7 +312,7 @@ static int dbgp_bulk_read(unsigned devnum, unsigned endpoint, void *data,
 
 	writel(addr, &ehci_debug->address);
 	writel(pids, &ehci_debug->pids);
-	ret = dbgp_wait_until_done(ctrl);
+	ret = dbgp_wait_until_done(ctrl, loops);
 	if (ret < 0)
 		return ret;
 
@@ -343,12 +353,12 @@ static int dbgp_control_msg(unsigned devnum, int requesttype,
 	dbgp_set_data(&req, sizeof(req));
 	writel(addr, &ehci_debug->address);
 	writel(pids, &ehci_debug->pids);
-	ret = dbgp_wait_until_done(ctrl);
+	ret = dbgp_wait_until_done(ctrl, DBGP_LOOPS);
 	if (ret < 0)
 		return ret;
 
 	/* Read the result */
-	return dbgp_bulk_read(devnum, 0, data, size);
+	return dbgp_bulk_read(devnum, 0, data, size, DBGP_LOOPS);
 }
 
 /* Find a PCI capability */
@@ -559,6 +569,7 @@ try_again:
 		goto err;
 	}
 	dbgp_endpoint_out = dbgp_desc.bDebugOutEndpoint;
+	dbgp_endpoint_in = dbgp_desc.bDebugInEndpoint;
 
 	/* Move the device to 127 if it isn't already there */
 	if (devnum != USB_DEBUG_DEVNUM) {
@@ -968,8 +979,9 @@ int dbgp_reset_prep(void)
 	if (!ehci_debug)
 		return 0;
 
-	if (early_dbgp_console.index != -1 &&
-		!(early_dbgp_console.flags & CON_BOOT))
+	if ((early_dbgp_console.index != -1 &&
+	     !(early_dbgp_console.flags & CON_BOOT)) ||
+	    dbgp_kgdb_mode)
 		return 1;
 	/* This means the console is not initialized, or should get
 	 * shutdown so as to allow for reuse of the usb device, which
@@ -982,3 +994,93 @@ int dbgp_reset_prep(void)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(dbgp_reset_prep);
+
+#ifdef CONFIG_KGDB
+
+static char kgdbdbgp_buf[DBGP_MAX_PACKET];
+static int kgdbdbgp_buf_sz;
+static int kgdbdbgp_buf_idx;
+static int kgdbdbgp_loop_cnt = DBGP_LOOPS;
+
+static int kgdbdbgp_read_char(void)
+{
+	int ret;
+
+	if (kgdbdbgp_buf_idx < kgdbdbgp_buf_sz) {
+		char ch = kgdbdbgp_buf[kgdbdbgp_buf_idx++];
+		return ch;
+	}
+
+	ret = dbgp_bulk_read(USB_DEBUG_DEVNUM, dbgp_endpoint_in,
+			     &kgdbdbgp_buf, DBGP_MAX_PACKET,
+			     kgdbdbgp_loop_cnt);
+	if (ret <= 0)
+		return NO_POLL_CHAR;
+	kgdbdbgp_buf_sz = ret;
+	kgdbdbgp_buf_idx = 1;
+	return kgdbdbgp_buf[0];
+}
+
+static void kgdbdbgp_write_char(u8 chr)
+{
+	early_dbgp_write(NULL, &chr, 1);
+}
+
+static struct kgdb_io kgdbdbgp_io_ops = {
+	.name = "kgdbdbgp",
+	.read_char = kgdbdbgp_read_char,
+	.write_char = kgdbdbgp_write_char,
+};
+
+static int kgdbdbgp_wait_time;
+
+static int __init kgdbdbgp_parse_config(char *str)
+{
+	char *ptr;
+
+	if (!ehci_debug) {
+		if (early_dbgp_init(str))
+			return -1;
+	}
+	ptr = strchr(str, ',');
+	if (ptr) {
+		ptr++;
+		kgdbdbgp_wait_time = simple_strtoul(ptr, &ptr, 10);
+	}
+	kgdb_register_io_module(&kgdbdbgp_io_ops);
+	kgdbdbgp_io_ops.is_console = early_dbgp_console.index != -1;
+
+	return 0;
+}
+early_param("kgdbdbgp", kgdbdbgp_parse_config);
+
+static int kgdbdbgp_reader_thread(void *ptr)
+{
+	int ret;
+
+	while (readl(&ehci_debug->control) & DBGP_ENABLED) {
+		kgdbdbgp_loop_cnt = 1;
+		ret = kgdbdbgp_read_char();
+		kgdbdbgp_loop_cnt = DBGP_LOOPS;
+		if (ret != NO_POLL_CHAR) {
+			if (ret == 0x3 || ret == '$') {
+				if (ret == '$')
+					kgdbdbgp_buf_idx--;
+				kgdb_breakpoint();
+			}
+			continue;
+		}
+		schedule_timeout_interruptible(kgdbdbgp_wait_time * HZ);
+	}
+	return 0;
+}
+
+static int __init kgdbdbgp_start_thread(void)
+{
+	if (dbgp_kgdb_mode && kgdbdbgp_wait_time)
+		kthread_run(kgdbdbgp_reader_thread, NULL, "%s", "dbgp");
+
+	return 0;
+}
+module_init(kgdbdbgp_start_thread);
+#endif /* CONFIG_KGDB */
-- 
1.6.4.rc1

Re: [PATCH 3/8] x86,kgdb: Implement early hardware breakpoint debugging

[Frederic Weisbecker]

On Thu, Feb 25, 2010 at 03:22:01PM -0600, Jason Wessel wrote:
> It is not possible to use the hw_breakpoint.c API prior to mm_init(),
> but it is possible to use hardware breakpoints with the kernel
> debugger.
> 
> Prior to smp_init() it is possible to simply write to the dr registers
> of the boot cpu directly.  This can be used up until the
> kgdb_arch_late() is invoked, at which point the standard hw_breakpoint.c
> API will get used.
> 
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---


That looks good to me at a quick glance.

Re: [PATCH 7/8] ehci-dbgp: split PID register updates for IN and OUT pipes

[Greg KH]

On Thu, Feb 18, 2010 at 04:02:58PM -0600, Jason Wessel wrote:
> Greg KH wrote:
> > On Fri, Feb 12, 2010 at 04:39:28PM -0600, Jason Wessel wrote:
> >   
> >> This patch addresses two problems:
> >>     
> >
> > Do you want me to take this patch, and 8/8 in my tree, or yours?
> >
> > If yours, please add a:
> > 	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> > to the patch.
> >
> >   
> 
> 
> I added the Ack's.
> 
> I will keep the patches in my tree because unless the
> kernel-parameters.txt patch is split it will not apply cleanly for patch
> 8/8.  It currently depends on the kdb tree, even though the source code
> portions do not depend on anything.  If the early debug patch set is
> rejected for some reason, I'll fix patch 8/8 and submit it to your tree.

Sounds good.

> You might consider patch 7/8 for -stable because I observed the dbgp
> device no longer dropping any characters when there is a significant
> printk volume in a short period of time.

When the patch goes into Linus's tree, just send stable@kernel.org the
git commit id of the patch and I will add it.

Or you can add:
	Cc: stable <stable@kernel.org>
to the signed-off-by area, and when it goes into Linus's tree, I will be
automatically notified about it, no need for you to do anything else.

thanks,

greg k-h

Re: [PATCH 7/8] ehci-dbgp: split PID register updates for IN and OUT pipes

[Jason Wessel]

Greg KH wrote:
> On Fri, Feb 12, 2010 at 04:39:28PM -0600, Jason Wessel wrote:
>   
>> This patch addresses two problems:
>>     
>
> Do you want me to take this patch, and 8/8 in my tree, or yours?
>
> If yours, please add a:
> 	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> to the patch.
>
>   


I added the Ack's.

I will keep the patches in my tree because unless the
kernel-parameters.txt patch is split it will not apply cleanly for patch
8/8.  It currently depends on the kdb tree, even though the source code
portions do not depend on anything.  If the early debug patch set is
rejected for some reason, I'll fix patch 8/8 and submit it to your tree.

You might consider patch 7/8 for -stable because I observed the dbgp
device no longer dropping any characters when there is a significant
printk volume in a short period of time.

Thanks,
Jason.

Re: [PATCH 7/8] ehci-dbgp: split PID register updates for IN and OUT pipes

[Greg KH]

On Fri, Feb 12, 2010 at 04:39:28PM -0600, Jason Wessel wrote:
> This patch addresses two problems:

Do you want me to take this patch, and 8/8 in my tree, or yours?

If yours, please add a:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
to the patch.

thanks,

greg k-h

[PATCH 7/8] ehci-dbgp: split PID register updates for IN and OUT pipes

[Jason Wessel]

This patch addresses two problems:

1) Bulk reads should always use the DATA0 for the pid, and the write
   PID should toggle between DATA0 and DATA1.  The fix is using
   dbgp_pid_write_update() and dbgp_pid_read_update().

2) The delay loop for waiting for a transaction was not long enough to
   always complete the initial handshake inside dbgp_wait_until_done().
   After the initial handshake the maximum delay length is never reached.

The combined result of these two changes allows for the removal of the
forced resynchronization where a bulk write was issued with a dummy
data payload only to get the device to start accepting data writes
again.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Eric Biederman <ebiederm@xmission.com>
CC: Yinghai Lu <yhlu.kernel@gmail.com>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/usb/early/ehci-dbgp.c |   68 +++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index 2958a12..6e98a36 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -66,8 +66,6 @@ static struct ehci_dev ehci_dev;
 
 #define USB_DEBUG_DEVNUM 127
 
-#define DBGP_DATA_TOGGLE	0x8800
-
 #ifdef DBGP_DEBUG
 #define dbgp_printk printk
 static void dbgp_ehci_status(char *str)
@@ -88,11 +86,6 @@ static inline void dbgp_ehci_status(char *str) { }
 static inline void dbgp_printk(const char *fmt, ...) { }
 #endif
 
-static inline u32 dbgp_pid_update(u32 x, u32 tok)
-{
-	return ((x ^ DBGP_DATA_TOGGLE) & 0xffff00) | (tok & 0xff);
-}
-
 static inline u32 dbgp_len_update(u32 x, u32 len)
 {
 	return (x & ~0x0f) | (len & 0x0f);
@@ -136,6 +129,19 @@ static inline u32 dbgp_len_update(u32 x, u32 len)
 
 #define DBGP_MAX_PACKET		8
 #define DBGP_TIMEOUT		(250 * 1000)
+#define DBGP_LOOPS		1000
+
+static inline u32 dbgp_pid_write_update(u32 x, u32 tok)
+{
+	static int data0 = USB_PID_DATA1;
+	data0 ^= USB_PID_DATA_TOGGLE;
+	return (x & 0xffff0000) | (data0 << 8) | (tok & 0xff);
+}
+
+static inline u32 dbgp_pid_read_update(u32 x, u32 tok)
+{
+	return (x & 0xffff0000) | (USB_PID_DATA0 << 8) | (tok & 0xff);
+}
 
 static int dbgp_wait_until_complete(void)
 {
@@ -180,7 +186,7 @@ static int dbgp_wait_until_done(unsigned ctrl)
 {
 	u32 pids, lpid;
 	int ret;
-	int loop = 3;
+	int loop = DBGP_LOOPS;
 
 retry:
 	writel(ctrl | DBGP_GO, &ehci_debug->control);
@@ -197,6 +203,8 @@ retry:
 		 */
 		if (ret == -DBGP_TIMEOUT && !dbgp_not_safe)
 			dbgp_not_safe = 1;
+		if (ret == -DBGP_ERR_BAD && --loop > 0)
+			goto retry;
 		return ret;
 	}
 
@@ -245,12 +253,20 @@ static inline void dbgp_get_data(void *buf, int size)
 		bytes[i] = (hi >> (8*(i - 4))) & 0xff;
 }
 
-static int dbgp_out(u32 addr, const char *bytes, int size)
+static int dbgp_bulk_write(unsigned devnum, unsigned endpoint,
+			 const char *bytes, int size)
 {
+	int ret;
+	u32 addr;
 	u32 pids, ctrl;
 
+	if (size > DBGP_MAX_PACKET)
+		return -1;
+
+	addr = DBGP_EPADDR(devnum, endpoint);
+
 	pids = readl(&ehci_debug->pids);
-	pids = dbgp_pid_update(pids, USB_PID_OUT);
+	pids = dbgp_pid_write_update(pids, USB_PID_OUT);
 
 	ctrl = readl(&ehci_debug->control);
 	ctrl = dbgp_len_update(ctrl, size);
@@ -260,34 +276,7 @@ static int dbgp_out(u32 addr, const char *bytes, int size)
 	dbgp_set_data(bytes, size);
 	writel(addr, &ehci_debug->address);
 	writel(pids, &ehci_debug->pids);
-	return dbgp_wait_until_done(ctrl);
-}
-
-static int dbgp_bulk_write(unsigned devnum, unsigned endpoint,
-			 const char *bytes, int size)
-{
-	int ret;
-	int loops = 5;
-	u32 addr;
-	if (size > DBGP_MAX_PACKET)
-		return -1;
-
-	addr = DBGP_EPADDR(devnum, endpoint);
-try_again:
-	if (loops--) {
-		ret = dbgp_out(addr, bytes, size);
-		if (ret == -DBGP_ERR_BAD) {
-			int try_loops = 3;
-			do {
-				/* Emit a dummy packet to re-sync communication
-				 * with the debug device */
-				if (dbgp_out(addr, "12345678", 8) >= 0) {
-					udelay(2);
-					goto try_again;
-				}
-			} while (try_loops--);
-		}
-	}
+	ret = dbgp_wait_until_done(ctrl);
 
 	return ret;
 }
@@ -304,7 +293,7 @@ static int dbgp_bulk_read(unsigned devnum, unsigned endpoint, void *data,
 	addr = DBGP_EPADDR(devnum, endpoint);
 
 	pids = readl(&ehci_debug->pids);
-	pids = dbgp_pid_update(pids, USB_PID_IN);
+	pids = dbgp_pid_read_update(pids, USB_PID_IN);
 
 	ctrl = readl(&ehci_debug->control);
 	ctrl = dbgp_len_update(ctrl, size);
@@ -362,7 +351,6 @@ static int dbgp_control_msg(unsigned devnum, int requesttype,
 	return dbgp_bulk_read(devnum, 0, data, size);
 }
 
-
 /* Find a PCI capability */
 static u32 __init find_cap(u32 num, u32 slot, u32 func, int cap)
 {
-- 
1.6.4.rc1