Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Plus tablet.
From: Daniel Smith @ 2019-05-23 19:09 UTC (permalink / raw)
  Cc: Daniel Smith, Hans de Goede, Darren Hart, Andy Shevchenko,
	linux-input, platform-driver-x86, linux-kernel

Added touch screen info for CHUWI Hi10 Plus tablet.

Signed-off-by: Daniel Smith <danct12@disroot.org>
---
 drivers/platform/x86/touchscreen_dmi.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index bd0856d2e825..1dbb53c3f1e7 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -91,6 +91,22 @@ static const struct ts_dmi_data chuwi_hi10_air_data = {
 	.properties	= chuwi_hi10_air_props,
 };
 
+static const struct property_entry chuwi_hi10_plus_props[] = {
+	PROPERTY_ENTRY_U32("touchscreen-min-x", 0),
+	PROPERTY_ENTRY_U32("touchscreen-min-y", 5),
+	PROPERTY_ENTRY_U32("touchscreen-size-x", 1914),
+	PROPERTY_ENTRY_U32("touchscreen-size-y", 1283),
+	PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10plus.fw"),
+	PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+	PROPERTY_ENTRY_BOOL("silead,home-button"),
+	{ }
+};
+
+static const struct ts_dmi_data chuwi_hi10_plus_data = {
+	.acpi_name      = "MSSL0017:00",
+	.properties     = chuwi_hi10_plus_props,
+};
+
 static const struct property_entry chuwi_vi8_props[] = {
 	PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
 	PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -605,6 +621,15 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
 		},
 	},
+	{
+		/* Chuwi Hi10 Plus (CWI527) */
+		.driver_data = (void *)&chuwi_hi10_plus_data,
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Hi10 plus tablet"),
+			DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
+		},
+	},
 	{
 		/* Chuwi Vi8 (CWI506) */
 		.driver_data = (void *)&chuwi_vi8_data,
-- 
2.21.0

^ permalink raw reply related

* [PATCH] input: silead: Add MSSL0017 to acpi_device_id.
From: Daniel Smith @ 2019-05-23 19:06 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Daniel Smith, Dmitry Torokhov, linux-input, platform-driver-x86,
	linux-kernel

On Chuwi Hi10 Plus, the Silead device id is MSSL0017.

Signed-off-by: Daniel Smith <danct12@disroot.org>
---
 drivers/input/touchscreen/silead.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 09241d4cdebc..06f0eb04a8fd 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -617,6 +617,7 @@ static const struct acpi_device_id silead_ts_acpi_match[] = {
 	{ "MSSL1680", 0 },
 	{ "MSSL0001", 0 },
 	{ "MSSL0002", 0 },
+	{ "MSSL0017", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match);
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH v2 00/10] Fix Elan I2C touchpads in latest generation from Lenovo
From: Benjamin Tissoires @ 2019-05-23 13:25 UTC (permalink / raw)
  To: Dmitry Torokhov, KT Liao, Rob Herring, Aaron Ma, Hans de Goede
  Cc: open list:HID CORE LAYER, lkml, devicetree
In-Reply-To: <20190521132712.2818-1-benjamin.tissoires@redhat.com>

Hi,

On Tue, May 21, 2019 at 3:27 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi,
>
> This is the v2 from https://lkml.org/lkml/2018/10/12/633
>
> So I initially thought it would be easy to integrate the suggested changes
> in the v1, but it turns our that the changes to have the touchscreen-width
> and height parameters were quite hard to do.
>
> I finally postponed the issue by blacklisting the 2 laptops we knew were
> not working and tried to devote more time to understand both drivers more.
>
> But it s the time where Lenovo is preparing the new models, and guess what,
> they suffer from the same symptoms.

I just received the confirmation from Lenovo that this series makes
the new laptops working.

Cheers,
Benjamin

>
> So I took a few time to work on this and finally got my head around the
> width and height problem. Once I got it, it was simple clear, but this also
> means we can not really rely on a device tree property for that.
>
> So in the elan* drivers, the "traces" are simply how many antennas there
> are on each axis. Which means that if a trace of 4 is reported in the
> events, it means it is simply seen by 4 antennas. So the computation of the
> width/height is the following: we take how many antennas there are, we
> subtract one to have the number of holes between the antennas, and we
> divide the number of unit we have in the axis by the value we just
> computed.
> This gives a rough 4mm on the P52, in both directions.
>
> And once you get that, you can just realize that the unit of the width and
> height are just the same than the X and Y coordinates, so we can apply the
> same resolution.
>
> So, in the end, that means that elan_i2c needs the information, or it will
> not be able to convert the number of crossed antennas into a size, but this
> is something specific to this touchpad.
>
> So here come, 7 months later the v2 on the subject.
>
> Cheers,
> Benjamin
>
> Benjamin Tissoires (10):
>   Input: elantech - query the min/max information beforehand too
>   Input: elantech - add helper function elantech_is_buttonpad()
>   Input: elantech - detect middle button based on firmware version
>   dt-bindings: add more optional properties for elan_i2c touchpads
>   Input: elan_i2c - do not query the info if they are provided
>   Input: elantech/SMBus - export all capabilities from the PS/2 node
>   Input: elan_i2c - handle physical middle button
>   Input: elan_i2c - export true width/height
>   Input: elan_i2c - correct the width/size base value
>   Input: elantech: remove P52 from SMBus blacklist
>
>  .../devicetree/bindings/input/elan_i2c.txt    |  11 +
>  drivers/input/mouse/elan_i2c_core.c           |  85 +++--
>  drivers/input/mouse/elantech.c                | 318 ++++++++++--------
>  drivers/input/mouse/elantech.h                |   8 +
>  4 files changed, 251 insertions(+), 171 deletions(-)
>
> --
> 2.21.0
>

^ permalink raw reply

* Re: [PATCH] input: silead: Add MSSL0017 to acpi_device_id.
From: Dmitry Torokhov @ 2019-05-23  7:20 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Danct12, linux-input, platform-driver-x86, linux-kernel
In-Reply-To: <6c18472f-bedd-6e6d-121c-8a311495c3c3@redhat.com>

On Wed, May 22, 2019 at 12:12:47PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-05-19 06:54, Danct12 wrote:
> > On Chuwi Hi10 Plus, the Silead device id is MSSL0017.
> > 
> > Signed-off-by: Danct12 <danct12@disroot.org>
> 
> Patch looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

We however need to have a real name for the Signed-off-by.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH RESEND] input: adp5589: Add gpio_set_multiple interface
From: Dmitry Torokhov @ 2019-05-23  7:18 UTC (permalink / raw)
  To: Bogdan Togorean; +Cc: linux-input, gustavo, linux-kernel, Michael.Hennerich
In-Reply-To: <20190521083821.26540-1-bogdan.togorean@analog.com>

Hi Bogdan,

