Re: Mouse works with eHCI, fails with xHCI - can't set config #1, error -110

2015-04-10 Thread Alistair Grant
Hi Alan,

On Fri, Apr 10, 2015 at 7:23 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 10 Apr 2015, Alistair Grant wrote:
 On Fri, Apr 10, 2015 at 5:29 PM, Alan Stern st...@rowland.harvard.edu 
 wrote:
  On Fri, 10 Apr 2015, Alistair Grant wrote:
  ...
  i.e. the mouse works reliably in eHCI controllers, but not in xHCI
  controllers (I've tried two different Intel xHCI controllers).  The
 
  Have you tried testing a different mouse?

 I've got two other mice that work(ed) with xHCI (the first has died,
 which led to buying the one with the problems, and a second one which
 was purchased to tide me over while I try and figure out the problem
 with the problem mouse).

 It does sound as though the mouse is the major part of the problem.

  Can you post the usbmon log for an EHCI controller?  Comparing the two
  logs may be helpful.

 Here's the relevant part of the xHCI trace:
 ...

Thanks very much for your detailed analysis, I really appreciate it
(as someone trying to learn a little about the USB protocol).

I was hoping that this was hitting an edge case with the xHCI driver,
but as you say, it looks like the mouse is at fault.

Thanks again,
Alistair
--
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: keyboard/trackpad combo unusable on MacBookPro4,1 with bcm5974.ko

2015-04-10 Thread Alan Stern
On Fri, 10 Apr 2015, Christian Böhme wrote:

 Alan Stern stern@... writes:
 
   Given that hid_mouse_ignore_list[] actually contains the proper
   (USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING2_ISO) pair
   as far as I can make out and assuming a properly working compiler,
   hid_match_one_id() returns FALSE only if the hdev's members in question
   are not properly set up. And I wouldn't know where to look for /that/.
  
  You ought to be able to figure out what's going on by adding a few 
  debugging statements in suitable places.
   ^^^
 
 What are those ominous places?

The places where the code sets and tests the HID_TYPE_USBMOUSE flag, 
and where it tests for a matching ID.  Basically, the places that 
affect the thing you want to accomplish.

   On a side note, I'm a little surprised that the arguments to …
   are not const-qualified, although none of their members are actually
   mutated there …
  
  That sort of thing happens all the time.  People don't remember to add 
  const qualifications because it's not obvious when they _could_ be 
  added,
 
 How can it not be obvious to the author(s) of their own code?

You seem to forget that the kernel was written by thousands of 
different people.  Even if somebody is familiar with his own code, he 
isn't necessarily familiar with code written by someone else.

  Does
 the semantics of the language randomly change when nobody's watching?

That would make computer science a lot more intriguing!  :-)

 the compiler doesn't complain about it,
 
 $ gcc -v
 Using built-in specs.
 Target: x86_64-redhat-linux
 Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
 --infodir=/usr/share/info --enable-shared --enable-threads=posix
 --enable-checking=release --with-system-zlib --enable-__cxa_atexit
 --disable-libunwind-exceptions --enable-libgcj-multifile
 --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk
 --disable-dssi --disable-plugin
 --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre --with-cpu=generic
 --host=x86_64-redhat-linux
 Thread model: posix
 gcc version 4.1.2 20080704 (Red Hat 4.1.2-51)
 $ gcc -Wall member.c -o member
 member.c: In function ‘g’:
 member.c:12: error: assignment of read-only location
 $ cat member.c 
 struct A {
 int x, y;
 };
 
 void f( struct A * a )
 {
 a-x = 1;
 }
 
 void g( const struct A * a )
 {
 a-y = 2;
 }
 
 int main( int argc, char * argv[] )
 {
 struct A a;
 
 f(a);
 g(a);
 
 return 0;
 }
 
 This one does. And I'm positive all newer versions do, too.

Your example doesn't contradict what I said.  Go back and reread my
comment, and then consider this example instead:

$ gcc -Wall const-test.c -o const-test
$ cat const-test.c
const char *y;

void a(char *x)
{
y = x;
}

int main(int argc, char **argv)
{
a(hello);
return 0;
}

