Regarding usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers
Hi Vincent, The issue you mentioned in your RFC patch is also reproducing on TI X15 board (dwc3) with Android master, when doing "adb root" command. Your patch fixes it just fine, for which I really thankful. Do you know is there anything preventing it to be merged? I have X15 (dwc3) and BeagleBone Black (I guess it has MUSB UDC controller on it) which I can run some tests on, if it's a blocker for merge. Thanks! P.S. Sorry to not respond in existing thread, I wasn't subscribed to linux-usb before. -- 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: Threaded interrupts for USB HCD instead of tasklets
On Fri, 1 Jun 2018, Sebastian Andrzej Siewior wrote: > On 2018-05-22 15:14:17 [-0400], Alan Stern wrote: > > Sorry, I don't understand that sentence at all. And I don't see how it > > could be relevant to the point I was trying to make. > > > > Consider, for example, drivers/hid/usbhid/hid-core.c. In that file, > > hid_io_error() is called by hid_irq_in(), which is an URB completion > > handler. hid_io_error() acquires usbhid_lock. Therefore it would be > > necessary to audit the usbhid driver to see whether interrupts are > > enabled or disabled any place where usbhid_lock is acquired. And in > > fact, hid_ctrl() (another completion handler) calls > > spin_lock(>lock) -- this could cause problems for you. And > > usbhid->lock is acquired in other places, ones that are not inside > > completion handlers. > > To pick up this example. hid_io_error() is called from process context > (usb_hid_open()) or the completion handler and disables interrupts while > acquiring the lock. hid_ctrl() acquires the lock without disabling the > interrupts. This is not a problem because hid_ctrl() can not be > preempted while it is holding the lock by anything that also wants the > lock. So hid-core could stay as-is and we would be fine. However > lockdep would complain if hid-core would be used on ehci > (softirq/tasklet completion) and ohci (IRQ-handler completion) because > then lockdep would think that hid_ctrl() invoked by ehci could be > preempted by the ohci driver. In any case, it's generally bad form for two routines to obtain the same lock in process context, if one of them uses spin_lock and the other uses spin_lock_irqsave. It indicates that the programmer didn't have a firm grasp on how the lock would be used. > > None of this has anything to do with IRQ usage or hrtimers. > hid-core sets up a timer_list timer hid_retry_timeout(). This timer runs > in softirq context which means it can't preempt the completion handler > (hid_ctrlhid_ctrl()) which does just spin_lock() (both run in BH). > However, if hid_retry_timeout() were a hrtimer timer then it would be > invoked in IRQ-context. In that case it could preempt hid_ctrlhid_ctrl() > and _irqsave() would be required in hid_ctrlhid_ctrl(). > > My counting reveals ~250 USB drivers. I think it is best to audit them > first and make sure that completion handler like hid_ctrl() do And any routines they call, obviously. > _irqsave(). Once that is done, the local_irq_save() in > __usb_hcd_giveback_urb() could go. Agreed. 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: Threaded interrupts for USB HCD instead of tasklets
On 2018-05-22 15:14:17 [-0400], Alan Stern wrote: > Sorry, I don't understand that sentence at all. And I don't see how it > could be relevant to the point I was trying to make. > > Consider, for example, drivers/hid/usbhid/hid-core.c. In that file, > hid_io_error() is called by hid_irq_in(), which is an URB completion > handler. hid_io_error() acquires usbhid_lock. Therefore it would be > necessary to audit the usbhid driver to see whether interrupts are > enabled or disabled any place where usbhid_lock is acquired. And in > fact, hid_ctrl() (another completion handler) calls > spin_lock(>lock) -- this could cause problems for you. And > usbhid->lock is acquired in other places, ones that are not inside > completion handlers. To pick up this example. hid_io_error() is called from process context (usb_hid_open()) or the completion handler and disables interrupts while acquiring the lock. hid_ctrl() acquires the lock without disabling the interrupts. This is not a problem because hid_ctrl() can not be preempted while it is holding the lock by anything that also wants the lock. So hid-core could stay as-is and we would be fine. However lockdep would complain if hid-core would be used on ehci (softirq/tasklet completion) and ohci (IRQ-handler completion) because then lockdep would think that hid_ctrl() invoked by ehci could be preempted by the ohci driver. > None of this has anything to do with IRQ usage or hrtimers. hid-core sets up a timer_list timer hid_retry_timeout(). This timer runs in softirq context which means it can't preempt the completion handler (hid_ctrlhid_ctrl()) which does just spin_lock() (both run in BH). However, if hid_retry_timeout() were a hrtimer timer then it would be invoked in IRQ-context. In that case it could preempt hid_ctrlhid_ctrl() and _irqsave() would be required in hid_ctrlhid_ctrl(). My counting reveals ~250 USB drivers. I think it is best to audit them first and make sure that completion handler like hid_ctrl() do _irqsave(). Once that is done, the local_irq_save() in __usb_hcd_giveback_urb() could go. Sebastian -- 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: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
Hi,
On 2018-06-01 16:08, Mats Karrman wrote:
Hi Peter,
On 2018-06-01 03:30, Peter Chen wrote:
After bisecting usb-next starting from tag 'v4.16' to current head I found
that the commit:
commit 4e88d4c083016454f179686529ae65d70b933b58
Author: Martin Blumenstingl
Date: Sat Mar 3 22:43:03 2018 +0100
usb: add a flag to skip PHY initialization to struct usb_hcd
causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL
SoC with chipidea controller).
Example:
-- Cold boot of system.
-- Plug in USB memory stick --> Attached OK.
-- Mount disk --> OK, "ls" works
-- Unmount disk --> OK
-- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events.
-- Plug in/unplug stick again --> Still nothing.
-- Plug in stick and try mounting --> Somehow causes both detach and attach
of the stick (mounting fails but mounting again now works).
-- All tested USB devices are working in a similar way. I.e first attach
after boot works and then trouble starts.
-- Tested using default device tree from kernel tree
(arch/arm/boot/dts/imx6dl-hummingboard.dts).
Please ask if you think I can help in any way to track down the issue.
Hi Mats,
I have tested the latest usb-next tree, the hot plug works at my imx6sx-sdb
board.
Would you please check the value of hcd->skip_phy_initialization at the end of
function host_start?
hcd->skip_phy_initialization=1
BR // Mats
And this is what the "decompiled" device tree entry for the USB controller and
phy look like:
usb@2184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x2184200 0x200>;
interrupts = <0x0 0x28 0x4>;
clocks = <0x4 0xa2>;
fsl,usbphy = <0x2c>;
fsl,usbmisc = <0x29 0x1>;
dr_mode = "host";
ahb-burst-config = <0x0>;
tx-burst-size-dword = <0x10>;
rx-burst-size-dword = <0x10>;
status = "okay";
disable-over-current;
vbus-supply = <0x2d>;
};
usbphy@20ca000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x20ca000 0x1000>;
interrupts = <0x0 0x2d 0x4>;
clocks = <0x4 0xb7>;
fsl,anatop = <0x2>;
phandle = <0x2c>;
};
So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
// Mats
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Fri, 1 Jun 2018, Anshuman Gupta wrote:
> On Thu, May 31, 2018 at 03:49:46PM -0400, Alan Stern wrote:
> > On Thu, 31 May 2018, Anshuman Gupta wrote:
> >
> > > > + resuming_ports = hcd->driver->get_resuming_ports(hcd);
> > > > + for (i = 0; i < hdev->maxchild; ++i) {
> > > > + if (test_bit(i, _ports)) {
> > > > + struct usb_port *port_dev =
> > > > hub->ports[i];
> > > > +
> > > > + pm_wakeup_event(_dev->dev, 0);
> > > port_dev does not have a wakeup source, there will not any wakeup_count
> > > increment
> > > for port_dev, usb_dev for a particular port has wakeup source
> > > associated with it,
> > > so using port_dev->child usb_dev instead of usb_port device.
> >
> > Okay. However, there might not be a child usb_dev below the port. For
> > example, a wakeup signal could happen as a result of the user plugging
> > in a new device.
> Wake on connect/disconnect wakeup sources are basiclly root hub devices
> i.e. usb_dev
> usb1 and usb2 "/sys/bus/usb/devices/usb1/power/wakeup"
That's right; a newly plugged-in device won't send a wakeup signal.
All right, forget I made that comment.
> > In any case, when the system wakes up because of a signal received by a
> > USB host controller, there should be a wakeup event associated with the
> > host controller. Don't you get those events?
> There are no wakeup attributes for usb_port devices
> Ex. "cat /sys/bus/usb/devices/1-0\:1.0/usb1-port5/power/"
> usb_port devcies are not wakeup capable.
> I think wake on connect pm_wakeup_events should be handle by root hub
> usb_dev.
You misunderstand. I'm talking about host controller devices like
/sys/bus/pci/devices/:00:14.0. Not usb_port devices.
These host controller devices are wakeup capable, and they are the
devices responsible for actually waking up the system when a wakeup
request is received over USB. You should be getting wakeup events from
the controller.
> > Alan Stern
> >
> > PS: Do you have a wakeup-capable USB-3 device? I'm not sure that the
> > patch will work the way you want for USB-3 devices.
> >
> I do not have any USB-3 wake capable device to test, but will try with
> USB type-c mouse,if that will be a USB 3 device,
> I think USB 3.0 pm_wakeup_event will be handle by
> https://elixir.bootlin.com/linux/latest/source/drivers/usb/core/hub.c#L66
A mouse won't be USB-3, not even a mouse with a type-C connector.
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: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
Hi Peter, On 2018-06-01 03:30, Peter Chen wrote: After bisecting usb-next starting from tag 'v4.16' to current head I found that the commit: commit 4e88d4c083016454f179686529ae65d70b933b58 Author: Martin Blumenstingl Date: Sat Mar 3 22:43:03 2018 +0100 usb: add a flag to skip PHY initialization to struct usb_hcd causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL SoC with chipidea controller). Example: -- Cold boot of system. -- Plug in USB memory stick --> Attached OK. -- Mount disk --> OK, "ls" works -- Unmount disk --> OK -- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events. -- Plug in/unplug stick again --> Still nothing. -- Plug in stick and try mounting --> Somehow causes both detach and attach of the stick (mounting fails but mounting again now works). -- All tested USB devices are working in a similar way. I.e first attach after boot works and then trouble starts. -- Tested using default device tree from kernel tree (arch/arm/boot/dts/imx6dl-hummingboard.dts). Please ask if you think I can help in any way to track down the issue. Hi Mats, I have tested the latest usb-next tree, the hot plug works at my imx6sx-sdb board. Would you please check the value of hcd->skip_phy_initialization at the end of function host_start? hcd->skip_phy_initialization=1 BR // Mats -- 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 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue
On Fri, Jun 01, 2018 at 01:14:22PM +0300, Mathias Nyman wrote: > On 01.06.2018 11:23, Greg KH wrote: > > On Wed, May 23, 2018 at 06:41:35PM +0100, Marc Zyngier wrote: > > > Back around the 4.13 timeframe, we tried to address a rather bad issue > > > with the Renesas uPD72020x USB3 controller family. They have trouble > > > with the programming of the base addresses which tend to stick on XHCI > > > reset. This makes transitionning from 64bit to 32bit addresses > > > completely unsafe. This was observed on a fairly popular arm64 > > > platform (AMD Opteron 1100, which has all of its memory above 4GB). > > > > > > The fix was to perform a PCI reset of the device, but we have had > > > multiple reports that this generated regressions (the controller not > > > being usable after the patch was applied). > > > > > > This series reverts the problematic patch, and tries to address it in > > > a more constrained way. If the controller is behind an IOMMU (and only > > > in that case), we zero its base addresses before reseting it. In the > > > affected configuration, this has the effect of putting the device in a > > > state where the XHCI reset will be effective. > > > > > > It must be noted that in the absence of an IOMMU (and maybe even in > > > its presence), this device is completely unsafe, and may silently > > > corrupt memory. > > > > Mathias, any objections for me queueing these up now for 4.18-rc1? > > > > Nope, 4.18-rc1 is fine for me. > Didn't want to bother you with anything this late in the cycle. > The following patch is also ready for 4.18-rc1: > "[PATCH v2] usb: xhci: force all memory allocations to node" > https://marc.info/?l=linux-usb=152701535103950=2 > > Feel free to pick it as well. If not then I'll re-send it after 4.18-rc1 > > For both the Renesas series, and memory allocation to node patch: > > Acked-by: Mathias Nyman Great, will go queue them up now, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue
On 01.06.2018 11:23, Greg KH wrote: On Wed, May 23, 2018 at 06:41:35PM +0100, Marc Zyngier wrote: Back around the 4.13 timeframe, we tried to address a rather bad issue with the Renesas uPD72020x USB3 controller family. They have trouble with the programming of the base addresses which tend to stick on XHCI reset. This makes transitionning from 64bit to 32bit addresses completely unsafe. This was observed on a fairly popular arm64 platform (AMD Opteron 1100, which has all of its memory above 4GB). The fix was to perform a PCI reset of the device, but we have had multiple reports that this generated regressions (the controller not being usable after the patch was applied). This series reverts the problematic patch, and tries to address it in a more constrained way. If the controller is behind an IOMMU (and only in that case), we zero its base addresses before reseting it. In the affected configuration, this has the effect of putting the device in a state where the XHCI reset will be effective. It must be noted that in the absence of an IOMMU (and maybe even in its presence), this device is completely unsafe, and may silently corrupt memory. Mathias, any objections for me queueing these up now for 4.18-rc1? Nope, 4.18-rc1 is fine for me. Didn't want to bother you with anything this late in the cycle. The following patch is also ready for 4.18-rc1: "[PATCH v2] usb: xhci: force all memory allocations to node" https://marc.info/?l=linux-usb=152701535103950=2 Feel free to pick it as well. If not then I'll re-send it after 4.18-rc1 For both the Renesas series, and memory allocation to node patch: Acked-by: Mathias Nyman -Mathias -- 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 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue
On Wed, May 23, 2018 at 06:41:35PM +0100, Marc Zyngier wrote: > Back around the 4.13 timeframe, we tried to address a rather bad issue > with the Renesas uPD72020x USB3 controller family. They have trouble > with the programming of the base addresses which tend to stick on XHCI > reset. This makes transitionning from 64bit to 32bit addresses > completely unsafe. This was observed on a fairly popular arm64 > platform (AMD Opteron 1100, which has all of its memory above 4GB). > > The fix was to perform a PCI reset of the device, but we have had > multiple reports that this generated regressions (the controller not > being usable after the patch was applied). > > This series reverts the problematic patch, and tries to address it in > a more constrained way. If the controller is behind an IOMMU (and only > in that case), we zero its base addresses before reseting it. In the > affected configuration, this has the effect of putting the device in a > state where the XHCI reset will be effective. > > It must be noted that in the absence of an IOMMU (and maybe even in > its presence), this device is completely unsafe, and may silently > corrupt memory. Mathias, any objections for me queueing these up now for 4.18-rc1? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patches for "Renesas uPD72020x workaround" don't work
Hi, this is from another e-mail account, where I now subscribed to this list. Don't know, if this will be of any help. git checkout v4.16 git revert --strategy-option=ours 0e1f0ea -n git cherry-pick 854e55ad289efe7991f0ada5846f5afb9 -n git cherry-pick ad343a98e74e85aa91d844310e797f96fee6983b -n First boot with this kernel, and that Renesas card, did not show up the attached devices. Power off, remove power cable, remove battery, push power button to switch on, card was removed. After that, reconnect all, put the USB3 card into the slot. Boot up. No device recognized. Remove card, reinsert card. device shows. SMART Values are not shown anymore, even on the USB 2.0 ports. I got to use that laptop, don't boot it so often. part from dmesg [0.00] Linux version 4.16.0-ARCH-renesas+ (u...@machine1.example.com) (gcc version 8.1.0 (GCC)) #2 SMP PREEMPT Wed May 30 20:26:31 CEST 2018 [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-linux4.16.0-ARCH-renesas+ "acpi_osi=!Windows 2012" root=/dev/sda12 zswap.enabled=1 zswap.compressor=lz4 rw acpi_osi=Linux threadirqs pcie_aspm=force i915.i915_enable_rc6=1 i915.i915_enable_fbc=1 i915.lvds_downclock=1 drm.debug=0xe intel_iommu=igfx_off intel_iommu=soft pciehp.pciehp_force=1 verbose debug [ 72.915304] xhci_hcd :05:00.0: xHCI Host Controller [ 72.915311] xhci_hcd :05:00.0: new USB bus registered, assigned bus number 3 [ 72.920586] xhci_hcd :05:00.0: hcc params 0x014051cf hci version 0x100 quirks 0x0090 [ 72.921124] hub 3-0:1.0: USB hub found [ 72.922088] hub 3-0:1.0: 2 ports detected [ 72.922680] xhci_hcd :05:00.0: xHCI Host Controller [ 72.922684] xhci_hcd :05:00.0: new USB bus registered, assigned bus number 4 [ 72.924101] usb usb4: We don't know the algorithms for LPM for this host, disabling LPM. [ 72.925733] hub 4-0:1.0: USB hub found [ 72.925744] hub 4-0:1.0: 2 ports detected [ 73.250020] usb 3-1: new high-speed USB device number 2 using xhci_hcd [ 74.273885] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [ 74.870888] usb-storage 3-1:1.0: USB Mass Storage device detected [ 74.871678] scsi host4: usb-storage 3-1:1.0 [ 74.872320] usbcore: registered new interface driver usb-storage [ 74.879429] scsi host5: uas [ 74.880120] usbcore: registered new interface driver uas [ 74.881221] scsi 5:0:0:0: Direct-Access Seagate M3 Portable 9300 PQ: 0 ANSI: 6 [ 74.882418] sd 5:0:0:0: Attached scsi generic sg2 type 0 [ 74.882765] sd 5:0:0:0: [sdc] 7814037167 512-byte logical blocks: (4.00 TB/3.64 TiB) ... [ 75.168568] sd 5:0:0:0: [sdc] Write Protect is off [ 75.171102] sd 5:0:0:0: [sdc] Mode Sense: 4f 00 00 00 [ 75.173896] sd 5:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 75.176015] xhci_hcd :05:00.0: ERROR Transfer event for unknown stream ring slot 3 ep 2 [ 75.177172] xhci_hcd :05:00.0: @0003f4f055f0 1b00 03038001 [ 75.538169] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.540504] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.566508] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.574355] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.575969] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.577960] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.581890] systemd-journald[286]: Successfully sent stream file descriptor to service manager. [ 75.894654] scsi 4:0:0:0: Direct-Access SanDisk Ultra Fit 1.00 PQ: 0 ANSI: 6 [ 75.896727] sd 4:0:0:0: Attached scsi generic sg3 type 0 [ 75.896928] sd 4:0:0:0: [sdd] 60062500 512-byte logical blocks: (30.8 GB/28.6 GiB) [ 75.900691] sd 4:0:0:0: [sdd] Write Protect is off [ 75.901941] sd 4:0:0:0: [sdd] Mode Sense: 43 00 00 00 [ 75.904268] sd 4:0:0:0: [sdd] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA [ 75.913761] sdd: sdd1 sdd2 [ 75.917525] sd 4:0:0:0: [sdd] Attached SCSI removable disk [ 75.988485] sdc: sdc1 sdc2 sdc3 [ 75.991170] sd 5:0:0:0: [sdc] Attached SCSI disk [ 86.713499] xhci_hcd :05:00.0: remove, state 1 [ 86.714589] usb usb4: USB disconnect, device number 1 [ 86.715705] usb 4-2: USB disconnect, device number 2 [ 86.718164] sd 5:0:0:0: [sdc] Synchronizing SCSI cache [ 86.920034] sd 5:0:0:0: [sdc] Synchronize Cache(10) failed: Result: hostbyte=0x07 driverbyte=0x00 [ 92.394569] xhci_hcd :05:00.0: xHCI host controller not responding, assume dead [ 92.395073] xhci_hcd :05:00.0: Timeout while waiting for configure endpoint command [ 92.395237] xhci_hcd :05:00.0: WARN Can't disable streams for endpoint 0x81, streams are being disabled already [ 92.395828] xhci_hcd :05:00.0: USB bus 4 deregistered [