On Tue, May 21, 2019 at 11:38:22AM +0300, Bogdan Togorean wrote:
> This patch implements the gpio_set_multiple interface for ADP558x chip.
> 
> Signed-off-by: Bogdan Togorean <bogdan.togorean@analog.com>
> ---
>  drivers/input/keyboard/adp5589-keys.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> index 2835fba71c33..143871bd60ef 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -416,6 +416,30 @@ static void adp5589_gpio_set_value(struct gpio_chip *chip,
>  	mutex_unlock(&kpad->gpio_lock);
>  }
>  
> +static void adp5589_gpio_set_multiple(struct gpio_chip *chip,
> +				      unsigned long *mask, unsigned long *bits)
> +{
> +	struct adp5589_kpad *kpad = container_of(chip, struct adp5589_kpad, gc);
> +	u8 bank, reg_mask, reg_bits;
> +
> +	mutex_lock(&kpad->gpio_lock);
> +
> +	for (bank = 0; bank <= kpad->var->bank(kpad->var->maxgpio); bank++) {
> +		if (bank > kpad->var->bank(get_bitmask_order(*mask) - 1))
> +			break;

I wonder if we should have:

	last_gpio = min(kpad->var->maxgpio, get_bitmask_order(*mask) - 1);
	last_bank = kpad->var->bank(last_bank);
	for (bank = 0; bank <= last_bank; bank++) {
		...
	}

> +		reg_mask = mask[bank / sizeof(*mask)] >>
> +			   ((bank % sizeof(*mask)) * BITS_PER_BYTE);
> +		reg_bits = bits[bank / sizeof(*bits)] >>
> +			   ((bank % sizeof(*bits)) * BITS_PER_BYTE);

This s really hard to parse. We know that "bank" is a byte, and mask is
long, we do not have to be this roundabout it.

> +		kpad->dat_out[bank] &= ~reg_mask;
> +		kpad->dat_out[bank] |= reg_bits & reg_mask;
> +		adp5589_write(kpad->client, kpad->var->reg(ADP5589_GPO_DATA_OUT_A) + bank,
> +			      kpad->dat_out[bank]);
> +	}

However the biggest issue is that this implementation seems to ignore
the kpad->gpiomap that translates GPIO numbers as seen by gpiolib to
GPIO numbers used by the chip. You need to reshuffle the mask and bits,
and only then do the writes.

Given the complexities, does set_multiple really save anything?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
From: Peter Hutterer @ 2019-05-23  6:01 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Xiaoxiao Liu, dmitry.torokhov, Hui Wang, XiaoXiao Liu,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com
In-Reply-To: <20190522074030.64sy7xt3wnomtxjb@pali>

On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote:
> On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote:
> > Hi Pali,
> > 
> > Ok, and cannot you set ALPS_DUALPOINT flag based on that
> > alps_check_is_trackpoint() result and then update
> > alps_process_packet_ss4_v3() code to supports also
> > V8 trackpoint packets?
> > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal.
> >       Then we choose the standard mouse driver.
> 
> Mouse speed is something which is configurable. Have you configured it
> somehow? Also there is libinput project should handle these settings
> more properly.
> 
> Adding Peter Hutterer, maintainer of libinput to loop. I think he could
> help with this problem.

libinput has a quirk for a magic multiplier on trackpoints. it was the only
solution I found that came close to "working" given that every device seems
to provide some other random magic data. Doc for it is here:
https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html

There are also different speeds depending on which xorg driver you'd use (or
libinput/Wayland), so a "mouse speed is not ideal" is almost a guarantee,
given a large enough variety of setups :) That's why we have the speed
toggle, but I'm happy to hear any suggestions on how to make the trackpoint
more useful (in libinput anyway).

> I do not think it is a good idea to force back to generic PS/2 mouse
> driver for touchpads and trackpoints. Native drivers for touchpads and
> trackpoints supports multitouch, absolute reporting and lot of other
> things... Also calculation of mouse speed from absolute positions on
> touchpads can be more easily fixed as from emulated relative movements.

Yeah, agree. Using PS/2 mouse drivers means you lose *all* the extra
features touchpads have like palm detection, tapping, scrolling, gestures,
etc.

Cheers,
   Peter

> 
> Dmitry, what is your opinion about this problem? What should psmouse.ko
> do in this situation? Disallow usage of absolute mode and force bare
> PS/2 relative mode?

^ permalink raw reply

* Re: [PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list
From: Aaron Ma @ 2019-05-23  5:02 UTC (permalink / raw)
  To: Benjamin Tissoires, Dmitry Torokhov
  Cc: open list:HID CORE LAYER, lkml, Christopher Heiny, Andrew Duggan,
	Hans de Goede
In-Reply-To: <CAO-hwJLXB8Qec9Yhz0y6WgvEpE6KHk_53g4VtPGj9mfvMzk4dg@mail.gmail.com>



On 5/21/19 2:49 PM, Benjamin Tissoires wrote:
> A quick google image search showed that the X240 had 2 versions: one
> with the top software buttons, one without.
> 
> And this definitively rings a bell. I am sure we asked Lenovo and
> Synaptics to change the PnPID when they would do such a change, but
> they "forgot" during the *40 series refresh.
> We have code in place to fix the reported ranges of the coordinates,
> and we had to check against the board id (see min_max_pnpid_table[] in
> synaptics.c).
> Unfortunately, X240 (LEN0035) is not part of this table, so I don't
> know which refresh of the board ID has implemented the non top
> software buttons.

After double confirm from Lenovo, looks like they mixed up with
touchpads on X240/X240s.

For now only one user reported this LEN0035 doesn't work on SMBus mode.
module parameter can be a workaround.
Maybe some touchpads with software top buttons are working well on
SMBus. Let's keep eyes on this issue for now.

Regards,
Aaron

> 
> Cheers,
> Benjamin

^ permalink raw reply

* [PATCH AUTOSEL 4.4 27/92] HID: logitech-hidpp: use RAP instead of FAP to get the protocol version
From: Sasha Levin @ 2019-05-22 19:30 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Benjamin Tissoires, Sasha Levin, linux-input
In-Reply-To: <20190522193127.27079-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 096377525cdb8251e4656085efc988bdf733fb4c ]

According to the logitech_hidpp_2.0_specification_draft_2012-06-04.pdf doc:
https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf

We should use a register-access-protocol request using the short input /
output report ids. This is necessary because 27MHz HID++ receivers have
a max-packetsize on their HIP++ endpoint of 8, so they cannot support
long reports. Using a feature-access-protocol request (which is always
long or very-long) with these will cause a timeout error, followed by
the hidpp driver treating the device as not being HID++ capable.

This commit fixes this by switching to using a rap request to get the
protocol version.