The function a() makes no assignments through x.  Therefore it could
and should have been defined as void a(const char *x) -- but the
compiler did not complain.

 and it doesn't make much 
  difference anyway.
 
 I disagree. It's precisely what aids in avoiding situations like the
 current one where nobody seems to know what's going on anymore. That's
 what const-qualifications where invented for (if there is such a thing).
 Somewhere a member's value is changed where it shouldn't. How can that
 not make a difference?

You might be able to find a few cases where the kernel code changes a
value that should have been declared const, but not many.  Like I said 
before, the real problem is that there's no penalty if you forget to 
mark something as const when the code _doesn't_ change it.

If you want, you could submit a series of patches that do nothing but 
add const declarations in the places where they are missing.

Alan Stern

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


[PATCH] usb: phy/isp1301: work around tps65010 dependency

2015-04-10 Thread Arnd Bergmann
The isp1301-omap driver contains special hooks for the TPS65010
power management controller. It provides its own 'tps65010_set_vbus_draw'
wrapper in case that driver is not enabled through Kconfig, but
fails to handle the case where isp1301-omap is built-in but TPS65010
is a loadable module, which currently results in a link error:

drivers/built-in.o: In function `isp1301_set_power':
:(.text+0x14e188): undefined reference to `tps65010_set_vbus_draw'

This is a workaround to use the same trick as before also when
tps65010 is a module. Doing a proper fix would require much larger
changes to the driver that is not really worth it when the usb-phy
drivers are going to eventually get replaced with generic-phy
drivers.

Signed-off-by: Arnd Bergmann a...@arndb.de

diff --git a/drivers/usb/phy/phy-isp1301-omap.c 
b/drivers/usb/phy/phy-isp1301-omap.c
index 1e0e10dd6ba5..3af263cc0caa 100644
--- a/drivers/usb/phy/phy-isp1301-omap.c
+++ b/drivers/usb/phy/phy-isp1301-omap.c
@@ -94,7 +94,7 @@ struct isp1301 {
 
 #if defined(CONFIG_MACH_OMAP_H2) || defined(CONFIG_MACH_OMAP_H3)
 
-#ifdefined(CONFIG_TPS65010) || defined(CONFIG_TPS65010_MODULE)
+#ifdefined(CONFIG_TPS65010) || (defined(CONFIG_TPS65010_MODULE)  
defined(MODULE))
 
 #include linux/i2c/tps65010.h
 

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


[PATCH] usb: gadget: remove incorrect __init/__exit annotations

2015-04-10 Thread Arnd Bergmann
A recent change introduced a link error for the composite
printer gadget driver:

