[PATCH] uhci-platform: use devm_ioremap resource
This patch replaces the memory allocation using request_mem_region and
the ioremap by a single call to managed interface devm_ioremap_reource.
The corresponding calls to release_mem_region and iounmap in the probe
and release functions are now unnecessary and are removed. Also a label
is done away with and linux/device.h is added to make sure the devm_*()
outine declarations are unambiguously available.
Signed-off-by: Himangi Saraogi
Acked-by: Julia Lawall
---
Not compile tested due to incompatible architecture.
drivers/usb/host/uhci-platform.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
index 01833ab..34248a6 100644
--- a/drivers/usb/host/uhci-platform.c
+++ b/drivers/usb/host/uhci-platform.c
@@ -8,6 +8,7 @@
*/
#include
+#include
#include
static int uhci_platform_init(struct usb_hcd *hcd)
@@ -88,33 +89,23 @@ static int uhci_hcd_platform_probe(struct platform_device
*pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
- if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
- pr_err("%s: request_mem_region failed\n", __func__);
- ret = -EBUSY;
+ hcd->regs = devm_ioremap_resource(&pdev->dev, hcd->rsrc_start,
+ hcd->rsrc_len);
+ if (IS_ERR(hcd->regs)) {
+ ret = PTR_ERR(hcd->regs);
goto err_rmr;
}
-
- hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
- if (!hcd->regs) {
- pr_err("%s: ioremap failed\n", __func__);
- ret = -ENOMEM;
- goto err_irq;
- }
uhci = hcd_to_uhci(hcd);
uhci->regs = hcd->regs;
ret = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
if (ret)
- goto err_uhci;
+ goto err_rmr;
device_wakeup_enable(hcd->self.controller);
return 0;
-err_uhci:
- iounmap(hcd->regs);
-err_irq:
- release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
err_rmr:
usb_put_hcd(hcd);
@@ -126,8 +117,6 @@ static int uhci_hcd_platform_remove(struct platform_device
*pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
usb_remove_hcd(hcd);
- iounmap(hcd->regs);
- release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
return 0;
--
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
[PATCH] usb: host: uhci-grlib.c : use devm_ functions
The various devm_ functions allocate memory that is released when a
driver detaches. This patch uses devm_ioremap_resource for data
that is allocated in the probe function of a platform device and
is only freed in the remove function. The corresponding free functions
are removed and two labels are done away with. Also, linux/device.h
is added to make sure the devm_*() routine declarations are
unambiguously available.
Signed-off-by: Himangi Saraogi
Acked-by: Julia Lawall
---
drivers/usb/host/uhci-grlib.c | 26 +++---
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/host/uhci-grlib.c b/drivers/usb/host/uhci-grlib.c
index ab25dc3..2468fb0 100644
--- a/drivers/usb/host/uhci-grlib.c
+++ b/drivers/usb/host/uhci-grlib.c
@@ -17,6 +17,7 @@
* (C) Copyright 2004-2007 Alan Stern, st...@rowland.harvard.edu
*/
+#include
#include
#include
#include
@@ -113,23 +114,17 @@ static int uhci_hcd_grlib_probe(struct platform_device
*op)
hcd->rsrc_start = res.start;
hcd->rsrc_len = resource_size(&res);
- if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
- printk(KERN_ERR "%s: request_mem_region failed\n", __FILE__);
- rv = -EBUSY;
- goto err_rmr;
- }
-
irq = irq_of_parse_and_map(dn, 0);
if (irq == NO_IRQ) {
printk(KERN_ERR "%s: irq_of_parse_and_map failed\n", __FILE__);
rv = -EBUSY;
- goto err_irq;
+ goto err_rmr;
}
- hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
- if (!hcd->regs) {
- printk(KERN_ERR "%s: ioremap failed\n", __FILE__);
- rv = -ENOMEM;
+ hcd->regs = devm_ioremap_resource(&op->dev, hcd->rsrc_start,
+ hcd->rsrc_len);
+ if (IS_ERR(hcd->regs)) {
+ rv = PTR_ERR(hcd->regs);
goto err_ioremap;
}
@@ -139,17 +134,13 @@ static int uhci_hcd_grlib_probe(struct platform_device
*op)
rv = usb_add_hcd(hcd, irq, 0);
if (rv)
- goto err_uhci;
+ goto err_ioremap;
device_wakeup_enable(hcd->self.controller);
return 0;
-err_uhci:
- iounmap(hcd->regs);
err_ioremap:
irq_dispose_mapping(irq);
-err_irq:
- release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
err_rmr:
usb_put_hcd(hcd);
@@ -164,10 +155,7 @@ static int uhci_hcd_grlib_remove(struct platform_device
*op)
usb_remove_hcd(hcd);
- iounmap(hcd->regs);
irq_dispose_mapping(hcd->irq);
- release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-
usb_put_hcd(hcd);
return 0;
--
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] usb: host: uhci-grlib.c : use devm_ functions
On Sun, Jun 08, 2014 at 01:42:13PM +0530, Himangi Saraogi wrote:
> The various devm_ functions allocate memory that is released when a
> driver detaches. This patch uses devm_ioremap_resource for data
> that is allocated in the probe function of a platform device and
> is only freed in the remove function. The corresponding free functions
> are removed and two labels are done away with. Also, linux/device.h
> is added to make sure the devm_*() routine declarations are
> unambiguously available.
>
> Signed-off-by: Himangi Saraogi
> Acked-by: Julia Lawall
Included Jan Andersson (the original author) in the loop.
Sam
> ---
> drivers/usb/host/uhci-grlib.c | 26 +++---
> 1 file changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/host/uhci-grlib.c b/drivers/usb/host/uhci-grlib.c
> index ab25dc3..2468fb0 100644
> --- a/drivers/usb/host/uhci-grlib.c
> +++ b/drivers/usb/host/uhci-grlib.c
> @@ -17,6 +17,7 @@
> * (C) Copyright 2004-2007 Alan Stern, st...@rowland.harvard.edu
> */
>
> +#include
> #include
> #include
> #include
> @@ -113,23 +114,17 @@ static int uhci_hcd_grlib_probe(struct platform_device
> *op)
> hcd->rsrc_start = res.start;
> hcd->rsrc_len = resource_size(&res);
>
> - if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
> - printk(KERN_ERR "%s: request_mem_region failed\n", __FILE__);
> - rv = -EBUSY;
> - goto err_rmr;
> - }
> -
> irq = irq_of_parse_and_map(dn, 0);
> if (irq == NO_IRQ) {
> printk(KERN_ERR "%s: irq_of_parse_and_map failed\n", __FILE__);
> rv = -EBUSY;
> - goto err_irq;
> + goto err_rmr;
> }
>
> - hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
> - if (!hcd->regs) {
> - printk(KERN_ERR "%s: ioremap failed\n", __FILE__);
> - rv = -ENOMEM;
> + hcd->regs = devm_ioremap_resource(&op->dev, hcd->rsrc_start,
> + hcd->rsrc_len);
> + if (IS_ERR(hcd->regs)) {
> + rv = PTR_ERR(hcd->regs);
> goto err_ioremap;
> }
>
> @@ -139,17 +134,13 @@ static int uhci_hcd_grlib_probe(struct platform_device
> *op)
>
> rv = usb_add_hcd(hcd, irq, 0);
> if (rv)
> - goto err_uhci;
> + goto err_ioremap;
>
> device_wakeup_enable(hcd->self.controller);
> return 0;
>
> -err_uhci:
> - iounmap(hcd->regs);
> err_ioremap:
> irq_dispose_mapping(irq);
> -err_irq:
> - release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
> err_rmr:
> usb_put_hcd(hcd);
>
> @@ -164,10 +155,7 @@ static int uhci_hcd_grlib_remove(struct platform_device
> *op)
>
> usb_remove_hcd(hcd);
>
> - iounmap(hcd->regs);
> irq_dispose_mapping(hcd->irq);
> - release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
> -
> usb_put_hcd(hcd);
>
> return 0;
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
--
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 net-next v2 00/12] cdc_ncm: add buffer tuning and stats using ethtool
On Mon, 2014-05-19 at 09:36 +0200, Bjørn Mork wrote: > Lars Melin writes: > > > Your target audience is embedded systems with limited cpu power and > > buffer memory, right? > > If so, then you can't expect them to have ethtool included and their > > developers are not likely to be happy over having to "waste" another > > 100KB in order to tune a 20KB driver. > > My vote goes for sysfs. > > That's of course a very good point. > > I will argue that you often need ethtool anyway for basic stuff like > forcing duplex/speed, enabling debug messages, turning on/off pause > frames etc. But I don't doubt you know what you are talking about here > :-) So I guess many small embedded devices don't include an ethtool > binary by default. I do wonder how much we should adapt to that though? > I understand that you don't see it this way, but others might actually > see it as an advantage if we're forcing ethtool onto these devices... [...] By the way, ethtool can now be configured without pretty-printing of register/EEPROM dumps. This reduces it in size from ~240K to ~60K. If that's still not small enough, the answer might be to put a limited reimplementation of ethtool in busybox, toybox or similar. Ben. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity. signature.asc Description: This is a digitally signed message part
Re: non-working UVC device 058f:5608
Hi Johannes, On Saturday 07 June 2014 23:51:43 Johannes Berg wrote: > I just obtained a new (special-purpose) webcam, and it doesn't seem to > work at all. On kernel torvals/linux.git next branch, it doesn't even > really connect, on 3.13 (which I'm running on my laptop) I get errors > like this: > > xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of > current TD > > when running uvccapture. When running e.g. cheese or the like, the > screen stays blank. uvccapture also reports: > > ioctl querycontrol error 22 > > and then the kernel message repeats forever, while I can't even exit > uvccapture unless I kill it hard, at which point I get > > xhci_hcd :00:14.0: Signal while waiting for configure endpoint command > usb 1-3.4.4.3: Not enough bandwidth for altsetting 0 > > from the kernel. This looks like low-level USB issues, CC'ing the linux-usb mailing list. > The device really is detected as UVC, of course: > > [ 3423.299311] usb 1-3.4.4.3: new high-speed USB device number 12 using > xhci_hcd > [ 3423.426280] usb 1-3.4.4.3: New USB device found, idVendor=058f, > idProduct=5608 > [ 3423.426286] usb 1-3.4.4.3: New USB device strings: Mfr=3, Product=1, > SerialNumber=0 [ 3423.426290] usb 1-3.4.4.3: Product: USB 2.0 PC Camera > [ 3423.426293] usb 1-3.4.4.3: Manufacturer: Alcor Micro, Corp. > [ 3423.432137] uvcvideo: Found UVC 1.00 device USB 2.0 PC Camera > (058f:5608) > [ 3423.435383] input: USB 2.0 PC Camera as > /devices/pci:00/:00:14.0/usb1/1-3/1-3.4/1-3.4.4/1-3.4.4.3/1-3.4.4.3 > :1.0/input/input36 > > (see also full lsusb below) > > I see a device from the same manufacturer has a kernel driver as a > vendor device but actually being UVC, but this one reports being UVC and > doesn't really work. > > Any thoughts? Just to rule out hardware defects I connected it to my > windows 7 work machine and it works fine without even installing a > driver. Could you try connecting it to an EHCI controller instead of XHCI on a Linux machine ? > I can arrange remote access to the device (maybe as a VM to be able to > experiment with the kernel more easily?) if anyone wants it. > > johannes > > lsusb: > > Bus 001 Device 012: ID 058f:5608 Alcor Micro Corp. > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 2.00 > bDeviceClass 239 Miscellaneous Device > bDeviceSubClass 2 ? > bDeviceProtocol 1 Interface Association > bMaxPacketSize064 > idVendor 0x058f Alcor Micro Corp. > idProduct 0x5608 > bcdDevice0.03 > iManufacturer 3 Alcor Micro, Corp. > iProduct1 USB 2.0 PC Camera > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 407 > bNumInterfaces 2 > bConfigurationValue 1 > iConfiguration 0 > bmAttributes 0x80 > (Bus Powered) > MaxPower 200mA > Interface Association: > bLength 8 > bDescriptorType11 > bFirstInterface 0 > bInterfaceCount 2 > bFunctionClass 14 Video > bFunctionSubClass 3 Video Interface Collection > bFunctionProtocol 0 > iFunction 1 USB 2.0 PC Camera > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSetting 0 > bNumEndpoints 1 > bInterfaceClass14 Video > bInterfaceSubClass 1 Video Control > bInterfaceProtocol 0 > iInterface 1 USB 2.0 PC Camera > VideoControl Interface Descriptor: > bLength13 > bDescriptorType36 > bDescriptorSubtype 1 (HEADER) > bcdUVC 1.00 > wTotalLength 85 > dwClockFrequency 30.00MHz > bInCollection 1 > baInterfaceNr( 0) 1 > VideoControl Interface Descriptor: > bLength18 > bDescriptorType36 > bDescriptorSubtype 2 (INPUT_TERMINAL) > bTerminalID 1 > wTerminalType 0x0201 Camera Sensor > bAssocTerminal 0 > iTerminal 0 > wObjectiveFocalLengthMin 0 > wObjectiveFocalLengthMax 0 > wOcularFocalLength0 > bControlSize 3 > bmControls 0x > VideoControl Interface Descriptor: > bLength 9 > bDescriptorType36 > bDescriptorSubtype 3 (OUTPUT_TERMINAL) > bTerminalID 3 > wTerminalType 0x0101 USB Streaming > bAssocTerminal 0 > bSourceID
Re: [PATCH 1/4] phy: Add provision for calibrating phy.
On Fri, Jun 06, 2014 at 08:12:12PM +0800, Vivek Gautam wrote: > Some PHY controllers may need to calibrate certain > PHY settings after initialization of the controller and > sometimes even after initializing the PHY-consumer too. > Add support for the same in order to let consumers do so in need. > > Signed-off-by: vivek Gautam > --- > drivers/phy/phy-core.c | 36 > include/linux/phy/phy.h |7 +++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > index 74d4346..92d31a3 100644 > --- a/drivers/phy/phy-core.c > +++ b/drivers/phy/phy-core.c > @@ -376,6 +376,42 @@ int phy_power_off(struct phy *phy) > EXPORT_SYMBOL_GPL(phy_power_off); > > /** > + * phy_calibrate - calibrate a phy post initialization > + * @phy: Pointer to 'phy' from consumer > + * > + * For certain PHYs, it may be needed to calibrate few phy parameters > + * post initialization. The need to calibrate may arise after the For USB you may need to calibrate phy after each new connection. If so, why not to use already existing struct usb_phy's notify_connect. pratyush -- 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: ohci-exynos: Use NULL instead of 0
On Friday, June 06, 2014 5:44 PM, Sachin Kamat wrote:
>
> The third argument of devm_of_phy_get expects a pointer.
> Hence use NULL instead of 0.
>
> Signed-off-by: Sachin Kamat
Acked-by: Jingoo Han
Best regards,
Jingoo Han
> ---
> drivers/usb/host/ohci-exynos.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 060a6a414750..a72ab8fe8cd3 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -87,7 +87,7 @@ static int exynos_ohci_get_phy(struct device *dev,
> return -EINVAL;
> }
>
> - phy = devm_of_phy_get(dev, child, 0);
> + phy = devm_of_phy_get(dev, child, NULL);
> of_node_put(child);
> if (IS_ERR(phy)) {
> ret = PTR_ERR(phy);
> --
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: ehci-exynos: Use NULL instead of 0
On Friday, June 06, 2014 5:44 PM, Sachin Kamat wrote:
>
> The third argument of devm_of_phy_get expects a pointer.
> Hence use NULL instead of 0. Fixes the following warning:
> drivers/usb/host/ehci-exynos.c:91:51: warning: Using plain integer as NULL
> pointer
>
> Signed-off-by: Sachin Kamat
Acked-by: Jingoo Han
Best regards,
Jingoo Han
> ---
> drivers/usb/host/ehci-exynos.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index d1c76216350f..cda0a2f5c467 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -88,7 +88,7 @@ static int exynos_ehci_get_phy(struct device *dev,
> return -EINVAL;
> }
>
> - phy = devm_of_phy_get(dev, child, 0);
> + phy = devm_of_phy_get(dev, child, NULL);
> of_node_put(child);
> if (IS_ERR(phy)) {
> ret = PTR_ERR(phy);
> --
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V3 0/2] usb: gadget: zero: Add support for interrupt EP
Felipe, Alan, On 5/23/2014 12:01 PM, Amit VIRDI wrote: This patchset adds support for interrupt EP and the corresponding test cases to gadget zero. The code has been rebased and tested on Kernel v3.15-rc5 V2 -> V3 - Rectified wMaxPacketSize for FS from 1023 to 64 - Modified default value of interrupt interval for FS to 1 ms V1 -> V2 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2 Isoc) to 2 EPs (Interrupt only) RFC -> V1 - Added support for configuring interrupt EP attributes from configfs interface Amit Virdi (2): usb: gadget: zero: Add support for interrupt EP usbtest: Add interrupt EP testcases Any comment on this patchset? Regards Amit Virdi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] ARM: Berlin: USB support
On Thu, Jun 05, 2014 at 05:48:37PM +0200, Antoine Ténart wrote: > This series adds the support for the Marvell Berlin USB controllers, > the USB PHYs and also adds a reset controller. > > The reset controller is used by the USB PHY driver and shares the > existing chip controller node with the clocks and one pin controller. > > The Marvell Berlin USB controllers are host only on the BG2Q and are > compatible with USB ChipIdea. We here add a glue to use the available > common functions for this kind of controllers. A USB PHY driver is also > added to control the PHY. > > Antoine Ténart (9): > reset: add the Berlin reset controller driver > ARM: Berlin: select the reset controller > ARM: dts: berlin: add a required reset property in the chip controller > node > usb: phy: add the Berlin USB PHY driver > Documentation: bindings: add doc for the Berlin USB PHY > usb: chipidea: add Berlin USB support > Documentation: bindings: add doc for the Berlin ChipIdea USB driver > ARM: dts: berlin: add BG2Q nodes for USB support > ARM: dts: Berlin: enable USB on the BG2Q DMP > > .../devicetree/bindings/usb/berlin-usbphy.txt | 18 ++ > .../devicetree/bindings/usb/ci-hdrc-berlin.txt | 18 ++ > arch/arm/boot/dts/berlin2.dtsi | 1 + > arch/arm/boot/dts/berlin2cd.dtsi | 1 + > arch/arm/boot/dts/berlin2q-marvell-dmp.dts | 20 ++ > arch/arm/boot/dts/berlin2q.dtsi| 52 + > arch/arm/mach-berlin/Kconfig | 2 + > drivers/reset/Makefile | 1 + > drivers/reset/reset-berlin.c | 113 +++ > drivers/usb/chipidea/Makefile | 1 + > drivers/usb/chipidea/ci_hdrc_berlin.c | 108 ++ > drivers/usb/phy/Kconfig| 9 + > drivers/usb/phy/Makefile | 1 + > drivers/usb/phy/phy-berlin-usb.c | 223 > + > 14 files changed, 568 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/berlin-usbphy.txt > create mode 100644 Documentation/devicetree/bindings/usb/ci-hdrc-berlin.txt > create mode 100644 drivers/reset/reset-berlin.c > create mode 100644 drivers/usb/chipidea/ci_hdrc_berlin.c > create mode 100644 drivers/usb/phy/phy-berlin-usb.c > > -- > 1.9.1 > I am fine with 6/9, 7/9, will queue them if no other objections. -- Best Regards, Peter Chen -- 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: Fix a warning while loading g_mass_storage
Hi Alan, Sorry for my late response, as I was 000, Additionally, I am very grateful for your review. On 06/06/2014 02:08 AM, Alan Stern wrote: On Wed, 4 Jun 2014 wei.y...@windriver.com wrote: From: Yang Wei While loading g_mass_storage module, the following warning is triggered. WARNING: at drivers/usb/gadget/composite.c: usb_composite_setup_continue: Unexpected call Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24) [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74) [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48) [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c) [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) The root cause is that Robert once introduced the following patch to fix disconnect handling of s3c-hsotg. [ commit d18f7116a5ddb8263fe62b05ad63e5ceb5875791 Author: Robert Baldyga Date: Thu Nov 21 13:49:18 2013 +0100 usb: gadget: s3c-hsotg: fix disconnect handling This patch moves s3c_hsotg_disconnect function call from USBSusp interrupt handler to SET_ADDRESS request handler. It's because disconnected state can't be detected directly, because this hardware doesn't support Disconnected interrupt for device mode. For both Suspend and Disconnect events there is one interrupt USBSusp, but calling s3c_hsotg_disconnect from this interrupt handler causes config reset in composite layer, which is not undesirable for Suspended state. For this reason s3c_hsotg_disconnect is called from SET_ADDRESS request handler, which occurs always after disconnection, so we do disconnect immediately before we are connected again. It's probably only way we can do handle disconnection correctly. Signed-off-by: Robert Baldyga Signed-off-by: Kyungmin Park Signed-off-by: Felipe Balbi ] So it also means that s3c_hsotg_disconnect is called from SET_ADDRESS request handler, ultimately reset_config would finally be called, it raises a FSG_STATE_CONFIG_CHANGE exception, and set common->new_fsg to NULL, and then wakes up fsg_main_thread to handle this exception. After handling SET_ADDRESS, subsequently the irq hanler of s3c-hsotg would also invokes composite_setup() function to handle USB_REQ_SET_CONFIGURATION request, set_config would be invoked, it not only raises a FSG_STATE_CONFIG_CHANGE and set common->new_fsg to new_fsg but also makes cdev->delayed_status plus one. If the execution ordering just likes the following scenario, the warning would be triggered. irq handler | |-> s3c_hsotg_disconnect() | |-> common->new_fsg = NULL |-> common->state = FSG_STATE_CONFIG |-> wakes up fsg_main_thread. |->set USB device address. fsg_main_thread | |-> handle_exception | |-> common->state = FSG_STATE_IDLE |-> do_set_interface() irq happens --> irq handler needs to handle USB_REQ_SET_CONFIGURATION request | |-> set_config() | |-> common->new_fsg = new_fsg; |-> common->state = FSG_STATE_CONFIG |-> cdev->delayed_status++ |-> wakes up fsg_main_thread fsg_main_thread | |-> Now the common->state = FSG_STATE_CONFIG and common->new_fsg is not equal to NULL |-> if(common->new_fsg) |-> usb_composite_setup_continue() |-> cdev->delayed_status-- |-> fsg_main_thread still finds the common->state is equal to FSG_STATE_IDLE |-> so it invokes handle_exception again, subsequently the usb_composite_setup_continue |-> is executed again. It would fininally results in the warning. So we also need to define a variable(struct fsg_dev *new) and then save common->new_fsg to it with the common->lock protection. Signed-off-by: Yang Wei --- drivers/usb/gadget/f_mass_storage.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Changes in v2: Only rephrase the commit log to make sense this issue. Wei diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_
[PATCH v3] USB:gadget: Fix a warning while loading g_mass_storage
From: Yang Wei While loading g_mass_storage module, the following warning is triggered. WARNING: at drivers/usb/gadget/composite.c: usb_composite_setup_continue: Unexpected call Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24) [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74) [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48) [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c) [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) The root cause is that the existing code fails to take into account the possibility that common->new_fsg can change while do_set_interface() is running, because the spinlock isn't held at this point. Signed-off-by: Yang Wei Signed-off-by: Alan Stern --- drivers/usb/gadget/f_mass_storage.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Hi Alan, Thanks for your review, there are a few changes in v3: 1) Fix a coding style issue. 2) Refine the commit log Regards Wei diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c index b963939..0cd8f21 100644 --- a/drivers/usb/gadget/f_mass_storage.c +++ b/drivers/usb/gadget/f_mass_storage.c @@ -2342,6 +2342,7 @@ static void handle_exception(struct fsg_common *common) struct fsg_buffhd *bh; enum fsg_state old_state; struct fsg_lun *curlun; + struct fsg_dev *new_fsg; unsigned intexception_req_tag; /* @@ -2421,6 +2422,7 @@ static void handle_exception(struct fsg_common *common) } common->state = FSG_STATE_IDLE; } + new_fsg = common->new_fsg; spin_unlock_irq(&common->lock); /* Carry out any extra actions required for the exception */ @@ -2460,8 +2462,8 @@ static void handle_exception(struct fsg_common *common) break; case FSG_STATE_CONFIG_CHANGE: - do_set_interface(common, common->new_fsg); - if (common->new_fsg) + do_set_interface(common, new_fsg); + if (new_fsg) usb_composite_setup_continue(common->cdev); break; -- 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
