lkml.org 
[lkml]   [2017]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 4/8] gpio: acpi: Even more tighten up ACPI GPIO lookups
    On Thu, Mar 23, 2017 at 09:46:14PM +0200, Andy Shevchenko wrote:
    > The commit 10cf4899f8af ("gpiolib: tighten up ACPI legacy gpio lookups")
    > prevents to getting same resource twice if the driver asks twice using same

    s/same/different/

    > connection ID.
    >
    > But the whole idea of fallback might bring some problems. Imagine the case when
    > we have two versions of BIOS/hardware where in one _DSD is introduced along
    > with GPIO resources, but the other one uses just plain GPIO resource for
    > another purpose
    >
    > Case 1:
    >
    > Device (DEVX)
    > {
    > ...
    > Name (_CRS, ResourceTemplate ()
    > {
    > GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
    > "\\_SB.GPO0", 0, ResourceConsumer) {15}
    > })
    > Name (_DSD, Package ()
    > {
    > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
    > Package ()
    > {
    > Package () {"some-gpios", Package() {^DEVX, 0, 0, 0 }},
    > }
    > })
    > }
    >
    > Case 2:
    >
    > Device (DEVX)
    > {
    > ...
    > Name (_CRS, ResourceTemplate ()
    > {
    > GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
    > "\\_SB.GPO0", 0, ResourceConsumer) {27}
    > })
    > }
    >
    > To prevent the possible misconfiguration tighten up even more ACPI GPIO lookups
    > for case without connection ID provided.

    I wonder if this will break Goodix. Irina, Bastien?

    >
    > In the past the issue had been triggered by "use mctrl_gpio helpers" series
    > [1,2].
    >
    > Besides above, removal of the main logic of acpi_can_fallback_to_crs()
    > eliminates a potential memory leak when the same device has been unbound and
    > bound again.

    Where? We'll reuse lookup table as ACPI device is still the same.

    >
    > [1] commit 4ef03d328769 ("tty/serial/8250: use mctrl_gpio helpers")
    > [2] https://patchwork.kernel.org/patch/9283745/
    >
    > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
    > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    > ---
    > drivers/gpio/gpiolib-acpi.c | 36 +-----------------------------------
    > 1 file changed, 1 insertion(+), 35 deletions(-)
    >
    > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
    > index 21e4930ca2db..e516b7a0cc50 100644
    > --- a/drivers/gpio/gpiolib-acpi.c
    > +++ b/drivers/gpio/gpiolib-acpi.c
    > @@ -1121,45 +1121,11 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
    > return count ? count : -ENOENT;
    > }
    >
    > -struct acpi_crs_lookup {
    > - struct list_head node;
    > - struct acpi_device *adev;
    > - const char *con_id;
    > -};
    > -
    > -static DEFINE_MUTEX(acpi_crs_lookup_lock);
    > -static LIST_HEAD(acpi_crs_lookup_list);
    > -
    > bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
    > {
    > - struct acpi_crs_lookup *l, *lookup = NULL;
    > -
    > /* Never allow fallback if the device has properties */
    > if (adev->data.properties || adev->driver_gpios)
    > return false;
    >
    > - mutex_lock(&acpi_crs_lookup_lock);
    > -
    > - list_for_each_entry(l, &acpi_crs_lookup_list, node) {
    > - if (l->adev == adev) {
    > - lookup = l;
    > - break;
    > - }
    > - }
    > -
    > - if (!lookup) {
    > - lookup = kmalloc(sizeof(*lookup), GFP_KERNEL);
    > - if (lookup) {
    > - lookup->adev = adev;
    > - lookup->con_id = kstrdup(con_id, GFP_KERNEL);
    > - list_add_tail(&lookup->node, &acpi_crs_lookup_list);
    > - }
    > - }
    > -
    > - mutex_unlock(&acpi_crs_lookup_lock);
    > -
    > - return lookup &&
    > - ((!lookup->con_id && !con_id) ||
    > - (lookup->con_id && con_id &&
    > - strcmp(lookup->con_id, con_id) == 0));
    > + return con_id == NULL;
    > }
    > --
    > 2.11.0
    >

    --
    Dmitry

    \
     
     \ /
      Last update: 2017-03-23 21:13    [W:2.265 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site