Besides being tested with a (046d:c517) 27MHz receiver with various
27MHz keyboards and mice, this has also been tested to not cause
regressions on a non-unifying dual-HID++ nano receiver (046d:c534) with
k270 and m185 HID++-2.0 devices connected and on a unifying/dj receiver
(046d:c52b) with a HID++-2.0 Logitech Rechargeable Touchpad T650.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-logitech-hidpp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5fd97860aec4d..3666e5064d0d3 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -414,13 +414,16 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 
 static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 {
+	const u8 ping_byte = 0x5a;
+	u8 ping_data[3] = { 0, 0, ping_byte };
 	struct hidpp_report response;
 	int ret;
 
-	ret = hidpp_send_fap_command_sync(hidpp,
+	ret = hidpp_send_rap_command_sync(hidpp,
+			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
-			NULL, 0, &response);
+			ping_data, sizeof(ping_data), &response);
 
 	if (ret == HIDPP_ERROR_INVALID_SUBID) {
 		hidpp->protocol_major = 1;
@@ -440,8 +443,14 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	hidpp->protocol_major = response.fap.params[0];
-	hidpp->protocol_minor = response.fap.params[1];
+	if (response.rap.params[2] != ping_byte) {
+		hid_err(hidpp->hid_dev, "%s: ping mismatch 0x%02x != 0x%02x\n",
+			__func__, response.rap.params[2], ping_byte);
+		return -EPROTO;
+	}
+
+	hidpp->protocol_major = response.rap.params[0];
+	hidpp->protocol_minor = response.rap.params[1];
 
 	return ret;
 }
-- 
2.20.1

^ permalink raw reply related

* [PATCH AUTOSEL 4.9 037/114] HID: logitech-hidpp: use RAP instead of FAP to get the protocol version
From: Sasha Levin @ 2019-05-22 19:29 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Benjamin Tissoires, Sasha Levin, linux-input
In-Reply-To: <20190522193017.26567-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 096377525cdb8251e4656085efc988bdf733fb4c ]

According to the logitech_hidpp_2.0_specification_draft_2012-06-04.pdf doc:
https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf

We should use a register-access-protocol request using the short input /
output report ids. This is necessary because 27MHz HID++ receivers have
a max-packetsize on their HIP++ endpoint of 8, so they cannot support
long reports. Using a feature-access-protocol request (which is always
long or very-long) with these will cause a timeout error, followed by
the hidpp driver treating the device as not being HID++ capable.

This commit fixes this by switching to using a rap request to get the
protocol version.

Besides being tested with a (046d:c517) 27MHz receiver with various
27MHz keyboards and mice, this has also been tested to not cause
regressions on a non-unifying dual-HID++ nano receiver (046d:c534) with
k270 and m185 HID++-2.0 devices connected and on a unifying/dj receiver
(046d:c52b) with a HID++-2.0 Logitech Rechargeable Touchpad T650.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-logitech-hidpp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 3198faf5cff4d..38d9deb03d16c 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -449,13 +449,16 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 
 static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 {
+	const u8 ping_byte = 0x5a;
+	u8 ping_data[3] = { 0, 0, ping_byte };
 	struct hidpp_report response;
 	int ret;
 
-	ret = hidpp_send_fap_command_sync(hidpp,
+	ret = hidpp_send_rap_command_sync(hidpp,
+			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
-			NULL, 0, &response);
+			ping_data, sizeof(ping_data), &response);
 
 	if (ret == HIDPP_ERROR_INVALID_SUBID) {
 		hidpp->protocol_major = 1;
@@ -475,8 +478,14 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	hidpp->protocol_major = response.fap.params[0];
-	hidpp->protocol_minor = response.fap.params[1];
+	if (response.rap.params[2] != ping_byte) {
+		hid_err(hidpp->hid_dev, "%s: ping mismatch 0x%02x != 0x%02x\n",
+			__func__, response.rap.params[2], ping_byte);
+		return -EPROTO;
+	}
+
+	hidpp->protocol_major = response.rap.params[0];
+	hidpp->protocol_minor = response.rap.params[1];
 
 	return ret;
 }
-- 
2.20.1

^ permalink raw reply related

* [PATCH AUTOSEL 4.14 054/167] HID: logitech-hidpp: use RAP instead of FAP to get the protocol version
From: Sasha Levin @ 2019-05-22 19:26 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Benjamin Tissoires, Sasha Levin, linux-input
In-Reply-To: <20190522192842.25858-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 096377525cdb8251e4656085efc988bdf733fb4c ]

According to the logitech_hidpp_2.0_specification_draft_2012-06-04.pdf doc:
https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf

We should use a register-access-protocol request using the short input /
output report ids. This is necessary because 27MHz HID++ receivers have
a max-packetsize on their HIP++ endpoint of 8, so they cannot support
long reports. Using a feature-access-protocol request (which is always
long or very-long) with these will cause a timeout error, followed by
the hidpp driver treating the device as not being HID++ capable.

This commit fixes this by switching to using a rap request to get the
protocol version.

Besides being tested with a (046d:c517) 27MHz receiver with various
27MHz keyboards and mice, this has also been tested to not cause
regressions on a non-unifying dual-HID++ nano receiver (046d:c534) with
k270 and m185 HID++-2.0 devices connected and on a unifying/dj receiver
(046d:c52b) with a HID++-2.0 Logitech Rechargeable Touchpad T650.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-logitech-hidpp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b83d4173fc7f5..d209b12057d59 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -725,13 +725,16 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 
 static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 {
+	const u8 ping_byte = 0x5a;
+	u8 ping_data[3] = { 0, 0, ping_byte };
 	struct hidpp_report response;
 	int ret;
 
-	ret = hidpp_send_fap_command_sync(hidpp,
+	ret = hidpp_send_rap_command_sync(hidpp,
+			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
-			NULL, 0, &response);
+			ping_data, sizeof(ping_data), &response);
 
 	if (ret == HIDPP_ERROR_INVALID_SUBID) {
 		hidpp->protocol_major = 1;
@@ -751,8 +754,14 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	hidpp->protocol_major = response.fap.params[0];
-	hidpp->protocol_minor = response.fap.params[1];
+	if (response.rap.params[2] != ping_byte) {
+		hid_err(hidpp->hid_dev, "%s: ping mismatch 0x%02x != 0x%02x\n",
+			__func__, response.rap.params[2], ping_byte);
+		return -EPROTO;
+	}
+
+	hidpp->protocol_major = response.rap.params[0];
+	hidpp->protocol_minor = response.rap.params[1];
 
 	return ret;
 }
-- 
2.20.1

^ permalink raw reply related

* [PATCH AUTOSEL 4.19 080/244] HID: logitech-hidpp: use RAP instead of FAP to get the protocol version
From: Sasha Levin @ 2019-05-22 19:23 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Benjamin Tissoires, Sasha Levin, linux-input
In-Reply-To: <20190522192630.24917-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 096377525cdb8251e4656085efc988bdf733fb4c ]

According to the logitech_hidpp_2.0_specification_draft_2012-06-04.pdf doc:
https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf

We should use a register-access-protocol request using the short input /
output report ids. This is necessary because 27MHz HID++ receivers have
a max-packetsize on their HIP++ endpoint of 8, so they cannot support
long reports. Using a feature-access-protocol request (which is always
long or very-long) with these will cause a timeout error, followed by
the hidpp driver treating the device as not being HID++ capable.

This commit fixes this by switching to using a rap request to get the
protocol version.

