Re: [PATCH 3/4] usb: Create link files between child device and usb port device.

[Lan Tianyu]

On 2012年10月25日 12:05, Greg KH wrote:
> On Thu, Oct 25, 2012 at 11:19:05AM +0800, Lan Tianyu wrote:
>> On 2012/10/25 5:49, Greg KH wrote:
>>> On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:
 于 2012/10/24 18:55, Sergei Shtylyov 写道:
>> +/* Create link files between child device and usb port device. */
>> +if (udev->parent) {
>> +int no_warn;
>
>I think 'ret' or 'result' is a better name...
>
>> +struct usb_port *port_dev =
>> +hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
>> +
>> +no_warn = sysfs_create_link(&udev->dev.kobj,
>> +&port_dev->dev.kobj, "port");
>> +no_warn = sysfs_create_link(&port_dev->dev.kobj,
>> +&udev->dev.kobj, "child");
>
> I guess you are not supposed to ignore the result if the function 
> requires
> it not to be ignored.
>
 Hi Sergei:
Great thanks for your review. From my opinion, failure to create link 
 will
 not affect usb device function and so the return value can be ignored, 
 Perharps
 producing
 some warning will be better. Do you have some suggestion? :)
>>>
>>> Properly handle the error, don't ignore it.
>>>
>> HI Greg:
>>  Ok, I get it. How about following?
>> +   /* Create link files between child device and usb port device. */
>> +   if (udev->parent) {
>> +   struct usb_port *port_dev =
>> +   hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
>> +
>> +   err = sysfs_create_link(&udev->dev.kobj,
>> +   &port_dev->dev.kobj, "port");
>> +   if (err)
>> +   goto fail;
>> +
>> +   err = sysfs_create_link(&port_dev->dev.kobj,
>> +   &udev->dev.kobj, "child");
>> +   if (err) {
>> +   sysfs_remove_link(&udev->dev.kobj, "port");
>> +   goto fail;
>> +   }
>> +   }
>> +
> 
> It's a good start, did you test it out?
> 
Yeah. I basicly check whether these link files are created under related
sysfs directory or not. It works. Did you find some potential problems?
Or give me more test ways. I'd like to do it. Thanks.

> And why are you calling the devices in a port a "child"?  It's a device.
> Otherwise everything is a child, and that doesn't make any sense, right?
Right. Renaming it with "device" is ok for you?
> 
> greg k-h
> 


-- 
Best regards
Tianyu Lan
--
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

USB isochronous frame lost

[Stefan May]

Dear USB-Developers,

I am having a strange Problem on Ubuntu Linux (11.10 and 12.04) using a 
thermal camera (Cypress FX2 chip) in isochronous mode. The transmission 
of data breaks with the error code -EPROTO. The curious thing is, that 
it runs very well on the same distribution, when it is a VMWare guest 
system. That means:

- Native Ubuntu Linux: Frequently USB isochronous frame lost (-71)
- Ubuntu Linux as VMWare guest on a Ubuntu Host!: No errors!

I changed cables, switched USB ports, added quirks ... no effect.

Below, I've attached traces from usbmon. Could someone give me a hint 
what is going wrong there? Do you know with parameters have to be 
respected on the firmware side? Below, I've attached lsusb output too.


Best regards,

   Stefan May




# Native Ubuntu 

f6966b40 1269492530 S Ii:1:002:5 -115:16 16 <
f6966b40 1269493391 C Ii:1:002:5 -2:16 0
f6966b40 1269493824 S Ii:1:002:5 -115:16 16 <
eba91300 1269495464 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 01000101 
48e80100     

eba91300 1269495626 C Co:1:002:0 0 26 >
eb30e0c0 1269495653 S Ci:1:002:0 s a1 82 0100 0001 001a 26 <
eb30e0c0 1269495747 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 

eb30e0c0 1269495790 S Ci:1:002:0 s a1 83 0100 0001 001a 26 <
eb30e0c0 1269495871 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
eb30e0c0 1269495910 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 01000101 
48e80100     

eb30e0c0 1269496033 C Co:1:002:0 0 26 >
eb30e0c0 1269496088 S Ci:1:002:0 s a1 81 0100 0001 001a 26 <
eb30e0c0 1269496259 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
eb30e0c0 1269496372 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 01000101 
48e80100     

eb30e0c0 1269496511 C Co:1:002:0 0 26 >
eb30e0c0 1269496566 S Ci:1:002:0 s a1 82 0100 0001 001a 26 <
eb30e0c0 1269496763 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 

eb82f180 1269496832 S Ci:1:002:0 s a1 83 0100 0001 001a 26 <
eb82f180 1269497011 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
eb82f180 1269497075 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 01000101 
48e80100     

eb82f180 1269497262 C Co:1:002:0 0 26 >
eb82f180 1269497322 S Ci:1:002:0 s a1 81 0100 0001 001a 26 <
eb82f180 1269497512 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
eb82f3c0 1269497630 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 0101 
48e80100   0060 030a 

eb82f3c0 1269497745 C Co:1:002:0 0 26 >
eb82f3c0 1269497793 S Ci:1:002:0 s a1 82 0100 0001 001a 26 <
eb82f3c0 1269497871 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 

eb82f3c0 1269497912 S Ci:1:002:0 s a1 83 0100 0001 001a 26 <
eb82f3c0 1269498018 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
eb82f3c0 1269498073 S Co:1:002:0 s 21 01 0100 0001 001a 26 = 0101 
48e80100   0060 030a 

eb82f3c0 1269498259 C Co:1:002:0 0 26 >
eb82f3c0 1269498320 S Ci:1:002:0 s a1 81 0100 0001 001a 26 <
eb82f3c0 1269498514 C Ci:1:002:0 0 26 = 0101 48e80100  
 0060 090a 
e93f7cc0 1269815050 S Co:1:002:0 s 21 01 0200 0001 001a 26 = 0101 
48e80100   0060 030a 

e93f7cc0 1269815209 C Co:1:002:0 0 26 >
e93f7cc0 1269816053 S Co:1:002:0 s 01 0b 0001 0001  0
e93f7cc0 1269816451 C Co:1:002:0 0 0
e416e800 1269816629 S Zi:1:002:2 -115:1:0 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e4169800 1269816702 S Zi:1:002:2 -115:1:0 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e416c800 1269816707 S Zi:1:002:2 -115:1:0 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e4168c00 1269816710 S Zi:1:002:2 -115:1:0 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e416e400 1269816713 S Zi:1:002:2 -115:1:0 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e416e800 1269830073 C Zi:1:002:2 0:1:2360:16 32 0:0:2306 -71:3072:1126 
0:6144:2058 -71:9216:1150 0:12288:2082 98304 = 0280733e 993ea73e 
b13ec03e d53eef3e 003f0d3f 1f3f343f 3f3f473f 533f4a3f
e416e800 1269830082 S Zi:1:002:2 -115:1:2360 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e4169800 1269834074 C Zi:1:002:2 0:1:2392:19 32 0:0:2086 -71:3072:1130 
0:6144:2062 -71:9216:1154 0:12288:2088 98304 = 6d3f663f 653f6b3f 
6c3f653f 633f633f 653f683f 683f6a3f 6e3f6e3f 653f623f
e4169800 1269834087 S Zi:1:002:2 -115:1:2392 32 -18:0:3072 -18:3072:3072 
-18:6144:3072 -18:9216:3072 -18:12288:3072 98304 <
e416c800 1269838086 C Zi:1:002:2 0:1:2424:32 32 -71:0:0 -7

Re: [PATCH v2 1/2] USB: dwc3-exynos: Add support for device tree

[Vivek Gautam]

Hi,

On Tue, Oct 16, 2012 at 3:38 PM, Felipe Balbi  wrote:
> Hi,
>
> On Tue, Oct 16, 2012 at 03:36:43PM +0530, kishon wrote:
>> Hi,
>>
>> On Tuesday 16 October 2012 03:23 PM, Felipe Balbi wrote:
>> >On Tue, Oct 16, 2012 at 02:15:56PM +0530, Vivek Gautam wrote:
>> >>This patch adds support to parse probe data for
>> >>dwc3-exynos driver using device tree.
>> >>
>> >>Signed-off-by: Vivek Gautam 
>> >>---
>> >>  drivers/usb/dwc3/dwc3-exynos.c |   20 
>> >>  1 files changed, 20 insertions(+), 0 deletions(-)
>> >>
>> >>diff --git a/drivers/usb/dwc3/dwc3-exynos.c 
>> >>b/drivers/usb/dwc3/dwc3-exynos.c
>> >>index ca65978..d11ef49 100644
>> >>--- a/drivers/usb/dwc3/dwc3-exynos.c
>> >>+++ b/drivers/usb/dwc3/dwc3-exynos.c
>> >>@@ -21,6 +21,7 @@
>> >>  #include 
>> >>  #include 
>> >>  #include 
>> >>+#include 
>> >>
>> >>  #include "core.h"
>> >>
>> >>@@ -87,6 +88,8 @@ err1:
>> >>return ret;
>> >>  }
>> >>
>> >>+static u64 dwc3_exynos_dma_mask = DMA_BIT_MASK(32);
>> >>+
>> >>  static int __devinit dwc3_exynos_probe(struct platform_device *pdev)
>> >>  {
>> >>struct dwc3_exynos_data *pdata = pdev->dev.platform_data;
>> >>@@ -103,6 +106,14 @@ static int __devinit dwc3_exynos_probe(struct 
>> >>platform_device *pdev)
>> >>goto err0;
>> >>}
>> >>
>> >>+   /*
>> >>+* Right now device-tree probed devices don't get dma_mask set.
>> >>+* Since shared usb code relies on it, set it here for now.
>> >>+* Once we move to full device tree support this will vanish off.
>> >>+*/
>> >>+   if (!pdev->dev.dma_mask)
>> >>+   pdev->dev.dma_mask = &dwc3_exynos_dma_mask;
>> >
>> >says who ?
>> >
>> >$ git grep -e dma_mask drivers/of/
>> >drivers/of/platform.c:  dev->dev.dma_mask = &dev->archdata.dma_mask;
>> >drivers/of/platform.c:  dev->archdata.dma_mask = 0xUL;
>> >drivers/of/platform.c:  dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>> >drivers/of/platform.c:  dev->dev.coherent_dma_mask = ~0;
>> >drivers/of/platform.c:  dev->dma_mask = ~0;
>> >
>> >-ECONFUSED
>>
>> dma_mask is set under some ifdef except for "dev->dma_mask = ~0;".
>> However I agree with you for coherent_dma_mask case.
>
> indeed. Should we try to patch that instead ?
>
> Rob, should we set dma_mask at the driver or do you have a nicer way to
> handle it ??
>
Can i have suggestions here please ? :)

> --
> balbi
>
> ___
> devicetree-discuss mailing list
> devicetree-disc...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

Thanks & Regards
Vivek
--
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 00/11] usbnet: usb_control_msg cleanup

[Ming Lei]

On Wed, Oct 24, 2012 at 1:05 AM, David Miller  wrote:
>
> Please repost it to netdev.

Sent out v2 which is rebased  on 3.7.0-rc2-next-20121025,  thanks.

Thanks,
--
Ming Lei
--
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 11/11] usbnet: smsc95xx: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/smsc95xx.c |  115 +++-
 1 file changed, 27 insertions(+), 88 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 7479a57..1730f75 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -65,11 +65,6 @@ struct smsc95xx_priv {
spinlock_t mac_cr_lock;
 };
 
-struct usb_context {
-   struct usb_ctrlrequest req;
-   struct usbnet *dev;
-};
-
 static bool turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -77,25 +72,20 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx 
transaction");
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
  u32 *data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_READ_REGISTER,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+   ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to read register index 0x%08x\n", 
index);
 
-   le32_to_cpus(buf);
-   *data = *buf;
-   kfree(buf);
+   le32_to_cpus(&buf);
+   *data = buf;
 
return ret;
 }
@@ -103,27 +93,22 @@ static int __must_check smsc95xx_read_reg(struct usbnet 
*dev, u32 index,
 static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
   u32 data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   *buf = data;
-   cpu_to_le32s(buf);
+   buf = data;
+   cpu_to_le32s(&buf);
 
-   ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_WRITE_REGISTER,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
 
+   ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+  USB_DIR_OUT | USB_TYPE_VENDOR |
+  USB_RECIP_DEVICE,
+  0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net, "Failed to write register index 
0x%08x\n", index);
 
-   kfree(buf);
-
return ret;
 }
 
@@ -132,11 +117,8 @@ static int smsc95xx_set_feature(struct usbnet *dev, u32 
feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
 
-   cpu_to_le32s(&feature);
-
-   return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   return usbnet_write_cmd(dev, USB_REQ_SET_FEATURE,
+   USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -144,11 +126,8 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 
feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
 
-   cpu_to_le32s(&feature);
-
-   return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   return usbnet_write_cmd(dev, USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 /* Loop until the read is completed with timeout
@@ -350,60 +329,20 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 
offset, u32 length,
return 0;
 }
 
-static void smsc95xx_async_cmd_callback(struct urb *urb)
-{
-   struct usb_context *usb_context = urb->context;
-   struct usbnet *dev = usb_context->dev;
-   int status = urb->status;
-
-   check_warn(status, "async callback failed with %d\n", status);
-
-   kfree(usb_context);
-   usb_free_urb(urb);
-}
-
 static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
 u32 *data)
 {
-   struct usb_context *usb_context;
-   int status;
-   struct urb *urb;
const u16 size = 4;
+   int ret;
 
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_warn(dev->net, "Error allocating URB\n");
-   return -ENOMEM;
-   }
-
-   usb_context = kmal

[PATCH v2 09/11] usbnet: sierra_net: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/sierra_net.c |   45 --
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index c27d277..eb5c7a8 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -311,10 +311,9 @@ static int sierra_net_send_cmd(struct usbnet *dev,
struct sierra_net_data *priv = sierra_net_get_private(dev);
int  status;
 
-   status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_CDC_SEND_ENCAPSULATED_COMMAND,
-   USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE, 0,
-   priv->ifnum, cmd, cmdlen, USB_CTRL_SET_TIMEOUT);
+   status = usbnet_write_cmd(dev, USB_CDC_SEND_ENCAPSULATED_COMMAND,
+ 
USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
+ 0, priv->ifnum, cmd, cmdlen);
 
if (status != cmdlen && status != -ENODEV)
netdev_err(dev->net, "Submit %s failed %d\n", cmd_name, status);
@@ -632,32 +631,22 @@ static int sierra_net_change_mtu(struct net_device *net, 
int new_mtu)
 static int sierra_net_get_fw_attr(struct usbnet *dev, u16 *datap)
 {
int result = 0;
-   u16 *attrdata;
-
-   attrdata = kmalloc(sizeof(*attrdata), GFP_KERNEL);
-   if (!attrdata)
-   return -ENOMEM;
-
-   result = usb_control_msg(
-   dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   /* _u8 vendor specific request */
-   SWI_USB_REQUEST_GET_FW_ATTR,
-   USB_DIR_IN | USB_TYPE_VENDOR,   /* __u8 request type */
-   0x, /* __u16 value not used */
-   0x, /* __u16 index  not used */
-   attrdata,   /* char *data */
-   sizeof(*attrdata),  /* __u16 size */
-   USB_CTRL_SET_TIMEOUT);  /* int timeout */
-
-   if (result < 0) {
-   kfree(attrdata);
+   u16 attrdata;
+
+   result = usbnet_read_cmd(dev,
+   /* _u8 vendor specific request */
+   SWI_USB_REQUEST_GET_FW_ATTR,
+   USB_DIR_IN | USB_TYPE_VENDOR,   /* __u8 request 
type */
+   0x, /* __u16 value not used */
+   0x, /* __u16 index  not used */
+   &attrdata,  /* char *data */
+   sizeof(attrdata)/* __u16 size */
+   );
+
+   if (result < 0)
return -EIO;
-   }
-
-   *datap = le16_to_cpu(*attrdata);
 
-   kfree(attrdata);
+   *datap = le16_to_cpu(attrdata);
return result;
 }
 
-- 
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 10/11] usbnet: smsc75xx: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/smsc75xx.c |   39 ++-
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index b77ae76..1baa53a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -85,26 +85,21 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx 
transaction");
 static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
  u32 *data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_READ_REGISTER,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+   ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+ USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to read reg index 0x%08x: %d", index, ret);
 
-   le32_to_cpus(buf);
-   *data = *buf;
-   kfree(buf);
+   le32_to_cpus(&buf);
+   *data = buf;
 
return ret;
 }
@@ -112,28 +107,22 @@ static int __must_check smsc75xx_read_reg(struct usbnet 
*dev, u32 index,
 static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
   u32 data)
 {
-   u32 *buf = kmalloc(4, GFP_KERNEL);
+   u32 buf;
int ret;
 
BUG_ON(!dev);
 
-   if (!buf)
-   return -ENOMEM;
-
-   *buf = data;
-   cpu_to_le32s(buf);
-
-   ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   USB_VENDOR_REQUEST_WRITE_REGISTER,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+   buf = data;
+   cpu_to_le32s(&buf);
 
+   ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+  USB_DIR_OUT | USB_TYPE_VENDOR |
+  USB_RECIP_DEVICE,
+  0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to write reg index 0x%08x: %d", index, ret);
 
-   kfree(buf);
-
return ret;
 }
 
-- 
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 08/11] usbnet: plusb: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/plusb.c |   11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 4584b9a..0fcc8e6 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -71,13 +71,10 @@
 static inline int
 pl_vendor_req(struct usbnet *dev, u8 req, u8 val, u8 index)
 {
-   return usb_control_msg(dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   val, index,
-   NULL, 0,
-   USB_CTRL_GET_TIMEOUT);
+   return usbnet_read_cmd(dev, req,
+   USB_DIR_IN | USB_TYPE_VENDOR |
+   USB_RECIP_DEVICE,
+   val, index, NULL, 0);
 }
 
 static inline int
-- 
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 07/11] usbnet: net1080: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/net1080.c |  110 +
 1 file changed, 30 insertions(+), 80 deletions(-)

diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index c062a3e..93e0716 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -109,13 +109,11 @@ struct nc_trailer {
 static int
 nc_vendor_read(struct usbnet *dev, u8 req, u8 regnum, u16 *retval_ptr)
 {
-   int status = usb_control_msg(dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   0, regnum,
-   retval_ptr, sizeof *retval_ptr,
-   USB_CTRL_GET_TIMEOUT);
+   int status = usbnet_read_cmd(dev, req,
+USB_DIR_IN | USB_TYPE_VENDOR |
+USB_RECIP_DEVICE,
+0, regnum, retval_ptr,
+sizeof *retval_ptr);
if (status > 0)
status = 0;
if (!status)
@@ -133,13 +131,9 @@ nc_register_read(struct usbnet *dev, u8 regnum, u16 
*retval_ptr)
 static void
 nc_vendor_write(struct usbnet *dev, u8 req, u8 regnum, u16 value)
 {
-   usb_control_msg(dev->udev,
-   usb_sndctrlpipe(dev->udev, 0),
-   req,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value, regnum,
-   NULL, 0,// data is in setup packet
-   USB_CTRL_SET_TIMEOUT);
+   usbnet_write_cmd(dev, req,
+USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+value, regnum, NULL, 0);
 }
 
 static inline void
@@ -288,37 +282,34 @@ static inline void nc_dump_ttl(struct usbnet *dev, u16 
ttl)
 static int net1080_reset(struct usbnet *dev)
 {
u16 usbctl, status, ttl;
-   u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+   u16 vp;
int retval;
 
-   if (!vp)
-   return -ENOMEM;
-
// nc_dump_registers(dev);
 
-   if ((retval = nc_register_read(dev, REG_STATUS, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_STATUS, &vp)) < 0) {
netdev_dbg(dev->net, "can't read %s-%s status: %d\n",
   dev->udev->bus->bus_name, dev->udev->devpath, 
retval);
goto done;
}
-   status = *vp;
+   status = vp;
nc_dump_status(dev, status);
 
-   if ((retval = nc_register_read(dev, REG_USBCTL, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_USBCTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read USBCTL, %d\n", retval);
goto done;
}
-   usbctl = *vp;
+   usbctl = vp;
nc_dump_usbctl(dev, usbctl);
 
nc_register_write(dev, REG_USBCTL,
USBCTL_FLUSH_THIS | USBCTL_FLUSH_OTHER);
 
-   if ((retval = nc_register_read(dev, REG_TTL, vp)) < 0) {
+   if ((retval = nc_register_read(dev, REG_TTL, &vp)) < 0) {
netdev_dbg(dev->net, "can't read TTL, %d\n", retval);
goto done;
}
-   ttl = *vp;
+   ttl = vp;
// nc_dump_ttl(dev, ttl);
 
nc_register_write(dev, REG_TTL,
@@ -331,7 +322,6 @@ static int net1080_reset(struct usbnet *dev)
retval = 0;
 
 done:
-   kfree(vp);
return retval;
 }
 
@@ -339,13 +329,10 @@ static int net1080_check_connect(struct usbnet *dev)
 {
int retval;
u16 status;
-   u16 *vp = kmalloc(sizeof (u16), GFP_KERNEL);
+   u16 vp;
 
-   if (!vp)
-   return -ENOMEM;
-   retval = nc_register_read(dev, REG_STATUS, vp);
-   status = *vp;
-   kfree(vp);
+   retval = nc_register_read(dev, REG_STATUS, &vp);
+   status = vp;
if (retval != 0) {
netdev_dbg(dev->net, "net1080_check_conn read - %d\n", retval);
return retval;
@@ -355,59 +342,22 @@ static int net1080_check_connect(struct usbnet *dev)
return 0;
 }
 
-static void nc_flush_complete(struct urb *urb)
-{
-   kfree(urb->context);
-   usb_free_urb(urb);
-}
-
 static void nc_ensure_sync(struct usbnet *dev)
 {
-   dev->frame_errors++;
-   if (dev->frame_errors > 5) {
-   struct urb  *urb;
-   struct usb_ctrlrequest  *req;
-   int status;
-
-   /* Send a flush */
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb)
-   return;
-
-   req = kmalloc(sizeof *req, GFP_ATOMIC);
-   if (!req) {
-   usb_free_urb(urb);
-   return;
-   }
+   if (++dev->frame_errors <= 5)
+   

[PATCH v2 06/11] usbnet: mcs7830: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/mcs7830.c |   85 -
 1 file changed, 6 insertions(+), 79 deletions(-)

diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index cc7e720..3f3f566 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -124,93 +124,20 @@ static const char driver_name[] = "MOSCHIP usb-ethernet 
driver";
 
 static int mcs7830_get_reg(struct usbnet *dev, u16 index, u16 size, void *data)
 {
-   struct usb_device *xdev = dev->udev;
-   int ret;
-   void *buffer;
-
-   buffer = kmalloc(size, GFP_NOIO);
-   if (buffer == NULL)
-   return -ENOMEM;
-
-   ret = usb_control_msg(xdev, usb_rcvctrlpipe(xdev, 0), MCS7830_RD_BREQ,
- MCS7830_RD_BMREQ, 0x, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
-   memcpy(data, buffer, size);
-   kfree(buffer);
-
-   return ret;
+   return usbnet_read_cmd(dev, MCS7830_RD_BREQ, MCS7830_RD_BMREQ,
+   0x, index, data, size);
 }
 
 static int mcs7830_set_reg(struct usbnet *dev, u16 index, u16 size, const void 
*data)
 {
-   struct usb_device *xdev = dev->udev;
-   int ret;
-   void *buffer;
-
-   buffer = kmemdup(data, size, GFP_NOIO);
-   if (buffer == NULL)
-   return -ENOMEM;
-
-   ret = usb_control_msg(xdev, usb_sndctrlpipe(xdev, 0), MCS7830_WR_BREQ,
- MCS7830_WR_BMREQ, 0x, index, buffer,
- size, MCS7830_CTRL_TIMEOUT);
-   kfree(buffer);
-   return ret;
-}
-
-static void mcs7830_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "%s() failed with %d\n",
-  __func__, status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+   0x, index, data, size);
 }
 
 static void mcs7830_set_reg_async(struct usbnet *dev, u16 index, u16 size, 
void *data)
 {
-   struct usb_ctrlrequest *req;
-   int ret;
-   struct urb *urb;
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   dev_dbg(&dev->udev->dev,
-   "Error allocating URB in write_cmd_async!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof *req, GFP_ATOMIC);
-   if (!req) {
-   dev_err(&dev->udev->dev,
-   "Failed to allocate memory for control request\n");
-   goto out;
-   }
-   req->bRequestType = MCS7830_WR_BMREQ;
-   req->bRequest = MCS7830_WR_BREQ;
-   req->wValue = 0;
-   req->wIndex = cpu_to_le16(index);
-   req->wLength = cpu_to_le16(size);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, size,
-mcs7830_async_cmd_callback, req);
-
-   ret = usb_submit_urb(urb, GFP_ATOMIC);
-   if (ret < 0) {
-   dev_err(&dev->udev->dev,
-   "Error submitting the control message: ret=%d\n", ret);
-   goto out;
-   }
-   return;
-out:
-   kfree(req);
-   usb_free_urb(urb);
+   usbnet_write_cmd_async(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+   0x, index, data, size);
 }
 
 static int mcs7830_hif_get_mac_address(struct usbnet *dev, unsigned char *addr)
-- 
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 05/11] usbnet: int51x1: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/int51x1.c |   52 +++--
 1 file changed, 3 insertions(+), 49 deletions(-)

diff --git a/drivers/net/usb/int51x1.c b/drivers/net/usb/int51x1.c
index 8de6417..ace9e74 100644
--- a/drivers/net/usb/int51x1.c
+++ b/drivers/net/usb/int51x1.c
@@ -116,23 +116,8 @@ static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev,
return skb;
 }
 
-static void int51x1_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   dev_warn(&urb->dev->dev, "async callback failed with %d\n", 
status);
-
-   kfree(req);
-   usb_free_urb(urb);
-}
-
 static void int51x1_set_multicast(struct net_device *netdev)
 {
-   struct usb_ctrlrequest *req;
-   int status;
-   struct urb *urb;
struct usbnet *dev = netdev_priv(netdev);
u16 filter = PACKET_TYPE_DIRECTED | PACKET_TYPE_BROADCAST;
 
@@ -149,40 +134,9 @@ static void int51x1_set_multicast(struct net_device 
*netdev)
netdev_dbg(dev->net, "receive own packets only\n");
}
 
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_warn(dev->net, "Error allocating URB\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(*req), GFP_ATOMIC);
-   if (!req) {
-   netdev_warn(dev->net, "Error allocating control msg\n");
-   goto out;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
-   req->bRequest = SET_ETHERNET_PACKET_FILTER;
-   req->wValue = cpu_to_le16(filter);
-   req->wIndex = 0;
-   req->wLength = 0;
-
-   usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
-   (void *)req, NULL, 0,
-   int51x1_async_cmd_callback,
-   (void *)req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_warn(dev->net, "Error submitting control msg, sts=%d\n",
-   status);
-   goto out1;
-   }
-   return;
-out1:
-   kfree(req);
-out:
-   usb_free_urb(urb);
+   usbnet_write_cmd_async(dev, SET_ETHERNET_PACKET_FILTER,
+  USB_DIR_OUT | USB_TYPE_CLASS | 
USB_RECIP_INTERFACE,
+  filter, 0, NULL, 0);
 }
 
 static const struct net_device_ops int51x1_netdev_ops = {
-- 
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 04/11] usbnet: dm9601: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/dm9601.c |  107 +++---
 1 file changed, 15 insertions(+), 92 deletions(-)

diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index e0433ce..3f554c1 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -56,27 +56,12 @@
 
 static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-   void *buf;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "dm_read() reg=0x%02x length=%d\n", reg, length);
-
-   buf = kmalloc(length, GFP_KERNEL);
-   if (!buf)
-   goto out;
-
-   err = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- DM_READ_REGS,
- USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-   if (err == length)
-   memcpy(data, buf, length);
-   else if (err >= 0)
+   int err;
+   err = usbnet_read_cmd(dev, DM_READ_REGS,
+  USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  0, reg, data, length);
+   if(err != length && err >= 0)
err = -EINVAL;
-   kfree(buf);
-
- out:
return err;
 }
 
@@ -87,91 +72,29 @@ static int dm_read_reg(struct usbnet *dev, u8 reg, u8 
*value)
 
 static int dm_write(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-   void *buf = NULL;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "dm_write() reg=0x%02x, length=%d\n", reg, length);
+   int err;
+   err = usbnet_write_cmd(dev, DM_WRITE_REGS,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   0, reg, data, length);
 
-   if (data) {
-   buf = kmemdup(data, length, GFP_KERNEL);
-   if (!buf)
-   goto out;
-   }
-
-   err = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- DM_WRITE_REGS,
- USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
- 0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-   kfree(buf);
if (err >= 0 && err < length)
err = -EINVAL;
- out:
return err;
 }
 
 static int dm_write_reg(struct usbnet *dev, u8 reg, u8 value)
 {
-   netdev_dbg(dev->net, "dm_write_reg() reg=0x%02x, value=0x%02x\n",
-  reg, value);
-   return usb_control_msg(dev->udev,
-  usb_sndctrlpipe(dev->udev, 0),
-  DM_WRITE_REG,
-  USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
-  value, reg, NULL, 0, USB_CTRL_SET_TIMEOUT);
-}
-
-static void dm_write_async_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "dm_write_async_callback() failed with %d\n",
-  status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, DM_WRITE_REGS,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   value, reg, NULL, 0);
 }
 
 static void dm_write_async_helper(struct usbnet *dev, u8 reg, u8 value,
  u16 length, void *data)
 {
-   struct usb_ctrlrequest *req;
-   struct urb *urb;
-   int status;
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_err(dev->net, "Error allocating URB in 
dm_write_async_helper!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-   if (!req) {
-   netdev_err(dev->net, "Failed to allocate memory for control 
request\n");
-   usb_free_urb(urb);
-   return;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-   req->bRequest = length ? DM_WRITE_REGS : DM_WRITE_REG;
-   req->wValue = cpu_to_le16(value);
-   req->wIndex = cpu_to_le16(reg);
-   req->wLength = cpu_to_le16(length);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, length,
-dm_write_async_callback, req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_err(dev->net, "Error submitting the control message: 
status=%d\n",
-  status);
-   kfree(req);
-   usb_free_urb(urb);
-   }
+   usbnet_write_cmd_async(dev, DM_WRITE_REGS,
+  USB_DIR_O

[PATCH v2 03/11] usbnet: cdc-ncm: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/cdc_ncm.c |   85 +
 1 file changed, 31 insertions(+), 54 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 0ed03b1..d0ea419 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -82,16 +82,15 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
u16 ntb_fmt_supported;
u32 min_dgram_size;
u32 min_hdr_size;
+   struct usbnet *dev = netdev_priv(ctx->netdev);
 
iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
-   err = usb_control_msg(ctx->udev,
-   usb_rcvctrlpipe(ctx->udev, 0),
-   USB_CDC_GET_NTB_PARAMETERS,
-   USB_TYPE_CLASS | USB_DIR_IN
-| USB_RECIP_INTERFACE,
-   0, iface_no, &ctx->ncm_parm,
-   sizeof(ctx->ncm_parm), 1);
+   err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
+ USB_TYPE_CLASS | USB_DIR_IN
+ |USB_RECIP_INTERFACE,
+ 0, iface_no, &ctx->ncm_parm,
+ sizeof(ctx->ncm_parm));
if (err < 0) {
pr_debug("failed GET_NTB_PARAMETERS\n");
return 1;
@@ -147,22 +146,12 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
/* inform device about NTB input size changes */
if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
-   __le32 *dwNtbInMaxSize;
+   __le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
 
-   dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize), GFP_KERNEL);
-   if (!dwNtbInMaxSize) {
-   err = -ENOMEM;
-   goto size_err;
-   }
-   *dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-   err = usb_control_msg(ctx->udev,
- usb_sndctrlpipe(ctx->udev, 0),
- USB_CDC_SET_NTB_INPUT_SIZE,
- USB_TYPE_CLASS | USB_DIR_OUT
- | USB_RECIP_INTERFACE,
- 0, iface_no, dwNtbInMaxSize, 4, 1000);
-   kfree(dwNtbInMaxSize);
-size_err:
+   err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE,
+  USB_TYPE_CLASS | USB_DIR_OUT
+  | USB_RECIP_INTERFACE,
+  0, iface_no, &dwNtbInMaxSize, 4);
if (err < 0)
pr_debug("Setting NTB Input Size failed\n");
}
@@ -218,23 +207,22 @@ size_err:
 
/* set CRC Mode */
if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
-   err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
-   USB_CDC_SET_CRC_MODE,
-   USB_TYPE_CLASS | USB_DIR_OUT
-| USB_RECIP_INTERFACE,
-   USB_CDC_NCM_CRC_NOT_APPENDED,
-   iface_no, NULL, 0, 1000);
+   err = usbnet_write_cmd(dev, USB_CDC_SET_CRC_MODE,
+  USB_TYPE_CLASS | USB_DIR_OUT
+  | USB_RECIP_INTERFACE,
+  USB_CDC_NCM_CRC_NOT_APPENDED,
+  iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting CRC mode off failed\n");
}
 
/* set NTB format, if both formats are supported */
if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
-   err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
-   USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
-| USB_DIR_OUT | USB_RECIP_INTERFACE,
-   USB_CDC_NCM_NTB16_FORMAT,
-   iface_no, NULL, 0, 1000);
+   err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_FORMAT,
+  USB_TYPE_CLASS | USB_DIR_OUT
+  | USB_RECIP_INTERFACE,
+  USB_CDC_NCM_NTB16_FORMAT,
+  iface_no, NULL, 0);
if (err < 0)
pr_debug("Setting NTB format to 16-bit failed\n");
}
@@ -243,7 +231,7 @@ size_err:
 
/* set Max Datagram Size (MTU) */
if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
-   __le16 *max_datagram_size;
+   __le16 max_datagram_size;
u16 eth_max_sz;
if (ctx->ether_desc != NULL)
eth_max_sz = 
le16_to_cpu(ctx->ether_desc->

[PATCH v2 02/11] usbnet: asix: apply introduced usb command APIs

[Ming Lei]

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/asix_common.c |  117 +
 1 file changed, 13 insertions(+), 104 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 774d9ce..50d1673 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -25,121 +25,30 @@
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data)
 {
-   void *buf;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   buf = kmalloc(size, GFP_KERNEL);
-   if (!buf)
-   goto out;
-
-   err = usb_control_msg(
-   dev->udev,
-   usb_rcvctrlpipe(dev->udev, 0),
-   cmd,
-   USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value,
-   index,
-   buf,
-   size,
-   USB_CTRL_GET_TIMEOUT);
-   if (err == size)
-   memcpy(data, buf, size);
-   else if (err >= 0)
-   err = -EINVAL;
-   kfree(buf);
+   int ret;
+   ret = usbnet_read_cmd(dev, cmd,
+  USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  value, index, data, size);
 
-out:
-   return err;
+   if (ret != size && ret >= 0)
+   return -EINVAL;
+   return ret;
 }
 
 int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
   u16 size, void *data)
 {
-   void *buf = NULL;
-   int err = -ENOMEM;
-
-   netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   if (data) {
-   buf = kmemdup(data, size, GFP_KERNEL);
-   if (!buf)
-   goto out;
-   }
-
-   err = usb_control_msg(
-   dev->udev,
-   usb_sndctrlpipe(dev->udev, 0),
-   cmd,
-   USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-   value,
-   index,
-   buf,
-   size,
-   USB_CTRL_SET_TIMEOUT);
-   kfree(buf);
-
-out:
-   return err;
-}
-
-static void asix_async_cmd_callback(struct urb *urb)
-{
-   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-   int status = urb->status;
-
-   if (status < 0)
-   printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
-   status);
-
-   kfree(req);
-   usb_free_urb(urb);
+   return usbnet_write_cmd(dev, cmd,
+   USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
+   value, index, data, size);
 }
 
 void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
  u16 size, void *data)
 {
-   struct usb_ctrlrequest *req;
-   int status;
-   struct urb *urb;
-
-   netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x 
index=0x%04x size=%d\n",
-  cmd, value, index, size);
-
-   urb = usb_alloc_urb(0, GFP_ATOMIC);
-   if (!urb) {
-   netdev_err(dev->net, "Error allocating URB in 
write_cmd_async!\n");
-   return;
-   }
-
-   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-   if (!req) {
-   netdev_err(dev->net, "Failed to allocate memory for control 
request\n");
-   usb_free_urb(urb);
-   return;
-   }
-
-   req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-   req->bRequest = cmd;
-   req->wValue = cpu_to_le16(value);
-   req->wIndex = cpu_to_le16(index);
-   req->wLength = cpu_to_le16(size);
-
-   usb_fill_control_urb(urb, dev->udev,
-usb_sndctrlpipe(dev->udev, 0),
-(void *)req, data, size,
-asix_async_cmd_callback, req);
-
-   status = usb_submit_urb(urb, GFP_ATOMIC);
-   if (status < 0) {
-   netdev_err(dev->net, "Error submitting the control message: 
status=%d\n",
-  status);
-   kfree(req);
-   usb_free_urb(urb);
-   }
+   usbnet_write_cmd_async(dev, cmd,
+  USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+  value, index, data, size);
 }
 
 int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-- 
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 01/11] usbnet: introduce usbnet 3 command helpers

[Ming Lei]

This patch introduces the below 3 usb command helpers:

usbnet_read_cmd / usbnet_write_cmd / usbnet_write_cmd_async

so that each low level driver doesn't need to implement them
by itself, and the dma buffer allocation for usb transfer and
runtime PM things can be handled just in one place.

Acked-by: Oliver Neukum 
Signed-off-by: Ming Lei 
---
 drivers/net/usb/usbnet.c   |  132 
 include/linux/usb/usbnet.h |6 ++
 2 files changed, 138 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index f9819d1..447d20d 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1611,6 +1611,138 @@ void usbnet_device_suggests_idle(struct usbnet *dev)
 EXPORT_SYMBOL(usbnet_device_suggests_idle);
 
 /*-*/
+int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+   u16 value, u16 index, void *data, u16 size)
+{
+   void *buf = NULL;
+   int err = -ENOMEM;
+
+   netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   if (data) {
+   buf = kmalloc(size, GFP_KERNEL);
+   if (!buf)
+   goto out;
+   }
+
+   err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_GET_TIMEOUT);
+   if (err > 0 && err <= size)
+   memcpy(data, buf, err);
+   kfree(buf);
+out:
+   return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd);
+
+int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+u16 value, u16 index, const void *data, u16 size)
+{
+   void *buf = NULL;
+   int err = -ENOMEM;
+
+   netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   if (data) {
+   buf = kmemdup(data, size, GFP_KERNEL);
+   if (!buf)
+   goto out;
+   }
+
+   err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+ cmd, reqtype, value, index, buf, size,
+ USB_CTRL_SET_TIMEOUT);
+   kfree(buf);
+
+out:
+   return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd);
+
+static void usbnet_async_cmd_cb(struct urb *urb)
+{
+   struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
+   int status = urb->status;
+
+   if (status < 0)
+   dev_dbg(&urb->dev->dev, "%s failed with %d",
+   __func__, status);
+
+   kfree(req);
+   usb_free_urb(urb);
+}
+
+int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+  u16 value, u16 index, const void *data, u16 size)
+{
+   struct usb_ctrlrequest *req = NULL;
+   struct urb *urb;
+   int err = -ENOMEM;
+   void *buf = NULL;
+
+   netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+  " value=0x%04x index=0x%04x size=%d\n",
+  cmd, reqtype, value, index, size);
+
+   urb = usb_alloc_urb(0, GFP_ATOMIC);
+   if (!urb) {
+   netdev_err(dev->net, "Error allocating URB in"
+  " %s!\n", __func__);
+   goto fail;
+   }
+
+   if (data) {
+   buf = kmemdup(data, size, GFP_ATOMIC);
+   if (!buf) {
+   netdev_err(dev->net, "Error allocating buffer"
+  " in %s!\n", __func__);
+   goto fail_free;
+   }
+   }
+
+   req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+   if (!req) {
+   netdev_err(dev->net, "Failed to allocate memory for %s\n",
+  __func__);
+   goto fail_free_buf;
+   }
+
+   req->bRequestType = reqtype;
+   req->bRequest = cmd;
+   req->wValue = cpu_to_le16(value);
+   req->wIndex = cpu_to_le16(index);
+   req->wLength = cpu_to_le16(size);
+
+   usb_fill_control_urb(urb, dev->udev,
+usb_sndctrlpipe(dev->udev, 0),
+(void *)req, buf, size,
+usbnet_async_cmd_cb, req);
+   urb->transfer_flags |= URB_FREE_BUFFER;
+
+   err = usb_submit_urb(urb, GFP_ATOMIC);
+   if (err < 0) {
+   netdev_err(dev->net, "Error submitting the control"
+  " message: status=%d\n", err);
+   goto fail_free;
+   }
+   return 0;
+
+fail_free_buf:
+   kfree(buf);
+fail_free:
+   kfree(req);
+   usb_free_urb(urb);
+fail:
+   return err;
+
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd

[PATCH v2 00/11] usbnet: usb_control_msg cleanup

[Ming Lei]

Hi,

This patch set introduces 3 helpers for handling usb read, write
and write_async command, and replaces the low level's implemention
with the generic ones.

This patchset is a cleanup and about 300 lines code is saved.

Also, the patchset fixes problem of DMA on buffer embedded inside
one dynamic allocated buffer, and such usages can be found
in cdc-ncm, sierra_net, mcs7830 and asix drivers.

v2:
- rebase on 3.7.0-rc2-next-20121025, only 01 and 03 changed

v1:
- drop previous patch 12

 drivers/net/usb/asix_common.c |  117 
 drivers/net/usb/cdc_ncm.c |   85 ++
 drivers/net/usb/dm9601.c  |  107 +
 drivers/net/usb/int51x1.c |   52 +---
 drivers/net/usb/mcs7830.c |   85 ++
 drivers/net/usb/net1080.c |  110 ++
 drivers/net/usb/plusb.c   |   11 ++--
 drivers/net/usb/sierra_net.c  |   45 ++
 drivers/net/usb/smsc75xx.c|   39 +---
 drivers/net/usb/smsc95xx.c|  115 +--
 drivers/net/usb/usbnet.c  |  132 +
 include/linux/usb/usbnet.h|6 ++
 12 files changed, 298 insertions(+), 606 deletions(-)


Thanks,
--
Ming Lei

--
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: USB on pandaboard

[kishon]

Hi,

On Thursday 25 October 2012 05:11 AM, Constantine Shulyupin wrote:

Hi,

Problem: USB (peripheral mode at least) on pandaboard on the last
kernel git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
still is not working.
Symptom: "couldn't find an available UDC"

The last revision when USB worked (I tested with zero gadget) is:
commit 657b306
Author: Kishon Vijay Abraham I 
Date:   Thu Sep 6 20:27:06 2012 +0530
 usb: phy: add a new driver for omap usb2 phy

Regression was introduced with the next commit:
commit 0e98de6
Author: Kishon Vijay Abraham I 
Date:   Thu Sep 6 20:27:07 2012 +0530
 usb: otg: make twl6030_usb as a comparator driver to omap_usb2

I suppose that current usb2 phy driver lacks FDT.
Suggestion Documentation/devicetree/bindings/usb/usb-phy.txt doesn’t help.


Nope. It should work with DT. Make sure you've enabled ocp2scp.

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: [PATCH 3/4] usb: Create link files between child device and usb port device.

[Greg KH]

On Thu, Oct 25, 2012 at 11:19:05AM +0800, Lan Tianyu wrote:
> On 2012/10/25 5:49, Greg KH wrote:
> >On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:
> >>于 2012/10/24 18:55, Sergei Shtylyov 写道:
> +/* Create link files between child device and usb port device. */
> +if (udev->parent) {
> +int no_warn;
> >>>
> >>>I think 'ret' or 'result' is a better name...
> >>>
> +struct usb_port *port_dev =
> +hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
> +
> +no_warn = sysfs_create_link(&udev->dev.kobj,
> +&port_dev->dev.kobj, "port");
> +no_warn = sysfs_create_link(&port_dev->dev.kobj,
> +&udev->dev.kobj, "child");
> >>>
> >>> I guess you are not supposed to ignore the result if the function 
> >>> requires
> >>>it not to be ignored.
> >>>
> >>Hi Sergei:
> >>Great thanks for your review. From my opinion, failure to create link 
> >> will
> >>not affect usb device function and so the return value can be ignored, 
> >>Perharps
> >>producing
> >>some warning will be better. Do you have some suggestion? :)
> >
> >Properly handle the error, don't ignore it.
> >
> HI Greg:
>   Ok, I get it. How about following?
> +   /* Create link files between child device and usb port device. */
> +   if (udev->parent) {
> +   struct usb_port *port_dev =
> +   hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
> +
> +   err = sysfs_create_link(&udev->dev.kobj,
> +   &port_dev->dev.kobj, "port");
> +   if (err)
> +   goto fail;
> +
> +   err = sysfs_create_link(&port_dev->dev.kobj,
> +   &udev->dev.kobj, "child");
> +   if (err) {
> +   sysfs_remove_link(&udev->dev.kobj, "port");
> +   goto fail;
> +   }
> +   }
> +

It's a good start, did you test it out?

And why are you calling the devices in a port a "child"?  It's a device.
Otherwise everything is a child, and that doesn't make any sense, right?

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 3/4] usb: Create link files between child device and usb port device.

[Lan Tianyu]

On 2012年10月25日 05:49, Greg KH wrote:
> On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:
>> 于 2012/10/24 18:55, Sergei Shtylyov 写道:
 +/* Create link files between child device and usb port device. */
 +if (udev->parent) {
 +int no_warn;
>>>
>>>I think 'ret' or 'result' is a better name...
>>>
 +struct usb_port *port_dev =
 +hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
 +
 +no_warn = sysfs_create_link(&udev->dev.kobj,
 +&port_dev->dev.kobj, "port");
 +no_warn = sysfs_create_link(&port_dev->dev.kobj,
 +&udev->dev.kobj, "child");
>>>
>>> I guess you are not supposed to ignore the result if the function 
>>> requires 
>>> it not to be ignored.
>>>
>> Hi Sergei:
>>  Great thanks for your review. From my opinion, failure to create link 
>> will
>> not affect usb device function and so the return value can be ignored, 
>> Perharps
>> producing
>> some warning will be better. Do you have some suggestion? :)
> 
> Properly handle the error, don't ignore it.
HI Greg:
How about following?
+   /* Create link files between child device and usb port device. */
+   if (udev->parent) {
+   struct usb_port *port_dev =
+   hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
+
+   err = sysfs_create_link(&udev->dev.kobj,
+   &port_dev->dev.kobj, "port");
+   if (err)
+   goto fail;
+
+   err = sysfs_create_link(&port_dev->dev.kobj,
+   &udev->dev.kobj, "child");
+   if (err) {
+   sysfs_remove_link(&udev->dev.kobj, "port");
+   goto fail;
+   }
+   }
+

