Re: [PATCH v2] gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2

[Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>]

On Mon, May 25, 2020 at 10:08:45PM +0300, Andy Shevchenko wrote:
> 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.
> 
> Suggested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> v2: did everything in the driver (Mika)
>  drivers/gpio/gpio-pca953x.c | 65 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 1fca8dd7824f..0d30f19782a2 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -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));

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

Otherwise looks good.