All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] mfd: arizona: Move IRQ GPIO to GPIO descriptor
@ 2026-05-27 13:05 Linus Walleij
  2026-05-27 13:05 ` [PATCH v5 1/2] dt-bindings: mfd: wlf,arizona: Add irq-gpios Linus Walleij
  2026-05-27 13:05 ` [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
  0 siblings, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2026-05-27 13:05 UTC (permalink / raw)
  To: Lee Jones, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Charles Keepax
  Cc: patches, linux-kernel, linux-gpio, devicetree, Linus Walleij,
	Conor Dooley, Bartosz Golaszewski

Move the hack using a platform data-provided GPIO line to use
a proper devicetree-provided GPIO descriptor to poll the IRQ.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Changes in v5:
- Rebased on v7.1-rc1
- Cache gpio_irq in a local variable.
- Check gpio_irq for error value.
- Drop surplus goto label.
- Pick up review tags.
- Link to v4: https://lore.kernel.org/r/20260326-mfd-arizona-irq-v4-0-50c47ed0a18e@kernel.org

Changes in v4:
- Fix error path to use dev_err_probe() and report mismatches GPIO
  IRQ numbers properly.
- Fix spelling error in commit message.
- Pick up ACKs.
- Link to v3: https://lore.kernel.org/r/20260319-mfd-arizona-irq-v3-0-24c251377ec2@kernel.org

Changes in v3:
- Reword the irq-gpios documentation as Charles Want it.
- Collect ACK.
- Link to v2: https://lore.kernel.org/r/20260317-mfd-arizona-irq-v2-0-9ea546f517dc@kernel.org

Changes in v2:
- Instead of deleting the feature, migrate it properly to devicetree.
- Add irq-gpios devicetree bindings.
- Add code to handle irq-gpios for GPIO polling of IRQ line.
- Link to v1: https://lore.kernel.org/r/20260314-mfd-arizona-irq-v1-1-4b77f524f832@kernel.org

---
Linus Walleij (2):
      dt-bindings: mfd: wlf,arizona: Add irq-gpios
      mfd: arizona: Convert GPIO IRQ handling to descriptors

 .../devicetree/bindings/mfd/wlf,arizona.yaml       | 10 +++++
 drivers/mfd/arizona-irq.c                          | 50 ++++++++++++----------
 include/linux/mfd/arizona/core.h                   |  2 +
 include/linux/mfd/arizona/pdata.h                  |  5 ---
 4 files changed, 39 insertions(+), 28 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260314-mfd-arizona-irq-6fe7fe179a09

Best regards,
--  
Linus Walleij <linusw@kernel.org>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/2] dt-bindings: mfd: wlf,arizona: Add irq-gpios
  2026-05-27 13:05 [PATCH v5 0/2] mfd: arizona: Move IRQ GPIO to GPIO descriptor Linus Walleij
@ 2026-05-27 13:05 ` Linus Walleij
  2026-05-27 13:05 ` [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2026-05-27 13:05 UTC (permalink / raw)
  To: Lee Jones, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Charles Keepax
  Cc: patches, linux-kernel, linux-gpio, devicetree, Linus Walleij,
	Conor Dooley, Bartosz Golaszewski

The Wolfson Microelectronics Arizona sometimes needs to poll the
GPIO line corresponding to the IRQ to counter bugs in the hardware
that appear on edge-triggered IRQs.

For this situation, provide the optional irq-gpios property.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 Documentation/devicetree/bindings/mfd/wlf,arizona.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/wlf,arizona.yaml b/Documentation/devicetree/bindings/mfd/wlf,arizona.yaml
index 7902f3c5d289..359e40b3dacb 100644
--- a/Documentation/devicetree/bindings/mfd/wlf,arizona.yaml
+++ b/Documentation/devicetree/bindings/mfd/wlf,arizona.yaml
@@ -194,6 +194,16 @@ properties:
   reset-gpios:
     maxItems: 1
 
+  irq-gpios:
+    maxItems: 1
+    description:
+      A functional IRQ should be provided in the interrupts
+      property. This property enables edge triggered IRQ emulation
+      for hosts that don't support the codecs level triggered IRQ
+      output. The GPIO line corresponding to the IRQ will be polled
+      until all IRQs have been handled, ensuring an edge is generated
+      for the next IRQ.
+
   wlf,reset:
     description:
       GPIO specifier for the GPIO controlling RESET

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors
  2026-05-27 13:05 [PATCH v5 0/2] mfd: arizona: Move IRQ GPIO to GPIO descriptor Linus Walleij
  2026-05-27 13:05 ` [PATCH v5 1/2] dt-bindings: mfd: wlf,arizona: Add irq-gpios Linus Walleij
@ 2026-05-27 13:05 ` Linus Walleij
  2026-05-27 13:38   ` sashiko-bot
  2026-06-11  8:20   ` Lee Jones
  1 sibling, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2026-05-27 13:05 UTC (permalink / raw)
  To: Lee Jones, Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Charles Keepax
  Cc: patches, linux-kernel, linux-gpio, devicetree, Linus Walleij,
	Bartosz Golaszewski

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>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/mfd/arizona-irq.c         | 50 +++++++++++++++++++++------------------
 include/linux/mfd/arizona/core.h  |  2 ++
 include/linux/mfd/arizona/pdata.h |  5 ----
 3 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
index 544016d420fe..cf8e42662585 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);
@@ -210,6 +208,7 @@ int arizona_irq_init(struct arizona *arizona)
 	const struct regmap_irq_chip *aod, *irq;
 	struct irq_data *irq_data;
 	unsigned int virq;
