Mailing List Archive

USB Serial device disconnect causes IRQ disable after using ehci controller halted
Hi
Alan
I am posting below detail log after adding debug messages
[ 307.695452] magtek 5-1:1.0: Magtek 75/Excella USB card reader converter detected
[ 307.695452] drivers/usb/serial/magtek.c: magtek_startup
[ 307.695452] usb 5-1: link qh0-00ff/ded67080 start 0 [1/0 us]
[ 307.695452] drivers/usb/serial/magtek.c: magtek_startup - usb_submit_urb(int urb)
[ 307.695452] usb 5-1: Magtek 75/Excella USB card reader converter now attached to ttyUSB0
[ 307.695452] drivers/usb/core/inode.c: creating file '006'
[ 307.696371] usb 5-1: New USB device found, idVendor=0801, idProduct=2231
[ 307.696377] usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 307.696383] usb 5-1: Product: STX
[ 307.696387] usb 5-1: Manufacturer: MagTek
[ 307.696391] usb 5-1: SerialNumber: STX001
[ 307.700515] ehci_hcd 0000:00:1d.7: irq status c028 masked 20
[ 307.700530] ehci_hcd 0000:00:1d.7: hcd state 1
[ 307.700536] ehci_hcd 0000:00:1d.7: hcd state 1
[ 307.700544] ehci_hcd 0000:00:1d.7: hcd state 1
[ 308.850650] ehci_hcd 0000:00:1d.7: irq status 600b masked 3
[ 308.850658] ehci_hcd 0000:00:1d.7: hcd state 1
[ 308.850665] ehci_hcd 0000:00:1d.7: devpath 1 ep2in 3strikes
[ 308.850670] drivers/usb/serial/magtek.c: magtek_read_int_callback - port 0
[ 308.850683] usb 5-1: unlink qh0-00ff/ded67080 start 0 [1/0 us]
[ 308.850846] usb 5-1: link qh0-00ff/ded67080 start 0 [1/0 us]
[ 308.851537] usb 5-1: unlink qh0-00ff/ded67080 start 0 [1/0 us]
[ 308.853487] ehci_hcd 0000:00:1d.7: handshake failed: controller halted
[ 308.853487] Pid: 0, comm: swapper Not tainted 2.6.26EHCIDBG #3
[ 308.853487] [<e08405aa>] handshake_on_error_set_halt+0x45/0x51 [ehci_hcd]
[ 308.853487] [<e08405d6>] disable_periodic+0x20/0x40 [ehci_hcd]
[ 308.853487] [<e0841fb4>] ehci_work+0x5e6/0x6ad [ehci_hcd]
[ 308.853487] [<c0424230>] ? printk+0x15/0x17
[ 308.853487] [<e0845c7e>] ehci_irq+0x28a/0x2fd [ehci_hcd]
[ 308.853487] [<e090005d>] ? cdrom_newpc_intr+0x52e/0x544 [ide_cd_mod]
[ 308.853487] [<c042addd>] ? lock_timer_base+0x1f/0x3e
[ 308.853487] [<c05764c4>] usb_hcd_irq+0x27/0x58
[ 308.853487] [<c044b852>] handle_IRQ_event+0x21/0x48
[ 308.853487] [<c044c9fb>] handle_fasteoi_irq+0x77/0xac
[ 308.853487] [<c044c984>] ? handle_fasteoi_irq+0x0/0xac
[ 308.853487] [<c040598c>] do_IRQ+0xa9/0xd1
[ 308.853487] [<c04025f2>] ? default_idle+0x0/0x42
[ 308.853487] [<c040429b>] common_interrupt+0x23/0x28
[ 308.853487] [<c04025f2>] ? default_idle+0x0/0x42
[ 308.853487] [<c041007b>] ? acpi_save_state_mem+0xa/0x12b
[ 308.853487] [<c040261f>] ? default_idle+0x2d/0x42
[ 308.853487] [<c040256d>] cpu_idle+0x8b/0x9f
[ 308.853487] [<c061f329>] start_secondary+0x156/0x15b
[ 308.853487] =======================
[ 308.853487] ehci_hcd 0000:00:1d.7: hcd state 0
[ 308.853487] ehci_hcd 0000:00:1d.7: hcd state 0
[ 308.853487] ehci_hcd 0000:00:1d.7: HC died; cleaning up
[ 308.854983] hub 5-0:1.0: state 0 ports 8 chg 0000 evt 0000
[ 308.854983] usb 5-1: USB disconnect, address 6
[ 308.854983] usb 5-1: unregistering device
[ 308.854983] usb 5-1: usb_disable_device nuking all URBs
[ 308.854983] usb 5-1: unregistering interface 5-1:1.0
[ 308.854983] drivers/usb/serial/magtek.c: magtek_shutdown
[ 308.854983] magtek ttyUSB0: Magtek 75/Excella USB card reader converter now disconnected from ttyUSB0
[ 308.854983] magtek 5-1:1.0: device disconnected
[ 308.854983] usb 5-1:1.0: uevent
[ 308.854983] usb 5-1: uevent
[ 309.153998] irq 23: nobody cared (try booting with the "irqpoll" option)
[ 309.154006] Pid: 0, comm: swapper Not tainted 2.6.26EHCIDBG #3
[ 309.154027] [<c044c0cc>] __report_bad_irq+0x2e/0x6f
[ 309.154042] [<c044c2e4>] note_interrupt+0x1d7/0x20b
[ 309.154054] [<c044b852>] ? handle_IRQ_event+0x21/0x48
[ 309.154068] [<c044ca0f>] handle_fasteoi_irq+0x8b/0xac
[ 309.154078] [<c044c984>] ? handle_fasteoi_irq+0x0/0xac
[ 309.154087] [<c040598c>] do_IRQ+0xa9/0xd1
[ 309.154096] [<c04025f2>] ? default_idle+0x0/0x42
[ 309.154107] [<c040429b>] common_interrupt+0x23/0x28
[ 309.154112] [<c04025f2>] ? default_idle+0x0/0x42
[ 309.154132] [<c040261f>] ? default_idle+0x2d/0x42
[ 309.154579] [<c040256d>] cpu_idle+0x8b/0x9f
[ 309.154579] [<c061f329>] start_secondary+0x156/0x15b
[ 309.154579] =======================
[ 309.154579] handlers:
[ 309.154579] [<c057649d>] (usb_hcd_irq+0x0/0x58)
[ 309.154579] Disabling IRQ #23



