Re: [PATCH v4] Move DWC2 driver out of staging
On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: On 01/31/2014 11:12 AM, Andre Heider wrote: On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: The DWC2 driver should now be in good enough shape to move out of staging. I have stress tested it overnight on RPI running mass storage and Ethernet transfers in parallel, and for several days on our proprietary PCI-based platform. ... this looks just fine, but for whatever reason it breaks sdhci on my rpi. With today's Linus' master the dwc2 controller seems to initialize fine, but I get this upon boot: [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12 That is: struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev, const struct sdhci_pltfm_data *pdata, size_t priv_size) { ... iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!iomem) { ret = -ENOMEM; goto err; } This is due to the following code: static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep) { struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); ... struct usb_device *udev; ... udev = to_usb_device(hsotg-dev); ... usb_settoggle(udev, epnum, is_out, 0); if (is_control) usb_settoggle(udev, epnum, !is_out, 0); The problem is that hsotg-dev is assigned as follows: static int dwc2_driver_probe(struct platform_device *dev) ... hsotg = devm_kzalloc(dev-dev, sizeof(*hsotg), GFP_KERNEL); ... hsotg-dev = dev-dev; As such, it's not legal to call to_usb_device() on it. What ends up happening, simply due to memory allocation order, is that the memory writes inside usb_settoggle() end up setting the SDHCI struct platform_device's num_resources to 0, so that it's call to platform_get_resource() fails. With the DWC2 move patch reverted, some other random piece of memory is being corrupted, which just happens not to cause any visible problem. Likely it's some other struct platform_device that's already had its resources read by the time DWC2 probes and corrupts them. (Yes, this was hard to find!) Nice work, but how did you pinpoint this? Am I missing some option/tool or did I just not stare for long enough? I honestly can't see how to solve this myself, since the whole DWC2 driver doesn't seem to have a struct usb_device * hanging around that we can stash somewhere for it to look up later. Perhaps someone more familiar with the USB stack can help with that. -- 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 RFC 1/1] usb: Tell xhci when usb data might be misaligned
On 14-02-01 02:54 AM, Ming Lei wrote: .. With SG enabled, for the iperf client test case, the average urb size for transmission will be increased from ~1500 to ~20K bytes in my test case: iperf -c $SRV -t 30 -P 4 -w 128K So I am wondering you guys do not care the improvement .. No, that's not it. Simply, the recent changes killed the driver for some users, something Linus calls a regression, and does not permit. Far better to have it continue to work than not to work. The plan discussed earlier calls for reintroduction of SG here once the problems are solved outside of the main tree. -- Mark Lord Real-Time Remedies Inc. ml...@pobox.com -- 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 RFC 1/1] usb: Tell xhci when usb data might be misaligned
On Sat, Feb 1, 2014 at 9:30 PM, Mark Lord ml...@pobox.com wrote: On 14-02-01 02:54 AM, Ming Lei wrote: .. With SG enabled, for the iperf client test case, the average urb size for transmission will be increased from ~1500 to ~20K bytes in my test case: iperf -c $SRV -t 30 -P 4 -w 128K So I am wondering you guys do not care the improvement .. No, that's not it. Simply, the recent changes killed the driver I just want to clarify the sg approach does improve performance, instead of no improvement mentioned by your guys. for some users, something Linus calls a regression, and does not permit. Even real regressions are easily/often introduced, and we are discussing how to fix that. I suggest to unset the flag only for the known buggy controllers. Thanks, -- Ming Lei -- 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: SCSI Sense Information in usb-storage
On Sat, 1 Feb 2014, Steph Nguyen wrote: Hi It seems that when sending SCSI commands over USB, the kernel doesn't necessary build a sense response. For instance, when sending REQUEST SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the same REQUEST SENSE command over a direct SCSI connect, the kernel builds the scsi_cmnd-sense_buffer. There's no need for the kernel to build a sense buffer when sending a REQUEST SENSE command. That command always succeeds, so there's no reason to ask for sense data which might explain why the command failed. Your question isn't very precise. How did you run a REQUEST SENSE command? When using usb-storage under normal conditions, the SCSI layer doesn't ever send that command. So is there a reason why usb-storage doesn't build the sense buffer? In general, usb-storage doesn't touch scsi_cmnd except to set scsi_cmnd-result. (There are a few exceptions, but this is mostly true.) If the SCSI layer wants to receive sense data, it has to set the scsi_cmnd-sense_buffer field itself. Normally it does so. For example, see what happens when the SCSI layer issues a TEST UNIT READY command to usb-storage. 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 v4] Move DWC2 driver out of staging
On Fri, 31 Jan 2014, Stephen Warren wrote: This is due to the following code: static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep) { struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); ... struct usb_device *udev; ... udev = to_usb_device(hsotg-dev); ... usb_settoggle(udev, epnum, is_out, 0); if (is_control) usb_settoggle(udev, epnum, !is_out, 0); The problem is that hsotg-dev is assigned as follows: static int dwc2_driver_probe(struct platform_device *dev) ... hsotg = devm_kzalloc(dev-dev, sizeof(*hsotg), GFP_KERNEL); ... hsotg-dev = dev-dev; As such, it's not legal to call to_usb_device() on it. This clearly is a bug. I suspect the driver has other bugs too, such as not holding the private lock over a large enough region. What ends up happening, simply due to memory allocation order, is that the memory writes inside usb_settoggle() end up setting the SDHCI struct platform_device's num_resources to 0, so that it's call to platform_get_resource() fails. With the DWC2 move patch reverted, some other random piece of memory is being corrupted, which just happens not to cause any visible problem. Likely it's some other struct platform_device that's already had its resources read by the time DWC2 probes and corrupts them. (Yes, this was hard to find!) I honestly can't see how to solve this myself, since the whole DWC2 driver doesn't seem to have a struct usb_device * hanging around that we can stash somewhere for it to look up later. Perhaps someone more familiar with the USB stack can help with that. The correct solution is to set qh = ep-hcpriv; udev = qh-udev; However, the driver doesn't store udev in qh (dwc2_qh_init() should do this, but it doesn't). In fact, the field doesn't even exist. 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: SCSI Sense Information in usb-storage
On Sat, 1 Feb 2014, Steph Nguyen wrote: On Sat, 1 Feb 2014, Steph Nguyen wrote: Hi It seems that when sending SCSI commands over USB, the kernel doesn't necessary build a sense response. For instance, when sending REQUEST SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the same REQUEST SENSE command over a direct SCSI connect, the kernel builds the scsi_cmnd-sense_buffer. There's no need for the kernel to build a sense buffer when sending a REQUEST SENSE command. That command always succeeds, so there's no reason to ask for sense data which might explain why the command failed. I'm using sg_requests. When running it against a disk attached to a usb port, the sense_buffer is simply empty. When run against the same disk directly attached to a scsi controller, the sense_buffer is built by the kernel. So same device, same command, two different hw paths lead to two different behaviors. Hold on a second. Above you said build a sense response and scsi_cmnd-sense_buffer is left NULL. Those are two different things. In one case we would have scsi_cmnd-sense_buffer == NULL. In the other case, we would have scsi_cmnd-sense_buffer != NULL and scsi_cmnd-sense_buffer[i] = 0 for each i. Which do you really mean? In general, usb-storage doesn't touch scsi_cmnd except to set scsi_cmnd-result. (There are a few exceptions, but this is mostly true.) Right, it makes sense that the scsi subsystem takes care of this. But, looking at drivers/usb/storage/cypress_atacb.c, I can see that the sense buffer for ATA_16 is built by the kernel. So I suppose this is one of these exceptions you are referring to. No. Even in cypress_atacb.c, usb-storage does not assign anything to scsi_cmnd-sense_buffer. That is, it does not do anything like scsi_cmnd-sense_buffer = X; However, it does fill in the contents of the sense buffer. That's as it should be; usb-storage fills in the sense buffer contents for any command that completes abnormally. The reason you didn't see anything get filled in for your REQUEST SENSE command is because the command completed normally. I have no idea what happens with other SCSI transports. 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: SCSI Sense Information in usb-storage
On Sat, 1 Feb 2014, Steph Nguyen wrote: On Sat, 1 Feb 2014, Steph Nguyen wrote: Hi It seems that when sending SCSI commands over USB, the kernel doesn't necessary build a sense response. For instance, when sending REQUEST SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the same REQUEST SENSE command over a direct SCSI connect, the kernel builds the scsi_cmnd-sense_buffer. There's no need for the kernel to build a sense buffer when sending a REQUEST SENSE command. That command always succeeds, so there's no reason to ask for sense data which might explain why the command failed. I'm using sg_requests. When running it against a disk attached to a usb port, the sense_buffer is simply empty. When run against the same disk directly attached to a scsi controller, the sense_buffer is built by the kernel. So same device, same command, two different hw paths lead to two different behaviors. Hold on a second. Above you said build a sense response and scsi_cmnd-sense_buffer is left NULL. Those are two different things. In one case we would have scsi_cmnd-sense_buffer == NULL. In the other case, we would have scsi_cmnd-sense_buffer != NULL and scsi_cmnd-sense_buffer[i] = 0 for each i. Which do you really mean? I mean scsi_cmnd-sense_buffer == NULL; In general, usb-storage doesn't touch scsi_cmnd except to set scsi_cmnd-result. (There are a few exceptions, but this is mostly true.) Right, it makes sense that the scsi subsystem takes care of this. But, looking at drivers/usb/storage/cypress_atacb.c, I can see that the sense buffer for ATA_16 is built by the kernel. So I suppose this is one of these exceptions you are referring to. No. Even in cypress_atacb.c, usb-storage does not assign anything to scsi_cmnd-sense_buffer. That is, it does not do anything like scsi_cmnd-sense_buffer = X; However, it does fill in the contents of the sense buffer. That's as it should be; usb-storage fills in the sense buffer contents for any command that completes abnormally. The reason you didn't see anything get filled in for your REQUEST SENSE command is because the command completed normally. I have no idea what happens with other SCSI transports. To be clear, here's what I meant by build - I'm just pasting the relevant bits of the code: static void cypress_atacb_passthrough(struct scsi_cmnd *srb, struct us_data *us) { ... unsigned char *sb = srb-sense_buffer; ... if ((srb-result != (DID_ERROR 16) srb-result != (DID_ABORT 16)) ...) memset(sb, 0, SCSI_SENSE_BUFFERSIZE); ... sb[0] = 0x72; So, unless I'm completely misunderstanding the code, here the command is supposed to have completed successfully, isn't it? And the sense_buffer[0] is filled with 0x72 when it completes successfully. Alan Stern Thanks Steph -- 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: SCSI Sense Information in usb-storage
On Sat, 1 Feb 2014, Steph Nguyen wrote: To be clear, here's what I meant by build - I'm just pasting the relevant bits of the code: static void cypress_atacb_passthrough(struct scsi_cmnd *srb, struct us_data *us) { ... unsigned char *sb = srb-sense_buffer; ... if ((srb-result != (DID_ERROR 16) srb-result != (DID_ABORT 16)) ...) memset(sb, 0, SCSI_SENSE_BUFFERSIZE); ... sb[0] = 0x72; So, unless I'm completely misunderstanding the code, here the command is supposed to have completed successfully, isn't it? And the sense_buffer[0] is filled with 0x72 when it completes successfully. That's right. But here the command isn't a SCSI command; it's an ATA command getting sent over a SCSI transport. Furthermore, this doesn't happen unless scsi_cmnd-cmnd[2] 0x20 is nonzero (part of the if condition that you left out), which apparently is a flag indicating that the command is always supposed to return sense data, even when it completes normally (the comment calls it ck_cond). 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
[DRIVER][WIP] Playstation controllers (DualShock 3/4, Navigation/Motion Controller, ...)
Hello, I am a total newbie in Linux kernel development (but not in C programming), and I started to write a USB driver for the DualShock 4 (and also others controllers/equipments for the PlayStation 3/4 while I'm at it). My objective is to sanitize what is reported to the userspace (most games don't like 28+ axes all with a value of -32767), add support for the trackpad/sound output on the DualShock 4, and (finally!) have proper Linux support for the DualShock 3. So far I managed to report all axes and buttons for the DualShock 4, and also the touchpad (currently reported as 4 axis, unsure how to deal with it for now...). I also wrote initial support for : Sony DualShock 3 Sony Navigation Controller Sony Motion Controller But it's more proof-of-concept than ready to release at this point. To use it, download all files, run make, then run ./setup.sh as root to install the driver (as a total newbie in Linux programming, I'm not sure how to integrate this in the Linux build process). ppad.c contains my driver for all controllers as it is now, ppad_dualshock4.c is the same file with only DualShock 4 support for ease of reviewing. I used the xpad driver (Xbox controllers) as a starting point. The files are hosted on Google Code : http://code.google.com/p/linux-playstation-drivers/source/browse/ PS : do NOT pull into a Linux Git repository, this driver is NOT integrated in the Linux source tree! Feedback would be much appreciated :-) Jean-Baptiste Boric -- 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 v4 00/14] port power control fixes
On Fri, 31 Jan 2014, Dan Williams wrote: Toggling port power currently leads to three unintended disconnect scenarios that are addressed by this rework of port power recovery and usb device resume: 1/ Superspeed devices downgrade to their hispeed connection: fix this by preventing superspeed poweroff until the peer port is suspended. This depends on the ability to identify peer ports (patches 1-4), and then patch 5 implements the policy. 2/ khubd prematurely disconnects ports that are in the process of being resumed or reset. khubd now ignores ports in the pm-runtime-suspended state (patch 9) and holds a lock to synchronize the port status changes of usb_port_{suspend|resume} (patch 11). 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of the port power state. Fixed by forcing a full recovery cycle of the usb_device before allowing the next suspend / khubd run (patch 12). Also, for devices that live lock on reconnect the port runtime resume path now has the capability to force a reset-resume to be a warm-reset-resume (patch 13). I haven't gone through all this in any detail. But a couple of things stand out immediately, matters of style. Although the USB stack still has plenty of old code with multiline comments looking this this: /* blah blah blah * blah blah blah */ the accepted style is that all new code should appear like this: /* * blah blah blah * blah blah blah */ Also, the style for indentation througout most of the USB stack is that continuation lines are indented by two tab stops beyond the first line of the statement. They are not aligned with opening parentheses of function calls or anything like that. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/14] port power control fixes
On Sat, Feb 1, 2014 at 10:44 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 31 Jan 2014, Dan Williams wrote: Toggling port power currently leads to three unintended disconnect scenarios that are addressed by this rework of port power recovery and usb device resume: 1/ Superspeed devices downgrade to their hispeed connection: fix this by preventing superspeed poweroff until the peer port is suspended. This depends on the ability to identify peer ports (patches 1-4), and then patch 5 implements the policy. 2/ khubd prematurely disconnects ports that are in the process of being resumed or reset. khubd now ignores ports in the pm-runtime-suspended state (patch 9) and holds a lock to synchronize the port status changes of usb_port_{suspend|resume} (patch 11). 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of the port power state. Fixed by forcing a full recovery cycle of the usb_device before allowing the next suspend / khubd run (patch 12). Also, for devices that live lock on reconnect the port runtime resume path now has the capability to force a reset-resume to be a warm-reset-resume (patch 13). I haven't gone through all this in any detail. But a couple of things stand out immediately, matters of style. Although the USB stack still has plenty of old code with multiline comments looking this this: /* blah blah blah * blah blah blah */ the accepted style is that all new code should appear like this: /* * blah blah blah * blah blah blah */ Ah, the eternal debate. No worries. Also, the style for indentation througout most of the USB stack is that continuation lines are indented by two tab stops beyond the first line of the statement. They are not aligned with opening parentheses of function calls or anything like that. Unfortunate that what goes for net/, or drivers/md/ doesn't go for drivers/usb/... sounds like checkpatch needs subsystem specific style rules to avoid this thrash. If it's just the style I'll put those changes on a git branch and spare the list if you're ok doing a pull... otherwise I'll hold off until you have a chance to take a closer look. -- 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 RFC 1/1] usb: Tell xhci when usb data might be misaligned
On 14-02-01 09:18 AM, Ming Lei wrote: Even real regressions are easily/often introduced, and we are discussing how to fix that. I suggest to unset the flag only for the known buggy controllers. It is not the controllers that are particularly buggy here. But rather the drivers and design of parts of the kernel. Cheers -- Mark Lord Real-Time Remedies Inc. ml...@pobox.com -- 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 v4 00/14] port power control fixes
On Sat, 1 Feb 2014, Dan Williams wrote: Unfortunate that what goes for net/, or drivers/md/ doesn't go for drivers/usb/... sounds like checkpatch needs subsystem specific style rules to avoid this thrash. Yeah, this is all stuff that checkpatch ignores. If it's just the style I'll put those changes on a git branch and spare the list if you're ok doing a pull... otherwise I'll hold off until you have a chance to take a closer look. Sure. Give me a chance to look through it some. At first glance, it seems basically okay. 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 v4 00/14] port power control fixes
On Sat, 2014-02-01 at 15:39 -0500, Alan Stern wrote: On Sat, 1 Feb 2014, Dan Williams wrote: Unfortunate that what goes for net/, or drivers/md/ doesn't go for drivers/usb/... sounds like checkpatch needs subsystem specific style rules to avoid this thrash. Yeah, this is all stuff that checkpatch ignores. If it's just the style I'll put those changes on a git branch and spare the list if you're ok doing a pull... otherwise I'll hold off until you have a chance to take a closer look. Sure. Give me a chance to look through it some. At first glance, it seems basically okay. Ok, if it helps: git://git.kernel.org/pub/scm/linux/kernel/git/djbw/usb usb3-port-power-v4 ...is the same patchset with the following changes: diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 243cf5767433..5f94e3180b88 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2697,7 +2697,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1, goto done; if (!hub_port_warm_reset_required(hub, port1, - portstatus)) + portstatus)) goto done; /* @@ -2772,7 +2772,7 @@ static int check_port_resume_type(struct usb_device *udev, { /* Is a warm reset needed to recover the connection? */ if (udev-reset_resume -hub_port_warm_reset_required(hub, port1, portstatus)) { +hub_port_warm_reset_required(hub, port1, portstatus)) { /* pass */; } /* Is the device still present? */ @@ -4403,7 +4403,7 @@ hub_power_remaining (struct usb_hub *hub) * always unlocked on exit */ static void hub_port_connect_change_unlock(struct usb_hub *hub, int port1, - u16 portstatus, u16 portchange) + u16 portstatus, u16 portchange) { struct usb_device *hdev = hub-hdev; struct device *hub_dev = hub-intfdev; @@ -4459,7 +4459,8 @@ static void hub_port_connect_change_unlock(struct usb_hub *hub, int port1, } } - /* from this point forward we no longer need synchronization + /* +* From this point forward we no longer need synchronization * with usb_port_{suspend|resume} because we are about to * disconnect / reconnect */ @@ -4719,7 +4720,7 @@ static void port_event(struct usb_hub *hub, int port1) if (portchange USB_PORT_STAT_C_ENABLE) { if (!connect_change) dev_dbg(hub_dev, port%d: enable change, status %08x\n, - port1, portstatus); + port1, portstatus); usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); /* @@ -4741,13 +4742,13 @@ static void port_event(struct usb_hub *hub, int port1) dev_dbg(hub_dev, port%d: over-current change\n, port1); usb_clear_port_feature(hdev, port1, - USB_PORT_FEAT_C_OVER_CURRENT); + USB_PORT_FEAT_C_OVER_CURRENT); msleep(100);/* Cool down */ hub_power_on(hub, true); hub_port_status(hub, port1, status, unused); if (status USB_PORT_STAT_OVERCURRENT) dev_err(hub_dev, port%d: over-current condition\n, - port1); + port1); } if (portchange USB_PORT_STAT_C_RESET) { @@ -4758,17 +4759,17 @@ static void port_event(struct usb_hub *hub, int port1) hub_is_superspeed(hdev)) { dev_dbg(hub_dev, port%d: warm reset change\n, port1); usb_clear_port_feature(hdev, port1, - USB_PORT_FEAT_C_BH_PORT_RESET); + USB_PORT_FEAT_C_BH_PORT_RESET); } if (portchange USB_PORT_STAT_C_LINK_STATE) { dev_dbg(hub_dev, port%d: link state change\n, port1); usb_clear_port_feature(hdev, port1, - USB_PORT_FEAT_C_PORT_LINK_STATE); + USB_PORT_FEAT_C_PORT_LINK_STATE); } if (portchange USB_PORT_STAT_C_CONFIG_ERROR) { dev_warn(hub_dev, port%d: config error\n, port1); usb_clear_port_feature(hdev, port1, - USB_PORT_FEAT_C_PORT_CONFIG_ERROR); + USB_PORT_FEAT_C_PORT_CONFIG_ERROR); } /* take port actions that require the port to be powered on */ @@ -4777,7 +4778,8 @@ static void port_event(struct usb_hub *hub, int port1) usb_lock_port(port_dev); do if (pm_runtime_active(port_dev-dev)) { - /*
Re: [PATCH v3 0/9] xhci: Add support for URB_ZERO_PACKET
Hello, I applied these along with the reverting : xhci: Set scatter-gather limit to avoid failed block writes. usb: xhci: Link TRB must not occur within a USB payload burst to Linus' tree. The excellent news is that my system no longer hard freezes with an ax88179_178a connected via a VIA VL800 host when doing certain types of network traffic. The bad news is the following: [ 3337.877698] [ cut here ] [ 3337.877721] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:264 dev_watchdog+0x256/0x260() [ 3337.877727] NETDEV WATCHDOG: enp2s0u1 (ax88179_178a): transmit queue 0 timed out [ 3337.877731] Modules linked in: nls_iso8859_1 vfat fat ax88179_178a usbnet mii usb_storage xhci_hcd fuse ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT xt_CHECKSUM iptable_mangle xt_tcpudp ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables af_packet bridge stp llc vhost_net vhost macvtap macvlan vfio_pci vfio_iommu_type1 vfio it87 hwmon_vid hid_generic usbhid hid btrfs raid6_pq libcrc32c xor mxm_wmi ehci_pci ohci_pci ohci_hcd kvm_amd ehci_hcd snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic kvm crct10dif_pclmul radeon fbcon ttm bitblit snd_hda_intel softcursor crc32_pclmul snd_hda_codec font tileblit ghash_clmulni_intel drm_kms_helper aesni_intel aes_x86_64 lrw gf128mul usbcore snd_hwdep [ 3337.877807] drm snd_pcm snd_timer snd agpgart cfbfillrect cfbimgblt cfbcopyarea i2c_algo_bit fb psmouse glue_helper fam15h_power k10temp usb_common serio_raw wmi ablk_helper pcspkr soundcore fbdev sp5100_tco i2c_piix4 cryptd edac_core tpm_infineon edac_mce_amd tpm_tis mac_hid microcode ipv6 autofs4 unix [ 3337.877843] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 3.13.0+ #4 [ 3337.877849] Hardware name: To be filled by O.E.M. To be filled by O.E.M./M5A99FX PRO R2.0, BIOS 2201 11/22/2013 [ 3337.877853] 0009 88043ed43de0 b04f07d6 88043ed43e28 [ 3337.877860] 88043ed43e18 b0041883 8804234f [ 3337.877866] 0001 0005 88042856 88043ed43e78 [ 3337.877873] Call Trace: [ 3337.877877] IRQ [b04f07d6] dump_stack+0x45/0x56 [ 3337.877893] [b0041883] warn_slowpath_common+0x73/0x90 [ 3337.877901] [b00418e7] warn_slowpath_fmt+0x47/0x50 [ 3337.877909] [b04677d6] dev_watchdog+0x256/0x260 [ 3337.877916] [b0467580] ? dev_graft_qdisc+0x80/0x80 [ 3337.877924] [b004b538] call_timer_fn.isra.36+0x18/0x80 [ 3337.877931] [b004b70a] run_timer_softirq+0x16a/0x200 [ 3337.877939] [b00457fc] __do_softirq+0xdc/0x1f0 [ 3337.877947] [b0045b1d] irq_exit+0x9d/0xb0 [ 3337.877956] [b002c21f] smp_apic_timer_interrupt+0x3f/0x50 [ 3337.877963] [b04fb68a] apic_timer_interrupt+0x6a/0x70 [ 3337.877968] EOI [b03ff217] ? cpuidle_enter_state+0x47/0xc0 [ 3337.877979] [b03ff326] cpuidle_idle_call+0x96/0x130 [ 3337.877987] [b000b989] arch_cpu_idle+0x9/0x20 [ 3337.877995] [b007edfa] cpu_startup_entry+0xda/0x1d0 [ 3337.878002] [b002a861] start_secondary+0x1e1/0x240 [ 3337.878008] ---[ end trace 95a68de5267295c3 ]--- [ 3337.878915] xhci_hcd :02:00.0: HC gave bad length of 124978 bytes left I honestly think it's worth pursuing this as my system is crashable even with sg disabled for the ax88179 without these patches. With this error i'm still able to just rmmod xhci_hcd and reload it and the whole system hasn't been brought down. 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 v4] Move DWC2 driver out of staging
On 02/01/2014 03:00 AM, Andre Heider wrote: On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote: On 01/31/2014 11:12 AM, Andre Heider wrote: On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote: The DWC2 driver should now be in good enough shape to move out of staging. I have stress tested it overnight on RPI running mass storage and Ethernet transfers in parallel, and for several days on our proprietary PCI-based platform. ... this looks just fine, but for whatever reason it breaks sdhci on my rpi. With today's Linus' master the dwc2 controller seems to initialize fine, but I get this upon boot: [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12 [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12 ... This is due to the following code: ... What ends up happening, simply due to memory allocation order, is that the memory writes inside usb_settoggle() end up setting the SDHCI struct platform_device's num_resources to 0, so that it's call to platform_get_resource() fails. With the DWC2 move patch reverted, some other random piece of memory is being corrupted, which just happens not to cause any visible problem. Likely it's some other struct platform_device that's already had its resources read by the time DWC2 probes and corrupts them. (Yes, this was hard to find!) Nice work, but how did you pinpoint this? Am I missing some option/tool or did I just not stare for long enough? Well, there was a clear place where an issue was present; the resource lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs into that function to dump out the data platform_get_resource() used. This clearly pointed at num_resources==0 being the problem. Next, I dumped the same data from the code in drivers/of that sets it up, and it was OK there, so I knew it was getting over-written somewhere. I then basically added hundreds of calls to the same data dumping function throughout kernel functions like really_probe() to track down the location of the problem. Luckily, the behaviour was stable, so I wasn't chasing a race/timing condition. Eventually I narrowed the window to the few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would have been much harder if it was e.g. the USB HW DMAing to memory that caused the corruption, so I was lucky:-) -- 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