All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Linus Walleij <linusw@kernel.org>
Cc: Bartosz Golaszewski <brgl@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Charles Keepax <ckeepax@opensource.cirrus.com>,
	patches@opensource.cirrus.com, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors
Date: Wed, 25 Mar 2026 17:20:36 +0000	[thread overview]
Message-ID: <20260325172036.GF1141718@google.com> (raw)
In-Reply-To: <20260319-mfd-arizona-irq-v3-2-24c251377ec2@kernel.org>

On Thu, 19 Mar 2026, Linus Walleij wrote:

> Convert the arizona polling GPIO handling to use a GPIO descripor
> instead of passing a global GPIO number as platform data.

s/descripor/descriptor/

> 
> 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         | 45 ++++++++++++++++++---------------------
>  include/linux/mfd/arizona/core.h  |  2 ++
>  include/linux/mfd/arizona/pdata.h |  5 -----
>  3 files changed, 23 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
> index 544016d420fe..b6f63031268e 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,25 @@ 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.
> +	 */
> +	arizona->irq_gpiod = devm_gpiod_get_optional(arizona->dev, "irq",
> +						     GPIOD_IN);
> +	if (IS_ERR(arizona->irq_gpiod)) {
> +		ret = PTR_ERR(arizona->irq_gpiod);
> +		dev_err(arizona->dev, "error getting IRQ GPIO (%d)\n", ret);

Since this function is called from the probe path and may return
-EPROBE_DEFER, should we use `dev_err_probe()` here to handle deferred
probing correctly?

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

How about a little for info for the user:

  "IRQ %d does not match GPIO's IRQ %d"

> +			arizona->irq = gpiod_to_irq(arizona->irq_gpiod);
>  		}
> +		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 +405,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:
>  	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 [李琼斯]

      reply	other threads:[~2026-03-25 17:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-19 18:50 [PATCH v3 0/2] mfd: arizona: Move IRQ GPIO to GPIO descriptor Linus Walleij
2026-03-19 18:50 ` [PATCH v3 1/2] dt-bindings: mfd: wlf,arizona: Add irq-gpios Linus Walleij
2026-03-20  9:24   ` Charles Keepax
2026-03-20 17:36   ` Conor Dooley
2026-03-19 18:50 ` [PATCH v3 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
2026-03-25 17:20   ` Lee Jones [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260325172036.GF1141718@google.com \
    --to=lee@kernel.org \
    --cc=brgl@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.