Thanks
Amruth p.v







--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
Karsten and Dave:

It looks like the logic in ehci-hcd's handshake_on_error_set_halt()
needs to be revisited.

On Tue, 26 Aug 2008, amruth wrote:

> Hi
> Alan
> I am posting below detail log after adding debug messages

...
> [ 308.850665] ehci_hcd 0000:00:1d.7: devpath 1 ep2in 3strikes
> [ 308.850670] drivers/usb/serial/magtek.c: magtek_read_int_callback - port 0
> [ 308.850683] usb 5-1: unlink qh0-00ff/ded67080 start 0 [1/0 us]
> [ 308.850846] usb 5-1: link qh0-00ff/ded67080 start 0 [1/0 us]
> [ 308.851537] usb 5-1: unlink qh0-00ff/ded67080 start 0 [1/0 us]
> [ 308.853487] ehci_hcd 0000:00:1d.7: handshake failed: controller halted

The line above was a debugging message added specially for this test.
The controller wasn't really halted, but hcd->state was set to
HC_STATE_HALTED.

> [ 308.853487] Pid: 0, comm: swapper Not tainted 2.6.26EHCIDBG #3
> [ 308.853487] [<e08405aa>] handshake_on_error_set_halt+0x45/0x51 [ehci_hcd]
> [ 308.853487] [<e08405d6>] disable_periodic+0x20/0x40 [ehci_hcd]
> [ 308.853487] [<e0841fb4>] ehci_work+0x5e6/0x6ad [ehci_hcd]
> [ 308.853487] [<c0424230>] ? printk+0x15/0x17
> [ 308.853487] [<e0845c7e>] ehci_irq+0x28a/0x2fd [ehci_hcd]
> [ 308.853487] [<e090005d>] ? cdrom_newpc_intr+0x52e/0x544 [ide_cd_mod]
> [ 308.853487] [<c042addd>] ? lock_timer_base+0x1f/0x3e
> [ 308.853487] [<c05764c4>] usb_hcd_irq+0x27/0x58
> [ 308.853487] [<c044b852>] handle_IRQ_event+0x21/0x48
> [ 308.853487] [<c044c9fb>] handle_fasteoi_irq+0x77/0xac
> [ 308.853487] [<c044c984>] ? handle_fasteoi_irq+0x0/0xac
> [ 308.853487] [<c040598c>] do_IRQ+0xa9/0xd1
> [ 308.853487] [<c04025f2>] ? default_idle+0x0/0x42
> [ 308.853487] [<c040429b>] common_interrupt+0x23/0x28
> [ 308.853487] [<c04025f2>] ? default_idle+0x0/0x42
> [ 308.853487] [<c041007b>] ? acpi_save_state_mem+0xa/0x12b
> [ 308.853487] [<c040261f>] ? default_idle+0x2d/0x42
> [ 308.853487] [<c040256d>] cpu_idle+0x8b/0x9f
> [ 308.853487] [<c061f329>] start_secondary+0x156/0x15b
> [ 308.853487] =======================
> [ 308.853487] ehci_hcd 0000:00:1d.7: hcd state 0
> [ 308.853487] ehci_hcd 0000:00:1d.7: hcd state 0
> [ 308.853487] ehci_hcd 0000:00:1d.7: HC died; cleaning up
> [ 308.854983] hub 5-0:1.0: state 0 ports 8 chg 0000 evt 0000
> [ 308.854983] usb 5-1: USB disconnect, address 6
> [ 308.854983] usb 5-1: unregistering device
> [ 308.854983] usb 5-1: usb_disable_device nuking all URBs
> [ 308.854983] usb 5-1: unregistering interface 5-1:1.0
> [ 308.854983] drivers/usb/serial/magtek.c: magtek_shutdown
> [ 308.854983] magtek ttyUSB0: Magtek 75/Excella USB card reader converter now disconnected from ttyUSB0
> [ 308.854983] magtek 5-1:1.0: device disconnected
> [ 308.854983] usb 5-1:1.0: uevent
> [ 308.854983] usb 5-1: uevent
> [ 309.153998] irq 23: nobody cared (try booting with the "irqpoll" option)