`printer_unbind' referenced in section `.ref.data' of drivers/built-in.o: 
defined in discarded section `.exit.text' of drivers/built-in.o

Evidently the unbind function should not be marked __exit here,
because it is called through a callback pointer that is not necessarily
discarded, __composite_unbind() is indeed called from the error path of
composite_bind(), which can never work for a built-in driver.

Looking at the surrounding code, I found the same problem in all other
composite gadget drivers in both the bind and unbind functions, as
well as the udc platform driver 'remove' functions. Those will break
if anyone uses the 'unbind' sysfs attribute to detach a device from a
built-in driver.

This patch removes the incorrect annotations from all the gadget
drivers.

Signed-off-by: Arnd Bergmann a...@arndb.de
---

I find it hard to believe that a bug like this has gone unnoticed for
over 10 years (I did not check the pre-git history), but I also can't
see any flaw in my logic here.

I did not actually run into this bug in a real system and only found
it by inspection, so please let me know if I was just wrong here and
the code actually works ok.

 drivers/usb/gadget/legacy/acm_ms.c | 10 +-
 drivers/usb/gadget/legacy/audio.c  | 10 +-
 drivers/usb/gadget/legacy/cdc2.c   | 10 +-
 drivers/usb/gadget/legacy/dbgp.c   |  4 ++--
 drivers/usb/gadget/legacy/ether.c  | 12 ++--
 drivers/usb/gadget/legacy/g_ffs.c  |  2 +-
 drivers/usb/gadget/legacy/gmidi.c  | 10 +-
 drivers/usb/gadget/legacy/hid.c| 12 ++--
 drivers/usb/gadget/legacy/mass_storage.c   |  6 +++---
 drivers/usb/gadget/legacy/multi.c  | 10 +-
 drivers/usb/gadget/legacy/ncm.c| 10 +-
 drivers/usb/gadget/legacy/nokia.c  | 10 +-
 drivers/usb/gadget/legacy/printer.c|  8 
 drivers/usb/gadget/legacy/serial.c |  4 ++--
 drivers/usb/gadget/legacy/tcm_usb_gadget.c |  2 +-
 drivers/usb/gadget/legacy/webcam.c |  8 
 drivers/usb/gadget/legacy/zero.c   |  4 ++--
 drivers/usb/gadget/udc/at91_udc.c  |  4 ++--
 drivers/usb/gadget/udc/atmel_usba_udc.c|  4 ++--
 drivers/usb/gadget/udc/fsl_udc_core.c  |  4 ++--
 drivers/usb/gadget/udc/fusb300_udc.c   |  4 ++--
 drivers/usb/gadget/udc/m66592-udc.c|  4 ++--
 drivers/usb/gadget/udc/r8a66597-udc.c  |  4 ++--
 23 files changed, 78 insertions(+), 78 deletions(-)

diff --git a/drivers/usb/gadget/legacy/acm_ms.c 
b/drivers/usb/gadget/legacy/acm_ms.c
index c30b7b572465..1194b09ae746 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -121,7 +121,7 @@ static struct usb_function *f_msg;
 /*
  * We _always_ have both ACM and mass storage functions.
  */
-static int __init acm_ms_do_config(struct usb_configuration *c)
+static int acm_ms_do_config(struct usb_configuration *c)
 {
struct fsg_opts *opts;
int status;
@@ -174,7 +174,7 @@ static struct usb_configuration acm_ms_config_driver = {
 
 /*-*/
 
-static int __init acm_ms_bind(struct usb_composite_dev *cdev)
+static int acm_ms_bind(struct usb_composite_dev *cdev)
 {
struct usb_gadget   *gadget = cdev-gadget;
struct fsg_opts *opts;
@@ -249,7 +249,7 @@ fail_get_msg:
return status;
 }
 
-static int __exit acm_ms_unbind(struct usb_composite_dev *cdev)
+static int acm_ms_unbind(struct usb_composite_dev *cdev)
 {
usb_put_function(f_msg);
usb_put_function_instance(fi_msg);
@@ -258,13 +258,13 @@ static int __exit acm_ms_unbind(struct usb_composite_dev 
*cdev)
return 0;
 }
 
-static __refdata struct usb_composite_driver acm_ms_driver = {
+static struct usb_composite_driver acm_ms_driver = {
.name   = g_acm_ms,
.dev= device_desc,
.max_speed  = USB_SPEED_SUPER,
.strings= dev_strings,
.bind   = acm_ms_bind,
-   .unbind = __exit_p(acm_ms_unbind),
+   .unbind = acm_ms_unbind,
 };
 
 module_usb_composite_driver(acm_ms_driver);
diff --git a/drivers/usb/gadget/legacy/audio.c 
b/drivers/usb/gadget/legacy/audio.c
index f46a3956e43d..f289caf18a45 100644
--- a/drivers/usb/gadget/legacy/audio.c
+++ b/drivers/usb/gadget/legacy/audio.c
@@ -167,7 +167,7 @@ static const struct usb_descriptor_header *otg_desc[] = {
 
 /*-*/
 
-static int __init audio_do_config(struct usb_configuration *c)
+static int audio_do_config(struct usb_configuration *c)
 {
int status;
 
@@ -216,7 +216,7 @@ static struct usb_configuration audio_config_driver = {
 
 /*-*/
 

[PATCH 2/2] drivers/usb/core: devio.c: Removed various warnings and errors generated by checkpatch.pl

2015-04-10 Thread Chase Metzger
Removed warnings and erros from checkpatch.pl that go against the coding style.

Lines 29 and 103: removed unwanted spaces.
Lines 1040 and 1591: changed dev_printk(KERN_DEBUG, ...) to dev_dbg(dev-dev).
Lines 1942: changed an else switch to else { switch (...) {...} }. (Makes more 
sense(to me)).
Lines 1319 and 1906: added spaces to fit coding style consistently.

Signed-off-by: Chase Metzger chasemetzge...@gmail.com
---
 drivers/usb/core/devio.c | 69 
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 4b0448c..1a97df0 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -29,7 +29,7 @@
  *22.12.1999   0.1   Initial release (split from proc_usb.c)
  *04.01.2000   0.2   Turned into its own filesystem
  *30.09.2005   0.3   Fix user-triggerable oops in async URB delivery
- *  (CAN-2005-3055)
+ *  (CAN-2005-3055)
  */
 
 /*/
@@ -103,7 +103,7 @@ MODULE_PARM_DESC(usbfs_snoop, true to log all usbfs 
traffic);
 #define snoop(dev, format, arg...) \
do {\
if (usbfs_snoop)\
-   dev_info(dev , format , ## arg);\
+   dev_info(dev, format, ## arg);  \
} while (0)
 
 enum snoop_when {
@@ -1040,7 +1040,7 @@ static int proc_control(struct usb_dev_state *ps, void 
__user *arg)
snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, 
NULL, 0);
}
if (i  0  i != -EPIPE) {
-   dev_printk(KERN_DEBUG, dev-dev, usbfs: USBDEVFS_CONTROL 
+   dev_dbg(dev-dev, usbfs: USBDEVFS_CONTROL 
   failed cmd %s rqt %u rq %u len %u ret %d\n,
   current-comm, ctrl.bRequestType, ctrl.bRequest,
   ctrl.wLength, i);
@@ -1319,7 +1319,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
is_in = (uurb-endpoint  USB_ENDPOINT_DIR_MASK) != 0;
 
u = 0;
-   switch(uurb-type) {
+   switch (uurb-type) {
case USBDEVFS_URB_TYPE_CONTROL:
if (!usb_endpoint_xfer_control(ep-desc))
return -EINVAL;
@@ -1591,8 +1591,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
}
 
if (ret) {
-   dev_printk(KERN_DEBUG, ps-dev-dev,
-  usbfs: usb_submit_urb returned %d\n, ret);
+   dev_dbg(ps-dev-dev, usbfs: usb_submit_urb returned %d\n, 
ret);
snoop_urb(ps-dev, as-userurb, as-urb-pipe,
0, ret, COMPLETE, NULL, 0);
async_removepending(as);
@@ -1906,7 +1905,7 @@ static int proc_releaseinterface(struct usb_dev_state 
*ps, void __user *arg)
return -EFAULT;
if ((ret = releaseintf(ps, ifnum))  0)
return ret;
-   destroy_async_on_interface (ps, ifnum);
+   destroy_async_on_interface(ps, ifnum);
return 0;
 }
 
@@ -1942,36 +1941,38 @@ static int proc_ioctl(struct usb_dev_state *ps, struct 
usbdevfs_ioctl *ctl)
retval = -EHOSTUNREACH;
else if (!(intf = usb_ifnum_to_if(ps-dev, ctl-ifno)))
retval = -EINVAL;
-   else switch (ctl-ioctl_code) {
-
-   /* disconnect kernel driver from interface */
-   case USBDEVFS_DISCONNECT:
-   if (intf-dev.driver) {
-   driver = to_usb_driver(intf-dev.driver);
-   dev_dbg(intf-dev, disconnect by usbfs\n);
-   usb_driver_release_interface(driver, intf);
-   } else
-   retval = -ENODATA;
-   break;
+   else {
+   switch (ctl-ioctl_code) {
+
+   /* disconnect kernel driver from interface */
+   case USBDEVFS_DISCONNECT:
+   if (intf-dev.driver) {
+   driver = to_usb_driver(intf-dev.driver);
+   dev_dbg(intf-dev, disconnect by usbfs\n);
+   usb_driver_release_interface(driver, intf);
+   } else
+   retval = -ENODATA;
+   break;
 
-   /* let kernel drivers try to (re)bind to the interface */
-   case USBDEVFS_CONNECT:
-   if (!intf-dev.driver)
-   retval = device_attach(intf-dev);
-   else
-   retval = -EBUSY;
-   break;
+   /* let kernel drivers try to (re)bind to the interface */
+   case USBDEVFS_CONNECT:
+   if (!intf-dev.driver)
+