Besides being tested with a (046d:c517) 27MHz receiver with various
27MHz keyboards and mice, this has also been tested to not cause
regressions on a non-unifying dual-HID++ nano receiver (046d:c534) with
k270 and m185 HID++-2.0 devices connected and on a unifying/dj receiver
(046d:c52b) with a HID++-2.0 Logitech Rechargeable Touchpad T650.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-logitech-hidpp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 8425d3548a414..edf224ad13369 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -725,13 +725,16 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 
 static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 {
+	const u8 ping_byte = 0x5a;
+	u8 ping_data[3] = { 0, 0, ping_byte };
 	struct hidpp_report response;
 	int ret;
 
-	ret = hidpp_send_fap_command_sync(hidpp,
+	ret = hidpp_send_rap_command_sync(hidpp,
+			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
-			NULL, 0, &response);
+			ping_data, sizeof(ping_data), &response);
 
 	if (ret == HIDPP_ERROR_INVALID_SUBID) {
 		hidpp->protocol_major = 1;
@@ -751,8 +754,14 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	hidpp->protocol_major = response.fap.params[0];
-	hidpp->protocol_minor = response.fap.params[1];
+	if (response.rap.params[2] != ping_byte) {
+		hid_err(hidpp->hid_dev, "%s: ping mismatch 0x%02x != 0x%02x\n",
+			__func__, response.rap.params[2], ping_byte);
+		return -EPROTO;
+	}
+
+	hidpp->protocol_major = response.rap.params[0];
+	hidpp->protocol_minor = response.rap.params[1];
 
 	return ret;
 }
-- 
2.20.1

^ permalink raw reply related

* [PATCH AUTOSEL 5.0 099/317] HID: logitech-hidpp: use RAP instead of FAP to get the protocol version
From: Sasha Levin @ 2019-05-22 19:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Benjamin Tissoires, Sasha Levin, linux-input
In-Reply-To: <20190522192338.23715-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 096377525cdb8251e4656085efc988bdf733fb4c ]

According to the logitech_hidpp_2.0_specification_draft_2012-06-04.pdf doc:
https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf

We should use a register-access-protocol request using the short input /
output report ids. This is necessary because 27MHz HID++ receivers have
a max-packetsize on their HIP++ endpoint of 8, so they cannot support
long reports. Using a feature-access-protocol request (which is always
long or very-long) with these will cause a timeout error, followed by
the hidpp driver treating the device as not being HID++ capable.

This commit fixes this by switching to using a rap request to get the
protocol version.

Besides being tested with a (046d:c517) 27MHz receiver with various
27MHz keyboards and mice, this has also been tested to not cause
regressions on a non-unifying dual-HID++ nano receiver (046d:c534) with
k270 and m185 HID++-2.0 devices connected and on a unifying/dj receiver
(046d:c52b) with a HID++-2.0 Logitech Rechargeable Touchpad T650.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-logitech-hidpp.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 199cc256e9d9d..ffd30c7492df8 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -836,13 +836,16 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature,
 
 static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 {
+	const u8 ping_byte = 0x5a;
+	u8 ping_data[3] = { 0, 0, ping_byte };
 	struct hidpp_report response;
 	int ret;
 
-	ret = hidpp_send_fap_command_sync(hidpp,
+	ret = hidpp_send_rap_command_sync(hidpp,
+			REPORT_ID_HIDPP_SHORT,
 			HIDPP_PAGE_ROOT_IDX,
 			CMD_ROOT_GET_PROTOCOL_VERSION,
-			NULL, 0, &response);
+			ping_data, sizeof(ping_data), &response);
 
 	if (ret == HIDPP_ERROR_INVALID_SUBID) {
 		hidpp->protocol_major = 1;
@@ -862,8 +865,14 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	hidpp->protocol_major = response.fap.params[0];
-	hidpp->protocol_minor = response.fap.params[1];
+	if (response.rap.params[2] != ping_byte) {
+		hid_err(hidpp->hid_dev, "%s: ping mismatch 0x%02x != 0x%02x\n",
+			__func__, response.rap.params[2], ping_byte);
+		return -EPROTO;
+	}
+
+	hidpp->protocol_major = response.rap.params[0];
+	hidpp->protocol_minor = response.rap.params[1];
 
 	return ret;
 }
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH] HID: logitech-hidpp: HID: make const array consumer_rdesc_start static
From: Jiri Kosina @ 2019-05-22 11:19 UTC (permalink / raw)
  To: Colin King; +Cc: Benjamin Tissoires, linux-input, kernel-janitors, linux-kernel
In-Reply-To: <20190510131722.5023-1-colin.king@canonical.com>