Simply setting hcd->state to HC_STATE_HALTED isn't a good idea,
especially in cases like this where the controller really _isn't_
halted. Here it was still generating IRQs, leading to the "nobody
cared" problem on the last line of the log.

While it certainly would be a good idea to prevent the handshake
failures from occurring in the first place -- Dave, aren't you working
on a patch for that? -- we should also make sure that when they do
occur, the controller gets reset properly. And an error message should
be printed in the log.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
On Tuesday 26 August 2008, Alan Stern wrote:
> While it certainly would be a good idea to prevent the handshake
> failures from occurring in the first place -- Dave, aren't you working
> on a patch for that? -- we should also make sure that when they do
> occur, the controller gets reset properly.  And an error message should
> be printed in the log.

Like this?

Greg, please queue for 2.6.27 unless someone finds a
problem with this patch.


========= SNIP! SNIP! SNIPPITY SNIP!
From: David Brownell <dbrownell@users.sourceforge.net>

I noticed that the "Refactor "if (handshake()) state = HC_STATE_HALT"
patch from earlier this year perpetuated a potential problem: it can
mark the controller as halted when it's still running (but not acting
as, perhaps wrongly, expected).

That caused some hangs and crashes, rather than more polite failure
modes of a truly halted controller. This patch forces a true halt,
and emits a (previously missing) diagnostic.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

--- a/drivers/usb/host/ehci-hcd.c 2008-08-15 14:04:27.000000000 -0700
+++ b/drivers/usb/host/ehci-hcd.c 2008-08-15 14:04:36.000000000 -0700
@@ -145,16 +145,6 @@ static int handshake (struct ehci_hcd *e
return -ETIMEDOUT;
}

