Re: GPIO bindings guidelines (Was: Re: [PATCH v5 10/12] gpio: Support for unified device properties interface)

From: Mika Westerberg
Date: Thu Oct 23 2014 - 08:57:22 EST


On Thu, Oct 23, 2014 at 01:21:06AM +0200, Rafael J. Wysocki wrote:
> OK, would the below make sense, then (completely untested, on top of the v6
> of the device properties patchset)?

Yes it does :-)

With the the below fix it works nicely with the modified rfkill-gpio.c
driver.

> +static bool acpi_get_driver_gpio_data(struct acpi_device *adev,
> + const char *name,
> + struct acpi_reference_args *args)
> +{
> + struct acpi_gpio_data *gd;
> +
> + mutex_lock(&driver_gpio_data_lock);
> +
> + list_for_each_entry(gd, &adev->driver_gpios, item)
> + if (!strcmp(name, gd->name)) {
> + args->adev = adev;
> + args->args[0] = gd->entry_index;
> + args->args[1] = gd->pin_index;
> + args->args[2] = gd->active_low;
> + args->nargs = 3;

mutex_unlock(&driver_gpio_data_lock);

> + return true;
> + }
> +
> + mutex_unlock(&driver_gpio_data_lock);
> +
> + return false;
> +}

Also I think the two functions should be exported to modules as well.

Here are the modifications needed for rfkill-gpio.c driver. I think it
is not that bad considering that now the driver supports both ACPI _DSD
and non-_DSD at the same time.

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 0f62326c0f5e..f8754e7d81ea 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -67,6 +67,8 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
struct rfkill_gpio_data *rfkill)
{
const struct acpi_device_id *id;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ int ret;

id = acpi_match_device(dev->driver->acpi_match_table, dev);
if (!id)
@@ -75,6 +77,20 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
rfkill->name = dev_name(dev);
rfkill->type = (unsigned)id->driver_data;

+ /*
+ * Add default mapping for ACPI _DSD properties now just in case
+ * there is no _DSD for this device.
+ */
+ ret = acpi_dev_add_driver_gpio(adev, "reset-gpios", 0, 0, false);
+ if (ret)
+ return ret;
+
+ ret = acpi_dev_add_driver_gpio(adev, "shutdown-gpios", 1, 0, false);
+ if (ret) {
+ acpi_dev_remove_driver_gpios(adev);
+ return ret;
+ }
+
return 0;
}

@@ -110,7 +126,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
rfkill->reset_gpio = gpio;
}

- gpio = devm_gpiod_get_index(&pdev->dev, "shutdown", 1);
+ gpio = devm_gpiod_get_index(&pdev->dev, "shutdown", 0);
if (!IS_ERR(gpio)) {
ret = gpiod_direction_output(gpio, 0);
if (ret)
@@ -150,6 +166,9 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
rfkill_unregister(rfkill->rfkill_dev);
rfkill_destroy(rfkill->rfkill_dev);

+ if (ACPI_COMPANION(&pdev->dev))
+ acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
return 0;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/