[PATCH] USB: remove incorrect __exit markups
Even if bus is not hot-pluggable, the devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes. Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com --- drivers/usb/host/ehci-mxc.c| 2 +- drivers/usb/host/ehci-orion.c | 4 ++-- drivers/usb/host/ehci-sh.c | 4 ++-- drivers/usb/otg/isp1301_omap.c | 4 ++-- drivers/usb/otg/twl4030-usb.c | 4 ++-- drivers/usb/otg/twl6030-usb.c | 4 ++-- drivers/usb/phy/mv_u3d_phy.c | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c index dedb80b..85c99c3 100644 --- a/drivers/usb/host/ehci-mxc.c +++ b/drivers/usb/host/ehci-mxc.c @@ -199,7 +199,7 @@ err_alloc: return ret; } -static int __exit ehci_mxc_drv_remove(struct platform_device *pdev) +static int ehci_mxc_drv_remove(struct platform_device *pdev) { struct mxc_usbh_platform_data *pdata = pdev-dev.platform_data; struct usb_hcd *hcd = platform_get_drvdata(pdev); diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index 914a3ec..38c45fb 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -305,7 +305,7 @@ err1: return err; } -static int __exit ehci_orion_drv_remove(struct platform_device *pdev) +static int ehci_orion_drv_remove(struct platform_device *pdev) { struct usb_hcd *hcd = platform_get_drvdata(pdev); struct clk *clk; @@ -333,7 +333,7 @@ MODULE_DEVICE_TABLE(of, ehci_orion_dt_ids); static struct platform_driver ehci_orion_driver = { .probe = ehci_orion_drv_probe, - .remove = __exit_p(ehci_orion_drv_remove), + .remove = ehci_orion_drv_remove, .shutdown = usb_hcd_platform_shutdown, .driver = { .name = orion-ehci, diff --git a/drivers/usb/host/ehci-sh.c b/drivers/usb/host/ehci-sh.c index 0c90a24..2deef81 100644 --- a/drivers/usb/host/ehci-sh.c +++ b/drivers/usb/host/ehci-sh.c @@ -171,7 +171,7 @@ fail_create_hcd: return ret; } -static int __exit ehci_hcd_sh_remove(struct platform_device *pdev) +static int ehci_hcd_sh_remove(struct platform_device *pdev) { struct ehci_sh_priv *priv = platform_get_drvdata(pdev); struct usb_hcd *hcd = priv-hcd; @@ -197,7 +197,7 @@ static void ehci_hcd_sh_shutdown(struct platform_device *pdev) static struct platform_driver ehci_hcd_sh_driver = { .probe = ehci_hcd_sh_probe, - .remove = __exit_p(ehci_hcd_sh_remove), + .remove = ehci_hcd_sh_remove .shutdown = ehci_hcd_sh_shutdown, .driver = { .name = sh_ehci, diff --git a/drivers/usb/otg/isp1301_omap.c b/drivers/usb/otg/isp1301_omap.c index af9cb11..8b9de95 100644 --- a/drivers/usb/otg/isp1301_omap.c +++ b/drivers/usb/otg/isp1301_omap.c @@ -1212,7 +1212,7 @@ static void isp1301_release(struct device *dev) static struct isp1301 *the_transceiver; -static int __exit isp1301_remove(struct i2c_client *i2c) +static int isp1301_remove(struct i2c_client *i2c) { struct isp1301 *isp; @@ -1634,7 +1634,7 @@ static struct i2c_driver isp1301_driver = { .name = isp1301_omap, }, .probe = isp1301_probe, - .remove = __exit_p(isp1301_remove), + .remove = isp1301_remove, .id_table = isp1301_id, }; diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c index 0a70193..4e04579 100644 --- a/drivers/usb/otg/twl4030-usb.c +++ b/drivers/usb/otg/twl4030-usb.c @@ -657,7 +657,7 @@ static int twl4030_usb_probe(struct platform_device *pdev) return 0; } -static int __exit twl4030_usb_remove(struct platform_device *pdev) +static int twl4030_usb_remove(struct platform_device *pdev) { struct twl4030_usb *twl = platform_get_drvdata(pdev); int val; @@ -701,7 +701,7 @@ MODULE_DEVICE_TABLE(of, twl4030_usb_id_table); static struct platform_driver twl4030_usb_driver = { .probe = twl4030_usb_probe, - .remove = __exit_p(twl4030_usb_remove), + .remove = twl4030_usb_remove, .driver = { .name = twl4030_usb, .owner = THIS_MODULE, diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c index 8cd6cf4..7f3c5b0 100644 --- a/drivers/usb/otg/twl6030-usb.c +++ b/drivers/usb/otg/twl6030-usb.c @@ -393,7 +393,7 @@ static int twl6030_usb_probe(struct platform_device *pdev) return 0; } -static int __exit twl6030_usb_remove(struct platform_device *pdev) +static int twl6030_usb_remove(struct platform_device *pdev) { struct twl6030_usb *twl = platform_get_drvdata(pdev); @@ -420,7 +420,7 @@ MODULE_DEVICE_TABLE(of,
Re: carl9170 A-MPDU transmit problem
On Sat, 23 Feb 2013, Seth Forshee wrote: On Sat, Feb 23, 2013 at 03:07:08PM +0100, Christian Lamparter wrote: usb_submit_urb() is async, so unless the URB data structure is bogus, the device is in the middle of a reset/is removed or OOM it won't return with an -ENUMBER. Since neither of us has probably access to an USB analyzer, the next best thing would be to enable ehci_hcd's debug facilities and check if the stuck frame produced any DataBufferErr, XactErr or something else. There aren't any occurrences of such errors in the usbmon trace. I've found a couple of things here. First, I wasn't sure if I had tested this on anything other than Ivy Bridge machines yet, so I tried it out on an Arrandale box and it worked perfectly. lspci on the Ivy Bridge boxes yields: 00:14.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series Chipset Family USB xHCI Host Controller [8086:1e31] (rev 04) (prog-if 30 [XHCI]) 00:1a.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series Chipset Family USB Enhanced Host Controller #2 [8086:1e2d] (rev 04) (prog-if 20 [EHCI]) 00:1d.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series Chipset Family USB Enhanced Host Controller #1 [8086:1e26] (rev 04) (prog-if 20 [EHCI]) And on the Arrandale box: 00:1a.0 USB controller [0c03]: Intel Corporation 5 Series/3400 Series Chipset USB2 Enhanced Host Controller [8086:3b3c] (rev 06) (prog-if 20 [EHCI]) 00:1d.0 USB controller [0c03]: Intel Corporation 5 Series/3400 Series Chipset USB2 Enhanced Host Controller [8086:3b34] (rev 06) (prog-if 20 [EHCI]) Second, I collected the usbmon trace as suggested by Alan. I don't know enough about USB to really read it, but normally the time between submission and callback for a given urb is short. However, some are much longer, e.g.: 88012fe19500 1519981417 S Bo:3:003:1 -115 126 = 7e00 190f0100 23232303 42b53600 82b11a00 01c02e00 6a00e846 c2ad3e00 ... 88012fe19500 1522200720 C Bo:3:003:1 0 126 I checked the urb addresses for a couple of the stalled frames and in each case found a matching urb in the usbmon trace with a similarly long duration between submission and callback. I've uploaded the full trace to: http://people.canonical.com/~sforshee/carl9170-usbmon.log The usbmon trace indicates that the data gets delivered to the device as it should, but the device doesn't accept it for several seconds. 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: carl9170 A-MPDU transmit problem
On Sun, 24 Feb 2013, Christian Lamparter wrote: The usbmon trace indicates that the data gets delivered to the device as it should, but the device doesn't accept it for several seconds. Looking at the logs, I find myself wondering how the 88012fe19500 urb-complete ninja'd right in between the 880146c8af00 xmit and complete. 88012fe19500 1519981417 S Bo:3:003:1 -115 126 = 7e00 190f0100 23232303 42b53600 82b11a00 01c02e00 6a00e846 c2ad3e00 ... 880146c8af00 1522200650 S Bo:3:003:1 -115 62 = 3e00 01000500 0300 22000$ 88012fe19500 1522200720 C Bo:3:003:1 0 126 880146c8af00 1522200756 C Bo:3:003:1 0 62 In fact this is both normal and required. Packets to any particular endpoint must always be delivered in order. Therefore the URBs have to complete in the same order as they were submitted. From the device side, taking the data shouldn't be a problem. The rx is handled by hardware dma. The data from the host is put into packages of 320 bytes (The carl9170 firmware has about 180 to 190 of these 320 byte packages reserved for this purpose. So at no point there should be any long delay because of lack of resources or whatever). Also, it doesn't look the was any unusually high load on the link. And the hardware can handle sustained wifi traffic up to 160mbit/s (udp peaks at about 180mbit/s) without choking. Alan, Do you know if there a way to monitor whenever individual attempts from the ehci/xhci hcd? Only with a USB bus analyzer. Unfortuantely these things tend to be expensive, especially if you want one that works at SuperSpeed. Or can you think of any other interesting bits that could help to explain why the Arrandale box [...] worked perfectly whereas (all his) Ivy Bridge ones have problems. (Of course, I assume that it is always the same device, the same firmware and the same kernel drivers in all tests, right)? No, not really. Unless one is using USB-2 and the other USB-3 -- the device might have a bug in its USB-3 firmware. What happens if xhci-hcd is unloaded before the test? 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/5] drivers: phy: add generic PHY framework
Hi, On Sunday 24 February 2013 04:14 AM, Rob Landley wrote: On 02/18/2013 11:53:14 PM, Kishon Vijay Abraham I wrote: The PHY framework provides a set of APIs for the PHY drivers to create/destroy a PHY and APIs for the PHY users to obtain a reference to the PHY with or without using phandle. To obtain a reference to the PHY without using phandle, the platform specfic intialization code (say from board file) should have already called phy_bind with the binding information. The binding information consists of phy's device name, phy user device name and an index. The index is used when the same phy user binds to mulitple phys. Given that this has a separately selectable config option, I'm guessing that it's useful all by itself even in the absence of a driver using Not really. It has to be thought of like a bridge that connects the device (can be USB/SATA/..) and the PHY. The PHY driver will register the PHY in this framework and the device controller driver can obtain a reference to this PHY from this framework. this phy? (Or it gives user visibility to the phy buried in an E1000 or SATA drive or some such?) The PHY and the device are generally independent entities and there has to be a way to bind them in-order for the device controller to use the PHY. For e.g., MUSB in OMAP4 uses a PHY which is different from PHY in OMAP3 and it's going to be different from a PHY used in other SoCs. So in-order for MUSB to use the correct PHY, there has to be a framework that maintains the list of PHY and returns the correct PHY when a device controller driver requests for it. So whenever the PHY driver gets probed, it registers itself with this framework and then the MUSB can get the reference to the PHY from this framework. The use of this framework is more prevalent when there are multiple MUSB controllers each using a different PHY in a single SoC. This holds true for other device controllers as well. +1. Introduction + +*PHY* is the abbreviation for physical layer. It is used to connect a device +to the physical medium e.g., the USB controller has a PHY to provide functions +such as serialization, de-serialization, encoding, decoding and is responsible +for obtaining the required data transmission rate. Note that some USB +controller has PHY functionality embedded into it and others use an external +PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet, +SATA etc. I've usually heard the word transciever used to describe these. hmm.. IMO there is a thin line differentiating transceiver and PHY and can be used interchangeably. Since it's been referred as PHY in data sheets and TRMs, I preferred to call it PHY. +The intention of creating this framework is to bring the phy drivers spread +all over the Linux kernel to drivers/phy to increase code re-use and to +increase code maintainability. + +This framework will be of use only to devices that uses external PHY (PHY +functionality is not embedded within the controller). + +2. Creating the PHY + +The PHY driver should create the PHY in order for other peripheral controllers +to make use of it. The PHY framework provides 2 APIs to create the PHY. Given that a PHY is a chip (random example http://ark.intel.com/products/47620/Intel-82579LM-Gigabit-Ethernet-PHY), you seem to be saying that software should manifest a piece of hardware out of thin air through sheer willpower. I'm pretty sure I've misunderstood this phrasing. +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc); +struct phy *devm_phy_create(struct device *dev, struct phy_descriptor *desc); + +The PHY drivers can use one of the above 2 APIs to create the PHY by passing Um, the driver should _bind_ to the phy, maybe? Allocate? Initialize? There is actually difference between allocating/initializing and binding. Binding is to bind the device controller with the PHY. My previous explanation might help to clarify it. +6. Destroying the PHY I've run drivers like that. I try not to, though. +7. Current Status + +Currently only USB in OMAP is made to use this framework. However using the +USB PHY library cannot be completely removed because it is intertwined with +OTG. Once we move OTG out of PHY completely, using the old PHY library can be +completely removed. SATA in OMAP will also more likely use this new framework +and we should have a patch for it soon. Does this paragraph belong in the documentation? (Git commit, sure, but I've seen a lot of stale paragraphs like these hang around a surprisingly long time.) hmm.. there are a few others who raised concern on having this paragraph. I've planned to remove it in my next version. Thanks Kishon -- 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: linux-next and usb gadget composite BUG
Hi, On Sun, Feb 24, 2013 at 12:06:35AM +0100, Robert Jarzmik wrote: Hi Felipe, I tried today's version of linux-next tree (commit id b1714a88bee1ae, Add linux-next specific files for 20130222). My PXA based platform bugs on bootup. I have no console available on my platform, just a JTAG, so this is the info I have : - I'm using a Mitac MIO A701 board (arch/arm/mach-pxa/mioa701.c) - on gadget side, I'm using g_ether + pxa27x_udc So far, I didn't have any composite driver enabled (as the PXA suffer from a severe hardware defect, see comment in header of pxa27x_udc.c). Now, what I gathered from the JTAG is in [1]. You can see that in sysfs_create_file(), the first argument has : dev-kobj-sd == NULL. And of course in fs/sysfs/file.c : BUG_ON(!kobj || !kobj-sd || !attr); Therefore, I hit a BUG. Could you tell me what changed recently that triggers this ? I saw in my .config CONFIG_USB_LIBCOMPOSITE=y which looks a bit suspect to me, but I'm sure you'll tell me what recent change prevents my platform from booting. this is not caused by current pull request. Well, you could argue that commit 70189a63d408d4ea0cddbf0ff0afe6020844e813 (usb: gadget: pxa27x_udc: convert to udc_start/udc_stop) is the culprit, but the fact is that what pxa27x is doing is pretty wrong. The bug is that pxa27x_udc tries to register gadget-dev at udc_start() time and that's only called after gadget driver has already been bound to the controller. Since commit above converted pxa27x to udc_start()/udc_stop() calls, it exposed the issue. Anyway, this patch should fix it (I didn't even compile test it, since pxa27x still depends on its architecture, which needs to be fixed). commit de7dedf26d906e3a2d6bf60385f8a55b9938ae16 Author: Felipe Balbi ba...@ti.com Date: Mon Feb 25 08:49:05 2013 +0200 usb: gadget: pxa27x: fix gadget-dev registration Whenever -udc_start() gets called, gadet driver has already being bound to the udc controller, which means that gadget-dev had to be already initialized and added to driver model. This patch fixes pxa27x mistake. Signed-off-by: Felipe Balbi ba...@ti.com diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c index 2b3b01d..8f39db2 100644 --- a/drivers/usb/gadget/pxa27x_udc.c +++ b/drivers/usb/gadget/pxa27x_udc.c @@ -1821,11 +1821,6 @@ static int pxa27x_udc_start(struct usb_gadget_driver *driver, udc-gadget.dev.driver = driver-driver; dplus_pullup(udc, 1); - retval = device_add(udc-gadget.dev); - if (retval) { - dev_err(udc-dev, device_add error %d\n, retval); - goto add_fail; - } retval = bind(udc-gadget, driver); if (retval) { dev_err(udc-dev, bind to driver %s -- error %d\n, @@ -1852,8 +1847,6 @@ transceiver_fail: if (driver-unbind) driver-unbind(udc-gadget); bind_fail: - device_del(udc-gadget.dev); -add_fail: udc-driver = NULL; udc-gadget.dev.driver = NULL; return retval; @@ -1905,7 +1898,6 @@ static int pxa27x_udc_stop(struct usb_gadget_driver *driver) driver-unbind(udc-gadget); udc-driver = NULL; - device_del(udc-gadget.dev); dev_info(udc-dev, unregistered gadget driver '%s'\n, driver-driver.name); @@ -2511,13 +2503,24 @@ static int __init pxa_udc_probe(struct platform_device *pdev) driver_name, udc-irq, retval); goto err_irq; } + + retval = device_add(udc-gadget.dev); + if (retval) { + dev_err(udc-dev, device_add error %d\n, retval); + goto err_dev_add; + } + retval = usb_add_gadget_udc(pdev-dev, udc-gadget); if (retval) goto err_add_udc; pxa_init_debugfs(udc); + return 0; + err_add_udc: + device_unregister(udc-gadget.dev); +err_dev_add: free_irq(udc-irq, udc); err_irq: iounmap(udc-regs); @@ -2538,6 +2541,7 @@ static int __exit pxa_udc_remove(struct platform_device *_dev) int gpio = udc-mach-gpio_pullup; usb_del_gadget_udc(udc-gadget); + device_del(udc-gadget.dev); usb_gadget_unregister_driver(udc-driver); free_irq(udc-irq, udc); pxa_cleanup_debugfs(udc); -- balbi signature.asc Description: Digital signature