On Thursday, October 23, 2014 03:56:55 PM Mika Westerberg wrote:
> 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.
OK, let's try to take that a bit farther. :-)
With the (untested) patch below applied (which is a replacement for the one
sent previously), the driver can use static tables like these:
static struct acpi_gpio_params reset_gpios = { 0, 0, false };
statuc struct acpi_gpio_params shutdown_gpios = { 1, 0, false };
static struct acpi_gpio_mapping my_gpio_mapping[] = {
{ "reset-gpios", &reset_gpios, 1 },
{ "shutdown-gpios", &shutdown_gpios, 1 },
NULL,
};
->
>
> 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;
> + }
-> and then simply do
ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev), &my_gpio_mapping);
instead of the above.
> 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));
> +
And here simply
acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
> return 0;
> }
and then we can even cover situations in which there is one name for a list of
GPIOs.
Rafael
---
drivers/gpio/gpiolib-acpi.c | 40 ++++++++++++++++++++++++++++++++++++++--
include/acpi/acpi_bus.h | 3 +++
include/linux/acpi.h | 30 ++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 2 deletions(-)
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -345,6 +345,8 @@ struct acpi_device_data {
const union acpi_object *of_compatible;
};
+struct acpi_gpio_mapping;
+
/* Device */
struct acpi_device {
int device_type;
@@ -366,6 +368,7 @@ struct acpi_device {
struct acpi_scan_handler *handler;
struct acpi_hotplug_context *hp;
struct acpi_driver *driver;
+ struct acpi_gpio_mapping *driver_gpios;
void *driver_data;
struct device dev;
unsigned int physical_node_count;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -672,6 +672,36 @@ do { \
#endif
#endif
+struct acpi_gpio_params {
+ unsigned int crs_entry_index;
+ unsigned int pin_index;
+ bool active_low;
+};
+
+struct acpi_gpio_mapping {
+ const char *name;
+ struct acpi_gpio_params *data;
+ unsigned int size;
+};
+
+#if defined(CONFIG_ACPI) && defined(CONFIG_GPIOLIB)
+int acpi_dev_add_driver_gpios(struct acpi_device *adev,
+ struct acpi_gpio_mapping *gpios);
+
+static inline void acpi_dev_remove_driver_gpios(struct acpi_device *adev)
+{
+ if (adev)
+ adev->driver_gpios = NULL;
+}
+#else
+static inline int acpi_dev_add_driver_gpios(struct acpi_device *adev,
+ struct acpi_gpio_mapping *gpios)
+{
+ return -ENXIO;
+}
+static inline void acpi_dev_remove_driver_gpios(struct acpi_device *adev) {}
+#endif
+
/* Device properties */
#define MAX_ACPI_REFERENCE_ARGS 8
Index: linux-pm/drivers/gpio/gpiolib-acpi.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib-acpi.c
+++ linux-pm/drivers/gpio/gpiolib-acpi.c
@@ -287,6 +287,38 @@ void acpi_gpiochip_free_interrupts(struc
}
}
+int acpi_dev_add_driver_gpios(struct acpi_device *adev,
+ struct acpi_gpio_mapping *gpios)
+{
+ if (adev && gpios) {
+ adev->driver_gpios = gpios;
+ return 0;
+ }
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_add_driver_gpios);
+
+static bool acpi_get_driver_gpio_data(struct acpi_device *adev,
+ const char *name, int index,
+ struct acpi_reference_args *args)
+{
+ struct acpi_gpio_mapping *gm;
+
+ for (gm = adev->driver_gpios; gm; gm++)
+ if (!strcmp(name, gm->name) && gm->data && index < gm->size) {
+ struct acpi_gpio_params *par = gm->data + index;
+
+ args->adev = adev;
+ args->args[0] = par->crs_entry_index;
+ args->args[1] = par->pin_index;
+ args->args[2] = par->active_low;
+ args->nargs = 3;
+ return true;
+ }
+
+ return false;
+}
+
struct acpi_gpio_lookup {
struct acpi_gpio_info info;
int index;
@@ -372,8 +404,12 @@ struct gpio_desc *acpi_get_gpiod_by_inde
memset(&args, 0, sizeof(args));
ret = acpi_dev_get_property_reference(adev, propname, NULL,
index, &args);
- if (ret)
- return ERR_PTR(ret);
+ if (ret) {
+ bool found = acpi_get_driver_gpio_data(adev, propname,
+ index, &args);
+ if (!found)
+ return ERR_PTR(ret);
+ }
/*
* The property was found and resolved so need to
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
