| Message ID | alpine.LNX.2.00.1105191801540.28291@pobox.suse.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 23746a66d7d9e73402c68ef00d708796b97ebd72 |
| Headers | show |
On 05/19/2011 12:03 PM, Jiri Kosina wrote: > On Thu, 19 May 2011, Chase Douglas wrote: > >>>> [ 195.491735] Bluetooth: HIDP (Human Interface Emulation) ver 1.2 >>>> [ 222.693947] input: cndougla’s trackpad as >>>> /devices/pci0000:00/0000:00:1a.0/usb1/1-1/1-1.3/1-1.3.3/1-1.3.3:1.0/bluetooth/hci0/hci0:12/input16 >>>> [ 222.694119] magicmouse 0005:05AC:030E.0005: input,hidraw3: BLUETOOTH >>>> HID v1.60 Mouse [cndougla’s trackpad] on 50:63:13:90:AF:A4 >>>> [ 222.694128] hidp_output_raw_report, report_type: 2 >>>> [ 222.808111] session ffff88012d9b1200 param 0x02 >>>> [ 222.808198] hidp: returnign -EIO because of !output_report_success >>>> [ 222.808209] magicmouse 0005:05AC:030E.0005: unable to request touch >>>> data (-5) >>>> [ 222.809358] session ffff88012d9b1200 param 0x02 >>>> [ 222.810606] session ffff88012d9b1200 param 0x02 >>>> [ 222.813113] session ffff88012d9b1200 param 0x00 >>>> [ 223.045363] magicmouse: probe of 0005:05AC:030E.0005 failed with error -5 >>> >>> Ok, so what is happening here is that magicmouse_probe() sends the { 0xd7, >>> 0x01 } feature report to the device, and it responds with >>> HIDP_HSHK_ERR_INVALID_REPORT_ID. >>> >>> That's why the _raw callback (correctly) returns error. >>> >>> So the question to the driver authors is -- what is the point behind the { >>> 0xd7, 0x01 } report? Clearly the device doesn't like it during probe >>> because of invalid report ID. >>> And it never did, we just silently ignored the error with the older >>> kernels. >> >> The feature report is what flips the devices (magic mouse, magic >> trackpad) into multitouch mode. Without the report, the mouse will not >> emit the location of any touches on its surface, and the trackpad will >> only emit single touch data. >> >> What do you think we should do? We can't get rid of the report. Should >> we silently ignore the error? > > Hmm, I am afraid that it's the only option (apart from thowing the whole > 'waiting for ack' infrastructure away, which would be sad, as it's a good > thing in principle). > > So how about the patch below? :/ Could you please verify that it makes > hid-magicmouse work again? Thanks. > > > > From: Jiri Kosina <jkosina@suse.cz> > Subject: [PATCH] HID: magicmouse: ignore 'ivalid report id' while switching modes > > The device reponds with 'invalid report id' when feature report switching it > into multitouch mode is sent to it. > > This has been silently ignored before 0825411ade ("HID: bt: Wait for ACK > on Sent Reports"), but since this commit, it propagates -EIO from the _raw > callback . > > So let the driver ignore -EIO as response to 0xd7,0x01 report, as that's > how the device reacts in normal mode. > > Sad, but following reality. > > This fixes https://bugzilla.kernel.org/show_bug.cgi?id=35022 > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > --- > drivers/hid/hid-magicmouse.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c > index 0ec91c1..a5eda4c 100644 > --- a/drivers/hid/hid-magicmouse.c > +++ b/drivers/hid/hid-magicmouse.c > @@ -501,9 +501,17 @@ static int magicmouse_probe(struct hid_device *hdev, > } > report->size = 6; > > + /* > + * The device reponds with 'invalid report id' when feature > + * report switching it into multitouch mode is sent to it. > + * > + * This results in -EIO from the _raw low-level transport callback, > + * but there seems to be no other way of switching the mode. > + * Thus the super-ugly hacky success check below. > + */ > ret = hdev->hid_output_raw_report(hdev, feature, sizeof(feature), > HID_FEATURE_REPORT); > - if (ret != sizeof(feature)) { > + if (ret != -EIO) { > hid_err(hdev, "unable to request touch data (%d)\n", ret); > goto err_stop_hw; > } Seems to work for me :). Reviewed-by: Chase Douglas <chase.douglas@canonical.com> Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 19 May 2011, Chase Douglas wrote:
> Seems to work for me :).
I have applied the patch with your Tested-by. Thanks,
On 05/20/2011 04:28 AM, Jiri Kosina wrote: > On Thu, 19 May 2011, Chase Douglas wrote: > >> Seems to work for me :). > > I have applied the patch with your Tested-by. Thanks, Thanks Jiri! Could you add 'Cc: stable@kernel.org' if it hasn't been merged yet? It looks like .39 was just released without this patch... -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 20 May 2011, Chase Douglas wrote: > >> Seems to work for me :). > > > > I have applied the patch with your Tested-by. Thanks, > > Thanks Jiri! > > Could you add 'Cc: stable@kernel.org' if it hasn't been merged yet? It > looks like .39 was just released without this patch... I will be sending this patch to stable once it's in Linus tree (I will be sending pull request likely today).
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 0ec91c1..a5eda4c 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -501,9 +501,17 @@ static int magicmouse_probe(struct hid_device *hdev, } report->size = 6; + /* + * The device reponds with 'invalid report id' when feature + * report switching it into multitouch mode is sent to it. + * + * This results in -EIO from the _raw low-level transport callback, + * but there seems to be no other way of switching the mode. + * Thus the super-ugly hacky success check below. + */ ret = hdev->hid_output_raw_report(hdev, feature, sizeof(feature), HID_FEATURE_REPORT); - if (ret != sizeof(feature)) { + if (ret != -EIO) { hid_err(hdev, "unable to request touch data (%d)\n", ret); goto err_stop_hw; }