On Fri, 10 May 2019, Colin King wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Don't populate the array consumer_rdesc_start on the stack but instead
> make it static. Makes the object code smaller by 88 bytes.
> 
> Before:
>    text	   data	    bss	    dec	    hex	filename
>   59155	   9840	    448	  69443	  10f43	drivers/hid/hid-logitech-hidpp.o
> 
> After:
>    text	   data	    bss	    dec	    hex	filename
>   59003	   9904	    448	  69355	  10eeb	drivers/hid/hid-logitech-hidpp.o
> 
> (gcc version 8.3.0, amd64)
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 72fc9c0566db..df960491e473 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -2862,7 +2862,7 @@ static u8 *hidpp10_consumer_keys_report_fixup(struct hidpp_device *hidpp,
>  					      u8 *_rdesc, unsigned int *rsize)
>  {
>  	/* Note 0 terminated so we can use strnstr to search for this. */
> -	const char consumer_rdesc_start[] = {
> +	static const char consumer_rdesc_start[] = {
>  		0x05, 0x0C,	/* USAGE_PAGE (Consumer Devices)       */
>  		0x09, 0x01,	/* USAGE (Consumer Control)            */
>  		0xA1, 0x01,	/* COLLECTION (Application)            */

Applied, thanks Colin.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: logitech-dj: make const array template static
From: Jiri Kosina @ 2019-05-22 11:19 UTC (permalink / raw)
  To: Colin King; +Cc: Benjamin Tissoires, linux-input, kernel-janitors, linux-kernel
In-Reply-To: <20190510131039.4675-1-colin.king@canonical.com>

On Fri, 10 May 2019, Colin King wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Don't populate the array template  on the stack but instead make it
> static. Makes the object code smaller by 10 bytes. Also reformat
> the declaration.
> 
> Before:
>    text	   data	    bss	    dec	    hex	filename
>   29376	   9360	    128	  38864	   97d0	drivers/hid/hid-logitech-dj.o
> 
> After:
>    text	   data	    bss	    dec	    hex	filename
>   29270	   9456	    128	  38854	   97c6	drivers/hid/hid-logitech-dj.o
> 
> (gcc version 8.3.0, amd64)
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/hid/hid-logitech-dj.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index b1e894618eed..72d0ab05401f 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -1111,12 +1111,14 @@ static int logi_dj_recv_send_report(struct dj_receiver_dev *djrcv_dev,
>  
>  static int logi_dj_recv_query_hidpp_devices(struct dj_receiver_dev *djrcv_dev)
>  {
> -	const u8 template[] = {REPORT_ID_HIDPP_SHORT,
> -			       HIDPP_RECEIVER_INDEX,
> -			       HIDPP_SET_REGISTER,
> -			       HIDPP_REG_CONNECTION_STATE,
> -			       HIDPP_FAKE_DEVICE_ARRIVAL,
> -			       0x00, 0x00};
> +	static const u8 template[] = {
> +		REPORT_ID_HIDPP_SHORT,
> +		HIDPP_RECEIVER_INDEX,
> +		HIDPP_SET_REGISTER,
> +		HIDPP_REG_CONNECTION_STATE,
> +		HIDPP_FAKE_DEVICE_ARRIVAL,
> +		0x00, 0x00
> +	};

Applied.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: multitouch: handle faulty Elo touch device
From: Jiri Kosina @ 2019-05-22 10:38 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER, lkml, Breno Leitao
In-Reply-To: <CAO-hwJJfRXEdwMq9KLXDMs37CnHXnVhUzD5sbd5uzoeVdKQdvw@mail.gmail.com>

On Tue, 21 May 2019, Benjamin Tissoires wrote:

> On Tue, May 21, 2019 at 3:38 PM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > Since kernel v5.0, one single win8 touchscreen device failed.
> > And it turns out this is because it reports 2 InRange usage per touch.
> >
> > It's a first, and I *really* wonder how this was allowed by Microsoft in
> > the first place. But IIRC, Breno told me this happened *after* a firmware
> > upgrade...
> >
> > Anyway, better be safe for those crappy devices, and make sure we have
> > a full slot before jumping to the next.
> > This won't prevent all crappy devices to fail here, but at least we will
> > have a safeguard as long as the contact ID and the X and Y coordinates
> > are placed in the report after the grabage.
> 
> Grmbl, I forgot:
> Fixes: 01eaac7e5713 ("HID: multitouch: remove one copy of values")
> CC: stable@vger.kernel.org # v5.0+

I've now pushed this to for-5.2/fixes. Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] input: silead: Add MSSL0017 to acpi_device_id.
From: Hans de Goede @ 2019-05-22 10:12 UTC (permalink / raw)
  To: Danct12; +Cc: Dmitry Torokhov, linux-input, platform-driver-x86, linux-kernel
In-Reply-To: <20190522045455.15769-1-danct12@disroot.org>

Hi,

On 22-05-19 06:54, Danct12 wrote:
> On Chuwi Hi10 Plus, the Silead device id is MSSL0017.
> 
> Signed-off-by: Danct12 <danct12@disroot.org>

Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>   drivers/input/touchscreen/silead.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
> index 09241d4cdebc..06f0eb04a8fd 100644
> --- a/drivers/input/touchscreen/silead.c
> +++ b/drivers/input/touchscreen/silead.c
> @@ -617,6 +617,7 @@ static const struct acpi_device_id silead_ts_acpi_match[] = {
>   	{ "MSSL1680", 0 },
>   	{ "MSSL0001", 0 },
>   	{ "MSSL0002", 0 },
> +	{ "MSSL0017", 0 },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match);
> 

^ permalink raw reply

* Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
From: Pali Rohár @ 2019-05-22  7:40 UTC (permalink / raw)
  To: Xiaoxiao Liu, Peter Hutterer, dmitry.torokhov
  Cc: Hui Wang, XiaoXiao Liu, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Xiaojian Cao, zhangfp1@lenovo.com
In-Reply-To: <OSBPR01MB48550B43F78BBFBDC20D414DDA000@OSBPR01MB4855.jpnprd01.prod.outlook.com>

On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote:
> Hi Pali,
> 
> Ok, and cannot you set ALPS_DUALPOINT flag based on that
> alps_check_is_trackpoint() result and then update
> alps_process_packet_ss4_v3() code to supports also
> V8 trackpoint packets?
> --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal.
>       Then we choose the standard mouse driver.

Mouse speed is something which is configurable. Have you configured it
somehow? Also there is libinput project should handle these settings
more properly.

Adding Peter Hutterer, maintainer of libinput to loop. I think he could
help with this problem.

I do not think it is a good idea to force back to generic PS/2 mouse
driver for touchpads and trackpoints. Native drivers for touchpads and
trackpoints supports multitouch, absolute reporting and lot of other
things... Also calculation of mouse speed from absolute positions on
touchpads can be more easily fixed as from emulated relative movements.

Dmitry, what is your opinion about this problem? What should psmouse.ko
do in this situation? Disallow usage of absolute mode and force bare
PS/2 relative mode?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* 答复: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
From: Xiaoxiao Liu @ 2019-05-22  7:30 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Hui Wang, XiaoXiao Liu, dmitry.torokhov@gmail.com,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com
In-Reply-To: <20190522063546.kb74mxeprkauicul@pali>

Hi Pali,

Ok, and cannot you set ALPS_DUALPOINT flag based on that
alps_check_is_trackpoint() result and then update
alps_process_packet_ss4_v3() code to supports also
V8 trackpoint packets?
--> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal.
      Then we choose the standard mouse driver.
      
XiaoXiao Liu
sliuuxiaonxiao@gmail.com
xiaoxiao.liu-1@cn.alps.com
> 
> -----邮件原件-----
发件人: Pali Rohár <pali.rohar@gmail.com> 
发送时间: Wednesday, May 22, 2019 2:36 PM
收件人: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>
抄送: Hui Wang <hui.wang@canonical.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com
主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.

Hi!

On Wednesday 22 May 2019 02:53:18 Xiaoxiao Liu wrote:
> Hi Pali,
> 
> Why it does not report input data?
> --> The alps devices which detected to use the ALPS_PROTO_V8  contains ALPS touchpad and ALPS track point. 
>      But the ALPS_PROTO_V8 do not support the track point device process. 
>      When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method  alps_process_packet_ss4_v2 will reject to report the data because the v8 device is      not set the ALPS_DUALPOINT flag. 
>      You can refer below code.

Ok, and cannot you set ALPS_DUALPOINT flag based on that
alps_check_is_trackpoint() result and then update
alps_process_packet_ss4_v3() code to supports also
V8 trackpoint packets?

> 	/* Report trackstick */
> 	if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) {
> 		if (!(priv->flags & ALPS_DUALPOINT)) {
> 			psmouse_warn(psmouse,
> 				     "Rejected trackstick packet from non DualPoint device");
> 			return;
> 		}
> 		...
> 		return;
> 	}
> 
> why is for non-ALPS trackpoints used ALPS driver?
> --> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad.

Ok, now I got it.

> The v8 protocol condition may modified  as below, it will be more easier to understand.
> 
> 	 if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) &&  (alps_check_is_trackpoint(psmouse) != 0)  ) {
>  		protocol = &alps_v8_protocol_data;
> 	}

Yes, this would be more easier to understand.

Anyway, more information should be in commit message.

> Best Regards
> XiaoXiao Liu
> sliuuxiaonxiao@gmail.com
> xiaoxiao.liu-1@cn.alps.com
> 
> -----邮件原件-----
> 发件人: Pali Rohár <pali.rohar@gmail.com>
> 发送时间: Tuesday, May 21, 2019 5:46 PM
> 收件人: Hui Wang <hui.wang@canonical.com>
> 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu 
> <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; 
> linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 
> Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com
> 主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
> 
> Hello!
> 
> On Tuesday 21 May 2019 10:26:47 Hui Wang wrote:
> > Tested-by: Hui Wang <hui.wang@canonical.com>
> > 
> > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:
> > > Add Pali Rohár.
> > > 
> > > -----邮件原件-----
> > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > > 发送时间: Monday, May 20, 2019 7:02 PM
> > > 收件人: dmitry.torokhov@gmail.com
> > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao 
> > > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu 
> > > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu 
> > > <sliuuxiaonxiao@gmail.com>
> > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
> > > 
> > > when the alps trackpoint is detected and using the 
> > > alps_v8_protocol_data procotol, the alps driver will not report the input data.
> 
> Why it does not report input data?
> 
> > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected.
> 
> This looks like an (undocumented) hack or workaround. Not a solution.
> 
> > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > > ---
> > >   drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++-
> > >   1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/input/mouse/alps.c 
> > > b/drivers/input/mouse/alps.c index 0a6f7ca883e7..516ae1d0eb17 
> > > 100644
> > > --- a/drivers/input/mouse/alps.c
> > > +++ b/drivers/input/mouse/alps.c
> > > @@ -24,7 +24,7 @@
> > >   #include "psmouse.h"
> > >   #include "alps.h"
> > > -
> > > +#include "trackpoint.h"
> > >   /*
> > >    * Definitions for ALPS version 3 and 4 command mode protocol
> > >    */
> > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
> > >   	return NULL;
> > >   }
> > > +int alps_check_is_trackpoint(struct psmouse *psmouse) {
> > > +	u8 param[2] = { 0 };
> > > +	int error;
> > > +
> > > +	error = ps2_command(&psmouse->ps2dev,
> > > +			    param, MAKE_PS2_CMD(0, 2, TP_READ_ID));
> > > +	if (error)
> > > +		return error;
> > > +
> > > +	if (param[0] == TP_VARIANT_ALPS)
> > > +		return 0;
> > > +	psmouse_warn(psmouse, "It is not alps trackpoint.\n");
> > > +	return -ENODEV;
> > > +}
> 
> So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not.
> 
> > > +
> > >   static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)  {
> > >   	const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> > >   			protocol = &alps_v3_protocol_data;
> > >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> > >   			   (e7[2] == 0x14 || e7[2] == 0x28)) {
> > > +				if (alps_check_is_trackpoint(psmouse) == 0) {
> > > +					psmouse_warn(psmouse,
> > > +					"It is alps trackpoint, use the standard mouse driver.\n");
> > > +					return -EINVAL;
> 
> And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used.
> 
> So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct...
> 
> And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too.
> 
> So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution.
> 
> > > +				}
> > >   			protocol = &alps_v8_protocol_data;
> > >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) {
> > >   			protocol = &alps_v9_protocol_data;
> > > --
> > > 2.20.1
> > > 
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

--
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* Re: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
From: Pali Rohár @ 2019-05-22  6:35 UTC (permalink / raw)
  To: Xiaoxiao Liu
  Cc: Hui Wang, XiaoXiao Liu, dmitry.torokhov@gmail.com,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com
In-Reply-To: <OSBPR01MB4855D744473149D037612506DA000@OSBPR01MB4855.jpnprd01.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 6124 bytes --]

