[RFT,v1] gpiolib: acpi: Introduce ACPI_GPIO_QUIRK_ONLY_GPIOIO
diff mbox series

Message ID 20190108180205.55109-1-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • [RFT,v1] gpiolib: acpi: Introduce ACPI_GPIO_QUIRK_ONLY_GPIOIO
Related show

Commit Message

Andy Shevchenko Jan. 8, 2019, 6:02 p.m. UTC
New quirk enforces search for GPIO based on its type.
Note, supplied index in the mapping table must be 0.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

- it was sent few weeks ago to Hans for testing, but better to re-test
- it's supposed to go via ASoC subsystem due to recent changes made for sound driver

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

Comments

Mika Westerberg Jan. 9, 2019, 9:20 a.m. UTC | #1
On Tue, Jan 08, 2019 at 08:02:05PM +0200, Andy Shevchenko wrote:
> New quirk enforces search for GPIO based on its type.
> Note, supplied index in the mapping table must be 0.

Can you document this where you define the new flag so new users do not
need go through git history?

Otherwise looks good. :)
Linus Walleij Jan. 11, 2019, 2:21 p.m. UTC | #2
On Tue, Jan 8, 2019 at 7:02 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> New quirk enforces search for GPIO based on its type.
> Note, supplied index in the mapping table must be 0.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(...)

>  sound/soc/intel/boards/bytcr_rt5651.c | 74 ++++-----------------------

Do we have an ACK from the maintainer of this file?
It's Hans I think. Ideally also from Mark Brown.

Yours,
Linus Walleij
Andy Shevchenko Jan. 11, 2019, 7:49 p.m. UTC | #3
On Fri, Jan 11, 2019 at 03:21:39PM +0100, Linus Walleij wrote:
> On Tue, Jan 8, 2019 at 7:02 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > New quirk enforces search for GPIO based on its type.
> > Note, supplied index in the mapping table must be 0.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> (...)
> 
> >  sound/soc/intel/boards/bytcr_rt5651.c | 74 ++++-----------------------
> 
> Do we have an ACK from the maintainer of this file?
> It's Hans I think. Ideally also from Mark Brown.

I hope to get Hans' Tested-by on this.
Andy Shevchenko Jan. 11, 2019, 7:50 p.m. UTC | #4
On Fri, Jan 11, 2019 at 09:49:29PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 11, 2019 at 03:21:39PM +0100, Linus Walleij wrote:

> I hope to get Hans' Tested-by on this.

Btw there is v2 of it with Mika's Ack (not enough, I know).
Hans de Goede Jan. 13, 2019, 7:50 a.m. UTC | #5
Hi,

On 11-01-19 20:49, Andy Shevchenko wrote:
> On Fri, Jan 11, 2019 at 03:21:39PM +0100, Linus Walleij wrote:
>> On Tue, Jan 8, 2019 at 7:02 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>
>>> New quirk enforces search for GPIO based on its type.
>>> Note, supplied index in the mapping table must be 0.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> (...)
>>
>>>   sound/soc/intel/boards/bytcr_rt5651.c | 74 ++++-----------------------
>>
>> Do we have an ACK from the maintainer of this file?
>> It's Hans I think. Ideally also from Mark Brown.
> 
> I hope to get Hans' Tested-by on this.

Yes I've testing this on my TODO.

Regards,

Hans

Patch
diff mbox series

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..7af06794e6fd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1020,6 +1020,8 @@  struct acpi_gpio_mapping {
 
 /* Ignore IoRestriction field */
 #define ACPI_GPIO_QUIRK_NO_IO_RESTRICTION	BIT(0)
+/* Choose one of GpioIo() type */
+#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..5bfbbdc2a62a 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. That's why quirk is enabled.
+	 */
+	{ "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);