acpi_find_gpio with absent GPIOs

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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.

I think both should be fixed.

For the first maybe something like below?

index 5db3445552b1..441be96e18e7 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html