linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Simon Wörner" <linux@simon-woerner.de>
To: Florian Echtler <floe@butterbrot.org>,
	Benjamin Tissoires <benjamin.tissoires@gmail.com>,
	Andrew Duggan <aduggan@synaptics.com>
Cc: "Jiri Kosina" <jkosina@suse.cz>,
	"Simon Wörner" <linux@simon-woerner.de>,
	linux-input <linux-input@vger.kernel.org>,
	"Andrew Duggan" <andrew.duggan@gmail.com>
Subject: Re: [PATCH] HID: Add driver for acer keybard with broken rdesc
Date: Tue, 24 Mar 2015 17:03:26 +0100	[thread overview]
Message-ID: <55118ACE.9090309@simon-woerner.de> (raw)
In-Reply-To: <54F07F0F.9040408@butterbrot.org>

On 27.02.2015 15:28, Florian Echtler wrote:
> Just as a quick side note, Simon's "hack" compiled as a standalone
> module fixes the issue for me on stock kernel 3.16.0 - keyboard works
> perfectly now. So device 06CB:2991 has exactly the same broken
> descriptor and should probably included in any future solution.
Okay, i will include it.
> Many thanks to everyone involved!
>
> Best, Florian
>
> On 27.02.2015 15:16, Benjamin Tissoires wrote:
>> [adding Florian to the thread, he is also affected by this]
>>
>> On Tue, Feb 17, 2015 at 9:01 PM, Andrew Duggan<aduggan@synaptics.com>  wrote:
>>> On 02/17/2015 04:30 AM, Jiri Kosina wrote:
>>>> On Thu, 29 Jan 2015,linux@simon-woerner.de  wrote:
>>>>
>>>>> From: Simon Wörner<mail@simon-woerner.de>
>>>>>
>>>>> HID: Add driver for acer keybard with broken rdesc
>>>> Hi Simon,
>>>>
>>>> to make sure proper device <-> driver binding is performed, you also have
>>>> to add device ID to the hid_have_special_driver[] array.
>>>>
>> To the cost of an error in the kernel log, the proper binding will be
>> done even if if there is no entry in hid_have_special_driver :)
>>
>> See Florian's log:
>> [    3.153892] hid-generic 0003:06CB:2991.0003: usage index exceeded
>> [    3.153896] hid-generic 0003:06CB:2991.0003: item 0 2 2 2 parsing failed
>> [    3.153921] hid-generic: probe of 0003:06CB:2991.0003 failed with error -22
>>
>> So hid-generic will fail to bind the device, so normally hid-acer
>> should bind it, fix the report descriptor and the keyboard should be
>> working.
>>
>> Not very clean, but it should work :)
The other solutions looks like more work and at least some other problems.
So this "best" and preferred solution, or did i got any thing wrong?
>>> I have been meaning to respond to this patch. Unfortunately, simply adding
>>> this driver to the hid_have_special_driver[] list will prevent
>>> hid-multitouch from binding to the touchpad which shares the same vid and
>>> pid. My suggestion would be to create a new scan_flag for GD_KEYBOARD and to
>>> add to the code in hid_scan_collection() which scans the GENDESK usages.
>>> Similar to the code which is searching for HID_GD_POINTER. Then in
>>> hid_scan_report() we could assign a group for this driver based on the pid
>>> and the GD_KEYBOARD scan flag in the vendor handling code.
>> This could work, but that means that the list of special quirks after
>> the parsing is going to explode :(
>>
>> Plus we have to be sure that the scan_report doesn't fail or we will
>> not be able to tag the device correctly.
>>
>>> I can help out with implementing this part of the patch if there aren't any
>>> other suggestions. Adding a new group and more code to the hid_scan_report()
>>> vendor handling code is definately not ideal. But, I am not sure of a better
>>> way of binding different sub drivers to devices with the same vid and pid.
>>>
>>> An alternative would be to have hid-rmi handle all Synaptics touchpads, even
>>> the ones which currently use hid-multitouch. Then the keyboard report
>>> descriptor fixup could just be handled in hid-rmi. Accessing the finger data
>>> via rmi mode would also have the benefit of being able to report events
>>> which are currently not supported by the PTP collection (ie ABS_MT_PRESSURE,
>>> ABS_MT_TOUCH_MAJOR/MINOR). The touchpads which currently use hid-multitouch
>>> report finger data a little differently then the ones currently using
>>> hid-rmi so there is some work which would need to be done to add support for
>>> them in hid-rmi.
>>>
>> I am not sure we want to do that. Because we don't want hid-rmi to fix
>> all crappy keyboards around when the OEM reuses the synaptics PID.
>>
>> Maybe a better way of handling such situation is to provide a generic
>> mechanism to overwrite/patch the report descriptor so that we could
>> get rid of the drivers which just fix the report descriptor.
>> I have on a branch a version where I look into /lib/firmware/hid if
>> there is a matching device and a matching report descriptor. Then, I
>> read this firmware dynamically and patch the report descriptor.
>> This however requires that the hid modules are not included directly
>> in the kernel, or the /lib/firmware dir is not available and the
>> device doesn't bind.
>>
>> Cheers,
>> Benjamin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message tomajordomo@vger.kernel.org
>> More majordomo info athttp://vger.kernel.org/majordomo-info.html
>>

Is there any point i missed or we're good to go now? (expect the new 
device id)

Greets,
Simon
--
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

      reply	other threads:[~2015-03-24 16:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 19:03 HID: Add driver for synaptics keybard with broken rdesc Simon Wörner
2015-01-23 19:03 ` [PATCH] " Simon Wörner
2015-01-27 10:17   ` Jiri Kosina
2015-01-27 14:16   ` Benjamin Tissoires
2015-01-27 15:10     ` Simon Wörner
2015-01-27 16:13       ` Benjamin Tissoires
2015-01-27 23:46         ` Andrew Duggan
2015-01-28  4:04           ` Simon Wörner
2015-01-29  0:43             ` [PATCH] HID: Add driver for acer " linux
2015-02-17 12:30               ` Jiri Kosina
2015-02-18  2:01                 ` Andrew Duggan
2015-02-27 14:16                   ` Benjamin Tissoires
2015-02-27 14:28                     ` Florian Echtler
2015-03-24 16:03                       ` Simon Wörner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55118ACE.9090309@simon-woerner.de \
    --to=linux@simon-woerner.de \
    --cc=aduggan@synaptics.com \
    --cc=andrew.duggan@gmail.com \
    --cc=benjamin.tissoires@gmail.com \
    --cc=floe@butterbrot.org \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).