[PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Jiri Kosina]

On Mon, 20 Feb 2017, Linus Torvalds wrote:

> > I'll have a more specific commit (or range) soon.
> 
> Hmm. It's commit 279967a65b32 ("HID: rmi: Handle all Synaptics
> touchpads using hid-rmi").
> 
> And the reason seems to be stupid: I don't have RMI enabled at all,
> because that didn't use to work or make a difference.
> 
> Maybe that "let's use RMI" code should depend on RMI actually being
> enabled? Because as-is, that code now breaks existing configurations.

I agree; that's in line with what we usually try to stick to (force 
specific drivers if the device doesn't work with the generic at all and 
switch over to generic in compile-time for devices that have limited 
functionality with the generic driver).

Andrew, Benjamin, how about the patch below?




From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built

Commit 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
unconditionally switches over handling of all Synaptics touchpads to hid-rmi
(to make use of extended features of the HW); in case CONFIG_HID_RMI is
disabled though this renders the touchpad unusable, as the

	HID_DEVICE(HID_BUS_ANY, HID_GROUP_RMI, HID_ANY_ID, HID_ANY_ID)

match doesn't exist and generic/multitouch doesn't bind to it either (due
to hid group mismatch).

Fix this by switching over to hid-rmi only if it has been actually built.

Fixes: 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 drivers/hid/hid-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 538ff697a4cf..e9e87d337446 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -827,7 +827,8 @@ static int hid_scan_report(struct hid_device *hid)
 				 * hid-rmi should take care of them,
 				 * not hid-generic
 				 */
-				hid->group = HID_GROUP_RMI;
+				if (IS_ENABLED(CONFIG_HID_RMI))
+					hid->group = HID_GROUP_RMI;
 		break;
 	}
 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Linus Torvalds]

On Tue, Feb 21, 2017 at 6:17 AM, Jiri Kosina <jikos@kernel.org> wrote:
>
> I agree; that's in line with what we usually try to stick to (force
> specific drivers if the device doesn't work with the generic at all and
> switch over to generic in compile-time for devices that have limited
> functionality with the generic driver).

.. and maybe we should do this same thing for the WACOM case a couple
of lines up from your patch?

              Linus

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Jiri Kosina]

On Tue, 21 Feb 2017, Linus Torvalds wrote:

> > I agree; that's in line with what we usually try to stick to (force
> > specific drivers if the device doesn't work with the generic at all and
> > switch over to generic in compile-time for devices that have limited
> > functionality with the generic driver).
> 
> .. and maybe we should do this same thing for the WACOM case a couple
> of lines up from your patch?

Well, that's far more questionable. I am pretty sure Wacom devices are 
completely dysfunctional without a specific driver, as they have their own 
protocol.

Adding Ping and Jason to CC.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Linus Torvalds]

On Tue, Feb 21, 2017 at 7:46 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Tue, 21 Feb 2017, Linus Torvalds wrote:
>>
>> .. and maybe we should do this same thing for the WACOM case a couple
>> of lines up from your patch?
>
> Well, that's far more questionable. I am pretty sure Wacom devices are
> completely dysfunctional without a specific driver, as they have their own
> protocol.

Ah, ok. Never mind then.

            Linus

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Benjamin Tissoires]

On Feb 21 2017 or thereabouts, Linus Torvalds wrote:
> On Tue, Feb 21, 2017 at 7:46 AM, Jiri Kosina <jikos@kernel.org> wrote:
> > On Tue, 21 Feb 2017, Linus Torvalds wrote:
> >>
> >> .. and maybe we should do this same thing for the WACOM case a couple
> >> of lines up from your patch?
> >
> > Well, that's far more questionable. I am pretty sure Wacom devices are
> > completely dysfunctional without a specific driver, as they have their own
> > protocol.
> 
> Ah, ok. Never mind then.
> 
>             Linus

Well, Wacom devices use to need a special driver, but the latest
generation should somewhat be able to work without a driver (IIRC).
The only thing I can think of is that Wacom devices require
HID_QUIRK_NO_INIT_REPORTS, so they might not work out of the box after
all.

Let's see what Ping and Jason think about the question.

Cheers,
Benjamin

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Jiri Kosina]

On Tue, 21 Feb 2017, Benjamin Tissoires wrote:

> Well, Wacom devices use to need a special driver, but the latest 
> generation should somewhat be able to work without a driver (IIRC). 

I vaguely recall this wasn't the case at all for the older generations 
I've had my hands on, and it wasn't just the matter of 
HID_QUIRK_NO_INIT_REPORTS quirk.

But anyway ... that wasn't reported as a regression for quite some time, 
so let's defer to what Ping and Jason have to say regarding 
protocol/driver compatibility.

I'll be pushing the RMI fix to Linus soon.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Jason Gerecke]

One of the main reasons Wacom devices have historically required a
special driver is that the HID descriptors on their branded devices are
useless. A generic driver would only know the length of a report; no
information about the contents or layout is actually provided by the
hardware. The latest generation of Wacom tablets somewhat improves the
situation, but its still not good enough for the devices to be at all
useful with an e.g. hid-generic fallback. The hardware now provides a
full descriptor, but every usage is in a Vendor Defined usage page. We
can use Wacom's usage table definitions to implement "generic Wacom"
support for these new devices (which is exactly what we did in 4.10),
but no vendor-agnostic generic driver will understand the usages.

Wacom's embedded tablet PC sensors are a different story. These devices
*do* provide generic HID descriptors and it should be possible to have
hid-generic power them as a fallback. That said, it seems that Wacom is
moving away from their 0x56A vendor ID for these embedded sensors to a
new 0x2D1F vendor ID. This VID isn't recognized by any of the existing
Wacom kernel drivers and is (as I understand) intended be a way to allow
new hardware to work with the hid-generic driver.

