diff mbox

[2/2] HID: i2c-hid: Silently fail probe for CHPN0001 touchscreen

Message ID 20180403131634.13187-2-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede April 3, 2018, 1:16 p.m. UTC
The CHPN0001 ACPI device has a _CID of PNP0C50 and even has the _DSM to
get the HID descriptor address, but it is not a HID device at all.

It uses its own protocol which is handled by the (still being upstreamed)
chipone_icn8505 driver. I guess the _CID and the _DSM are the result of
a copy and paste job when the vendor was building the ACPI tables.

Before this patch the i2c_hid_driver's probe function will fail with a
"hid_descr_cmd failed" error.

This commit makes the i2c_hid_driver's probe function instead silently
ignored devices with an ACPI id of CHPN0001.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/i2c-hid/i2c-hid.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Sasha Levin April 6, 2018, 8:05 p.m. UTC | #1
Hi,

[This is an automated email]

This commit has been processed by the -stable helper bot and determined
to be a high probability candidate for -stable trees. (score: 20.8863)

The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.

v4.16: Build OK!
v4.15.15: Build OK!
v4.14.32: Build OK!
v4.9.92: Failed to apply! Possible dependencies:
    94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")

v4.4.126: Failed to apply! Possible dependencies:
    94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")


Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks,
Sasha--
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
Hans de Goede April 7, 2018, 9:42 a.m. UTC | #2
Hi,

On 06-04-18 22:05, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed by the -stable helper bot and determined
> to be a high probability candidate for -stable trees. (score: 20.8863)
> 
> The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.
> 
> v4.16: Build OK!
> v4.15.15: Build OK!
> v4.14.32: Build OK!
> v4.9.92: Failed to apply! Possible dependencies:
>      94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")
> 
> v4.4.126: Failed to apply! Possible dependencies:
>      94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")

Cool, first time I see this bot in action, nice work.

> Please let us know if you'd like to have this patch included in a stable tree.

So FWIW this commit is NOT stable material (it cannot hurt, but it is not
necessary). This commit really only makes sense together with a new touchscreen
driver which does actually know how to handle the CHPN0001 touchscreen.

For future mails like this, do I understand the mail correctly that if I
do NOT want a patch picked up by the -stable helper bot to go to stable
I do not have to do anything?

Regards,

Hans
--
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
Sasha Levin April 9, 2018, 5:07 p.m. UTC | #3
On Sat, Apr 07, 2018 at 11:42:03AM +0200, Hans de Goede wrote:
>Hi,
>
>On 06-04-18 22:05, Sasha Levin wrote:
>>Hi,
>>
>>[This is an automated email]
>>
>>This commit has been processed by the -stable helper bot and determined
>>to be a high probability candidate for -stable trees. (score: 20.8863)
>>
>>The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.
>>
>>v4.16: Build OK!
>>v4.15.15: Build OK!
>>v4.14.32: Build OK!
>>v4.9.92: Failed to apply! Possible dependencies:
>>     94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")
>>
>>v4.4.126: Failed to apply! Possible dependencies:
>>     94116f8126de ("ACPI: Switch to use generic guid_t in acpi_evaluate_dsm()")
>
>Cool, first time I see this bot in action, nice work.

I'll leave it out, thanks!

>>Please let us know if you'd like to have this patch included in a stable tree.
>
>So FWIW this commit is NOT stable material (it cannot hurt, but it is not
>necessary). This commit really only makes sense together with a new touchscreen
>driver which does actually know how to handle the CHPN0001 touchscreen.
>
>For future mails like this, do I understand the mail correctly that if I
>do NOT want a patch picked up by the -stable helper bot to go to stable
>I do not have to do anything?

I would probably try and pick it up without an ack. If possible, even a
brief "no" reply if you don't want it in stable would be great. Thanks!--
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
Jiri Kosina April 12, 2018, 2:21 p.m. UTC | #4
On Tue, 3 Apr 2018, Hans de Goede wrote:

> The CHPN0001 ACPI device has a _CID of PNP0C50 and even has the _DSM to
> get the HID descriptor address, but it is not a HID device at all.
> 
> It uses its own protocol which is handled by the (still being upstreamed)
> chipone_icn8505 driver. I guess the _CID and the _DSM are the result of
> a copy and paste job when the vendor was building the ACPI tables.
> 
> Before this patch the i2c_hid_driver's probe function will fail with a
> "hid_descr_cmd failed" error.
> 
> This commit makes the i2c_hid_driver's probe function instead silently
> ignored devices with an ACPI id of CHPN0001.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Hans,

as I don't know exactly what is the timeline for chipone_icn8505 
upstreaming, I'd rather ask -- would you prefer these two patches to go 
into 4.17 still, or is queuing for 4.18 ok?

Thanks,
Hans de Goede April 12, 2018, 3:47 p.m. UTC | #5
Hi,

On 12-04-18 16:21, Jiri Kosina wrote:
> On Tue, 3 Apr 2018, Hans de Goede wrote:
> 
>> The CHPN0001 ACPI device has a _CID of PNP0C50 and even has the _DSM to
>> get the HID descriptor address, but it is not a HID device at all.
>>
>> It uses its own protocol which is handled by the (still being upstreamed)
>> chipone_icn8505 driver. I guess the _CID and the _DSM are the result of
>> a copy and paste job when the vendor was building the ACPI tables.
>>
>> Before this patch the i2c_hid_driver's probe function will fail with a
>> "hid_descr_cmd failed" error.
>>
>> This commit makes the i2c_hid_driver's probe function instead silently
>> ignored devices with an ACPI id of CHPN0001.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Hans,
> 
> as I don't know exactly what is the timeline for chipone_icn8505
> upstreaming, I'd rather ask -- would you prefer these two patches to go
> into 4.17 still, or is queuing for 4.18 ok?

Whatever is easiest for you, I guess that is queuing for 4.18 and
that is fine.

Regards,

Hans
--
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
diff mbox

Patch

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 0d210928a57e..e2e911f7b6d5 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -862,6 +862,15 @@  static int i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid)
 }
 
 #ifdef CONFIG_ACPI
+static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
+	/*
+	 * The CHPN0001 ACPI device, which is used to describe the Chipone
+	 * ICN8505 controller, has a _CID of PNP0C50 but is not HID compatible.
+	 */
+	{"CHPN0001", 0 },
+	{ },
+};
+
 static int i2c_hid_acpi_pdata(struct i2c_client *client,
 		struct i2c_hid_platform_data *pdata)
 {
@@ -878,6 +887,9 @@  static int i2c_hid_acpi_pdata(struct i2c_client *client,
 		return -ENODEV;
 	}
 
+	if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+		return -ENODEV;
+
 	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
 				      ACPI_TYPE_INTEGER);
 	if (!obj) {