-static int handshake_on_error_set_halt(struct ehci_hcd *ehci, void __iomem *ptr,
- u32 mask, u32 done, int usec)
-{
- int error = handshake(ehci, ptr, mask, done, usec);
- if (error)
- ehci_to_hcd(ehci)->state = HC_STATE_HALT;
-
- return error;
-}
-
/* force HC to halt state from unknown (EHCI spec section 2.3) */
static int ehci_halt (struct ehci_hcd *ehci)
{
@@ -173,6 +163,22 @@ static int ehci_halt (struct ehci_hcd *e
STS_HALT, STS_HALT, 16 * 125);
}

+static int handshake_on_error_set_halt(struct ehci_hcd *ehci, void __iomem *ptr,
+ u32 mask, u32 done, int usec)
+{
+ int error;
+
+ error = handshake(ehci, ptr, mask, done, usec);
+ if (error) {
+ ehci_halt(ehci);
+ ehci_to_hcd(ehci)->state = HC_STATE_HALT;
+ ehci_err(ehci, "force halt; handhake %p %08x %08x -> %d\n",
+ ptr, mask, done, error);
+ }
+
+ return error;
+}
+
/* put TDI/ARC silicon into EHCI mode */
static void tdi_reset (struct ehci_hcd *ehci)
{


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
Hi
Alan/David
This patch below does not fix the issue it just stops IRQ being disabled but still ehci hcd crashes. Please let me know what could be causing the issue.Is the hardware having any issue.
The log is below
[ 552.905001] magtek 5-2:1.0: Magtek 75/Excella USB card reader converter detected
[ 552.905001] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: magtek_startup
[ 552.905001] usb 5-2: link qh0-00ff/d8f44080 start 0 [1/0 us]
[ 552.905001] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: magtek_startup - usb_submit_urb(int urb)
[ 552.905001] usb 5-2: Magtek 75/Excella USB card reader converter now attached to ttyUSB0
[ 552.905001] drivers/usb/core/inode.c: creating file '006'
[ 552.905001] usb 5-2: New USB device found, idVendor=0801, idProduct=2231
[ 552.905001] usb 5-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 552.905025] usb 5-2: Product: STX
[ 552.905030] usb 5-2: Manufacturer: MagTek
[ 552.905035] usb 5-2: SerialNumber: STX001
[ 552.909786] ehci_hcd 0000:00:1d.7: irq status c028 masked 20
[ 552.909793] ehci_hcd 0000:00:1d.7: hcd state 1
[ 552.909796] ehci_hcd 0000:00:1d.7: hcd state 1
[ 552.909799] ehci_hcd 0000:00:1d.7: hcd state 1
[ 558.654520] ehci_hcd 0000:00:1d.7: irq status 600f masked 7
[ 558.654520] ehci_hcd 0000:00:1d.7: hcd state 1
[ 558.654520] ehci_hcd 0000:00:1d.7: devpath 2 ep2in 3strikes
[ 558.654520] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: magtek_read_int_callback - port 0
[ 558.654520] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: return!!
[ 558.654520] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: urb status is -71
[ 558.654520] usb 5-2: unlink qh0-00ff/d8f44080 start 0 [1/0 us]
[ 558.654520] usb 5-2: link qh0-00ff/d8f44080 start 0 [1/0 us]
[ 558.654520] usb 5-2: unlink qh0-00ff/d8f44080 start 0 [1/0 us]
[ 558.656520] ehci_hcd 0000:00:1d.7: handshake failed: controller halted
[ 558.656520] ehci_hcd 0000:00:1d.7: force halt; handshake e0840824 00004000 00004000 -> -110
[ 558.656520] Pid: 0, comm: swapper Not tainted 2.6.26patched #2
[ 558.656520] [<e0b1e5e7>] handshake_on_error_set_halt+0x82/0x8c [ehci_hcd]
[ 558.656520] [<e0b1e611>] disable_periodic+0x20/0x40 [ehci_hcd]
[ 558.656520] [<e0b1ffef>] ehci_work+0x5e6/0x6ad [ehci_hcd]
[ 558.656520] [<c0424230>] ? printk+0x15/0x17
[ 558.656520] [<e0b23cb9>] ehci_irq+0x28a/0x2fd [ehci_hcd]
[ 558.656520] [<e096905d>] ? cdrom_newpc_intr+0x52e/0x544 [ide_cd_mod]
[ 558.656520] [<c042addd>] ? lock_timer_base+0x1f/0x3e
[ 558.656520] [<c05764c4>] usb_hcd_irq+0x27/0x58
[ 558.656520] [<c044b852>] handle_IRQ_event+0x21/0x48
[ 558.656520] [<c044c9fb>] handle_fasteoi_irq+0x77/0xac
[ 558.656520] [<c044c984>] ? handle_fasteoi_irq+0x0/0xac
[ 558.656520] [<c040598c>] do_IRQ+0xa9/0xd1
[ 558.656520] [<c04025f2>] ? default_idle+0x0/0x42
[ 558.656520] [<c040429b>] common_interrupt+0x23/0x28
[ 558.656520] [<c04025f2>] ? default_idle+0x0/0x42
[ 558.656520] [<c041007b>] ? acpi_save_state_mem+0xa/0x12b
[ 558.656520] [<c040261f>] ? default_idle+0x2d/0x42
[ 558.656520] [<c040256d>] cpu_idle+0x8b/0x9f
[ 558.656520] [<c061f329>] start_secondary+0x156/0x15b
[ 558.656520] =======================
[ 558.656520] ehci_hcd 0000:00:1d.7: hcd state 0
[ 558.656520] ehci_hcd 0000:00:1d.7: hcd state 0
[ 558.656520] ehci_hcd 0000:00:1d.7: HC died; cleaning up
[ 558.657583] hub 5-0:1.0: state 0 ports 8 chg 0000 evt 0000
[ 558.657583] usb 5-2: USB disconnect, address 6
[ 558.657583] usb 5-2: unregistering device
[ 558.657583] usb 5-2: usb_disable_device nuking all URBs
[ 558.657583] usb 5-2: unregistering interface 5-2:1.0
[ 558.657583] /usr/src/linux-2.6.26/drivers/usb/serial/magtek.c: magtek_shutdown
[ 558.657583] magtek ttyUSB0: Magtek 75/Excella USB card reader converter now disconnected from ttyUSB0
[ 558.657584] magtek 5-2:1.0: device disconnected
[ 558.657584] usb 5-2:1.0: uevent
[ 558.658057] usb 5-2: uevent

