Re: [PATCH v4 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors

Lee Jones
2026-04-09T14:21:17Z

On Thu, 26 Mar 2026, Linus Walleij wrote:

> Convert the arizona polling GPIO handling to use a GPIO descriptor
> instead of passing a global GPIO number as platform data.
>
> This mechanism is not used in the kernel, but let's preserve
> the mechanism to be nice.
>
> Users can define "irq-gpios" in the devicetree or software node
> for the Arizona chip to provide the GPIO line corresponding to
> the IRQ.
>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
> drivers/mfd/arizona-irq.c | 46 +++++++++++++++++++--------------------
> include/linux/mfd/arizona/core.h | 2 ++
> include/linux/mfd/arizona/pdata.h | 5 -----
> 3 files changed, 24 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
> index 544016d420fe..8b752a1257b1 100644
> --- a/drivers/mfd/arizona-irq.c
> +++ b/drivers/mfd/arizona-irq.c
> @@ -136,21 +136,19 @@ static irqreturn_t arizona_irq_thread(int irq, void *data)
> dev_err(arizona->dev,
> "Failed to read main IRQ status: %d\n", ret);
> }
> -#ifdef CONFIG_GPIOLIB_LEGACY
> /*
> * Poll the IRQ pin status to see if we're really done
> * if the interrupt controller can't do it for us.
> */
> - if (!arizona->pdata.irq_gpio) {
> + if (!arizona->irq_gpiod) {
> break;
> } else if (arizona->pdata.irq_flags & IRQF_TRIGGER_RISING &&
> - gpio_get_value_cansleep(arizona->pdata.irq_gpio)) {
> + gpiod_get_value_cansleep(arizona->irq_gpiod)) {
> poll = true;
> } else if (arizona->pdata.irq_flags & IRQF_TRIGGER_FALLING &&
> - !gpio_get_value_cansleep(arizona->pdata.irq_gpio)) {
> + !gpiod_get_value_cansleep(arizona->irq_gpiod)) {
> poll = true;
> }
> -#endif
> } while (poll);
>
> pm_runtime_put_autosuspend(arizona->dev);
> @@ -350,27 +348,26 @@ int arizona_irq_init(struct arizona *arizona)
> goto err_map_main_irq;
> }
>
> -#ifdef CONFIG_GPIOLIB_LEGACY
> - /* Used to emulate edge trigger and to work around broken pinmux */
> - if (arizona->pdata.irq_gpio) {
> - if (gpio_to_irq(arizona->pdata.irq_gpio) != arizona->irq) {
> - dev_warn(arizona->dev, "IRQ %d is not GPIO %d (%d)\n",
> - arizona->irq, arizona->pdata.irq_gpio,
> - gpio_to_irq(arizona->pdata.irq_gpio));
> - arizona->irq = gpio_to_irq(arizona->pdata.irq_gpio);
> - }
> -
> - ret = devm_gpio_request_one(arizona->dev,
> - arizona->pdata.irq_gpio,
> - GPIOF_IN, "arizona IRQ");
> - if (ret != 0) {
> - dev_err(arizona->dev,
> - "Failed to request IRQ GPIO %d:: %d\n",
> - arizona->pdata.irq_gpio, ret);
> - arizona->pdata.irq_gpio = 0;
> + /*
> + * Used to emulate edge trigger and to work around broken pinmux
> + * define "irq-gpios" in device tree or software node.
> + */

Nit: Device Tree (not sure about software node).

> + arizona->irq_gpiod = devm_gpiod_get_optional(arizona->dev, "irq",
> + GPIOD_IN);
> + if (IS_ERR(arizona->irq_gpiod)) {
> + ret = dev_err_probe(arizona->dev,
> + PTR_ERR(arizona->irq_gpiod),
> + "error getting IRQ GPIO\n");

Nit: All other prints in this file are capitalised.

> + goto err_irq_gpiod;
> + }
> + if (arizona->irq_gpiod) {
> + if (gpiod_to_irq(arizona->irq_gpiod) != arizona->irq) {
> + dev_warn(arizona->dev, "IRQ %d does not match GPIO's IRQ %d\n",
> + arizona->irq, gpiod_to_irq(arizona->irq_gpiod));
> + arizona->irq = gpiod_to_irq(arizona->irq_gpiod);

Could we cache the result of 'gpiod_to_irq()' into a local variable rather than
calling it multiple times?

Does it make sense to check the value's for errors?

> }
> + gpiod_set_consumer_name(arizona->irq_gpiod, "arizona IRQ");
> }
> -#endif
>
> ret = request_threaded_irq(arizona->irq, NULL, arizona_irq_thread,
> flags, "arizona", arizona);
> @@ -409,6 +406,7 @@ int arizona_irq_init(struct arizona *arizona)
> arizona_free_irq(arizona, ARIZONA_IRQ_BOOT_DONE, arizona);
> err_boot_done:
> free_irq(arizona->irq, arizona);
> +err_irq_gpiod:
> err_main_irq:

Do we really need another goto label that jumps to the same place?

> regmap_del_irq_chip(irq_find_mapping(arizona->virq,
> ARIZONA_MAIN_IRQ_INDEX),
> diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
> index 6d6f96b2b29f..7129651c9af6 100644
> --- a/include/linux/mfd/arizona/core.h
> +++ b/include/linux/mfd/arizona/core.h
> @@ -11,6 +11,7 @@
> #define _WM_ARIZONA_CORE_H
>
> #include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/interrupt.h>
> #include <linux/notifier.h>
> #include <linux/regmap.h>
> @@ -136,6 +137,7 @@ struct arizona {
> struct irq_domain *virq;
> struct regmap_irq_chip_data *aod_irq_chip;
> struct regmap_irq_chip_data *irq_chip;
> + struct gpio_desc *irq_gpiod;
>
> bool hpdet_clamp;
> unsigned int hp_ena;
> diff --git a/include/linux/mfd/arizona/pdata.h b/include/linux/mfd/arizona/pdata.h
> index f72e6d4b14a7..20118bad869a 100644
> --- a/include/linux/mfd/arizona/pdata.h
> +++ b/include/linux/mfd/arizona/pdata.h
> @@ -188,11 +188,6 @@ struct arizona_pdata {
> /** Haptic actuator type */
> unsigned int hap_act;
>
> -#ifdef CONFIG_GPIOLIB_LEGACY
> - /** GPIO for primary IRQ (used for edge triggered emulation) */
> - int irq_gpio;
> -#endif
> -
> /** General purpose switch control */
> unsigned int gpsw;
> };
>
> --
> 2.53.0
>

--
Lee Jones [李琼斯]