[PATCH] USB: remove incorrect __exit markups

2013-02-24 Thread Dmitry Torokhov
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

2013-02-24 Thread Alan Stern
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

2013-02-24 Thread Alan Stern
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

2013-02-24 Thread kishon

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

2013-02-24 Thread Felipe Balbi
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