Hi!

On Wednesday 22 May 2019 02:53:18 Xiaoxiao Liu wrote:
> Hi Pali,
> 
> Why it does not report input data?
> --> The alps devices which detected to use the ALPS_PROTO_V8  contains ALPS touchpad and ALPS track point. 
>      But the ALPS_PROTO_V8 do not support the track point device process. 
>      When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method  alps_process_packet_ss4_v2 will reject to report the data because the v8 device is      not set the ALPS_DUALPOINT flag. 
>      You can refer below code.

Ok, and cannot you set ALPS_DUALPOINT flag based on that
alps_check_is_trackpoint() result and then update
alps_process_packet_ss4_v3() code to supports also
V8 trackpoint packets?

> 	/* Report trackstick */
> 	if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) {
> 		if (!(priv->flags & ALPS_DUALPOINT)) {
> 			psmouse_warn(psmouse,
> 				     "Rejected trackstick packet from non DualPoint device");
> 			return;
> 		}
> 		...
> 		return;
> 	}
> 
> why is for non-ALPS trackpoints used ALPS driver?
> --> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad.

Ok, now I got it.

> The v8 protocol condition may modified  as below, it will be more easier to understand.
> 
> 	 if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) &&  (alps_check_is_trackpoint(psmouse) != 0)  ) {
>  		protocol = &alps_v8_protocol_data;
> 	}

Yes, this would be more easier to understand.

Anyway, more information should be in commit message.

