Hi Greg, I'm glad you can take a look at my patches. :) On Wed, Aug 28, 2013 at 11:38:28AM +0800, Greg Kroah-Hartman wrote: > On Wed, Aug 28, 2013 at 11:13:12AM +0800, Huang Rui wrote: > > This patch add a routine to judge if the usb device is a mouse or not. > > The is_usb_mouse only has one input parameter of usb_device. The > > return value is bool type, and true means this device is mouse and > > vice-versa. > > No, sorry, this isn't ok for a number of reasons. > > You got "lucky" in that you can duplicate this with a mouse, the fact > that it is a mouse that caused your problems should never mean that the > os only triggers on a mouse device. I can create a "keyboard" device > that has the exact same USB characteristics that a mouse does, that > should also be able to trigger your problem. Or a vendor specific > device. > Yes, you're right. This issue is specific to the devices that use Pixart controller, and the Pixart controller is only used by mouse. That's why I filtered for usb mouse. Below answer is our HW guys' feedback. In cover letter, you would see more details. [Q] Why the special devices are only mice? Would high speed devices such as 3G modem or USB Bluetooth adapter trigger this issue? - Current this sensitivity is only confined to devices that use Pixart controllers. This controller is designed for use with LS mouse devices only. We have not observed any other devices failing. There may be a small risk for other devices also but this patch (reset device in resume phase) will cover the cases if required. > That's why the USB core doesn't care about the device type, they are all > just "pipes". So please deal with it on that level, never do something > only depending on the "type" of device plugged into the system. > I got it. Do you mean if I want do filter usb devices by usb mouse, I should do it at hid or usbhid level? > > It also add a remote wakeup quirk for some special AMD platforms. This > > quirk filters southbridge revision which is 0x39 or 0x3a. > > Whenever your changelog description has the phrase "it also...", that is > a big hint that you should be breaking this up into multiple patches. > You are doing two different things in this patch, which is another > reason why I can't take it. Thanks to reminder me, I will split it in two patches in v3. > > Please fix this up properly, and that means, not doing things depending > on the "type" of device, from a protocol standpoint, as that is always > wrong. > OK, again, shall I do it at hid or usbhid level? If not, I've no idea how I should do, could you teach me? Or filter devices by "Pixart"? > Hint, I bet you didn't fix this bug in Windows this way, right? :) > Actually, this issue is found in Windows firstly, and we set a registry to force the driver to reset the device on S3 resume to work around it. Then I reproduce it in Linux OS, and make a same solution. :) Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