Thanks
Amruth p.v


--- On Tue, 8/26/08, David Brownell <david-b@pacbell.net> wrote:

> From: David Brownell <david-b@pacbell.net>
> Subject: Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted
> To: "Alan Stern" <stern@rowland.harvard.edu>
> Cc: "Karsten Wiese" <fzu@wemgehoertderstaat.de>, "amruth" <amruth_pv@yahoo.com>, "Oliver Neukum" <oliver@neukum.org>, "USB list" <linux-usb@vger.kernel.org>, "Kernel development list" <linux-kernel@vger.kernel.org>, "Greg KH" <greg@kroah.com>
> Date: Tuesday, August 26, 2008, 4:43 PM
> On Tuesday 26 August 2008, Alan Stern wrote:
> > While it certainly would be a good idea to prevent the
> handshake
> > failures from occurring in the first place -- Dave,
> aren't you working
> > on a patch for that? -- we should also make sure that
> when they do
> > occur, the controller gets reset properly. And an
> error message should
> > be printed in the log.
>
> Like this?
>
> Greg, please queue for 2.6.27 unless someone finds a
> problem with this patch.
>
>
> ========= SNIP! SNIP! SNIPPITY SNIP!
> From: David Brownell
> <dbrownell@users.sourceforge.net>
>
> I noticed that the "Refactor "if (handshake())
> state = HC_STATE_HALT"
> patch from earlier this year perpetuated a potential
> problem: it can
> mark the controller as halted when it's still running
> (but not acting
> as, perhaps wrongly, expected).
>
> That caused some hangs and crashes, rather than more polite
> failure
> modes of a truly halted controller. This patch forces a
> true halt,
> and emits a (previously missing) diagnostic.
>
> Signed-off-by: David Brownell
> <dbrownell@users.sourceforge.net>
> ---
> drivers/usb/host/ehci-hcd.c | 26
> ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> --- a/drivers/usb/host/ehci-hcd.c 2008-08-15
> 14:04:27.000000000 -0700
> +++ b/drivers/usb/host/ehci-hcd.c 2008-08-15
> 14:04:36.000000000 -0700
> @@ -145,16 +145,6 @@ static int handshake (struct ehci_hcd
> *e
> return -ETIMEDOUT;
> }
>
> -static int handshake_on_error_set_halt(struct ehci_hcd
> *ehci, void __iomem *ptr,
> - u32 mask, u32 done, int usec)
> -{
> - int error = handshake(ehci, ptr, mask, done, usec);
> - if (error)
> - ehci_to_hcd(ehci)->state = HC_STATE_HALT;
> -
> - return error;
> -}
> -
> /* force HC to halt state from unknown (EHCI spec section
> 2.3) */
> static int ehci_halt (struct ehci_hcd *ehci)
> {
> @@ -173,6 +163,22 @@ static int ehci_halt (struct ehci_hcd
> *e
> STS_HALT, STS_HALT, 16 * 125);
> }
>
> +static int handshake_on_error_set_halt(struct ehci_hcd
> *ehci, void __iomem *ptr,
> + u32 mask, u32 done, int usec)
> +{
> + int error;
> +
> + error = handshake(ehci, ptr, mask, done, usec);
> + if (error) {
> + ehci_halt(ehci);
> + ehci_to_hcd(ehci)->state = HC_STATE_HALT;
> + ehci_err(ehci, "force halt; handhake %p %08x %08x
> -> %d\n",
> + ptr, mask, done, error);
> + }
> +
> + return error;
> +}
> +
> /* put TDI/ARC silicon into EHCI mode */
> static void tdi_reset (struct ehci_hcd *ehci)
> {
>
>
> --
> To unsubscribe from this list: send the line
> "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
On Tuesday 26 August 2008, amruth wrote:
> This patch below does not fix the issue it just stops IRQ being
> disabled but still ehci hcd crashes.

That patch was only intended to address the issue of bogus error
handling.


> Please let me know what could be causing the issue.

If it's like the other case, I'd hope this patch would solve it.

Note that you also seem to be having hardware or firmware issues
with the peripheral you're connecting ... this won't change that
stuff at all.

- Dave

================ SNIP!
From: David Brownell <dbrownell@users.sourceforge.net>

As noted by Stefan Neis <Stefan.Neis@kobil.com>, we had a recent
regression with EHCI periodic transfers, in some (seemingly not
all that common) cases.

The root cause was that the schedule activation was only loosely
coupled to the addition or removal of transfers, so two different
execution contexts could both think they had to deactivate (or
conversely activate) the schedule. So this fix tightens that
coupling, managing it more like a refcount.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
drivers/usb/host/ehci-sched.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)

