Re: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On 08/14/2013 08:24 AM, Sarah Sharp wrote: On Fri, Aug 09, 2013 at 05:14:48PM -0400, Alan Stern wrote: On Fri, 9 Aug 2013, Greg KH wrote: On Thu, Aug 08, 2013 at 10:07:19PM +0200, Martin MOKREJŠ wrote: Hi, I get plenty of these in /sys/kernel/debug/kmemleak: unreferenced object 0x88019f675268 (size 32): comm usb-storage, pid 11411, jiffies 4310515592 (age 1538.100s) hex dump (first 32 bytes): 05 0f 16 00 02 07 10 02 02 00 00 00 0a 10 03 00 0e 00 01 0a ff 07 00 00 00 00 00 00 00 00 00 00 backtrace: [81811ca1] kmemleak_alloc+0x21/0x50 [81166c5e] __kmalloc+0xce/0x170 [815270d5] usb_get_bos_descriptor+0xc5/0x230 [81519048] hub_port_init+0x768/0xa20 [81519416] usb_reset_and_verify_device+0xd6/0x540 [81519990] usb_reset_device+0x110/0x190 [8154aed4] usb_stor_port_reset+0x74/0x80 [8154af6f] usb_stor_invoke_transport+0x8f/0x550 [81549df9] usb_stor_transparent_scsi_command+0x9/0x10 [8154b7ab] usb_stor_control_thread+0x16b/0x280 [810b8c95] kthread+0xe5/0xf0 [8182caec] ret_from_fork+0x7c/0xb0 [] 0x Odd, Andiry and Sarah, any thoughts? dmesg left below for completeness. Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. best regards, ksenia -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND V3 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
This driver supports devices using the NCM protocol as an encapsulation layer
for other protocols, like the E3131 Huawei 3G modem. This drivers approach was
heavily inspired by the qmi_wwan approach code model.
Suggested-by: Bjorn Mork bj...@mork.no
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/Kconfig | 11 ++
drivers/net/usb/Makefile |1 +
drivers/net/usb/huawei_cdc_ncm.c | 210 ++
3 files changed, 222 insertions(+)
create mode 100644 drivers/net/usb/huawei_cdc_ncm.c
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index d84bfd4..6e56751 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -242,6 +242,17 @@ config USB_NET_CDC_NCM
* ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
* Ericsson F5521gw Mobile Broadband Module
+config USB_NET_HUAWEI_CDC_NCM
+ tristate Huawei-style CDC NCM support
+ depends on USB_USBNET
+ select USB_WDM
+ select USB_NET_CDC_NCM
+ help
+ This driver aims to support huawei-style NCM devices, that use
ncm as a
+ transport for other protocols.
+ To compile this driver as a module, choose M here: the module
will be
+ called huawei_cdc_ncm.
+
config USB_NET_CDC_MBIM
tristate CDC MBIM support
depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index e817178..fd0e6a7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH) += ipheth.o
obj-$(CONFIG_USB_SIERRA_NET) += sierra_net.o
obj-$(CONFIG_USB_NET_CX82310_ETH) += cx82310_eth.o
obj-$(CONFIG_USB_NET_CDC_NCM) += cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM) += huawei_cdc_ncm.o
obj-$(CONFIG_USB_VL600)+= lg-vl600.o
obj-$(CONFIG_USB_NET_QMI_WWAN) += qmi_wwan.o
obj-$(CONFIG_USB_NET_CDC_MBIM) += cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
new file mode 100644
index 000..d3426b9
--- /dev/null
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,210 @@
+/*
+ * huawei_cdc_ncm.c - handles huawei-style NCM devices.
+ * Copyright (C) 2013 Enrico Mioso mrkiko...@gmail.com
+ * This driver handles devices resembling the CDC NCM standard, but
+ * encapsulating another protocol inside it. An example are some Huawei 3G
+ * devices, exposing an embedded AT channel where you can set up the NCM
+ * connection.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012 Smith Micro Software, Inc.
+ * Copyright (c) 2012 Bjørn Mork bj...@mork.no
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/netdevice.h
+#include linux/ethtool.h
+#include linux/if_vlan.h
+#include linux/ip.h
+#include linux/mii.h
+#include linux/usb.h
+#include linux/usb/cdc.h
+#include linux/usb/usbnet.h
+#include linux/usb/cdc-wdm.h
+#include linux/usb/cdc_ncm.h
+
+/* Driver data */
+struct huawei_cdc_ncm_state {
+ struct cdc_ncm_ctx *ctx;
+ atomic_t pmcount;
+ struct usb_driver *subdriver;
+ struct usb_interface *control;
+ struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+ struct huawei_cdc_ncm_state *drvstate = (void *)usbnet_dev-data;
+ int rv = 0;
+
+ if ((on atomic_add_return(1, drvstate-pmcount) == 1) || (!on
atomic_dec_and_test(drvstate-pmcount))) {
+ rv = usb_autopm_get_interface(usbnet_dev-intf);
+ if (rv 0)
+ goto err;
+ usbnet_dev-intf-needs_remote_wakeup = on;
+ usb_autopm_put_interface(usbnet_dev-intf);
+ }
+err:
+ return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int
status)
+{
+ struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+ /* can be called while disconnecting */
+ if (!usbnet_dev)
+ return 0;
+
+ return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface
*intf)
+{
+ struct cdc_ncm_ctx *ctx;
+ struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+ int ret = -ENODEV;
+ struct huawei_cdc_ncm_state *drvstate = (void *)usbnet_dev-data;
+
+ ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be
1 for NCM devices */
+ if (ret)
+ goto err;
+
+ ctx = drvstate-ctx;
+
+ if (usbnet_dev-status)
+ subdriver = usb_cdc_wdm_register(ctx-control,
+usbnet_dev-status-desc,
+
[PATCH RESEND V3 net-next 0/3] huawei_cdc_ncm driver introduction
These patches are all related to the new huawei_cdc_ncm driver, supporting
devices that use the NCM protocol as a transport layer for other protocols.
this is the case of the Huawei E3131 3G modem.
In this version - I actually added the driver file! :) I don't know how I ended
up forgetting about it, I should have learned git usage better after all this
time! Sorry for the time I wasted you.
Enrico Mioso (3):
net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
net: cdc_ncm: remove non-standard NCM device IDs
drivers/net/usb/Kconfig | 11 ++
drivers/net/usb/Makefile |1 +
drivers/net/usb/cdc_ncm.c| 17 +--
drivers/net/usb/huawei_cdc_ncm.c | 210 ++
include/linux/usb/cdc_ncm.h |3 +
5 files changed, 229 insertions(+), 13 deletions(-)
create mode 100644 drivers/net/usb/huawei_cdc_ncm.c
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND V3 net-next 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
Some drivers implementing NCM-like protocols, may re-use those functions, as is
the case in the huawei_cdc_ncm driver.
Export them via EXPORT_SYMBOL_GPL, in accordance with how other functions have
been exported.
Signed-off-by: Enrico Mioso mrkiko...@gmail.com
---
drivers/net/usb/cdc_ncm.c |6 --
include/linux/usb/cdc_ncm.h |3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..62686be 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
}
}
-static struct sk_buff *
+struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
struct sk_buff *skb_out;
@@ -885,6 +885,7 @@ error:
return NULL;
}
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
/* verify NTB header and return offset of first NDP, or negative error */
int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -965,7 +966,7 @@ error:
}
EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
{
struct sk_buff *skb;
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev-data[0];
@@ -1040,6 +1041,7 @@ err_ndp:
error:
return 0;
}
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
static void
cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..163244b 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct
usb_interface *intf);
extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct
sk_buff *skb, __le32 sign);
extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff
*skb_in);
extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *
+cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
Le Thu, 22 Aug 2013 21:39:17 +0100, Alan Stern st...@rowland.harvard.edu a écrit : This patch divides ehci-hcd's interrupt handler into a top half and a bottom half, using a tasklet to execute the latter. The conversion is very straightforward. The only subtle point is that we have to ignore interrupts that arrive while the tasklet is running (i.e., from another device on a shared IRQ line). Do you have any reason to use a tasklet instead of a thread for handling the bottom half ? We do some embedded product and we saw some scenario where usb stack and drivers can do lot's of processing in irq context. For example video uvc driver do a copy of the current image in the urb completion handler. And that's harm real time. Moving to tasklet will solve only a part of the problem : other irq won't be delayed by the usb irq handler. But realtime threads will still be preempted by tasklets. Matthieu -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Cash Awaiting Pick Up
You have won 500,000.00 Contact :wutransfer...@xtra.co.nz -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] gadgetfs: use after free in dev_release()
The call to put_dev() releases dev. Hopefully, we don't need to set the state to STATE_DEV_DISABLED anyway so I have removed those lines. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Static checker stuff. I haven't tested this, but I'm pretty sure it's correct. diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c index f255ad7..b94c049 100644 --- a/drivers/usb/gadget/inode.c +++ b/drivers/usb/gadget/inode.c @@ -1269,10 +1269,6 @@ dev_release (struct inode *inode, struct file *fd) dev-buf = NULL; put_dev (dev); - /* other endpoints were all decoupled from this device */ - spin_lock_irq(dev-lock); - dev-state = STATE_DEV_DISABLED; - spin_unlock_irq(dev-lock); return 0; } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 18/18] xhci: add trace for missed periodic transfers
This patch adds a trace event to the class 'xhci_log_msg', called
'xhci_dbg_missed_periodic_tx', that traces the debug statements which
signal that the xHC is unable to service an isochronous endpoint within
its service interval either because the endpoint's ring is full and can
not receive further data, for IN endpoints, or because the endpoint's ring
is empty, for OUT endpoints, or due to xHC internal buffers' overrun or
underrun caused by excessive latency on the transfer path.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 19 ---
drivers/usb/host/xhci-trace.h | 5 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 03f65dc..cbf3e2a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2484,18 +2484,22 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* a Ring Overrun Event for IN Isoch endpoint or Ring
* Underrun Event for OUT Isoch endpoint.
*/
- xhci_dbg(xhci, underrun event on endpoint\n);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ underrun event on endpoint);
if (!list_empty(ep_ring-td_list))
- xhci_dbg(xhci, Underrun Event for slot %d ep %d
- still with TDs queued?\n,
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ Underrun Event for slot %d ep %d
+ still with TDs queued?,
TRB_TO_SLOT_ID(le32_to_cpu(event-flags)),
ep_index);
goto cleanup;
case COMP_OVERRUN:
- xhci_dbg(xhci, overrun event on endpoint\n);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ overrun event on endpoint);
if (!list_empty(ep_ring-td_list))
- xhci_dbg(xhci, Overrun Event for slot %d ep %d
- still with TDs queued?\n,
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ Overrun Event for slot %d ep %d
+ still with TDs queued?,
TRB_TO_SLOT_ID(le32_to_cpu(event-flags)),
ep_index);
goto cleanup;
@@ -2511,7 +2515,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
* short transfer when process the ep_ring next time.
*/
ep-skip = true;
- xhci_dbg(xhci, Miss service interval error, set skip flag\n);
+ xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
+ Miss service interval error, set skip flag);
goto cleanup;
default:
if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 20364cc..c156685 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsigned int ep_num),
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 06/18] xhci: refactor TRB_RESET_DEV case into function
This patch refactors the code in TRB_RESET_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_reset_dev().
Here, in the original code, the slot id is retrieved by the command TRB.
Since this slot id matches the one in Slot ID field of the command completion
event, which is available, there is no need to determine it again.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f00d9ef..0f9863e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1400,6 +1400,20 @@ static void xhci_handle_cmd_addr_dev(struct xhci_hcd
*xhci, int slot_id,
complete(xhci-addr_dev);
}
+static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event)
+{
+ struct xhci_virt_device *virt_dev;
+
+ xhci_dbg(xhci, Completed reset device command.\n);
+ virt_dev = xhci-devs[slot_id];
+ if (virt_dev)
+ handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
+ else
+ xhci_warn(xhci, Reset device command completion
+ for disabled slot %u\n, slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1527,15 +1541,7 @@ bandwidth_change:
xhci_handle_cmd_reset_ep(xhci, event, xhci-cmd_ring-dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
- xhci_dbg(xhci, Completed reset device command.\n);
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3]));
- virt_dev = xhci-devs[slot_id];
- if (virt_dev)
- handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
- else
- xhci_warn(xhci, Reset device command completion
- for disabled slot %u\n, slot_id);
+ xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
case TRB_TYPE(TRB_NEC_GET_FW):
if (!(xhci-quirks XHCI_NEC_HOST)) {
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 02/18] xhci: rename existing functions
This patch renames the function handlers of a triggered Command Completion
Event that correspond to each command type into 'xhci_handle_cmd_type'.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ddbda35..ffd224c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,7 +755,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void handle_stopped_endpoint(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
unsigned int slot_id;
@@ -1063,9 +1063,8 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void handle_set_deq_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
unsigned int slot_id;
unsigned int ep_index;
@@ -1157,9 +1156,8 @@ static void handle_set_deq_completion(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void handle_reset_ep_completion(struct xhci_hcd *xhci,
- struct xhci_event_cmd *event,
- union xhci_trb *trb)
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+ struct xhci_event_cmd *event, union xhci_trb *trb)
{
int slot_id;
unsigned int ep_index;
@@ -1497,15 +1495,15 @@ bandwidth_change:
complete(xhci-addr_dev);
break;
case TRB_TYPE(TRB_STOP_RING):
- handle_stopped_endpoint(xhci, xhci-cmd_ring-dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, xhci-cmd_ring-dequeue, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- handle_set_deq_completion(xhci, event, xhci-cmd_ring-dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, xhci-cmd_ring-dequeue);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- handle_reset_ep_completion(xhci, event,
xhci-cmd_ring-dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, xhci-cmd_ring-dequeue);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_dbg(xhci, Completed reset device command.\n);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 08/18] xhci: refactor TRB_EVAL_CONTEXT case into function
This patch refactors the code in TRB_EVAL_CONTEXT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_eval_ctx().
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 89b4c05..0f571c5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,18 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci-devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ virt_dev-cmd_status = cmd_comp_code;
+ complete(virt_dev-cmd_completion);
+}
+
static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
u32 cmd_comp_code)
{
@@ -1532,11 +1544,8 @@ bandwidth_change:
complete(xhci-devs[slot_id]-cmd_completion);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- virt_dev = xhci-devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- xhci-devs[slot_id]-cmd_status =
GET_COMP_CODE(le32_to_cpu(event-status));
- complete(xhci-devs[slot_id]-cmd_completion);
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
+ GET_COMP_CODE(le32_to_cpu(event-status)));
break;
case TRB_TYPE(TRB_ADDR_DEV):
xhci_handle_cmd_addr_dev(xhci, slot_id,
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 04/18] xhci: refactor TRB_DISABLE_SLOT case into function
This patch refactors the code in TRB_DISABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_disable_slot().
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0abf88c..f9c380e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1380,6 +1380,19 @@ static void xhci_handle_cmd_enable_slot(struct xhci_hcd
*xhci, int slot_id,
complete(xhci-addr_dev);
}
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
+{
+ struct xhci_virt_device *virt_dev;
+
+ virt_dev = xhci-devs[slot_id];
+ if (!virt_dev)
+ return;
+ if (xhci-quirks XHCI_EP_LIMIT_QUIRK)
+ /* Delete default control endpoint resources */
+ xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ xhci_free_virt_device(xhci, slot_id);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1431,13 +1444,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
GET_COMP_CODE(le32_to_cpu(event-status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
- if (xhci-devs[slot_id]) {
- if (xhci-quirks XHCI_EP_LIMIT_QUIRK)
- /* Delete default control endpoint resources */
- xhci_free_device_endpoint_resources(xhci,
- xhci-devs[slot_id], true);
- xhci_free_virt_device(xhci, slot_id);
- }
+ xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
virt_dev = xhci-devs[slot_id];
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 11/18] xhci: add variable 'cmd_comp_code' in handle_cmd_completion()
This patch adds a new variable 'cmd_comp_code' to hold the command completion
status code aiming to reduce code duplication and to improve code readability.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7103bd9..5b85582 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1503,6 +1503,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
+ u32 cmd_comp_code;
cmd_dma = le64_to_cpu(event-cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci-cmd_ring-deq_seg,
@@ -1521,16 +1522,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
trace_xhci_cmd_completion(xhci-cmd_ring-dequeue-generic,
(struct xhci_generic_trb *) event);
- if ((GET_COMP_CODE(le32_to_cpu(event-status)) == COMP_CMD_ABORT) ||
- (GET_COMP_CODE(le32_to_cpu(event-status)) == COMP_CMD_STOP)) {
+ cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event-status));
+ if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
/* If the return value is 0, we think the trb pointed by
* command ring dequeue pointer is a good trb. The good
* trb means we don't want to cancel the trb, but it have
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci,
- GET_COMP_CODE(le32_to_cpu(event-status {
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
inc_deq(xhci, xhci-cmd_ring);
return;
}
@@ -1539,23 +1539,19 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3])
TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- xhci_handle_cmd_enable_slot(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event-status)));
+ xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- xhci_handle_cmd_config_ep(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event-status)));
+ xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_EVAL_CONTEXT):
- xhci_handle_cmd_eval_ctx(xhci, slot_id, event,
- GET_COMP_CODE(le32_to_cpu(event-status)));
+ xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci_handle_cmd_addr_dev(xhci, slot_id,
- GET_COMP_CODE(le32_to_cpu(event-status)));
+ xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci-cmd_ring-dequeue, event);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 10/18] xhci: refactor TRB_CONFIG_EP case into function
This patch refactors the code in TRB_CONFIG_EP switch case, in
handle_cmd_completion(), into a fuction named xhci_handle_cmd_config_ep().
There were added two additional variables, 'add_flags' and 'drop_flags',
to reduce line length below 80 chars and improve code readability.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 112 +++
1 file changed, 60 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b96..7103bd9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,64 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id,
+ struct xhci_event_cmd *event, u32 cmd_comp_code)
+{
+ struct xhci_virt_device *virt_dev;
+ struct xhci_input_control_ctx *ctrl_ctx;
+ unsigned int ep_index;
+ unsigned int ep_state;
+ u32 add_flags, drop_flags;
+
+ virt_dev = xhci-devs[slot_id];
+ if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
+ return;
+ /*
+* Configure endpoint commands can come from the USB core
+* configuration or alt setting changes, or because the HW
+* needed an extra configure endpoint command after a reset
+* endpoint command or streams were being configured.
+* If the command was for a halted endpoint, the xHCI driver
+* is not waiting on the configure endpoint command.
+*/
+ ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev-in_ctx);
+ if (!ctrl_ctx) {
+ xhci_warn(xhci, Could not get input context, bad type.\n);
+ return;
+ }
+ add_flags = le32_to_cpu(ctrl_ctx-add_flags);
+ drop_flags = le32_to_cpu(ctrl_ctx-drop_flags);
+ /* Input ctx add_flags are the endpoint index plus one */
+ ep_index = xhci_last_valid_endpoint(add_flags) - 1;
+ /* A usb_set_interface() call directly after clearing a halted
+* condition may race on this quirky hardware. Not worth
+* worrying about, since this is prototype hardware. Not sure
+* if this will work for streams, but streams support was
+* untested on this prototype.
+*/
+ if (xhci-quirks XHCI_RESET_EP_QUIRK
+ ep_index != (unsigned int) -1
+ add_flags - SLOT_FLAG == drop_flags) {
+ ep_state = virt_dev-eps[ep_index].ep_state;
+ if (!(ep_state EP_HALTED))
+ goto bandwidth_change;
+ xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
+ Completed config ep cmd -
+ last ep index = %d, state = %d,
+ ep_index, ep_state);
+ /* Clear internal halted state and restart ring(s) */
+ virt_dev-eps[ep_index].ep_state = ~EP_HALTED;
+ ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
+ return;
+ }
+bandwidth_change:
+ xhci_dbg_trace(xhci, trace_xhci_dbg_context_change,
+ Completed config ep cmd);
+ virt_dev-cmd_status = cmd_comp_code;
+ complete(virt_dev-cmd_completion);
+ return;
+}
+
static void xhci_handle_cmd_eval_ctx(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, u32 cmd_comp_code)
{
@@ -1445,10 +1503,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
int slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event-flags));
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
- struct xhci_input_control_ctx *ctrl_ctx;
- struct xhci_virt_device *virt_dev;
- unsigned int ep_index;
- unsigned int ep_state;
cmd_dma = le64_to_cpu(event-cmd_trb);
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci-cmd_ring-deq_seg,
@@ -1492,54 +1546,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
case TRB_TYPE(TRB_CONFIG_EP):
- virt_dev = xhci-devs[slot_id];
- if (handle_cmd_in_cmd_wait_list(xhci, virt_dev, event))
- break;
- /*
-* Configure endpoint commands can come from the USB core
-* configuration or alt setting changes, or because the HW
-* needed an extra configure endpoint command after a reset
-* endpoint command or streams were being configured.
-* If the command was for a halted endpoint, the xHCI driver
-* is not waiting on the configure endpoint command.
-*/
- ctrl_ctx = xhci_get_input_control_ctx(xhci,
-
[RFC v3 05/18] xhci: refactor TRB_ADDR_DEV case into function
This patch refactors the code in TRB_ADDR_DEV switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_addr_dev().
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f9c380e..f00d9ef 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1393,6 +1393,13 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd
*xhci, int slot_id)
xhci_free_virt_device(xhci, slot_id);
}
+static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ xhci-devs[slot_id]-cmd_status = cmd_comp_code;
+ complete(xhci-addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1505,8 +1512,8 @@ bandwidth_change:
complete(xhci-devs[slot_id]-cmd_completion);
break;
case TRB_TYPE(TRB_ADDR_DEV):
- xhci-devs[slot_id]-cmd_status =
GET_COMP_CODE(le32_to_cpu(event-status));
- complete(xhci-addr_dev);
+ xhci_handle_cmd_addr_dev(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event-status)));
break;
case TRB_TYPE(TRB_STOP_RING):
xhci_handle_cmd_stop_ep(xhci, xhci-cmd_ring-dequeue, event);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 14/18] xhci: replace 'xhci-cmd_ring-dequeue' with 'trb' in stop_ep cmd handler
This patch replaces 'xhci-cmd_ring-dequeue' with 'trb', the address of
the command TRB, since it is available to reduce line length.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 60cf547..7a58095 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -769,10 +769,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
- if (unlikely(TRB_TO_SUSPEND_PORT(
-
le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(
- le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3]));
+ if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb-generic.field[3] {
+ slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb-generic.field[3]));
virt_dev = xhci-devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 15/18] xhci: add argument 'slot_id' in stop_ep, set_deq and reset_ep cmd handlers
Since the Slot ID field in the command completion event matches the Slot ID
field in the associated command TRB for the Stop Endpoint, Set Dequeue Pointer
and Reset Endpoint commands, this patch adds in the handlers of their
completion events a 'slot_id' argument and removes the slot id calculation
in each of them.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7a58095..405db44 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,10 +755,9 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
* 2. Otherwise, we turn all the TRBs in the TD into No-op TRBs (with the
chain
* bit cleared) so that the HW will skip over them.
*/
-static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
union xhci_trb *trb, struct xhci_event_cmd *event)
{
- unsigned int slot_id;
unsigned int ep_index;
struct xhci_virt_device *virt_dev;
struct xhci_ring *ep_ring;
@@ -770,7 +769,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
struct xhci_dequeue_state deq_state;
if (unlikely(TRB_TO_SUSPEND_PORT(le32_to_cpu(trb-generic.field[3] {
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb-generic.field[3]));
virt_dev = xhci-devs[slot_id];
if (virt_dev)
handle_cmd_in_cmd_wait_list(xhci, virt_dev,
@@ -783,7 +781,6 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci,
}
memset(deq_state, 0, sizeof(deq_state));
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb-generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb-generic.field[3]));
ep = xhci-devs[slot_id]-eps[ep_index];
@@ -1061,10 +1058,9 @@ static void update_ring_for_set_deq_completion(struct
xhci_hcd *xhci,
* endpoint doorbell to restart the ring, but only if there aren't more
* cancellations pending.
*/
-static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- unsigned int slot_id;
unsigned int ep_index;
unsigned int stream_id;
struct xhci_ring *ep_ring;
@@ -1072,7 +1068,6 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci,
struct xhci_ep_ctx *ep_ctx;
struct xhci_slot_ctx *slot_ctx;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb-generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb-generic.field[3]));
stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb-generic.field[2]));
dev = xhci-devs[slot_id];
@@ -1154,13 +1149,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd
*xhci,
ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
}
-static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci,
+static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event, union xhci_trb *trb)
{
- int slot_id;
unsigned int ep_index;
- slot_id = TRB_TO_SLOT_ID(le32_to_cpu(trb-generic.field[3]));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb-generic.field[3]));
/* This command will only fail if the endpoint wasn't halted,
* but we don't care.
@@ -1554,15 +1547,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_STOP_RING:
- xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
+ xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event);
break;
case TRB_SET_DEQ:
- xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
+ xhci_handle_cmd_set_deq(xhci, slot_id, event, cmd_trb);
break;
case TRB_CMD_NOOP:
break;
case TRB_RESET_EP:
- xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
+ xhci_handle_cmd_reset_ep(xhci, slot_id, event, cmd_trb);
break;
case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 13/18] xhci: add variable 'cmd_type' in handle_cmd_completion()
This patch adds a new variable 'cmd_type' to hold the command type so that
switch cases can be simplified by removing TRB_TYPE() macro.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4ddb72e..60cf547 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1505,6 +1505,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
union xhci_trb *cmd_trb;
+ u32 cmd_type;
cmd_dma = le64_to_cpu(event-cmd_trb);
cmd_trb = xhci-cmd_ring-dequeue;
@@ -1537,38 +1538,38 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
- switch (le32_to_cpu(cmd_trb-generic.field[3])
-TRB_TYPE_BITMASK) {
- case TRB_TYPE(TRB_ENABLE_SLOT):
+ cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb-generic.field[3]));
+ switch (cmd_type) {
+ case TRB_ENABLE_SLOT:
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_DISABLE_SLOT):
+ case TRB_DISABLE_SLOT:
xhci_handle_cmd_disable_slot(xhci, slot_id);
break;
- case TRB_TYPE(TRB_CONFIG_EP):
+ case TRB_CONFIG_EP:
xhci_handle_cmd_config_ep(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_EVAL_CONTEXT):
+ case TRB_EVAL_CONTEXT:
xhci_handle_cmd_eval_ctx(xhci, slot_id, event, cmd_comp_code);
break;
- case TRB_TYPE(TRB_ADDR_DEV):
+ case TRB_ADDR_DEV:
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
- case TRB_TYPE(TRB_STOP_RING):
+ case TRB_STOP_RING:
xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
- case TRB_TYPE(TRB_SET_DEQ):
+ case TRB_SET_DEQ:
xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_CMD_NOOP):
+ case TRB_CMD_NOOP:
break;
- case TRB_TYPE(TRB_RESET_EP):
+ case TRB_RESET_EP:
xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
- case TRB_TYPE(TRB_RESET_DEV):
+ case TRB_RESET_DEV:
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
break;
- case TRB_TYPE(TRB_NEC_GET_FW):
+ case TRB_NEC_GET_FW:
xhci_handle_cmd_nec_get_fw(xhci, event);
break;
default:
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] gadgetfs: potential use after free in unbind()
ffs_data_put() can sometimes free ffs so I have moved the call down a line below the dereference. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Static checker stuff. I can't actually compile this file. diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index f394f29..1a66c5b 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -1417,8 +1417,8 @@ static void functionfs_unbind(struct ffs_data *ffs) usb_ep_free_request(ffs-gadget-ep0, ffs-ep0req); ffs-ep0req = NULL; ffs-gadget = NULL; - ffs_data_put(ffs); clear_bit(FFS_FL_BOUND, ffs-flags); + ffs_data_put(ffs); } } -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 12/18] xhci: add variable 'cmd_trb' in handle_cmd_completion()
This patch adds a new variable 'cmd_trb' to hold the address of the
command TRB, that is associated with the command completion event,
and to replace repetitions of xhci-cmd_ring-dequeue into the code.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5b85582..4ddb72e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1504,10 +1504,12 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
u64 cmd_dma;
dma_addr_t cmd_dequeue_dma;
u32 cmd_comp_code;
+ union xhci_trb *cmd_trb;
cmd_dma = le64_to_cpu(event-cmd_trb);
+ cmd_trb = xhci-cmd_ring-dequeue;
cmd_dequeue_dma = xhci_trb_virt_to_dma(xhci-cmd_ring-deq_seg,
- xhci-cmd_ring-dequeue);
+ cmd_trb);
/* Is the command ring deq ptr out of sync with the deq seg ptr? */
if (cmd_dequeue_dma == 0) {
xhci-error_bitmask |= 1 4;
@@ -1519,8 +1521,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
return;
}
- trace_xhci_cmd_completion(xhci-cmd_ring-dequeue-generic,
- (struct xhci_generic_trb *) event);
+ trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event-status));
if (cmd_comp_code == COMP_CMD_ABORT || cmd_comp_code == COMP_CMD_STOP) {
@@ -1536,7 +1537,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
}
- switch (le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3])
+ switch (le32_to_cpu(cmd_trb-generic.field[3])
TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
xhci_handle_cmd_enable_slot(xhci, slot_id, cmd_comp_code);
@@ -1554,15 +1555,15 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_addr_dev(xhci, slot_id, cmd_comp_code);
break;
case TRB_TYPE(TRB_STOP_RING):
- xhci_handle_cmd_stop_ep(xhci, xhci-cmd_ring-dequeue, event);
+ xhci_handle_cmd_stop_ep(xhci, cmd_trb, event);
break;
case TRB_TYPE(TRB_SET_DEQ):
- xhci_handle_cmd_set_deq(xhci, event, xhci-cmd_ring-dequeue);
+ xhci_handle_cmd_set_deq(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_CMD_NOOP):
break;
case TRB_TYPE(TRB_RESET_EP):
- xhci_handle_cmd_reset_ep(xhci, event, xhci-cmd_ring-dequeue);
+ xhci_handle_cmd_reset_ep(xhci, event, cmd_trb);
break;
case TRB_TYPE(TRB_RESET_DEV):
xhci_handle_cmd_reset_dev(xhci, slot_id, event);
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 09/18] xhci: remove unused 'ep_ring' variable in handle_cmd_completion()
This patch removes the variable 'ep_ring' that is assigned in
TRB_CONFIG_EP switch case but never used.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0f571c5..b96 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1448,7 +1448,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_input_control_ctx *ctrl_ctx;
struct xhci_virt_device *virt_dev;
unsigned int ep_index;
- struct xhci_ring *ep_ring;
unsigned int ep_state;
cmd_dma = le64_to_cpu(event-cmd_trb);
@@ -1522,7 +1521,6 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
ep_index != (unsigned int) -1
le32_to_cpu(ctrl_ctx-add_flags) - SLOT_FLAG ==
le32_to_cpu(ctrl_ctx-drop_flags)) {
- ep_ring = xhci-devs[slot_id]-eps[ep_index].ring;
ep_state = xhci-devs[slot_id]-eps[ep_index].ep_state;
if (!(ep_state EP_HALTED))
goto bandwidth_change;
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 03/18] xhci: refactor TRB_ENABLE_SLOT case into function
This patch refactors the code in TRB_ENABLE_SLOT switch case in
handle_cmd_completion() into a fuction named xhci_handle_cmd_enable_slot().
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ffd224c..0abf88c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1370,6 +1370,16 @@ static int handle_stopped_cmd_ring(struct xhci_hcd *xhci,
return cur_trb_is_good;
}
+static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
+{
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci-slot_id = slot_id;
+ else
+ xhci-slot_id = 0;
+ complete(xhci-addr_dev);
+}
+
static void handle_cmd_completion(struct xhci_hcd *xhci,
struct xhci_event_cmd *event)
{
@@ -1417,11 +1427,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
switch (le32_to_cpu(xhci-cmd_ring-dequeue-generic.field[3])
TRB_TYPE_BITMASK) {
case TRB_TYPE(TRB_ENABLE_SLOT):
- if (GET_COMP_CODE(le32_to_cpu(event-status)) == COMP_SUCCESS)
- xhci-slot_id = slot_id;
- else
- xhci-slot_id = 0;
- complete(xhci-addr_dev);
+ xhci_handle_cmd_enable_slot(xhci, slot_id,
+ GET_COMP_CODE(le32_to_cpu(event-status)));
break;
case TRB_TYPE(TRB_DISABLE_SLOT):
if (xhci-devs[slot_id]) {
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 17/18] xhci: add label 'update_ring' in handle_cmd_completion()
This patch adds the label 'update_ring' for the common code path:
inc_deq(xhci, xhci-cmd_ring);
return;
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e13531f..03f65dc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1522,10 +1522,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
* been stopped by host. So we should handle it normally.
* Otherwise, driver should invoke inc_deq() and return.
*/
- if (handle_stopped_cmd_ring(xhci, cmd_comp_code)) {
- inc_deq(xhci, xhci-cmd_ring);
- return;
- }
+ if (handle_stopped_cmd_ring(xhci, cmd_comp_code))
+ goto update_ring;
}
cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb-generic.field[3]));
@@ -1567,7 +1565,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci-error_bitmask |= 1 6;
break;
}
+update_ring:
inc_deq(xhci, xhci-cmd_ring);
+ return;
}
static void handle_vendor_event(struct xhci_hcd *xhci,
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3 01/18] xhci: remove unused argument from xhci_giveback_urb_in_irq()
This patch removes the adjective argument from xhci_giveback_urb_in_irq(),
since it is not used in the function anymore.
Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com
---
Differences from version 2:
The original patchset was splitted into multiple 'single change'
patches to help patch review. Also, there were added some cleanup
patches that change the existed command handlers to use the new
variables created.
Finally, in the patchset was added the patch for missed periodic
transfer trace so that it can be applied cleanly over the previous
patches, since it affects the same file.
drivers/usb/host/xhci-ring.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b35af1..ddbda35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -716,7 +716,7 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd
*xhci,
/* Must be called with xhci-lock held in interrupt context */
static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
- struct xhci_td *cur_td, int status, char *adjective)
+ struct xhci_td *cur_td, int status)
{
struct usb_hcd *hcd;
struct urb *urb;
@@ -877,7 +877,7 @@ remove_finished_td:
/* Doesn't matter what we pass for status, since the core will
* just overwrite it (because the URB has been unlinked).
*/
- xhci_giveback_urb_in_irq(xhci, cur_td, 0, cancelled);
+ xhci_giveback_urb_in_irq(xhci, cur_td, 0);
/* Stop processing the cancelled list if the watchdog timer is
* running.
@@ -987,7 +987,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
if (!list_empty(cur_td-cancelled_td_list))
list_del_init(cur_td-cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, killed);
+ -ESHUTDOWN);
}
while (!list_empty(temp_ep-cancelled_td_list)) {
cur_td = list_first_entry(
@@ -996,7 +996,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
cancelled_td_list);
list_del_init(cur_td-cancelled_td_list);
xhci_giveback_urb_in_irq(xhci, cur_td,
- -ESHUTDOWN, killed);
+ -ESHUTDOWN);
}
}
}
--
1.8.3.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] staging: ozwpan: Create deferred work to destroy PD object.
On Thu, Aug 22, 2013 at 05:38:51PM +0100, Rupesh Gujare wrote: +/*-- * Context: softirq-serialized */ Don't resend the patch, but these comments are not in kernel style. It's explained in Documentation/kernel-doc-nano-HOWTO.txt The main thing is that could you just delete all the lines? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] usb: chipidea: udc: Consolidate the calling of ci-driver-disconnect
The udc-core will call gadget's driver-disconnect, so we should avoid
calling gadget's disconnect again at ci_udc_stop in case the gadget's
unbind free some structs which is still used at gadget's disconnect.
Tested-by: Marek Vasut ma...@denx.de
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/udc.c | 10 +++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 6b4c2f2..858f535 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -686,9 +686,6 @@ static int _gadget_stop_activity(struct usb_gadget *gadget)
usb_ep_fifo_flush(ci-ep0out-ep);
usb_ep_fifo_flush(ci-ep0in-ep);
- if (ci-driver)
- ci-driver-disconnect(gadget);
-
/* make sure to disable all endpoints */
gadget_for_each_ep(ep, gadget) {
usb_ep_disable(ep);
@@ -717,6 +714,11 @@ __acquires(ci-lock)
{
int retval;
+ if (ci-gadget.speed != USB_SPEED_UNKNOWN) {
+ if (ci-driver)
+ ci-driver-disconnect(ci-gadget);
+ }
+
spin_unlock(ci-lock);
retval = _gadget_stop_activity(ci-gadget);
if (retval)
@@ -1461,6 +1463,8 @@ static int ci_udc_vbus_session(struct usb_gadget
*_gadget, int is_active)
hw_device_state(ci, ci-ep0out-qh.dma);
dev_dbg(ci-dev, Connected to host\n);
} else {
+ if (ci-driver)
+ ci-driver-disconnect(ci-gadget);
hw_device_state(ci, 0);
if (ci-platdata-notify_event)
ci-platdata-notify_event(ci,
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget
When we rmmod gadget, the ci-driver needs to be cleared. Otherwise, when we plug in usb cable again, the driver will consider gadget is there, and go to enumeration procedure, but in fact, it was removed. ci_hdrc ci_hdrc.0: Connected to host Unable to handle kernel paging request at virtual address 7f02a42c pgd = 80004000 [7f02a42c] *pgd=3f13d811, *pte=, *ppte= Internal error: Oops: 7 [#1] SMP ARM Modules linked in: usb_f_acm u_serial libcomposite configfs [last unloaded: g_serial] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.10.0+ #42 task: 807dba88 ti: 807d task.ti: 807d PC is at udc_irq+0x8fc/0xea4 LR is at l2x0_cache_sync+0x5c/0x6c pc : [803de7f4]lr : [8001d0f0]psr: 2193 sp : 807d1d98 ip : 807d1d80 fp : 807d1df4 r10: af809900 r9 : 808184d4 r8 : 00080001 r7 : 00082001 r6 : afb711f8 r5 : afb71010 r4 : ffea r3 : 7f02a41c r2 : afb71010 r1 : 807d1dc0 r0 : afb71068 Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c53c7d Table: 3f01804a DAC: 0017 Process swapper/0 (pid: 0, stack limit = 0x807d0238) Stack: (0x807d1d98 to 0x807d2000) 1d80: afb71014 1da0: 40f6 0001 7530 afb71010 001dcd65 1dc0: 01000680 0040 807d1e2c afb71010 004e 004b 1de0: 808184d4 af809900 807d1e0c 807d1df8 803dbc24 803ddf04 afba75c0 004e 1e00: 807d1e44 807d1e10 8007a19c 803dbb9c 8108e7e0 8108e7e0 9ceddce0 af809900 1e20: 004e 807d 004b 0010 807d1e5c 807d1e48 1e40: 8007a334 8007a154 af809900 004e 807d1e74 807d1e60 8007d3b4 8007a2f0 1e60: 004b 807cce3c 807d1e8c 807d1e78 80079b08 8007d300 0180 807d8ba0 1e80: 807d1eb4 807d1e90 8000eef4 80079aec f400010c 807d8ce4 807d1ed8 1ea0: f4000100 96d5c75d 807d1ed4 807d1eb8 80008600 8000eeac 8042699c 6013 1ec0: 807d1f0c 807d1f54 807d1ed8 8000e180 800085dc 807d1f20 0046 1ee0: 9cedd275 0010 8108f080 807de294 0001 807de248 96d5c75d 0010 1f00: 807d1f54 807d1f20 8005ff54 8042699c 6013 1f20: 9cedd275 0010 0005 8108f080 8108f080 0001 807de248 8086bd00 1f40: 807d 0001 807d1f7c 807d1f58 80426af0 80426950 807d 1f60: 808184c0 808184c0 807d8954 805b886c 807d1f8c 807d1f80 8000f294 80426a44 1f80: 807d1fac 807d1f90 8005f110 8000f288 807d1fac 807d8908 805b4748 807dc86c 1fa0: 807d1fbc 807d1fb0 805aa58c 8005f068 807d1ff4 807d1fc0 8077c860 805aa530 1fc0: 8077c330 807bef88 10c53c7d 1fe0: 807d88d0 807bef84 807d1ff8 10008074 8077c594 Backtrace: [803ddef8] (udc_irq+0x0/0xea4) from [803dbc24] (ci_irq+0x94/0x14c) [803dbb90] (ci_irq+0x0/0x14c) from [8007a19c] (handle_irq_event_percpu+0x54/0x19c) r5:004e r4:afba75c0 [8007a148] (handle_irq_event_percpu+0x0/0x19c) from [8007a334] (handle_irq_event+0x50/0x70) [8007a2e4] (handle_irq_event+0x0/0x70) from [8007d3b4] (handle_fasteoi_irq+0xc0/0x16c) r5:004e r4:af809900 [8007d2f4] (handle_fasteoi_irq+0x0/0x16c) from [80079b08] (generic_handle_irq+0x28/0x38) r5:807cce3c r4:004b [80079ae0] (generic_handle_irq+0x0/0x38) from [8000eef4] (handle_IRQ+0x54/0xb4) r4:807d8ba0 r3:0180 [8000eea0] (handle_IRQ+0x0/0xb4) from [80008600] (gic_handle_irq+0x30/0x64) r8:96d5c75d r7:f4000100 r6:807d1ed8 r5:807d8ce4 r4:f400010c r3: [800085d0] (gic_handle_irq+0x0/0x64) from [8000e180] (__irq_svc+0x40/0x54) Exception stack(0x807d1ed8 to 0x807d1f20) 1ec0: 807d1f20 0046 1ee0: 9cedd275 0010 8108f080 807de294 0001 807de248 96d5c75d 0010 1f00: 807d1f54 807d1f20 8005ff54 8042699c 6013 r7:807d1f0c r6: r5:6013 r4:8042699c [80426944] (cpuidle_enter_state+0x0/0xf4) from [80426af0] (cpuidle_idle_call+0xb8/0x174) r9:0001 r8:807d r7:8086bd00 r6:807de248 r5:0001 r4:8108f080 [80426a38] (cpuidle_idle_call+0x0/0x174) from [8000f294] (arch_cpu_idle+0x18/0x5c) [8000f27c] (arch_cpu_idle+0x0/0x5c) from [8005f110] (cpu_startup_entry+0xb4/0x148) [8005f05c] (cpu_startup_entry+0x0/0x148) from [805aa58c] (rest_init+0x68/0x80) r7:807dc86c [805aa524] (rest_init+0x0/0x80) from [8077c860] (start_kernel+0x2d8/0x334) [8077c588] (start_kernel+0x0/0x334) from [10008074] (0x10008074) Code: e59031e0 e51b203c e24b1034 e2820058 (e5933010) ---[ end trace f874b2c5533c04bc ]--- Kernel panic - not syncing: Fatal exception in interrupt Tested-by: Marek Vasut ma...@denx.de Signed-off-by: Peter Chen peter.c...@freescale.com --- drivers/usb/chipidea/udc.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 858f535..2ea5871 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -1671,13 +1671,13 @@ static int ci_udc_stop(struct usb_gadget *gadget,
[PATCH 0/5] Chipidea Misc patchset
Hi Alex, Below are un-queued chipidea patches, some of them were reviewd. Peter Chen (5): usb: host: delete chipidea dependency usb: chipidea: udc: Consolidate the calling of ci-driver-disconnect usb: chipidea: udc: fix the oops when plugs in usb cable after rmmod gadget usb: chipidea: Fix memleak for ci-hw_bank.regmap when driver removal usb: chipidea: move more platform related things to ci_get_platdata drivers/usb/chipidea/core.c | 20 ++-- drivers/usb/chipidea/udc.c | 12 drivers/usb/host/Kconfig|4 ++-- 3 files changed, 20 insertions(+), 16 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] usb: host: delete chipidea dependency
Now, chipidea host has already depended on USB_EHCI_HCD Acked-by: Alan Stern st...@rowland.harvard.edu Signed-off-by: Peter Chen peter.c...@freescale.com --- drivers/usb/host/Kconfig |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 344d5e2..61466c5 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -65,7 +65,7 @@ config USB_EHCI_HCD config USB_EHCI_ROOT_HUB_TT bool Root Hub Transaction Translators - depends on USB_EHCI_HCD || USB_CHIPIDEA_HOST + depends on USB_EHCI_HCD ---help--- Some EHCI chips have vendor-specific extensions to integrate transaction translators, so that no OHCI or UHCI companion @@ -77,7 +77,7 @@ config USB_EHCI_ROOT_HUB_TT config USB_EHCI_TT_NEWSCHED bool Improved Transaction Translator scheduling - depends on USB_EHCI_HCD || USB_CHIPIDEA_HOST + depends on USB_EHCI_HCD default y ---help--- This changes the periodic scheduling code to fill more of the low -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] usb: chipidea: Fix memleak for ci-hw_bank.regmap when driver removal
It needs to free ci-hw_bank.regmap explicitly since it is not managed resource. Signed-off-by: Peter Chen peter.c...@freescale.com --- drivers/usb/chipidea/core.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 9462640..23763dc 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -605,6 +605,7 @@ static int ci_hdrc_remove(struct platform_device *pdev) dbg_remove_files(ci); free_irq(ci-irq, ci); ci_role_destroy(ci); + kfree(ci-hw_bank.regmap); return 0; } -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] usb: chipidea: move more platform related things to ci_get_platdata
Like vbus, the dr_mode and phy_mode are also got from glue layer's
platform data or device node.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/core.c | 19 +--
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 23763dc..ba8d6ce 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -381,6 +381,15 @@ static int ci_get_platdata(struct device *dev,
return PTR_ERR(platdata-reg_vbus);
}
+ if (!ci-platdata-phy_mode)
+ ci-platdata-phy_mode = of_usb_get_phy_mode(dev-of_node);
+
+ if (!ci-platdata-dr_mode)
+ ci-platdata-dr_mode = of_usb_get_dr_mode(dev-of_node);
+
+ if (ci-platdata-dr_mode == USB_DR_MODE_UNKNOWN)
+ ci-platdata-dr_mode = USB_DR_MODE_OTG;
+
return 0;
}
@@ -473,7 +482,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
void __iomem*base;
int ret;
enum usb_dr_mode dr_mode;
- struct device_node *of_node = dev-of_node ?: dev-parent-of_node;
if (!dev-platform_data) {
dev_err(dev, platform data missing\n);
@@ -514,17 +522,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
ci_get_otg_capable(ci);
- if (!ci-platdata-phy_mode)
- ci-platdata-phy_mode = of_usb_get_phy_mode(of_node);
-
hw_phymode_configure(ci);
- if (!ci-platdata-dr_mode)
- ci-platdata-dr_mode = of_usb_get_dr_mode(of_node);
-
- if (ci-platdata-dr_mode == USB_DR_MODE_UNKNOWN)
- ci-platdata-dr_mode = USB_DR_MODE_OTG;
-
dr_mode = ci-platdata-dr_mode;
/* initialize role(s) before the interrupt is requested */
if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] usb: gadget: zero: Add flexible auto remote wakeup test method
On Wed, Aug 07, 2013 at 05:18:13AM +0800, Peter Chen wrote:
In order to increase test coverage, we can change the interval between
two remote wakeups every time, and the interval can be any user defined
value. This change will no affect current behavior if the user does not
use two introduced module paramters.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Changes for v2:
- Change some typo and description
Ping.
drivers/usb/gadget/zero.c | 25 +++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c
index 0deb9d6..21da094 100644
--- a/drivers/usb/gadget/zero.c
+++ b/drivers/usb/gadget/zero.c
@@ -95,6 +95,18 @@ unsigned autoresume = DEFAULT_AUTORESUME;
module_param(autoresume, uint, S_IRUGO);
MODULE_PARM_DESC(autoresume, zero, or seconds before remote wakeup);
+/* Maximum Autoresume time */
+unsigned max_autoresume;
+module_param(max_autoresume, uint, S_IRUGO);
+MODULE_PARM_DESC(max_autoresume, maximum seconds before remote wakeup);
+
+/* Interval between two remote wakeups */
+unsigned autoresume_interval_ms;
+module_param(autoresume_interval_ms, uint, S_IRUGO);
+MODULE_PARM_DESC(autoresume_interval_ms,
+ milliseconds to increase successive wakup delays);
+
+static unsigned autoresume_step_ms;
/*-*/
static struct usb_device_descriptor device_desc = {
@@ -183,8 +195,16 @@ static void zero_suspend(struct usb_composite_dev *cdev)
return;
if (autoresume) {
- mod_timer(autoresume_timer, jiffies + (HZ * autoresume));
- DBG(cdev, suspend, wakeup in %d seconds\n, autoresume);
+ if (max_autoresume
+ (autoresume_step_ms max_autoresume * 1000))
+ autoresume_step_ms = autoresume * 1000;
+
+ mod_timer(autoresume_timer, jiffies +
+ msecs_to_jiffies(autoresume_step_ms));
+ DBG(cdev, suspend, wakeup in %d milliseconds\n,
+ autoresume_step_ms);
+
+ autoresume_step_ms += autoresume_interval_ms;
} else
DBG(cdev, %s\n, __func__);
}
@@ -316,6 +336,7 @@ static int __init zero_bind(struct usb_composite_dev
*cdev)
if (autoresume) {
sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
+ autoresume_step_ms = autoresume * 1000;
}
/* support OTG systems */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: implement AMD remote wakeup quirk
Hi Alan, Thank you to your comments. On Fri, Aug 23, 2013 at 12:51:44AM +0800, Alan Stern wrote: On Thu, 22 Aug 2013, Huang Rui wrote: Hi Sarah, Alan, The following patches are required to resolve remote wake issues with certain devices. Issue description: If the remote wake is issued from the device in a specific timing condition while the system is entering sleep state then it may cause system to auto wake on subsequent sleep cycle. Root Cause: Host controller rebroadcasts the Resume signal 100 µseconds after receiving the original resume event from the device. For proper function, some devices may require the rebroadcast of resume event within the USB spec of 100µS. Without this quirk, some AMD platform doesn't hold the resume right away when there is a resume signal from LS device like mouse. So it should reset the port which attached with mouse. What's special about LS devices? Or mice? Shouldn't the resume signal be sent within 100 us for every device? Yes, only mice would trigger this issue. Reproduce steps: 1. Enable remote wakeup for usb mouse. 2. Execute S3. 3. Click mouse _intensely_ (more than 10 times) to wake the system up. 4. Then execute S3 again. 5. Observe that the system auto wake up. Actually, I tested all the mice in my side, this issue existed. Patch 1 is the implementation for xHCI driver. Patch 2 is the implementation for OHCI driver. They are generated on gregkh/usb usb-next. You should have put the changes to core/hub.c and pci-quirks into a third patch, instead of mixing them in with the xHCI changes. I got it, will do it in v2. Thanks, Rui -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: ohci: add AMD remote wakeup quirk into ohci driver
On Fri, Aug 23, 2013 at 01:00:29AM +0800, Alan Stern wrote:
On Thu, 22 Aug 2013, Huang Rui wrote:
The following patch is required to resolve remote wake issues with
certain devices.
Issue description:
If the remote wake is issued from the device in a specific timing
condition while the system is entering sleep state then it may cause
system to auto wake on subsequent sleep cycle.
Root Cause:
Host controller rebroadcasts the Resume signal 100 µseconds after
receiving the original resume event from the device. For proper
function, some devices may require the rebroadcast of resume event
within the USB spec of 100µS.
This patch is for OHCI driver to add AMD remote wakeup fix.
Here you need to put a description of what the fix is and how it works.
OK, I will do it in v2.
This should be backported to kernels as old as 3.9, that contrain the
commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add
find_raw_port_number callback to struct hc_driver() and the last
patch that AMD remote wakeup quirk for xhci.
Cc: sta...@vger.kernel.org
Signed-off-by: Huang Rui ray.hu...@amd.com
---
drivers/usb/host/ohci-hub.c | 42 ++
drivers/usb/host/ohci-pci.c | 13 +
drivers/usb/host/ohci.h | 23 ---
3 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index 2347ab8..0af45e3 100644
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -41,6 +41,7 @@
static void dl_done_list (struct ohci_hcd *);
static void finish_unlinks (struct ohci_hcd *, u16);
+static inline int root_port_reset (struct ohci_hcd *, unsigned);
#ifdef CONFIG_PM
static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
@@ -293,6 +294,44 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
return rc;
}
+/*
+ * Reset port which attached with mouse.
+ *
+ * If the remote wake is issued from the device in a specific timing
+ * condition while the system is entering sleep state then it may
+ * cause system to auto wake on subsequent sleep cycle.
+ *
+ * Host controller rebroadcasts the Resume signal 100 µseconds after
+ * receiving the original resume event from the device. For proper
+ * function, some devices may require the rebroadcast of resume event
+ * within the USB spec of 100µS.
+ *
+ * Without this quirk, some AMD platform doesn't hold the resume right
+ * away when there is a resume signal from LS device like mouse. So it
+ * should reset the port which attached with mouse.
+ */
+static int ohci_reset_port_by_mouse(struct ohci_hcd *ohci)
+{
+ u32 temp;
+ struct usb_device *hdev, *child;
+ int port1, wIndex;
+
+ hdev = hcd_to_bus(ohci_to_hcd(ohci))-root_hub;
+
+ usb_hub_for_each_child(hdev, port1, child) {
+ wIndex = port1 - 1;
+ temp = roothub_portstatus (ohci, wIndex);
+ dbg_port(ohci, Get port status, wIndex, temp);
+ if (is_usb_mouse(child)) {
+ ohci_dbg(ohci, Connencted mouse on port1%d.\n,
+ wIndex);
+ root_port_reset(ohci, wIndex);
+ }
+ }
+
+ return 0;
+}
Without more explanation, this seems completely wrong. Why do you want
to reset the port? What happens if you don't reset the port?
The resume signal from the port attached mouse can't be cleared or
holden if wakeup from S3 with click mouse intensely, so excute S3 at
the second time, then system detects the wrong resume signal and auto
wake. If reset this port, the resume signal is cleared. Then system
doesn't auto wake at the second time.
Wouldn't it be simpler to disable the port?
Sorry, I don't get your meaning of these words. Why?
Then usbcore would do the
reset for you. That certainly is safer than a reset.
Could you point out?
Instead of doing this always for all mice, wouldn't it be better to
test first and see whether it really is needed?
Actually, I tested all the mice in my hand, this issue existed. Only
mice would trigger this bugy.
Thanks,
Rui
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel Oops: 0003 on usbhid_submit_report
Hi, I just wanna followup this issue a bit more. We have now tested on several hardware platform, and it is only on one platform it fails. The platform it fails on is Atom Intel e6xx CPU with EG20T PCH platform controller. We do not observe it on the Intel i7 3517UE CPU with Intel 6/7 series system controller. We see this error around 4 times over 100 boots / 2 hours runtime. After the Oops our process is in uninterruptable sleep state (D state). The process is not killable when in this state. The process also uses around 70% CPU, maybe because it is waiting for the ioctl system call to return? We haven't found another way to recover from besides rebooting the system. How can we proceed to fix this issue? Andreas On Thursday, August 15, 2013 at 8:37 AM, Greg KH wrote: When communicating with AT90USB1287, at random intervals (1/25 boots) the linux hid_output_field Oopses and kills the communicating thread. The AT90USB1287 microcontroller uses LUFA library for usb/hid communication. It is trigged by a ioctl call from userspace and fails in a kernel paging request. The system is after the oops in a state where no hid commands is sent anymore and only a boot can fix the system. Keywords: usbhid hid Kernel version: Linux version 3.8.13-03081301-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201305311535 SMP Fri May 31 19:44:30 UTC 2013 Oopses: http://paste.debian.net/24305 http://paste.debian.net/24306 http://paste.debian.net/24307 Code: The error is triggered by: ioctl(fd,HIDIOCSUSAGES, ref_multi_u); ioctl(fd,HIDIOCSREPORT, rep_info_u); Notes: It is very hard to reproduce so seems like race condition… Any tips to resolve/workaround this issue is appreciated and please let me know if my information is incomplete (This is my first kernel bug report) Any chance you can try a supported kernel, like 3.10.6 or 3.11-rc5 to see if that also causes problems? We can't do anything with distro-specific kernel releases like your 3.8.13 release from Ubuntu, sorry. I've now tried with kernel 3.10.6 (http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.10.6-saucy/) and I can trigger the same issue on this kernel. Here is a paste from last Oops on this kernel: http://paste.debian.net/24993/ I am also using usbmon to monitor the usb bus traffic, but cannot see anything that should cause the driver to Oops. Is there any way to find out what can trigger this issue? -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] staging: ozwpan: Create deferred work to destroy PD object.
On 23/08/13 10:05, Dan Carpenter wrote: On Thu, Aug 22, 2013 at 05:38:51PM +0100, Rupesh Gujare wrote: +/*-- * Context: softirq-serialized */ Don't resend the patch, but these comments are not in kernel style. It's explained in Documentation/kernel-doc-nano-HOWTO.txt The main thing is that could you just delete all the lines? OK Dan, looks reasonable. -- Regards, Rupesh Gujare -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel Oops: 0003 on usbhid_submit_report
On Fri, Aug 23, 2013 at 12:52 PM, Andreas Lillebø Holm andr...@sqhead.com wrote: Hi, I just wanna followup this issue a bit more. We have now tested on several hardware platform, and it is only on one platform it fails. The platform it fails on is Atom Intel e6xx CPU with EG20T PCH platform controller. We do not observe it on the Intel i7 3517UE CPU with Intel 6/7 series system controller. We see this error around 4 times over 100 boots / 2 hours runtime. After the Oops our process is in uninterruptable sleep state (D state). The process is not killable when in this state. The process also uses around 70% CPU, maybe because it is waiting for the ioctl system call to return? We haven't found another way to recover from besides rebooting the system. How can we proceed to fix this issue? IIRC, Jiri fixed some paging requests with the two following patches: http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=bc197eedef1ae082ec662c64c3f4aa302821fb7a At least, these two patches are related to hid_output_report(). I think they are scheduled for 3.12, so I can not find them in Linus' tree. It would worth trying them :) Cheers, Benjamin On Thursday, August 15, 2013 at 8:37 AM, Greg KH wrote: When communicating with AT90USB1287, at random intervals (1/25 boots) the linux hid_output_field Oopses and kills the communicating thread. The AT90USB1287 microcontroller uses LUFA library for usb/hid communication. It is trigged by a ioctl call from userspace and fails in a kernel paging request. The system is after the oops in a state where no hid commands is sent anymore and only a boot can fix the system. Keywords: usbhid hid Kernel version: Linux version 3.8.13-03081301-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201305311535 SMP Fri May 31 19:44:30 UTC 2013 Oopses: http://paste.debian.net/24305 http://paste.debian.net/24306 http://paste.debian.net/24307 Code: The error is triggered by: ioctl(fd,HIDIOCSUSAGES, ref_multi_u); ioctl(fd,HIDIOCSREPORT, rep_info_u); Notes: It is very hard to reproduce so seems like race condition… Any tips to resolve/workaround this issue is appreciated and please let me know if my information is incomplete (This is my first kernel bug report) Any chance you can try a supported kernel, like 3.10.6 or 3.11-rc5 to see if that also causes problems? We can't do anything with distro-specific kernel releases like your 3.8.13 release from Ubuntu, sorry. I've now tried with kernel 3.10.6 (http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.10.6-saucy/) and I can trigger the same issue on this kernel. Here is a paste from last Oops on this kernel: http://paste.debian.net/24993/ I am also using usbmon to monitor the usb bus traffic, but cannot see anything that should cause the driver to Oops. Is there any way to find out what can trigger this issue? -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel Oops: 0003 on usbhid_submit_report
On Fri, 23 Aug 2013, Benjamin Tissoires wrote: IIRC, Jiri fixed some paging requests with the two following patches: http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=bc197eedef1ae082ec662c64c3f4aa302821fb7a At least, these two patches are related to hid_output_report(). I think they are scheduled for 3.12, so I can not find them in Linus' tree. It would worth trying them :) All three reported oopses happened on page boundary exactly, so this pretty much looks like my implement() fix is the one. Andreas, could you please apply http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca and report back whether it makes your problem go away? Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel Oops: 0003 on usbhid_submit_report
All three reported oopses happened on page boundary exactly, so this pretty much looks like my implement() fix is the one. Andreas, could you please apply http://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-nextid=27ce405039bfe6d3f4143415c638f56a3df77dca and report back whether it makes your problem go away? I will build and test this patch and hopefully it will resolve my issues :) -- Andreas Lillebø Holm -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v3 17/18] xhci: add label 'update_ring' in handle_cmd_completion()
Hello. On 23-08-2013 12:15, Xenia Ragiadakou wrote: This patch adds the label 'update_ring' for the common code path: inc_deq(xhci, xhci-cmd_ring); return; Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- drivers/usb/host/xhci-ring.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index e13531f..03f65dc 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c [...] @@ -1567,7 +1565,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci-error_bitmask |= 1 6; break; } +update_ring: inc_deq(xhci, xhci-cmd_ring); + return; *return* not needed here, at end of function. } WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v3 17/18] xhci: add label 'update_ring' in handle_cmd_completion()
On 08/23/2013 04:19 PM, Sergei Shtylyov wrote: Hello. On 23-08-2013 12:15, Xenia Ragiadakou wrote: This patch adds the label 'update_ring' for the common code path: inc_deq(xhci, xhci-cmd_ring); return; Signed-off-by: Xenia Ragiadakou burzalod...@gmail.com --- drivers/usb/host/xhci-ring.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index e13531f..03f65dc 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xcorrect ithci-ring.c [...] @@ -1567,7 +1565,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, xhci-error_bitmask |= 1 6; break; } +update_ring: inc_deq(xhci, xhci-cmd_ring); +return; *return* not needed here, at end of function. } WBR, Sergei Hi Sergei, Yes, you are right. Thx! I will wait Sarah to review the patchset and i will send another version to fix this. If you have any other suggestions, please let me know :) best regards, ksenia -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Fri, 23 Aug 2013, Xenia Ragiadakou wrote: Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? Yes. I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. usb_reset_and_verify_device() checks to see if the descriptors changed when the device was reset. (If they did then we need to re-enumerate the device.) It calls descriptors_changed() to do this checking. Originally there were no BOS descriptors, so of course the routine didn't try to compare them. But now that they exist, we need to compare the old and new descriptor values, same as for the device and config descriptors. And of course we need to deallocate one of the BOS descriptors, instead of leaking it. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
On Fri, 23 Aug 2013, Matthieu CASTET wrote: Le Thu, 22 Aug 2013 21:39:17 +0100, Alan Stern st...@rowland.harvard.edu a �crit : This patch divides ehci-hcd's interrupt handler into a top half and a bottom half, using a tasklet to execute the latter. The conversion is very straightforward. The only subtle point is that we have to ignore interrupts that arrive while the tasklet is running (i.e., from another device on a shared IRQ line). Do you have any reason to use a tasklet instead of a thread for handling the bottom half ? Yes. Measurements a few weeks ago showed that using a threaded handler significantly decreased the throughput for USB mass-storage devices. We do some embedded product and we saw some scenario where usb stack and drivers can do lot's of processing in irq context. For example video uvc driver do a copy of the current image in the urb completion handler. And that's harm real time. The uvcvideo driver could always move its processing to a work queue or kernel thread. Moving to tasklet will solve only a part of the problem : other irq won't be delayed by the usb irq handler. But realtime threads will still be preempted by tasklets. On the other hand, kernels with the RT patches don't have this problem. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT RFC] USB: Fix USB device disconnects on resume.
On Wed, 21 Aug 2013, Sarah Sharp wrote: Background -- The USB 2.0 specification, section 7.1.7.7, says that upon device remote wakeup signaling, the first active hub (which is often the roothub) must rebroadcast the resume signaling for at least 20 ms (TDRSMDN). After that's done, the hub's suspend status change bit will be set, and system software must not access the device for at least 10 ms (TRSMRCY). It turns out that TRSMRCY is a *minimum*, not a *maximum*, according to Table 7-14. That means the port can actually take longer than TRSMRCY to resume. Any attempt to communicate with the device, or reset the device, will result in a USB device disconnect. By the way, I just noticed your Google+ posting about this. I think you (and perhaps the engineers you spoke with) may have misunderstood what Table 7-14 means when it lists 10 ms as the _minimum_ value for TRSMRCY. This delay value is a requirement on the OS. The host system must not access the device until at least 10 ms after the resume is complete. The system can wait longer than that if it wants -- that's why 10 ms is a minimum. It just has to avoid accessing the device sooner. A _minimum_ value on the host side translates into a _maximum_ value on the device side. The device can safely assume that it can spend up to 10 ms getting back into shape after a resume, but no more. After 10 ms, the host may try to communicate with it. Then, when the USB core calls into get port status, it transitions the port from the Resume state to the RExit state by changing the port link state to U0. The xHCI driver will get a port status change event when that transition is complete, but that port status change event is currently ignored. The excess delay you observe with xHCI is the time spent in the RExit substate? That probably should not be counted as part of the TRSMRCY period. It's hard to say for certain, because TRSMRCY is described only in the USB-2 spec and not in the xHCI spec, and vice versa for RExit. Still, it's reasonable to assume that the TRSMRCY period should begin when the port changes back to U0, not when it leaves the RESUME state and enters RExit. So in the end this appears to be a simple bug in xhci-hcd. The Get-Port-Status request that terminates the resume signalling should wait until the port goes back into U0 (which agrees with what you have already decided, of course). ehci-hcd does something similar: /* stop resume signaling */ temp = ~(PORT_RWC_BITS | PORT_SUSPEND | PORT_RESUME); ehci_writel(ehci, temp, status_reg); clear_bit(wIndex, ehci-resuming_ports); retval = ehci_handshake(ehci, status_reg, PORT_RESUME, 0, 2000 /* 2msec */); The ehci_handshake call busy-waits until the controller turns off the PORT_RESUME bit, which happens when the port has switched to a high-speed idle. It's supposed to take no more than 2 ms but hopefully is a lot faster. (Hmmm, maybe the private lock should be dropped during this handshake...) Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: implement AMD remote wakeup quirk
On Fri, 23 Aug 2013, Huang Rui wrote: What's special about LS devices? Or mice? Shouldn't the resume signal be sent within 100 us for every device? Yes, only mice would trigger this issue. Reproduce steps: 1. Enable remote wakeup for usb mouse. 2. Execute S3. 3. Click mouse _intensely_ (more than 10 times) to wake the system up. 4. Then execute S3 again. 5. Observe that the system auto wake up. Actually, I tested all the mice in my side, this issue existed. It sounds like a bug in the mice. The fact that no other devices have the same problem tends to support this view. Do you really know for certain that the timing of the wakeup signal is what causes this to happen? And do you know why it happens only with AMD controllers? Have you tested other types of controllers? A much simpler solution would be to have the usbhid driver set the USB_QUIRK_RESET_RESUME flag whenever it probes a mouse. This would work for all of the HCDs, so none of them would need to be changed. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: xhci: implement AMD remote wakeup quirk
Hi Sarah,
On Thu, Aug 22, 2013 at 04:02:54PM -0700, Sarah Sharp wrote:
Hi Huang,
Thanks for notifying us of this issue and submitting a patch to attempt
to fix it.
Thank you to your reply. Any comments are warm for me. :)
I am the replacement of Andiry Xu, will work on usb driver for AMD
platforms. If any issue you want to test or verify in different
platform in future, I'm pleasue to do it.
However, the patches look very odd to me, and I don't
understand the details of what they're trying to fix, so you will have
to help me understand what you're trying to do.
Sorry, that's because I don't explain very clearly. I will try my best
to make you clear.
On Thu, Aug 22, 2013 at 11:56:22PM +0800, Huang Rui wrote:
The following patch is required to resolve remote wake issues with
certain devices.
Issue description:
If the remote wake is issued from the device in a specific timing
condition while the system is entering sleep state then it may cause
system to auto wake on subsequent sleep cycle.
What do you mean by subsequent sleep cycle? Do you mean that if I
enable a device to wake the system on resume, and the system goes into
S3 while the device signals a wake, then the system doesn't wakeup? And
then the system wakes up erroneously on the next sleep cycle?
I'm confused, please explain this issue in more detail.
As I explained the reproduce steps in last mail, the subsequent sleep
cycle means that execute S3 at the second time after use usb mouse
with clicking _intensely_ to wake up system from the first time to
access S3. If do it above steps, the system would auto wake
erroneously as soon as sleep in S3 at the second time.
That's because hardware can't hold or clear the resume signal if we do
it like above steps to wake from S3. So the patch is that reset the
port attached at mouse during the resume phase, then the resume signal
is cleared, so this issue never encountered again.
Root cause:
Host controller rebroadcasts the Resume signal 100 µseconds after
receiving the original resume event from the device. For proper
function, some devices may require the rebroadcast of resume event
within the USB spec of 100µS.
As Alan said, why only _some_ devices? The host isn't compliant with
the spec, and you see that behavior happen on low speed mice. That
doesn't mean the issue won't occur with other USB devices that implement
remote wakeup. Did you test with, say, a high speed 3G modem or a USB
bluetooth adapter to see if you can trigger the remote wakeup issue?
Only the mice would trigger this issue, high speed devices doesn't. I
will check with HW guys in my side.
This patch is only for xHCI driver and the quirk will be also added
into OHCI driver too.
This should be backported to kernels as old as 3.9, that contrain the
commit 3f5eb14135ba9d97ba4b8514fc7ef5e0dac2abf4 usb: add
find_raw_port_number callback to struct hc_driver().
Why that kernel in particular?
Because I bisect the kernel and found this commit implement
find_raw_port_number which my patch called. I expected this fix would
backport to stable tree.
That commit seems to have nothing to do
with your current patch, aside from the fact that it will make it
harder to back port. This problem will show up in any kernel with AMD
xHCI host support, right?
Yes, you're right. This problem shows up in any kernel with only some
AMD xhci host.
So why aren't you suggesting backporting to
kernels that added older AMD quirks?
This quirk is dependent of older AMD quirk, why do you suggest that?
More comments below.
Cc sta...@vger.kernel.org
Signed-off-by: Huang Rui ray.hu...@amd.com
---
drivers/usb/core/hub.c| 28
drivers/usb/host/pci-quirks.c | 17 +++-
drivers/usb/host/pci-quirks.h | 1 +
drivers/usb/host/xhci-pci.c | 4 +++
drivers/usb/host/xhci.c | 61
+++
drivers/usb/host/xhci.h | 25 +-
include/linux/usb.h | 1 +
7 files changed, 124 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 175179e..117196c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -27,6 +27,7 @@
#include linux/freezer.h
#include linux/random.h
#include linux/pm_qos.h
+#include linux/hid.h
#include asm/uaccess.h
#include asm/byteorder.h
@@ -5437,3 +5438,30 @@ acpi_handle usb_get_hub_port_acpi_handle(struct
usb_device *hdev,
return DEVICE_ACPI_HANDLE(hub-ports[port1 - 1]-dev);
}
#endif
+
+bool is_usb_mouse(struct usb_device *udev)
+{
+ struct usb_interface *intf;
+ struct usb_interface_descriptor intf_desc;
+
+ if (!udev) {
+ dev_warn(udev-dev, Warn: no device attached!\n);
+ return false;
+ }
+
+ intf = usb_ifnum_to_if(udev, 0);
+ intf_desc = intf-cur_altsetting-desc;
[PATCH 2/3] staging: ozwpan: Fix wrong error check.
schedule_work() returns true if succeeded false on failure,
error check was doing exactly reverse.
Also removes extra variable.
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozpd.c |6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index 5d24af3..d39a3df 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -261,17 +261,13 @@ void oz_pd_free(struct work_struct *work)
*/
void oz_pd_destroy(struct oz_pd *pd)
{
- int ret;
-
if (hrtimer_active(pd-timeout))
hrtimer_cancel(pd-timeout);
if (hrtimer_active(pd-heartbeat))
hrtimer_cancel(pd-heartbeat);
INIT_WORK(pd-workitem, oz_pd_free);
- ret = schedule_work(pd-workitem);
-
- if (ret)
+ if (!schedule_work(pd-workitem))
oz_pd_dbg(pd, ON, failed to schedule workitem\n);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] staging: ozwpan: Remove memset
As we are initialising structure, we do not require memset(). Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozpd.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c index 06004c8..5d24af3 100644 --- a/drivers/staging/ozwpan/ozpd.c +++ b/drivers/staging/ozwpan/ozpd.c @@ -268,7 +268,6 @@ void oz_pd_destroy(struct oz_pd *pd) if (hrtimer_active(pd-heartbeat)) hrtimer_cancel(pd-heartbeat); - memset(pd-workitem, 0, sizeof(pd-workitem)); INIT_WORK(pd-workitem, oz_pd_free); ret = schedule_work(pd-workitem); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Fri, 23 Aug 2013, Ming Lei wrote: On Fri, Aug 23, 2013 at 4:36 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 22 Aug 2013, Ming Lei wrote: This seems to be the most important factor. When you think about it, though, does it really minimize the changes? Consider all the other adjustments we had to make to ehci-hcd: the interrupt QH unlink change and the isochronous stream stuff (which also affects the usb-audio drivers). For other HCDs, this changes on interrupt QH unlink may not need at all if there is no much cost about relink. But for some HCDs, it will be needed. Which HCDs need them? I haven't checked in detail. It seems likely that uhci-hcd and ohci-hcd will need changes. Even for EHCI HCD, the interrupt QH unlink change isn't necessary, and the patch is only for improving the situation, isn't it? For other HCDs, basically unlink interrupt queue need't the wait time, so don't need the change. Wrong: For other HCDs, unlinks _do_ need to wait for the hardware. About isochronous stream stuff, the only change with moving giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can solve it easily, can't we? No, it's not so easy. I have done some work on it, and it's more complicated than you might think. Not to mention that the snd-usb-audio driver (and perhaps others) will also need to be modified. As I said, the only change introduced with running giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can handle it inside HCD and usbcore, nothing to do with drivers. That's not true, at least, not for ehci-hcd. OK, I give you a simple solution in which only one line change is needed for these HCDs, see attachment patch. The patch is wrong. It keeps track of whether the URB is being resubmitted by the completion handler, not whether the endpoint queue has emptied out. What happens if the completion handler for URB0 submits URB1? To do it properly, you would need to count the number of URBs on the endpoint queue. The count would be decremented after the completion handler returned, which means it probably would have to be an atomic_t variable -- and I know that you hate those! Also I remembered that you said the isochronous stream stuff needn't to consider on other HCDs. No, I didn't say that. It will have to be considered for ohci-hcd and uhci-hcd. Maybe I understand it incorrectly: We also don't have to change the isochronous code in every HCD http://www.spinics.net/lists/linux-usb/msg89826.html Right. We don't have to change the isochronous code in _every_ HCD, but we do need to change it in _some_ of them. So maybe the change on ehci HCD is a bit much, but for other HCDs, the change is just a little. The other HCDs will have to change too. Maybe not quite as much as ehci-hcd, but more than you seem to think. Per my solution, only one line change for the affected HCD is enough. No, it isn't. See above. Besides, even if your patch was correct, it still wouldn't be sufficient. ehci-hcd relies on the fact that the entries in a non-empty isochronous queue expire on or after ehci-last_iso_frame. With your patch, that wouldn't be true any more. - one thing is that the HCD private lock can't be held in interrupt handler any more, so new lock need to introduce, not mention each hard irq handler need to split to two parts. No new lock is needed -- the interrupt handler runs okay without one. Yes, the handler has to be split into two parts. But that's small and self-contained, and very few other changes are needed. How can you make sure the lockless hw operations in hard interrupt handler won't cause race with all other hw operations on host controller, anyway, It's a producer-consumer relationship. The IRQ handler produces events and the tasklet consumes them. It's well known that this sort of thing doesn't require locks. (It did require one memory barrier, though.) previously hcd lock is held to operate on host controller, and no such race. We should be careful since the change might depends on each hardware internal things. No, it doesn't. - also it should have been better to avoid resubmissions in its giveback handler. Why? We are all set up to allow them now. Ruling them out won't make things much easier than they are already. Only for one example, we needn't consider race between qh unlink and completion inside HCD anymore, so for ehci, the below variable and related code can be removed: ehci-async_unlinking ehci-intr_unlinking qh-exception QH_STATE_COMPLETING Each of those uses about three lines of code. And you're wrong about qh-exception; that one is needed (or will be needed) for other reasons. Doesn't it make HCD simpler? No,
Re: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
On Fri, 23 Aug 2013, Ming Lei wrote:
On Fri, Aug 23, 2013 at 4:39 AM, Alan Stern st...@rowland.harvard.edu wrote:
This patch divides ehci-hcd's interrupt handler into a top half and a
bottom half, using a tasklet to execute the latter.
The conversion is very straightforward. The only subtle point is that
we have to ignore interrupts that arrive while the tasklet is running
(i.e., from another device on a shared IRQ line).
Not only for another device, the interrupt from ehci itself is still
delay-handled.
No. The EHCI interrupt-enable register is set to 0 in the IRQ handler,
and it can't generate interrupt requests until the tasklet finishes.
Therefore there won't be any interrupts from the controller to ignore.
@@ -691,13 +696,40 @@ EXPORT_SYMBOL_GPL(ehci_setup);
/*-*/
-static irqreturn_t ehci_irq (struct usb_hcd *hcd)
+static irqreturn_t ehci_irq(struct usb_hcd *hcd)
{
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
+ u32 status, masked_status;
+
+ if (ehci-irqs_are_masked)
+ return IRQ_NONE;
+
+ /*
+* We don't use STS_FLR, but some controllers don't like it to
+* remain on, so mask it out along with the other status bits.
+*/
+ status = ehci_readl(ehci, ehci-regs-status);
+ masked_status = status (INTR_MASK | STS_FLR);
+
+ /* Shared IRQ? */
+ if (!masked_status || unlikely(ehci-rh_state == EHCI_RH_HALTED))
+ return IRQ_NONE;
+
+ /* Mask IRQs and let the tasklet do the work */
+ ehci_writel(ehci, 0, ehci-regs-intr_enable);
+ ehci-irqs_are_masked = true;
+ tasklet_hi_schedule(ehci-tasklet);
+ return IRQ_HANDLED;
The irq is handled but not clearing its status, so interrupt controller
might interrupt CPU continuously and may cause irq flood before
the status is cleared. Disabling ehci interrupt may not prevent the
irq flooding since interrupt controller may latch the unhandled(from
ehci hw view) irq source.
I don't understand. If the interrupt controller has latched something,
the latch will be cleared by the system's IRQ handler. Otherwise _any_
IRQ would cause a flood.
Secondly, not clearing USBSTS here may delay the next interrupt
handling for the host controller itself, and the delay depends on the
tasklet schedule delay.
Yes, of course. The same sort of thing is true for the original driver
-- the host controller can't issue a new interrupt until the handler
for the old one finishes.
Not handling interrupts right away is the price we pay for using a
bottom half. Your tasklet implementation isn't very different.
Although the interrupt handler may realize quickly that a transfer has
finished, there will still be a delay before the URB's completion
handler can run. And the length of that delay will depend on the
tasklet schedule delay.
Thirdly, fatal error should have been handled immediately inside hard
interrupt handler.
Why? What difference does it make?
Finally, tasklet schedule should have been optimized here if the tasklet
is running on another CPU, otherwise the current CPU may spin on
the tasklet lock until the same tasklet is completed on another CPU.
That could be added in. But doesn't the core kernel's tasklet
implementation already take care of this? The tasklet_hi_action()
routine uses tasklet_trylock(), so it doesn't spin.
@@ -833,10 +863,16 @@ dead:
if (bh)
ehci_work (ehci);
- spin_unlock (ehci-lock);
+
+ done:
+ /* Unmask IRQs again */
+ ehci-irqs_are_masked = false;
The 'irqs_are_masked' should have been set 'false' after clearing
USBSTS for enabling hard irq handling asap.
There's no point in enabling interrupts before they can be handled. As
long as the tasklet is running, it doesn't do any good to receive more
IRQs.
Also if the latest value of irqs_are_masked isn't see by ehci_irq
immediately, it will cause CPU interrupted by extra and unnecessary
irq signal.
+ smp_wmb(); /* Make sure ehci_irq() sees that
assignment */
That's why I put in this memory barrier. It guarantees that
irqs_are_masked _will_ be seen by ehci_irq.
+ ehci_writel(ehci, ehci-intr_mask, ehci-regs-intr_enable);
+
+ spin_unlock_irq(ehci-lock);
if (pcd_status)
usb_hcd_poll_rh_status(hcd);
- return IRQ_HANDLED;
}
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: implement AMD remote wakeup quirk
On Fri, Aug 23, 2013 at 10:53:38PM +0800, Alan Stern wrote: On Fri, 23 Aug 2013, Huang Rui wrote: What's special about LS devices? Or mice? Shouldn't the resume signal be sent within 100 us for every device? Yes, only mice would trigger this issue. Reproduce steps: 1. Enable remote wakeup for usb mouse. 2. Execute S3. 3. Click mouse _intensely_ (more than 10 times) to wake the system up. 4. Then execute S3 again. 5. Observe that the system auto wake up. Actually, I tested all the mice in my side, this issue existed. It sounds like a bug in the mice. The fact that no other devices have the same problem tends to support this view. It's not in the mice but in host controller. Do you really know for certain that the timing of the wakeup signal is what causes this to happen? I checked with HW guys in my side, and will feedback soon. And do you know why it happens only with AMD controllers? Have you tested other types of controllers? Yes, I tested on Renesas controller, didn't encounter. And not all AMD controllers has this issue, only in some special revisions. A much simpler solution would be to have the usbhid driver set the USB_QUIRK_RESET_RESUME flag whenever it probes a mouse. This would work for all of the HCDs, so none of them would need to be changed. Due to the controller issue, I think we only reset port attached on mouse, not reset mouse itself. Am I right? Thanks, Rui -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ACPI / PCI: don't allow PCI devices without irq to be enabled
On Mon, Aug 12, 2013 at 4:32 PM, Bjorn Helgaas bhelg...@google.com wrote: On Mon, Aug 12, 2013 at 3:32 PM, Yinghai Lu ying...@kernel.org wrote: On Mon, Aug 12, 2013 at 2:14 PM, Bjorn Helgaas bhelg...@google.com wrote: On Thu, Aug 8, 2013 at 7:57 AM, Rafael J. Wysocki r...@sisk.pl wrote: On Thursday, August 08, 2013 03:35:13 PM Heikki Krogerus wrote: If there is no ACPI entry for the irq, returning error from acpi_pci_enable_irq() if the irq is 0. Prarit Bhargava reported an issue where he noticed that his Dell PowerEdge 840 has buggy BIOS that does not supply ACPI entries for irq with some devices. That lead into kernel generating a warning genirq: Flags mismatch irq 0 This will fix that issue. Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Tested-by: Prarit Bhargava pra...@redhat.com Hi Bjorn, Any objections to this? Acked-by: Bjorn Helgaas bhelg...@google.com some USB3 host controller does not have intx configured, but still work with MSI? Huh, OK, I take back my ack, at least while we investigate this. Is anybody looking into this? I'm afraid I gave the impression that *I* would investigate this. But I don't really have enough information, and it would be better if the patch author and the reporter could investigate it first. It would help if there were a bugzilla with complete dmesg log, acpidump, etc. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] usb: implement AMD remote wakeup quirk
On Sat, 24 Aug 2013, Huang Rui wrote: On Fri, Aug 23, 2013 at 10:53:38PM +0800, Alan Stern wrote: On Fri, 23 Aug 2013, Huang Rui wrote: What's special about LS devices? Or mice? Shouldn't the resume signal be sent within 100 us for every device? Yes, only mice would trigger this issue. Reproduce steps: 1. Enable remote wakeup for usb mouse. 2. Execute S3. 3. Click mouse _intensely_ (more than 10 times) to wake the system up. 4. Then execute S3 again. 5. Observe that the system auto wake up. Actually, I tested all the mice in my side, this issue existed. It sounds like a bug in the mice. The fact that no other devices have the same problem tends to support this view. It's not in the mice but in host controller. How do you know? You mentioned earlier that the mouse would not clear its wakeup request. But resuming a device is always supposed to clear its wakeup request; if this doesn't happen then the device is buggy. Do you really know for certain that the timing of the wakeup signal is what causes this to happen? I checked with HW guys in my side, and will feedback soon. And do you know why it happens only with AMD controllers? Have you tested other types of controllers? Yes, I tested on Renesas controller, didn't encounter. And not all AMD controllers has this issue, only in some special revisions. What about OHCI controllers? A much simpler solution would be to have the usbhid driver set the USB_QUIRK_RESET_RESUME flag whenever it probes a mouse. This would work for all of the HCDs, so none of them would need to be changed. Due to the controller issue, I think we only reset port attached on mouse, not reset mouse itself. Am I right? They are the same thing -- resetting the mouse means resetting the mouse's USB port. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.4.4: disabling irq
On 2013-01-26 19:48, Alan Stern wrote: Please explain. I can't. I don't know what's going on with your systems. But nobody else seems to be experiencing this problem. Well, I replaced the motherboard by one of the same type and same BIOS version. And the problem is still here. kernel:[ 3013.199945] Disabling IRQ #18 So what can we conclude? Kernel bug? Hardware bug? Something else? Kind regards, Udo -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] staging: ozwpan: Check for correct config number.
Check for valid config number before completing set interface.
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozhcd.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index ab93b74..83ed64c 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -909,7 +909,7 @@ static void oz_hcd_complete_set_interface(struct oz_port
*port, struct urb *urb,
struct usb_hcd *hcd = port-ozhcd-hcd;
int rc = 0;
- if (rcode == 0) {
+ if ((rcode == 0) (port-config_num 0)) {
struct usb_host_config *config;
struct usb_host_interface *intf;
oz_dbg(ON, Set interface %d alt %d\n, if_num, alt);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] staging: ozwpan: Fix error checking while transmitting frame.
Make sure that we return negative value if oz_build_frame()
returns NULL.
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozpd.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c
index daaff2a..e1757aa 100644
--- a/drivers/staging/ozwpan/ozpd.c
+++ b/drivers/staging/ozwpan/ozpd.c
@@ -618,14 +618,14 @@ static int oz_send_next_queued_frame(struct oz_pd *pd,
int more_data)
pd-last_sent_frame = e;
skb = oz_build_frame(pd, f);
spin_unlock(pd-tx_frame_lock);
+ if (!skb)
+ return -1;
if (more_data)
oz_set_more_bit(skb);
oz_dbg(TX_FRAMES, TX frame PN=0x%x\n, f-hdr.pkt_num);
- if (skb) {
- if (dev_queue_xmit(skb) 0)
- return -1;
+ if (dev_queue_xmit(skb) 0)
+ return -1;
- }
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] staging: ozwpan: change variable type.
We have icreased interrupt end point buffer size to 512 bytes,
Change variable data type to accomodate it.
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozhcd.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 16fd60b..3548860 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -441,9 +441,9 @@ static void oz_complete_buffered_urb(struct oz_port *port,
struct oz_endpoint *ep,
struct urb *urb)
{
- u8 data_len, available_space, copy_len;
+ int data_len, available_space, copy_len;
- memcpy(data_len, ep-buffer[ep-out_ix], sizeof(u8));
+ data_len = ep-buffer[ep-out_ix];
if (data_len = urb-transfer_buffer_length)
available_space = data_len;
else
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] staging: ozwpan: change max. TX frame size supported.
Max. TX frame size supported is changed to 760 bytes. Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/ozproto.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ozwpan/ozproto.h b/drivers/staging/ozwpan/ozproto.h index e532347..0c49c8a 100644 --- a/drivers/staging/ozwpan/ozproto.h +++ b/drivers/staging/ozwpan/ozproto.h @@ -19,7 +19,7 @@ #define OZ_PRESLEEP_TOUT 11 /* Maximun sizes of tx frames. */ -#define OZ_MAX_TX_SIZE 1514 +#define OZ_MAX_TX_SIZE 760 /* Maximum number of uncompleted isoc frames that can be pending in network. */ #define OZ_MAX_SUBMITTED_ISOC 16 -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] staging: ozwpan: Increase interrupt end point buffer size
Increase interrupt end point buffer size convert hard coded
value to macro for better readability.
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozhcd.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index 89e498b..16fd60b 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -74,6 +74,7 @@ struct oz_urb_link {
/* Holds state information about a USB endpoint.
*/
#define OZ_EP_BUFFER_SIZE_ISOC (1024 * 24)
+#define OZ_EP_BUFFER_SIZE_INT 512
struct oz_endpoint {
struct list_head urb_list; /* List of oz_urb_link items. */
struct list_head link; /* For isoc ep, links in to isoc
@@ -1265,7 +1266,7 @@ static int oz_build_endpoints_for_interface(struct
usb_hcd *hcd,
buffer_size = OZ_EP_BUFFER_SIZE_ISOC;
break;
case USB_ENDPOINT_XFER_INT:
- buffer_size = 128;
+ buffer_size = OZ_EP_BUFFER_SIZE_INT;
break;
}
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Wed, Aug 21, 2013 at 04:10:12PM +0800, Ming Lei wrote: This patch introduces ehci_disable_event(), which is applied on IAA_WATCHDOG and START_UNLINK_INTR timeouts if the two corresponding events(IAA and intr URB submission) happened, so that we may avoid unnecessary CPU wakeup by canceling the timer. One simple test indicates about 7~8 timer expirations/sec (20% ~ 30% of totoal timer expirations/sec) can be saved when one asix network NIC is connected to EHCI without network traffic(the device responds interrupt poll 7~8 times per second constantly for link status query). Due to the discussions about this, I'm dropping this from my to-apply queue for now. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: acm gadget: Null termintate strings table
On Fri, Aug 23, 2013 at 05:43:51PM +, Graham Williams wrote: The gadget strings table should be null terminated. usb_gadget_get_string() loops through the table expecting a null at the end of the list. Signed-off-by: Graham Williamsgwi...@broadcom.com Minor nit, you need a space after your last name and before the ''. Third time's a charm? -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: acm gadget: Null termintate strings table
The gadget strings table should be null terminated.
usb_gadget_get_string() loops through the table
expecting a null at the end of the list.
Signed-off-by: Graham Williams gwi...@broadcom.com
---
drivers/usb/gadget/f_acm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 4b7e33e..ab1065a 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -285,6 +285,7 @@ static struct usb_string acm_string_defs[] = {
[ACM_CTRL_IDX].s = CDC Abstract Control Model (ACM),
[ACM_DATA_IDX].s = CDC ACM Data,
[ACM_IAD_IDX ].s = CDC Serial,
+ { } /* end of list */
};
static struct usb_gadget_strings acm_string_table = {
--
1.8.0.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: acm gadget: Null termintate strings table
On Fri, Aug 23, 2013 at 05:51:12PM +, Graham Williams wrote:
The gadget strings table should be null terminated.
usb_gadget_get_string() loops through the table
expecting a null at the end of the list.
Signed-off-by: Graham Williams gwi...@broadcom.com
Nice, that works.
---
drivers/usb/gadget/f_acm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 4b7e33e..ab1065a 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -285,6 +285,7 @@ static struct usb_string acm_string_defs[] = {
[ACM_CTRL_IDX].s = CDC Abstract Control Model (ACM),
[ACM_DATA_IDX].s = CDC ACM Data,
[ACM_IAD_IDX ].s = CDC Serial,
+ { } /* end of list */
But this doesn't at all :(
Please fix your email client to not convert tabs to spaces. Email the
patch to yourself and see if you can apply it properly first. Also, the
kernel file, Documentation/email_clients.txt will help in setting things
up properly.
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug 60738 - USB 3.0 connection delay seems to be too short for HP USB 3.0 hard drive
Anything to do with Sarah Sharp's (from Intel) comment (https://plus.google.com/u/0/116960357493251979546/posts/RZpndv4BCCD) and this patch (http://marc.info/?l=linux-usbm=137714769606183w=2) suggested to be tested? I'll test it in the next couple of days just in case. I'm asking so we don't work on the same thing twice. Waiting for a reply, Alexandre Demers On Wed, Aug 14, 2013 at 8:50 PM, Alexandre Demers alexandre.f.dem...@gmail.com wrote: OK, so I had to plug the hard drive a couple of time to have the behavior. I uploaded the files (2 usbmon log files and 1 corresponding dmesg) as attachments to kernel bug 60738 ( https://bugzilla.kernel.org/show_bug.cgi?id=60738) Here are the links to the files: usbmon on all buses (https://bugzilla.kernel.org/attachment.cgi?id=107205) usbmon on bus 9 (https://bugzilla.kernel.org/attachment.cgi?id=107206) dmesg containing both manipulations (https://bugzilla.kernel.org/attachment.cgi?id=107207) I hope it is what you were expecting. Alexandre Demers On 08/13/2013 01:21 PM, Kumar Gaurav wrote: On Tuesday 13 August 2013 12:01 PM, Alexandre Demers wrote: lsusb gives me the following: sudo lsusb --verbose -t /: Bus 13.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M /: Bus 12.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 11.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M /: Bus 10.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 09.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M |__ Port 1: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M /: Bus 08.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 480M /: Bus 07.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/4p, 12M |__ Port 1: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 12M |__ Port 1: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 12M |__ Port 1: Dev 2, If 2, Class=Human Interface Device, Driver=usbhid, 12M /: Bus 06.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/2p, 12M /: Bus 05.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/5p, 12M |__ Port 4: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=ohci_hcd/5p, 12M /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/4p, 480M /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/5p, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/5p, 480M |__ Port 1: Dev 2, If 0, Class=Vendor Specific Class, Driver=r8712u, 480M So I assume it is a combinaision of usb-storage over xhci_hcd (the hp hard drive is working right now and is on Bus 09). Alex On Tue 13 Aug 2013 02:23:50 AM EDT, Kumar Gaurav wrote: Hi Alexandre, I'm new to kernel development. I want to explore this bug. From you dmesg output I'm unable to get the driver your device is using. can you tell me which driver are you using? regards Kumar Gaurav On Tuesday 13 August 2013 11:15 AM, Alexandre Demers wrote: Sorry, here are more details. I'm refering to https://bugzilla.kernel.org/show_bug.cgi?id=60738 To sum up: Most of the time, my HP USB 3.0 hard drive will not be properly mounted when booting or when connecting it. I usually end up with something similar to the following in dmesg: [ 204.081693] usb usb9: usb wakeup-resume [ 204.081700] usb usb9: usb auto-resume [ 204.081711] hub 9-0:1.0: hub_resume [ 204.081722] hub 9-0:1.0: port 1: status 02c1 change 0001 [ 204.090114] usb usb8: usb wakeup-resume [ 204.090119] usb usb8: usb auto-resume [ 204.090131] hub 8-0:1.0: hub_resume [ 204.090140] hub 8-0:1.0: port 1: status 0101 change 0001 [ 204.182172] hub 9-0:1.0: state 7 ports 2 chg 0002 evt [ 204.182192] hub 9-0:1.0: port 1, status 02a0, change 0001, 5.0 Gb/s [ 204.286239] hub 9-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x2a0 [ 204.286250] hub 8-0:1.0: state 7 ports 2 chg 0002 evt 0002 [ 204.286262] hub 8-0:1.0: port 1, status 0101, change 0001, 12 Mb/s [ 204.286285] hub 9-0:1.0: hub_suspend [ 204.286291] usb usb9: bus auto-suspend, wakeup 1 [ 204.390320] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x101 [ 204.492391] usb 8-1: new high-speed USB device number 8 using xhci_hcd [ 204.697607] usb 8-1: Device not responding to set address. [ 204.898877] usb 8-1: Device not responding to set address. [ 205.099841] usb 8-1: device not accepting address 8, error -71 [ 205.099868] kobject: '8-1' (88020d7fe098): kobject_cleanup [ 205.099870] kobject: '8-1' (88020d7fe098): calling ktype release [ 205.099883] kobject: '8-1': free name [ 205.150907] kobject: '8-1' (88020a605898): kobject_cleanup [ 205.150918] kobject: '8-1' (88020a605898): calling ktype release [ 205.150921] kobject: '8-1': free name [ 205.150929] hub 8-0:1.0: state 7 ports 2 chg evt 0002 [ 205.150939] hub 8-0:1.0: port 1, status 0100, change 0001, 12 Mb/s [ 205.254964] hub 8-0:1.0: debounce: port 1: total 100ms stable 100ms
Re: 3.4.4: disabling irq
On Fri, 23 Aug 2013, Udo van den Heuvel wrote: On 2013-01-26 19:48, Alan Stern wrote: Please explain. I can't. I don't know what's going on with your systems. But nobody else seems to be experiencing this problem. Well, I replaced the motherboard by one of the same type and same BIOS version. And the problem is still here. kernel:[ 3013.199945] Disabling IRQ #18 So what can we conclude? Kernel bug? Hardware bug? Something else? Any of the above. On the new hardware, is it still true that /proc/interrupts shows IRQ 18 being used by ohci_hcd:usb1, 2, 3, and nothing else? Suppose you don't use the webcam, or better yet, unplug it (so that it can't possibly be used without your knowledge). If you do, does the IRQ still get disabled? By the way, you aren't still running a 3.4 kernel are you? Assuming you are using 3.10 or later, here's something else you can do. Turn on CONFIG_DUMMY_IRQ in the kernel config. Then do rmmod ohci-pci modprobe dummy-irq irq=18 and see if the IRQ line gets disabled after that. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: core: be specific about attribute permissions
From: Greg KH gre...@linuxfoundation.org
Instead of having to audit all sysfs attributes, to ensure we get them
right, use the default macros the driver core provides us (read-only,
read-write) to make the code simpler, and to prevent any mistakes from
ever happening.
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
drivers/usb/core/endpoint.c | 36 ++---
drivers/usb/core/hcd.c | 17 --
drivers/usb/core/port.c |7 -
drivers/usb/core/sysfs.c| 303
4 files changed, 166 insertions(+), 197 deletions(-)
diff --git a/drivers/usb/core/endpoint.c b/drivers/usb/core/endpoint.c
index f13a289a..39a24021 100644
--- a/drivers/usb/core/endpoint.c
+++ b/drivers/usb/core/endpoint.c
@@ -32,31 +32,31 @@ struct ep_attribute {
container_of(_attr, struct ep_attribute, attr)
#define usb_ep_attr(field, format_string) \
-static ssize_t show_ep_##field(struct device *dev, \
+static ssize_t field##_show(struct device *dev,\
struct device_attribute *attr, \
char *buf) \
{ \
struct ep_device *ep = to_ep_device(dev); \
return sprintf(buf, format_string, ep-desc-field);\
} \
-static DEVICE_ATTR(field, S_IRUGO, show_ep_##field, NULL);
+static DEVICE_ATTR_RO(field)
-usb_ep_attr(bLength, %02x\n)
-usb_ep_attr(bEndpointAddress, %02x\n)
-usb_ep_attr(bmAttributes, %02x\n)
-usb_ep_attr(bInterval, %02x\n)
+usb_ep_attr(bLength, %02x\n);
+usb_ep_attr(bEndpointAddress, %02x\n);
+usb_ep_attr(bmAttributes, %02x\n);
+usb_ep_attr(bInterval, %02x\n);
-static ssize_t show_ep_wMaxPacketSize(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t wMaxPacketSize_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct ep_device *ep = to_ep_device(dev);
return sprintf(buf, %04x\n,
usb_endpoint_maxp(ep-desc) 0x07ff);
}
-static DEVICE_ATTR(wMaxPacketSize, S_IRUGO, show_ep_wMaxPacketSize, NULL);
+static DEVICE_ATTR_RO(wMaxPacketSize);
-static ssize_t show_ep_type(struct device *dev, struct device_attribute *attr,
- char *buf)
+static ssize_t type_show(struct device *dev, struct device_attribute *attr,
+char *buf)
{
struct ep_device *ep = to_ep_device(dev);
char *type = unknown;
@@ -77,10 +77,10 @@ static ssize_t show_ep_type(struct device *dev, struct
device_attribute *attr,
}
return sprintf(buf, %s\n, type);
}
-static DEVICE_ATTR(type, S_IRUGO, show_ep_type, NULL);
+static DEVICE_ATTR_RO(type);
-static ssize_t show_ep_interval(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t interval_show(struct device *dev, struct device_attribute *attr,
+char *buf)
{
struct ep_device *ep = to_ep_device(dev);
char unit;
@@ -123,10 +123,10 @@ static ssize_t show_ep_interval(struct device *dev,
return sprintf(buf, %d%cs\n, interval, unit);
}
-static DEVICE_ATTR(interval, S_IRUGO, show_ep_interval, NULL);
+static DEVICE_ATTR_RO(interval);
-static ssize_t show_ep_direction(struct device *dev,
-struct device_attribute *attr, char *buf)
+static ssize_t direction_show(struct device *dev, struct device_attribute
*attr,
+ char *buf)
{
struct ep_device *ep = to_ep_device(dev);
char *direction;
@@ -139,7 +139,7 @@ static ssize_t show_ep_direction(struct device *dev,
direction = out;
return sprintf(buf, %s\n, direction);
}
-static DEVICE_ATTR(direction, S_IRUGO, show_ep_direction, NULL);
+static DEVICE_ATTR_RO(direction);
static struct attribute *ep_dev_attrs[] = {
dev_attr_bLength.attr,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 19ad3d2f..d6a8d23f 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -849,9 +849,8 @@ static int usb_rh_urb_dequeue(struct usb_hcd *hcd, struct
urb *urb, int status)
/*
* Show store the current value of authorized_default
*/
-static ssize_t usb_host_authorized_default_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+static ssize_t authorized_default_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct usb_device *rh_usb_dev = to_usb_device(dev);
struct usb_bus *usb_bus = rh_usb_dev-bus;
@@ -863,9 +862,9 @@ static ssize_t
[PATCH] USB: usbtmc: fix up attribute permissions
From: Greg Kroah-Hartman gre...@linuxfoundation.org
In auditing the usbtmc sysfs files, a bunch of them were being created
as read only, yet they have logic to handle writing to. So fix them
up by setting the permissions properly.
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
drivers/usb/class/usbtmc.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 66c40013..09de131e 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -899,7 +899,7 @@ err_out:
}
#define capability_attribute(name) \
-static ssize_t show_##name(struct device *dev, \
+static ssize_t name##_show(struct device *dev, \
struct device_attribute *attr, char *buf)\
{ \
struct usb_interface *intf = to_usb_interface(dev); \
@@ -907,7 +907,7 @@ static ssize_t show_##name(struct device *dev,
\
\
return sprintf(buf, %d\n, data-capabilities.name); \
} \
-static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+static DEVICE_ATTR_RO(name)
capability_attribute(interface_capabilities);
capability_attribute(device_capabilities);
@@ -926,7 +926,7 @@ static struct attribute_group capability_attr_grp = {
.attrs = capability_attrs,
};
-static ssize_t show_TermChar(struct device *dev,
+static ssize_t TermChar_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct usb_interface *intf = to_usb_interface(dev);
@@ -935,7 +935,7 @@ static ssize_t show_TermChar(struct device *dev,
return sprintf(buf, %c\n, data-TermChar);
}
-static ssize_t store_TermChar(struct device *dev,
+static ssize_t TermChar_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -947,10 +947,10 @@ static ssize_t store_TermChar(struct device *dev,
data-TermChar = buf[0];
return count;
}
-static DEVICE_ATTR(TermChar, S_IRUGO, show_TermChar, store_TermChar);
+static DEVICE_ATTR_RW(TermChar);
#define data_attribute(name) \
-static ssize_t show_##name(struct device *dev, \
+static ssize_t name##_show(struct device *dev, \
struct device_attribute *attr, char *buf)\
{ \
struct usb_interface *intf = to_usb_interface(dev); \
@@ -958,7 +958,7 @@ static ssize_t show_##name(struct device *dev,
\
\
return sprintf(buf, %d\n, data-name);\
} \
-static ssize_t store_##name(struct device *dev,
\
+static ssize_t name##_store(struct device *dev,
\
struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
@@ -976,7 +976,7 @@ static ssize_t store_##name(struct device *dev,
\
else\
return count; \
} \
-static DEVICE_ATTR(name, S_IRUGO, show_##name, store_##name)
+static DEVICE_ATTR_RW(name)
data_attribute(TermCharEnabled);
data_attribute(auto_abort);
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: serial: clean up attribute permissions
From: Greg Kroah-Hartman gre...@linuxfoundation.org
Clean up the DEVICE_ATTR usage in the USB serial drivers, making them
more obvious as to the permissions that the sysfs files should be.
Note: ftdi_sio.c still has a DEVICE_ATTR() used, that will have to wait
until after 3.12-rc1 comes out when DEVICE_ATTR_WO() shows up in Linus's
tree.
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
drivers/usb/serial/bus.c |5 ++---
drivers/usb/serial/ftdi_sio.c| 15 ++-
drivers/usb/serial/io_ti.c |8 +++-
drivers/usb/serial/iuu_phoenix.c |8 +++-
4 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
index f053b302..0ce51f21 100644
--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -38,15 +38,14 @@ static int usb_serial_device_match(struct device *dev,
return 0;
}
-static ssize_t show_port_number(struct device *dev,
+static ssize_t port_number_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
return sprintf(buf, %d\n, port-port_number);
}
-
-static DEVICE_ATTR(port_number, S_IRUGO, show_port_number, NULL);
+static DEVICE_ATTR_RO(port_number);
static int usb_serial_device_probe(struct device *dev)
{
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index dbf2f281..c45f9c0a 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1561,8 +1561,8 @@ static void ftdi_set_max_packet_size(struct
usb_serial_port *port)
* ***
*/
-static ssize_t show_latency_timer(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t latency_timer_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
struct ftdi_private *priv = usb_get_serial_port_data(port);
@@ -1572,11 +1572,10 @@ static ssize_t show_latency_timer(struct device *dev,
return sprintf(buf, %i\n, priv-latency);
}
-
/* Write a new value of the latency timer, in units of milliseconds. */
-static ssize_t store_latency_timer(struct device *dev,
- struct device_attribute *attr, const char *valbuf,
- size_t count)
+static ssize_t latency_timer_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *valbuf, size_t count)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
struct ftdi_private *priv = usb_get_serial_port_data(port);
@@ -1589,6 +1588,7 @@ static ssize_t store_latency_timer(struct device *dev,
return -EIO;
return count;
}
+static DEVICE_ATTR_RW(latency_timer);
/* Write an event character directly to the FTDI register. The ASCII
value is in the low 8 bits, with the enable bit in the 9th bit. */
@@ -1616,9 +1616,6 @@ static ssize_t store_event_char(struct device *dev,
return count;
}
-
-static DEVICE_ATTR(latency_timer, S_IWUSR | S_IRUGO, show_latency_timer,
- store_latency_timer);
static DEVICE_ATTR(event_char, S_IWUSR, NULL, store_event_char);
static int create_sysfs_attrs(struct usb_serial_port *port)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 9c18f598..b7187bf3 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2484,7 +2484,7 @@ static int edge_port_remove(struct usb_serial_port *port)
/* Sysfs Attributes */
-static ssize_t show_uart_mode(struct device *dev,
+static ssize_t uart_mode_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
@@ -2493,7 +2493,7 @@ static ssize_t show_uart_mode(struct device *dev,
return sprintf(buf, %d\n, edge_port-bUartMode);
}
-static ssize_t store_uart_mode(struct device *dev,
+static ssize_t uart_mode_store(struct device *dev,
struct device_attribute *attr, const char *valbuf, size_t count)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
@@ -2509,9 +2509,7 @@ static ssize_t store_uart_mode(struct device *dev,
return count;
}
-
-static DEVICE_ATTR(uart_mode, S_IWUSR | S_IRUGO, show_uart_mode,
- store_uart_mode);
+static DEVICE_ATTR_RW(uart_mode);
static int edge_create_sysfs_attrs(struct usb_serial_port *port)
{
diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index 790673e5..57c439a2 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -1130,7 +1130,7 @@ static int
[PATCH] usb: acm gadget: Null termintate strings table
I couldn't get the whitespace formatting correctly from Windows Office (work email) so I attached the patch file. I ran it on it applies correctly. 0001-usb-acm-gadget-Null-termintate-strings-table.patch Description: 0001-usb-acm-gadget-Null-termintate-strings-table.patch
[PATCH] USB: gadget: audit sysfs attribute permissions
From: Greg Kroah-Hartman gre...@linuxfoundation.org
Convert all USB gadget sysfs attributes to use the _RO or _RW variants,
to make them easier to audit and ensure that the permissions are
correct.
Note, two are left using the DEVICE_ATTR() macro, as there is no
DEVICE_ATTR_WO() in Linus's tree, that will happen after 3.12-rc1 is
out, a follow-on patch will be sent then.
Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
drivers/usb/gadget/composite.c |8 +++-
drivers/usb/gadget/dummy_hcd.c |8
drivers/usb/gadget/f_mass_storage.c | 14 ++
drivers/usb/gadget/net2272.c|4 ++--
drivers/usb/gadget/net2280.c| 18 +-
drivers/usb/gadget/storage_common.c | 25 -
drivers/usb/gadget/udc-core.c | 14 +++---
7 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 55f4df60..d4f0f330 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1497,17 +1497,15 @@ void composite_disconnect(struct usb_gadget *gadget)
/*-*/
-static ssize_t composite_show_suspended(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+static ssize_t suspended_show(struct device *dev, struct device_attribute
*attr,
+ char *buf)
{
struct usb_gadget *gadget = dev_to_usb_gadget(dev);
struct usb_composite_dev *cdev = get_gadget_data(gadget);
return sprintf(buf, %d\n, cdev-suspended);
}
-
-static DEVICE_ATTR(suspended, 0444, composite_show_suspended, NULL);
+static DEVICE_ATTR_RO(suspended);
static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
{
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index c588e8e4..06ecd08f 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -868,7 +868,7 @@ static const struct usb_gadget_ops dummy_ops = {
/*-*/
/* function sysfs attribute */
-static ssize_t show_function(struct device *dev, struct device_attribute *attr,
+static ssize_t function_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct dummy*dum = gadget_dev_to_dummy(dev);
@@ -877,7 +877,7 @@ static ssize_t show_function(struct device *dev, struct
device_attribute *attr,
return 0;
return scnprintf(buf, PAGE_SIZE, %s\n, dum-driver-function);
}
-static DEVICE_ATTR(function, S_IRUGO, show_function, NULL);
+static DEVICE_ATTR_RO(function);
/*-*/
@@ -2290,7 +2290,7 @@ static inline ssize_t show_urb(char *buf, size_t size,
struct urb *urb)
urb-actual_length, urb-transfer_buffer_length);
}
-static ssize_t show_urbs(struct device *dev, struct device_attribute *attr,
+static ssize_t urbs_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -2311,7 +2311,7 @@ static ssize_t show_urbs(struct device *dev, struct
device_attribute *attr,
return size;
}
-static DEVICE_ATTR(urbs, S_IRUGO, show_urbs, NULL);
+static DEVICE_ATTR_RO(urbs);
static int dummy_start_ss(struct dummy_hcd *dum_hcd)
{
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 4d4e96a5..313b835e 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -2578,14 +2578,12 @@ static int fsg_main_thread(void *common_)
/*** DEVICE ATTRIBUTES ***/
-static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
-static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, fsg_store_nofua);
-static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);
-
-static struct device_attribute dev_attr_ro_cdrom =
- __ATTR(ro, 0444, fsg_show_ro, NULL);
-static struct device_attribute dev_attr_file_nonremovable =
- __ATTR(file, 0444, fsg_show_file, NULL);
+static DEVICE_ATTR_RW(ro);
+static DEVICE_ATTR_RW(nofua);
+static DEVICE_ATTR_RW(file);
+
+static struct device_attribute dev_attr_ro_cdrom = __ATTR_RO(ro);
+static struct device_attribute dev_attr_file_nonremovable = __ATTR_RO(file);
/** FSG COMMON **/
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index f1e50a3e..bf2bb39f 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -1184,7 +1184,7 @@ static const struct usb_gadget_ops net2272_ops = {
Re: [PATCH] usb: acm gadget: Null termintate strings table
On Fri, Aug 23, 2013 at 11:19:55PM +, Graham Williams wrote: I couldn't get the whitespace formatting correctly from Windows Office (work email) so I attached the patch file. I ran it on it applies correctly. I can't take attachments, and especially not base64 ones :( Take a look at the file I pointed you at in the Documentation directory for how to set up other email clients to work. Lots of other Broadcom developers have this working properly for their work emails, so perhaps you should contact them and ask what the magic incantantation is. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: usbtmc: fix up attribute permissions
Hello. On 08/24/2013 03:09 AM, Greg Kroah-Hartman wrote: From: Greg Kroah-Hartman gre...@linuxfoundation.org In auditing the usbtmc sysfs files, a bunch of them were being created as read only, yet they have logic to handle writing to. So fix them up by setting the permissions properly. Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org I have noticed that you're now using signature cutoff (--) instead of usual --- tearline which makes it impossible to comment on your patches without further cutpaste. Is that intentional or just an overlook? WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: usbtmc: fix up attribute permissions
On Sat, Aug 24, 2013 at 04:10:17AM +0400, Sergei Shtylyov wrote: Hello. On 08/24/2013 03:09 AM, Greg Kroah-Hartman wrote: From: Greg Kroah-Hartman gre...@linuxfoundation.org In auditing the usbtmc sysfs files, a bunch of them were being created as read only, yet they have logic to handle writing to. So fix them up by setting the permissions properly. Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org I have noticed that you're now using signature cutoff (--) instead of usual --- tearline which makes it impossible to comment on your patches without further cutpaste. Is that intentional or just an overlook? Unintentional, as my email client doesn't cut it off at all, perhaps you should upgrade? :) I put those in by hand, hence the default signature cutoff. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/3] EHCI: convert the IRQ handler to a tasklet
On Sat, Aug 24, 2013 at 12:06 AM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 23 Aug 2013, Ming Lei wrote:
On Fri, Aug 23, 2013 at 4:39 AM, Alan Stern st...@rowland.harvard.edu
wrote:
This patch divides ehci-hcd's interrupt handler into a top half and a
bottom half, using a tasklet to execute the latter.
The conversion is very straightforward. The only subtle point is that
we have to ignore interrupts that arrive while the tasklet is running
(i.e., from another device on a shared IRQ line).
Not only for another device, the interrupt from ehci itself is still
delay-handled.
No. The EHCI interrupt-enable register is set to 0 in the IRQ handler,
and it can't generate interrupt requests until the tasklet finishes.
Therefore there won't be any interrupts from the controller to ignore.
But the current interrupt from EHCI isn't be acked(USBSTS isn't cleared),
so EHCI HW don't know the irq has been handled, then it is reasonable
that the EHCI interrupt still interrupts CPU.
EHCI spec(4.15) also states the point clearly:
the host controller is not required to de-assert a currently active
interrupt condition when software sets the interrupt enables (in the
USBINR register, see Section 2.3.3) to a zero. The only reliable
method software should use for acknowledging an interrupt is by
transitioning the appropriate status bits in the USBSTS register
(Section 2.3.2) from a one to a zero.
@@ -691,13 +696,40 @@ EXPORT_SYMBOL_GPL(ehci_setup);
/*-*/
-static irqreturn_t ehci_irq (struct usb_hcd *hcd)
+static irqreturn_t ehci_irq(struct usb_hcd *hcd)
{
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
+ u32 status, masked_status;
+
+ if (ehci-irqs_are_masked)
+ return IRQ_NONE;
+
+ /*
+* We don't use STS_FLR, but some controllers don't like it to
+* remain on, so mask it out along with the other status bits.
+*/
+ status = ehci_readl(ehci, ehci-regs-status);
+ masked_status = status (INTR_MASK | STS_FLR);
+
+ /* Shared IRQ? */
+ if (!masked_status || unlikely(ehci-rh_state == EHCI_RH_HALTED))
+ return IRQ_NONE;
+
+ /* Mask IRQs and let the tasklet do the work */
+ ehci_writel(ehci, 0, ehci-regs-intr_enable);
+ ehci-irqs_are_masked = true;
+ tasklet_hi_schedule(ehci-tasklet);
+ return IRQ_HANDLED;
The irq is handled but not clearing its status, so interrupt controller
might interrupt CPU continuously and may cause irq flood before
the status is cleared. Disabling ehci interrupt may not prevent the
irq flooding since interrupt controller may latch the unhandled(from
ehci hw view) irq source.
I don't understand. If the interrupt controller has latched something,
the latch will be cleared by the system's IRQ handler. Otherwise _any_
IRQ would cause a flood.
As I said above, EHCI HW don't know the irq has been handled.
Secondly, not clearing USBSTS here may delay the next interrupt
handling for the host controller itself, and the delay depends on the
tasklet schedule delay.
Yes, of course. The same sort of thing is true for the original driver
-- the host controller can't issue a new interrupt until the handler
for the old one finishes.
Not handling interrupts right away is the price we pay for using a
bottom half. Your tasklet implementation isn't very different.
Although the interrupt handler may realize quickly that a transfer has
finished, there will still be a delay before the URB's completion
handler can run. And the length of that delay will depend on the
tasklet schedule delay.
In my tasklet implementation, the next EHCI interrupt can be handled
after the current hard interrupt is handled, but in this patch the EHCI
hard irq can't be handled until the tasklet is handled, right?
Thirdly, fatal error should have been handled immediately inside hard
interrupt handler.
Why? What difference does it make?
At least, EHCI HW generates the fatal irq ASAP, This interrupt is not
delayed to the next interrupt threshold.(4.15.2.4), so it is reasonable for
software to handle it asap.
Finally, tasklet schedule should have been optimized here if the tasklet
is running on another CPU, otherwise the current CPU may spin on
the tasklet lock until the same tasklet is completed on another CPU.
That could be added in. But doesn't the core kernel's tasklet
implementation already take care of this? The tasklet_hi_action()
routine uses tasklet_trylock(), so it doesn't spin.
OK, but extra tasklet schedule delay might be caused.
@@ -833,10 +863,16 @@ dead:
if (bh)
ehci_work (ehci);
- spin_unlock (ehci-lock);
+
+ done:
+ /* Unmask IRQs again */
+ ehci-irqs_are_masked =
Re: [PATCH v1] USB: EHCI: disable IAA_WATCHDOG and START_UNLINK_INTR if they needn't to be handled
On Fri, Aug 23, 2013 at 11:40 PM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 23 Aug 2013, Ming Lei wrote: On Fri, Aug 23, 2013 at 4:36 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 22 Aug 2013, Ming Lei wrote: This seems to be the most important factor. When you think about it, though, does it really minimize the changes? Consider all the other adjustments we had to make to ehci-hcd: the interrupt QH unlink change and the isochronous stream stuff (which also affects the usb-audio drivers). For other HCDs, this changes on interrupt QH unlink may not need at all if there is no much cost about relink. But for some HCDs, it will be needed. Which HCDs need them? I haven't checked in detail. It seems likely that uhci-hcd and ohci-hcd will need changes. Are you sure? If the change is absolutely required, there is certainly bug in them, since HCDs still need to support submitting interrupt URB in tasklet scheduled by its complete() handler? Even for EHCI HCD, the interrupt QH unlink change isn't necessary, and the patch is only for improving the situation, isn't it? For other HCDs, basically unlink interrupt queue need't the wait time, so don't need the change. Wrong: For other HCDs, unlinks _do_ need to wait for the hardware. Just take a quick look at UHCI code, looks there is no much difference about unlinking qh between async qh and intr qh, which means the change might not need for uhci since usb-storage is common case to support. Correct me if it is wrong. About isochronous stream stuff, the only change with moving giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can solve it easily, can't we? No, it's not so easy. I have done some work on it, and it's more complicated than you might think. Not to mention that the snd-usb-audio driver (and perhaps others) will also need to be modified. As I said, the only change introduced with running giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and we can handle it inside HCD and usbcore, nothing to do with drivers. That's not true, at least, not for ehci-hcd. Could you explain it in detail? I mean we only discuss the change on isoc schedule in case of underrun when giveback in tasklet. OK, I give you a simple solution in which only one line change is needed for these HCDs, see attachment patch. The patch is wrong. It keeps track of whether the URB is being resubmitted by the completion handler, not whether the endpoint queue list_empty(stream-td_list) is already checked in iso_stream_schedule(), so I am wondering what is the 'wrong' you mean. We need to know if the empty endpoint queue is caused by the last completed URB in queue, which will be resubmitted later. has emptied out. What happens if the completion handler for URB0 submits URB1? Do you have such example? It is possible for the two URBs which belong to different endpoint and let one of URBs to start the stream, but I am wondering it is reasonable that the two belong to same endpoint? Even for this case, we can handle it easily by checking if the two URBs belong to same endpoint. To do it properly, you would need to count the number of URBs on the endpoint queue. The count would be decremented after the completion handler returned, which means it probably would have to be an atomic_t variable -- and I know that you hate those! percpu variable should be ok since no preemption is possible during completing? and no irq handler may operate the variable. Also I remembered that you said the isochronous stream stuff needn't to consider on other HCDs. No, I didn't say that. It will have to be considered for ohci-hcd and uhci-hcd. Maybe I understand it incorrectly: We also don't have to change the isochronous code in every HCD http://www.spinics.net/lists/linux-usb/msg89826.html Right. We don't have to change the isochronous code in _every_ HCD, but we do need to change it in _some_ of them. So maybe the change on ehci HCD is a bit much, but for other HCDs, the change is just a little. The other HCDs will have to change too. Maybe not quite as much as ehci-hcd, but more than you seem to think. Per my solution, only one line change for the affected HCD is enough. No, it isn't. See above. Besides, even if your patch was correct, it still wouldn't be sufficient. ehci-hcd relies on the fact that the entries in a non-empty isochronous queue expire on or after ehci-last_iso_frame. With your patch, that wouldn't be true any more. Sorry, I don't understand your point, as I said, the only change on ISOC schedule introduced with running giveback in tasklet is that iso_stream_schedule() may see list_empty(stream-td_list) when underrun, and the above discussed patch should address this one. If you mean the delay introduced by tasklet, I
