With the latest HEAD, I've noticed that I needed to add: HIDRAW=y
in order for my apple magicmouse to connect.
After receiving some posts, it seems this it not
the case(HIDRAW just relays HID events to userspace).
The bisect results of this issue resulted in commit:
HID:magicmouse: fix oops after device removal.
(hash:28918c211d86b6eeb70182c523800c7bc442960c)
After examining this commit, I've noticed that
HID_CONNECT_DEFAULT has also an entry in there
of HID_CONNECT_HIDINPUT, not sure what "~" means,
before HID_CONNECT_HIDINPUT, but after removing
this(correct me if I'm wrong)"double" definition,
I'm able to have my magicmouse connect without the
need of HIDRAW, and also this in dmesg:
magicmouse 0005:05AC:030D.0004: claimed by neither input, hiddev nor hidraw
magicmouse 0005:05AC:030D.0004: magicmouse hw start failed
and as well as the crash that the above commit fixes when waking
up from suspend(mouse connects perfectly upon wakeup).
Please have a look, and if this works, take it
if theres another solution let me know.
(so I can enjoy the power of the magicmouse!!).
Signed-off-by: Justin P. Mattock <[email protected]>
---
drivers/hid/hid-magicmouse.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 0d471fc..0de5e96 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
goto err_free;
}
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
dev_err(&hdev->dev, "magicmouse hw start failed\n");
goto err_free;
--
1.6.5.GIT
On 05/12/2010 07:03 AM, Jiri Kosina wrote:
> On Wed, 12 May 2010, Justin P. Mattock wrote:
>
>
>>>>>> --- a/drivers/hid/hid-magicmouse.c
>>>>>> +++ b/drivers/hid/hid-magicmouse.c
>>>>>> @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device
>>>>>> *hdev,
>>>>>> goto err_free;
>>>>>> }
>>>>>>
>>>>>> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT&
>>>>>> ~HID_CONNECT_HIDINPUT);
>>>>>> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>>>>>
>>>>>>
>>>>>>
>>>>> This is not particularly right, as we'll end up having dangling
>>>>> input device.
>>>>>
>>>>> The problem is, that when HIDRAW is not set, hid_hw_start()
>>>>> returns ENODEV as no subsystem has claimed the device, and probe
>>>>> routine bails out. Which is not what we want.
>>>>>
>>>>> Does the testing patch below fix the problems you are seeing?
>>>>>
>>>>>
>>>> works good.. rebooted a few times mouse connects. suspended a few times
>>>> mouse reconnects.
>>>>
>>>>
>>> I'd be glad if you could also double-check that device removal and
>>> re-connecting it works well as well with this patch.
>>>
>>>
>> with test I did different techniques,
>> 1) regular suspend(leave device on)
>> 2)suspend then shut off device
>> 3)shut off device then suspend
>> all of these techniques work properly
>>
> Thanks for reporting and testing, I have queued the patch.
>
>
cool...
Justin P. Mattock
On Wed, 12 May 2010, Justin P. Mattock wrote:
> > > > > --- a/drivers/hid/hid-magicmouse.c
> > > > > +++ b/drivers/hid/hid-magicmouse.c
> > > > > @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device
> > > > > *hdev,
> > > > > goto err_free;
> > > > > }
> > > > >
> > > > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT&
> > > > > ~HID_CONNECT_HIDINPUT);
> > > > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > > > >
> > > > >
> > > > This is not particularly right, as we'll end up having dangling
> > > > input device.
> > > >
> > > > The problem is, that when HIDRAW is not set, hid_hw_start()
> > > > returns ENODEV as no subsystem has claimed the device, and probe
> > > > routine bails out. Which is not what we want.
> > > >
> > > > Does the testing patch below fix the problems you are seeing?
> > > >
> > > works good.. rebooted a few times mouse connects. suspended a few times
> > > mouse reconnects.
> > >
> > I'd be glad if you could also double-check that device removal and
> > re-connecting it works well as well with this patch.
> >
> with test I did different techniques,
> 1) regular suspend(leave device on)
> 2)suspend then shut off device
> 3)shut off device then suspend
> all of these techniques work properly
Thanks for reporting and testing, I have queued the patch.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On 05/12/2010 06:41 AM, Jiri Kosina wrote:
> On Wed, 12 May 2010, Justin P. Mattock wrote:
>
>
>>>> --- a/drivers/hid/hid-magicmouse.c
>>>> +++ b/drivers/hid/hid-magicmouse.c
>>>> @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
>>>> goto err_free;
>>>> }
>>>>
>>>> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
>>>> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>>>
>>>>
>>> This is not particularly right, as we'll end up having dangling input
>>> device.
>>>
>>> The problem is, that when HIDRAW is not set, hid_hw_start() returns ENODEV
>>> as no subsystem has claimed the device, and probe routine bails out. Which
>>> is not what we want.
>>>
>>> Does the testing patch below fix the problems you are seeing?
>>>
>>>
>>>
>>>
>> works good.. rebooted a few times mouse connects. suspended a few times
>> mouse reconnects.
>>
> I'd be glad if you could also double-check that device removal and
> re-connecting it works well as well with this patch.
>
>
with test I did different techniques,
1) regular suspend(leave device on)
2)suspend then shut off device
3)shut off device then suspend
all of these techniques work properly
(let me know if this is what you meant).
>>> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
>>> index 0d471fc..f10d56a 100644
>>> --- a/drivers/hid/hid-magicmouse.c
>>> +++ b/drivers/hid/hid-magicmouse.c
>>> @@ -354,12 +354,15 @@ static int magicmouse_probe(struct hid_device *hdev,
>>> goto err_free;
>>> }
>>>
>>> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
>>> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>> if (ret) {
>>> dev_err(&hdev->dev, "magicmouse hw start failed\n");
>>> goto err_free;
>>> }
>>>
>>> + /* we are handling the input ourselves */
>>> + hidinput_disconnect(hdev);
>>> +
>>> report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
>>> if (!report) {
>>> dev_err(&hdev->dev, "unable to register touch report\n");
>>>
>>>
>>>
>> looks good over here.. If you'd like I can re-du this patch, add your
>> sign off etc.. and re-send, or not worry.. either way this little
>> quirk/problem is fixed.
>>
> No problem, once you confirm that device removal wasn't broken again and
> if I don't hear any objections from Michael, I will queue the patch
> myself.
>
> Thanks,
>
>
alright buddy,
looks good over here.
cheers.
Justin P. Mattock
Jiri Kosina writes:
> On Wed, 12 May 2010, Justin P. Mattock wrote:
>
>> > > --- a/drivers/hid/hid-magicmouse.c
>> > > +++ b/drivers/hid/hid-magicmouse.c
>> > > @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
>> > > goto err_free;
>> > > }
>> > >
>> > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
>> > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> > >
>> > This is not particularly right, as we'll end up having dangling input
>> > device.
>> >
>> > The problem is, that when HIDRAW is not set, hid_hw_start() returns ENODEV
>> > as no subsystem has claimed the device, and probe routine bails out. Which
>> > is not what we want.
>> >
>> > Does the testing patch below fix the problems you are seeing?
>> >
>> >
>> >
>> works good.. rebooted a few times mouse connects. suspended a few times
>> mouse reconnects.
>
> I'd be glad if you could also double-check that device removal and
> re-connecting it works well as well with this patch.
>
>> > diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
>> > index 0d471fc..f10d56a 100644
>> > --- a/drivers/hid/hid-magicmouse.c
>> > +++ b/drivers/hid/hid-magicmouse.c
>> > @@ -354,12 +354,15 @@ static int magicmouse_probe(struct hid_device *hdev,
>> > goto err_free;
>> > }
>> >
>> > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
>> > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> > if (ret) {
>> > dev_err(&hdev->dev, "magicmouse hw start failed\n");
>> > goto err_free;
>> > }
>> >
>> > + /* we are handling the input ourselves */
>> > + hidinput_disconnect(hdev);
>> > +
>> > report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
>> > if (!report) {
>> > dev_err(&hdev->dev, "unable to register touch report\n");
>> >
>> >
>>
>> looks good over here.. If you'd like I can re-du this patch, add your
>> sign off etc.. and re-send, or not worry.. either way this little
>> quirk/problem is fixed.
>
> No problem, once you confirm that device removal wasn't broken again and
> if I don't hear any objections from Michael, I will queue the patch
> myself.
It looks good to me. Thanks for doing this -- work has been busy this
week, so I haven't had time to dig into the issue yet.
Michael
On Wed, 12 May 2010, Justin P. Mattock wrote:
> > > --- a/drivers/hid/hid-magicmouse.c
> > > +++ b/drivers/hid/hid-magicmouse.c
> > > @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
> > > goto err_free;
> > > }
> > >
> > > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
> > > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > >
> > This is not particularly right, as we'll end up having dangling input
> > device.
> >
> > The problem is, that when HIDRAW is not set, hid_hw_start() returns ENODEV
> > as no subsystem has claimed the device, and probe routine bails out. Which
> > is not what we want.
> >
> > Does the testing patch below fix the problems you are seeing?
> >
> >
> >
> works good.. rebooted a few times mouse connects. suspended a few times
> mouse reconnects.
I'd be glad if you could also double-check that device removal and
re-connecting it works well as well with this patch.
> > diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> > index 0d471fc..f10d56a 100644
> > --- a/drivers/hid/hid-magicmouse.c
> > +++ b/drivers/hid/hid-magicmouse.c
> > @@ -354,12 +354,15 @@ static int magicmouse_probe(struct hid_device *hdev,
> > goto err_free;
> > }
> >
> > - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
> > + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > if (ret) {
> > dev_err(&hdev->dev, "magicmouse hw start failed\n");
> > goto err_free;
> > }
> >
> > + /* we are handling the input ourselves */
> > + hidinput_disconnect(hdev);
> > +
> > report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
> > if (!report) {
> > dev_err(&hdev->dev, "unable to register touch report\n");
> >
> >
>
> looks good over here.. If you'd like I can re-du this patch, add your
> sign off etc.. and re-send, or not worry.. either way this little
> quirk/problem is fixed.
No problem, once you confirm that device removal wasn't broken again and
if I don't hear any objections from Michael, I will queue the patch
myself.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On 05/12/2010 05:58 AM, Jiri Kosina wrote:
> On Tue, 11 May 2010, Justin P. Mattock wrote:
>
>
>> With the latest HEAD, I've noticed that I needed to add: HIDRAW=y in
>> order for my apple magicmouse to connect. After receiving some posts, it
>> seems this it not the case(HIDRAW just relays HID events to userspace).
>>
>> The bisect results of this issue resulted in commit: HID:magicmouse: fix
>> oops after device removal.
>> (hash:28918c211d86b6eeb70182c523800c7bc442960c)
>>
>> After examining this commit, I've noticed that HID_CONNECT_DEFAULT has
>> also an entry in there of HID_CONNECT_HIDINPUT, not sure what "~" means,
>>
> ~ is zeroing out the HID_CONENCT_HIDINPUT bits from the connect mask, as
> magicmouse driver is handling the input itself.
>
>
ah.. cool thanks for that..(I still have lots to learn(so many
expressions)).
>> before HID_CONNECT_HIDINPUT, but after removing
>> this(correct me if I'm wrong)"double" definition,
>> I'm able to have my magicmouse connect without the
>> need of HIDRAW, and also this in dmesg:
>>
>> magicmouse 0005:05AC:030D.0004: claimed by neither input, hiddev nor hidraw
>> magicmouse 0005:05AC:030D.0004: magicmouse hw start failed
>>
>> and as well as the crash that the above commit fixes when waking
>> up from suspend(mouse connects perfectly upon wakeup).
>>
>> Please have a look, and if this works, take it
>> if theres another solution let me know.
>> (so I can enjoy the power of the magicmouse!!).
>>
>>
>> Signed-off-by: Justin P. Mattock<[email protected]>
>>
>> ---
>> drivers/hid/hid-magicmouse.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
>> index 0d471fc..0de5e96 100644
>> --- a/drivers/hid/hid-magicmouse.c
>> +++ b/drivers/hid/hid-magicmouse.c
>> @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
>> goto err_free;
>> }
>>
>> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
>> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>
> This is not particularly right, as we'll end up having dangling input
> device.
>
> The problem is, that when HIDRAW is not set, hid_hw_start() returns ENODEV
> as no subsystem has claimed the device, and probe routine bails out. Which
> is not what we want.
>
> Does the testing patch below fix the problems you are seeing?
>
>
>
works good.. rebooted a few times
mouse connects. suspended a few times mouse reconnects.
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 0d471fc..f10d56a 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -354,12 +354,15 @@ static int magicmouse_probe(struct hid_device *hdev,
> goto err_free;
> }
>
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT& ~HID_CONNECT_HIDINPUT);
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret) {
> dev_err(&hdev->dev, "magicmouse hw start failed\n");
> goto err_free;
> }
>
> + /* we are handling the input ourselves */
> + hidinput_disconnect(hdev);
> +
> report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
> if (!report) {
> dev_err(&hdev->dev, "unable to register touch report\n");
>
>
looks good over here.. If you'd like I can re-du
this patch, add your sign off etc.. and re-send,
or not worry.. either way this little quirk/problem
is fixed.
Justin P. Mattock
On Tue, 11 May 2010, Justin P. Mattock wrote:
> With the latest HEAD, I've noticed that I needed to add: HIDRAW=y in
> order for my apple magicmouse to connect. After receiving some posts, it
> seems this it not the case(HIDRAW just relays HID events to userspace).
>
> The bisect results of this issue resulted in commit: HID:magicmouse: fix
> oops after device removal.
> (hash:28918c211d86b6eeb70182c523800c7bc442960c)
>
> After examining this commit, I've noticed that HID_CONNECT_DEFAULT has
> also an entry in there of HID_CONNECT_HIDINPUT, not sure what "~" means,
~ is zeroing out the HID_CONENCT_HIDINPUT bits from the connect mask, as
magicmouse driver is handling the input itself.
> before HID_CONNECT_HIDINPUT, but after removing
> this(correct me if I'm wrong)"double" definition,
> I'm able to have my magicmouse connect without the
> need of HIDRAW, and also this in dmesg:
>
> magicmouse 0005:05AC:030D.0004: claimed by neither input, hiddev nor hidraw
> magicmouse 0005:05AC:030D.0004: magicmouse hw start failed
>
> and as well as the crash that the above commit fixes when waking
> up from suspend(mouse connects perfectly upon wakeup).
>
> Please have a look, and if this works, take it
> if theres another solution let me know.
> (so I can enjoy the power of the magicmouse!!).
>
>
> Signed-off-by: Justin P. Mattock <[email protected]>
>
> ---
> drivers/hid/hid-magicmouse.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 0d471fc..0de5e96 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -354,7 +354,7 @@ static int magicmouse_probe(struct hid_device *hdev,
> goto err_free;
> }
>
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
This is not particularly right, as we'll end up having dangling input
device.
The problem is, that when HIDRAW is not set, hid_hw_start() returns ENODEV
as no subsystem has claimed the device, and probe routine bails out. Which
is not what we want.
Does the testing patch below fix the problems you are seeing?
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 0d471fc..f10d56a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -354,12 +354,15 @@ static int magicmouse_probe(struct hid_device *hdev,
goto err_free;
}
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDINPUT);
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
dev_err(&hdev->dev, "magicmouse hw start failed\n");
goto err_free;
}
+ /* we are handling the input ourselves */
+ hidinput_disconnect(hdev);
+
report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
if (!report) {
dev_err(&hdev->dev, "unable to register touch report\n");
--
Jiri Kosina
SUSE Labs, Novell Inc.