Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753110AbbF2QFQ (ORCPT ); Mon, 29 Jun 2015 12:05:16 -0400 Received: from mga03.intel.com ([134.134.136.65]:63850 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753397AbbF2QFE (ORCPT ); Mon, 29 Jun 2015 12:05:04 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,699,1427785200"; d="scan'208";a="736986665" From: "Tirdea, Irina" To: "Purdila, Octavian" , Bastien Nocera CC: Dmitry Torokhov , Mark Rutland , "linux-input@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Rob Herring , Pawel Moll , Ian Campbell , "Kumar Gala" Subject: RE: [PATCH v2 4/8] input: goodix: reset device at init Thread-Topic: [PATCH v2 4/8] input: goodix: reset device at init Thread-Index: AQHQrcP7EfpWE6TJYUGqcngIfDBOW53DriMA Date: Mon, 29 Jun 2015 16:04:58 +0000 Deferred-Delivery: Mon, 29 Jun 2015 16:04:00 +0000 Message-ID: <1F3AC3675D538145B1661F571FE1805F2F063F9D@irsmsx105.ger.corp.intel.com> References: <1433774273-23103-1-git-send-email-irina.tirdea@intel.com> <1433774273-23103-5-git-send-email-irina.tirdea@intel.com> <1433864084.5707.4.camel@hadess.net> <1433865182.5707.7.camel@hadess.net> <1F3AC3675D538145B1661F571FE1805F2F061650@irsmsx105.ger.corp.intel.com> <1435068777.22211.4.camel@hadess.net> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t5TG5KgP006764 Content-Length: 5019 Lines: 117 > -----Original Message----- > From: Octavian Purdila [mailto:octavian.purdila@intel.com] > Sent: 23 June, 2015 17:51 > To: Bastien Nocera > Cc: Tirdea, Irina; Dmitry Torokhov; Mark Rutland; linux-input@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Rob Herring; Pawel Moll; Ian Campbell; Kumar Gala > Subject: Re: [PATCH v2 4/8] input: goodix: reset device at init > > On Tue, Jun 23, 2015 at 5:12 PM, Bastien Nocera wrote: > > > > On Tue, 2015-06-23 at 13:23 +0000, Tirdea, Irina wrote: > > > > > > > -----Original Message----- > > > > From: linux-input-owner@vger.kernel.org [mailto: > > > > linux-input-owner@vger.kernel.org] On Behalf Of Bastien Nocera > > > > Sent: 09 June, 2015 18:53 > > > > To: Tirdea, Irina > > > > Cc: Dmitry Torokhov; Mark Rutland; linux-input@vger.kernel.org; > > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Rob > > > > Herring; Pawel Moll; Ian Campbell; Kumar Gala; Purdila, Octavian > > > > Subject: Re: [PATCH v2 4/8] input: goodix: reset device at init > > > > > > > > On Tue, 2015-06-09 at 17:34 +0200, Bastien Nocera wrote: > > > > > On Mon, 2015-06-08 at 17:37 +0300, Irina Tirdea wrote: > > > > > > After power on, it is recommended that the driver resets the > > > > > > device. > > > > > > The reset procedure timing is described in the datasheet and is > > > > > > used > > > > > > at device init (before writing device configuration) and > > > > > > for power management. It is a sequence of setting the interrupt > > > > > > and reset pins high/low at specific timing intervals. This > > > > > > procedure > > > > > > also includes setting the slave address to the one specified in > > > > > > the > > > > > > ACPI/device tree. > > > > > > > > > > This breaks the touchscreen on my Onda v975w: > > > > > [ 239.732858] Goodix-TS i2c-GDIX1001:00: ID 9271, version: 1020 > > > > > [ 239.732977] Goodix-TS i2c-GDIX1001:00: Failed to get reset > > > > > GPIO: > > > > > -16 > > > > > [ 239.736071] Goodix-TS: probe of i2c-GDIX1001:00 failed with > > > > > error > > > > > -16 > > > > > > > > > > This is the DSDT for that device: > > > > > https://bugzilla.kernel.org/attachment.cgi?id=149331 > > > > > > > > > > Oops. That's right - I am using named interrupts which are available > > > only for ACPI 5.1, so > > > devices already out there will not work. > > > > > > The problem here is that handling -ENOENT will not help. The gpio > > > pins are declared in the > > > ACPI DSDT, but are not named. The devm_gpiod_get API will look for > > > the named interrupt > > > first and fallback to searching by index if not found. Index will be > > > 0 in both cases, this is why > > > the first call will succeed and the second will fail with -EBUSY. > > > > > > One way to handle this would be to use indexed gpio pins instead of > > > named gpio pins: > > > e.g. consider the first gpio pin to be the reset pin and the second > > > to be the interrupt pin. > > > This is used in other drivers as well, e.g. zforce_ts. The problem > > > with this approach is that > > > any devices that declare the gpio pins in reversed order in the DSDT > > > will not work and give > > > no warning (the pins will be requested successfully, but some of the > > > functionality will not > > > work). The DSDT mentioned in > > > https://bugzilla.kernel.org/attachment.cgi?id=149331 lists > > > the reset pin first, while I am working on some devices that declare > > > the interrupt gpio pin > > > first. > > > > > > Another way to handle this is to treat -EBUSY like -ENOENT and not > > > use any functionality > > > that depends on the gpio pins. Unfortunately, this means we will not > > > have suspend, esd and > > > custom configs even if the pins are connected on the board and > > > available in ACPI(just not > > > named). > > > > > > I would go with the first approach and document the order requested > > > for the pins, but I would > > > like to hear what you think first. Is there a better way to do this? > > > > > > > (Note that this means that I haven't been able to test any > > > > following > > > > patches in that series than 4/8). > > > > > > Sure, that makes sense. The following patches depend on the gpio pins > > > so they would not have > > > worked either. > > > > We can apply quirks based on DMI information, so that devices with ACPI > > in different orders will work after applying a quirk (as long as > > there's a way to detect that it's in the wrong order, obviously). > > > > I think even using the ACPI id (_HID) should be enough (at least in > the cases we know so far) to make the difference in how the pins are > used. Thanks for the suggestions Bastien and Octavian! I will use the ACPI ID in v3 considering the ACPI table Bastien has mentioned [1]. Thanks, Irina [1] https://bugzilla.kernel.org/attachment.cgi?id=149331 ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?