Re: [PATCH 1/2] usb: gadget/uvc: remove connect/disconnect calls on open/release
Hi Vladimir, On Monday 06 May 2013 13:42:45 Vladimir Zapolskiy wrote: > On 05/04/13 21:22, Bhupesh SHARMA wrote: > > On 5/3/2013 6:00 PM, Vladimir Zapolskiy wrote: > >> On 05/03/13 02:05, Laurent Pinchart wrote: > >>> On Friday 03 May 2013 02:00:29 Vladimir Zapolskiy wrote: > On 05/03/13 01:18, Laurent Pinchart wrote: > > On Friday 03 May 2013 01:13:48 Vladimir Zapolskiy wrote: > >> This change removes redundant calls to uvc_function_connect() and > >> uvc_function_disconnect() on V4L2 device node open and release. > >> > >> These two functions attemp to control pull-up on D+ line directly, > >> however such an action should be performed by an UDC iteself, and > >> within the gadget there is no information about current mode of the > >> controller. > >> > >> The UDC may be in suspended state, or an OTG controller may be in > >> host mode, therefore it seems better not to try to forcibly pull-up > >> D+ line on open() syscall. > > > > OK, but we then need to fix the problem properly. The UVC gadget must > > not appear connected until an application opens the corresponding > > device. Likewise, it must disconnect from the bus when the application > > closes the device. How can this be guaranteed properly ? > > For better understanding of the issue, could you explain briefly why > do you prefer to have the gadget not connected to the bus, if device > node is not opened? > >>> > >>> As soon as the gadget will connect to the bus the device will be > >>> enumerated by the host and bound to a host driver that will query the > >>> device using UVC-specific requests. The userspace application is > >>> involved in replying to those requests, so it needs to be bound to the > >>> device on the gadget side or the initialization process on the host side > >>> will fail. > >> > >> It might be a flaw in design, if a kernel space component depends on a > >> user space application to be operable. Also the same scenario seems to be > >> invalid, if an application unawared of specific to UVC features of > >> /dev/videoX opens the device node, e.g. > >> http://git.kernel.org/cgit/linux/hotplug/udev.git/tree/src/v4l_id/v4l_id. > >> c or yavta etc. I presume a host should dictate behaviour of device and > >> gadget in particular, and not a target's user space application, please > >> correct me. As Bhupesh already explained, the complexity of handling video streaming requires a userspace component. Port of the uvc-gadget application could be moved to the kernel driver, but we will always need a userspace component to supply video data and reply to control requests. > >> About this particular change, as I mentioned in a cover letter an > >> alternative approach may be to add sanity checks to .pullup operations > >> for every UDC driver (or probably to usb_gadget_connect()), but in this > >> case it is not clear how UVC gadget is going to be notified about changes > >> of UDC state, e.g. assume a test that /dev/videoX is opened, when OTG is > >> in Host mode, device registration doesn't happen on open(), and then USB > >> B cable is inserted to the port. > >> > >> I would appreciate your thoughts. > > > > The whole point of having a user-space application governing the > > behavior of UVC webcam gadget as per commands from a UVC host is to plug > > the same with a real video capture source driver to provide the video > > frames captured from say a camera sensor and route the UVC specific > > control requests to a real video capture device by converting the same > > to equivalent V4L2 commands. > > thank you for the explanation, however my original question is about > avoiding critical hardware errors attended to the current gadget's design. > If you think that UDC should be controlled every time on open()/close() > syscalls, how do you see an optimal way to mitigate problems described > above? I think the issue is on the UDC side. A function driver should be allowed to tell when it's ready to be connected and when it needs to be disconnected. The UDC of course isn't required to connect the device to the bus as soon as the function request the connection, it can delay that until it's ready (switched from host mode to device mode in case of OTG, resumed if it has been suspended, ...). The API offered to function drivers should handle this, either in the gadget core or in the UDC themselves (or possibly a combination of the two). -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Testing phonet
On Wednesday, May 01, 2013 1:53 PM Daniele Forsi wrote: > 2013/4/30 Andrzej Pietrasiewicz: > > > As I already wrote I am now stuck with sending correct data, so that > when > > I sendto() from host I can recvfrom() on device (or vice versa). > > payload data doesn't matter so you don't need my dumps, maybe you are > using the same address on both sides of the link or you are sending to > the wrong address or the route isn't set up correctly > This is how I set up the host and the device, using the tools from Rémi (http://gitorious.org/meego-cellular/phonet-utils/trees/master): Device: $ ./phonet -a 0x6c -i upnlink0 $ ./phonet -l -i upnlink0 phonet addr: 6c $ ./pnroute add 0x10 upnlink0 $ ./pnroute 10 upnlink0 $ ifconfig upnlink0 up Host: $ ./phonet -a 0x10 -i usbpn0 $ ./phonet -l -i usbpn0 phonet addr: 10 $ ./pnroute add 0x6c usbpn0 $ ./pnroute add 0x10 usbpn0 $ ./pnroute 10 usbpn0 6C usbpn0 $ ifconfig usbpn0 up I attach the test program I use. So far I have learnt that if I do $ ./pnxmit -a 0x10 -s 0x6c on the host, I can see that on the device the pn_rx_complete (drivers/usb/gadget/f_phonet.c) is called, then netif_rx (net/core/dev.c), then packet_type->func [phonet_rcv] (net/phonet/af_phonet.c). The phonet_rcv fails at if ((len > skb->len) || pskb_trim(skb, len)) goto out; with len=61182 and skb->len=10. The len looks suspicious, but I don't know why it is set to such a large value. In consequence NET_RX_DROP is returned from phonet_rcv, so I assume no client will ever be able to recvfrom a socket. Any ideas? Andrzej pnxmit.c Description: Binary data
[RFC PATCH 0/2] USB: OHCI: Start splitting up the driver
This series of patches begins the process of splitting ohci-hcd up into a core library module and independent pci driver modules. Patch 1/2 prepares the way by exporting a few functions from ohci-hcd and adding a new mechanism for platform-specific drivers to initialize their hc_driver structures. This deserves to be done in the core because almost all of the entries in these structures are pure boilerplate -- practically none of the drivers need to override more than three of the standard core values. Patch 2/2 separate out ohci-pci into independent driver modules. Manjunath Goudar (2): USB: OHCI: prepare to make ohci-hcd a library module USB: OHCI: make ohci-pci a separate driver drivers/usb/host/Kconfig |9 ++- drivers/usb/host/Makefile|3 + drivers/usb/host/ohci-hcd.c | 142 +- drivers/usb/host/ohci-pci.c | 129 ++ drivers/usb/host/ohci-platform.c |2 +- drivers/usb/host/ohci-q.c| 13 drivers/usb/host/ohci.h | 30 +++- 7 files changed, 197 insertions(+), 131 deletions(-) -- 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
[RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
This patch prepares ohci-hcd for being split up into a core
library and separate platform driver modules. A generic
ohci_hc_driver structure is created, containing all the "standard"
values, and a new mechanism is added whereby a driver module can
specify a set of overrides to those values. In addition the
ohci_restart(),ohci_suspend() and ohci_resume() routines need
to be EXPORTed for use by the drivers.
In V2:
-ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
-ohci_setup() and ohci_start() functions written to generic OHCI initialization
for all ohci bus glue.
Signed-off-by: Manjunath Goudar
Cc: Arnd Bergmann
Cc: Greg KH
Cc: Alan Stern
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/Makefile|2 +
drivers/usb/host/ohci-hcd.c | 123 +-
drivers/usb/host/ohci-pci.c |2 +-
drivers/usb/host/ohci-platform.c |2 +-
drivers/usb/host/ohci.h | 30 +-
5 files changed, 138 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 58c14c1..a38d76b 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA)+=ehci-tegra.o
obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
+
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9e6de95..7922c61 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -79,13 +79,7 @@ static const charhcd_name [] = "ohci_hcd";
#include "pci-quirks.h"
static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static int ohci_init (struct ohci_hcd *ohci);
-static void ohci_stop (struct usb_hcd *hcd);
-
-#if defined(CONFIG_PM) || defined(CONFIG_PCI)
-static int ohci_restart (struct ohci_hcd *ohci);
-#endif
-
+static void ohci_stop(struct usb_hcd *hcd);
#ifdef CONFIG_PCI
static void sb800_prefetch(struct ohci_hcd *ohci, int on);
#else
@@ -520,7 +514,7 @@ done:
/* init memory, and kick BIOS/SMM off */
-static int ohci_init (struct ohci_hcd *ohci)
+int ohci_init(struct ohci_hcd *ohci)
{
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
@@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
return ret;
}
+EXPORT_SYMBOL_GPL(ohci_init);
/*-*/
@@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
* resets USB and controller
* enable interrupts
*/
-static int ohci_run (struct ohci_hcd *ohci)
+static int ohci_run(struct usb_hcd *hcd)
{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
u32 mask, val;
int first = ohci->fminterval == 0;
- struct usb_hcd *hcd = ohci_to_hcd(ohci);
ohci->rh_state = OHCI_RH_HALTED;
@@ -767,7 +762,37 @@ retry:
return 0;
}
+/**/
+
+/* ohci generic function for all OHCI bus glue */
+
+static int ohci_setup(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int retval;
+
+ ohci->sbrn = HCD_USB11;
+
+ /* data structure init */
+ retval = ohci_init(ohci);
+ if (retval)
+ return retval;
+ ohci_usb_reset(ohci);
+ return 0;
+}
+static int ohci_start(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int ret;
+ ret = ohci_run(hcd);
+ if (ret < 0) {
+ ohci_err(ohci, "can't start\n");
+ ohci_stop(hcd);
+ }
+ return ret;
+}
+/*-*/
/*-*/
/* an interrupt happens */
@@ -949,11 +974,12 @@ static void ohci_stop (struct usb_hcd *hcd)
#if defined(CONFIG_PM) || defined(CONFIG_PCI)
/* must not be called from interrupt context */
-static int ohci_restart (struct ohci_hcd *ohci)
+int ohci_restart(struct ohci_hcd *ohci)
{
int temp;
int i;
struct urb_priv *priv;
+ struct usb_hcd *hcd;
spin_lock_irq(&ohci->lock);
ohci->rh_state = OHCI_RH_HALTED;
@@ -1001,19 +1027,22 @@ static int ohci_restart (struct ohci_hcd *ohci)
ohci->ed_controltail = NULL;
ohci->ed_bulktail= NULL;
- if ((temp = ohci_run (ohci)) < 0) {
- ohci_err (ohci, "can't restart, %d\n", temp);
+ hcd = ohci_to_hcd(ohci);
+ temp = ohci_run(hcd);
+ if (temp < 0) {
+ ohci_err(ohci, "can't restart, %d\n", temp);
return temp;
Re: Linux USB file storage gadget with new UDC
Hi, >> How the UDC driver know when the request is really complete? > > An OUT request is really complete when either: > > The total number of bytes copied into req.buffer (i.e., > req.actual) is equal to req.length, or > > The number of bytes received in the last packet is smaller > than ep.maxpacket. I made some changes regarding req.actual. Now the UDC driver still cannot process SCSI_WRITE_10 command. Please see the attached UDC driver log when i try to write to a text file. There should be three SCSI commands in the log: SCSI_REQUEST_SENSE, SCSI_TEST_UNIT_READY and SCSI_WRITE_10. SCSI_WRITE_10 is not received properly. Thanks, victor g_file_storage gadget: bulk-out, length 31: : 55 53 42 43 0d 01 00 00 12 00 00 00 80 00 06 03 0010: 00 00 00 12 00 00 00 00 00 00 00 00 c3 63 4a g_file_storage gadget: SCSI command: REQUEST SENSE; Dc=6, Di=18; Hc=6, Hi=18 g_file_storage gadget: bulk-in, length 18: : 70 00 06 00 00 00 00 0a 00 00 00 00 29 00 00 00 0010: 00 00 [start_transfer] 60070 a00 ept1 in queue len 0x12, buffer 0xc1344000 0: 0x60070 4: 0xa00 8: 0x0 c: 0x29 bulk_in_complete --> 0, 18/18 g_file_storage gadget: bulk-in, length 13: : 55 53 42 53 0d 01 00 00 00 00 00 00 00 [start_transfer] 53425355 10d ept1 in queue len 0xd, buffer 0xc1304000 0: 0x53425355 4: 0x10d 8: 0x0 bulk_in_complete --> 0, 13/13 EP1 OUT IRQ 0x28 ep1_out: RX DMA done : NULL REQ on OUT EP-1 [start_transfer] 60070 a00 ept1 out queue len 0x200, buffer 0xc1344000 g_file_storage gadget: bulk-out, length 31: : 55 53 42 43 0e 01 00 00 00 00 00 00 00 00 06 00 0010: 00 00 00 00 00 00 00 00 00 00 00 00 c3 63 4a g_file_storage gadget: SCSI command: TEST UNIT READY; Dc=6, Dn=0; Hc=6, Hn=0 g_file_storage gadget: bulk-in, length 13: : 55 53 42 53 0e 01 00 00 00 00 00 00 00 [start_transfer] 53425355 10e ept1 in queue len 0xd, buffer 0xc1344000 0: 0x53425355 4: 0x10e 8: 0x0 bulk_in_complete --> 0, 13/13 EP1 OUT IRQ 0x28 ep1_out: RX DMA done : NULL REQ on OUT EP-1 [start_transfer] 53425355 10d ept1 out queue len 0x200, buffer 0xc1304000 g_file_storage gadget: bulk-out, length 31: EP1 OUT IRQ 0x28 epnum 1 in 0 len 0 512 0 g_file_storage gadget: bulk-out, length 0: g_file_storage gadget: bulk_out_complete --> 0, 0/31 g_file_storage gadget: bulk_out_complete --> 0, 0/31 g_file_storage gadget: invalid CBW: len 0 sig 0x43425355 g_file_storage gadget: bulk-in set wedge g_file_storage gadget: get_next_command [start_transfer] 43425355 10f ept1 out queue len 0x200, buffer 0xc1304000
[RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver
This patch splits the PCI portion of ohci-hcd out into its
own separate driver module, called ohci-pci. Consistently with the
current practice, the decision whether to build this module is not
user-configurable. If OHCI_PCI are enabled then the module will
be built, always.
V2:
- few specific content of pci related code in ohci_pci_start function has
been moved to ohci_pci_reset
and rest of the generic code is written in ohci_start of ohci-hcd.c file.
Signed-off-by: Manjunath Goudar
Cc: Arnd Bergmann
Cc: Greg KH
Cc: Alan Stern
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/Kconfig|9 ++-
drivers/usb/host/Makefile |1 +
drivers/usb/host/ohci-hcd.c | 19 +--
drivers/usb/host/ohci-pci.c | 129 +--
drivers/usb/host/ohci-q.c | 13 +
5 files changed, 60 insertions(+), 111 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 7a1a248..3620ecce 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -333,8 +333,13 @@ config USB_ISP1362_HCD
To compile this driver as a module, choose M here: the
module will be called isp1362-hcd.
+config USB_OHCI_PCI
+ tristate
+ depends on USB_OHCI_HCD && PCI
+ default y
+
config USB_OHCI_HCD
- tristate "OHCI HCD support"
+ tristate "OHCI HCD (USB 1.1) support"
depends on USB && USB_ARCH_HAS_OHCI
select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
select USB_OTG_UTILS if ARCH_OMAP
@@ -402,7 +407,7 @@ config USB_OHCI_HCD_PPC_OF
default USB_OHCI_HCD_PPC_OF_BE || USB_OHCI_HCD_PPC_OF_LE
config USB_OHCI_HCD_PCI
- bool "OHCI support for PCI-bus USB controllers"
+ tristate "OHCI support for PCI-bus USB controllers"
depends on USB_OHCI_HCD && PCI && (STB03xxx || PPC_MPC52xx ||
USB_OHCI_HCD_PPC_OF)
default y
select USB_OHCI_LITTLE_ENDIAN
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index a38d76b..a20a8d9 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+obj-$(CONFIG_USB_OHCI_PCI) += ohci-pci.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 7922c61..2e62e7d 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -95,7 +95,6 @@ static inline void sb800_prefetch(struct ohci_hcd *ohci, int
on)
#include "ohci-mem.c"
#include "ohci-q.c"
-
/*
* On architectures with edge-triggered interrupts we must never return
* IRQ_NONE.
@@ -585,7 +584,6 @@ int ohci_init(struct ohci_hcd *ohci)
return ret;
}
EXPORT_SYMBOL_GPL(ohci_init);
-
/*-*/
/* Start an OHCI controller, set the BUS operational
@@ -1181,11 +1179,6 @@ MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
-#ifdef CONFIG_PCI
-#include "ohci-pci.c"
-#define PCI_DRIVER ohci_pci_driver
-#endif
-
#if defined(CONFIG_ARCH_SA1100) && defined(CONFIG_SA)
#include "ohci-sa.c"
#define SA_DRIVER ohci_hcd_sa_driver
@@ -1281,7 +1274,7 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVERohci_platform_driver
#endif
-#if!defined(PCI_DRIVER) && \
+#if!IS_ENABLED(CONFIG_USB_OHCI_PCI) && \
!defined(PLATFORM_DRIVER) &&\
!defined(OMAP1_PLATFORM_DRIVER) && \
!defined(OMAP3_PLATFORM_DRIVER) && \
@@ -1356,12 +1349,6 @@ static int __init ohci_hcd_mod_init(void)
goto error_sa;
#endif
-#ifdef PCI_DRIVER
- retval = pci_register_driver(&PCI_DRIVER);
- if (retval < 0)
- goto error_pci;
-#endif
-
#ifdef SM501_OHCI_DRIVER
retval = platform_driver_register(&SM501_OHCI_DRIVER);
if (retval < 0)
@@ -1455,10 +1442,6 @@ static int __init ohci_hcd_mod_init(void)
platform_driver_unregister(&SM501_OHCI_DRIVER);
error_sm501:
#endif
-#ifdef PCI_DRIVER
- pci_unregister_driver(&PCI_DRIVER);
- error_pci:
-#endif
#ifdef SA_DRIVER
sa_driver_unregister(&SA_DRIVER);
error_sa:
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 0b34b59..20f6187 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -14,12 +14,19 @@
* This file is licenced under the GPL.
*/
-#ifndef CONFIG_PCI
-#error "This file is PCI bus glue. CONFIG_PCI must be defined."
-#endif
-
-#include
#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ohci.h"
+#include "pci-quirks.h"
+
+#define DRIVER_DESC "OHCI PCI platform driver"
+
+static const char hcd_name[] = "ohci-pci";
/*-
[RFC PATCH 0/2] USB: OHCI: Start splitting up the driver
This series of patches begins the process of splitting ohci-hcd up into a core library module and independent pci driver modules. Patch 1/2 prepares the way by exporting a few functions from ohci-hcd and adding a new mechanism for platform-specific drivers to initialize their hc_driver structures. This deserves to be done in the core because almost all of the entries in these structures are pure boilerplate -- practically none of the drivers need to override more than three of the standard core values. Patch 2/2 separate out ohci-pci into independent driver modules. Manjunath Goudar (2): USB: OHCI: prepare to make ohci-hcd a library module USB: OHCI: make ohci-pci a separate driver drivers/usb/host/Kconfig |9 ++- drivers/usb/host/Makefile|3 + drivers/usb/host/ohci-hcd.c | 142 +- drivers/usb/host/ohci-pci.c | 129 ++ drivers/usb/host/ohci-platform.c |2 +- drivers/usb/host/ohci-q.c| 13 drivers/usb/host/ohci.h | 30 +++- 7 files changed, 197 insertions(+), 131 deletions(-) -- 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
[RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
This patch prepares ohci-hcd for being split up into a core
library and separate platform driver modules. A generic
ohci_hc_driver structure is created, containing all the "standard"
values, and a new mechanism is added whereby a driver module can
specify a set of overrides to those values. In addition the
ohci_restart(),ohci_suspend() and ohci_resume() routines need
to be EXPORTed for use by the drivers.
In V2:
-ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
-ohci_setup() and ohci_start() functions written to generic OHCI initialization
for all ohci bus glue.
Signed-off-by: Manjunath Goudar
Cc: Arnd Bergmann
Cc: Greg KH
Cc: Alan Stern
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/Makefile|2 +
drivers/usb/host/ohci-hcd.c | 123 +-
drivers/usb/host/ohci-pci.c |2 +-
drivers/usb/host/ohci-platform.c |2 +-
drivers/usb/host/ohci.h | 30 +-
5 files changed, 138 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 58c14c1..a38d76b 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA)+=ehci-tegra.o
obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
+
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9e6de95..7922c61 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -79,13 +79,7 @@ static const charhcd_name [] = "ohci_hcd";
#include "pci-quirks.h"
static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static int ohci_init (struct ohci_hcd *ohci);
-static void ohci_stop (struct usb_hcd *hcd);
-
-#if defined(CONFIG_PM) || defined(CONFIG_PCI)
-static int ohci_restart (struct ohci_hcd *ohci);
-#endif
-
+static void ohci_stop(struct usb_hcd *hcd);
#ifdef CONFIG_PCI
static void sb800_prefetch(struct ohci_hcd *ohci, int on);
#else
@@ -520,7 +514,7 @@ done:
/* init memory, and kick BIOS/SMM off */
-static int ohci_init (struct ohci_hcd *ohci)
+int ohci_init(struct ohci_hcd *ohci)
{
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
@@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
return ret;
}
+EXPORT_SYMBOL_GPL(ohci_init);
/*-*/
@@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
* resets USB and controller
* enable interrupts
*/
-static int ohci_run (struct ohci_hcd *ohci)
+static int ohci_run(struct usb_hcd *hcd)
{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
u32 mask, val;
int first = ohci->fminterval == 0;
- struct usb_hcd *hcd = ohci_to_hcd(ohci);
ohci->rh_state = OHCI_RH_HALTED;
@@ -767,7 +762,37 @@ retry:
return 0;
}
+/**/
+
+/* ohci generic function for all OHCI bus glue */
+
+static int ohci_setup(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int retval;
+
+ ohci->sbrn = HCD_USB11;
+
+ /* data structure init */
+ retval = ohci_init(ohci);
+ if (retval)
+ return retval;
+ ohci_usb_reset(ohci);
+ return 0;
+}
+static int ohci_start(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int ret;
+ ret = ohci_run(hcd);
+ if (ret < 0) {
+ ohci_err(ohci, "can't start\n");
+ ohci_stop(hcd);
+ }
+ return ret;
+}
+/*-*/
/*-*/
/* an interrupt happens */
@@ -949,11 +974,12 @@ static void ohci_stop (struct usb_hcd *hcd)
#if defined(CONFIG_PM) || defined(CONFIG_PCI)
/* must not be called from interrupt context */
-static int ohci_restart (struct ohci_hcd *ohci)
+int ohci_restart(struct ohci_hcd *ohci)
{
int temp;
int i;
struct urb_priv *priv;
+ struct usb_hcd *hcd;
spin_lock_irq(&ohci->lock);
ohci->rh_state = OHCI_RH_HALTED;
@@ -1001,19 +1027,22 @@ static int ohci_restart (struct ohci_hcd *ohci)
ohci->ed_controltail = NULL;
ohci->ed_bulktail= NULL;
- if ((temp = ohci_run (ohci)) < 0) {
- ohci_err (ohci, "can't restart, %d\n", temp);
+ hcd = ohci_to_hcd(ohci);
+ temp = ohci_run(hcd);
+ if (temp < 0) {
+ ohci_err(ohci, "can't restart, %d\n", temp);
return temp;
[RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver
This patch splits the PCI portion of ohci-hcd out into its
own separate driver module, called ohci-pci. Consistently with the
current practice, the decision whether to build this module is not
user-configurable. If OHCI_PCI are enabled then the module will
be built, always.
V2:
- few specific content of pci related code in ohci_pci_start function has
been moved to ohci_pci_reset
and rest of the generic code is written in ohci_start of ohci-hcd.c file.
Signed-off-by: Manjunath Goudar
Cc: Arnd Bergmann
Cc: Greg KH
Cc: Alan Stern
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/Kconfig|9 ++-
drivers/usb/host/Makefile |1 +
drivers/usb/host/ohci-hcd.c | 19 +--
drivers/usb/host/ohci-pci.c | 129 +--
drivers/usb/host/ohci-q.c | 13 +
5 files changed, 60 insertions(+), 111 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 7a1a248..3620ecce 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -333,8 +333,13 @@ config USB_ISP1362_HCD
To compile this driver as a module, choose M here: the
module will be called isp1362-hcd.
+config USB_OHCI_PCI
+ tristate
+ depends on USB_OHCI_HCD && PCI
+ default y
+
config USB_OHCI_HCD
- tristate "OHCI HCD support"
+ tristate "OHCI HCD (USB 1.1) support"
depends on USB && USB_ARCH_HAS_OHCI
select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
select USB_OTG_UTILS if ARCH_OMAP
@@ -402,7 +407,7 @@ config USB_OHCI_HCD_PPC_OF
default USB_OHCI_HCD_PPC_OF_BE || USB_OHCI_HCD_PPC_OF_LE
config USB_OHCI_HCD_PCI
- bool "OHCI support for PCI-bus USB controllers"
+ tristate "OHCI support for PCI-bus USB controllers"
depends on USB_OHCI_HCD && PCI && (STB03xxx || PPC_MPC52xx ||
USB_OHCI_HCD_PPC_OF)
default y
select USB_OHCI_LITTLE_ENDIAN
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index a38d76b..a20a8d9 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+obj-$(CONFIG_USB_OHCI_PCI) += ohci-pci.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 7922c61..2e62e7d 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -95,7 +95,6 @@ static inline void sb800_prefetch(struct ohci_hcd *ohci, int
on)
#include "ohci-mem.c"
#include "ohci-q.c"
-
/*
* On architectures with edge-triggered interrupts we must never return
* IRQ_NONE.
@@ -585,7 +584,6 @@ int ohci_init(struct ohci_hcd *ohci)
return ret;
}
EXPORT_SYMBOL_GPL(ohci_init);
-
/*-*/
/* Start an OHCI controller, set the BUS operational
@@ -1181,11 +1179,6 @@ MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
-#ifdef CONFIG_PCI
-#include "ohci-pci.c"
-#define PCI_DRIVER ohci_pci_driver
-#endif
-
#if defined(CONFIG_ARCH_SA1100) && defined(CONFIG_SA)
#include "ohci-sa.c"
#define SA_DRIVER ohci_hcd_sa_driver
@@ -1281,7 +1274,7 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVERohci_platform_driver
#endif
-#if!defined(PCI_DRIVER) && \
+#if!IS_ENABLED(CONFIG_USB_OHCI_PCI) && \
!defined(PLATFORM_DRIVER) &&\
!defined(OMAP1_PLATFORM_DRIVER) && \
!defined(OMAP3_PLATFORM_DRIVER) && \
@@ -1356,12 +1349,6 @@ static int __init ohci_hcd_mod_init(void)
goto error_sa;
#endif
-#ifdef PCI_DRIVER
- retval = pci_register_driver(&PCI_DRIVER);
- if (retval < 0)
- goto error_pci;
-#endif
-
#ifdef SM501_OHCI_DRIVER
retval = platform_driver_register(&SM501_OHCI_DRIVER);
if (retval < 0)
@@ -1455,10 +1442,6 @@ static int __init ohci_hcd_mod_init(void)
platform_driver_unregister(&SM501_OHCI_DRIVER);
error_sm501:
#endif
-#ifdef PCI_DRIVER
- pci_unregister_driver(&PCI_DRIVER);
- error_pci:
-#endif
#ifdef SA_DRIVER
sa_driver_unregister(&SA_DRIVER);
error_sa:
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 0b34b59..20f6187 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -14,12 +14,19 @@
* This file is licenced under the GPL.
*/
-#ifndef CONFIG_PCI
-#error "This file is PCI bus glue. CONFIG_PCI must be defined."
-#endif
-
-#include
#include
+#include
+#include
+#include
+#include
+#include
+
+#include "ohci.h"
+#include "pci-quirks.h"
+
+#define DRIVER_DESC "OHCI PCI platform driver"
+
+static const char hcd_name[] = "ohci-pci";
/*-
[PATCH] usb: musb: dsps: fix error return code in dsps_create_musb_pdev()
From: Wei Yongjun
Fix to return -ENOMEM in the devm_kzalloc() error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/musb/musb_dsps.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 3a18e44..e1b661d 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -560,6 +560,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue, u8
id)
if (!config) {
dev_err(&pdev->dev,
"failed to allocate musb hdrc config\n");
+ ret = -ENOMEM;
goto err2;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: s3c2410_udc: fix error return code in s3c2410_udc_probe()
From: Wei Yongjun
Fix to return a negative error code in the gpio_to_irq() error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/s3c2410_udc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
index d0e75e1..c974776 100644
--- a/drivers/usb/gadget/s3c2410_udc.c
+++ b/drivers/usb/gadget/s3c2410_udc.c
@@ -1851,6 +1851,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
irq = gpio_to_irq(udc_info->vbus_pin);
if (irq < 0) {
dev_err(dev, "no irq for gpio vbus pin\n");
+ retval = irq;
goto err_gpio_claim;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: r8a66597-udc: fix error return code in r8a66597_probe()
From: Wei Yongjun
Fix to return -ENOMEM in the request alloc error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/r8a66597-udc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/r8a66597-udc.c
b/drivers/usb/gadget/r8a66597-udc.c
index 0b742d1..7ff7d9c 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1977,8 +1977,10 @@ static int __init r8a66597_probe(struct platform_device
*pdev)
r8a66597->ep0_req = r8a66597_alloc_request(&r8a66597->ep[0].ep,
GFP_KERNEL);
- if (r8a66597->ep0_req == NULL)
+ if (r8a66597->ep0_req == NULL) {
+ ret = -ENOMEM;
goto clean_up3;
+ }
r8a66597->ep0_req->complete = nop_completion;
ret = usb_add_gadget_udc(&pdev->dev, &r8a66597->gadget);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: m66592-udc: fix error return code in m66592_probe()
From: Wei Yongjun
Fix to return -ENOMEM in the request alloc error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/m66592-udc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/m66592-udc.c b/drivers/usb/gadget/m66592-udc.c
index 866ef09..51cfe72 100644
--- a/drivers/usb/gadget/m66592-udc.c
+++ b/drivers/usb/gadget/m66592-udc.c
@@ -1660,8 +1660,10 @@ static int __init m66592_probe(struct platform_device
*pdev)
m66592->epaddr2ep[0] = &m66592->ep[0];
m66592->ep0_req = m66592_alloc_request(&m66592->ep[0].ep, GFP_KERNEL);
- if (m66592->ep0_req == NULL)
+ if (m66592->ep0_req == NULL) {
+ ret = -ENOMEM;
goto clean_up3;
+ }
m66592->ep0_req->complete = nop_completion;
init_controller(m66592);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fusb300_udc: fix error return code in fusb300_probe()
From: Wei Yongjun
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/fusb300_udc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/fusb300_udc.c b/drivers/usb/gadget/fusb300_udc.c
index cec8871..b8632d4 100644
--- a/drivers/usb/gadget/fusb300_udc.c
+++ b/drivers/usb/gadget/fusb300_udc.c
@@ -1461,8 +1461,10 @@ static int __init fusb300_probe(struct platform_device
*pdev)
fusb300->ep0_req = fusb300_alloc_request(&fusb300->ep[0]->ep,
GFP_KERNEL);
- if (fusb300->ep0_req == NULL)
+ if (fusb300->ep0_req == NULL) {
+ ret = -ENOMEM;
goto clean_up3;
+ }
init_controller(fusb300);
ret = usb_add_gadget_udc(&pdev->dev, &fusb300->gadget);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: dummy_hcd: fix error return code in init()
From: Wei Yongjun
Fix to return -ENOMEM in the kzalloc() error handling case instead
of 0(following platform_device_add_data() will overwrite it to 0), as
done elsewhere in this function.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/dummy_hcd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index a792e32..77e3023 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -2661,8 +2661,10 @@ static int __init init(void)
}
for (i = 0; i < mod_data.num; i++) {
dum[i] = kzalloc(sizeof(struct dummy), GFP_KERNEL);
- if (!dum[i])
+ if (!dum[i]) {
+ retval = -ENOMEM;
goto err_add_pdata;
+ }
retval = platform_device_add_data(the_hcd_pdev[i], &dum[i],
sizeof(void *));
if (retval)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: fix error return code in configfs_composite_bind()
From: Wei Yongjun
Fix to return a negative error code in the go through all configs error
handling case instead of 0(usb_add_function() will overwrite ret to 0).
Also use error code from usb_gstrings_attach() in all strings init error
case instead of -EINVAL.
Signed-off-by: Wei Yongjun
---
drivers/usb/gadget/configfs.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 3d5cfc9..80e7f75 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -821,8 +821,10 @@ static int configfs_composite_bind(struct usb_gadget
*gadget,
gi->gstrings[i] = NULL;
s = usb_gstrings_attach(&gi->cdev, gi->gstrings,
USB_GADGET_FIRST_AVAIL_IDX);
- if (IS_ERR(s))
+ if (IS_ERR(s)) {
+ ret = PTR_ERR(s);
goto err_comp_cleanup;
+ }
gi->cdev.desc.iManufacturer = s[USB_GADGET_MANUFACTURER_IDX].id;
gi->cdev.desc.iProduct = s[USB_GADGET_PRODUCT_IDX].id;
@@ -847,8 +849,10 @@ static int configfs_composite_bind(struct usb_gadget
*gadget,
}
cfg->gstrings[i] = NULL;
s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
- if (IS_ERR(s))
+ if (IS_ERR(s)) {
+ ret = PTR_ERR(s);
goto err_comp_cleanup;
+ }
c->iConfiguration = s[0].id;
}
--
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
NULL pointer dereference in xhci_free_dev
Hi,
there is a NULL pointer dereference in xhci_free_dev if
xhci_alloc_dev timeouts while waiting for a slot. Fedora has several bugs
reporting this problem.
https://bugzilla.redhat.com/show_bug.cgi?id=957500
https://bugzilla.redhat.com/show_bug.cgi?id=959016
https://bugzilla.redhat.com/show_bug.cgi?id=921659
https://bugzilla.redhat.com/show_bug.cgi?id=892935
I also found one mention about this on lkml without any reply
Date: Fri, 14 Dec 2012 22:54:28 +0100
From: Heinz Diehl
To: linux-ker...@vger.kernel.org
Cc: sta...@vger.kernel.org
Subject: [3.6.10] Null pointer dereference (xhci)
Message-ID: <20121214215428.ga4...@fancy-poultry.org>
https://lkml.org/lkml/2012/12/14/369
This was first seen on 3.6.7 and it is also reported for 3.9.0. IMHO the current
kernel has the same problem.
Here follows more info for one of the oops. It's for x86, but other reports are
for x86_64.
vanilla kernel 3.9.0 drivers/usb/host/xhci.c
3110 void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
3111 {
3112 struct xhci_hcd *xhci = hcd_to_xhci(hcd);
3113 struct xhci_virt_device *virt_dev;
3114 unsigned long flags;
3115 u32 state;
3116 int i, ret;
3117 ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__);
3118 /* If the host is halted due to driver unload, we still need to
free the
3119 * device.
3120 */
3121 if (ret <= 0 && ret != -ENODEV)
3122 return;
3123 virt_dev = xhci->devs[udev->slot_id];
3124 /* Stop any wayward timer functions (which may grab the lock) */
3125 for (i = 0; i < 31; ++i) {
3126 virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING;
3127 del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
3128 }
The problem is that virt_dev could be NULL(line 3123) and we get NULL deref.
while accessing endpoints(line 3126).
Linux version 3.9.0-0.rc8.git0.2.fc19.i686.PAE
BUG: unable to handle kernel NULL pointer dereference at 0024
IP: [] xhci_free_dev+0x60/0x130
*pdpt = *pde = f000eef3f000eef3
Oops: 0002 [#1] SMP
Modules linked in: ebtable_nat ipt_MASQUERADE nf_conntrack_netbios_ns
nf_conntrack_broadcast ip6table_nat nf_nat_ipv6 ip6table_mangle ip6t_REJECT
nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack bnep bluetooth
rfkill ebtable_filter ebtables ip6table_filter ip6_tables iTCO_wdt
iTCO_vendor_support ppdev snd_ice1724 snd_ak4113 snd_pt2258 snd_ak4114 mperf
snd_i2c snd_ice17xx_ak4xxx coretemp snd_ak4xxx_adda crc32_pclmul snd_rawmidi
crc32c_intel snd_ac97_codec ac97_bus serio_raw microcode i2c_i801 snd_seq
snd_seq_device lpc_ich snd_pcm mfd_core snd_page_alloc snd_timer snd soundcore
atl1c mei parport_pc parport vhost_net tun macvtap macvlan kvm_intel
binfmt_misc kvm usb_storage i915 i2c_algo_bit drm_kms_helper drm i2c_core video
uinput
Pid: 34, comm: khubd Not tainted 3.9.0-0.rc8.git0.2.fc19.i686.PAE #1 To Be
Filled By O.E.M. To Be Filled By O.E.M./H61M/U3S3
EIP: 0060:[] EFLAGS: 00010246 CPU: 0
EIP is at xhci_free_dev+0x60/0x130
EAX: EBX: 001f ECX: f12f7000 EDX:
ESI: ec2f4000 EDI: 003c EBP: f1323ec8 ESP: f1323ea8
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 80050033 CR2: 0024 CR3: 00d0f000 CR4: 000407f0
DR0: DR1: DR2: DR3:
DR6: 0ff0 DR7: 0400
Process khubd (pid: 34, ti=f1322000 task=f122d940 task.ti=f1322000)
Stack:
0001 c09f83d8 f09f4e00 f12f7000 ec2f4000 ff00 f0aec034
f1323f6c c07d97b4 0001 c0b7f660 0004 c0c00540 f1323f64 c09911c6
f75ba2c0 f1323efc f122d940 f0aec0a0 0064 f122d940 f0aafe1c f09f4e00
Call Trace:
[] hub_thread+0x704/0x1520
[] ? __schedule+0x366/0x780
[] ? wake_up_bit+0x20/0x20
[] ? hub_port_debounce+0x140/0x140
[] kthread+0x94/0xa0
[] ret_from_kernel_thread+0x1b/0x28
[] ? insert_kthread_work+0x30/0x30
Code: e8 16 ba ff ff 83 f8 ed 0f 85 bd 00 00 00 8b 86 a8 02 00 00 bb 1f 00 00
00 8b 55 f0 8b 94 82 a4 00 00 00 8d 7a 3c 90 8d 74 26 00 <83> 67 e8 fb 89 f8 81
c7 8c 00 00 00 e8 bf 4f c5 ff 83 eb 01 75
EIP: [] xhci_free_dev+0x60/0x130 SS:ESP 0068:f1323ea8
CR2: 0024
Code: e8 16 ba ff ff 83 f8 ed 0f 85 bd 00 00 00 8b 86 a8 02 00 00 bb 1f 00 00
00 8b 55 f0 8b 94 82 a4 00 00 00 8d 7a 3c 90 8d 74 26 00 <83> 67 e8 fb 89 f8 81
c7 8c 00 00 00 e8 bf 4f c5 ff 83 eb 01 75
All code
0: e8 16 ba ff ff call 0xba1b
5: 83 f8 edcmp$0xffed,%eax
8: 0f 85 bd 00 00 00 jne0xcb
e: 8b 86 a8 02 00 00 mov0x2a8(%esi),%eax
14: bb 1f 00 00 00 mov$0x1f,%ebx
19: 8b 55 f0mov-0x10(%ebp),%edx
1c: 8b 94 82 a4 00 00 00mov0xa4(%edx,%eax,4),%edx
23: 8d 7a 3clea0x3c(%edx),%edi
Re: [PATCH] usb: gadget: dummy_hcd: fix error return code in init()
On Tue, 7 May 2013, Wei Yongjun wrote:
> From: Wei Yongjun
>
> Fix to return -ENOMEM in the kzalloc() error handling case instead
> of 0(following platform_device_add_data() will overwrite it to 0), as
> done elsewhere in this function.
>
> Signed-off-by: Wei Yongjun
> ---
> drivers/usb/gadget/dummy_hcd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> index a792e32..77e3023 100644
> --- a/drivers/usb/gadget/dummy_hcd.c
> +++ b/drivers/usb/gadget/dummy_hcd.c
> @@ -2661,8 +2661,10 @@ static int __init init(void)
> }
> for (i = 0; i < mod_data.num; i++) {
> dum[i] = kzalloc(sizeof(struct dummy), GFP_KERNEL);
> - if (!dum[i])
> + if (!dum[i]) {
> + retval = -ENOMEM;
> goto err_add_pdata;
> + }
> retval = platform_device_add_data(the_hcd_pdev[i], &dum[i],
> sizeof(void *));
> if (retval)
Acked-by: Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Linux USB file storage gadget with new UDC
On Tue, 7 May 2013, victor yeo wrote: > Hi, > > >> How the UDC driver know when the request is really complete? > > > > An OUT request is really complete when either: > > > > The total number of bytes copied into req.buffer (i.e., > > req.actual) is equal to req.length, or > > > > The number of bytes received in the last packet is smaller > > than ep.maxpacket. > > I made some changes regarding req.actual. Now the UDC driver still > cannot process SCSI_WRITE_10 command. Please see the attached UDC > driver log when i try to write to a text file. There should be three > SCSI commands in the log: SCSI_REQUEST_SENSE, SCSI_TEST_UNIT_READY and > SCSI_WRITE_10. SCSI_WRITE_10 is not received properly. No, it isn't. Here's what the log says: > EP1 OUT IRQ 0x28 > ep1_out: RX DMA done : NULL REQ on OUT EP-1 > [start_transfer] 53425355 10d > ept1 out queue len 0x200, buffer 0xc1304000 > g_file_storage gadget: bulk-out, length 31: This is from bulk_out_complete, when the WRITE(10) was received. > EP1 OUT IRQ 0x28 > epnum 1 in 0 len 0 512 0 > g_file_storage gadget: bulk-out, length 0: This line indicates a bug. It means the UDC driver called bulk_out_complete again, even though the previous request was no longer queued and no new requests had been submitted yet. It is likely that this bug occurs because you don't use a spinlock in kagen2_ep_queue. Does the interrupt handler routine use a spinlock? Maybe you haven't noticed this, but the REQUEST SENSE and TEST UNIT READY commands weren't received correctly either. The last three bytes in each command should be 0, but they aren't. They are: c3 63 4a. Where did those values come from? 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: Linux USB file storage gadget with new UDC
Hi, >> I made some changes regarding req.actual. Now the UDC driver still >> cannot process SCSI_WRITE_10 command. Please see the attached UDC >> driver log when i try to write to a text file. There should be three >> SCSI commands in the log: SCSI_REQUEST_SENSE, SCSI_TEST_UNIT_READY and >> SCSI_WRITE_10. SCSI_WRITE_10 is not received properly. > > No, it isn't. Here's what the log says: > >> EP1 OUT IRQ 0x28 >> ep1_out: RX DMA done : NULL REQ on OUT EP-1 >> [start_transfer] 53425355 10d >> ept1 out queue len 0x200, buffer 0xc1304000 >> g_file_storage gadget: bulk-out, length 31: > > This is from bulk_out_complete, when the WRITE(10) was received. > >> EP1 OUT IRQ 0x28 >> epnum 1 in 0 len 0 512 0 >> g_file_storage gadget: bulk-out, length 0: > > This line indicates a bug. It means the UDC driver called > bulk_out_complete again, even though the previous request was no longer > queued and no new requests had been submitted yet. > > It is likely that this bug occurs because you don't use a spinlock in > kagen2_ep_queue. Does the interrupt handler routine use a spinlock? Spinlock is Not used in interrupt handler routine. >[start_transfer] 53425355 10d >ept1 out queue len 0x200, buffer 0xc1304000 >g_file_storage gadget: bulk-out, length 31: Is the kagen2_ep_queue function gotten interrupted here? So the kagen2_ep_queue and interrupt routine need spinlock for synchronisation? >EP1 OUT IRQ 0x28 >epnum 1 in 0 len 0 512 0 >g_file_storage gadget: bulk-out, length 0: > Maybe you haven't noticed this, but the REQUEST SENSE and TEST UNIT > READY commands weren't received correctly either. The last three bytes > in each command should be 0, but they aren't. They are: c3 63 4a. > Where did those values come from? Yes, i haven't noticed the c3 63 4a. Clearly the last three bytes should be zero. Maybe the UDC driver has a bug (Do the last 3 bytes matter for file gadget? ). Here is the usbmon trace that corresponds to the UDC log. It is the proof that the last three bytes are zero. Thanks, victor scsi_write_10_again02.log Description: Binary data
Re: [PATCH 1/2] usb: gadget/uvc: remove connect/disconnect calls on open/release
Hi, On 5/7/2013 12:35 PM, Laurent Pinchart wrote: Hi Vladimir, On Monday 06 May 2013 13:42:45 Vladimir Zapolskiy wrote: On 05/04/13 21:22, Bhupesh SHARMA wrote: On 5/3/2013 6:00 PM, Vladimir Zapolskiy wrote: On 05/03/13 02:05, Laurent Pinchart wrote: On Friday 03 May 2013 02:00:29 Vladimir Zapolskiy wrote: On 05/03/13 01:18, Laurent Pinchart wrote: On Friday 03 May 2013 01:13:48 Vladimir Zapolskiy wrote: This change removes redundant calls to uvc_function_connect() and uvc_function_disconnect() on V4L2 device node open and release. These two functions attemp to control pull-up on D+ line directly, however such an action should be performed by an UDC iteself, and within the gadget there is no information about current mode of the controller. The UDC may be in suspended state, or an OTG controller may be in host mode, therefore it seems better not to try to forcibly pull-up D+ line on open() syscall. OK, but we then need to fix the problem properly. The UVC gadget must not appear connected until an application opens the corresponding device. Likewise, it must disconnect from the bus when the application closes the device. How can this be guaranteed properly ? For better understanding of the issue, could you explain briefly why do you prefer to have the gadget not connected to the bus, if device node is not opened? As soon as the gadget will connect to the bus the device will be enumerated by the host and bound to a host driver that will query the device using UVC-specific requests. The userspace application is involved in replying to those requests, so it needs to be bound to the device on the gadget side or the initialization process on the host side will fail. It might be a flaw in design, if a kernel space component depends on a user space application to be operable. Also the same scenario seems to be invalid, if an application unawared of specific to UVC features of /dev/videoX opens the device node, e.g. http://git.kernel.org/cgit/linux/hotplug/udev.git/tree/src/v4l_id/v4l_id. c or yavta etc. I presume a host should dictate behaviour of device and gadget in particular, and not a target's user space application, please correct me. As Bhupesh already explained, the complexity of handling video streaming requires a userspace component. Port of the uvc-gadget application could be moved to the kernel driver, but we will always need a userspace component to supply video data and reply to control requests. About this particular change, as I mentioned in a cover letter an alternative approach may be to add sanity checks to .pullup operations for every UDC driver (or probably to usb_gadget_connect()), but in this case it is not clear how UVC gadget is going to be notified about changes of UDC state, e.g. assume a test that /dev/videoX is opened, when OTG is in Host mode, device registration doesn't happen on open(), and then USB B cable is inserted to the port. I would appreciate your thoughts. The whole point of having a user-space application governing the behavior of UVC webcam gadget as per commands from a UVC host is to plug the same with a real video capture source driver to provide the video frames captured from say a camera sensor and route the UVC specific control requests to a real video capture device by converting the same to equivalent V4L2 commands. thank you for the explanation, however my original question is about avoiding critical hardware errors attended to the current gadget's design. If you think that UDC should be controlled every time on open()/close() syscalls, how do you see an optimal way to mitigate problems described above? I think the issue is on the UDC side. A function driver should be allowed to tell when it's ready to be connected and when it needs to be disconnected. The UDC of course isn't required to connect the device to the bus as soon as the function request the connection, it can delay that until it's ready (switched from host mode to device mode in case of OTG, resumed if it has been suspended, ...). The API offered to function drivers should handle this, either in the gadget core or in the UDC themselves (or possibly a combination of the two). I agree with Laurent here. There is no point in connecting function driver with the UDC unless we are sure that the control requests/STREAMON event can be handled at the real video capture device. Remember that the real video capture sensor can be controlled by some slow external bus like I2C, SPI. So, we should not connect to the UDC unless we actually have the real video streaming device up and configured. Note that a number of real camera sensors require a large no of registers to be written via I2C commands to put them into a "good" default state. As the V4L2 subsystem is designed to work on basis of IOCTLs from user-space, it is possible but not that easy, to move a part of what is now handled in user-space for interaction b/w UVC and V4L2 subsystems into kernel-space, but still
Re: [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
On Tue, 7 May 2013, Manjunath Goudar wrote:
> This patch prepares ohci-hcd for being split up into a core
> library and separate platform driver modules. A generic
> ohci_hc_driver structure is created, containing all the "standard"
> values, and a new mechanism is added whereby a driver module can
> specify a set of overrides to those values. In addition the
> ohci_restart(),ohci_suspend() and ohci_resume() routines need
> to be EXPORTed for use by the drivers.
This patch still has several problems. For example, the description
doesn't mention the fact that ohci_init() was EXPORTed.
In fact, why was ohci_init() EXPORTed? I don't see any reason for
this. ohci_pci.c doesn't need to call it; just insert a call to
ohci_init() at the beginning of ohci_restart().
> In V2:
> -ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
They don't "revert" since they have never been non-static. You should
say something more like "ohci_hcd_init(), ohci_run(), and ohci_stop()
are not made non-static."
> -ohci_setup() and ohci_start() functions written to generic OHCI
> initialization
> for all ohci bus glue.
Fix the grammar in that sentence. And you should mention these new
functions in the main part of the patch description, not just down here.
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 58c14c1..a38d76b 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA)+=ehci-tegra.o
> obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
> obj-$(CONFIG_USB_ISP116X_HCD)+= isp116x-hcd.o
> obj-$(CONFIG_USB_ISP1362_HCD)+= isp1362-hcd.o
> +
> obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
> +
You do not need to add these blank lines in this patch. If you want,
you can add them in the ohci-pci patch.
> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
> obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 9e6de95..7922c61 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -79,13 +79,7 @@ static const char hcd_name [] = "ohci_hcd";
> #include "pci-quirks.h"
>
> static void ohci_dump (struct ohci_hcd *ohci, int verbose);
> -static int ohci_init (struct ohci_hcd *ohci);
> -static void ohci_stop (struct usb_hcd *hcd);
I thought ohci_stop() wasn't going to be changed in this patch. Why
was this line updated?
> -
> -#if defined(CONFIG_PM) || defined(CONFIG_PCI)
> -static int ohci_restart (struct ohci_hcd *ohci);
> -#endif
> -
> +static void ohci_stop(struct usb_hcd *hcd);
> #ifdef CONFIG_PCI
> static void sb800_prefetch(struct ohci_hcd *ohci, int on);
> #else
> @@ -520,7 +514,7 @@ done:
>
> /* init memory, and kick BIOS/SMM off */
>
> -static int ohci_init (struct ohci_hcd *ohci)
> +int ohci_init(struct ohci_hcd *ohci)
> {
> int ret;
> struct usb_hcd *hcd = ohci_to_hcd(ohci);
> @@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(ohci_init);
>
> /*-*/
>
> @@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
> * resets USB and controller
> * enable interrupts
> */
> -static int ohci_run (struct ohci_hcd *ohci)
> +static int ohci_run(struct usb_hcd *hcd)
Why did you change the signature of this function? By doing so, you
just broke all the bus glue files. (Except for ohci_pci and
ohci_platform, which explicitly get fixed below.)
Since this function remains static, there's no reason to change it.
> {
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> u32 mask, val;
> int first = ohci->fminterval == 0;
> - struct usb_hcd *hcd = ohci_to_hcd(ohci);
>
> ohci->rh_state = OHCI_RH_HALTED;
>
> @@ -767,7 +762,37 @@ retry:
>
> return 0;
> }
> +/**/
> +
> +/* ohci generic function for all OHCI bus glue */
> +
> +static int ohci_setup(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> + int retval;
> +
> + ohci->sbrn = HCD_USB11;
What is this doing here? Why did you add this "sbrn" member to struct
ohci_hcd?
> +
> + /* data structure init */
> + retval = ohci_init(ohci);
> + if (retval)
> + return retval;
> + ohci_usb_reset(ohci);
Why is this call here? Doesn't ohci_init() already call
ohci_usb_reset()?
> + return 0;
> +}
>
> +static int ohci_start(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> + int ret;
There should be a blank line between the declarations and the
executable statements.
> + ret = ohci_run(hcd);
> + if (ret < 0) {
> + ohci_err(ohci, "can't start\n");
> +
Re: Linux USB file storage gadget with new UDC
On Tue, 7 May 2013, victor yeo wrote: > > It is likely that this bug occurs because you don't use a spinlock in > > kagen2_ep_queue. Does the interrupt handler routine use a spinlock? > > Spinlock is Not used in interrupt handler routine. Then that's the reason for this bug. > >[start_transfer] 53425355 10d > >ept1 out queue len 0x200, buffer 0xc1304000 > >g_file_storage gadget: bulk-out, length 31: > > Is the kagen2_ep_queue function gotten interrupted here? So the > kagen2_ep_queue and interrupt routine need spinlock for > synchronisation? That's right. Interrupts can occur at almost any time (on multiprocessor systems they can occur even when interrupts are disabled on some of the CPUs). > > Maybe you haven't noticed this, but the REQUEST SENSE and TEST UNIT > > READY commands weren't received correctly either. The last three bytes > > in each command should be 0, but they aren't. They are: c3 63 4a. > > Where did those values come from? > > Yes, i haven't noticed the c3 63 4a. Clearly the last three bytes > should be zero. Maybe the UDC driver has a bug (Do the last 3 bytes > matter for file gadget? ). Well, it certainly matters if some of the bytes in the data blocks for a WRITE command get messed up! 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 1/2 v6 RESEND] usbnet: allow status interrupt URB to always be active
On Monday 06 May 2013 16:29:23 Dan Williams wrote: > Some drivers (sierra_net) need the status interrupt URB > active even when the device is closed, because they receive > custom indications from firmware. Add functions to refcount > the status interrupt URB submit/kill operation so that > sub-drivers and the generic driver don't fight over whether > the status interrupt URB is active or not. > > A sub-driver can call usbnet_status_start() at any time, but > the URB is only submitted the first time the function is > called. Likewise, when the sub-driver is done with the URB, > it calls usbnet_status_stop() but the URB is only killed when > all users have stopped it. The URB is still killed and > re-submitted for suspend/resume, as before, with the same > refcount it had at suspend. > > Signed-off-by: Dan Williams Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for May 7 (usb/chipidea)
On 05/06/13 20:49, Stephen Rothwell wrote: > Hi all, > > Please do not add any v3.11 destined work to your linux-next included > branches until after v3.10-rc1 is released. > > I am receiving a (un)reasonable number of conflicts from there being > multiple copies of some commits in various trees. Please clean this up > and resist the temptataion to rebase your trees on the way to your > upstream ... > > Changes since 20130506: > on i386: drivers/built-in.o: In function `ci_hdrc_host_init': (.text+0x2ce75c): undefined reference to `ehci_init_driver' when USB_EHCI_HCD=m and USB_CHIPIDEA=y. -- ~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/3] ARM: shmobile: BOCK-W: add USB support
Hello. On 05/01/2013 03:14 AM, Sergei Shtylyov wrote: Register the USB PHY device from bockw_init(), passing the platform data to it. Set machine's init_late() method to r8a7778_init_late() in order for [EO]HCI to get registered too... The patch has been tested on the BOCK-W board. Signed-off-by: Sergei Shtylyov Oops, the patch misses the USB pin setup. The USB pin groups in the sh-pfc driver have also not been created. This needs to be addressed in the next iteration... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
dwc3 on omap5432 no SuperSpeed host mode
We are using an OMAP5432 ES2.0 on an UEVM board and running into issues with the dwc3 usb. The first issue is that the xhci reset code stops the entire core from working after it is executed. We have commented the xhci reset code from issuing the CMD_RESET and this has allowed us to get the XHCI platform driver to work in some capacity (and swap back to gadget mode if needed). Is the xhci_reset issue known? Should I supply a patch to allow the platform code to override the xhci reset with a custom reset hook? The second issue is that once this is working, we have not been able to get the board to function as a SuperSpeed host (it will work as a SuperSpeed gadget to a Linux PC). Any device plugged in (we are testing with a pen-drive, a usb3 network adapter and a usb3 transcend card reader) will either fail to enumerate at-all, or if it does get detected it falls back to high-speed (which does work). I can supply logs if needed. -- Ben Dooks, Senior Engineer http://www.codethink.co.uk/ -- 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: set device dma_mask without reference to global data
From: Stephen Warren
Many USB host drivers contain code such as:
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &tegra_ehci_dma_mask;
... where tegra_ehci_dma_mask is a global. I suspect this code originated
in commit 4a53f4e "USB: ehci-tegra: add probing through device tree" and
was simply copied everywhere else.
This works fine when the code is built-in, but can cause a crash when the
code is in a module. The first module load sets up the dma_mask pointer,
but if the module is removed and re-inserted, the value is now non-NULL,
and hence is not updated to point at the new location, and hence points
at a stale location within the previous module load address, which in
turn causes a crash if the pointer is de-referenced.
The simplest way of solving this seems to be to copy the code from
ehci-platform.c, which uses the coherent_dma_mask as the target for the
dma_mask pointer.
Suggested-by: Arnd Bergmann
Signed-off-by: Stephen Warren
---
drivers/usb/chipidea/ci13xxx_imx.c | 15 ---
drivers/usb/dwc3/dwc3-exynos.c |6 +++---
drivers/usb/host/ehci-atmel.c |6 +++---
drivers/usb/host/ehci-omap.c |8
drivers/usb/host/ehci-orion.c |6 +++---
drivers/usb/host/ehci-s5p.c|4 +---
drivers/usb/host/ehci-spear.c |6 +++---
drivers/usb/host/ehci-tegra.c |6 +++---
drivers/usb/host/ohci-at91.c |6 +++---
drivers/usb/host/ohci-exynos.c |4 +---
drivers/usb/host/ohci-omap3.c |8
drivers/usb/host/ohci-pxa27x.c |6 +++---
drivers/usb/host/ohci-spear.c |6 +++---
drivers/usb/host/uhci-platform.c |6 +++---
14 files changed, 41 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8faec9d..73f9d5f 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -173,17 +173,10 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
ci13xxx_imx_platdata.phy = data->phy;
- if (!pdev->dev.dma_mask) {
- pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
- sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
- if (!pdev->dev.dma_mask) {
- ret = -ENOMEM;
- dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
- goto err;
- }
- *pdev->dev.dma_mask = DMA_BIT_MASK(32);
- dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
- }
+ if (!pdev->dev.dma_mask)
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ if (!pdev->dev.coherent_dma_mask)
+ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
if (usbmisc_ops && usbmisc_ops->init) {
ret = usbmisc_ops->init(&pdev->dev);
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index a8afe6e..929e7dd 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -95,8 +95,6 @@ static int dwc3_exynos_remove_child(struct device *dev, void
*unused)
return 0;
}
-static u64 dwc3_exynos_dma_mask = DMA_BIT_MASK(32);
-
static int dwc3_exynos_probe(struct platform_device *pdev)
{
struct dwc3_exynos *exynos;
@@ -118,7 +116,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
* Once we move to full device tree support this will vanish off.
*/
if (!dev->dma_mask)
- dev->dma_mask = &dwc3_exynos_dma_mask;
+ dev->dma_mask = &dev->coherent_dma_mask;
+ if (!dev->coherent_dma_mask)
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
platform_set_drvdata(pdev, exynos);
diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index 6642009..02f4611 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -63,8 +63,6 @@ static void atmel_stop_ehci(struct platform_device *pdev)
/*-*/
-static u64 at91_ehci_dma_mask = DMA_BIT_MASK(32);
-
static int ehci_atmel_drv_probe(struct platform_device *pdev)
{
struct usb_hcd *hcd;
@@ -93,7 +91,9 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
* Once we have dma capability bindings this can go away.
*/
if (!pdev->dev.dma_mask)
- pdev->dev.dma_mask = &at91_ehci_dma_mask;
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+ if (!pdev->dev.coherent_dma_mask)
+ pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
if (!hcd) {
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 3d1491b..16d7150 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -90,8 +90,6 @@ static const st
Re: [PATCH] USB: set device dma_mask without reference to global data
On Tue, May 07, 2013 at 04:53:52PM -0600, Stephen Warren wrote: > From: Stephen Warren > > Many USB host drivers contain code such as: > > if (!pdev->dev.dma_mask) > pdev->dev.dma_mask = &tegra_ehci_dma_mask; > > ... where tegra_ehci_dma_mask is a global. I suspect this code originated > in commit 4a53f4e "USB: ehci-tegra: add probing through device tree" and > was simply copied everywhere else. > > This works fine when the code is built-in, but can cause a crash when the > code is in a module. The first module load sets up the dma_mask pointer, > but if the module is removed and re-inserted, the value is now non-NULL, > and hence is not updated to point at the new location, and hence points > at a stale location within the previous module load address, which in > turn causes a crash if the pointer is de-referenced. > > The simplest way of solving this seems to be to copy the code from > ehci-platform.c, which uses the coherent_dma_mask as the target for the > dma_mask pointer. > > Suggested-by: Arnd Bergmann > Signed-off-by: Stephen Warren So this needs to go in for 3.10, right? Any older kernels as well? If so, which ones? 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] USB: set device dma_mask without reference to global data
On Wednesday 08 May 2013, Greg Kroah-Hartman wrote: > On Tue, May 07, 2013 at 04:53:52PM -0600, Stephen Warren wrote: > > From: Stephen Warren > > Suggested-by: Arnd Bergmann > > Signed-off-by: Stephen Warren > > So this needs to go in for 3.10, right? Any older kernels as well? If > so, which ones? The fix should definitely go into 3.10, but I'd suggest waiting with the backport for a couple of -rc releases to avoid possible regressions. We know that the current code is broken, but few people fully understand what is going on with coherent_dma_mask, so it might cause new problems in combination with some other unknown bug, and I don't see this as urgent: none of the ARM defconfigs build this driver as a loadable module and there is no bug in the built-in case. For some reason, only the ARM back-end drivers are broken. The first occurence was apparently in 3.3, but only in ehci-tegra.c, while the other drivers subsequently copied the bug. Arnd -- 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: set device dma_mask without reference to global data
On Wed, May 8, 2013 at 6:53 AM, Stephen Warren wrote: > From: Stephen Warren > > Many USB host drivers contain code such as: > > if (!pdev->dev.dma_mask) > pdev->dev.dma_mask = &tegra_ehci_dma_mask; > > ... where tegra_ehci_dma_mask is a global. I suspect this code originated > in commit 4a53f4e "USB: ehci-tegra: add probing through device tree" and > was simply copied everywhere else. > One question: why device tree can't do this when create device? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: s3c2410_udc: fix error return code in s3c2410_udc_probe()
On Tuesday, May 07, 2013 8:48 PM, Wei Yongjun wrote:
>
> From: Wei Yongjun
>
> Fix to return a negative error code in the gpio_to_irq() error handling
> case instead of 0, as done elsewhere in this function.
>
> Signed-off-by: Wei Yongjun
Reviewed-by: Jingoo Han
Best regards,
Jingoo Han
> ---
> drivers/usb/gadget/s3c2410_udc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/gadget/s3c2410_udc.c
> b/drivers/usb/gadget/s3c2410_udc.c
> index d0e75e1..c974776 100644
> --- a/drivers/usb/gadget/s3c2410_udc.c
> +++ b/drivers/usb/gadget/s3c2410_udc.c
> @@ -1851,6 +1851,7 @@ static int s3c2410_udc_probe(struct platform_device
> *pdev)
> irq = gpio_to_irq(udc_info->vbus_pin);
> if (irq < 0) {
> dev_err(dev, "no irq for gpio vbus pin\n");
> + retval = irq;
> goto err_gpio_claim;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> 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: [PATCH] USB: set device dma_mask without reference to global data
On 05/07/2013 07:13 PM, Peter Chen wrote: > On Wed, May 8, 2013 at 6:53 AM, Stephen Warren wrote: >> From: Stephen Warren >> >> Many USB host drivers contain code such as: >> >> if (!pdev->dev.dma_mask) >> pdev->dev.dma_mask = &tegra_ehci_dma_mask; >> >> ... where tegra_ehci_dma_mask is a global. I suspect this code originated >> in commit 4a53f4e "USB: ehci-tegra: add probing through device tree" and >> was simply copied everywhere else. > > One question: why device tree can't do this when create device? This probably could be initialized from some DT property. However, there's no such property defined right now, and considering that DT is supposed to be an ABI, we'd always need the code in this patch as a fallback for DTs that were created before any such property was defined. Equally, since the data is SoC-specific rather than board-specific, and is even fairly unlikely to vary between SoC versions since these values are all 0x anyway, I don't really see much point in putting it into DT, rather than just putting the static data into the driver. -- 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: set device dma_mask without reference to global data
> > This probably could be initialized from some DT property. However, > there's no such property defined right now, and considering that DT is > supposed to be an ABI, we'd always need the code in this patch as a > fallback for DTs that were created before any such property was defined. > > Equally, since the data is SoC-specific rather than board-specific, and > is even fairly unlikely to vary between SoC versions since these values > are all 0x anyway, I don't really see much point in putting it > into DT, rather than just putting the static data into the driver. I mean there is already dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); at function of_platform_device_create, why can't add dev->dev.dma_mask = &dev->dev.coherent_dma_mask after that? If DT core can do above things, can we delete dma_mask assignment at every driver? -- BR, Peter Chen -- 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: set device dma_mask without reference to global data
On 08/05/13 10:53, Stephen Warren wrote: From: Stephen Warren Many USB host drivers contain code such as: if (!pdev->dev.dma_mask) pdev->dev.dma_mask = &tegra_ehci_dma_mask; ... where tegra_ehci_dma_mask is a global. I suspect this code originated in commit 4a53f4e "USB: ehci-tegra: add probing through device tree" and was simply copied everywhere else. This works fine when the code is built-in, but can cause a crash when the code is in a module. The first module load sets up the dma_mask pointer, but if the module is removed and re-inserted, the value is now non-NULL, and hence is not updated to point at the new location, and hence points at a stale location within the previous module load address, which in turn causes a crash if the pointer is de-referenced. The simplest way of solving this seems to be to copy the code from ehci-platform.c, which uses the coherent_dma_mask as the target for the dma_mask pointer. Suggested-by: Arnd Bergmann Signed-off-by: Stephen Warren --- In the case of uhci-platform you would be absolutely correct. I copied the example from tegra when we first had the problem on arch-vt8500.Because we have no NAND support yet, I have always booted myrootfs from USB so it's always been builtin and the problem wasnever a problem. The same problem would have existed on ehci-vt8500 but Arnd replaced it with ehci-platform due to the multiplatform issues. for uhci-platform.c Acked-by: Tony Prisk Regards Tony P -- 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
