From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH v2] Input: silead - Do not try to directly access the GPIO when using ACPI pm Date: Mon, 13 Feb 2017 13:00:49 +0200 Message-ID: <1486983649.2133.453.camel@linux.intel.com> References: <20170122200008.27027-1-hdegoede@redhat.com> <20170122222015.GA31009@dtor-ws> <8a23b7b2-a7aa-d62d-947d-31301a0c92cc@redhat.com> <20170201174257.GE40045@dtor-ws> <20170202104130.GJ2053@lahna.fi.intel.com> <8e91084e-e0ea-b055-5c62-67a4e0e56df4@redhat.com> <20170202121018.GN2053@lahna.fi.intel.com> <20170202123206.GP2053@lahna.fi.intel.com> <20170202131251.GQ2053@lahna.fi.intel.com> <3f433773-27ba-8d07-3209-6df71d6d4b33@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Return-path: Received: from mga02.intel.com ([134.134.136.20]:10637 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752194AbdBMLBE (ORCPT ); Mon, 13 Feb 2017 06:01:04 -0500 In-Reply-To: <3f433773-27ba-8d07-3209-6df71d6d4b33@redhat.com> Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Hans de Goede , Mika Westerberg Cc: Dmitry Torokhov , "russianneuromancer @ ya . ru" , Gregor Riepl , linux-input@vger.kernel.org, Linus Walleij On Sun, 2017-02-12 at 11:40 +0100, Hans de Goede wrote: > Hi, > > On 10-02-17 12:52, Hans de Goede wrote: > > Hi, > > > > On 02-02-17 14:12, Mika Westerberg wrote: > > > On Thu, Feb 02, 2017 at 01:50:58PM +0100, Hans de Goede wrote: > > > > Hi, > > > > > > > > On 02-02-17 13:32, Mika Westerberg wrote: > > > > > On Thu, Feb 02, 2017 at 02:10:18PM +0200, Mika Westerberg > > > > > wrote: > > > > > > I do not have a copy of the patch in this thread but sounds > > > > > > like > > > > > > something that might work. > > > > > > > > > > Actually, I seem have a copy of that patch. > > > > > > > > > > So you are saying that the device has a power GPIO in ACPI > > > > > _CRS but it > > > > > should not be used for some reason? > > > > > > > >                 Method (_CRS, 0, NotSerialized)  // _CRS: > > > > Current Resource Setti > > > >                 { > > > >                     Name (WBUF, ResourceTemplate () > > > >                     { > > > >                         I2cSerialBusV2 (0x0040, > > > > ControllerInitiated, 0x00061A80, > > > >                             AddressingMode7Bit, > > > > "\\_SB.PCI0.I2C6", > > > >                             0x00, ResourceConsumer, , Exclusive, > > > >                             ) > > > >                         GpioIo (Exclusive, PullDefault, 0x0000, > > > > 0x0000, IoRestri > > > >                             "\\_SB.GPO1", 0x00, > > > > ResourceConsumer, , > > > >                             ) > > > >                             {   // Pin list > > > >                                 0x0019 > > > >                             } > > > >                         GpioInt (Edge, ActiveHigh, Shared, > > > > PullDefault, 0x0000, > > > >                             "\\_SB.GPO1", 0x00, > > > > ResourceConsumer, , > > > >                             ) > > > >                             {   // Pin list > > > >                                 0x0013 > > > >                             } > > > >                     }) > > > >                     Return (WBUF) /* > > > > \_SB_.PCI0.I2C6.TCS4._CRS.WBUF */ > > > >                 } > > > > > > > > The setting of the special bit in the gpio control register > > > > leads to > > > > drivers/pinctrl/intel/pinctrl-cherryview.c > > > > chv_gpio_request_enable() > > > > returning -EBUSY, which in return makes gpiod_get_optional > > > > return -EBUSY for this pin, rather then NULL (as we would like). > > > > > > Actually what is wrong here is that your gpiod_get(dev, "power") > > > falls > > > back to use plain indexes and returns the first GPIO even though > > > it > > > should not as the driver specifically requests GPIO with name > > > "power" > > > and there is no _DSD. > > > > > > Andy (Cc'd) has a patch that tries to make the fallback mechanism > > > more > > > stricter which should in theory fix the problem as well. The patch > > > series is here: > > > > > > https://bitbucket.org/andy-shev/linux/commits/338c0226b631b8b497d1 > > > 43070a301d8b8883c349?at=master > > > > Ok, that patches fixes the issues I was seeing with the silead > > driver > > on my cube iwork8 air cherrytrail tablet. > > But unfortunately it causes regressions for drivers which actually use > gpiod_get_by_index, e.g. drivers/extcon/extcon-intel-int3496.c, which > does: > >          data->gpio_usb_id = devm_gpiod_get_index(dev, "id", >                                                  INT3496_GPIO_USB_ID, >                                                  GPIOD_IN); > > Where GPIOD_IN is 0, (it also gets gpios for index 1 and 2), I guess > this driver can be fixed by replacing "id" with NULL, but the name > gets used in things like /sys/kernel/debug/gpio and is actually > useful there, so it looks like that patch from Andy needs some > work so as to not see getting by index as an undesirable fallback > while the driver is actually doing a request gpio by index. Yes, this part is missed. We would like to introduce a flag for that to encourage drivers to fix this mess by adding compatible lookup table. -- Andy Shevchenko Intel Finland Oy