Re: qmi wwan interface does not work in this version 3.5.rc7.next.20120720
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
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
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
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.
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
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
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
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
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
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/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
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/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
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
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
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
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/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
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
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/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/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
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
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'
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
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
* 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()
* 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
* 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
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
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.
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.
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
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
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
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
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
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
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
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
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
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
(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