> Best Regards
> XiaoXiao Liu
> sliuuxiaonxiao@gmail.com
> xiaoxiao.liu-1@cn.alps.com
> 
> -----邮件原件-----
> 发件人: Pali Rohár <pali.rohar@gmail.com> 
> 发送时间: Tuesday, May 21, 2019 5:46 PM
> 收件人: Hui Wang <hui.wang@canonical.com>
> 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com
> 主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
> 
> Hello!
> 
> On Tuesday 21 May 2019 10:26:47 Hui Wang wrote:
> > Tested-by: Hui Wang <hui.wang@canonical.com>
> > 
> > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:
> > > Add Pali Rohár.
> > > 
> > > -----邮件原件-----
> > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > > 发送时间: Monday, May 20, 2019 7:02 PM
> > > 收件人: dmitry.torokhov@gmail.com
> > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao 
> > > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu 
> > > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu 
> > > <sliuuxiaonxiao@gmail.com>
> > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
> > > 
> > > when the alps trackpoint is detected and using the 
> > > alps_v8_protocol_data procotol, the alps driver will not report the input data.
> 
> Why it does not report input data?
> 
> > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected.
> 
> This looks like an (undocumented) hack or workaround. Not a solution.
> 
> > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > > ---
> > >   drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++-
> > >   1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
> > > index 0a6f7ca883e7..516ae1d0eb17 100644
> > > --- a/drivers/input/mouse/alps.c
> > > +++ b/drivers/input/mouse/alps.c
> > > @@ -24,7 +24,7 @@
> > >   #include "psmouse.h"
> > >   #include "alps.h"
> > > -
> > > +#include "trackpoint.h"
> > >   /*
> > >    * Definitions for ALPS version 3 and 4 command mode protocol
> > >    */
> > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
> > >   	return NULL;
> > >   }
> > > +int alps_check_is_trackpoint(struct psmouse *psmouse) {
> > > +	u8 param[2] = { 0 };
> > > +	int error;
> > > +
> > > +	error = ps2_command(&psmouse->ps2dev,
> > > +			    param, MAKE_PS2_CMD(0, 2, TP_READ_ID));
> > > +	if (error)
> > > +		return error;
> > > +
> > > +	if (param[0] == TP_VARIANT_ALPS)
> > > +		return 0;
> > > +	psmouse_warn(psmouse, "It is not alps trackpoint.\n");
> > > +	return -ENODEV;
> > > +}
> 
> So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not.
> 
> > > +
> > >   static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)  {
> > >   	const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> > >   			protocol = &alps_v3_protocol_data;
> > >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> > >   			   (e7[2] == 0x14 || e7[2] == 0x28)) {
> > > +				if (alps_check_is_trackpoint(psmouse) == 0) {
> > > +					psmouse_warn(psmouse,
> > > +					"It is alps trackpoint, use the standard mouse driver.\n");
> > > +					return -EINVAL;
> 
> And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used.
> 
> So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct...
> 
> And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too.
> 
> So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution.
> 
> > > +				}
> > >   			protocol = &alps_v8_protocol_data;
> > >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) {
> > >   			protocol = &alps_v9_protocol_data;
> > > --
> > > 2.20.1
> > > 
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply

* [PATCH] input: silead: Add MSSL0017 to acpi_device_id.
From: Danct12 @ 2019-05-22  4:54 UTC (permalink / raw)
  To: hdegoede
  Cc: Danct12, Dmitry Torokhov, linux-input, platform-driver-x86,
	linux-kernel

On Chuwi Hi10 Plus, the Silead device id is MSSL0017.

Signed-off-by: Danct12 <danct12@disroot.org>
---
 drivers/input/touchscreen/silead.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 09241d4cdebc..06f0eb04a8fd 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -617,6 +617,7 @@ static const struct acpi_device_id silead_ts_acpi_match[] = {
 	{ "MSSL1680", 0 },
 	{ "MSSL0001", 0 },
 	{ "MSSL0002", 0 },
+	{ "MSSL0017", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match);
-- 
2.21.0

^ permalink raw reply related

* 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
From: Xiaoxiao Liu @ 2019-05-22  2:53 UTC (permalink / raw)
  To: Pali Rohár, Hui Wang
  Cc: XiaoXiao Liu, dmitry.torokhov@gmail.com,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xiaojian Cao, zhangfp1@lenovo.com
In-Reply-To: <20190521094622.syeub6tcqhbyc7sg@pali>

Hi Pali,

Why it does not report input data?
--> The alps devices which detected to use the ALPS_PROTO_V8  contains ALPS touchpad and ALPS track point. 
     But the ALPS_PROTO_V8 do not support the track point device process. 
     When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method  alps_process_packet_ss4_v2 will reject to report the data because the v8 device is      not set the ALPS_DUALPOINT flag. 
     You can refer below code.

	/* Report trackstick */
	if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) {
		if (!(priv->flags & ALPS_DUALPOINT)) {
			psmouse_warn(psmouse,
				     "Rejected trackstick packet from non DualPoint device");
			return;
		}
		...
		return;
	}

why is for non-ALPS trackpoints used ALPS driver?
--> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad.

The v8 protocol condition may modified  as below, it will be more easier to understand.

	 if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) &&  (alps_check_is_trackpoint(psmouse) != 0)  ) {
 		protocol = &alps_v8_protocol_data;
	}

Best Regards
XiaoXiao Liu
sliuuxiaonxiao@gmail.com
xiaoxiao.liu-1@cn.alps.com

-----邮件原件-----
发件人: Pali Rohár <pali.rohar@gmail.com> 
发送时间: Tuesday, May 21, 2019 5:46 PM
收件人: Hui Wang <hui.wang@canonical.com>
抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com
主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.

Hello!

On Tuesday 21 May 2019 10:26:47 Hui Wang wrote:
> Tested-by: Hui Wang <hui.wang@canonical.com>
> 
> On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:
> > Add Pali Rohár.
> > 
> > -----邮件原件-----
> > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > 发送时间: Monday, May 20, 2019 7:02 PM
> > 收件人: dmitry.torokhov@gmail.com
> > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao 
> > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu 
> > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu 
> > <sliuuxiaonxiao@gmail.com>
> > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.
> > 
> > when the alps trackpoint is detected and using the 
> > alps_v8_protocol_data procotol, the alps driver will not report the input data.

Why it does not report input data?

> > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected.

This looks like an (undocumented) hack or workaround. Not a solution.

> > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com>
> > ---
> >   drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++-
> >   1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
> > index 0a6f7ca883e7..516ae1d0eb17 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -24,7 +24,7 @@
> >   #include "psmouse.h"
> >   #include "alps.h"
> > -
> > +#include "trackpoint.h"
> >   /*
> >    * Definitions for ALPS version 3 and 4 command mode protocol
> >    */
> > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
> >   	return NULL;
> >   }
> > +int alps_check_is_trackpoint(struct psmouse *psmouse) {
> > +	u8 param[2] = { 0 };
> > +	int error;
> > +
> > +	error = ps2_command(&psmouse->ps2dev,
> > +			    param, MAKE_PS2_CMD(0, 2, TP_READ_ID));
> > +	if (error)
> > +		return error;
> > +
> > +	if (param[0] == TP_VARIANT_ALPS)
> > +		return 0;
> > +	psmouse_warn(psmouse, "It is not alps trackpoint.\n");
> > +	return -ENODEV;
> > +}

So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not.

> > +
> >   static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)  {
> >   	const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
> >   			protocol = &alps_v3_protocol_data;
> >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> >   			   (e7[2] == 0x14 || e7[2] == 0x28)) {
> > +				if (alps_check_is_trackpoint(psmouse) == 0) {
> > +					psmouse_warn(psmouse,
> > +					"It is alps trackpoint, use the standard mouse driver.\n");
> > +					return -EINVAL;

And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used.

So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct...

And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too.

So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution.

> > +				}
> >   			protocol = &alps_v8_protocol_data;
> >   		} else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) {
> >   			protocol = &alps_v9_protocol_data;
> > --
> > 2.20.1
> > 

--
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* [PATCH] Input: uinput - add compat ioctl number translation for UI_*_FF_UPLOAD
From: Andrey Smirnov @ 2019-05-22  1:39 UTC (permalink / raw)
  To: linux-input
  Cc: Andrey Smirnov, Pierre-Loup A . Griffais, Dmitry Torokhov, stable,
	linux-kernel

In the case of compat syscall ioctl numbers for UI_BEGIN_FF_UPLOAD and
UI_END_FF_UPLOAD need to be adjusted before being passed on
uinput_ioctl_handler() since code built with -m32 will be passing
slightly different values. Extend the code already covering
UI_SET_PHYS to cover UI_BEGIN_FF_UPLOAD and UI_END_FF_UPLOAD as well.

Reported-by: Pierre-Loup A. Griffais <pgriffais@valvesoftware.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: stable@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/input/misc/uinput.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 1a6762fc38f9..1116d4cd5695 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -1051,13 +1051,24 @@ static long uinput_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 #ifdef CONFIG_COMPAT
 
-#define UI_SET_PHYS_COMPAT	_IOW(UINPUT_IOCTL_BASE, 108, compat_uptr_t)
+#define UI_SET_PHYS_COMPAT		_IOW(UINPUT_IOCTL_BASE, 108, compat_uptr_t)
+#define UI_BEGIN_FF_UPLOAD_COMPAT	_IOWR(UINPUT_IOCTL_BASE, 200, struct uinput_ff_upload_compat)
+#define UI_END_FF_UPLOAD_COMPAT		_IOW(UINPUT_IOCTL_BASE, 201, struct uinput_ff_upload_compat)
 
 static long uinput_compat_ioctl(struct file *file,
 				unsigned int cmd, unsigned long arg)
 {
-	if (cmd == UI_SET_PHYS_COMPAT)
+	switch (cmd) {
+	case UI_SET_PHYS_COMPAT:
 		cmd = UI_SET_PHYS;
+		break;
+	case UI_BEGIN_FF_UPLOAD_COMPAT:
+		cmd = UI_BEGIN_FF_UPLOAD;
+		break;
+	case UI_END_FF_UPLOAD_COMPAT:
+		cmd = UI_END_FF_UPLOAD;
+		break;
+	}
 
 	return uinput_ioctl_handler(file, cmd, arg, compat_ptr(arg));
 }
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH v4 1/2] HID: quirks: Refactor ELAN 400 and 401 handling
From: Bjorn Andersson @ 2019-05-21 16:43 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: dmitry.torokhov, jikos, benjamin.tissoires, lee.jones, robh+dt,
	mark.rutland, agross, david.brown, hdegoede, linux-input,
	devicetree, linux-arm-msm, linux-kernel
