public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jurgen Braam <j.braam@homeautomationeurope.com>
To: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Cc: Sascha Hauer <s.hauer@pengutronix.de>,
	e9hack <e9hack@googlemail.com>, Johan Hovold <jhovold@gmail.com>
Subject: [PATCH][RFC] USB: EHCI: add missing {endpoint_reset,clear_tt_buffer_complete} operations
Date: Wed, 13 Oct 2010 10:06:00 +0000 (UTC)	[thread overview]
Message-ID: <13257307.1383581286964359341.JavaMail.PC-43$@PC-43> (raw)
In-Reply-To: <18494093.1383561286964261272.JavaMail.PC-43$@PC-43>

From: Jurgen Braam <j.braam@homeautomationeurope.com>

The ehci-{atmel,mxc,w90x900,xilinx-of} HCD drivers did not yet have the
endpoint_reset and clear_tt_buffer_complete operations set in their struct
hc_driver. This is a follow up on commit
b18ffd49e86102a9ed0a1cc83fdafe3891e844e5 ("USB: EHCI: update toggle state
for linked QHs") and 914b701280a76f96890ad63eb0fa99bf204b961c ("USB: EHCI:
use the new clear_tt_buffer interface").

When usb-serial's generic_cleanup(), or any XXX_close() function for that
matter, terminates all read/write URBs with usb_kill_urb() then the queues
to their respective endpoints generally stop. Sometimes the Transaction
Translator buffers may be left in an indeterminate state and need to be
cleaned up with clear_tt_buffer(). After this clear_tt_buffer_complete() is
called if it is defined in the hc_driver struct. In this process the
endpoint_reset() is needed to ensure the URB queues are restarted.

Signed-off-by: Jurgen Braam <j.braam@homeautomationeurope.com>
---

The reason I stumbled onto this was that I have been on a bug-tracking
mission for the past 40 hours. I was hunting for the reason why some
usb-serial-based convertors like ftdi_sio stopped working after a close()
and subsequent open(). The observed effect is that after working perfectly
the first time, the second time reads from the device no longer seem to
work. The ehci-mxc driver for the iMX27 platform is one of the HCDs that do
not yet set these operations.

I'm not familiar with or able to test the other ehci-hcd drivers
(atmel,w90x900,xilinx-of) so maybe these need some testing. Could this
maybe also be relevant for other *-hcd drivers? This patch applies to
2.6.36-rc7, please comment.

For the record: I found the other HCDs with missing operations with:
 git grep -i -n --color -E "\.(clear_tt_buffer_complete|urb_enqueue|endpoint_)" drivers/usb/host/ehci*
Maybe we could do something like usb-serial.c's #define
set_to_generic_if_null() to prevent future omissions?

My search lead me past the following, I list them here for future reverse
googleability:
- http://www.spinics.net/lists/linux-usb/msg34099.html "Problem with FTDI_SIO version 1.5.0. and 1.6.0"
- http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg32747.html "Endpoint queues and URB unlinking"
- http://kerneltrap.org/mailarchive/linux-usb/2010/8/24/6264209 "ftdi_sioserial adapter data toggle problem."
- http://lkml.org/lkml/2009/9/12/70 "Trouble with USB serial driver(ftdi_sio.c) on 2.6.31"
- http://linux.derkeiler.com/Mailing-Lists/Kernel/2009-09/msg01780.html "[patch 58/71] USB: fix the clear_tt_buffer interface"
- http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg27905.html "[PATCH] - ftdi_sio.c, usb-serial.c usb_kill_urb()"
- copy browser tab url, paste in this list, close tab, rinse, repeat. ;)

kind regards,
Jurgen

