gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2

PATCH Superseded Andy Shevchenko 3 participants 6 messages May 25, 2020 – May 27, 2020 GPIO

Share & tools

Andy Shevchenko

author

2020-05-25 19:08 UTC #0 lore raw

PATCH v2 1/1 gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2

ACPI table on Intel Galileo Gen 2 has wrong pin number for IRQ resource of one of the I²C GPIO expanders. Since we know what that number is and luckily have GPIO bases fixed for SoC's controllers, we may use a simple DMI quirk to match the platform and retrieve GpioInt() pin on it for the expander in question.

Signed-off-by Andy Shevchenko <[email protected]>

---
v2: did everything in the driver (Mika)

 drivers/gpio/gpio-pca953x.c | 65 +++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

drivers/gpio/gpio-pca953x.c +65 -0

index 1fca8dd7824f..0d30f19782a2 100644
@@ -10,6 +10,7 @@
 
 #include <linux/acpi.h>
 #include <linux/bitmap.h>
+#include <linux/dmi.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>

@@ -107,6 +108,62 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+#ifdef CONFIG_GPIO_PCA953X_IRQ
+static const struct dmi_system_id pca953x_dmi_acpi_irq_info[] = {
+	{
+		/*
+		 * On Intel Galileo Gen 2 board the IRQ pin of one of
+		 * the I²C GPIO expanders, which has GpioInt() resource,
+		 * is provided as an absolute number instead of being
+		 * relative. Since first controller (gpio-sch.c) and
+		 * second (gpio-dwapb.c) are at the fixed bases, we may
+		 * safely refer to the number in the global space to get
+		 * an IRQ out of it.
+		 */
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
+		},
+	},
+	{}
+};
+
+#ifdef CONFIG_ACPI
+static acpi_status pca953x_acpi_get_pin(struct acpi_resource *ares, void *data)
+{
+	struct acpi_resource_gpio *agpio;
+	int *pin = data;
+
+	if (!acpi_gpio_get_irq_resource(ares, &agpio))
+		return AE_OK;
+
+	*pin = agpio->pin_table[0];
+	return AE_CTRL_TERMINATE;
+}
+
+static int pca953x_acpi_find_pin(acpi_handle handle)
+{
+	int p = -ENOENT;
+
+	acpi_walk_resources(handle, METHOD_NAME__CRS, pca953x_acpi_get_pin, &p);
+	return p;
+}
+#else
+static inline int pca953x_acpi_find_pin(acpi_handle handle) { return -ENXIO; }
+#endif
+
+static int pca953x_acpi_get_irq(struct device *dev)
+{
+	int pin;
+
+	pin = pca953x_acpi_find_pin(ACPI_HANDLE(dev));
+	if (pin < 0)
+		return pin;
+
+	dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+	return gpiod_to_irq(gpio_to_desc(pin));
+}
+#endif
+
 static const struct acpi_device_id pca953x_acpi_ids[] = {
 	{ "INT3491", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
 	{ }

@@ -750,8 +807,16 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base)
 	struct irq_chip *irq_chip = &chip->irq_chip;
 	DECLARE_BITMAP(reg_direction, MAX_LINE);
 	DECLARE_BITMAP(irq_stat, MAX_LINE);
+	const struct dmi_system_id *id;
 	int ret;
 
+	id = dmi_first_match(pca953x_dmi_acpi_irq_info);
+	if (id) {
+		ret = pca953x_acpi_get_irq(&client->dev);
+		if (ret > 0)
+			client->irq = ret;
+	}
+
 	if (!client->irq)
 		return 0;

Signature · 1 line

2.26.2

Mika Westerberg

2020-05-26 05:31 UTC #1 lore raw

Quoted 72 lines · Andy Shevchenko

Signed-off-by Andy Shevchenko <[email protected]>

---
v2: did everything in the driver (Mika)

 drivers/gpio/gpio-pca953x.c | 65 +++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

drivers/gpio/gpio-pca953x.c +44 -0

index 1fca8dd7824f..0d30f19782a2 100644
@@ -10,6 +10,7 @@
 
 #include <linux/acpi.h>
 #include <linux/bitmap.h>
+#include <linux/dmi.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>

@@ -107,6 +108,62 @@ static const struct i2c_device_id pca953x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
 
+#ifdef CONFIG_GPIO_PCA953X_IRQ
+static const struct dmi_system_id pca953x_dmi_acpi_irq_info[] = {
+	{
+		/*
+		 * On Intel Galileo Gen 2 board the IRQ pin of one of
+		 * the I²C GPIO expanders, which has GpioInt() resource,
+		 * is provided as an absolute number instead of being
+		 * relative. Since first controller (gpio-sch.c) and
+		 * second (gpio-dwapb.c) are at the fixed bases, we may
+		 * safely refer to the number in the global space to get
+		 * an IRQ out of it.
+		 */
+		.matches = {
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "GalileoGen2"),
+		},
+	},
+	{}
+};
+
+#ifdef CONFIG_ACPI
+static acpi_status pca953x_acpi_get_pin(struct acpi_resource *ares, void *data)
+{
+	struct acpi_resource_gpio *agpio;
+	int *pin = data;
+
+	if (!acpi_gpio_get_irq_resource(ares, &agpio))
+		return AE_OK;
+
+	*pin = agpio->pin_table[0];
+	return AE_CTRL_TERMINATE;
+}
+
+static int pca953x_acpi_find_pin(acpi_handle handle)
+{
+	int p = -ENOENT;
+
+	acpi_walk_resources(handle, METHOD_NAME__CRS, pca953x_acpi_get_pin, &p);
+	return p;
+}
+#else
+static inline int pca953x_acpi_find_pin(acpi_handle handle) { return -ENXIO; }
+#endif
+