> 
> greg k-h
> 


-- 
Best regards
Tianyu Lan
--
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 3/4] usb: Create link files between child device and usb port device.

[Lan Tianyu]

On 2012/10/25 5:49, Greg KH wrote:

On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:

于 2012/10/24 18:55, Sergei Shtylyov 写道:

+/* Create link files between child device and usb port device. */
+if (udev->parent) {
+int no_warn;


I think 'ret' or 'result' is a better name...


+struct usb_port *port_dev =
+hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
+
+no_warn = sysfs_create_link(&udev->dev.kobj,
+&port_dev->dev.kobj, "port");
+no_warn = sysfs_create_link(&port_dev->dev.kobj,
+&udev->dev.kobj, "child");


 I guess you are not supposed to ignore the result if the function requires
it not to be ignored.


Hi Sergei:
Great thanks for your review. From my opinion, failure to create link 
will
not affect usb device function and so the return value can be ignored, Perharps
producing
some warning will be better. Do you have some suggestion? :)


Properly handle the error, don't ignore it.


HI Greg:
Ok, I get it. How about following?
+   /* Create link files between child device and usb port device. */
+   if (udev->parent) {
+   struct usb_port *port_dev =
+   hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
+
+   err = sysfs_create_link(&udev->dev.kobj,
+   &port_dev->dev.kobj, "port");
+   if (err)
+   goto fail;
+
+   err = sysfs_create_link(&port_dev->dev.kobj,
+   &udev->dev.kobj, "child");
+   if (err) {
+   sysfs_remove_link(&udev->dev.kobj, "port");
+   goto fail;
+   }
+   }
+


greg k-h




--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader

[Ming Lei]

On Thu, Oct 25, 2012 at 5:31 AM, Rafael J. Wysocki  wrote:

>> I don't know how to use those.  Is there a doc somewhere?
>
> I'm not sure, but let's ask the developer who added them.
>
> Lei, do you have any doc describing how to use those tracepoints?

Hi Bjorn Helgaas,

Firstly, please enable these tracepoints via below

  echo 1 > /sys/kernel/debug/tracing/events/rpm/enable

then store the trace event result by below once you make sure your interested
events are completed:

  echo 0 > /sys/kernel/debug/tracing/events/rpm/enable
  cp /sys/kernel/debug/tracing/trace /media/ram/rpm-trace

For the format of the captured trace log(rpm-trace), please see

include/trace/events/rpm.h

which is just a replacement of previous printk and add some extra fields
into that.  If you still don't understand the format, please post the
result onto list, and I think many guys here may help you out.

Hope the above can help you.


Thanks,
--
Ming Lei
--
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/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware

[Laurent Pinchart]

Hi Felipe,

On Monday 22 October 2012 13:56:01 Felipe Balbi wrote:
> On Mon, Oct 22, 2012 at 12:47:21PM +0200, Laurent Pinchart wrote:
> > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote:
> > > On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote:
> > > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote:
> > > > > Some gadget drivers may attempt to dequeue requests for an endpoint
> > > > > that has already been disabled. For example, in the UVC gadget
> > > > > driver, uvc_function_set_alt() will call usb_ep_disable() when alt
> > > > > setting 0 is selected. When the userspace application subsequently
> > > > > issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes
> > > > > usb_ep_dequeue() to ensure that all requests have been cancelled.
> > > > 
> > > > bug is on uvc gadget, then. Laurent ?
> > 
> > We've discussed this topic a couple of months before. I believe that's not
> > a bug.
> > 
> > http://68.183.106.108/lists/linux-usb/msg68869.html
> 
> fair enough :-)
> 
> That's a different case, however. At the link above we're discussing
> dequeueing a request which is already being dequeued. $SUBJECT is trying
> to fix dequeueing of a request for an endpoint which isn't even enabled.

You've got a point there :-) That's a different case indeed, I'm open to (at 
least evaluating) a fix in the UVC gadget driver if you think that's better.

-- 
Regards,

Laurent Pinchart


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware

[Laurent Pinchart]

Hi Simon,

On Monday 22 October 2012 12:49:51 Simon Haggett wrote:
> On 22/10/12 11:47, Laurent Pinchart wrote:
> > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote:
> >> On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote:
> >>> On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote:
>  Some gadget drivers may attempt to dequeue requests for an endpoint
>  that has already been disabled. For example, in the UVC gadget driver,
>  uvc_function_set_alt() will call usb_ep_disable() when alt setting 0
>  is selected. When the userspace application subsequently issues the
>  VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to
> >>> 
> >>> ensure that all requests have been cancelled.
> >>> 
> >>> bug is on uvc gadget, then. Laurent ?
> > 
> > We've discussed this topic a couple of months before. I believe that's not
> > a bug.
> > 
> > http://68.183.106.108/lists/linux-usb/msg68869.html
> > 
> >>> Also, fsl should be removed from the tree, I'm trying to persuade iMX
> >>> folks to use drivers/usb/chipidea instead.
> >> 
> >> Besides the iMX usage, the driver is also being used by many Freescale
> >> PowerPC/Coldfire SoCs.  I agree that it's ideal to move to a common
> >> driver.
> >> But it is a large task to make the chipidea driver works for all the
> >> hardware that fsl_udc had supported and been tested on.
> >> 
>  For the Freescale High Speed Dual-Role USB controller,
>  fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If
>  this is called for a disabled endpoint, a kernel oops will occur when
> >>> 
> >>> the ep->ep.desc field is dereferenced (by ep_index()).
> >>> 
>  fsl_ep_disable() sets this field to NULL, as well as deleting all
>  pending requests for the endpoint.
>  
>  This patch adds an additional check to fsl_ep_dequeue() to ensure that
>  the endpoint has not already been disabled before attempting to dequeue
> >>> 
> >>> requests.
> >>> 
>  Signed-off-by: Simon Haggett 
>  ---
>  
>    drivers/usb/gadget/fsl_udc_core.c |5 -
>    1 files changed, 4 insertions(+), 1 deletions(-)
>  
>  diff --git a/drivers/usb/gadget/fsl_udc_core.c
>  b/drivers/usb/gadget/fsl_udc_core.c
>  index 6ae70cb..acd513b 100644
>  --- a/drivers/usb/gadget/fsl_udc_core.c
>  +++ b/drivers/usb/gadget/fsl_udc_core.c
>  @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep,
> >>> 
> >>> struct usb_request *_req)
> >>> 
>   int ep_num, stopped, ret = 0;
>   u32 epctrl;
>  
>  -if (!_ep || !_req)
>  +/* Ensure that the ep and request are valid, and the ep is not
>  + * disabled
>  + */
>  +if (!_ep || !_req || !ep->ep.desc)
>  
>   return -EINVAL;
> > 
> > Shouldn't that last check be done with a lock taken ?
> 
> I had presumed a lock wasn't necessary because ep->ep.desc is only set
> to NULL by fsl_ep_disable() which, since it is called by
> usb_ep_disable(), should only be invoked when no other task is using the
> endpoint (according to include/linux/usb/gadget.h). Furthermore, the
> chipidea UDC driver does check the equivalent of this field is not NULL
> without taking a lock (ep_dequeue() in drivers/usb/chipidea/udc.c).
> 
> However, it is possible that I'm misunderstanding something here, so
> apologies if I am.

I might be wrong as well :-) I just wanted to point out something that 
appeared to me as a possible issue.

>   spin_lock_irqsave(&ep->udc->lock, flags);

-- 
Regards,

Laurent Pinchart

--
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

USB on pandaboard

[Constantine Shulyupin]

Hi,

Problem: USB (peripheral mode at least) on pandaboard on the last
kernel git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
still is not working.
Symptom: "couldn't find an available UDC"

The last revision when USB worked (I tested with zero gadget) is:
commit 657b306
Author: Kishon Vijay Abraham I 
Date:   Thu Sep 6 20:27:06 2012 +0530
usb: phy: add a new driver for omap usb2 phy

Regression was introduced with the next commit:
commit 0e98de6
Author: Kishon Vijay Abraham I 
Date:   Thu Sep 6 20:27:07 2012 +0530
usb: otg: make twl6030_usb as a comparator driver to omap_usb2

I suppose that current usb2 phy driver lacks FDT.
Suggestion Documentation/devicetree/bindings/usb/usb-phy.txt doesn’t help.

Tell me please, is working revision of usb2 exists somewhere or
required patches exist?
If the work of usb2 phy implementation is not yet finished and needs
assistance, I can assist.

Thanks

-- 
Constantine Shulyupin
http://www.MakeLinux.com/
Embedded Linux Systems,
Device Drivers, TI DaVinci
--
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] qmi_wwan/cdc_ether: move Novatel 551 and E362 to qmi_wwan

[Greg KH]

On Wed, Oct 24, 2012 at 05:10:34PM -0500, Dan Williams wrote:
> These devices provide QMI and ethernet functionality via a standard CDC
> ethernet descriptor.  But when driven by cdc_ether, the QMI
> functionality is unavailable because only cdc_ether can claim the USB
> interface.  Thus blacklist the devices in cdc_ether and add their IDs to
> qmi_wwan, which enables both QMI and ethernet simultaneously.
> 
> Signed-off-by: Dan Williams 
> Cc: sta...@vger.kernel.org
> ---
> Greg: CC-ed to linux-usb@ for visibility but should probably go through davem

Yes, it should go through netdev:

Acked-by: Greg Kroah-Hartman 

--
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] qmi_wwan/cdc_ether: move Novatel 551 and E362 to qmi_wwan

[Dan Williams]

These devices provide QMI and ethernet functionality via a standard CDC
ethernet descriptor.  But when driven by cdc_ether, the QMI
functionality is unavailable because only cdc_ether can claim the USB
interface.  Thus blacklist the devices in cdc_ether and add their IDs to
qmi_wwan, which enables both QMI and ethernet simultaneously.

Signed-off-by: Dan Williams 
Cc: sta...@vger.kernel.org
---
Greg: CC-ed to linux-usb@ for visibility but should probably go through davem


diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a03de71..d012982 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -592,6 +592,32 @@ static const struct usb_device_id  products [] = {
.driver_info= 0,
 },
 
