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
next parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.