[patch -next] smsc95xx: signedness bug in get_regs()
"retval" has to be a signed integer for the error handling to work. Signed-off-by: Dan Carpenter diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 05ecf14..bd7cbaa 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -589,7 +589,8 @@ smsc95xx_ethtool_getregs(struct net_device *netdev, struct ethtool_regs *regs, void *buf) { struct usbnet *dev = netdev_priv(netdev); - unsigned int i, j, retval; + unsigned int i, j; + int retval; u32 *data = buf; retval = smsc95xx_read_reg(dev, ID_REV, ®s->version); -- 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: Logitech HD Webcam C525 does not work when connected to USB 2 port, works on USB 3
Hi, I have exactly the same webcam, lsusb says "Bus 002 Device 004: ID 046d:0826 Logitech, Inc."... On 06.07.2012, Alan Stern wrote: > On Thu, 5 Jul 2012, Frederik Himpe wrote: > > I tried setting to on again, and then this appears in the logs: > > Jul 5 21:21:59 piranha kernel: [ 9788.223118] usb 1-1.2: USB disconnect, > > device number 6 > > Jul 5 21:22:00 piranha kernel: [ 9788.330415] usb 1-1.2: new full-speed > > USB device number 7 using ehci_hcd > > Jul 5 21:22:00 piranha kernel: [ 9788.402207] usb 1-1.2: device descriptor > > read/64, error -32 > > Jul 5 21:22:00 piranha kernel: [ 9788.577706] usb 1-1.2: device descriptor > > read/64, error -32 > > Jul 5 21:22:00 piranha kernel: [ 9788.753203] usb 1-1.2: new full-speed > > USB device number 8 using ehci_hcd > > Jul 5 21:22:00 piranha kernel: [ 9788.824996] usb 1-1.2: device descriptor > > read/64, error -32 > > Jul 5 21:22:00 piranha kernel: [ 9789.000493] usb 1-1.2: device descriptor > > read/64, error -32 [] ...and I do not have these messages at all, with or without autosuspend enabled... > In the long run, it may be necessary to add a quirk entry to always > use a reset-resume with this device. You can find similar entries in > drivers/usb/core/quirks.c for other Logitech webcams, which makes it > seem very likely that one is needed for yours. If you want you > can try adding the appropriate entry and submit it as a patch. ...and it runs fine with or without the quirk, which I added for testing purposes. > None of this explains why there isn't any problem when the webcam is > plugged into a USB-3 port, unfortunately. All this is purely on USB-2, my machine doesn't have any USB-3 port, with vanilla 3.4.4 kernel. If nobody did, should I submit a patch containing the quirk anyway? If I can help testing, feel free to contact me. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb sound issue - any ideas?
On 07/11/2012 03:37 AM, Dr. Ing. Dieter Jurzitza wrote: Dear listmembers, dear Sarah, dear Andiry, Andiry's suggestion (uncomment the return 0 - statement) fixed my issue, the usb soundcard remains active after a login-logout; however, as I understood Sarah's comment this is no more than a plaster on the problem. So, I am willing to do more testing - but I depend on your inputs as I am lacking the required programming skills to actually understand what's happening within the driver. Please let me know what you'd suggest to do next ... Thank you very much, take care Hi Dieter, Thanks for the test. Sarah, I don't know why you regard it as an invalid fix and think the disabled endpoint is abnormal. See xHCI spec 4.6.6: == For each endpoint indicated by a Drop Context flag = '1': If the Output Endpoint Context is not in the Disabled state: The endpoint related resources are subtracted from the Resource Required variable. If the endpoint is periodic, then the bandwidth assigned to the endpoint is subtracted from the bandwidth Required variable. else // Output Endpoint Context is in the Disabled state: Do nothing == Please note this is host controller behavior description, so "Do nothing" means the host will do nothing with a Disabled endpoint with drop flag = 1. It does not mean software should ignore the drop request and return -EINVAL for SET_INTERFACE request. And - Spec says what it should do with Disabled endpoint situation, that implies the situation is expected - and also valid. What do you think? Thanks, Andiry -- 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 v10] USB: notify phy when root hub port connect change
Phy may need to change settings when port connect change. Signed-off-by: Richard Zhao Tested-by: Subodh Nijsure --- Changes since last version: - remove unlikely drivers/usb/core/hub.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4cc8dc9..3183837 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, } } + if (hcd->phy && !hdev->parent) { + if (portstatus & USB_PORT_STAT_CONNECTION) + usb_phy_notify_connect(hcd->phy, port1); + else + usb_phy_notify_disconnect(hcd->phy, port1); + } + /* Return now if debouncing failed or nothing is connected or * the device was "removed". */ -- 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change
On Tue, Jul 10, 2012 at 11:07:14AM -0400, Alan Stern wrote: > On Tue, 10 Jul 2012, Richard Zhao wrote: > > > On Sat, Jul 07, 2012 at 10:56:45PM +0800, Richard Zhao wrote: > > > Phy may need to change settings when port connect change. > > > > > > Signed-off-by: Richard Zhao > > > Tested-by: Subodh Nijsure > > > --- > > > drivers/usb/core/hub.c |8 > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > > index 4cc8dc9..2ba9d84 100644 > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -20,6 +20,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub > > > *hub, int port1, > > > } > > > } > > > > > > + if (unlikely(hcd->phy && !hdev->parent)) { > > > + if (portstatus & USB_PORT_STAT_CONNECTION) > > > + usb_phy_notify_connect(hcd->phy, port1); > > > + else > > > + usb_phy_notify_disconnect(hcd->phy, port1); > > There's another issue. When hcd is removed, notify disconnect is not > > called. Is it ok, if I remove the above two line and add below patch: > > You should keep those two lines, since it's possible for a disconnect > to occur without usb_disconnect() being called. This happens when a > device fails to enumerate. > > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev) > > */ > > device_del(&udev->dev); > > > > + if (udev->parent && !udev->parent->parent) { > > + struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > + usb_phy_notify_disconnect(hcd->phy, udev->portnum); > > + } > > You might not want to add this. usb_disconnect() can get called in > situations where the device remains physically connected (if the > firmware changes during a reset, for example). > > If you want to notify the phy when the root hub is removed, there are > better places to do it. One possibility is in hub_quiesce(), another > is in usb_remove_hcd(). Thanks. I'll need more testing, and won't add it in this patch. Richard > > 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: xhci_hcd: external drive not initialised if already connected during restart or cold boot
On 07/11/2012 01:56 AM, Matt wrote: Lee Harris writes: Hi Sarah Whenever I restart or coldboot, my external drive (3.5 sata in a USB3 enclosure) is not detected / initialised correctly. fdisk -l does not show the drive at all. I have found that I have to: turn it off ( or disconnect the usb cable) modprobe -r xhci_hcd modprobe xhci_hcd and then turn it on. It is then found and initialised. I'm having this same issue. Does anyone have any ideas? Can you post the bootup dmesg with device connected and CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING enabled? Thanks, Andiry -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb hub is n't recognised during boot up(sometimes)
> ehci_hcd :00:1d.0: port 1 reset error -110 It stands for software wants bus reset to complete, but hardware refuses it after 1ms handshake. If Alan is correct that it is a internal hub integrated in chipset, you may can't do nothing except for asking chip vendor as you can't measure the dp/dm at the board. > hub 2-0:1.0: hub_port_status failed (err = -32) > > Error here . > hub 2-0:1.0: port_wait_reset: err = -32 > hub 2-0:1.0: port 1 not enabled, trying reset again... > ehci_hcd :00:1d.0: USBCMD = 00010001 USBSTS = 2008 USBINTR = > 0037 FR > INDEX = 2d3a CTRLDSSEGMENT = PERIODICLISTBASE = 98006000 > ASYNCLIS > TADDR = 98004000 CONFIGFLAG = 0001 > ehci_hcd :00:1d.0: PORTSC4096 = > ehci_hcd :00:1d.0: PORTSC4096 = > ehci_hcd :00:1d.0: PORTSC4353 = > ehci_hcd :00:1d.0: port 1 full speed --> companion > ehci_hcd :00:1d.0: GetStatus port 1 status 003801 POWER OWNER sig=j > CONNECT > hub 2-0:1.0: port 1 not reset yet, waiting 200ms > ehci_hcd :00:1d.0: GetStatus port 1 status 003002 POWER OWNER sig=se0 > CSC > hub 1-1:1.0: state 7 ports 6 chg evt > > > dmesg output > -- > > nitializing cgroup subsys cpuset > Initializing cgroup subsys cpu > Linux version 2.6.32.39 (shshaiju@mcp-bld-lnx-92) (gcc version 4.4.1 > (MontaVista > ) ) #33 SMP Mon Jul 9 14:06:17 PDT 2012 > Command line: root=/dev/ram rw console=ttyS1,9600 max_loop=64 wdt=0 > SR_BOOT=boot > flash:/ghaidiny/rp_super_universalk9.2kp.bin.shaiju > KERNEL supported cpus: > Intel GenuineIntel > AMD AuthenticAMD > Centaur CentaurHauls > BIOS-provided physical RAM map: > BIOS-e820: - 0008f000 (usable) > BIOS-e820: 0008f000 - 0009 (ACPI NVS) > BIOS-e820: 0009 - 000a (usable) > BIOS-e820: 0010 - 6df16000 (usable) > BIOS-e820: 6df16000 - 8421f000 (reserved) > BIOS-e820: 8421f000 - 9f4bf000 (usable) > BIOS-e820: 9f4bf000 - 9f6bf000 (reserved) > BIOS-e820: 9f6bf000 - 9f7bf000 (ACPI NVS) > BIOS-e820: 9f7bf000 - 9f7ef000 (ACPI data) > BIOS-e820: 9f7ef000 - 9f80 (usable) > BIOS-e820: e000 - f000 (reserved) > BIOS-e820: fec0 - fec01000 (reserved) > BIOS-e820: fed1c000 - fed2 (reserved) > BIOS-e820: ffdf - (reserved) > BIOS-e820: 0001 - 00016000 (usable) > DMI not present or invalid. > last_pfn = 0x16 max_arch_pfn = 0x4 > MTRR default type: uncachable > MTRR fixed ranges enabled: > 0-9 write-back > A-B uncachable > C-E write-through > F-F write-combining > MTRR variable ranges enabled: > 0 base 00 mask FF write-back > 1 base 01 mask FFC000 write-back > 2 base 014000 mask FFE000 write-back > 3 base 00A000 mask FFE000 uncachable > 4 base 00C000 mask FFC000 uncachable > 5 disabled > 6 disabled > 7 disabled > e820 update range: a000 - 0001 (usable) ==> > (reserved) > last_pfn = 0x9f800 max_arch_pfn = 0x4 > initial memory mapped : 0 - 2000 > init_memory_mapping: -9f80 > 00 - 009f80 page 2M > kernel direct mapping tables up to 9f80 @ 8000-c000 > init_memory_mapping: 0001-00016000 > 01 - 016000 page 2M > kernel direct mapping tables up to 16000 @ a000-11000 > RAMDISK: 6df16018 - 8421e818 > ACPI: RSDP 000e 00024 (v02 INSYDE) > ACPI: XSDT 9f7ee120 000AC (v01 INSYDE PicketPo 0001 > 0113) > ACPI: FACP 9f7ec000 000F4 (v04 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: DSDT 9f7e1000 076A5 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: FACS 9f729000 00040 > ACPI: ASF! 9f7ed000 000A5 (v32 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: HPET 9f7eb000 00038 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: APIC 9f7ea000 000EA (v02 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: MCFG 9f7e9000 0003C (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: SLIC 9f7e 00176 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: BOOT 9f7dd000 00028 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: SSDT 9f7dc000 002F6 (v01 INTEL SataAhci 1000 INTL > 20100121) > ACPI: SRAT 9f7d9000 00400 (v02 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: SLIT 9f7d8000 00030 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: WDAT 9f7d7000 00224 (v01 INSYDE PicketPo 0001 MSFT > 0113) > ACPI: DMAR 9f7d6
[PATCH] usb/host/ehci-hub: Fix the issue EG20T USB host controller has long resuming time, when pen drive is attached.
Signed-off-by: Tomoya MORINAGA --- drivers/usb/host/ehci-hub.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index fc9e7cc..d596d0f 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -207,6 +207,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) int port; int mask; int changed; + int temp; ehci_dbg(ehci, "suspend root hub\n"); @@ -324,6 +325,9 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) * want, and so we must delete any pending watchdog timer events. */ del_timer_sync(&ehci->watchdog); + temp = ehci_readl(ehci, &ehci->regs->status); + if (temp & STS_FLR) + ehci_writel(ehci, STS_FLR, &ehci->regs->status); return 0; } -- 1.7.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -v2] USB: add USB_VENDOR_AND_INTERFACE_INFO() macro
From: Gustavo Padovan A lot of Broadcom Bluetooth devices provides vendor specific interface class and we are getting flooded by patches adding new device support. This change will help us enable support for any other Broadcom with vendor specific device that arrives in the future. Only the product id changes for those devices, so this macro would be perfect for us: { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) } Signed-off-by: Marcel Holtmann Signed-off-by: Gustavo Padovan --- include/linux/usb.h | 21 + 1 file changed, 21 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index dea39dc..edb1dc8 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -829,6 +829,27 @@ static inline int usb_make_path(struct usb_device *dev, char *buf, size_t size) .bInterfaceSubClass = (sc), \ .bInterfaceProtocol = (pr) +/** + * USB_VENDOR_AND_INTERFACE_INFO - describe a specific usb vendor with a class of usb interfaces + * @vend: the 16 bit USB Vendor ID + * @cl: bInterfaceClass value + * @sc: bInterfaceSubClass value + * @pr: bInterfaceProtocol value + * + * This macro is used to create a struct usb_device_id that matches a + * specific vendor with a specific class of interfaces. + * + * This is especially useful when explicitly matching devices that have + * vendor specific bDeviceClass values, but standards-compliant interfaces. + */ +#define USB_VENDOR_AND_INTERFACE_INFO(vend, cl, sc, pr) \ + .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \ + | USB_DEVICE_ID_MATCH_VENDOR, \ + .idVendor = (vend), \ + .bInterfaceClass = (cl), \ + .bInterfaceSubClass = (sc), \ + .bInterfaceProtocol = (pr) + /* --- */ /* Stuff for dynamic usb ids */ -- 1.7.10.2 -- 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: add USB_VENDOR_AND_INTERFACE_INFO() macro
Hi Gustavo, > A lot of Broadcom Bluetooth devices provides vendor specific interface > class and we are getting flooded by patches adding new device support. > This change will help us enable support for any other Broadcom with vendor > specific device that arrives in the future. > > Only the product id changes for those devices, so this macro would be > perfect for us: > > { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) } > > Signed-off-by: Gustavo Padovan > --- > include/linux/usb.h | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/include/linux/usb.h b/include/linux/usb.h > index dea39dc..dad156b 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -829,6 +829,27 @@ static inline int usb_make_path(struct usb_device *dev, > char *buf, size_t size) > .bInterfaceSubClass = (sc), \ > .bInterfaceProtocol = (pr) > > +/** > + * USB_VENDOR_AND_INTERFACE_INFO - describe a specific usb vendor with a > class of usb interfaces > + * @vend: the 16 bit USB Vendor ID > + * @cl: bInterfaceClass value > + * @sc: bInterfaceSubClass value > + * @pr: bInterfaceProtocol value > + * > + * This macro is used to create a struct usb_device_id that matches a > + * specific vendor with a specific class of interfaces. > + * > + * This is especially useful when explicitly matching devices that have > + * vendor specific bDeviceClass values, but standards-compliant interfaces. > + */ > +#define USB_VENDOR_AND_INTERFACE_INFO(vend, cl, sc, pr) \ > + .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \ > + | USB_DEVICE_ID_MATCH_DEVICE, \ this should be USB_DEVICE_ID_MATCH_VENDOR. > + .idVendor = (vend), \ > + .bInterfaceClass = (cl), \ > + .bInterfaceSubClass = (sc), \ > + .bInterfaceProtocol = (pr) > + > /* --- */ > > /* Stuff for dynamic usb ids */ Regards Marcel -- 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: add USB_VENDOR_AND_INTERFACE_INFO() macro
On Tue, Jul 10, 2012 at 06:51:03PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > A lot of Broadcom Bluetooth devices provides vendor specific interface > class and we are getting flooded by patches adding new device support. > This change will help us enable support for any other Broadcom with vendor > specific device that arrives in the future. > > Only the product id changes for those devices, so this macro would be > perfect for us: > > { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) } Two entries in the cdc_wdm driver can also be converted to use this interface. Do you want to send a patch doing that, or do you want me to do it? 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
[PATCH] USB: add USB_VENDOR_AND_INTERFACE_INFO() macro
From: Gustavo Padovan A lot of Broadcom Bluetooth devices provides vendor specific interface class and we are getting flooded by patches adding new device support. This change will help us enable support for any other Broadcom with vendor specific device that arrives in the future. Only the product id changes for those devices, so this macro would be perfect for us: { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) } Signed-off-by: Gustavo Padovan --- include/linux/usb.h | 21 + 1 file changed, 21 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index dea39dc..dad156b 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -829,6 +829,27 @@ static inline int usb_make_path(struct usb_device *dev, char *buf, size_t size) .bInterfaceSubClass = (sc), \ .bInterfaceProtocol = (pr) +/** + * USB_VENDOR_AND_INTERFACE_INFO - describe a specific usb vendor with a class of usb interfaces + * @vend: the 16 bit USB Vendor ID + * @cl: bInterfaceClass value + * @sc: bInterfaceSubClass value + * @pr: bInterfaceProtocol value + * + * This macro is used to create a struct usb_device_id that matches a + * specific vendor with a specific class of interfaces. + * + * This is especially useful when explicitly matching devices that have + * vendor specific bDeviceClass values, but standards-compliant interfaces. + */ +#define USB_VENDOR_AND_INTERFACE_INFO(vend, cl, sc, pr) \ + .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \ + | USB_DEVICE_ID_MATCH_DEVICE, \ + .idVendor = (vend), \ + .bInterfaceClass = (cl), \ + .bInterfaceSubClass = (sc), \ + .bInterfaceProtocol = (pr) + /* --- */ /* Stuff for dynamic usb ids */ -- 1.7.10.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: usb hub is n't recognised during boot up(sometimes)
Hi Peter, I tried to do some more debugging . I am getting a time out in port reset . Here I am copying the dmesg . Here is the error I am getting . ehci_hcd :00:1d.0: port 1 reset error -110 hub 2-0:1.0: hub_port_status failed (err = -32) > Error here . hub 2-0:1.0: port_wait_reset: err = -32 hub 2-0:1.0: port 1 not enabled, trying reset again... ehci_hcd :00:1d.0: USBCMD = 00010001 USBSTS = 2008 USBINTR = 0037 FR INDEX = 2d3a CTRLDSSEGMENT = PERIODICLISTBASE = 98006000 ASYNCLIS TADDR = 98004000 CONFIGFLAG = 0001 ehci_hcd :00:1d.0: PORTSC4096 = ehci_hcd :00:1d.0: PORTSC4096 = ehci_hcd :00:1d.0: PORTSC4353 = ehci_hcd :00:1d.0: port 1 full speed --> companion ehci_hcd :00:1d.0: GetStatus port 1 status 003801 POWER OWNER sig=j CONNECT hub 2-0:1.0: port 1 not reset yet, waiting 200ms ehci_hcd :00:1d.0: GetStatus port 1 status 003002 POWER OWNER sig=se0 CSC hub 1-1:1.0: state 7 ports 6 chg evt dmesg output -- nitializing cgroup subsys cpuset Initializing cgroup subsys cpu Linux version 2.6.32.39 (shshaiju@mcp-bld-lnx-92) (gcc version 4.4.1 (MontaVista ) ) #33 SMP Mon Jul 9 14:06:17 PDT 2012 Command line: root=/dev/ram rw console=ttyS1,9600 max_loop=64 wdt=0 SR_BOOT=boot flash:/ghaidiny/rp_super_universalk9.2kp.bin.shaiju KERNEL supported cpus: Intel GenuineIntel AMD AuthenticAMD Centaur CentaurHauls BIOS-provided physical RAM map: BIOS-e820: - 0008f000 (usable) BIOS-e820: 0008f000 - 0009 (ACPI NVS) BIOS-e820: 0009 - 000a (usable) BIOS-e820: 0010 - 6df16000 (usable) BIOS-e820: 6df16000 - 8421f000 (reserved) BIOS-e820: 8421f000 - 9f4bf000 (usable) BIOS-e820: 9f4bf000 - 9f6bf000 (reserved) BIOS-e820: 9f6bf000 - 9f7bf000 (ACPI NVS) BIOS-e820: 9f7bf000 - 9f7ef000 (ACPI data) BIOS-e820: 9f7ef000 - 9f80 (usable) BIOS-e820: e000 - f000 (reserved) BIOS-e820: fec0 - fec01000 (reserved) BIOS-e820: fed1c000 - fed2 (reserved) BIOS-e820: ffdf - (reserved) BIOS-e820: 0001 - 00016000 (usable) DMI not present or invalid. last_pfn = 0x16 max_arch_pfn = 0x4 MTRR default type: uncachable MTRR fixed ranges enabled: 0-9 write-back A-B uncachable C-E write-through F-F write-combining MTRR variable ranges enabled: 0 base 00 mask FF write-back 1 base 01 mask FFC000 write-back 2 base 014000 mask FFE000 write-back 3 base 00A000 mask FFE000 uncachable 4 base 00C000 mask FFC000 uncachable 5 disabled 6 disabled 7 disabled e820 update range: a000 - 0001 (usable) ==> (reserved) last_pfn = 0x9f800 max_arch_pfn = 0x4 initial memory mapped : 0 - 2000 init_memory_mapping: -9f80 00 - 009f80 page 2M kernel direct mapping tables up to 9f80 @ 8000-c000 init_memory_mapping: 0001-00016000 01 - 016000 page 2M kernel direct mapping tables up to 16000 @ a000-11000 RAMDISK: 6df16018 - 8421e818 ACPI: RSDP 000e 00024 (v02 INSYDE) ACPI: XSDT 9f7ee120 000AC (v01 INSYDE PicketPo 0001 0113) ACPI: FACP 9f7ec000 000F4 (v04 INSYDE PicketPo 0001 MSFT 0113) ACPI: DSDT 9f7e1000 076A5 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: FACS 9f729000 00040 ACPI: ASF! 9f7ed000 000A5 (v32 INSYDE PicketPo 0001 MSFT 0113) ACPI: HPET 9f7eb000 00038 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: APIC 9f7ea000 000EA (v02 INSYDE PicketPo 0001 MSFT 0113) ACPI: MCFG 9f7e9000 0003C (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: SLIC 9f7e 00176 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: BOOT 9f7dd000 00028 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: SSDT 9f7dc000 002F6 (v01 INTEL SataAhci 1000 INTL 20100121) ACPI: SRAT 9f7d9000 00400 (v02 INSYDE PicketPo 0001 MSFT 0113) ACPI: SLIT 9f7d8000 00030 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: WDAT 9f7d7000 00224 (v01 INSYDE PicketPo 0001 MSFT 0113) ACPI: DMAR 9f7d6000 00068 (v01 INTEL CP_FIELD 0001 INTL 0001) ACPI: SSDT 9f7d4000 01129 (v01 PmRefCpuPm 3000 INTL 20100121) ACPI: HEST 9f7d3000 000A8 (v01 INSYDE THURLEY 0001? 0001) ACPI: ERST 9f7d2000 00230 (v01 INSYDE THURLEY 0001? 0001) ACPI: BERT 000
Re: [PATCH v3 1/1] Input: xpad - Handle all variations of Mad Catz Beat Pad
Hi Yuri, On Wed, Jul 11, 2012 at 12:33:22AM +0700, Yuri Khan wrote: > * Add this device to usbhid ignore list Please do not forget your "Signed-off-by: " so that I can apply the patch. Thanks. > --- > drivers/hid/hid-core.c|1 + > drivers/hid/hid-ids.h |3 +++ > drivers/input/joystick/xpad.c |1 + > 3 files changed, 5 insertions(+) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 6ac0286..1540934 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1995,6 +1995,7 @@ static const struct hid_device_id hid_ignore_list[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_MCT) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) > }, > { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) }, > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index d1cdd2d..43c3d75 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -518,6 +518,9 @@ > #define USB_DEVICE_ID_CRYSTALTOUCH 0x0006 > #define USB_DEVICE_ID_CRYSTALTOUCH_DUAL 0x0007 > > +#define USB_VENDOR_ID_MADCATZ0x0738 > +#define USB_DEVICE_ID_MADCATZ_BEATPAD0x4540 > + > #define USB_VENDOR_ID_MCC0x09db > #define USB_DEVICE_ID_MCC_PMD1024LS 0x0076 > #define USB_DEVICE_ID_MCC_PMD1208LS 0x007a > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > index ee16fb6..16974ef 100644 > --- a/drivers/input/joystick/xpad.c > +++ b/drivers/input/joystick/xpad.c > @@ -238,6 +238,7 @@ static struct usb_device_id xpad_table [] = { > XPAD_XBOX360_VENDOR(0x045e),/* Microsoft X-Box 360 > controllers */ > XPAD_XBOX360_VENDOR(0x046d),/* Logitech X-Box 360 style > controllers */ > XPAD_XBOX360_VENDOR(0x0738),/* Mad Catz X-Box 360 > controllers */ > + { USB_DEVICE(0x0738, 0x4540) }, /* Mad Catz Beat Pad */ > XPAD_XBOX360_VENDOR(0x0e6f),/* 0x0e6f X-Box 360 controllers > */ > XPAD_XBOX360_VENDOR(0x12ab),/* X-Box 360 dance pads */ > XPAD_XBOX360_VENDOR(0x1430),/* RedOctane X-Box 360 > controllers */ > -- > 1.7.9.5 > -- Dmitry -- 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 v3 1/1] Input: xpad - Handle all variations of Mad Catz Beat Pad
On Wed, 11 Jul 2012, Yuri Khan wrote: > * Add this device to usbhid ignore list > --- > drivers/hid/hid-core.c|1 + > drivers/hid/hid-ids.h |3 +++ Acked-by: Jiri Kosina > drivers/input/joystick/xpad.c |1 + > 3 files changed, 5 insertions(+) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 6ac0286..1540934 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1995,6 +1995,7 @@ static const struct hid_device_id hid_ignore_list[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_MCT) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) > }, > { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) }, > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index d1cdd2d..43c3d75 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -518,6 +518,9 @@ > #define USB_DEVICE_ID_CRYSTALTOUCH 0x0006 > #define USB_DEVICE_ID_CRYSTALTOUCH_DUAL 0x0007 > > +#define USB_VENDOR_ID_MADCATZ0x0738 > +#define USB_DEVICE_ID_MADCATZ_BEATPAD0x4540 > + > #define USB_VENDOR_ID_MCC0x09db > #define USB_DEVICE_ID_MCC_PMD1024LS 0x0076 > #define USB_DEVICE_ID_MCC_PMD1208LS 0x007a > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c > index ee16fb6..16974ef 100644 > --- a/drivers/input/joystick/xpad.c > +++ b/drivers/input/joystick/xpad.c > @@ -238,6 +238,7 @@ static struct usb_device_id xpad_table [] = { > XPAD_XBOX360_VENDOR(0x045e),/* Microsoft X-Box 360 > controllers */ > XPAD_XBOX360_VENDOR(0x046d),/* Logitech X-Box 360 style > controllers */ > XPAD_XBOX360_VENDOR(0x0738),/* Mad Catz X-Box 360 > controllers */ > + { USB_DEVICE(0x0738, 0x4540) }, /* Mad Catz Beat Pad */ > XPAD_XBOX360_VENDOR(0x0e6f),/* 0x0e6f X-Box 360 controllers > */ > XPAD_XBOX360_VENDOR(0x12ab),/* X-Box 360 dance pads */ > XPAD_XBOX360_VENDOR(0x1430),/* RedOctane X-Box 360 > controllers */ > -- > 1.7.9.5 > -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb sound issue - any ideas?
Dear listmembers, dear Sarah, dear Andiry, Andiry's suggestion (uncomment the return 0 - statement) fixed my issue, the usb soundcard remains active after a login-logout; however, as I understood Sarah's comment this is no more than a plaster on the problem. So, I am willing to do more testing - but I depend on your inputs as I am lacking the required programming skills to actually understand what's happening within the driver. Please let me know what you'd suggest to do next ... Thank you very much, take care Dieter Jurzitza -- --- | \ /\_/\ | | ~x~ |/-\ / \ /- \_/ ^^__ _/ _ / <°°__ \- \_/ | |/| | || || _| _|_| _| if you really want to see the pictures above - use some font with constant spacing like courier! :-) --- -- 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: xhci_hcd: external drive not initialised if already connected during restart or cold boot
Lee Harris writes: > > > Hi Sarah > > Whenever I restart or coldboot, my external drive (3.5 sata in a USB3 > enclosure) is not detected / initialised correctly. fdisk -l does not > show the drive at all. > I have found that I have to: > turn it off ( or disconnect the usb cable) > modprobe -r xhci_hcd > modprobe xhci_hcd > and then turn it on. It is then found and initialised. I'm having this same issue. Does anyone have any ideas? -- Matt > -- 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 v3 1/1] Input: xpad - Handle all variations of Mad Catz Beat Pad
* Add this device to usbhid ignore list --- drivers/hid/hid-core.c|1 + drivers/hid/hid-ids.h |3 +++ drivers/input/joystick/xpad.c |1 + 3 files changed, 5 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 6ac0286..1540934 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1995,6 +1995,7 @@ static const struct hid_device_id hid_ignore_list[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_MCT) }, { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) }, { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) }, + { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index d1cdd2d..43c3d75 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -518,6 +518,9 @@ #define USB_DEVICE_ID_CRYSTALTOUCH 0x0006 #define USB_DEVICE_ID_CRYSTALTOUCH_DUAL0x0007 +#define USB_VENDOR_ID_MADCATZ 0x0738 +#define USB_DEVICE_ID_MADCATZ_BEATPAD 0x4540 + #define USB_VENDOR_ID_MCC 0x09db #define USB_DEVICE_ID_MCC_PMD1024LS0x0076 #define USB_DEVICE_ID_MCC_PMD1208LS0x007a diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c index ee16fb6..16974ef 100644 --- a/drivers/input/joystick/xpad.c +++ b/drivers/input/joystick/xpad.c @@ -238,6 +238,7 @@ static struct usb_device_id xpad_table [] = { XPAD_XBOX360_VENDOR(0x045e),/* Microsoft X-Box 360 controllers */ XPAD_XBOX360_VENDOR(0x046d),/* Logitech X-Box 360 style controllers */ XPAD_XBOX360_VENDOR(0x0738),/* Mad Catz X-Box 360 controllers */ + { USB_DEVICE(0x0738, 0x4540) }, /* Mad Catz Beat Pad */ XPAD_XBOX360_VENDOR(0x0e6f),/* 0x0e6f X-Box 360 controllers */ XPAD_XBOX360_VENDOR(0x12ab),/* X-Box 360 dance pads */ XPAD_XBOX360_VENDOR(0x1430),/* RedOctane X-Box 360 controllers */ -- 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] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, Jul 10, 2012 at 10:11 AM, Alan Stern wrote: > On Tue, 10 Jul 2012, Greg KH wrote: > >> > I'm fine with this patch. I was going to add these: >> > >> > Based-on-patch-by: AceLan Kao >> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 >> > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 >> > >> > I don't have the previous iteration (c2fb8a3fa25513d) in my "next" > > The commit is already in 3.5-rc6. And I intended this fix to get into > 3.5-final, not into Linux-next (I should have said so explicitly in the > patch). > >> > branch. I think it went through you, Greg. Do you want to handle >> > this one as well? >> >> I can easily take it if you don't want to. >> >> > I *could* do it, but it looks like a messy merge -- I think I'd have >> > to rebase almost everything in my "next" branch -- so I'd rather not. >> > Of course, I do have some D3-related updates in pci_prepare_to_sleep() >> > which will conflict with this, too, so I guess it will be a bit of >> > work for somebody either way. >> >> Ick, no, don't rebase anything. >> >> If I take this then we will have merge issues in linux-next, which we >> can work out, and then we will have the same issues for 3.6-rc1 as well. >> >> Or, Alan can redo the patch based on your next branch, which might make >> it easier for everyone involved? > > If Bjorn's "next" branch diverged from Linus's tree before c2fb8a3 was > added, that would make things more difficult. It did. My "next" branch is based on v3.5-rc2, and I think c2fb8a3 first appeared in -rc3. -- 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] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, Jul 10, 2012 at 10:17 AM, Greg KH wrote: > On Tue, Jul 10, 2012 at 12:11:41PM -0400, Alan Stern wrote: >> On Tue, 10 Jul 2012, Greg KH wrote: >> >> > > I'm fine with this patch. I was going to add these: >> > > >> > > Based-on-patch-by: AceLan Kao >> > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 >> > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 >> > > >> > > I don't have the previous iteration (c2fb8a3fa25513d) in my "next" >> >> The commit is already in 3.5-rc6. And I intended this fix to get into >> 3.5-final, not into Linux-next (I should have said so explicitly in the >> patch). > > Ah, ok, if so, and Bjorn doesn't mind, I can add it to the other USB > patches that I have queued up to go in before 3.5-final is released. > > Bjorn, want me to take it? If so, can I get your ack? Yes, please. Acked-by: Bjorn Helgaas -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1] usb: host: Fix possible kernel crash
Hi Greg/Venu/Alan and others, The defect discussed in this thread was found in 2006, and was marked in Coverity Scan as false positive - intentional ( by linux developer or coverity admin that we don't know)... As a general rule, 1. what was discussed with some of the Linux folks, Focus on NEW defects... 2. Do NOT fix anything that is already marked as Intentional /False Positive For any defects found by Covierty Scan there could be potential 3 actions 1. Review and Fix the defect 2. Mark it as Intentional in Coverity Scan, and it will not appear as Defect in the future 3. Contact Coverity Scan-admin, if you would like to understand it more why it was flagged as defect. • As we all know, inherent in the technology foundation, static analysis will report some false positives. We put a lot of effort into our product to drive this rate to a very low, acceptable limit (commonly between 5% and 25%) • In order to address FPs, the SCAN part of our product offers a triage process that utilizes a persistent database based on defect hashing. This gives developers the option to declare a defect as FP and then never have to look at it again. For instance, 3 weeks ago, Coverity reported 7 new defects introduced based on recent code changes, and as we can see in the various threads almost everything was fixed and committed by maintainer. But a week after that, out of 6 new defects reported based on latest code change, 1 was ignored stating False positive, Intentional... I hope this clarifies the issue that was discussed here. Thanks Coverity Scan-admin scan-ad...@coverity.com Dakshesh Vyas | Technical Manager - SCAN Coverity | 185 Berry Street | Suite 6500, Lobby 3 | San Francisco, CA 94107 Office: 415.935.2957 | dv...@coverity.com From: linux-kernel-ow...@vger.kernel.org [linux-kernel-ow...@vger.kernel.org] On Behalf Of gre...@linuxfoundation.org [gre...@linuxfoundation.org] Sent: Tuesday, July 10, 2012 7:45 AM To: Venu Byravarasu Cc: Alan Stern; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH v1] usb: host: Fix possible kernel crash On Tue, Jul 10, 2012 at 09:56:39AM +0530, Venu Byravarasu wrote: > Thanks Alan for your comments. > > On Monday 09 July 2012 08:04 PM, Alan Stern wrote: > >On Mon, 9 Jul 2012, Venu Byravarasu wrote: > > > >>In functions itd_complete & sitd_complete, a pointer > >>by name stream may get dereferenced after freeing it, when > >>iso_stream_put is called with stream->refcount = 2. > >I don't understand the problem. Did you actually see this happen or is > >it only theoretical? > > Yes it is a theoretical problem, as complained by Coverity. > > > /* for each uframe with a packet */ > > for (uframe = 0; uframe < 8; uframe++) { > >@@ -1783,7 +1784,8 @@ itd_complete ( > > dev->devpath, stream->bEndpointAddress & 0x0f, > > (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out"); > > } > >-iso_stream_put (ehci, stream); > >+stream_ref_count = stream->refcount; > >+iso_stream_put(ehci, stream); > >This iso_stream_put removes the reference held by the URB. Before it > >is called, stream->refcount must be >= 3: > > > > refcount is set to 1 when the stream is created; > > > > each active URB holds a reference; > > > > each itd holds a reference. > > > >So after the call, the refcount value must be >= 2 and the stream could > >not have been deallocated. > > > >> done: > >>itd->urb = NULL; > >>@@ -1797,7 +1799,7 @@ done: > >> * Move it to a safe place until a new frame starts. > >> */ > >>list_move(&itd->itd_list, &ehci->cached_itd_list); > >>- if (stream->refcount == 2) { > >>+ if (stream_ref_count == 3) { > >Therefore this seems unnecessary. > > As per the logic you explained above, this change is not needed. > However coverity was complaining as below: > > /kernel/drivers/usb/host/ehci-sched.c 1777 USE_AFTER_FREE > Dereferencing freed pointer "stream" > > Hence to pacify coverity, this change is done. Why are you trying to "pacify" coverity, when the tool is wrong in this case? Go poke the owners of that tool to get it to stop emitting this false warning. Don't paper over it in the kernel. Especially for a tool that none of us can run on our own. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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: NULL pointer dereference in at91_udc on start of connection
Hi, I put a printk before the check and this is what I got udc: at91_udc version 3 May 2006 g_ether gadget: using random host ethernet address usb0: MAC a2:46:1a:43:ea:9f usb0: HOST MAC de:bd:69:82:1d:68 g_ether gadget: Ethernet Gadget, version: Memorial Day 2008 g_ether gadget: g_ether ready TCP: cubic registered NET: Registered protocol family 17 input: gpio-keys as /devices/platform/gpio-keys/input/input0 g_ether gadget: full-speed config #2: RNDIS In at91_ep_enable. desc=c38a2350, ep->ep.desc=c38a2350 bad ep or descriptor In at91_ep_enable. desc=c38a2360, ep->ep.desc=c38a2360 bad ep or descriptor [ cut here ] WARNING: at drivers/usb/gadget/u_ether.c:941 rndis_disable+0x28/0x48() [] (unwind_backtrace+0x0/0xe4) from [] (warn_slowpath_common+0x48/0x60) [] (warn_slowpath_common+0x48/0x60) from [] (warn_slowpath_null+0x18/0x1c) [] (warn_slowpath_null+0x18/0x1c) from [] (rndis_disable+0x28/0x48) [] (rndis_disable+0x28/0x48) from [] (reset_config.clone.32+0x30/0x5c) [] (reset_config.clone.32+0x30/0x5c) from [] (composite_setup+0x6cc/0xa0c) [] (composite_setup+0x6cc/0xa0c) from [] (at91_udc_irq+0x5dc/0x79c) [] (at91_udc_irq+0x5dc/0x79c) from [] (handle_irq_event_percpu+0x30/0x1a0) [] (handle_irq_event_percpu+0x30/0x1a0) from [] (handle_irq_event+0x28/0x38) [] (handle_irq_event+0x28/0x38) from [] (handle_level_irq+0xbc/0xcc) [] (handle_level_irq+0xbc/0xcc) from [] (generic_handle_irq+0x30/0x44) [] (generic_handle_irq+0x30/0x44) from [] (handle_IRQ+0x60/0x98) [] (handle_IRQ+0x60/0x98) from [] (__irq_svc+0x38/0x60) [] (__irq_svc+0x38/0x60) from [] (default_idle+0x2c/0x34) [] (default_idle+0x2c/0x34) from [] (cpu_idle+0x7c/0xdc) [] (cpu_idle+0x7c/0xdc) from [] (start_kernel+0x23c/0x27c) [] (start_kernel+0x23c/0x27c) from [<20008040>] (0x20008040) ---[ end trace 23c64a1fe388174b ]--- This is after reverting commit f3d8bf34c2c925867322197096ed501ceab8085a but without changing the "|| !desc || ep->ep.desc" line in at91_udc.c Either removing the line as Fabio suggested or adding the ! makes it work. As soon as i have time i will try to figure out why the above warning happens. Best regards, Mário Isidoro -Original Message- From: Sebastian Andrzej Siewior [mailto:sebast...@breakpoint.cc] Sent: terça-feira, 10 de Julho de 2012 16:37 To: Mario Jorge Isidoro Cc: Fabio Porcedda; Sebastian Andrzej Siewior; ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; Nicolas Ferre; Ido Shayevitz; Jean-Christophe PLAGNIOL-VILLARD Subject: Re: NULL pointer dereference in at91_udc on start of connection On Tue, Jul 10, 2012 at 03:54:06PM +0100, Mario Jorge Isidoro wrote: > I've found that the following change also works, if someone doesn't want to > simply eliminate the check > diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c > index 7687ccd..33a6999 100644 > --- a/drivers/usb/gadget/at91_udc.c > +++ b/drivers/usb/gadget/at91_udc.c > @@ -475,7 +475,7 @@ static int at91_ep_enable(struct usb_ep *_ep, > unsigned long flags; > > if (!_ep || !ep > - || !desc || ep->ep.desc > + || !desc || !ep->ep.desc This check ensures that you do not try to enable an endpoint twice. Once enabled, ep->ep.desc should be set. > || _ep->name == ep0name ep.desc is always NULL for ep0 and this one should not be enabled. Therefore you have this check here. > || desc->bDescriptorType != USB_DT_ENDPOINT > || (maxpacket = usb_endpoint_maxp(desc)) == 0 That means with this change you should not get any endpoints enabled and it should not work at all. Can you acknowledge this? The crash happens in composite_setup() mind to figure out what is beeing accessed here? > Best regards, > Mário Isidoro > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: remove 8 bytes of padding from usb_host_interface on 64 bit builds
Reorder elements in the usb_host_interface structure to remove 8 bytes of padding on 64 bit builds , and so shrink it's size to 40 bytes. usb_interface_descriptor is a odd size which leaves a gap that is not big enough to hold a pointer, so moving extralen into that gap removes the need for more padding. Signed-off-by: Richard Kennedy --- patch against v3.5-rc6 compiled and tested on x86_64 I've been running this patch for several weeks with no obvious problems. This will allow the usb_host_interface array to be a bit smaller and should better align with the cachelines. regards Richard diff --git a/include/linux/usb.h b/include/linux/usb.h index dea39dc..2abec02 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -77,14 +77,14 @@ struct usb_host_endpoint { struct usb_host_interface { struct usb_interface_descriptor desc; + int extralen; + unsigned char *extra; /* Extra descriptors */ /* array of desc.bNumEndpoint endpoints associated with this * interface setting. these will be in no particular order. */ struct usb_host_endpoint *endpoint; char *string; /* iInterface string, if present */ - unsigned char *extra; /* Extra descriptors */ - int extralen; }; enum usb_interface_condition { -- 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] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, Jul 10, 2012 at 12:11:41PM -0400, Alan Stern wrote: > On Tue, 10 Jul 2012, Greg KH wrote: > > > > I'm fine with this patch. I was going to add these: > > > > > > Based-on-patch-by: AceLan Kao > > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 > > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 > > > > > > I don't have the previous iteration (c2fb8a3fa25513d) in my "next" > > The commit is already in 3.5-rc6. And I intended this fix to get into > 3.5-final, not into Linux-next (I should have said so explicitly in the > patch). Ah, ok, if so, and Bjorn doesn't mind, I can add it to the other USB patches that I have queued up to go in before 3.5-final is released. Bjorn, want me to take it? If so, can I get your ack? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, 10 Jul 2012, Greg KH wrote: > > I'm fine with this patch. I was going to add these: > > > > Based-on-patch-by: AceLan Kao > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 > > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 > > > > I don't have the previous iteration (c2fb8a3fa25513d) in my "next" The commit is already in 3.5-rc6. And I intended this fix to get into 3.5-final, not into Linux-next (I should have said so explicitly in the patch). > > branch. I think it went through you, Greg. Do you want to handle > > this one as well? > > I can easily take it if you don't want to. > > > I *could* do it, but it looks like a messy merge -- I think I'd have > > to rebase almost everything in my "next" branch -- so I'd rather not. > > Of course, I do have some D3-related updates in pci_prepare_to_sleep() > > which will conflict with this, too, so I guess it will be a bit of > > work for somebody either way. > > Ick, no, don't rebase anything. > > If I take this then we will have merge issues in linux-next, which we > can work out, and then we will have the same issues for 3.6-rc1 as well. > > Or, Alan can redo the patch based on your next branch, which might make > it easier for everyone involved? If Bjorn's "next" branch diverged from Linus's tree before c2fb8a3 was added, that would make things more difficult. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, Jul 10, 2012 at 09:32:59AM -0600, Bjorn Helgaas wrote: > On Mon, Jul 9, 2012 at 10:47 AM, Greg KH wrote: > > On Mon, Jul 09, 2012 at 06:50:24PM +0200, Rafael J. Wysocki wrote: > >> On Monday, July 09, 2012, Alan Stern wrote: > >> > Quite a few ASUS computers experience a nasty problem, related to the > >> > EHCI controllers, when going into system suspend. It was observed > >> > that the problem didn't occur if the controllers were not put into the > >> > D3 power state before starting the suspend, and commit > >> > 151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during > >> > suspend on ASUS computers) was created to do this. > >> > > >> > It turned out this approach messed up other computers that didn't have > >> > the problem -- it prevented USB wakeup from working. Consequently > >> > commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add > >> > NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it > >> > reverted the earlier commit and added a whitelist of known good board > >> > names. > >> > > >> > Now we know the actual cause of the problem. Thanks to AceLan Kao for > >> > tracking it down. > >> > > >> > According to him, an engineer at ASUS explained that some of their > >> > BIOSes contain a bug that was added in an attempt to work around a > >> > problem in early versions of Windows. When the computer goes into S3 > >> > suspend, the BIOS tries to verify that the EHCI controllers were first > >> > quiesced by the OS. Nothing's wrong with this, but the BIOS does it > >> > by checking that the PCI COMMAND registers contain 0 without checking > >> > the controllers' power state. If the register isn't 0, the BIOS > >> > assumes the controller needs to be quiesced and tries to do so. This > >> > involves making various MMIO accesses to the controller, which don't > >> > work very well if the controller is already in D3. The end result is > >> > a system hang or memory corruption. > >> > > >> > Since the value in the PCI COMMAND register doesn't matter once the > >> > controller has been suspended, and since the value will be restored > >> > anyway when the controller is resumed, we can work around the BIOS bug > >> > simply by setting the register to 0 during system suspend. This patch > >> > (as1590) does so and also reverts the second commit mentioned above, > >> > which is now unnecessary. > >> > > >> > In theory we could do this for every PCI device. However to avoid > >> > introducing new problems, the patch restricts itself to EHCI host > >> > controllers. > >> > > >> > Finally the affected systems can suspend with USB wakeup working > >> > properly. > >> > > >> > Signed-off-by: Alan Stern > >> > Tested-by: Dâniel Fraga > >> > Tested-by: Javier Marcet > >> > Tested-by: Andrey Rahmatullin > >> > Tested-by: Oleksij Rempel > >> > Tested-by: Pavel Pisa > >> > CC: > >> > >> Acked-by: Rafael J. Wysocki > > > > Acked-by: Greg Kroah-Hartman > > I'm fine with this patch. I was going to add these: > > Based-on-patch-by: AceLan Kao > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 > Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 > > I don't have the previous iteration (c2fb8a3fa25513d) in my "next" > branch. I think it went through you, Greg. Do you want to handle > this one as well? I can easily take it if you don't want to. > I *could* do it, but it looks like a messy merge -- I think I'd have > to rebase almost everything in my "next" branch -- so I'd rather not. > Of course, I do have some D3-related updates in pci_prepare_to_sleep() > which will conflict with this, too, so I guess it will be a bit of > work for somebody either way. Ick, no, don't rebase anything. If I take this then we will have merge issues in linux-next, which we can work out, and then we will have the same issues for 3.6-rc1 as well. Or, Alan can redo the patch based on your next branch, which might make it easier for everyone involved? Which ever you want to do is fine with me. 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: NULL pointer dereference in at91_udc on start of connection
On Tue, Jul 10, 2012 at 03:54:06PM +0100, Mario Jorge Isidoro wrote: > I've found that the following change also works, if someone doesn't want to > simply eliminate the check > diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c > index 7687ccd..33a6999 100644 > --- a/drivers/usb/gadget/at91_udc.c > +++ b/drivers/usb/gadget/at91_udc.c > @@ -475,7 +475,7 @@ static int at91_ep_enable(struct usb_ep *_ep, > unsigned long flags; > > if (!_ep || !ep > - || !desc || ep->ep.desc > + || !desc || !ep->ep.desc This check ensures that you do not try to enable an endpoint twice. Once enabled, ep->ep.desc should be set. > || _ep->name == ep0name ep.desc is always NULL for ep0 and this one should not be enabled. Therefore you have this check here. > || desc->bDescriptorType != USB_DT_ENDPOINT > || (maxpacket = usb_endpoint_maxp(desc)) == 0 That means with this change you should not get any endpoints enabled and it should not work at all. Can you acknowledge this? The crash happens in composite_setup() mind to figure out what is beeing accessed here? > Best regards, > Mário Isidoro > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Mon, Jul 9, 2012 at 10:47 AM, Greg KH wrote: > On Mon, Jul 09, 2012 at 06:50:24PM +0200, Rafael J. Wysocki wrote: >> On Monday, July 09, 2012, Alan Stern wrote: >> > Quite a few ASUS computers experience a nasty problem, related to the >> > EHCI controllers, when going into system suspend. It was observed >> > that the problem didn't occur if the controllers were not put into the >> > D3 power state before starting the suspend, and commit >> > 151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during >> > suspend on ASUS computers) was created to do this. >> > >> > It turned out this approach messed up other computers that didn't have >> > the problem -- it prevented USB wakeup from working. Consequently >> > commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add >> > NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it >> > reverted the earlier commit and added a whitelist of known good board >> > names. >> > >> > Now we know the actual cause of the problem. Thanks to AceLan Kao for >> > tracking it down. >> > >> > According to him, an engineer at ASUS explained that some of their >> > BIOSes contain a bug that was added in an attempt to work around a >> > problem in early versions of Windows. When the computer goes into S3 >> > suspend, the BIOS tries to verify that the EHCI controllers were first >> > quiesced by the OS. Nothing's wrong with this, but the BIOS does it >> > by checking that the PCI COMMAND registers contain 0 without checking >> > the controllers' power state. If the register isn't 0, the BIOS >> > assumes the controller needs to be quiesced and tries to do so. This >> > involves making various MMIO accesses to the controller, which don't >> > work very well if the controller is already in D3. The end result is >> > a system hang or memory corruption. >> > >> > Since the value in the PCI COMMAND register doesn't matter once the >> > controller has been suspended, and since the value will be restored >> > anyway when the controller is resumed, we can work around the BIOS bug >> > simply by setting the register to 0 during system suspend. This patch >> > (as1590) does so and also reverts the second commit mentioned above, >> > which is now unnecessary. >> > >> > In theory we could do this for every PCI device. However to avoid >> > introducing new problems, the patch restricts itself to EHCI host >> > controllers. >> > >> > Finally the affected systems can suspend with USB wakeup working >> > properly. >> > >> > Signed-off-by: Alan Stern >> > Tested-by: Dâniel Fraga >> > Tested-by: Javier Marcet >> > Tested-by: Andrey Rahmatullin >> > Tested-by: Oleksij Rempel >> > Tested-by: Pavel Pisa >> > CC: >> >> Acked-by: Rafael J. Wysocki > > Acked-by: Greg Kroah-Hartman I'm fine with this patch. I was going to add these: Based-on-patch-by: AceLan Kao Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 I don't have the previous iteration (c2fb8a3fa25513d) in my "next" branch. I think it went through you, Greg. Do you want to handle this one as well? I *could* do it, but it looks like a messy merge -- I think I'd have to rebase almost everything in my "next" branch -- so I'd rather not. Of course, I do have some D3-related updates in pci_prepare_to_sleep() which will conflict with this, too, so I guess it will be a bit of work for somebody either way. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: twl6030-usb: variable otg not declared in twl6030_usbotg_irq() in linux-next
Yes, ignore my patch. It was applied to usb-next already and I forgot to check there. Jerry On Tue Jul 10 12, Greg KH wrote: > On Mon, Jul 09, 2012 at 11:32:20PM -0700, Gerard Snitselaar wrote: > > commit ff9cce82 added back 2 lines that were removed by commit > > c83a8542 causing build of twl6030-usb to get an error due to otg being > > referenced, but not declared. This patch removes those 2 lines again > > to restore intent of commit c83a8542. > > > > Signed-off-by: Gerard Snitselaar > > --- > > drivers/usb/otg/twl6030-usb.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c > > index a3d0c04..6907d8d 100644 > > --- a/drivers/usb/otg/twl6030-usb.c > > +++ b/drivers/usb/otg/twl6030-usb.c > > @@ -302,8 +302,6 @@ static irqreturn_t twl6030_usbotg_irq(int irq, void > > *_twl) > > twl6030_writeb(twl, TWL_MODULE_USB, 0x1, USB_ID_INT_EN_HI_CLR); > > twl6030_writeb(twl, TWL_MODULE_USB, 0x10, USB_ID_INT_EN_HI_SET); > > status = OMAP_MUSB_ID_GROUND; > > - otg->default_a = true; > > - twl->phy.state = OTG_STATE_A_IDLE; > > Didn't I apply this same patch yesterday? Can you verify that this is > still needed in the usb-next branch of my git tree? > > confused, > > 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 v1] usb: host: Fix possible kernel crash
On Tue, 10 Jul 2012, Venu Byravarasu wrote: > Thanks Alan for your comments. > > On Monday 09 July 2012 08:04 PM, Alan Stern wrote: > > On Mon, 9 Jul 2012, Venu Byravarasu wrote: > > > >> In functions itd_complete & sitd_complete, a pointer > >> by name stream may get dereferenced after freeing it, when > >> iso_stream_put is called with stream->refcount = 2. > > I don't understand the problem. Did you actually see this happen or is > > it only theoretical? > > Yes it is a theoretical problem, as complained by Coverity. > As per the logic you explained above, this change is not needed. > However coverity was complaining as below: > > /kernel/drivers/usb/host/ehci-sched.c 1777 USE_AFTER_FREE Dereferencing > freed pointer "stream" > > Hence to pacify coverity, this change is done. > Please let me know if you see any other better way to handle it. This seems to be a false positive from Coverity. In any case, I'm about to submit some patches which get rid of the reference counting entirely. So let's not worry about this. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 REBASE 6/9] USB: notify phy when root hub port connect change
On Tue, 10 Jul 2012, Richard Zhao wrote: > On Sat, Jul 07, 2012 at 10:56:45PM +0800, Richard Zhao wrote: > > Phy may need to change settings when port connect change. > > > > Signed-off-by: Richard Zhao > > Tested-by: Subodh Nijsure > > --- > > drivers/usb/core/hub.c |8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 4cc8dc9..2ba9d84 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -20,6 +20,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub > > *hub, int port1, > > } > > } > > > > + if (unlikely(hcd->phy && !hdev->parent)) { > > + if (portstatus & USB_PORT_STAT_CONNECTION) > > + usb_phy_notify_connect(hcd->phy, port1); > > + else > > + usb_phy_notify_disconnect(hcd->phy, port1); > There's another issue. When hcd is removed, notify disconnect is not > called. Is it ok, if I remove the above two line and add below patch: You should keep those two lines, since it's possible for a disconnect to occur without usb_disconnect() being called. This happens when a device fails to enumerate. > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev) >*/ > device_del(&udev->dev); > > + if (udev->parent && !udev->parent->parent) { > + struct usb_hcd *hcd = bus_to_hcd(udev->bus); > + usb_phy_notify_disconnect(hcd->phy, udev->portnum); > + } You might not want to add this. usb_disconnect() can get called in situations where the device remains physically connected (if the firmware changes during a reset, for example). If you want to notify the phy when the root hub is removed, there are better places to do it. One possibility is in hub_quiesce(), another is in usb_remove_hcd(). 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: NULL pointer dereference in at91_udc on start of connection
Hi, I can confirm that with the latest rc and the alterations you mentioned also solve the problem for me I've found that the following change also works, if someone doesn't want to simply eliminate the check diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 7687ccd..33a6999 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -475,7 +475,7 @@ static int at91_ep_enable(struct usb_ep *_ep, unsigned long flags; if (!_ep || !ep - || !desc || ep->ep.desc + || !desc || !ep->ep.desc || _ep->name == ep0name || desc->bDescriptorType != USB_DT_ENDPOINT || (maxpacket = usb_endpoint_maxp(desc)) == 0 Best regards, Mário Isidoro -Original Message- From: Fabio Porcedda [mailto:fabio.porce...@gmail.com] Sent: terça-feira, 10 de Julho de 2012 10:04 To: Mario Jorge Isidoro; Sebastian Andrzej Siewior Cc: ba...@ti.com; gre...@linuxfoundation.org; linux-usb@vger.kernel.org; Nicolas Ferre; Ido Shayevitz; Jean-Christophe PLAGNIOL-VILLARD Subject: Re: NULL pointer dereference in at91_udc on start of connection On Fri, Jul 6, 2012 at 8:06 PM, Mario Jorge Isidoro wrote: > Hi Fabio, > > I tried 3.4 and you were right, it still works fine. > > For the 'struct at91_ep' has no member named 'desc' error I tried commenting > the offending declaration (|| ep->desc) > and it builds without any error. During the bisect run this happened several > times and I think, but am not sure, that some of > this attempts worked fine without displaying the original error. I'm happy to say that with the v3.5-rc6, after reverting the commit f3d8bf34c2c925867322197096ed501ceab8085a and removing the following line: diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 7687ccd..98339a2 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -475,7 +475,6 @@ static int at91_ep_enable(struct usb_ep *_ep, unsigned long flags; if (!_ep || !ep - || !desc || ep->ep.desc || _ep->name == ep0name || desc->bDescriptorType != USB_DT_ENDPOINT || (maxpacket = usb_endpoint_maxp(desc)) == 0 Now the at91_udc driver with g_ether it's working again. I don't understand the reason, but now it seems to work fine. If there isn't a better solution i propose to revert the commit f3d8bf34c2c925867322197096ed501ceab8085a and remove the line on the at91_udc driver. Best regards -- Fabio Porcedda -- 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change
On Tue, Jul 10, 2012 at 10:24:07AM -0400, Alan Stern wrote: > On Tue, 10 Jul 2012, Richard Zhao wrote: > > > > > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct > > > > usb_hub *hub, int port1, > > > > } > > > > } > > > > > > > > + if (unlikely(hcd->phy && !hdev->parent)) { > > > > > > Why is this "unlikely"? And why mark it as such, is this a "fast path" > > > that needs the compiler to know this hint to optimize things here? > > > > > > Please don't use likely() or unlikely() except in places it really is > > > needed, _and_ you have measured the difference. Have you done so in > > > this place? > > It's from a comment by Alan Stern. > > http://www.spinics.net/lists/linux-usb/msg64987.html > > That comment was made in a somewhat different context. At the time > the code was part of an interrupt handler; now it isn't. > > > Actually, for my board, it's not unlikely. But for others which don't > > have notify_connect/disconnect, it's unlikely. > > > > Because it's not unlikely for all boards, I prefer remove "unlikely". > > It's no longer a big deal one way or another. I don't care about the > "unlikely" because it's on a cold path running in process context. Go > ahead and remove it. Ok. I'll remove it. Thanks Richard > > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] usb: host: Fix possible kernel crash
On Tue, Jul 10, 2012 at 09:56:39AM +0530, Venu Byravarasu wrote: > Thanks Alan for your comments. > > On Monday 09 July 2012 08:04 PM, Alan Stern wrote: > >On Mon, 9 Jul 2012, Venu Byravarasu wrote: > > > >>In functions itd_complete & sitd_complete, a pointer > >>by name stream may get dereferenced after freeing it, when > >>iso_stream_put is called with stream->refcount = 2. > >I don't understand the problem. Did you actually see this happen or is > >it only theoretical? > > Yes it is a theoretical problem, as complained by Coverity. > > > /* for each uframe with a packet */ > > for (uframe = 0; uframe < 8; uframe++) { > >@@ -1783,7 +1784,8 @@ itd_complete ( > > dev->devpath, stream->bEndpointAddress & 0x0f, > > (stream->bEndpointAddress & USB_DIR_IN) ? "in" : "out"); > > } > >-iso_stream_put (ehci, stream); > >+stream_ref_count = stream->refcount; > >+iso_stream_put(ehci, stream); > >This iso_stream_put removes the reference held by the URB. Before it > >is called, stream->refcount must be >= 3: > > > > refcount is set to 1 when the stream is created; > > > > each active URB holds a reference; > > > > each itd holds a reference. > > > >So after the call, the refcount value must be >= 2 and the stream could > >not have been deallocated. > > > >> done: > >>itd->urb = NULL; > >>@@ -1797,7 +1799,7 @@ done: > >> * Move it to a safe place until a new frame starts. > >> */ > >>list_move(&itd->itd_list, &ehci->cached_itd_list); > >>- if (stream->refcount == 2) { > >>+ if (stream_ref_count == 3) { > >Therefore this seems unnecessary. > > As per the logic you explained above, this change is not needed. > However coverity was complaining as below: > > /kernel/drivers/usb/host/ehci-sched.c 1777 USE_AFTER_FREE > Dereferencing freed pointer "stream" > > Hence to pacify coverity, this change is done. Why are you trying to "pacify" coverity, when the tool is wrong in this case? Go poke the owners of that tool to get it to stop emitting this false warning. Don't paper over it in the kernel. Especially for a tool that none of us can run on our own. 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] usb: twl6030-usb: variable otg not declared in twl6030_usbotg_irq() in linux-next
On Mon, Jul 09, 2012 at 11:32:20PM -0700, Gerard Snitselaar wrote: > commit ff9cce82 added back 2 lines that were removed by commit > c83a8542 causing build of twl6030-usb to get an error due to otg being > referenced, but not declared. This patch removes those 2 lines again > to restore intent of commit c83a8542. > > Signed-off-by: Gerard Snitselaar > --- > drivers/usb/otg/twl6030-usb.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c > index a3d0c04..6907d8d 100644 > --- a/drivers/usb/otg/twl6030-usb.c > +++ b/drivers/usb/otg/twl6030-usb.c > @@ -302,8 +302,6 @@ static irqreturn_t twl6030_usbotg_irq(int irq, void *_twl) > twl6030_writeb(twl, TWL_MODULE_USB, 0x1, USB_ID_INT_EN_HI_CLR); > twl6030_writeb(twl, TWL_MODULE_USB, 0x10, USB_ID_INT_EN_HI_SET); > status = OMAP_MUSB_ID_GROUND; > - otg->default_a = true; > - twl->phy.state = OTG_STATE_A_IDLE; Didn't I apply this same patch yesterday? Can you verify that this is still needed in the usb-next branch of my git tree? confused, 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: [RFC v2] USB: Add a sysfs file to show LTM capabilities.
On Tue, Jul 10, 2012 at 07:50:30AM -0400, Sarah Sharp wrote: > USB 3.0 devices can optionally support Latency Tolerance Messaging > (LTM). Add a new sysfs file in the device directory to show whether a > device is LTM capable. This file will be present for both USB 2.0 and > USB 3.0 devices. > > Signed-off-by: Sarah Sharp > --- > > > v2: Add ABI documentation. > > I think this is only change needed for the LPM bug fixes and LTM > patchset. I did want to change the "xhci" prefix on the first two > patches to "USB", since they touch the USB core and not the xHCI driver. > However, I don't think that warrants a second revision to the mailing > list. > > Greg, let me know if the ABI documentation seems ok, and I'll send you a > pull request. Looks good to me, thanks for adding it. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 REBASE 6/9] USB: notify phy when root hub port connect change
On Tue, 10 Jul 2012, Richard Zhao wrote: > > > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub > > > *hub, int port1, > > > } > > > } > > > > > > + if (unlikely(hcd->phy && !hdev->parent)) { > > > > Why is this "unlikely"? And why mark it as such, is this a "fast path" > > that needs the compiler to know this hint to optimize things here? > > > > Please don't use likely() or unlikely() except in places it really is > > needed, _and_ you have measured the difference. Have you done so in > > this place? > It's from a comment by Alan Stern. > http://www.spinics.net/lists/linux-usb/msg64987.html That comment was made in a somewhat different context. At the time the code was part of an interrupt handler; now it isn't. > Actually, for my board, it's not unlikely. But for others which don't > have notify_connect/disconnect, it's unlikely. > > Because it's not unlikely for all boards, I prefer remove "unlikely". It's no longer a big deal one way or another. I don't care about the "unlikely" because it's on a cold path running in process context. Go ahead and remove it. 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: How to get drivers/usb/musb/omap2430.c built into the kernel, not as module? (bug in kconfig?)
+wider auditory (linux-kernel) -- Forwarded message -- From: Ruslan Bilovol Date: Fri, Jul 6, 2012 at 7:53 PM Subject: How to get drivers/usb/musb/omap2430.c built into the kernel, not as module? (bug in kconfig?) To: linux-usb@vger.kernel.org Hi all, It seems we have an issue in kconfig (or I missed something). I need drivers/usb/musb/omap2430.c to be built into the kernel. For this, I enable next option in the defconfig: CONFIG_USB_MUSB_OMAP2PLUS=y It's clear (from the drivers/usb/musb/Makefile) that it allows build of that file: obj-$(CONFIG_USB_MUSB_OMAP2PLUS) += omap2430.o In the drivers/usb/musb/Makefile we have next: choice prompt "Platform Glue Layer" [...] config USB_MUSB_OMAP2PLUS tristate "OMAP2430 and onwards" depends on ARCH_OMAP2PLUS [...] endchoice But suddenly after make ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabi- omap2plus_defconfig I see that it is defined as module (CONFIG_USB_MUSB_OMAP2PLUS=m) in my .config I have slightly rootcaused this and find that looks like we have an issue with kconfig. Do you have any idea or observations on this? After next fix it works as expected: diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 22a3c40..6a03622 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -273,8 +273,6 @@ static struct symbol *sym_calc_choice(struct symbol *sym) flags &= def_sym->flags; } - sym->flags &= flags | ~SYMBOL_DEF_USER; - /* is the user choice visible? */ def_sym = sym->def[S_DEF_USER].val; if (def_sym && def_sym->visible != no) This fix changes only state of CONFIG_USB_MUSB_OMAP2PLUS in the final .config. Also it works as expected if I change tristate to bool for all choices under "Platform Glue Layer" My custom omap2plus_defconfig: CONFIG_EXPERIMENTAL=y CONFIG_SYSVIPC=y CONFIG_POSIX_MQUEUE=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=16 CONFIG_BLK_DEV_INITRD=y CONFIG_EXPERT=y # CONFIG_SYSCTL_SYSCALL is not set CONFIG_KALLSYMS_EXTRA_PASS=y CONFIG_SLAB=y CONFIG_PROFILING=y CONFIG_OPROFILE=y CONFIG_KPROBES=y CONFIG_MODULES=y CONFIG_MODULE_FORCE_LOAD=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_FORCE_UNLOAD=y CONFIG_MODVERSIONS=y CONFIG_MODULE_SRCVERSION_ALL=y # CONFIG_BLK_DEV_BSG is not set CONFIG_ARCH_OMAP=y CONFIG_OMAP_RESET_CLOCKS=y CONFIG_OMAP_MUX_DEBUG=y CONFIG_ARM_THUMBEE=y CONFIG_ARM_ERRATA_411920=y CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y CONFIG_SMP=y CONFIG_NR_CPUS=2 CONFIG_LEDS=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_CMDLINE="init=/sbin/init console=ttyO2,115200n8 mem=1G vmalloc=768M omap_wdt.timer_margin=30" CONFIG_CMDLINE_EXTEND=y CONFIG_KEXEC=y CONFIG_FPE_NWFPE=y CONFIG_BINFMT_MISC=y CONFIG_PM_DEBUG=y CONFIG_NET=y CONFIG_PACKET=y CONFIG_UNIX=y CONFIG_XFRM_USER=y CONFIG_NET_KEY=y CONFIG_NET_KEY_MIGRATE=y CONFIG_INET=y CONFIG_IP_MULTICAST=y CONFIG_IP_PNP=y CONFIG_IP_PNP_DHCP=y CONFIG_IP_PNP_BOOTP=y CONFIG_IP_PNP_RARP=y # CONFIG_INET_LRO is not set # CONFIG_IPV6 is not set CONFIG_NETFILTER=y CONFIG_BT=m CONFIG_BT_HCIUART=m CONFIG_BT_HCIUART_H4=y CONFIG_BT_HCIUART_BCSP=y CONFIG_BT_HCIUART_LL=y CONFIG_BT_HCIBCM203X=m CONFIG_BT_HCIBPA10X=m CONFIG_CFG80211=m CONFIG_MAC80211=m CONFIG_MAC80211_RC_PID=y CONFIG_MAC80211_RC_DEFAULT_PID=y CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug" CONFIG_CONNECTOR=y CONFIG_MTD=y CONFIG_MTD_CMDLINE_PARTS=y CONFIG_MTD_CHAR=y CONFIG_MTD_BLOCK=y CONFIG_MTD_OOPS=y CONFIG_MTD_CFI=y CONFIG_MTD_CFI_INTELEXT=y CONFIG_MTD_NAND=y CONFIG_MTD_NAND_OMAP2=y CONFIG_MTD_ONENAND=y CONFIG_MTD_ONENAND_VERIFY_WRITE=y CONFIG_MTD_ONENAND_OMAP2=y CONFIG_MTD_UBI=y CONFIG_BLK_DEV_LOOP=y CONFIG_BLK_DEV_RAM=y CONFIG_BLK_DEV_RAM_SIZE=16384 CONFIG_SCSI=y CONFIG_BLK_DEV_SD=y CONFIG_SCSI_MULTI_LUN=y CONFIG_SCSI_SCAN_ASYNC=y CONFIG_MD=y CONFIG_NETDEVICES=y CONFIG_SMSC_PHY=y CONFIG_NET_ETHERNET=y CONFIG_SMC91X=y CONFIG_SMSC911X=y CONFIG_KS8851=y CONFIG_KS8851_MLL=y CONFIG_LIBERTAS=m CONFIG_LIBERTAS_USB=m CONFIG_LIBERTAS_SDIO=m CONFIG_LIBERTAS_DEBUG=y CONFIG_USB_USBNET=y CONFIG_USB_NET_SMSC95XX=y CONFIG_USB_ALI_M5632=y CONFIG_USB_AN2720=y CONFIG_USB_EPSON2888=y CONFIG_USB_KC2190=y CONFIG_INPUT_JOYDEV=y CONFIG_INPUT_EVDEV=y CONFIG_KEYBOARD_GPIO=y CONFIG_KEYBOARD_TWL4030=y CONFIG_INPUT_TOUCHSCREEN=y CONFIG_TOUCHSCREEN_ADS7846=y CONFIG_INPUT_MISC=y CONFIG_INPUT_TWL4030_PWRBUTTON=y CONFIG_VT_HW_CONSOLE_BINDING=y # CONFIG_LEGACY_PTYS is not set CONFIG_SERIAL_8250_NR_UARTS=32 CONFIG_SERIAL_8250_EXTENDED=y CONFIG_SERIAL_8250_MANY_PORTS=y CONFIG_SERIAL_8250_SHARE_IRQ=y CONFIG_SERIAL_8250_DETECT_IRQ=y CONFIG_SERIAL_8250_RSA=y CONFIG_HW_RANDOM=y CONFIG_I2C_CHARDEV=y CONFIG_SPI=y CONFIG_SPI_OMAP24XX=y CONFIG_DEBUG_GPIO=y CONFIG_GPIO_SYSFS=y CONFIG_GPIO_TWL4030=y CONFIG_W1=y CONFIG_POWER_SUPPLY=y CONFIG_WATCHDOG=y CONFIG_OMAP_WATCHDOG=y CONFIG_TWL4030_
Re: "usb: dwc3: resume phy during gadget initialization on recent cores" breaks phy suspend
On 7/8/2012 4:20 AM, Paul Zimmerman wrote: Commit "usb: dwc3: resume phy during gadget initialization on recent cores" breaks phy suspend on cores newer than 1.94a. The core will still operate, but the power savings provided by phy suspend are lost. Pratyush, did you see an actual problem caused by the original code? Yes, AFAICR, If I did not use this patch over your commit"usb: dwc3: support new revisions of DWC3 core", then I did not receive even reset event. I am using 2.10a core. If so, then you need to reenable phy suspend at some point after initialization to restore the correct behavior on newer cores. So , what could be the other place than initialization. -- 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: ehci-fsl: Update ifdef check to work on 64-bit ppc
We need to use CONFIG_FSL_SOC_BOOKE instead of CONFIG_PPC_85xx as CONFIG_PPC_85xx isn't defined when we build support for 64-bit embedded FSL PPC SoCs. Signed-off-by: Kumar Gala --- drivers/usb/host/ehci-fsl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index 4336257..f002525 100644 --- a/drivers/usb/host/ehci-fsl.c +++ b/drivers/usb/host/ehci-fsl.c @@ -313,7 +313,7 @@ static void ehci_fsl_usb_setup(struct ehci_hcd *ehci) } if (pdata->have_sysif_regs) { -#ifdef CONFIG_PPC_85xx +#ifdef CONFIG_FSL_SOC_BOOKE out_be32(non_ehci + FSL_SOC_USB_PRICTRL, 0x0008); out_be32(non_ehci + FSL_SOC_USB_AGECNTTHRSH, 0x0080); #else -- 1.7.3.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2] USB: Add a sysfs file to show LTM capabilities.
USB 3.0 devices can optionally support Latency Tolerance Messaging (LTM). Add a new sysfs file in the device directory to show whether a device is LTM capable. This file will be present for both USB 2.0 and USB 3.0 devices. Signed-off-by: Sarah Sharp --- v2: Add ABI documentation. I think this is only change needed for the LPM bug fixes and LTM patchset. I did want to change the "xhci" prefix on the first two patches to "USB", since they touch the USB core and not the xHCI driver. However, I don't think that warrants a second revision to the mailing list. Greg, let me know if the ABI documentation seems ok, and I'll send you a pull request. Sarah Documentation/ABI/testing/sysfs-bus-usb | 12 drivers/usb/core/hub.c |7 --- drivers/usb/core/sysfs.c| 10 ++ include/linux/usb.h |8 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index 6df4e6f..5f75f8f 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -208,3 +208,15 @@ Description: such as ACPI. This file will read either "removable" or "fixed" if the information is available, and "unknown" otherwise. + +What: /sys/bus/usb/devices/.../ltm_capable +Date: July 2012 +Contact: Sarah Sharp +Description: + USB 3.0 devices may optionally support Latency Tolerance + Messaging (LTM). They indicate their support by setting a bit + in the bmAttributes field of their SuperSpeed BOS descriptors. + If that bit is set for the device, ltm_capable will read "yes". + If the device doesn't support LTM, the file will read "no". + The file will be present for all speeds of USB devices, and will + always read "no" for USB 1.1 and USB 2.0 devices. diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 7ca94d7..1f09018 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2607,13 +2607,6 @@ static int check_port_resume_type(struct usb_device *udev, return status; } -static bool usb_device_supports_ltm(struct usb_device *udev) -{ - if (udev->speed != USB_SPEED_SUPER || !udev->bos || !udev->bos->ss_cap) - return false; - return udev->bos->ss_cap->bmAttributes & USB_LTM_SUPPORT; -} - int usb_disable_ltm(struct usb_device *udev) { struct usb_hcd *hcd = bus_to_hcd(udev->bus); diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 777f03c..682e825 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -253,6 +253,15 @@ show_removable(struct device *dev, struct device_attribute *attr, char *buf) } static DEVICE_ATTR(removable, S_IRUGO, show_removable, NULL); +static ssize_t +show_ltm_capable(struct device *dev, struct device_attribute *attr, char *buf) +{ + if (usb_device_supports_ltm(to_usb_device(dev))) + return sprintf(buf, "%s\n", "yes"); + return sprintf(buf, "%s\n", "no"); +} +static DEVICE_ATTR(ltm_capable, S_IRUGO, show_ltm_capable, NULL); + #ifdef CONFIG_PM static ssize_t @@ -649,6 +658,7 @@ static struct attribute *dev_attrs[] = { &dev_attr_authorized.attr, &dev_attr_remove.attr, &dev_attr_removable.attr, + &dev_attr_ltm_capable.attr, NULL, }; static struct attribute_group dev_attr_grp = { diff --git a/include/linux/usb.h b/include/linux/usb.h index aaac9e4..29147ca 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -631,6 +631,14 @@ extern void usb_unlocked_enable_lpm(struct usb_device *udev); extern int usb_disable_ltm(struct usb_device *udev); extern void usb_enable_ltm(struct usb_device *udev); +static inline bool usb_device_supports_ltm(struct usb_device *udev) +{ + if (udev->speed != USB_SPEED_SUPER || !udev->bos || !udev->bos->ss_cap) + return false; + return udev->bos->ss_cap->bmAttributes & USB_LTM_SUPPORT; +} + + /*-*/ /* for drivers using iso endpoints */ -- 1.7.9 -- 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change
Dear Richard Zhao, [...] > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev) > > > > > >*/ > > > > > > device_del(&udev->dev); > > > > > > + if (udev->parent && !udev->parent->parent) { > > > + struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > > + usb_phy_notify_disconnect(hcd->phy, udev->portnum); > > > + } > > > > Shouldn't that go before device_del() ? > > Any difference? I was worried some corruption of other members in udev structure might happen, but I'm not so sure anymore after taking deer look. > Thanks > Richard Best regards, Marek Vasut -- 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: dwc3: Do not handle xfercomplete if EP is already stopped
On 7/6/2012 4:01 PM, Pratyush ANAND wrote: In some rare cases, we have observed that core generates xfercomplete event for last transfer even when "end transfer" was issued for that endpoint. Now since we have already called the giveback for the submitted request, so we do not need to handle this interrupt at all. Please ignore this patch. I see some issues with it. Regards Pratyush -- 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 v9 REBASE 6/9] USB: notify phy when root hub port connect change
On Tue, Jul 10, 2012 at 05:22:20AM +0200, Marek Vasut wrote: > Dear Richard Zhao, > > > On Sat, Jul 07, 2012 at 10:56:45PM +0800, Richard Zhao wrote: > > > Phy may need to change settings when port connect change. > > > > > > Signed-off-by: Richard Zhao > > > Tested-by: Subodh Nijsure > > > --- > > > > > > drivers/usb/core/hub.c |8 > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > > index 4cc8dc9..2ba9d84 100644 > > > --- a/drivers/usb/core/hub.c > > > +++ b/drivers/usb/core/hub.c > > > @@ -20,6 +20,7 @@ > > > > > > #include > > > #include > > > #include > > > > > > +#include > > > > > > #include > > > #include > > > #include > > > > > > @@ -4037,6 +4038,13 @@ static void hub_port_connect_change(struct usb_hub > > > *hub, int port1, > > > > > > } > > > > > > } > > > > > > + if (unlikely(hcd->phy && !hdev->parent)) { > > > + if (portstatus & USB_PORT_STAT_CONNECTION) > > > + usb_phy_notify_connect(hcd->phy, port1); > > > + else > > > + usb_phy_notify_disconnect(hcd->phy, port1); > > > > There's another issue. When hcd is removed, notify disconnect is not > > called. Is it ok, if I remove the above two line and add below patch: > > > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -1924,6 +1924,11 @@ void usb_disconnect(struct usb_device **pdev) > > */ > > device_del(&udev->dev); > > > > + if (udev->parent && !udev->parent->parent) { > > + struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > + usb_phy_notify_disconnect(hcd->phy, udev->portnum); > > + } > > Shouldn't that go before device_del() ? Any difference? Thanks Richard > > > + > > /* Free the device number and delete the parent's children[] > > * (or root_hub) pointer. > > */ > > > > > > Thanks > > Richard > > Best regards, > Marek Vasut > -- 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/3] usb: musb: dsps: add phy control logic to glue
Hi, > > Hi, > > On Tue, Jul 10, 2012 at 11:35 AM, Gupta, Ajay Kumar > > wrote: > > > Hi, > > >> > > >> On Mon, Jul 9, 2012 at 7:18 PM, Damodar Santhapuri > > >> wrote: > > >> > From: Ajay Kumar Gupta > > >> > > > >> > AM335x uses NOP transceiver driver and need to enable builtin PHY > > >> > by writing into usb_ctrl register available in system control > > >> > module register space. This is being added at musb glue driver > > >> > layer untill a separate system control module driver is available. > > >> > > > >> > Signed-off-by: Ajay Kumar Gupta > > >> > Signed-off-by: Damodar Santhapuri > > >> > --- > > >> > arch/arm/mach-omap2/board-ti8168evm.c |1 - > > >> > arch/arm/mach-omap2/omap_phy_internal.c | 35 > > >> > arch/arm/plat-omap/include/plat/usb.h |5 +- > > >> > drivers/usb/musb/musb_dsps.c| 88 > > >> +-- > > >> > 4 files changed, 74 insertions(+), 55 deletions(-) > > >> > > > >> > diff --git a/arch/arm/mach-omap2/board-ti8168evm.c > > b/arch/arm/mach- > > >> omap2/board-ti8168evm.c > > >> > index d4c8392..0c7c098 100644 > > >> > --- a/arch/arm/mach-omap2/board-ti8168evm.c > > >> > +++ b/arch/arm/mach-omap2/board-ti8168evm.c > > >> > @@ -26,7 +26,6 @@ > > >> > #include > > >> > > > >> > static struct omap_musb_board_data musb_board_data = { > > >> > - .set_phy_power = ti81xx_musb_phy_power, > > >> > .interface_type = MUSB_INTERFACE_ULPI, > > >> > .mode = MUSB_OTG, > > >> > .power = 500, > > >> > diff --git a/arch/arm/mach-omap2/omap_phy_internal.c > > b/arch/arm/mach- > > >> omap2/omap_phy_internal.c > > >> > index d52651a..d80bb16 100644 > > >> > --- a/arch/arm/mach-omap2/omap_phy_internal.c > > >> > +++ b/arch/arm/mach-omap2/omap_phy_internal.c > > >> > @@ -254,38 +254,3 @@ void am35x_set_mode(u8 musb_mode) > > >> > > > >> > omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2); > > >> > } > > >> > - > > >> > -void ti81xx_musb_phy_power(u8 on) > > >> > -{ > > >> > - void __iomem *scm_base = NULL; > > >> > - u32 usbphycfg; > > >> > - > > >> > - scm_base = ioremap(TI81XX_SCM_BASE, SZ_2K); > > >> > - if (!scm_base) { > > >> > - pr_err("system control module ioremap failed\n"); > > >> > - return; > > >> > - } > > >> > - > > >> > - usbphycfg = __raw_readl(scm_base + USBCTRL0); > > >> > - > > >> > - if (on) { > > >> > - if (cpu_is_ti816x()) { > > >> > - usbphycfg |= TI816X_USBPHY0_NORMAL_MODE; > > >> > - usbphycfg &= ~TI816X_USBPHY_REFCLK_OSC; > > >> > - } else if (cpu_is_ti814x()) { > > >> > - usbphycfg &= ~(USBPHY_CM_PWRDN | > > >> USBPHY_OTG_PWRDN > > >> > - | USBPHY_DPINPUT | > > USBPHY_DMINPUT); > > >> > - usbphycfg |= (USBPHY_OTGVDET_EN | > > >> USBPHY_OTGSESSEND_EN > > >> > - | USBPHY_DPOPBUFCTL | > > >> USBPHY_DMOPBUFCTL); > > >> > - } > > >> > - } else { > > >> > - if (cpu_is_ti816x()) > > >> > - usbphycfg &= ~TI816X_USBPHY0_NORMAL_MODE; > > >> > - else if (cpu_is_ti814x()) > > >> > - usbphycfg |= USBPHY_CM_PWRDN | > > >> USBPHY_OTG_PWRDN; > > >> > - > > >> > - } > > >> > - __raw_writel(usbphycfg, scm_base + USBCTRL0); > > >> > - > > >> > - iounmap(scm_base); > > >> > -} > > >> > diff --git a/arch/arm/plat-omap/include/plat/usb.h > > b/arch/arm/plat- > > >> omap/include/plat/usb.h > > >> > index 548a4c8..c2aa4ae 100644 > > >> > --- a/arch/arm/plat-omap/include/plat/usb.h > > >> > +++ b/arch/arm/plat-omap/include/plat/usb.h > > >> > @@ -95,7 +95,6 @@ extern void am35x_musb_reset(void); > > >> > extern void am35x_musb_phy_power(u8 on); > > >> > extern void am35x_musb_clear_irq(void); > > >> > extern void am35x_set_mode(u8 musb_mode); > > >> > -extern void ti81xx_musb_phy_power(u8 on); > > >> > > > >> > /* AM35x */ > > >> > /* USB 2.0 PHY Control */ > > >> > @@ -120,8 +119,8 @@ extern void ti81xx_musb_phy_power(u8 on); > > >> > #define CONF2_DATPOL (1 << 1) > > >> > > > >> > /* TI81XX specific definitions */ > > >> > -#define USBCTRL0 0x620 > > >> > -#define USBSTAT0 0x624 > > >> > +#define MUSB_USBSS_REV_816X0x9 > > >> > +#define MUSB_USBSS_REV_814X0xb > > >> > > > >> > /* TI816X PHY controls bits */ > > >> > #define TI816X_USBPHY0_NORMAL_MODE (1 << 0) > > >> > diff --git a/drivers/usb/musb/musb_dsps.c > > >> b/drivers/usb/musb/musb_dsps.c > > >> > index 494772f..f7271c3 100644 > > >> > --- a/drivers/usb/musb/musb_dsps.c > > >> > +++ b/drivers/usb/musb/musb_dsps.c > > >> > @@ -115,9 +115,46 @@ struct dsps_glue { > > >> > struct platform_device *musb; /* child musb pdev */ > > >> > const struct dsps_musb_wrapper *wrp; /* wrapper register > > >> offsets
Re: Mac USB 3.0 Linux support Fwd: [tova-technical-support] Request 2012/06/14/00000005 from Nancy Blum (SN: 20051 )
On Tue, Jul 10, 2012 at 02:59:27AM +0200, Peter Stuge wrote: > Ian Osgood wrote: > > dmesg errors: > > xhci_hcd :00:14.0: xhci_run > > xhci_hcd :00:14.0: Failed to enable MSI-X > > xhci_hcd :00:14.0: failed to allocate MSI entry > > xhci_hcd :00:14.0: No msi-x/msi found and no IRQ in BIOS > > xhci_hcd :00:14.0: startup error -22 > > xhci_hcd :00:14.0: USB bus 3 deregistered > > xhci_hcd :00:14.0: can't derive routing for PCI INT A > > xhci_hcd :00:14.0: init :00:14.0 fail, -22 > > xhci_hcd: probe of :00:14.0 failed with error -22 > > Either the firmware isn't setting up interrupt routing correctly > (less common) or it is not passing correct data structures to the > kernel about how interrupt routing has been set up (very common) or > the kernel does not have support compiled-in for the particular type > of data structures specifying interrupt routing which are passed by > the firmware (there are at least four kinds, some firmware provides > all of them since different OSes understand different types, and > often the different data is inconsistent). > > The no MSI is strange though. There are a couple Fresco Logic hosts that can't handle MSI-X or MSI, but I'm pretty sure the Mac is using an Intel host that should be MSI-capable. 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: NULL pointer dereference in at91_udc on start of connection
On Fri, Jul 6, 2012 at 8:06 PM, Mario Jorge Isidoro wrote: > Hi Fabio, > > I tried 3.4 and you were right, it still works fine. > > For the 'struct at91_ep' has no member named 'desc' error I tried commenting > the offending declaration (|| ep->desc) > and it builds without any error. During the bisect run this happened several > times and I think, but am not sure, that some of > this attempts worked fine without displaying the original error. I'm happy to say that with the v3.5-rc6, after reverting the commit f3d8bf34c2c925867322197096ed501ceab8085a and removing the following line: diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 7687ccd..98339a2 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -475,7 +475,6 @@ static int at91_ep_enable(struct usb_ep *_ep, unsigned long flags; if (!_ep || !ep - || !desc || ep->ep.desc || _ep->name == ep0name || desc->bDescriptorType != USB_DT_ENDPOINT || (maxpacket = usb_endpoint_maxp(desc)) == 0 Now the at91_udc driver with g_ether it's working again. I don't understand the reason, but now it seems to work fine. If there isn't a better solution i propose to revert the commit f3d8bf34c2c925867322197096ed501ceab8085a and remove the line on the at91_udc driver. Best regards -- Fabio Porcedda -- 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 0/2] USB gadget - configfs
Dear Joel, Thank you for your review. @Sebastian, Alan, Felipe: Thank you, too. On Monday, July 02, 2012 11:09 AM Joel Becker wrote: > > > As a prerequisite it adds an operation to configfs. The operation allows > > checking if it is ok to remove a pseudo directory corresponding to a > > configfs item/group. > > I NAK'd that patch because you should be using > configfs_depend_item(). If you have trouble with that, let's talk. > Now I see the configfs_depend_item() is the way to go. I am in doubt, though, so could you please throw some light on it? Here is why: As an example I did a quick-and-dirty port of f_mass_storage to the new, configfs-based approach. The business logic of this function is that once a lun is opened, it must not be changed (deleted, in particular) until it is closed. The moment the lun is opened is defined by a write to a configfs "file" attribute of a lun config item: +-/lunX | | | +-file | | | +-nofua | | | +-removable | | | +-ro So, the config item corresponding to the lun becomes depended on during the write file operation, the same with undepend. Can this be expressed with configfs_depend/undepend_item()? Your code in fs/configfs/dir.c contains a warning not to call the configfs_depend_item() from a configfs callback. In this case, is store_attribute a configfs callback? Thanks, Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 11/11] arm: omap: phy: remove unused functions from omap-phy-internal.c
On Tuesday 10 July 2012 01:46 PM, ABRAHAM, KISHON VIJAY wrote: Hi, On Tue, Jul 10, 2012 at 11:59 AM, Rajendra Nayak wrote: On Thursday 28 June 2012 05:21 PM, Kishon Vijay Abraham I wrote: All the unnessary functions in omap-phy-internal is removed. These functionality are now handled by omap-usb2 phy driver. Cc: Felipe Balbi Signed-off-by: Kishon Vijay Abraham I Acked-by: Tony Lindgren --- arch/arm/mach-omap2/omap_phy_internal.c | 138 --- arch/arm/mach-omap2/twl-common.c|5 - arch/arm/mach-omap2/usb-musb.c |3 - 3 files changed, 0 insertions(+), 146 deletions(-) diff --git a/arch/arm/mach-omap2/omap_phy_internal.c b/arch/arm/mach-omap2/omap_phy_internal.c index 4c90477..0c610b4 100644 --- a/arch/arm/mach-omap2/omap_phy_internal.c +++ b/arch/arm/mach-omap2/omap_phy_internal.c @@ -31,144 +31,6 @@ #include #include "control.h" -/* OMAP control module register for UTMI PHY */ -#define CONTROL_DEV_CONF 0x300 -#define PHY_PD 0x1 - -#define USBOTGHS_CONTROL 0x33c -#defineAVALID BIT(0) -#defineBVALID BIT(1) -#defineVBUSVALID BIT(2) -#defineSESSEND BIT(3) -#defineIDDIG BIT(4) - -static struct clk *phyclk, *clk48m, *clk32k; -static void __iomem *ctrl_base; -static int usbotghs_control; - -int omap4430_phy_init(struct device *dev) -{ - ctrl_base = ioremap(OMAP443X_SCM_BASE, SZ_1K); - if (!ctrl_base) { - pr_err("control module ioremap failed\n"); - return -ENOMEM; - } - /* Power down the phy */ - __raw_writel(PHY_PD, ctrl_base + CONTROL_DEV_CONF); Just checking, but I hope your new driver handles this too. You might not see any issues with it now, but not doing this could gate OMAP hitting low power in idle. I power down the phy during probe in omap-usb2 phy driver. ok, thanks, good to know. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 10/11] arm/dts: omap: Add usb_otg and glue data
On Tuesday 10 July 2012 01:43 PM, ABRAHAM, KISHON VIJAY wrote: Hi, On Tue, Jul 10, 2012 at 11:57 AM, Rajendra Nayak wrote: On Thursday 28 June 2012 05:21 PM, Kishon Vijay Abraham I wrote: Add usb otg data node in omap4/omap3 device tree file. Also update the node with board specific setting in omapx-.dts file. Signed-off-by: Kishon Vijay Abraham I --- arch/arm/boot/dts/omap3-beagle.dts |6 ++ arch/arm/boot/dts/omap3-evm.dts|6 ++ arch/arm/boot/dts/omap3.dtsi |8 arch/arm/boot/dts/omap4-panda.dts |6 ++ arch/arm/boot/dts/omap4-sdp.dts|6 ++ arch/arm/boot/dts/omap4.dtsi |8 6 files changed, 40 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index 5b4506c..f3d7076 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -67,3 +67,9 @@ &mmc3 { status = "disable"; }; + +&usb_otg_hs { + interface_type =<0>; + mode =<3>; + power =<50>; +}; diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts index 2eee16e..8963b3d 100644 --- a/arch/arm/boot/dts/omap3-evm.dts +++ b/arch/arm/boot/dts/omap3-evm.dts @@ -18,3 +18,9 @@ reg =<0x8000 0x1000>; /* 256 MB */ }; }; + +&usb_otg_hs { + interface_type =<0>; + mode =<3>; + power =<50>; +}; diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi index 99474fa..2f565d6 100644 --- a/arch/arm/boot/dts/omap3.dtsi +++ b/arch/arm/boot/dts/omap3.dtsi @@ -215,5 +215,13 @@ compatible = "ti,omap3-hsmmc"; ti,hwmods = "mmc3"; }; + + usb_otg_hs: usb_otg_hs@4a0ab000 { + compatible = "ti,musb-omap2430"; this compatible doesn't seem right in omap3.dtsi. Same with the below entry in omap4.dtsi. See other IP blocks which are reused across OMAP2/3/4 on how the compatible for those are handled. Ok. So it should be like *ti,omap4-musb*, *ti,omap3-musb*? Yes, that would be more appropriate. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: infos about device ZTE MF821D 2G,3G,4G/LTE usb-modem/networkcard
Thomas Schäfer writes: > Am Montag, 9. Juli 2012, 10:25:27 schrieben Sie: >> Thomas Schäfer writes: >> > The modem works at /dev/USB2. > >> posted: >> | diag 19d2:0326 MI00\6 >> | nema 19d2:0326 MI01\6 >> | at- 19d2:0326 MI02\6 >> | modem 19d2:0326 MI03\6 >> | ndis 19d2:0326 MI04\6 >> >> I am wondering a bit about the "modem" port here. Is that a second AT >> command interface? According to your dmesg, it should be ttyUSB3 in >> your setup: > > Yes, there are two interfaces (/dev/ttyUSB2 and /dev/ttyUSB3) which answers > "ok" to "at" and some other at-commands. > > The classic serial-ppp-internet-connection works at least with /dev/ttyUSB2 > > I have to test /dev/ttyUSB3 if only "at" works or the whole ppp-stuff too. Well, it doesn't matter for the drivers. I just wanted to verify that it is a serial interface and not something else. >> Please prepare patches for the option and qmi_wwan (after verifying the >> above assumption) drivers. Yes, I could do so, or someone else could, >> but you've already done most of the work so why not make sure you get >> the proper credit? :-) > > I afraid if I create a working patch to a certain version, the version is > very > obsolete. Or you or the maintainers have extra work, because of a lot of > beginner's mistakes. Having been there, and still doing such mistakes, I can honestly say that it's nothing to be afraid of. That's just considered part of the learning process. The USB list is full of very kind and forgiving people. The worst that could possibly happen is that you have to redo the patch. > My knowledge about c-programming and usb-driver-layouts is at a very low > level. > What for "proper credit"? > For adding some IDs?, while you and others wrote the whole driver, maintain > version for version, find bugs and improve the smallest details? > > That are your credits! (seriously) Well, if you think about it, you will realize that my contribution is insignificant compared to e.g. the usbnet infrastructure I am leaning on, and that contribution is still small compared to some other part of the kernel, etc, etc. Yes, I know that most of us don't do this for credits, but to make something work. Proper accounting of the work being done still means something to the community. Anyway, it's an easy way to get to know the process. So that the next time you get some new device with no driver, you might think "I can make a driver for that". And if you're like me, then you won't realize you're wrong until it's too late :-) > If you have time, please send me RFT-patches and the link to the version I > have to apply them. > > This division of work was not wrong in past. OK, I can of course do that. 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 v1 05/11] drivers: usb: twl6030: Add dt support for twl6030 usb
Hi, On Tue, Jul 10, 2012 at 12:14 PM, Rajendra Nayak wrote: > On Tuesday 10 July 2012 11:58 AM, ABRAHAM, KISHON VIJAY wrote: >> >> Hi, >> >> On Tue, Jul 10, 2012 at 11:28 AM, Rajendra Nayak wrote: >>> >>> On Thursday 28 June 2012 05:21 PM, Kishon Vijay Abraham I wrote: Add device tree support for twl6030 usb driver. Update the Documentation with device tree binding information. Signed-off-by: Kishon Vijay Abraham I --- .../devicetree/bindings/usb/twl-usb.txt| 18 drivers/usb/otg/twl6030-usb.c | 45 ++-- 2 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/twl-usb.txt diff --git a/Documentation/devicetree/bindings/usb/twl-usb.txt b/Documentation/devicetree/bindings/usb/twl-usb.txt new file mode 100644 index 000..f293271 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/twl-usb.txt @@ -0,0 +1,18 @@ +USB COMPARATOR OF TWL CHIPS + +TWL6030 USB COMPARATOR + - compatible : Should be "ti,twl6030-usb" + - interrupts : Two interrupt numbers to the cpu should be specified. First + interrupt number is the otg interrupt number that raises ID interrupts when + the controller has to act as host and the second interrupt number is the + usb interrupt number that raises VBUS interrupts when the controller has to + act as device + - regulator : can be "vusb" or "ldousb" + --supply : phandle to the regulator device tree node + +twl6030-usb { + compatible = "ti,twl6030-usb"; + interrupts =< 4 10>; + regulator = "vusb"; + vusb-supply =<&vusb>; >>> >>> >>> >>> This doesn't seem right. Why do you ned a 'regulator' string along >>> with the phandle? >> >> >> The original code was something like >> if (twl->features& TWL6025_SUBCLASS) >> >> regulator_name = "ldousb"; >> else >> regulator_name = "vusb"; >> >> I wasn't sure how to handle this *TWL6025_SUBCLASS* stuff. >> >>> +}; diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c index 6a361d2..20b7abe 100644 --- a/drivers/usb/otg/twl6030-usb.c +++ b/drivers/usb/otg/twl6030-usb.c @@ -105,7 +105,7 @@ struct twl6030_usb { u8 asleep; boolirq_enabled; boolvbus_enable; - unsigned long features; + const char *regulator; }; #define comparator_to_twl(x) container_of((x), struct twl6030_usb, comparator) @@ -153,13 +153,6 @@ static int twl6030_start_srp(struct phy_companion *comparator) static int twl6030_usb_ldo_init(struct twl6030_usb *twl) { - char *regulator_name; - - if (twl->features& TWL6025_SUBCLASS) - regulator_name = "ldousb"; - else - regulator_name = "vusb"; - /* Set to OTG_REV 1.3 and turn on the ID_WAKEUP_COMP */ twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x1, TWL6030_BACKUP_REG); @@ -169,7 +162,7 @@ static int twl6030_usb_ldo_init(struct twl6030_usb *twl) /* Program MISC2 register and set bit VUSB_IN_VBAT */ twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x10, TWL6030_MISC2); - twl->usb3v3 = regulator_get(twl->dev, regulator_name); + twl->usb3v3 = regulator_get(twl->dev, twl->regulator); if (IS_ERR(twl->usb3v3)) return -ENODEV; @@ -324,9 +317,9 @@ static int __devinit twl6030_usb_probe(struct platform_device *pdev) { struct twl6030_usb *twl; int status, err; - struct twl4030_usb_data *pdata; - struct device *dev =&pdev->dev; - pdata = dev->platform_data; + struct device_node *np = pdev->dev.of_node; + struct device *dev =&pdev->dev; + struct twl4030_usb_data *pdata = dev->platform_data; twl = devm_kzalloc(dev, sizeof *twl, GFP_KERNEL); if (!twl) @@ -335,13 +328,28 @@ static int __devinit twl6030_usb_probe(struct platform_device *pdev) twl->dev=&pdev->dev; twl->irq1 = platform_get_irq(pdev, 0); twl->irq2 = platform_get_irq(pdev, 1); - twl->features = pdata->features; twl->linkstat = OMAP_MUSB_UNKNOWN; twl->comparator.set_vbus= twl6030_set_vbus; twl->comparator.start_
Re: [PATCH v1 11/11] arm: omap: phy: remove unused functions from omap-phy-internal.c
Hi, On Tue, Jul 10, 2012 at 11:59 AM, Rajendra Nayak wrote: > On Thursday 28 June 2012 05:21 PM, Kishon Vijay Abraham I wrote: >> >> All the unnessary functions in omap-phy-internal is removed. >> These functionality are now handled by omap-usb2 phy driver. >> >> Cc: Felipe Balbi >> Signed-off-by: Kishon Vijay Abraham I >> Acked-by: Tony Lindgren >> --- >> arch/arm/mach-omap2/omap_phy_internal.c | 138 >> --- >> arch/arm/mach-omap2/twl-common.c|5 - >> arch/arm/mach-omap2/usb-musb.c |3 - >> 3 files changed, 0 insertions(+), 146 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/omap_phy_internal.c >> b/arch/arm/mach-omap2/omap_phy_internal.c >> index 4c90477..0c610b4 100644 >> --- a/arch/arm/mach-omap2/omap_phy_internal.c >> +++ b/arch/arm/mach-omap2/omap_phy_internal.c >> @@ -31,144 +31,6 @@ >> #include >> #include "control.h" >> >> -/* OMAP control module register for UTMI PHY */ >> -#define CONTROL_DEV_CONF 0x300 >> -#define PHY_PD 0x1 >> - >> -#define USBOTGHS_CONTROL 0x33c >> -#defineAVALID BIT(0) >> -#defineBVALID BIT(1) >> -#defineVBUSVALID BIT(2) >> -#defineSESSEND BIT(3) >> -#defineIDDIG BIT(4) >> - >> -static struct clk *phyclk, *clk48m, *clk32k; >> -static void __iomem *ctrl_base; >> -static int usbotghs_control; >> - >> -int omap4430_phy_init(struct device *dev) >> -{ >> - ctrl_base = ioremap(OMAP443X_SCM_BASE, SZ_1K); >> - if (!ctrl_base) { >> - pr_err("control module ioremap failed\n"); >> - return -ENOMEM; >> - } >> - /* Power down the phy */ >> - __raw_writel(PHY_PD, ctrl_base + CONTROL_DEV_CONF); > > > Just checking, but I hope your new driver handles this too. > You might not see any issues with it now, but not doing this could > gate OMAP hitting low power in idle. I power down the phy during probe in omap-usb2 phy driver. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 10/11] arm/dts: omap: Add usb_otg and glue data
Hi, On Tue, Jul 10, 2012 at 11:57 AM, Rajendra Nayak wrote: > On Thursday 28 June 2012 05:21 PM, Kishon Vijay Abraham I wrote: >> >> Add usb otg data node in omap4/omap3 device tree file. Also update >> the node with board specific setting in omapx-.dts file. >> >> Signed-off-by: Kishon Vijay Abraham I >> --- >> arch/arm/boot/dts/omap3-beagle.dts |6 ++ >> arch/arm/boot/dts/omap3-evm.dts|6 ++ >> arch/arm/boot/dts/omap3.dtsi |8 >> arch/arm/boot/dts/omap4-panda.dts |6 ++ >> arch/arm/boot/dts/omap4-sdp.dts|6 ++ >> arch/arm/boot/dts/omap4.dtsi |8 >> 6 files changed, 40 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/boot/dts/omap3-beagle.dts >> b/arch/arm/boot/dts/omap3-beagle.dts >> index 5b4506c..f3d7076 100644 >> --- a/arch/arm/boot/dts/omap3-beagle.dts >> +++ b/arch/arm/boot/dts/omap3-beagle.dts >> @@ -67,3 +67,9 @@ >> &mmc3 { >> status = "disable"; >> }; >> + >> +&usb_otg_hs { >> + interface_type =<0>; >> + mode =<3>; >> + power =<50>; >> +}; >> diff --git a/arch/arm/boot/dts/omap3-evm.dts >> b/arch/arm/boot/dts/omap3-evm.dts >> index 2eee16e..8963b3d 100644 >> --- a/arch/arm/boot/dts/omap3-evm.dts >> +++ b/arch/arm/boot/dts/omap3-evm.dts >> @@ -18,3 +18,9 @@ >> reg =<0x8000 0x1000>; /* 256 MB */ >> }; >> }; >> + >> +&usb_otg_hs { >> + interface_type =<0>; >> + mode =<3>; >> + power =<50>; >> +}; >> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi >> index 99474fa..2f565d6 100644 >> --- a/arch/arm/boot/dts/omap3.dtsi >> +++ b/arch/arm/boot/dts/omap3.dtsi >> @@ -215,5 +215,13 @@ >> compatible = "ti,omap3-hsmmc"; >> ti,hwmods = "mmc3"; >> }; >> + >> + usb_otg_hs: usb_otg_hs@4a0ab000 { >> + compatible = "ti,musb-omap2430"; > > > this compatible doesn't seem right in omap3.dtsi. Same with > the below entry in omap4.dtsi. > See other IP blocks which are reused across OMAP2/3/4 on > how the compatible for those are handled. Ok. So it should be like *ti,omap4-musb*, *ti,omap3-musb*? Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 09/11] drivers: usb: musb: Add device tree support for omap musb glue
Hi, > Documentation/devicetree/bindings/usb/omap-usb.txt | 34 > - > drivers/usb/musb/omap2430.c| 52 > [...] > + of_property_read_u32(np, "mode", (u32 *)&pdata->mode); > + of_property_read_u32(np, "interface_type", > + (u32 *)&data->interface_type); > + of_property_read_u32(np, "num_eps", (u32 *)&config- > >num_eps); > + of_property_read_u32(np, "ram_bits", (u32 *)&config- > >ram_bits); > + of_property_read_u32(np, "mode", (u32 *)&pdata->mode); 'mode' has already been read so this should be dropped. Ajay > + of_property_read_u32(np, "power", (u32 *)&pdata->power); > + config->multipoint = of_property_read_bool(np, > "multipoint"); > + > + pdata->board_data = data; > + pdata->config = config; > + } > + > pdata->platform_ops = &omap2430_ops; > > platform_set_drvdata(pdev, glue); > @@ -597,12 +638,23 @@ static struct dev_pm_ops omap2430_pm_ops = { > #define DEV_PM_OPS NULL > #endif > > +#ifdef CONFIG_OF > +static const struct of_device_id omap2430_id_table[] = { > + { .compatible = "ti,musb-omap2430" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, omap2430_id_table); > +#else > +#define omap2430_id_table NULL > +#endif > + > static struct platform_driver omap2430_driver = { > .probe = omap2430_probe, > .remove = __devexit_p(omap2430_remove), > .driver = { > .name = "musb-omap2430", > .pm = DEV_PM_OPS, > + .of_match_table = omap2430_id_table, > }, > }; > > -- > 1.7.5.4 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
On Tuesday 10 July 2012 12:18 PM, ABRAHAM, KISHON VIJAY wrote: Hi, On Tue, Jul 10, 2012 at 11:33 AM, Venu Byravarasu wrote: + +#ifdef CONFIG_PM Should it not be CONFIG_PM_SLEEP instead of just CONFIG_PM? Why? I think we should have CONFIG_PM_SLEEP only when we have *suspend*, *resume* hooks. But this driver has only *runtime_suspend* and *runtime_resume* hooks. CONFIG_PM_RUNTIME maybe then? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
> Hi, > > On Tue, Jul 10, 2012 at 11:33 AM, Venu Byravarasu > wrote: > >> > + > >> > +#ifdef CONFIG_PM > > > > Should it not be CONFIG_PM_SLEEP instead of just CONFIG_PM? > > Why? I think we should have CONFIG_PM_SLEEP only when we have > *suspend*, *resume* hooks. But this driver has only *runtime_suspend* > and *runtime_resume* hooks. Yes, you are right. CONFIG_PM_SLEEP is not needed for runtime. > > > >> > + > >> > +static int omap_usb2_runtime_suspend(struct device *dev) > >> > +{ > >> > + struct platform_device *pdev = to_platform_device(dev); > >> > + struct omap_usb *phy = platform_get_drvdata(pdev); > >> > + > > > > > >> > +static int __init usb2_omap_init(void) > >> > +{ > >> > + return platform_driver_register(&omap_usb2_driver); > >> > +} > >> > +arch_initcall(usb2_omap_init); > >> > + > >> > +static void __exit usb2_omap_exit(void) > >> > +{ > >> > + platform_driver_unregister(&omap_usb2_driver); > >> > +} > >> > +module_exit(usb2_omap_exit); > >> > + > > > > Do you really need arch_initcall here? > > If not, then you can replace above two function calls with > module_platform_driver(). > > I indeed want it to be arch_initcall. When the module is built-in, I > want this module to loaded before twl6030-usb.c Should be fine. > Thanks > Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 07/11] drivers: usb: twl4030: Add device tree support for twl4030 usb
On Tuesday 10 July 2012 12:22 PM, ABRAHAM, KISHON VIJAY wrote: +TWL4030 USB PHY AND COMPARATOR >> + - compatible : Should be "ti,twl4030-usb" >> + - interrupts : The interrupt numbers to the cpu should be specified. >> First >> + interrupt number is the otg interrupt number that raises ID interrupts >> + and VBUS interrupts. The second interrupt number is optional. >> + --supply : phandle to the regulator device tree node. >> + should be vusb1v5, vusb1v8 and vusb3v1 >> + - usb_mode : The mode used by the phy to connect to the controller. "1" >> + specifies "ULPI" mode and "2" specifies "CEA2011_3PIN" mode. > > > Are these standard usb phy modes or something specific to the twl4030 > usb phy? These are standard modes used to connect the phy to the controller. I think it's used by other chips other than twl4030 (Something in am35xx??). So would it make sense to document these bindings independent of a given phy and a given controller, so it could be reused and not duplicated in various forms for various different controllers. -- 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