--- a/drivers/usb/host/ehci-sched.c 2008-08-15 16:38:19.000000000 -0700
+++ b/drivers/usb/host/ehci-sched.c 2008-08-15 17:47:02.000000000 -0700
@@ -437,6 +437,9 @@ static int enable_periodic (struct ehci_
u32 cmd;
int status;

+ if (ehci->periodic_sched++)
+ return 0;
+
/* did clearing PSE did take effect yet?
* takes effect only at frame boundaries...
*/
@@ -461,6 +464,9 @@ static int disable_periodic (struct ehci
u32 cmd;
int status;

+ if (--ehci->periodic_sched)
+ return 0;
+
/* did setting PSE not take effect yet?
* takes effect only at frame boundaries...
*/
@@ -544,13 +550,10 @@ static int qh_link_periodic (struct ehci
: (qh->usecs * 8);

/* maybe enable periodic schedule processing */
- if (!ehci->periodic_sched++)
- return enable_periodic (ehci);
-
- return 0;
+ return enable_periodic(ehci);
}

-static void qh_unlink_periodic (struct ehci_hcd *ehci, struct ehci_qh *qh)
+static int qh_unlink_periodic(struct ehci_hcd *ehci, struct ehci_qh *qh)
{
unsigned i;
unsigned period;
@@ -586,9 +589,7 @@ static void qh_unlink_periodic (struct e
qh_put (qh);

/* maybe turn off periodic schedule */
- ehci->periodic_sched--;
- if (!ehci->periodic_sched)
- (void) disable_periodic (ehci);
+ return disable_periodic(ehci);
}

static void intr_deschedule (struct ehci_hcd *ehci, struct ehci_qh *qh)
@@ -1562,9 +1563,7 @@ itd_link_urb (
urb->hcpriv = NULL;

timer_action (ehci, TIMER_IO_WATCHDOG);
- if (unlikely (!ehci->periodic_sched++))
- return enable_periodic (ehci);
- return 0;
+ return enable_periodic(ehci);
}

#define ISO_ERRS (EHCI_ISOC_BUF_ERR | EHCI_ISOC_BABBLE | EHCI_ISOC_XACTERR)
@@ -1642,7 +1641,7 @@ itd_complete (
ehci_urb_done(ehci, urb, 0);
retval = true;
urb = NULL;
- ehci->periodic_sched--;
+ (void) disable_periodic(ehci);
ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;

if (unlikely (list_empty (&stream->td_list))) {
@@ -1951,9 +1950,7 @@ sitd_link_urb (
urb->hcpriv = NULL;

timer_action (ehci, TIMER_IO_WATCHDOG);
- if (!ehci->periodic_sched++)
- return enable_periodic (ehci);
- return 0;
+ return enable_periodic(ehci);
}

/*-------------------------------------------------------------------------*/
@@ -2019,7 +2016,7 @@ sitd_complete (
ehci_urb_done(ehci, urb, 0);
retval = true;
urb = NULL;
- ehci->periodic_sched--;
+ (void) disable_periodic(ehci);
ehci_to_hcd(ehci)->self.bandwidth_isoc_reqs--;

if (list_empty (&stream->td_list)) {
@@ -2243,8 +2240,7 @@ restart:
if (unlikely (modified)) {
if (likely(ehci->periodic_sched > 0))
goto restart;
- /* maybe we can short-circuit this scan! */
- disable_periodic(ehci);
+ /* short-circuit this scan */
now_uframe = clock;
break;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
On Tue, Aug 26, 2008 at 11:35:04PM -0700, David Brownell wrote:
> On Tuesday 26 August 2008, amruth wrote:
> > This patch below does not fix the issue it just stops IRQ being
> > disabled but still ehci hcd crashes.
>
> That patch was only intended to address the issue of bogus error
> handling.
>
>
> > Please let me know what could be causing the issue.
>
> If it's like the other case, I'd hope this patch would solve it.
>
> Note that you also seem to be having hardware or firmware issues
> with the peripheral you're connecting ... this won't change that
> stuff at all.
>
> - Dave
>
> ================ SNIP!
> From: David Brownell <dbrownell@users.sourceforge.net>
>
> As noted by Stefan Neis <Stefan.Neis@kobil.com>, we had a recent
> regression with EHCI periodic transfers, in some (seemingly not
> all that common) cases.
>
> The root cause was that the schedule activation was only loosely
> coupled to the addition or removal of transfers, so two different
> execution contexts could both think they had to deactivate (or
> conversely activate) the schedule. So this fix tightens that
> coupling, managing it more like a refcount.
>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

Do you want me to also apply this one? If so, for .27 or .28?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: USB Serial device disconnect causes IRQ disable after using ehci controller halted [ In reply to ]
On Wednesday 27 August 2008, Greg KH wrote:
>
> > As noted by Stefan Neis <Stefan.Neis@kobil.com>, we had a recent
> > regression with EHCI periodic transfers, in some (seemingly not
> > all that common) cases.
> >
> > The root cause was that the schedule activation was only loosely
> > coupled to the addition or removal of transfers, so two different
> > execution contexts could both think they had to deactivate (or
> > conversely activate) the schedule.  So this fix tightens that
> > coupling, managing it more like a refcount.
> >
> > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
>
> Do you want me to also apply this one?  If so, for .27 or .28?

It should get into 2.6.27, and probably 26.stable ... assuming
that it checks out properly. A somewhat cruftier version resolved
Stefan's problem, but I'd rather not merge that one. Hmm, I'll
ask Stefan to confirm this version.

- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/