+/* Novatel USB551L and MC551 - handled by qmi_wwan */
+{
+   .match_flags=   USB_DEVICE_ID_MATCH_VENDOR
+| USB_DEVICE_ID_MATCH_PRODUCT
+| USB_DEVICE_ID_MATCH_INT_INFO,
+   .idVendor   = NOVATEL_VENDOR_ID,
+   .idProduct  = 0xB001,
+   .bInterfaceClass= USB_CLASS_COMM,
+   .bInterfaceSubClass = USB_CDC_SUBCLASS_ETHERNET,
+   .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+   .driver_info = 0,
+},
+
+/* Novatel E362 - handled by qmi_wwan */
+{
+   .match_flags=   USB_DEVICE_ID_MATCH_VENDOR
+| USB_DEVICE_ID_MATCH_PRODUCT
+| USB_DEVICE_ID_MATCH_INT_INFO,
+   .idVendor   = NOVATEL_VENDOR_ID,
+   .idProduct  = 0x9010,
+   .bInterfaceClass= USB_CLASS_COMM,
+   .bInterfaceSubClass = USB_CDC_SUBCLASS_ETHERNET,
+   .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+   .driver_info = 0,
+},
+
 /*
  * WHITELIST!!!
  *
@@ -604,21 +630,6 @@ static const struct usb_device_id  products [] = {
  * because of bugs/quirks in a given product (like Zaurus, above).
  */
 {
-   /* Novatel USB551L */
-   /* This match must come *before* the generic CDC-ETHER match so that
-* we get FLAG_WWAN set on the device, since it's descriptors are
-* generic CDC-ETHER.
-*/
-   .match_flags=   USB_DEVICE_ID_MATCH_VENDOR
-| USB_DEVICE_ID_MATCH_PRODUCT
-| USB_DEVICE_ID_MATCH_INT_INFO,
-   .idVendor   = NOVATEL_VENDOR_ID,
-   .idProduct  = 0xB001,
-   .bInterfaceClass= USB_CLASS_COMM,
-   .bInterfaceSubClass = USB_CDC_SUBCLASS_ETHERNET,
-   .bInterfaceProtocol = USB_CDC_PROTO_NONE,
-   .driver_info = (unsigned long)&wwan_info,
-}, {
/* ZTE (Vodafone) K3805-Z */
.match_flags=   USB_DEVICE_ID_MATCH_VENDOR
 | USB_DEVICE_ID_MATCH_PRODUCT
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6883c37..05fd4a1 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -369,6 +369,20 @@ static const struct usb_device_id products[] = {
USB_VENDOR_AND_INTERFACE_INFO(0x106c, USB_CLASS_VENDOR_SPEC, 
0xf1, 0xff),
.driver_info= (unsigned long)&qmi_wwan_info,
},
+   {   /* Novatel USB551L and MC551 */
+   USB_DEVICE_AND_INTERFACE_INFO(0x1410, 0xb001,
+ USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+   .driver_info= (unsigned long)&qmi_wwan_info,
+   },
+   {   /* Novatel E362 */
+   USB_DEVICE_AND_INTERFACE_INFO(0x1410, 0x9010,
+ USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+   .driver_info= (unsigned long)&qmi_wwan_info,
+   },
 
/* 3. Combined interface devices matching on interface number */
{QMI_FIXED_INTF(0x19d2, 0x0002, 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: tegra remove include of

[Greg Kroah-Hartman]

On Wed, Oct 24, 2012 at 03:44:27PM -0600, Stephen Warren wrote:
> On 10/24/2012 03:37 PM, Greg Kroah-Hartman wrote:
> > On Tue, Oct 02, 2012 at 04:49:25PM -0600, Stephen Warren wrote:
> >> From: Stephen Warren 
> >>
> >> Almost nothing from this file is used, and the file will hopefully be
> >> deleted soon. Copy the tiny portions that are used directly into
> >> ehci-tegra.c. I believe that Venu Byravarasu is working on cleaning up
> >> our USB driver, and those cleanups will remove the need for these
> >> constants.
> >>
> >> Signed-off-by: Stephen Warren 
> >> Acked-by: Venu Byravarasu 
> >> ---
> >> Greg, if this patch can get into 3.7 (it's late, I know) at a point before
> >> wherever you branch off your USB tree for 3.8, then that would be great.
> >> If not, may I please request this patch be merged into a separate 
> >> branch/tag
> >> that is merged into both the Tegra and USB trees for 3.8, since I will
> >> probably have cleanup patches in the Tegra tree that depend on this change,
> >> and Venu will likely be sending patches through the USB tree that will need
> >> to be merged with this too.
> > 
> > I don't know how you want to do this, but this, and the other tegra
> > patch, are now in the usb-next branch of my usb.git tree.
> 
> Aargh! Felipe has already applied the PHY patch to his branch. I believe
> he was also going to apply the EHCI driver patch too.

That's ok, they will merge together just fine.

> Having this just be applied to a next branch rather than a topic branch
> was exactly what I wanted to avoid; it's why I wrote the heads-up above...
> 
> > I would recommend just cherry-picking the thing into your tree if you
> > want to do work on top of it, we can handle merge issues later on in
> > 3.8-rc1.
> 
> If I just cherry-pick the change, then that will cause the same commit
> to exist under different commit IDs in two different trees. Isn't that a
> no-no? That's exactly why I wanted this in a topic branch so we could
> both merge it rather than duplicating it.

This is a 3 line patch, not a big deal either way if it's duplicated.  A
whole different branch isn't really needed, I'm not insane like the arm
development trees, I only have 2 branches :)

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 3/4] usb: Create link files between child device and usb port device.

[Greg KH]

On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:
> 于 2012/10/24 18:55, Sergei Shtylyov 写道:
> >> +/* Create link files between child device and usb port device. */
> >> +if (udev->parent) {
> >> +int no_warn;
> > 
> >I think 'ret' or 'result' is a better name...
> > 
> >> +struct usb_port *port_dev =
> >> +hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
> >> +
> >> +no_warn = sysfs_create_link(&udev->dev.kobj,
> >> +&port_dev->dev.kobj, "port");
> >> +no_warn = sysfs_create_link(&port_dev->dev.kobj,
> >> +&udev->dev.kobj, "child");
> > 
> > I guess you are not supposed to ignore the result if the function 
> > requires 
> > it not to be ignored.
> > 
> Hi Sergei:
>   Great thanks for your review. From my opinion, failure to create link 
> will
> not affect usb device function and so the return value can be ignored, 
> Perharps
> producing
> some warning will be better. Do you have some suggestion? :)

Properly handle the error, don't ignore it.

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 1/2] ARM: OMAP2+: Introduce local usb.h

[Tony Lindgren]

* Felipe Balbi  [121019 00:45]:
> On Thu, Oct 18, 2012 at 07:20:05PM -0700, Tony Lindgren wrote:
> > Let's move what we can from plat/usb.h to the local usb.h
> > for ARM common zImage support.
> > 
> > This is needed so we can remove plat/usb.h for ARM common
> > zImage support.
> > 
> > Cc: Felipe Balbi 
> 
> Acked-by: Felipe Balbi 

Thanks, I've now pushed these two patches on v3.7-rc1 to
omap-for-v3.8/cleanup-headers-usb branch.

Greg and Samuel, in case you need to, it's OK to merge the
branch above now that plat/usb.h is moved.

Regards,

Tony
--
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: tegra remove include of

[Stephen Warren]

On 10/24/2012 03:37 PM, Greg Kroah-Hartman wrote:
> On Tue, Oct 02, 2012 at 04:49:25PM -0600, Stephen Warren wrote:
>> From: Stephen Warren 
>>
>> Almost nothing from this file is used, and the file will hopefully be
>> deleted soon. Copy the tiny portions that are used directly into
>> ehci-tegra.c. I believe that Venu Byravarasu is working on cleaning up
>> our USB driver, and those cleanups will remove the need for these
>> constants.
>>
>> Signed-off-by: Stephen Warren 
>> Acked-by: Venu Byravarasu 
>> ---
>> Greg, if this patch can get into 3.7 (it's late, I know) at a point before
>> wherever you branch off your USB tree for 3.8, then that would be great.
>> If not, may I please request this patch be merged into a separate branch/tag
>> that is merged into both the Tegra and USB trees for 3.8, since I will
>> probably have cleanup patches in the Tegra tree that depend on this change,
>> and Venu will likely be sending patches through the USB tree that will need
>> to be merged with this too.
> 
> I don't know how you want to do this, but this, and the other tegra
> patch, are now in the usb-next branch of my usb.git tree.

Aargh! Felipe has already applied the PHY patch to his branch. I believe
he was also going to apply the EHCI driver patch too.

Having this just be applied to a next branch rather than a topic branch
was exactly what I wanted to avoid; it's why I wrote the heads-up above...

> I would recommend just cherry-picking the thing into your tree if you
> want to do work on top of it, we can handle merge issues later on in
> 3.8-rc1.

If I just cherry-pick the change, then that will cause the same commit
to exist under different commit IDs in two different trees. Isn't that a
no-no? That's exactly why I wanted this in a topic branch so we could
both merge it rather than duplicating it.
--
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: tegra remove include of

[Greg Kroah-Hartman]

On Tue, Oct 02, 2012 at 04:49:25PM -0600, Stephen Warren wrote:
> From: Stephen Warren 
> 
> Almost nothing from this file is used, and the file will hopefully be
> deleted soon. Copy the tiny portions that are used directly into
> ehci-tegra.c. I believe that Venu Byravarasu is working on cleaning up
> our USB driver, and those cleanups will remove the need for these
> constants.
> 
> Signed-off-by: Stephen Warren 
> Acked-by: Venu Byravarasu 
> ---
> Greg, if this patch can get into 3.7 (it's late, I know) at a point before
> wherever you branch off your USB tree for 3.8, then that would be great.
> If not, may I please request this patch be merged into a separate branch/tag
> that is merged into both the Tegra and USB trees for 3.8, since I will
> probably have cleanup patches in the Tegra tree that depend on this change,
> and Venu will likely be sending patches through the USB tree that will need
> to be merged with this too.

I don't know how you want to do this, but this, and the other tegra
patch, are now in the usb-next branch of my usb.git tree.

I would recommend just cherry-picking the thing into your tree if you
want to do work on top of it, we can handle merge issues later on in
3.8-rc1.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] Enable USB peripheral mode on dm365 EVM

[Greg KH]

On Wed, Oct 10, 2012 at 02:33:32PM +0200, Constantine Shulyupin wrote:
> From: Constantine Shulyupin 
> 
> Sets USB PHY clock source to 24 MHz clock and call USB configuration from 
> board initialization.
> 
> Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC.
> 
> References:
> 
> Definition of USB_PHY_CTRL and PHYCLKFREQ:
> - http://www.makelinux.com/lib/ti/DM36x_ARM/doc-141
> 
> Original patch by miguel.agui...@ridgerun.com three years ago:
> - 
> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html
> 
> Signed-off-by: Constantine Shulyupin 

Note, Felipe is the right person to be sending these types of patches
to, not me.  scripts/get_maintainer.pl is your friend :)

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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader

[Rafael J. Wysocki]

On Wednesday 24 of October 2012 15:13:45 Bjorn Helgaas wrote:
> On Wed, Oct 24, 2012 at 3:10 PM, Rafael J. Wysocki  wrote:
> > On Wednesday 24 of October 2012 14:18:40 Bjorn Helgaas wrote:
> >> On Tue, Oct 23, 2012 at 11:14 AM, Ulrich Eckhardt  
> >> wrote:
> >> > Am 23.10.2012 01:40, schrieb Sarah Sharp:
> >> >> On Sun, Oct 21, 2012 at 06:03:24PM +0200, Ulrich Eckhardt wrote:
> >> >
> >> >>> I have tested it with Kernel 3.6.2. The USB3 port is the one built
> >> >>> in the Mainboard M4A87DT EVO.
> >> >>
> >> >> Did a previous kernel version work?
> >> >
> >> > Yes, I tried the Kernel shipped with OpenSUSE (3.4.11) several 3.5 and 
> >> > all
> >> > available 3.6 kernel versions.
> >>
> >> I assume you mean that every kernel you tried failed?  If some
> >> kernel(s) did work, what is the newest working version and the oldest
> >> broken version?
> >>
> >> > I attach the complete dmesg logs and the lspci -vv output as requested by
> >> > Bjorn for the situations when the controller does not work after startup
> >> > (extension notworking), when the controller is working (extension OK), 
> >> > and
> >> > after stopping (extension stop).
> >>
> >> In the "notworking" logs, my guess is that the xHCI device is being
> >> powered off.  The "notworking" lspci shows this:
> >>
> >>   00:0a.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD790 PCI
> >> to PCI bridge (PCI express gpp port F) (prog-if 00 [Normal decode])
> >> Bus: primary=00, secondary=04, subordinate=04, sec-latency=0
> >> LnkCap: Port #247, Speed 5GT/s, Width x2, ASPM L0s L1,
> >> Latency L0 <64ns, L1 <1us
> >> LnkSta: Speed unknown, Width x1, TrErr- Train-
> >> SlotClk+ DLActive- BWMgmt+ ABWMgmt+
> >>   04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
> >> Controller (rev ff) (prog-if ff)
> >> !!! Unknown header type 7f
> >> Kernel driver in use: xhci_hcd
> >>
> >> The bridge's "DLActive-" means that the downstream link leading to the
> >> xHCI device is not active.  The "rev ff", "prog-if ff", and "header
> >> type 7f" suggest that we read all 0xff's from the xHCI device, which
> >> is what we get if the device doesn't respond.
> >>
> >> The xHCI device must have been powered up and working originally,
> >> because we enumerated it and bound the xhci_hcd driver to it.  I don't
> >> have any ideas about why it would be powered off.
> >>
> >> Rafael, Alan, is there any way we can disable power management or
> >> enable related debug messages?
> >
> > It can be disabled by writing "on" to the controller's
> > /sys/devices/.../power/control file.
> 
> Ulrich, if you boot in the working situation and write "on" to the
> control file as above, can you still reproduce the failure?
> 
> > For debug, there are trace points in rpm_suspend() and rpm_resume().
> 
> I don't know how to use those.  Is there a doc somewhere?

I'm not sure, but let's ask the developer who added them.

Lei, do you have any doc describing how to use those tracepoints?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader

[Bjorn Helgaas]

On Wed, Oct 24, 2012 at 3:10 PM, Rafael J. Wysocki  wrote:
> On Wednesday 24 of October 2012 14:18:40 Bjorn Helgaas wrote:
>> On Tue, Oct 23, 2012 at 11:14 AM, Ulrich Eckhardt  
>> wrote:
>> > Am 23.10.2012 01:40, schrieb Sarah Sharp:
>> >> On Sun, Oct 21, 2012 at 06:03:24PM +0200, Ulrich Eckhardt wrote:
>> >
>> >>> I have tested it with Kernel 3.6.2. The USB3 port is the one built
>> >>> in the Mainboard M4A87DT EVO.
>> >>
>> >> Did a previous kernel version work?
>> >
>> > Yes, I tried the Kernel shipped with OpenSUSE (3.4.11) several 3.5 and all
>> > available 3.6 kernel versions.
>>
>> I assume you mean that every kernel you tried failed?  If some
>> kernel(s) did work, what is the newest working version and the oldest
>> broken version?
>>
>> > I attach the complete dmesg logs and the lspci -vv output as requested by
>> > Bjorn for the situations when the controller does not work after startup
>> > (extension notworking), when the controller is working (extension OK), and
>> > after stopping (extension stop).
>>
>> In the "notworking" logs, my guess is that the xHCI device is being
>> powered off.  The "notworking" lspci shows this:
>>
>>   00:0a.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD790 PCI
>> to PCI bridge (PCI express gpp port F) (prog-if 00 [Normal decode])
>> Bus: primary=00, secondary=04, subordinate=04, sec-latency=0
>> LnkCap: Port #247, Speed 5GT/s, Width x2, ASPM L0s L1,
>> Latency L0 <64ns, L1 <1us
>> LnkSta: Speed unknown, Width x1, TrErr- Train-
>> SlotClk+ DLActive- BWMgmt+ ABWMgmt+
>>   04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
>> Controller (rev ff) (prog-if ff)
>> !!! Unknown header type 7f
>> Kernel driver in use: xhci_hcd
>>
>> The bridge's "DLActive-" means that the downstream link leading to the
>> xHCI device is not active.  The "rev ff", "prog-if ff", and "header
>> type 7f" suggest that we read all 0xff's from the xHCI device, which
>> is what we get if the device doesn't respond.
>>
>> The xHCI device must have been powered up and working originally,
>> because we enumerated it and bound the xhci_hcd driver to it.  I don't
>> have any ideas about why it would be powered off.
>>
>> Rafael, Alan, is there any way we can disable power management or
>> enable related debug messages?
>
> It can be disabled by writing "on" to the controller's
> /sys/devices/.../power/control file.

Ulrich, if you boot in the working situation and write "on" to the
control file as above, can you still reproduce the failure?

> For debug, there are trace points in rpm_suspend() and rpm_resume().

I don't know how to use those.  Is there a doc somewhere?
--
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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader

[Rafael J. Wysocki]

On Wednesday 24 of October 2012 14:18:40 Bjorn Helgaas wrote:
> On Tue, Oct 23, 2012 at 11:14 AM, Ulrich Eckhardt  
> wrote:
> > Am 23.10.2012 01:40, schrieb Sarah Sharp:
> >> On Sun, Oct 21, 2012 at 06:03:24PM +0200, Ulrich Eckhardt wrote:
> >
> >>> I have tested it with Kernel 3.6.2. The USB3 port is the one built
> >>> in the Mainboard M4A87DT EVO.
> >>
> >> Did a previous kernel version work?
> >
> > Yes, I tried the Kernel shipped with OpenSUSE (3.4.11) several 3.5 and all
> > available 3.6 kernel versions.
> 
> I assume you mean that every kernel you tried failed?  If some
> kernel(s) did work, what is the newest working version and the oldest
> broken version?
> 
> > I attach the complete dmesg logs and the lspci -vv output as requested by
> > Bjorn for the situations when the controller does not work after startup
> > (extension notworking), when the controller is working (extension OK), and
> > after stopping (extension stop).
> 
> In the "notworking" logs, my guess is that the xHCI device is being
> powered off.  The "notworking" lspci shows this:
> 
>   00:0a.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD790 PCI
> to PCI bridge (PCI express gpp port F) (prog-if 00 [Normal decode])
> Bus: primary=00, secondary=04, subordinate=04, sec-latency=0
> LnkCap: Port #247, Speed 5GT/s, Width x2, ASPM L0s L1,
> Latency L0 <64ns, L1 <1us
> LnkSta: Speed unknown, Width x1, TrErr- Train-
> SlotClk+ DLActive- BWMgmt+ ABWMgmt+
>   04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
> Controller (rev ff) (prog-if ff)
> !!! Unknown header type 7f
> Kernel driver in use: xhci_hcd
> 
> The bridge's "DLActive-" means that the downstream link leading to the
> xHCI device is not active.  The "rev ff", "prog-if ff", and "header
> type 7f" suggest that we read all 0xff's from the xHCI device, which
> is what we get if the device doesn't respond.
> 
> The xHCI device must have been powered up and working originally,
> because we enumerated it and bound the xhci_hcd driver to it.  I don't
> have any ideas about why it would be powered off.
> 
> Rafael, Alan, is there any way we can disable power management or
> enable related debug messages?

It can be disabled by writing "on" to the controller's
/sys/devices/.../power/control file.

For debug, there are trace points in rpm_suspend() and rpm_resume().

That's if we're talking about runtime PM, of course.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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

usb/host/xhci.c:2257: possible coding error in logical expression ?

[David Binderman]

Hello there,

The source code is from the subject line is

static bool xhci_is_sync_in_ep(unsigned int ep_type)
{
return (ep_type == ISOC_IN_EP || ep_type != INT_IN_EP);
}

The static analyser cppcheck says

[linux-3.7-rc2/drivers/usb/host/xhci.c:2257]: (style) Redundant condition: If
ep_type == 5, the comparison ep_type != 7 is always true.

Maybe the original programmer intention was something like

static bool xhci_is_sync_in_ep(unsigned int ep_type)
{
return (ep_type == ISOC_IN_EP || ep_type == INT_IN_EP);
}

Suggest code rework.
Regards

David Binderman
  --
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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader

[Bjorn Helgaas]

On Tue, Oct 23, 2012 at 11:14 AM, Ulrich Eckhardt  wrote:
> Am 23.10.2012 01:40, schrieb Sarah Sharp:
>> On Sun, Oct 21, 2012 at 06:03:24PM +0200, Ulrich Eckhardt wrote:
>
>>> I have tested it with Kernel 3.6.2. The USB3 port is the one built
>>> in the Mainboard M4A87DT EVO.
>>
>> Did a previous kernel version work?
>
> Yes, I tried the Kernel shipped with OpenSUSE (3.4.11) several 3.5 and all
> available 3.6 kernel versions.

I assume you mean that every kernel you tried failed?  If some
kernel(s) did work, what is the newest working version and the oldest
broken version?

> I attach the complete dmesg logs and the lspci -vv output as requested by
> Bjorn for the situations when the controller does not work after startup
> (extension notworking), when the controller is working (extension OK), and
> after stopping (extension stop).

In the "notworking" logs, my guess is that the xHCI device is being
powered off.  The "notworking" lspci shows this:

  00:0a.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD790 PCI
to PCI bridge (PCI express gpp port F) (prog-if 00 [Normal decode])
Bus: primary=00, secondary=04, subordinate=04, sec-latency=0
LnkCap: Port #247, Speed 5GT/s, Width x2, ASPM L0s L1,
Latency L0 <64ns, L1 <1us
LnkSta: Speed unknown, Width x1, TrErr- Train-
SlotClk+ DLActive- BWMgmt+ ABWMgmt+
  04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
Controller (rev ff) (prog-if ff)
!!! Unknown header type 7f
Kernel driver in use: xhci_hcd

The bridge's "DLActive-" means that the downstream link leading to the
xHCI device is not active.  The "rev ff", "prog-if ff", and "header
type 7f" suggest that we read all 0xff's from the xHCI device, which
is what we get if the device doesn't respond.

The xHCI device must have been powered up and working originally,
because we enumerated it and bound the xhci_hcd driver to it.  I don't
have any ideas about why it would be powered off.

Rafael, Alan, is there any way we can disable power management or
enable related debug messages?

Bjorn
--
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] bas_gigaset: fix pre_reset handling

[Tilman Schmidt]

The delayed work function int_in_work() may call usb_reset_device()
and thus, indirectly, the driver's pre_reset method. Trying to
cancel the work synchronously in that situation would deadlock.
Fix by avoiding cancel_work_sync() in the pre_reset method.

If the reset was NOT initiated by int_in_work() this might cause
int_in_work() to run after the post_reset method, with urb_int_in
already resubmitted, so handle that case gracefully.

Signed-off-by: Tilman Schmidt 
---
 drivers/isdn/gigaset/bas-gigaset.c |   19 ---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c 
b/drivers/isdn/gigaset/bas-gigaset.c
index 5275887..c44950d 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -617,7 +617,13 @@ static void int_in_work(struct work_struct *work)
if (rc == 0)
/* success, resubmit interrupt read URB */
rc = usb_submit_urb(urb, GFP_ATOMIC);
-   if (rc != 0 && rc != -ENODEV) {
+
+   switch (rc) {
+   case 0: /* success */
+   case -ENODEV:   /* device gone */
+   case -EINVAL:   /* URB already resubmitted, or terminal badness */
+   break;
+   default:/* failure: try to recover by resetting the device */
dev_err(cs->dev, "clear halt failed: %s\n", get_usb_rcmsg(rc));
rc = usb_lock_device_for_reset(ucs->udev, ucs->interface);
if (rc == 0) {
@@ -2442,7 +2448,9 @@ static void gigaset_disconnect(struct usb_interface 
*interface)
 }
 
 /* gigaset_suspend
- * This function is called before the USB connection is suspended.
+ * This function is called before the USB connection is suspended
+ * or before the USB device is reset.
+ * In the latter case, message == PMSG_ON.
  */
 static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 {
@@ -2498,7 +2506,12 @@ static int gigaset_suspend(struct usb_interface *intf, 
pm_message_t message)
del_timer_sync(&ucs->timer_atrdy);
del_timer_sync(&ucs->timer_cmd_in);
del_timer_sync(&ucs->timer_int_in);
-   cancel_work_sync(&ucs->int_in_wq);
+
+   /* don't try to cancel int_in_wq from within reset as it
+* might be the one requesting the reset
+*/
+   if (message.event != PM_EVENT_ON)
+   cancel_work_sync(&ucs->int_in_wq);
 
gig_dbg(DEBUG_SUSPEND, "suspend complete");
return 0;
-- 
1.7.3.4

--
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 REPOST] usb: host: tegra remove include of

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> From: Stephen Warren 
> 
> Almost nothing from this file is used, and the file will hopefully be
> deleted soon. Copy the tiny portions that are used directly into
> ehci-tegra.c. I believe that Venu Byravarasu is working on cleaning up
> our USB driver, and those cleanups will remove the need for these
> constants.
> 
> Signed-off-by: Stephen Warren 
> Acked-by: Venu Byravarasu 
> ---
> Felipe,
> 
> This patch is needed in the Tegra branch, since it's a dependency for
> cleanup required to support single zImage.
> 
> However, I suspect/hope there will be significant improvements to the Tegra
> USB driver in 3.8, and this change may merge-conflict with those. It may be
> simplest if this patch can /also/ be taken through the PHY tree and so act
> as a baseline for any Tegra PHY work.
> 
> As such, may I please request this patch be merged into a separate branch/tag
> in the PHY tree, which is then merged into both the Tegra and PHY trees.
> 
> Note that this affects an EHCI driver rather than a PHY driver, but per
> previous discussions, it sounds like the PHY branch will take any EHCI
> changes required by Tegra's PHY driver cleanup, and hence the PHY branch
> is the right place to merge this.
> 
> Thanks.
> ---
>  drivers/usb/host/ehci-tegra.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 6223d17..2de0890 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -28,7 +28,10 @@
>  #include 
>  
>  #include 
> -#include 
> +
> +#define TEGRA_USB_BASE   0xC500
> +#define TEGRA_USB2_BASE  0xC5004000
> +#define TEGRA_USB3_BASE  0xC5008000
>  
>  #define TEGRA_USB_DMA_ALIGN 32

You did make sure it still compiles, right?  :-)

Acked-by: 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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> > What's the reason for listing the generic value if drivers can't key
> > off it?  Does it contain any information that's not already present in 
> > the specific values?
> 
> This may or may not be a mistake.
> 
> The idea is that usb-ehci is included in the device node's compatible
> list iff the HW is 100% compatible with a "usb-ehci" HW device, and

But there is no such thing as a "usb-ehci" HW device.  That's part of 
the reason this whole discussion got started.

> hence a pure usb-ehci driver can handle the hardware without any
> additional knowledge. (That's true in general for any compatible value).

To as great an extent as is reasonable, ehci-platform is supposed to 
be that "pure usb-ehci driver".

> It's quite possible that this often gets translated to the subtly
> different "the HW is an EHCI controller, so it gets
> compatible="usb-ehci"" when writing .dts files.
> 
> So yes we probably should remove compatible="usb-ehci" from any device
> node for HW which really isn't a pure EHCI device. That would presumably
> require looking at the existing custom drivers and/or HW specs to
> determine what, if anything, is different about the HW.

Of course, removing it from the board files in the new kernel won't
have any effect on old board files embedded in firmware.  Those old 
firmwares then might not work with newer kernels.

> Related, at least on Tegra, the PHY handling is IIRC pretty tightly
> coupled into the EHCI driver. We probably should have separate nodes
> for, and drivers for, the PHYs instead. I don't know if that would
> remove all the non-standard attributes of the Tegra EHCI HW and hence
> allow Tegra's EHCI to be handled by the generic driver. From memory,
> there are certainly some HW workarounds in the Tegra EHCI driver that
> would need to be ported though.

In fact, ehci-tegra is probably the _most_ non-standard of the EHCI 
drivers.

> > It sounds like the ehci-platform driver should simply list all the
> > specific HW device types (or base HW device types) that it handles, and
> > it shouldn't include "usb-ehci" in the list.  As more DT board files
> > are created or as ehci-platform becomes capable to taking over from
> > other drivers, its device list would grow.
> 
> That's certainly a reasonable way to go too. I think the only downside
> with that approach is that every new device needs a kernel update to add
> it to the table, whereas using a generic compatible="usb-ehci" wouldn't,
> assuming no quirks were required for the new device.

New devices could list an older device they are upwardly compatible 
with.  Then no kernel update would be needed.  Now, I agree that a 
generic entry would be more logical, but there's no generic "standard" 
hardware for it to describe.

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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Mitch Bradley wrote:

> >> Ah, now I'm starting to get the picture.
> >>
> >> So by listing "usb-ehci" in its device ID table, a driver would
> >> essentially be saying that it can handle _all_ USB EHCI controllers.  
> 
> 
> Actually, it means that the driver can handle at least USB EHCI
> controllers that are 100% compatible with the EHCI spec (requiring
> nothing extra).  The driver might be able to handle almost-compatible
> controllers, possibly with the help of additional properties.

Unfortunately that's not saying very much.  The EHCI spec describes
only PCI-based controllers.  Every other sort does require something
extra.

> If a DT node lists "usb-ehci" as a "fallback", it's not guaranteed that
> a given version of that driver will work, but it's worth a try in the
> event that no more-specific driver is matched.

While I haven't checked in detail, it seems quite likely that this 
would not work with some of the devices that have "usb-ehci" listed in 
their compatible values.

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

[PATCH REPOST] usb: host: tegra remove include of

[Stephen Warren]

From: Stephen Warren 

Almost nothing from this file is used, and the file will hopefully be
deleted soon. Copy the tiny portions that are used directly into
ehci-tegra.c. I believe that Venu Byravarasu is working on cleaning up
our USB driver, and those cleanups will remove the need for these
constants.

Signed-off-by: Stephen Warren 
Acked-by: Venu Byravarasu 
---
Felipe,

This patch is needed in the Tegra branch, since it's a dependency for
cleanup required to support single zImage.

However, I suspect/hope there will be significant improvements to the Tegra
USB driver in 3.8, and this change may merge-conflict with those. It may be
simplest if this patch can /also/ be taken through the PHY tree and so act
as a baseline for any Tegra PHY work.

As such, may I please request this patch be merged into a separate branch/tag
in the PHY tree, which is then merged into both the Tegra and PHY trees.

Note that this affects an EHCI driver rather than a PHY driver, but per
previous discussions, it sounds like the PHY branch will take any EHCI
changes required by Tegra's PHY driver cleanup, and hence the PHY branch
is the right place to merge this.

Thanks.
---
 drivers/usb/host/ehci-tegra.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 6223d17..2de0890 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -28,7 +28,10 @@
 #include 
 
 #include 
-#include 
+
+#define TEGRA_USB_BASE 0xC500
+#define TEGRA_USB2_BASE0xC5004000
+#define TEGRA_USB3_BASE0xC5008000
 
 #define TEGRA_USB_DMA_ALIGN 32
 
-- 
1.7.0.4

--
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: qmi-wwan & cdc-wdm & cdc-ether interaction for Novatel devices

[Bjørn Mork]

Dan Williams  writes:
> (sorry, forgot to send to linux-usb@..., adding now)
>
> On Wed, 2012-10-24 at 13:25 -0500, Dan Williams wrote:
>> Hi Bjorn,
>> 
>> The Novatel USB 551L and E362 actually have cdc-ether interfaces which
>> speak QMI.  That's not really handled by our architecture at the moment.
>> I can get the cdc-wdm0 port by adding the ID for the master cdc-ether
>> interface (0x2, 0x6, 0x0) directly to the cdc-wdm driver, or by adding
>> them to qmi_wwan and blacklisting cdc-ether so it doesn't load first and
>> claim the interface.  Both of those options suck.  So what do we do
>> here?
>> 
>> But given they are cdc-ether, *and* you can use AT commands to start a
>> data session on the cdc-ether port without using QMI in any way, it
>> seems like cdc-ether is probably the right driver to use for the net
>> port.  However, they speak QMI on the net port interface with cdc-wdm
>> too.
>> 
>> So, should we:
>> 
>> 1) test if they work with qmi_wwan and then blacklist them from
>> cdc-ether?  Seems somewhat icky since now something that's actually
>> cdc-ether is no longer driven by cdc-ether.

I believe this option is best.  The vendors choice of descriptors does
not matter that much.  The fact that they choose to present these modems
as cdc_ether devices is most likely ignored by the Windows driver
anyway.  If we know that vid:pid is cdc_ether plus QMI, then lets just
point to the driver that handles that particular combo, i.e. qmi_wwan.

And I must point out that you already got the device specific entries in
cdc_ether to get the wwanX device names, so there is no clean class
match here in any case...

>> 2) figure out how to weld cdc-wdm into cdc-ether like we've done with
>> qmi_wwan

That would complicate cdc_ether and end up very similar to qmi_wwan.
The drivers are identical wrt handling network frames.  The only
differences are the probing and the cdc-wdm interface.  I do not think
we want to replicate the cdc-wdm stuff in cdc_ether just to have it
reuse the class probing.

So I vote for blacklisting these devices in cdc_ether and adding them to
qmi_wwan.  That is the absolutely simplest solution, requiring no driver
changes except for the two device ID tables.  Which makes it acceptable
for 3.7.  And it also makes a backport to 3.4 and 3.6 possible if that
is of interest to anyone.


Bjørn
--
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 00/17] USB: serial: fixes for v3.7

[Johan Hovold]

On Wed, Oct 24, 2012 at 11:43:02AM -0700, Greg KH wrote:
> On Tue, Oct 23, 2012 at 05:16:54PM +0200, Johan Hovold wrote:
> > Here are some more fixes against v3.7-rc2. The first five
> 
> Hm, I have three different series from you, at least, and some v2
> patches sprinkled in here, making it all very confusing as to what
> exactly I should be applying.
> 
> So, to make it easy on me, can you just resend all outstanding patches
> of yours that I have not yet applied?  I'll dump all of your patches
> that are in my to-apply queue right now, so we can get properly synced
> up.

No problem, I'll resend everything first thing tomorrow morning.

Johan
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Mitch Bradley]

On 10/24/2012 8:09 AM, Stephen Warren wrote:
> On 10/24/2012 11:46 AM, Alan Stern wrote:
>> On Wed, 24 Oct 2012, Stephen Warren wrote:
>>
 How do we determine which existing drivers claim to support usb-ehci?  
 A quick search under arch/ and drivers/ turns up nothing but
 drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
 match should be easy enough, and then "usb-ehci" would be safe to use
 in ehci-platform.c.