drivers/gpio/gpio-pca953x.c static const struct i2c_device_id pca953x_id[] = { · line 163

Quoted 10 lines

drivers/gpio/gpio-pca953x.c +10 -0

+static int pca953x_acpi_get_irq(struct device *dev)
+{
+	int pin;
+
+	pin = pca953x_acpi_find_pin(ACPI_HANDLE(dev));
+	if (pin < 0)
+		return pin;
+
+	dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+	return gpiod_to_irq(gpio_to_desc(pin));

You need to request the GPIO as well, I missed that from my example. Sorry about that.

Otherwise looks good.

Andy Shevchenko

author

2020-05-26 13:11 UTC #2 lore raw

Quoted 1 line · Mika Westerberg

On Mon, May 25, 2020 at 10:08:45PM +0300, Andy Shevchenko wrote:

...

Quoted 4 lines

drivers/gpio/gpio-pca953x.c static const struct i2c_device_id pca953x_id[] = { · line 163
Quoted 2 lines
drivers/gpio/gpio-pca953x.c +2 -0
+	dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+	return gpiod_to_irq(gpio_to_desc(pin));
You need to request the GPIO as well, I missed that from my example.

How? I can't find a function to request GPIO by its descriptor in include/linux/gpio/*.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Mika Westerberg

2020-05-26 13:34 UTC #3 lore raw

Quoted 5 lines · Andy Shevchenko

Quoted 1 line · Mika Westerberg
On Mon, May 25, 2020 at 10:08:45PM +0300, Andy Shevchenko wrote:
...

Quoted 7 lines

Quoted 4 lines
drivers/gpio/gpio-pca953x.c static const struct i2c_device_id pca953x_id[] = { · line 163
Quoted 2 lines
drivers/gpio/gpio-pca953x.c +2 -0
+	dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+	return gpiod_to_irq(gpio_to_desc(pin));
You need to request the GPIO as well, I missed that from my example.
How? I can't find a function to request GPIO by its descriptor in include/linux/gpio/*.

Not by descriptor but by number which you are already using here anyway. The function that does that is gpio_request(). I know we should use GPIO descriptors everywhere but this is a special case for a special hardware with a broken firmware, so IMHO it should be fine ;-) Once the legacy API is gets removed this whole hack can be removed as well and nobody will notice.

Linus Walleij

2020-05-27 05:43 UTC #4 lore raw

In reply to Andy Shevchenko

Quoted 5 lines · Andy Shevchenko

Quoted 1 line · Mika Westerberg
On Mon, May 25, 2020 at 10:08:45PM +0300, Andy Shevchenko wrote:
...

Quoted 7 lines

Quoted 4 lines
drivers/gpio/gpio-pca953x.c static const struct i2c_device_id pca953x_id[] = { · line 163
Quoted 2 lines
drivers/gpio/gpio-pca953x.c +2 -0
+   dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+   return gpiod_to_irq(gpio_to_desc(pin));
You need to request the GPIO as well, I missed that from my example.
How? I can't find a function to request GPIO by its descriptor in include/linux/gpio/*.

If a gpiochip need to request one of its own GPIOs it should use gpiochip_request_own_desc() but I don't know if you have all info at hand to use that in this driver.

Yours,
Linus Walleij

Andy Shevchenko

author

2020-05-27 09:10 UTC #5 lore raw

Quoted 10 lines · Linus Walleij

Quoted 8 lines · Andy Shevchenko
Quoted 1 line · Mika Westerberg
On Mon, May 25, 2020 at 10:08:45PM +0300, Andy Shevchenko wrote:
...
Quoted 3 lines
Quoted 2 lines
drivers/gpio/gpio-pca953x.c +2 -0
+   dev_info(dev, "Applying ACPI interrupt quirk (GPIO %d)\n", pin);
+   return gpiod_to_irq(gpio_to_desc(pin));

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
You need to request the GPIO as well, I missed that from my example.
How? I can't find a function to request GPIO by its descriptor in include/linux/gpio/*.
If a gpiochip need to request one of its own GPIOs it should use gpiochip_request_own_desc()

It's not the case here.

Signature · 2 lines

With Best Regards,
Andy Shevchenko