Re: qmi wwan interface does not work in this version 3.5.rc7.next.20120720

2012-07-24 Thread Bjørn Mork
Bjørn Mork  writes:
> Alan Stern  writes:
>
>> I suspect that answer is to make usb_wwan_disconnect a .port_remove
>> callback instead of a .disconnect callback.  By the time the disconnect
>> method runs, the ports have been unregistered and are basically gone.
>
> I have a hard time trying to figure out the logic in usb_wwan.
>
> There is the usb_wwan_release which is called from destroy_serial,
> attempting to access and free driver specific serial_port_data.
>
> And there is the usb_wwan_disconnect which is called from the USB driver
> disconnect, and will attempt to kill a number of urbs stored in the
> serial_port_data.
>
> This seems a bit backwards, and AFAICS both of these must expect to be
> called after the port is gone.  Should both be merged and converted to a
> .port_remove callback?  That does seem sanest, given that they operate
> on port data.

One might have guessed this, but the commit which make these Oopses
reliable is

 0998d0631 device-core: Ensure drvdata = NULL when no driver is bound


I did a bisect between 3.5 and next-20120723, and it pointed out this
one without doubt.

But I believe it only makes the underlying problem in usb_wwan obvious.
The use after free has been there forever, just using a pointer which
would happen to be valid most of the time.

So I guess that commit really is a very nice fix, flushing out things
like this.  usb_wwan certainly needs fixing.


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: g_serial and cdc_acm communications

2012-07-24 Thread Felipe Balbi
Hi,

On Mon, Jul 23, 2012 at 02:34:33PM +, Dipen Patel wrote:
> Can anyone help me with this? How my application should detect device
> plugout/plugin and SET_CONFIGURATION events? 
> 
> Is my current solution of generating input event on SET_CONFIGURATION call for
> application to close and reopen device node is Okay? 
> 
> Appreciate any help on this. 

your application shouldn't care if it's running on gserial or a real
UART port. You should stick to what a serial port gives you. In case of
gserial, you should always open the port and if the user detaches the
cable, you will receive a SIGHUP on your serial port.

This will also happen if the host changes configuration, btw.

-- 
balbi


signature.asc
Description: Digital signature


Re: Discussion about implementation of usb port power off mechanism for port with device

2012-07-24 Thread Lan Tianyu

On 2012年07月23日 23:27, Alan Stern wrote:

On Mon, 23 Jul 2012, Lan Tianyu wrote:


@@ -3027,6 +3070,24 @@ int usb_port_resume(struct usb_device *u
int status;
u16 portchange, portstatus;

+   if (hub->ports[port1 - 1]->port_power_policy == USB_PORT_POWER_AUTO
+   &&  hub->ports[port1 - 1]->power_state == 
USB_PORT_POWER_STATE_OFF) {
+   set_port_feature(udev->parent, port1, USB_PORT_FEAT_POWER);
+
+   /*
+* Wait for usb hub port to be reconnected in order to make
+* the resume procedure successful.
+*/
+   status = usb_port_wait_for_connected(hub, port1);
+   if (status<  0) {
+   dev_dbg(&udev->dev, "can't get reconnection after setting  
port " \
+   "power on, status %d\n", status);
+   return status;
+   }
+   hub->ports[port1 - 1]->power_state = USB_PORT_POWER_STATE_ON;
+   pr_info("%s: port%d connect state on %ld\n", __func__, port1, 
jiffies);
+   }
+
/* Skip the initial Clear-Suspend step for a remote wakeup */
status = hub_port_status(hub, port1,&portstatus,&portchange);
if (status == 0&&  !port_is_suspended(hub, portstatus))


A few lines later the driver does:

set_bit(port1, hub->busy_bits);

You merely need to move this line up before the point where you turn
port power back on.  Make it the first executable line of the function.


I test and it works. Thanks




Alan Stern



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


[RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu
The quirk USB_QUIRK_RESET_MORPHS means the device can't be reset
and reset-resume. So persist_enabled also should be set simultaneously
with USB_QUIRK_RESET_MORPHS flag. But Currently in the attribute
avoid_reset_quirk store callback just set USB_QUIRK_RESET_MORPHS.
The persist_enabled will only be updated in the usb_detect_quirks()
due to USB_QUIRK_RESET_MORPHS. usb_detect_quirks() is only called when
a device is enumerated. So after a device enumerated, persist_enabled will
not be changed when attribute avoid_reset_quirk is changed. This patch
is to change persist_enabled when atrribute avoid_reset_quirk is changed.

Signed-off-by: Lan Tianyu 
---
 drivers/usb/core/sysfs.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 682e825..12b4bf9 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
device_attribute *attr,
if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)
return -EINVAL;
usb_lock_device(udev);
-   if (config)
+   if (config) {
udev->quirks |= USB_QUIRK_RESET_MORPHS;
-   else
+   udev->persist_enabled = 0;
+   } else {
udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
+   udev->persist_enabled = 1;
+   }
usb_unlock_device(udev);
return count;
 }
-- 
1.7.6.rc2.8.g28eb

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


Re: [PATCH 2/2] xhci: EHCI/XHCI ports switching on Intense-PC.

2012-07-24 Thread Denis Turischev
On 07/23/2012 08:44 PM, Sarah Sharp wrote:
> Did you try disabling the wake on bits in the port status registers?  I
> would prefer that solution if it works.
No, I didn't. Why we need to disable wake on capabilities?
> 
> Otherwise this patchset looks fine.  I'll work with the hardware folks
> and see if this is exposed on other boards.  Also, this is not a BIOS
> bug, but a hardware issue that can be worked around in BIOS.  So I might
> edit your description on the first patch a bit when I submit it to Greg.
Of course, please edit it if required.
> 
> Sarah Sharp
> 
> On Mon, Jul 23, 2012 at 02:46:32PM +0300, Denis Turischev wrote:
>> Intense-PC is Compulab's mini-desktop with Intel Panther Point
>> chipset.
>>
>> Unconditional switchover to xHCI provided by function
>> usb_enable_xhci_ports() leads to surprising results, after shutdown
>> system powered-on again after a few seconds. On Windows power
>> related problems were not observed. Switching back to EHCI solves
>> the problem.
>>
>> The patch switches usb ports back to EHCI during xhci shutdown
>> for Intense-PC.
>>
>> Signed-off-by: Denis Turischev 
>> ---
>>  drivers/usb/host/xhci.c |   14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index a979cd0..541efaa 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -26,6 +26,8 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include "pci-quirks.h"
>>
>>  #include "xhci.h"
>>
>> @@ -656,8 +658,20 @@ void xhci_stop(struct usb_hcd *hcd)
>>   */
>>  void xhci_shutdown(struct usb_hcd *hcd)
>>  {
>> +struct pci_dev *pdev;
>> +const char *brd_name;
>>  struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>
>> +pdev = to_pci_dev(hcd->self.controller);
>> +
>> +if (usb_is_intel_switchable_xhci(pdev)) {
>> +brd_name = dmi_get_system_info(DMI_BOARD_NAME);
>> +
>> +/* quirk for Compulab's Intense-PC board */
>> +if (brd_name && strstr(brd_name, "Intense-PC"))
>> +usb_disable_xhci_ports(pdev);
>> +}
>> +
>>  spin_lock_irq(&xhci->lock);
>>  xhci_halt(xhci);
>>  spin_unlock_irq(&xhci->lock);
>> -- 
>> 1.7.9.5
>>

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


Re: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Oliver Neukum
On Tuesday 24 July 2012 16:39:37 Lan Tianyu wrote:
> The quirk USB_QUIRK_RESET_MORPHS means the device can't be reset
> and reset-resume. So persist_enabled also should be set simultaneously

Set? Why bother? If anything reset_resume() is hopeless, so why
delay the resume by trying?

Regards
Oliver

--
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: controlling record light on camera

2012-07-24 Thread Laurent Pinchart
Hi Adam,

On Monday 23 July 2012 16:26:16 Adam Wozniak wrote:
> I'm trying to control the "recording" light on a Logitech C270 webcam
> from a linux userspace app.
> 
> There is a windows app that does this, so I know it is possible.
> 
> Running the app in a VM, and looking at usbmon output, the following
> lines get printed when I turn the light on and off:
> 
> 8802adc33c80 1951017902 S Co:1:004:0 s 21 01 0900 0700 0005 5 =
> 0101 0a
> 8802adc33c80 1952527826 S Co:1:004:0 s 21 01 0900 0700 0005 5 =
> 0100 0a
> 
> so I know this is a "control output" message with "setup" values
> bmRequestType=21, bRequest=01, wValue=0900, wIndex=0700, wLength=0005
> and data of 01,0N,00,00,0a where N=1 for on and N=0 for off
> 
> How would I send this same packet from a userspace application?

Short answer: don't :-)

The right way to control the LED is to map it as a V4L2 control, and then use 
any V4L2 utility to modify the control value.

The uvcvideo driver supports creating UVC -> V4L2 control mappings at runtime. 
You will need the uvcdynctrl tool and the Logitech control mapping XML 
description file. The uvcdynctrl package for your distribution should include 
both, as well as a udev rule to create mappings automatically when the camera 
is plugged in.

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


Re: g_serial and cdc_acm communications

2012-07-24 Thread Dipen Patel


Hi Felipe,

In my device application I am not receiving SIGHUP when HOST sends
SET_CONFIGURATION or my device is replugged into HOST. 
Signals are another mechanism for application to know that either 
device is reattached or SET_CONFIGURATION is called by HOST apart from 
my custom  solution of generating input event from g_serial driver.

What i don't understand is that why device application need to reset 
port which is already open. i.e. when set_alt() is called why it is 
calling gserial_connect() again instead of using previously opened tty 
port.


Regards,
Dipen Patel

--
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: [RFC PATCH for v3.5 0/2] usb: gadget: at91_udc: fix oops regression

2012-07-24 Thread Fabio Porcedda
On Mon, Jul 23, 2012 at 12:53 PM, Felipe Balbi  wrote:
> On Fri, Jul 20, 2012 at 10:33:24AM +0200, Fabio Porcedda wrote:
>> On Fri, Jul 20, 2012 at 12:50 AM, Greg Kroah-Hartman
>>  wrote:
>> > On Thu, Jul 19, 2012 at 07:16:54PM +0200, Sebastian Andrzej Siewior wrote:
>> >> On Thu, Jul 19, 2012 at 03:50:54PM +0200, Fabio Porcedda wrote:
>> >> > > I would prefer to fix the bug causing the oops instead of reverting 
>> >> > > patches.
>> >> >
>> >> > Me too, it's just that i don't have much time to work on that, and so
>> >> > I'm not confident to be able to fix the kernel panic oops in time for
>> >> > the v3.5 release.
>> >> Yes. If nobody looks into this then the v3.5 kernel will be released and 
>> >> other
>> >> kernels will follow.
>> >> Would you now please test it and say that it works with those two patches 
>> >> I
>> >> just posted?
>>
>> The patches fix  both issues, now the driver works again!
>
> please then resend with all acks and a Cc: sta...@vger.kernel.org so I
> can queue it when -rc1 is tagged.

Sebastian had already resent the patches with my Tested-by tag added,
and the tag: Cc: sta...@kernel.org

please read these emails:

bug fixes for at91_udc, introduced in v3.5 release cycle
http://marc.info/?l=linux-usb&m=134280952400955&w=2

[PATCH 1/2] usb/at91udc: don't overwrite driver data
http://marc.info/?l=linux-usb&m=134280952700957&w=2

[PATCH 2/2] usb/at91udc: Don't check for ep->ep.desc
http://marc.info/?l=linux-usb&m=134280952700958&w=2

P.S.
I'm sorry for the previous email, they where rejected for the
erroneous html contents.

Best regards
-- 
Fabio Porcedda
Software Developer - R&D Cagliari
Telit Communications S.p.A.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] usb: gadget: composite: parse dt values

2012-07-24 Thread Alexandre Pereira da Silva
Grab the devicetree node properties to set VendorId, ProductId,
bcdDevice, Manucacturer, Product and SerialNumber

Signed-off-by: Alexandre Pereira da Silva 
Acked-by: Michal Nazarewicz 
Acked-by: Rob Herring 
---
Applies to v3.5

Changes since V1:
* Minor patch description and code comments updates

 Documentation/devicetree/bindings/usb/gadget.txt |   20 +++
 drivers/usb/gadget/composite.c   |   39 ++
 2 files changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/gadget.txt

diff --git a/Documentation/devicetree/bindings/usb/gadget.txt 
b/Documentation/devicetree/bindings/usb/gadget.txt
new file mode 100644
index 000..93388d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/gadget.txt
@@ -0,0 +1,20 @@
+Usb Gadget DeviceTree bindings
+
+These optional properties inside the usb device controller node are used to
+change some of the gadget drivers configuration:
+- vendor-id: Usb vendor id
+- product-id: Usb product id
+- release: Version of this device
+- vendor: Textual description of the vendor
+- device: Textual description of this device
+- serial: Textual representation of the device's serial number
+
+Binding Example:
+   usbd@3102 {
+   vendor-id = <0x0525>;
+   product-id = <0xa4a6>;
+   release = <1>;
+   vendor = "Some Corp";
+   device = "Test Device";
+   serial = "12345";
+   };
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 390749b..a02be8c 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1419,10 +1420,44 @@ static u8 override_id(struct usb_composite_dev *cdev, 
u8 *desc)
return *desc;
 }
 
