Hi David,
On Tue, Sep 20, 2011 at 3:57 PM, Jaikumar Ganesh <jaikumarg@xxxxxxxxxxx> wrote:
> Hi David:
>
> On Tue, Sep 20, 2011 at 3:43 PM, David Herrmann
> <dh.herrmann@xxxxxxxxxxxxxx> wrote:
>> Hi Jaikumar
>>
>> On Sat, Sep 17, 2011 at 12:12 AM, Jaikumar Ganesh <jaikumarg@xxxxxxxxxxx> wrote:
>>> magicmouse_select_input was being called after device
>>
>> This should be "*setup_input" not "*select_input".
>>
>>> registration. Hence, any thread scanning "/dev" for new
>>> devices will see the input bits change on an open file
>>> descriptor. Fix this by calling select_input inside
>>> input_mapping.
>>
>> Isn't input_mapped more appropriate here? Otherwise
>> hidinput_configure_usage could probably overwrite your abs values.
>>
>>> Based on discussions with Michael Poole <mdpoole@xxxxxxxxxxx>
>>>
>>> Change-Id: I289a37a850977c2cbbc9eb76fbf6c9a28d1833c5
>>> Signed-off-by: Jaikumar Ganesh <jaikumarg@xxxxxxxxxxx>
>>> ---
>>> drivers/hid/hid-magicmouse.c | 13 +++++++------
>>> 1 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
>>> index 56d0539..1175452 100644
>>> --- a/drivers/hid/hid-magicmouse.c
>>> +++ b/drivers/hid/hid-magicmouse.c
>>> @@ -98,6 +98,7 @@ struct magicmouse_sc {
>>> int ntouches;
>>> int scroll_accel;
>>> unsigned long scroll_jiffies;
>>> + bool setup_input;
>>>
>>> struct {
>>> short x;
>>> @@ -437,6 +438,11 @@ static int magicmouse_input_mapping(struct hid_device *hdev,
>>> if (!msc->input)
>>> msc->input = hi->input;
>>>
>>> + if (!msc->setup_input) {
>>> + magicmouse_setup_input(msc->input, hdev);
>>> + msc->setup_input = true;
>>> + }
>>> +
>>> /* Magic Trackpad does not give relative data after switching to MT */
>>> if (hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD &&
>>> field->flags & HID_MAIN_ITEM_RELATIVE)
>>> @@ -465,6 +471,7 @@ static int magicmouse_probe(struct hid_device *hdev,
>>> hid_set_drvdata(hdev, msc);
>>>
>>> msc->single_touch_id = NO_TOUCHES;
>>> + msc->setup_input = false;
>>>
>>> ret = hid_parse(hdev);
>>> if (ret) {
>>> @@ -478,12 +485,6 @@ static int magicmouse_probe(struct hid_device *hdev,
>>> goto err_free;
>>> }
>>>
>>> - /* We do this after hid-input is done parsing reports so that
>>> - * hid-input uses the most natural button and axis IDs.
>>> - */
>>> - if (msc->input)
>>> - magicmouse_setup_input(msc->input, hdev);
>>> -
>>> if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE)
>>> report = hid_register_report(hdev, HID_INPUT_REPORT,
>>> MOUSE_REPORT_ID);
>>> --
>>> 1.7.3.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>> I think this is the same problem as:
>> http://thread.gmane.org/gmane.linux.kernel.input/21660/focus=21669
>>
>> Several drivers need to set additional flags on the input device. I'd
>> recommend an "input_register" callback that is called after
>> hidinput_configure_usage() is called but before the input device is
>> registered. But it is only called once and not for every hid-descr
>> field. Then we could skip the "msc->setup_input" boolean flag.
>> However, until then this fix seems fine, although I'd use input_mapped
>> instead of input_mapping.
>
> This was one of my suggestions too in this thread"
> "http://thread.gmane.org/gmane.linux.kernel.input/21604/";
>
> This needs to be discussed on a more general basis since it will
> affect many drivers.
>
It will be good to have this callback added now. I have sent another
patch to the mailing list,
Have a look.
Thanks
Jaikumar
>>
>> Regards
>> David
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
