[PATCH net v3 0/4] r8152 bug fixes
I split the code of stopping/waking tx queue into the another patch. Hayes Wang (4): r8152: fix tx/rx memory overflow r8152: modify the tx flow r8152: support stopping/waking tx queue r8152: fix incorrect type in assignment drivers/net/usb/r8152.c | 109 1 file changed, 45 insertions(+), 64 deletions(-) -- 1.8.3.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 net v3 1/4] r8152: fix tx/rx memory overflow
The tx/rx would access the memory which is out of the desired range. Modify the method of checking the end of the memory to avoid it. For r8152_tx_agg_fill(), the variable remain may become negative. However, the declaration is unsigned, so the while loop wouldn't break when reaching the end of the desied memory. Although to change the declaration from unsigned to signed is enough to fix it, I also modify the checking method for safe. Replace remain = rx_buf_sz - sizeof(*tx_desc) - (u32)((void *)tx_data - agg->head); with remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head); to make sure the variable remain is always positive. Then, the overflow wouldn't happen. For rx_bottom(), the rx_desc should not be used to calculate the packet length before making sure the rx_desc is in the desired range. Change the checking to two parts. First, check the descriptor is in the memory. The other, using the descriptor to find out the packet length and check if the packet is in the memory. Signed-off-by: Hayes Wang --- drivers/net/usb/r8152.c | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index f3fce41..5dbfe50 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -24,7 +24,7 @@ #include /* Version Information */ -#define DRIVER_VERSION "v1.01.0 (2013/08/12)" +#define DRIVER_VERSION "v1.02.0 (2013/10/28)" #define DRIVER_AUTHOR "Realtek linux nic maintainers " #define DRIVER_DESC "Realtek RTL8152 Based USB 2.0 Ethernet Adapters" #define MODULENAME "r8152" @@ -1136,14 +1136,14 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb) static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg) { - u32 remain; + int remain; u8 *tx_data; tx_data = agg->head; agg->skb_num = agg->skb_len = 0; - remain = rx_buf_sz - sizeof(struct tx_desc); + remain = rx_buf_sz; - while (remain >= ETH_ZLEN) { + while (remain >= ETH_ZLEN + sizeof(struct tx_desc)) { struct tx_desc *tx_desc; struct sk_buff *skb; unsigned int len; @@ -1152,12 +1152,14 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg) if (!skb) break; + remain -= sizeof(*tx_desc); len = skb->len; if (remain < len) { skb_queue_head(&tp->tx_queue, skb); break; } + tx_data = tx_agg_align(tx_data); tx_desc = (struct tx_desc *)tx_data; tx_data += sizeof(*tx_desc); @@ -1167,9 +1169,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg) agg->skb_len += len; dev_kfree_skb_any(skb); - tx_data = tx_agg_align(tx_data + len); - remain = rx_buf_sz - sizeof(*tx_desc) - -(u32)((void *)tx_data - agg->head); + tx_data += len; + remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head); } usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2), @@ -1188,7 +1189,6 @@ static void rx_bottom(struct r8152 *tp) list_for_each_safe(cursor, next, &tp->rx_done) { struct rx_desc *rx_desc; struct rx_agg *agg; - unsigned pkt_len; int len_used = 0; struct urb *urb; u8 *rx_data; @@ -1204,17 +1204,22 @@ static void rx_bottom(struct r8152 *tp) rx_desc = agg->head; rx_data = agg->head; - pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK; - len_used += sizeof(struct rx_desc) + pkt_len; + len_used += sizeof(struct rx_desc); - while (urb->actual_length >= len_used) { + while (urb->actual_length > len_used) { struct net_device *netdev = tp->netdev; struct net_device_stats *stats; + unsigned pkt_len; struct sk_buff *skb; + pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK; if (pkt_len < ETH_ZLEN) break; + len_used += pkt_len; + if (urb->actual_length < len_used) + break; + stats = rtl8152_get_stats(netdev); pkt_len -= 4; /* CRC */ @@ -1234,9 +1239,8 @@ static void rx_bottom(struct r8152 *tp) rx_data = rx_agg_align(rx_data + pkt_len + 4); rx_desc = (struct rx_desc *)rx_data; - pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
[PATCH net v3 4/4] r8152: fix incorrect type in assignment
The data from the hardware should be little endian. Correct the declaration. Signed-off-by: Hayes Wang --- drivers/net/usb/r8152.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 81a4171..25e8fa8 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -307,22 +307,22 @@ enum rtl8152_flags { #define MCU_TYPE_USB 0x struct rx_desc { - u32 opts1; + __le32 opts1; #define RX_LEN_MASK0x7fff - u32 opts2; - u32 opts3; - u32 opts4; - u32 opts5; - u32 opts6; + __le32 opts2; + __le32 opts3; + __le32 opts4; + __le32 opts5; + __le32 opts6; }; struct tx_desc { - u32 opts1; + __le32 opts1; #define TX_FS (1 << 31) /* First segment of a packet */ #define TX_LS (1 << 30) /* Final segment of a packet */ #define TX_LEN_MASK0x3 - u32 opts2; + __le32 opts2; #define UDP_CS (1 << 31) /* Calculate UDP/IP checksum */ #define TCP_CS (1 << 30) /* Calculate TCP/IP checksum */ #define IPV4_CS(1 << 29) /* Calculate IPv4 checksum */ @@ -877,7 +877,7 @@ static void write_bulk_callback(struct urb *urb) static void intr_callback(struct urb *urb) { struct r8152 *tp; - __u16 *d; + __le16 *d; int status = urb->status; int res; -- 1.8.3.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 net v3 2/4] r8152: modify the tx flow
Remove the code for sending the packet in the rtl8152_start_xmit(). Let rtl8152_start_xmit() to queue the packet only, and schedule a tasklet to send the queued packets. This simplify the code and make sure all the packet would be sent by the original order. Signed-off-by: Hayes Wang --- drivers/net/usb/r8152.c | 46 +++--- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 5dbfe50..763234d 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1388,53 +1388,13 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb, struct net_device *netdev) { struct r8152 *tp = netdev_priv(netdev); - struct net_device_stats *stats = rtl8152_get_stats(netdev); - unsigned long flags; - struct tx_agg *agg = NULL; - struct tx_desc *tx_desc; - unsigned int len; - u8 *tx_data; - int res; skb_tx_timestamp(skb); - /* If tx_queue is not empty, it means at least one previous packt */ - /* is waiting for sending. Don't send current one before it. */ - if (skb_queue_empty(&tp->tx_queue)) - agg = r8152_get_tx_agg(tp); - - if (!agg) { - skb_queue_tail(&tp->tx_queue, skb); - return NETDEV_TX_OK; - } - - tx_desc = (struct tx_desc *)agg->head; - tx_data = agg->head + sizeof(*tx_desc); - agg->skb_num = agg->skb_len = 0; + skb_queue_tail(&tp->tx_queue, skb); - len = skb->len; - r8152_tx_csum(tp, tx_desc, skb); - memcpy(tx_data, skb->data, len); - dev_kfree_skb_any(skb); - agg->skb_num++; - agg->skb_len += len; - usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2), - agg->head, len + sizeof(*tx_desc), - (usb_complete_t)write_bulk_callback, agg); - res = usb_submit_urb(agg->urb, GFP_ATOMIC); - if (res) { - /* Can we get/handle EPIPE here? */ - if (res == -ENODEV) { - netif_device_detach(tp->netdev); - } else { - netif_warn(tp, tx_err, netdev, - "failed tx_urb %d\n", res); - stats->tx_dropped++; - spin_lock_irqsave(&tp->tx_lock, flags); - list_add_tail(&agg->list, &tp->tx_free); - spin_unlock_irqrestore(&tp->tx_lock, flags); - } - } + if (!list_empty(&tp->tx_free)) + tasklet_schedule(&tp->tl); return NETDEV_TX_OK; } -- 1.8.3.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 2/3] usb: phy: generic: clean up the probe function
Remove an extra return from the bottom. Signed-off-by: Heikki Krogerus --- drivers/usb/phy/phy-generic.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c index 68c5548..2b96311 100644 --- a/drivers/usb/phy/phy-generic.c +++ b/drivers/usb/phy/phy-generic.c @@ -271,8 +271,6 @@ static int usb_phy_gen_xceiv_probe(struct platform_device *pdev) platform_set_drvdata(pdev, nop); return 0; - - return err; } static int usb_phy_gen_xceiv_remove(struct platform_device *pdev) -- 1.8.4.3 -- 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 0/3] usb: phy: generic: fix for dwc3-pci
Hi, phy-generic broke dwc3-pci after the gpio support was added. The last patch is fixing that issue. The other two are just cleanups. Thanks, Heikki Krogerus (3): usb: phy: generic: fix a compiler warning usb: phy: generic: clean up the probe function usb: dwc3: fix the glue drivers using the nop phy drivers/usb/dwc3/dwc3-exynos.c | 1 + drivers/usb/dwc3/dwc3-pci.c| 1 + drivers/usb/phy/phy-generic.c | 4 +--- 3 files changed, 3 insertions(+), 3 deletions(-) -- 1.8.4.3 -- 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] usb: phy: generic: fix a compiler warning
Just because it annoys me. Signed-off-by: Heikki Krogerus --- drivers/usb/phy/phy-generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c index fce3a9e..68c5548 100644 --- a/drivers/usb/phy/phy-generic.c +++ b/drivers/usb/phy/phy-generic.c @@ -234,7 +234,7 @@ static int usb_phy_gen_xceiv_probe(struct platform_device *pdev) if (dev->of_node) { struct device_node *node = dev->of_node; - enum of_gpio_flags flags; + enum of_gpio_flags flags = 0; if (of_property_read_u32(node, "clock-frequency", &clk_rate)) clk_rate = 0; -- 1.8.4.3 -- 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/3] usb: dwc3: fix the glue drivers using the nop phy
The reset_gpio member of the usb_phy_gen_xceiv_platform_data structure needs the have negative value or phy-generic's probe will fail unless DT is used. 0 is a valid gpio number. This fixes an issue where phy-generic fails to probe with message: "usb_phy_gen_xceiv.0: Error requesting RESET GPIO 0". Signed-off-by: Heikki Krogerus --- drivers/usb/dwc3/dwc3-exynos.c | 1 + drivers/usb/dwc3/dwc3-pci.c| 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index 8b20c70..28c8ad7 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -50,6 +50,7 @@ static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) exynos->usb2_phy = pdev; pdata.type = USB_PHY_TYPE_USB2; + pdata.gpio_reset = -1; ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata)); if (ret) diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index 31443ae..7e71bfa 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -52,6 +52,7 @@ static int dwc3_pci_register_phys(struct dwc3_pci *glue) glue->usb2_phy = pdev; pdata.type = USB_PHY_TYPE_USB2; + pdata.gpio_reset = -1; ret = platform_device_add_data(glue->usb2_phy, &pdata, sizeof(pdata)); if (ret) -- 1.8.4.3 -- 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: ohci-pxa27x: include linux/dma-mapping.h
Include linux/dma-mapping.h to make the new functions available that are used since 22d9d8e83 ("DMA-API: usb: use dma_set_coherent_mask()"). Signed-off-by: Daniel Mack --- I got the following error while building for PXA platforms from Linus' current git head: drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’: drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function ‘dma_coerce_mask_and_coherent’ [-Werror=implicit-function-declaration] drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration] drivers/usb/host/ohci-pxa27x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c index e89ac4d..3963834 100644 --- a/drivers/usb/host/ohci-pxa27x.c +++ b/drivers/usb/host/ohci-pxa27x.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include -- 1.8.4.2 -- 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 PATCH] PM / Runtime: Allow to inactivate devices during system suspend
On 14 November 2013 18:57, Alan Stern wrote: > On Thu, 14 Nov 2013, Ulf Hansson wrote: > >> > Bear in mind that drivers _cannot_ rely on runtime PM to inactivate a >> > device during system suspend. The user can always prevent a device >> > from going into runtime suspend by writing "on" to the >> > /sys/.../power/control file. >> >> Good that you brought this up. From my point of view I think the sysfs >> for runtime PM could be debated whether it should exist at all, at >> least in it's current form. > > What other form would you suggest? I think it is kind of strange to give provision to userspace to control a "request based" power management feature in the kernel. Me personally can't think of any good use case, but I comes from the embedded/ARM world so I might not have the full picture. What would make sense to me, is to move the runtime PM sysfs to debugfs, because it is a good feature to make use of during development and debugging. > >> Anyway, if userspace decides to prevent runtime_suspend, I guess it >> will have take the consequences for it as well. :-) > > Right now, those consequences don't include also preventing the device > from going to low power during system system. What would you do if you > had a buggy device, where you knew it couldn't handle runtime suspend? > If you still wanted to put the whole system to sleep, you'd be stuck. Hmm, this seems like a use case for runtime PM sysfs then. :-) It sounds like vague arguments. If the driver has bugs we need to fix them, right? Should we really let userspace workaround bugs in the kernel? > >> So as a way forward, I am thinking of a similar approach as you >> suggested with the "generic" suspend_late. But instead add a new >> runtime PM API, which intent is to let drivers to specify for PM core, >> if it should care to prevent the runtime suspend from happen during >> system system - or not. Could that work? > > I don't see what good it would do. Instead of adding a new flag, why > not just let drivers point their .suspend_late methods to the new > generic function? That would accomplish almost the same thing without > changing the API. Two minor functions will be added to the API, no big deal I think. More importantly these will actually make life easier for some drivers since they don't need to set up any additional PM callbacks. Using the generic suspend_late approach would for sure work, but it will spread to buses and power domains as well, which is not the case when adding when adding a new API. > > (Actually, it would be superior. The generic suspend_late approach > will work even if the user writes "on" to /sys/.../power/control, > whereas your approach won't work.) We have different view here. :-) Whether I like it or not, the sysfs interface exist and then I think the kernel must act accordingly. We shouldn’t override it. I mean if userspace has decided to keep a device active, there is probably a reason. Thanks a lot Alan for being patient with me and continuing the discussion! Kind regards Ulf Hansson > > 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 PATCH] PM / Runtime: Allow to inactivate devices during system suspend
Ulf Hansson writes: > I think it is kind of strange to give provision to userspace to > control a "request based" power management feature in the kernel. Me > personally can't think of any good use case, but I comes from the > embedded/ARM world so I might not have the full picture. Because resuming a device inevitably adds latency, and that only userspace can decide the latency vs power policy for any given device, system and user? There are also other reasons, mostly related to device bugs. But the above is the most important in my view. Without that control knob I don't think I would have enabled runtime PM in any network driver while the netif is running. The additional latency is noticable. Anyway, this is a pointless discussion. The power/control ABI is in use so many places that it cannot be changed. I'd suggest googling "break userspace" before even suggesting anything like that again. Bjørn -- 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
xhci: complete USB freeze
Accessing the audio capture device (Em28xxAudio) of a USB TV adapter (Hauppauge HVR-900) causes a complete USB freeze (see lspci/lsusb and logging extract at https://bugzilla.kernel.org/show_bug.cgi?id=65021). Tested with various kernel releases up to 3.12 (currently in use is 3.11.8). This device is working at least until kernel 3.2 with the same hardware (Intel DH87RL/Haswell, BIOS RLH8710H.86A.0320.2013.0606.1802). Switching from a USB3 to a USB2 port doesn't change anything; if xhci is available (xhci and ehci compiled into kernel), it is used and the bus freezes. If ehci is forced by disabling xhci completely, the capture device is working again. Any feedback welcome. -- Diese E-Mail wurde aus dem Sicherheitsverbund E-Mail made in Germany versendet: http://www.gmx.net/e-mail-made-in-germany -- 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
Testing glue layer for DWC3 device controller
Hi All, I'm new to this USB driver development so posting this email. Currently, I'm using Kernel version 3.12-rc5 In my SoC, there's Synopsys' DWC3 USB controller configured in device mode. This is integrated with our internal USB3.0 PHY and USB 2.0 PHY taken from synopsys. I'm done with the initial coding and now I want to test the glue layer logic implemented. I have compiled and statically linked the drivers. When I boot my kernel, I see that the PHY and the DWC3 driver is probed successfully. However, the device doesn't enumerate. I debugged this and found that the device specific registers are not configured at all. More debugging followed and I figured out that the probe for zero gadget is called before the phy and the device is probed. So, how can I test the driver by *not* modularizing the phy and the controller drivers. I would be thankful for any help. Regards Amit Virdi -- 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] usb: xhci: Link TRB must not occur within a USB payload burst
> From: Sarah Sharp [mailto:sarah.a.sh...@linux.intel.com] > > On Mon, Nov 11, 2013 at 12:26:54PM -, David Laight wrote: > > > > Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB > > can only occur at a boundary between underlying USB frames (512 bytes for > > 480M). > > > > If this isn't done the USB frames aren't formatted correctly and, for > > example, > > the USB3 ethernet ax88179_178a card will stop sending (while still > > receiving) > > when running a netperf tcp transmit test with (say) and 8k buffer. > > Can you send the exact command line you used to cause the stall? I'd > like to reproduce this with my USB 3.0 ethernet adapter. Make sure you are running 3.12.0 or later. The usbnet code to support segmentation isn't in anything much earlier and the as88179_178a driver needs to have TSO enabled. I'm using netperf 2.6 (compiled from source) running the netserver on the system with the USB ethernet. The ethernet is directly connected to an e1000 PCIe card on the other system (in a random subnet). The control connection is using the main LAN. I'm running netperf from a script that can loop through various parameters. The TCP receive test should be from: control="main LAN address of remote system" target="USB3 address of remote system" output=REQUEST_SIZE,PROTOCOL,ELAPSED_TIME,TRANSACTION_RATE,LOCAL_SEND_THROUGHPUT,LOCAL_RECV_THROUGHPUT,REMOTE_RECV_THROUGHPUT,THROUGHPUT_UNITS netperf -l 60 -f B -D5 -t omni -H $control -- \ -T tcp -H $target -D -d in -r 8192 -O $output I'd added some diagnostic prints to every TRB setup so I could see the fragment sizes, ring slot addresses and the flags. You should see URB for almost 64k split into 3 or 4 parts. When it stopped it was always just after a LINK TRB that had been mid-TD. The USB packets are still processed, but the TCP packets don't get transmitted. If you have a USB3 analyser I think you'll see an unexpected short packet if you split a data TRB across the link TRB on a non-1k boundary. I then wrote the patch to see if it would make a difference - it did. David -- 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 PATCH] PM / Runtime: Allow to inactivate devices during system suspend
On 15 November 2013 10:29, Bjørn Mork wrote: > Ulf Hansson writes: > >> I think it is kind of strange to give provision to userspace to >> control a "request based" power management feature in the kernel. Me >> personally can't think of any good use case, but I comes from the >> embedded/ARM world so I might not have the full picture. > > Because resuming a device inevitably adds latency, and that only > userspace can decide the latency vs power policy for any given device, > system and user? That is a valid point. Though we have pm_qos, should not that be the input for these kind of requirement for a driver? In other words, if the latency requirement can't be met for a device while re-activating it, it should not enter runtime suspend state. > > There are also other reasons, mostly related to device bugs. But the > above is the most important in my view. Without that control knob I > don't think I would have enabled runtime PM in any network driver while > the netif is running. The additional latency is noticable. > > Anyway, this is a pointless discussion. The power/control ABI is in use > so many places that it cannot be changed. I'd suggest googling "break > userspace" before even suggesting anything like that again. > I agree, and has never been a suggestion from my side either, more like a statement that I don't like the sysfs for runtime PM. :-) Kind regards Ulf Hansson > > Bjørn -- 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 6/9] usbfs: proc_do_submiturb use a local variable for number_of_packets
Hi, Thanks for the review. On 11/14/2013 11:16 PM, Sarah Sharp wrote: On Wed, Oct 09, 2013 at 05:19:28PM +0200, Hans de Goede wrote: This is a preparation patch for adding support for bulk streams. This patch seems to have the (unintended?) side effect of enabling scatter gather for interrupt transfers over 16384 bytes (USB_SG_SIZE): switch(uurb->type) { ... case USBDEVFS_URB_TYPE_BULK: switch (usb_endpoint_type(&ep->desc)) { case USB_ENDPOINT_XFER_CONTROL: case USB_ENDPOINT_XFER_ISOC: return -EINVAL; case USB_ENDPOINT_XFER_INT: /* allow single-shot interrupt transfers */ uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; - goto interrupt_urb; + break; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; break; case USBDEVFS_URB_TYPE_INTERRUPT: if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; - interrupt_urb: - uurb->number_of_packets = 0; break; ... } ... u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length + num_sgs * sizeof(struct scatterlist); ret = usbfs_increase_memory_usage(u); if (ret) goto error; as->mem_usage = u; if (num_sgs) { The code breaks out of the inner switch statement, and falls through to setting num_sgs, which will later cause a scatter gather list to be allocated in the struct urb for the interrupt transfer. Right, my bad. I did not notice there was a nested switch, so my intent was for the break to break out of the top-level switch, which of course it does not. I've modified the patch in my local tree to put the goto back. Regards, Hans -- 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] staging: dwc2: do not clear pending interrupts twice
Pending interrupts clearing is done in dwc2_enable_common_interrupts so we don't need to do it twice. Signed-off-by: Julien Delacou --- drivers/staging/dwc2/core.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index c8ff668..8374ec3 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -533,9 +533,6 @@ void dwc2_enable_host_interrupts(struct dwc2_hsotg *hsotg) writel(0, hsotg->regs + GINTMSK); writel(0, hsotg->regs + HAINTMSK); - /* Clear any pending interrupts */ - writel(0x, hsotg->regs + GINTSTS); - /* Enable the common interrupts */ dwc2_enable_common_interrupts(hsotg); -- 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] staging: dwc2: do not clear pending interrupts twice
On Fri, Nov 15, 2013 at 11:39:38AM +0100, Julien DELACOU wrote: > Pending interrupts clearing is done in dwc2_enable_common_interrupts > so we don't need to do it twice. > Are there any user visible effects to this bug? How did you spot it? 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
Re: [PATCH 9/9] usbfs: Add support for allocating / freeing streams
Hi, On 11/15/2013 12:08 AM, Sarah Sharp wrote: All right, this patchset looks sane other than the one small bug I mentioned, and the couple comments I have below. Have you talked to the other libusb or libusbX developers about adding streams support to libusb? ISTR that you said you needed libusb changes in order to get the virtualized UAS device to work in qemu. Yes, this has all been coordinated with the other libusb devs, I've a libusb tree with patches adding support (and docs) here: https://github.com/jwrdegoede/libusbx/commits/master The patches have been reviewed by the Mac OS X port maintainer, and he also has patches ready to add bulk-stream support for Mac OS X. On Wed, Oct 09, 2013 at 05:19:31PM +0200, Hans de Goede wrote: Signed-off-by: Hans de Goede You need a commit message here, so we can tell usbfs users what bulk streams are (reference the documentation), and how to use this new interface (e.g. they need to allocate streams before using the new uurb->stream_id field). Agreed, will fix in my next version. Note I've been working on uas for the last 3 weeks running a large battery of tests, and adding small fixes left and right. I was about to send out a pull-req for you with everything from your "fun-streams-fixes" branch + later uas patches I've send + new uas patches from the last weeks, rebased on top of current usb-next, when I received your review mails. I'll incorporate your review comments in my tree, run a final set of tests and then send the pull-req. --- drivers/usb/core/devio.c | 118 ++ include/uapi/linux/usbdevice_fs.h | 7 +++ 2 files changed, 125 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index bfb2821..4ca7e86 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct dev_state *ps, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ifnum, ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 1024 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 1024)) + return -EINVAL; We really shouldn't hard-code the 1024 streams value. I'm actually not sure where you got that value. I don't remember, I think I got this value from misreading the HCC_MAX_PSA macro. Upon second reading that macro leads to a max value of 2 ^ 16. Each xHCI host can indicate the max primary stream array size (basically the number of streams) it supports, see MaxPSASize in section 5.3.6 of the xHCI spec. An xHCI host can choose to indicate it doesn't support streams at all, by setting that value to zero. Ideally, we should have something like bus->sg_tablesize that allows the xHCI host to indicate the number of streams it supports. Perhaps usb_bus->max_streams? I don't think that is necessary. The usb_alloc_streams API says that it can return less then requested. The check for > 1024 is only there to detect that what userspace is asking for is utter non-sense. I'll raise it to 2 ^ 16, anything below 2 ^ 16 is fair game even if the controller supports less, the application will simply get less streams then requested. Something which we do need to handle, which the xhci driver currently does not seem to handle is MaxPSASize being 0. But that should be handled at the xhci level IMHO. I'll add a patch for this to my tree. Regards, Hans -- 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/2] PM / Runtime: Fix error path for prepare
If a device prepare callback for some reason would fail, the PM core prevented the device from going inactive forever. In this case, to reverse the pm_runtime_get_noresume() we invokes the asyncronous pm_runtime_put(), thus restoring the usage count. Cc: Kevin Hilman Cc: Alan Stern Signed-off-by: Ulf Hansson --- drivers/base/power/main.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index ee039af..2a1b06a 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -1350,6 +1350,9 @@ static int device_prepare(struct device *dev, pm_message_t state) device_unlock(dev); + if (error) + pm_runtime_put(dev); + return error; } -- 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
[RFC PATCH 2/2] PM / Runtime: Allow devices to become inactive during system suspend
The PM core was unconditionally preventing devices from being runtime suspended during system suspend. The reason is that we don't want asynchronous runtime suspends to happen during system suspend and thus race with the system suspend process, which some drivers and buses are not able to handle. However, for drivers that don't have issues with the races above, are somewhat suffering from the constraints this cause and then need to inactivate their devices outside the runtime PM control during system suspend. In other words, those implements .suspend|suspend_late callbacks to handle this. To simplify for these drivers, we add new runtime PM API, pm_runtime_no_prevent_suspend(), which drivers typically should call from their probe routines to tell PM core that it must not prevent runtime suspend during system suspend. Additonally the PM core will in this case instead invode the pm_runtime_idle just before it shall disable runtime PM in suspend_late. Drivers making use if this feature are then able to trigger runtime suspend with for example a pm_runtime_put_sync from their .suspend callbacks. Additionally, for drivers that are able to cope with only runtime PM callbacks, these can rely on the PM core to trigger a runtime suspend (if the runtime PM usage count doesn't prevent it) to inactivate their devices during system suspend. Cc: Kevin Hilman Cc: Alan Stern Signed-off-by: Ulf Hansson --- Some discussion started recently while I posted below patch: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend After some thinking I decided to cook another RFC, that might be better material for discussion. If we decide to go in this direction, the doc obviously needs some update as well, which is not a part of this RFC yet. --- drivers/base/power/main.c| 13 ++--- drivers/base/power/runtime.c |1 + include/linux/pm.h |1 + include/linux/pm_runtime.h | 13 + 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 2a1b06a..838ebb6 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -832,7 +832,10 @@ static void device_complete(struct device *dev, pm_message_t state) device_unlock(dev); - pm_runtime_put(dev); + if (pm_runtime_suspend_prevented(dev)) + pm_runtime_put(dev); + else + pm_request_idle(dev); } /** @@ -1004,6 +1007,9 @@ static int device_suspend_late(struct device *dev, pm_message_t state) pm_callback_t callback = NULL; char *info = NULL; + if (!pm_runtime_suspend_prevented(dev)) + pm_runtime_idle(dev); + __pm_runtime_disable(dev, false); if (dev->power.syscore) @@ -1318,7 +1324,8 @@ static int device_prepare(struct device *dev, pm_message_t state) * block runtime suspend here, during the prepare phase, and allow * it again during the complete phase. */ - pm_runtime_get_noresume(dev); + if (pm_runtime_suspend_prevented(dev)) + pm_runtime_get_noresume(dev); device_lock(dev); @@ -1350,7 +1357,7 @@ static int device_prepare(struct device *dev, pm_message_t state) device_unlock(dev); - if (error) + if (error && pm_runtime_suspend_prevented(dev)) pm_runtime_put(dev); return error; diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 72e00e6..bdac1e2 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1373,6 +1373,7 @@ void pm_runtime_init(struct device *dev) atomic_set(&dev->power.child_count, 0); pm_suspend_ignore_children(dev, false); dev->power.runtime_auto = true; + dev->power.prevent_suspend = true; dev->power.request_pending = false; dev->power.request = RPM_REQ_NONE; diff --git a/include/linux/pm.h b/include/linux/pm.h index a224c7f..98ef412 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -551,6 +551,7 @@ struct dev_pm_info { unsigned intuse_autosuspend:1; unsigned inttimer_autosuspends:1; unsigned intmemalloc_noio:1; + unsigned intprevent_suspend:1; enum rpm_requestrequest; enum rpm_status runtime_status; int runtime_error; diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 6fa7cea..2b72e30 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -106,6 +106,16 @@ static inline void pm_runtime_mark_last_busy(struct device *dev) ACCESS_ONCE(dev->power.last_busy) = jiffies; } +static inline void pm_runtime_no_prevent_suspend(struct device *dev) +{ + dev->power.prevent_suspend = false; +} + +static inline bool pm_runtime_suspend_prevented(struct device *dev) +{ + return dev->power.prevent_suspe
Advertencia Final.
Su contraseña caducará en 3 días formulario llenar y enviar de inmediato para validar su dirección de e-mail. Nombre de Usuario: . Contraseña anterior: . Nueva Contraseña: gracias administrador del sistema -- 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] staging: dwc2: do not clear pending interrupts twice
On 11/15/2013 11:55 AM, Dan Carpenter wrote: > On Fri, Nov 15, 2013 at 11:39:38AM +0100, Julien DELACOU wrote: >> Pending interrupts clearing is done in dwc2_enable_common_interrupts >> so we don't need to do it twice. >> > Are there any user visible effects to this bug? How did you spot it? > > regards, > dan carpenter > > Honestly, there is not. I am working on another issue using that controller and noticed it during reviewing the global behavior. Best regards, Julien -- 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
[RFCv5 17/21] fusbh200: always compile debugfs support
From: Oliver Neukum This is a step in the conversion to only use dynamic debugging. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index bbe1e22..10cae6c 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -307,13 +307,6 @@ dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status) /*-*/ -#ifdef STUB_DEBUG_FILES - -static inline void create_debug_files (struct fusbh200_hcd *bus) { } -static inline void remove_debug_files (struct fusbh200_hcd *bus) { } - -#else - /* troubleshooting help: expose state in debugfs */ static int debug_async_open(struct inode *, struct file *); @@ -889,7 +882,6 @@ static inline void remove_debug_files (struct fusbh200_hcd *fusbh200) debugfs_remove_recursive(fusbh200->debug_dir); } -#endif /* STUB_DEBUG_FILES */ /*-*/ /* -- 1.8.3.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
[RFCv5 02/21] ohci: kill ohci_vdbg
From: Oliver Neukum With the introduction of dynamic debugging it has become redundant. Collapse it with ohci_dbg() Signed-off-by: Oliver Neukum --- drivers/usb/host/ohci-hcd.c | 4 ++-- drivers/usb/host/ohci-q.c | 8 drivers/usb/host/ohci.h | 6 -- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index e1c6809..501ecea 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -832,7 +832,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd) } if (ints & OHCI_INTR_RHSC) { - ohci_vdbg(ohci, "rhsc\n"); + ohci_dbg(ohci, "rhsc\n"); ohci->next_statechange = jiffies + STATECHANGE_DELAY; ohci_writel(ohci, OHCI_INTR_RD | OHCI_INTR_RHSC, ®s->intrstatus); @@ -854,7 +854,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd) * this might not happen. */ else if (ints & OHCI_INTR_RD) { - ohci_vdbg(ohci, "resume detect\n"); + ohci_dbg(ohci, "resume detect\n"); ohci_writel(ohci, OHCI_INTR_RD, ®s->intrstatus); set_bit(HCD_FLAG_POLL_RH, &hcd->flags); if (ohci->autostop) { diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c index 182eaa2..d4253e3 100644 --- a/drivers/usb/host/ohci-q.c +++ b/drivers/usb/host/ohci-q.c @@ -143,7 +143,7 @@ static void periodic_link (struct ohci_hcd *ohci, struct ed *ed) { unsignedi; - ohci_vdbg (ohci, "link %sed %p branch %d [%dus.], interval %d\n", + ohci_dbg(ohci, "link %sed %p branch %d [%dus.], interval %d\n", (ed->hwINFO & cpu_to_hc32 (ohci, ED_ISO)) ? "iso " : "", ed, ed->branch, ed->load, ed->interval); @@ -290,7 +290,7 @@ static void periodic_unlink (struct ohci_hcd *ohci, struct ed *ed) } ohci_to_hcd(ohci)->self.bandwidth_allocated -= ed->load / ed->interval; - ohci_vdbg (ohci, "unlink %sed %p branch %d [%dus.], interval %d\n", + ohci_dbg(ohci, "unlink %sed %p branch %d [%dus.], interval %d\n", (ed->hwINFO & cpu_to_hc32 (ohci, ED_ISO)) ? "iso " : "", ed, ed->branch, ed->load, ed->interval); } @@ -761,7 +761,7 @@ static int td_done(struct ohci_hcd *ohci, struct urb *urb, struct td *td) urb->iso_frame_desc [td->index].status = cc_to_error [cc]; if (cc != TD_CC_NOERROR) - ohci_vdbg (ohci, + ohci_dbg(ohci, "urb %p iso td %p (%d) len %d cc %d\n", urb, td, 1 + td->index, dlen, cc); @@ -793,7 +793,7 @@ static int td_done(struct ohci_hcd *ohci, struct urb *urb, struct td *td) } if (cc != TD_CC_NOERROR && cc < 0x0E) - ohci_vdbg (ohci, + ohci_dbg(ohci, "urb %p td %p (%d) cc %d, len=%d/%d\n", urb, td, 1 + td->index, cc, urb->actual_length, diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h index ea02722..9250cad 100644 --- a/drivers/usb/host/ohci.h +++ b/drivers/usb/host/ohci.h @@ -482,12 +482,6 @@ static inline struct usb_hcd *ohci_to_hcd (const struct ohci_hcd *ohci) #define ohci_warn(ohci, fmt, args...) \ dev_warn (ohci_to_hcd(ohci)->self.controller , fmt , ## args ) -#ifdef OHCI_VERBOSE_DEBUG -# define ohci_vdbg ohci_dbg -#else -# define ohci_vdbg(ohci, fmt, args...) do { } while (0) -#endif - /*-*/ /* -- 1.8.3.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
[RFCv5 03/21] ohci:always register debug files
From: Oliver Neukum Just remove the conditional compilation. Signed-off-by: Oliver Neukum --- drivers/usb/host/ohci-hcd.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 501ecea..de0e3e4 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1225,13 +1225,11 @@ static int __init ohci_hcd_mod_init(void) sizeof (struct ed), sizeof (struct td)); set_bit(USB_OHCI_LOADED, &usb_hcds_loaded); -#ifdef DEBUG ohci_debug_root = debugfs_create_dir("ohci", usb_debug_root); if (!ohci_debug_root) { retval = -ENOENT; goto error_debug; } -#endif #ifdef PS3_SYSTEM_BUS_DRIVER retval = ps3_ohci_driver_register(&PS3_SYSTEM_BUS_DRIVER); @@ -1306,11 +1304,9 @@ static int __init ohci_hcd_mod_init(void) ps3_ohci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER); error_ps3: #endif -#ifdef DEBUG debugfs_remove(ohci_debug_root); ohci_debug_root = NULL; error_debug: -#endif clear_bit(USB_OHCI_LOADED, &usb_hcds_loaded); return retval; @@ -1340,9 +1336,7 @@ static void __exit ohci_hcd_mod_exit(void) #ifdef PS3_SYSTEM_BUS_DRIVER ps3_ohci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER); #endif -#ifdef DEBUG debugfs_remove(ohci_debug_root); -#endif clear_bit(USB_OHCI_LOADED, &usb_hcds_loaded); } module_exit(ohci_hcd_mod_exit); -- 1.8.3.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
[RFCv5 14/21] uhci: compile debugfs conditional on CONFIG_DYNAMIC_DEBUG || DEBUG
From: Oliver Neukum This makes sure the header files are all there Signed-off-by: Oliver Neukum --- drivers/usb/host/uhci-debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c index 8e239cd..3989f0a 100644 --- a/drivers/usb/host/uhci-debug.c +++ b/drivers/usb/host/uhci-debug.c @@ -20,7 +20,7 @@ static struct dentry *uhci_debugfs_root; -#ifdef DEBUG +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) /* Handle REALLY large printks so we don't overflow buffers */ static void lprintk(char *buf) @@ -635,7 +635,7 @@ static const struct file_operations uhci_debug_operations = { #endif /* CONFIG_DEBUG_FS */ -#else /* DEBUG */ +#else /* DEBUG || CONFIG_DYNAMIC_DEBUG*/ static inline void lprintk(char *buf) {} -- 1.8.3.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
[RFCv5 15/21] fusbh200: unconditionally compile debugging helpers
From: Oliver Neukum These helpers are used only during setup of a HCD. A small overhead is no problem. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index b9f9592..17374a3 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -114,8 +114,6 @@ MODULE_PARM_DESC(hird, "host initiated resume duration, +1 for each 75us"); static inline void fusbh200_vdbg(struct fusbh200_hcd *fusbh200, ...) {} #endif -#ifdef DEBUG - /* check the values in the HCSPARAMS register * (host controller _Structural_ parameters) * see EHCI spec, Table 2-4 for each value @@ -130,13 +128,6 @@ static void dbg_hcs_params (struct fusbh200_hcd *fusbh200, char *label) HCS_N_PORTS (params) ); } -#else - -static inline void dbg_hcs_params (struct fusbh200_hcd *fusbh200, char *label) {} - -#endif - -#ifdef DEBUG /* check the values in the HCCPARAMS register * (host controller _Capability_ parameters) @@ -153,11 +144,6 @@ static void dbg_hcc_params (struct fusbh200_hcd *fusbh200, char *label) HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024", HCC_CANPARK(params) ? " park" : ""); } -#else - -static inline void dbg_hcc_params (struct fusbh200_hcd *fusbh200, char *label) {} - -#endif static void __maybe_unused dbg_qtd (const char *label, struct fusbh200_hcd *fusbh200, struct fusbh200_qtd *qtd) -- 1.8.3.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
[RFCv5 12/21] fusbh200: always build debugfs support
From: Oliver Neukum This gets rid of conditional compilation. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 31 --- drivers/usb/host/fusbh200.h | 8 2 files changed, 39 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index e1c6d85..b9f9592 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -159,8 +159,6 @@ static inline void dbg_hcc_params (struct fusbh200_hcd *fusbh200, char *label) { #endif -#ifdef DEBUG - static void __maybe_unused dbg_qtd (const char *label, struct fusbh200_hcd *fusbh200, struct fusbh200_qtd *qtd) { @@ -302,29 +300,6 @@ dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status) (status & PORT_CONNECT) ? " CONNECT" : ""); } -#else -static inline void __maybe_unused -dbg_qh (char *label, struct fusbh200_hcd *fusbh200, struct fusbh200_qh *qh) -{} - -static inline int __maybe_unused -dbg_status_buf (char *buf, unsigned len, const char *label, u32 status) -{ return 0; } - -static inline int __maybe_unused -dbg_command_buf (char *buf, unsigned len, const char *label, u32 command) -{ return 0; } - -static inline int __maybe_unused -dbg_intr_buf (char *buf, unsigned len, const char *label, u32 enable) -{ return 0; } - -static inline int __maybe_unused -dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status) -{ return 0; } - -#endif /* DEBUG */ - /* functions have the "wrong" filename when they're output... */ #define dbg_status(fusbh200, label, status) { \ char _buf [80]; \ @@ -5936,13 +5911,11 @@ static int __init fusbh200_hcd_init(void) sizeof(struct fusbh200_qh), sizeof(struct fusbh200_qtd), sizeof(struct fusbh200_itd)); -#ifdef DEBUG fusbh200_debug_root = debugfs_create_dir("fusbh200", usb_debug_root); if (!fusbh200_debug_root) { retval = -ENOENT; goto err_debug; } -#endif retval = platform_driver_register(&fusbh200_hcd_fusbh200_driver); if (retval < 0) @@ -5951,11 +5924,9 @@ static int __init fusbh200_hcd_init(void) platform_driver_unregister(&fusbh200_hcd_fusbh200_driver); clean: -#ifdef DEBUG debugfs_remove(fusbh200_debug_root); fusbh200_debug_root = NULL; err_debug: -#endif clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded); return retval; } @@ -5964,9 +5935,7 @@ module_init(fusbh200_hcd_init); static void __exit fusbh200_hcd_cleanup(void) { platform_driver_unregister(&fusbh200_hcd_fusbh200_driver); -#ifdef DEBUG debugfs_remove(fusbh200_debug_root); -#endif clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded); } module_exit(fusbh200_hcd_cleanup); diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h index 797c9e8..5b73205 100644 --- a/drivers/usb/host/fusbh200.h +++ b/drivers/usb/host/fusbh200.h @@ -173,9 +173,7 @@ struct fusbh200_hcd { /* one per controller */ #endif /* debug files */ -#ifdef DEBUG struct dentry *debug_dir; -#endif }; /* convert between an HCD pointer and the corresponding FUSBH200_HCD */ @@ -734,10 +732,4 @@ static inline unsigned fusbh200_read_frame_index(struct fusbh200_hcd *fusbh200) }) /*-*/ -#ifndef DEBUG -#define STUB_DEBUG_FILES -#endif /* DEBUG */ - -/*-*/ - #endif /* __LINUX_FUSBH200_H */ -- 1.8.3.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
[RFCv5 09/21] fotg210: change dbg_port() to evaluate parameters only if needed
From: Oliver Neukum For dynamic debug the overhead for evaluating parameters must be sacrificed only if the message is actually printed Signed-off-by: Oliver Neukum --- drivers/usb/host/fotg210-hcd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 15ee566..4acb6a4 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -254,8 +254,8 @@ dbg_command_buf(char *buf, unsigned len, const char *label, u32 command) ); } -static int -dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) +static char +*dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) { char*sig; @@ -275,7 +275,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) break; } - return scnprintf(buf, len, + scnprintf(buf, len, "%s%sport:%d status %06x %d " "sig=%s%s%s%s%s%s%s%s", label, label[0] ? " " : "", port, status, @@ -288,6 +288,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) (status & PORT_PE) ? " PE" : "", (status & PORT_CSC) ? " CSC" : "", (status & PORT_CONNECT) ? " CONNECT" : ""); + return buf; } /* functions have the "wrong" filename when they're output... */ @@ -305,8 +306,7 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) #define dbg_port(fotg210, label, port, status) { \ char _buf[80]; \ - dbg_port_buf(_buf, sizeof(_buf), label, port, status); \ - fotg210_dbg(fotg210, "%s\n", _buf); \ + fotg210_dbg(fotg210, "%s\n", dbg_port_buf(_buf, sizeof(_buf), label, port, status) ); \ } /*-*/ -- 1.8.3.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
[RFCv5 08/21] fotg210: always compile the support for debugfs
From: Oliver Neukum Simply remove the conditional compilation and remove the empty stubs. Signed-off-by: Oliver Neukum --- drivers/usb/host/fotg210-hcd.c | 47 -- 1 file changed, 47 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 3af0533..15ee566 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -59,9 +59,7 @@ static const char hcd_name[] = "fotg210_hcd"; #undef VERBOSE_DEBUG #undef FOTG210_URB_TRACE -#ifdef DEBUG #define FOTG210_STATS -#endif /* magic numbers that can affect system performance */ #defineFOTG210_TUNE_CERR 3 /* 0-3 qtd retries; 0 == don't stop */ @@ -113,8 +111,6 @@ MODULE_PARM_DESC(hird, "host initiated resume duration, +1 for each 75us"); static inline void fotg210_vdbg(struct fotg210_hcd *fotg210, ...) {} #endif -#ifdef DEBUG - /* check the values in the HCSPARAMS register * (host controller _Structural_ parameters) * see EHCI spec, Table 2-4 for each value @@ -129,13 +125,6 @@ static void dbg_hcs_params(struct fotg210_hcd *fotg210, char *label) HCS_N_PORTS(params) ); } -#else - -static inline void dbg_hcs_params(struct fotg210_hcd *fotg210, char *label) {} - -#endif - -#ifdef DEBUG /* check the values in the HCCPARAMS register * (host controller _Capability_ parameters) @@ -152,13 +141,6 @@ static void dbg_hcc_params(struct fotg210_hcd *fotg210, char *label) HCC_PGM_FRAMELISTLEN(params) ? "256/512/1024" : "1024", HCC_CANPARK(params) ? " park" : ""); } -#else - -static inline void dbg_hcc_params(struct fotg210_hcd *fotg210, char *label) {} - -#endif - -#ifdef DEBUG static void __maybe_unused dbg_qtd(const char *label, struct fotg210_hcd *fotg210, struct fotg210_qtd *qtd) @@ -308,29 +290,6 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) (status & PORT_CONNECT) ? " CONNECT" : ""); } -#else -static inline void __maybe_unused -dbg_qh(char *label, struct fotg210_hcd *fotg210, struct fotg210_qh *qh) -{} - -static inline int __maybe_unused -dbg_status_buf(char *buf, unsigned len, const char *label, u32 status) -{ return 0; } - -static inline int __maybe_unused -dbg_command_buf(char *buf, unsigned len, const char *label, u32 command) -{ return 0; } - -static inline int __maybe_unused -dbg_intr_buf(char *buf, unsigned len, const char *label, u32 enable) -{ return 0; } - -static inline int __maybe_unused -dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) -{ return 0; } - -#endif /* DEBUG */ - /* functions have the "wrong" filename when they're output... */ #define dbg_status(fotg210, label, status) { \ char _buf[80]; \ @@ -6005,13 +5964,11 @@ static int __init fotg210_hcd_init(void) sizeof(struct fotg210_qh), sizeof(struct fotg210_qtd), sizeof(struct fotg210_itd)); -#ifdef DEBUG fotg210_debug_root = debugfs_create_dir("fotg210", usb_debug_root); if (!fotg210_debug_root) { retval = -ENOENT; goto err_debug; } -#endif retval = platform_driver_register(&fotg210_hcd_driver); if (retval < 0) @@ -6020,11 +5977,9 @@ static int __init fotg210_hcd_init(void) platform_driver_unregister(&fotg210_hcd_driver); clean: -#ifdef DEBUG debugfs_remove(fotg210_debug_root); fotg210_debug_root = NULL; err_debug: -#endif clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded); return retval; } @@ -6033,9 +5988,7 @@ module_init(fotg210_hcd_init); static void __exit fotg210_hcd_cleanup(void) { platform_driver_unregister(&fotg210_hcd_driver); -#ifdef DEBUG debugfs_remove(fotg210_debug_root); -#endif clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded); } module_exit(fotg210_hcd_cleanup); -- 1.8.3.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
[RFCv5 20/21] usb: kill DEBUG compile option
From: Oliver Neukum In the drivers that no longer need it, it is removed. It is removed from the Makefile. Drivers not fully converted to dynamic debug have it shifted down into the individual drivers. Signed-off-by: Oliver Neukum usb: kill DEBUG compile option --- drivers/usb/host/Makefile | 2 -- drivers/usb/host/ehci-dbg.c | 8 drivers/usb/host/ehci-fsl.c | 2 +- drivers/usb/host/ehci-hcd.c | 6 +++--- drivers/usb/host/ehci-q.c | 4 ++-- drivers/usb/host/ehci.h | 8 drivers/usb/host/imx21-dbg.c| 4 drivers/usb/host/imx21-hcd.c| 4 drivers/usb/host/imx21-hcd.h| 4 drivers/usb/host/oxu210hp-hcd.c | 4 drivers/usb/host/uhci-debug.c | 4 ++-- drivers/usb/host/uhci-hcd.c | 8 12 files changed, 36 insertions(+), 22 deletions(-) diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 01e879e..7530468 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -2,8 +2,6 @@ # Makefile for USB Host Controller Drivers # -ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG - # tell define_trace.h where to find the xhci trace header CFLAGS_xhci-trace.o := -I$(src) diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c index 4a9c2ed..9269782 100644 --- a/drivers/usb/host/ehci-dbg.c +++ b/drivers/usb/host/ehci-dbg.c @@ -18,7 +18,7 @@ /* this file is part of ehci-hcd.c */ -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG /* check the values in the HCSPARAMS register * (host controller _Structural_ parameters) @@ -62,7 +62,7 @@ static inline void dbg_hcs_params (struct ehci_hcd *ehci, char *label) {} #endif -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG /* check the values in the HCCPARAMS register * (host controller _Capability_ parameters) @@ -101,7 +101,7 @@ static inline void dbg_hcc_params (struct ehci_hcd *ehci, char *label) {} #endif -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG static void __maybe_unused dbg_qtd (const char *label, struct ehci_hcd *ehci, struct ehci_qtd *qtd) @@ -301,7 +301,7 @@ static inline int __maybe_unused dbg_port_buf (char *buf, unsigned len, const char *label, int port, u32 status) { return 0; } -#endif /* DEBUG || CONFIG_DYNAMIC_DEBUG */ +#endif /* CONFIG_DYNAMIC_DEBUG */ /* functions have the "wrong" filename when they're output... */ #define dbg_status(ehci, label, status) { \ diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index a06d501..87a7426 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -413,7 +413,7 @@ static int ehci_fsl_mpc512x_drv_suspend(struct device *dev) struct fsl_usb2_platform_data *pdata = dev_get_platdata(dev); u32 tmp; -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG u32 mode = ehci_readl(ehci, hcd->regs + FSL_SOC_USB_USBMODE); mode &= USBMODE_CM_MASK; tmp = ehci_readl(ehci, hcd->regs + 0x140); /* usbcmd */ diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index b57e997..1e21a36 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -1313,7 +1313,7 @@ static int __init ehci_hcd_init(void) sizeof(struct ehci_qh), sizeof(struct ehci_qtd), sizeof(struct ehci_itd), sizeof(struct ehci_sitd)); -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG ehci_debug_root = debugfs_create_dir("ehci", usb_debug_root); if (!ehci_debug_root) { retval = -ENOENT; @@ -1362,7 +1362,7 @@ clean2: platform_driver_unregister(&PLATFORM_DRIVER); clean0: #endif -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG debugfs_remove(ehci_debug_root); ehci_debug_root = NULL; err_debug: @@ -1386,7 +1386,7 @@ static void __exit ehci_hcd_cleanup(void) #ifdef PS3_SYSTEM_BUS_DRIVER ps3_ehci_driver_unregister(&PS3_SYSTEM_BUS_DRIVER); #endif -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG debugfs_remove(ehci_debug_root); #endif clear_bit(USB_EHCI_LOADED, &usb_hcds_loaded); diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c index db05bd8..54f5332 100644 --- a/drivers/usb/host/ehci-q.c +++ b/drivers/usb/host/ehci-q.c @@ -168,13 +168,13 @@ static void ehci_clear_tt_buffer(struct ehci_hcd *ehci, struct ehci_qh *qh, * Note: this routine is never called for Isochronous transfers. */ if (urb->dev->tt && !usb_pipeint(urb->pipe) && !qh->clearing_tt) { -#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) +#ifdef CONFIG_DYNAMIC_DEBUG struct usb_device *tt = urb->dev->tt->hub; dev_dbg(&tt->dev, "clear tt buffer port %d, a%d ep%d t%08x\n",
[RFCv5 06/21] ehci: Remove debugging at every interrupt
From: Oliver Neukum This is overkill. Just removeit. Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hcd.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index e8ba4c4..b57e997 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -714,13 +714,6 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) cmd = ehci_readl(ehci, &ehci->regs->command); bh = 0; -#ifdef VERBOSE_DEBUG - /* unrequested/ignored: Frame List Rollover */ - dbg_status (ehci, "irq", status); -#endif - - /* INT, ERR, and IAA interrupt rates can be throttled */ - /* normal [4.15.1.2] or error [4.15.1.1] completion */ if (likely ((status & (STS_INT|STS_ERR)) != 0)) { if (likely ((status & STS_ERR) == 0)) -- 1.8.3.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
[RFCv5 18/21] fusb200h: always compile in debugfs support
From: Oliver Neukum This allows removal of much conditional compilation. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 20 drivers/usb/host/fusbh200.h | 4 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index 10cae6c..b561e87b 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -60,10 +60,6 @@ static const charhcd_name [] = "fusbh200_hcd"; #undef VERBOSE_DEBUG #undef FUSBH200_URB_TRACE -#ifdef DEBUG -#define FUSBH200_STATS -#endif - /* magic numbers that can affect system performance */ #defineFUSBH200_TUNE_CERR 3 /* 0-3 qtd retries; 0 == don't stop */ #defineFUSBH200_TUNE_RL_HS 4 /* nak throttle; see 4.9 */ @@ -729,7 +725,6 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf) next += temp; } -#ifdef FUSBH200_STATS temp = scnprintf (next, size, "irq normal %ld err %ld iaa %ld (lost %ld)\n", fusbh200->stats.normal, fusbh200->stats.error, fusbh200->stats.iaa, @@ -741,7 +736,6 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf) fusbh200->stats.complete, fusbh200->stats.unlink); size -= temp; next += temp; -#endif done: spin_unlock_irqrestore (&fusbh200->lock, flags); @@ -1722,10 +1716,8 @@ static int fusbh200_hub_control ( if (test_bit(wIndex, &fusbh200->port_c_suspend)) status |= USB_PORT_STAT_C_SUSPEND << 16; -#ifndefVERBOSE_DEBUG - if (status & ~0x) /* only if wPortChange is interesting */ -#endif - dbg_port (fusbh200, "GetStatus", wIndex + 1, temp); + if (status & ~0x) /* only if wPortChange is interesting */ + dbg_port(fusbh200, "GetStatus", wIndex + 1, temp); put_unaligned_le32(status, buf); break; case SetHubFeature: @@ -2183,13 +2175,13 @@ static void fusbh200_clear_tt_buffer(struct fusbh200_hcd *fusbh200, struct fusbh * Note: this routine is never called for Isochronous transfers. */ if (urb->dev->tt && !usb_pipeint(urb->pipe) && !qh->clearing_tt) { -#ifdef DEBUG struct usb_device *tt = urb->dev->tt->hub; + dev_dbg(&tt->dev, "clear tt buffer port %d, a%d ep%d t%08x\n", urb->dev->ttport, urb->dev->devnum, usb_pipeendpoint(urb->pipe), token); -#endif /* DEBUG */ + if (urb->dev->tt->hub != fusbh200_to_hcd(fusbh200)->self.root_hub) { if (usb_hub_clear_tt_buffer(urb) == 0) @@ -3482,11 +3474,9 @@ periodic_usecs (struct fusbh200_hcd *fusbh200, unsigned frame, unsigned uframe) break; } } -#ifdef DEBUG if (usecs > fusbh200->uframe_periodic_max) fusbh200_err (fusbh200, "uframe %d sched overrun: %d usecs\n", frame * 8 + uframe, usecs); -#endif return usecs; } @@ -5068,13 +5058,11 @@ static void fusbh200_stop (struct usb_hcd *hcd) spin_unlock_irq (&fusbh200->lock); fusbh200_mem_cleanup (fusbh200); -#ifdef FUSBH200_STATS fusbh200_dbg(fusbh200, "irq normal %ld err %ld iaa %ld (lost %ld)\n", fusbh200->stats.normal, fusbh200->stats.error, fusbh200->stats.iaa, fusbh200->stats.lost_iaa); fusbh200_dbg (fusbh200, "complete %ld unlink %ld\n", fusbh200->stats.complete, fusbh200->stats.unlink); -#endif dbg_status (fusbh200, "fusbh200_stop completed", fusbh200_readl(fusbh200, &fusbh200->regs->status)); diff --git a/drivers/usb/host/fusbh200.h b/drivers/usb/host/fusbh200.h index 5b73205..6b719e0 100644 --- a/drivers/usb/host/fusbh200.h +++ b/drivers/usb/host/fusbh200.h @@ -165,12 +165,8 @@ struct fusbh200_hcd { /* one per controller */ u8 sbrn; /* packed release number */ /* irq statistics */ -#ifdef FUSBH200_STATS struct fusbh200_stats stats; # define COUNT(x) do { (x)++; } while (0) -#else -# define COUNT(x) do {} while (0) -#endif /* debug files */ struct dentry *debug_dir; -- 1.8.3.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
[RFCv5 05/21] ehci: no conditional compilation for interestingness
From: Oliver Neukum Simple elemination of the conditional compilation Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hub.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 835fc08..47b858f 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -1114,10 +1114,8 @@ static int ehci_hub_control ( if (test_bit(wIndex, &ehci->port_c_suspend)) status |= USB_PORT_STAT_C_SUSPEND << 16; -#ifndefVERBOSE_DEBUG - if (status & ~0x) /* only if wPortChange is interesting */ -#endif - dbg_port (ehci, "GetStatus", wIndex + 1, temp); + if (status & ~0x) /* only if wPortChange is interesting */ + dbg_port(ehci, "GetStatus", wIndex + 1, temp); put_unaligned_le32(status, buf); break; case SetHubFeature: -- 1.8.3.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
[RFCv5 21/21] kill #undef OHCI_VERBOSE_DEBUG
From: Oliver Neukum It is useless now. Straight removal. Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hcd.c | 1 - drivers/usb/host/fotg210-hcd.c | 1 - drivers/usb/host/fusbh200-hcd.c | 1 - drivers/usb/host/ohci-hcd.c | 2 -- 4 files changed, 5 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 1e21a36..4711427 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -71,7 +71,6 @@ static const char hcd_name [] = "ehci_hcd"; -#undef VERBOSE_DEBUG #undef EHCI_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 070582b..97d6939 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -56,7 +56,6 @@ static const char hcd_name[] = "fotg210_hcd"; -#undef VERBOSE_DEBUG #undef FOTG210_URB_TRACE #define FOTG210_STATS diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index d5e379b..9ea85b6 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -57,7 +57,6 @@ static const char hcd_name [] = "fusbh200_hcd"; -#undef VERBOSE_DEBUG #undef FUSBH200_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index de0e3e4..c8e0e76 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -51,8 +51,6 @@ /*-*/ -#undef OHCI_VERBOSE_DEBUG /* not always helpful */ - /* For initializing controller (mask in an HCFS mode too) */ #defineOHCI_CONTROL_INIT OHCI_CTRL_CBSR #defineOHCI_INTR_INIT \ -- 1.8.3.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
[RFCv5 10/21] fotg210: remove conditional compilation
From: Oliver Neukum The decision what is interesting is made in user space. Signed-off-by: Oliver Neukum --- drivers/usb/host/fotg210-hcd.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 4acb6a4..22f0b68 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -1761,10 +1761,8 @@ static int fotg210_hub_control( if (test_bit(wIndex, &fotg210->port_c_suspend)) status |= USB_PORT_STAT_C_SUSPEND << 16; -#ifndefVERBOSE_DEBUG - if (status & ~0x) /* only if wPortChange is interesting */ -#endif - dbg_port(fotg210, "GetStatus", wIndex + 1, temp); + if (status & ~0x) /* only if wPortChange is interesting */ + dbg_port(fotg210, "GetStatus", wIndex + 1, temp); put_unaligned_le32(status, buf); break; case SetHubFeature: @@ -2225,13 +2223,12 @@ static void fotg210_clear_tt_buffer(struct fotg210_hcd *fotg210, * Note: this routine is never called for Isochronous transfers. */ if (urb->dev->tt && !usb_pipeint(urb->pipe) && !qh->clearing_tt) { -#ifdef DEBUG struct usb_device *tt = urb->dev->tt->hub; dev_dbg(&tt->dev, "clear tt buffer port %d, a%d ep%d t%08x\n", urb->dev->ttport, urb->dev->devnum, usb_pipeendpoint(urb->pipe), token); -#endif /* DEBUG */ + if (urb->dev->tt->hub != fotg210_to_hcd(fotg210)->self.root_hub) { if (usb_hub_clear_tt_buffer(urb) == 0) @@ -3534,11 +3531,9 @@ periodic_usecs(struct fotg210_hcd *fotg210, unsigned frame, unsigned uframe) break; } } -#ifdef DEBUG if (usecs > fotg210->uframe_periodic_max) fotg210_err(fotg210, "uframe %d sched overrun: %d usecs\n", frame * 8 + uframe, usecs); -#endif return usecs; } @@ -5395,10 +5390,8 @@ static irqreturn_t fotg210_irq(struct usb_hcd *hcd) cmd = fotg210_readl(fotg210, &fotg210->regs->command); bh = 0; -#ifdef VERBOSE_DEBUG /* unrequested/ignored: Frame List Rollover */ dbg_status(fotg210, "irq", status); -#endif /* INT, ERR, and IAA interrupt rates can be throttled */ -- 1.8.3.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
[RFCv5 07/21] fotg210: remove conditional compilation
From: Oliver Neukum Always compile in the debugfs support --- drivers/usb/host/fotg210-hcd.c | 8 drivers/usb/host/fotg210.h | 8 2 files changed, 16 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 55486bd..3af0533 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -352,13 +352,6 @@ dbg_port_buf(char *buf, unsigned len, const char *label, int port, u32 status) /*-*/ -#ifdef STUB_DEBUG_FILES - -static inline void create_debug_files(struct fotg210_hcd *bus) { } -static inline void remove_debug_files(struct fotg210_hcd *bus) { } - -#else - /* troubleshooting help: expose state in debugfs */ static int debug_async_open(struct inode *, struct file *); @@ -954,7 +947,6 @@ static inline void remove_debug_files(struct fotg210_hcd *fotg210) debugfs_remove_recursive(fotg210->debug_dir); } -#endif /* STUB_DEBUG_FILES */ /*-*/ /* diff --git a/drivers/usb/host/fotg210.h b/drivers/usb/host/fotg210.h index 8920f9d..ac6cd1b 100644 --- a/drivers/usb/host/fotg210.h +++ b/drivers/usb/host/fotg210.h @@ -174,9 +174,7 @@ struct fotg210_hcd {/* one per controller */ #endif /* debug files */ -#ifdef DEBUG struct dentry *debug_dir; -#endif }; /* convert between an HCD pointer and the corresponding FOTG210_HCD */ @@ -741,10 +739,4 @@ static inline unsigned fotg210_read_frame_index(struct fotg210_hcd *fotg210) }) /*-*/ -#ifndef DEBUG -#define STUB_DEBUG_FILES -#endif /* DEBUG */ - -/*-*/ - #endif /* __LINUX_FOTG210_H */ -- 1.8.3.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
[RFCv5 16/21] fusb200h: don't log on every interrupt
From: Oliver Neukum That logging is overkill. Simply remove it. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index 17374a3..bbe1e22 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -5326,13 +5326,6 @@ static irqreturn_t fusbh200_irq (struct usb_hcd *hcd) cmd = fusbh200_readl(fusbh200, &fusbh200->regs->command); bh = 0; -#ifdef VERBOSE_DEBUG - /* unrequested/ignored: Frame List Rollover */ - dbg_status (fusbh200, "irq", status); -#endif - - /* INT, ERR, and IAA interrupt rates can be throttled */ - /* normal [4.15.1.2] or error [4.15.1.1] completion */ if (likely ((status & (STS_INT|STS_ERR)) != 0)) { if (likely ((status & STS_ERR) == 0)) -- 1.8.3.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
[RFCv5 19/21] fusbh200: kill fusbh200_vdbg
From: Oliver Neukum With dynamic debugging this log level is no longer supported. The decision which messages are interesting is done in user space. Signed-off-by: Oliver Neukum --- drivers/usb/host/fusbh200-hcd.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index b561e87b..d5e379b 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -104,12 +104,6 @@ MODULE_PARM_DESC(hird, "host initiated resume duration, +1 for each 75us"); #define fusbh200_warn(fusbh200, fmt, args...) \ dev_warn (fusbh200_to_hcd(fusbh200)->self.controller , fmt , ## args ) -#ifdef VERBOSE_DEBUG -# define fusbh200_vdbg fusbh200_dbg -#else - static inline void fusbh200_vdbg(struct fusbh200_hcd *fusbh200, ...) {} -#endif - /* check the values in the HCSPARAMS register * (host controller _Structural_ parameters) * see EHCI spec, Table 2-4 for each value @@ -1309,7 +1303,7 @@ static void fusbh200_iaa_watchdog(struct fusbh200_hcd *fusbh200) fusbh200_writel(fusbh200, STS_IAA, &fusbh200->regs->status); } - fusbh200_vdbg(fusbh200, "IAA watchdog: status %x cmd %x\n", + fusbh200_dbg(fusbh200, "IAA watchdog: status %x cmd %x\n", status, cmd); end_unlink_async(fusbh200); } @@ -1759,7 +1753,7 @@ static int fusbh200_hub_control ( * which can be fine if this root hub has a * transaction translator built in. */ - fusbh200_vdbg (fusbh200, "port %d reset\n", wIndex + 1); + fusbh200_dbg(fusbh200, "port %d reset\n", wIndex + 1); temp |= PORT_RESET; temp &= ~PORT_PE; @@ -2242,7 +2236,7 @@ static int qtd_copy_status ( status = -EPROTO; } - fusbh200_vdbg (fusbh200, + fusbh200_dbg(fusbh200, "dev%d ep%d%s qtd token %08x --> status %d\n", usb_pipedevice (urb->pipe), usb_pipeendpoint (urb->pipe), @@ -4529,7 +4523,7 @@ static void itd_link_urb( if (unlikely (list_empty(&stream->td_list))) { fusbh200_to_hcd(fusbh200)->self.bandwidth_allocated += stream->bandwidth; - fusbh200_vdbg (fusbh200, + fusbh200_dbg(fusbh200, "schedule devp %s ep%d%s-iso period %d start %d.%d\n", urb->dev->devpath, stream->bEndpointAddress & 0x0f, (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out", @@ -4660,7 +4654,7 @@ static bool itd_complete(struct fusbh200_hcd *fusbh200, struct fusbh200_itd *itd if (unlikely(list_is_singular(&stream->td_list))) { fusbh200_to_hcd(fusbh200)->self.bandwidth_allocated -= stream->bandwidth; - fusbh200_vdbg (fusbh200, + fusbh200_dbg(fusbh200, "deschedule devp %s ep%d%s-iso\n", dev->devpath, stream->bEndpointAddress & 0x0f, (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out"); -- 1.8.3.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
[RFCv5 01/21] ohci: remove conditional compilation
From: Oliver Neukum Conditional compilation for debugging is removed in favor of dynamic debugging. To do so 1. the support for debugfs is always compiled 2. the support for the ancient print_urb debugging aid is removed Signed-off-by: Oliver Neukum --- drivers/usb/host/ohci-dbg.c | 69 - drivers/usb/host/ohci-hcd.c | 8 -- drivers/usb/host/ohci-q.c | 4 --- drivers/usb/host/ohci.h | 7 + 4 files changed, 1 insertion(+), 87 deletions(-) diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c index 3fca52e..45032e9 100644 --- a/drivers/usb/host/ohci-dbg.c +++ b/drivers/usb/host/ohci-dbg.c @@ -9,8 +9,6 @@ /*-*/ -#ifdef DEBUG - #define edstring(ed_type) ({ char *temp; \ switch (ed_type) { \ case PIPE_CONTROL: temp = "ctrl"; break; \ @@ -20,57 +18,6 @@ } temp;}) #define pipestring(pipe) edstring(usb_pipetype(pipe)) -/* debug| print the main components of an URB - * small: 0) header + data packets 1) just header - */ -static void __maybe_unused -urb_print(struct urb * urb, char * str, int small, int status) -{ - unsigned int pipe= urb->pipe; - - if (!urb->dev || !urb->dev->bus) { - printk(KERN_DEBUG "%s URB: no dev\n", str); - return; - } - -#ifndefOHCI_VERBOSE_DEBUG - if (status != 0) -#endif - printk(KERN_DEBUG "%s %p dev=%d ep=%d%s-%s flags=%x len=%d/%d stat=%d\n", - str, - urb, - usb_pipedevice (pipe), - usb_pipeendpoint (pipe), - usb_pipeout (pipe)? "out" : "in", - pipestring (pipe), - urb->transfer_flags, - urb->actual_length, - urb->transfer_buffer_length, - status); - -#ifdef OHCI_VERBOSE_DEBUG - if (!small) { - int i, len; - - if (usb_pipecontrol (pipe)) { - printk (KERN_DEBUG "%s: setup(8):", __FILE__); - for (i = 0; i < 8 ; i++) - printk (" %02x", ((__u8 *) urb->setup_packet) [i]); - printk ("\n"); - } - if (urb->transfer_buffer_length > 0 && urb->transfer_buffer) { - printk (KERN_DEBUG "%s: data(%d/%d):", __FILE__, - urb->actual_length, - urb->transfer_buffer_length); - len = usb_pipeout (pipe)? - urb->transfer_buffer_length: urb->actual_length; - for (i = 0; i < 16 && i < len; i++) - printk (" %02x", ((__u8 *) urb->transfer_buffer) [i]); - printk ("%s stat:%d\n", i < len? "...": "", status); - } - } -#endif -} #define ohci_dbg_sw(ohci, next, size, format, arg...) \ do { \ @@ -407,22 +354,8 @@ ohci_dump_ed (const struct ohci_hcd *ohci, const char *label, } } -#else -static inline void ohci_dump (struct ohci_hcd *controller, int verbose) {} - -#undef OHCI_VERBOSE_DEBUG - -#endif /* DEBUG */ - /*-*/ -#ifdef STUB_DEBUG_FILES - -static inline void create_debug_files (struct ohci_hcd *bus) { } -static inline void remove_debug_files (struct ohci_hcd *bus) { } - -#else - static int debug_async_open(struct inode *, struct file *); static int debug_periodic_open(struct inode *, struct file *); static int debug_registers_open(struct inode *, struct file *); @@ -871,7 +804,5 @@ static inline void remove_debug_files (struct ohci_hcd *ohci) debugfs_remove(ohci->debug_dir); } -#endif - /*-*/ diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 8ada13f..e1c6809 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -127,10 +127,6 @@ static int ohci_urb_enqueue ( unsigned long flags; int retval = 0; -#ifdef OHCI_VERBOSE_DEBUG - urb_print(urb, "SUB", usb_pipein(pipe), -EINPROGRESS); -#endif - /* every endpoint has a ed, locate and maybe (re)initialize it */ if (! (ed = ed_get (ohci, urb->ep, urb->dev, pipe, urb->interval))) return -ENOMEM; @@ -284,10 +280,6 @@ static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) unsigned long flags; int rc; -#ifdef OHCI_VERBOSE_DEBUG - urb_print(urb, "UNLINK", 1, status); -#endif - spin_lock_irqsave (&ohci->lock, flags); rc = usb_hcd_check_unlink_urb(hcd, urb, status); if (rc) { diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c index e7
[RFCv5 04/21] ohci: no conditional debugging in root hub hadling
From: Oliver Neukum With dynamic debugging the selection is done in user space Signed-off-by: Oliver Neukum --- drivers/usb/host/ohci-hub.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 61705a7..c81c872 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -725,10 +725,8 @@ static int ohci_hub_control ( temp = roothub_portstatus (ohci, wIndex); put_unaligned_le32(temp, buf); -#ifndefOHCI_VERBOSE_DEBUG - if (*(u16*)(buf+2)) /* only if wPortChange is interesting */ -#endif - dbg_port (ohci, "GetStatus", wIndex, temp); + if (*(u16*)(buf+2)) /* only if wPortChange is interesting */ + dbg_port(ohci, "GetStatus", wIndex, temp); break; case SetHubFeature: switch (wValue) { -- 1.8.3.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
[RFCv5 11/21] fotg210: kill fotg210_vdbg()
From: Oliver Neukum The decision what is interesting is shifted to user space by dynamic debugging. Signed-off-by: Oliver Neukum --- drivers/usb/host/fotg210-hcd.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 22f0b68..070582b 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -105,12 +105,6 @@ MODULE_PARM_DESC(hird, "host initiated resume duration, +1 for each 75us"); #define fotg210_warn(fotg210, fmt, args...) \ dev_warn(fotg210_to_hcd(fotg210)->self.controller , fmt , ## args) -#ifdef VERBOSE_DEBUG -# define fotg210_vdbg fotg210_dbg -#else - static inline void fotg210_vdbg(struct fotg210_hcd *fotg210, ...) {} -#endif - /* check the values in the HCSPARAMS register * (host controller _Structural_ parameters) * see EHCI spec, Table 2-4 for each value @@ -1349,7 +1343,7 @@ static void fotg210_iaa_watchdog(struct fotg210_hcd *fotg210) &fotg210->regs->status); } - fotg210_vdbg(fotg210, "IAA watchdog: status %x cmd %x\n", + fotg210_dbg(fotg210, "IAA watchdog: status %x cmd %x\n", status, cmd); end_unlink_async(fotg210); } @@ -1805,7 +1799,7 @@ static int fotg210_hub_control( * which can be fine if this root hub has a * transaction translator built in. */ - fotg210_vdbg(fotg210, "port %d reset\n", wIndex + 1); + fotg210_dbg(fotg210, "port %d reset\n", wIndex + 1); temp |= PORT_RESET; temp &= ~PORT_PE; @@ -2289,7 +2283,7 @@ static int qtd_copy_status( status = -EPROTO; } - fotg210_vdbg(fotg210, + fotg210_dbg(fotg210, "dev%d ep%d%s qtd token %08x --> status %d\n", usb_pipedevice(urb->pipe), usb_pipeendpoint(urb->pipe), @@ -4592,7 +4586,7 @@ static void itd_link_urb( if (unlikely(list_empty(&stream->td_list))) { fotg210_to_hcd(fotg210)->self.bandwidth_allocated += stream->bandwidth; - fotg210_vdbg(fotg210, + fotg210_dbg(fotg210, "schedule devp %s ep%d%s-iso period %d start %d.%d\n", urb->dev->devpath, stream->bEndpointAddress & 0x0f, (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out", @@ -4725,7 +4719,7 @@ static bool itd_complete(struct fotg210_hcd *fotg210, struct fotg210_itd *itd) if (unlikely(list_is_singular(&stream->td_list))) { fotg210_to_hcd(fotg210)->self.bandwidth_allocated -= stream->bandwidth; - fotg210_vdbg(fotg210, + fotg210_dbg(fotg210, "deschedule devp %s ep%d%s-iso\n", dev->devpath, stream->bEndpointAddress & 0x0f, (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out"); -- 1.8.3.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
[RFCv5 13/21] uhci: change dependency for debug parameter
From: Oliver Neukum To allow a full switch to dynamic debugging make the debug parameter conditional on defined(DEBUF) || defined(CONFIG_DYNAMIC_DEBUG) Signed-off-by: Oliver Neukum --- drivers/usb/host/uhci-hcd.c | 44 +--- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c index 4a86b63..9bdbff6 100644 --- a/drivers/usb/host/uhci-hcd.c +++ b/drivers/usb/host/uhci-hcd.c @@ -69,18 +69,21 @@ MODULE_PARM_DESC(ignore_oc, "ignore hardware overcurrent indications"); *show all queues in /sys/kernel/debug/uhci/[pci_addr] * debug = 3, show all TDs in URBs when dumping */ -#ifdef DEBUG -#define DEBUG_CONFIGURED 1 +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) + static int debug = 1; module_param(debug, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(debug, "Debug level"); +static char *errbuf; #else -#define DEBUG_CONFIGURED 0 -#define debug 0 + +#define debug 0 +#define errbuf NULL + #endif -static char *errbuf; + #define ERRBUF_LEN(32 * 1024) static struct kmem_cache *uhci_up_cachep; /* urb_priv */ @@ -516,13 +519,12 @@ static void release_uhci(struct uhci_hcd *uhci) { int i; - if (DEBUG_CONFIGURED) { - spin_lock_irq(&uhci->lock); - uhci->is_initialized = 0; - spin_unlock_irq(&uhci->lock); - debugfs_remove(uhci->dentry); - } + spin_lock_irq(&uhci->lock); + uhci->is_initialized = 0; + spin_unlock_irq(&uhci->lock); + + debugfs_remove(uhci->dentry); for (i = 0; i < UHCI_NUM_SKELQH; i++) uhci_free_qh(uhci, uhci->skelqh[i]); @@ -868,14 +870,14 @@ static int __init uhci_hcd_init(void) ignore_oc ? ", overcurrent ignored" : ""); set_bit(USB_UHCI_LOADED, &usb_hcds_loaded); - if (DEBUG_CONFIGURED) { - errbuf = kmalloc(ERRBUF_LEN, GFP_KERNEL); - if (!errbuf) - goto errbuf_failed; - uhci_debugfs_root = debugfs_create_dir("uhci", usb_debug_root); - if (!uhci_debugfs_root) - goto debug_failed; - } +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) + errbuf = kmalloc(ERRBUF_LEN, GFP_KERNEL); + if (!errbuf) + goto errbuf_failed; +#endif + uhci_debugfs_root = debugfs_create_dir("uhci", usb_debug_root); + if (!uhci_debugfs_root) + goto debug_failed; uhci_up_cachep = kmem_cache_create("uhci_urb_priv", sizeof(struct urb_priv), 0, 0, NULL); @@ -909,9 +911,11 @@ up_failed: debugfs_remove(uhci_debugfs_root); debug_failed: +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) kfree(errbuf); errbuf_failed: +#endif clear_bit(USB_UHCI_LOADED, &usb_hcds_loaded); return retval; @@ -927,7 +931,9 @@ static void __exit uhci_hcd_cleanup(void) #endif kmem_cache_destroy(uhci_up_cachep); debugfs_remove(uhci_debugfs_root); +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG) kfree(errbuf); +#endif clear_bit(USB_UHCI_LOADED, &usb_hcds_loaded); } -- 1.8.3.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
[RFCv5] kill #undef OHCI_VERBOSE_DEBUG
From: Oliver Neukum It is useless now. Straight removal. Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hcd.c | 1 - drivers/usb/host/fotg210-hcd.c | 1 - drivers/usb/host/fusbh200-hcd.c | 1 - drivers/usb/host/ohci-hcd.c | 2 -- 4 files changed, 5 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 1e21a36..4711427 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -71,7 +71,6 @@ static const char hcd_name [] = "ehci_hcd"; -#undef VERBOSE_DEBUG #undef EHCI_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 070582b..97d6939 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -56,7 +56,6 @@ static const char hcd_name[] = "fotg210_hcd"; -#undef VERBOSE_DEBUG #undef FOTG210_URB_TRACE #define FOTG210_STATS diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index d5e379b..9ea85b6 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -57,7 +57,6 @@ static const char hcd_name [] = "fusbh200_hcd"; -#undef VERBOSE_DEBUG #undef FUSBH200_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index de0e3e4..c8e0e76 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -51,8 +51,6 @@ /*-*/ -#undef OHCI_VERBOSE_DEBUG /* not always helpful */ - /* For initializing controller (mask in an HCFS mode too) */ #defineOHCI_CONTROL_INIT OHCI_CTRL_CBSR #defineOHCI_INTR_INIT \ -- 1.8.3.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: ohci-pxa27x: include linux/dma-mapping.h
On Fri, Nov 15, 2013 at 09:45:16AM +0100, Daniel Mack wrote: > Include linux/dma-mapping.h to make the new functions available that are > used since 22d9d8e83 ("DMA-API: usb: use dma_set_coherent_mask()"). > > Signed-off-by: Daniel Mack > --- > I got the following error while building for PXA platforms from Linus' > current git head: > > drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’: > drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function > ‘dma_coerce_mask_and_coherent’ [-Werror=implicit-function-declaration] > drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function > ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration] Please put the errors (and warnings) in the commit description; they're useful documentation for others who may encounter the same problem. > diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c > index e89ac4d..3963834 100644 > --- a/drivers/usb/host/ohci-pxa27x.c > +++ b/drivers/usb/host/ohci-pxa27x.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include Please review the #include statements and see if you can spot a broad pattern there, and then decide whether your placement follows the established pattern, thanks. :) -- 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: ohci-pxa27x: include linux/dma-mapping.h
On 11/15/2013 02:01 PM, Russell King - ARM Linux wrote: > On Fri, Nov 15, 2013 at 09:45:16AM +0100, Daniel Mack wrote: >> Include linux/dma-mapping.h to make the new functions available that are >> used since 22d9d8e83 ("DMA-API: usb: use dma_set_coherent_mask()"). >> >> Signed-off-by: Daniel Mack >> --- >> I got the following error while building for PXA platforms from Linus' >> current git head: >> >> drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’: >> drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of >> function ‘dma_coerce_mask_and_coherent’ >> [-Werror=implicit-function-declaration] >> drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of >> function ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration] > > Please put the errors (and warnings) in the commit description; they're > useful documentation for others who may encounter the same problem. > >> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c >> index e89ac4d..3963834 100644 >> --- a/drivers/usb/host/ohci-pxa27x.c >> +++ b/drivers/usb/host/ohci-pxa27x.c >> @@ -29,6 +29,7 @@ >> #include >> #include >> #include >> +#include > > Please review the #include statements and see if you can spot a broad > pattern there, and then decide whether your placement follows the > established pattern, thanks. :) Uh, didn't expect alphanumeric ordering here. Thanks :) -- 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 v2] usb: ohci-pxa27x: include linux/dma-mapping.h
Include linux/dma-mapping.h to make the new functions available that are used since 22d9d8e83 ("DMA-API: usb: use dma_set_coherent_mask()"). drivers/usb/host/ohci-pxa27x.c: In function ‘ohci_pxa_of_init’: drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function ‘dma_coerce_mask_and_coherent’ [-Werror=implicit-function-declaration] drivers/usb/host/ohci-pxa27x.c:310:2: error: implicit declaration of function ‘DMA_BIT_MASK’ [-Werror=implicit-function-declaration] Signed-off-by: Daniel Mack --- drivers/usb/host/ohci-pxa27x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c index e89ac4d..9b7435f 100644 --- a/drivers/usb/host/ohci-pxa27x.c +++ b/drivers/usb/host/ohci-pxa27x.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include -- 1.8.4.2 -- 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: [RFCv5] kill #undef OHCI_VERBOSE_DEBUG
Hello. On 15-11-2013 16:59, oli...@neukum.org wrote: From: Oliver Neukum Perhaps you meant [OHCI_]VERBOSE_DEBUG in the subject? Also, you've posted the patch twice for some reason, once as part of the series, and yet once as a separate patch. It is useless now. Straight removal. Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hcd.c | 1 - drivers/usb/host/fotg210-hcd.c | 1 - drivers/usb/host/fusbh200-hcd.c | 1 - drivers/usb/host/ohci-hcd.c | 2 -- 4 files changed, 5 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 1e21a36..4711427 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -71,7 +71,6 @@ static const char hcd_name [] = "ehci_hcd"; -#undef VERBOSE_DEBUG #undef EHCI_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 070582b..97d6939 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -56,7 +56,6 @@ static const char hcd_name[] = "fotg210_hcd"; -#undef VERBOSE_DEBUG #undef FOTG210_URB_TRACE #define FOTG210_STATS diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index d5e379b..9ea85b6 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -57,7 +57,6 @@ static const char hcd_name [] = "fusbh200_hcd"; -#undef VERBOSE_DEBUG #undef FUSBH200_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index de0e3e4..c8e0e76 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -51,8 +51,6 @@ /*-*/ -#undef OHCI_VERBOSE_DEBUG /* not always helpful */ - /* For initializing controller (mask in an HCFS mode too) */ #define OHCI_CONTROL_INIT OHCI_CTRL_CBSR #define OHCI_INTR_INIT \ 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: [RFCv5] kill #undef OHCI_VERBOSE_DEBUG
On Fri, 2013-11-15 at 18:03 +0400, Sergei Shtylyov wrote: > Hello. > > On 15-11-2013 16:59, oli...@neukum.org wrote: > > > From: Oliver Neukum > > Perhaps you meant [OHCI_]VERBOSE_DEBUG in the subject? > Also, you've posted the patch twice for some reason, once as part of the > series, and yet once as a separate patch. Yes, thank you. A corrected version has been sent out. Regards Oliver -- 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
[RFCv5-corrected subject] USB: kill #undef VERBOSE_DEBUG
From: Oliver Neukum It is useless now. Straight removal. Signed-off-by: Oliver Neukum --- drivers/usb/host/ehci-hcd.c | 1 - drivers/usb/host/fotg210-hcd.c | 1 - drivers/usb/host/fusbh200-hcd.c | 1 - drivers/usb/host/ohci-hcd.c | 2 -- 4 files changed, 5 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 1e21a36..4711427 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -71,7 +71,6 @@ static const char hcd_name [] = "ehci_hcd"; -#undef VERBOSE_DEBUG #undef EHCI_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 070582b..97d6939 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -56,7 +56,6 @@ static const char hcd_name[] = "fotg210_hcd"; -#undef VERBOSE_DEBUG #undef FOTG210_URB_TRACE #define FOTG210_STATS diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index d5e379b..9ea85b6 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -57,7 +57,6 @@ static const char hcd_name [] = "fusbh200_hcd"; -#undef VERBOSE_DEBUG #undef FUSBH200_URB_TRACE /* magic numbers that can affect system performance */ diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index de0e3e4..c8e0e76 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -51,8 +51,6 @@ /*-*/ -#undef OHCI_VERBOSE_DEBUG /* not always helpful */ - /* For initializing controller (mask in an HCFS mode too) */ #defineOHCI_CONTROL_INIT OHCI_CTRL_CBSR #defineOHCI_INTR_INIT \ -- 1.8.3.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 v3] chipidea: ci_hdrc_imx: Allow handling the clock for an external USB hub
On Fri, Nov 15, 2013 at 12:42 AM, Shawn Guo wrote: > Hmm, I would argue if it can be done in bootloader before kernel has a > proper little driver for it. Yes, I can turn on CLKO2 in the bootloader for the time being. Regards, Fabio Estevam -- 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] PM / Runtime: Fix error path for prepare
On Friday, November 15, 2013 12:40:44 PM Ulf Hansson wrote: > If a device prepare callback for some reason would fail, the PM core > prevented the device from going inactive forever. > > In this case, to reverse the pm_runtime_get_noresume() we invokes the > asyncronous pm_runtime_put(), thus restoring the usage count. > > Cc: Kevin Hilman > Cc: Alan Stern > Signed-off-by: Ulf Hansson This should be in linux-next already. Thanks! > --- > drivers/base/power/main.c |3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index ee039af..2a1b06a 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -1350,6 +1350,9 @@ static int device_prepare(struct device *dev, > pm_message_t state) > > device_unlock(dev); > > + if (error) > + pm_runtime_put(dev); > + > return error; > } > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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
[GIT pull for 3.14] xhci streams and uas fixes + usbfs streams support
Hi Sarah et al, I've collected all the work first staged for 3.13 in your fun-streams-fixes branch and my recent uas work into a branch for you to pull into your xhci tree for 3.14 . Patches 1 - 35 are patches which have been send earlier, here is a list with those which have changed because of review remarks and/or issues found during testing: "xhci: For streams the css flag most be read from the stream-ctx on ep stop": Fix sparse warning caused by using __le64 for a native endian variable. "usbfs: proc_do_submiturb use a local variable for number_of_packet": Removed part which would cause interrupt urbs to use sg. "usb-core: Track if an endpoint has streams": Documented new usb_host_endpoint streams member. "xhci: Remove segments from radix tree on failed insert": Patch by Sarah Sharp, I've added a check to only update stream mappings in xhci_ring_expansion when the ring is a stream ring. "uas: Deal with externally triggered reset, and fix reset locking" Dropped, replaced with 2 later patches which also keep the device alive on an externally triggered reset. "uas: Fix reset handling for externally triggered reset" Simplified the code for waiting for pending requests to finish. "uas: Fix response iu struct definition" This is a new patch inserted into the series of existing patches to avoid the later "uas: Use proper packet size when submitting reponse urbs" patch to cause regressions. "uas: Pack iu struct definitions" Updated the commit message to reflect that with the fixed response iu struct definition packing makes no difference on archs with 32 bit alignment. "usb-storage: Don't bind to uas devices if the uas driver is enabled" This has been moved to be part of the series of new patches, where it is based on top of a couple of new patches to allow blacklisting uas devices from using the uas driver (making them fall-back to the usb-storage driver). Patches 36 - 38 are 3 new xhci fixes Patches 39 - 40 are 3 new usb core fixes Patches 41 - 68 is a new series of uas fixes / cleanups The following changes since commit 7d49f0bac41ee9b012af1efe2f725d91a87a8fe9: USB: Maintainers change for usb serial drivers (2013-10-31 08:53:52 -0700) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git usb-next-for-sarah for you to fetch changes up to 8abebe07cfb6e06049082535e6a2835f9e80fbc5: uas: Remove comment about registering a uas scsi controller for each usb bus (2013-11-15 15:51:50 +0100) Gerd Hoffmann (6): xhci: fix usb3 streams uas: properly reinitialize in uas_eh_bus_reset_handler uas: make work list per-device uas: add dead request list uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() uas: remove BROKEN Hans de Goede (61): xhci: Free streams when they are still allocated on a set_interface call xhci: Check size rather then number of streams when allocating stream ctxs xhci: For streams the css flag most be read from the stream-ctx on ep stop xhci: Set SCT field for Set TR dequeue on streams xhci: For streams the dequeue ptr must be read from the stream ctx xhci: use usb_ss_max_streams in xhci_check_streams_endpoint usb-core: Fix usb_free_streams return value documentation usb-core: Move USB_MAXENDPOINTS definitions to usb.h usb-core: Track if an endpoint has streams usb-core: Free bulk streams on interface release usbfs: Kill urbs on interface before doing a set_interface usbfs: proc_do_submiturb use a local variable for number_of_packets usbfs: Add support for bulk stream ids usbfs: Add ep_to_host_endpoint helper function usbfs: Add support for allocating / freeing streams uas: Urbs must be anchored before submitting them uas: Properly set interface to altsetting 0 on probe failure uas: Avoid unnecessary unlock / lock calls around unlink_data_urbs uas: uas_alloc_cmd_urb: drop unused stream_id parameter uas: Fix uas not working when plugged into an ehci port uas: Fix reset locking uas: Fix reset handling for externally triggered reset uas: s/response_ui/response_iu/ uas: Fix response iu struct definition uas: Pack iu struct definitions uas: Use proper packet size when submitting reponse urbs uas: Use all available stream ids uas: Add a uas_find_uas_alt_setting helper function uas: Move uas detect code to uas-detect.h xhci: xhci_mem_cleanup: make sure cmd_ring_reserved_trbs really is 0 xhci: The trb_address_map radix tree expects 1KB segment memory aligment xhci: Handle MaxPSASize == 0 usb: Clear host_endpoint->streams when implicitly freeing streams usb: Reset USB-3 devices on USB-3 link bounce uas: Add the posibilty to blacklist uas devices from using the uas driver uas: Add a usbcore.nouas kernel cmdline option usb-storage: Don't bind to ua
[PATCH 06/68] xhci: For streams the dequeue ptr must be read from the stream ctx
This fixes TR dequeue validation failing on Intel XHCI controllers with the following warning: Mismatch between completed Set TR Deq Ptr command & xHCI internal state. Interestingly enough reading the deq ptr from the ep ctx after a TR Deq Ptr command does work on a Nec XHCI controller, it seems the Nec writes the ptr to both the ep and stream contexts when streams are used. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-ring.c | 22 +++--- drivers/usb/host/xhci.h | 1 + 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4908c79..38f64d2 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1087,12 +1087,14 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, unsigned int stream_id; struct xhci_ring *ep_ring; struct xhci_virt_device *dev; + struct xhci_virt_ep *ep; struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; 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]; + ep = &dev->eps[ep_index]; ep_ring = xhci_stream_id_to_ring(dev, ep_index, stream_id); if (!ep_ring) { @@ -1144,12 +1146,19 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, * cancelling URBs, which might not be an error... */ } else { + u64 deq; + /* 4.6.10 deq ptr is written to the stream ctx for streams */ + if (ep->ep_state & EP_HAS_STREAMS) { + struct xhci_stream_ctx *ctx = + &ep->stream_info->stream_ctx_array[stream_id]; + deq = le64_to_cpu(ctx->stream_ring) & SCTX_DEQ_MASK; + } else { + deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK; + } xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Successful Set TR Deq Ptr cmd, deq = @%08llx", -le64_to_cpu(ep_ctx->deq)); - if (xhci_trb_virt_to_dma(dev->eps[ep_index].queued_deq_seg, -dev->eps[ep_index].queued_deq_ptr) == - (le64_to_cpu(ep_ctx->deq) & ~(EP_CTX_CYCLE_MASK))) { + "Successful Set TR Deq Ptr cmd, deq = @%08llx", deq); + if (xhci_trb_virt_to_dma(ep->queued_deq_seg, +ep->queued_deq_ptr) == deq) { /* Update the ring's dequeue segment and dequeue pointer * to reflect the new position. */ @@ -1159,8 +1168,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, xhci_warn(xhci, "Mismatch between completed Set TR Deq " "Ptr command & xHCI internal state.\n"); xhci_warn(xhci, "ep deq seg = %p, deq ptr = %p\n", - dev->eps[ep_index].queued_deq_seg, - dev->eps[ep_index].queued_deq_ptr); + ep->queued_deq_seg, ep->queued_deq_ptr); } } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 81bafb9..d411c88 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -703,6 +703,7 @@ struct xhci_ep_ctx { /* deq bitmasks */ #define EP_CTX_CYCLE_MASK (1 << 0) +#define SCTX_DEQ_MASK (~0xfL) /** -- 1.8.4.2 -- 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 14/68] usbfs: proc_do_submiturb use a local variable for number_of_packets
This is a preparation patch for adding support for bulk streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index b823bc5..f810d29 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1192,6 +1192,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, struct usb_ctrlrequest *dr = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; + int number_of_packets = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1245,7 +1246,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, le16_to_cpup(&dr->wIndex)); if (ret) goto error; - uurb->number_of_packets = 0; uurb->buffer_length = le16_to_cpup(&dr->wLength); uurb->buffer += 8; if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) { @@ -1275,7 +1275,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; goto interrupt_urb; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; @@ -1285,7 +1284,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; interrupt_urb: - uurb->number_of_packets = 0; break; case USBDEVFS_URB_TYPE_ISO: @@ -1295,15 +1293,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -EINVAL; if (!usb_endpoint_xfer_isoc(&ep->desc)) return -EINVAL; + number_of_packets = uurb->number_of_packets; isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * - uurb->number_of_packets; + number_of_packets; if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL))) return -ENOMEM; if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) { ret = -EFAULT; goto error; } - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { /* * arbitrary limit need for USB 3.0 * bMaxBurst (0~15 allowed, 1~16 packets) @@ -1334,7 +1333,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ret = -EFAULT; goto error; } - as = alloc_async(uurb->number_of_packets); + as = alloc_async(number_of_packets); if (!as) { ret = -ENOMEM; goto error; @@ -1428,7 +1427,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->setup_packet = (unsigned char *)dr; dr = NULL; as->urb->start_frame = uurb->start_frame; - as->urb->number_of_packets = uurb->number_of_packets; + as->urb->number_of_packets = number_of_packets; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); @@ -1436,7 +1435,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->interval = ep->desc.bInterval; as->urb->context = as; as->urb->complete = async_completed; - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { as->urb->iso_frame_desc[u].offset = totlen; as->urb->iso_frame_desc[u].length = isopkt[u].length; totlen += isopkt[u].length; -- 1.8.4.2 -- 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 12/68] usb-core: Free bulk streams on interface release
Documentation/usb/bulk-streams.txt says: All stream IDs will be deallocated when the driver releases the interface, to ensure that drivers that don't support streams will be able to use the endpoint This commit actually implements this. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/driver.c | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 47aade2..802878b 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -369,8 +369,9 @@ static int usb_unbind_interface(struct device *dev) { struct usb_driver *driver = to_usb_driver(dev->driver); struct usb_interface *intf = to_usb_interface(dev); + struct usb_host_endpoint *ep, **eps = NULL; struct usb_device *udev; - int error, r, lpm_disable_error; + int i, j, error, r, lpm_disable_error; intf->condition = USB_INTERFACE_UNBINDING; @@ -394,6 +395,26 @@ static int usb_unbind_interface(struct device *dev) driver->disconnect(intf); usb_cancel_queued_reset(intf); + /* Free streams */ + for (i = 0, j = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { + ep = &intf->cur_altsetting->endpoint[i]; + if (ep->streams == 0) + continue; + if (j == 0) { + eps = kmalloc(USB_MAXENDPOINTS * sizeof(void *), + GFP_KERNEL); + if (!eps) { + dev_warn(dev, "oom, leaking streams\n"); + break; + } + } + eps[j++] = ep; + } + if (j) { + usb_free_streams(intf, eps, j, GFP_KERNEL); + kfree(eps); + } + /* Reset other interface state. * We cannot do a Set-Interface if the device is suspended or * if it is prepared for a system sleep (since installing a new -- 1.8.4.2 -- 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 05/68] xhci: Set SCT field for Set TR dequeue on streams
Nec XHCI controllers don't seem to care, but without this Intel XHCI controllers reject Set TR dequeue commands with a COMP_TRB_ERR, leading to the following warning: WARN Set TR Deq Ptr cmd invalid because of stream ID configuration And very shortly after this the system completely freezes. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-ring.c | 5 - drivers/usb/host/xhci.h | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index fb6df2c..4908c79 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -4084,6 +4084,7 @@ static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id, u32 trb_slot_id = SLOT_ID_FOR_TRB(slot_id); u32 trb_ep_index = EP_ID_FOR_TRB(ep_index); u32 trb_stream_id = STREAM_ID_FOR_TRB(stream_id); + u32 trb_sct = 0; u32 type = TRB_TYPE(TRB_SET_DEQ); struct xhci_virt_ep *ep; @@ -4102,7 +4103,9 @@ static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id, } ep->queued_deq_seg = deq_seg; ep->queued_deq_ptr = deq_ptr; - return queue_command(xhci, lower_32_bits(addr) | cycle_state, + if (stream_id) + trb_sct = SCT_FOR_TRB(SCT_PRI_TR); + return queue_command(xhci, lower_32_bits(addr) | trb_sct | cycle_state, upper_32_bits(addr), trb_stream_id, trb_slot_id | trb_ep_index | type, false); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index f882481..81bafb9 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1110,9 +1110,10 @@ struct xhci_event_cmd { #define TRB_TO_SUSPEND_PORT(p) (((p) & (1 << 23)) >> 23) #define LAST_EP_INDEX 30 -/* Set TR Dequeue Pointer command TRB fields */ +/* Set TR Dequeue Pointer command TRB fields, 6.4.3.9 */ #define TRB_TO_STREAM_ID(p)p) & (0x << 16)) >> 16)) #define STREAM_ID_FOR_TRB(p) p)) & 0x) << 16) +#define SCT_FOR_TRB(p) (((p) << 1) & 0x7) /* Port Status Change Event TRB fields */ -- 1.8.4.2 -- 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 02/68] xhci: Free streams when they are still allocated on a set_interface call
And warn about this, as that would be a driver bug. Like wise drivers should ensure that streams are properly free-ed before a device is reset. So lets warn about that too. This already causes warnings in the form of: [ 96.982398] xhci_hcd :01:00.0: WARN Can't disable streams for endpoint 0x81 , streams are already disabled! [ 96.982400] xhci_hcd :01:00.0: WARN xhci_free_streams() called with non-streams endpoint But it is better to also warn about the actual cause of this later warnings. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 4265b48..28e84bd 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2674,6 +2674,20 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci, return ret; } +static void xhci_check_bw_drop_ep_streams(struct xhci_hcd *xhci, + struct xhci_virt_device *vdev, int i) +{ + struct xhci_virt_ep *ep = &vdev->eps[i]; + + if (ep->ep_state & EP_HAS_STREAMS) { + xhci_warn(xhci, "WARN: endpoint 0x%02x has streams on set_interface, freeing streams.\n", + xhci_get_endpoint_address(i)); + xhci_free_stream_info(xhci, ep->stream_info); + ep->stream_info = NULL; + ep->ep_state &= ~EP_HAS_STREAMS; + } +} + /* Called after one or more calls to xhci_add_endpoint() or * xhci_drop_endpoint(). If this call fails, the USB core is expected * to call xhci_reset_bandwidth(). @@ -2738,8 +2752,10 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev) /* Free any rings that were dropped, but not changed. */ for (i = 1; i < 31; ++i) { if ((le32_to_cpu(ctrl_ctx->drop_flags) & (1 << (i + 1))) && - !(le32_to_cpu(ctrl_ctx->add_flags) & (1 << (i + 1 + !(le32_to_cpu(ctrl_ctx->add_flags) & (1 << (i + 1 { xhci_free_or_cache_endpoint_ring(xhci, virt_dev, i); + xhci_check_bw_drop_ep_streams(xhci, virt_dev, i); + } } xhci_zero_in_ctx(xhci, virt_dev); /* @@ -2755,6 +2771,7 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev) if (virt_dev->eps[i].ring) { xhci_free_or_cache_endpoint_ring(xhci, virt_dev, i); } + xhci_check_bw_drop_ep_streams(xhci, virt_dev, i); virt_dev->eps[i].ring = virt_dev->eps[i].new_ring; virt_dev->eps[i].new_ring = NULL; } @@ -3515,6 +3532,8 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev) struct xhci_virt_ep *ep = &virt_dev->eps[i]; if (ep->ep_state & EP_HAS_STREAMS) { + xhci_warn(xhci, "WARN: endpoint 0x%02x has streams on device reset, freeing streams.\n", + xhci_get_endpoint_address(i)); xhci_free_stream_info(xhci, ep->stream_info); ep->stream_info = NULL; ep->ep_state &= ~EP_HAS_STREAMS; -- 1.8.4.2 -- 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 03/68] xhci: Check size rather then number of streams when allocating stream ctxs
Before this a device needing ie 32 stream ctxs would end up with an entry from the small_streams_pool which has 256 bytes entries, where as 32 stream ctxs need 512 bytes. Things actually keep running for a surprisingly long time before crashing because of this. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-mem.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 1d39b35..a455c56 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -519,12 +519,12 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci, struct xhci_stream_ctx *stream_ctx, dma_addr_t dma) { struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); + size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; - if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE) - dma_free_coherent(&pdev->dev, - sizeof(struct xhci_stream_ctx)*num_stream_ctxs, + if (size > MEDIUM_STREAM_ARRAY_SIZE) + dma_free_coherent(&pdev->dev, size, stream_ctx, dma); - else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE) + else if (size <= SMALL_STREAM_ARRAY_SIZE) return dma_pool_free(xhci->small_streams_pool, stream_ctx, dma); else @@ -547,12 +547,12 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci, gfp_t mem_flags) { struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); + size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; - if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE) - return dma_alloc_coherent(&pdev->dev, - sizeof(struct xhci_stream_ctx)*num_stream_ctxs, + if (size > MEDIUM_STREAM_ARRAY_SIZE) + return dma_alloc_coherent(&pdev->dev, size, dma, mem_flags); - else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE) + else if (size <= SMALL_STREAM_ARRAY_SIZE) return dma_pool_alloc(xhci->small_streams_pool, mem_flags, dma); else -- 1.8.4.2 -- 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 11/68] usb-core: Track if an endpoint has streams
This is a preparation patch for adding support for bulk streams to usbfs. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hcd.c | 40 +++- include/linux/usb.h| 2 ++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index e60e08d..b080696 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2049,7 +2049,7 @@ int usb_alloc_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); @@ -2058,13 +2058,24 @@ int usb_alloc_streams(struct usb_interface *interface, if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) return -EINVAL; + /* Re-alloc is not allowed */ + if (eps[i]->streams) + return -EINVAL; + } - return hcd->driver->alloc_streams(hcd, dev, eps, num_eps, + ret = hcd->driver->alloc_streams(hcd, dev, eps, num_eps, num_streams, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = ret; + + return ret; } EXPORT_SYMBOL_GPL(usb_alloc_streams); @@ -2086,19 +2097,30 @@ int usb_free_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) - if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc)) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ + if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) + return -EINVAL; + /* Double-free is not allowed */ + if (!eps[i]->streams) return -EINVAL; + } + + ret = hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = 0; - return hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + return ret; } EXPORT_SYMBOL_GPL(usb_free_streams); diff --git a/include/linux/usb.h b/include/linux/usb.h index 4c53d54..a4c8406 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -57,6 +57,7 @@ struct ep_device; * @extra: descriptors following this endpoint in the configuration * @extralen: how many bytes of "extra" are valid * @enabled: URBs may be submitted to this endpoint + * @streams: number of USB-3 streams allocated on the endpoint * * USB requests are always queued to a given endpoint, identified by a * descriptor within an active interface in a given USB configuration. @@ -71,6 +72,7 @@ struct usb_host_endpoint { unsigned char *extra; /* Extra descriptors */ int extralen; int enabled; + int streams; }; /* host-side wrapper for one interface setting's parsed descriptors */ -- 1.8.4.2 -- 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 13/68] usbfs: Kill urbs on interface before doing a set_interface
The usb_set_interface documentation says: * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed * (perhaps forced by unlinking). For in kernel drivers we trust the drivers to get this right, but we cannot trust userspace to get this right, so enforce it by killing any urbs still pending on the interface. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 967152a..b823bc5 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1127,6 +1127,9 @@ static int proc_setintf(struct dev_state *ps, void __user *arg) return -EFAULT; if ((ret = checkintf(ps, setintf.interface))) return ret; + + destroy_async_on_interface(ps, setintf.interface); + return usb_set_interface(ps->dev, setintf.interface, setintf.altsetting); } -- 1.8.4.2 -- 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 10/68] usb-core: Move USB_MAXENDPOINTS definitions to usb.h
So that it can be used in other places too. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/config.c | 1 - include/linux/usb.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index a6b2cab..e4f970c 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -11,7 +11,6 @@ #define USB_MAXALTSETTING 128 /* Hard limit */ -#define USB_MAXENDPOINTS 30 /* Hard limit */ #define USB_MAXCONFIG 8 /* Arbitrary limit */ diff --git a/include/linux/usb.h b/include/linux/usb.h index 7454865..4c53d54 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -202,6 +202,8 @@ static inline void usb_set_intfdata(struct usb_interface *intf, void *data) struct usb_interface *usb_get_intf(struct usb_interface *intf); void usb_put_intf(struct usb_interface *intf); +/* Hard limit */ +#define USB_MAXENDPOINTS 30 /* this maximum is arbitrary */ #define USB_MAXINTERFACES 32 #define USB_MAXIADS(USB_MAXINTERFACES/2) -- 1.8.4.2 -- 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 07/68] xhci: use usb_ss_max_streams in xhci_check_streams_endpoint
The ss_ep_comp bmAttributes filed can contain more info then just the streams, use usb_ss_max_streams to properly get max streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 28e84bd..00ba201 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2967,7 +2967,7 @@ static int xhci_check_streams_endpoint(struct xhci_hcd *xhci, ret = xhci_check_args(xhci_to_hcd(xhci), udev, ep, 1, true, __func__); if (ret <= 0) return -EINVAL; - if (ep->ss_ep_comp.bmAttributes == 0) { + if (usb_ss_max_streams(&ep->ss_ep_comp) == 0) { xhci_warn(xhci, "WARN: SuperSpeed Endpoint Companion" " descriptor for ep 0x%x does not support streams\n", ep->desc.bEndpointAddress); -- 1.8.4.2 -- 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 09/68] usb-core: Fix usb_free_streams return value documentation
Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hcd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 6bffb8c..e60e08d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2078,8 +2078,7 @@ EXPORT_SYMBOL_GPL(usb_alloc_streams); * Reverts a group of bulk endpoints back to not using stream IDs. * Can fail if we are given bad arguments, or HCD is broken. * - * Return: On success, the number of allocated streams. On failure, a negative - * error code. + * Return: 0 on success. On failure, a negative error code. */ int usb_free_streams(struct usb_interface *interface, struct usb_host_endpoint **eps, unsigned int num_eps, -- 1.8.4.2 -- 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 08/68] xhci: Remove segments from radix tree on failed insert.
From: Sarah Sharp If we're expanding a stream ring, we want to make sure we can add those ring segments to the radix tree that maps segments to ring pointers. Try the radix tree insert after the new ring segments have been allocated (the last segment in the new ring chunk will point to the first newly allocated segment), but before the new ring segments are linked into the old ring. If insert fails on any one segment, remove each segment from the radix tree, deallocate the new segments, and return. Otherwise, link the new segments into the tree. HdG: Add a check to only update stream mappings in xhci_ring_expansion when the ring is a stream ring. Signed-off-by: Sarah Sharp Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-mem.c | 107 +--- 1 file changed, 81 insertions(+), 26 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index a455c56..2c589e5 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,53 +180,98 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, * extended systems (where the DMA address can be bigger than 32-bits), * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that. */ -static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +static int xhci_insert_segment_mapping(struct radix_tree_root *trb_address_map, + struct xhci_ring *ring, + struct xhci_segment *seg, + gfp_t mem_flags) { - struct xhci_segment *seg; unsigned long key; int ret; - if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + /* Skip any segments that were already added. */ + if (radix_tree_lookup(trb_address_map, key)) return 0; - seg = ring->first_seg; - do { - key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); - /* Skip any segments that were already added. */ - if (radix_tree_lookup(ring->trb_address_map, key)) - continue; + ret = radix_tree_maybe_preload(mem_flags); + if (ret) + return ret; + ret = radix_tree_insert(trb_address_map, + key, ring); + radix_tree_preload_end(); + return ret; +} - ret = radix_tree_maybe_preload(mem_flags); - if (ret) - return ret; - ret = radix_tree_insert(ring->trb_address_map, - key, ring); - radix_tree_preload_end(); +static void xhci_remove_segment_mapping(struct radix_tree_root *trb_address_map, + struct xhci_segment *seg) +{ + unsigned long key; + + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + if (radix_tree_lookup(trb_address_map, key)) + radix_tree_delete(trb_address_map, key); +} + +static int xhci_update_stream_segment_mapping( + struct radix_tree_root *trb_address_map, + struct xhci_ring *ring, + struct xhci_segment *first_seg, + struct xhci_segment *last_seg, + gfp_t mem_flags) +{ + struct xhci_segment *seg; + struct xhci_segment *failed_seg; + int ret; + + if (WARN_ON_ONCE(trb_address_map == NULL)) + return 0; + + seg = first_seg; + do { + ret = xhci_insert_segment_mapping(trb_address_map, + ring, seg, mem_flags); if (ret) - return ret; + goto remove_streams; + if (seg == last_seg) + return 0; seg = seg->next; - } while (seg != ring->first_seg); + } while (seg != first_seg); return 0; + +remove_streams: + failed_seg = seg; + seg = first_seg; + do { + xhci_remove_segment_mapping(trb_address_map, seg); + if (seg == failed_seg) + return ret; + seg = seg->next; + } while (seg != first_seg); + + return ret; } static void xhci_remove_stream_mapping(struct xhci_ring *ring) { struct xhci_segment *seg; - unsigned long key; if (WARN_ON_ONCE(ring->trb_address_map == NULL)) return; seg = ring->first_seg; do { - key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); - if (radix_tree_lookup(ring->trb_address_map, key)) - radix_tree_delete(ring->trb_address_map, key); + xhci_remove_segment_mapping(ring->trb_address_map, seg); seg = seg->next; } while (seg != ring->first_seg); } +static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +{ + return xhci_update_stream_segment_
[PATCH 01/68] xhci: fix usb3 streams
From: Gerd Hoffmann xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. Result is that things work initially, but as soon as the first segment is full xhci can't map the trb address from the completion event to the stream ring any more -> BOOM. You'll find this message in the logs: ERROR Transfer event for disabled endpoint or incorrect stream ring This patch adds a helper function to update the radix tree, and a function to remove ring segments from the tree. Both functions loop over the segment list and handles all segments instead of just the first. [Note: Sarah changed this patch to add radix_tree_maybe_preload() and radix_tree_preload_end() calls around the radix tree insert, since we can now insert entries in interrupt context. There are now two helper functions to make the code cleaner, and those functions are moved to make them static.] Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-mem.c | 132 +--- drivers/usb/host/xhci.h | 1 + 2 files changed, 90 insertions(+), 43 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 49b8bd0..1d39b35 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -149,14 +149,95 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, } } +/* + * We need a radix tree for mapping physical addresses of TRBs to which stream + * ID they belong to. We need to do this because the host controller won't tell + * us which stream ring the TRB came from. We could store the stream ID in an + * event data TRB, but that doesn't help us for the cancellation case, since the + * endpoint may stop before it reaches that event data TRB. + * + * The radix tree maps the upper portion of the TRB DMA address to a ring + * segment that has the same upper portion of DMA addresses. For example, say I + * have segments of size 1KB, that are always 64-byte aligned. A segment may + * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the + * key to the stream ID is 0x43244. I can use the DMA address of the TRB to + * pass the radix tree a key to get the right stream ID: + * + * 0x10c90fff >> 10 = 0x43243 + * 0x10c912c0 >> 10 = 0x43244 + * 0x10c91400 >> 10 = 0x43245 + * + * Obviously, only those TRBs with DMA addresses that are within the segment + * will make the radix tree return the stream ID for that ring. + * + * Caveats for the radix tree: + * + * The radix tree uses an unsigned long as a key pair. On 32-bit systems, an + * unsigned long will be 32-bits; on a 64-bit system an unsigned long will be + * 64-bits. Since we only request 32-bit DMA addresses, we can use that as the + * key on 32-bit or 64-bit systems (it would also be fine if we asked for 64-bit + * PCI DMA addresses on a 64-bit system). There might be a problem on 32-bit + * extended systems (where the DMA address can be bigger than 32-bits), + * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that. + */ +static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +{ + struct xhci_segment *seg; + unsigned long key; + int ret; + + if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + return 0; + + seg = ring->first_seg; + do { + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + /* Skip any segments that were already added. */ + if (radix_tree_lookup(ring->trb_address_map, key)) + continue; + + ret = radix_tree_maybe_preload(mem_flags); + if (ret) + return ret; + ret = radix_tree_insert(ring->trb_address_map, + key, ring); + radix_tree_preload_end(); + if (ret) + return ret; + seg = seg->next; + } while (seg != ring->first_seg); + + return 0; +} + +static void xhci_remove_stream_mapping(struct xhci_ring *ring) +{ + struct xhci_segment *seg; + unsigned long key; + + if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + return; + + seg = ring->first_seg; + do { + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + if (radix_tree_lookup(ring->trb_address_map, key)) + radix_tree_delete(ring->trb_address_map, key); + seg = seg->next; + } while (seg != ring->first_seg); +} + /* XXX: Do we need the hcd structure in all these functions? */ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring) { if (!ring)
[PATCH 04/68] xhci: For streams the css flag most be read from the stream-ctx on ep stop
Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-ring.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1e2f3f4..fb6df2c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -552,9 +552,9 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, struct xhci_dequeue_state *state) { struct xhci_virt_device *dev = xhci->devs[slot_id]; + struct xhci_virt_ep *ep = &dev->eps[ep_index]; struct xhci_ring *ep_ring; struct xhci_generic_trb *trb; - struct xhci_ep_ctx *ep_ctx; dma_addr_t addr; ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id, @@ -579,8 +579,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, /* Dig out the cycle state saved by the xHC during the stop ep cmd */ xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Finding endpoint context"); - ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); - state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); + /* 4.6.9 the css flag is written to the stream context for streams */ + if (ep->ep_state & EP_HAS_STREAMS) { + struct xhci_stream_ctx *ctx = + &ep->stream_info->stream_ctx_array[stream_id]; + state->new_cycle_state = 0x1 & le64_to_cpu(ctx->stream_ring); + } else { + struct xhci_ep_ctx *ep_ctx + = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); + state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); + } state->new_deq_ptr = cur_td->last_trb; xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, -- 1.8.4.2 -- 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 24/68] uas: Avoid unnecessary unlock / lock calls around unlink_data_urbs
All callers of unlink_data_urbs drop devinfo->lock before calling it, and then immediately take it again after the call. And the first thing unlink_data_urbs does is take the lock again, and the last thing it does is drop it. This commit removes all the unnecessary lock dropping and taking. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 20 +++- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index ec1b22d..dcaf6119 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -93,28 +93,26 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); +/* Must be called with devinfo->lock held, will temporary unlock the lock */ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, -struct uas_cmd_info *cmdinfo) +struct uas_cmd_info *cmdinfo, +unsigned long *lock_flags) { - unsigned long flags; - /* * The UNLINK_DATA_URBS flag makes sure uas_try_complete * (called by urb completion) doesn't release cmdinfo * underneath us. */ - spin_lock_irqsave(&devinfo->lock, flags); cmdinfo->state |= UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); + spin_unlock_irqrestore(&devinfo->lock, *lock_flags); if (cmdinfo->data_in_urb) usb_unlink_urb(cmdinfo->data_in_urb); if (cmdinfo->data_out_urb) usb_unlink_urb(cmdinfo->data_out_urb); - spin_lock_irqsave(&devinfo->lock, flags); + spin_lock_irqsave(&devinfo->lock, *lock_flags); cmdinfo->state &= ~UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_do_work(struct work_struct *work) @@ -361,9 +359,7 @@ static void uas_stat_cmplt(struct urb *urb) uas_sense(urb, cmnd); if (cmnd->result != 0) { /* cancel data transfers on error */ - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); } cmdinfo->state &= ~COMMAND_INFLIGHT; uas_try_complete(cmnd, __func__); @@ -787,9 +783,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); } else { - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); uas_try_complete(cmnd, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); ret = SUCCESS; -- 1.8.4.2 -- 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 32/68] uas: Use proper packet size when submitting reponse urbs
task management commands expect a response_iu rather then a sense_iu, and these have different sizes. Make the urb we submit to get the reply the right size. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 33f9dcd..83c2ef8 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -429,7 +429,8 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, return urb; } -static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, +static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, + int size, gfp_t gfp, struct Scsi_Host *shost, u16 stream_id) { struct usb_device *udev = devinfo->udev; @@ -439,11 +440,11 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, if (!urb) goto out; - iu = kzalloc(sizeof(*iu), gfp); + iu = kzalloc(size, gfp); if (!iu) goto free; - usb_fill_bulk_urb(urb, udev, devinfo->status_pipe, iu, sizeof(*iu), + usb_fill_bulk_urb(urb, udev, devinfo->status_pipe, iu, size, uas_stat_cmplt, shost); urb->stream_id = stream_id; urb->transfer_flags |= URB_FREE_BUFFER; @@ -548,13 +549,13 @@ err: * daft to me. */ -static int uas_submit_sense_urb(struct Scsi_Host *shost, +static int uas_submit_sense_urb(struct Scsi_Host *shost, int size, gfp_t gfp, unsigned int stream) { struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; struct urb *urb; - urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); + urb = uas_alloc_sense_urb(devinfo, size, gfp, shost, stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; usb_anchor_urb(urb, &devinfo->sense_urbs); @@ -576,7 +577,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { - err = uas_submit_sense_urb(cmnd->device->host, gfp, + err = uas_submit_sense_urb(cmnd->device->host, + sizeof(struct sense_iu), gfp, cmdinfo->stream); if (err) { return err; @@ -724,10 +726,13 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; u16 tag = devinfo->qdepth - 1; unsigned long flags; + int r; spin_lock_irqsave(&devinfo->lock, flags); memset(&devinfo->response, 0, sizeof(devinfo->response)); - if (uas_submit_sense_urb(shost, GFP_ATOMIC, tag)) { + r = uas_submit_sense_urb(shost, sizeof(struct response_iu), +GFP_ATOMIC, tag); + if (r != 0) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", __func__, fname); -- 1.8.4.2 -- 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 20/68] uas: add dead request list
From: Gerd Hoffmann This patch adds a new list where all requests which are canceled are added to, so we don't loose them. Then, after killing all inflight urbs on bus reset (and disconnect) we'll walk over the list and clean them up. Without this we can end up with aborted requests lingering around in case of status pipe transfer errors. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 50 +++ 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3cf5a5f..f049038 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -53,6 +53,7 @@ struct uas_dev_info { spinlock_t lock; struct work_struct work; struct list_head work_list; + struct list_head dead_list; }; enum { @@ -80,6 +81,7 @@ struct uas_cmd_info { struct urb *data_in_urb; struct urb *data_out_urb; struct list_head work; + struct list_head dead; }; /* I hate forward declarations, but I actually have a loop */ @@ -89,6 +91,7 @@ static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); +static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) @@ -150,16 +153,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that -* when a device reset is in -* progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); list_del(&cmdinfo->work); + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); } spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -176,6 +175,28 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) schedule_work(&devinfo->work); } +static void uas_zap_dead(struct uas_dev_info *devinfo) +{ + struct uas_cmd_info *cmdinfo; + struct uas_cmd_info *temp; + unsigned long flags; + + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, dead) { + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + /* all urbs are killed, clear inflight bits */ + cmdinfo->state &= ~(COMMAND_INFLIGHT | + DATA_IN_URB_INFLIGHT | + DATA_OUT_URB_INFLIGHT); + uas_try_complete(cmnd, __func__); + } + spin_unlock_irqrestore(&devinfo->lock, flags); +} + static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) { struct sense_iu *sense_iu = urb->transfer_buffer; @@ -263,6 +284,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) if (cmdinfo->state & COMMAND_ABORTED) { scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; + list_del(&cmdinfo->dead); } cmnd->scsi_done(cmnd); return 0; @@ -292,7 +314,13 @@ static void uas_stat_cmplt(struct urb *urb) u16 tag; if (urb->status) { - dev_err(&urb->dev->dev, "URB BAD STATUS %d\n", urb->status); + if (urb->status == -ENOENT) { + dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n", + urb->stream_id); + } else { + dev_err(&urb->dev->dev, "stat urb: status %d\n", + urb->status); + } usb_free_urb(urb); return; } @@ -743,7 +771,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); + WARN_ON(cmdinfo->state & COMMAND_ABORTED);
[PATCH 36/68] xhci: xhci_mem_cleanup: make sure cmd_ring_reserved_trbs really is 0
cmd_ring_reserved_trbs gets decremented by xhci_free_stream_info(), so set it to 0 after freeing all rings, otherwise it wraps around to a very large value when rings with streams are free-ed. Before this patch the wrap-around could be triggered when xhci_resume calls xhci_mem_cleanup if the controller resume fails. Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 2c589e5..c5fbb96 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1812,7 +1812,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) if (xhci->lpm_command) xhci_free_command(xhci, xhci->lpm_command); - xhci->cmd_ring_reserved_trbs = 0; if (xhci->cmd_ring) xhci_ring_free(xhci, xhci->cmd_ring); xhci->cmd_ring = NULL; @@ -1877,6 +1876,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } no_bw: + xhci->cmd_ring_reserved_trbs = 0; xhci->num_usb2_ports = 0; xhci->num_usb3_ports = 0; xhci->num_active_eps = 0; -- 1.8.4.2 -- 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 37/68] xhci: The trb_address_map radix tree expects 1KB segment memory aligment
If we align segment dma pool memory to 64 bytes, then a segment can be located at 0x1040 - 0x143f, and a segment from another ring at 0x1440 - 0x183f. The last trb in the first segment at 0x1430 will then translate to the same radix tree key as the first trb of the second segment, while they are in different rings! This patches fixes this by changing the alignment of the dma pool to be 1KB rather then 64 bytes. An alternative fix would be to reduce the shift used to calculate the radix tree keys, but that would (slighlty) grow the radix trees so I believe this is the better fix. Note this patch is mostly theoretical since in practice I've not seen the dma_pool actually return not 1KB aligned memory. Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-mem.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index c5fbb96..005aa7e 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -158,7 +158,7 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, * * The radix tree maps the upper portion of the TRB DMA address to a ring * segment that has the same upper portion of DMA addresses. For example, say I - * have segments of size 1KB, that are always 64-byte aligned. A segment may + * have segments of size 1KB, that are always 1KB aligned. A segment may * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the * key to the stream ID is 0x43244. I can use the DMA address of the TRB to * pass the radix tree a key to get the right stream ID: @@ -2375,11 +2375,12 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) /* * Initialize the ring segment pool. The ring must be a contiguous * structure comprised of TRBs. The TRBs must be 16 byte aligned, -* however, the command ring segment needs 64-byte aligned segments, -* so we pick the greater alignment need. +* however, the command ring segment needs 64-byte aligned segments +* and our use of dma addresses in the trb_address_map radix tree needs +* TRB_SEGMENT_SIZE alignment, so we pick the greater alignment need. */ xhci->segment_pool = dma_pool_create("xHCI ring segments", dev, - TRB_SEGMENT_SIZE, 64, xhci->page_size); + TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size); /* See Table 46 and Note on Figure 55 */ xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev, -- 1.8.4.2 -- 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 33/68] uas: Use all available stream ids
If we get ie 16 streams we can use stream-id 1-16, not 1-15. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 83c2ef8..18339e5 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -724,7 +724,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, { struct Scsi_Host *shost = cmnd->device->host; struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; - u16 tag = devinfo->qdepth - 1; + u16 tag = devinfo->qdepth; unsigned long flags; int r; @@ -848,7 +848,7 @@ static int uas_slave_configure(struct scsi_device *sdev) { struct uas_dev_info *devinfo = sdev->hostdata; scsi_set_tag_type(sdev, MSG_ORDERED_TAG); - scsi_activate_tcq(sdev, devinfo->qdepth - 3); + scsi_activate_tcq(sdev, devinfo->qdepth - 2); return 0; } @@ -1032,7 +1032,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_LIST_HEAD(&devinfo->dead_list); uas_configure_endpoints(devinfo); - result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); + result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2); if (result) goto free_streams; -- 1.8.4.2 -- 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 18/68] uas: properly reinitialize in uas_eh_bus_reset_handler
From: Gerd Hoffmann Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d966b59..fc08ee9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp); static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); +static void uas_configure_endpoints(struct uas_dev_info *devinfo); +static void uas_free_streams(struct uas_dev_info *devinfo); static DECLARE_WORK(uas_work, uas_do_work); static DEFINE_SPINLOCK(uas_work_lock); @@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_free_streams(devinfo); err = usb_reset_device(udev); + if (!err) + uas_configure_endpoints(devinfo); devinfo->resetting = 0; if (err) { -- 1.8.4.2 -- 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 41/68] uas: Add the posibilty to blacklist uas devices from using the uas driver
Once we start supporting uas hardware, and as more and more uas devices become available, we will likely start seeing broken devices. This patch prepares for the inevitable need for blacklisting those devices from using the uas driver (they will use usb-storage instead). Signed-off-by: Hans de Goede --- drivers/usb/storage/uas-detect.h | 11 + drivers/usb/storage/uas.c | 17 ++--- drivers/usb/storage/unusual_uas.h | 52 +++ include/linux/usb_usual.h | 6 +++-- 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 drivers/usb/storage/unusual_uas.h diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 28101c7..02bf5ec 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -38,3 +38,14 @@ static int uas_find_uas_alt_setting(struct usb_interface *intf) return -ENODEV; } + +static int uas_use_uas_driver(struct usb_interface *intf, + const struct usb_device_id *id) +{ + unsigned long flags = id->driver_info; + + if (flags & US_FL_IGNORE_UAS) + return 0; + + return uas_find_uas_alt_setting(intf) >= 0; +} diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 303181b..624ff5f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -871,7 +872,14 @@ static struct scsi_host_template uas_host_template = { .ordered_tag = 1, }; +#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ + vendorName, productName, useProtocol, useTransport, \ + initFunction, flags) \ +{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \ + .driver_info = (flags) } + static struct usb_device_id uas_usb_ids[] = { +# include "unusual_uas.h" { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) }, { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) }, /* 0xaa is a prototype device I happen to have access to */ @@ -880,6 +888,8 @@ static struct usb_device_id uas_usb_ids[] = { }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); +#undef UNUSUAL_DEV + static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { @@ -978,6 +988,9 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) struct uas_dev_info *devinfo; struct usb_device *udev = interface_to_usbdev(intf); + if (!uas_use_uas_driver(intf, id)) + return -ENODEV; + if (uas_switch_interface(udev, intf)) return -ENODEV; @@ -1088,10 +1101,6 @@ static void uas_disconnect(struct usb_interface *intf) kfree(devinfo); } -/* - * XXX: Should this plug into libusual so we can auto-upgrade devices from - * Bulk-Only to UAS? - */ static struct usb_driver uas_driver = { .name = "uas", .probe = uas_probe, diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h new file mode 100644 index 000..724 --- /dev/null +++ b/drivers/usb/storage/unusual_uas.h @@ -0,0 +1,52 @@ +/* Driver for USB Attached SCSI devices - Unusual Devices File + * + * (c) 2013 Hans de Goede + * + * Based on the same file for the usb-storage driver, which is: + * (c) 2000-2002 Matthew Dharm (mdharm-...@one-eyed-alien.net) + * (c) 2000 Adam J. Richter (a...@yggdrasil.com), Yggdrasil Computing, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +/* + * IMPORTANT NOTE: This file must be included in another file which defines + * a UNUSUAL_DEV macro before this file is included. + */ + +/* + * If you edit this file, please try to keep it sorted first by VendorID, + * then by ProductID. + * + * If you want to add an entry for this file, be sure to include the + * following information: + * - a patch that adds the entry for your device, including your + * email address right above the entry (plus maybe a brief + * explanation of the reason for the entry), + * - lsusb -v output for the device + * Send your submission to Hans de Goede + * and don't forget to CC: th
[PATCH 46/68] uas: Move uas_find_endpoints to uas-detect.h
No changes, just the move. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas-detect.h | 27 +++ drivers/usb/storage/uas.c| 27 --- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index bac33df..3529872 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -39,6 +39,33 @@ static int uas_find_uas_alt_setting(struct usb_interface *intf) return -ENODEV; } +static int uas_find_endpoints(struct usb_host_interface *alt, + struct usb_host_endpoint *eps[]) +{ + struct usb_host_endpoint *endpoint = alt->endpoint; + unsigned i, n_endpoints = alt->desc.bNumEndpoints; + + for (i = 0; i < n_endpoints; i++) { + unsigned char *extra = endpoint[i].extra; + int len = endpoint[i].extralen; + while (len >= 3) { + if (extra[1] == USB_DT_PIPE_USAGE) { + unsigned pipe_id = extra[2]; + if (pipe_id > 0 && pipe_id < 5) + eps[pipe_id - 1] = &endpoint[i]; + break; + } + len -= extra[0]; + extra += extra[0]; + } + } + + if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) + return -ENODEV; + + return 0; +} + static int uas_use_uas_driver(struct usb_interface *intf, const struct usb_device_id *id) { diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index b14eee2..49ae714 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -903,33 +903,6 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static int uas_find_endpoints(struct usb_host_interface *alt, - struct usb_host_endpoint *eps[]) -{ - struct usb_host_endpoint *endpoint = alt->endpoint; - unsigned i, n_endpoints = alt->desc.bNumEndpoints; - - for (i = 0; i < n_endpoints; i++) { - unsigned char *extra = endpoint[i].extra; - int len = endpoint[i].extralen; - while (len >= 3) { - if (extra[1] == USB_DT_PIPE_USAGE) { - unsigned pipe_id = extra[2]; - if (pipe_id > 0 && pipe_id < 5) - eps[pipe_id - 1] = &endpoint[i]; - break; - } - len -= extra[0]; - extra += extra[0]; - } - } - - if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) - return -ENODEV; - - return 0; -} - static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- 1.8.4.2 -- 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 30/68] uas: Fix response iu struct definition
The response iu struct before this patch has a size of 7 bytes (discounting padding), which is weird since all other iu-s are explictly padded to a multiple of 4 bytes. More over submitting a 7 byte bulk transfer to the status endpoint when expecting a response iu results in an USB babble error, as the device actually sends 8 bytes. Up on closer reading of the UAS spec: http://www.t10.org/cgi-bin/ac.pl?t=f&f=uas2r00.pdf The reason for this becomes clear, the 2 entries in "Table 17 — RESPONSE IU" are numbered 4 and 6, looking at other iu definitions in the spec, esp. multi-byte fields, this indicates that the ADDITIONAL RESPONSE INFORMATION field is not a 2 byte field as one might assume at a first look, but is a multi-byte field containing 3 bytes. This also aligns with the SCSI Architecture Model 4 spec, which UAS is based on which states in paragraph "7.1 Task management function procedure calls" that the "Additional Response Information" output argument for a Task management function procedure call is 3 bytes. Last but not least I've verified this by sending a logical unit reset task management call with an invalid lun to an actual uasp device, and received back a response-iu with byte 6 being 0, and byte 7 being 9, which is the responce code for an invalid iu, which confirms that the response code is being reported in byte 7 of the response iu rather then in byte 6. Things were working before despite this error in the response iu struct definition because the additional response info field is normally filled with zeros, and 0 is the response code value for success. Signed-off-by: Hans de Goede --- include/linux/usb/uas.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 1404178..772b66b 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -83,7 +83,7 @@ struct response_iu { __u8 iu_id; __u8 rsvd1; __be16 tag; - __be16 add_response_info; + __u8 add_response_info[3]; __u8 response_code; }; -- 1.8.4.2 -- 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 44/68] uas: Add uas_find_endpoints() helper function
This is a preparation patch for adding better descriptor validation. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 34 +++--- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 624ff5f..957ca8a 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -903,16 +903,11 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static void uas_configure_endpoints(struct uas_dev_info *devinfo) +static int uas_find_endpoints(struct usb_host_interface *alt, + struct usb_host_endpoint *eps[]) { - struct usb_host_endpoint *eps[4] = { }; - struct usb_interface *intf = devinfo->intf; - struct usb_device *udev = devinfo->udev; - struct usb_host_endpoint *endpoint = intf->cur_altsetting->endpoint; - unsigned i, n_endpoints = intf->cur_altsetting->desc.bNumEndpoints; - - devinfo->uas_sense_old = 0; - devinfo->cmnd = NULL; + struct usb_host_endpoint *endpoint = alt->endpoint; + unsigned i, n_endpoints = alt->desc.bNumEndpoints; for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; @@ -929,12 +924,29 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) } } + if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) + return -ENODEV; + + return 0; +} + +static void uas_configure_endpoints(struct uas_dev_info *devinfo) +{ + struct usb_host_endpoint *eps[4] = { }; + struct usb_device *udev = devinfo->udev; + int r; + + devinfo->uas_sense_old = 0; + devinfo->cmnd = NULL; + + r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); + /* -* Assume that if we didn't find a control pipe descriptor, we're +* Assume that if we didn't find a proper set of descriptors, we're * using a device with old firmware that happens to be set up like * this. */ - if (!eps[0]) { + if (r != 0) { devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1); devinfo->status_pipe = usb_rcvbulkpipe(udev, 1); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2); -- 1.8.4.2 -- 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 29/68] uas: s/response_ui/response_iu/
Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 2 +- include/linux/usb/uas.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 0ee5a05..33f9dcd 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -46,7 +46,7 @@ struct uas_dev_info { struct usb_anchor sense_urbs; struct usb_anchor data_urbs; int qdepth, resetting; - struct response_ui response; + struct response_iu response; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 5499ab5..1404178 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -79,7 +79,7 @@ struct sense_iu { __u8 sense[SCSI_SENSE_BUFFERSIZE]; }; -struct response_ui { +struct response_iu { __u8 iu_id; __u8 rsvd1; __be16 tag; -- 1.8.4.2 -- 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 43/68] usb-storage: Don't bind to uas devices if the uas driver is enabled
uas devices have 2 alternative settings on their usb-storage interface, one for usb-storage and one for uas. Using the uas driver is preferred, so if the uas driver is enabled, and the device has an uas alt setting, don't bind. Signed-off-by: Hans de Goede --- drivers/usb/storage/unusual_devs.h | 5 + drivers/usb/storage/usb.c | 10 ++ 2 files changed, 15 insertions(+) diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index de32cfa..028613e 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2072,6 +2072,11 @@ UNUSUAL_DEV( 0xed10, 0x7636, 0x0001, 0x0001, "Digital MP3 Audio Player", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* Unusual uas devices */ +#if IS_ENABLED(CONFIG_USB_UAS) +#include "unusual_uas.h" +#endif + /* Control/Bulk transport for all SubClass values */ USUAL_DEV(USB_SC_RBC, USB_PR_CB), USUAL_DEV(USB_SC_8020, USB_PR_CB), diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 5c4fe07..2055bc7 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -73,6 +73,10 @@ #include "sierra_ms.h" #include "option_ms.h" +#if IS_ENABLED(CONFIG_USB_UAS) +#include "uas-detect.h" +#endif + /* Some informational data */ MODULE_AUTHOR("Matthew Dharm "); MODULE_DESCRIPTION("USB Mass Storage driver for Linux"); @@ -1036,6 +1040,12 @@ static int storage_probe(struct usb_interface *intf, int result; int size; + /* If uas is enabled and this device can do uas then ignore it. */ +#if IS_ENABLED(CONFIG_USB_UAS) + if (uas_use_uas_driver(intf, id)) + return -ENXIO; +#endif + /* * If the device isn't standard (is handled by a subdriver * module) then don't accept it. -- 1.8.4.2 -- 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 28/68] uas: Fix reset handling for externally triggered reset
Handle usb-device resets not triggered from uas_eh_bus_reset_handler(), when this happens, disable cmd queuing during the reset, and wait for existing requests to finish in pre_reset. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 36 +++- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 36ef82a..0ee5a05 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -818,10 +819,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); uas_zap_dead(devinfo); - uas_free_streams(devinfo); err = usb_reset_device(udev); - if (!err) - uas_configure_endpoints(devinfo); devinfo->resetting = 0; usb_unlock_device(udev); @@ -1055,13 +1053,41 @@ set_alt0: static int uas_pre_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + /* Block new requests */ + spin_lock_irqsave(shost->host_lock, flags); + scsi_block_requests(shost); + spin_unlock_irqrestore(shost->host_lock, flags); + + /* Wait for any pending requests to complete */ + flush_work(&devinfo->work); + if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + return 1; + } + + uas_free_streams(devinfo); + return 0; } static int uas_post_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + uas_configure_endpoints(devinfo); + + spin_lock_irqsave(shost->host_lock, flags); + scsi_report_bus_reset(shost, 0); + spin_unlock_irqrestore(shost->host_lock, flags); + + scsi_unblock_requests(shost); + return 0; } -- 1.8.4.2 -- 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 35/68] uas: Move uas detect code to uas-detect.h
This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas-detect.h | 40 drivers/usb/storage/uas.c| 40 ++-- 2 files changed, 42 insertions(+), 38 deletions(-) create mode 100644 drivers/usb/storage/uas-detect.h diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h new file mode 100644 index 000..28101c7 --- /dev/null +++ b/drivers/usb/storage/uas-detect.h @@ -0,0 +1,40 @@ +#include +#include + +static int uas_is_interface(struct usb_host_interface *intf) +{ + return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE && + intf->desc.bInterfaceSubClass == USB_SC_SCSI && + intf->desc.bInterfaceProtocol == USB_PR_UAS); +} + +static int uas_isnt_supported(struct usb_device *udev) +{ + struct usb_hcd *hcd = bus_to_hcd(udev->bus); + + dev_warn(&udev->dev, "The driver for the USB controller %s does not " + "support scatter-gather which is\n", + hcd->driver->description); + dev_warn(&udev->dev, "required by the UAS driver. Please try an" + "alternative USB controller if you wish to use UAS.\n"); + return -ENODEV; +} + +static int uas_find_uas_alt_setting(struct usb_interface *intf) +{ + int i; + struct usb_device *udev = interface_to_usbdev(intf); + int sg_supported = udev->bus->sg_tablesize != 0; + + for (i = 0; i < intf->num_altsetting; i++) { + struct usb_host_interface *alt = &intf->altsetting[i]; + + if (uas_is_interface(alt)) { + if (!sg_supported) + return uas_isnt_supported(udev); + return alt->desc.bAlternateSetting; + } + } + + return -ENODEV; +} diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 4d70bbb..303181b 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -25,6 +25,8 @@ #include #include +#include "uas-detect.h" + /* * The r00-r01c specs define this version of the SENSE IU data structure. * It's still in use by several different firmware releases. @@ -878,44 +880,6 @@ static struct usb_device_id uas_usb_ids[] = { }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); -static int uas_is_interface(struct usb_host_interface *intf) -{ - return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE && - intf->desc.bInterfaceSubClass == USB_SC_SCSI && - intf->desc.bInterfaceProtocol == USB_PR_UAS); -} - -static int uas_isnt_supported(struct usb_device *udev) -{ - struct usb_hcd *hcd = bus_to_hcd(udev->bus); - - dev_warn(&udev->dev, "The driver for the USB controller %s does not " - "support scatter-gather which is\n", - hcd->driver->description); - dev_warn(&udev->dev, "required by the UAS driver. Please try an" - "alternative USB controller if you wish to use UAS.\n"); - return -ENODEV; -} - -static int uas_find_uas_alt_setting(struct usb_interface *intf) -{ - int i; - struct usb_device *udev = interface_to_usbdev(intf); - int sg_supported = udev->bus->sg_tablesize != 0; - - for (i = 0; i < intf->num_altsetting; i++) { - struct usb_host_interface *alt = &intf->altsetting[i]; - - if (uas_is_interface(alt)) { - if (!sg_supported) - return uas_isnt_supported(udev); - return alt->desc.bAlternateSetting; - } - } - - return -ENODEV; -} - static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { -- 1.8.4.2 -- 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 15/68] usbfs: Add support for bulk stream ids
This patch makes it possible to specify a bulk stream id when submitting an urb using the async usbfs API. It overloads the number_of_packets usbdevfs_urb field for this. This is not pretty, but given other constraints it is the best we can do. The reasoning leading to this goes as follows: 1) We want to support bulk streams in the usbfs API 2) We do not want to extend the usbdevfs_urb struct with a new member, as that would mean defining new ioctl numbers for all async API ioctls + adding compat versions for the old ones (times 2 for 32 bit support) 3) 1 + 2 means we need to re-use an existing field 4) number_of_packets is only used for isoc urbs, and streams are bulk only so it is the best (and only) candidate for re-using Note that: 1) This patch only uses number_of_packets as stream_id if the app has actually allocated streams on the ep, so that old apps which may have garbage in there (as it was unused until now in the bulk case), will not break 2) This patch does not add support for allocating / freeing bulk-streams, that is done in a follow up patch Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 4 include/uapi/linux/usbdevice_fs.h | 5 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index f810d29..d1c2258 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1193,6 +1193,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; + unsigned int stream_id = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1278,6 +1279,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; + if (ep->streams) + stream_id = uurb->stream_id; break; case USBDEVFS_URB_TYPE_INTERRUPT: @@ -1428,6 +1431,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, dr = NULL; as->urb->start_frame = uurb->start_frame; as->urb->number_of_packets = number_of_packets; + as->urb->stream_id = stream_id; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b..cbf122d 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -102,7 +102,10 @@ struct usbdevfs_urb { int buffer_length; int actual_length; int start_frame; - int number_of_packets; + union { + int number_of_packets; /* Only used for isoc urbs */ + unsigned int stream_id; /* Only used with bulk streams */ + }; int error_count; unsigned int signr; /* signal to be sent on completion, or 0 if none should be sent. */ -- 1.8.4.2 -- 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 23/68] uas: Properly set interface to altsetting 0 on probe failure
- Rename labels to properly reflect this - Don't skip free-ing the streams when scsi_init_shared_tag_map fails Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 059ce62..ec1b22d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -993,8 +993,8 @@ static void uas_free_streams(struct uas_dev_info *devinfo) */ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) { - int result; - struct Scsi_Host *shost; + int result = -ENOMEM; + struct Scsi_Host *shost = NULL; struct uas_dev_info *devinfo; struct usb_device *udev = interface_to_usbdev(intf); @@ -1003,12 +1003,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL); if (!devinfo) - return -ENOMEM; + goto set_alt0; - result = -ENOMEM; shost = scsi_host_alloc(&uas_host_template, sizeof(void *)); if (!shost) - goto free; + goto set_alt0; shost->max_cmd_len = 16 + 252; shost->max_id = 1; @@ -1030,11 +1029,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); if (result) - goto free; + goto free_streams; result = scsi_add_host(shost, &intf->dev); if (result) - goto deconfig_eps; + goto free_streams; shost->hostdata[0] = (unsigned long)devinfo; @@ -1042,9 +1041,10 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) usb_set_intfdata(intf, shost); return result; -deconfig_eps: +free_streams: uas_free_streams(devinfo); - free: +set_alt0: + usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); kfree(devinfo); if (shost) scsi_host_put(shost); -- 1.8.4.2 -- 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 16/68] usbfs: Add ep_to_host_endpoint helper function
Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index d1c2258..b7f732a 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -769,6 +769,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, return ret; } +static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, +unsigned char ep) +{ + if (ep & USB_ENDPOINT_DIR_MASK) + return dev->ep_in[ep & USB_ENDPOINT_NUMBER_MASK]; + else + return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1214,15 +1223,10 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (ret) return ret; } - if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0) { - is_in = 1; - ep = ps->dev->ep_in[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } else { - is_in = 0; - ep = ps->dev->ep_out[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } + ep = ep_to_host_endpoint(ps->dev, uurb->endpoint); if (!ep) return -ENOENT; + is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0; u = 0; switch(uurb->type) { -- 1.8.4.2 -- 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 34/68] uas: Add a uas_find_uas_alt_setting helper function
This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 18339e5..4d70bbb 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -897,10 +897,10 @@ static int uas_isnt_supported(struct usb_device *udev) return -ENODEV; } -static int uas_switch_interface(struct usb_device *udev, - struct usb_interface *intf) +static int uas_find_uas_alt_setting(struct usb_interface *intf) { int i; + struct usb_device *udev = interface_to_usbdev(intf); int sg_supported = udev->bus->sg_tablesize != 0; for (i = 0; i < intf->num_altsetting; i++) { @@ -909,15 +909,26 @@ static int uas_switch_interface(struct usb_device *udev, if (uas_is_interface(alt)) { if (!sg_supported) return uas_isnt_supported(udev); - return usb_set_interface(udev, - alt->desc.bInterfaceNumber, - alt->desc.bAlternateSetting); + return alt->desc.bAlternateSetting; } } return -ENODEV; } +static int uas_switch_interface(struct usb_device *udev, + struct usb_interface *intf) +{ + int alt; + + alt = uas_find_uas_alt_setting(intf); + if (alt < 0) + return alt; + + return usb_set_interface(udev, + intf->altsetting[0].desc.bInterfaceNumber, alt); +} + static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- 1.8.4.2 -- 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 17/68] usbfs: Add support for allocating / freeing streams
This allows userspace to use bulk-streams, just like in kernel drivers, see Documentation/usb/bulk-streams.txt for details on the in kernel API. This is exported pretty much one on one to userspace. To use streams an app must first make a USBDEVFS_ALLOC_STREAMS ioctl, on success this will return the number of streams available (which may be less then requested). If there are n streams the app can then submit usbdevfs_urb-s with their stream_id member set to 1-n to use a specific stream. IE if USBDEVFS_ALLOC_STREAMS returns 4 then stream_id 1-4 can be used. When the app is done using streams it should call USBDEVFS_FREE_STREAMS Note applications are advised to use libusb rather then using the usbdevfs api directly. The latest version of libusb has support for streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 118 ++ include/uapi/linux/usbdevice_fs.h | 7 +++ 2 files changed, 125 insertions(+) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index b7f732a..e653b49 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct dev_state *ps, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ifnum, ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 2 ^ 16 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 65536)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(ps->dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ifnum = findintfep(ps->dev, ep); + if (ifnum < 0) { + ret = ifnum; + goto error; + } + + if (i == 0) { + ret = checkintf(ps, ifnum); + if (ret < 0) + goto error; + intf = usb_ifnum_to_if(ps->dev, ifnum); + } else { + /* Verify all eps belong to the same interface */ + if (ifnum != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1993,6 +2066,45 @@ static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) return claimintf(ps, dc.interface); } +static int proc_alloc_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, &num_streams, &num_eps, + &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL); + kfree(eps); + return r; +} + +static int proc_free_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, NULL, &num_eps, &eps, &intf); + if (r) + ret
[PATCH 21/68] uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE()
From: Gerd Hoffmann Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index f049038..046eedf 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; list_del(&cmdinfo->work); @@ -169,7 +169,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); struct uas_dev_info *devinfo = cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); list_add_tail(&cmdinfo->work, &devinfo->work_list); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); @@ -187,7 +187,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + WARN_ON_ONCE(!(cmdinfo->state & COMMAND_ABORTED)); /* all urbs are killed, clear inflight bits */ cmdinfo->state &= ~(COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | @@ -271,13 +271,13 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | DATA_OUT_URB_INFLIGHT | UNLINK_DATA_URBS)) return -EBUSY; - BUG_ON(cmdinfo->state & COMMAND_COMPLETED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); cmdinfo->state |= COMMAND_COMPLETED; usb_free_urb(cmdinfo->data_in_urb); usb_free_urb(cmdinfo->data_out_urb); @@ -398,8 +398,9 @@ static void uas_data_cmplt(struct urb *urb) sdb = scsi_out(cmnd); cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT; } - BUG_ON(sdb == NULL); - if (urb->status) { + if (sdb == NULL) { + WARN_ON_ONCE(1); + } else if (urb->status) { /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -573,7 +574,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; int err; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { err = uas_submit_sense_urb(cmnd->device->host, gfp, cmdinfo->stream); @@ -771,7 +772,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; list_add_tail(&cmdinfo->dead, &devinfo->dead_list); if (cmdinfo->state & IS_IN_WORK_LIST) { -- 1.8.4.2 -- 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 26/68] uas: Fix uas not working when plugged into an ehci port
I thought it would be a good idea to also test uas with usb-2, and it turns out it was, as it did not work. The problem is that the uas driver was passing the bEndpointAddress' direction bit to usb_rcvbulkpipe, the xhci code seems to not care about this, but with the ehci code this causes usb_submit_urb failure. With this fixed the uas code works nicely with an uas device plugged into an ehci port. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 5eacb80..6ad5de9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -948,13 +948,13 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); } else { devinfo->cmd_pipe = usb_sndbulkpipe(udev, - eps[0]->desc.bEndpointAddress); +usb_endpoint_num(&eps[0]->desc)); devinfo->status_pipe = usb_rcvbulkpipe(udev, - eps[1]->desc.bEndpointAddress); +usb_endpoint_num(&eps[1]->desc)); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, - eps[2]->desc.bEndpointAddress); +usb_endpoint_num(&eps[2]->desc)); devinfo->data_out_pipe = usb_sndbulkpipe(udev, - eps[3]->desc.bEndpointAddress); +usb_endpoint_num(&eps[3]->desc)); } devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, -- 1.8.4.2 -- 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 22/68] uas: Urbs must be anchored before submitting them
Otherwise they may complete before they get anchored and thus never get unanchored (as the unanchoring is done by the usb core on completion). This commit also remove the usb_get_urb / usb_put_urb around cmd submission + anchoring, since if done in the proper order this is not necessary. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 046eedf..059ce62 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -531,10 +531,12 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, usb_free_urb, NULL); urb->transfer_flags |= URB_FREE_BUFFER; + usb_anchor_urb(urb, &devinfo->cmd_urbs); err = usb_submit_urb(urb, gfp); - if (err) + if (err) { + usb_unanchor_urb(urb); goto err; - usb_anchor_urb(urb, &devinfo->cmd_urbs); + } return 0; @@ -558,13 +560,14 @@ static int uas_submit_sense_urb(struct Scsi_Host *shost, urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; + usb_anchor_urb(urb, &devinfo->sense_urbs); if (usb_submit_urb(urb, gfp)) { + usb_unanchor_urb(urb); shost_printk(KERN_INFO, shost, "sense urb submission failure\n"); usb_free_urb(urb); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(urb, &devinfo->sense_urbs); return 0; } @@ -594,14 +597,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_IN_URB) { + usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_in_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_in_urb); scmd_printk(KERN_INFO, cmnd, "data in urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; cmdinfo->state |= DATA_IN_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_DATA_OUT_URB) { @@ -614,14 +618,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_OUT_URB) { + usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_out_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_out_urb); scmd_printk(KERN_INFO, cmnd, "data out urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; cmdinfo->state |= DATA_OUT_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_CMD_URB) { @@ -633,14 +638,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_CMD_URB) { - usb_get_urb(cmdinfo->cmd_urb); + usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); if (usb_submit_urb(cmdinfo->cmd_urb, gfp)) { + usb_unanchor_urb(cmdinfo->cmd_urb); scmd_printk(KERN_INFO, cmnd, "cmd urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); - usb_put_urb(cmdinfo->cmd_urb); cmdinfo->cmd_urb = NULL; cmdinfo->state &= ~SUBMIT_CMD_URB; cmdinfo->state |= COMMAND_INFLIGHT; -- 1.8.4.2 -- 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 39/68] usb: Clear host_endpoint->streams when implicitly freeing streams
If streams are still allocated on device-reset or set-interface then the hcd code implictly frees the streams. Clear host_endpoint->streams in this case so that if a driver later tries to re-allocate them it won't run afoul of the device already having streams check in usb_alloc_streams(). Note normally streams still being allocated at reset / set-intf would be a driver bug, but this can happen without it being a driver bug on reset-resume. Signed-off-by: Hans de Goede --- drivers/usb/core/hub.c | 5 - drivers/usb/core/message.c | 7 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 06cec63..d673d25 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5102,7 +5102,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) struct usb_hcd *hcd = bus_to_hcd(udev->bus); struct usb_device_descriptordescriptor = udev->descriptor; struct usb_host_bos *bos; - int i, ret = 0; + int i, j, ret = 0; int port1 = udev->portnum; if (udev->state == USB_STATE_NOTATTACHED || @@ -5228,6 +5228,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev) ret); goto re_enumerate; } + /* Resetting also frees any allocated streams */ + for (j = 0; j < intf->cur_altsetting->desc.bNumEndpoints; j++) + intf->cur_altsetting->endpoint[j].streams = 0; } done: diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index bb31597..08c12a0 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1294,8 +1294,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) struct usb_interface *iface; struct usb_host_interface *alt; struct usb_hcd *hcd = bus_to_hcd(dev->bus); - int ret; - int manual = 0; + int i, ret, manual = 0; unsigned int epaddr; unsigned int pipe; @@ -1330,6 +1329,10 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) mutex_unlock(hcd->bandwidth_mutex); return -ENOMEM; } + /* Changing alt-setting also frees any allocated streams */ + for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++) + iface->cur_altsetting->endpoint[i].streams = 0; + ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt); if (ret < 0) { dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n", -- 1.8.4.2 -- 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 42/68] uas: Add a usbcore.nouas kernel cmdline option
Note this is done through a usbcore module parameter as the option is needed by both the uas and usb-storage drivers. Signed-off-by: Hans de Goede --- drivers/usb/core/usb.c | 12 drivers/usb/storage/uas-detect.h | 3 +++ include/linux/usb.h | 1 + 3 files changed, 16 insertions(+) diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 4d11449..80e3604 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -48,6 +48,7 @@ const char *usbcore_name = "usbcore"; static bool nousb; /* Disable USB when built into kernel image */ +static bool nouas; /* Disable use of UAS on UAS capable devices */ #ifdef CONFIG_PM_RUNTIME static int usb_autosuspend_delay = 2; /* Default delay value, @@ -981,6 +982,17 @@ int usb_disabled(void) } EXPORT_SYMBOL_GPL(usb_disabled); +module_param(nouas, bool, 0644); + +/* + * for external read access to + */ +int usb_uas_disabled(void) +{ + return nouas; +} +EXPORT_SYMBOL_GPL(usb_uas_disabled); + /* * Notifications of device and interface registration */ diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 02bf5ec..bac33df 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -44,6 +44,9 @@ static int uas_use_uas_driver(struct usb_interface *intf, { unsigned long flags = id->driver_info; + if (usb_uas_disabled()) + return 0; + if (flags & US_FL_IGNORE_UAS) return 0; diff --git a/include/linux/usb.h b/include/linux/usb.h index a4c8406..60d5b36 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1166,6 +1166,7 @@ extern void usb_deregister_dev(struct usb_interface *intf, struct usb_class_driver *class_driver); extern int usb_disabled(void); +extern int usb_uas_disabled(void); /* --- */ -- 1.8.4.2 -- 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 27/68] uas: Fix reset locking
Fix the uas_eh_bus_reset_handler not properly taking the usbdev lock before calling usb_device_reset, the usb-core expects this lock to be taken when usb_device_reset is called. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ad5de9..36ef82a 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -804,6 +804,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct usb_device *udev = devinfo->udev; int err; + err = usb_lock_device_for_reset(udev, devinfo->intf); + if (err) { + shost_printk(KERN_ERR, sdev->host, +"%s FAILED to get lock err %d\n", __func__, err); + return FAILED; + } + shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; uas_abort_work(devinfo); @@ -817,6 +824,8 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) uas_configure_endpoints(devinfo); devinfo->resetting = 0; + usb_unlock_device(udev); + if (err) { shost_printk(KERN_INFO, sdev->host, "%s FAILED\n", __func__); return FAILED; -- 1.8.4.2 -- 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 31/68] uas: Pack iu struct definitions
The iu struct definitions are usb packet definitions, so no alignment should happen. Notice that assuming 32 bit alignment this does not make any difference at all. Signed-off-by: Hans de Goede --- include/linux/usb/uas.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 772b66b..3fc8e8b 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -9,7 +9,7 @@ struct iu { __u8 iu_id; __u8 rsvd1; __be16 tag; -}; +} __attribute__((__packed__)); enum { IU_ID_COMMAND = 0x01, @@ -52,7 +52,7 @@ struct command_iu { __u8 rsvd7; struct scsi_lun lun; __u8 cdb[16]; /* XXX: Overflow-checking tools may misunderstand */ -}; +} __attribute__((__packed__)); struct task_mgmt_iu { __u8 iu_id; @@ -62,7 +62,7 @@ struct task_mgmt_iu { __u8 rsvd2; __be16 task_tag; struct scsi_lun lun; -}; +} __attribute__((__packed__)); /* * Also used for the Read Ready and Write Ready IUs since they have the @@ -77,7 +77,7 @@ struct sense_iu { __u8 rsvd7[7]; __be16 len; __u8 sense[SCSI_SENSE_BUFFERSIZE]; -}; +} __attribute__((__packed__)); struct response_iu { __u8 iu_id; @@ -85,7 +85,7 @@ struct response_iu { __be16 tag; __u8 add_response_info[3]; __u8 response_code; -}; +} __attribute__((__packed__)); struct usb_pipe_usage_descriptor { __u8 bLength; -- 1.8.4.2 -- 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 40/68] usb: Reset USB-3 devices on USB-3 link bounce
On disconnect USB3 protocol ports transit from U0 to SS.Inactive to Rx.Detect, on a recoverable error, the port stays in SS.Inactive and we recover from it by doing a warm-reset (through usb_device_reset if we have a udev for the port). If this really is a disconnect we may end up trying the warm-reset anyways, since khubd may run before the SS.Inactive to Rx.Detect transition, or it may get skipped if the transition to Rx.Detect happens before khubd gets run. With a loose connector, or in the case which actually led me to debugging this bad ACPI firmware toggling Vbus off and on in quick succession, the port may transition from Rx.Detect to U0 again before khubd gets run. In this case the device state is unknown really, but khubd happily goes into the resuscitate an existing device path, and the device driver never gets notified about the device state being messed up. If the above scenario happens with a streams using device, as soon as an urb is submitted to an endpoint with streams, the following appears in dmesg: ERROR Transfer event for disabled endpoint or incorrect stream ring @36807420 0400 04078000 Notice how the TRB address is all zeros. I've seen this both on Intel Pantherpoint and Nec xhci hosts. Luckily we can detect the U0 to SS.Inactive to Rx.Detect to U0 all having happened before khubd runs case since the C_LINK_STATE bit gets set in the portchange bits on the U0 -> SS.Inactive change. This bit will also be set on suspend / resume, but then it gets cleared by port_hub_init before khubd runs. So if the C_LINK_STATE bit is set and a warm-reset is not needed, iow the port is not still in SS.Inactive, and the port still has a connection, then the device needs to be reset to put it back in a known state. I've verified that doing the device reset also fixes the transfer event with all zeros address issue. Signed-off-by: Hans de Goede --- drivers/usb/core/hub.c | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d673d25..fd38e3e 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4730,6 +4730,8 @@ static void hub_events(void) /* deal with port status changes */ for (i = 1; i <= hdev->maxchild; i++) { + struct usb_device *udev = hub->ports[i - 1]->child; + if (test_bit(i, hub->busy_bits)) continue; connect_change = test_bit(i, hub->change_bits); @@ -4828,8 +4830,6 @@ static void hub_events(void) */ if (hub_port_warm_reset_required(hub, portstatus)) { int status; - struct usb_device *udev = - hub->ports[i - 1]->child; dev_dbg(hub_dev, "warm reset port %d\n", i); if (!udev || !(portstatus & @@ -4845,6 +4845,24 @@ static void hub_events(void) usb_unlock_device(udev); connect_change = 0; } + /* +* On disconnect USB3 protocol ports transit from U0 to +* SS.Inactive to Rx.Detect. If this happens a warm- +* reset is not needed, but a (re)connect may happen +* before khubd runs and sees the disconnect, and the +* device may be an unknown state. +* +* If the port went through SS.Inactive without khubd +* seeing it the C_LINK_STATE change flag will be set, +* and we reset the dev to put it in a known state. +*/ + } else if (udev && hub_is_superspeed(hub->hdev) && + (portchange & USB_PORT_STAT_C_LINK_STATE) && + (portstatus & USB_PORT_STAT_CONNECTION)) { + usb_lock_device(udev); + usb_reset_device(udev); + usb_unlock_device(udev); + connect_change = 0; } if (connect_change) -- 1.8.4.2 -- 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 19/68] uas: make work list per-device
From: Gerd Hoffmann Simplifies locking, we'll protect the list with the device spin lock. Also plugs races which can happen when two devices operate on the global list. While being at it rename the list head from "list" to "work", preparing for the addition of a second list. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 106 +++--- 1 file changed, 44 insertions(+), 62 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fc08ee9..3cf5a5f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -51,6 +51,8 @@ struct uas_dev_info { unsigned uas_sense_old:1; struct scsi_cmnd *cmnd; spinlock_t lock; + struct work_struct work; + struct list_head work_list; }; enum { @@ -77,7 +79,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head list; + struct list_head work; }; /* I hate forward declarations, but I actually have a loop */ @@ -88,10 +90,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); -static DECLARE_WORK(uas_work, uas_do_work); -static DEFINE_SPINLOCK(uas_work_lock); -static LIST_HEAD(uas_work_list); - static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) { @@ -118,75 +116,66 @@ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, static void uas_do_work(struct work_struct *work) { + struct uas_dev_info *devinfo = + container_of(work, struct uas_dev_info, work); struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; int err; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - spin_lock_irqsave(&devinfo->lock, flags); + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); - if (!err) + if (!err) { cmdinfo->state &= ~IS_IN_WORK_LIST; - spin_unlock_irqrestore(&devinfo->lock, flags); - if (err) { - list_del(&cmdinfo->list); - spin_lock_irq(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - spin_unlock_irq(&uas_work_lock); - schedule_work(&uas_work); + list_del(&cmdinfo->work); + } else { + schedule_work(&devinfo->work); } } + spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_abort_work(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *di = (void *)cmnd->device->hostdata; - - if (di == devinfo) { - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that -* when a device reset is in -* progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); - } else { - /* not our uas device,
[PATCH 45/68] uas: Fix bounds check in uas_find_endpoints
The loop uses up to 3 bytes of the endpoint extra data. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 957ca8a..b14eee2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -912,7 +912,7 @@ static int uas_find_endpoints(struct usb_host_interface *alt, for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; int len = endpoint[i].extralen; - while (len > 1) { + while (len >= 3) { if (extra[1] == USB_DT_PIPE_USAGE) { unsigned pipe_id = extra[2]; if (pipe_id > 0 && pipe_id < 5) -- 1.8.4.2 -- 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 25/68] uas: uas_alloc_cmd_urb: drop unused stream_id parameter
The cmd endpoint never has streams, so the stream_id parameter is unused. Signed-off-by: Hans de Goede --- drivers/usb/storage/uas.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index dcaf6119..5eacb80 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -454,7 +454,7 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, } static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, - struct scsi_cmnd *cmnd, u16 stream_id) + struct scsi_cmnd *cmnd) { struct usb_device *udev = devinfo->udev; struct scsi_device *sdev = cmnd->device; @@ -626,8 +626,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & ALLOC_CMD_URB) { - cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd, -cmdinfo->stream); + cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd); if (!cmdinfo->cmd_urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~ALLOC_CMD_URB; -- 1.8.4.2 -- 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 38/68] xhci: Handle MaxPSASize == 0
Signed-off-by: Hans de Goede --- drivers/usb/host/xhci.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 00ba201..bb6c3b36 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3134,6 +3134,12 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev, xhci_dbg(xhci, "Driver wants %u stream IDs (including stream 0).\n", num_streams); + /* MaxPSASize value 0 (2 streams) means streams are not supported */ + if (HCC_MAX_PSA(xhci->hcc_params) < 4) { + xhci_dbg(xhci, "xHCI controller does not support streams.\n"); + return -ENOSYS; + } + config_cmd = xhci_alloc_command(xhci, true, true, mem_flags); if (!config_cmd) { xhci_dbg(xhci, "Could not allocate xHCI command structure.\n"); -- 1.8.4.2 -- 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