+	int gpio_irq;
 
 	arizona->ctrlif_error = true;
 
@@ -350,27 +349,32 @@ 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);
+	/*
+	 * 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 = dev_err_probe(arizona->dev,
+				    PTR_ERR(arizona->irq_gpiod),
+				    "Error getting IRQ GPIO\n");
+		goto err_main_irq;
+	}
+	if (arizona->irq_gpiod) {
+		gpio_irq = gpiod_to_irq(arizona->irq_gpiod);
+		if (gpio_irq < 0) {
+			ret = dev_err_probe(arizona->dev, gpio_irq,
+					    "GPIO IRQ line is invalid\n");
+			goto err_main_irq;
 		}
-
-		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;
+		if (gpio_irq != arizona->irq) {
+			dev_warn(arizona->dev, "IRQ %d does not match GPIO's IRQ %d\n",
+				 arizona->irq, gpio_irq);
+			arizona->irq = gpio_irq;
 		}
+		gpiod_set_consumer_name(arizona->irq_gpiod, "arizona IRQ");
 	}
-#endif
 
 	ret = request_threaded_irq(arizona->irq, NULL, arizona_irq_thread,
 				   flags, "arizona", arizona);
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 d465dcd8c90a..2f980d51cdfa 100644
--- a/include/linux/mfd/arizona/pdata.h
+++ b/include/linux/mfd/arizona/pdata.h
@@ -178,11 +178,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.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors
  2026-05-27 13:05 ` [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
@ 2026-05-27 13:38   ` sashiko-bot
  2026-06-11  8:20   ` Lee Jones
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-27 13:38 UTC (permalink / raw)
  To: Linus Walleij; +Cc: robh, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Migration from legacy gpio to gpiod incorrectly replaces physical line reads with logical value reads, breaking IRQ polling logic for active-low GPIOs.

Pre-existing issues:
- [High] Negative error codes from GPIO reads are implicitly treated as boolean true, causing an infinite loop in the IRQ thread on read failure.
--

commit 2b527ef2c7b9b60397ceb6125cd15341ac4e28ff
Author: Linus Walleij <linusw@kernel.org>

mfd: arizona: Convert GPIO IRQ handling to descriptors

This commit converts the arizona polling GPIO handling to use a GPIO descriptor
instead of passing a global GPIO number as platform data.

> diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
> index 544016d420fe2..cf8e42662585a 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)
[ ... ]
>  		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)) {

[Severity: High]
Does this incorrectly check the logical value of the GPIO instead of the raw
physical state?

The legacy gpio_get_value_cansleep() returned the raw physical state of the
pin. The new gpiod_get_value_cansleep() returns the logical state, which
incorporates the ACTIVE_LOW flag from the devicetree or software node.

Because the IRQ trigger flags describe physical voltage transitions, comparing
these physical flags against a logical value might be incorrect. For example,
if a user defines irq-gpios with GPIO_ACTIVE_LOW for a falling edge interrupt,
a physically low line becomes logically 1, causing the polling condition
!gpiod_get_value_cansleep() to evaluate to 0, which fails to poll when the
interrupt is active.

Conversely, when the line is idle, it evaluates to 1, causing the IRQ handler
to poll infinitely.

Should this use gpiod_get_raw_value_cansleep() to preserve the original
physical semantics?

[Severity: High]
This is a pre-existing issue, but can this loop forever if the GPIO read
returns a negative error code?

gpiod_get_value_cansleep() (and its legacy predecessor) can return a negative
error code (e.g., -EIO) if reading the GPIO state fails over the bus. Since
negative values are non-zero, this evaluates to true for IRQF_TRIGGER_RISING,
setting poll = true and causing an infinite loop in the IRQ handler thread if
the read continues to fail.

Conversely, for IRQF_TRIGGER_FALLING, !(-EIO) evaluates to false, which
immediately aborts the poll.

Could the return value be stored in an integer and explicitly checked for
negative errors before evaluating it as a boolean?

>  			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;
>  		}
>  	} while (poll);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-mfd-arizona-irq-v5-0-ebeda2e925da@kernel.org?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors
  2026-05-27 13:05 ` [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
  2026-05-27 13:38   ` sashiko-bot
@ 2026-06-11  8:20   ` Lee Jones
  2026-06-11  9:03     ` Charles Keepax
  1 sibling, 1 reply; 6+ messages in thread
From: Lee Jones @ 2026-06-11  8:20 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Charles Keepax, patches, linux-kernel, linux-gpio,
	devicetree, Bartosz Golaszewski

Couple of Sashiko issues I'd like you to comment on please buddy.

On Wed, 27 May 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>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  drivers/mfd/arizona-irq.c         | 50 +++++++++++++++++++++------------------
>  include/linux/mfd/arizona/core.h  |  2 ++
>  include/linux/mfd/arizona/pdata.h |  5 ----
>  3 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
> index 544016d420fe..cf8e42662585 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)) {

[Severity: High]
Does this incorrectly check the logical value of the GPIO instead of the raw
physical state?

The legacy gpio_get_value_cansleep() returned the raw physical state of the
pin. The new gpiod_get_value_cansleep() returns the logical state, which
incorporates the ACTIVE_LOW flag from the devicetree or software node.

Because the IRQ trigger flags describe physical voltage transitions, comparing
these physical flags against a logical value might be incorrect. For example,
if a user defines irq-gpios with GPIO_ACTIVE_LOW for a falling edge interrupt,
a physically low line becomes logically 1, causing the polling condition
!gpiod_get_value_cansleep() to evaluate to 0, which fails to poll when the
interrupt is active.

Conversely, when the line is idle, it evaluates to 1, causing the IRQ handler
to poll infinitely.

Should this use gpiod_get_raw_value_cansleep() to preserve the original
physical semantics?



[Severity: High]
This is a pre-existing issue, but can this loop forever if the GPIO read
returns a negative error code?

gpiod_get_value_cansleep() (and its legacy predecessor) can return a negative
error code (e.g., -EIO) if reading the GPIO state fails over the bus. Since
negative values are non-zero, this evaluates to true for IRQF_TRIGGER_RISING,
setting poll = true and causing an infinite loop in the IRQ handler thread if
the read continues to fail.

Conversely, for IRQF_TRIGGER_FALLING, !(-EIO) evaluates to false, which
immediately aborts the poll.

Could the return value be stored in an integer and explicitly checked for
negative errors before evaluating it as a boolean?


>  			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);
> @@ -210,6 +208,7 @@ int arizona_irq_init(struct arizona *arizona)
>  	const struct regmap_irq_chip *aod, *irq;
>  	struct irq_data *irq_data;
>  	unsigned int virq;
> +	int gpio_irq;
>  
>  	arizona->ctrlif_error = true;
>  
> @@ -350,27 +349,32 @@ 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);
> +	/*
> +	 * 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 = dev_err_probe(arizona->dev,
> +				    PTR_ERR(arizona->irq_gpiod),
> +				    "Error getting IRQ GPIO\n");
> +		goto err_main_irq;
> +	}
> +	if (arizona->irq_gpiod) {
> +		gpio_irq = gpiod_to_irq(arizona->irq_gpiod);
> +		if (gpio_irq < 0) {
> +			ret = dev_err_probe(arizona->dev, gpio_irq,
> +					    "GPIO IRQ line is invalid\n");
> +			goto err_main_irq;
>  		}
> -
> -		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;
> +		if (gpio_irq != arizona->irq) {
> +			dev_warn(arizona->dev, "IRQ %d does not match GPIO's IRQ %d\n",
> +				 arizona->irq, gpio_irq);
> +			arizona->irq = gpio_irq;
>  		}
> +		gpiod_set_consumer_name(arizona->irq_gpiod, "arizona IRQ");
>  	}
> -#endif
>  
>  	ret = request_threaded_irq(arizona->irq, NULL, arizona_irq_thread,
>  				   flags, "arizona", arizona);
> 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 d465dcd8c90a..2f980d51cdfa 100644
> --- a/include/linux/mfd/arizona/pdata.h
> +++ b/include/linux/mfd/arizona/pdata.h
> @@ -178,11 +178,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.54.0
> 

-- 
Lee Jones

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors
  2026-06-11  8:20   ` Lee Jones
@ 2026-06-11  9:03     ` Charles Keepax
  0 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2026-06-11  9:03 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, patches, linux-kernel,
	linux-gpio, devicetree, Bartosz Golaszewski

On Thu, Jun 11, 2026 at 09:20:00AM +0100, Lee Jones wrote:
> On Wed, 27 May 2026, Linus Walleij wrote:
> > -			   gpio_get_value_cansleep(arizona->pdata.irq_gpio)) {
> > +			   gpiod_get_value_cansleep(arizona->irq_gpiod)) {
> 
> [Severity: High]
> Does this incorrectly check the logical value of the GPIO instead of the raw
> physical state?
> 
> The legacy gpio_get_value_cansleep() returned the raw physical state of the
> pin. The new gpiod_get_value_cansleep() returns the logical state, which
> incorporates the ACTIVE_LOW flag from the devicetree or software node.
> 
> Because the IRQ trigger flags describe physical voltage transitions, comparing
> these physical flags against a logical value might be incorrect. For example,
> if a user defines irq-gpios with GPIO_ACTIVE_LOW for a falling edge interrupt,
> a physically low line becomes logically 1, causing the polling condition
> !gpiod_get_value_cansleep() to evaluate to 0, which fails to poll when the
> interrupt is active.
> 
> Conversely, when the line is idle, it evaluates to 1, causing the IRQ handler
> to poll infinitely.
> 
> Should this use gpiod_get_raw_value_cansleep() to preserve the original
> physical semantics?

Hmm.. yeah that is a good spot we should probably use the
raw_value version here as we presumably can't trust that the
device trees specify the ACTIVE_LOW/HIGH flags correctly.

> [Severity: High]
> This is a pre-existing issue, but can this loop forever if the GPIO read
> returns a negative error code?
> 
> gpiod_get_value_cansleep() (and its legacy predecessor) can return a negative
> error code (e.g., -EIO) if reading the GPIO state fails over the bus. Since
> negative values are non-zero, this evaluates to true for IRQF_TRIGGER_RISING,
> setting poll = true and causing an infinite loop in the IRQ handler thread if
> the read continues to fail.
> 
> Conversely, for IRQF_TRIGGER_FALLING, !(-EIO) evaluates to false, which
> immediately aborts the poll.
> 
> Could the return value be stored in an integer and explicitly checked for
> negative errors before evaluating it as a boolean?

This is also fair, although given this feature is only really
supporting some quite old things that have been doing this for
ages I would be inclined to not stress too much about this.

Thanks,
Charles

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-11  9:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 13:05 [PATCH v5 0/2] mfd: arizona: Move IRQ GPIO to GPIO descriptor Linus Walleij
2026-05-27 13:05 ` [PATCH v5 1/2] dt-bindings: mfd: wlf,arizona: Add irq-gpios Linus Walleij
2026-05-27 13:05 ` [PATCH v5 2/2] mfd: arizona: Convert GPIO IRQ handling to descriptors Linus Walleij
2026-05-27 13:38   ` sashiko-bot
2026-06-11  8:20   ` Lee Jones
2026-06-11  9:03     ` Charles Keepax

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.