Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence
On Mon, 2016-04-11 at 08:07 +0300, Felipe Balbi wrote: > Hi, > > chunfeng yun writes: > > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote: > >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote: > >> > NULL pointer dereferrence will happen when class driver > >> > wants to allocate zero length buffer and pool_max[0] > >> > can't be used, so skip reserved pool in this case. > >> > >> Why would a driver want to allocate a 0 length buffer? What driver does > >> this? > > It's misc/usbtest.c > > that'll do what you ask it to do with the userspace tool testusb. Are > you trying to pass a size of 0 ? > No, I just ran "testusb -t10" which called test_ctrl_queue(). In this function, sub-case 8 passed a parameter @len as 0 to simple_alloc_urb(), and then it tried to allocate a 0-length buffer. > >> Shouldn't we fix that issue instead? > > I don't know which way is better, but it seems simple to fix it up in > > buffer.c > > I think we should, really, avoid a 0-length allocation, but passing a > size of 0 to testusb isn't very good either ;-) How are you calling > testusb ? > As explained above. -- 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 v5 0/2] ?hci-platform: Add support for controllers with more then one reset line
Am Samstag, den 09.04.2016, 16:15 +0200 schrieb Hans de Goede: > Hi Greg, et al, > > Here is a resend of my last version of the patch set to support usb > controllers which have multiple resets. > > Unfortunately the shared reset controller support this depends on did > not make it into 4.6. Philipp Zabel, the reset maintainer (in the Cc) > has put these patches in a stable branch / tag here: > > git://git.pengutronix.de/git/pza/linux.git tags/reset-for-4.7 > > Can you please merge in this tag and then these 2 patches for 4.7 ? Acked-by: Philipp Zabel Note that as of now, the tag is not yet merged into the arm-soc maintainer's for-next branch. I've sent the pull request on Mar 31 and expect it to be merged in the next round. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] dt-bindings: phy-mt65xx-usb: add support for mt2701 platform
A new compatible string, "mediatek,mt2701-u3phy", is added. Signed-off-by: Chunfeng Yun --- .../devicetree/bindings/phy/phy-mt65xx-usb.txt |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt index 00100cf..33a2b1e 100644 --- a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt +++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt @@ -4,7 +4,9 @@ mt65xx USB3.0 PHY binding This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC. Required properties (controller (parent) node): - - compatible : should be "mediatek,mt8173-u3phy" + - compatible : should be one of + "mediatek,mt2701-u3phy" + "mediatek,mt8173-u3phy" - reg : offset and length of register for phy, exclude port's register. - clocks : a list of phandle + clock-specifier pairs, one for each -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] phy: phy-mt65xx-usb3: add support for mt2701 platform
A new compatible string, "mediatek,mt2701-u3phy", is added. Some register settings to avoid RX sensitivity level degradation which may arise on mt8173 platform are separated from other platforms. Signed-off-by: Chunfeng Yun --- drivers/phy/Kconfig |5 ++- drivers/phy/phy-mt65xx-usb3.c | 77 - 2 files changed, 54 insertions(+), 28 deletions(-) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 26566db..3037f28 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -218,9 +218,8 @@ config PHY_MT65XX_USB3 depends on ARCH_MEDIATEK && OF select GENERIC_PHY help - Say 'Y' here to add support for Mediatek USB3.0 PHY driver - for mt65xx SoCs. it supports two usb2.0 ports and - one usb3.0 port. + Say 'Y' here to add support for Mediatek USB3.0 PHY driver, + it supports multiple usb2.0 and usb3.0 ports. config PHY_HI6220_USB tristate "hi6220 USB PHY support" diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c index c0e7b4b..4d85e73 100644 --- a/drivers/phy/phy-mt65xx-usb3.c +++ b/drivers/phy/phy-mt65xx-usb3.c @@ -134,6 +134,11 @@ #define U3P_SR_COEF_DIVISOR1000 #define U3P_FM_DET_CYCLE_CNT 1024 +struct mt65xx_phy_pdata { + /* avoid RX sensitivity level degradation only for mt8173 */ + bool avoid_rx_sen_degradation; +}; + struct mt65xx_phy_instance { struct phy *phy; void __iomem *port_base; @@ -145,6 +150,7 @@ struct mt65xx_u3phy { struct device *dev; void __iomem *sif_base; /* include sif2, but exclude port's */ struct clk *u3phya_ref; /* reference clock of usb3 anolog phy */ + const struct mt65xx_phy_pdata *pdata; struct mt65xx_phy_instance **phys; int nphys; }; @@ -241,22 +247,26 @@ static void phy_instance_init(struct mt65xx_u3phy *u3phy, tmp = readl(port_base + U3P_U2PHYACR4); tmp &= ~P2C_U2_GPIO_CTR_MSK; writel(tmp, port_base + U3P_U2PHYACR4); + } - tmp = readl(port_base + U3P_USBPHYACR2); - tmp |= PA2_RG_SIF_U2PLL_FORCE_EN; - writel(tmp, port_base + U3P_USBPHYACR2); - - tmp = readl(port_base + U3D_U2PHYDCR0); - tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON; - writel(tmp, port_base + U3D_U2PHYDCR0); - } else { - tmp = readl(port_base + U3D_U2PHYDCR0); - tmp |= P2C_RG_SIF_U2PLL_FORCE_ON; - writel(tmp, port_base + U3D_U2PHYDCR0); - - tmp = readl(port_base + U3P_U2PHYDTM0); - tmp |= P2C_RG_SUSPENDM | P2C_FORCE_SUSPENDM; - writel(tmp, port_base + U3P_U2PHYDTM0); + if (u3phy->pdata->avoid_rx_sen_degradation) { + if (!index) { + tmp = readl(port_base + U3P_USBPHYACR2); + tmp |= PA2_RG_SIF_U2PLL_FORCE_EN; + writel(tmp, port_base + U3P_USBPHYACR2); + + tmp = readl(port_base + U3D_U2PHYDCR0); + tmp &= ~P2C_RG_SIF_U2PLL_FORCE_ON; + writel(tmp, port_base + U3D_U2PHYDCR0); + } else { + tmp = readl(port_base + U3D_U2PHYDCR0); + tmp |= P2C_RG_SIF_U2PLL_FORCE_ON; + writel(tmp, port_base + U3D_U2PHYDCR0); + + tmp = readl(port_base + U3P_U2PHYDTM0); + tmp |= P2C_RG_SUSPENDM | P2C_FORCE_SUSPENDM; + writel(tmp, port_base + U3P_U2PHYDTM0); + } } tmp = readl(port_base + U3P_USBPHYACR6); @@ -318,7 +328,7 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy, tmp |= XC3_RG_U3_XTAL_RX_PWD | XC3_RG_U3_FRC_XTAL_RX_PWD; writel(tmp, u3phy->sif_base + U3P_XTALCTL3); - /* [mt8173]switch 100uA current to SSUSB */ + /* switch 100uA current to SSUSB */ tmp = readl(port_base + U3P_USBPHYACR5); tmp |= PA5_RG_U2_HS_100U_U3_EN; writel(tmp, port_base + U3P_USBPHYACR5); @@ -335,7 +345,7 @@ static void phy_instance_power_on(struct mt65xx_u3phy *u3phy, tmp |= PA5_RG_U2_HSTX_SRCTRL_VAL(4); writel(tmp, port_base + U3P_USBPHYACR5); - if (index) { + if (u3phy->pdata->avoid_rx_sen_degradation && index) { tmp = readl(port_base + U3D_U2PHYDCR0); tmp |= P2C_RG_SIF_U2PLL_FORCE_ON; writel(tmp, port_base + U3D_U2PHYDCR0); @@ -386,7 +396,9 @@ static void phy_instance_power_off(struct mt65xx_u3phy *u3phy, tmp = readl(port_base + U3P_U3_PHYA_REG0); tmp &= ~P3A_RG_U3_VUSB10_ON; writel(tmp, port_base + U3P_U3_PHYA_REG0); - } else { + } + + if (u3phy->pdata->avoid_rx_sen_degradation && index) {
Re: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
changbin...@intel.com writes: > From: "Du, Changbin" > > dwc->regset is allocated on dwc3_debugfs_init, and should > be released on dwc3_debugfs_exit. > > Signed-off-by: Du, Changbin > --- > This patch is seperated from patch set: > [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs > Because it is not related to other 2 patches. > > --- > drivers/usb/dwc3/debugfs.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c > index 9ac37fe..9eeb444 100644 > --- a/drivers/usb/dwc3/debugfs.c > +++ b/drivers/usb/dwc3/debugfs.c > @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc) > { > debugfs_remove_recursive(dwc->root); > dwc->root = NULL; > + > + kfree(dwc->regset); we also need a kfree() on dwc3_debugfs_init(). > + dwc->regset = NULL; setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit() when removing the driver. -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void
Hi, changbin...@intel.com writes: > diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h > index 07fbc2d..71e3180 100644 > --- a/drivers/usb/dwc3/debug.h > +++ b/drivers/usb/dwc3/debug.h > @@ -217,11 +217,11 @@ static inline const char > *dwc3_gadget_event_type_string(u8 event) > void dwc3_trace(void (*trace)(struct va_format *), const char *fmt, ...); > > #ifdef CONFIG_DEBUG_FS > -extern int dwc3_debugfs_init(struct dwc3 *); > +extern void dwc3_debugfs_init(struct dwc3 *); > extern void dwc3_debugfs_exit(struct dwc3 *); > #else > -static inline int dwc3_debugfs_init(struct dwc3 *d) > -{ return 0; } > +static inline void dwc3_debugfs_init(struct dwc3 *d) > +{ } > static inline void dwc3_debugfs_exit(struct dwc3 *d) > { } > #endif > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c > index 9ac37fe..071b286 100644 > --- a/drivers/usb/dwc3/debugfs.c > +++ b/drivers/usb/dwc3/debugfs.c > @@ -618,69 +618,39 @@ static const struct file_operations > dwc3_link_state_fops = { > .release= single_release, > }; > > -int dwc3_debugfs_init(struct dwc3 *dwc) > +void dwc3_debugfs_init(struct dwc3 *dwc) > { > struct dentry *root; > - struct dentry *file; > - int ret; > > root = debugfs_create_dir(dev_name(dwc->dev), NULL); > - if (!root) { > - ret = -ENOMEM; > - goto err0; > - } > + if (IS_ERR_OR_NULL(root)) > + return; We can definitely keep on going, but I'd still like to know that we enabled CONFIG_DEBUG_FS but failed to create a file or a directory. Seems like this should read as follows: if (IS_ERR_OR_NULL(root)) { if (!root) dev_err(dwc->dev, "Can't create debugfs root\n"); return; } ditto to all bellow. > dwc->root = root; > > dwc->regset = kzalloc(sizeof(*dwc->regset), GFP_KERNEL); > if (!dwc->regset) { > - ret = -ENOMEM; > - goto err1; > + debugfs_remove_recursive(root); you're now duplicating debugfs_remove_recursive(root) in all braches and that's error prone. It's probably better to keep our gotos, but change them so they read as follows: if (!dwc->regset) goto err1; [...] return; /* this is our successful exit point */ err1: debugfs_remove_recursive(root); kfree(dwc->regset); -- balbi signature.asc Description: PGP signature
Re: XHCI is slow during boot (bios/efi) and leaves many dmesg messages
Hey all, On 11-04-16 07:12, Felipe Balbi wrote: Hi, Janna Martl writes: On 2016-04-04 9:06:28, Olliver Schinagl wrote: Hi list, I have a Apple Inc. MacBookPro11,1 (with the most recent 'bios': BIOS MBP111.88Z.0138.B16.1509081438 09/08/2015). At the beginning, USB worked normally. After a while (and after newer kernel versions released by debian?) things started to act strangely. For one, the you need to try with latest kernel from Linus. Please download v4.6-rc3 from kernel.org, compile and try with that. I've tried with kernels from 3.16 to v4.6-rc2 updating every few weeks. Anything special I should watch out for in rc3 that changed since rc2? bios/efi boot takes a very long time (probably due to the same reason I describe later) just to get to the bootloader/grub. Likley resetting and probing for USB ports/mass storage. When grub finally pops up, I can use the (internal USB based keyboard) normally to select a grub entry etc. Booting the kernel then works reasonably fine, until it loads the xhci module. It spews some messages in dmesg (taking some 15 seconds) and only then, the keyboard starts to work again. which messages ? We need these dmesg messages As mentioned by Janna, they are at the bugzilla issue number 115741 [0] (unless you prefer them zipped otherwise they sometimes don't fit). I have the same hardware and am having the same problem. For me, it works properly about half the time. I've discovered two things that might be helpful: (1) When I don't have the problem, I have an efivars entry /sys/firmware/efi/efivars/usb-cr-rec-7c436110-ab2a-4bbb-a880-fe41995c9f82 and when I do have the problem, this entry is not present. (AFAICT this is the only thing that changes in efivars, except for something that looks like a boot counter.) For the record: $ hexdump usb-cr-rec-7c436110-ab2a-4bbb-a880-fe41995c9f82 000 0006 8000 0001 008 (2) Unlike (1) this is not a 100% correlation, but usually when it works properly, my internal keyboard turns up as /dev/input/event5, and when there's a delay, it turns up as event12, event13, or event14 (and this sometimes this is a consequence of XHCI taking too much time to register... And re-registering all its input devices. Since I keep my machine mostly suspended and not reboot that often, it has been quite a while that I had a successfull boot. I'll try to find some time to do a few boot loops (alternating with booting OSX in the hope that it resets the xhci controller properly). and check the content of that file. changes after suspend, when there's also a delay correlated to the boot delay). From /proc/bus/input/devices when it works: I: Bus=0003 Vendor=05ac Product=0259 Version=0111 N: Name="Apple Inc. Apple Internal Keyboard / Trackpad" P: Phys=usb-:00:14.0-5/input0 ... note that your keyboard is just a USB device. [0]https://bugzilla.kernel.org/show_bug.cgi?id=115741 -- 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 01/16] dt: davinci: use proper address after @
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > TI has been using the physical address in DT after the @ in device nodes. > The device tree convention is to use the same address that is used for > the reg property. This updates all davinci DT files to use the proper > convention. > > Signed-off-by: David Lechner > --- > > v3 changes: This is a new patch. Applied through a copy of this patch posted by Petr in thread "ARM: DTS: da850: add node for i2c1" Thanks, Sekhar -- 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 is slow during boot (bios/efi) and leaves many dmesg messages
Hi, Janna Martl writes: >> > On 2016-04-04 9:06:28, Olliver Schinagl wrote: >> >> It spews some messages in dmesg (taking some 15 seconds) and only then, >> >> the >> >> keyboard starts to work again. >> >> which messages ? We need these dmesg messages > > Olliver posted his dmesg here [0] (I excerpted his message when replying > to it). I tried with 4.6.0-rc3 and the relevant messages are: okay, thanks :) > Apr 11 01:51:54 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:51:54 kernel: usb 1-3: hub failed to enable device, error -62 > Apr 11 01:51:59 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:00 kernel: usb 1-3: new full-speed USB device number 3 using > xhci_hcd > Apr 11 01:52:00 kernel: usb 2-3: device not accepting address 2, error -62 > Apr 11 01:52:00 kernel: hub 1-3:1.0: USB hub found > Apr 11 01:52:00 kernel: hub 1-3:1.0: 3 ports detected > Apr 11 01:52:00 kernel: usb 1-5: new full-speed USB device number 4 using > xhci_hcd > Apr 11 01:52:00 kernel: usb 1-3.1: new full-speed USB device number 5 using > xhci_hcd > Apr 11 01:52:00 kernel: usb 1-3.2: new full-speed USB device number 6 using > xhci_hcd > Apr 11 01:52:00 kernel: usb 1-3.3: new full-speed USB device number 7 using > xhci_hcd > Apr 11 01:52:05 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:11 kernel: xhci_hcd :00:14.0: Command completion event does > not match command > Apr 11 01:52:11 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:11 kernel: usb 2-3: device not accepting address 3, error -62 > Apr 11 01:52:13 kernel: random: nonblocking pool is initialized > Apr 11 01:52:22 kernel: xhci_hcd :00:14.0: Command completion event does > not match command > Apr 11 01:52:22 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:27 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:27 kernel: usb 2-3: device not accepting address 4, error -62 > Apr 11 01:52:33 kernel: xhci_hcd :00:14.0: Command completion event does > not match command > Apr 11 01:52:33 kernel: xhci_hcd :00:14.0: Timeout while waiting for > setup device command > Apr 11 01:52:38 kernel: usb 2-3: device not accepting address 5, error -62 > Apr 11 01:52:38 kernel: usb usb2-port3: unable to enumerate USB device > Apr 11 01:52:38 kernel: input: bcm5974 as > /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.2/input/input12 > Apr 11 01:52:38 kernel: hidraw: raw HID events driver (C) Jiri Kosina > Apr 11 01:52:38 kernel: usbcore: registered new interface driver bcm5974 > Apr 11 01:52:38 kernel: usbcore: registered new interface driver usbhid > Apr 11 01:52:38 kernel: usbhid: USB HID core driver > Apr 11 01:52:38 kernel: mousedev: PS/2 mouse device common for all mice > Apr 11 01:52:38 kernel: input: Apple Inc. Apple Internal Keyboard / Trackpad > as > /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/0003:05AC:0259.0001/input/input13 > Apr 11 01:52:38 kernel: usbcore: registered new interface driver btusb > Apr 11 01:52:38 kernel: apple 0003:05AC:0259.0001: input,hidraw0: USB HID > v1.11 Keyboard [Apple Inc. Apple Internal Keyboard / Trackpad] on > usb-:00:14.0-5/input0 > Apr 11 01:52:38 kernel: apple 0003:05AC:0259.0002: hidraw1: USB HID v1.11 > Device [Apple Inc. Apple Internal Keyboard / Trackpad] on > usb-:00:14.0-5/input1 > Apr 11 01:52:38 kernel: usb 1-3.1: USB disconnect, device number 5 > > I can post a full log if you want. not needed, this is good enough. I was chatting with Mathias about this and it really seems like LPM is what's causing problems here. > Also, once in a while it never finds my keyboard, and I get messages: > > Jan 07 03:03:46 kernel: xhci_hcd :00:14.0: Secondary root hub is not > suspended > Jan 07 03:03:46 kernel: pci_pm_runtime_suspend(): > hcd_pci_runtime_suspend+0x0/0x60 [usbcore] returns -16 hmm, this is just telling you that the host controller can't be suspended right now. Shouldn't be harmful :-s -- balbi signature.asc Description: PGP signature
Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence
Hi, chunfeng yun writes: >> > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote: >> >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote: >> >> > NULL pointer dereferrence will happen when class driver >> >> > wants to allocate zero length buffer and pool_max[0] >> >> > can't be used, so skip reserved pool in this case. >> >> >> >> Why would a driver want to allocate a 0 length buffer? What driver does >> >> this? >> > It's misc/usbtest.c >> >> that'll do what you ask it to do with the userspace tool testusb. Are >> you trying to pass a size of 0 ? >> > No, I just ran "testusb -t10" which called test_ctrl_queue(). > In this function, sub-case 8 passed a parameter @len as 0 to > simple_alloc_urb(), and then it tried to allocate a 0-length buffer. odd, I have never seen this problem running testusb with the same arguments. -- balbi signature.asc Description: PGP signature
Re: XHCI is slow during boot (bios/efi) and leaves many dmesg messages
Hi, Olliver Schinagl writes: > Hey all, > > On 11-04-16 07:12, Felipe Balbi wrote: >> Hi, >> >> Janna Martl writes: >>> On 2016-04-04 9:06:28, Olliver Schinagl wrote: Hi list, I have a Apple Inc. MacBookPro11,1 (with the most recent 'bios': BIOS MBP111.88Z.0138.B16.1509081438 09/08/2015). At the beginning, USB worked normally. After a while (and after newer kernel versions released by debian?) things started to act strangely. For one, the >> you need to try with latest kernel from Linus. Please download v4.6-rc3 >> from kernel.org, compile and try with that. > I've tried with kernels from 3.16 to v4.6-rc2 updating every few weeks. > Anything special I should watch out for in rc3 that changed since rc2? nothing special. Just to confirm, can you check that disabling LPM solves the problem ? diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index f0640b7a1c42..9c3ead114ad5 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -127,8 +127,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) xhci->quirks |= XHCI_TRUST_TX_LENGTH; if (pdev->vendor == PCI_VENDOR_ID_INTEL) { - xhci->quirks |= XHCI_LPM_SUPPORT; - xhci->quirks |= XHCI_INTEL_HOST; + /* xhci->quirks |= XHCI_LPM_SUPPORT; */ + /* xhci->quirks |= XHCI_INTEL_HOST; */ xhci->quirks |= XHCI_AVOID_BEI; } if (pdev->vendor == PCI_VENDOR_ID_INTEL && bios/efi boot takes a very long time (probably due to the same reason I describe later) just to get to the bootloader/grub. Likley resetting and probing for USB ports/mass storage. When grub finally pops up, I can use the (internal USB based keyboard) normally to select a grub entry etc. Booting the kernel then works reasonably fine, until it loads the xhci module. It spews some messages in dmesg (taking some 15 seconds) and only then, the keyboard starts to work again. >> which messages ? We need these dmesg messages > As mentioned by Janna, they are at the bugzilla issue number 115741 [0] > (unless you prefer them zipped otherwise they sometimes don't fit). no, it's okay. -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 02/16] mfd: da8xx-cfgchip: New header file for CFGCHIP registers.
On Monday 28 March 2016 10:12 PM, Sergei Shtylyov wrote: > Hello. > > On 03/28/2016 06:02 PM, David Lechner wrote: > +/* register offsets */ +#define CFGCHIP_REG(n)(n * 4) +#define CFGCHIP0_REGCFGCHIP_REG(0) +#define CFGCHIP1_REGCFGCHIP_REG(1) +#define CFGCHIP2_REGCFGCHIP_REG(2) +#define CFGCHIP3_REGCFGCHIP_REG(3) +#define CFGCHIP4_REGCFGCHIP_REG(4) >>> >>> Why not just use CFGCHIP_REG(n) directly? >> >> I considered that, but I went this way because A) the TRM uses, for >> example, >> "CFGCHIP2", so I wanted to keep "CFGCHIP" and "2" together IMO, this is not that big of an issue. Anyone reading should be able to make out that CFGCHIP_REG(0) is same as CFGCHIP0 referred to in the TRM. > >I'd just drop the _REG suffix. > >> and B) this tells >> you how many CFGCHIP registers there are, i.e. there is no CFGCHIP5_REG. > >You can tell that in a comment. Having a parametrized macro and using > it to just #define more macros doesn't appeal to me at all... Agree with Sergei, I don't prefer the additional #defines as well. Regards, Sekhar -- 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] drivers/usb/gadget/udc/r8a66597-udc.c: Deinline pipe_change, save 2176 bytes
Hi again, Felipe Balbi writes: > Denys Vlasenko writes: >> This function compiles to 298 bytes of machine code, has ~10 callsites. > > fair enough > >> This is a USB 2.0 device, USB 2.0 is limited to 35 MB/s, so should be > > it's not limited to 35MB/sec, sorry. USB 2.0 has a theoretical maximum > of 60MB/sec. But 44MB/sec is what people consider 'possible'. I've > gotten to 41MB/sec with g_mas_storage gadget. also, you need to break you commit log lines at 72 characters. Please, fix and resend -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 03/16] ARM: davinici: da8xx: move usb code to new file
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > We will be adding more da8xx-specific code for phy and clocks, so it will > be better to have this in a separate file. This way we don't have a bunch > of #ifdefs for all of the da8xx stuff. > > Signed-off-by: David Lechner Applied this patch with a couple of local changes. 1) The subject line has a typo: s/davinici/davinci 2) There are checkpatch warnings in new code that are coming from old code. Even though this is just code movement, I prefer new code is free of checkpatch warnings. Here is the updated patch. Thanks, Sekhar ---8<--- From: David Lechner Date: Thu, 24 Mar 2016 18:51:28 -0500 Subject: [PATCH] ARM: davinci: da8xx: move usb code to new file We will be adding more da8xx-specific code for phy and clocks, so it will be better to have this in a separate file. This way we don't have a bunch of #ifdefs for all of the da8xx stuff. While at it, fix some checkpatch warnings coming from existing code. Signed-off-by: David Lechner [nsek...@ti.com: typo and checkpatch fixes] Signed-off-by: Sekhar Nori --- arch/arm/mach-davinci/Makefile| 4 +- arch/arm/mach-davinci/usb-da8xx.c | 124 ++ arch/arm/mach-davinci/usb.c | 74 +-- 3 files changed, 127 insertions(+), 75 deletions(-) create mode 100644 arch/arm/mach-davinci/usb-da8xx.c diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile index 2e3464b8bab4..da4c336b4637 100644 --- a/arch/arm/mach-davinci/Makefile +++ b/arch/arm/mach-davinci/Makefile @@ -14,8 +14,8 @@ obj-$(CONFIG_ARCH_DAVINCI_DM644x) += dm644x.o devices.o obj-$(CONFIG_ARCH_DAVINCI_DM355)+= dm355.o devices.o obj-$(CONFIG_ARCH_DAVINCI_DM646x) += dm646x.o devices.o obj-$(CONFIG_ARCH_DAVINCI_DM365) += dm365.o devices.o -obj-$(CONFIG_ARCH_DAVINCI_DA830)+= da830.o devices-da8xx.o -obj-$(CONFIG_ARCH_DAVINCI_DA850)+= da850.o devices-da8xx.o +obj-$(CONFIG_ARCH_DAVINCI_DA830) += da830.o devices-da8xx.o usb-da8xx.o +obj-$(CONFIG_ARCH_DAVINCI_DA850) += da850.o devices-da8xx.o usb-da8xx.o obj-$(CONFIG_AINTC)+= irq.o obj-$(CONFIG_CP_INTC) += cp_intc.o diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c new file mode 100644 index ..0cab01f79825 --- /dev/null +++ b/arch/arm/mach-davinci/usb-da8xx.c @@ -0,0 +1,124 @@ +/* + * DA8xx USB + */ +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#define DA8XX_USB0_BASE0x01e0 +#define DA8XX_USB1_BASE0x01e25000 + +#if IS_ENABLED(CONFIG_USB_MUSB_HDRC) + +static struct musb_hdrc_eps_bits musb_eps[] = { + { "ep1_tx", 8, }, + { "ep1_rx", 8, }, + { "ep2_tx", 8, }, + { "ep2_rx", 8, }, + { "ep3_tx", 5, }, + { "ep3_rx", 5, }, + { "ep4_tx", 5, }, + { "ep4_rx", 5, }, +}; + +static struct musb_hdrc_config musb_config = { + .multipoint = true, + .dyn_fifo = true, + .soft_con = true, + .dma= true, + + .num_eps= 5, + .dma_channels = 8, + .ram_bits = 10, + .eps_bits = musb_eps, +}; + +static struct musb_hdrc_platform_data usb_data = { + /* OTG requires a Mini-AB connector */ + .mode = MUSB_OTG, + .clock = "usb20", + .config = &musb_config, +}; + +static struct resource da8xx_usb20_resources[] = { + { + .start = DA8XX_USB0_BASE, + .end= DA8XX_USB0_BASE + SZ_64K - 1, + .flags = IORESOURCE_MEM, + }, + { + .start = IRQ_DA8XX_USB_INT, + .flags = IORESOURCE_IRQ, + .name = "mc", + }, +}; + +static u64 usb_dmamask = DMA_BIT_MASK(32); + +static struct platform_device usb_dev = { + .name = "musb-da8xx", + .id = -1, + .dev = { + .platform_data = &usb_data, + .dma_mask = &usb_dmamask, + .coherent_dma_mask = DMA_BIT_MASK(32), + }, + .resource = da8xx_usb20_resources, + .num_resources = ARRAY_SIZE(da8xx_usb20_resources), +}; + +int __init da8xx_register_usb20(unsigned int mA, unsigned int potpgt) +{ + usb_data.power = mA > 510 ? 255 : mA / 2; + usb_data.potpgt = (potpgt + 1) / 2; + + return platform_device_register(&usb_dev); +} + +#else + +int __init da8xx_register_usb20(unsigned int mA, unsigned int potpgt) +{ + return 0; +} + +#endif /* CONFIG_USB_MUSB_HDRC */ + +static struct resource da8xx_usb11_resources[] = { + [0] = { + .start = DA8XX_USB1_BASE, + .end= DA8XX_USB1_BASE + SZ_4K - 1, + .flags = IORESOURCE_MEM, + }, + [1] =
[-next] BUG_ON in scsi_target_destroy()
Hello, commit 7b106f2de6938c31ce5e9c86bc70ad3904666b96 Author: Johannes Thumshirn Date: Tue Apr 5 11:50:44 2016 +0200 scsi: Add intermediate STARGET_REMOVE state to scsi_target_state BUG_ON()s (next-20160411) each time I remove a usb flash [ 49.561600] [] scsi_target_destroy+0x5a/0xcb [scsi_mod] [ 49.561607] [] scsi_target_reap+0x4a/0x4f [scsi_mod] [ 49.561613] [] __scsi_remove_device+0xc3/0xd0 [scsi_mod] [ 49.561619] [] scsi_forget_host+0x52/0x63 [scsi_mod] [ 49.561623] [] scsi_remove_host+0x8c/0x102 [scsi_mod] [ 49.561627] [] usb_stor_disconnect+0x6b/0xab [usb_storage] [ 49.561634] [] usb_unbind_interface+0x77/0x1ca [usbcore] [ 49.561636] [] __device_release_driver+0x9d/0x121 [ 49.561638] [] device_release_driver+0x23/0x30 [ 49.561639] [] bus_remove_device+0xfb/0x10e [ 49.561641] [] device_del+0x164/0x1e6 [ 49.561648] [] ? remove_intf_ep_devs+0x3b/0x48 [usbcore] [ 49.561655] [] usb_disable_device+0x84/0x1a5 [usbcore] [ 49.561661] [] usb_disconnect+0x94/0x19f [usbcore] [ 49.561667] [] hub_event+0x5c1/0xdea [usbcore] [ 49.561670] [] process_one_work+0x1dc/0x37f [ 49.561672] [] worker_thread+0x282/0x36d [ 49.561673] [] ? rescuer_thread+0x2ae/0x2ae [ 49.561675] [] kthread+0xd2/0xda [ 49.561678] [] ret_from_fork+0x22/0x40 [ 49.561679] [] ? kthread_worker_fn+0x13e/0x13e -ss -- 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 04/16] ARM: davinci: Move clock init after ioremap.
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > Some clocks (such as the USB PHY clocks in DA8xx) will need to use iomem. > The davinci_common_init() function must be called before the ioremap, so > the clock init is now split out as separate function. > > Signed-off-by: David Lechner > --- > > v3 changes: This is a new patch. It takes care of the issue of unwanted > ioremap > in clock set_parent functions. > > > arch/arm/mach-davinci/clock.c | 4 ++-- > arch/arm/mach-davinci/clock.h | 7 ++- > arch/arm/mach-davinci/common.c | 6 -- > arch/arm/mach-davinci/da830.c | 2 ++ > arch/arm/mach-davinci/da850.c | 2 ++ > arch/arm/mach-davinci/dm355.c | 1 + > arch/arm/mach-davinci/dm365.c | 1 + > arch/arm/mach-davinci/dm644x.c | 1 + > arch/arm/mach-davinci/dm646x.c | 1 + > 9 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c > index 3424eac6..a5c2629 100644 > --- a/arch/arm/mach-davinci/clock.c > +++ b/arch/arm/mach-davinci/clock.c > @@ -560,7 +560,7 @@ EXPORT_SYMBOL(davinci_set_pllrate); > * than that used by default in .c file. The reference clock rate > * should be updated early in the boot process; ideally soon after the > * clock tree has been initialized once with the default reference clock > - * rate (davinci_common_init()). > + * rate (davinci_clk_init()). > * > * Returns 0 on success, error otherwise. > */ > @@ -581,7 +581,7 @@ int davinci_set_refclk_rate(unsigned long rate) > return 0; > } > > -int __init davinci_clk_init(struct clk_lookup *clocks) > +int __init _davinci_clk_init(struct clk_lookup *clocks) > { > struct clk_lookup *c; > struct clk *clk; > diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h > index 1e4e836..8b0fbbe 100644 > --- a/arch/arm/mach-davinci/clock.h > +++ b/arch/arm/mach-davinci/clock.h > @@ -124,7 +124,12 @@ struct clk { > .clk = ck, \ > } \ > > -int davinci_clk_init(struct clk_lookup *clocks); > +int _davinci_clk_init(struct clk_lookup *clocks); > +static inline void davinci_clk_init(struct davinci_soc_info *soc_info) > +{ > + if (soc_info->cpu_clks && _davinci_clk_init(soc_info->cpu_clks)) > + panic("davinci_clk_init: Failed to init clocks.\n"); > +} I am not sure about the need for an additional API (_davinci_clk_init) especially when you end up exposing both through clock.h. Just make make calls from SoC files like: davinci_clk_init(davinci_soc_info_da830.cpu_clks); I would ignore the panic() call too since davinci_clk_init() does not really return an error (never has). Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] dt-bindings: phy-mt65xx-usb: add support for mt2701 platform
add Matthias sorry On Mon, 2016-04-11 at 15:41 +0800, Chunfeng Yun wrote: > A new compatible string, "mediatek,mt2701-u3phy", is added. > > Signed-off-by: Chunfeng Yun > --- > .../devicetree/bindings/phy/phy-mt65xx-usb.txt |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt > b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt > index 00100cf..33a2b1e 100644 > --- a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt > +++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt > @@ -4,7 +4,9 @@ mt65xx USB3.0 PHY binding > This binding describes a usb3.0 phy for mt65xx platforms of Medaitek SoC. > > Required properties (controller (parent) node): > - - compatible: should be "mediatek,mt8173-u3phy" > + - compatible: should be one of > + "mediatek,mt2701-u3phy" > + "mediatek,mt8173-u3phy" > - reg : offset and length of register for phy, exclude port's > register. > - clocks: a list of phandle + clock-specifier pairs, one for each -- 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 05/16] ARM: davinci: add set_parent callback for mux clocks
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > Introduce a set_parent callback that will be used for mux clocks, such as > the USB PHY muxes and the async3 clock domain mux. > > Signed-off-by: David Lechner > --- > > v3 changes: none. > > > arch/arm/mach-davinci/clock.c | 17 - > arch/arm/mach-davinci/clock.h | 1 + > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c > index a5c2629..dfc2eb3 100644 > --- a/arch/arm/mach-davinci/clock.c > +++ b/arch/arm/mach-davinci/clock.c > @@ -195,6 +195,13 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > return -EINVAL; > > mutex_lock(&clocks_mutex); > + if (clk->set_parent) { > + int ret = clk->set_parent(clk, parent); Need empty line here. > + if (ret) { > + mutex_unlock(&clocks_mutex); > + return ret; > + } > + } > clk->parent = parent; > list_del_init(&clk->childnode); > list_add(&clk->childnode, &clk->parent->children); > @@ -224,8 +231,16 @@ int clk_register(struct clk *clk) > > mutex_lock(&clocks_mutex); > list_add_tail(&clk->node, &clocks); > - if (clk->parent) > + if (clk->parent) { > + if (clk->set_parent) { > + int ret = clk->set_parent(clk, clk->parent); Here too. Applying this patch with these local changes. BTW, checkpatch complained about these. Please try to address all warnings unless there is a good reason not to fix one. Thanks, Sekhar -- 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 06/16] ARM: davinci: da850: use clk->set_parent for async3
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > The da850 family of processors has an async3 clock domain that can be > muxed to either pll0_sysclk2 or pll1_sysclk2. Now that the davinci clocks > have a set_parent callback, we can use this to control the async3 mux > instead of a stand-alone function. > > This adds a new async3_clk and sets the appropriate child clocks. The > default is use to pll1_sysclk2 since it is not affected by processor > frequency scaling. > > Signed-off-by: David Lechner > +static int da850_async3_set_parent(struct clk *clk, struct clk *parent) > +{ > + u32 val; > + > + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG)); > + > + /* Set the Async3 clock domain mux based on the parent clock. */ > + if (parent == &pll0_sysclk2) > + val &= ~CFGCHIP3_ASYNC3_CLKSRC; > + else if (parent == &pll1_sysclk2) > + val |= CFGCHIP3_ASYNC3_CLKSRC; > + else { > + pr_err("Bad parent on async3 clock mux.\n"); > + return -EINVAL; > + } Since else has braces, need braces on all arm of the if-else construct. Applied this patch with this fixed locally. checkpatch complains about this too when --strict option is passed. Thanks, Sekhar -- 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 07/12] usb: otg: add OTG/dual-role core
On 08/04/16 14:22, Yoshihiro Shimoda wrote: > Hi, > >> From: Roger Quadros >> Sent: Thursday, April 07, 2016 8:45 PM >> >> Hi, >> >> On 07/04/16 11:52, Yoshihiro Shimoda wrote: >>> Hi, >>> From: Roger Quadros Sent: Tuesday, April 05, 2016 11:05 PM > < snip > >>> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c >>> index 41e762a..4d7f043 100644 >>> --- a/drivers/usb/common/usb-otg.c >>> +++ b/drivers/usb/common/usb-otg.c >>> @@ -825,11 +825,16 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, >>> unsigned int irqnum, >>> if (otg->primary_hcd.hcd) { >>> /* probably a shared HCD ? */ >>> if (usb_otg_hcd_is_primary_hcd(hcd)) { >>> + if (hcd->driver->flags & HCD_USB11) { >>> + dev_info(otg_dev, "this assumes usb 1.1 hc is >>> as shared_hcd\n"); >>> + goto check_shared_hcd; >>> + } >>> dev_err(otg_dev, "otg: primary host already >>> registered\n"); >>> goto err; >>> } >>> >>> if (hcd->shared_hcd == otg->primary_hcd.hcd) { >>> +check_shared_hcd: >>> if (otg->shared_hcd.hcd) { >>> dev_err(otg_dev, "otg: shared host already >>> registered\n"); >>> goto err; >>> >>> What do you think this local hack? >> >> Is it guaranteed that EHCI hcd registers before OHCI hcd? > > Thank you for the comment. No, it is not guaranteed. > >> If not we need to improve the code still. >> We will also need to remove the constraint that primary_hcd must be >> registered >> first in usb_otg_register_hcd(). I think that constraint is no longer >> needed anyways. > > I got it. > So, I made a patch to avoid the constraint using an additional property > "otg-hcds". > The patch is in this end of email. What do you think about the patch? This might only solve the issue for device tree but what about non-device tree? We need to deal with both cases. > >> If EHCI & OHCI HCDs register before OTG driver then things will break unless >> we fix usb_otg_hcd_wait_add(). We need to add this HCD_USB11 check there to >> populate wait->shared_hcd. > > I see. In my environment, since EHCI & OHCI HCDs need phy driver and phy > driver > will calls usb_otg_register(), the order is right. In other words, > EHCI & OHCI HCDs never register before OTG driver because even if EHCI driver > is probed first, the driver will be deferred (EHCI driver needs the phy > driver). But still we cannot assume this is true for all platforms. OTG driver can also be a separate entity than PHY driver. > >>> I also wonder if array of hcd may be good for both xHCI and EHCI/OHCI. >>> For example of xHCI: >>> - otg->hcds[0] = primary_hcd >>> - otg->hcds[1] = shared_hcd >>> >>> For example of EHCI/OHCI: >>> - otg->hcds[0] = primary_hcd of EHCI >>> - otg->hcds[1] = primary_hcd of OHCI >> >> The bigger problem is that how do we know in the OHCI/EHCI case that we need >> to wait >> for both of them before starting the OTG FSM? >> Some systems might use just OHCI or just EHCI. >> >> There is no guarantee that OTG driver registers before the HCDs so this piece >> of information must come from the HCD itself. i.e. whether it needs a >> companion or not. > > I understood these problems. I wonder if my patch can resolve these problems. > > Best regards, > Yoshihiro Shimoda > --- > diff --git a/Documentation/devicetree/bindings/usb/generic.txt > b/Documentation/devicetree/bindings/usb/generic.txt > index f6866c1..518b9eb 100644 > --- a/Documentation/devicetree/bindings/usb/generic.txt > +++ b/Documentation/devicetree/bindings/usb/generic.txt > @@ -27,6 +27,12 @@ Optional properties: > - otg-controller: phandle to otg controller. Host or gadget controllers can > contain this property to link it to a particular OTG > controller. > + - otg-hcds: phandle to host controller(s). An OTG controller can contain > this > + property. The first one is as "primary", and (optional) the second > + one is as "shared". > + - For example for xHCI:otg-hcds = <&xhci>, <&xhci>; > + - For example for EHCI/OHCI: otg-hcds = <&ehci>, <&ohci>; > + - For example for just EHCI: otg-hcds = <&ehci>; > This is kind of duplicating the information. We are already specifying in the hcd nodes as to which otg controller it is linked to. Instead, how about just adding a boolean property in the otg node saying that HCD needs companion. e.g. hcd-needs-compainion: must be present if otg controller is dealing with EHCI host controller that needs a companion OHCI host controller. This can also be added in struct usb_otg_config so that it can deal with non-DT cases. So if this property is true, the OTG core will wait for 2 primary HCDs to be registered. cheers, -roger >
RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
> > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c > > index 9ac37fe..9eeb444 100644 > > --- a/drivers/usb/dwc3/debugfs.c > > +++ b/drivers/usb/dwc3/debugfs.c > > @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc) > > { > > debugfs_remove_recursive(dwc->root); > > dwc->root = NULL; > > + > > + kfree(dwc->regset); > > we also need a kfree() on dwc3_debugfs_init(). This patch is based on the patch set [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void> So, they do has dependency. :) > > + dwc->regset = NULL; > > setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit() > when removing the driver. > > -- > Balbi I'd like keep this line even it is unnecessary, because It is a good habit to Avoid wild pointers. Just like the dwc->root = NULL. Thanks, Du, Changbin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] net: cdc_ncm: update datagram size after changing mtu
> Robert Dobrowolski writes: > >> From: Rafal Redzimski >> >> Current implementation updates the mtu size and notify cdc_ncm >> device using USB_CDC_SET_MAX_DATAGRAM_SIZE request about datagram >> size change instead of changing rx_urb_size. >> >> Whenever mtu is being changed, datagram size should also be >> updated. > > Definitely! Thanks for this. But looking at the code I believe you > need to fix the calculation of maxmtu too. It is currently: > > int maxmtu = ctx->max_datagram_size - cdc_ncm_eth_hlen(dev); > > And cdc_ncm_set_dgram_size() updates ctx->max_datagram_size with the new > mtu, meaning that you can only reduce the mtu. We should probably use > cdc_ncm_max_dgram_size() instead here. > > And cdc_ncm_set_dgram_size() takes the datagram size with header as > input (ref the above maxmtu calucalution), so it probably needs to > called as > > cdc_ncm_set_dgram_size(dev, new_mtu + cdc_ncm_eth_hlen(dev)); > > to get it right. I think. None of this is tested on an actual device > yet... Care to test if I'm right, and do a v2 if necessry? > >> Cc: > > This should be dropped for net. Ask David to queue it for stable > instead. I usually do that by using a subject prefix like > > [PATCH net,stable v1] ... > > > > > Bjørn > ok, thanks for feedback I will send v2 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] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
"Du, Changbin" writes: >> > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c >> > index 9ac37fe..9eeb444 100644 >> > --- a/drivers/usb/dwc3/debugfs.c >> > +++ b/drivers/usb/dwc3/debugfs.c >> > @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc) >> > { >> >debugfs_remove_recursive(dwc->root); >> >dwc->root = NULL; >> > + >> > + kfree(dwc->regset); >> >> we also need a kfree() on dwc3_debugfs_init(). > This patch is based on the patch set > [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void> > So, they do has dependency. :) I _did_ mention on previous set that fixes and new-features shouldn't have dependencies between them. That was the whole point of splitting the series in two. >> > + dwc->regset = NULL; >> >> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit() >> when removing the driver. >> >> -- >> Balbi > I'd like keep this line even it is unnecessary, because It is a good habit to > Avoid wild pointers. Just like the dwc->root = NULL. there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. -- balbi signature.asc Description: PGP signature
Re: [PATCH v10 1/4] gadget: Introduce the usb charger framework
Hi Jun, Sorry for late reply. >> >> >> >> +/* >> >> >> >> + * usb_charger_detect_type() - detect the charger type manually. >> >> >> >> + * @uchger - usb charger device >> >> >> >> + * >> >> >> >> + * Note: You should ensure you need to detect the charger type >> >> >> >> +manually on your >> >> >> >> + * platform. >> >> >> >> + * You should call it at the right gadget state to avoid >> >> >> >> +affecting gadget >> >> >> >> + * enumeration. >> >> >> >> + */ >> >> >> >> +int usb_charger_detect_type(struct usb_charger *uchger) { >> >> >> >> + enum usb_charger_type type; >> >> >> >> + >> >> >> >> + if (WARN(!uchger->charger_detect, >> >> >> >> + "charger detection method should not be NULL")) >> >> >> >> + return -EINVAL; >> >> >> >> + >> >> >> >> + type = uchger->charger_detect(uchger); >> >> >> >> + >> >> >> >> + mutex_lock(&uchger->lock); >> >> >> >> + uchger->type = type; >> >> >> >> + mutex_unlock(&uchger->lock); >> >> >> >> + >> >> >> >> + return 0; >> >> >> >> +} >> >> >> >> +EXPORT_SYMBOL_GPL(usb_charger_detect_type); >> >> >> > >> >> >> > I still recommend have a central place to call >> >> >> > usb_charger_detect_type() to cover real charger detection >> >> >> > instead of leaving this question open to diff users. This can be >> >> >> > done after vbus-on is detected and before do gadget connect. I >> >> >> > don't think this >> >> >> will make your framework complicated. >> >> >> >> >> >> This API is used for detecting the charger type manually (software >> >> >> charger detection), so if user's udc driver is needed to do this, >> >> >> then they can issue it at their right place to make it more flexible. >> >> >> But let us see other people's suggestion. Thanks. >> >> > >> >> > Ok, actually this API can be used for what ever charger detection >> >> > type, user can implement any method(manual detection, directly read >> >> > result from some HW...what ever) in uchger->charger_detect(), >> >> > target is >> >> >> >> But reading result from some HW dose not means *detect*, actually is >> *get*. >> >> We can not mix them together with the different semantics. >> > >> > It doesn’t matter here, you already define the _get_ API for just read >> > the charger type from the saved value(uchger->type), so we can think >> > the API to make uchger->type from UNKNOW to ONE KNOWN type is >> > *detect*, because we don't need 2 separate API one for *get* type from >> > HW and another one for *detect* from HW, only one API involve HW access >> is enough. >> >> But another issue is some users may need to get the charger type from >> power supply by "power_supply_get_property()" function, we need to >> integrate with the power supply things in the usb charger framework, not >> user to implement that. > > Why this user(your case) can't put the charger type get by > "power_supply_get_property()" in its uchger->charger_detect(), > any special reason prevent you doing it? I am not sure if this > case is very common, even if it is, you also can put it in > usb_charger_detect_type() > > usb_charger_detect_type() > { > If can get from power_supply_get_property() > ... > else if uchger->charger_detect > uchger->charger_detect(); > ... > } If user want to implement above method, they need to get the 'power_supply' structure to do this action, which maybe can not get in some contexts. So we need to integrate with the power supply things to avoid this situation. IIRC, Felipe suggested me to flesh out the charger getting method. Hi Felipe, what do you think of Jun's suggestion? Thanks. > > I don't know if most design of usb charger detection now days > is as easy as your PMIC for software(HW does the whole detection > process and prevent the vbus generation until detection has completed), > anyway if your framework can guarantee the detection process finished > before gadget connect, then each user don't have to worry about the > HW detection process details and seek a proper place to do it. > > Li Jun >> >> > >> > - Detect: (write to and) read from HW to get the charger type, >> > save to uchger->type; >> > - Get: read the charger type from uchger->type. >> > >> >> >> >> > to have the charger type and set uchger->type, then you no need to >> >> > add the comments/description to limit it usage. Also I do see there >> >> > is >> >> possible central place to do this. >> >> > >> >> >> >> -- >> >> Baolin.wang >> >> Best Regards >> >> >> >> -- >> Baolin.wang >> Best Regards -- Baolin.wang Best Regards -- 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 1/2] usb: dwc3: make dwc3_debugfs_init return value be void
> > root = debugfs_create_dir(dev_name(dwc->dev), NULL); > > - if (!root) { > > - ret = -ENOMEM; > > - goto err0; > > - } > > + if (IS_ERR_OR_NULL(root)) > > + return; > > We can definitely keep on going, but I'd still like to know that we > enabled CONFIG_DEBUG_FS but failed to create a file or a > directory. Seems like this should read as follows: > > if (IS_ERR_OR_NULL(root)) { > if (!root) > dev_err(dwc->dev, "Can't create debugfs root\n"); > return; > } > > ditto to all bellow. > Balbi, so you mean we should not let the failure go silent, right? > > dwc->root = root; > > > > dwc->regset = kzalloc(sizeof(*dwc->regset), GFP_KERNEL); > > if (!dwc->regset) { > > - ret = -ENOMEM; > > - goto err1; > > + debugfs_remove_recursive(root); > > you're now duplicating debugfs_remove_recursive(root) in all braches and > that's error prone. It's probably better to keep our gotos, but change > them so they read as follows: > > if (!dwc->regset) > goto err1; > > [...] > > return; /* this is our successful exit point */ > > err1: > debugfs_remove_recursive(root); > kfree(dwc->regset); > > > -- > Balbi No, no need anymore. Because no branch share this code now. Then remove the goto would make code a little clear. Thanks, Du, Changbin -- 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 1/2] usb: dwc3: make dwc3_debugfs_init return value be void
Hi, "Du, Changbin" writes: >> >root = debugfs_create_dir(dev_name(dwc->dev), NULL); >> > - if (!root) { >> > - ret = -ENOMEM; >> > - goto err0; >> > - } >> > + if (IS_ERR_OR_NULL(root)) >> > + return; >> >> We can definitely keep on going, but I'd still like to know that we >> enabled CONFIG_DEBUG_FS but failed to create a file or a >> directory. Seems like this should read as follows: >> >> if (IS_ERR_OR_NULL(root)) { >> if (!root) >> dev_err(dwc->dev, "Can't create debugfs root\n"); >> return; >> } >> >> ditto to all bellow. >> > Balbi, so you mean we should not let the failure go silent, right? yeah, but only iff CONFIG_DEBUG_FS is enabled. From what I can tell, in case CONFIG_DEBUG_FS is enabled and it fails, it'll return a NULL pointer instead of ERR_PTR() ;-) >> >dwc->root = root; >> > >> >dwc->regset = kzalloc(sizeof(*dwc->regset), GFP_KERNEL); >> >if (!dwc->regset) { >> > - ret = -ENOMEM; >> > - goto err1; >> > + debugfs_remove_recursive(root); >> >> you're now duplicating debugfs_remove_recursive(root) in all braches and >> that's error prone. It's probably better to keep our gotos, but change >> them so they read as follows: >> >> if (!dwc->regset) >> goto err1; >> >> [...] >> >> return; /* this is our successful exit point */ >> >> err1: >> debugfs_remove_recursive(root); >> kfree(dwc->regset); >> >> >> -- >> Balbi > > No, no need anymore. Because no branch share this code now. > Then remove the goto would make code a little clear. fair enough. -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 00/16] da8xx USB clocks
Hi David, On Thursday 07 April 2016 09:59 PM, David Lechner wrote: > Any further comments before I submit a v4 patchset? Particularly on > patches 3 and 4 which are new in this v3 submission and have not been > commented on yet. I applied some patches which could be applied independently without driver dependency. I have pushed those to my for-testing[1] branch where they will get tested a little more before I send my pull request to ARM-SoC. For your future submissions, you can drop the patches already applied to the branch above. Also, I think it will help if you divide the series into driver and platform changes. You should keep all maintainers copied in both places. But dividing the series this way should help concentrate review. This is a complex series with multiple maintainers involved so it will take some co-ordination. Hopefully it can still all go into the upcoming merge window. Regards, Sekhar [1] http://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=for-testing -- 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 v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode
Gadget controller might not be always active during suspend/ resume when we are operating in dual-role/otg mode. Check if we're active and only if we are then perform necessary actions during suspend/resume. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/gadget.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 83d5c57..1ca5ac0 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2952,6 +2952,9 @@ void dwc3_gadget_exit(struct dwc3 *dwc) int dwc3_gadget_suspend(struct dwc3 *dwc) { + if (!dwc->gadget_driver) + return 0; + if (dwc->pullups_connected) { dwc3_gadget_disable_irq(dwc); dwc3_gadget_run_stop(dwc, true, true); @@ -2970,6 +2973,9 @@ int dwc3_gadget_resume(struct dwc3 *dwc) struct dwc3_ep *dep; int ret; + if (!dwc->gadget_driver) + return 0; + /* Start with SuperSpeed Default */ dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 10/10] ARM: dts: am43xx: Enable dual-role on USB1
USB1 port is micro-AB type and can function as peripheral as well as host. Enable dual-role mode for USB1. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/am437x-gp-evm.dts | 2 +- arch/arm/boot/dts/am437x-sk-evm.dts | 2 +- arch/arm/boot/dts/am43x-epos-evm.dts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts index 8889be1..5834448 100644 --- a/arch/arm/boot/dts/am437x-gp-evm.dts +++ b/arch/arm/boot/dts/am437x-gp-evm.dts @@ -765,7 +765,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; }; diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts index d82dd6e..ad772da 100644 --- a/arch/arm/boot/dts/am437x-sk-evm.dts +++ b/arch/arm/boot/dts/am437x-sk-evm.dts @@ -571,7 +571,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; pinctrl-names = "default"; pinctrl-0 = <&usb1_pins>; diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts index 83dfafa..b2b596b 100644 --- a/arch/arm/boot/dts/am43x-epos-evm.dts +++ b/arch/arm/boot/dts/am43x-epos-evm.dts @@ -675,7 +675,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; status = "okay"; }; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 05/10] usb: dwc3: core: cleanup IRQ resources
Implementations might use different IRQs for host, gadget and OTG so use named interrupt resources to allow Device tree to specify the 3 interrupts. Following are the interrupt names Peripheral Interrupt - peripheral HOST Interrupt - host OTG Interrupt - otg We still maintain backward compatibility for a single named interrupt for all 3 interrupts (e.g. for dwc3-pci) and single unnamed interrupt for all 3 interrupts (e.g. old DT). Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.c | 26 -- drivers/usb/dwc3/core.h | 5 + drivers/usb/dwc3/gadget.c | 19 ++- drivers/usb/dwc3/host.c | 19 +++ 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 17fd8144..1c754749 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -746,6 +746,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) { struct device *dev = dwc->dev; int ret; + struct resource *res; + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); switch (dwc->dr_mode) { case USB_DR_MODE_PERIPHERAL: @@ -765,6 +767,20 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) } break; case USB_DR_MODE_OTG: + dwc->otg_irq = platform_get_irq_byname(dwc3_pdev, "otg"); + if (dwc->otg_irq <= 0) { + dwc->otg_irq = platform_get_irq_byname(dwc3_pdev, + "dwc_usb3"); + if (dwc->otg_irq <= 0) { + res = platform_get_resource(dwc3_pdev, + IORESOURCE_IRQ, 0); + if (!res) { + dev_err(dwc->dev, "missing otg IRQ\n"); + return -ENODEV; + } + dwc->otg_irq = res->start; + } + } dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG); ret = dwc3_host_init(dwc); if (ret) { @@ -831,16 +847,6 @@ static int dwc3_probe(struct platform_device *pdev) dwc->mem = mem; dwc->dev = dev; - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!res) { - dev_err(dev, "missing IRQ\n"); - return -ENODEV; - } - dwc->xhci_resources[1].start = res->start; - dwc->xhci_resources[1].end = res->end; - dwc->xhci_resources[1].flags = res->flags; - dwc->xhci_resources[1].name = res->name; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { dev_err(dev, "missing memory resource\n"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 81039f7..79422dd 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -754,6 +754,8 @@ struct dwc3_scratchpad_array { * @maximum_speed: maximum speed requested (mainly for testing purposes) * @revision: revision register contents * @dr_mode: requested mode of operation + * @gadget_irq: IRQ number for Peripheral IRQs + * @otg_irq: IRQ number for OTG IRQs * @usb2_phy: pointer to USB2 PHY * @usb3_phy: pointer to USB3 PHY * @usb2_generic_phy: pointer to USB2 PHY @@ -857,6 +859,9 @@ struct dwc3 { enum usb_dr_modedr_mode; + int gadget_irq; + int otg_irq; + /* used for suspend/resume */ u32 dcfg; u32 gctl; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 3ac170f..90f514c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1617,7 +1617,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, int irq; u32 reg; - irq = platform_get_irq(to_platform_device(dwc->dev), 0); + irq = dwc->gadget_irq; ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, IRQF_SHARED, "dwc3", dwc); if (ret) { @@ -2795,6 +2795,23 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc) int dwc3_gadget_init(struct dwc3 *dwc) { int ret; + struct resource *res; + struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); + + dwc->gadget_irq = platform_get_irq_byname(dwc3_pdev, "peripheral"); + if (dwc->gadget_irq <= 0) { + dwc->gadget_irq = platform_get_irq_byname(dwc3_pdev, + "dwc_usb3"); + if (dwc->gadget_irq <= 0) { + res = platform_get_resource(dwc3_pdev, IORESOURCE_IRQ, + 0); +
[PATCH v6 09/10] ARM: dts: dra7*-evm: Enable dual-role for usb1
Now that we have dual-role support working at USB core, enable dual-role support for usb1 controller. Signed-off-by: Roger Quadros --- arch/arm/boot/dts/dra7-evm.dts | 2 +- arch/arm/boot/dts/dra72-evm.dts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts index d9b8723..61c88f7 100644 --- a/arch/arm/boot/dts/dra7-evm.dts +++ b/arch/arm/boot/dts/dra7-evm.dts @@ -722,7 +722,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; pinctrl-names = "default"; pinctrl-0 = <&usb1_pins>; }; diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts index 6affe2d..f7d345e 100644 --- a/arch/arm/boot/dts/dra72-evm.dts +++ b/arch/arm/boot/dts/dra72-evm.dts @@ -592,7 +592,7 @@ }; &usb1 { - dr_mode = "peripheral"; + dr_mode = "otg"; pinctrl-names = "default"; pinctrl-0 = <&usb1_pins>; }; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
> > >> > +dwc->regset = NULL; > >> > >> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit() > >> when removing the driver. > >> > >> -- > >> Balbi > > I'd like keep this line even it is unnecessary, because It is a good habit > > to > > Avoid wild pointers. Just like the dwc->root = NULL. > > there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. > > -- > Balbi I agree the dwc will be freed in current code. But the 'free' logical is out of the debugfs code. They should be treat as some logical independent. Per this point, I still think set pointer to null is not bad. For example, if dwc3 core code invoke dwc3_debugfs_exit twice by mistake(just an example case, not really), then no crash/impact for the second call. Thanks, Du, Changbin -- 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 v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently
Don't make any decisions regarding VBUS session based on ID status. That is best left to the OTG core. Pass ID and VBUS events independent of each other so that OTG core knows exactly what to do. This makes dual-role with extcon work with OTG irq on OMAP platforms. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 51ca098..c9b918d 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, } val = dwc3_omap_read_utmi_ctrl(omap); - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; dwc3_omap_write_utmi_ctrl(omap, val); break; case OMAP_DWC3_VBUS_VALID: val = dwc3_omap_read_utmi_ctrl(omap); val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND; - val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID + val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID | USBOTGSS_UTMI_OTG_CTRL_SESSVALID | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; dwc3_omap_write_utmi_ctrl(omap, val); @@ -254,14 +249,16 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, case OMAP_DWC3_ID_FLOAT: if (omap->vbus_reg) regulator_disable(omap->vbus_reg); + val = dwc3_omap_read_utmi_ctrl(omap); + val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG; + dwc3_omap_write_utmi_ctrl(omap, val); case OMAP_DWC3_VBUS_OFF: val = dwc3_omap_read_utmi_ctrl(omap); val &= ~(USBOTGSS_UTMI_OTG_CTRL_SESSVALID | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT); - val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND - | USBOTGSS_UTMI_OTG_CTRL_IDDIG; + val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND; dwc3_omap_write_utmi_ctrl(omap, val); break; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED. Use request_threaded_irq() to ensure that irqflags match for the shared interrupt handlers. If we don't use request_treaded_irq() then forced threaded irq will set IRQF_ONESHOT and this won't match with the OTG irq handler. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 22e9606..51ca098 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) { struct dwc3_omap*omap = _omap; u32 reg; + int ret = IRQ_NONE; reg = dwc3_omap_read_irqmisc_status(omap); + if (reg) + ret = IRQ_HANDLED; + if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) omap->dma_status = false; dwc3_omap_write_irqmisc_status(omap, reg); reg = dwc3_omap_read_irq0_status(omap); + if (reg) + ret = IRQ_HANDLED; dwc3_omap_write_irq0_status(omap, reg); - return IRQ_HANDLED; + return ret; } static void dwc3_omap_enable_irqs(struct dwc3_omap *omap) @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev) reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); - ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, - "dwc3-omap", omap); + ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt, + NULL, IRQF_SHARED, "dwc3-omap", omap); if (ret) { dev_err(dev, "failed to request IRQ #%d --> %d\n", omap->irq, ret); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 08/10] usb: dwc3: core: fix PHY handling during suspend
From: Felipe Balbi we need to power off the PHY during suspend and power it back on during resume. Signed-off-by: Felipe Balbi [nsek...@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()] Signed-off-by: Sekhar Nori Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.c | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index f24c091..60665dd 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1649,6 +1649,11 @@ static int dwc3_suspend(struct device *dev) phy_exit(dwc->usb2_generic_phy); phy_exit(dwc->usb3_generic_phy); + usb_phy_set_suspend(dwc->usb2_phy, 1); + usb_phy_set_suspend(dwc->usb3_phy, 1); + WARN_ON(phy_power_off(dwc->usb2_generic_phy) < 0); + WARN_ON(phy_power_off(dwc->usb3_generic_phy) < 0); + pinctrl_pm_select_sleep_state(dev); return 0; @@ -1662,11 +1667,21 @@ static int dwc3_resume(struct device *dev) pinctrl_pm_select_default_state(dev); + usb_phy_set_suspend(dwc->usb2_phy, 0); + usb_phy_set_suspend(dwc->usb3_phy, 0); + ret = phy_power_on(dwc->usb2_generic_phy); + if (ret < 0) + return ret; + + ret = phy_power_on(dwc->usb3_generic_phy); + if (ret < 0) + goto err_usb2phy_power; + usb_phy_init(dwc->usb3_phy); usb_phy_init(dwc->usb2_phy); ret = phy_init(dwc->usb2_generic_phy); if (ret < 0) - return ret; + goto err_usb3phy_power; ret = phy_init(dwc->usb3_generic_phy); if (ret < 0) @@ -1718,6 +1733,12 @@ static int dwc3_resume(struct device *dev) err_usb2phy_init: phy_exit(dwc->usb2_generic_phy); +err_usb3phy_power: + phy_power_off(dwc->usb3_generic_phy); + +err_usb2phy_power: + phy_power_off(dwc->usb2_generic_phy); + return ret; } -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()
From: Felipe Balbi Even if pm_runtime_get*() fails, we *MUST* call pm_runtime_put_sync() before disabling PM. While at it, remove superfluous dwc3_omap_disable_irqs() in error path. Signed-off-by: Felipe Balbi [nsek...@ti.com: patch description updates] Signed-off-by: Sekhar Nori Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index c9b918d..3497b25 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -499,7 +499,7 @@ static int dwc3_omap_probe(struct platform_device *pdev) ret = pm_runtime_get_sync(dev); if (ret < 0) { dev_err(dev, "get_sync failed with err %d\n", ret); - goto err0; + goto err1; } dwc3_omap_map_offset(omap); @@ -519,28 +519,24 @@ static int dwc3_omap_probe(struct platform_device *pdev) ret = dwc3_omap_extcon_register(omap); if (ret < 0) - goto err2; + goto err1; ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); - goto err3; + goto err2; } dwc3_omap_enable_irqs(omap); return 0; -err3: +err2: extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb); extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb); -err2: - dwc3_omap_disable_irqs(omap); err1: pm_runtime_put_sync(dev); - -err0: pm_runtime_disable(dev); return ret; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 06/10] usb: dwc3: add dual-role support
Register with the USB OTG core. Since we don't support OTG yet we just work as a dual-role device even if device tree says "otg". Get ID and VBUS information from the OTG controller and kick the OTG state machine. Make sure dual-role functionality works across system suspend/resume. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.c | 544 -- drivers/usb/dwc3/core.h | 20 ++ drivers/usb/dwc3/gadget.c | 6 +- drivers/usb/dwc3/host.c | 2 + 4 files changed, 550 insertions(+), 22 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 1c754749..f24c091 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -57,6 +57,7 @@ void dwc3_set_mode(struct dwc3 *dwc, u32 mode) reg = dwc3_readl(dwc->regs, DWC3_GCTL); reg &= ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG)); reg |= DWC3_GCTL_PRTCAPDIR(mode); + dwc->current_mode = mode; dwc3_writel(dwc->regs, DWC3_GCTL, reg); } @@ -742,13 +743,444 @@ static int dwc3_core_get_phy(struct dwc3 *dwc) return 0; } -static int dwc3_core_init_mode(struct dwc3 *dwc) +/* Get OTG events and sync it to OTG fsm */ +static void dwc3_otg_fsm_sync(struct dwc3 *dwc) +{ + u32 reg; + int id, vbus; + + /* +* calling usb_otg_sync_inputs() during resume breaks host +* if adapter was removed during suspend as xhci driver +* is not prepared to see hcd removal before xhci_resume. +*/ + if (dwc->otg_prevent_sync) + return; + + reg = dwc3_readl(dwc->regs, DWC3_OSTS); + dwc3_trace(trace_dwc3_core, "otgstatus 0x%x\n", reg); + + id = !!(reg & DWC3_OSTS_CONIDSTS); + vbus = !!(reg & DWC3_OSTS_BSESVLD); + + dwc3_trace(trace_dwc3_core, "id %d vbus %d\n", id, vbus); + dwc->otg->fsm.id = id; + dwc->otg->fsm.b_sess_vld = vbus; + usb_otg_sync_inputs(dwc->otg); +} + +static void dwc3_otg_mask_irq(struct dwc3 *dwc) +{ + dwc->oevten = dwc3_readl(dwc->regs, DWC3_OEVTEN); + dwc3_writel(dwc->regs, DWC3_OEVTEN, 0); +} + +static void dwc3_otg_unmask_irq(struct dwc3 *dwc) +{ + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten); +} + +static void dwc3_otg_disable_events(struct dwc3 *dwc, u32 disable_mask) +{ + dwc->oevten &= ~(disable_mask); + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten); +} + +static void dwc3_otg_enable_events(struct dwc3 *dwc, u32 enable_mask) +{ + dwc->oevten |= (enable_mask); + dwc3_writel(dwc->regs, DWC3_OEVTEN, dwc->oevten); +} + +#define DWC3_OTG_ALL_EVENTS(DWC3_OEVTEN_XHCIRUNSTPSETEN | \ + DWC3_OEVTEN_DEVRUNSTPSETEN | DWC3_OEVTEN_HIBENTRYEN | \ + DWC3_OEVTEN_CONIDSTSCHNGEN | DWC3_OEVTEN_HRRCONFNOTIFEN | \ + DWC3_OEVTEN_HRRINITNOTIFEN | DWC3_OEVTEN_ADEVIDLEEN | \ + DWC3_OEVTEN_ADEVBHOSTENDEN | DWC3_OEVTEN_ADEVHOSTEN | \ + DWC3_OEVTEN_ADEVHNPCHNGEN | DWC3_OEVTEN_ADEVSRPDETEN | \ + DWC3_OEVTEN_ADEVSESSENDDETEN | DWC3_OEVTEN_BDEVHOSTENDEN | \ + DWC3_OEVTEN_BDEVHNPCHNGEN | DWC3_OEVTEN_BDEVSESSVLDDETEN | \ + DWC3_OEVTEN_BDEVVBUSCHNGE) + +static int dwc3_drd_start_host(struct usb_otg *otg, int on); +static int dwc3_drd_start_gadget(struct usb_otg *otg, int on); +static irqreturn_t dwc3_otg_thread_irq(int irq, void *_dwc) +{ + struct dwc3 *dwc = _dwc; + unsigned long flags; + + spin_lock_irqsave(&dwc->lock, flags); + + /* +* this bit is needed for otg-host to work after system suspend/resume +*/ + if ((dwc->otg->state == OTG_STATE_A_HOST) && + !(dwc->oevt & DWC3_OEVT_DEVICEMODE)) { + spin_unlock_irqrestore(&dwc->lock, flags); + dwc3_drd_start_host(dwc->otg, true); + spin_lock_irqsave(&dwc->lock, flags); + } + + dwc3_otg_fsm_sync(dwc); + dwc3_otg_unmask_irq(dwc); + + dwc->oevt = 0; + spin_unlock_irqrestore(&dwc->lock, flags); + + return IRQ_HANDLED; +} + +static irqreturn_t dwc3_otg_irq(int irq, void *_dwc) +{ + struct dwc3 *dwc = _dwc; + irqreturn_t ret = IRQ_NONE; + u32 reg; + + reg = dwc3_readl(dwc->regs, DWC3_OEVT); + if (reg) { + dwc->oevt = reg; + dwc3_writel(dwc->regs, DWC3_OEVT, reg); + dwc3_otg_mask_irq(dwc); + ret = IRQ_WAKE_THREAD; + } + + return ret; +} + +/* - Dual-Role management --- */ +static void dwc3_otgregs_init(struct dwc3 *dwc) +{ + u32 reg; + + /* +* Prevent host/device reset from resetting OTG core. +* If we don't do this then xhci_reset (USBCMD.HCRST) will reset +* the signal outputs sent to the PHY, the OTG FSM logic of the +* core and also the resets to the VBUS filters inside the core. +*/ + reg = dwc3_readl(dwc->regs,
[PATCH v6 00/10] usb: dwc3: add dual-role support
Hi, This series adds dual role support to dwc3 controller driver. Series depends on the OTG/dual-role framework [1]. [1] - https://lkml.org/lkml/2016/4/5/492 Patches are based on v4.6-rc1. v6: - use just otg irq to get otg events and don't depend on extcon at all. - follow OTG flow in TRM strictly. - use tracepoints instead of dev_dbg(). - match IRQ flags in dwc3_omap and core.c for shared otg interrupt. v5: Internal revision. Not sent to mailing list. v4: first version that was reviewed. cheers, -roger Felipe Balbi (2): usb: dwc3: omap: fix up error path on probe() usb: dwc3: core: fix PHY handling during suspend Roger Quadros (8): usb: dwc3: core.h: add some register definitions usb: dwc3: omap: Make the wrapper interrupt shared usb: dwc3: omap: Pass VBUS and ID events transparently usb: dwc3: core: cleanup IRQ resources usb: dwc3: add dual-role support usb: dwc3: gadget: Fix suspend/resume during dual-role mode ARM: dts: dra7*-evm: Enable dual-role for usb1 ARM: dts: am43xx: Enable dual-role on USB1 arch/arm/boot/dts/am437x-gp-evm.dts | 2 +- arch/arm/boot/dts/am437x-sk-evm.dts | 2 +- arch/arm/boot/dts/am43x-epos-evm.dts | 2 +- arch/arm/boot/dts/dra7-evm.dts | 2 +- arch/arm/boot/dts/dra72-evm.dts | 2 +- drivers/usb/dwc3/core.c | 565 +-- drivers/usb/dwc3/core.h | 109 ++- drivers/usb/dwc3/dwc3-omap.c | 39 ++- drivers/usb/dwc3/gadget.c| 31 +- drivers/usb/dwc3/host.c | 21 ++ 10 files changed, 729 insertions(+), 46 deletions(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 01/10] usb: dwc3: core.h: add some register definitions
Add OTG and GHWPARAMS6 register definitions Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.h | 84 - 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 6254b2f..81039f7 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -176,6 +176,15 @@ #define DWC3_GCTL_GBLHIBERNATIONEN (1 << 1) #define DWC3_GCTL_DSBLCLKGTNG (1 << 0) +/* Global Status Register */ +#define DWC3_GSTS_OTG_IP BIT(10) +#define DWC3_GSTS_BC_IPBIT(9) +#define DWC3_GSTS_ADP_IP BIT(8) +#define DWC3_GSTS_HOST_IP BIT(7) +#define DWC3_GSTS_DEVICE_IPBIT(6) +#define DWC3_GSTS_CSR_TIMEOUT BIT(5) +#define DWC3_GSTS_BUS_ERR_ADDR_VLD BIT(4) + /* Global USB2 PHY Configuration Register */ #define DWC3_GUSB2PHYCFG_PHYSOFTRST(1 << 31) #define DWC3_GUSB2PHYCFG_SUSPHY(1 << 6) @@ -239,7 +248,12 @@ #define DWC3_MAX_HIBER_SCRATCHBUFS 15 /* Global HWPARAMS6 Register */ -#define DWC3_GHWPARAMS6_EN_FPGA(1 << 7) +#define DWC3_GHWPARAMS6_BCSUPPORT BIT(14) +#define DWC3_GHWPARAMS6_OTG3SUPPORTBIT(13) +#define DWC3_GHWPARAMS6_ADPSUPPORT BIT(12) +#define DWC3_GHWPARAMS6_HNPSUPPORT BIT(11) +#define DWC3_GHWPARAMS6_SRPSUPPORT BIT(10) +#define DWC3_GHWPARAMS6_EN_FPGABIT(7) /* Global Frame Length Adjustment Register */ #define DWC3_GFLADJ_30MHZ_SDBND_SEL(1 << 7) @@ -404,6 +418,74 @@ #define DWC3_DEPCMD_TYPE_BULK 2 #define DWC3_DEPCMD_TYPE_INTR 3 +/* OTG Configuration Register */ +#define DWC3_OCFG_DISPWRCUTTOFFBIT(5) +#define DWC3_OCFG_HIBDISMASK BIT(4) +#define DWC3_OCFG_SFTRSTMASK BIT(3) +#define DWC3_OCFG_OTGVERSION BIT(2) +#define DWC3_OCFG_HNPCAP BIT(1) +#define DWC3_OCFG_SRPCAP BIT(0) + +/* OTG CTL Register */ +#define DWC3_OCTL_OTG3GOERRBIT(7) +#define DWC3_OCTL_PERIMODE BIT(6) +#define DWC3_OCTL_PRTPWRCTLBIT(5) +#define DWC3_OCTL_HNPREQ BIT(4) +#define DWC3_OCTL_SESREQ BIT(3) +#define DWC3_OCTL_TERMSELIDPULSE BIT(2) +#define DWC3_OCTL_DEVSETHNPEN BIT(1) +#define DWC3_OCTL_HSTSETHNPEN BIT(0) + +/* OTG Event Register */ +#define DWC3_OEVT_DEVICEMODE BIT(31) +#define DWC3_OEVT_XHCIRUNSTPSETBIT(27) +#define DWC3_OEVT_DEVRUNSTPSET BIT(26) +#define DWC3_OEVT_HIBENTRY BIT(25) +#define DWC3_OEVT_CONIDSTSCHNG BIT(24) +#define DWC3_OEVT_HRRCONFNOTIF BIT(23) +#define DWC3_OEVT_HRRINITNOTIF BIT(22) +#define DWC3_OEVT_ADEVIDLE BIT(21) +#define DWC3_OEVT_ADEVBHOSTEND BIT(20) +#define DWC3_OEVT_ADEVHOST BIT(19) +#define DWC3_OEVT_ADEVHNPCHNG BIT(18) +#define DWC3_OEVT_ADEVSRPDET BIT(17) +#define DWC3_OEVT_ADEVSESSENDDET BIT(16) +#define DWC3_OEVT_BDEVBHOSTEND BIT(11) +#define DWC3_OEVT_BDEVHNPCHNG BIT(10) +#define DWC3_OEVT_BDEVSESSVLDDET BIT(9) +#define DWC3_OEVT_BDEVVBUSCHNG BIT(8) +#define DWC3_OEVT_BSESSVLD BIT(3) +#define DWC3_OEVT_HSTNEGSTSBIT(2) +#define DWC3_OEVT_SESREQSTSBIT(1) +#define DWC3_OEVT_ERRORBIT(0) + +/* OTG Event Enable Register */ +#define DWC3_OEVTEN_XHCIRUNSTPSETENBIT(27) +#define DWC3_OEVTEN_DEVRUNSTPSETEN BIT(26) +#define DWC3_OEVTEN_HIBENTRYEN BIT(25) +#define DWC3_OEVTEN_CONIDSTSCHNGEN BIT(24) +#define DWC3_OEVTEN_HRRCONFNOTIFEN BIT(23) +#define DWC3_OEVTEN_HRRINITNOTIFEN BIT(22) +#define DWC3_OEVTEN_ADEVIDLEEN BIT(21) +#define DWC3_OEVTEN_ADEVBHOSTENDEN BIT(20) +#define DWC3_OEVTEN_ADEVHOSTEN BIT(19) +#define DWC3_OEVTEN_ADEVHNPCHNGEN BIT(18) +#define DWC3_OEVTEN_ADEVSRPDETEN BIT(17) +#define DWC3_OEVTEN_ADEVSESSENDDETEN BIT(16) +#define DWC3_OEVTEN_BDEVHOSTENDEN BIT(11) +#define DWC3_OEVTEN_BDEVHNPCHNGEN BIT(10) +#define DWC3_OEVTEN_BDEVSESSVLDDETEN BIT(9) +#define DWC3_OEVTEN_BDEVVBUSCHNGE BIT(8) + +/* OTG Status Register */ +#define DWC3_OSTS_DEVRUNSTPBIT(13) +#define DWC3_OSTS_XHCIRUNSTP BIT(12) +#define DWC3_OSTS_PERIPHERALSTATE BIT(4) +#define DWC3_OSTS_XHCIPRTPOWER BIT(3) +#define DWC3_OSTS_BSESVLD BIT(2) +#define DWC3_OSTS_VBUSVLD BIT(1) +#define DWC3_OSTS_CONIDSTS BIT(0) + /* Structures */ struct dwc3_trb; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
Hi, "Du, Changbin" writes: >> >> >> > + dwc->regset = NULL; >> >> >> >> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit() >> >> when removing the driver. >> >> >> >> -- >> >> Balbi >> > I'd like keep this line even it is unnecessary, because It is a good habit >> > to >> > Avoid wild pointers. Just like the dwc->root = NULL. >> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. >> >> -- >> Balbi > I agree the dwc will be freed in current code. But the 'free' logical is out > of the debugfs code. They should be treat as some logical independent. Per > this point, I still think set pointer to null is not bad. For example, if > dwc3 core > code invoke dwc3_debugfs_exit twice by mistake(just an example case, not > really), then no crash/impact for the second call. the second call should crash because it's clearly wrong ;-) If dwc3 ever calls dwc3_debugfs_exit() twice, it really deserves to crash. It's something so wrong that we want the verbosity and urgency of a kernel oops to make sure we fix it ASAP. If, however, we set it to null, it might be years before we notice anything's wrong. -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
Hi, Roger Quadros writes: > The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED. > > Use request_threaded_irq() to ensure that irqflags match for the > shared interrupt handlers. If we don't use request_treaded_irq() then > forced threaded irq will set IRQF_ONESHOT and this won't match with > the OTG irq handler. then it seems you need to _first_ fix the OTG IRQ handler. Why does it defer ? At a minimum, first switch to threaded IRQ handler, then (in another patch) switch to IRQF_SHARED > Signed-off-by: Roger Quadros > --- > drivers/usb/dwc3/dwc3-omap.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c > index 22e9606..51ca098 100644 > --- a/drivers/usb/dwc3/dwc3-omap.c > +++ b/drivers/usb/dwc3/dwc3-omap.c > @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void > *_omap) > { > struct dwc3_omap*omap = _omap; > u32 reg; > + int ret = IRQ_NONE; > > reg = dwc3_omap_read_irqmisc_status(omap); > > + if (reg) > + ret = IRQ_HANDLED; you can avoid the local variable by returning early here. if (!reg) return IRQ_NONE; > if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) > omap->dma_status = false; > > dwc3_omap_write_irqmisc_status(omap, reg); > > reg = dwc3_omap_read_irq0_status(omap); > + if (reg) > + ret = IRQ_HANDLED; and this check is probably unnecessary. > @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev) > reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); > omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); > > - ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, > - "dwc3-omap", omap); > + ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt, this switch to threaded IRQ is not part of $subject. -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently
Hi, Roger Quadros writes: > Don't make any decisions regarding VBUS session based on ID > status. That is best left to the OTG core. what about builds who don't want OTG and/or dual-role ? > Pass ID and VBUS events independent of each other so that OTG > core knows exactly what to do. > > This makes dual-role with extcon work with OTG irq on OMAP platforms. > > Signed-off-by: Roger Quadros > --- > drivers/usb/dwc3/dwc3-omap.c | 15 ++- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c > index 51ca098..c9b918d 100644 > --- a/drivers/usb/dwc3/dwc3-omap.c > +++ b/drivers/usb/dwc3/dwc3-omap.c > @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap > *omap, > } > > val = dwc3_omap_read_utmi_ctrl(omap); > - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG > - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID > - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); > - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID > - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; > + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; this creates the possibility of having a USB peripheral with VBUS_VALID, right > dwc3_omap_write_utmi_ctrl(omap, val); > break; > > case OMAP_DWC3_VBUS_VALID: > val = dwc3_omap_read_utmi_ctrl(omap); > val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND; > - val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG > - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID > + val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID > | USBOTGSS_UTMI_OTG_CTRL_SESSVALID > | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; I remember discussing this with TI's IP owner back in OMAP5 days. This code was a result of talking to that guy and was, back then, tested by Silicon Validation team. I would strongly advise that before changing these bits you check with whoever's currently handling this IP inside TI to make sure your changes are still within the expectations of the wrapper block. -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()
Hi, Roger Quadros writes: > From: Felipe Balbi > > Even if pm_runtime_get*() fails, we *MUST* call > pm_runtime_put_sync() before disabling PM. > > While at it, remove superfluous dwc3_omap_disable_irqs() > in error path. > > Signed-off-by: Felipe Balbi > [nsek...@ti.com: patch description updates] > Signed-off-by: Sekhar Nori > Signed-off-by: Roger Quadros this fix is unrelated to DRD/OTG work, right ? Care to rebase only this patch on v4.6-rc3 so I can apply it during current -rc ? -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 08/10] usb: dwc3: core: fix PHY handling during suspend
Hi, Roger Quadros writes: > From: Felipe Balbi > > we need to power off the PHY during suspend and > power it back on during resume. > > Signed-off-by: Felipe Balbi > [nsek...@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()] > Signed-off-by: Sekhar Nori > Signed-off-by: Roger Quadros is this also a fix which needs to be rebased on v4.6-rc3 and merged during current -rc cycle ? -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode
Hi, Roger Quadros writes: > Gadget controller might not be always active during suspend/ > resume when we are operating in dual-role/otg mode. > Check if we're active and only if we are then perform > necessary actions during suspend/resume. I don't get this. If we're operating in OTG, we should have a gadget driver loaded, no ? -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
On 04/04/16 11:10, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: > We will need this function for a workaround. > The function issues a softreset only to the device > controller and performs minimal re-initialization > so that the device controller can be usable. > > As some code is similar to dwc3_core_init() take out > common code into dwc3_get_gctl_quirks(). > > We add a new member (prtcap_mode) to struct dwc3 to > keep track of the current mode in the PRTCAPDIR register. > > Signed-off-by: Roger Quadros I must say, I don't like this at all :-p There's ONE known silicon which needs this because of a poor silicon integration which took an IP with a known erratum where it can't be made to work on lower speeds and STILL was integrated without a superspeed PHY. There's a reason why I never tried to push this upstream myself ;-) I'm really thinking we might be better off adding a quirk flag to skip the metastability workaround and allow this ONE silicon to set the controller to lower speed. John, can you check with your colleagues if we would ever fall into STAR#9000525659 if we set maximum speed to high speed during driver probe and never touch it again ? I would assume we don't really fall into the metastability workaround, right ? We're not doing any sort of PM for dwc3... > > Hi Felipe, > > Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS? > I don't see an issue with that as long as we always ignore > dwc->maximum_speed when programming DCFG.speed for all affected > versions of the core. As long as the DCFG.speed = SS, you should not > hit the STAR. I actually mean changing DCFG.speed during driver probe and never touching it again. Would that still cause problems ? >>> >>> In that case I'm not sure. The engineer who would know is off until >>> next week so I'll get back to you as soon as I can talk to him about >>> it. >> > > So the engineers said that this issue can occur while set to HS and > the run/stop bit is changed so it seems that won't work. Thanks John. Felipe, any suggestion how we can fix this upstream? >>> >>> no idea, I don't have a lot of memory about this problem. I really don't >>> remember the details about this, is there an openly available errata >>> document which I could read ? /me goes search for it. >>> >>> I found [1] which tells me, the following: >>> >>> >>> | i819| A Device Control Bit Meta-Stability for USB3.0 Controller >>> in USB2.0 Mode | >>> |-+| >>> | Criticality | Medium >>>| >>> | | >>>| >>> | Descritiion | When USB3.0 controller core is programmed to be a USB >>> 2.0-only device | >>> | | hardware meta-stability on USB_DCTL[31]RUNSTOP bit causing >>> the core to | >>> | | attempt high speed as well as SuperSpeed connection or >>> completely miss | >>> | | the attach request. >>>| >>> | | >>>| >>> | Workaround | If the requirement is to always function in USB 2.0 mode, >>> there is no | >>> | | workaround. >>>| >>> | | Otherwise, you can always program the USB controller core >>> to be SuperSpeed | >>> | | 3.0 capable (USB_DCFG[2:0]DEVSPD = 0x4) >>>| >>> | | >>>| >>> | Revisions | SR 1.1, 2.0 >>>| >>> | Impacted| >>>| >>> |-+| >>> >>> So, TI's own documentation says that there is _no_ workaround. My >> >> We are working on updating that document. Apparently Synopsis has suggested >> this workaround. >> pasting verbatim >> >> " >> - As last step of device configuration we set the RUNSTOP bit in >> DCTL. >
[PATCH 1/7] usb: core: hcd: simplify usb_hcd_irq()
we can simplify usb_hcd_irq() by trusting the value returned by HCDs' irq handlers. Note that this will make conversion to threaded IRQs slightly simpler. Signed-off-by: Felipe Balbi --- drivers/usb/core/hcd.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2ca2cef7f681..76a6cd0d3af9 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2425,16 +2425,11 @@ EXPORT_SYMBOL_GPL(usb_bus_start_enum); irqreturn_t usb_hcd_irq (int irq, void *__hcd) { struct usb_hcd *hcd = __hcd; - irqreturn_t rc; if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd))) - rc = IRQ_NONE; - else if (hcd->driver->irq(hcd) == IRQ_NONE) - rc = IRQ_NONE; - else - rc = IRQ_HANDLED; + return IRQ_NONE; - return rc; + return hcd->driver->irq(hcd); } EXPORT_SYMBOL_GPL(usb_hcd_irq); -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] usb: host: add support for threaded IRQs
Hi guys, this patchset introduces support for threaded IRQs for host controllers drivers to use. Right now, only XHCI has been converted, but more drivers could easily be converted as well. With this series we can, eventually, spend much less time with IRQs disabled. Note that, because we're masking XHCI's IRQs, we could run our thread handlers with IRQs enabled, but I haven't tested that yet. Felipe Balbi (7): usb: core: hcd: simplify usb_hcd_irq() usb: core: hcd: let HCDs pass a threaded irq handler usb: host: xhci: add a threaded irq handler usb: hcd: add HCD_NO_ONESHOT flag usb: hcd: conditionally set IRQF_ONESHOT flag usb: host: xhci: mask and unmask interrupt generation usb: host: xhci: pass HCD_NO_ONESHOT drivers/usb/core/hcd.c | 59 -- drivers/usb/host/xhci-ring.c | 75 ++-- drivers/usb/host/xhci.c | 14 + drivers/usb/host/xhci.h | 2 ++ include/linux/usb/hcd.h | 3 ++ 5 files changed, 114 insertions(+), 39 deletions(-) -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] usb: hcd: conditionally set IRQF_ONESHOT flag
only force IRQF_ONESHOT in case HCD_NO_ONESHOT is *not* set. As a precaution, WARN() in case IRQF_ONESHOT is already set elsewhere and we have HCD_NO_ONESHOT also enabled. Signed-off-by: Felipe Balbi --- drivers/usb/core/hcd.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 66dc5a8dfca8..6d4b4f773c92 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2677,14 +2677,21 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd, * leaving IRQF_ONESHOT in place, but soon enough we plan on * removing that after all HCDs are fixed. */ - if (hcd->driver->threaded_irq) + if (hcd->driver->threaded_irq) { + if (!(hcd->driver->flags & HCD_NO_ONESHOT)) + irqflags |= IRQF_ONESHOT; + else + WARN_ONCE(irqflags & IRQF_ONESHOT, + "HCD_NO_ONESHOT set, why IRQF_ONESHOT?\n"); + retval = request_threaded_irq(irqnum, usb_hcd_irq, - usb_hcd_threaded_irq, - irqflags | IRQF_ONESHOT, hcd->irq_descr, - hcd); - else + usb_hcd_threaded_irq, irqflags, + hcd->irq_descr, hcd); + } else { retval = request_irq(irqnum, usb_hcd_irq, irqflags, hcd->irq_descr, hcd); + } + if (retval != 0) { dev_err(hcd->self.controller, "request interrupt %d failed\n", -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] usb: core: hcd: let HCDs pass a threaded irq handler
in case an HCD wants to use threaded irqs to spend less time in hardirq context, we need this. Eventually we will remove IRQF_ONESHOT but only after all HCDs are fixed. Signed-off-by: Felipe Balbi --- drivers/usb/core/hcd.c | 43 --- include/linux/usb/hcd.h | 2 ++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 76a6cd0d3af9..66dc5a8dfca8 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2420,7 +2420,8 @@ EXPORT_SYMBOL_GPL(usb_bus_start_enum); * If the controller isn't HALTed, calls the driver's irq handler. * Checks whether the controller is now dead. * - * Return: %IRQ_HANDLED if the IRQ was handled. %IRQ_NONE otherwise. + * Return: %IRQ_HANDLED if the IRQ was handled or %IRQ_WAKE_THREAD if we need to + * wake our threaded handler. %IRQ_NONE otherwise. */ irqreturn_t usb_hcd_irq (int irq, void *__hcd) { @@ -2433,6 +2434,29 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd) } EXPORT_SYMBOL_GPL(usb_hcd_irq); +/** + * usb_hcd_threaded_irq - hook threaded IRQs to HCD framework (bus glue) + * @irq the IRQ for which a thread is being woken + * @__hcd: pointer to the HCD whose IRQ is being signaled + * If the controller isn't HALTed, calls the driver's irq handler. + * Checks whether the controller is now dead. + * + * Return: %IRQ_HANDLED if the IRQ was handled. %IRQ_NONE otherwise. + */ +irqreturn_t usb_hcd_threaded_irq (int irq, void *__hcd) +{ + struct usb_hcd *hcd = __hcd; + + if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd))) + return IRQ_NONE; + + if (hcd->driver->threaded_irq) + return hcd->driver->threaded_irq(hcd); + + return IRQ_NONE; +} +EXPORT_SYMBOL_GPL(usb_hcd_threaded_irq); + /*-*/ /** @@ -2646,8 +2670,21 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd, snprintf(hcd->irq_descr, sizeof(hcd->irq_descr), "%s:usb%d", hcd->driver->description, hcd->self.busnum); - retval = request_irq(irqnum, &usb_hcd_irq, irqflags, - hcd->irq_descr, hcd); + + /* +* NOTICE: we're assuming here that if an HCD implements +* threaded_irq, it's because it wants to use it. For now, we're +* leaving IRQF_ONESHOT in place, but soon enough we plan on +* removing that after all HCDs are fixed. +*/ + if (hcd->driver->threaded_irq) + retval = request_threaded_irq(irqnum, usb_hcd_irq, + usb_hcd_threaded_irq, + irqflags | IRQF_ONESHOT, hcd->irq_descr, + hcd); + else + retval = request_irq(irqnum, usb_hcd_irq, irqflags, + hcd->irq_descr, hcd); if (retval != 0) { dev_err(hcd->self.controller, "request interrupt %d failed\n", diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index b98f831dcda3..75287dd91a52 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -242,6 +242,7 @@ struct hc_driver { /* irq handler */ irqreturn_t (*irq) (struct usb_hcd *hcd); + irqreturn_t (*threaded_irq) (struct usb_hcd *hcd); int flags; #defineHCD_MEMORY 0x0001 /* HC regs use memory (else I/O) */ @@ -479,6 +480,7 @@ void hcd_buffer_free(struct usb_bus *bus, size_t size, /* generic bus glue, needed for host controllers that don't use PCI */ extern irqreturn_t usb_hcd_irq(int irq, void *__hcd); +extern irqreturn_t usb_hcd_threaded_irq(int irq, void *__hcd); extern void usb_hc_died(struct usb_hcd *hcd); extern void usb_hcd_poll_rh_status(struct usb_hcd *hcd); -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] usb: host: xhci: add a threaded irq handler
Split xhci_irq() into top and bottom half handlers. Note that to make this work, we had to fix MSI interrupts as well to use threaded_irqs. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 70 +--- drivers/usb/host/xhci.c | 15 +- drivers/usb/host/xhci.h | 2 ++ 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 7cf66212ceae..695b04d7751e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2679,36 +2679,18 @@ static int xhci_handle_event(struct xhci_hcd *xhci) return 1; } -/* - * xHCI spec says we can get an interrupt, and if the HC has an error condition, - * we might get bad data out of the event ring. Section 4.10.2.7 has a list of - * indicators of an event TRB error, but we check the status *first* to be safe. - */ -irqreturn_t xhci_irq(struct usb_hcd *hcd) +irqreturn_t xhci_threaded_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); u32 status; u64 temp_64; union xhci_trb *event_ring_deq; dma_addr_t deq; + unsigned long flags; - spin_lock(&xhci->lock); + spin_lock_irqsave(&xhci->lock, flags); /* Check if the xHC generated the interrupt, or the irq is shared */ status = readl(&xhci->op_regs->status); - if (status == 0x) - goto hw_died; - - if (!(status & STS_EINT)) { - spin_unlock(&xhci->lock); - return IRQ_NONE; - } - if (status & STS_FATAL) { - xhci_warn(xhci, "WARNING: Host System Error\n"); - xhci_halt(xhci); -hw_died: - spin_unlock(&xhci->lock); - return IRQ_HANDLED; - } /* * Clear the op reg interrupt status first, @@ -2737,7 +2719,7 @@ hw_died: temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue); xhci_write_64(xhci, temp_64 | ERST_EHB, &xhci->ir_set->erst_dequeue); - spin_unlock(&xhci->lock); + spin_unlock_irqrestore(&xhci->lock, flags); return IRQ_HANDLED; } @@ -2765,11 +2747,53 @@ hw_died: temp_64 |= ERST_EHB; xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue); - spin_unlock(&xhci->lock); + spin_unlock_irqrestore(&xhci->lock, flags); return IRQ_HANDLED; } +/* + * xHCI spec says we can get an interrupt, and if the HC has an error condition, + * we might get bad data out of the event ring. Section 4.10.2.7 has a list of + * indicators of an event TRB error, but we check the status *first* to be safe. + */ +irqreturn_t xhci_irq(struct usb_hcd *hcd) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + u32 status; + + /* Check if the xHC generated the interrupt, or the irq is shared */ + status = readl(&xhci->op_regs->status); + if (status == 0x) + return IRQ_HANDLED; + + if (!(status & STS_EINT)) + return IRQ_NONE; + + if (status & STS_FATAL) { + xhci_warn(xhci, "WARNING: Host System Error\n"); + xhci_halt(xhci); + return IRQ_HANDLED; + } + + /* +* Clear the op reg interrupt status first, +* so we can receive interrupts from other MSI-X interrupters. +* Write 1 to clear the interrupt status. +*/ + status |= STS_EINT; + writel(status, &xhci->op_regs->status); + /* FIXME when MSI-X is supported and there are multiple vectors */ + /* Clear the MSI-X event interrupt status */ + + return IRQ_WAKE_THREAD; +} + +irqreturn_t xhci_msi_threaded_irq(int irq, void *hcd) +{ + return xhci_threaded_irq(hcd); +} + irqreturn_t xhci_msi_irq(int irq, void *hcd) { return xhci_irq(hcd); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d51ee0c3cf9f..13af72be51c6 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -239,8 +239,8 @@ static int xhci_setup_msi(struct xhci_hcd *xhci) return ret; } - ret = request_irq(pdev->irq, xhci_msi_irq, - 0, "xhci_hcd", xhci_to_hcd(xhci)); + ret = request_threaded_irq(pdev->irq, xhci_msi_irq, xhci_msi_threaded_irq, + IRQF_ONESHOT, "xhci_hcd", xhci_to_hcd(xhci)); if (ret) { xhci_dbg_trace(xhci, trace_xhci_dbg_init, "disable MSI interrupt"); @@ -312,9 +312,9 @@ static int xhci_setup_msix(struct xhci_hcd *xhci) } for (i = 0; i < xhci->msix_count; i++) { - ret = request_irq(xhci->msix_entries[i].vector, - xhci_msi_irq, - 0, "xhci_hcd", xhci_to_hcd(xhci)); + ret = request_threaded_
[PATCH 7/7] usb: host: xhci: pass HCD_NO_ONESHOT
We, now, have all the pieces necessary to properly use threaded IRQ infrastructure, so let's pass the flag to usbcore. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 13af72be51c6..a423a4dc498f 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -240,7 +240,7 @@ static int xhci_setup_msi(struct xhci_hcd *xhci) } ret = request_threaded_irq(pdev->irq, xhci_msi_irq, xhci_msi_threaded_irq, - IRQF_ONESHOT, "xhci_hcd", xhci_to_hcd(xhci)); + 0, "xhci_hcd", xhci_to_hcd(xhci)); if (ret) { xhci_dbg_trace(xhci, trace_xhci_dbg_init, "disable MSI interrupt"); @@ -314,7 +314,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci) for (i = 0; i < xhci->msix_count; i++) { ret = request_threaded_irq(xhci->msix_entries[i].vector, xhci_msi_irq, xhci_msi_threaded_irq, - IRQF_ONESHOT, "xhci_hcd", xhci_to_hcd(xhci)); + 0, "xhci_hcd", xhci_to_hcd(xhci)); if (ret) goto disable_msix; } @@ -409,7 +409,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd) /* fall back to legacy interrupt*/ ret = request_threaded_irq(pdev->irq, usb_hcd_irq, usb_hcd_threaded_irq, - IRQF_SHARED | IRQF_ONESHOT, hcd->irq_descr, hcd); + IRQF_SHARED, hcd->irq_descr, hcd); if (ret) { xhci_err(xhci, "request interrupt %d failed\n", pdev->irq); @@ -4990,7 +4990,8 @@ static const struct hc_driver xhci_hc_driver = { */ .irq = xhci_irq, .threaded_irq = xhci_threaded_irq, - .flags =HCD_MEMORY | HCD_USB3 | HCD_SHARED, + .flags =HCD_MEMORY | HCD_USB3 | HCD_SHARED | + HCD_NO_ONESHOT, /* * basic lifecycle operations -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] usb: hcd: add HCD_NO_ONESHOT flag
this flag will tell usbcore that HCD doesn't want to use IRQF_ONESHOT flag. HCDs setting this flag must make sure that their top-half handler masks HCD's IRQs and bottom half unmasks them. Signed-off-by: Felipe Balbi --- include/linux/usb/hcd.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 75287dd91a52..62ec5f6719d2 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -255,6 +255,7 @@ struct hc_driver { #defineHCD_USB31 0x0050 /* USB 3.1 */ #defineHCD_MASK0x0070 #defineHCD_BH 0x0100 /* URB complete in BH context */ +#define HCD_NO_ONESHOT 0x0200 /* Don't need to set IRQF_ONESHOT */ /* called to init HCD and root hub */ int (*reset) (struct usb_hcd *hcd); -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] usb: host: xhci: mask and unmask interrupt generation
in order to be able to properly use threaded IRQ infrastructure, let's make that we mask our interrupts on top half and unmask them on bottom half. After this patch we can finally enable HCD_NO_ONESHOT for XHCI. Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 695b04d7751e..a89771fe6c67 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2683,6 +2683,7 @@ irqreturn_t xhci_threaded_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); u32 status; + u32 cmd; u64 temp_64; union xhci_trb *event_ring_deq; dma_addr_t deq; @@ -2747,6 +2748,11 @@ irqreturn_t xhci_threaded_irq(struct usb_hcd *hcd) temp_64 |= ERST_EHB; xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue); + /* unmask interrupt */ + cmd = readl(&xhci->op_regs->command); + cmd |= CMD_EIE; + writel(cmd, &xhci->op_regs->command); + spin_unlock_irqrestore(&xhci->lock, flags); return IRQ_HANDLED; @@ -2761,6 +2767,7 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); u32 status; + u32 cmd; /* Check if the xHC generated the interrupt, or the irq is shared */ status = readl(&xhci->op_regs->status); @@ -2776,13 +2783,11 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) return IRQ_HANDLED; } - /* -* Clear the op reg interrupt status first, -* so we can receive interrupts from other MSI-X interrupters. -* Write 1 to clear the interrupt status. -*/ - status |= STS_EINT; - writel(status, &xhci->op_regs->status); + /* mask interrupt */ + cmd = readl(&xhci->op_regs->command); + cmd &= ~CMD_EIE; + writel(cmd, &xhci->op_regs->command); + /* FIXME when MSI-X is supported and there are multiple vectors */ /* Clear the MSI-X event interrupt status */ -- 2.8.0.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
Hi, Roger Quadros writes: >> I don't have this text anywhere so I don't know. Is this something TI >> came up with or Synopsys ? Unless I can see a document (preferrably from >> Synopsys) stating this, I can't really accept $subject. > > OK. I'll try to find out if there is an official document about this. > >> >> Another question is: if all it takes is an extra SoftReset, why don't we >> just reset it during probe() if max_speed < SUPER and we're running on >> rev < 2.20a ? BTW, which revision of the IP is on AM57x/DRA7x ? > > The issue might happen on any Run/Stop transition so not sure if doing it > SoftReset just at probe fixes anything. > > On DRA7x it is rev 2.02a. oh, same block as OMAP5 ES2.0 :-( question is, then: How are you sure that resetting the device actually solves the issue ? Did you really hit the metastability problem and noted that it works after a soft-reset ? How did you verify that >>> >>> I don't know if it solves the issue or not. It was suggested by >>> Synopsis to TI's silicon team. >> >> now that's a bummer ;-) >> >>> I never hit the metastability problem detection condition in my >>> overnight tests (i.e. LTDB_LINK_STATE != 4). >> >> overnight is not enough. You need to keep this running for weeks. > > how many weeks is acceptable for you? I can run for that long, no problem. > And what if the issue doesn't happen in that time frame, would you still > consider this case? Well, there's always the possibility we have never triggered the issue to start with :-) What happens if you remove the the current workaround, set maximum-speed to high-speed and constantly toggle run/stop bit (there's a soft-connect file under the UDC's directory in sysfs). Can you ever cause the problems ? Run/Stop was in a metastable state, considering that Run/Stop signal is not visible outside the die ? >>> >>> LTDB_LINK_STATE != 4 within 100ms or RUNSTOP set is the condition to >>> detect that the issue occurred. >> >> this doesn't prove anything. This just means that your 100ms timer >> expired. Unless you can verify that Run/Stop is in metastability, you >> cannot be sure this workaround works. >> >> Did anybody run silicon simulation to verify this case ? It's really the >> only way to be sure. > > AFAIK this wasn't reproducible during silicon simulation either. now this is a big problem. We just don't know if $subject is really avoiding the problem ;-) Unless we can trigger the problem, we can't be sure. We are, however, sure that current workaround avoids the problem completely. It seems to me that resetting the IP is just as "dangerous" as setting the IP to High-speed in the first place. No ? >>> >>> The soft-reset is just a recovery mechanism if that error is ever hit. >> >> but you don't know if that's a *proper* recovery mechanism because you >> never even *hit* the error. >> >>> Putting the controller in reset state means it is in a known >>> state. Why do you say it would be dangerous? >> >> Because you can't predict the systems' behavior. If the flip-flop didn't >> have time to settle into 0 or 1 state, you don't know what the >> combinatorial part of the IP will do with that bogus value. It's truly >> unpredictable. You also cannot know, for sure, that a SoftReset will be >> enough to bring that flip-flop out of metastability. > > I'm not an expert in this area and can only follow the advice the > Silicon team gives. fair enough. But you must understand we can't just accept anything even if we never trigger we problems. Unless we're certain about the fix, without a shadow of a doubt, we might be creating a very, very hard to debug regression which might end up with sales drop and what not. It's the kinda thing that we all must be concerned about ;-) >>> The original workaround i.e. setting the High-speed instance to >>> Super-speed to avoid this errata is causing another side >>> effect. i.e. erratic interrupts happen and more than 2 seconds delay >> >> this should have been an expected side-effect when you design a >> SuperSpeed controller without a SuperSpeed PHY and don't properly >> terminate inputs. What you have is a floating PIPE3 interface not >> properly terminated and capturing random noise (basically acting as a >> very poor antenna inside your die). Of course the IP will go bonkers and >> give you "erratic error" interrupts. It has no idea what the hell this >> "PHY" on the PIPE3 interface is doing. > > We know that. The damage is already done. :) right, and I'm trying to avoid further damage caused by a fix that hasn't been properly validated :) >>> to enumerations. This problem is more serious and so we have to keep >>> the controller in High-speed and tackle the meta-stability condition >>> if it happens. >> >> you have to tackle the meta-stability, sure, but we need guarantee that >> $subject IS indeed tackling that problem. Unless there's proof that this >> really solves the meta-stability issue, I won't take $sub
Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
On 11/04/16 15:13, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED. >> >> Use request_threaded_irq() to ensure that irqflags match for the >> shared interrupt handlers. If we don't use request_treaded_irq() then >> forced threaded irq will set IRQF_ONESHOT and this won't match with >> the OTG irq handler. > > then it seems you need to _first_ fix the OTG IRQ handler. Why does it There is no OTG irq handler yet. > defer ? At a minimum, first switch to threaded IRQ handler, then (in > another patch) switch to IRQF_SHARED OK. > >> Signed-off-by: Roger Quadros >> --- >> drivers/usb/dwc3/dwc3-omap.c | 12 +--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >> index 22e9606..51ca098 100644 >> --- a/drivers/usb/dwc3/dwc3-omap.c >> +++ b/drivers/usb/dwc3/dwc3-omap.c >> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void >> *_omap) >> { >> struct dwc3_omap*omap = _omap; >> u32 reg; >> +int ret = IRQ_NONE; >> >> reg = dwc3_omap_read_irqmisc_status(omap); >> >> +if (reg) >> +ret = IRQ_HANDLED; > > you can avoid the local variable by returning early here. How can we return early? we need to check irq0_status as well right? > > if (!reg) > return IRQ_NONE; > >> if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) >> omap->dma_status = false; >> >> dwc3_omap_write_irqmisc_status(omap, reg); >> >> reg = dwc3_omap_read_irq0_status(omap); >> +if (reg) >> +ret = IRQ_HANDLED; > > and this check is probably unnecessary. > >> @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev) >> reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); >> omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); >> >> -ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, >> -"dwc3-omap", omap); >> +ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt, > > this switch to threaded IRQ is not part of $subject. > OK. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
On 11/04/16 15:51, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: > > > >>> I don't have this text anywhere so I don't know. Is this something TI >>> came up with or Synopsys ? Unless I can see a document (preferrably from >>> Synopsys) stating this, I can't really accept $subject. >> >> OK. I'll try to find out if there is an official document about this. >> >>> >>> Another question is: if all it takes is an extra SoftReset, why don't we >>> just reset it during probe() if max_speed < SUPER and we're running on >>> rev < 2.20a ? BTW, which revision of the IP is on AM57x/DRA7x ? >> >> The issue might happen on any Run/Stop transition so not sure if doing it >> SoftReset just at probe fixes anything. >> >> On DRA7x it is rev 2.02a. > > oh, same block as OMAP5 ES2.0 :-( > > question is, then: How are you sure that resetting the device actually > solves the issue ? Did you really hit the metastability problem and > noted that it works after a soft-reset ? How did you verify that I don't know if it solves the issue or not. It was suggested by Synopsis to TI's silicon team. >>> >>> now that's a bummer ;-) >>> I never hit the metastability problem detection condition in my overnight tests (i.e. LTDB_LINK_STATE != 4). >>> >>> overnight is not enough. You need to keep this running for weeks. >> >> how many weeks is acceptable for you? I can run for that long, no problem. >> And what if the issue doesn't happen in that time frame, would you still >> consider this case? > > Well, there's always the possibility we have never triggered the issue > to start with :-) What happens if you remove the the current workaround, > set maximum-speed to high-speed and constantly toggle run/stop bit > (there's a soft-connect file under the UDC's directory in sysfs). Can > you ever cause the problems ? I had tried a with max-speed set to high-speed and a script that just reloads g_zero. That should toggle Run/Stop right? Running this overnight didn't cause any problems. cheers, -roger -- 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 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
Hi, Roger Quadros writes: >>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >>> index 22e9606..51ca098 100644 >>> --- a/drivers/usb/dwc3/dwc3-omap.c >>> +++ b/drivers/usb/dwc3/dwc3-omap.c >>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void >>> *_omap) >>> { >>> struct dwc3_omap*omap = _omap; >>> u32 reg; >>> + int ret = IRQ_NONE; >>> >>> reg = dwc3_omap_read_irqmisc_status(omap); >>> >>> + if (reg) >>> + ret = IRQ_HANDLED; >> >> you can avoid the local variable by returning early here. > > How can we return early? we need to check irq0_status as well right? Oh, that's true. There's one thing that I noticed though. dma_status is only written to, never read, so you should be able to remove it completely (a bit off-topic, sorry). -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently
On 11/04/16 15:18, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> Don't make any decisions regarding VBUS session based on ID >> status. That is best left to the OTG core. > > what about builds who don't want OTG and/or dual-role ? > >> Pass ID and VBUS events independent of each other so that OTG >> core knows exactly what to do. >> >> This makes dual-role with extcon work with OTG irq on OMAP platforms. >> >> Signed-off-by: Roger Quadros >> --- >> drivers/usb/dwc3/dwc3-omap.c | 15 ++- >> 1 file changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >> index 51ca098..c9b918d 100644 >> --- a/drivers/usb/dwc3/dwc3-omap.c >> +++ b/drivers/usb/dwc3/dwc3-omap.c >> @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap >> *omap, >> } >> >> val = dwc3_omap_read_utmi_ctrl(omap); >> -val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG >> -| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> -| USBOTGSS_UTMI_OTG_CTRL_SESSEND); >> -val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID >> -| USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; >> +val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; > > this creates the possibility of having a USB peripheral with VBUS_VALID, > right Sorry, I didn't get what you meant. > >> dwc3_omap_write_utmi_ctrl(omap, val); >> break; >> >> case OMAP_DWC3_VBUS_VALID: >> val = dwc3_omap_read_utmi_ctrl(omap); >> val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND; >> -val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG >> -| USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> +val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> | USBOTGSS_UTMI_OTG_CTRL_SESSVALID >> | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; > > I remember discussing this with TI's IP owner back in OMAP5 days. This > code was a result of talking to that guy and was, back then, tested by > Silicon Validation team. I would strongly advise that before changing > these bits you check with whoever's currently handling this IP inside TI > to make sure your changes are still within the expectations of the > wrapper block. > OK, I wasn't aware about this. I will check with the Silicon team. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: r8a66597-udc: Deinline pipe_change, save 2176 bytes
This function compiles to 298 bytes of machine code, has ~10 callsites. This is a USB 2.0 device, USB 2.0 is limited to ~40 MB/s, so should be almost never CPU bound. No need to optimize for speed this agressively. Signed-off-by: Denys Vlasenko CC: Felipe Balbi CC: linux-usb@vger.kernel.org CC: linux-ker...@vger.kernel.org --- Changes since v1: better commit message drivers/usb/gadget/udc/r8a66597-udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c index baa0609..1d79a47 100644 --- a/drivers/usb/gadget/udc/r8a66597-udc.c +++ b/drivers/usb/gadget/udc/r8a66597-udc.c @@ -296,7 +296,7 @@ static void r8a66597_change_curpipe(struct r8a66597 *r8a66597, u16 pipenum, } while ((tmp & mask) != loop); } -static inline void pipe_change(struct r8a66597 *r8a66597, u16 pipenum) +static void pipe_change(struct r8a66597 *r8a66597, u16 pipenum) { struct r8a66597_ep *ep = r8a66597->pipenum2ep[pipenum]; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 04/10] usb: dwc3: omap: fix up error path on probe()
On 11/04/16 15:20, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> From: Felipe Balbi >> >> Even if pm_runtime_get*() fails, we *MUST* call >> pm_runtime_put_sync() before disabling PM. >> >> While at it, remove superfluous dwc3_omap_disable_irqs() >> in error path. >> >> Signed-off-by: Felipe Balbi >> [nsek...@ti.com: patch description updates] >> Signed-off-by: Sekhar Nori >> Signed-off-by: Roger Quadros > > this fix is unrelated to DRD/OTG work, right ? Care to rebase only this > patch on v4.6-rc3 so I can apply it during current -rc ? > Sure, will do that. cheers, -roger -- 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 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode
On 11/04/16 15:23, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> Gadget controller might not be always active during suspend/ >> resume when we are operating in dual-role/otg mode. >> Check if we're active and only if we are then perform >> necessary actions during suspend/resume. > > I don't get this. If we're operating in OTG, we should have a gadget > driver loaded, no ? > At boot gadget driver is not automatically loaded. We're still in OTG mode but OTG state machine hasn't started. System suspend/resume can still happen. User might also load/unload the gadget driver prior to system suspend. cheers, -roger -- 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 08/10] usb: dwc3: core: fix PHY handling during suspend
On 11/04/16 15:24, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> From: Felipe Balbi >> >> we need to power off the PHY during suspend and >> power it back on during resume. >> >> Signed-off-by: Felipe Balbi >> [nsek...@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()] >> Signed-off-by: Sekhar Nori >> Signed-off-by: Roger Quadros > > is this also a fix which needs to be rebased on v4.6-rc3 and merged > during current -rc cycle ? > Yes, I'll post it separately. cheers, -roger -- 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 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
On 11/04/16 15:58, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 22e9606..51ca098 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) { struct dwc3_omap*omap = _omap; u32 reg; + int ret = IRQ_NONE; reg = dwc3_omap_read_irqmisc_status(omap); + if (reg) + ret = IRQ_HANDLED; >>> >>> you can avoid the local variable by returning early here. >> >> How can we return early? we need to check irq0_status as well right? > > Oh, that's true. > > There's one thing that I noticed though. dma_status is only written to, > never read, so you should be able to remove it completely (a bit > off-topic, sorry). > I'll send a patch for that. Can it go in -rc as well along with the other 2? cheers, -roger -- 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 02/10] usb: dwc3: omap: Make the wrapper interrupt shared
Hi, Roger Quadros writes: > On 11/04/16 15:58, Felipe Balbi wrote: >> >> Hi, >> >> Roger Quadros writes: > diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c > index 22e9606..51ca098 100644 > --- a/drivers/usb/dwc3/dwc3-omap.c > +++ b/drivers/usb/dwc3/dwc3-omap.c > @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, > void *_omap) > { > struct dwc3_omap*omap = _omap; > u32 reg; > + int ret = IRQ_NONE; > > reg = dwc3_omap_read_irqmisc_status(omap); > > + if (reg) > + ret = IRQ_HANDLED; you can avoid the local variable by returning early here. >>> >>> How can we return early? we need to check irq0_status as well right? >> >> Oh, that's true. >> >> There's one thing that I noticed though. dma_status is only written to, >> never read, so you should be able to remove it completely (a bit >> off-topic, sorry). >> > I'll send a patch for that. Can it go in -rc as well along with the other 2? probably not, as it's not fixing any bug ;-) -- balbi signature.asc Description: PGP signature
Re: USB xHCI regression after upgrading from kernel 3.19.0-51 to 4.2.0-34.]
On 11.04.2016 08:28, Felipe Balbi wrote: Hi, Matthew Giassa writes: *Migrating from linux-media mailing list. Good day, I maintain an SDK for USB2.0 and USB3.0 U3V machine vision cameras, and several of our customers have reported severe issues since upgrading from kernel 3.19.0-51 (Ubuntu 14.04.3 LTS) to kernel 4.2.0-34 (Ubuntu 14.04.4 LTS). I've received helpful advice from members of the libusb and linux-usb mailing lists on how to generate useful logs to help diagnose the issue, and have filed a bug to track this issue at: https://bugzilla.kernel.org/show_bug.cgi?id=115961 It seems that with kernels newer than the 3.19 series (I've tested on 4.2.0-34, and just repeated the tests on the latest 4.5.0 vanilla release), the cameras lock up, and cannot stream image data to the user application. I am able to resolve the issue on 4.2.0-34 by disabling USB power management by adding "usbcore.autosuspend=-1". On the 4.5 kernel, this "trick" doesn't work at all, and I have no way to get the cameras to stream data. I can do simple USB control requests to query things like register values and serial numbers, but that's it. Asynchronous bulk transfers never succeed. I am using the libusb library to communicate with the cameras, and have a fairly simple minimal working example to test the cameras, which: * Creates a default libusb context. * Enumerates available USB devices. * Filters out a select device based on the vendor/product ID values. * Opens the device, claims the bulk interface, and starts requesting frames of image data via async bulk transfer requests. There are select cases where the issue does not arise: Special Cases: * The issue does not occur when using USB2.0 cameras on a USB2.0 port, regardless of the kernel in use. * The issues occur only on Intel 8 Series and Intel 9 Series USB3.0 host controllers with 4.x kernels. * Intel 10 Series host controllers have not yet been tested. * The issues never occur on Fresco or Renesas host controllers, regardless of the kernel in use. * From visual inspection of lsusb output, the issue only appears to happen when the U1 and U2 options are available to the device. okay, this is probably what's happening. Intel hosts are the only ones actively doing LPM, for all other hosts we don't have support. Here are a few question: a) Are you sure your cameras implement proper LPM ? b) I'm assuming the following makes the problem go away, can you test ? diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index f0640b7a1c42..9c3ead114ad5 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -127,8 +127,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) xhci->quirks |= XHCI_TRUST_TX_LENGTH; if (pdev->vendor == PCI_VENDOR_ID_INTEL) { - xhci->quirks |= XHCI_LPM_SUPPORT; - xhci->quirks |= XHCI_INTEL_HOST; + /* xhci->quirks |= XHCI_LPM_SUPPORT; */ + /* xhci->quirks |= XHCI_INTEL_HOST; */ xhci->quirks |= XHCI_AVOID_BEI; } if (pdev->vendor == PCI_VENDOR_ID_INTEL && c) I remember that I mentioned to Mathias that I'd seen XHCI LPM go a bit crazy and trigger constant LPM transactions; maybe you're just the first one to notice an actual functional breakage due to that. LPM (Link power management) is only enabled on Intel Hosts in the xhci driver. usbmon show there is a constant loop of enabling and disabling LPM. Parsing the usbmon output it boils down to: Set SEL 0a0a0002 0002(usb_enabme_lpm) Set port feature U1 timeout 50us Set device feaurte U1 Enable Set SEL 0a0a0002 0002 Set port feature U2 timeout ~10ms Set device feaurte U2 Enable Set port feature U1 timeout 0us (usb_disable_lpm) Clear device feaurte U1 Enable Set port feature U2 timeout 0us Clear device feaurte U2 Enable -back to beginning. setting SEL (system exit latency) sometimes there's a 24 byte bulk out transfer followed by a 16 byte bulk in transfer in between the enable_lpm() and usb_disable_lpm() But how we end up in the LPM loop, or how we sustain it is not yet clear. This code is in usb core. Could you add usb core debugging with: echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control and show the dmesg output. and skip the xhci debugging, otherwise it will be very hard to read. -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 v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode
Hi, Roger Quadros writes: > On 11/04/16 15:23, Felipe Balbi wrote: >> >> Hi, >> >> Roger Quadros writes: >>> Gadget controller might not be always active during suspend/ >>> resume when we are operating in dual-role/otg mode. >>> Check if we're active and only if we are then perform >>> necessary actions during suspend/resume. >> >> I don't get this. If we're operating in OTG, we should have a gadget >> driver loaded, no ? >> > At boot gadget driver is not automatically loaded. We're still in OTG mode > but OTG state machine hasn't started. > System suspend/resume can still happen. > > User might also load/unload the gadget driver prior to system suspend. good point, this should go in the -rc too. -- balbi signature.asc Description: PGP signature
Re: [PATCH v6 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently
Hi, Roger Quadros writes: > On 11/04/16 15:18, Felipe Balbi wrote: >> >> Hi, >> >> Roger Quadros writes: >>> Don't make any decisions regarding VBUS session based on ID >>> status. That is best left to the OTG core. >> >> what about builds who don't want OTG and/or dual-role ? >> >>> Pass ID and VBUS events independent of each other so that OTG >>> core knows exactly what to do. >>> >>> This makes dual-role with extcon work with OTG irq on OMAP platforms. >>> >>> Signed-off-by: Roger Quadros >>> --- >>> drivers/usb/dwc3/dwc3-omap.c | 15 ++- >>> 1 file changed, 6 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >>> index 51ca098..c9b918d 100644 >>> --- a/drivers/usb/dwc3/dwc3-omap.c >>> +++ b/drivers/usb/dwc3/dwc3-omap.c >>> @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap >>> *omap, >>> } >>> >>> val = dwc3_omap_read_utmi_ctrl(omap); >>> - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG >>> - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >>> - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); >>> - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID >>> - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; >>> + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; >> >> this creates the possibility of having a USB peripheral with VBUS_VALID, >> right > > Sorry, I didn't get what you meant. if you're touching these bits independently from each other, won't you have a situation where you notify IDFLOAT but don't notify that VBUS is off ? -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
Hi, Roger Quadros writes: >>> how many weeks is acceptable for you? I can run for that long, no problem. >>> And what if the issue doesn't happen in that time frame, would you still >>> consider this case? >> >> Well, there's always the possibility we have never triggered the issue >> to start with :-) What happens if you remove the the current workaround, >> set maximum-speed to high-speed and constantly toggle run/stop bit >> (there's a soft-connect file under the UDC's directory in sysfs). Can >> you ever cause the problems ? > > I had tried a with max-speed set to high-speed and a script that just > reloads g_zero. > > That should toggle Run/Stop right? yes it should. > Running this overnight didn't cause any problems. as you can see, it's a difficult problem to trigger :-) -- balbi signature.asc Description: PGP signature
Re: [PATCH 0/7] usb: host: add support for threaded IRQs
Felipe Balbi writes: > Hi guys, > > this patchset introduces support for threaded IRQs > for host controllers drivers to use. Right now, only > XHCI has been converted, but more drivers could > easily be converted as well. > > With this series we can, eventually, spend much less > time with IRQs disabled. Note that, because we're > masking XHCI's IRQs, we could run our thread > handlers with IRQs enabled, but I haven't tested > that yet. just replying to myself here. I have a patch removing _irqsave/_irqrestore from our spin_lock calls in XHCI's IRQ handler and it's still working fine. I have 7x 8GiB HS devices on a hub, set them up as raid0 and I'm reading/writing to this raid0 device. So far no problems. I'll test this further with other devices to see if no issues surface, but expect another patch to this series (assuming people are okay with the change). -- balbi signature.asc Description: PGP signature
Re: [PATCH 2/7] usb: core: hcd: let HCDs pass a threaded irq handler
Hi Felipe, [auto build test WARNING on v4.6-rc3] [also build test WARNING on next-20160411] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Felipe-Balbi/usb-host-add-support-for-threaded-IRQs/20160411-205041 reproduce: make htmldocs All warnings (new ones prefixed by >>): >> drivers/usb/core/hcd.c:2447: warning: No description found for parameter >> 'irq' vim +/irq +2447 drivers/usb/core/hcd.c 2431 return IRQ_NONE; 2432 2433 return hcd->driver->irq(hcd); 2434 } 2435 EXPORT_SYMBOL_GPL(usb_hcd_irq); 2436 2437 /** 2438 * usb_hcd_threaded_irq - hook threaded IRQs to HCD framework (bus glue) 2439 * @irq the IRQ for which a thread is being woken 2440 * @__hcd: pointer to the HCD whose IRQ is being signaled 2441 * If the controller isn't HALTed, calls the driver's irq handler. 2442 * Checks whether the controller is now dead. 2443 * 2444 * Return: %IRQ_HANDLED if the IRQ was handled. %IRQ_NONE otherwise. 2445 */ 2446 irqreturn_t usb_hcd_threaded_irq (int irq, void *__hcd) > 2447 { 2448 struct usb_hcd *hcd = __hcd; 2449 2450 if (unlikely(HCD_DEAD(hcd) || !HCD_HW_ACCESSIBLE(hcd))) 2451 return IRQ_NONE; 2452 2453 if (hcd->driver->threaded_irq) 2454 return hcd->driver->threaded_irq(hcd); 2455 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 0/7] usb: host: add support for threaded IRQs
On Mon, Apr 11, 2016 at 03:44:09PM +0300, Felipe Balbi wrote: > Hi guys, > > this patchset introduces support for threaded IRQs > for host controllers drivers to use. Right now, only > XHCI has been converted, but more drivers could > easily be converted as well. > > With this series we can, eventually, spend much less > time with IRQs disabled. Note that, because we're > masking XHCI's IRQs, we could run our thread > handlers with IRQs enabled, but I haven't tested > that yet. Does it really benifit anything? Do you have any measurements? Why the added work for no real need, and increased latency? 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 v6 07/10] usb: dwc3: gadget: Fix suspend/resume during dual-role mode
On 11/04/16 16:26, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> On 11/04/16 15:23, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Roger Quadros writes: Gadget controller might not be always active during suspend/ resume when we are operating in dual-role/otg mode. Check if we're active and only if we are then perform necessary actions during suspend/resume. >>> >>> I don't get this. If we're operating in OTG, we should have a gadget >>> driver loaded, no ? >>> >> At boot gadget driver is not automatically loaded. We're still in OTG mode >> but OTG state machine hasn't started. >> System suspend/resume can still happen. >> >> User might also load/unload the gadget driver prior to system suspend. > > good point, this should go in the -rc too. > But there is no dual-role mode currently so it won't fix any bug yet :). cheers, -roger -- 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 03/10] usb: dwc3: omap: Pass VBUS and ID events transparently
On 11/04/16 16:26, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >> On 11/04/16 15:18, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Roger Quadros writes: Don't make any decisions regarding VBUS session based on ID status. That is best left to the OTG core. >>> >>> what about builds who don't want OTG and/or dual-role ? >>> Pass ID and VBUS events independent of each other so that OTG core knows exactly what to do. This makes dual-role with extcon work with OTG irq on OMAP platforms. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 51ca098..c9b918d 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, } val = dwc3_omap_read_utmi_ctrl(omap); - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; >>> >>> this creates the possibility of having a USB peripheral with VBUS_VALID, >>> right >> >> Sorry, I didn't get what you meant. > > if you're touching these bits independently from each other, won't you > have a situation where you notify IDFLOAT but don't notify that VBUS is > off ? > We're setting the mailbox state for both ID and VBUS in dwc3_omap_extcon_register() depending on the initial extcon state. After that, ID and VBUS will be signalled only when they change. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] usb: dwc3: fixes for v4.6-rc
Hi, These are some dwc3 fixes that were done by Felipe. I've just rebased and posted them. cheers, -roger Felipe Balbi (2): usb: dwc3: omap: fix up error path on probe() usb: dwc3: core: fix PHY handling during suspend drivers/usb/dwc3/core.c | 23 ++- drivers/usb/dwc3/dwc3-omap.c | 12 2 files changed, 26 insertions(+), 9 deletions(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: dwc3: omap: fix up error path on probe()
From: Felipe Balbi Even if pm_runtime_get*() fails, we *MUST* call pm_runtime_put_sync() before disabling PM. While at it, remove superfluous dwc3_omap_disable_irqs() in error path. Signed-off-by: Felipe Balbi [nsek...@ti.com: patch description updates] Signed-off-by: Sekhar Nori Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 22e9606..55da2c7 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -496,7 +496,7 @@ static int dwc3_omap_probe(struct platform_device *pdev) ret = pm_runtime_get_sync(dev); if (ret < 0) { dev_err(dev, "get_sync failed with err %d\n", ret); - goto err0; + goto err1; } dwc3_omap_map_offset(omap); @@ -516,28 +516,24 @@ static int dwc3_omap_probe(struct platform_device *pdev) ret = dwc3_omap_extcon_register(omap); if (ret < 0) - goto err2; + goto err1; ret = of_platform_populate(node, NULL, NULL, dev); if (ret) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); - goto err3; + goto err2; } dwc3_omap_enable_irqs(omap); return 0; -err3: +err2: extcon_unregister_notifier(omap->edev, EXTCON_USB, &omap->vbus_nb); extcon_unregister_notifier(omap->edev, EXTCON_USB_HOST, &omap->id_nb); -err2: - dwc3_omap_disable_irqs(omap); err1: pm_runtime_put_sync(dev); - -err0: pm_runtime_disable(dev); return ret; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: dwc3: core: fix PHY handling during suspend
From: Felipe Balbi we need to power off the PHY during suspend and power it back on during resume. Signed-off-by: Felipe Balbi [nsek...@ti.com: fix call to usb_phy_set_suspend() in dwc3_suspend()] Signed-off-by: Sekhar Nori Signed-off-by: Roger Quadros --- drivers/usb/dwc3/core.c | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 17fd8144..44ff68e 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1162,6 +1162,11 @@ static int dwc3_suspend(struct device *dev) phy_exit(dwc->usb2_generic_phy); phy_exit(dwc->usb3_generic_phy); + usb_phy_set_suspend(dwc->usb2_phy, 1); + usb_phy_set_suspend(dwc->usb3_phy, 1); + WARN_ON(phy_power_off(dwc->usb2_generic_phy) < 0); + WARN_ON(phy_power_off(dwc->usb3_generic_phy) < 0); + pinctrl_pm_select_sleep_state(dev); return 0; @@ -1175,11 +1180,21 @@ static int dwc3_resume(struct device *dev) pinctrl_pm_select_default_state(dev); + usb_phy_set_suspend(dwc->usb2_phy, 0); + usb_phy_set_suspend(dwc->usb3_phy, 0); + ret = phy_power_on(dwc->usb2_generic_phy); + if (ret < 0) + return ret; + + ret = phy_power_on(dwc->usb3_generic_phy); + if (ret < 0) + goto err_usb2phy_power; + usb_phy_init(dwc->usb3_phy); usb_phy_init(dwc->usb2_phy); ret = phy_init(dwc->usb2_generic_phy); if (ret < 0) - return ret; + goto err_usb3phy_power; ret = phy_init(dwc->usb3_generic_phy); if (ret < 0) @@ -1212,6 +1227,12 @@ static int dwc3_resume(struct device *dev) err_usb2phy_init: phy_exit(dwc->usb2_generic_phy); +err_usb3phy_power: + phy_power_off(dwc->usb3_generic_phy); + +err_usb2phy_power: + phy_power_off(dwc->usb2_generic_phy); + return ret; } -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc3: omap: get rid of dma_status
dma_status bit flag is set but never really used so get rid of it. Reported-by: Felipe Balbi Signed-off-by: Roger Quadros --- drivers/usb/dwc3/dwc3-omap.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 55da2c7..890b8a6 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -126,8 +126,6 @@ struct dwc3_omap { u32 debug_offset; u32 irq0_offset; - u32 dma_status:1; - struct extcon_dev *edev; struct notifier_block vbus_nb; struct notifier_block id_nb; @@ -277,9 +275,6 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) reg = dwc3_omap_read_irqmisc_status(omap); - if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) - omap->dma_status = false; - dwc3_omap_write_irqmisc_status(omap, reg); reg = dwc3_omap_read_irq0_status(omap); @@ -504,7 +499,6 @@ static int dwc3_omap_probe(struct platform_device *pdev) /* check the DMA Status */ reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); - omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, "dwc3-omap", omap); -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: core: buffer: avoid NULL pointer dereferrence
On Mon, 11 Apr 2016, chunfeng yun wrote: > On Mon, 2016-04-11 at 08:07 +0300, Felipe Balbi wrote: > > Hi, > > > > chunfeng yun writes: > > > On Fri, 2016-04-08 at 07:07 -0700, Greg Kroah-Hartman wrote: > > >> On Fri, Apr 08, 2016 at 05:08:03PM +0800, Chunfeng Yun wrote: > > >> > NULL pointer dereferrence will happen when class driver > > >> > wants to allocate zero length buffer and pool_max[0] > > >> > can't be used, so skip reserved pool in this case. > > >> > > >> Why would a driver want to allocate a 0 length buffer? What driver does > > >> this? > > > It's misc/usbtest.c > > > > that'll do what you ask it to do with the userspace tool testusb. Are > > you trying to pass a size of 0 ? > > > No, I just ran "testusb -t10" which called test_ctrl_queue(). > In this function, sub-case 8 passed a parameter @len as 0 to > simple_alloc_urb(), and then it tried to allocate a 0-length buffer. That should be easy enough to fix. Just skip the calls that allocate the buffer if the length would be 0. 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: PROBLEM: Mouse connected to USB-3 stopped working 2.6.38->39 regression
On Mon, 11 Apr 2016, Sam Sany wrote: > Sam Sany writes: > > No go. Just booted into the newly compiled kernel, and I still have the > exact same symptoms. The edited portion of the hub.c file I used to > compile looks as such: > > int usb_device_supports_lpm(struct usb_device *udev) > { > return 0; > > /* USB 2.1 (and greater) devices indicate LPM support through >* their USB 2.0 Extended Capabilities BOS descriptor. >*/ ... > Did I do something wrong? The dmesg output is identical to before :( If you're certain that the kernel you booted was the modified one, then I'm out of ideas. Maybe Mathias can help. > Since the dmesg output always prints "usb_set_interface failed" could it have > something to do with the portions of the hub.c file that deal with those > functions? It only occurs twice through out the hub.c file... No, the problem is caused by the xHCI hardware or driver. That's where the Set-Interface request fails. 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: [PATCHv3 2/3] usb: storage: scsiglue: limit USB3 devices to 2048 sectors
On Mon, 11 Apr 2016, Felipe Balbi wrote: > USB3 devices, because they are much newer, have much > less chance of having issues with larger transfers. > > We still keep a limit because anything above 2048 > sectors really rendered negligible speed > improvements, so we will simply ignore > that. Transferring 1MiB should already give us > pretty good performance. > > Signed-off-by: Felipe Balbi > --- > drivers/usb/storage/scsiglue.c | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c > index 9da1fb3d0ff4..4512cf78a284 100644 > --- a/drivers/usb/storage/scsiglue.c > +++ b/drivers/usb/storage/scsiglue.c > @@ -115,13 +115,19 @@ static int slave_configure(struct scsi_device *sdev) > { > struct us_data *us = host_to_us(sdev->host); > > - /* Many devices have trouble transferring more than 32KB at a time, > - * while others have trouble with more than 64K. At this time we > - * are limiting both to 32K (64 sectores). > - */ > - if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) { > + if (us->pusb_dev->speed >= USB_SPEED_SUPER) { > + /* USB3 devices will be limited to 2048 sectors. This gives us > + * better throughput on most devices. > + */ > + blk_queue_max_hw_sectors(sdev->request_queue, 2048); > + } else if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) > { > unsigned int max_sectors = 64; > > + /* Many devices have trouble transferring more than 32KB at a > time, > + * while others have trouble with more than 64K. At this time we > + * are limiting both to 32K (64 sectores). > + */ > + > if (us->fflags & US_FL_MAX_SECTORS_MIN) > max_sectors = PAGE_SIZE >> 9; > if (queue_max_hw_sectors(sdev->request_queue) > max_sectors) I should have noticed this before. If something goes wrong with the 2048-sector limit, this doesn't permit the user to override. Shouldn't the test for USB_SPEED_SUPER go after the other FL_MAX_SECTORS tests? 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 v2 1/2] dt/bindings/usb: Add bindings for PIC32 MUSB driver.
On Thu, Apr 07, 2016 at 06:02:59PM +0530, Purna Chandra Mandal wrote: > Document devicetree binding for the USB controller > and USB Phy found on Microchip PIC32 class devices. > > Signed-off-by: Purna Chandra Mandal > > --- > > Changes in v2: None > > .../bindings/usb/microchip,pic32-musb.txt | 67 > ++ > 1 file changed, 67 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt > > diff --git a/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt > b/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt > new file mode 100644 > index 000..e1cec9d > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt > @@ -0,0 +1,67 @@ > +Microchip PIC32 MUSB DRC/OTG controller > +--- > + > +Required properties: > + - compatible : should be "microchip,pic32mzda-usb". > + - reg : offset and length of "MUSB Core Registers" and > + "USB Clock & Reset Registers". > + - reg-names: should be "mc", and "usbcr" in order > + - clocks : clock specifier for the musb controller clock > + - clock-names : should be "usb_clk" > + - interrupts : interrupt number for MUSB Core General interrupt > + and DMA interrupt > + - interrupt-names : must be "mc" and "dma" in order. > + - phys : phy specifier for the otg phy. > + - dr_mode : should be one of "host", "peripheral" or "otg". > + - mentor,multipoint: Should be "1" indicating the musb controller supports > + multipoint. This is MUSB configuration-specific > setting. > + - mentor,num-eps : Specifies the number of endpoints. This is also a > + MUSB configuration-specific setting. Should be set to > "8". > + - mentor,ram-bits : Specifies the ram address size. Should be set to "11". > + - mentor,power : Should be "500". This signifies the controller can > supply > + up to 500mA when operating in host mode. > + - phys : phandle of the USB phy. > + - usb_overcurrent : phandle to MUSB over-current note. It should have > + interrupt number for over-current detection logic. This node is only an interrupt? Then you should use interrupts-extended and make this interrupt the 3rd one. interrupts-extended will let you have different interrupt parents for each irq. > + > +Optional properties: > + - microchip,fifo-mode: Specifies layout of internal SRAM for end-point > fifos. > +Should be 0 (default) or 1. Make this a boolean. > + > +Example: > + aliases { > + usb1 = &usb1; > + phy1 = &usb1_phy; > + }; > + > + usb1: hsusb1_core@1f8e3000 { > + compatible = "microchip,pic32mzda-usb"; > + reg = <0x1f8e3000 0x1000>, > + <0x1f884000 0x1000>; > + reg-names = "mc", "usbcr"; > + interrupts = <132 IRQ_TYPE_EDGE_RISING>, > + <133 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "mc", "dma"; > + dr_mode = "host"; > + mentor,multipoint = <1>; > + mentor,num-eps = <8>; > + mentor,ram-bits = <11>; > + mentor,power = <500>; > + phys = <&usb1_phy>; > + clocks = <&rootclk PB5CLK>; > + clock-names = "usb_clk"; > + usb_overcurrent = <&usb1_overcurrent>; > + }; > + > + usb1_phy: hsusb1_phy@1f8e4000 { > + compatible = "usb-nop-xceiv"; > + reg = <0x1f8e4000 0x1000>; > + clocks = <&rootclk UPLLCLK>; > + clock-names = "main_clk"; > + clock-frequency = <2400>; > + }; > + > + usb1_overcurrent: hsusb1_oc@0 { > + interrupt-parent = <&gpio1>; > + interrupts = <11 IRQ_TYPE_EDGE_FALLING>; > + }; > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: xhci-plat: Make enum xhci_plat_type start at a non zero value
Hi Mathias, On Fri, 25 Mar 2016, Peter Griffin wrote: > Otherwise generic-xhci and xhci-platform which have no data get wrongly > detected as XHCI_PLAT_TYPE_MARVELL_ARMADA by xhci_plat_type_is(). > > This fixes a regression in v4.5 for STiH407 family SoC's which use the > synopsis dwc3 IP, whereby the disable_clk error path gets taken due to > wrongly being detected as XHCI_PLAT_TYPE_MARVELL_ARMADA and the hcd never > gets added. > > I suspect this will also fix other dwc3 DT platforms such as Exynos, > although I've only tested on STih410 SoC. > > Fixes: 4efb2f694114 ("usb: host: xhci-plat: add struct xhci_plat_priv") Can you take this 1 charcter fix for the next -rc? It fixes a regression introduced in commit 4efb2f694114 for usb3 for some DT platforms. Then Felipes series which re-works this code will hopefully be merged in the next merge window. regards, 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: [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits
Sorry if you get this twice, I was having some client problems, but wanted to make sure you got this one... Grigori Goronzy wrote: > With the new reinitialization method, configuring parity, > different frame lengths and different stop bit settings work as > expected on both CH340G and CH341A. This has been extensively > tested with a logic analyzer. > > v2: only set mark/space when parity is enabled, > simplifications, patch termios HW flags. > > Tested-by: Ryan Barber > Signed-off-by: Grigori Goronzy > --- > drivers/usb/serial/ch341.c | 40 ++-- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/serial/ch341.c > b/drivers/usb/serial/ch341.c index 6181616..99b4621 100644 > --- a/drivers/usb/serial/ch341.c > +++ b/drivers/usb/serial/ch341.c > @@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct *tty, > struct usb_serial_port *port, struct ktermios *old_termios) > { > struct ch341_private *priv = usb_get_serial_port_data(port); > - unsigned baud_rate; > unsigned long flags; > unsigned char ctrl; > int r; > @@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct *tty, > if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios)) > return; > > - baud_rate = tty_get_baud_rate(tty); > + priv->baud_rate = tty_get_baud_rate(tty); > > - priv->baud_rate = baud_rate; > + ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; > > - ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8; > + switch (C_CSIZE(tty)) { > + case CS5: > + ctrl |= CH341_LCR_CS5; > + break; > + case CS6: > + ctrl |= CH341_LCR_CS6; > + break; > + case CS7: > + ctrl |= CH341_LCR_CS7; > + break; > + default: > + tty->termios.c_cflag |= CS8; > + case CS8: > + ctrl |= CH341_LCR_CS8; > + break; > + } > + > + if (C_PARENB(tty)) { > + ctrl |= CH341_LCR_ENABLE_PAR; > + if (C_PARODD(tty)) > + ctrl |= CH341_LCR_PAR_EVEN; Are you sure this does the right thing now? this is, as best as I can tell, the inverse of what you had earlier, and doesn't read right, if this is working, then I suggest renaming _LCR_PAR_EVEN to LCR_PAR_ODD? Cheers, Karl P > + if (C_CMSPAR(tty)) > + ctrl |= CH341_LCR_MARK_SPACE; > + } > + > + if (C_CSTOPB(tty)) > + ctrl |= CH341_LCR_STOP_BITS_2; > > - if (baud_rate) { > + if (priv->baud_rate) { > spin_lock_irqsave(&priv->lock, flags); > priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS); > spin_unlock_irqrestore(&priv->lock, flags); > @@ -373,11 +398,6 @@ static void ch341_set_termios(struct tty_struct *tty, > > ch341_set_handshake(port->serial->dev, priv->line_control); > > - /* Unimplemented: > - * (cflag & CSIZE) : data bits [5, 8] > - * (cflag & PARENB) : parity {NONE, EVEN, ODD} > - * (cflag & CSTOPB) : stop bits [1, 2] > - */ > } > > static void ch341_break_ctl(struct tty_struct *tty, int break_state) > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe > linux-usb" in the body of a message to > majord...@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html signature.asc Description: OpenPGP Digital Signature
Re: XHCI is slow during boot (bios/efi) and leaves many dmesg messages
On Mon, Apr 11, 2016 at 11:22:12AM +0300, Felipe Balbi wrote: >Just to confirm, can you check that disabling LPM > solves the problem ? I tried your patch on 4.6.0-rc3 but it doesn't seem to change anything. Here's a log: Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: xHCI Host Controller Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: new USB bus registered, assigned bus number 1 Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: hcc params 0x200077c1 hci version 0x100 quirks 0x0004a090 Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: cache line size of 256 is not supported Apr 11 14:05:40 kernel: hub 1-0:1.0: USB hub found Apr 11 14:05:40 kernel: hub 1-0:1.0: 9 ports detected Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: xHCI Host Controller Apr 11 14:05:40 kernel: xhci_hcd :00:14.0: new USB bus registered, assigned bus number 2 Apr 11 14:05:40 kernel: usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. Apr 11 14:05:40 kernel: hub 2-0:1.0: USB hub found Apr 11 14:05:40 kernel: hub 2-0:1.0: 4 ports detected Apr 11 14:05:43 kernel: fbcon: inteldrmfb (fb0) is primary device Apr 11 14:05:43 kernel: ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) Apr 11 14:05:43 kernel: ata1.00: FORCE: horkage modified (noncq) Apr 11 14:05:43 kernel: ata1.00: unexpected _GTF length (8) Apr 11 14:05:43 kernel: ata1.00: ATA-8: APPLE SSD SM0512F, UXM2JA1Q, max UDMA/133 Apr 11 14:05:43 kernel: ata1.00: 977105060 sectors, multi 16: LBA48 NCQ (not used) Apr 11 14:05:43 kernel: ata1.00: unexpected _GTF length (8) Apr 11 14:05:43 kernel: ata1.00: configured for UDMA/133 Apr 11 14:05:43 kernel: scsi 0:0:0:0: Direct-Access ATA APPLE SSD SM0512 JA1Q PQ: 0 ANSI: 5 Apr 11 14:05:43 kernel: usb 1-3: new full-speed USB device number 2 using xhci_hcd Apr 11 14:05:43 kernel: tsc: Refined TSC clocksource calibration: 3000.001 MHz Apr 11 14:05:43 kernel: clocksource: tsc: mask: 0x max_cycles: 0x2b3e4685b41, max_idle_ns: 440795254784 ns Apr 11 14:05:43 kernel: clocksource: Switched to clocksource tsc Apr 11 14:05:43 kernel: Console: switching to colour frame buffer device 320x100 Apr 11 14:05:43 kernel: i915 :00:02.0: fb0: inteldrmfb frame buffer device Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] 977105060 512-byte logical blocks: (500 GB/466 GiB) Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] 4096-byte physical blocks Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] Write Protect is off Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Apr 11 14:05:43 kernel: sda: sda1 sda2 sda3 sda4 sda5 sda6 sda7 sda8 Apr 11 14:05:43 kernel: sd 0:0:0:0: [sda] Attached SCSI disk Apr 11 14:05:43 kernel: PM: Starting manual resume from disk Apr 11 14:05:43 kernel: PM: Hibernation image partition 8:7 present Apr 11 14:05:43 kernel: PM: Looking for hibernation image. Apr 11 14:05:43 kernel: PM: Image not found (code -22) Apr 11 14:05:43 kernel: PM: Hibernation image not present or could not be loaded. Apr 11 14:05:43 kernel: EXT4-fs (sda4): mounted filesystem with ordered data mode. Opts: (null) Apr 11 14:05:43 kernel: ip_tables: (C) 2000-2006 Netfilter Core Team Apr 11 14:05:43 kernel: ACPI: AC Adapter [ADP1] (on-line) Apr 11 14:05:43 kernel: ACPI: SBS HC: EC = 0x88045c40dc00, offset = 0x20, query_bit = 0x10 Apr 11 14:05:43 kernel: shpchp: Standard Hot Plug PCI Controller Driver version: 0.4 Apr 11 14:05:43 kernel: ACPI Warning: SystemIO range 0xEFA0-0xEFBF conflicts with OpRegion 0xEFA0-0xEFAF (\_SB.PCI0.SBUS.SMBI) (20160108/utaddress-255) Apr 11 14:05:43 kernel: ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver Apr 11 14:05:43 kernel: input: PC Speaker as /devices/platform/pcspkr/input/input5 Apr 11 14:05:43 kernel: snd_hda_intel :00:03.0: bound :00:02.0 (ops i915_exit [i915]) Apr 11 14:05:43 kernel: AVX2 version of gcm_enc/dec engaged. Apr 11 14:05:43 kernel: AES CTR mode by8 optimization enabled Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0: autoconfig for CS4208: line_outs=2 (0x12/0x13/0x0/0x0/0x0) type:speaker Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0:speaker_outs=0 (0x0/0x0/0x0/0x0/0x0) Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0:hp_outs=1 (0x10/0x0/0x0/0x0/0x0) Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0:mono: mono_out=0x0 Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0:dig-out=0x21/0x0 Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0:inputs: Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0: Internal Mic=0x1c Apr 11 14:05:43 kernel: snd_hda_codec_cirrus hdaudioC0D0: Mic=0x18 Apr 11 14:05:43 kernel: input: HDA Intel PCH Mic as /devices/pci:00/:00:1b.0/sound/card0/input6 Apr 11 14:05:43 kernel: wl: module license 'Mixed/Proprietary' tain
Re: [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits
On 2016-04-11 19:25, Karl Palsson wrote: Sorry if you get this twice, I was having some client problems, but wanted to make sure you got this one... Grigori Goronzy wrote: With the new reinitialization method, configuring parity, different frame lengths and different stop bit settings work as expected on both CH340G and CH341A. This has been extensively tested with a logic analyzer. v2: only set mark/space when parity is enabled, simplifications, patch termios HW flags. Tested-by: Ryan Barber Signed-off-by: Grigori Goronzy --- drivers/usb/serial/ch341.c | 40 ++-- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c index 6181616..99b4621 100644 --- a/drivers/usb/serial/ch341.c +++ b/drivers/usb/serial/ch341.c @@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { struct ch341_private *priv = usb_get_serial_port_data(port); - unsigned baud_rate; unsigned long flags; unsigned char ctrl; int r; @@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct *tty, if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios)) return; - baud_rate = tty_get_baud_rate(tty); + priv->baud_rate = tty_get_baud_rate(tty); - priv->baud_rate = baud_rate; + ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX; - ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8; + switch (C_CSIZE(tty)) { + case CS5: + ctrl |= CH341_LCR_CS5; + break; + case CS6: + ctrl |= CH341_LCR_CS6; + break; + case CS7: + ctrl |= CH341_LCR_CS7; + break; + default: + tty->termios.c_cflag |= CS8; + case CS8: + ctrl |= CH341_LCR_CS8; + break; + } + + if (C_PARENB(tty)) { + ctrl |= CH341_LCR_ENABLE_PAR; + if (C_PARODD(tty)) + ctrl |= CH341_LCR_PAR_EVEN; Are you sure this does the right thing now? this is, as best as I can tell, the inverse of what you had earlier, and doesn't read right, if this is working, then I suggest renaming _LCR_PAR_EVEN to LCR_PAR_ODD? No, this is absolutely wrong, of course. I only did some sporadic testing because my refactoring wasn't supposed to change functionality. Thanks for pointing it out! Grigori -- 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 is slow during boot (bios/efi) and leaves many dmesg messages
On Mon, Apr 11, 2016 at 02:15:24PM -0400, Janna Martl wrote: > On Mon, Apr 11, 2016 at 11:22:12AM +0300, Felipe Balbi wrote: > >Just to confirm, can you check that disabling LPM > > solves the problem ? > > I tried your patch on 4.6.0-rc3 but it doesn't seem to change anything. Also I'm quite sure I was using some 3.17.x (and probably 3.18.x) before this issue started for me, but I just tried booting 3.17.1 a few times and am getting the same behavior (there's a delay about half the time). I think the start of the issue might have coincided with trying to boot from usb (but I've booted from usb since then and it hasn't made any difference). -- J.M. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: gadget: composite: Clear reserved fields of SSP Dev Cap
On 4/10/2016 10:17 PM, Felipe Balbi wrote: > > Hi, > > John Youn writes: >> Set the reserved fields of the SuperSpeed Plus Device Capability >> descriptor to 0. Otherwise there might be stale data there which will >> cause USB CV to fail. >> >> Fixes: f228a8de242a ("usb: gadget: composite: Return SSP Dev Cap descriptor") >> Signed-off-by: John Youn >> --- >> >> v2: >> * Use cpu_to_le16() to set wReserved >> * Correct Felipe's email address >> >> drivers/usb/gadget/composite.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c >> index de9ffd6..556c78f 100644 >> --- a/drivers/usb/gadget/composite.c >> +++ b/drivers/usb/gadget/composite.c >> @@ -651,6 +651,8 @@ static int bos_desc(struct usb_composite_dev *cdev) >> ssp_cap->bLength = USB_DT_USB_SSP_CAP_SIZE(1); >> ssp_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY; >> ssp_cap->bDevCapabilityType = USB_SSP_CAP_TYPE; >> +ssp_cap->bReserved = 0; >> +ssp_cap->wReserved = cpu_to_le16(0); > > fixing endianness of 0 is kinda pointless, right ? > Yes, just being thorough :) You can take v1 if you prefer. I can resend if needed. Regards, John -- 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: DWC3: Clock Domain Crossing erratum description.
Hi Felipe, On 04/07/2016 08:10 AM, Felipe Balbi wrote: Hi, Kirill Dronov writes: I'm working on USB device bringup on Intel E3800 – based board. DWC3 core configured as DRD in device mode. The only connected device phy is SMSC 3310 (USB2 ULPI). DWC3 core version is 2.10A. Gadget zero driver can be loaded, but device enumeration fails: device is detected by host, speed is negotiated (HS), host successfully reset device. On device side – conndone interrupt is followed by linksts change with link_state 0. Then host sends USBREQ_GET_DESCRIPTOR and tries to set address but device does not react – no events generated. which kernel are you using ? It's 3.14.29 kernel (WindRiver Linux). I have backported dwc3 upstream patches up to ~v3.17-rc4 (+ all trace patches). But the same issue exists for original 3.14.29 kernel. Please collect driver traces and send them to us: # mount -t debugfs none /sys/kernel/debug # cd /sys/kernel/debug/tracing # echo 2048 > buffer_size_kb # echo 1 > events/dwc3/enable (now trigger the problem) # cp trace /path/to/non/volatile/media/ Send that trace file (you need to compress it, probably) I've attached both dev_dbg and trace outputs. Also attached regdump taken after enumeration failed and gzero is suspended. I'm not sure if I hit “run/stop metastability” issue [“STAR#9000525659: Clock Domain Crossing on DCTL in USB 2.0 Mode”]. I don't have DWC3 we have a workaround for that, you shouldn't hit it unless you removed the workaround. No, I didn't. databook or erratum description (other than mentioned in driver code). Can somebody provide more detailed description of STAR#9000525659? Where can I get DWC3 Databook? you need to talk to a Synopsys representative for that. Unfortunately Synopsis points out to Intel as SoC manufacturer and Intel share only short notes on driver configuration and loading - no Databook. -- regards, Kirill dwc3.dev_dbg.tgz Description: application/compressed-tar dwc3.trace.tgz Description: application/compressed-tar dwc3.regdump.tgz Description: application/compressed-tar
[PATCH v3] usb: serial: mos7840.c Support TIOCGRS485 and TIOCSRS485 ioctls()
Add callbacks to handle TIOCGRS485 and TIOCSRS485 ioctl calls in mos7840.c, allowing configuration of the chip's "scratchpad" register to strobe the DTR line while transmitting. This functionality is required for RS485 mode on the B&B Electronics USOPTL4-4P and USOPTL4-2P USB-to-RS485/422 hubs based on this chip. Signed-off-by: Aaron Marburg --- Changes relative to v2: -- As suggested, implemented handlers for TIOC?RS485 ioctl callbacks rather than command-line options to enable RS485 configurations. drivers/usb/serial/mos7840.c | 78 1 file changed, 78 insertions(+) diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index ed378fb..8a78143 100644 --- a/drivers/usb/serial/mos7840.c +++ b/drivers/usb/serial/mos7840.c @@ -1977,6 +1977,74 @@ static int mos7840_get_serial_info(struct moschip_port *mos7840_port, } /* + * mos7840_get_rs485_info + * function to handle RS485 "get" ioctl + */ + +static int mos7840_get_rs485_info(struct usb_serial_port *port, + struct serial_rs485 __user *retinfo) +{ + struct serial_rs485 tmp; + int status; + __u16 scratchpad; + + if (port == NULL) + return -EFAULT; + + if (!retinfo) + return -EFAULT; + + memset(&tmp, 0, sizeof(tmp)); + + status = mos7840_get_uart_reg(port, SCRATCH_PAD_REGISTER, &scratchpad); + if (status < 0) { + dev_dbg(&port->dev, "Reading ScratchPad (RS485 config) register failed status=0x%x\n", status); + return -EFAULT; + } + + tmp.flags |= (scratchpad & 0x80) ? SER_RS485_ENABLED : 0x00; + tmp.flags |= (scratchpad & 0x40) ? SER_RS485_RTS_ON_SEND : 0x00; + + if (copy_to_user(retinfo, &tmp, sizeof(*retinfo))) + return -EFAULT; + + return 0; +} + +/* + * mos7840_set_rs485_info + * function to handle RS485 "set" ioctl + */ + +static int mos7840_set_rs485_info(struct usb_serial_port *port, + struct serial_rs485 __user *retinfo) +{ + struct serial_rs485 tmp; + int status; + __u16 scratchpad = 0; + + if (port == NULL) + return -EFAULT; + + if (!retinfo) + return -EFAULT; + + if (copy_from_user(&tmp, retinfo, sizeof(*retinfo))) + return -EFAULT; + + if (tmp.flags & SER_RS485_ENABLED) + scratchpad = 0x80 | ((tmp.flags & SER_RS485_RTS_ON_SEND) ? 0x40 : 0x00); + + status = mos7840_set_uart_reg(port, SCRATCH_PAD_REGISTER, scratchpad); + if (status < 0) { + dev_dbg(&port->dev, "Writing ScratchPad (RS485 config) register failed status=0x%x\n", status); + return -EFAULT; + } + + return 0; +} + +/* * SerialIoctl * this function handles any ioctl calls to the driver */ @@ -2010,6 +2078,16 @@ static int mos7840_ioctl(struct tty_struct *tty, case TIOCSSERIAL: dev_dbg(&port->dev, "%s TIOCSSERIAL\n", __func__); break; + + case TIOCGRS485: + dev_dbg(&port->dev, "%s TIOCGRS485\n", __func__); + return mos7840_get_rs485_info(port, argp); + + case TIOCSRS485: + dev_dbg(&port->dev, "%s TIOCSRS485\n", __func__); + return mos7840_set_rs485_info(port, argp); + + default: break; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc2: Add reset control to dwc2
On Thu, 7 Apr 2016, John Youn wrote: > Can you move into the lowlevel_hw_init()? The probe is a bit bloated > and we have these existing "lowlevel" functions where we consolidate > stuff like this. Same with the assert/deassert if possible. Yes, I can do that. Thanks for the review. BR, Dinh -- 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
[PATCHv2] usb: dwc2: Add reset control to dwc2
From: Dinh Nguyen Allow for platforms that have a reset controller driver in place to bring the USB IP out of reset. Signed-off-by: Dinh Nguyen --- v2: move to lowlevel_hw_init() --- drivers/usb/dwc2/core.h |1 + drivers/usb/dwc2/platform.c | 15 +++ 2 files changed, 16 insertions(+) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 3c58d63..f748132 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -837,6 +837,7 @@ struct dwc2_hsotg { void *priv; int irq; struct clk *clk; + struct reset_control *reset; unsigned int queuing_high_bandwidth:1; unsigned int srp_success:1; diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 88629be..be6b18a 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -45,6 +45,7 @@ #include #include #include +#include #include @@ -337,6 +338,17 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg) { int i, ret; + hsotg->reset = devm_reset_control_get(&dev->dev, "dwc2"); + if (IS_ERR(hsotg->reset)) { + dev_info(&dev->dev, "Could not get reset control!\n"); + if (PTR_ERR(hsotg->reset) == -EPROBE_DEFER) + return -EPROBE_DEFER; + hsotg->reset = NULL; + } + + if(hsotg->reset) + reset_control_deassert(hsotg->reset); + /* Set default UTMI width */ hsotg->phyif = GUSBCFG_PHYIF16; @@ -434,6 +446,9 @@ static int dwc2_driver_remove(struct platform_device *dev) if (hsotg->ll_hw_enabled) dwc2_lowlevel_hw_disable(hsotg); + if (hsotg->reset) + reset_control_assert(hsotg->reset); + return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: dwc2: Add reset control to dwc2
Hi Dinh, [auto build test ERROR on balbi-usb/next] [also build test ERROR on v4.6-rc3 next-20160411] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/dinguyen-opensource-altera-com/usb-dwc2-Add-reset-control-to-dwc2/20160412-055956 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next config: x86_64-randconfig-x010-201615 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/usb/dwc2/platform.c: In function 'dwc2_lowlevel_hw_init': >> drivers/usb/dwc2/platform.c:341:41: error: 'dev' undeclared (first use in >> this function) hsotg->reset = devm_reset_control_get(&dev->dev, "dwc2"); ^ drivers/usb/dwc2/platform.c:341:41: note: each undeclared identifier is reported only once for each function it appears in vim +/dev +341 drivers/usb/dwc2/platform.c 335 } 336 337 static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg) 338 { 339 int i, ret; 340 > 341 hsotg->reset = devm_reset_control_get(&dev->dev, "dwc2"); 342 if (IS_ERR(hsotg->reset)) { 343 dev_info(&dev->dev, "Could not get reset control!\n"); 344 if (PTR_ERR(hsotg->reset) == -EPROBE_DEFER) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
ジェームズHartopから
ジェームズHartopから、 私はあなたとあなたの家族がやっているか、私の手紙が今日健康とあなたの最高の気分でお会いすることを期待しますか?親切にあなたのプライバシーに侵入するための私を許してください。あなたは私達の両方の相互利益になります金融ビジネス関係で信頼することはできますか?私はあなたが私はあなたを伝えることを約午前何に興味を持つであろうことを期待して、あなたの国の国際ビジネス情報から自分の名前と連絡先を得ました。 私はここイギリスでHarlesdenからジェームズ・ウィリアムHartop、北西ロンドン、と思います。私は、UBS銀行、ロンドンのために働きます。私は私達の両方に莫大な利益をもたらすであろうビジネスの提案についてあなたを書いています。私の部署では、UBS AGのカバレッジおよびアドバイザリーのマネージングディレクター/ヘッドなので、私は次のいずれかに属するアカウントで22.3百万英ポンドスターリング(二十二百三十万英ポンドスターリング)の放棄合計を発見しました私たちの外国人の故人の顧客、彼の死と彼の家族に生じたヘリコプターのクラッシュ2010年1月10日の犠牲者だった億万長者のビジネスモーグル後期氏モイゼスサバマスリ、メキシコからのユダヤ人。サバは47歳でした。また、衝突時のチョッパーに彼の妻、息子アブラハム(アルバート)と彼の義理の娘でした。パイロットはまた、死んでいました。 ご連絡の選択は、特にによるトランザクションの感度および本明細書の機密保持のために、あなたが住んでいる場所の地理的な性質から、興奮しています。現在、私たちの銀行は来アップするために請求のために親戚のいずれかを待っている誰もがそれをやっていません。私は個人的に、今、私は22.3百万ポンドの価値がこのアカウントの収益があなたに支払うことができるようにあなたの同意が故人の親族/ウィル受益次のようにあなたを提供しようと5年間の親戚を見つけるに失敗しました。 これは支払わまたは共有これらの割合で、私には60%と、あなたに40%になります。私たちが作っているこの主張をバックアップするために使用することができ、必要なすべての法的文書を確保しています。私が必要とするすべての文書にあなたの名前に充填し、正当な受益者としてあなたを証明するためにここに法廷でそれを合法化することです。私が今必要とするすべては、私たちはこのトランザクションを介して、参照可能にするためにあなたの正直な協力、機密性と信頼です。私は、これは法律の違反からあなたを保護する正当な配置の下で実行されることを保証します。 我々はそれを介して、実行するために7日を持っているように、私に次のようにしてください。これはPLEASE非常に急務です。 1.氏名: 2.あなたの直接携帯電話番号: 3.あなたの連絡先住所: 4.あなたの職業: 5.あなたの国籍: 6.あなたの性別/年齢: 系統的検索を経てきたが、私はあなたがこの提案が興味深い見つけることを期待してあなたに連絡することを決めました。より多くの情報をご提出します。このメッセージの確認に喜ばとご関心を示しています。私はあなたの決定を知らせるのではなく待っている私を保つに努めます。 よろしく、 ジェームズW.Hartop UBS AG、投資銀行 1フィンズベリー・アベニュー、ロンドン EC2M 2PP、イギリス。 www.ubs.comN�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�
[RFC] Change bcdUSB in older kernels to return 0x0310 for SuperSpeed
Hi, I would like to port the following commit from 4.6 to older kernels. 1a85329171094951956a37acc8abb7e51c1e742e ("usb: gadget: composite: Return bcdUSB 0x0310") https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=1a85329171094951956a37acc8abb7e51c1e742e There is a some concern that revving the bcdUSB to 0x0310 will cause problems for SuperSpeed devices based on older kernels that don't otherwise have any USB 3.1 specific code. On the device side I don't think there will be any problems because there aren't really any checks against bcdUSB. We just set it to 0x0300 (0x0310 for kernel 4.6+) based on if the gadget tells us it supports SuperSpeed or higher. And I don't think there should be any problems for a host connecting to these devices since any SuperSpeed capable host should know about bcdUSB = 0x03XX. And I don't think the USB spec defines any different functionality based solely on 0x0300 vs 0x0310. The reason for this backport is that the USB CV tool now checks this and ostensibly USB IF certification will require this for "new" devices. So if you are basing a new device on a kernel older than 4.6, it will not pass CV. Any comments or recommendations or anything I missed? Thanks, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
> Hi, > > "Du, Changbin" writes: > >> > >> >> > + dwc->regset = NULL; > >> >> > >> >> setting regset to NULL is unnecessary. We only call > dwc3_debugfs_exit() > >> >> when removing the driver. > >> >> > >> >> -- > >> >> Balbi > >> > I'd like keep this line even it is unnecessary, because It is a good > >> > habit to > >> > Avoid wild pointers. Just like the dwc->root = NULL. > >> > >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. > >> > >> -- > >> Balbi > > I agree the dwc will be freed in current code. But the 'free' logical is out > > of the debugfs code. They should be treat as some logical independent. > Per > > this point, I still think set pointer to null is not bad. For example, if > > dwc3 > core > > code invoke dwc3_debugfs_exit twice by mistake(just an example case, > not > > really), then no crash/impact for the second call. > > the second call should crash because it's clearly wrong ;-) If dwc3 ever > calls dwc3_debugfs_exit() twice, it really deserves to crash. It's > something so wrong that we want the verbosity and urgency of a kernel > oops to make sure we fix it ASAP. > > If, however, we set it to null, it might be years before we notice > anything's wrong. > > -- > Balbi Hmm, I agree from this point. I will combine this patch with other two patches (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well, Is it ok for you? Thx, Du, Changbin -- 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
[PATCHv3] usb: dwc2: Add reset control to dwc2
From: Dinh Nguyen Allow for platforms that have a reset controller driver in place to bring the USB IP out of reset. Signed-off-by: Dinh Nguyen --- v3: fix compile error v2: move to lowlevel_hw_init() --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/platform.c | 15 +++ 2 files changed, 16 insertions(+) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 3c58d63..f748132 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -837,6 +837,7 @@ struct dwc2_hsotg { void *priv; int irq; struct clk *clk; + struct reset_control *reset; unsigned int queuing_high_bandwidth:1; unsigned int srp_success:1; diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 88629be..6987ef3 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -45,6 +45,7 @@ #include #include #include +#include #include @@ -337,6 +338,17 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg) { int i, ret; + hsotg->reset = devm_reset_control_get(hsotg->dev, "dwc2"); + if (IS_ERR(hsotg->reset)) { + dev_info(hsotg->dev, "Could not get reset control!\n"); + if (PTR_ERR(hsotg->reset) == -EPROBE_DEFER) + return -EPROBE_DEFER; + hsotg->reset = NULL; + } + + if(hsotg->reset) + reset_control_deassert(hsotg->reset); + /* Set default UTMI width */ hsotg->phyif = GUSBCFG_PHYIF16; @@ -434,6 +446,9 @@ static int dwc2_driver_remove(struct platform_device *dev) if (hsotg->ll_hw_enabled) dwc2_lowlevel_hw_disable(hsotg); + if (hsotg->reset) + reset_control_assert(hsotg->reset); + return 0; } -- 2.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/3] usb: storage: scsiglue: limit USB3 devices to 2048 sectors
Alan Stern writes: > On Mon, 11 Apr 2016, Felipe Balbi wrote: > >> USB3 devices, because they are much newer, have much >> less chance of having issues with larger transfers. >> >> We still keep a limit because anything above 2048 >> sectors really rendered negligible speed >> improvements, so we will simply ignore >> that. Transferring 1MiB should already give us >> pretty good performance. >> >> Signed-off-by: Felipe Balbi >> --- >> drivers/usb/storage/scsiglue.c | 16 +++- >> 1 file changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c >> index 9da1fb3d0ff4..4512cf78a284 100644 >> --- a/drivers/usb/storage/scsiglue.c >> +++ b/drivers/usb/storage/scsiglue.c >> @@ -115,13 +115,19 @@ static int slave_configure(struct scsi_device *sdev) >> { >> struct us_data *us = host_to_us(sdev->host); >> >> -/* Many devices have trouble transferring more than 32KB at a time, >> - * while others have trouble with more than 64K. At this time we >> - * are limiting both to 32K (64 sectores). >> - */ >> -if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) { >> +if (us->pusb_dev->speed >= USB_SPEED_SUPER) { >> +/* USB3 devices will be limited to 2048 sectors. This gives us >> + * better throughput on most devices. >> + */ >> +blk_queue_max_hw_sectors(sdev->request_queue, 2048); >> +} else if (us->fflags & (US_FL_MAX_SECTORS_64 | US_FL_MAX_SECTORS_MIN)) >> { >> unsigned int max_sectors = 64; >> >> +/* Many devices have trouble transferring more than 32KB at a >> time, >> + * while others have trouble with more than 64K. At this time we >> + * are limiting both to 32K (64 sectores). >> + */ >> + >> if (us->fflags & US_FL_MAX_SECTORS_MIN) >> max_sectors = PAGE_SIZE >> 9; >> if (queue_max_hw_sectors(sdev->request_queue) > max_sectors) > > I should have noticed this before. If something goes wrong with the > 2048-sector limit, this doesn't permit the user to override. Shouldn't > the test for USB_SPEED_SUPER go after the other FL_MAX_SECTORS tests? you mean that a USB3 device with US_FL_MAX_SECTORS* should still respect the 64 limit ? yeah I can do that. I'll send a new version tomorrow (so I can wait for confirmation from you) -- balbi signature.asc Description: PGP signature
RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
Hi Changbin, "Du, Changbin" writes: >> Hi, >> >> "Du, Changbin" writes: >> >> >> >> >> > +dwc->regset = NULL; >> >> >> >> >> >> setting regset to NULL is unnecessary. We only call >> dwc3_debugfs_exit() >> >> >> when removing the driver. >> >> >> >> >> >> -- >> >> >> Balbi >> >> > I'd like keep this line even it is unnecessary, because It is a good >> >> > habit to >> >> > Avoid wild pointers. Just like the dwc->root = NULL. >> >> >> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. >> >> >> >> -- >> >> Balbi >> > I agree the dwc will be freed in current code. But the 'free' logical is >> > out >> > of the debugfs code. They should be treat as some logical independent. >> Per >> > this point, I still think set pointer to null is not bad. For example, if >> > dwc3 >> core >> > code invoke dwc3_debugfs_exit twice by mistake(just an example case, >> not >> > really), then no crash/impact for the second call. >> >> the second call should crash because it's clearly wrong ;-) If dwc3 ever >> calls dwc3_debugfs_exit() twice, it really deserves to crash. It's >> something so wrong that we want the verbosity and urgency of a kernel >> oops to make sure we fix it ASAP. >> >> If, however, we set it to null, it might be years before we notice >> anything's wrong. >> >> -- >> Balbi > > Hmm, I agree from this point. I will combine this patch with other two patches > (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well, you are creating a dependency that doesn't exist. Please stop that. You should have two separate branches based on v4.6-rc3 (or, if you prefer, one based on my testing/fixes and another based on my testing/next). On one branch you have *only* $subject and you fix *all* the memory leaks. On the other branch you have the other two patches. Ignore the fact that we might have a conflict, that's for git (and maintainers) to handle when they happen. Again, don't create dependencies between fixes for the -rc cycle and changes for the next merge window. > Is it ok for you? yeah, please remove root = NULL as that's completely unnecessary, but split these patches in separate branches and fix all memory leaks. -- balbi signature.asc Description: PGP signature
Re: [RFC] Change bcdUSB in older kernels to return 0x0310 for SuperSpeed
Hi, John Youn writes: > Hi, > > I would like to port the following commit from 4.6 to older kernels. > > 1a85329171094951956a37acc8abb7e51c1e742e ("usb: gadget: composite: > Return bcdUSB 0x0310") > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=1a85329171094951956a37acc8abb7e51c1e742e > > There is a some concern that revving the bcdUSB to 0x0310 will cause > problems for SuperSpeed devices based on older kernels that don't > otherwise have any USB 3.1 specific code. > > On the device side I don't think there will be any problems because > there aren't really any checks against bcdUSB. We just set it to > 0x0300 (0x0310 for kernel 4.6+) based on if the gadget tells us it > supports SuperSpeed or higher. > > And I don't think there should be any problems for a host connecting > to these devices since any SuperSpeed capable host should know about > bcdUSB = 0x03XX. And I don't think the USB spec defines any different > functionality based solely on 0x0300 vs 0x0310. well, there is the actual SuperSpeedPlus speed. And, sure, that won't be supported by XHCI driver on those older kernels which begs the question: What's the benefit of backporting that change ? > The reason for this backport is that the USB CV tool now checks this > and ostensibly USB IF certification will require this for "new" > devices. So if you are basing a new device on a kernel older than 4.6, > it will not pass CV. Wait, even SuperSpeed GEN1-only devices *must* set bcdUSB to 0x0310 ? Do you have a reference to that ? I find that pretty odd because this means that if I take any of my USB3 devices that I already have today, and run them against latest USBCV, they'll all fail and that's not very nice. Are you, perhaps, miss-reading the recommendation ? For example, when we got the LPM ECN, all device were required to set bcdUSB to 0x0210 iff they supported LPM. This means that you would only fail USBCV if you had bcdUSB set to 0x0210 and didn't support LPM. You can still certify USB 2.0 without LPM, right ? -- balbi signature.asc Description: PGP signature