>>>
>>> It's not drivers that claim to support usb-ehci, but device tree files
>>> that contain nodes that include "usb-ehci" in their compatible value,
>>> yet need to be handled by a driver that isn't the generic USB EHCI
>>> driver, and that driver binds to the other more specific compatible value:
>>>
 $ grep -HrnI usb-ehci arch/*/boot/dts
> ...
>>> and then search for all those other compatible values in drivers. The
>>> ARM instances certainly all have this issue (although perhaps it's worth
>>> investigating if a generic could work for them instead). The PPC
>>> instances need more investigation; the values don't show up in of match
>>> tables but rather in code.
>>
>> Ah, now I'm starting to get the picture.
>>
>> So by listing "usb-ehci" in its device ID table, a driver would
>> essentially be saying that it can handle _all_ USB EHCI controllers.  


Actually, it means that the driver can handle at least USB EHCI
controllers that are 100% compatible with the EHCI spec (requiring
nothing extra).  The driver might be able to handle almost-compatible
controllers, possibly with the help of additional properties.

If a DT node lists "usb-ehci" as a "fallback", it's not guaranteed that
a given version of that driver will work, but it's worth a try in the
event that no more-specific driver is matched.


> 
> Yes, exactly.
> 
>> (Which means that the entry in ehci-ppc-of.c is questionable; perhaps
>> the intention is that this driver really does handle all EHCI
>> controllers on any PPC-based OpenFirmware platform bus.)
> 
> Yes, that seems questionable, although perhaps within the context of
> only enabling the driver on PPC specifically, it may be reasonable.
> 
>>> This doesn't continue forever though; you typically only list the
>>> specific HW model, the base specific model it's compatible with, and any
>>> generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
>>>
>>> compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
>>
>> What's the reason for listing the generic value if drivers can't key
>> off it?  Does it contain any information that's not already present in 
>> the specific values?
> 
> This may or may not be a mistake.
> 
> The idea is that usb-ehci is included in the device node's compatible
> list iff the HW is 100% compatible with a "usb-ehci" HW device, and
> hence a pure usb-ehci driver can handle the hardware without any
> additional knowledge. (That's true in general for any compatible value).
> 
> It's quite possible that this often gets translated to the subtly
> different "the HW is an EHCI controller, so it gets
> compatible="usb-ehci"" when writing .dts files.
> 
> So yes we probably should remove compatible="usb-ehci" from any device
> node for HW which really isn't a pure EHCI device. That would presumably
> require looking at the existing custom drivers and/or HW specs to
> determine what, if anything, is different about the HW.
> 
> Related, at least on Tegra, the PHY handling is IIRC pretty tightly
> coupled into the EHCI driver. We probably should have separate nodes
> for, and drivers for, the PHYs instead. I don't know if that would
> remove all the non-standard attributes of the Tegra EHCI HW and hence
> allow Tegra's EHCI to be handled by the generic driver. From memory,
> there are certainly some HW workarounds in the Tegra EHCI driver that
> would need to be ported though.
> 
>> It sounds like the ehci-platform driver should simply list all the
>> specific HW device types (or base HW device types) that it handles, and
>> it shouldn't include "usb-ehci" in the list.  As more DT board files
>> are created or as ehci-platform becomes capable to taking over from
>> other drivers, its device list would grow.
> 
> That's certainly a reasonable way to go too. I think the only downside
> with that approach is that every new device needs a kernel update to add
> it to the table, whereas using a generic compatible="usb-ehci" wouldn't,
> assuming no quirks were required for the new device.
> ___
> devicetree-discuss mailing list
> devicetree-disc...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 
--
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/4] USB: sierra: memory-leak fixes for v3.7

[Lennart Sorensen]

On Wed, Oct 24, 2012 at 07:24:15PM +0200, Johan Hovold wrote:
> This is an updated series of fixes rebased on the interface-data memory leak
> discovered by Lennart.
> 
> One of the reasons that I missed it was that the interface data was allocated
> in probe rather than attach, which in itself is a bug as the memory would not
> get released if usb-serial probe failes before attach returns successfully. 
> The
> third patch fixes this.
> 
> I also split out the fix for the port-data memory leak in the error path of
> attach that was part of my previous patch. This way the first three patches 
> can
> be backported to most (all?) stable trees, whereas the last patch applies to
> v3.6 (which contains the (in)famous 0998d063100 ("device-core: Ensure drvdata
> = NULL when no driver is bound" commit).
> 
> I included Lennart's patch with an updated commit message for completeness.
> 
> Lennart, if you care to test this series before it is applied that would be
> much appreciated.

I backported 1, 2 and 3 for the 3.0.23 kernel I am running, and so far it
looks to be working.  I think our USB HCD driver may have a leak still,
but someone else is investigating that right now.  At least the sierra
driver appears to have stopped leaking.

-- 
Len Sorensen
--
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/gadget: let file storage gadget select libcomposite

[Michal Nazarewicz]

> On Wed, Oct 24, 2012 at 07:30:42PM +0200, Michal Nazarewicz wrote:
>> At first it looks strange as FSG does not use composite, but yeah:

On Wed, Oct 24 2012, Sebastian Andrzej Siewior wrote:
> Yeah. However, it should be removed in v3.8 anyway :)

Yep, that's what feature-removal-schedule.txt says. :]

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--

pgpPzkO2Rd3Df.pgp
Description: PGP signature

Re: [PATCH 00/17] USB: serial: fixes for v3.7

[Greg KH]

On Tue, Oct 23, 2012 at 05:16:54PM +0200, Johan Hovold wrote:
> Here are some more fixes against v3.7-rc2. The first five

Hm, I have three different series from you, at least, and some v2
patches sprinkled in here, making it all very confusing as to what
exactly I should be applying.

So, to make it easy on me, can you just resend all outstanding patches
of yours that I have not yet applied?  I'll dump all of your patches
that are in my to-apply queue right now, so we can get properly synced
up.

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: qmi-wwan & cdc-wdm & cdc-ether interaction for Novatel devices

[Dan Williams]

(sorry, forgot to send to linux-usb@..., adding now)

On Wed, 2012-10-24 at 13:25 -0500, Dan Williams wrote:
> Hi Bjorn,
> 
> The Novatel USB 551L and E362 actually have cdc-ether interfaces which
> speak QMI.  That's not really handled by our architecture at the moment.
> I can get the cdc-wdm0 port by adding the ID for the master cdc-ether
> interface (0x2, 0x6, 0x0) directly to the cdc-wdm driver, or by adding
> them to qmi_wwan and blacklisting cdc-ether so it doesn't load first and
> claim the interface.  Both of those options suck.  So what do we do
> here?
> 
> But given they are cdc-ether, *and* you can use AT commands to start a
> data session on the cdc-ether port without using QMI in any way, it
> seems like cdc-ether is probably the right driver to use for the net
> port.  However, they speak QMI on the net port interface with cdc-wdm
> too.
> 
> So, should we:
> 
> 1) test if they work with qmi_wwan and then blacklist them from
> cdc-ether?  Seems somewhat icky since now something that's actually
> cdc-ether is no longer driven by cdc-ether.
> 
> 2) figure out how to weld cdc-wdm into cdc-ether like we've done with
> qmi_wwan
> 
> Thoughts?
> 
> Dan


--
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 RESEND 4/4] USB: sierra: fix port-data memory leak

[Johan Hovold]

Fix port-data memory leak by moving port data allocation and
deallocation to port_probe and port_remove.

Since commit 0998d0631001288 (device-core: Ensure drvdata = NULL when no
driver is bound) the port private data is no longer freed at release as
it is no longer accessible.

Note also that urb-count for multi-port interfaces has not been changed
even though the usb-serial port number is now determined from the port
and interface minor numbers.

Compile-only tested.

Cc: 
Signed-off-by: Johan Hovold 
---

Resending as this patch did not show up on linux-usb. Maybe because
git-send-email added "#v3.6" as CC?

Sorry about the noise,
Johan


 drivers/usb/serial/sierra.c | 125 
 1 file changed, 58 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index bb2ecaf..270860f 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -878,12 +878,7 @@ static void sierra_dtr_rts(struct usb_serial_port *port, 
int on)
 
 static int sierra_startup(struct usb_serial *serial)
 {
-   struct usb_serial_port *port;
struct sierra_intf_private *intfdata;
-   struct sierra_port_private *portdata;
-   struct sierra_iface_info *himemoryp = NULL;
-   int i;
-   u8 ifnum;
 
intfdata = kzalloc(sizeof(*intfdata), GFP_KERNEL);
if (!intfdata)
@@ -900,77 +895,71 @@ static int sierra_startup(struct usb_serial *serial)
if (nmea)
sierra_vsc_set_nmea(serial->dev, 1);
 
-   /* Now setup per port private data */
-   for (i = 0; i < serial->num_ports; i++) {
-   port = serial->port[i];
-   portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
-   if (!portdata) {
-   dev_dbg(&port->dev, "%s: kmalloc for "
-   "sierra_port_private (%d) failed!\n",
-   __func__, i);
-   goto err;
-   }
-   spin_lock_init(&portdata->lock);
-   init_usb_anchor(&portdata->active);
-   init_usb_anchor(&portdata->delayed);
-   ifnum = i;
-   /* Assume low memory requirements */
-   portdata->num_out_urbs = N_OUT_URB;
-   portdata->num_in_urbs  = N_IN_URB;
-
-   /* Determine actual memory requirements */
-   if (serial->num_ports == 1) {
-   /* Get interface number for composite device */
-   ifnum = sierra_calc_interface(serial);
-   himemoryp =
-   (struct sierra_iface_info *)&typeB_interface_list;
-   if (is_himemory(ifnum, himemoryp)) {
-   portdata->num_out_urbs = N_OUT_URB_HM;
-   portdata->num_in_urbs  = N_IN_URB_HM;
-   }
-   }
-   else {
-   himemoryp =
-   (struct sierra_iface_info *)&typeA_interface_list;
-   if (is_himemory(i, himemoryp)) {
-   portdata->num_out_urbs = N_OUT_URB_HM;
-   portdata->num_in_urbs  = N_IN_URB_HM;
-   }
-   }
-   dev_dbg(&serial->dev->dev,
-   "Memory usage (urbs) interface #%d, in=%d, out=%d\n",
-   ifnum,portdata->num_in_urbs, portdata->num_out_urbs );
-   /* Set the port private data pointer */
-   usb_set_serial_port_data(port, portdata);
+   return 0;
+}
+
+static void sierra_release(struct usb_serial *serial)
+{
+   struct sierra_intf_private *intfdata;
+
+   intfdata = usb_get_serial_data(serial);
+   kfree(intfdata);
+}
+
+static int sierra_port_probe(struct usb_serial_port *port)
+{
+   struct usb_serial *serial = port->serial;
+   struct sierra_port_private *portdata;
+   const struct sierra_iface_info *himemoryp;
+   u8 ifnum;
+
+   portdata = kzalloc(sizeof(*portdata), GFP_KERNEL);
+   if (!portdata)
+   return -ENOMEM;
+
+   spin_lock_init(&portdata->lock);
+   init_usb_anchor(&portdata->active);
+   init_usb_anchor(&portdata->delayed);
+
+   /* Assume low memory requirements */
+   portdata->num_out_urbs = N_OUT_URB;
+   portdata->num_in_urbs  = N_IN_URB;
+
+   /* Determine actual memory requirements */
+   if (serial->num_ports == 1) {
+   /* Get interface number for composite device */
+   ifnum = sierra_calc_interface(serial);
+   himemoryp = &typeB_interface_list;
+   } else {
+   /* This is really the usb-serial port number of the interface
+* rather than the interface number.
+*/
+   ifnum = port->number - serial->minor;
+   himemoryp = &typeA_interface_list;
   

Re: HDD spins up to slow for USB and/or Mass-Storage Driver

[Matthias Schniedermeyer]

On 24.10.2012 12:58, Alan Stern wrote:
> On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> 
> > > According to this, it should work if you do:
> > > 
> > >   echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> > > 
> > > Note that the "2-3" part may change from time to time.  Always use the 
> > > values the showed up in the log when the device was turned on.
> > 
> > This didn't work:
> > 
> > I tried it twice, with a few minutes wait in between:
> > - snip -
> > leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> > -bash: echo: write error: Invalid argument
> > leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> > -bash: echo: write error: Invalid argument
> > - snip -
> > 
> > The syslog-lines:
> > Oct 24 18:09:11 leeloo kernel: [930673.742358] usb 2-3: new SuperSpeed USB 
> > device number 13 using xhci_hcd
> > Oct 24 18:09:21 leeloo kernel: [930683.743447] usb 2-3: New USB device 
> > found, idVendor=174c, idProduct=5106
> > Oct 24 18:09:21 leeloo kernel: [930683.743452] usb 2-3: New USB device 
> > strings: Mfr=2, Product=3, SerialNumber=1
> > Oct 24 18:09:21 leeloo kernel: [930683.743454] usb 2-3: Product: AS2105
> > Oct 24 18:09:21 leeloo kernel: [930683.743456] usb 2-3: Manufacturer: 
> > ASMedia
> > Oct 24 18:09:26 leeloo kernel: [930688.738595] usb 2-3: can't set config 
> > #1, error -110
> > 
> > First-Try:
> > Oct 24 18:09:51 leeloo kernel: [930713.543126] xhci_hcd :00:14.0: 
> > Trying to add endpoint 0x81 without dropping it.
> 
> Okay, this looks like a bug in the xhci-hcd driver.  It may possibly
> have been fixed already in the 3.7-rc2 kernel; you could try out that
> version.
> 
> If that also gives the same error message about adding endpoint 0x81, 
> maybe Sarah can take a look at the problem.

I tried 3.7-rc2 (exactly, i think)

This isn't the same computer, but the same('oldconfig'ed) 
configuration. This computer has a NEC-chip for xhci.

uname -a:
Linux exp 3.7.0-rc2 #1 SMP Wed Oct 24 19:45:48 CEST 2012 x86_64 GNU/Linux

First:
Oct 24 20:17:52 localhost kernel: [  118.531900] usb 4-1: new SuperSpeed USB 
device number 2 using xhci_hcd
Oct 24 20:17:52 localhost kernel: [  118.546081] usb 4-1: Parent hub missing 
LPM exit latency info.  Power management will be impacted.
Oct 24 20:18:02 localhost kernel: [  128.518344] usb 4-1: New USB device found, 
idVendor=174c, idProduct=5106
Oct 24 20:18:02 localhost kernel: [  128.519551] usb 4-1: New USB device 
strings: Mfr=2, Product=3, SerialNumber=1
Oct 24 20:18:02 localhost kernel: [  128.520696] usb 4-1: Product: AS2105
Oct 24 20:18:02 localhost kernel: [  128.521828] usb 4-1: Manufacturer: ASMedia
Oct 24 20:18:07 localhost kernel: [  133.507705] usb 4-1: can't set config #1, 
error -110

Command:
echo 1 >/sys/bus/usb/devices/4-1/bConfigurationValue

Same response as before:
Oct 24 20:18:50 localhost kernel: [  176.642856] xhci_hcd :04:00.0: Trying 
to add endpoint 0x81 without dropping it.





Bis denn

-- 
Real Programmers consider "what you see is what you get" to be just as 
bad a concept in Text Editors as it is in women. No, the Real Programmer
wants a "you asked for it, you got it" text editor -- complicated, 
cryptic, powerful, unforgiving, dangerous.

--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Florian Fainelli]

On Wednesday 24 October 2012 14:04:12 Alan Stern wrote:
> On Wed, 24 Oct 2012, Florian Fainelli wrote:
> 
> > As long as no one enables both ehci-platform and ehci-ppc-of at the same 
> > time
> > there is no problem. ehci-ppc-of should be removed in favor of ehci-platform
> > and make sure that the specific quirk in ehci-ppc-of also gets ported, 
> > other 
> > that I see no issue using "usb-ehci" as the least detailed compatible 
> > property
> > name.
> 
> Suppose a DT board file is created for a oontroller which ehci-platform
> can't handle.  Then by your proposal, the board file shouldn't have
> "usb-ehci" in its compatible property.
> 
> Now later on, suppose ehci-platform is enhanced so that it can manage
> that controller.  It's too late to update the board file because the
> information has already been written to various firmwares.  The
> enhanced ehci-platform would have to include a special entry to match
> the controller.

In any case you are supposed to use a compatible property which describes
as much as possible your hardware, and this one should have the precedence
if a special treatment is required, so I see no problem with this approach.

> 
> Since this reasoning applies every time ehci-platform is updated, it 
> seems reasonable to use the same approach right from the beginning.
> 
> Alan Stern
> 
-- 
Florian
--
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/4] USB: sierra: memory-leak fixes for v3.7

[Dan Williams]

On Wed, 2012-10-24 at 19:24 +0200, Johan Hovold wrote:
> This is an updated series of fixes rebased on the interface-data memory leak
> discovered by Lennart.
> 
> One of the reasons that I missed it was that the interface data was allocated
> in probe rather than attach, which in itself is a bug as the memory would not
> get released if usb-serial probe failes before attach returns successfully. 
> The
> third patch fixes this.
> 
> I also split out the fix for the port-data memory leak in the error path of
> attach that was part of my previous patch. This way the first three patches 
> can
> be backported to most (all?) stable trees, whereas the last patch applies to
> v3.6 (which contains the (in)famous 0998d063100 ("device-core: Ensure drvdata
> = NULL when no driver is bound" commit).
> 
> I included Lennart's patch with an updated commit message for completeness.
> 
> Lennart, if you care to test this series before it is applied that would be
> much appreciated.

Are we missing a 4/4 patch?  I see 1, 2, and 3, but no 4, while the
title of patch #4 seems suspiciously like "[PATCH 13/17] USB: sierra:
fix port-data memory leaks" but is slightly different.

Dan

--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Stephen Warren]

On 10/24/2012 11:46 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>>> How do we determine which existing drivers claim to support usb-ehci?  
>>> A quick search under arch/ and drivers/ turns up nothing but
>>> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
>>> match should be easy enough, and then "usb-ehci" would be safe to use
>>> in ehci-platform.c.
>>
>> It's not drivers that claim to support usb-ehci, but device tree files
>> that contain nodes that include "usb-ehci" in their compatible value,
>> yet need to be handled by a driver that isn't the generic USB EHCI
>> driver, and that driver binds to the other more specific compatible value:
>>
>>> $ grep -HrnI usb-ehci arch/*/boot/dts
...
>> and then search for all those other compatible values in drivers. The
>> ARM instances certainly all have this issue (although perhaps it's worth
>> investigating if a generic could work for them instead). The PPC
>> instances need more investigation; the values don't show up in of match
>> tables but rather in code.
> 
> Ah, now I'm starting to get the picture.
> 
> So by listing "usb-ehci" in its device ID table, a driver would
> essentially be saying that it can handle _all_ USB EHCI controllers.  

Yes, exactly.

> (Which means that the entry in ehci-ppc-of.c is questionable; perhaps
> the intention is that this driver really does handle all EHCI
> controllers on any PPC-based OpenFirmware platform bus.)

Yes, that seems questionable, although perhaps within the context of
only enabling the driver on PPC specifically, it may be reasonable.

>> This doesn't continue forever though; you typically only list the
>> specific HW model, the base specific model it's compatible with, and any
>> generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
>>
>> compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> 
> What's the reason for listing the generic value if drivers can't key
> off it?  Does it contain any information that's not already present in 
> the specific values?

This may or may not be a mistake.

The idea is that usb-ehci is included in the device node's compatible
list iff the HW is 100% compatible with a "usb-ehci" HW device, and
hence a pure usb-ehci driver can handle the hardware without any
additional knowledge. (That's true in general for any compatible value).

It's quite possible that this often gets translated to the subtly
different "the HW is an EHCI controller, so it gets
compatible="usb-ehci"" when writing .dts files.

So yes we probably should remove compatible="usb-ehci" from any device
node for HW which really isn't a pure EHCI device. That would presumably
require looking at the existing custom drivers and/or HW specs to
determine what, if anything, is different about the HW.

Related, at least on Tegra, the PHY handling is IIRC pretty tightly
coupled into the EHCI driver. We probably should have separate nodes
for, and drivers for, the PHYs instead. I don't know if that would
remove all the non-standard attributes of the Tegra EHCI HW and hence
allow Tegra's EHCI to be handled by the generic driver. From memory,
there are certainly some HW workarounds in the Tegra EHCI driver that
would need to be ported though.

> It sounds like the ehci-platform driver should simply list all the
> specific HW device types (or base HW device types) that it handles, and
> it shouldn't include "usb-ehci" in the list.  As more DT board files
> are created or as ehci-platform becomes capable to taking over from
> other drivers, its device list would grow.

That's certainly a reasonable way to go too. I think the only downside
with that approach is that every new device needs a kernel update to add
it to the table, whereas using a generic compatible="usb-ehci" wouldn't,
assuming no quirks were required for the new device.
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Florian Fainelli wrote:

> As long as no one enables both ehci-platform and ehci-ppc-of at the same time
> there is no problem. ehci-ppc-of should be removed in favor of ehci-platform
> and make sure that the specific quirk in ehci-ppc-of also gets ported, other 
> that I see no issue using "usb-ehci" as the least detailed compatible property
> name.

Suppose a DT board file is created for a oontroller which ehci-platform
can't handle.  Then by your proposal, the board file shouldn't have
"usb-ehci" in its compatible property.

Now later on, suppose ehci-platform is enhanced so that it can manage
that controller.  It's too late to update the board file because the
information has already been written to various firmwares.  The
enhanced ehci-platform would have to include a special entry to match
the controller.

Since this reasoning applies every time ehci-platform is updated, it 
seems reasonable to use the same approach right from the beginning.

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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Rob Herring wrote:

> On 10/24/2012 11:44 AM, Alan Stern wrote:
> > On Wed, 24 Oct 2012, Stephen Warren wrote:
> > 
> >> We should absolutely avoid Linux-specific properties where possible.
> >>
> >> That said, what Linux-specific properties are you talking about? The
> >> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
> >> are all purely a description of HW, aren't they.
> > 
> > "has-tt" is definitely a description of the HW.
> 
> Can you spell out tt.

It stands for Transaction Translator.  The acronym is a standard one, 
used in the USB specs.

> > "has-synopsys-hc-bug" is too, although determining whether or not it 
> > should apply to a particular controller might be difficult.  I'm 
> > inclined not to include it among the properties.
> 
> What happens when there are 2 synopsys hc bugs? Something more specific
> about what the bug is would be better.

We will leave it out.

> > "no-io-watchdog" is not the greatest name.  It describes to controllers 
> > that always do generate IRQs for I/O events when they are supposed to 
> > (and hence the driver doesn't need to set up a watchdog timer to detect 
> > I/O completions that didn't generate an IRQ).  So while the concept is 
> > HW-specific, the name refers to a driver implementation issue.  A 
> > better name might be something like "reliable-IRQs".  Again, it's not 
> > such an easy thing to test for.  Almost all the existing drivers leave 
> > it unset.
> 
> Shouldn't the default be reliable irqs? What about "unreliable-irqs"?

I don't know why the default was set the way it was.  That was before 
my time as EHCI maintainer.  Right now only a few EHCI drivers claim to 
have reliable IRQs.

Avoiding the watchdog timer is a fairly minor optimization in any case.  
It fires only once every 100 ms, and only while I/O is in progress.

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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> > How do we determine which existing drivers claim to support usb-ehci?  
> > A quick search under arch/ and drivers/ turns up nothing but
> > drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> > match should be easy enough, and then "usb-ehci" would be safe to use
> > in ehci-platform.c.
> 
> It's not drivers that claim to support usb-ehci, but device tree files
> that contain nodes that include "usb-ehci" in their compatible value,
> yet need to be handled by a driver that isn't the generic USB EHCI
> driver, and that driver binds to the other more specific compatible value:
> 
> > $ grep -HrnI usb-ehci arch/*/boot/dts
> > arch/arm/boot/dts/spear3xx.dtsi:76: compatible = 
> > "st,spear600-ehci", "usb-ehci";
> > arch/arm/boot/dts/at91sam9x5.dtsi:399:  compatible = 
> > "atmel,at91sam9g45-ehci", "usb-ehci";
> > arch/arm/boot/dts/spear13xx.dtsi:145:   compatible = 
> > "st,spear600-ehci", "usb-ehci";
> > arch/arm/boot/dts/spear13xx.dtsi:152:   compatible = 
> > "st,spear600-ehci", "usb-ehci";
> > arch/arm/boot/dts/tegra20.dtsip:215:compatible = 
> > "nvidia,tegra20-ehci", "usb-ehci";
> > arch/arm/boot/dts/tegra20.dtsip:224:compatible = 
> > "nvidia,tegra20-ehci", "usb-ehci";
> > arch/arm/boot/dts/tegra20.dtsip:232:compatible = 
> > "nvidia,tegra20-ehci", "usb-ehci";
> > arch/arm/boot/dts/spear600.dtsi:88: compatible = 
> > "st,spear600-ehci", "usb-ehci";
> > arch/arm/boot/dts/spear600.dtsi:96: compatible = 
> > "st,spear600-ehci", "usb-ehci";
> > arch/arm/boot/dts/at91sam9g45.dtsi:392: compatible = 
> > "atmel,at91sam9g45-ehci", "usb-ehci";
> > arch/powerpc/boot/dts/sequoia.dts:156:  compatible = 
> > "ibm,usb-ehci-440epx", "usb-ehci";
> > arch/powerpc/boot/dts/wii.dts:120:  compatible = 
> > "nintendo,hollywood-usb-ehci",
> > arch/powerpc/boot/dts/wii.dts:121:  
> > "usb-ehci";
> > arch/powerpc/boot/dts/canyonlands.dts:167:  compatible = 
> > "ibm,usb-ehci-460ex", "usb-ehci";
> 
> and then search for all those other compatible values in drivers. The
> ARM instances certainly all have this issue (although perhaps it's worth
> investigating if a generic could work for them instead). The PPC
> instances need more investigation; the values don't show up in of match
> tables but rather in code.

Ah, now I'm starting to get the picture.

So by listing "usb-ehci" in its device ID table, a driver would
essentially be saying that it can handle _all_ USB EHCI controllers.  
(Which means that the entry in ehci-ppc-of.c is questionable; perhaps
the intention is that this driver really does handle all EHCI
controllers on any PPC-based OpenFirmware platform bus.)

> This doesn't continue forever though; you typically only list the
> specific HW model, the base specific model it's compatible with, and any
> generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
> 
> compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".

What's the reason for listing the generic value if drivers can't key
off it?  Does it contain any information that's not already present in 
the specific values?

It sounds like the ehci-platform driver should simply list all the
specific HW device types (or base HW device types) that it handles, and
it shouldn't include "usb-ehci" in the list.  As more DT board files
are created or as ehci-platform becomes capable to taking over from
other drivers, its device list would grow.

And it sounds like the only property we really need to add to the 
usb-ehci binding at the moment is "has-tt".

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] usb/gadget: let file storage gadget select libcomposite

[Sebastian Andrzej Siewior]

On Wed, Oct 24, 2012 at 07:30:42PM +0200, Michal Nazarewicz wrote:
> At first it looks strange as FSG does not use composite, but yeah:

Yeah. However, it should be removed in v3.8 anyway :)

> Acked-by: Michal Nazarewicz 

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver

[Rob Herring]

On 10/24/2012 11:44 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>> We should absolutely avoid Linux-specific properties where possible.
>>
>> That said, what Linux-specific properties are you talking about? The
>> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
>> are all purely a description of HW, aren't they.
> 
> "has-tt" is definitely a description of the HW.

Can you spell out tt.

> "has-synopsys-hc-bug" is too, although determining whether or not it 
> should apply to a particular controller might be difficult.  I'm 
> inclined not to include it among the properties.

What happens when there are 2 synopsys hc bugs? Something more specific
about what the bug is would be better.

> "no-io-watchdog" is not the greatest name.  It describes to controllers 
> that always do generate IRQs for I/O events when they are supposed to 
> (and hence the driver doesn't need to set up a watchdog timer to detect 
> I/O completions that didn't generate an IRQ).  So while the concept is 
> HW-specific, the name refers to a driver implementation issue.  A 
> better name might be something like "reliable-IRQs".  Again, it's not 
> such an easy thing to test for.  Almost all the existing drivers leave 
> it unset.

Shouldn't the default be reliable irqs? What about "unreliable-irqs"?

Rob

--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Florian Fainelli]

On Wednesday 24 October 2012 12:54:05 Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
> > Device tree files always need a completely specific value in the
> > compatible property, even when less-specific/more-generic values are
> > also present. So for example, the Linux driver might only care about the
> > following existing:
> > 
> > compatible = "usb-ehci".
> > 
> > However, any actual EHCI controller is always some specific model, so
> > the compatible value must define which specific model it is, e.g.:
> > 
> > compatible = "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > Now lets say the Tegra30 EHCI controller is identical to the Tegra20
> > controller. That needs to be explicitly represented too:
> > 
> > compatible = "nvidia,tegra30-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > In that case, the driver might still only declare support for
> > "nvidia,tegra20-ehci", but "nvidia,tegra30-ehci" is added to be explicit
> > about the actual HW model.
> > 
> > This doesn't continue forever though; you typically only list the
> > specific HW model, the base specific model it's compatible with, and any
> > generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
> > 
> > compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> > 
> > Given that, there is always something to key any newly required quirk
> > off, so even if the need for a quirk is found later, the device tree
> > already contains the information needed to trigger it.
> 
> Okay.  It appears that quite a few .dts/.dtsi files mention "usb-ehci", 
> for no apparent reason (given that the drivers don't list it):
> 
> [stern@iolanthe usb-3.6]$ find arch -name '*.dts*' | xargs grep usb-ehci
> arch/mips/cavium-octeon/octeon_3xxx.dts:
> compatible = "cavium,octeon-6335-ehci","usb-ehci";
> arch/mips/cavium-octeon/octeon_68xx.dts:
> compatible = "cavium,octeon-6335-ehci","usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
> "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
> "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
> "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:  compatible = 
> "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:   compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:   compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear3xx.dtsi:compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi: compatible = 
> "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:  compatible = 
> "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:  "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:  compatible = 
> "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:  compatible = 
> "ibm,usb-ehci-460ex", "usb-ehci";
> 
> Is there a real reason that I'm not aware of?  Or can all these entries
> safely be removed?

Apart from the powerpc entries for which a real driver exists, the others were
probably added in the hope that sooner or later, someone will come up with
a matching driver, and the corresponding dts file would not even have to be
updated to benefit from this.
--
Florian
--
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/gadget: let file storage gadget select libcomposite

[Michal Nazarewicz]

On Wed, Oct 24 2012, Sebastian Andrzej Siewior wrote:
> From: Sebastian Andrzej Siewior 
> Date: Wed, 24 Oct 2012 16:40:10 +0200
>
> found by randconfig, Randy Dunlap and Stephen Rothwell:
>
> |drivers/built-in.o: In function `fsg_setup':
> |file_storage.c:(.text+0x24db7c): undefined reference to 
> `usb_gadget_config_buf'
> |file_storage.c:(.text+0x24db98): undefined reference to 
> `usb_gadget_get_string'
> |drivers/built-in.o: In function `fsg_bind':
> |file_storage.c:(.init.text+0xee42): undefined reference to 
> `usb_ep_autoconfig_reset'
> |file_storage.c:(.init.text+0xee51): undefined reference to 
> `usb_ep_autoconfig'
> |file_storage.c:(.init.text+0xee73): undefined reference to 
> `usb_ep_autoconfig'
> |file_storage.c:(.init.text+0xee9e): undefined reference to 
> `usb_ep_autoconfig'
>
> Signed-off-by: Sebastian Andrzej Siewior 

At first it looks strange as FSG does not use composite, but yeah:

Acked-by: Michal Nazarewicz 

> ---
> for v3.7 please.
>
>  drivers/usb/gadget/Kconfig |1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index bbcec83..66bc3ab 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -737,6 +737,7 @@ config USB_FUNCTIONFS_GENERIC
>  
>  config USB_FILE_STORAGE
>   tristate "File-backed Storage Gadget (DEPRECATED)"
> + select USB_LIBCOMPOSITE
>   depends on BLOCK
>   help
> The File-backed Storage Gadget acts as a USB Mass Storage

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--

pgpVNjoje83FD.pgp
Description: PGP signature

[PATCH 3/4] USB: sierra: fix memory leak in probe error path

[Johan Hovold]

Move interface data allocation to attach so that it is deallocated on
errors in usb-serial probe.

Cc: 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/sierra.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 2cb27e4..bb2ecaf 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -161,7 +161,6 @@ static int sierra_probe(struct usb_serial *serial,
 {
int result = 0;
struct usb_device *udev;
-   struct sierra_intf_private *data;
u8 ifnum;
 
udev = serial->dev;
@@ -188,11 +187,6 @@ static int sierra_probe(struct usb_serial *serial,
return -ENODEV;
}
 
-   data = serial->private = kzalloc(sizeof(struct sierra_intf_private), 
GFP_KERNEL);
-   if (!data)
-   return -ENOMEM;
-   spin_lock_init(&data->susp_lock);
-
return result;
 }
 
@@ -885,11 +879,20 @@ static void sierra_dtr_rts(struct usb_serial_port *port, 
int on)
 static int sierra_startup(struct usb_serial *serial)
 {
struct usb_serial_port *port;
+   struct sierra_intf_private *intfdata;
struct sierra_port_private *portdata;
struct sierra_iface_info *himemoryp = NULL;
int i;
u8 ifnum;
 
+   intfdata = kzalloc(sizeof(*intfdata), GFP_KERNEL);
+   if (!intfdata)
+   return -ENOMEM;
+
+   spin_lock_init(&intfdata->susp_lock);
+
+   usb_set_serial_data(serial, intfdata);
+
/* Set Device mode to D0 */
sierra_set_power_state(serial->dev, 0x);
 
@@ -947,6 +950,7 @@ err:
portdata = usb_get_serial_port_data(serial->port[i]);
kfree(portdata);
}
+   kfree(intfdata);
 
return -ENOMEM;
 }
-- 
1.7.12.4

--
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/4] USB: sierra: fix memory leak at release

[Johan Hovold]

From: Lennart Sorensen 

Make sure interface private data is deallocated at release.

[jhov...@gmail.com: fix commit subject and message]
Signed-off-by: Len Sorensen 
Cc: 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/sierra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 01d882c..76ef95b 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -959,6 +959,7 @@ static void sierra_release(struct usb_serial *serial)
continue;
kfree(portdata);
}
+   kfree(serial->private);
 }
 
 #ifdef CONFIG_PM
-- 
1.7.12.4

--
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/4] USB: sierra: memory-leak fixes for v3.7

[Johan Hovold]

This is an updated series of fixes rebased on the interface-data memory leak
discovered by Lennart.

One of the reasons that I missed it was that the interface data was allocated
in probe rather than attach, which in itself is a bug as the memory would not
get released if usb-serial probe failes before attach returns successfully. The
third patch fixes this.

I also split out the fix for the port-data memory leak in the error path of
attach that was part of my previous patch. This way the first three patches can
be backported to most (all?) stable trees, whereas the last patch applies to
v3.6 (which contains the (in)famous 0998d063100 ("device-core: Ensure drvdata
= NULL when no driver is bound" commit).

I included Lennart's patch with an updated commit message for completeness.

Lennart, if you care to test this series before it is applied that would be
much appreciated.

Thanks,
Johan


Johan Hovold (3):
  USB: sierra: fix memory leak in attach error path
  USB: sierra: fix memory leak in probe error path
  USB: sierra: fix port-data memory leak

Lennart Sorensen (1):
  USB: sierra: fix memory leak at release

 drivers/usb/serial/sierra.c | 133 ++--
 1 file changed, 68 insertions(+), 65 deletions(-)

-- 
1.7.12.4

--
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/4] USB: sierra: fix memory leak in attach error path

[Johan Hovold]

Make sure port private data is deallocated on errors in attach.

Cc: 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/sierra.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 76ef95b..2cb27e4 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -905,7 +905,7 @@ static int sierra_startup(struct usb_serial *serial)
dev_dbg(&port->dev, "%s: kmalloc for "
"sierra_port_private (%d) failed!\n",
__func__, i);
-   return -ENOMEM;
+   goto err;
}
spin_lock_init(&portdata->lock);
init_usb_anchor(&portdata->active);
@@ -942,6 +942,13 @@ static int sierra_startup(struct usb_serial *serial)
}
 
return 0;
+err:
+   for (--i; i >= 0; --i) {
+   portdata = usb_get_serial_port_data(serial->port[i]);
+   kfree(portdata);
+   }
+
+   return -ENOMEM;
 }
 
 static void sierra_release(struct usb_serial *serial)
-- 
1.7.12.4

--
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: [balbi-usb:i2c-transferred-bytes-on-NACK 7/9] drivers/media/i2c/adv7604.c:406:9: warning: initialization makes integer from pointer without a cast

[Fengguang Wu]

On Wed, Oct 24, 2012 at 04:37:20PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Oct 24, 2012 at 09:23:23PM +0800, Fengguang Wu wrote:
> > Sorry, this should really be CCed to the media list.
> > I'll use the list recommended by get_maintainer.pl in future.
> 
> Actually, I would suggest only testing the following branches from my
> tree:
> 
> dwc3, musb, xceiv, gadget and fixes
> 
> Those are my final branches, everything else is a temporary branch that
> I'm using just so I don't loose some patches, or to make it easy for
> other guys to test patches.

Balbi, thanks for the instructions. I'll limit future tests to the
above branches.

Thanks,
Fengguang
--
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: Alan Stern , Greg Kroah-Hartman

[Felipe Balbi]

Hi,

On Wed, Oct 24, 2012 at 12:28:07PM -0400, Alan Stern wrote:
> On Wed, 24 Oct 2012, Felipe Balbi wrote:
> 
> > > We see that after making the below change in "ohci_irq", the device
> > > gets detected.
> > > ---
> > > --- a/drivers/usb/host/ohci-hcd.c
> > > +++ b/drivers/usb/host/ohci-hcd.c
> > > @@ -819,7 +819,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
> > >* to turn on RHSC along with RD.  But for remote wakeup events
> > >* this might not happen.
> > >*/
> > > - else if (ints & OHCI_INTR_RD) {
> > > + if (ints & OHCI_INTR_RD) {
> > > ---
> > > Even in the OHCI specification (Sec 5.3 page 80), it is recommended to
> > > check individual bits
> > > (RHSC and RD).
> > > 
> > > Can you please let us know if this change is valid and will not cause
> > > any regression?
> > 
> > This change makes sense to me. Unless we are dead sure those IRQ bits
> > are mutually exclusive.
> 
> They are not mutually exclusive.  That's the problem.  Basically it 
> comes down to this:
> 
>   if (OHCI_INTR_RHSC is set) {
>   do A;
>   do B;
>   } else if (OHCI_INTR_RD is set)
>   do B;
> 
> Since the bits aren't mutually exclusive, you can see that without the 
> "else" we would end up doing B twice.

aha, I see... should've read the code to see that detail.

-- 
balbi


signature.asc
Description: Digital signature

Re: HDD spins up to slow for USB and/or Mass-Storage Driver

[Alan Stern]

On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:

> > According to this, it should work if you do:
> > 
> > echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> > 
> > Note that the "2-3" part may change from time to time.  Always use the 
> > values the showed up in the log when the device was turned on.
> 
> This didn't work:
> 
> I tried it twice, with a few minutes wait in between:
> - snip -
> leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> -bash: echo: write error: Invalid argument
> leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> -bash: echo: write error: Invalid argument
> - snip -
> 
> The syslog-lines:
> Oct 24 18:09:11 leeloo kernel: [930673.742358] usb 2-3: new SuperSpeed USB 
> device number 13 using xhci_hcd
> Oct 24 18:09:21 leeloo kernel: [930683.743447] usb 2-3: New USB device found, 
> idVendor=174c, idProduct=5106
> Oct 24 18:09:21 leeloo kernel: [930683.743452] usb 2-3: New USB device 
> strings: Mfr=2, Product=3, SerialNumber=1
> Oct 24 18:09:21 leeloo kernel: [930683.743454] usb 2-3: Product: AS2105
> Oct 24 18:09:21 leeloo kernel: [930683.743456] usb 2-3: Manufacturer: ASMedia
> Oct 24 18:09:26 leeloo kernel: [930688.738595] usb 2-3: can't set config #1, 
> error -110
> 
> First-Try:
> Oct 24 18:09:51 leeloo kernel: [930713.543126] xhci_hcd :00:14.0: Trying 
> to add endpoint 0x81 without dropping it.

Okay, this looks like a bug in the xhci-hcd driver.  It may possibly
have been fixed already in the 3.7-rc2 kernel; you could try out that
version.

If that also gives the same error message about adding endpoint 0x81, 
maybe Sarah can take a look at the problem.

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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> Device tree files always need a completely specific value in the
> compatible property, even when less-specific/more-generic values are
> also present. So for example, the Linux driver might only care about the
> following existing:
> 
> compatible = "usb-ehci".
> 
> However, any actual EHCI controller is always some specific model, so
> the compatible value must define which specific model it is, e.g.:
> 
> compatible = "nvidia,tegra20-ehci", "usb-ehci".
> 
> Now lets say the Tegra30 EHCI controller is identical to the Tegra20
> controller. That needs to be explicitly represented too:
> 
> compatible = "nvidia,tegra30-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> 
> In that case, the driver might still only declare support for
> "nvidia,tegra20-ehci", but "nvidia,tegra30-ehci" is added to be explicit
> about the actual HW model.
> 
> This doesn't continue forever though; you typically only list the
> specific HW model, the base specific model it's compatible with, and any
> generic value needed. So, a hypothetical Tegra40 EHCI controller would be:
> 
> compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".
> 
> Given that, there is always something to key any newly required quirk
> off, so even if the need for a quirk is found later, the device tree
> already contains the information needed to trigger it.

Okay.  It appears that quite a few .dts/.dtsi files mention "usb-ehci", 
for no apparent reason (given that the drivers don't list it):

[stern@iolanthe usb-3.6]$ find arch -name '*.dts*' | xargs grep usb-ehci
arch/mips/cavium-octeon/octeon_3xxx.dts:
compatible = "cavium,octeon-6335-ehci","usb-ehci";
arch/mips/cavium-octeon/octeon_68xx.dts:
compatible = "cavium,octeon-6335-ehci","usb-ehci";
arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
"usb-ehci";
arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
"usb-ehci";
arch/arm/boot/dts/tegra20.dtsi: compatible = "nvidia,tegra20-ehci", 
"usb-ehci";
arch/arm/boot/dts/at91sam9x5.dtsi:  compatible = 
"atmel,at91sam9g45-ehci", "usb-ehci";
arch/arm/boot/dts/spear13xx.dtsi:   compatible = 
"st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear13xx.dtsi:   compatible = 
"st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear3xx.dtsi:compatible = 
"st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear600.dtsi:compatible = 
"st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/spear600.dtsi:compatible = 
"st,spear600-ehci", "usb-ehci";
arch/arm/boot/dts/at91sam9g45.dtsi: compatible = 
"atmel,at91sam9g45-ehci", "usb-ehci";
arch/powerpc/boot/dts/wii.dts:  compatible = 
"nintendo,hollywood-usb-ehci",
arch/powerpc/boot/dts/wii.dts:  "usb-ehci";
arch/powerpc/boot/dts/sequoia.dts:  compatible = 
"ibm,usb-ehci-440epx", "usb-ehci";
arch/powerpc/boot/dts/canyonlands.dts:  compatible = 
"ibm,usb-ehci-460ex", "usb-ehci";

Is there a real reason that I'm not aware of?  Or can all these entries
safely be removed?

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 with Roseweil eusb3 enclosure

[covici]

Sebastian Andrzej Siewior  wrote:

> On Wed, Oct 24, 2012 at 11:13:51AM -0400, cov...@ccs.covici.com wrote:
> > I cannot post dmesg output befcause it starts spewing messages before
> > any disk is mounted -- I have to shut down and reboot without the drive
> > plugged in.  But see a previous message where I posted some more
> > information in this thread.  But to answer your questions, the enclosure
> > has its own power and it does work with usb2 -- although I have not
> > booted with it plugged in in that state, so I will have to check.
> 
> So it does not get better if you unplugg the device later on? You could either
> try netconsole (Documentation/networking/netconsole.txt assuming you have
> already network) if this works or maybe even simpler, boot your own kernel and
> after you booted modprobe the xhci driver (and hope that it still behaves that
> way).

I will have to try that, but when this has happened in the past with
kernels like 3.4.0, unplugging and replugging the drive did not fix
anything.  I will have to see if the xhci module is in there at the
time.


-- 
Your life is like a penny.  You're going to lose it.  The question is:
How do
you spend it?

 John Covici
 cov...@ccs.covici.com
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Stephen Warren]

On 10/24/2012 10:44 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>> We should absolutely avoid Linux-specific properties where possible.
>>
>> That said, what Linux-specific properties are you talking about? The
>> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
>> are all purely a description of HW, aren't they.
> 
> "has-tt" is definitely a description of the HW.

OK.

> "has-synopsys-hc-bug" is too, although determining whether or not it 
> should apply to a particular controller might be difficult.  I'm 
> inclined not to include it among the properties.
> 
> "no-io-watchdog" is not the greatest name.  It describes to controllers 
> that always do generate IRQs for I/O events when they are supposed to 
> (and hence the driver doesn't need to set up a watchdog timer to detect 
> I/O completions that didn't generate an IRQ).  So while the concept is 
> HW-specific, the name refers to a driver implementation issue.  A 
> better name might be something like "reliable-IRQs".  Again, it's not 
> such an easy thing to test for.  Almost all the existing drivers leave 
> it unset.

OK, I'd be inclined to drive those last two by quirks then, since they
aren't architectural features of EHCI but rather implementation issues.
And indeed have the quirk table have a "reliable IRQs" field instead of
"no IO watchdog", to minimize the table size.
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Florian Fainelli]

On Wednesday 24 October 2012 12:38:42 Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
> > On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> > > On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> > >> Under the circumstances, do we really need a new binding document for 
> > >> the ehci-platform driver?
> > 
> > It seems reasonable to add the new properties to usb-ehci.txt, since
> > they do describe the HW.
> > 
> > >> We should be able to use the existing 
> > >> usb-ehci binding, perhaps with some new properties added:
> > >>
> > >>  has-synopsys-hc-bug
> > >>  no-io-watchdog
> > >>  has-tt
> > 
> > That sounds fine to me.
> > 
> > However, there is an implementation issue here. I believe the way Linux
> > searches for a driver for a particular node is:
> > 
> > for every driver that's registered
> > if the driver's supported compatible list matches the device
> > use the driver
> > 
> > (See drivers/base/platform.c:platform_match() which implements the if
> > statement above, and I assume the driver core implements the outer for
> > loop above)
> 
> Yes, it does.
> 
> > That means that if the generic driver supports compatible="usb-ehci", it
> > may "steal" device nodes that have
> > compatible="something-custom","usb-ehci", even if there's a driver
> > specifically for "something-custom". We would need to re-arrange the
> > driver matching code to:
> > 
> > for each compatible value in the node:
> > for each driver that's registered:
> > if the driver supports the compatible value:
> > use the driver.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

As long as no one enables both ehci-platform and ehci-ppc-of at the same time
there is no problem. ehci-ppc-of should be removed in favor of ehci-platform
and make sure that the specific quirk in ehci-ppc-of also gets ported, other 
that I see no issue using "usb-ehci" as the least detailed compatible property
name.
--
Florian
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Stephen Warren]

On 10/24/2012 10:38 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, Stephen Warren wrote:
> 
>> On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
>>> On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
 Under the circumstances, do we really need a new binding document for 
 the ehci-platform driver?
>>
>> It seems reasonable to add the new properties to usb-ehci.txt, since
>> they do describe the HW.
>>
 We should be able to use the existing 
 usb-ehci binding, perhaps with some new properties added:

has-synopsys-hc-bug
no-io-watchdog
has-tt
>>
>> That sounds fine to me.
>>
>> However, there is an implementation issue here. I believe the way Linux
>> searches for a driver for a particular node is:
>>
>> for every driver that's registered
>> if the driver's supported compatible list matches the device
>> use the driver
>>
>> (See drivers/base/platform.c:platform_match() which implements the if
>> statement above, and I assume the driver core implements the outer for
>> loop above)
> 
> Yes, it does.
> 
>> That means that if the generic driver supports compatible="usb-ehci", it
>> may "steal" device nodes that have
>> compatible="something-custom","usb-ehci", even if there's a driver
>> specifically for "something-custom". We would need to re-arrange the
>> driver matching code to:
>>
>> for each compatible value in the node:
>> for each driver that's registered:
>> if the driver supports the compatible value:
>> use the driver.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

It's not drivers that claim to support usb-ehci, but device tree files
that contain nodes that include "usb-ehci" in their compatible value,
yet need to be handled by a driver that isn't the generic USB EHCI
driver, and that driver binds to the other more specific compatible value:

> $ grep -HrnI usb-ehci arch/*/boot/dts
> arch/arm/boot/dts/spear3xx.dtsi:76:   compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:399:compatible = 
> "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:145: compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:152: compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:215:  compatible = 
> "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:224:  compatible = 
> "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:232:  compatible = 
> "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:88:   compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:96:   compatible = 
> "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi:392:   compatible = 
> "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:156:compatible = 
> "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:120:compatible = 
> "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:121:
> "usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:167:compatible = 
> "ibm,usb-ehci-460ex", "usb-ehci";

and then search for all those other compatible values in drivers. The
ARM instances certainly all have this issue (although perhaps it's worth
investigating if a generic could work for them instead). The PPC
instances need more investigation; the values don't show up in of match
tables but rather in code.
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> We should absolutely avoid Linux-specific properties where possible.
> 
> That said, what Linux-specific properties are you talking about? The
> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
> are all purely a description of HW, aren't they.

"has-tt" is definitely a description of the HW.

"has-synopsys-hc-bug" is too, although determining whether or not it 
should apply to a particular controller might be difficult.  I'm 
inclined not to include it among the properties.

"no-io-watchdog" is not the greatest name.  It describes to controllers 
that always do generate IRQs for I/O events when they are supposed to 
(and hence the driver doesn't need to set up a watchdog timer to detect 
I/O completions that didn't generate an IRQ).  So while the concept is 
HW-specific, the name refers to a driver implementation issue.  A 
better name might be something like "reliable-IRQs".  Again, it's not 
such an easy thing to test for.  Almost all the existing drivers leave 
it unset.

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: usbview 2.0 release

[Greg KH]

On Wed, Oct 24, 2012 at 10:05:48PM +0530, Anil Nair wrote:
> Hello Greg,
> 
> > Look at how the lsusb tool walks the list of all USB devices in the
> > system, using libusb, and port that code over to usbview, replacing the
> > big "walk and parse the devices file" functionality.
> >
> 
> You meant to say replace the functions calling or using debugfs by
> API's of libusb?

Yes.  But note, the debugfs "usage" is just reading a single file and
parsing the structure of that file and filling out an internal "tree" of
USB devices.  So moving that to use libusb should be pretty simple.

> Well i will try working on it, but i can't say a sure time when it
> will be completed. i know working of USB just need to go through
> libusb API's and some examples.

Again, look at the usbutils source code, it does most of this for you
already, and you can take the code from there since the license is the
same.

good luck,

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 v2 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Wed, 24 Oct 2012, Stephen Warren wrote:

> On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> > On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> >> Under the circumstances, do we really need a new binding document for 
> >> the ehci-platform driver?
> 
> It seems reasonable to add the new properties to usb-ehci.txt, since
> they do describe the HW.
> 
> >> We should be able to use the existing 
> >> usb-ehci binding, perhaps with some new properties added:
> >>
> >>has-synopsys-hc-bug
> >>no-io-watchdog
> >>has-tt
> 
> That sounds fine to me.
> 
> However, there is an implementation issue here. I believe the way Linux
> searches for a driver for a particular node is:
> 
> for every driver that's registered
> if the driver's supported compatible list matches the device
> use the driver
> 
> (See drivers/base/platform.c:platform_match() which implements the if
> statement above, and I assume the driver core implements the outer for
> loop above)

Yes, it does.

> That means that if the generic driver supports compatible="usb-ehci", it
> may "steal" device nodes that have
> compatible="something-custom","usb-ehci", even if there's a driver
> specifically for "something-custom". We would need to re-arrange the
> driver matching code to:
> 
> for each compatible value in the node:
> for each driver that's registered:
> if the driver supports the compatible value:
> use the driver.

Which might be difficult since the inner loop would be controlled by
the outer code in the driver core.

How do we determine which existing drivers claim to support usb-ehci?  
A quick search under arch/ and drivers/ turns up nothing but
drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
match should be easy enough, and then "usb-ehci" would be safe to use
in ehci-platform.c.

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 2/2] USB: doc: Binding document for ehci-platform driver

[Florian Fainelli]

On Wednesday 24 October 2012 10:16:31 Stephen Warren wrote:
> On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> > On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> >> Under the circumstances, do we really need a new binding document for 
> >> the ehci-platform driver?
> 
> It seems reasonable to add the new properties to usb-ehci.txt, since
> they do describe the HW.
> 
> >> We should be able to use the existing 
> >> usb-ehci binding, perhaps with some new properties added:
> >>
> >>has-synopsys-hc-bug
> >>no-io-watchdog
> >>has-tt
> 
> That sounds fine to me.
> 
> However, there is an implementation issue here. I believe the way Linux
> searches for a driver for a particular node is:
> 
> for every driver that's registered
> if the driver's supported compatible list matches the device
> use the driver
> 
> (See drivers/base/platform.c:platform_match() which implements the if
> statement above, and I assume the driver core implements the outer for
> loop above)
> 
> That means that if the generic driver supports compatible="usb-ehci", it
> may "steal" device nodes that have
> compatible="something-custom","usb-ehci", even if there's a driver
> specifically for "something-custom". We would need to re-arrange the
> driver matching code to:
> 
> for each compatible value in the node:
> for each driver that's registered:
> if the driver supports the compatible value:
> use the driver.
> 
> > On the PCI side we have VID, PID which is used for quirks. Sometimes we 
> > have a
> > revision ID which can be used to figure out if "this quirk" is still 
> > required.
> > The PCI driver probes by class so the ehci driver does not have a large 
> > table
> > of VID/PID for each controller out there. And the USB controller in two
> > different Intel boards has a different PID so a quirk, if required, could be
> > applied only to the specific mainboard.
> > 
> > Based on this we need atleast two compatible entries one "HW-Specific"
> > followed by a generic one (similar to PCI class).
> > Doing it the PCI way we would have to have table and figure out which
> > HW-specific compatible entry sets the TT flag and which one does the
> > no-io-watchdog. Having has-tt in compatible isn't right.
> 
> Yes, the driver could easily bind to anything compatible with
> "usb-ehci", then use the HW-specific compatible value to index into a
> quirk table in the same way that specific PCI IDs index into a quirk table.

Sounds good.

> 
> I agree that having separate compatible values like usb-ehci and
> usb-ehci-with-tt probably doesn't make sense here.
> 
> > I'm all with Alan here, to make a shortcut and allow Linux specific 
> > properties
> > which describe a specific quirk in less code lines. Other OS can just ignore
> > those and build their table based on the compatible entry if they want to.
> 
> We should absolutely avoid Linux-specific properties where possible.
> 
> That said, what Linux-specific properties are you talking about? The
> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
> are all purely a description of HW, aren't they.

has-tt and has-synopsys-hc-bug are certainly hardware properties, while
no-io-watchdog is a Linux driver software workaround for a hardware issue,
so that's kind of in a grey zone to decide whether this describes hardware or
not. Let's just assume that this is a hardware issue :)

> 
> > So usb-ehci should be fine. It is a generic USB-EHCI controller after all.
> > Quirks or no quirks, the register layout is the same, the functionality is 
> > the
> > same. If you can't map memory >4GiB then you need a quirk for this but you 
> > may
> > discover it way too late.
> > If your platform driver requires extra code for the PHY then it is still an
> > EHCI controller. The PHY wasn't setup by the bootloader / bios so Linux has 
> > to
> > deal with it.
> > 
> >> We probably can omit has-synopsys-hc-bug, as that is specific to one
> >> type of MIPS ATH79 controller.  The driver can check for it explicitly,
> >> if necessary.
> >>
> >> In fact, it's not clear that these new properties need to be added now.  
> >> They can be added over time as needed, as existing drivers are
> >> converted over to DT.  Or is it preferable to document these things
> >> now, preemptively as it were?
> 
> It's best to define the binding up-front so it doesn't churn, where
> possible. This will also ensure that multiple people don't try to update
> the binding document to add the same feature in different ways.

Agreed, we do support these properties in the non-DT case, so I see no reason
why we should not document them in the binding too.

> 
> > I would add it only if required / has users.
--
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: usbview 2.0 release

[Anil Nair]

Hello Greg,

> Look at how the lsusb tool walks the list of all USB devices in the
> system, using libusb, and port that code over to usbview, replacing the
> big "walk and parse the devices file" functionality.
>

You meant to say replace the functions calling or using debugfs by
API's of libusb?
Well i will try working on it, but i can't say a sure time when it
will be completed. i know working of USB just need to go through
libusb API's and some examples.


-- 
Regards,
Anil Nair
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Stephen Warren]

On 10/24/2012 08:57 AM, Alan Stern wrote:
> On Tue, 23 Oct 2012, Rob Herring wrote:
> 
>> On 10/23/2012 02:33 PM, Alan Stern wrote:
>>> On Tue, 23 Oct 2012, Stephen Warren wrote:
>>>
> Nothing intrinsically distinguishes this class of hardware.  The only
> thing these devices have in common is that they can be managed by
> Linux's ehci-platform driver.

 I don't agree. They're all EHCI USB controllers (or all EHCI USB
 controllers that don't require custom drivers due to quirks), which is a
 much more general statement than anything related to which driver Linux
 uses for them.
>>>
>>> That's true, but it doesn't distinguish these devices.  There are other
>>> EHCI controllers with no quirks that nevertheless can't be handled by
>>> ehci-platform because they have requirements (_not_ quirks) that
>>> ehci-platform is unable to deal with currently -- clocks or power
>>> supplies or special register settings or PHYs or etc.
>>
>> But what if the h/w has quirks you haven't yet discovered? You need the
>> compatible property to be specific now, so if there are any future
>> driver changes needed the DT does not have to change (as it may be in
>> firmware which you can't change).
> 
> That argument applies always, doesn't it?  There's always a chance that 
> you might discover a new quirk in a device for which the DT board file 
> has already been written and committed to firmware.  The board file 
> could declare that the device is compatible with something older, but 
> the new quirk might ruin that compatibility.

If the board file /only/ declares that the device is compatible with the
older/most-generic thing, that would be a bug in the .dts file. As a
general rule, the device tree should be fixed, although there may be
reasons to work around some buggy device trees in the kernel we should
avoid it if possible.

Device tree files always need a completely specific value in the
compatible property, even when less-specific/more-generic values are
also present. So for example, the Linux driver might only care about the
following existing:

compatible = "usb-ehci".

However, any actual EHCI controller is always some specific model, so
the compatible value must define which specific model it is, e.g.:

compatible = "nvidia,tegra20-ehci", "usb-ehci".

Now lets say the Tegra30 EHCI controller is identical to the Tegra20
controller. That needs to be explicitly represented too:

compatible = "nvidia,tegra30-ehci", "nvidia,tegra20-ehci", "usb-ehci".

In that case, the driver might still only declare support for
"nvidia,tegra20-ehci", but "nvidia,tegra30-ehci" is added to be explicit
about the actual HW model.

This doesn't continue forever though; you typically only list the
specific HW model, the base specific model it's compatible with, and any
generic value needed. So, a hypothetical Tegra40 EHCI controller would be:

compatible = "nvidia,tegra40-ehci", "nvidia,tegra20-ehci", "usb-ehci".

Given that, there is always something to key any newly required quirk
off, so even if the need for a quirk is found later, the device tree
already contains the information needed to trigger it.

This is why quirks should always be keyed off compatible values, since
they're guaranteed to be there in a correctly written device tree.

Generic HW features (such as has-tt) don't need to be derived from the
compatible value (although they could be), since they are something
that's known up-front and hence should be present in any non-buggy
device tree from day one. That's why the binding needs to thought out
ahead of time, or if necessary (e.g. if expanded to support both EHCI
and XHCI and XYZHCI which might require extra standard properties)
evolved in a backwards-compatible fashion.
--
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: Alan Stern , Greg Kroah-Hartman

[Alan Stern]

On Wed, 24 Oct 2012, Felipe Balbi wrote:

> > We see that after making the below change in "ohci_irq", the device
> > gets detected.
> > ---
> > --- a/drivers/usb/host/ohci-hcd.c
> > +++ b/drivers/usb/host/ohci-hcd.c
> > @@ -819,7 +819,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
> >  * to turn on RHSC along with RD.  But for remote wakeup events
> >  * this might not happen.
> >  */
> > -   else if (ints & OHCI_INTR_RD) {
> > +   if (ints & OHCI_INTR_RD) {
> > ---
> > Even in the OHCI specification (Sec 5.3 page 80), it is recommended to
> > check individual bits
> > (RHSC and RD).
> > 
> > Can you please let us know if this change is valid and will not cause
> > any regression?
> 
> This change makes sense to me. Unless we are dead sure those IRQ bits
> are mutually exclusive.

They are not mutually exclusive.  That's the problem.  Basically it 
comes down to this:

if (OHCI_INTR_RHSC is set) {
do A;
do B;
} else if (OHCI_INTR_RD is set)
do B;

Since the bits aren't mutually exclusive, you can see that without the 
"else" we would end up doing B twice.

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

Problem with OHCI on OMAP4430

[Alan Stern]

[Changed the Subject: line to something that makes more sense]

On Wed, 24 Oct 2012, Mohan V wrote:

> Hello All,
> 
> We are working on OMAP4430 based board which has EHCI and OHCI ports and
> are testing OHCI functionality. We are using kernel-3.4.
> 
> When the USB bus is in suspend, a device connected to OHCI port does
> not get detected, whereas the device connected to EHCI port is getting 
> detected.

Can you provide a dmesg log showing the OHCI problem with 
CONFIG_USB_DEBUG enabled?

> We are testing using the below commands:
> 
> echo enabled > /sys/bus/usb/devices/usb2/power/wakeup
> echo enabled > /sys/bus/usb/devices/2-2/power/wakeup

The wakeup settings are irrelevant here because you are not putting 
your system to sleep.

> echo auto > /sys/bus/usb/devices/usb2/power/control
> echo auto > /sys/bus/usb/devices/2-2/power/control

What is the 2-2 device?  Is it a hub?  If it is, do you plug the new 
device into the hub or directly into the OHCI controller?

> We see that after making the below change in "ohci_irq", the device
> gets detected.
> ---
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -819,7 +819,7 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
>* to turn on RHSC along with RD.  But for remote wakeup events
>* this might not happen.
>*/
> - else if (ints & OHCI_INTR_RD) {
> + if (ints & OHCI_INTR_RD) {
> ---
> Even in the OHCI specification (Sec 5.3 page 80), it is recommended to
> check individual bits
> (RHSC and RD).

The driver does check the individual bits.  (And are you sure you don't 
mean page 82?)

> Can you please let us know if this change is valid and will not cause
> any regression?

Yes, it is likely to cause a regression.  To understand why, you should 
read the original commit that added the code you want to change (commit 
583ceada075597a5b6acab1140d61ac81586a2a6, USB: OHCI: fix root-hub 
resume bug).  In short, your proposed change would cause the driver to 
try to resume the root hub twice.

More to the point, why doesn't the current code work?  It appears that 
the OHCI_INTR_RHCS bit is set, so the handler should call 
usb_hcd_poll_rh_status(), which in turn calls ohci_hub_status_data(), 
which calls ohci_root_hub_state_changes(), which should call 
usb_hcd_resume_root_hub().  If this isn't happening, maybe you can add 
some debugging statements to find out what's going wrong.

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: HDD spins up to slow for USB and/or Mass-Storage Driver

[Matthias Schniedermeyer]

On 24.10.2012 07:45, Greg KH wrote:
> On Wed, Oct 24, 2012 at 10:12:47AM -0400, Alan Stern wrote:
> > On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> > 
> > > Hi
> > > 
> > > 
> > > I have a 3TB WD HDD in an USB3 enclosure that fails to show up if i 
> > > connect the USB3-cable first and then switch on the enclosure. 
> > > (Example-Log see below)
> > > 
> > > If i either:
> > > - Wait a little before connecting the USB3-cable
> > > - Disconnect/Reconnect the USB3-cable
> > > - rmmod xhci; modprobe xhci
> > > The HDD shows up and works normally.
> > > 
> > > Question is, is there something that i can do that doesn't involve 
> > > 'rmmod'ing the xhci-driver or doing something physical?
> > > Is there a way to force a "rescan"?
> > > For e.g. is there something that i can do to a sysfs-file to force a 
> > > "rescan" of the USB-port?
> > > Or can i prolong the timeout, which appears to be 5 seconds.
> > 
> > The 5-second limit is part of the USB-2 specification (sections 9.2.6.1 
> > and 9.2.6.4).
> 
> And it can be increased with the delay_use module parameter to the
> usb_storage driver.  Matthias, try increasing that value to something
> much larger (20?) and see if that works for you.

First i tried the suggestion by Alan, which didn't work (for me).

This also didn't work (for me).
I switched off the HDD in between these 2 tests.

leeloo:~ # rmmod usb_storage
leeloo:~ # modprobe usb_storage delay_use=20

And the syslog-lines:
Oct 24 18:13:17 leeloo kernel: [930919.373921] usbcore: deregistering interface 
driver usb-storage
Oct 24 18:13:30 leeloo kernel: [930932.962571] Initializing USB Mass Storage 
driver...
Oct 24 18:13:30 leeloo kernel: [930932.962676] usbcore: registered new 
interface driver usb-storage
Oct 24 18:13:30 leeloo kernel: [930932.962676] USB Mass Storage support 
registered.
Oct 24 18:13:39 leeloo kernel: [930941.954879] usb 2-3: new SuperSpeed USB 
device number 15 using xhci_hcd
Oct 24 18:13:49 leeloo kernel: [930951.956044] usb 2-3: New USB device found, 
idVendor=174c, idProduct=5106
Oct 24 18:13:49 leeloo kernel: [930951.956048] usb 2-3: New USB device strings: 
Mfr=2, Product=3, SerialNumber=1
Oct 24 18:13:49 leeloo kernel: [930951.956051] usb 2-3: Product: AS2105
Oct 24 18:13:49 leeloo kernel: [930951.956053] usb 2-3: Manufacturer: ASMedia
Oct 24 18:13:54 leeloo kernel: [930956.951021] usb 2-3: can't set config #1, 
error -110


That delay_use has an effect can be seen when i dis/re-connect the 
cable:
Oct 24 18:21:06 leeloo kernel: [931388.451468] usb 2-3: USB disconnect, device 
number 15
Oct 24 18:21:10 leeloo kernel: [931392.587389] usb 1-3: new high-speed USB 
device number 4 using xhci_hcd
Oct 24 18:21:10 leeloo kernel: [931392.609296] usb 1-3: New USB device found, 
idVendor=174c, idProduct=5106
Oct 24 18:21:10 leeloo kernel: [931392.609301] usb 1-3: New USB device strings: 
Mfr=2, Product=3, SerialNumber=1
Oct 24 18:21:10 leeloo kernel: [931392.609303] usb 1-3: Product: AS2105
Oct 24 18:21:10 leeloo kernel: [931392.609306] usb 1-3: Manufacturer: ASMedia
Oct 24 18:21:10 leeloo kernel: [931392.609308] usb 1-3: SerialNumber: 
xxx
Oct 24 18:21:10 leeloo kernel: [931392.609477] usb 1-3: ep 0x81 - rounding 
interval to 32768 microframes, ep desc says 0 microframes
Oct 24 18:21:10 leeloo kernel: [931392.609479] usb 1-3: ep 0x2 - rounding 
interval to 32768 microframes, ep desc says 0 microframes
Oct 24 18:21:10 leeloo kernel: [931392.609784] scsi38 : usb-storage 1-3:1.0
Here is the 20 second delay:
Oct 24 18:21:30 leeloo kernel: [931412.665955] scsi 38:0:0:0: Direct-Access 
WDC WD30 EZRX-00MMMB0 80.0 PQ: 0 ANSI: 5
Oct 24 18:21:30 leeloo kernel: [931412.666168] sd 38:0:0:0: Attached scsi 
generic sg3 type 0
Oct 24 18:21:30 leeloo kernel: [931412.666328] sd 38:0:0:0: [sdd] Very big 
device. Trying to use READ CAPACITY(16).
Oct 24 18:21:30 leeloo kernel: [931412.666492] sd 38:0:0:0: [sdd] 5860533168 
512-byte logical blocks: (3.00 TB/2.72 TiB)
Oct 24 18:21:30 leeloo kernel: [931412.666791] sd 38:0:0:0: [sdd] Write Protect 
is off
Oct 24 18:21:30 leeloo kernel: [931412.666802] sd 38:0:0:0: [sdd] Mode Sense: 
23 00 00 00
Oct 24 18:21:30 leeloo kernel: [931412.667157] sd 38:0:0:0: [sdd] No Caching 
mode page present
Oct 24 18:21:30 leeloo kernel: [931412.667159] sd 38:0:0:0: [sdd] Assuming 
drive cache: write through
Oct 24 18:21:30 leeloo kernel: [931412.667427] sd 38:0:0:0: [sdd] Very big 
device. Trying to use READ CAPACITY(16).
Oct 24 18:21:30 leeloo kernel: [931412.668244] sd 38:0:0:0: [sdd] No Caching 
mode page present
Oct 24 18:21:30 leeloo kernel: [931412.668248] sd 38:0:0:0: [sdd] Assuming 
drive cache: write through
Oct 24 18:21:31 leeloo kernel: [931413.095380]  sdd: sdd1
Oct 24 18:21:31 leeloo kernel: [931413.095832] sd 38:0:0:0: [sdd] Very big 
device. Trying to use READ CAPACITY(16).
Oct 24 18:21:31 leeloo kernel: [931413.096924] sd 38:0:0:0: [sdd] No Caching 
mode page present
Oct 24 18:21:31 leeloo kernel: [931413.096929] sd 38:0:0:0: [sdd] As

Re: lockdep says circular locking since "tty: localise the lock"

[Sebastian Andrzej Siewior]

On Wed, Oct 24, 2012 at 11:13:18AM -0400, Alan Stern wrote:
> Are you pinging yourself?  That's what it looks like...  :-)
Hehe. It seems that it got the job done :)

> Alan Stern

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: problem with Roseweil eusb3 enclosure

[Sebastian Andrzej Siewior]

On Wed, Oct 24, 2012 at 11:13:51AM -0400, cov...@ccs.covici.com wrote:
> I cannot post dmesg output befcause it starts spewing messages before
> any disk is mounted -- I have to shut down and reboot without the drive
> plugged in.  But see a previous message where I posted some more
> information in this thread.  But to answer your questions, the enclosure
> has its own power and it does work with usb2 -- although I have not
> booted with it plugged in in that state, so I will have to check.

So it does not get better if you unplugg the device later on? You could either
try netconsole (Documentation/networking/netconsole.txt assuming you have
already network) if this works or maybe even simpler, boot your own kernel and
after you booted modprobe the xhci driver (and hope that it still behaves that
way).

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: HDD spins up to slow for USB and/or Mass-Storage Driver

[Matthias Schniedermeyer]

On 24.10.2012 10:12, Alan Stern wrote:
> On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> 
> > Hi
> > 
> > 
> > I have a 3TB WD HDD in an USB3 enclosure that fails to show up if i 
> > connect the USB3-cable first and then switch on the enclosure. 
> > (Example-Log see below)
> > 
> > If i either:
> > - Wait a little before connecting the USB3-cable
> > - Disconnect/Reconnect the USB3-cable
> > - rmmod xhci; modprobe xhci
> > The HDD shows up and works normally.
> > 
> > Question is, is there something that i can do that doesn't involve 
> > 'rmmod'ing the xhci-driver or doing something physical?
> > Is there a way to force a "rescan"?
> > For e.g. is there something that i can do to a sysfs-file to force a 
> > "rescan" of the USB-port?
> > Or can i prolong the timeout, which appears to be 5 seconds.
> 
> The 5-second limit is part of the USB-2 specification (sections 9.2.6.1 
> and 9.2.6.4).
> 
> > Kernel in this case is a vanilla 3.6.2.
> > xhci should be Intel but could also be from ASMedia (Z77-Chipset 
> > mainboard with 2 additional ASMedia chips soldered on)
> > If more information is needed, i will happily provide it.
> > 
> > 
> > First i connected the USB3-cable and then powered up the device:
> > Oct 23 20:25:02 leeloo kernel: [852502.177231] usb 2-3: new SuperSpeed USB 
> > device number 2 using xhci_hcd
> > Oct 23 20:25:12 leeloo kernel: [852512.178456] usb 2-3: New USB device 
> > found, idVendor=174c, idProduct=5106
> > Oct 23 20:25:12 leeloo kernel: [852512.178460] usb 2-3: New USB device 
> > strings: Mfr=2, Product=3, SerialNumber=1
> > Oct 23 20:25:12 leeloo kernel: [852512.178462] usb 2-3: Product: AS2105
> > Oct 23 20:25:12 leeloo kernel: [852512.178465] usb 2-3: Manufacturer: 
> > ASMedia
> > Oct 23 20:25:17 leeloo kernel: [852517.173409] usb 2-3: can't set config 
> > #1, error -110
> 
> According to this, it should work if you do:
> 
>   echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
> 
> Note that the "2-3" part may change from time to time.  Always use the 
> values the showed up in the log when the device was turned on.

This didn't work:

I tried it twice, with a few minutes wait in between:
- snip -
leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
-bash: echo: write error: Invalid argument
leeloo:~ # echo 1 >/sys/bus/usb/devices/2-3/bConfigurationValue
-bash: echo: write error: Invalid argument
- snip -

The syslog-lines:
Oct 24 18:09:11 leeloo kernel: [930673.742358] usb 2-3: new SuperSpeed USB 
device number 13 using xhci_hcd
Oct 24 18:09:21 leeloo kernel: [930683.743447] usb 2-3: New USB device found, 
idVendor=174c, idProduct=5106
Oct 24 18:09:21 leeloo kernel: [930683.743452] usb 2-3: New USB device strings: 
Mfr=2, Product=3, SerialNumber=1
Oct 24 18:09:21 leeloo kernel: [930683.743454] usb 2-3: Product: AS2105
Oct 24 18:09:21 leeloo kernel: [930683.743456] usb 2-3: Manufacturer: ASMedia
Oct 24 18:09:26 leeloo kernel: [930688.738595] usb 2-3: can't set config #1, 
error -110

First-Try:
Oct 24 18:09:51 leeloo kernel: [930713.543126] xhci_hcd :00:14.0: Trying to 
add endpoint 0x81 without dropping it.

Second-Try:
Oct 24 18:12:21 leeloo kernel: [930864.012372] xhci_hcd :00:14.0: Trying to 
add endpoint 0x81 without dropping it.








Bis denn

-- 
Real Programmers consider "what you see is what you get" to be just as 
bad a concept in Text Editors as it is in women. No, the Real Programmer
wants a "you asked for it, you got it" text editor -- complicated, 
cryptic, powerful, unforgiving, dangerous.

--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Stephen Warren]

On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
>> Under the circumstances, do we really need a new binding document for 
>> the ehci-platform driver?

It seems reasonable to add the new properties to usb-ehci.txt, since
they do describe the HW.

>> We should be able to use the existing 
>> usb-ehci binding, perhaps with some new properties added:
>>
>>  has-synopsys-hc-bug
>>  no-io-watchdog
>>  has-tt

That sounds fine to me.

However, there is an implementation issue here. I believe the way Linux
searches for a driver for a particular node is:

for every driver that's registered
if the driver's supported compatible list matches the device
use the driver

(See drivers/base/platform.c:platform_match() which implements the if
statement above, and I assume the driver core implements the outer for
loop above)

That means that if the generic driver supports compatible="usb-ehci", it
may "steal" device nodes that have
compatible="something-custom","usb-ehci", even if there's a driver
specifically for "something-custom". We would need to re-arrange the
driver matching code to:

for each compatible value in the node:
for each driver that's registered:
if the driver supports the compatible value:
use the driver.

> On the PCI side we have VID, PID which is used for quirks. Sometimes we have a
> revision ID which can be used to figure out if "this quirk" is still required.
> The PCI driver probes by class so the ehci driver does not have a large table
> of VID/PID for each controller out there. And the USB controller in two
> different Intel boards has a different PID so a quirk, if required, could be
> applied only to the specific mainboard.
> 
> Based on this we need atleast two compatible entries one "HW-Specific"
> followed by a generic one (similar to PCI class).
> Doing it the PCI way we would have to have table and figure out which
> HW-specific compatible entry sets the TT flag and which one does the
> no-io-watchdog. Having has-tt in compatible isn't right.

Yes, the driver could easily bind to anything compatible with
"usb-ehci", then use the HW-specific compatible value to index into a
quirk table in the same way that specific PCI IDs index into a quirk table.

I agree that having separate compatible values like usb-ehci and
usb-ehci-with-tt probably doesn't make sense here.

> I'm all with Alan here, to make a shortcut and allow Linux specific properties
> which describe a specific quirk in less code lines. Other OS can just ignore
> those and build their table based on the compatible entry if they want to.

We should absolutely avoid Linux-specific properties where possible.

That said, what Linux-specific properties are you talking about? The
properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
are all purely a description of HW, aren't they.

> So usb-ehci should be fine. It is a generic USB-EHCI controller after all.
> Quirks or no quirks, the register layout is the same, the functionality is the
> same. If you can't map memory >4GiB then you need a quirk for this but you may
> discover it way too late.
> If your platform driver requires extra code for the PHY then it is still an
> EHCI controller. The PHY wasn't setup by the bootloader / bios so Linux has to
> deal with it.
> 
>> We probably can omit has-synopsys-hc-bug, as that is specific to one
>> type of MIPS ATH79 controller.  The driver can check for it explicitly,
>> if necessary.
>>
>> In fact, it's not clear that these new properties need to be added now.  
>> They can be added over time as needed, as existing drivers are
>> converted over to DT.  Or is it preferable to document these things
>> now, preemptively as it were?

It's best to define the binding up-front so it doesn't churn, where
possible. This will also ensure that multiple people don't try to update
the binding document to add the same feature in different ways.

> I would add it only if required / has users.
--
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: Fix memory leak in sierra_release() (this time with signed-off-by)

[Lennart Sorensen]

On Wed, Oct 24, 2012 at 04:56:44PM +0200, Johan Hovold wrote:
> Good catch! I missed this one when I fixed a bunch of other memory
> leaks in the sierra with recent kernels:
> 
>   http://marc.info/?l=linux-usb&m=135100550421848&w=2
> 
> I'll rebase my patch on top of this one as your patch should be
> backported to all stable kernels, whereas mine is only required for
> v3.6 and later.

Oh that looks useful.  I was trying to track down if there were more
leaks in the driver.  I just fixed the one that was most obvious to me.

-- 
Len Sorensen
--
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: HDD spins up to slow for USB and/or Mass-Storage Driver

[Greg KH]

On Wed, Oct 24, 2012 at 11:12:18AM -0400, Alan Stern wrote:
> On Wed, 24 Oct 2012, Greg KH wrote:
> 
> > On Wed, Oct 24, 2012 at 10:12:47AM -0400, Alan Stern wrote:
> > > On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> > > 
> > > > Hi
> > > > 
> > > > 
> > > > I have a 3TB WD HDD in an USB3 enclosure that fails to show up if i 
> > > > connect the USB3-cable first and then switch on the enclosure. 
> > > > (Example-Log see below)
> > > > 
> > > > If i either:
> > > > - Wait a little before connecting the USB3-cable
> > > > - Disconnect/Reconnect the USB3-cable
> > > > - rmmod xhci; modprobe xhci
> > > > The HDD shows up and works normally.
> > > > 
> > > > Question is, is there something that i can do that doesn't involve 
> > > > 'rmmod'ing the xhci-driver or doing something physical?
> > > > Is there a way to force a "rescan"?
> > > > For e.g. is there something that i can do to a sysfs-file to force a 
> > > > "rescan" of the USB-port?
> > > > Or can i prolong the timeout, which appears to be 5 seconds.
> > > 
> > > The 5-second limit is part of the USB-2 specification (sections 9.2.6.1 
> > > and 9.2.6.4).
> > 
> > And it can be increased with the delay_use module parameter to the
> > usb_storage driver.
> 
> No, it can't.  That module parameter affects a different delay, in 
> usb-storage.  Matthias's problem is the delay in 
> usb_set_configuration().

Ah, my mistake, you are right, nevermind...

--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Sebastian Andrzej Siewior]

On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> Under the circumstances, do we really need a new binding document for 
> the ehci-platform driver?  We should be able to use the existing 
> usb-ehci binding, perhaps with some new properties added:
> 
>   has-synopsys-hc-bug
>   no-io-watchdog
>   has-tt

On the PCI side we have VID, PID which is used for quirks. Sometimes we have a
revision ID which can be used to figure out if "this quirk" is still required.
The PCI driver probes by class so the ehci driver does not have a large table
of VID/PID for each controller out there. And the USB controller in two
different Intel boards has a different PID so a quirk, if required, could be
applied only to the specific mainboard.

Based on this we need atleast two compatible entries one "HW-Specific"
followed by a generic one (similar to PCI class).
Doing it the PCI way we would have to have table and figure out which
HW-specific compatible entry sets the TT flag and which one does the
no-io-watchdog. Having has-tt in compatible isn't right.

I'm all with Alan here, to make a shortcut and allow Linux specific properties
which describe a specific quirk in less code lines. Other OS can just ignore
those and build their table based on the compatible entry if they want to.

So usb-ehci should be fine. It is a generic USB-EHCI controller after all.
Quirks or no quirks, the register layout is the same, the functionality is the
same. If you can't map memory >4GiB then you need a quirk for this but you may
discover it way too late.
If your platform driver requires extra code for the PHY then it is still an
EHCI controller. The PHY wasn't setup by the bootloader / bios so Linux has to
deal with it.

> We probably can omit has-synopsys-hc-bug, as that is specific to one
> type of MIPS ATH79 controller.  The driver can check for it explicitly,
> if necessary.
> 
> In fact, it's not clear that these new properties need to be added now.  
> They can be added over time as needed, as existing drivers are
> converted over to DT.  Or is it preferable to document these things
> now, preemptively as it were?

I would add it only if required / has users.

> Alan Stern

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/16] usb/gadget: uac2: provide a variable for interface and alt settings

[Jassi Brar]

On Tue, Oct 23, 2012 at 1:45 AM, Sebastian Andrzej Siewior
 wrote:
> This patch removes the shifting and masking of interface and its alt
> setting and provides its own variable.
> This looks better and is smaller:
>  textdata bss dec hex filename
> x86-32
>  6940 956  5679521f10 gadget/audio.o.old
>  6908 956  5679201ef0 gadget/audio.o.new
> arm
>  7914 956  56892622de gadget/audio.o.old
>  7886 956  56889822c2 gadget/audio.o.new
>
> Cc: jassisinghb...@gmail.com
> Signed-off-by: Sebastian Andrzej Siewior 

Acked-by: Jassi Brar 
--
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 15/16] usb/gadget: f_uac2: use the strings directly