+static void composite_parse_dt(struct usb_composite_dev *cdev,
+   struct device_node *np)
+{
+   u32 reg;
+
+   if (!idVendor && of_property_read_u32(np, "vendor-id", ®) == 0)
+   idVendor = reg;
+
+   if (!idProduct && of_property_read_u32(np, "product-id", ®) == 0)
+   idProduct = reg;
+
+   if (!bcdDevice && of_property_read_u32(np, "release", ®) == 0)
+   bcdDevice = reg;
+
+   if (!iManufacturer)
+   if (of_property_read_string(np, "vendor",
+   &composite->iManufacturer) == 0)
+   cdev->manufacturer_override = override_id(cdev,
+   &cdev->desc.iManufacturer);
+
+   if (!iProduct)
+   if (of_property_read_string(np, "device",
+   &composite->iProduct) == 0)
+   cdev->product_override = override_id(cdev,
+   &cdev->desc.iProduct);
+
+   if (!iSerialNumber)
+   if (of_property_read_string(np, "serial",
+   &composite->iSerialNumber) == 0)
+   cdev->serial_override = override_id(cdev,
+   &cdev->desc.iSerialNumber);
+}
+
 static int composite_bind(struct usb_gadget *gadget)
 {
struct usb_composite_dev*cdev;
int status = -ENOMEM;
+   struct device_node  *np = gadget->dev.of_node;
 
cdev = kzalloc(sizeof *cdev, GFP_KERNEL);
if (!cdev)
@@ -1470,6 +1505,10 @@ static int composite_bind(struct usb_gadget *gadget)
 
cdev->desc = *composite->dev;
 
+   /* grab values from devicetree */
+   if (np)
+   composite_parse_dt(cdev, np);
+
/* standardized runtime overrides for device ID data */
if (idVendor)
cdev->desc.idVendor = cpu_to_le16(idVendor);
-- 
1.7.10

--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu

于 2012/7/24 16:57, Oliver Neukum 写道:

On Tuesday 24 July 2012 16:39:37 Lan Tianyu wrote:

The quirk USB_QUIRK_RESET_MORPHS means the device can't be reset
and reset-resume. So persist_enabled also should be set simultaneously


Set? Why bother? If anything reset_resume() is hopeless, so why
delay the resume by trying?

hi Oliver:

	Maybe my change log confuse you.I mean the persist_enabled should be 
changed when USB_QUIRK_RESET_MORPHS is changed, right?


When USB_QUIRK_RESET_MORPHS is set, persist_enabled should be set to 0 
to prevent reset-resume() when the device resumes.


Current only in the usb_detect_quirks(),  persist_enabled will
be set to 0 depending on whether the dev's flag USB_QUIRK_RESET_MORPHS 
is set or not. And usb_detect_quirks() is only called in the 
hub_port_connect_change() when a new device is found.


So after a device being enumerated, Changing attribute avod_reset_quirk 
will not set persist_enabled to 0 to prevent reset-resume.


If something wrong, please help me to correct. Thanks.




Regards
Oliver

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



--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Oliver Neukum
On Tuesday 24 July 2012 21:28:30 Lan Tianyu wrote:
> Current only in the usb_detect_quirks(),  persist_enabled will
> be set to 0 depending on whether the dev's flag USB_QUIRK_RESET_MORPHS 
> is set or not. And usb_detect_quirks() is only called in the 
> hub_port_connect_change() when a new device is found.
> 
> So after a device being enumerated, Changing attribute avod_reset_quirk 
> will not set persist_enabled to 0 to prevent reset-resume.

Hi,

your description is correct, however the initial changelog suggested the 
oposite change.

Regards
Oliver

--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu

于 2012/7/24 21:35, Oliver Neukum 写道:

On Tuesday 24 July 2012 21:28:30 Lan Tianyu wrote:

Current only in the usb_detect_quirks(),  persist_enabled will
be set to 0 depending on whether the dev's flag USB_QUIRK_RESET_MORPHS
is set or not. And usb_detect_quirks() is only called in the
hub_port_connect_change() when a new device is found.

So after a device being enumerated, Changing attribute avod_reset_quirk
will not set persist_enabled to 0 to prevent reset-resume.


Hi,

your description is correct, however the initial changelog suggested the 
oposite change.



Ok. I will improve the changelog later. So you agree my patch?

Regards
Oliver



--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Oliver Neukum
On Tuesday 24 July 2012 21:48:37 Lan Tianyu wrote:
> 于 2012/7/24 21:35, Oliver Neukum 写道:
> > On Tuesday 24 July 2012 21:28:30 Lan Tianyu wrote:
> >> Current only in the usb_detect_quirks(),  persist_enabled will
> >> be set to 0 depending on whether the dev's flag USB_QUIRK_RESET_MORPHS
> >> is set or not. And usb_detect_quirks() is only called in the
> >> hub_port_connect_change() when a new device is found.
> >>
> >> So after a device being enumerated, Changing attribute avod_reset_quirk
> >> will not set persist_enabled to 0 to prevent reset-resume.
> >
> > Hi,
> >
> > your description is correct, however the initial changelog suggested the 
> > oposite change.
> >
> 
> Ok. I will improve the changelog later. So you agree my patch?

Yes, the patch is good.

Regards
Oliver

--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Alan Stern
On Tue, 24 Jul 2012, Lan Tianyu wrote:

> The quirk USB_QUIRK_RESET_MORPHS means the device can't be reset
> and reset-resume. So persist_enabled also should be set simultaneously
> with USB_QUIRK_RESET_MORPHS flag. But Currently in the attribute
> avoid_reset_quirk store callback just set USB_QUIRK_RESET_MORPHS.
> The persist_enabled will only be updated in the usb_detect_quirks()
> due to USB_QUIRK_RESET_MORPHS. usb_detect_quirks() is only called when
> a device is enumerated. So after a device enumerated, persist_enabled will
> not be changed when attribute avoid_reset_quirk is changed. This patch
> is to change persist_enabled when atrribute avoid_reset_quirk is changed.

As Oliver noted, this description is terribly confusing.

> Signed-off-by: Lan Tianyu 
> ---
>  drivers/usb/core/sysfs.c |7 +--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> index 682e825..12b4bf9 100644
> --- a/drivers/usb/core/sysfs.c
> +++ b/drivers/usb/core/sysfs.c
> @@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
> device_attribute *attr,
>   if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)

