Hi,

On 09-01-19 11:52, Andy Shevchenko wrote:
New quirk enforces search for GPIO based on its type,
i.e. iterate over GpioIo resources only.

Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Acked-by: Mika Westerberg <mika.westerb...@linux.intel.com>
---

- it was sent few weeks ago to Hans for testing, but better to re-test

I've just re-tested this on a device which uses the speaker-enable
GPIO with a rt5651 codec, still works as advertised:

Tested-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans




- it's supposed to go via ASoC subsystem due to recent changes made for
   sound driver

v2:
- Expand explanation why this quirk might be needed (Mika)

  drivers/gpio/gpiolib-acpi.c           | 15 ++++--
  include/linux/acpi.h                  |  7 +++
  sound/soc/intel/boards/bytcr_rt5651.c | 74 ++++-----------------------
  3 files changed, 27 insertions(+), 69 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 259cf6ab969b..4d291b75cb9f 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -530,17 +530,24 @@ static int acpi_populate_gpio_lookup(struct acpi_resource 
*ares, void *data)
        if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
                return 1;
- if (lookup->n++ == lookup->index && !lookup->desc) {
+       if (!lookup->desc) {
                const struct acpi_resource_gpio *agpio = &ares->data.gpio;
-               int pin_index = lookup->pin_index;
+               bool gpioint = agpio->connection_type == 
ACPI_RESOURCE_GPIO_TYPE_INT;
+               int pin_index;
+ if (lookup->info.quirks & ACPI_GPIO_QUIRK_ONLY_GPIOIO && gpioint)
+                       lookup->index++;
+
+               if (lookup->n++ != lookup->index)
+                       return 1;
+
+               pin_index = lookup->pin_index;
                if (pin_index >= agpio->pin_table_length)
                        return 1;
lookup->desc = acpi_get_gpiod(agpio->resource_source.string_ptr,
                                              agpio->pin_table[pin_index]);
-               lookup->info.gpioint =
-                       agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
+               lookup->info.gpioint = gpioint;
/*
                 * Polarity and triggering are only specified for GpioInt
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 1e89c688139f..430fd0b6df7b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1020,6 +1020,13 @@ struct acpi_gpio_mapping {
/* Ignore IoRestriction field */
  #define ACPI_GPIO_QUIRK_NO_IO_RESTRICTION     BIT(0)
+/*
+ * When ACPI GPIO mapping table is in use the index parameter inside it
+ * refers to the GPIO resource in _CRS method. That index has no
+ * distinction of actual type of the resource. When consumer wants to
+ * get GpioIo type explicitly, this quirk may be used.
+ */
+#define ACPI_GPIO_QUIRK_ONLY_GPIOIO            BIT(1)
unsigned int quirks;
  };
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c 
b/sound/soc/intel/boards/bytcr_rt5651.c
index b618d984e2d5..99d0c01a1bec 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -844,74 +844,18 @@ static const struct x86_cpu_id cherrytrail_cpu_ids[] = {
        {}
  };
-static const struct acpi_gpio_params first_gpio = { 0, 0, false };
-static const struct acpi_gpio_params second_gpio = { 1, 0, false };
+static const struct acpi_gpio_params ext_amp_enable_gpios = { 0, 0, false };
-static const struct acpi_gpio_mapping byt_rt5651_amp_en_first[] = {
-       { "ext-amp-enable-gpios", &first_gpio, 1 },
-       { },
-};
-
-static const struct acpi_gpio_mapping byt_rt5651_amp_en_second[] = {
-       { "ext-amp-enable-gpios", &second_gpio, 1 },
+static const struct acpi_gpio_mapping cht_rt5651_gpios[] = {
+       /*
+        * Some boards have I2cSerialBusV2, GpioIo, GpioInt as ACPI resources,
+        * other boards may  have I2cSerialBusV2, GpioInt, GpioIo instead.
+        * We want the GpioIo one for the ext-amp-enable-gpio.
+        */
+       { "ext-amp-enable-gpios", &ext_amp_enable_gpios, 1, 
ACPI_GPIO_QUIRK_ONLY_GPIOIO },
        { },
  };
-/*
- * Some boards have I2cSerialBusV2, GpioIo, GpioInt as ACPI resources, other
- * boards may  have I2cSerialBusV2, GpioInt, GpioIo instead. We want the
- * GpioIo one for the ext-amp-enable-gpio and both count for the index in
- * acpi_gpio_params index.  So we have 2 different mappings and the code
- * below figures out which one to use.
- */
-struct byt_rt5651_acpi_resource_data {
-       int gpio_count;
-       int gpio_int_idx;
-};
-
-static int snd_byt_rt5651_acpi_resource(struct acpi_resource *ares, void *arg)
-{
-       struct byt_rt5651_acpi_resource_data *data = arg;
-
-       if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
-               return 0;
-
-       if (ares->data.gpio.connection_type == ACPI_RESOURCE_GPIO_TYPE_INT)
-               data->gpio_int_idx = data->gpio_count;
-
-       data->gpio_count++;
-       return 0;
-}
-
-static void snd_byt_rt5651_mc_pick_amp_en_gpio_mapping(struct device *codec)
-{
-       struct byt_rt5651_acpi_resource_data data = { 0, -1 };
-       LIST_HEAD(resources);
-       int ret;
-
-       ret = acpi_dev_get_resources(ACPI_COMPANION(codec), &resources,
-                                    snd_byt_rt5651_acpi_resource, &data);
-       if (ret < 0) {
-               dev_warn(codec, "Failed to get ACPI resources, not adding external 
amplifier GPIO mapping\n");
-               return;
-       }
-
-       /* All info we need is gathered during the walk */
-       acpi_dev_free_resource_list(&resources);
-
-       switch (data.gpio_int_idx) {
-       case 0:
-               byt_rt5651_gpios = byt_rt5651_amp_en_second;
-               break;
-       case 1:
-               byt_rt5651_gpios = byt_rt5651_amp_en_first;
-               break;
-       default:
-               dev_warn(codec, "Unknown GpioInt index %d, not adding external 
amplifier GPIO mapping\n",
-                        data.gpio_int_idx);
-       }
-}
-
  struct acpi_chan_package {   /* ACPICA seems to require 64 bit integers */
        u64 aif_value;       /* 1: AIF1, 2: AIF2 */
        u64 mclock_value;    /* usually 25MHz (0x17d7940), ignored */
@@ -1037,7 +981,7 @@ static int snd_byt_rt5651_mc_probe(struct platform_device 
*pdev)
/* Cherry Trail devices use an external amplifier enable gpio */
        if (x86_match_cpu(cherrytrail_cpu_ids) && !byt_rt5651_gpios)
-               snd_byt_rt5651_mc_pick_amp_en_gpio_mapping(codec_dev);
+               byt_rt5651_gpios = cht_rt5651_gpios;
if (byt_rt5651_gpios) {
                devm_acpi_dev_add_driver_gpios(codec_dev, byt_rt5651_gpios);

Reply via email to