[Jassi Brar]

On Tue, Oct 23, 2012 at 1:45 AM, Sebastian Andrzej Siewior
 wrote:
> There is no need for this variable in between.
>
> Cc: jassisinghb...@gmail.com
> Signed-off-by: Sebastian Andrzej Siewior 

Acked-by: Jassi Brar 
--
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 05/16] usb/gadget: add some error recovery in afunc_bind()

[Jassi Brar]

On Tue, Oct 23, 2012 at 1:44 AM, Sebastian Andrzej Siewior
 wrote:
> In case something goes wrong here, don't leak memory / endpoints.
>
> Cc: jassisinghb...@gmail.com
> Signed-off-by: Sebastian Andrzej Siewior 

Acked-by: Jassi Brar 
--
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 with Roseweil eusb3 enclosure

[covici]

I cannot post dmesg output befcause it starts spewing messages before
any disk is mounted -- I have to shut down and reboot without the drive
plugged in.  But see a previous message where I posted some more
information in this thread.  But to answer your questions, the enclosure
has its own power and it does work with usb2 -- although I have not
booted with it plugged in in that state, so I will have to check.


Sebastian Andrzej Siewior  wrote:

> On Mon, Oct 15, 2012 at 09:54:08AM -0400, cov...@ccs.covici.com wrote:
> > Hi.  I am using linux-3.6.2 kernel with gentoo patches and I have been
> > having problems with the usb3 enclosure made by Roseweil.  If I boot
> > with the drive plugged in, in this kernel, I cannot really boot because
> > it spits out continuous error messages -- something about code -71 maybe
> > cannot allocate address or something like that, right from the initrd
> > onwards.  In previous kernels such as 3.4.0, it didn't do anything
> > unless I unplugged and replugged in the drive.  Now if I have the drive
> > unplugged and wait till the system is booted, I can plug the drive in
> > and everything works OK.  I looked in the BIOS and it sees the drive as
> > a boot drive, but not a hard drive if the drive is plugged in.
> > 
> > Is there anything I can do to boot with the drive plugged in?
> 
> You could post the dmesg output which could give more hints what is going on.
> Is the drive powered over USB or does it have its own power supply? Is it 
> working
> well if you plug it into an USB2 port?
> 
> > Thanks in advance for any ideas.
> 
> Sebastian

-- 
Your life is like a penny.  You're going to lose it.  The question is:
How do
you spend it?

 John Covici
 cov...@ccs.covici.com
--
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: lockdep says circular locking since "tty: localise the lock"

[Alan Stern]

On Wed, 24 Oct 2012, Sebastian Andrzej Siewior wrote:

> On Wed, Oct 17, 2012 at 09:20:35PM +0200, Sebastian Andrzej Siewior wrote:
> > With dummy_hcd and g_nokia (that is CONFIG_USB_GADGET=m,
> > CONFIG_USB_DUMMY_HCD=m, CONFIG_USB_G_NOKIA=m) I see a lockdep complaing
> > about a "circular locking dependency" after executing
> 
> ping? Or is it an obvious false positive?
> 
> Sebastian

Are you pinging yourself?  That's what it looks like...  :-)

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: HDD spins up to slow for USB and/or Mass-Storage Driver

[Alan Stern]

On Wed, 24 Oct 2012, Greg KH wrote:

> On Wed, Oct 24, 2012 at 10:12:47AM -0400, Alan Stern wrote:
> > On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> > 
> > > Hi
> > > 
> > > 
> > > I have a 3TB WD HDD in an USB3 enclosure that fails to show up if i 
> > > connect the USB3-cable first and then switch on the enclosure. 
> > > (Example-Log see below)
> > > 
> > > If i either:
> > > - Wait a little before connecting the USB3-cable
> > > - Disconnect/Reconnect the USB3-cable
> > > - rmmod xhci; modprobe xhci
> > > The HDD shows up and works normally.
> > > 
> > > Question is, is there something that i can do that doesn't involve 
> > > 'rmmod'ing the xhci-driver or doing something physical?
> > > Is there a way to force a "rescan"?
> > > For e.g. is there something that i can do to a sysfs-file to force a 
> > > "rescan" of the USB-port?
> > > Or can i prolong the timeout, which appears to be 5 seconds.
> > 
> > The 5-second limit is part of the USB-2 specification (sections 9.2.6.1 
> > and 9.2.6.4).
> 
> And it can be increased with the delay_use module parameter to the
> usb_storage driver.

No, it can't.  That module parameter affects a different delay, in 
usb-storage.  Matthias's problem is the delay in 
usb_set_configuration().

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: Fix memory leak in sierra_release() (this time with signed-off-by)

[Johan Hovold]

On Wed, Oct 24, 2012 at 10:23:09AM -0400, Lennart Sorensen wrote:
> I found a memory leak in sierra_release() (well sierra_probe() I guess)
> that looses 8 bytes each time the driver releases a device.

Good catch! I missed this one when I fixed a bunch of other memory
leaks in the sierra with recent kernels:

http://marc.info/?l=linux-usb&m=135100550421848&w=2

I'll rebase my patch on top of this one as your patch should be
backported to all stable kernels, whereas mine is only required for
v3.6 and later.

> Signed-off-by: Len Sorensen 

Cc: 
Acked-by: Johan Hovold 

Thanks,
Johan


> diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
> index 01d882c..76ef95b 100644
> --- a/drivers/usb/serial/sierra.c
> +++ b/drivers/usb/serial/sierra.c
> @@ -959,6 +959,7 @@ static void sierra_release(struct usb_serial *serial)
>   continue;
>   kfree(portdata);
>   }
> + kfree(serial->private);
>  }
>  
>  #ifdef CONFIG_PM
--
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 2/2] USB: doc: Binding document for ehci-platform driver

[Alan Stern]

On Tue, 23 Oct 2012, Rob Herring wrote:

> On 10/23/2012 02:33 PM, Alan Stern wrote:
> > On Tue, 23 Oct 2012, Stephen Warren wrote:
> > 
> >>> Nothing intrinsically distinguishes this class of hardware.  The only
> >>> thing these devices have in common is that they can be managed by
> >>> Linux's ehci-platform driver.
> >>
> >> I don't agree. They're all EHCI USB controllers (or all EHCI USB
> >> controllers that don't require custom drivers due to quirks), which is a
> >> much more general statement than anything related to which driver Linux
> >> uses for them.
> > 
> > That's true, but it doesn't distinguish these devices.  There are other
> > EHCI controllers with no quirks that nevertheless can't be handled by
> > ehci-platform because they have requirements (_not_ quirks) that
> > ehci-platform is unable to deal with currently -- clocks or power
> > supplies or special register settings or PHYs or etc.
> 
> But what if the h/w has quirks you haven't yet discovered? You need the
> compatible property to be specific now, so if there are any future
> driver changes needed the DT does not have to change (as it may be in
> firmware which you can't change).

That argument applies always, doesn't it?  There's always a chance that 
you might discover a new quirk in a device for which the DT board file 
has already been written and committed to firmware.  The board file 
could declare that the device is compatible with something older, but 
the new quirk might ruin that compatibility.

> > Okay, fine.  But then what should the binding documentation specify for
> > the compatible value?  A list of all the HW models that the driver
> > currently supports?
> 
> The binding docs should be independent of the driver. There may be
> fields the driver ignores. So you need to document all defined
> compatible strings. It is fine if the driver only has "usb-ehci", but it
> is not fine for the DT to only have that.

Under the circumstances, do we really need a new binding document for 
the ehci-platform driver?  We should be able to use the existing 
usb-ehci binding, perhaps with some new properties added:

has-synopsys-hc-bug
no-io-watchdog
has-tt

We probably can omit has-synopsys-hc-bug, as that is specific to one
type of MIPS ATH79 controller.  The driver can check for it explicitly,
if necessary.

In fact, it's not clear that these new properties need to be added now.  
They can be added over time as needed, as existing drivers are
converted over to DT.  Or is it preferable to document these things
now, preemptively as it were?

The one that matters most and is most clearly a property of the HW is
"has-tt".  IMO it should be added right away, even though there may
already be DT board files that could specify it but don't.  Right now,
for example, the ehci-xilinx-of driver checks instead for the
"support-usb-fs" property, as defined in
Documentation/devicetree/bindings/xilinx.txt.

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: HDD spins up to slow for USB and/or Mass-Storage Driver

[Greg KH]

On Wed, Oct 24, 2012 at 10:12:47AM -0400, Alan Stern wrote:
> On Wed, 24 Oct 2012, Matthias Schniedermeyer wrote:
> 
> > Hi
> > 
> > 
> > I have a 3TB WD HDD in an USB3 enclosure that fails to show up if i 
> > connect the USB3-cable first and then switch on the enclosure. 
> > (Example-Log see below)
> > 
> > If i either:
> > - Wait a little before connecting the USB3-cable
> > - Disconnect/Reconnect the USB3-cable
> > - rmmod xhci; modprobe xhci
> > The HDD shows up and works normally.
> > 
> > Question is, is there something that i can do that doesn't involve 
> > 'rmmod'ing the xhci-driver or doing something physical?
> > Is there a way to force a "rescan"?
> > For e.g. is there something that i can do to a sysfs-file to force a 
> > "rescan" of the USB-port?
> > Or can i prolong the timeout, which appears to be 5 seconds.
> 
> The 5-second limit is part of the USB-2 specification (sections 9.2.6.1 
> and 9.2.6.4).

And it can be increased with the delay_use module parameter to the
usb_storage driver.  Matthias, try increasing that value to something
much larger (20?) and see if that works for you.

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

[PATCH] usb/gadget: let file storage gadget select libcomposite

[Sebastian Andrzej Siewior]

From: Sebastian Andrzej Siewior 
Date: Wed, 24 Oct 2012 16:40:10 +0200

found by randconfig, Randy Dunlap and Stephen Rothwell:

|drivers/built-in.o: In function `fsg_setup':
|file_storage.c:(.text+0x24db7c): undefined reference to `usb_gadget_config_buf'
|file_storage.c:(.text+0x24db98): undefined reference to `usb_gadget_get_string'
|drivers/built-in.o: In function `fsg_bind':
|file_storage.c:(.init.text+0xee42): undefined reference to 
`usb_ep_autoconfig_reset'
|file_storage.c:(.init.text+0xee51): undefined reference to `usb_ep_autoconfig'
|file_storage.c:(.init.text+0xee73): undefined reference to `usb_ep_autoconfig'
|file_storage.c:(.init.text+0xee9e): undefined reference to `usb_ep_autoconfig'

Signed-off-by: Sebastian Andrzej Siewior 
---
for v3.7 please.

 drivers/usb/gadget/Kconfig |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bbcec83..66bc3ab 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -737,6 +737,7 @@ config USB_FUNCTIONFS_GENERIC
 
 config USB_FILE_STORAGE
tristate "File-backed Storage Gadget (DEPRECATED)"
+   select USB_LIBCOMPOSITE
depends on BLOCK
help
  The File-backed Storage Gadget acts as a USB Mass Storage
-- 
1.7.10.4

--
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

Fix memory leak in sierra_release() (this time with signed-off-by)

[Lennart Sorensen]

I found a memory leak in sierra_release() (well sierra_probe() I guess)
that looses 8 bytes each time the driver releases a device.

Signed-off-by: Len Sorensen 

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 01d882c..76ef95b 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -959,6 +959,7 @@ static void sierra_release(struct usb_serial *serial)
continue;
kfree(portdata);
}
+   kfree(serial->private);
 }
 
 #ifdef CONFIG_PM

-- 
Len Sorensen
--
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