Jason Gerecke, Linux Software Engineer
Wacom Technology Corporation
tel: 360-896-9833 ext. 229 (direct) / fax: 360-896-9724
http://www.wacom.com

On 02/21/2017 01:03 PM, Jiri Kosina wrote:
> On Tue, 21 Feb 2017, Benjamin Tissoires wrote: > >> Well, Wacom devices use to need a special driver, but the latest
>> generation should somewhat be able to work without a driver (IIRC).
>> > > I vaguely recall this wasn't the case at all for the older >
generations I've had my hands on, and it wasn't just the matter of >
HID_QUIRK_NO_INIT_REPORTS quirk. > > But anyway ... that wasn't reported
as a regression for quite some > time, so let's defer to what Ping and
Jason have to say regarding > protocol/driver compatibility. > > I'll be
pushing the RMI fix to Linus soon. > > Thanks, >

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Benjamin Tissoires]

On Feb 21 2017 or thereabouts, Jiri Kosina wrote:
> On Mon, 20 Feb 2017, Linus Torvalds wrote:
> 
> > > I'll have a more specific commit (or range) soon.
> > 
> > Hmm. It's commit 279967a65b32 ("HID: rmi: Handle all Synaptics
> > touchpads using hid-rmi").
> > 
> > And the reason seems to be stupid: I don't have RMI enabled at all,
> > because that didn't use to work or make a difference.
> > 
> > Maybe that "let's use RMI" code should depend on RMI actually being
> > enabled? Because as-is, that code now breaks existing configurations.
> 
> I agree; that's in line with what we usually try to stick to (force 
> specific drivers if the device doesn't work with the generic at all and 
> switch over to generic in compile-time for devices that have limited 
> functionality with the generic driver).
> 
> Andrew, Benjamin, how about the patch below?
> 

Hi,

Sorry, I am on PTO for the rest of the week with limited internet
access.

Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Sorry for the waste of time :(

Cheers,
Benjamin

> 
> 
> 
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built
> 
> Commit 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
> unconditionally switches over handling of all Synaptics touchpads to hid-rmi
> (to make use of extended features of the HW); in case CONFIG_HID_RMI is
> disabled though this renders the touchpad unusable, as the
> 
> 	HID_DEVICE(HID_BUS_ANY, HID_GROUP_RMI, HID_ANY_ID, HID_ANY_ID)
> 
> match doesn't exist and generic/multitouch doesn't bind to it either (due
> to hid group mismatch).
> 
> Fix this by switching over to hid-rmi only if it has been actually built.
> 
> Fixes: 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>  drivers/hid/hid-core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 538ff697a4cf..e9e87d337446 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -827,7 +827,8 @@ static int hid_scan_report(struct hid_device *hid)
>  				 * hid-rmi should take care of them,
>  				 * not hid-generic
>  				 */
> -				hid->group = HID_GROUP_RMI;
> +				if (IS_ENABLED(CONFIG_HID_RMI))
> +					hid->group = HID_GROUP_RMI;
>  		break;
>  	}
>  
> 
> -- 
> Jiri Kosina
> SUSE Labs
> 

Re: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built (was Re: [GIT PULL] HID for 4.11)

[Andrew Duggan]

On 02/21/2017 06:17 AM, Jiri Kosina wrote:
> On Mon, 20 Feb 2017, Linus Torvalds wrote:
>
>>> I'll have a more specific commit (or range) soon.
>> Hmm. It's commit 279967a65b32 ("HID: rmi: Handle all Synaptics
>> touchpads using hid-rmi").
>>
>> And the reason seems to be stupid: I don't have RMI enabled at all,
>> because that didn't use to work or make a difference.
>>
>> Maybe that "let's use RMI" code should depend on RMI actually being
>> enabled? Because as-is, that code now breaks existing configurations.
> I agree; that's in line with what we usually try to stick to (force
> specific drivers if the device doesn't work with the generic at all and
> switch over to generic in compile-time for devices that have limited
> functionality with the generic driver).
>
> Andrew, Benjamin, how about the patch below?
>

I tested this patch and hid-core now correctly binds hid-mulitouch to 
the touchpad if CONFIG_HID_RMI is disabled. Sorry, about the oversight.

Tested-by: Andrew Duggan <aduggan@synaptics.com>

Thanks,
Andrew

>
>
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: rmi: fallback to generic/multitouch if hid-rmi is not built
>
> Commit 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
> unconditionally switches over handling of all Synaptics touchpads to hid-rmi
> (to make use of extended features of the HW); in case CONFIG_HID_RMI is
> disabled though this renders the touchpad unusable, as the
>
> 	HID_DEVICE(HID_BUS_ANY, HID_GROUP_RMI, HID_ANY_ID, HID_ANY_ID)
>
> match doesn't exist and generic/multitouch doesn't bind to it either (due
> to hid group mismatch).
>
> Fix this by switching over to hid-rmi only if it has been actually built.
>
> Fixes: 279967a65b32 ("HID: rmi: Handle all Synaptics touchpads using hid-rmi")
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>   drivers/hid/hid-core.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 538ff697a4cf..e9e87d337446 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -827,7 +827,8 @@ static int hid_scan_report(struct hid_device *hid)
>   				 * hid-rmi should take care of them,
>   				 * not hid-generic
>   				 */
> -				hid->group = HID_GROUP_RMI;
> +				if (IS_ENABLED(CONFIG_HID_RMI))
> +					hid->group = HID_GROUP_RMI;
>   		break;
>   	}
>   
>