---
 drivers/usb/host/ehci-atmel.c     |    3 +++
 drivers/usb/host/ehci-mxc.c       |    3 +++
 drivers/usb/host/ehci-w90x900.c   |    3 +++
 drivers/usb/host/ehci-xilinx-of.c |    1 +
 4 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 51bd0ed..39f09e6 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -99,6 +99,7 @@ static const struct hc_driver ehci_atmel_hc_driver = {
 	.urb_enqueue		= ehci_urb_enqueue,
 	.urb_dequeue		= ehci_urb_dequeue,
 	.endpoint_disable	= ehci_endpoint_disable,
+	.endpoint_reset		= ehci_endpoint_reset,
 
 	/* scheduling support */
 	.get_frame_number	= ehci_get_frame,
@@ -110,6 +111,8 @@ static const struct hc_driver ehci_atmel_hc_driver = {
 	.bus_resume		= ehci_bus_resume,
 	.relinquish_port	= ehci_relinquish_port,
 	.port_handed_over	= ehci_port_handed_over,
+
+	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
 static int __init ehci_atmel_drv_probe(struct platform_device *pdev)
diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
index a8ad8ac..14537f6 100644
--- a/drivers/usb/host/ehci-mxc.c
+++ b/drivers/usb/host/ehci-mxc.c
@@ -95,6 +95,7 @@ static const struct hc_driver ehci_mxc_hc_driver = {
 	.urb_enqueue = ehci_urb_enqueue,
 	.urb_dequeue = ehci_urb_dequeue,
 	.endpoint_disable = ehci_endpoint_disable,
+	.endpoint_reset = ehci_endpoint_reset,
 
 	/*
 	 * scheduling support
@@ -110,6 +111,8 @@ static const struct hc_driver ehci_mxc_hc_driver = {
 	.bus_resume = ehci_bus_resume,
 	.relinquish_port = ehci_relinquish_port,
 	.port_handed_over = ehci_port_handed_over,
+
+	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
 static int ehci_mxc_drv_probe(struct platform_device *pdev)
diff --git a/drivers/usb/host/ehci-w90x900.c b/drivers/usb/host/ehci-w90x900.c
index cfa21ea..6bc3580 100644
--- a/drivers/usb/host/ehci-w90x900.c
+++ b/drivers/usb/host/ehci-w90x900.c
@@ -130,6 +130,7 @@ static const struct hc_driver ehci_w90x900_hc_driver = {
 	.urb_enqueue = ehci_urb_enqueue,
 	.urb_dequeue = ehci_urb_dequeue,
 	.endpoint_disable = ehci_endpoint_disable,
+	.endpoint_reset = ehci_endpoint_reset,
 
 	/*
 	 * scheduling support
@@ -147,6 +148,8 @@ static const struct hc_driver ehci_w90x900_hc_driver = {
 #endif
 	.relinquish_port	= ehci_relinquish_port,
 	.port_handed_over	= ehci_port_handed_over,
+
+	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
 static int __devinit ehci_w90x900_probe(struct platform_device *pdev)
diff --git a/drivers/usb/host/ehci-xilinx-of.c b/drivers/usb/host/ehci-xilinx-of.c
index 6c8076a..e8f4f36 100644
--- a/drivers/usb/host/ehci-xilinx-of.c
+++ b/drivers/usb/host/ehci-xilinx-of.c
@@ -117,6 +117,7 @@ static const struct hc_driver ehci_xilinx_of_hc_driver = {
 	.urb_enqueue		= ehci_urb_enqueue,
 	.urb_dequeue		= ehci_urb_dequeue,
 	.endpoint_disable	= ehci_endpoint_disable,
+	.endpoint_reset		= ehci_endpoint_reset,
 
 	/*
 	 * scheduling support
-- 
1.5.6.5

       reply	other threads:[~2010-10-13 10:12 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <18494093.1383561286964261272.JavaMail.PC-43$@PC-43>
2010-10-13 10:06 ` Jurgen Braam [this message]
2010-10-13 12:01   ` [PATCH][RFC] USB: EHCI: add missing {endpoint_reset,clear_tt_buffer_complete} operations Jurgen Braam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='13257307.1383581286964359341.JavaMail.PC-43$@PC-43' \
    --to=j.braam@homeautomationeurope.com \
    --cc=e9hack@googlemail.com \
    --cc=jhovold@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox