RE: PROBLEM: XHCI Host Controller on Intel Panther Point with CDC/ACM dead after massive NAK
Hello Peter, one short remark Application-specific or vendor-specific are often frowned upon in other contexts but if the protocol is documented publically then it is a great way to take advantage of all that USB offers, and it is explicitly supported by the specification. Use bDeviceClass or bInterfaceClass 0xff. Certainly we try to use only the CDC/ACM standard and not using any custom communication Best regards Andreas-- 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: new USB serial device: Garmin USB ANT stick
Greg KH gregkh@... writes: On Sat, Aug 03, 2013 at 09:32:00AM -0700, Scott Alfter wrote: I have a Garmin USB ANT stick that shows up in lsusb as the following: Bus 009 Device 002: ID 0fcf:1008 Dynastream Innovations, Inc. Mini stick Suunto It's a wireless serial device that talks to Garmin's GPS watches so you can transfer data into and out of the watch. I can get it working with the following command: modprobe usbserial vendor=0x0fcf product=0x100 dmesg spits out this information: [751657.368773] usbcore: registered new interface driver usbserial [751657.368794] usbcore: registered new interface driver usbserial_generic [751657.368803] usbserial: USB Serial support registered for generic [751657.368823] usbserial_generic 9-1:1.0: The generic usb-serial driver is only for testing and one-off prototypes. [751657.368825] usbserial_generic 9-1:1.0: Tell linux-usb@... to add your device to a proper driver. [751657.368826] usbserial_generic 9-1:1.0: generic converter detected [751657.370262] usb 9-1: generic converter now attached to ttyUSB0 The module won't load without the vendor and product parameters, which also means it won't auto-load. Is there more information you need to add this device to the kernel so it will auto-load? There is a new driver in the 3.11-rc3 kernel release for this device, called suunto. It will be in the final 3.11 release in a few weeks, so no need to do anything special, when you upgrade to that release, it will auto-load properly. Hope this helps, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majordomo@... More majordomo info at http://vger.kernel.org/majordomo-info.html In my case for a chrome web app on ubuntu 13.10/linux 3.11.0-15-generic it seems like the suunto module somehow takes over/interferes with the usb interface and its impossible to claim the interface via chrome.usb.claimInterface API that interfaces with libusb. To disable the module I added 'blacklist suunto' to the /etc/modprobe.d/blacklist.conf and rebooted. -- 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
[no subject]
Congratulation !! Confirm your 500,000,00 Euros. Contact claims office via : claimsoffic...@yeah.net -- 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
FunctionFS: .iInterface has random value
Using the test application ffs-test.c I can reproduce this error that seems to be a bug in FunctionFS. At the host, my device shows up sometimes with .Interface having a random value, in which case Language 0x0409 : Source/Sink would be displayed. But most of the times it's 0x00 which leads to no String being displayed. Has anyone else noticed this behaviour? -- 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: [RFCv2 00/10] xhci: re-work command queue management
From: Dan Williams Adding another list that will have its own set of bugs seems retrograde top me. What bugs? Please be specific. The problem to be addressed is not the allocation of commands, but that timeouts of one command eat the timeout periods of subsequent commands. I'm thinking a single-threaded workqueue better models what the hardware is doing. See my comments in patch 1, but that is orthogonal to how the command contexts are allocated. No software is bug free. The best way to get reasonably bug-free software is the KISS principle (Keep It Simple Stupid). You just seem to be adding a lot of additional complexity. Maybe it would look better if more of the code was factored out of the call sites. IIRC at the moment every call site has to: 1) find the trb address that will be used (massive layer break). 2) actually write the trb (through several layers of function call). 3) sort out any timeout (I didn't really look into this bit). 4) ring the bell. ISTM that the call site should just be a single function call. If you do that the implementation of the timeouts/completions is removed from the callers. 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: xhci and other woes
From: Mark Lord Which means that the controller is obeying the rules, and the software is wrong. .. In this case, the bug has been worked around (not perfectly), but we've had no customer reports that this is an issue. There is no user-visible impact as far as we know. So fixing this race condition is a really low priority for me, and the fix would have to very minimally touch the driver. I'm seeing the problem with the ASMedia controller (rev 0.96). Your fix is only active for rev 1.00 controllers. You are assuming that all the changes between the 0.96 and 1.00 versions of the specification are changes in behaviour between controllers that report the relevant versions numbers. Some of the changes will be new features, some will just be documenting the way things have always been. There are a gazillion reports out there (google) that using any XHCI controller other than the NEC chip (and some Intel chips) causes instability, in particular when using the AMD and VIA chips. Right now, Linux USB3 has a very bad reputation -- buggy and unstable. If there's a bug we/you know about, then let's get it fixed. It could help some of those anonymous reports. The reason I've been looking at the xhci driver is because we (as a company) tried to use it and found it didn't work reliably. Firstly with the intel panther point controller at USB2 speeds certain types of line noise would cause the controller to stop processing requests. (This is connected to the smsc95xx silicon, which is a USB hub with ethernet and I2C (or similar) - we use all of those external ports.) I've not really got anywhere near looking at those code paths, unfortunately even the systems that failed 'often' would only fail about once a day - and I rather failed to do anything that changed that. The corporate solution was to connect to one of the USB2 headers. The other thing I've been doing is an evaluation of the ASIX USB3 Ge silicon. My initial tests showed that it sort of worked, but there were some lost packets. With my patched kernel the lost packets disappeared. I suspect you've just reverted the patch that fixed them. Over the last few weeks I've spent a lot of man-hours investigating what is actually happening with the controller and device driver to see WHY it is misbehaving. AFAICT everyone else is just randomly reverting patches assuming that the old behaviour was better, rather than that the patch fixes something but shows up another bug that has always been present - and then doing a proper investigation to find out which circumstances cause the new failure. Reverting patches can be a tool in finding out what has had an effect, but it doesn't mean that reverting the patch is the actual fix. I've been writing hardware device drivers and comms protocol stacks (along with a few bare-board systems, and a bit of hardware design) for most of the last 30 years. I do know what I'm talking about. David
RE: PROBLEM: XHCI Host Controller on Intel Panther Point with CDC/ACM dead after massive NAK
From: Kasberger Andreas What is still puzzling me is the fact that the host controller stops any communication. That means there is really electrically no communication (bulk_out) from HC to device anymore. It seems that the host controller has shut down communication port to one particular device. unbind and bind host controller will solve the problem The complete stop is probably a bug in the error recovery code. But anyway I will try do my best to find out the root cause of mis-communication between between both sides. You mention device-side buffering and that the device at some point can't accept anything more from the host. With USB this means that you must ensure that the host will know when it must not send more. I thought sending NAK as response for each package is the correct way to tell the host not now but maybe later.Please try again. After the internal device queue is not completely full namyore the comunication is done in normal way. But after some time HC stops completely any communication. In real life it means a huge firmware update takes long time and so it could happens the internal device queue is full. But a broken firmware update is a bad thing IIRC the host controller should repeat any transfer that is responded to with a NAK. But I'm not sure how long it will do this for before signalling an error. The fact that you caused the ring to be expanded several times does rather indicate that you are not waiting for earlier transfers to finish before adding the subsequent ones. You should at least limit the number of in-flight transfers. 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: [RFT 1/2] xhci 1.0: Limit arbitrarily-aligned scatter gather.
From: Sarah Sharp xHCI 1.0 hosts have a set of requirements on how to align transfer buffers on the endpoint rings called TD fragment rules. When the ax88179_178a driver added support for scatter gather in 3.12, with commit 804fad45411b48233b48003e33a78f290d227c8 USBNET: ax88179_178a: enable tso if usb host supports sg dma, it broke the device under xHCI 1.0 hosts. Under certain network loads, the device would see an unexpected short packet from the host, which would cause the device to stop sending ethernet packets, even through USB packets would still be sent. Commit 35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst attempted to fix this. It was a quick hack to partially implement the TD fragment rules. However, it caused regressions in the usb-storage layer and userspace USB drivers using libusb. The patches to attempt to fix this are too far reaching into the USB core, and we really need to implement the TD fragment rules correctly in the xHCI driver, instead of continuing to wallpaper over the issues. Disable arbitrarily-aligned scatter-gather in the xHCI driver for 1.0 hosts. Only the ax88179_178a driver checks the no_sg_constraint flag, so don't set it for 1.0 hosts. This should not impact usb-storage or usbfs behavior, since they pass down max packet sized aligned sg-list entries (512 for USB 2.0 and 1024 for USB 3.0). I believe that this will cause the ax88179 driver to discard some receive packets on (at least) my panther point 1.00 controller. I certainly saw bursts of lost packets in some testing I did before the scatter-gather transfers were enabled, and before any of my patches. The problem is that the ax88179_178a driver submits receive URBs that cross 64k boundaries, and are not aligned (they start at an 0x40 boundary). Receive USB frames can contain multiple ethernet frames, by default they are 20kB (and sit in 24kB of memory). I'm not entirely convinced that this is acceptable long term behaviour. 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: xhci and other woes
Messing with the Realtek nic driver didn't work my pc crashed soon after. It looks like i've hit on a stable combination at the moment. It's looking like either... * Passing nomsi as a kernel parameter has worked somehow, when doing /proc/interrupts it looks like everything that used to be using msi is still using msi, except possibly the hdmi audio on the video card. * This patch: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/buildid=b399fe355b30d0102e7690c99e6f764ddfd32ec3 I'm currently running a bit of a frankenstien kernel because I have tried a bunch of stuff. Everything is working at the moment stabily for me, i'm only seeing this when doing transfers through the wireless VPN [ 839.482251] xhci_hcd :02:00.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [ 844.480798] handle_tx_event: 2158 callbacks suppressed * I set the TRBS per segment back to 64 and the sg list back to ~0u * readq/writeq reversion * LPM reversion * merged usb3-port-power-v4 from https://git.kernel.org/cgit/linux/kernel/git/djbw/usb.git/ * merged x86/build and x86/urgent from https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/ Regards, Will Trives -- 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/3] Phytec phyFLEX-i.MX6 : Added USB_HOST Support
On Tue, Feb 4, 2014 at 2:34 AM, Ashutosh singh ashutos...@phytec.in wrote:
+
+ reg_usb_h1_vbus: regulator@1 {
+ compatible = regulator-fixed;
+ regulator-name = usb_h1_vbus;
+ regulator-min-microvolt = 500;
+ regulator-max-microvolt = 500;
+ gpio = gpio1 0 0;
+ enable-active-low;
You should remove this 'enable-active-low' as this is not a valid property.
By default the gpio is active low according to
Documentation/devicetree/bindings/regulator/gpio-regulator.txt
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: xhci and other woes
From: renev...@internode.on.net Messing with the Realtek nic driver didn't work my pc crashed soon after. It looks like i've hit on a stable combination at the moment. It's looking like either... * Passing nomsi as a kernel parameter has worked somehow, when doing /proc/interrupts it looks like everything that used to be using msi is still using msi, except possibly the hdmi audio on the video card. I've not tried that, but I know that MSI interrupts can be a bag of worms. * This patch: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/buildid=b399fe355b30d0102e7690 c99e6f764ddfd32ec3 Ah yes gcc developers: when I say -fno-sse I want that to disable sse2, sse3, avx, avx2 and all the other extra instructions that might appear in the next version. In a local driver makefile I have: # With gcc 4.7 (and maybe some earlier ones) we have to explicitly # disable sse in order to stop the xmm registers being saved to stack # in every varargs function. # I think we have to explicitly disable all the newer instructions, so set # core2 (the first sane Intel 64bit cpu) and disable all its simd instructions. CFLAGS += -mno-mmx -mno-sse -mno-sse2 -mno-sse3 # The compiler on kapit doesn't support these option... XOPTS := -march=core2 -mtune=generic XFLAGS := $(shell $(CC) $(XOPTS) -E - /dev/null /dev/null 21 echo $(XOPTS)) CFLAGS += $(XFLAGS) I must have a look at the NetBSD kernel build options. i386 still targets 486 so is ok, but amd64 will need the instructions disabled. I'm currently running a bit of a frankenstien kernel because I have tried a bunch of stuff. Everything is working at the moment stabily for me, i'm only seeing this when doing transfers through the wireless VPN [ 839.482251] xhci_hcd :02:00.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [ 844.480798] handle_tx_event: 2158 callbacks suppressed If you are seeing that, you need to add the quirk. (Or make the code in xhci-ring.c assume the quirk is set.) Without the quirk the xhci code believes that the transfer filled the RX buffer, even though the controller indicated a short length. Given that one of the VHDL definitions used to build xhci controllers has this bug. It would make sense for this to be enabled without a quirk. The driver is already unconditionally doing the check. * I set the TRBS per segment back to 64 and the sg list back to ~0u 256 is probably a better value for TRBS per segment. It will save some ring expansion requests happening later. * readq/writeq reversion Definitely, that one wasn't tested on a range of hosts at all. * LPM reversion That didn't make any difference to the problems I'm seeing with the ax88179 card using the ASMedia controller. * merged usb3-port-power-v4 from https://git.kernel.org/cgit/linux/kernel/git/djbw/usb.git/ * merged x86/build and x86/urgent from https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/ I wonder if there is anything in there that has a real effect? 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: xhci and other woes
It couldn't have been nomsi because I forgot to preface it with pci= I also have the nic plugged in via a hub which I included in all my messing around lol. [ 96.210387] usb 9-4: Product: USB3.0 Hub [ 96.210391] usb 9-4: Manufacturer: GenesysLogic [ 96.213442] hub 9-4:1.0: USB hub found [ 96.214050] hub 9-4:1.0: 4 ports detected [ 96.426198] usb 6-4: new high-speed USB device number 2 using xhci_hcd [ 96.553004] usb 6-4: New USB device found, idVendor=05e3, idProduct=0610 [ 96.553011] usb 6-4: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 96.553016] usb 6-4: Product: USB2.0 Hub [ 96.553020] usb 6-4: Manufacturer: GenesysLogic [ 96.555184] hub 6-4:1.0: USB hub found [ 96.556005] hub 6-4:1.0: 4 ports detected [ 96.633247] usb 9-4.3: new SuperSpeed USB device number 4 using xhci_hcd [ 96.649945] usb 9-4.3: Parent hub missing LPM exit latency info. Power management will be impacted. [ 96.656079] usb 9-4.3: New USB device found, idVendor=0b95, idProduct=1790 [ 96.656086] usb 9-4.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 96.656091] usb 9-4.3: Product: AX88179 [ 96.656095] usb 9-4.3: Manufacturer: ASIX Elec. Corp. [ 96.656098] usb 9-4.3: SerialNumber: 00803F5D080C65 I'm just feeling like a bit of uptime for tonight it could have been a bios option I flipped. I'll peel things back a bit tomorrow. I got a feeling if I plug the nic into the Asmedia again i'm still going to get a bunch of kevent 4 spam. Regards, Will Trives On Wednesday 05 February 2014 13:26:20 David Laight wrote: From: renev...@internode.on.net Messing with the Realtek nic driver didn't work my pc crashed soon after. It looks like i've hit on a stable combination at the moment. It's looking like either... * Passing nomsi as a kernel parameter has worked somehow, when doing /proc/interrupts it looks like everything that used to be using msi is still using msi, except possibly the hdmi audio on the video card. I've not tried that, but I know that MSI interrupts can be a bag of worms. * This patch: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/bui ldid=b399fe355b30d0102e7690 c99e6f764ddfd32ec3 Ah yes gcc developers: when I say -fno-sse I want that to disable sse2, sse3, avx, avx2 and all the other extra instructions that might appear in the next version. In a local driver makefile I have: # With gcc 4.7 (and maybe some earlier ones) we have to explicitly # disable sse in order to stop the xmm registers being saved to stack # in every varargs function. # I think we have to explicitly disable all the newer instructions, so set # core2 (the first sane Intel 64bit cpu) and disable all its simd instructions. CFLAGS += -mno-mmx -mno-sse -mno-sse2 -mno-sse3 # The compiler on kapit doesn't support these option... XOPTS := -march=core2 -mtune=generic XFLAGS := $(shell $(CC) $(XOPTS) -E - /dev/null /dev/null 21 echo $(XOPTS)) CFLAGS += $(XFLAGS) I must have a look at the NetBSD kernel build options. i386 still targets 486 so is ok, but amd64 will need the instructions disabled. I'm currently running a bit of a frankenstien kernel because I have tried a bunch of stuff. Everything is working at the moment stabily for me, i'm only seeing this when doing transfers through the wireless VPN [ 839.482251] xhci_hcd :02:00.0: WARN Successful completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk? [ 844.480798] handle_tx_event: 2158 callbacks suppressed If you are seeing that, you need to add the quirk. (Or make the code in xhci-ring.c assume the quirk is set.) Without the quirk the xhci code believes that the transfer filled the RX buffer, even though the controller indicated a short length. Given that one of the VHDL definitions used to build xhci controllers has this bug. It would make sense for this to be enabled without a quirk. The driver is already unconditionally doing the check. * I set the TRBS per segment back to 64 and the sg list back to ~0u 256 is probably a better value for TRBS per segment. It will save some ring expansion requests happening later. * readq/writeq reversion Definitely, that one wasn't tested on a range of hosts at all. * LPM reversion That didn't make any difference to the problems I'm seeing with the ax88179 card using the ASMedia controller. * merged usb3-port-power-v4 from https://git.kernel.org/cgit/linux/kernel/git/djbw/usb.git/ * merged x86/build and x86/urgent from https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/ I wonder if there is anything in there that has a real effect? 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: xhci and other woes
From: renev...@internode.on. I got a feeling if I plug the nic into the Asmedia again i'm still going to get a bunch of kevent 4 spam. Did you see my message about those yesterday? Try adding a printk() to the usbnet_write_cmd_async() code paths called from ax88179's set_multicast code. I found that slowing that down so that each request completed before the next one was requested helped. I've got some other work to do today... 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
[PATCH] usb: musb: correct use of schedule_delayed_work()
schedule_delayed_work() takes the delay in jiffies, not msecs.
This bug slipped in with the recent reset logic cleanup
(8ed1fb790ea: usb: musb: finish suspend/reset work independently from
musb_hub_control()).
Signed-off-by: Daniel Mack dan...@zonque.org
---
drivers/usb/musb/musb_core.c| 3 ++-
drivers/usb/musb/musb_virthub.c | 12 +++-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c147d66..d55350e 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -481,7 +481,8 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
musb-rh_timer = jiffies
+ msecs_to_jiffies(20);
schedule_delayed_work(
- musb-finish_resume_work, 20);
+ musb-finish_resume_work,
+ msecs_to_jiffies(20));
musb-xceiv-state = OTG_STATE_A_HOST;
musb-is_active = 1;
diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
index 64a4ee7..5780254 100644
--- a/drivers/usb/musb/musb_virthub.c
+++ b/drivers/usb/musb/musb_virthub.c
@@ -136,7 +136,8 @@ void musb_port_suspend(struct musb *musb, bool do_suspend)
/* later, GetPortStatus will stop RESUME signaling */
musb-port1_status |= MUSB_PORT_STAT_RESUME;
- schedule_delayed_work(musb-finish_resume_work, 20);
+ schedule_delayed_work(musb-finish_resume_work,
+ msecs_to_jiffies(20));
}
}
@@ -172,8 +173,7 @@ void musb_port_reset(struct musb *musb, bool do_reset)
if (musb-rh_timer 0 remain 0) {
/* take into account the minimum delay after
resume */
schedule_delayed_work(
- musb-deassert_reset_work,
- jiffies_to_msecs(remain));
+ musb-deassert_reset_work, remain);
return;
}
@@ -181,7 +181,8 @@ void musb_port_reset(struct musb *musb, bool do_reset)
power ~MUSB_POWER_RESUME);
/* Give the core 1 ms to clear MUSB_POWER_RESUME */
- schedule_delayed_work(musb-deassert_reset_work, 1);
+ schedule_delayed_work(musb-deassert_reset_work,
+ msecs_to_jiffies(1));
return;
}
@@ -191,7 +192,8 @@ void musb_port_reset(struct musb *musb, bool do_reset)
musb-port1_status |= USB_PORT_STAT_RESET;
musb-port1_status = ~USB_PORT_STAT_ENABLE;
- schedule_delayed_work(musb-deassert_reset_work, 50);
+ schedule_delayed_work(musb-deassert_reset_work,
+ msecs_to_jiffies(50));
} else {
dev_dbg(musb-controller, root port reset stopped\n);
musb_writeb(mbase, MUSB_POWER,
--
1.8.5.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
Re: [RFCv2 01/10] xhci: Use command structures when calling xhci_configure_endpoint
On 02/05/2014 04:21 AM, Dan Williams wrote: Hi Mathias, comments below: s/xhci_check_bandwith/xhci_check_bandwidth/ s/strucure/structure/ s/strucure/structure/ s/requre/require/ s/strucure/structure/ Thanks I guess I need to start using a spell checker for commit messages. One cleanup we may want to consider in this series is making xhci_alloc_command() more readable. My brain hurts when I see false, false as I wonder what that means. I took a look and of the 4 possible ways to call xhci_alloc_command, we only use 2: $ git grep xhci_alloc_command\(.*\) | grep -o xhci_alloc_command\(xhci,.*,.*, | sort -u xhci_alloc_command(xhci, false, true, xhci_alloc_command(xhci, true, true, So a first take is to just have a xhci_alloc_command() for true, true and a xhci_alloc_command_no_ctx() for false, true. ...uh oh, this series adds a usage of: xhci_alloc_command(xhci, false, false, ...any reason we can't just use something like xhci_alloc_command_no_ctx() instead? Actually just make xhci_alloc_command() take an option in_ctx parameter, when it is NULL xhci_alloc_command will allocate one, otherwise it will use the passed in one. This would make the code more readable. The same thing needs to be done for the completion parameter as well then. Do you think this change would fit in this patch series, or maybe as a separate fix? xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, Queueing configure endpoint command); xhci_queue_configure_endpoint(xhci, xhci-devs[slot_id]-in_ctx-dma, slot_id, false); xhci_ring_cmd_db(xhci); + kfree(command); It's not really acceptable to add dead code in a patch. Consider the case where some of the patches are reverted due to a regression. If, for example we revert patch 2, the unused infrastructure in patch1 does not get deleted. Patch size minimization is good, but not when it separates new infrastructure from its first user. This was a tradeoff I wasn't sure how to do. The first six patches make sure there exists a command structure every time a command is submitted. I added the kfrees because I didn't want to leak memory up to the patch where the command can be freed in its right place (patch 9). Actually, now looking at it, the command is still not properly freed between patches 7 and 9. Any suggestions? squashing first most of the commits together, or just ignoring that memory is leaked mid-series? -bandwidth_change: - xhci_dbg_trace(xhci, trace_xhci_dbg_context_change, - Completed config ep cmd); - virt_dev-cmd_status = cmd_comp_code; - complete(virt_dev-cmd_completion); return; This change has no description in the change log. What's the reason for deleting the goto? Previously xhci_configure_endpoint() could also be called without a command parameter. In this case the completion was _not_ added to device's own command wait list. xhci_configure_endpoint() would wait for completion on xhci-devs[udev-slot_id]-cmd_completion, and the code after the bandwith_change goto was run. Now this patch forces all xhci_configure_endpoint() callers to have a command structure parameter, and now in all cases we're waiting for a configure endpoint completion, the completion is added to the device's own command wait list. These completions are called in the beginning of handle_cmd_completion_ep by handle_cmd_in_cmd_wait_list(). I probably should add some description about this in the changelog as well. Given that we are waiting for the command to finish within xhci_configure_endpoint() shouldn't we free the completion in xhci_configure_endpoint as well? In other words in what cases do we need an xhci_command to have a longer lifetime than the scope of the execution routine (xhci_stop_device, xhci_configure_endpoint, xhci_discover_or_reset_device, xhci_alloc_dev, xhci_setup_device). Many of the functions that call xhci_configure_endpoint() handle their command strucure and completion allocation/freeing in their own little way. I didn't want to mess with these. For example xhci_free_streams() uses some pre-allocated command strucure command = vdev-eps[ep_index].stream_info-free_streams_command; while xhci_update_hub_device() allocates a new command with completion before calling xhci_configure_endpoint(), and frees them both afterwards Taking things a step further it seems that all the locations where we asynchronously queue commands are in the completion handlers for other commands. I'm wondering if this would be cleaner if we simply queued all command submissions and completion events to a single threaded workqueue. I'll go through the rest of the series to see if that impression makes sense, but something to consider... Handling the command completions in a workqueue could make sense, then all the async-queued
Re: new USB serial device: Garmin USB ANT stick
On Wed, Feb 05, 2014 at 08:39:33AM +, Henning Knut Skoglund wrote: Greg KH gregkh@... writes: On Sat, Aug 03, 2013 at 09:32:00AM -0700, Scott Alfter wrote: I have a Garmin USB ANT stick that shows up in lsusb as the following: Bus 009 Device 002: ID 0fcf:1008 Dynastream Innovations, Inc. Mini stick Suunto It's a wireless serial device that talks to Garmin's GPS watches so you can transfer data into and out of the watch. I can get it working with the following command: modprobe usbserial vendor=0x0fcf product=0x100 dmesg spits out this information: [751657.368773] usbcore: registered new interface driver usbserial [751657.368794] usbcore: registered new interface driver usbserial_generic [751657.368803] usbserial: USB Serial support registered for generic [751657.368823] usbserial_generic 9-1:1.0: The generic usb-serial driver is only for testing and one-off prototypes. [751657.368825] usbserial_generic 9-1:1.0: Tell linux-usb@... to add your device to a proper driver. [751657.368826] usbserial_generic 9-1:1.0: generic converter detected [751657.370262] usb 9-1: generic converter now attached to ttyUSB0 The module won't load without the vendor and product parameters, which also means it won't auto-load. Is there more information you need to add this device to the kernel so it will auto-load? There is a new driver in the 3.11-rc3 kernel release for this device, called suunto. It will be in the final 3.11 release in a few weeks, so no need to do anything special, when you upgrade to that release, it will auto-load properly. Hope this helps, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majordomo@... More majordomo info at http://vger.kernel.org/majordomo-info.html In my case for a chrome web app on ubuntu 13.10/linux 3.11.0-15-generic it seems like the suunto module somehow takes over/interferes with the usb interface and its impossible to claim the interface via chrome.usb.claimInterface API that interfaces with libusb. To disable the module I added 'blacklist suunto' to the /etc/modprobe.d/blacklist.conf and rebooted. Is there not a chrome.usb. function that allows you to disconnect the device from the driver the operating system attached to it? If not, I suggest submitting a bug/patch to chrome, as there's nothing we can do here about it, sorry. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: XHCI Host Controller on Intel Panther Point with CDC/ACM dead after massive NAK
Kasberger Andreas andreaskasber...@hotmail.com writes: On the protocol design: First, using CDC-ACM means sacrificing all structured communication offered by the USB packet bus and settling for such primitive use of USB is not a decision that should be made lightly. Almost all applications can benefit quite significantly both in end-user usability and in ease of implementation from an application-specific protocol which takes advantage of what USB offers. Yes you are absolutely right. No the best idea. The usage for this protocol is to make firmware updates. Maybe consider DFU instead? 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 1/6] i2c: bcm-kona: register with subsys_initcall
On Wed, Feb 05, 2014 at 10:08:18AM +0100, Wolfram Sang wrote: On Tue, Feb 04, 2014 at 07:19:07AM -0500, Matt Porter wrote: Voltage regulators are needed very early due to deferred probe being incompatible with built-in USB gadget drivers. What does it need to fix those instead? [added Alan/Felipe for more insight] Discussion on that topic came about from this submission: http://www.spinics.net/lists/linux-usb/msg94217.html End of it is: http://www.spinics.net/lists/linux-usb/msg94731.html We can either add to the many drivers that already do subsys_initcall() for similar reasons...or I can drop this from the series and add gadget probe ordering to my TODO list. In short, it can't be a late_initcall() hack like the original post and really could be solved by converting to a real bus (and letting deferred probe do its job)..but Alan voiced concerns about that. -Matt -- 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/6] i2c: bcm-kona: register with subsys_initcall
On Wed, 5 Feb 2014, Matt Porter wrote: On Wed, Feb 05, 2014 at 10:08:18AM +0100, Wolfram Sang wrote: On Tue, Feb 04, 2014 at 07:19:07AM -0500, Matt Porter wrote: Voltage regulators are needed very early due to deferred probe being incompatible with built-in USB gadget drivers. What does it need to fix those instead? [added Alan/Felipe for more insight] Discussion on that topic came about from this submission: http://www.spinics.net/lists/linux-usb/msg94217.html End of it is: http://www.spinics.net/lists/linux-usb/msg94731.html We can either add to the many drivers that already do subsys_initcall() for similar reasons...or I can drop this from the series and add gadget probe ordering to my TODO list. In short, it can't be a late_initcall() hack like the original post and really could be solved by converting to a real bus (and letting deferred probe do its job)..but Alan voiced concerns about that. Don't worry too much about what I said. If adding a gadget bus will solve the problem in an appropriate way, and if nobody else objects (particularly Felipe, who is on vacation now), then go for it. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
Hi Olof,
Thank you for your review.
From: Olof Johansson [mailto:o...@lixom.net]
Sent: Wednesday, January 29, 2014 9:55 PM
Hi,
On Wed, Jan 29, 2014 at 9:29 AM, Kamil Debski k.deb...@samsung.com
wrote:
Change the phy provider used from the old one using the USB phy
framework to a new one using the Generic phy framework.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
.../devicetree/bindings/usb/exynos-usb.txt | 13 +++
drivers/usb/host/ehci-exynos.c | 97
+---
2 files changed, 76 insertions(+), 34 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..25e199a 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,10 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are EHCI phys, they should be listed
here.
+One phy per port. Each port should have its reg entry with a
+consecutive number. Also it should contain phys and phy-names
entries
+specifying the phy used by the port.
Optional properties:
- samsung,vbus-gpio: if present, specifies the GPIO that @@ -27,6
+31,15 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ phy-names = host;
+ status = disabled;
+ };
};
OHCI
[...]
@@ -102,14 +132,26 @@ static int exynos_ehci_probe(struct
platform_device *pdev)
samsung,exynos5440-ehci))
goto skip_phy;
- phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(phy)) {
- usb_put_hcd(hcd);
- dev_warn(pdev-dev, no platform data or transceiver
defined\n);
- return -EPROBE_DEFER;
- } else {
- exynos_ehci-phy = phy;
- exynos_ehci-otg = phy-otg;
+ for_each_available_child_of_node(pdev-dev.of_node, child) {
+ err = of_property_read_u32(child, reg,
phy_number);
+ if (err) {
+ dev_err(pdev-dev, Failed to parse device
tree\n);
+ of_node_put(child);
+ return err;
+ }
+ if (phy_number = PHY_NUMBER) {
+ dev_err(pdev-dev, Failed to parse device
tree - number out of range\n);
+ of_node_put(child);
+ return -EINVAL;
+ }
+ phy = devm_of_phy_get(pdev-dev, child, 0);
+ of_node_put(child);
+ if (IS_ERR(phy)) {
+ dev_err(pdev-dev, Failed to get phy number
%d,
+
phy_number);
+ return PTR_ERR(phy);
+ }
+ exynos_ehci-phy[phy_number] = phy;
this looks like it is now breaking older device trees, where ports
might not be described. Since device tree interfaces need to be
backwards compatible, you still need to handle the old case of not
having ports described.
There are two ways of doing this:
1. Fall back to the old behavior if there are no ports 2. Use a new
compatible value for the new model with port subnodes, and if the old
compatible value is used, then fall back to the old behavior.
I'm guessing (1) might be easiest since you can check for the presence
of #address-cells to tell if this is just an old style node, or if it's
a new-style node without any ports below it.
The ultimate goal is to remove the old phy driver. Unfortunately
this has to be synced with the new USB3 phy driver by Vivek Gautam. I think
he
is also close to completion. What about this case? In the end the old driver
will be removed and no longer be supported. Having backward compatibility in
mind, it is possible to have the old and the new phy driver together in one
kernel release. But do we want to have two drivers doing the same thing at
the same time?
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
--
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 v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
Hi Alan,
Thank you for your review.
From: Alan Stern [mailto:st...@rowland.harvard.edu]
Sent: Wednesday, January 29, 2014 9:43 PM
On Wed, 29 Jan 2014, Kamil Debski wrote:
Change the phy provider used from the old one using the USB phy
framework to a new one using the Generic phy framework.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
.../devicetree/bindings/usb/exynos-usb.txt | 13 +++
drivers/usb/host/ehci-exynos.c | 97
+---
2 files changed, 76 insertions(+), 34 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..25e199a 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,10 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are EHCI phys, they should be listed
here.
+One phy per port. Each port should have its reg entry with a
+consecutive number. Also it should contain phys and phy-names
entries
+specifying the phy used by the port.
What is the reg entry number used for? As far as I can see, it isn't
used for anything. In which case, why have it at all?
Tomasz Figa already commented this. I agree with him, that this should
be better described in the documentation.
@@ -42,10 +42,10 @@
static const char hcd_name[] = ehci-exynos; static struct
hc_driver __read_mostly exynos_ehci_hc_driver;
+#define PHY_NUMBER 3
struct exynos_ehci_hcd {
struct clk *clk;
- struct usb_phy *phy;
- struct usb_otg *otg;
You have removed all the OTG stuff from the driver. This wasn't
mentioned in the patch description, and it has no connection with the
PHY work.
Maybe I'll explain more about what are we trying to achieve. The goal
is to replace the old phy driver with the new one. In the old driver it was
difficult to add support to new SoC. It also had issues with having device
and
host working together.
You're right that until the old phy driver is removed support for the it
should
remain. To be able to remove the old driver both new USB2 and new USB3 phy
drivers
have to be ready. The USB3 driver is written by Vivek Gautam and as I see
it, he
is also close to completion.
Regarding the otg part. The old phy driver is the only provider of the otg
structure. It sets the host field of the structure. It is then used by
samsung_usb2phy_init (drivers/usb/phy/phy-samsung-usb2.c) to check which
driver is requesting the phy (is it host or device). In the new driver this
is determined by the entry in device tree. So no need to check the otg
struct
and strstr (!) to check if dev_name is ehci, ohci or other, like the old
driver.
+ struct phy *phy[PHY_NUMBER];
};
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd
*)(hcd_to_ehci(hcd)-priv) @@ -69,13 +69,43 @@ static void
exynos_setup_vbus_gpio(struct platform_device *pdev)
dev_err(dev, can't request ehci vbus gpio %d, gpio); }
+static int exynos_phys_on(struct phy *p[]) {
+ int i;
+ int ret = 0;
+
+ for (i = 0; ret == 0 i PHY_NUMBER; i++)
+ if (p[i])
+ ret = phy_power_on(p[i]);
+ if (ret)
+ for (i--; i 0; i--)
+ if (p[i])
+ phy_power_off(p[i]);
This loop runs while i 0. Therefore you will never turn off the
power to p[0].
Ups, my bad. Thank you for spotting this.
@@ -102,14 +132,26 @@ static int exynos_ehci_probe(struct
platform_device *pdev)
samsung,exynos5440-ehci))
goto skip_phy;
- phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(phy)) {
- usb_put_hcd(hcd);
You omitted this line from the new error returns below.
I see.
- dev_warn(pdev-dev, no platform data or transceiver
defined\n);
- return -EPROBE_DEFER;
- } else {
- exynos_ehci-phy = phy;
- exynos_ehci-otg = phy-otg;
+ for_each_available_child_of_node(pdev-dev.of_node, child) {
+ err = of_property_read_u32(child, reg, phy_number);
+ if (err) {
+ dev_err(pdev-dev, Failed to parse device
tree\n);
+ of_node_put(child);
+ return err;
Here, for example. Wouldn't it be better to goto fail_clk?
Right, I will fix this. Thank you.
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
--
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 v6 3/8] dts: Add usb2phy to Exynos 4
Hi Olof, From: Olof Johansson [mailto:o...@lixom.net] Sent: Wednesday, January 29, 2014 9:51 PM On Wed, Jan 29, 2014 at 9:29 AM, Kamil Debski k.deb...@samsung.com wrote: Add support to PHY of USB2 of the Exynos 4 SoC. Signed-off-by: Kamil Debski k.deb...@samsung.com --- .../devicetree/bindings/arm/samsung/pmu.txt|2 ++ arch/arm/boot/dts/exynos4.dtsi | 31 arch/arm/boot/dts/exynos4210.dtsi | 17 +++ arch/arm/boot/dts/exynos4x12.dtsi | 17 +++ 4 files changed, 67 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.txt b/Documentation/devicetree/bindings/arm/samsung/pmu.txt index 307e727..a76f91d 100644 --- a/Documentation/devicetree/bindings/arm/samsung/pmu.txt +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.txt @@ -3,6 +3,8 @@ SAMSUNG Exynos SoC series PMU Registers Properties: - name : should be 'syscon'; - compatible : should contain two values. First value must be one from following list: + - samsung,exynos4210-pmu - for Exynos4210 SoC, + - samsung,exynos4x12-pmu - for Exynos4212 SoC, - samsung,exynos5250-pmu - for Exynos5250 SoC, - samsung,exynos5420-pmu - for Exynos5420 SoC. second value must be always syscon. This and other PMU related bindings/dts changes should probably go in separate patch(es) instead of being snuck in with USB changes. O, I will move this to a separate patch. Best wishes, -- Kamil Debski Samsung RD Institute Poland -- 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/6] i2c: bcm-kona: register with subsys_initcall
On Wed, Feb 05, 2014 at 10:30:29AM -0500, Alan Stern wrote: On Wed, 5 Feb 2014, Matt Porter wrote: On Wed, Feb 05, 2014 at 10:08:18AM +0100, Wolfram Sang wrote: On Tue, Feb 04, 2014 at 07:19:07AM -0500, Matt Porter wrote: Voltage regulators are needed very early due to deferred probe being incompatible with built-in USB gadget drivers. What does it need to fix those instead? [added Alan/Felipe for more insight] Discussion on that topic came about from this submission: http://www.spinics.net/lists/linux-usb/msg94217.html End of it is: http://www.spinics.net/lists/linux-usb/msg94731.html We can either add to the many drivers that already do subsys_initcall() for similar reasons...or I can drop this from the series and add gadget probe ordering to my TODO list. In short, it can't be a late_initcall() hack like the original post and really could be solved by converting to a real bus (and letting deferred probe do its job)..but Alan voiced concerns about that. Don't worry too much about what I said. If adding a gadget bus will solve the problem in an appropriate way, and if nobody else objects (particularly Felipe, who is on vacation now), then go for it. Ok, I'll take a look at what can be done and restart the conversation when Felipe returns. Wolfram: given this, as I mentioned, I'll simply drop this patch from the series and work around it for now. This will probably make Lee and Mark happy to not see subsys_initcall() in the MFD/regulator drivers as well. -Matt -- 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: [RFCv2 00/10] xhci: re-work command queue management
On Wed, Feb 5, 2014 at 1:22 AM, David Laight david.lai...@aculab.com wrote: From: Dan Williams Adding another list that will have its own set of bugs seems retrograde top me. What bugs? Please be specific. The problem to be addressed is not the allocation of commands, but that timeouts of one command eat the timeout periods of subsequent commands. I'm thinking a single-threaded workqueue better models what the hardware is doing. See my comments in patch 1, but that is orthogonal to how the command contexts are allocated. No software is bug free. The best way to get reasonably bug-free software is the KISS principle (Keep It Simple Stupid). I find this condescending. Perhaps you did not mean for it to come out that way, but this is far from a specific technical argument. You just seem to be adding a lot of additional complexity. Maybe it would look better if more of the code was factored out of the call sites. IIRC at the moment every call site has to: 1) find the trb address that will be used (massive layer break). 2) actually write the trb (through several layers of function call). 3) sort out any timeout (I didn't really look into this bit). 4) ring the bell. I think there is room to push these pre-requisites down. ISTM that the call site should just be a single function call. If you do that the implementation of the timeouts/completions is removed from the callers. Yes, but I think we need to centralize the context under which commands are submitted. The complicating factor is the mix of synchronous command submission and interrupt driven asynchronous command queuing. I think we can simplify it by making it all submitted from a single event queue context. I'm investigating if that is a workable solution... -- 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: [RFCv2 00/10] xhci: re-work command queue management
From: Dan Williams Yes, but I think we need to centralize the context under which commands are submitted. The complicating factor is the mix of synchronous command submission and interrupt driven asynchronous command queuing. I think we can simplify it by making it all submitted from a single event queue context. I'm investigating if that is a workable solution... Given that the entire network stack runs from (I believe) 'process level' contexts (NAPI functions), it is rather surprising that the USB stack runs everything from the actual interrupt context (or in the callers context with interrupts disabled). For large SMP systems disabling interrupts and acquiring global locks like that just doesn't scale. Even for 'normal' URB it ought to be possible to queue them for later submission - rather than having to immediately put them on the transfer ring. Ideally the 'event' ring ought to be large enough for all the events that the controller can add. This is approximately 1 per TD (2 for short RX) and one per command. So much like the 'reserved command' slots (which aren't implemented correctly) there should probably be 'reserved event' slots - otherwise the event ring might become full (if the host stops servicing interrupts for any length of time.) Avoiding the need for large numbers of events really means restricting the number of TD (that request interrupts). isoc and disk might limit this anyway, but network traffic can queue a much larger number of TD. I've seen 20 tx TD (all nearly 60kB) queued. Every time one completes another is immediately added. usbnet needs to be taught how to do BQL, and something (somehow) reduce the number of interrupts. (end of ramble) 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: [RFCv2 00/10] xhci: re-work command queue management
On Wed, Feb 5, 2014 at 9:05 AM, David Laight david.lai...@aculab.com wrote: From: Dan Williams Yes, but I think we need to centralize the context under which commands are submitted. The complicating factor is the mix of synchronous command submission and interrupt driven asynchronous command queuing. I think we can simplify it by making it all submitted from a single event queue context. I'm investigating if that is a workable solution... Given that the entire network stack runs from (I believe) 'process level' contexts (NAPI functions), Not really. Much of the network stack runs under softirq context. -- 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 v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
On Wed, Feb 5, 2014 at 7:57 AM, Kamil Debski k.deb...@samsung.com wrote:
Hi Olof,
Thank you for your review.
From: Olof Johansson [mailto:o...@lixom.net]
Sent: Wednesday, January 29, 2014 9:55 PM
Hi,
On Wed, Jan 29, 2014 at 9:29 AM, Kamil Debski k.deb...@samsung.com
wrote:
Change the phy provider used from the old one using the USB phy
framework to a new one using the Generic phy framework.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
.../devicetree/bindings/usb/exynos-usb.txt | 13 +++
drivers/usb/host/ehci-exynos.c | 97
+---
2 files changed, 76 insertions(+), 34 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt
b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index d967ba1..25e199a 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -12,6 +12,10 @@ Required properties:
- interrupts: interrupt number to the cpu.
- clocks: from common clock binding: handle to usb clock.
- clock-names: from common clock binding: Shall be usbhost.
+ - port: if in the SoC there are EHCI phys, they should be listed
here.
+One phy per port. Each port should have its reg entry with a
+consecutive number. Also it should contain phys and phy-names
entries
+specifying the phy used by the port.
Optional properties:
- samsung,vbus-gpio: if present, specifies the GPIO that @@ -27,6
+31,15 @@ Example:
clocks = clock 285;
clock-names = usbhost;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ port@0 {
+ reg = 0;
+ phys = usb2phy 1;
+ phy-names = host;
+ status = disabled;
+ };
};
OHCI
[...]
@@ -102,14 +132,26 @@ static int exynos_ehci_probe(struct
platform_device *pdev)
samsung,exynos5440-ehci))
goto skip_phy;
- phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(phy)) {
- usb_put_hcd(hcd);
- dev_warn(pdev-dev, no platform data or transceiver
defined\n);
- return -EPROBE_DEFER;
- } else {
- exynos_ehci-phy = phy;
- exynos_ehci-otg = phy-otg;
+ for_each_available_child_of_node(pdev-dev.of_node, child) {
+ err = of_property_read_u32(child, reg,
phy_number);
+ if (err) {
+ dev_err(pdev-dev, Failed to parse device
tree\n);
+ of_node_put(child);
+ return err;
+ }
+ if (phy_number = PHY_NUMBER) {
+ dev_err(pdev-dev, Failed to parse device
tree - number out of range\n);
+ of_node_put(child);
+ return -EINVAL;
+ }
+ phy = devm_of_phy_get(pdev-dev, child, 0);
+ of_node_put(child);
+ if (IS_ERR(phy)) {
+ dev_err(pdev-dev, Failed to get phy number
%d,
+
phy_number);
+ return PTR_ERR(phy);
+ }
+ exynos_ehci-phy[phy_number] = phy;
this looks like it is now breaking older device trees, where ports
might not be described. Since device tree interfaces need to be
backwards compatible, you still need to handle the old case of not
having ports described.
There are two ways of doing this:
1. Fall back to the old behavior if there are no ports 2. Use a new
compatible value for the new model with port subnodes, and if the old
compatible value is used, then fall back to the old behavior.
I'm guessing (1) might be easiest since you can check for the presence
of #address-cells to tell if this is just an old style node, or if it's
a new-style node without any ports below it.
The ultimate goal is to remove the old phy driver. Unfortunately
this has to be synced with the new USB3 phy driver by Vivek Gautam. I think
he
is also close to completion. What about this case? In the end the old driver
will be removed and no longer be supported. Having backward compatibility in
mind, it is possible to have the old and the new phy driver together in one
kernel release. But do we want to have two drivers doing the same thing at
the same time?
It is mostly irrelevant if there is a new driver or not -- the old
device tree has to keep working. In this case it would mean that the
new driver needs to work with older device trees as well, or people
will see functionality regressing.
The device tree is a description of the hardware, not an extension of
the driver.
-Olof
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to
RE: [RFC PATCHv1] usb: dwc2: Combine the dwc2 and s3c_hsotg into a single USB DRD driver.
From: Dinh Nguyen [mailto:dingu...@altera.com] Sent: Tuesday, February 04, 2014 10:14 PM On Wed, 2014-02-05 at 00:42 +, Paul Zimmerman wrote: From: dingu...@altera.com [mailto:dingu...@altera.com] Sent: Tuesday, February 04, 2014 1:46 PM From: Dinh Nguyen dingu...@altera.com This means that the driver can be in host or peripheral mode when the appropriate connector is used. When an A-cable is plugged in, the driver behaves in host mode, and when a B-cable is used, the driver will be in peripheral mode. This commit: - Replaces in the defines used in s3c_hsotg.h with the defines used in the dwc2 hw.h defines. - Use the dw2_hsotg as the unified data structure for the host/gadget. - Uses the dwc2 IRQ handler for host/gadget. - A single spinlock. Hi Dinh, Putting all of these changes into a single patch makes them unreviewable as far I am concerned. You need to break this into a series of smaller patches. I would suggest something like this: 1 of n: Make the minimum changes to the dwc2 header files needed to support s3c-hsotg as a standalone driver. 2 of n: Make the spelling changes to s3c-hsotg.c needed to use the dwc2 headers, and move it to the dwc2/ directory. Make the Kconfig and Makefile changes needed for the move. Delete s3c-hsotg.h. 3 of n: Move the struct defines etc. from s3c-hsotg.c to the dwc2 header files. .. of n: Make the changes required to combine the functionality of both drivers into one. Preferably this would also be a series of patches instead of one big one. At each step of the series, both drivers should still compile and work. I agree. My original thought was to also split this patch, but I just didn't know how to split it. This is why I designated as an RFC. I was really looking for feedback as this is the correct way to combine this driver. I was also looking for testing purpose to make sure I did not break anything for the s3c platform. The problem is, it's almost impossible to see what the functional changes are because of all the noise of the other changes. Also, please follow the patch style used on the linux lists. 'git format-patch --cover-letter' should do most of this for you automatically. I did use --cover-letter on this patch series. And you should probably trim the Cc list to something more reasonable. I looked through all the commits for the dwc2 driver for the cc list. I also CC a bunch of the Samsung people as I figured that the biggest impact of the work would affect the s3c folks. I would suggest just CCing the Samsung folks to start with, since s3c-hsotg is a driver for their hardware. And the linux-usb and linux-samsung-soc lists. -- Paul
Re: [GIT PULL] xhci: Fix some regressions introduced in 3.14.
On Tue, Feb 04, 2014 at 12:44:23PM -0800, Greg Kroah-Hartman wrote:
On Tue, Feb 04, 2014 at 12:29:22PM -0800, Sarah Sharp wrote:
Please pull usb-linus into usb-next, as I have feature patches that rely on
140e3026a57a Revert usbcore: set lpm_capable field for LPM capable root
hubs
usb-linus and usb-next are now based on 3.14-rc1, so all should be fine
for you now.
I don't think that's going to be sufficient.
The commit I mentioned was one I just sent you for usb-linus. It's a
revert to keep things simple for stable, that touches
usb_device_supports_lpm.
The patch I want to add to usb-next reworks usb_device_supports_lpm and
some xHCI code. It's attached, if you want to take a look at it. I'm
happy to queue it to usb-linus if you think it's small enough. That
would be simpler.
Sarah Sharp
From 295ec6c8115cb5d8a7637ee02042d041214362dd Mon Sep 17 00:00:00 2001
From: Sarah Sharp sarah.a.sh...@linux.intel.com
Date: Fri, 17 Jan 2014 14:15:44 -0800
Subject: [PATCH] usb/xhci: Change how we indicate a host supports Link PM.
The xHCI driver currently uses a USB core internal field,
udev-lpm_capable, to indicate the xHCI driver knows how to calculate
the LPM timeout values. If this value is set for the host controller
udev, it means Link PM can be enabled for child devices under that host.
Change the code so the xHCI driver isn't mucking with USB core internal
fields. Instead, indicate the xHCI driver doesn't support Link PM on
this host by clearing the U1 and U2 exit latencies in the roothub
SuperSpeed Extended Capabilities BOS descriptor.
The code to check for the roothub setting U1 and U2 exit latencies to
zero will also disable LPM for external devices that do that same. This
was already effectively done with commit
ae8963adb4ad8c5f2a89ca1d99fb7bb721e7599f usb: Don't enable LPM if the
exit latency is zero. Leave that code in place, so that if a device
sets one exit latency value to zero, but the other is set to a valid
value, LPM is only enabled for the U1 or U2 state that had the valid
value. This is the same behavior the code had before.
Also, change messages about missing Link PM information from warning
level to info level. Only print a warning about the first device that
doesn't support LPM, to avoid log spam. Further, cleanup some
unnecessary line breaks to help people to grep for the error messages.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: Alan Stern st...@rowland.harvard.edu
---
drivers/usb/core/hub.c | 24
drivers/usb/host/xhci-hub.c | 8 +---
drivers/usb/host/xhci-pci.c | 6 --
3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 64ea21971be2..4fa546f227dc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -141,19 +141,27 @@ static int usb_device_supports_lpm(struct usb_device *udev)
return 0;
}
- /* All USB 3.0 must support LPM, but we need their max exit latency
- * information from the SuperSpeed Extended Capabilities BOS descriptor.
+ /*
+ * According to the USB 3.0 spec, all USB 3.0 devices must support LPM.
+ * However, there are some that don't, and they set the U1/U2 exit
+ * latencies to zero.
*/
if (!udev-bos-ss_cap) {
- dev_warn(udev-dev, No LPM exit latency info found.
-Power management will be impacted.\n);
+ dev_info(udev-dev, No LPM exit latency info found, disabling LPM.\n);
return 0;
}
- if (udev-parent-lpm_capable)
- return 1;
- dev_warn(udev-dev, Parent hub missing LPM exit latency info.
- Power management will be impacted.\n);
+ if (udev-bos-ss_cap-bU1devExitLat == 0
+ udev-bos-ss_cap-bU2DevExitLat == 0) {
+ if (udev-parent)
+ dev_info(udev-dev, LPM exit latency is zeroed, disabling LPM.\n);
+ else
+ dev_info(udev-dev, We don't know the algorithms for LPM for this host, disabling LPM.\n);
+ return 0;
+ }
+
+ if (!udev-parent || udev-parent-lpm_capable)
+ return 1;
return 0;
}
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 9992fbfec85f..1ad6bc1951c7 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -732,9 +732,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
/* Set the U1 and U2 exit latencies. */
memcpy(buf, usb_bos_descriptor,
USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE);
- temp = readl(xhci-cap_regs-hcs_params3);
- buf[12] = HCS_U1_LATENCY(temp);
- put_unaligned_le16(HCS_U2_LATENCY(temp), buf[13]);
+ if ((xhci-quirks XHCI_LPM_SUPPORT)) {
+ temp = readl(xhci-cap_regs-hcs_params3);
+ buf[12] = HCS_U1_LATENCY(temp);
+ put_unaligned_le16(HCS_U2_LATENCY(temp), buf[13]);
+ }
/* Indicate whether the host has LTM support. */
temp = readl(xhci-cap_regs-hcc_params);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 04f986d9234f..6c03584ac15f 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -222,12 +222,6
Re: [GIT PULL] xhci: Fix some regressions introduced in 3.14.
On Wed, Feb 05, 2014 at 11:23:22AM -0800, Sarah Sharp wrote: On Tue, Feb 04, 2014 at 12:44:23PM -0800, Greg Kroah-Hartman wrote: On Tue, Feb 04, 2014 at 12:29:22PM -0800, Sarah Sharp wrote: Please pull usb-linus into usb-next, as I have feature patches that rely on 140e3026a57a Revert usbcore: set lpm_capable field for LPM capable root hubs usb-linus and usb-next are now based on 3.14-rc1, so all should be fine for you now. I don't think that's going to be sufficient. The commit I mentioned was one I just sent you for usb-linus. It's a revert to keep things simple for stable, that touches usb_device_supports_lpm. Ah, so sorry, you are right, I was thinking you just wanted 3.14-rc1 in usb-next, my mistake. usb-linus and usb-next should be in sync now, if that's not ok, let me know. The patch I want to add to usb-next reworks usb_device_supports_lpm and some xHCI code. It's attached, if you want to take a look at it. I'm happy to queue it to usb-linus if you think it's small enough. That would be simpler. No, you are right, it should wait for 3.15. Sorry for the confusion, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: dwc2: handle the Host Port Interrupt when it occurs in device mode
On Tue, Feb 04, 2014 at 03:19:40PM -0800, Paul Zimmerman wrote: From: Dinh Nguyen dingu...@altera.com According to the spec for the DWC2 controller, when the PRTINT interrupt fires, the application must clear the appropriate status bit in the Host Port Control and Status register to clear this bit. When disconnecting an A-cable when the dwc2 host driver, the PRTINT fires, but only the GINTSTS_PRTINT status is cleared, no action is done with the HPRT0 register. The HPRT0_ENACHG bit in the HPRT0 must also be poked to correctly clear the GINTSTS_PRTINT interrupt. I am seeing this behavoir on v2.93 of the DWC2 IP. When I disconnect an OTG A-cable adapter, the PRTINT interrupt fires when the DWC2 is in device mode and is never cleared. This patch adds the function to read the HPRT0 register when the PRTINT fires and the dwc2 IP has already transitioned to device mode. This function is only clearing the HPRT0_ENACHG bit for now, but can be modified to handle more. Signed-off-by: Dinh Nguyen dingu...@altera.com [ paulz: modified patch to preserve HPRT0_ENA bit ] Signed-off-by: Paul Zimmerman pa...@synopsys.com --- drivers/usb/dwc2/core_intr.c | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) Is this a 3.14 or 3.15 patch? -- 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] usb: dwc2: handle the Host Port Interrupt when it occurs in device mode
From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Wednesday, February 05, 2014 11:36 AM On Tue, Feb 04, 2014 at 03:19:40PM -0800, Paul Zimmerman wrote: From: Dinh Nguyen dingu...@altera.com According to the spec for the DWC2 controller, when the PRTINT interrupt fires, the application must clear the appropriate status bit in the Host Port Control and Status register to clear this bit. When disconnecting an A-cable when the dwc2 host driver, the PRTINT fires, but only the GINTSTS_PRTINT status is cleared, no action is done with the HPRT0 register. The HPRT0_ENACHG bit in the HPRT0 must also be poked to correctly clear the GINTSTS_PRTINT interrupt. I am seeing this behavoir on v2.93 of the DWC2 IP. When I disconnect an OTG A-cable adapter, the PRTINT interrupt fires when the DWC2 is in device mode and is never cleared. This patch adds the function to read the HPRT0 register when the PRTINT fires and the dwc2 IP has already transitioned to device mode. This function is only clearing the HPRT0_ENACHG bit for now, but can be modified to handle more. Signed-off-by: Dinh Nguyen dingu...@altera.com [ paulz: modified patch to preserve HPRT0_ENA bit ] Signed-off-by: Paul Zimmerman pa...@synopsys.com --- drivers/usb/dwc2/core_intr.c | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) Is this a 3.14 or 3.15 patch? It's not fixing a regression, so 3.15 I would think. -- Paul -- 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 v6 8/8] usb: ehci-exynos: Change to use phy provided by the generic phy framework
On Wed, 5 Feb 2014, Kamil Debski wrote: Hi Alan, Thank you for your review. You're welcome. Change the phy provider used from the old one using the USB phy You have removed all the OTG stuff from the driver. This wasn't mentioned in the patch description, and it has no connection with the PHY work. Maybe I'll explain more about what are we trying to achieve. The goal is to replace the old phy driver with the new one. In the old driver it was difficult to add support to new SoC. It also had issues with having device and host working together. You're right that until the old phy driver is removed support for the it should remain. To be able to remove the old driver both new USB2 and new USB3 phy drivers have to be ready. The USB3 driver is written by Vivek Gautam and as I see it, he is also close to completion. Regarding the otg part. The old phy driver is the only provider of the otg structure. It sets the host field of the structure. It is then used by samsung_usb2phy_init (drivers/usb/phy/phy-samsung-usb2.c) to check which driver is requesting the phy (is it host or device). In the new driver this is determined by the entry in device tree. So no need to check the otg struct and strstr (!) to check if dev_name is ehci, ohci or other, like the old driver. Okay, that's fine. But please explain this in the patch description next time. Otherwise the connection between the phy driver and the otg structure is not at all clear; they look like two unrelated things. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] UAS: fallback to storage if no streams are available
From: Oliver Neukum oneu...@suse.de uas_probe() calls usb_alloc_streams(). That can fail on XHCI with -ENOSYS if the controller doesn't support streams. In that case devices should be handed over to storage. Thus the driver needs to return -ENODEV so that the driver core will give other drivers a chance. Signed-off-by: Oliver Neukum oneu...@suse.de --- 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 a7ac97c..4dc1aa5 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1110,7 +1110,7 @@ set_alt0: usb_set_interface(udev, intf-altsetting[0].desc.bInterfaceNumber, 0); if (shost) scsi_host_put(shost); - return result; + return result == -ENOSYS ? -ENODEV : result; } static int uas_pre_reset(struct usb_interface *intf) -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: XHCI Host Controller on Intel Panther Point with CDC/ACM dead after massive NAK
On Wed, Feb 05, 2014 at 07:33:15AM +, Kasberger Andreas wrote: Hello Peter, many many thanks for your long and detailed answer. On the protocol design: First, using CDC-ACM means sacrificing all structured communication offered by the USB packet bus and settling for such primitive use of USB is not a decision that should be made lightly. Almost all applications can benefit quite significantly both in end-user usability and in ease of implementation from an application-specific protocol which takes advantage of what USB offers. Yes you are absolutely right. No the best idea. The usage for this protocol is to make firmware updates. In normal life it is a simple keyboard. And sending out bulk messages is the great advantage of CDC/ACM What is still puzzling me is the fact that the host controller stops any communication. Perhaps looking at the USB mon trace would help? I'm wondering if the driver is continuing to submit URBs even though the next ones are NAKed? http://lxr.free-electrons.com/source/Documentation/usb/usbmon.txt It could also be a problem with the xHCI cancellation code. ISTR that we have an issue in the driver where if no URBs get completed, the dequeue pointer gets left on the last TRB that completed, and we keep expanding the rings indefinitely. When you turn on xHCI debugging (either through CONFIG_USB_XHCI_HCD_DEBUG or by running `echo -n 'module xhci_hcd =p' /sys/kernel/debug/dynamic_debug/control`) do you see messages about URB cancellation? That means there is really electrically no communication (bulk_out) from HC to device anymore. It seems that the host controller has shut down communication port to one particular device. unbind and bind host controller will solve the problem Do you have auto-suspend enabled for the device? Perhaps the CDC-ACM driver is suspending the device because there's no reader? You can see if auto-suspend is enabled by finding the device in /sys/bus/usb/devices and seeing if power/control is set to 'auto'. But anyway I will try do my best to find out the root cause of mis-communication between between both sides. You mention device-side buffering and that the device at some point can't accept anything more from the host. With USB this means that you must ensure that the host will know when it must not send more. I thought sending NAK as response for each package is the correct way to tell the host not now but maybe later.Please try again. After the internal device queue is not completely full namyore the comunication is done in normal way. But after some time HC stops completely any communication. In real life it means a huge firmware update takes long time and so it could happens the internal device queue is full. But a broken firmware update is a bad thing The USB way to do this, were you using an application-specific protocol instead of serial port simulation, would be to stall the endpoint. Unfortunately CDC-ACM doesn't allow doing that. Ok. I will think about this if another way is possible So you have to include some kind of in-band signalling for this. :\ This is just one reason why ACM is a poor choice for when you need structured communication. Anyway many many thanks for your precious time and detailed answers. My conclusions and todo : 1. Thinking about design 2. Still try to find out the main reason why host controller shutdown connection Arrrghhh Just saw also USB 2.0 has some problems. Host controller is resetting after some hours but not getting in work state again. By USB 2.0, do you mean your device under an EHCI host doesn't work as well? There may be USB 2.0 only ports under your xHCI host, and the best way to find out which host you're under is to run `sudo lsusb -t`. Sarah Sharp -- 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: [GIT PULL] xhci: Fix some regressions introduced in 3.14.
On Wed, Feb 05, 2014 at 11:30:39AM -0800, Greg Kroah-Hartman wrote: On Wed, Feb 05, 2014 at 11:23:22AM -0800, Sarah Sharp wrote: On Tue, Feb 04, 2014 at 12:44:23PM -0800, Greg Kroah-Hartman wrote: On Tue, Feb 04, 2014 at 12:29:22PM -0800, Sarah Sharp wrote: Please pull usb-linus into usb-next, as I have feature patches that rely on 140e3026a57a Revert usbcore: set lpm_capable field for LPM capable root hubs usb-linus and usb-next are now based on 3.14-rc1, so all should be fine for you now. I don't think that's going to be sufficient. The commit I mentioned was one I just sent you for usb-linus. It's a revert to keep things simple for stable, that touches usb_device_supports_lpm. Ah, so sorry, you are right, I was thinking you just wanted 3.14-rc1 in usb-next, my mistake. usb-linus and usb-next should be in sync now, if that's not ok, let me know. Looks fine now, no worries! Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 1/2] xhci 1.0: Limit arbitrarily-aligned scatter gather.
On Wed, Feb 05, 2014 at 11:58:12AM +, David Laight wrote: From: Sarah Sharp xHCI 1.0 hosts have a set of requirements on how to align transfer buffers on the endpoint rings called TD fragment rules. When the ax88179_178a driver added support for scatter gather in 3.12, with commit 804fad45411b48233b48003e33a78f290d227c8 USBNET: ax88179_178a: enable tso if usb host supports sg dma, it broke the device under xHCI 1.0 hosts. Under certain network loads, the device would see an unexpected short packet from the host, which would cause the device to stop sending ethernet packets, even through USB packets would still be sent. Commit 35773dac5f86 usb: xhci: Link TRB must not occur within a USB payload burst attempted to fix this. It was a quick hack to partially implement the TD fragment rules. However, it caused regressions in the usb-storage layer and userspace USB drivers using libusb. The patches to attempt to fix this are too far reaching into the USB core, and we really need to implement the TD fragment rules correctly in the xHCI driver, instead of continuing to wallpaper over the issues. Disable arbitrarily-aligned scatter-gather in the xHCI driver for 1.0 hosts. Only the ax88179_178a driver checks the no_sg_constraint flag, so don't set it for 1.0 hosts. This should not impact usb-storage or usbfs behavior, since they pass down max packet sized aligned sg-list entries (512 for USB 2.0 and 1024 for USB 3.0). I believe that this will cause the ax88179 driver to discard some receive packets on (at least) my panther point 1.00 controller. Please go test with that branch, and see if you can trigger this issue. If you can reproduce this, please send me the exact instructions or commands to do so. I certainly saw bursts of lost packets in some testing I did before the scatter-gather transfers were enabled, and before any of my patches. What is the user impact of these lost packets? Does the device drop one particular transfer, and continue with the next transfer, or does the device get wedged and stop sending packets all together? Ethernet protocols are designed to deal with packet loss. Does the user care about a dropped packet every once and while? Especially for USB ethernet devices, which really shouldn't be used in an enterprise setting? I'm not saying this shouldn't be fixed. I'm simply trying to see how much of a priority it is to fix this. I really want to re-architect the code and do this right, and it will take some time. The problem is that the ax88179_178a driver submits receive URBs that cross 64k boundaries, and are not aligned (they start at an 0x40 boundary). Receive USB frames can contain multiple ethernet frames, by default they are 20kB (and sit in 24kB of memory). Perhaps you should add printks when a TRB is split on 64KB boundaries and see if the device drops packets around that time? I'm not entirely convinced that this is acceptable long term behaviour. If the 64KB boundaries are an issue, it will mean that bug has been in the xHCI driver for a very long time. In the kernel community, if a fix for something that has been historically broken causes regressions, we revert it. Sarah Sharp -- 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] UAS: fallback to storage if no streams are available
Hi Oliver, On 02/05/2014 09:13 PM, oli...@neukum.org wrote: From: Oliver Neukum oneu...@suse.de uas_probe() calls usb_alloc_streams(). That can fail on XHCI with -ENOSYS if the controller doesn't support streams. In that case devices should be handed over to storage. Thus the driver needs to return -ENODEV so that the driver core will give other drivers a chance. I'm afraid that that won't work, it won't change the result of uas_use_uas_driver() so usb-storage still will fail to bind. I'm not sure which version you're looking at, but the new re-enabled uas code as it will be going upstream for 3.15 is here: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/usb-next-for-sarah For the above specifically see: http://git.linuxtv.org/hgoede/gspca.git/commitdiff/dda9e7db4e293defeff4956018183e06749528dc So to fix this we would need to export if an hcd supports streams with some hcd flag / property and then as_use_uas_driver() can check this flag (in combination with connection speed). Regards, Hans Signed-off-by: Oliver Neukum oneu...@suse.de --- 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 a7ac97c..4dc1aa5 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1110,7 +1110,7 @@ set_alt0: usb_set_interface(udev, intf-altsetting[0].desc.bInterfaceNumber, 0); if (shost) scsi_host_put(shost); - return result; + return result == -ENOSYS ? -ENODEV : result; } static int uas_pre_reset(struct usb_interface *intf) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 1/2] xhci 1.0: Limit arbitrarily-aligned scatter gather.
On Wed, 5 Feb 2014, Sarah Sharp wrote: On Wed, Feb 05, 2014 at 11:58:12AM +, David Laight wrote: The problem is that the ax88179_178a driver submits receive URBs that cross 64k boundaries, and are not aligned (they start at an 0x40 boundary). Receive USB frames can contain multiple ethernet frames, by default they are 20kB (and sit in 24kB of memory). Perhaps you should add printks when a TRB is split on 64KB boundaries and see if the device drops packets around that time? This seems kind of puzzling. In theory, any URB that's more than 1 byte long can cross a 64-KB boundary. I don't understand why this should cause problems for xhci-hcd. Sure, the transfer has to be split into multiple TDs where the boundary crossing occurs. But why would that be problematic? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 1/2] xhci 1.0: Limit arbitrarily-aligned scatter gather.
On Wed, Feb 05, 2014 at 04:23:50PM -0500, Alan Stern wrote: On Wed, 5 Feb 2014, Sarah Sharp wrote: On Wed, Feb 05, 2014 at 11:58:12AM +, David Laight wrote: The problem is that the ax88179_178a driver submits receive URBs that cross 64k boundaries, and are not aligned (they start at an 0x40 boundary). Receive USB frames can contain multiple ethernet frames, by default they are 20kB (and sit in 24kB of memory). Perhaps you should add printks when a TRB is split on 64KB boundaries and see if the device drops packets around that time? This seems kind of puzzling. In theory, any URB that's more than 1 byte long can cross a 64-KB boundary. I don't understand why this should cause problems for xhci-hcd. Sure, the transfer has to be split into multiple TDs where the boundary crossing occurs. But why would that be problematic? If we split a non-scatter gather URB into two TRBs because of the 64-KB boundary rule, and we have to put a link TRB in between them, that may violate the TD fragments rule for 1.0 hosts. So the debugging should really only trigger when there's a link TRB in between two TRBs on the bulk ring. Sarah Sharp -- 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: xhci and other woes
Sarah Sharp wrote: Yes, this is a slight race condition, and we should wait for the successful event. However, we have not seen any issues with this race condition. I'm glad that you say that having the race is wrong (we should) and I guess that we should wait for the sake of correctness? Which means that the controller is obeying the rules, and the software is wrong. I think your mindset isn't helpful here. I think his mindset is helpful for xHCI users in the future. Software will never be perfect. I think software can be perfect. It just takes well-defined scope and desire for perfection. But let's ignore perfect and stay at correct, because it doesn't make sense for many businesses to invest in perfection. David mentioned that his company is investing in xHCI correctness, and you seem to principally agree with correctness since you write that we shouldn't settle for having a race. side_note The questions to ask when discovering a bug are: 1. What is the impact of this bug? Is it user-visible? 2. How many customers are impacted by this bug? 3. What is the software complexity of fixing this bug? 4. What are the chances of the fix causing other issues? 2. sounds like you reduce humans to business entities customers which I don't think is cool, but I do realize that most kernel work is completely business-driven. Users are other humans and when I work on software I want those other humans to have the best possible software I can give them, not just the best possible software I can give them without causing (or exposing?) other issues. That seems like sweeping dust under the carpet. :) My fault in libusb is that I naïvely expected other open source developers to share the desire for correctness, when in fact many developers don't care about correctness at all, much less perfection. In this case, the bug has been worked around (not perfectly), but we've had no customer reports that this is an issue. Maybe perfection could be expressed as pre-empting any and all issues. Again - not economical. Much more about what we leave behind for those who come after us, than about what we can profit from today. /side_node So fixing this race condition is a really low priority for me If fixing the race requires larger architectural changes doesn't that suggest that the software model for the hardware interface is still (perhaps just a little) incorrect, and should be fixed? //Peter -- 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: [RFCv2 08/10] xhci: Add a global command queue
On Tue, Feb 04, 2014 at 10:57:09PM -0800, Dan Williams wrote:
On Thu, Jan 30, 2014 at 6:10 AM, Mathias Nyman
mathias.ny...@linux.intel.com wrote:
@@ -1722,6 +1723,12 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
kfree(cur_cd);
}
+ list_for_each_entry_safe(cur_cmd, next_cmd,
+ xhci-cmd_list, cmd_list) {
+ list_del(cur_cmd-cmd_list);
+ kfree(cur_cmd);
+ }
+
Aren't commands on the cmd_list currently being executed, or are there
other guarantees that make sure all commands have terminated?
By the time we get to xhci_mem_cleanup, we've done our best effort to
halt the xHCI host controller. That could timeout, I suppose, but I'm
not sure what we're expected to do in that case. If the host won't
halt, I'm not sure it will be able to, say, respond to the request to
cancel the current command.
Sarah Sharp
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: usb: obex in g_nokia.ko causing kernel panic
Hi Felipe, cool, I'll send this during the -rc and Cc stable, then I'll manually backport it to stable later. PING! I still do not see this patch in linus tree. What happened? -- Pali Rohár pali.ro...@gmail.com BUMP Is there any problem with above patch? If you are short on time or have some another reasons preventing you pushing that patch, I can send that patch for you, are you ok with that? Ivo -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT 1/2] xhci 1.0: Limit arbitrarily-aligned scatter gather.
Sarah, Sarah Sharp wrote: I'm simply trying to see how much of a priority it is to fix this. I really want to re-architect the code and do this right, and it will take some time. Awesome! I think wanting to do it right is very close to desire for perfection, or at the very least correctness. :) I do appreciate having to prioritize. //Peter -- 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
affiliate
May I request for your cooperation to move funds and invest same in your country. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: usb: obex in g_nokia.ko causing kernel panic
On Thu, Feb 06, 2014 at 12:25:21AM +0200, Ivaylo Dimitrov wrote: Hi Felipe, cool, I'll send this during the -rc and Cc stable, then I'll manually backport it to stable later. PING! I still do not see this patch in linus tree. What happened? -- Pali Rohár pali.ro...@gmail.com BUMP Is there any problem with above patch? If you are short on time or have some another reasons preventing you pushing that patch, I can send that patch for you, are you ok with that? Felipe is on vacation, care to resend the patch and cc: me? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Add for Devices on USB to Serial Device
Dear Sirs I´ve an USB to Serial Adapter ,who works under Linux mint 16. I use it for control Label Printers they connect to Serial. Please add and set this Driver for Device 067b:2303 Prolific Technology, Inc. PL2303 Serial Port With kind regards. one Linux user. -- 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: xhci and other woes
On Wed, Feb 05, 2014 at 09:44:20AM +, David Laight wrote: From: Mark Lord Which means that the controller is obeying the rules, and the software is wrong. .. In this case, the bug has been worked around (not perfectly), but we've had no customer reports that this is an issue. There is no user-visible impact as far as we know. So fixing this race condition is a really low priority for me, and the fix would have to very minimally touch the driver. I'm seeing the problem with the ASMedia controller (rev 0.96). Your fix is only active for rev 1.00 controllers. Ok, then let's add a quirk for that host. Are you sure you don't need XHCI_TRUST_TX_LENGTH instead of XHCI_SPURIOUS_SUCCESS? Will reported seeing messages about needing XHCI_TRUST_TX_LENGTH. Sarah Sharp -- 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 PATCHv1] usb: dwc2: Combine the dwc2 and s3c_hsotg into a single USB DRD driver.
On Thursday, February 06, 2014 4:21 AM, Paul Zimmerman wrote: On Tuesday, February 04, 2014 10:14 PM, Dinh Nguyen wrote: On Wed, 2014-02-05 at 00:42 +, Paul Zimmerman wrote: From: dingu...@altera.com [mailto:dingu...@altera.com] Sent: Tuesday, February 04, 2014 1:46 PM From: Dinh Nguyen dingu...@altera.com This means that the driver can be in host or peripheral mode when the appropriate connector is used. When an A-cable is plugged in, the driver behaves in host mode, and when a B-cable is used, the driver will be in peripheral mode. This commit: - Replaces in the defines used in s3c_hsotg.h with the defines used in the dwc2 hw.h defines. - Use the dw2_hsotg as the unified data structure for the host/gadget. - Uses the dwc2 IRQ handler for host/gadget. - A single spinlock. Hi Dinh, Putting all of these changes into a single patch makes them unreviewable as far I am concerned. You need to break this into a series of smaller patches. I would suggest something like this: 1 of n: Make the minimum changes to the dwc2 header files needed to support s3c-hsotg as a standalone driver. 2 of n: Make the spelling changes to s3c-hsotg.c needed to use the dwc2 headers, and move it to the dwc2/ directory. Make the Kconfig and Makefile changes needed for the move. Delete s3c-hsotg.h. 3 of n: Move the struct defines etc. from s3c-hsotg.c to the dwc2 header files. .. of n: Make the changes required to combine the functionality of both drivers into one. Preferably this would also be a series of patches instead of one big one. At each step of the series, both drivers should still compile and work. I agree. My original thought was to also split this patch, but I just didn't know how to split it. This is why I designated as an RFC. I was really looking for feedback as this is the correct way to combine this driver. I was also looking for testing purpose to make sure I did not break anything for the s3c platform. The problem is, it's almost impossible to see what the functional changes are because of all the noise of the other changes. Yes, right. I think so, too. Also, please follow the patch style used on the linux lists. 'git format-patch --cover-letter' should do most of this for you automatically. I did use --cover-letter on this patch series. And you should probably trim the Cc list to something more reasonable. I looked through all the commits for the dwc2 driver for the cc list. I also CC a bunch of the Samsung people as I figured that the biggest impact of the work would affect the s3c folks. I would suggest just CCing the Samsung folks to start with, since s3c-hsotg is a driver for their hardware. And the linux-usb and linux-samsung-soc lists. Please keep CCing linux-samsung-soc. Some Samsung folks will test the patches with Samsung hardware. Thank you. Best regards, Jingoo Han -- 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] r8152: fix the submission of the interrupt transfer
The submission of the interrupt transfer should be done after setting
the bit of WORK_ENABLE, otherwise the callback function would have
the opportunity to be returned directly.
Clear the bit of WORK_ENABLE before killing the interrupt transfer.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e8fac73..d89dbe3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2273,22 +2273,21 @@ static int rtl8152_open(struct net_device *netdev)
struct r8152 *tp = netdev_priv(netdev);
int res = 0;
+ rtl8152_set_speed(tp, AUTONEG_ENABLE,
+ tp-mii.supports_gmii ? SPEED_1000 : SPEED_100,
+ DUPLEX_FULL);
+ tp-speed = 0;
+ netif_carrier_off(netdev);
+ netif_start_queue(netdev);
+ set_bit(WORK_ENABLE, tp-flags);
res = usb_submit_urb(tp-intr_urb, GFP_KERNEL);
if (res) {
if (res == -ENODEV)
netif_device_detach(tp-netdev);
netif_warn(tp, ifup, netdev, intr_urb submit failed: %d\n,
res);
- return res;
}
- rtl8152_set_speed(tp, AUTONEG_ENABLE,
- tp-mii.supports_gmii ? SPEED_1000 : SPEED_100,
- DUPLEX_FULL);
- tp-speed = 0;
- netif_carrier_off(netdev);
- netif_start_queue(netdev);
- set_bit(WORK_ENABLE, tp-flags);
return res;
}
@@ -2298,8 +2297,8 @@ static int rtl8152_close(struct net_device *netdev)
struct r8152 *tp = netdev_priv(netdev);
int res = 0;
- usb_kill_urb(tp-intr_urb);
clear_bit(WORK_ENABLE, tp-flags);
+ usb_kill_urb(tp-intr_urb);
cancel_delayed_work_sync(tp-schedule);
netif_stop_queue(netdev);
tasklet_disable(tp-tl);
--
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