Please do me a favor: Rename the "config" variable to something else,
such as "morphs" or "val".  In USB, the word "config" already has a
separate meaning.

>   return -EINVAL;
>   usb_lock_device(udev);
> - if (config)
> + if (config) {
>   udev->quirks |= USB_QUIRK_RESET_MORPHS;
> - else
> + udev->persist_enabled = 0;
> + } else {
>   udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
> + udev->persist_enabled = 1;

This line doesn't seem right.  What if the user _wants_ persist_enabled 
to be 0?

> + }
>   usb_unlock_device(udev);
>   return count;
>  }

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: g_serial and cdc_acm communications

2012-07-24 Thread Dipen Patel
Hi Felipe,

I think I know why my application is not receiving SIGHUP. 

When HOST sends SET_CONFIGURATION the set_config() in composite.c calls
acm_set_alt(). Now acm_set_alt() calls gserial_connect() and it will 
create new port. But it never called gserial_disconnect() to close 
previously called gserial_connect(), so SIGHUP is not generated. 

But why acm_set_alt() does not do gserial_disconnect() before calling
gserial_connect()?

Regards,
Dipen Patel




--
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 interface does not work in this version 3.5.rc7.next.20120720

2012-07-24 Thread Alan Stern
On Tue, 24 Jul 2012, Bjørn Mork wrote:

> Bjørn Mork  writes:
> > Alan Stern  writes:
> >
> >> I suspect that answer is to make usb_wwan_disconnect a .port_remove
> >> callback instead of a .disconnect callback.  By the time the disconnect
> >> method runs, the ports have been unregistered and are basically gone.
> >
> > I have a hard time trying to figure out the logic in usb_wwan.
> >
> > There is the usb_wwan_release which is called from destroy_serial,
> > attempting to access and free driver specific serial_port_data.
> >
> > And there is the usb_wwan_disconnect which is called from the USB driver
> > disconnect, and will attempt to kill a number of urbs stored in the
> > serial_port_data.
> >
> > This seems a bit backwards, and AFAICS both of these must expect to be
> > called after the port is gone.  Should both be merged and converted to a
> > .port_remove callback?  That does seem sanest, given that they operate
> > on port data.

In general it's not that simple.  Even though the port may be gone
(i.e., the logical abstraction of a physical port has been destroyed),
the port's data structure may still exist.  The data structure won't be
deallocated until all the references to it are dropped.

Therefore it's reasonable for a driver to access a device's data
structure after the device has been deregistered, provided the driver
is very careful about not doing anything too extensive.  For example,
the data structure may now be under the control of a second driver, so
the first driver should avoid doing anything that would interfere with
the second driver's operation.

On the other hand, if the necessary actions can be carried out in the
port_remove callback, it will be better and simpler to do so.  Since
the usb-serial core does not invoke any callbacks when the port
structures are deallocated in usb-serial.c:port_release(), in this case
you don't have a choice anyway.

In fact, I would say that the three loops in stop_read_write_urbs() and 
usb_wwan_release() could be combined into a single loop that should run 
in the port_remove callback.

> One might have guessed this, but the commit which make these Oopses
> reliable is
> 
>  0998d0631 device-core: Ensure drvdata = NULL when no driver is bound

Yes, that's what I expected.

> I did a bisect between 3.5 and next-20120723, and it pointed out this
> one without doubt.
> 
> But I believe it only makes the underlying problem in usb_wwan obvious.
> The use after free has been there forever, just using a pointer which
> would happen to be valid most of the time.
> 
> So I guess that commit really is a very nice fix, flushing out things
> like this.  usb_wwan certainly needs fixing.

Agreed.  And its users.

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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu

于 2012/7/24 22:21, Alan Stern 写道:

On Tue, 24 Jul 2012, Lan Tianyu wrote:

@@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
device_attribute *attr,
if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)


Please do me a favor: Rename the "config" variable to something else,
such as "morphs" or "val".  In USB, the word "config" already has a
separate meaning.

Ok. I will do it. A seperate patch?



