Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755146AbdC1Qoo (ORCPT ); Tue, 28 Mar 2017 12:44:44 -0400 Received: from mga11.intel.com ([192.55.52.93]:55687 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752466AbdC1Qom (ORCPT ); Tue, 28 Mar 2017 12:44:42 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,237,1486454400"; d="scan'208";a="949078500" Message-ID: <1490719163.708.40.camel@linux.intel.com> Subject: Re: [PATCH v1 6/8] gpio: acpi: Explain how to get GPIO descriptors in ACPI case From: Andy Shevchenko To: Dmitry Torokhov Cc: Linus Walleij , Alexandre Courbot , linux-gpio@vger.kernel.org, Hans de Goede , linux-kernel@vger.kernel.org, Mika Westerberg , Jarkko Nikula , linux-acpi@vger.kernel.org Date: Tue, 28 Mar 2017 19:39:23 +0300 In-Reply-To: <20170323202838.GA11818@dtor-ws> References: <20170323194618.26548-1-andriy.shevchenko@linux.intel.com> <20170323194618.26548-7-andriy.shevchenko@linux.intel.com> <20170323202838.GA11818@dtor-ws> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3869 Lines: 105 On Thu, 2017-03-23 at 13:28 -0700, Dmitry Torokhov wrote: > On Thu, Mar 23, 2017 at 09:46:16PM +0200, Andy Shevchenko wrote: > > +Using the _CRS fallback > > +----------------------- > > + > > +If a device does not have _DSD or the driver does not create ACPI > > GPIO > > +mapping, the Linux GPIO framework refuses to return any GPIOs. This > > is > > +because the driver does not know what it actually gets. For example > > if we > > +have a device like below: > > + > > +  Device (BTH) > > +  { > > +      Name (_HID, ...) > > + > > +      Name (_CRS, ResourceTemplate () { > > +          GpioIo (Exclusive, PullNone, 0, 0, IoRestrictionNone, > > +                  "\\_SB.GPO0", 0, ResourceConsumer) {15} > > +          GpioIo (Exclusive, PullNone, 0, 0, IoRestrictionNone, > > +                  "\\_SB.GPO0", 0, ResourceConsumer) {27} > > +      }) > > +  } > > + > > +The driver might expect to get the right GPIO when it does: > > + > > +  desc = gpiod_get(dev, "reset", GPIOD_OUT_LOW); > > + > > +but since there is no way to know the mapping between "reset" and > > +the GpioIo() in _CRS desc will hold ERR_PTR(-ENOENT). > > + > > +The driver author can solve this by passing the mapping explictly > > +(the recommended way and documented in the above chapter). > > If the driver is not platform specific, then it would have no idea > about > mapping between _CRS GPIOs and names. All such stuff should be hidden > in > platform glue (i.e drivers/platform/x86/platform_crap.c). It might be interpreted that all platform data from all the drivers should gone. While ideal case should be like this and I totally agree with you, we are living in non-ideal world, that's why we used to and continue using some ID-based quirks (PCI enumeration, I2C enumeration, ACPI enumeration, SPI enumeration, UART enumeration, an so on, so on). Moreover ACPI comes into ARM(64) world which might have its own troubles with generating correct tables and we might end up with quirks there. So, I disagree that here is possible to hide like you said "all such stuff in ...platform_crap.c". > > + > > +Getting GPIO descriptor > > +----------------------- > > + > > +There are two main approaches to get GPIO resource from ACPI: > > + desc = gpiod_get(dev, connection_id, flags); > > + desc = gpiod_get_index(dev, connection_id, index, flags); > > + > > +We may consider two different cases here, i.e. when connection ID > > is > > +provided and otherwise. > > + > > +Case 1: > > + desc = gpiod_get(dev, "non-null-connection-id", flags); > > + desc = gpiod_get_index(dev, "non-null-connection-id", > > index, flags); > > + > > +Case 2: > > + desc = gpiod_get(dev, NULL, flags); > > + desc = gpiod_get_index(dev, NULL, index, flags); > > + > > +Case 1 assumes that corresponding ACPI device description must have > > +defined device properties and will prevent to getting any GPIO > > resources > > +otherwise. > > + > > +Case 2 explicitly tells GPIO core to look for resources in _CRS. > > + > > +Be aware that gpiod_get_index() in cases 1 and 2, assuming that > > there > > +are two versions of ACPI device description provided and no mapping > > is > > +present in the driver, will return different resources. That's why > > a > > +certain driver has to handle them carefully as explained in > > previous > > +chapter. > > I think that this wording is too x86-centric. We are talking about > consumers of GPIOs here (i.e. drivers), which need unified behavior > between ACPI, DT, and static board properties, they do not really care > about _CRS or _DSD. If the certain driver cares about ACPI enumerated devices it might care about supporting it disregarding on how new firmware is used (supporting _DSD or not). -- Andy Shevchenko Intel Finland Oy