In-Reply-To: <20190423160605.9970-1-jeffrey.l.hugo@gmail.com>

On Tue 23 Apr 09:06 PDT 2019, Jeffrey Hugo wrote:

> There needs to be coordination between hid-quirks and the elan_i2c driver
> about which devices are handled by what drivers.  Currently, both use
> whitelists, which results in valid devices being unhandled by default,
> when they should not be rejected by hid-quirks.  This is quickly becoming
> an issue.
> 
> Since elan_i2c has a maintained whitelist of what devices it will handle,
> use that to implement a blacklist in hid-quirks so that only the devices
> that need to be handled by elan_i2c get rejected by hid-quirks, and
> everything else is handled by default.  The downside is the whitelist and
> blacklist need to be kept in sync.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Jiri, the two patches in this series doesn't have a build time
dependency, so if you take this one through your tree I'll take 2/2
through arm-soc.

Regards,
Bjorn

> Suggested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/hid/hid-quirks.c            | 64 ++++++++++++++++++++++++-----
>  drivers/input/mouse/elan_i2c_core.c |  4 ++
>  2 files changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 77ffba48cc73..656485e08eb7 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -987,17 +987,61 @@ bool hid_ignore(struct hid_device *hdev)
>  		break;
>  	case USB_VENDOR_ID_ELAN:
>  		/*
> -		 * Many Elan devices have a product id of 0x0401 and are handled
> -		 * by the elan_i2c input driver. But the ACPI HID ELAN0800 dev
> -		 * is not (and cannot be) handled by that driver ->
> -		 * Ignore all 0x0401 devs except for the ELAN0800 dev.
> +		 * Blacklist of everything that gets handled by the elan_i2c
> +		 * input driver.  This should be kept in sync with the whitelist
> +		 * that exists in that driver.  This avoids disabling valid
> +		 * touchpads and other ELAN devices.
>  		 */
> -		if (hdev->product == 0x0401 &&
> -		    strncmp(hdev->name, "ELAN0800", 8) != 0)
> -			return true;
> -		/* Same with product id 0x0400 */
> -		if (hdev->product == 0x0400 &&
> -		    strncmp(hdev->name, "QTEC0001", 8) != 0)
> +		if ((hdev->product == 0x0401 || hdev->product == 0x0400) &&
> +		   (strncmp(hdev->name, "ELAN0000", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0100", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0600", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0601", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0602", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0603", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0604", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0605", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0606", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0607", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0608", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0609", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN060B", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN060C", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN060F", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0610", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0611", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0612", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0613", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0614", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0615", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0616", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0617", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0618", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0619", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061A", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061B", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061C", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061D", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061E", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN061F", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0620", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0621", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0622", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0623", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0624", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0625", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0626", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0627", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0628", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0629", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN062A", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN062B", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN062C", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN062D", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0631", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN0632", 8) == 0 ||
> +		    strncmp(hdev->name, "ELAN1000", 8) == 0 ||
> +		    strncmp(hdev->name, "elan,ekth3000", 13) == 0))
>  			return true;
>  		break;
>  	}
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index f9525d6f0bfe..3ded19528cd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1332,6 +1332,10 @@ static const struct i2c_device_id elan_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, elan_id);
>  
> +/*
> + * when these whtielists get updated, the corresponding blacklist in hid-quirks
> + * needs to be updated to match.
> + */
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id elan_acpi_id[] = {
>  	{ "ELAN0000", 0 },
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH] HID: Increase maximum report size allowed by hid_field_extract()
From: Kai Heng Feng @ 2019-05-21 16:24 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, open list:HID CORE LAYER, lkml, Ronald Tschalär
In-Reply-To: <CAO-hwJKctsp9=ZJuJB6YRtA+RHuv1NJ+9cWp9hub8oATh1MXCA@mail.gmail.com>



> On May 21, 2019, at 9:58 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> 
> On Thu, May 9, 2019 at 9:30 PM Jiri Kosina <jikos@kernel.org> wrote:
>> 
>> On Fri, 26 Apr 2019, Kai-Heng Feng wrote:
>> 
>>>> Ronald (Cc-ed) raised quite a good point:
>>>> what's the benefit of removing the error message if this function (and
>>>> __extract) can only report an unsigned 32 bits value?
>>> 
>>> I didn’t spot this, sorry.
>>> 
>>>> 
>>>> My take is we should revert 94a9992f7dbdfb28976b upstream and think at
>>>> a better solution.
>>> 
>>> I think using a new fix to replace it will be a better approach, as it at
>>> least partially solves the issue.
>> 
>> Guys, did this fall in between cracks? Is anyone planning to send a fixup?
>> 
> 
> Kai-Heng, have you been able to work on that?

Sorry, I haven’t been able to work on this.

Please revert the commit and possibly use *_once() macro to reduce the noise.

Kai-Heng

> 
> Cheers,
> Benjamin

^ permalink raw reply

* Re: [PATCH] HID: Increase maximum report size allowed by hid_field_extract()
From: Benjamin Tissoires @ 2019-05-21 13:58 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Kai-Heng Feng, open list:HID CORE LAYER, lkml,
	Ronald Tschalär
In-Reply-To: <nycvar.YFH.7.76.1905092130010.17054@cbobk.fhfr.pm>

On Thu, May 9, 2019 at 9:30 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Fri, 26 Apr 2019, Kai-Heng Feng wrote:
>
> > >Ronald (Cc-ed) raised quite a good point:
> > >what's the benefit of removing the error message if this function (and
> > >__extract) can only report an unsigned 32 bits value?
> >
> > I didn’t spot this, sorry.
> >
> > >
> > >My take is we should revert 94a9992f7dbdfb28976b upstream and think at
> > >a better solution.
> >
> > I think using a new fix to replace it will be a better approach, as it at
> > least partially solves the issue.
>
> Guys, did this fall in between cracks? Is anyone planning to send a fixup?
>

Kai-Heng, have you been able to work on that?

Cheers,
Benjamin

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox