acpi_find_gpio with absent GPIOs

acpi_find_gpio with absent GPIOs

[Daniel Glöckner]

Hi,

I'm currently trying to use rfkill-gpio with a device that has just a
single GPIO assigned by ACPI. rfkill-gpio calls acpi_dev_add_driver_gpios
to assign names to the ACPI GPIOs and then uses devm_gpiod_get_optional
to request both of them. The problem is that on the second call to
devm_gpiod_get_optional acpi_find_gpio falls back to using the GPIO index
0 (from gpiod_get) in _CRS, which leads to the same GPIO being returned
as in the first call. Probing the driver then fails with -EBUSY.

In my opinion it is a bad idea to fall back to indexing the _CRS if the
con_id was found in the _DSD or the GPIOs added by
acpi_dev_add_driver_gpios, but I don't know if there are drivers relying
on this behavior.

Luckily acpi_get_gpiod_by_index returns -ENODATA if the name can't be
found and -ENOENT if the GPIO is absent, so we can distinguish the two
cases. -EPROBE_DEFER also should not make acpi_find_gpio try to use
another GPIO from the _CRS.

There is also the possibility that the GPIO index exceeds the size of
the package found in _DSD or added with acpi_dev_add_driver_gpios.
The former will return -EPROTO, the latter will forward the error
from acpi_dev_get_property_reference (usually -ENODATA). of_find_gpio
returns -ENOENT in this case.

So, what of this should be fixed?

Best regards,

  Daniel

-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Bertha-von-Suttner-Straße 9, 37085 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführer: Dr. Uwe Kracke, Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: acpi_find_gpio with absent GPIOs

[Mika Westerberg]

On Fri, Oct 23, 2015 at 04:56:54PM +0200, Daniel Glöckner wrote:
> Hi,
> 
> I'm currently trying to use rfkill-gpio with a device that has just a
> single GPIO assigned by ACPI. rfkill-gpio calls acpi_dev_add_driver_gpios
> to assign names to the ACPI GPIOs and then uses devm_gpiod_get_optional
> to request both of them. The problem is that on the second call to
> devm_gpiod_get_optional acpi_find_gpio falls back to using the GPIO index
> 0 (from gpiod_get) in _CRS, which leads to the same GPIO being returned
> as in the first call. Probing the driver then fails with -EBUSY.
> 
> In my opinion it is a bad idea to fall back to indexing the _CRS if the
> con_id was found in the _DSD or the GPIOs added by
> acpi_dev_add_driver_gpios, but I don't know if there are drivers relying
> on this behavior.

I agree it is bad idea and I think this is actually a bug in the
implementation rather than wanted behavior. No drivers should rely on
that anyway.

> Luckily acpi_get_gpiod_by_index returns -ENODATA if the name can't be
> found and -ENOENT if the GPIO is absent, so we can distinguish the two
> cases. -EPROBE_DEFER also should not make acpi_find_gpio try to use
> another GPIO from the _CRS.
> 
> There is also the possibility that the GPIO index exceeds the size of
> the package found in _DSD or added with acpi_dev_add_driver_gpios.
> The former will return -EPROTO, the latter will forward the error
> from acpi_dev_get_property_reference (usually -ENODATA). of_find_gpio
> returns -ENOENT in this case.
> 
> So, what of this should be fixed?

I think both should be fixed.

For the first maybe something like below?

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5db3445552b1..441be96e18e7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1765,6 +1765,11 @@ static struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id,
 
 	/* Then from plain _CRS GPIOs */
 	if (IS_ERR(desc)) {
+		/* Only fallback if the device has no properties at all */
+		if (PTR_ERR(desc) == -ENODATA &&
+		    (adev->data.properties || adev->driver_gpios))
+			return ERR_PTR(-ENOENT);
+
 		desc = acpi_get_gpiod_by_index(adev, NULL, idx, &info);
 		if (IS_ERR(desc))
 			return desc;
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html