[{"id":1726795,"web_url":"http://patchwork.ozlabs.org/comment/1726795/","msgid":"<20170724081928.GB18097@mail.corp.redhat.com>","list_archive_url":null,"date":"2017-07-24T08:19:28","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":66704,"url":"http://patchwork.ozlabs.org/api/people/66704/","name":"Benjamin Tissoires","email":"benjamin.tissoires@redhat.com"},"content":"On Jul 22 2017 or thereabouts, Hans de Goede wrote:\n> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n> it uses its own protocol which is handled by the chipone_icn8318 driver.\n> \n> If the i2c_hid_driver's probe functon gets called it will fail with a\n> \"hid_descr_cmd failed\" error.\n> \n> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n> device in D3 state and when the chipone_icn8318 driver then loads the\n> device is put back in D0 state, executing its PS0 ACPI method, which\n> resets the controller, causing the controller to loose its firmware\n> which was loaded by the BIOS. The chipone_icn8318 driver has a workaround\n> for this, but that requires it to be the only (or the first) driver to\n> probe the device.\n> \n> This commit adds a match callback and returns -ENODEV for i2c_client-s\n> with a CHPN0001 ACPI device id, so that the probe function never gets\n> called, fixing the controller losing its firmware.\n> \n> Signed-off-by: Hans de Goede \n> ---\n> Changes in v2:\n> -Use the new i2c_driver match callback to only filter out the CHPN0001\n> ACPI device, rather then use acpi_dev_present in probe and not\n> registering the driver at all when the system has a CHPN0001 device\n\nI like the v2 much more than the v1.\n\nReviewed-By: Benjamin Tissoires \n\nCheers,\nBenjamin\n\n> ---\n> drivers/hid/i2c-hid/i2c-hid.c | 22 ++++++++++++++++++++++\n> 1 file changed, 22 insertions(+)\n> \n> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c\n> index 046f692fd0a2..79bed9afc388 100644\n> --- a/drivers/hid/i2c-hid/i2c-hid.c\n> +++ b/drivers/hid/i2c-hid/i2c-hid.c\n> @@ -891,6 +891,26 @@ static void i2c_hid_acpi_fix_up_power(struct device *dev)\n> \t\tacpi_device_fix_up_power(adev);\n> }\n> \n> +static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {\n> +\t/*\n> +\t * The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID\n> +\t * compatible, just probing it puts the device in an unusable state due\n> +\t * to it also have ACPI power management issues.\n> +\t */\n> +\t{\"CHPN0001\", 0 },\n> +\t{ },\n> +};\n> +\n> +static int i2c_hid_match(struct i2c_client *client)\n> +{\n> +\tstruct acpi_device *adev = ACPI_COMPANION(&client->dev);\n> +\n> +\tif (adev && acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)\n> +\t\treturn -ENODEV;\n> +\n> +\treturn 0;\n> +}\n> +\n> static const struct acpi_device_id i2c_hid_acpi_match[] = {\n> \t{\"ACPI0C50\", 0 },\n> \t{\"PNP0C50\", 0 },\n> @@ -905,6 +925,7 @@ static inline int i2c_hid_acpi_pdata(struct i2c_client *client,\n> }\n> \n> static inline void i2c_hid_acpi_fix_up_power(struct device *dev) {}\n> +static int i2c_hid_match(struct i2c_client *client) { return 0; }\n> #endif\n> \n> #ifdef CONFIG_OF\n> @@ -1255,6 +1276,7 @@ static struct i2c_driver i2c_hid_driver = {\n> \t\t.of_match_table = of_match_ptr(i2c_hid_of_match),\n> \t},\n> \n> +\t.match\t\t= i2c_hid_match,\n> \t.probe\t\t= i2c_hid_probe,\n> \t.remove\t\t= i2c_hid_remove,\n> \t.shutdown\t= i2c_hid_shutdown,\n> -- \n> 2.13.0\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=pass smtp.mailfrom=benjamin.tissoires@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xGDn532Fsz9s3w\n\tfor ;\n\tMon, 24 Jul 2017 18:19:37 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752997AbdGXITf (ORCPT );\n\tMon, 24 Jul 2017 04:19:35 -0400","from mx1.redhat.com ([209.132.183.28]:44274 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752990AbdGXITf (ORCPT );\n\tMon, 24 Jul 2017 04:19:35 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id B858D61D37;\n\tMon, 24 Jul 2017 08:19:34 +0000 (UTC)","from mail.corp.redhat.com (ovpn-116-222.ams2.redhat.com\n\t[10.36.116.222])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 9C73260603;\n\tMon, 24 Jul 2017 08:19:32 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com B858D61D37","DKIM-Filter":"OpenDKIM Filter v2.11.0 mx1.redhat.com B858D61D37","Date":"Mon, 24 Jul 2017 10:19:28 +0200","From":"Benjamin Tissoires ","To":"Hans de Goede ","Cc":"Jiri Kosina , Wolfram Sang ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","Message-ID":"<20170724081928.GB18097@mail.corp.redhat.com>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170722185537.12696-2-hdegoede@redhat.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tMon, 24 Jul 2017 08:19:34 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1728435,"web_url":"http://patchwork.ozlabs.org/comment/1728435/","msgid":"","list_archive_url":null,"date":"2017-07-25T12:58:45","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":67576,"url":"http://patchwork.ozlabs.org/api/people/67576/","name":"Jiri Kosina","email":"jikos@kernel.org"},"content":"On Mon, 24 Jul 2017, Benjamin Tissoires wrote:\n\n> > The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n> > it uses its own protocol which is handled by the chipone_icn8318 driver.\n> > \n> > If the i2c_hid_driver's probe functon gets called it will fail with a\n> > \"hid_descr_cmd failed\" error.\n> > \n> > Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n> > device in D3 state and when the chipone_icn8318 driver then loads the\n> > device is put back in D0 state, executing its PS0 ACPI method, which\n> > resets the controller, causing the controller to loose its firmware\n> > which was loaded by the BIOS. The chipone_icn8318 driver has a workaround\n> > for this, but that requires it to be the only (or the first) driver to\n> > probe the device.\n> > \n> > This commit adds a match callback and returns -ENODEV for i2c_client-s\n> > with a CHPN0001 ACPI device id, so that the probe function never gets\n> > called, fixing the controller losing its firmware.\n> > \n> > Signed-off-by: Hans de Goede \n> > ---\n> > Changes in v2:\n> > -Use the new i2c_driver match callback to only filter out the CHPN0001\n> > ACPI device, rather then use acpi_dev_present in probe and not\n> > registering the driver at all when the system has a CHPN0001 device\n> \n> I like the v2 much more than the v1.\n> \n> Reviewed-By: Benjamin Tissoires \n\nWolfram,\n\nwhat do we do with this patchset? I think it makes sense for both patches \nto go in via one tree.\n\nEither I can take it through hid.git with your Ack for the first patch, or \nvice versa. Please let me know what you'd prefer.\n\nThanks,","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xGywr1pZWz9rxm\n\tfor ;\n\tTue, 25 Jul 2017 22:58:52 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751103AbdGYM6u (ORCPT );\n\tTue, 25 Jul 2017 08:58:50 -0400","from mx2.suse.de ([195.135.220.15]:52122 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1750861AbdGYM6t (ORCPT );\n\tTue, 25 Jul 2017 08:58:49 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 00BADAE6E;\n\tTue, 25 Jul 2017 12:58:48 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 25 Jul 2017 14:58:45 +0200 (CEST)","From":"Jiri Kosina ","X-X-Sender":"jkosina@pobox.suse.cz","To":"Benjamin Tissoires ,\n\tWolfram Sang ","cc":"Hans de Goede , linux-input@vger.kernel.org,\n\tlinux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","In-Reply-To":"<20170724081928.GB18097@mail.corp.redhat.com>","Message-ID":"","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170724081928.GB18097@mail.corp.redhat.com>","User-Agent":"Alpine 2.20 (LSU 67 2015-01-07)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1728495,"web_url":"http://patchwork.ozlabs.org/comment/1728495/","msgid":"<20170725134658.GB5777@katana>","list_archive_url":null,"date":"2017-07-25T13:46:58","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":22495,"url":"http://patchwork.ozlabs.org/api/people/22495/","name":"Wolfram Sang","email":"wsa@the-dreams.de"},"content":"> what do we do with this patchset? I think it makes sense for both patches \n> to go in via one tree.\n\nYes.\n\n> Either I can take it through hid.git with your Ack for the first patch, or \n> vice versa. Please let me know what you'd prefer.\n\nI'd like use the i2c tree for it. The changes to i2c core are an API\naddition while the changes to the HID driver are basically boilerplate.\n\nMakes sense?","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xH00Q6gMtz9s7F\n\tfor ;\n\tTue, 25 Jul 2017 23:47:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752118AbdGYNrB (ORCPT );\n\tTue, 25 Jul 2017 09:47:01 -0400","from www.zeus03.de ([194.117.254.33]:39132 \"EHLO mail.zeus03.de\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751843AbdGYNrA (ORCPT );\n\tTue, 25 Jul 2017 09:47:00 -0400","(qmail 10159 invoked from network); 25 Jul 2017 15:46:59 +0200","from p200300cf5bc73400021de0fffea0c865.dip0.t-ipconnect.de (HELO\n\tlocalhost) (l3s3148p1@2003:cf:5bc7:3400:21d:e0ff:fea0:c865)\n\tby mail.zeus03.de with ESMTPSA (ECDHE-RSA-AES256-GCM-SHA384\n\tencrypted, authenticated); 25 Jul 2017 15:46:59 +0200"],"Date":"Tue, 25 Jul 2017 15:46:58 +0200","From":"Wolfram Sang ","To":"Jiri Kosina ","Cc":"Benjamin Tissoires ,\n\tHans de Goede ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","Message-ID":"<20170725134658.GB5777@katana>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170724081928.GB18097@mail.corp.redhat.com>\n\t","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"QKdGvSO+nmPlgiQ/\"","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.24 (2015-08-30)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1728517,"web_url":"http://patchwork.ozlabs.org/comment/1728517/","msgid":"","list_archive_url":null,"date":"2017-07-25T13:58:46","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":67576,"url":"http://patchwork.ozlabs.org/api/people/67576/","name":"Jiri Kosina","email":"jikos@kernel.org"},"content":"On Tue, 25 Jul 2017, Wolfram Sang wrote:\n\n> > Either I can take it through hid.git with your Ack for the first patch, or \n> > vice versa. Please let me know what you'd prefer.\n> \n> I'd like use the i2c tree for it. The changes to i2c core are an API\n> addition while the changes to the HID driver are basically boilerplate.\n> \n> Makes sense?\n\nAbsolutely.\n\nPlease feel free to add\n\n\tReviewed-by: Jiri Kosina \n\nto patch 2/2.\n\nThanks,","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xH0G32qmHz9ryk\n\tfor ;\n\tTue, 25 Jul 2017 23:58:51 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752029AbdGYN6t (ORCPT );\n\tTue, 25 Jul 2017 09:58:49 -0400","from mx2.suse.de ([195.135.220.15]:57971 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1751940AbdGYN6t (ORCPT );\n\tTue, 25 Jul 2017 09:58:49 -0400","from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id C63D3AC10;\n\tTue, 25 Jul 2017 13:58:47 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 25 Jul 2017 15:58:46 +0200 (CEST)","From":"Jiri Kosina ","X-X-Sender":"jkosina@pobox.suse.cz","To":"Wolfram Sang ","cc":"Benjamin Tissoires ,\n\tHans de Goede ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","In-Reply-To":"<20170725134658.GB5777@katana>","Message-ID":"","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170724081928.GB18097@mail.corp.redhat.com>\n\t\n\t<20170725134658.GB5777@katana>","User-Agent":"Alpine 2.20 (LSU 67 2015-01-07)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1750303,"web_url":"http://patchwork.ozlabs.org/comment/1750303/","msgid":"<20170817193912.hccxyjwstrtr5oiv@ninjato>","list_archive_url":null,"date":"2017-08-17T19:39:12","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":22495,"url":"http://patchwork.ozlabs.org/api/people/22495/","name":"Wolfram Sang","email":"wsa@the-dreams.de"},"content":"Hey guys,\n\nSorry, I don't understand some of the stuff here. But I'd like to\nunderstand it before I add something to the I2C core. Especially as it\nfeels a bit a the edge of the driver model to me.\n\nOn Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n> it uses its own protocol which is handled by the chipone_icn8318 driver.\n> \n> If the i2c_hid_driver's probe functon gets called it will fail with a\n> \"hid_descr_cmd failed\" error.\n\nThat sounds like it fails pretty late. I'd assume we could check the\nblacklist right at the beginning of probe and bail out immediately?\n\n> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n> device in D3 state\n\nWhere does that happen? Sorry, I can't find it. Would it be an idea to\nadd a flag somewhere telling the device should not be put into D3? That\nwould be way more generic in case this happens outside I2C world, or?\nDisclaimer: I am brainstorming here, don't know super much about ACPI.\n\n> This commit adds a match callback and returns -ENODEV for i2c_client-s\n> with a CHPN0001 ACPI device id, so that the probe function never gets\n> called, fixing the controller losing its firmware.\n\nDo you know if something like a match-callback exists somewhere else in\nthe kernel?\n\nRegards,\n\n Wolfram","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xYGkD4Mr7z9s8P\n\tfor ;\n\tFri, 18 Aug 2017 05:39:16 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753212AbdHQTjP (ORCPT );\n\tThu, 17 Aug 2017 15:39:15 -0400","from sauhun.de ([88.99.104.3]:60920 \"EHLO pokefinder.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752632AbdHQTjO (ORCPT );\n\tThu, 17 Aug 2017 15:39:14 -0400","from localhost (p54B3365B.dip0.t-ipconnect.de [84.179.54.91])\n\tby pokefinder.org (Postfix) with ESMTPSA id 0E55E2C0058;\n\tThu, 17 Aug 2017 21:39:13 +0200 (CEST)"],"Date":"Thu, 17 Aug 2017 21:39:12 +0200","From":"Wolfram Sang ","To":"Hans de Goede ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","Message-ID":"<20170817193912.hccxyjwstrtr5oiv@ninjato>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"tzsrhrz7gahwvwmv\"","Content-Disposition":"inline","In-Reply-To":"<20170722185537.12696-2-hdegoede@redhat.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1750420,"web_url":"http://patchwork.ozlabs.org/comment/1750420/","msgid":"","list_archive_url":null,"date":"2017-08-17T22:15:24","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":695,"url":"http://patchwork.ozlabs.org/api/people/695/","name":"Dmitry Torokhov","email":"dmitry.torokhov@gmail.com"},"content":"On Thu, Aug 17, 2017 at 12:39 PM, Wolfram Sang wrote:\n> Hey guys,\n>\n> Sorry, I don't understand some of the stuff here. But I'd like to\n> understand it before I add something to the I2C core. Especially as it\n> feels a bit a the edge of the driver model to me.\n>\n> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>\n>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>> \"hid_descr_cmd failed\" error.\n>\n> That sounds like it fails pretty late. I'd assume we could check the\n> blacklist right at the beginning of probe and bail out immediately?\n>\n>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>> device in D3 state\n\nSo I guess this will also happen if I unbind/rebind chipone_icn8318 driver?\n\n>\n> Where does that happen? Sorry, I can't find it. Would it be an idea to\n> add a flag somewhere telling the device should not be put into D3? That\n> would be way more generic in case this happens outside I2C world, or?\n> Disclaimer: I am brainstorming here, don't know super much about ACPI.\n>\n>> This commit adds a match callback and returns -ENODEV for i2c_client-s\n>> with a CHPN0001 ACPI device id, so that the probe function never gets\n>> called, fixing the controller losing its firmware.\n>\n> Do you know if something like a match-callback exists somewhere else in\n> the kernel?\n\nHaving blacklist means that we are failing at design somewhere...\n\nIn this case what we have is wrong ACPI table. Instead of adding hooks\nto i2c core can we fix it (DSDT) up? I know people do not like it, but\nI'd say this is task for yet another platform driver to adjust ACPI\nproperties (kill the wrong _CID, disable PS0) before instantiating the\ndevice.\n\nThanks.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"kAb0a0Yd\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xYLBT18HZz9t2W\n\tfor ;\n\tFri, 18 Aug 2017 08:15:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753885AbdHQWP1 (ORCPT );\n\tThu, 17 Aug 2017 18:15:27 -0400","from mail-vk0-f68.google.com ([209.85.213.68]:37176 \"EHLO\n\tmail-vk0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753860AbdHQWP0 (ORCPT\n\t); Thu, 17 Aug 2017 18:15:26 -0400","by mail-vk0-f68.google.com with SMTP id r199so3341730vke.4;\n\tThu, 17 Aug 2017 15:15:26 -0700 (PDT)","by 10.176.19.242 with HTTP; Thu, 17 Aug 2017 15:15:24 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=3hFXxZHplrn5X20hrv1/nUSt2pke08FbaL/CDIUojF8=;\n\tb=kAb0a0Ydbsa2a+gaqUz8WCkJx5M0Olqy21safhcwceuBkd3H2PrRYKAVCfZs7wz7Gy\n\twUk0ispZ/miMpxonzHA61Tr/cVrKAbW0BMQ2OtdxikP5DgCmc5ldAfjZEX3OmMJOBECS\n\tJiQuy1mQ6HngABF2CXoTnjf7BswZGfqVfkFPHkgbkYZfBvOVR45Ajp5xBp/D2LcKWrgQ\n\tfvMmlWF96Bfr9N9KqM9oVTuCzNIgOiXvg+4OKSG9l+i8PnN8yM0ZTG0mqOTcpn7IlXgj\n\t6ICHVKNnl6g0v3dQQzZYwfENgrpyjvkfRE1Oa4WtZhzFgpUd+K5TcR+w83/MHJYfuTdG\n\tSEIA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=3hFXxZHplrn5X20hrv1/nUSt2pke08FbaL/CDIUojF8=;\n\tb=PpiuJPKM/70cnwgEsUaiGc7YJyF8aCiUEG15hhj5lGm8Vk/wniV07i7P1VpsyKWYA/\n\tD+Ji/hj6EQ/PssCdt1ojUW79pqdYXu1yS9SkMpKMfNQjAo08BhSlHOX1e3a0nKZ9+YsR\n\t43+6lyMm2p7swXkF5LhOvWl+5m5mOvltl2SBYRnnQGZgvZrgdGZiyk3vz8kfgSKqylIQ\n\tHJE0+7UyZy1Xs+yBPAjJREoo/5rfh+iZ0iHXMNHylhGFtiYk/HGaW7exUHT0JM5rsdL4\n\tm+IPQgNEp0/jymmoJuC0Ip1RPJ285OrMx2brcMi1FhzFldB5dSqTIWgk676q0r0IELYV\n\tDA4w==","X-Gm-Message-State":"AHYfb5jTjpTv/hExdG8qRM9oTSOpHUZ1uxAPA0yEm6HkfPcx2yYAB0Kq\n\tzeQ16JJc66p0drzbvjP1H2yLipBneWo2AI8=","X-Received":"by 10.31.214.131 with SMTP id n125mr4664262vkg.198.1503008125430;\n\tThu, 17 Aug 2017 15:15:25 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170817193912.hccxyjwstrtr5oiv@ninjato>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>","From":"Dmitry Torokhov ","Date":"Thu, 17 Aug 2017 15:15:24 -0700","Message-ID":"","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","To":"Wolfram Sang ","Cc":"Hans de Goede , Jiri Kosina ,\n\tBenjamin Tissoires ,\n\t\"linux-input@vger.kernel.org\" ,\n\tLinux I2C ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1758556,"web_url":"http://patchwork.ozlabs.org/comment/1758556/","msgid":"","list_archive_url":null,"date":"2017-08-28T12:44:30","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 17-08-17 21:39, Wolfram Sang wrote:\n> Hey guys,\n> \n> Sorry, I don't understand some of the stuff here. But I'd like to\n> understand it before I add something to the I2C core. Especially as it\n> feels a bit a the edge of the driver model to me.\n> \n> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>\n>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>> \"hid_descr_cmd failed\" error.\n> \n> That sounds like it fails pretty late. I'd assume we could check the\n> blacklist right at the beginning of probe and bail out immediately?\n\nThe problem is that calling probe (and then failing) leads to the device-core\npowering up and then powering down (put in D0 / D3 state) the device, after\nwhich the touchscreen controller has lost its firmware.\n\nSo the trick is to get the device-core to never call i2c_bus_type.probe\nfor i2c-hid at all, which means that i2c_bus_type.match must return false\nin this case.\n\n>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>> device in D3 state\n> \n> Where does that happen? Sorry, I can't find it. Would it be an idea to\n> add a flag somewhere telling the device should not be put into D3? That\n> would be way more generic in case this happens outside I2C world, or?\n> Disclaimer: I am brainstorming here, don't know super much about ACPI.\n\ni2c_device_probe() calls dev_pm_domain_attach(&client->dev, true)\nwhich calls acpi_dev_pm_attach(dev, true) which then does a\nacpi_dev_pm_full_power(adev) moving the device to D0 (if it was not\nin D0 already).\n\nWhen the probe fails i2c_device_probe() then does\ndev_pm_domain_detach(&client->dev, true); which calls\nacpi_dev_pm_detach(dev, true) which does acpi_dev_pm_low_power(...)\nand now the device is in D3.\n\n>> This commit adds a match callback and returns -ENODEV for i2c_client-s\n>> with a CHPN0001 ACPI device id, so that the probe function never gets\n>> called, fixing the controller losing its firmware.\n> \n> Do you know if something like a match-callback exists somewhere else in\n> the kernel?\n\nNo AFAIK there is no driver level match callback (only bus level) at\nother places in the kernel.\n\nRegards,\n\nHans","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgs0k0nGWz9sN5\n\tfor ;\n\tMon, 28 Aug 2017 22:44:38 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751227AbdH1Mog (ORCPT );\n\tMon, 28 Aug 2017 08:44:36 -0400","from mx1.redhat.com ([209.132.183.28]:33398 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751171AbdH1Mof (ORCPT );\n\tMon, 28 Aug 2017 08:44:35 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id DD9DE52779;\n\tMon, 28 Aug 2017 12:44:34 +0000 (UTC)","from shalem.localdomain (ovpn-117-211.ams2.redhat.com\n\t[10.36.117.211])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id BC5865C899;\n\tMon, 28 Aug 2017 12:44:31 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DD9DE52779","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","To":"Wolfram Sang ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>","From":"Hans de Goede ","Message-ID":"","Date":"Mon, 28 Aug 2017 14:44:30 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170817193912.hccxyjwstrtr5oiv@ninjato>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tMon, 28 Aug 2017 12:44:35 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1758561,"web_url":"http://patchwork.ozlabs.org/comment/1758561/","msgid":"<0c72f919-eab2-2f3f-a760-1aacc25d2550@redhat.com>","list_archive_url":null,"date":"2017-08-28T12:50:49","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi again,\n\nI realized I did not answer 1 of your questions:\n\nOn 17-08-17 21:39, Wolfram Sang wrote:\n> Hey guys,\n> \n> Sorry, I don't understand some of the stuff here. But I'd like to\n> understand it before I add something to the I2C core. Especially as it\n> feels a bit a the edge of the driver model to me.\n> \n> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>\n>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>> \"hid_descr_cmd failed\" error.\n> \n> That sounds like it fails pretty late. I'd assume we could check the\n> blacklist right at the beginning of probe and bail out immediately?\n> \n>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>> device in D3 state\n> \n> Where does that happen? Sorry, I can't find it. Would it be an idea to\n> add a flag somewhere telling the device should not be put into D3?\n\nIt is already possible to do this and my patches for the icn8318 driver\ndo this:\n\n\tstruct acpi_device *adev;\n\n\tadev = ACPI_COMPANION(dev);\n\n\t/*\n\t * Disable ACPI power management the _PS3 method is empty, so\n\t * there is no powersaving when using ACPI power management.\n\t * The _PS0 method resets the controller causing it to loose its\n\t * firmware, which has been loaded by the BIOS and we do not\n\t * know how to restore the firmware.\n\t */\n\tadev->flags.power_manageable = 0;\n\nThe problem is that this happens in the probe() from the icn8318 driver\nand if the i2c-hid drivers probe() executes first we end up in the\ndev_pm_domain_detach() path of i2c_device_probe() and after that the\ntouchscreen-controller no longer works (*), iow after that it is too late\nto disable acpi pm for the device.\n\n*) Unless we find a way to reload the firmware, which technically is\ndoable, but then we get into the problem of now having to distribute the\nfirmware in linux-firmware\n\nRegards,\n\nHans","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgs802y3pz9sDB\n\tfor ;\n\tMon, 28 Aug 2017 22:50:56 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751225AbdH1Muy (ORCPT );\n\tMon, 28 Aug 2017 08:50:54 -0400","from mx1.redhat.com ([209.132.183.28]:53862 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751161AbdH1Muy (ORCPT );\n\tMon, 28 Aug 2017 08:50:54 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E51367E43A;\n\tMon, 28 Aug 2017 12:50:53 +0000 (UTC)","from shalem.localdomain (ovpn-117-211.ams2.redhat.com\n\t[10.36.117.211])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 2414B5C899;\n\tMon, 28 Aug 2017 12:50:50 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E51367E43A","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","To":"Wolfram Sang ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>","From":"Hans de Goede ","Message-ID":"<0c72f919-eab2-2f3f-a760-1aacc25d2550@redhat.com>","Date":"Mon, 28 Aug 2017 14:50:49 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170817193912.hccxyjwstrtr5oiv@ninjato>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tMon, 28 Aug 2017 12:50:54 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1758572,"web_url":"http://patchwork.ozlabs.org/comment/1758572/","msgid":"<5093164f-72a5-1217-a2b5-52e0a6d9a9d1@redhat.com>","list_archive_url":null,"date":"2017-08-28T13:04:04","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 18-08-17 00:15, Dmitry Torokhov wrote:\n> On Thu, Aug 17, 2017 at 12:39 PM, Wolfram Sang wrote:\n>> Hey guys,\n>>\n>> Sorry, I don't understand some of the stuff here. But I'd like to\n>> understand it before I add something to the I2C core. Especially as it\n>> feels a bit a the edge of the driver model to me.\n>>\n>> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>>\n>>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>>> \"hid_descr_cmd failed\" error.\n>>\n>> That sounds like it fails pretty late. I'd assume we could check the\n>> blacklist right at the beginning of probe and bail out immediately?\n>>\n>>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>>> device in D3 state\n> \n> So I guess this will also happen if I unbind/rebind chipone_icn8318 driver?\n\nNo because on probe the chipone_icn8318 driver does (with the patches\nI've pending) :\n\n struct acpi_device *adev;\n\n adev = ACPI_COMPANION(dev);\n\n /*\n * Disable ACPI power management the _PS3 method is empty, so\n * there is no powersaving when using ACPI power management.\n * The _PS0 method resets the controller causing it to loose its\n * firmware, which has been loaded by the BIOS and we do not\n * know how to restore the firmware.\n */\n adev->flags.power_manageable = 0;\n\nSo it disables ACPI pm for the device, keeping the controller in D0 so that\nit will never loose its firmware.\n\n\n>> Where does that happen? Sorry, I can't find it. Would it be an idea to\n>> add a flag somewhere telling the device should not be put into D3? That\n>> would be way more generic in case this happens outside I2C world, or?\n>> Disclaimer: I am brainstorming here, don't know super much about ACPI.\n>>\n>>> This commit adds a match callback and returns -ENODEV for i2c_client-s\n>>> with a CHPN0001 ACPI device id, so that the probe function never gets\n>>> called, fixing the controller losing its firmware.\n>>\n>> Do you know if something like a match-callback exists somewhere else in\n>> the kernel?\n> \n> Having blacklist means that we are failing at design somewhere...\n> \n> In this case what we have is wrong ACPI table. Instead of adding hooks\n> to i2c core can we fix it (DSDT) up? I know people do not like it, but\n> I'd say this is task for yet another platform driver to adjust ACPI\n> properties (kill the wrong _CID, disable PS0) before instantiating the\n> device.\n\nI don't think that is going to fly. Certainly DSDT patching is out of the\nquestion, we could modify the acpi_dev after enumeration, but that is not\ngoing to be pretty. This is going to be drivers/platform/x86/silead_dmi.c\nall over again, but where that made sense as to decouple the board-info\nfrom the driver this case is different as we are not talking board\nspecific behavior here, but device specific so this really belongs in the\ndrivers IMHO, also the reception of silead_dmi.c has not been 100% positive.\n\nI don't think the blacklist entry in i2c-hid is a problem per se, we've\nsimilar issues with USB devices where they claim a generic class but need\na specific driver and we typically solve that with a blacklist for the\nspecific vid:pid in the class driver.\n\nThe nastiness here is not the blacklist, but the pm issues when the wrong\ndriver gets its probe() method called first.\n\nRegards,\n\nHans","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgsRH4rjXz9sN5\n\tfor ;\n\tMon, 28 Aug 2017 23:04:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751190AbdH1NEK (ORCPT );\n\tMon, 28 Aug 2017 09:04:10 -0400","from mx1.redhat.com ([209.132.183.28]:51300 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751171AbdH1NEJ (ORCPT );\n\tMon, 28 Aug 2017 09:04:09 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3D3541F561;\n\tMon, 28 Aug 2017 13:04:09 +0000 (UTC)","from shalem.localdomain (ovpn-117-211.ams2.redhat.com\n\t[10.36.117.211])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 850AC80F65;\n\tMon, 28 Aug 2017 13:04:05 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3D3541F561","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","To":"Dmitry Torokhov ,\n\tWolfram Sang ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\t\"linux-input@vger.kernel.org\" ,\n\tLinux I2C ","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>\n\t","From":"Hans de Goede ","Message-ID":"<5093164f-72a5-1217-a2b5-52e0a6d9a9d1@redhat.com>","Date":"Mon, 28 Aug 2017 15:04:04 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tMon, 28 Aug 2017 13:04:09 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1758699,"web_url":"http://patchwork.ozlabs.org/comment/1758699/","msgid":"<20170828163131.GA12195@dtor-ws>","list_archive_url":null,"date":"2017-08-28T16:31:31","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":695,"url":"http://patchwork.ozlabs.org/api/people/695/","name":"Dmitry Torokhov","email":"dmitry.torokhov@gmail.com"},"content":"Hi Hans,\n\nOn Mon, Aug 28, 2017 at 03:04:04PM +0200, Hans de Goede wrote:\n> Hi,\n> \n> On 18-08-17 00:15, Dmitry Torokhov wrote:\n> >On Thu, Aug 17, 2017 at 12:39 PM, Wolfram Sang wrote:\n> >>Hey guys,\n> >>\n> >>Sorry, I don't understand some of the stuff here. But I'd like to\n> >>understand it before I add something to the I2C core. Especially as it\n> >>feels a bit a the edge of the driver model to me.\n> >>\n> >>On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n> >>>The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n> >>>it uses its own protocol which is handled by the chipone_icn8318 driver.\n> >>>\n> >>>If the i2c_hid_driver's probe functon gets called it will fail with a\n> >>>\"hid_descr_cmd failed\" error.\n> >>\n> >>That sounds like it fails pretty late. I'd assume we could check the\n> >>blacklist right at the beginning of probe and bail out immediately?\n> >>\n> >>>Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n> >>>device in D3 state\n> >\n> >So I guess this will also happen if I unbind/rebind chipone_icn8318 driver?\n> \n> No because on probe the chipone_icn8318 driver does (with the patches\n> I've pending) :\n> \n> struct acpi_device *adev;\n> \n> adev = ACPI_COMPANION(dev);\n> \n> /*\n> * Disable ACPI power management the _PS3 method is empty, so\n> * there is no powersaving when using ACPI power management.\n> * The _PS0 method resets the controller causing it to loose its\n> * firmware, which has been loaded by the BIOS and we do not\n> * know how to restore the firmware.\n> */\n> adev->flags.power_manageable = 0;\n> \n> So it disables ACPI pm for the device, keeping the controller in D0 so that\n> it will never loose its firmware.\n> \n> \n> >>Where does that happen? Sorry, I can't find it. Would it be an idea to\n> >>add a flag somewhere telling the device should not be put into D3? That\n> >>would be way more generic in case this happens outside I2C world, or?\n> >>Disclaimer: I am brainstorming here, don't know super much about ACPI.\n> >>\n> >>>This commit adds a match callback and returns -ENODEV for i2c_client-s\n> >>>with a CHPN0001 ACPI device id, so that the probe function never gets\n> >>>called, fixing the controller losing its firmware.\n> >>\n> >>Do you know if something like a match-callback exists somewhere else in\n> >>the kernel?\n> >\n> >Having blacklist means that we are failing at design somewhere...\n> >\n> >In this case what we have is wrong ACPI table. Instead of adding hooks\n> >to i2c core can we fix it (DSDT) up? I know people do not like it, but\n> >I'd say this is task for yet another platform driver to adjust ACPI\n> >properties (kill the wrong _CID, disable PS0) before instantiating the\n> >device.\n> \n> I don't think that is going to fly. Certainly DSDT patching is out of the\n> question, we could modify the acpi_dev after enumeration, but that is not\n> going to be pretty. This is going to be drivers/platform/x86/silead_dmi.c\n> all over again, but where that made sense as to decouple the board-info\n> from the driver this case is different as we are not talking board\n> specific behavior here, but device specific so this really belongs in the\n> drivers IMHO, also the reception of silead_dmi.c has not been 100% positive.\n\nNo, this is definitely board-specific as well. This is a brain dead\nfirmware on a particular board that declares PS0 and wrong CID. The\ndevice itself can be made working perfectly well in another box.\n\nI think I also mentioned before that encoding particular platform\nbehavior in the device driver is not the best option.\n\nI understand that nobody likes silead_dmi as it is nasty (as it shoudl\nbe, it fixes missing/wrong data suplied by the platform), but I think\nthe main objection is that we had to make it built-in. I wonder if we\ncould work on making it usable as a module, by having driver core try\nloading \"-quirk\" module before doing probes so that quirk\nis guaranteed to be available?\n\n> \n> I don't think the blacklist entry in i2c-hid is a problem per se, we've\n> similar issues with USB devices where they claim a generic class but need\n> a specific driver and we typically solve that with a blacklist for the\n> specific vid:pid in the class driver.\n\nYes, you do that for a particular _device_. You move this device to a\nbrand new box and you still need the same quirk. Here it is the box that\nis deficient.\n\n> \n> The nastiness here is not the blacklist, but the pm issues when the wrong\n> driver gets its probe() method called first.\n\nBTW, how do we order probing? Should we try matching on _HID before\ntrying _CID?\n\nThanks.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"dS0Qd15x\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgy2g23bDz9sNr\n\tfor ;\n\tTue, 29 Aug 2017 02:31:39 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751192AbdH1Qbh (ORCPT );\n\tMon, 28 Aug 2017 12:31:37 -0400","from mail-pg0-f43.google.com ([74.125.83.43]:32901 \"EHLO\n\tmail-pg0-f43.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751172AbdH1Qbg (ORCPT\n\t); Mon, 28 Aug 2017 12:31:36 -0400","by mail-pg0-f43.google.com with SMTP id t3so2932486pgt.0;\n\tMon, 28 Aug 2017 09:31:35 -0700 (PDT)","from dtor-ws ([2620:0:1000:1611:c84c:5172:bda1:9f7a])\n\tby smtp.gmail.com with ESMTPSA id\n\to64sm1826274pfk.84.2017.08.28.09.31.33\n\t(version=TLS1_2 cipher=AES128-SHA bits=128/128);\n\tMon, 28 Aug 2017 09:31:34 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=D4F/ouDz6LrSCJbUv2cGn48eZ3hZO957moKi9m2O/lA=;\n\tb=dS0Qd15xPTfxyk8Y+AMx3x7eZ+kUc5xOgfPPCsiCSJYdsvhEFlc44uE+MDI2+Q10FK\n\tcBy5ZZcMSMGaPVenlAZrzHLmaeOSpZAianIatjtXnlYUXYILuw93eifKTsRFxXYAEws9\n\tsVFWbRyUHVRHDPwb07At5BurZng9z8+z4F6vGorSuCS8n8JRmu/rn2vYeIoEm0BOB1aI\n\t2RZCe5iywvpKKdMtMQZYxzURSOhr9iV0yUcjukNu6pHg83t9Jd1Dhg8UKUq5XTyd9wgW\n\tcN6jh5qwdmJMXnIWYnSL3sdBJ8/tgDkyw4kn10O4R/jxrnTE8XB8B7cQrnrSs88KaObY\n\t1Wfw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=D4F/ouDz6LrSCJbUv2cGn48eZ3hZO957moKi9m2O/lA=;\n\tb=ILZAhIZ0ckOQ9FNqcAwZFr0OeLHADpUYa7NQeCVqByLKJGDjeFY0aWrPEhWns/2A54\n\tzOs88KSgIyg+dWE1hGONRCBQGzr+UvK8tEkn97nG9DGYnQ0CGzAqbrjAtHhlZRIzb3S4\n\t0bDmk2q8D2BrvoNKJ7tYB9+07qr9hLV0NGvcDC4CJfuixAM0PLEQr7Xnfx/KHRH6y8QY\n\tV3sbCaocoGtjxVh55bl5BhvGPJF1utyWFqr4ELT98WTFZcPNCVwUGqdqUlp2cUus4Kn1\n\tYRfIOFgEjqsWwYsFlL6V/mvj4zr/svTyIOyhPnc43uH60Jme7KO5A6FwBtoh6BPqJniM\n\tGglw==","X-Gm-Message-State":"AHYfb5hZ9YyMLeyc+JiKL4Ke0Vb8ZS4wBXkeGqPFX3norADAJ0WpMoLx\n\tuyZa+prq1TvllZsx11k=","X-Received":"by 10.98.200.90 with SMTP id z87mr1036541pff.211.1503937894982; \n\tMon, 28 Aug 2017 09:31:34 -0700 (PDT)","Date":"Mon, 28 Aug 2017 09:31:31 -0700","From":"Dmitry Torokhov ","To":"Hans de Goede ","Cc":"Wolfram Sang , Jiri Kosina ,\n\tBenjamin Tissoires ,\n\t\"linux-input@vger.kernel.org\" ,\n\tLinux I2C ","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","Message-ID":"<20170828163131.GA12195@dtor-ws>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>\n\t\n\t<5093164f-72a5-1217-a2b5-52e0a6d9a9d1@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<5093164f-72a5-1217-a2b5-52e0a6d9a9d1@redhat.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1758704,"web_url":"http://patchwork.ozlabs.org/comment/1758704/","msgid":"<997c5cb8-34b2-2a08-efcc-ece2b5f0fd5a@redhat.com>","list_archive_url":null,"date":"2017-08-28T16:46:01","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi Dmitry,\n\nOn 28-08-17 18:31, Dmitry Torokhov wrote:\n> Hi Hans,\n> \n> On Mon, Aug 28, 2017 at 03:04:04PM +0200, Hans de Goede wrote:\n>> Hi,\n>>\n>> On 18-08-17 00:15, Dmitry Torokhov wrote:\n>>> On Thu, Aug 17, 2017 at 12:39 PM, Wolfram Sang wrote:\n>>>> Hey guys,\n>>>>\n>>>> Sorry, I don't understand some of the stuff here. But I'd like to\n>>>> understand it before I add something to the I2C core. Especially as it\n>>>> feels a bit a the edge of the driver model to me.\n>>>>\n>>>> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>>>>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>>>>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>>>>\n>>>>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>>>>> \"hid_descr_cmd failed\" error.\n>>>>\n>>>> That sounds like it fails pretty late. I'd assume we could check the\n>>>> blacklist right at the beginning of probe and bail out immediately?\n>>>>\n>>>>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>>>>> device in D3 state\n>>>\n>>> So I guess this will also happen if I unbind/rebind chipone_icn8318 driver?\n>>\n>> No because on probe the chipone_icn8318 driver does (with the patches\n>> I've pending) :\n>>\n>> struct acpi_device *adev;\n>>\n>> adev = ACPI_COMPANION(dev);\n>>\n>> /*\n>> * Disable ACPI power management the _PS3 method is empty, so\n>> * there is no powersaving when using ACPI power management.\n>> * The _PS0 method resets the controller causing it to loose its\n>> * firmware, which has been loaded by the BIOS and we do not\n>> * know how to restore the firmware.\n>> */\n>> adev->flags.power_manageable = 0;\n>>\n>> So it disables ACPI pm for the device, keeping the controller in D0 so that\n>> it will never loose its firmware.\n>>\n>>\n>>>> Where does that happen? Sorry, I can't find it. Would it be an idea to\n>>>> add a flag somewhere telling the device should not be put into D3? That\n>>>> would be way more generic in case this happens outside I2C world, or?\n>>>> Disclaimer: I am brainstorming here, don't know super much about ACPI.\n>>>>\n>>>>> This commit adds a match callback and returns -ENODEV for i2c_client-s\n>>>>> with a CHPN0001 ACPI device id, so that the probe function never gets\n>>>>> called, fixing the controller losing its firmware.\n>>>>\n>>>> Do you know if something like a match-callback exists somewhere else in\n>>>> the kernel?\n>>>\n>>> Having blacklist means that we are failing at design somewhere...\n>>>\n>>> In this case what we have is wrong ACPI table. Instead of adding hooks\n>>> to i2c core can we fix it (DSDT) up? I know people do not like it, but\n>>> I'd say this is task for yet another platform driver to adjust ACPI\n>>> properties (kill the wrong _CID, disable PS0) before instantiating the\n>>> device.\n>>\n>> I don't think that is going to fly. Certainly DSDT patching is out of the\n>> question, we could modify the acpi_dev after enumeration, but that is not\n>> going to be pretty. This is going to be drivers/platform/x86/silead_dmi.c\n>> all over again, but where that made sense as to decouple the board-info\n>> from the driver this case is different as we are not talking board\n>> specific behavior here, but device specific so this really belongs in the\n>> drivers IMHO, also the reception of silead_dmi.c has not been 100% positive.\n> \n> No, this is definitely board-specific as well. This is a brain dead\n> firmware on a particular board that declares PS0 and wrong CID. The\n> device itself can be made working perfectly well in another box.\nI believe all x86 tablet models with this touchscreen controller have\nthe same issues, so although technically this is a board problem we\nmight just as well treat it as a device problem.\n\n> I think I also mentioned before that encoding particular platform\n> behavior in the device driver is not the best option.\n> \n> I understand that nobody likes silead_dmi as it is nasty (as it shoudl\n> be, it fixes missing/wrong data suplied by the platform), but I think\n> the main objection is that we had to make it built-in. I wonder if we\n> could work on making it usable as a module, by having driver core try\n> loading \"-quirk\" module before doing probes so that quirk\n> is guaranteed to be available?\n\nThat is tricky when using an initrd, what if the initial attempt to\nload \"-quirk\" fails because we are running from the initrd\nshould we retry ? When ? Etc.\n\n> \n>>\n>> I don't think the blacklist entry in i2c-hid is a problem per se, we've\n>> similar issues with USB devices where they claim a generic class but need\n>> a specific driver and we typically solve that with a blacklist for the\n>> specific vid:pid in the class driver.\n> \n> Yes, you do that for a particular _device_. You move this device to a\n> brand new box and you still need the same quirk. Here it is the box that\n> is deficient.\n\nSee above to the best of my knowledge all models using this touchscreen\ncontroller have the same issue.\n\n>> The nastiness here is not the blacklist, but the pm issues when the wrong\n>> driver gets its probe() method called first.\n> \n> BTW, how do we order probing? Should we try matching on _HID before\n> trying _CID?\n\nThat won't help much, udev ends up loading the modules based on a modalias\nso playing tricks with ordering is hard (and again we may have an initrd\nmaking things interesting, or have the wrong driver built in).\n\nRegards,\n\nHans","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgyMN3rBLz9sP5\n\tfor ;\n\tTue, 29 Aug 2017 02:46:08 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751203AbdH1QqG (ORCPT );\n\tMon, 28 Aug 2017 12:46:06 -0400","from mx1.redhat.com ([209.132.183.28]:54960 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751192AbdH1QqF (ORCPT );\n\tMon, 28 Aug 2017 12:46:05 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 8EDB9267C4;\n\tMon, 28 Aug 2017 16:46:05 +0000 (UTC)","from shalem.localdomain (ovpn-117-211.ams2.redhat.com\n\t[10.36.117.211])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 4B4B58231C;\n\tMon, 28 Aug 2017 16:46:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8EDB9267C4","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","To":"Dmitry Torokhov ","Cc":"Wolfram Sang , Jiri Kosina ,\n\tBenjamin Tissoires ,\n\t\"linux-input@vger.kernel.org\" ,\n\tLinux I2C ","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>\n\t\n\t<5093164f-72a5-1217-a2b5-52e0a6d9a9d1@redhat.com>\n\t<20170828163131.GA12195@dtor-ws>","From":"Hans de Goede ","Message-ID":"<997c5cb8-34b2-2a08-efcc-ece2b5f0fd5a@redhat.com>","Date":"Mon, 28 Aug 2017 18:46:01 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170828163131.GA12195@dtor-ws>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tMon, 28 Aug 2017 16:46:05 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1759132,"web_url":"http://patchwork.ozlabs.org/comment/1759132/","msgid":"","list_archive_url":null,"date":"2017-08-29T08:37:35","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":1893,"url":"http://patchwork.ozlabs.org/api/people/1893/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 28-08-17 14:50, Hans de Goede wrote:\n> Hi again,\n> \n> I realized I did not answer 1 of your questions:\n> \n> On 17-08-17 21:39, Wolfram Sang wrote:\n>> Hey guys,\n>>\n>> Sorry, I don't understand some of the stuff here. But I'd like to\n>> understand it before I add something to the I2C core. Especially as it\n>> feels a bit a the edge of the driver model to me.\n>>\n>> On Sat, Jul 22, 2017 at 08:55:37PM +0200, Hans de Goede wrote:\n>>> The CHPN0001 ACPI device has a _CID of PNP0C50 but is not HID compatible,\n>>> it uses its own protocol which is handled by the chipone_icn8318 driver.\n>>>\n>>> If the i2c_hid_driver's probe functon gets called it will fail with a\n>>> \"hid_descr_cmd failed\" error.\n>>\n>> That sounds like it fails pretty late. I'd assume we could check the\n>> blacklist right at the beginning of probe and bail out immediately?\n>>\n>>> Worse, after the probe failure the i2c / ACPI core code will put the ACPI\n>>> device in D3 state\n>>\n>> Where does that happen? Sorry, I can't find it. Would it be an idea to\n>> add a flag somewhere telling the device should not be put into D3?\n> \n> It is already possible to do this and my patches for the icn8318 driver\n> do this:\n> \n> struct acpi_device *adev;\n> \n> adev = ACPI_COMPANION(dev);\n> \n> /*\n> * Disable ACPI power management the _PS3 method is empty, so\n> * there is no powersaving when using ACPI power management.\n> * The _PS0 method resets the controller causing it to loose its\n> * firmware, which has been loaded by the BIOS and we do not\n> * know how to restore the firmware.\n> */\n> adev->flags.power_manageable = 0;\n> \n> The problem is that this happens in the probe() from the icn8318 driver\n> and if the i2c-hid drivers probe() executes first we end up in the\n> dev_pm_domain_detach() path of i2c_device_probe() and after that the\n> touchscreen-controller no longer works (*), iow after that it is too late\n> to disable acpi pm for the device.\n\nSo thinking more about this it might be cleaner to add a blacklist\nof _CID / _HID ACPI-ids for which power-management should be disabled\nto drivers/acpi/device_pm.c : acpi_bus_init_power().\n\nWhen I've some time to look into this I will write a patch following\nthat approach.\n\nSo lets forget about the approach to add an i2c_driver match callback\nfor now.\n\nRegards,\n\nHans\n\n\n> *) Unless we find a way to reload the firmware, which technically is\n> doable, but then we get into the problem of now having to distribute the\n> firmware in linux-firmware","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=hdegoede@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhMTP6h3dz9t3P\n\tfor ;\n\tTue, 29 Aug 2017 18:37:45 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750815AbdH2Iho (ORCPT );\n\tTue, 29 Aug 2017 04:37:44 -0400","from mx1.redhat.com ([209.132.183.28]:47362 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750779AbdH2Ihk (ORCPT );\n\tTue, 29 Aug 2017 04:37:40 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D93CB5CE;\n\tTue, 29 Aug 2017 08:37:39 +0000 (UTC)","from shalem.localdomain (ovpn-117-125.ams2.redhat.com\n\t[10.36.117.125])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 172816BC0D;\n\tTue, 29 Aug 2017 08:37:36 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D93CB5CE","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","From":"Hans de Goede ","To":"Wolfram Sang ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>\n\t<0c72f919-eab2-2f3f-a760-1aacc25d2550@redhat.com>","Message-ID":"","Date":"Tue, 29 Aug 2017 10:37:35 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<0c72f919-eab2-2f3f-a760-1aacc25d2550@redhat.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tTue, 29 Aug 2017 08:37:40 +0000 (UTC)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1759142,"web_url":"http://patchwork.ozlabs.org/comment/1759142/","msgid":"<20170829085103.yqgyphzcd4sihkca@ninjato>","list_archive_url":null,"date":"2017-08-29T08:51:03","subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","submitter":{"id":22495,"url":"http://patchwork.ozlabs.org/api/people/22495/","name":"Wolfram Sang","email":"wsa@the-dreams.de"},"content":"> So thinking more about this it might be cleaner to add a blacklist\n> of _CID / _HID ACPI-ids for which power-management should be disabled\n> to drivers/acpi/device_pm.c : acpi_bus_init_power().\n> \n> When I've some time to look into this I will write a patch following\n> that approach.\n> \n> So lets forget about the approach to add an i2c_driver match callback\n> for now.\n\nThanks for the update! That sounds better to me from the high level view\nthat I have. Thanks to you and Dmitry for discussing the details\nthroughly. Very much appreciated!","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhMmv649Pz9sRV\n\tfor ;\n\tTue, 29 Aug 2017 18:51:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751393AbdH2IvH (ORCPT );\n\tTue, 29 Aug 2017 04:51:07 -0400","from sauhun.de ([88.99.104.3]:43596 \"EHLO pokefinder.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750779AbdH2IvF (ORCPT );\n\tTue, 29 Aug 2017 04:51:05 -0400","from localhost (p54B33289.dip0.t-ipconnect.de [84.179.50.137])\n\tby pokefinder.org (Postfix) with ESMTPSA id ADCD32C3232;\n\tTue, 29 Aug 2017 10:51:03 +0200 (CEST)"],"Date":"Tue, 29 Aug 2017 10:51:03 +0200","From":"Wolfram Sang ","To":"Hans de Goede ","Cc":"Jiri Kosina ,\n\tBenjamin Tissoires ,\n\tlinux-input@vger.kernel.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH 2/2] HID: i2c-hid: Do not bind to CHPN0001 touchscreen","Message-ID":"<20170829085103.yqgyphzcd4sihkca@ninjato>","References":"<20170722185537.12696-1-hdegoede@redhat.com>\n\t<20170722185537.12696-2-hdegoede@redhat.com>\n\t<20170817193912.hccxyjwstrtr5oiv@ninjato>\n\t<0c72f919-eab2-2f3f-a760-1aacc25d2550@redhat.com>\n\t","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"pjvk3c6tcx4oualm\"","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170113 (1.7.2)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}}]