return -EINVAL;
usb_lock_device(udev);
-   if (config)
+   if (config) {
udev->quirks |= USB_QUIRK_RESET_MORPHS;
-   else
+   udev->persist_enabled = 0;
+   } else {
udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
+   udev->persist_enabled = 1;


This line doesn't seem right.  What if the user _wants_ persist_enabled
to be 0?

This seems attribute avoid_reset_quirk is conflicted with persist.
From my opinion, avoid_reset_quirk may have high priority.
So we can add a USB_QUIRK_RESET_MORPHS check in the attribute persist
store callback, if USB_QUIRK_RESET_MORPHS flag had been set, persist 
couldn't be changed and remain 0 since this patch set persist to 0 when

USB_QUIRK_RESET_MORPHS flag is set. Do this make sense?





+   }
usb_unlock_device(udev);
return count;
  }


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



--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Alan Stern
On Tue, 24 Jul 2012, Lan Tianyu wrote:

> 于 2012/7/24 22:21, Alan Stern 写道:
> > On Tue, 24 Jul 2012, Lan Tianyu wrote:
> >> @@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
> >> device_attribute *attr,
> >>if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)
> >
> > Please do me a favor: Rename the "config" variable to something else,
> > such as "morphs" or "val".  In USB, the word "config" already has a
> > separate meaning.
> Ok. I will do it. A seperate patch?

Up to you; I don't care.

> >
> >>return -EINVAL;
> >>usb_lock_device(udev);
> >> -  if (config)
> >> +  if (config) {
> >>udev->quirks |= USB_QUIRK_RESET_MORPHS;
> >> -  else
> >> +  udev->persist_enabled = 0;
> >> +  } else {
> >>udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
> >> +  udev->persist_enabled = 1;
> >
> > This line doesn't seem right.  What if the user _wants_ persist_enabled
> > to be 0?
> This seems attribute avoid_reset_quirk is conflicted with persist.
>  From my opinion, avoid_reset_quirk may have high priority.
> So we can add a USB_QUIRK_RESET_MORPHS check in the attribute persist
> store callback, if USB_QUIRK_RESET_MORPHS flag had been set, persist 
> couldn't be changed and remain 0 since this patch set persist to 0 when
> USB_QUIRK_RESET_MORPHS flag is set. Do this make sense?

Yes, good idea.

When you do this, you should also add another little paragraph to 
Documentation/usb/persist.txt.  At the end of the "What is the 
solution?" section, explain why the persist attribute cannot be set if 
the avoid_reset_quirk attribute is on.

Hmmm, what about hubs?  They don't have a persist attribute because 
persist is _always_ supposed to be enabled for a hub.  Maybe this means 
they shouldn't have an avoid_reset_quirk attribute either.

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 2/2] drivers/mmc/host/vub300.c: add missing usb_free_urb

2012-07-24 Thread Julia Lawall
From: Julia Lawall 

Add missing usb_free_urb on failure path after usb_alloc_urb.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
@km exists@
local idexpression e;
expression e1,e2,e3;
type T,T1;
identifier f;
@@

* e = usb_alloc_urb(...)
... when any
when != e = e1
when != e1 = (T)e
when != e1(...,(T)e,...)
when != &e->f
if(...) { ... when != e2(...,(T1)e,...)
 when != e3 = e
 when forall
(
 return <+...e...+>;
|
* return ...;
) }
// 

Signed-off-by: Julia Lawall 

---
 drivers/mmc/host/vub300.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 3135a1a..6c39bf1 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2358,9 +2358,9 @@ error5:
 * which is contained at the end of struct mmc
 */
 error4:
-   usb_free_urb(command_out_urb);
-error1:
usb_free_urb(command_res_urb);
+error1:
+   usb_free_urb(command_out_urb);
 error0:
return retval;
 }

--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu

于 2012/7/24 22:59, Alan Stern 写道:

On Tue, 24 Jul 2012, Lan Tianyu wrote:


于 2012/7/24 22:21, Alan Stern 写道:

On Tue, 24 Jul 2012, Lan Tianyu wrote:

@@ -209,10 +209,13 @@ set_avoid_reset_quirk(struct device *dev, struct 
device_attribute *attr,
if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)


Please do me a favor: Rename the "config" variable to something else,
such as "morphs" or "val".  In USB, the word "config" already has a
separate meaning.

Ok. I will do it. A seperate patch?


Up to you; I don't care.




return -EINVAL;
usb_lock_device(udev);
-   if (config)
+   if (config) {
udev->quirks |= USB_QUIRK_RESET_MORPHS;
-   else
+   udev->persist_enabled = 0;
+   } else {
udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
+   udev->persist_enabled = 1;


This line doesn't seem right.  What if the user _wants_ persist_enabled
to be 0?

This seems attribute avoid_reset_quirk is conflicted with persist.
  From my opinion, avoid_reset_quirk may have high priority.
So we can add a USB_QUIRK_RESET_MORPHS check in the attribute persist
store callback, if USB_QUIRK_RESET_MORPHS flag had been set, persist
couldn't be changed and remain 0 since this patch set persist to 0 when
USB_QUIRK_RESET_MORPHS flag is set. Do this make sense?


Yes, good idea.

When you do this, you should also add another little paragraph to
Documentation/usb/persist.txt.  At the end of the "What is the
solution?" section, explain why the persist attribute cannot be set if
the avoid_reset_quirk attribute is on.


Ok. I get it.


Hmmm, what about hubs?  They don't have a persist attribute because
persist is _always_ supposed to be enabled for a hub.  Maybe this means
they shouldn't have an avoid_reset_quirk attribute either.

Oh. It;s tough since the avoid_reset_quirk attribute is added when device
is created. How about remove it in the hub probe()?


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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Lan Tianyu

于 2012/7/24 22:59, Alan Stern 写道:

Yes, good idea.

When you do this, you should also add another little paragraph to
Documentation/usb/persist.txt.  At the end of the "What is the
solution?" section, explain why the persist attribute cannot be set if
the avoid_reset_quirk attribute is on.

Hmmm, what about hubs?  They don't have a persist attribute because
persist is_always_  supposed to be enabled for a hub.  Maybe this means
they shouldn't have an avoid_reset_quirk attribute either.

Another proposal is to add USB_CLASS_HUB check in the avoid_reset_quirk
attribute callback. If it was hub, return directly.
--
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: [RFC PATCH] usb: Change persist_enabled when attribute avoid_reset_quirk is changed

2012-07-24 Thread Alan Stern
On Tue, 24 Jul 2012, Lan Tianyu wrote:

> 于 2012/7/24 22:59, Alan Stern 写道:
> > Yes, good idea.
> >
> > When you do this, you should also add another little paragraph to
> > Documentation/usb/persist.txt.  At the end of the "What is the
> > solution?" section, explain why the persist attribute cannot be set if
> > the avoid_reset_quirk attribute is on.
> >
> > Hmmm, what about hubs?  They don't have a persist attribute because
> > persist is_always_  supposed to be enabled for a hub.  Maybe this means
> > they shouldn't have an avoid_reset_quirk attribute either.
> Another proposal is to add USB_CLASS_HUB check in the avoid_reset_quirk
> attribute callback. If it was hub, return directly.

Or else take the &dev_attr_avoid_reset_quirk.attr entry out of 
dev_attrs[], and instead make usb_create_sysfs_dev_files() and 
usb_remove_sysfs_dev_files() explicitly create and remove the 
avoid_reset_quirk attribute if the device isn't a hub.

Adding the check to the avoid_reset_quirk callback is easier, though.

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: Serious problems with empty port power management in linux-next

2012-07-24 Thread Alan Stern
On Mon, 23 Jul 2012, Bjørn Mork wrote:

> Alan Stern  writes:
> > On Mon, 23 Jul 2012, Bjørn Mork wrote:
> >
> >> Hello,
> >> 
> >> I have no idea where to start debugging this.  But USB hotplug does not
> >> seem to work at all in linux-next if autosuspend is enabled.
> >> 
> >> I am running next-20120723 with this:
> >> 
> >> nemi:/tmp# grep . /sys/bus/usb/devices/usb*/power/control 
> >> /sys/bus/usb/devices/usb1/power/control:auto
> >> /sys/bus/usb/devices/usb2/power/control:auto
> >> /sys/bus/usb/devices/usb3/power/control:auto
> >> /sys/bus/usb/devices/usb4/power/control:auto
> >> /sys/bus/usb/devices/usb5/power/control:auto
> >> /sys/bus/usb/devices/usb6/power/control:auto
> >> /sys/bus/usb/devices/usb7/power/control:auto
> >> /sys/bus/usb/devices/usb8/power/control:auto
> >> 
> >> 
> >> Connecting a device to any port, whether it is external or internal
> >> (using rfkill to "connect") has no effect.  Nothing in the log.  No
> >> uevent. 
> >> 
> >> Setting control to "on" on a port will reenable hotplug on that port.
> >> Any device connected while in "auto" state will suddenly be discovered.
> >
> > This sounds like a bug in the host controller driver.  What HCDs are 
> > you using?  Does the host controller also get suspended?
> 
> uhci_hcd + ehci_hcd.  Really standard intel based laptop from 2008/2009:

I tried reproducing this without success.  But I wasn't using the same
kernel as you; I used 3.5 plus Greg's usb-next branch.  So it looks
like the problem lies somewhere else.

At any rate, you should be able to prevent the problem by preventing 
the host controllers from suspending in the first place:

for a in /sys/bus/usb/devices/usb* ; do
echo on >$a/../power/control
done

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] omap: usb: host: remove deprecated flag 'es2_compatibility'

2012-07-24 Thread Ruslan Bilovol
Currently this flag is not used anywhere and may be safely removed.

Signed-off-by: Ruslan Bilovol 
---
 arch/arm/mach-omap2/usb-host.c|1 -
 arch/arm/plat-omap/include/plat/usb.h |4 
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index dde8a11..5e1cb53 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -500,7 +500,6 @@ void __init usbhs_init(const struct usbhs_omap_board_data 
*pdata)
ehci_data.regulator[i] = pdata->regulator[i];
}
ehci_data.phy_reset = pdata->phy_reset;
-   ohci_data.es2_compatibility = pdata->es2_compatibility;
usbhs_data.ehci_data = &ehci_data;
usbhs_data.ohci_data = &ohci_data;
 
diff --git a/arch/arm/plat-omap/include/plat/usb.h 
b/arch/arm/plat-omap/include/plat/usb.h
index 762eeb0..f8c1eb8 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -32,9 +32,6 @@ struct usbhs_omap_board_data {
/* have to be valid if phy_reset is true and portx is in phy mode */
int reset_gpio_port[OMAP3_HS_USB_PORTS];
 
-   /* Set this to true for ES2.x silicon */
-   unsignedes2_compatibility:1;
-
unsignedphy_reset:1;
 
/*
@@ -53,7 +50,6 @@ struct ehci_hcd_omap_platform_data {
 
 struct ohci_hcd_omap_platform_data {
enum usbhs_omap_port_mode   port_mode[OMAP3_HS_USB_PORTS];
-   unsignedes2_compatibility:1;
 };
 
 struct usbhs_omap_platform_data {
-- 
1.7.1

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


[PATCH] cdc-ncm: tag Ericsson WWAN devices (eg F5521gw) with FLAG_WWAN

2012-07-24 Thread Dan Williams
Tag Ericsson NCM devices as WWAN modems, since they almost certainly all
are.  This way userspace clients know that the device requires further
setup on the AT-capable serial ports before connectivity is available.

Signed-off-by: Dan Williams 
---

NOTE: it seems unlikely that all NCM devices are WWAN modems, but it
also seems unlikely that Ericsson will produce a plain USB Ethernet
adapter using CDC-NCM.

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4b9513f..1931587 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -138,20 +138,7 @@ struct cdc_ncm_ctx {
 static void cdc_ncm_txpath_bh(unsigned long param);
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
-static const struct driver_info cdc_ncm_info;
 static struct usb_driver cdc_ncm_driver;
-static const struct ethtool_ops cdc_ncm_ethtool_ops;
-
-static const struct usb_device_id cdc_devs[] = {
-   { USB_INTERFACE_INFO(USB_CLASS_COMM,
-   USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-   .driver_info = (unsigned long)&cdc_ncm_info,
-   },
-   {
-   },
-};
-
-MODULE_DEVICE_TABLE(usb, cdc_devs);
 
 static void
 cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
@@ -454,6 +441,16 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
kfree(ctx);
 }
 
+static const struct ethtool_ops cdc_ncm_ethtool_ops = {
+   .get_drvinfo = cdc_ncm_get_drvinfo,
+   .get_link = usbnet_get_link,
+   .get_msglevel = usbnet_get_msglevel,
+   .set_msglevel = usbnet_set_msglevel,
+   .get_settings = usbnet_get_settings,
+   .set_settings = usbnet_set_settings,
+   .nway_reset = usbnet_nway_reset,
+};
+
 static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 {
struct cdc_ncm_ctx *ctx;
@@ -1203,6 +1200,41 @@ static const struct driver_info cdc_ncm_info = {
.tx_fixup = cdc_ncm_tx_fixup,
 };
 
+/* Same as cdc_ncm_info, but with FLAG_WWAN */
+static const struct driver_info wwan_info = {
+   .description = "Mobile Broadband Network Device",
+   .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+   | FLAG_WWAN,
+   .bind = cdc_ncm_bind,
+   .unbind = cdc_ncm_unbind,
+   .check_connect = cdc_ncm_check_connect,
+   .manage_power = cdc_ncm_manage_power,
+   .status = cdc_ncm_status,
+   .rx_fixup = cdc_ncm_rx_fixup,
+   .tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id cdc_devs[] = {
+   /* Ericsson MBM devices like F5521gw */
+   { .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
+   | USB_DEVICE_ID_MATCH_VENDOR,
+ .idVendor = 0x0bdb,
+ .bInterfaceClass = USB_CLASS_COMM,
+ .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
+ .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+ .driver_info = (unsigned long) &wwan_info,
+   },
+
+   /* Generic CDC-NCM devices */
+   { USB_INTERFACE_INFO(USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
+   .driver_info = (unsigned long)&cdc_ncm_info,
+   },
+   {
+   },
+};
+MODULE_DEVICE_TABLE(usb, cdc_devs);
+
 static struct usb_driver cdc_ncm_driver = {
.name = "cdc_ncm",
.id_table = cdc_devs,
@@ -1215,16 +1247,6 @@ static struct usb_driver cdc_ncm_driver = {
.disable_hub_initiated_lpm = 1,
 };
 
-static const struct ethtool_ops cdc_ncm_ethtool_ops = {
-   .get_drvinfo = cdc_ncm_get_drvinfo,
-   .get_link = usbnet_get_link,
-   .get_msglevel = usbnet_get_msglevel,
-   .set_msglevel = usbnet_set_msglevel,
-   .get_settings = usbnet_get_settings,
-   .set_settings = usbnet_set_settings,
-   .nway_reset = usbnet_nway_reset,
-};
-
 module_usb_driver(cdc_ncm_driver);
 
 MODULE_AUTHOR("Hans Petter Selasky");


--
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] Fixup mos7840 timeout

2012-07-24 Thread Mark Ferrell
* mos7840 driver was using multiple of HZ for the timeout handed off to
  usb_control_msg().  Changed the timeout to use msecs instead.

* Remove unused WAIT_FOR_EVER definition

Signed-off-by: Mark Ferrell 

---
 drivers/usb/serial/mos7840.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 57eca24..6516f30 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -82,8 +82,7 @@
  * Defines used for sending commands to port
  */
 
-#define WAIT_FOR_EVER   (HZ * 0)   /* timeout urb is wait for ever */
-#define MOS_WDR_TIMEOUT (HZ * 5)   /* default urb timeout */
+#define MOS_WDR_TIMEOUT5000/* default urb timeout */
 
 #define MOS_PORT1   0x0200
 #define MOS_PORT2   0x0300
@@ -1344,7 +1343,7 @@ static void mos7840_close(struct usb_serial_port *port)
 static void mos7840_block_until_chase_response(struct tty_struct *tty,
struct moschip_port *mos7840_port)
 {
-   int timeout = 1 * HZ;
+   int timeout = msecs_to_jiffies(1000);
int wait = 10;
int count;
 
@@ -2672,7 +2671,7 @@ static int mos7840_startup(struct usb_serial *serial)
 
/* setting configuration feature to one */
usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
-   (__u8) 0x03, 0x00, 0x01, 0x00, NULL, 0x00, 5 * HZ);
+   (__u8) 0x03, 0x00, 0x01, 0x00, NULL, 0x00, 
MOS_WDR_TIMEOUT);
return 0;
 error:
for (/* nothing */; i >= 0; i--) {
-- 
tg: (8a7298b..) usb/serial/mos7840/timeout (depends on: master)
--
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: serial: mos7840: Fixup mos7840_chars_in_buffer()

2012-07-24 Thread Mark Ferrell

 * Use the buffer content length as opposed to the total buffer size.  This can
   be a real problem when using the mos7840 as a usb serial-console as all
   kernel output is truncated during boot.

Signed-off-by: Mark Ferrell 

---
 drivers/usb/serial/mos7840.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 57eca24..009c1d9 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1232,9 +1232,12 @@ static int mos7840_chars_in_buffer(struct tty_struct 
*tty)
return 0;
 
spin_lock_irqsave(&mos7840_port->pool_lock, flags);
-   for (i = 0; i < NUM_URBS; ++i)
-   if (mos7840_port->busy[i])
-   chars += URB_TRANSFER_BUFFER_SIZE;
+   for (i = 0; i < NUM_URBS; ++i) {
+   if (mos7840_port->busy[i]) {
+   struct urb *urb = mos7840_port->write_urb_pool[i];
+   chars += urb->transfer_buffer_length;
+   }
+   }
spin_unlock_irqrestore(&mos7840_port->pool_lock, flags);
dbg("%s - returns %d", __func__, chars);
return chars;
-- 
tg: (8a7298b..) usb/serial/mos7840/chars_in_buf (depends on: master)
--
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: serial: mos7840: Avoid kfree() on bad pointer

2012-07-24 Thread Mark Ferrell
* mos7840_startup() blindly calls kfree() on memory that may have never been
  allocated.

Signed-off-by: Mark Ferrell 

---
 drivers/usb/serial/mos7840.c |   13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 57eca24..1afb141 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -2678,10 +2678,15 @@ error:
for (/* nothing */; i >= 0; i--) {
mos7840_port = mos7840_get_port_private(serial->port[i]);
 
-   kfree(mos7840_port->dr);
-   kfree(mos7840_port->ctrl_buf);
-   usb_free_urb(mos7840_port->control_urb);
-   kfree(mos7840_port);
+   if (mos7840_port) {
+   if (mos7840_port->control_urb)
+   usb_free_urb(mos7840_port->control_urb);
+   if (mos7840_port->ctrl_buf)
+   kfree(mos7840_port->ctrl_buf);
+   if (mos7840_port->dr)
+   kfree(mos7840_port->dr);
+   kfree(mos7840_port);
+   }
serial->port[i] = NULL;
}
return status;
-- 
tg: (8a7298b..) usb/serial/mos7840/kfree (depends on: master)
--
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: serial: mos7840: Avoid kfree() on bad pointer

2012-07-24 Thread Sergei Shtylyov
Hello.

On 07/24/2012 11:17 PM, Mark Ferrell wrote:

> * mos7840_startup() blindly calls kfree() on memory that may have never been
>   allocated.

   Calling kfree() on a NULL pointers is perfectly valid.

> Signed-off-by: Mark Ferrell 

> ---
>  drivers/usb/serial/mos7840.c |   13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)

> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 57eca24..1afb141 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -2678,10 +2678,15 @@ error:
>   for (/* nothing */; i >= 0; i--) {
>   mos7840_port = mos7840_get_port_private(serial->port[i]);
>  
> - kfree(mos7840_port->dr);
> - kfree(mos7840_port->ctrl_buf);
> - usb_free_urb(mos7840_port->control_urb);
> - kfree(mos7840_port);
> + if (mos7840_port) {
> + if (mos7840_port->control_urb)
> + usb_free_urb(mos7840_port->control_urb);
> + if (mos7840_port->ctrl_buf)
> + kfree(mos7840_port->ctrl_buf);
> + if (mos7840_port->dr)
> + kfree(mos7840_port->dr);
> + kfree(mos7840_port);
> + }
>   serial->port[i] = NULL;
>   }
>   return status;

WBR, Sergei

--
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: serial: mos7840: Avoid kfree() on bad pointer

2012-07-24 Thread Oliver Neukum
On Tuesday 24 July 2012 14:17:31 Mark Ferrell wrote:
> * mos7840_startup() blindly calls kfree() on memory that may have never been
>   allocated.

kfree() includes a check for NULL.

Regards
Oliver

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


Re: [PATCH 2/2] xhci: EHCI/XHCI ports switching on Intense-PC.

2012-07-24 Thread Sarah Sharp
On Tue, Jul 24, 2012 at 11:50:42AM +0300, Denis Turischev wrote:
> On 07/23/2012 08:44 PM, Sarah Sharp wrote:
> > Did you try disabling the wake on bits in the port status registers?  I
> > would prefer that solution if it works.
> No, I didn't. Why we need to disable wake on capabilities?

My theory was that the xHCI host controller was waking up the system
because one of the wake on bits was set.  But you said that didn't help,
so we'll just switch the ports over on shutdown.

Sarah Sharp
--
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: [RFT] xhci: Increase reset timeout for renesas 720201 host.

2012-07-24 Thread Sarah Sharp
On Tue, Jul 24, 2012 at 01:24:07PM +0200, Edwin Klein Mentink wrote:
> Hallo Sarah,
> 
> Thanks for the fast reply!
> I've applied the patch. It seems to work now :-)

Excellent!  I'll send that patch upstream then.

> I have a fairly old computer (AMD Athlon(tm) 64 X2 Dual Core Processor
> 3800+), i'm not sure if it depends on the speed of the actual
> computer.

Possibly.  I'm not sure though.

> I found earlier some tickets with problems when devices attached to the
> device. Is this related to this timeout?

No, probably not.  There may be other quirks needed for this host
controller, or bugs in the xHCI driver.  Let me know if you experience
any issues with your USB devices under this new host controller.

Sarah Sharp
--
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] ene_ub6250: Use macros for firmware names

2012-07-24 Thread Greg Kroah-Hartman
On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:
> Advertise firmware files using MODULE_FIRMWARE macros.
> 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Cc: usb-stor...@lists.one-eyed-alien.net
> Signed-off-by: Tim Gardner 
> ---
>  drivers/usb/storage/ene_ub6250.c |   26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/storage/ene_ub6250.c 
> b/drivers/usb/storage/ene_ub6250.c
> index b28f2ad..3fec82f 100644
> --- a/drivers/usb/storage/ene_ub6250.c
> +++ b/drivers/usb/storage/ene_ub6250.c
> @@ -29,9 +29,21 @@
>  #include "protocol.h"
>  #include "debug.h"
>  
> +#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> +#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> +#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> +#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> +#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> +#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> +
>  MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
>  MODULE_LICENSE("GPL");
> -
> +MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> +MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> +MODULE_FIRMWARE(SD_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> +MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_RW_FIRMWARE);

Why do you need the #defines here at all?  What's wrong with just using
the file names in the MODULE_FIRMWARE() macro directly?  That cuts the
size of the patch in half :)

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: Serious problems with empty port power management in linux-next

2012-07-24 Thread Bjørn Mork
Alan Stern  writes:
> On Mon, 23 Jul 2012, Bjørn Mork wrote:
>
>> uhci_hcd + ehci_hcd.  Really standard intel based laptop from 2008/2009:
>
> I tried reproducing this without success.  But I wasn't using the same
> kernel as you; I used 3.5 plus Greg's usb-next branch.  So it looks
> like the problem lies somewhere else.

Maybe a PCI thing?  Well, I'll just take another round bisecting now
that I can test linux-next without crashing.  Bisecting all changes in
linux-next is only a few more rounds than just doing usb-next.

> At any rate, you should be able to prevent the problem by preventing 
> the host controllers from suspending in the first place:
>
>   for a in /sys/bus/usb/devices/usb* ; do
>   echo on >$a/../power/control
>   done

Yes, no problem working around it.



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] cdc-ncm: tag Ericsson WWAN devices (eg F5521gw) with FLAG_WWAN

2012-07-24 Thread David Miller
From: Dan Williams 
Date: Tue, 24 Jul 2012 13:43:22 -0500

> Tag Ericsson NCM devices as WWAN modems, since they almost certainly all
> are.  This way userspace clients know that the device requires further
> setup on the AT-capable serial ports before connectivity is available.
> 
> Signed-off-by: Dan Williams 

Applied.
--
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] ene_ub6250: Use macros for firmware names

2012-07-24 Thread Tim Gardner
Advertise firmware files using MODULE_FIRMWARE macros.

Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: usb-stor...@lists.one-eyed-alien.net
Signed-off-by: Tim Gardner 
---
 drivers/usb/storage/ene_ub6250.c |   26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index b28f2ad..3fec82f 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -29,9 +29,21 @@
 #include "protocol.h"
 #include "debug.h"
 
+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
+
 MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
 MODULE_LICENSE("GPL");
-
+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
+MODULE_FIRMWARE(SD_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_RW_FIRMWARE);
 
 /*
  * The table of devices
@@ -1883,28 +1895,28 @@ static int ene_load_bincode(struct us_data *us, 
unsigned char flag)
/* For SD */
case SD_INIT1_PATTERN:
US_DEBUGP("SD_INIT1_PATTERN\n");
-   fw_name = "ene-ub6250/sd_init1.bin";
+   fw_name = SD_INIT1_FIRMWARE;
break;
case SD_INIT2_PATTERN:
US_DEBUGP("SD_INIT2_PATTERN\n");
-   fw_name = "ene-ub6250/sd_init2.bin";
+   fw_name = SD_INIT2_FIRMWARE;
break;
case SD_RW_PATTERN:
US_DEBUGP("SD_RDWR_PATTERN\n");
-   fw_name = "ene-ub6250/sd_rdwr.bin";
+   fw_name = SD_RW_FIRMWARE;
break;
/* For MS */
case MS_INIT_PATTERN:
US_DEBUGP("MS_INIT_PATTERN\n");
-   fw_name = "ene-ub6250/ms_init.bin";
+   fw_name = MS_INIT_FIRMWARE;
break;
case MSP_RW_PATTERN:
US_DEBUGP("MSP_RW_PATTERN\n");
-   fw_name = "ene-ub6250/msp_rdwr.bin";
+   fw_name = MSP_RW_FIRMWARE;
break;
case MS_RW_PATTERN:
US_DEBUGP("MS_RW_PATTERN\n");
-   fw_name = "ene-ub6250/ms_rdwr.bin";
+   fw_name = MS_RW_FIRMWARE;
break;
default:
US_DEBUGP("--- Unknown PATTERN --\n");
-- 
1.7.9.5

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


Re: [PATCH] ene_ub6250: Use macros for firmware names

2012-07-24 Thread Greg Kroah-Hartman
On Tue, Jul 24, 2012 at 03:00:06PM -0600, Tim Gardner wrote:
> On 07/24/2012 02:34 PM, Greg Kroah-Hartman wrote:
> >On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:
> >>Advertise firmware files using MODULE_FIRMWARE macros.
> >>
> >>Cc: Greg Kroah-Hartman 
> >>Cc: linux-usb@vger.kernel.org
> >>Cc: usb-stor...@lists.one-eyed-alien.net
> >>Signed-off-by: Tim Gardner 
> >>---
> >>  drivers/usb/storage/ene_ub6250.c |   26 +++---
> >>  1 file changed, 19 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/drivers/usb/storage/ene_ub6250.c 
> >>b/drivers/usb/storage/ene_ub6250.c
> >>index b28f2ad..3fec82f 100644
> >>--- a/drivers/usb/storage/ene_ub6250.c
> >>+++ b/drivers/usb/storage/ene_ub6250.c
> >>@@ -29,9 +29,21 @@
> >>  #include "protocol.h"
> >>  #include "debug.h"
> >>
> >>+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> >>+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> >>+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> >>+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> >>+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> >>+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> >>+
> >>  MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
> >>  MODULE_LICENSE("GPL");
> >>-
> >>+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> >>+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> >>+MODULE_FIRMWARE(SD_RW_FIRMWARE);
> >>+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> >>+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> >>+MODULE_FIRMWARE(MS_RW_FIRMWARE);
> >
> >Why do you need the #defines here at all?  What's wrong with just using
> >the file names in the MODULE_FIRMWARE() macro directly?  That cuts the
> >size of the patch in half :)
> >
> >thanks,
> >
> >greg k-h
> >
> 
> If the firmware file name ever changes, then you'll have to find and
> modify it in 2 places.

Oops, sorry, I missed the second place it was used in the file,
nevermind, time for more coffee...

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] ene_ub6250: Use macros for firmware names

2012-07-24 Thread Tim Gardner

On 07/24/2012 02:34 PM, Greg Kroah-Hartman wrote:

On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:

Advertise firmware files using MODULE_FIRMWARE macros.

Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: usb-stor...@lists.one-eyed-alien.net
Signed-off-by: Tim Gardner 
---
  drivers/usb/storage/ene_ub6250.c |   26 +++---
  1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index b28f2ad..3fec82f 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -29,9 +29,21 @@
  #include "protocol.h"
  #include "debug.h"

+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
+
  MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
  MODULE_LICENSE("GPL");
-
+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
+MODULE_FIRMWARE(SD_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_RW_FIRMWARE);


Why do you need the #defines here at all?  What's wrong with just using
the file names in the MODULE_FIRMWARE() macro directly?  That cuts the
size of the patch in half :)

thanks,

greg k-h



If the firmware file name ever changes, then you'll have to find and 
modify it in 2 places.


I don't really have a strong preference, but I would like to see 
MODULE_FIRMWARE() used so I can cut down on the number of false 
positives as I go through the kernel firmware directory and the 
linux-firmware package to filter out unused files using modinfo.


rtg
--
Tim Gardner tim.gard...@canonical.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] ene_ub6250: Use macros for firmware names

2012-07-24 Thread Betty Dall
Hi Tim,

I reviewed this patch and it looks good. Once small suggestion you can
take or leave...

On Tue, 2012-07-24 at 14:31 -0600, Tim Gardner wrote:
> Advertise firmware files using MODULE_FIRMWARE macros.
> 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Cc: usb-stor...@lists.one-eyed-alien.net
> Signed-off-by: Tim Gardner 
> ---
>  drivers/usb/storage/ene_ub6250.c |   26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/storage/ene_ub6250.c 
> b/drivers/usb/storage/ene_ub6250.c
> index b28f2ad..3fec82f 100644
> --- a/drivers/usb/storage/ene_ub6250.c
> +++ b/drivers/usb/storage/ene_ub6250.c
> @@ -29,9 +29,21 @@
>  #include "protocol.h"
>  #include "debug.h"
>  
> +#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> +#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> +#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> +#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> +#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> +#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> +
>  MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
>  MODULE_LICENSE("GPL");
> -
> +MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> +MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> +MODULE_FIRMWARE(SD_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> +MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_RW_FIRMWARE);
>  
>  /*
>   * The table of devices
> @@ -1883,28 +1895,28 @@ static int ene_load_bincode(struct us_data *us, 
> unsigned char flag)
>   /* For SD */
>   case SD_INIT1_PATTERN:
>   US_DEBUGP("SD_INIT1_PATTERN\n");
> - fw_name = "ene-ub6250/sd_init1.bin";
> + fw_name = SD_INIT1_FIRMWARE;
>   break;
>   case SD_INIT2_PATTERN:
>   US_DEBUGP("SD_INIT2_PATTERN\n");
> - fw_name = "ene-ub6250/sd_init2.bin";
> + fw_name = SD_INIT2_FIRMWARE;
>   break;
>   case SD_RW_PATTERN:
>   US_DEBUGP("SD_RDWR_PATTERN\n");

All the other rdwr patterns are RW not RDWR. So you could change this
one to be SD_RW_PATTERN to be consistent with MSP_RW_PATTERN and
MS_RW_PATTERN.  This is a nit.

> - fw_name = "ene-ub6250/sd_rdwr.bin";
> + fw_name = SD_RW_FIRMWARE;
>   break;
>   /* For MS */
>   case MS_INIT_PATTERN:
>   US_DEBUGP("MS_INIT_PATTERN\n");
> - fw_name = "ene-ub6250/ms_init.bin";
> + fw_name = MS_INIT_FIRMWARE;
>   break;
>   case MSP_RW_PATTERN:
>   US_DEBUGP("MSP_RW_PATTERN\n");
> - fw_name = "ene-ub6250/msp_rdwr.bin";
> + fw_name = MSP_RW_FIRMWARE;
>   break;
>   case MS_RW_PATTERN:
>   US_DEBUGP("MS_RW_PATTERN\n");
> - fw_name = "ene-ub6250/ms_rdwr.bin";
> + fw_name = MS_RW_FIRMWARE;
>   break;
>   default:
>   US_DEBUGP("--- Unknown PATTERN --\n");

-Betty


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


bisected regression, v3.5 -> next-20120724: PCI PM causes USB hotplug failure

2012-07-24 Thread Bjørn Mork
Enabling autosuspend for USB causes hotplug failure in the current
linux-next. Newly plugged devices are not detected at all until the
port/controller is manually powered on by writing "on" to power/control.
Testing is pretty simple:

  1) for f in /sys/bus/usb/devices/*/power/control; do echo auto > $f; done
  2) wait for the controllers to suspend
  3) plugin a new USB device


I've bisected the regression down to this commit:


commit 448bd857d48e69b33ef323739dc6d8ca20d4cda7
Author: Huang Ying 
Date:   Sat Jun 23 10:23:51 2012 +0800

PCI/PM: add PCIe runtime D3cold support


Looks like this somehow powers down my USB host controllers in such a
way that they are unable to detect connections to their ports.  The
system is a pretty standard 4 year old intel based laptop.  Full lspci
listing is attached.

Please let me know if you need further information.  And please fix this
before rc1.  It effectively makes USB non-functional if autosuspend is
enabled.



Thanks,
Bjørn

00:00.0 Host bridge [0600]: Intel Corporation Mobile 4 Series Chipset Memory 
Controller Hub [8086:2a40] (rev 07)
Subsystem: Lenovo Device [17aa:20e0]
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- 
Kernel driver in use: agpgart-intel

00:02.0 VGA compatible controller [0300]: Intel Corporation Mobile 4 Series 
Chipset Integrated Graphics Controller [8086:2a42] (rev 07) (prog-if 00 [VGA 
controller])
Subsystem: Lenovo Device [17aa:20e4]
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR-  [disabled]
Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
Address: fee0300c  Data: 41a1
Capabilities: [d0] Power Management version 3
Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA 
PME(D0-,D1-,D2-,D3hot-,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
Kernel driver in use: i915

00:02.1 Display controller [0380]: Intel Corporation Mobile 4 Series Chipset 
Integrated Graphics Controller [8086:2a43] (rev 07)
Subsystem: Lenovo Device [17aa:20e4]
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- TAbort- SERR- TAbort- SERR- TAbort- 
SERR- TAbort- 
SERR- TAbort- 
SERR- TAbort- 
SERR- TAbort- SERR- TAbort- SERR- TAbort- Reset- FastB2B-
PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, 
L1 <1us
ExtTag- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- 
Unsupported-
RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 128 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ 
TransPend-
LnkCap: Port #1, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency 
L0 <1us, L1 <4us
ClockPM- Surprise- LLActRep+ BwNot-
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ 
DLActive- BWMgmt- ABWMgmt-
SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ 
Surprise+
Slot #0, PowerLimit 6.500W; Interlock- NoCompl-
SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet+ CmdCplt+ HPIrq+ 
LinkChg-
Control: AttnInd Unknown, PwrInd Unknown, Power- 
Interlock-
SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- 
Interlock-
Changed: MRL- PresDet- LinkState-
RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ 
CRSVisible-
RootCap: CRSVisible-
RootSta: PME ReqID , PMEStatus- PMEPending-
Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit-
Address: fee0300c  Data: 4179
Capabilities: [90] Subsystem: Lenovo Device [17aa:20f3]
Capabilities: [a0] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA 
PME(D0+,D1-,D2-,D3hot+,D3cold+)
Status: D3 NoSoftRst- PME-Enable+ DSel=0 DScale=0 PME-
Capabilities: [100 v1] Virtual Channel
Caps:   LPEVC=0 RefClk=100ns PATEntryBits=1
Arb:Fixed+ WRR32- WRR64- WRR128-
Ctrl:   ArbSelect=Fixed
Status: InProgress-
VC0:Caps:   PATOffset=00 MaxTimeSlo

Re: bisected regression, v3.5 -> next-20120724: PCI PM causes USB hotplug failure

2012-07-24 Thread Huang Ying
Hi, Bjorn,

On Wed, 2012-07-25 at 06:08 +0200, Bjørn Mork wrote:
> Enabling autosuspend for USB causes hotplug failure in the current
> linux-next. Newly plugged devices are not detected at all until the
> port/controller is manually powered on by writing "on" to power/control.
> Testing is pretty simple:
> 
>   1) for f in /sys/bus/usb/devices/*/power/control; do echo auto > $f; done

Have you done:

for f in /sys/bus/pci/devices/*/power/confol; do echo auto > $f; done

?

If not, the pci device will not be suspended at all.

>   2) wait for the controllers to suspend
>   3) plugin a new USB device

After plugin the new USB device, is there anything in dmesg?

> 
> I've bisected the regression down to this commit:
> 
> 
> commit 448bd857d48e69b33ef323739dc6d8ca20d4cda7
> Author: Huang Ying 
> Date:   Sat Jun 23 10:23:51 2012 +0800
> 
> PCI/PM: add PCIe runtime D3cold support
> 
> 
> Looks like this somehow powers down my USB host controllers in such a
> way that they are unable to detect connections to their ports.  The
> system is a pretty standard 4 year old intel based laptop.  Full lspci
> listing is attached.
>
> Please let me know if you need further information.  And please fix this
> before rc1.  It effectively makes USB non-functional if autosuspend is
> enabled.

Can you provide the output of the following command lines?

grep . /sys/bus/pci/devices/*/power/*
grep . /sys/bus/usb/devices/*/power/*
lsusb

After the controllers suspended.

Best Reards,
Huang Ying


--
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: g_serial and cdc_acm communications

2012-07-24 Thread Felipe Balbi
(please use reply to all and keep the previous mail)

On Tue, Jul 24, 2012 at 02:29:57PM +, Dipen Patel wrote:
> Hi Felipe,
> 
> I think I know why my application is not receiving SIGHUP. 
> 
> When HOST sends SET_CONFIGURATION the set_config() in composite.c calls
> acm_set_alt(). Now acm_set_alt() calls gserial_connect() and it will 
> create new port. But it never called gserial_disconnect() to close 
> previously called gserial_connect(), so SIGHUP is not generated. 
> 
> But why acm_set_alt() does not do gserial_disconnect() before calling
> gserial_connect()?

In case of configuration change, composite.c will call f->disable()
which is implemented on acm as:

464 static void acm_disable(struct usb_function *f)
465 {
466 struct f_acm*acm = func_to_acm(f);
467 struct usb_composite_dev *cdev = f->config->cdev;
468 
469 DBG(cdev, "acm ttyGS%d deactivated\n", acm->port_num);
470 gserial_disconnect(&acm->port);
471 usb_ep_disable(acm->notify);
472 acm->notify->driver_data = NULL;
473 }

there you have yor gserial_disconnect(). I suggest you enable debugging
and see where things are going fishy.

-- 
balbi


signature.asc
Description: Digital signature