* [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support
@ 2026-10-27 4:18 Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 01/11] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
` (10 more replies)
0 siblings, 11 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel,
Conor Dooley, Chanho Min, Mika Westerberg, linux-acpi
The ambarella vendor prefix [1] is in linux-next (applied by Rob).
The binding can be applied with this series.
Please take this series through the GPIO tree. The quirk, the PL061
conversion and the Ambarella layout belong together.
This is v12 of the GPIO patches from the combined CV75 series. The
posting on Oct 9 numbered them v1 after the split. That was a
mistake.
These patches convert the ARM PL061 data path to regmap, then add
the CV75 layout. The series now carries a gpiolib patch that
requests ACPI GPIO events for gpiochips using an external irq
domain.
GPIO data path uses gpio-regmap with two layouts (ARM PrimeCell and
Ambarella). ARM PL061 sets GPIO_REGMAP_QUIRK_SET_AFTER_DIR. IRQ stays
a custom PL061 irqchip. The linear domain is instantiated with
devm_irq_domain_instantiate() using the PL061 private data and
attached by gpio-regmap. regmap-irq is not used.
Tested with lockdep enabled on the CV75 EVK. The board boots.
gpio-hammer toggles gpiochip0 line 26 ten times, and gpio-event-mon
on line 27 records alternating rising and falling edges. unbind and
bind work; the chip re-registers and both tools work again after
the rebind.
Changes in v12:
- gpiolib: request ACPI GPIO events when a gpiochip gets an
external irq domain, matching gpiochip_add_irqchip(). A gpiochip
that gets its IRQ domain through gpiochip_irqchip_add_domain()
never went through that path, so _AEI events, such as the power
button on ARM64 ACPI platforms, were not requested (Sashiko).
- switch: give the child IRQs their own lockdep classes (the gc->irq
keys stay NULL on the external irq_domain path), and clear the
chained handler only after it was installed. The previous mix-up
could leave a live IRQ with no chained handler after a failed
probe. Checked with lockdep on the CV75 board (Sashiko).
- switch: keep not setting the gpiochip IRQ resource helpers. The
gpio_chip is not reachable through gpio-regmap's external
irq_domain path, so GPIOD_FLAG_USED_AS_IRQ is never tracked. This
matches the other users of that path (gpio-idio-16,
gpio-pcie-idio-24, gpio-104-idi-48) (Sashiko).
- convert: put the locals in the blocks this patch touches into
reverse Christmas tree order, and keep regmap_assign_bits() in
pl061_set_value() like pl061_get_value() (Andy).
- switch: leave the chained handler's regmap_read() unchecked.
mis stays 0 on failure, so the loop is already a no-op. Call
irq_dispose_mapping() directly (Andy).
- pl061: join the split dev_err() string, and use str_high_low()
for the level debug line. str_rising_falling() is not in
linux-next, so the edge line is unchanged (Andy).
- Link to v11: https://patch.msgid.link/20261020-cv75-gpio-v11-0-e058bae19571@ambarella.com
Changes in v11 (from Andy's v10 review):
- pending: keep generic_handle_domain_irq() on one line.
- convert: fit regmap_test_bits() on two lines, return
regmap_assign_bits() directly, and put the new locals in
reverse Christmas tree order.
- cleanup: fix the guard()() reference in the commit message and
use scoped_guard() in pl061_irq_mask()/unmask().
- pl061: keep gc->parent in a local before the gpio-regmap switch,
so that patch does not rewrite every log line.
- switch: keep the existing GPIO* register macros. Split
reg_mask_xlate, drop pl061->dev for regmap_get_device(), and
instantiate the IRQ domain with devm_irq_domain_instantiate().
The remove action is registered before the gpiochip so devres
unwinds it after gpiochip_remove() and before the domain removal.
- Drop the flags variable left unused by the guard() conversion.
- unbind/bind cycle tested on the CV75 EVK.
- Link to v10: https://patch.msgid.link/20261015-cv75-gpio-v10-0-3fe3eb315a55@ambarella.com
Changes since the combined v9 [2]:
- dt-bindings: explain that CV75 is a modified PL061 (Conor).
- gpio-regmap: move quirks after fixed_direction_output (Andy).
- gpio: pl061: Andy's review on the regmap conversion and switch:
use regmap_test_bits()/clear_bits()/set_bits()/assign_bits(),
drop the redundant if (pending) and fast_io, introduce the ARM
regmap_config (including REGCACHE_FLAT_S) in the convert patch
and leave it there, blank line after guard() plus the blank-line
and reverse Christmas tree cleanups, and keep the upstream
pl061_irq_chip name and unsigned irq_type() trigger without
adding .name.
- gpio: pl061: cache DAT (REGCACHE_FLAT_S, not volatile) so
gpio_set() does not read-modify-write the pin level (Sashiko).
Changes since the Oct 9 GPIO posting [3]:
- gpio: pl061: Andy's follow-ups since that posting: assign_bits
for DAT writes, drop fast_io, name the ARM regmap_config in the
convert patch, the guard()/blank-line/irq_type cleanups, and keep
the upstream pl061_irq_chip name and unsigned trigger without
adding .name.
- gpiolib: regmap: add Michael Walle's Reviewed-by on
GPIO_REGMAP_QUIRK_SET_AFTER_DIR.
- gpio: pl061: restore Andy's and Linus's Reviewed-by on the
Ambarella variant.
[1] https://patch.msgid.link/20261014-cv75-prefix-v1-1-fe95ed567880@ambarella.com
[2] https://patch.msgid.link/20261001-cv75-v5-v9-0-486cc289a4b8@ambarella.com
[3] https://patch.msgid.link/20261009-cv75-gpio-v1-0-5836bd438423@ambarella.com
To: Bartosz Golaszewski <brgl@kernel.org>
To: linux-gpio@vger.kernel.org
Cc: Linus Walleij <linusw@kernel.org>
Cc: Michael Walle <mwalle@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: Long Zhao <longzhao@ambarella.com>
Cc: Long Zhao <zl020895@163.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
Long Zhao (11):
dt-bindings: gpio: pl061: add Ambarella CV75 variant
gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
gpio: pl061: drop redundant pending check in the IRQ handler
gpio: pl061: convert register access to regmap
gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH
gpio: pl061: use cleanup helpers for locks
gpio: pl061: keep the parent device in a local
gpiolib: request ACPI GPIO events for external IRQ domains
gpio: pl061: switch data path to gpio-regmap
gpio: pl061: add Ambarella register-layout variant
gpio: pl061: tidy up IRQ trigger logging
.../devicetree/bindings/gpio/pl061-gpio.yaml | 25 +-
drivers/gpio/Kconfig | 2 +
drivers/gpio/gpio-pl061.c | 543 ++++++++++++++-------
drivers/gpio/gpio-regmap.c | 15 +-
drivers/gpio/gpiolib.c | 10 +-
include/linux/amba/bus.h | 1 +
include/linux/gpio/regmap.h | 14 +
7 files changed, 415 insertions(+), 195 deletions(-)
---
base-commit: 34cf6dafc47441dfb6b356a095b89c3585a93714
change-id: 20261007-cv75-gpio-333d7c1dfe11
Best regards,
--
Long Zhao <longzhao@ambarella.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 01/11] dt-bindings: gpio: pl061: add Ambarella CV75 variant
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 02/11] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
` (9 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel,
Conor Dooley
From: Long Zhao <longzhao@ambarella.com>
The CV75 GPIO is a modified PL061. The basic registers have the same
names and roles (DAT/DIR/IS/IBE/IEV/IE/MIS/IC). Each bank is 32-bit
and 32 lines and adds MASK and ENABLE, so it is not
register-compatible with the ARM PL061 and arm,pl061 must not be
used as a fallback.
The AMBA core matches the PL061 driver by PrimeCell ID, not by the DT
compatible string. These blocks do not implement the standard PL061 ID
(0x00041061) in the ID registers, so arm,primecell-periphid is required
and is fixed to 0x000e8061; the driver uses that ID to select the
Ambarella layout. gpio-ranges is required because each bank is a slice
of the SoC pinctrl pin space.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
.../devicetree/bindings/gpio/pl061-gpio.yaml | 25 +++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml b/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
index 4d970e55104b..c22dd8a1d2c1 100644
--- a/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
@@ -15,7 +15,9 @@ select:
properties:
compatible:
contains:
- const: arm,pl061
+ enum:
+ - arm,pl061
+ - ambarella,cv75-gpio
required:
- compatible
@@ -25,7 +27,9 @@ properties:
compatible:
items:
- - const: arm,pl061
+ - enum:
+ - ambarella,cv75-gpio
+ - arm,pl061
- const: arm,primecell
reg:
@@ -64,6 +68,21 @@ required:
- "#gpio-cells"
- gpio-controller
-additionalProperties: false
+allOf:
+ - $ref: /schemas/arm/primecell.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ambarella,cv75-gpio
+ then:
+ properties:
+ arm,primecell-periphid:
+ const: 0x000e8061
+ required:
+ - arm,primecell-periphid
+ - gpio-ranges
+
+unevaluatedProperties: false
...
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 02/11] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 01/11] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 03/11] gpio: pl061: drop redundant pending check in the IRQ handler Long Zhao via B4 Relay
` (8 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Some controllers ignore output writes while a line is still an input.
Add a behaviour flag so the output value is written after the direction
change. This is a legacy quirk. New hardware must not use it.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-regmap.c | 15 +++++++++++++--
include/linux/gpio/regmap.h | 14 ++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 51b4d69b8740..0a44deae3d01 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -33,6 +33,7 @@ struct gpio_regmap {
unsigned int reg_dir_out_base;
unsigned long *fixed_direction_mask;
unsigned long *fixed_direction_output;
+ unsigned long quirks;
#ifdef CONFIG_REGMAP_IRQ
int regmap_irq_line;
@@ -271,9 +272,17 @@ static int gpio_regmap_direction_output(struct gpio_chip *chip,
return ret;
}
- gpio_regmap_set(chip, offset, value);
+ if (!(gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR))
+ gpio_regmap_set(chip, offset, value);
- return gpio_regmap_set_direction(chip, offset, true);
+ ret = gpio_regmap_set_direction(chip, offset, true);
+ if (ret)
+ return ret;
+
+ if (gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR)
+ gpio_regmap_set(chip, offset, value);
+
+ return 0;
}
void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio)
@@ -376,6 +385,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
config->fixed_direction_output, chip->ngpio);
}
+ gpio->quirks = config->quirks;
+
/* if not set, assume there is only one register */
gpio->ngpio_per_reg = config->ngpio_per_reg;
if (!gpio->ngpio_per_reg)
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index 06255756710d..f9090073c852 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -3,6 +3,8 @@
#ifndef _LINUX_GPIO_REGMAP_H
#define _LINUX_GPIO_REGMAP_H
+#include <linux/bits.h>
+
struct device;
struct fwnode_handle;
struct gpio_regmap;
@@ -13,6 +15,14 @@ struct regmap;
#define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
#define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
+/*
+ * Write the output value after the line is switched to output.
+ * Needed when hardware ignores output writes while the line is an input.
+ * This is a legacy quirk; new hardware must not use it. Direction changes
+ * will glitch.
+ */
+#define GPIO_REGMAP_QUIRK_SET_AFTER_DIR BIT(0)
+
/**
* struct gpio_regmap_config - Description of a generic regmap gpio_chip.
* @parent: The parent device
@@ -48,6 +58,7 @@ struct regmap;
* (Optional) Bitmap representing the fixed direction of
* the GPIO lines. Useful when there are GPIO lines with a
* fixed direction mixed together in the same register.
+ * @quirks: (Optional) Behaviour flags, OR of GPIO_REGMAP_QUIRK_*.
* @drvdata: (Optional) Pointer to driver specific data which is
* not used by gpio-remap but is provided "as is" to the
* driver callback(s).
@@ -98,6 +109,9 @@ struct gpio_regmap_config {
unsigned long *fixed_direction_mask;
unsigned long *fixed_direction_output;
+ /* Regmap GPIO behaviour flags */
+ unsigned long quirks;
+
#ifdef CONFIG_REGMAP_IRQ
struct regmap_irq_chip *regmap_irq_chip;
int regmap_irq_line;
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 03/11] gpio: pl061: drop redundant pending check in the IRQ handler
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 01/11] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 02/11] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 04/11] gpio: pl061: convert register access to regmap Long Zhao via B4 Relay
` (7 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
for_each_set_bit() does nothing when the mask is zero, so the
if (pending) wrapper around it is redundant.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 919cf86fd590..721f14cf404e 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -219,11 +219,8 @@ static void pl061_irq_handler(struct irq_desc *desc)
chained_irq_enter(irqchip, desc);
pending = readb(pl061->base + GPIOMIS);
- if (pending) {
- for_each_set_bit(offset, &pending, PL061_GPIO_NR)
- generic_handle_domain_irq(gc->irq.domain,
- offset);
- }
+ for_each_set_bit(offset, &pending, PL061_GPIO_NR)
+ generic_handle_domain_irq(gc->irq.domain, offset);
chained_irq_exit(irqchip, desc);
}
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 04/11] gpio: pl061: convert register access to regmap
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (2 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 03/11] gpio: pl061: drop redundant pending check in the IRQ handler Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 05/11] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH Long Zhao via B4 Relay
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel,
Chanho Min
From: Long Zhao <longzhao@ambarella.com>
Replace readb()/writeb() with a MMIO regmap while keeping the existing
gpio_chip and girq irqchip. Use a raw spinlock so irq_ack() can run
under the irq descriptor lock.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Chanho Min <chanho.min@lge.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-pl061.c | 144 ++++++++++++++++++++++++++++++----------------
2 files changed, 94 insertions(+), 51 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f03c05288376..2a1487bd1a94 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -583,6 +583,7 @@ config GPIO_PL061
depends on ARM_AMBA || COMPILE_TEST
select IRQ_DOMAIN
select GPIOLIB_IRQCHIP
+ select REGMAP_MMIO
help
Say yes here to support the PrimeCell PL061 GPIO device.
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 721f14cf404e..781bef987339 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -19,9 +19,11 @@
#include <linux/ioport.h>
#include <linux/irq.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/log2.h>
#include <linux/module.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pm.h>
+#include <linux/regmap.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -36,6 +38,7 @@
#define GPIOIC 0x41C
#define PL061_GPIO_NR 8
+#define PL061_DATA_OFFSET 2
struct pl061_context_save_regs {
u8 gpio_data;
@@ -49,18 +52,51 @@ struct pl061_context_save_regs {
struct pl061 {
raw_spinlock_t lock;
- void __iomem *base;
+ struct regmap *regmap;
struct gpio_chip gc;
int parent_irq;
struct pl061_context_save_regs csave_regs;
};
+static bool pl061_arm_is_data_reg(unsigned int reg)
+{
+ return reg >= BIT(PL061_DATA_OFFSET) &&
+ reg <= BIT(PL061_DATA_OFFSET + PL061_GPIO_NR - 1) &&
+ is_power_of_2(reg);
+}
+
+static bool pl061_arm_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case GPIOMIS:
+ case GPIOIC:
+ return true;
+ default:
+ return pl061_arm_is_data_reg(reg);
+ }
+}
+
+static const struct regmap_config pl061_arm_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .reg_stride = 4,
+ .max_register = GPIOIC,
+ .volatile_reg = pl061_arm_volatile_reg,
+ .cache_type = REGCACHE_FLAT_S,
+ .use_raw_spinlock = true,
+};
+
static int pl061_get_direction(struct gpio_chip *gc, unsigned offset)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
+ int ret;
+
+ ret = regmap_test_bits(pl061->regmap, GPIODIR, BIT(offset));
+ if (ret < 0)
+ return ret;
- if (readb(pl061->base + GPIODIR) & BIT(offset))
+ if (ret)
return GPIO_LINE_DIRECTION_OUT;
return GPIO_LINE_DIRECTION_IN;
@@ -70,12 +106,9 @@ static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
unsigned long flags;
- unsigned char gpiodir;
raw_spin_lock_irqsave(&pl061->lock, flags);
- gpiodir = readb(pl061->base + GPIODIR);
- gpiodir &= ~(BIT(offset));
- writeb(gpiodir, pl061->base + GPIODIR);
+ regmap_clear_bits(pl061->regmap, GPIODIR, BIT(offset));
raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
@@ -85,20 +118,20 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
int value)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
+ unsigned int mask = BIT(offset);
unsigned long flags;
- unsigned char gpiodir;
raw_spin_lock_irqsave(&pl061->lock, flags);
- writeb(!!value << offset, pl061->base + (BIT(offset + 2)));
- gpiodir = readb(pl061->base + GPIODIR);
- gpiodir |= BIT(offset);
- writeb(gpiodir, pl061->base + GPIODIR);
+ regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ BIT(offset), value);
+ regmap_set_bits(pl061->regmap, GPIODIR, mask);
/*
* gpio value is set again, because pl061 doesn't allow to set value of
* a gpio pin before configuring it in OUT mode.
*/
- writeb(!!value << offset, pl061->base + (BIT(offset + 2)));
+ regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ BIT(offset), value);
raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
@@ -108,16 +141,16 @@ static int pl061_get_value(struct gpio_chip *gc, unsigned offset)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
- return !!readb(pl061->base + (BIT(offset + 2)));
+ return regmap_test_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ BIT(offset));
}
static int pl061_set_value(struct gpio_chip *gc, unsigned int offset, int value)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
- writeb(!!value << offset, pl061->base + (BIT(offset + 2)));
-
- return 0;
+ return regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ BIT(offset), value);
}
static int pl061_irq_type(struct irq_data *d, unsigned trigger)
@@ -125,9 +158,11 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pl061 *pl061 = gpiochip_get_data(gc);
int offset = irqd_to_hwirq(d);
+ unsigned int bit = BIT(offset);
+ unsigned int gpioiev = 0;
+ unsigned int gpioibe = 0;
+ unsigned int gpiois = 0;
unsigned long flags;
- u8 gpiois, gpioibe, gpioiev;
- u8 bit = BIT(offset);
if (offset < 0 || offset >= PL061_GPIO_NR)
return -EINVAL;
@@ -145,9 +180,9 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
raw_spin_lock_irqsave(&pl061->lock, flags);
- gpioiev = readb(pl061->base + GPIOIEV);
- gpiois = readb(pl061->base + GPIOIS);
- gpioibe = readb(pl061->base + GPIOIBE);
+ regmap_read(pl061->regmap, GPIOIEV, &gpioiev);
+ regmap_read(pl061->regmap, GPIOIS, &gpiois);
+ regmap_read(pl061->regmap, GPIOIBE, &gpioibe);
if (trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
bool polarity = trigger & IRQ_TYPE_LEVEL_HIGH;
@@ -199,9 +234,9 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
offset);
}
- writeb(gpiois, pl061->base + GPIOIS);
- writeb(gpioibe, pl061->base + GPIOIBE);
- writeb(gpioiev, pl061->base + GPIOIEV);
+ regmap_write(pl061->regmap, GPIOIS, gpiois);
+ regmap_write(pl061->regmap, GPIOIBE, gpioibe);
+ regmap_write(pl061->regmap, GPIOIEV, gpioiev);
raw_spin_unlock_irqrestore(&pl061->lock, flags);
@@ -210,15 +245,17 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
static void pl061_irq_handler(struct irq_desc *desc)
{
- unsigned long pending;
- int offset;
struct gpio_chip *gc = irq_desc_get_handler_data(desc);
- struct pl061 *pl061 = gpiochip_get_data(gc);
struct irq_chip *irqchip = irq_desc_get_chip(desc);
+ struct pl061 *pl061 = gpiochip_get_data(gc);
+ unsigned long pending;
+ unsigned int mis = 0;
+ int offset;
chained_irq_enter(irqchip, desc);
- pending = readb(pl061->base + GPIOMIS);
+ regmap_read(pl061->regmap, GPIOMIS, &mis);
+ pending = mis;
for_each_set_bit(offset, &pending, PL061_GPIO_NR)
generic_handle_domain_irq(gc->irq.domain, offset);
@@ -230,11 +267,9 @@ static void pl061_irq_mask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pl061 *pl061 = gpiochip_get_data(gc);
u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
- u8 gpioie;
raw_spin_lock(&pl061->lock);
- gpioie = readb(pl061->base + GPIOIE) & ~mask;
- writeb(gpioie, pl061->base + GPIOIE);
+ regmap_clear_bits(pl061->regmap, GPIOIE, mask);
raw_spin_unlock(&pl061->lock);
gpiochip_disable_irq(gc, d->hwirq);
@@ -245,13 +280,11 @@ static void pl061_irq_unmask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pl061 *pl061 = gpiochip_get_data(gc);
u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
- u8 gpioie;
gpiochip_enable_irq(gc, d->hwirq);
raw_spin_lock(&pl061->lock);
- gpioie = readb(pl061->base + GPIOIE) | mask;
- writeb(gpioie, pl061->base + GPIOIE);
+ regmap_set_bits(pl061->regmap, GPIOIE, mask);
raw_spin_unlock(&pl061->lock);
}
@@ -269,9 +302,7 @@ static void pl061_irq_ack(struct irq_data *d)
struct pl061 *pl061 = gpiochip_get_data(gc);
u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
- raw_spin_lock(&pl061->lock);
- writeb(mask, pl061->base + GPIOIC);
- raw_spin_unlock(&pl061->lock);
+ regmap_write(pl061->regmap, GPIOIC, mask);
}
static int pl061_irq_set_wake(struct irq_data *d, unsigned int state)
@@ -303,17 +334,22 @@ static const struct irq_chip pl061_irq_chip = {
static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
- struct pl061 *pl061;
struct gpio_irq_chip *girq;
+ struct pl061 *pl061;
+ void __iomem *base;
int ret, irq;
pl061 = devm_kzalloc(dev, sizeof(*pl061), GFP_KERNEL);
if (pl061 == NULL)
return -ENOMEM;
- pl061->base = devm_ioremap_resource(dev, &adev->res);
- if (IS_ERR(pl061->base))
- return PTR_ERR(pl061->base);
+ base = devm_ioremap_resource(dev, &adev->res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ pl061->regmap = devm_regmap_init_mmio(dev, base, &pl061_arm_regmap_config);
+ if (IS_ERR(pl061->regmap))
+ return PTR_ERR(pl061->regmap);
raw_spin_lock_init(&pl061->lock);
pl061->gc.request = gpiochip_generic_request;
@@ -332,7 +368,7 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
/*
* irq_chip support
*/
- writeb(0, pl061->base + GPIOIE); /* disable irqs */
+ regmap_write(pl061->regmap, GPIOIE, 0); /* disable irqs */
irq = adev->irq[0];
if (!irq)
dev_warn(&adev->dev, "IRQ support disabled\n");
@@ -363,14 +399,20 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
static int pl061_suspend(struct device *dev)
{
struct pl061 *pl061 = dev_get_drvdata(dev);
+ unsigned int val = 0;
int offset;
pl061->csave_regs.gpio_data = 0;
- pl061->csave_regs.gpio_dir = readb(pl061->base + GPIODIR);
- pl061->csave_regs.gpio_is = readb(pl061->base + GPIOIS);
- pl061->csave_regs.gpio_ibe = readb(pl061->base + GPIOIBE);
- pl061->csave_regs.gpio_iev = readb(pl061->base + GPIOIEV);
- pl061->csave_regs.gpio_ie = readb(pl061->base + GPIOIE);
+ regmap_read(pl061->regmap, GPIODIR, &val);
+ pl061->csave_regs.gpio_dir = val;
+ regmap_read(pl061->regmap, GPIOIS, &val);
+ pl061->csave_regs.gpio_is = val;
+ regmap_read(pl061->regmap, GPIOIBE, &val);
+ pl061->csave_regs.gpio_ibe = val;
+ regmap_read(pl061->regmap, GPIOIEV, &val);
+ pl061->csave_regs.gpio_iev = val;
+ regmap_read(pl061->regmap, GPIOIE, &val);
+ pl061->csave_regs.gpio_ie = val;
for (offset = 0; offset < PL061_GPIO_NR; offset++) {
if (pl061->csave_regs.gpio_dir & (BIT(offset)))
@@ -395,10 +437,10 @@ static int pl061_resume(struct device *dev)
pl061_direction_input(&pl061->gc, offset);
}
- writeb(pl061->csave_regs.gpio_is, pl061->base + GPIOIS);
- writeb(pl061->csave_regs.gpio_ibe, pl061->base + GPIOIBE);
- writeb(pl061->csave_regs.gpio_iev, pl061->base + GPIOIEV);
- writeb(pl061->csave_regs.gpio_ie, pl061->base + GPIOIE);
+ regmap_write(pl061->regmap, GPIOIS, pl061->csave_regs.gpio_is);
+ regmap_write(pl061->regmap, GPIOIBE, pl061->csave_regs.gpio_ibe);
+ regmap_write(pl061->regmap, GPIOIEV, pl061->csave_regs.gpio_iev);
+ regmap_write(pl061->regmap, GPIOIE, pl061->csave_regs.gpio_ie);
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 05/11] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (3 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 04/11] gpio: pl061: convert register access to regmap Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 06/11] gpio: pl061: use cleanup helpers for locks Long Zhao via B4 Relay
` (5 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Use the standard IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH helpers
instead of open-coded edge and level comparisons.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 781bef987339..e0ac8335bae4 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -167,9 +167,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
if (offset < 0 || offset >= PL061_GPIO_NR)
return -EINVAL;
- if ((trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) &&
- (trigger & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)))
- {
+ if ((trigger & IRQ_TYPE_LEVEL_MASK) && (trigger & IRQ_TYPE_EDGE_BOTH)) {
dev_err(gc->parent,
"trying to configure line %d for both level and edge "
"detection, choose one!\n",
@@ -184,7 +182,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
regmap_read(pl061->regmap, GPIOIS, &gpiois);
regmap_read(pl061->regmap, GPIOIBE, &gpioibe);
- if (trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) {
+ if (trigger & IRQ_TYPE_LEVEL_MASK) {
bool polarity = trigger & IRQ_TYPE_LEVEL_HIGH;
/* Disable edge detection */
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 06/11] gpio: pl061: use cleanup helpers for locks
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (4 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 05/11] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 07/11] gpio: pl061: keep the parent device in a local Long Zhao via B4 Relay
` (4 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Convert raw_spinlock critical sections to guard()() from cleanup.h.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index e0ac8335bae4..a232b63fbc0b 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -10,6 +10,7 @@
*/
#include <linux/amba/bus.h>
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/gpio/driver.h>
@@ -105,11 +106,10 @@ static int pl061_get_direction(struct gpio_chip *gc, unsigned offset)
static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
- unsigned long flags;
- raw_spin_lock_irqsave(&pl061->lock, flags);
+ guard(raw_spinlock_irqsave)(&pl061->lock);
+
regmap_clear_bits(pl061->regmap, GPIODIR, BIT(offset));
- raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
}
@@ -119,9 +119,9 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
{
struct pl061 *pl061 = gpiochip_get_data(gc);
unsigned int mask = BIT(offset);
- unsigned long flags;
- raw_spin_lock_irqsave(&pl061->lock, flags);
+ guard(raw_spinlock_irqsave)(&pl061->lock);
+
regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
BIT(offset), value);
regmap_set_bits(pl061->regmap, GPIODIR, mask);
@@ -132,7 +132,6 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
*/
regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
BIT(offset), value);
- raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
}
@@ -162,7 +161,6 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
unsigned int gpioiev = 0;
unsigned int gpioibe = 0;
unsigned int gpiois = 0;
- unsigned long flags;
if (offset < 0 || offset >= PL061_GPIO_NR)
return -EINVAL;
@@ -175,8 +173,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
return -EINVAL;
}
-
- raw_spin_lock_irqsave(&pl061->lock, flags);
+ guard(raw_spinlock_irqsave)(&pl061->lock);
regmap_read(pl061->regmap, GPIOIEV, &gpioiev);
regmap_read(pl061->regmap, GPIOIS, &gpiois);
@@ -236,8 +233,6 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
regmap_write(pl061->regmap, GPIOIBE, gpioibe);
regmap_write(pl061->regmap, GPIOIEV, gpioiev);
- raw_spin_unlock_irqrestore(&pl061->lock, flags);
-
return 0;
}
@@ -266,9 +261,8 @@ static void pl061_irq_mask(struct irq_data *d)
struct pl061 *pl061 = gpiochip_get_data(gc);
u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
- raw_spin_lock(&pl061->lock);
- regmap_clear_bits(pl061->regmap, GPIOIE, mask);
- raw_spin_unlock(&pl061->lock);
+ scoped_guard(raw_spinlock, &pl061->lock)
+ regmap_clear_bits(pl061->regmap, GPIOIE, mask);
gpiochip_disable_irq(gc, d->hwirq);
}
@@ -281,9 +275,8 @@ static void pl061_irq_unmask(struct irq_data *d)
gpiochip_enable_irq(gc, d->hwirq);
- raw_spin_lock(&pl061->lock);
- regmap_set_bits(pl061->regmap, GPIOIE, mask);
- raw_spin_unlock(&pl061->lock);
+ scoped_guard(raw_spinlock, &pl061->lock)
+ regmap_set_bits(pl061->regmap, GPIOIE, mask);
}
/**
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 07/11] gpio: pl061: keep the parent device in a local
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (5 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 06/11] gpio: pl061: use cleanup helpers for locks Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 7:42 ` Andy Shevchenko
2026-10-27 4:18 ` [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains Long Zhao via B4 Relay
` (3 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
IRQ logging uses gc->parent. Keep that device in a local so the
gpio-regmap switch can change where it comes from without rewriting
every log line.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index a232b63fbc0b..f101f48ce68a 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -156,6 +156,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pl061 *pl061 = gpiochip_get_data(gc);
+ struct device *parent = gc->parent;
int offset = irqd_to_hwirq(d);
unsigned int bit = BIT(offset);
unsigned int gpioiev = 0;
@@ -166,7 +167,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
return -EINVAL;
if ((trigger & IRQ_TYPE_LEVEL_MASK) && (trigger & IRQ_TYPE_EDGE_BOTH)) {
- dev_err(gc->parent,
+ dev_err(parent,
"trying to configure line %d for both level and edge "
"detection, choose one!\n",
offset);
@@ -192,7 +193,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
else
gpioiev &= ~bit;
irq_set_handler_locked(d, handle_level_irq);
- dev_dbg(gc->parent, "line %d: IRQ on %s level\n",
+ dev_dbg(parent, "line %d: IRQ on %s level\n",
offset,
polarity ? "HIGH" : "LOW");
} else if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
@@ -201,7 +202,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
/* Select both edges, setting this makes GPIOEV be ignored */
gpioibe |= bit;
irq_set_handler_locked(d, handle_edge_irq);
- dev_dbg(gc->parent, "line %d: IRQ on both edges\n", offset);
+ dev_dbg(parent, "line %d: IRQ on both edges\n", offset);
} else if ((trigger & IRQ_TYPE_EDGE_RISING) ||
(trigger & IRQ_TYPE_EDGE_FALLING)) {
bool rising = trigger & IRQ_TYPE_EDGE_RISING;
@@ -216,7 +217,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
else
gpioiev &= ~bit;
irq_set_handler_locked(d, handle_edge_irq);
- dev_dbg(gc->parent, "line %d: IRQ on %s edge\n",
+ dev_dbg(parent, "line %d: IRQ on %s edge\n",
offset,
rising ? "RISING" : "FALLING");
} else {
@@ -225,7 +226,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
gpioibe &= ~bit;
gpioiev &= ~bit;
irq_set_handler_locked(d, handle_bad_irq);
- dev_warn(gc->parent, "no trigger selected for line %d\n",
+ dev_warn(parent, "no trigger selected for line %d\n",
offset);
}
@@ -307,8 +308,9 @@ static int pl061_irq_set_wake(struct irq_data *d, unsigned int state)
static void pl061_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct device *parent = gc->parent;
- seq_puts(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(parent));
}
static const struct irq_chip pl061_irq_chip = {
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (6 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 07/11] gpio: pl061: keep the parent device in a local Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 5:29 ` Mika Westerberg
2026-10-27 4:18 ` [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
` (2 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel,
Mika Westerberg, linux-acpi
From: Long Zhao <longzhao@ambarella.com>
gpiochip_add_irqchip() requests GPIO-signaled ACPI events after
installing the irqchip. A gpiochip that gets its IRQ domain through
gpiochip_irqchip_add_domain() never goes through that path, so _AEI
events, such as the power button on ARM64 ACPI platforms, are not
requested.
Request them here as well. gpiochip_remove() already calls
acpi_gpiochip_free_interrupts().
Cc: Mika Westerberg <westeri@kernel.org>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpiolib.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e5fb60111151..63c51d9ee0bf 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2353,7 +2353,15 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gc)
int gpiochip_irqchip_add_domain(struct gpio_chip *gc,
struct irq_domain *domain)
{
- return gpiochip_irqchip_add_allocated_domain(gc, domain, true);
+ int ret;
+
+ ret = gpiochip_irqchip_add_allocated_domain(gc, domain, true);
+ if (ret)
+ return ret;
+
+ acpi_gpiochip_request_interrupts(gc);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(gpiochip_irqchip_add_domain);
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (7 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 8:06 ` Andy Shevchenko
2026-10-27 4:18 ` [PATCH v12 10/11] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging Long Zhao via B4 Relay
10 siblings, 1 reply; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Move get, set and direction onto gpio-regmap. ARM PL061 sets
GPIO_REGMAP_QUIRK_SET_AFTER_DIR so the output value is written
after the direction change.
Keep a custom irqchip and irq domain. Create the domain with
the PL061 private data and let gpio-regmap attach it.
Leave the chained handler's regmap_read() unchecked. mis stays 0
on failure, so the pending loop does nothing, and
chained_irq_exit() still runs. irq_type still ignores MMIO write
returns.
The irqchip uses its own child IRQ lockdep class and clears the
chained handler only after it was installed, checked with lockdep
on the CV75 board. irq_set_parent() nests
the GPIO irq_desc under the AMBA parent for irq_set_wake(). The
gpiochip IRQ resource helpers cannot be kept here: the gpio_chip is
not reachable through gpio-regmap's external irq_domain path, so
GPIOD_FLAG_USED_AS_IRQ is never tracked. This matches the other
users of that path (gpio-idio-16, gpio-pcie-idio-24,
gpio-104-idi-48).
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-pl061.c | 419 +++++++++++++++++++++++++++-------------------
2 files changed, 248 insertions(+), 172 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 2a1487bd1a94..55a129836158 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -583,6 +583,7 @@ config GPIO_PL061
depends on ARM_AMBA || COMPILE_TEST
select IRQ_DOMAIN
select GPIOLIB_IRQCHIP
+ select GPIO_REGMAP
select REGMAP_MMIO
help
Say yes here to support the PrimeCell PL061 GPIO device.
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index f101f48ce68a..b715c3a2326e 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -12,21 +12,19 @@
#include <linux/bitops.h>
#include <linux/cleanup.h>
#include <linux/device.h>
-#include <linux/errno.h>
-#include <linux/gpio/driver.h>
-#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/gpio/regmap.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/irq.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
#include <linux/log2.h>
#include <linux/module.h>
-#include <linux/pinctrl/consumer.h>
#include <linux/pm.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
-#include <linux/slab.h>
#include <linux/spinlock.h>
#define GPIODIR 0x400
@@ -41,23 +39,41 @@
#define PL061_GPIO_NR 8
#define PL061_DATA_OFFSET 2
-struct pl061_context_save_regs {
- u8 gpio_data;
- u8 gpio_dir;
- u8 gpio_is;
- u8 gpio_ibe;
- u8 gpio_iev;
- u8 gpio_ie;
+struct pl061_regs {
+ unsigned int dat;
+ unsigned int dir;
+ unsigned int is;
+ unsigned int ibe;
+ unsigned int iev;
+ unsigned int ie;
+ unsigned int mis;
+ unsigned int ic;
+ unsigned int mask;
+ unsigned int enable;
+};
+
+struct pl061_drvdata {
+ const struct regmap_config *regmap_config;
+ const struct pl061_regs *regs;
+ unsigned int ngpio;
+ unsigned long quirks;
+ bool clear_irq_on_type;
+ bool pm;
+ int (*reg_mask_xlate)(struct gpio_regmap *gpio,
+ unsigned int base, unsigned int offset,
+ unsigned int *reg, unsigned int *mask);
};
struct pl061 {
raw_spinlock_t lock;
struct regmap *regmap;
- struct gpio_chip gc;
+ const struct pl061_drvdata *data;
+ struct irq_domain *irq_domain;
int parent_irq;
- struct pl061_context_save_regs csave_regs;
+ u32 saved_dat;
+ bool irq_handler_installed;
};
static bool pl061_arm_is_data_reg(unsigned int reg)
@@ -88,82 +104,36 @@ static const struct regmap_config pl061_arm_regmap_config = {
.use_raw_spinlock = true,
};
-static int pl061_get_direction(struct gpio_chip *gc, unsigned offset)
+static unsigned int pl061_line_mask(struct pl061 *pl061, irq_hw_number_t hwirq)
{
- struct pl061 *pl061 = gpiochip_get_data(gc);
- int ret;
-
- ret = regmap_test_bits(pl061->regmap, GPIODIR, BIT(offset));
- if (ret < 0)
- return ret;
-
- if (ret)
- return GPIO_LINE_DIRECTION_OUT;
-
- return GPIO_LINE_DIRECTION_IN;
+ return BIT(hwirq % pl061->data->ngpio);
}
-static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
+static int pl061_arm_reg_mask_xlate(struct gpio_regmap *gpio,
+ unsigned int base, unsigned int offset,
+ unsigned int *reg, unsigned int *mask)
{
- struct pl061 *pl061 = gpiochip_get_data(gc);
-
- guard(raw_spinlock_irqsave)(&pl061->lock);
-
- regmap_clear_bits(pl061->regmap, GPIODIR, BIT(offset));
+ if (base == 0)
+ *reg = BIT(offset + PL061_DATA_OFFSET);
+ else
+ *reg = base;
+ *mask = BIT(offset);
return 0;
}
-static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
- int value)
-{
- struct pl061 *pl061 = gpiochip_get_data(gc);
- unsigned int mask = BIT(offset);
-
- guard(raw_spinlock_irqsave)(&pl061->lock);
-
- regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- BIT(offset), value);
- regmap_set_bits(pl061->regmap, GPIODIR, mask);
-
- /*
- * gpio value is set again, because pl061 doesn't allow to set value of
- * a gpio pin before configuring it in OUT mode.
- */
- regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- BIT(offset), value);
-
- return 0;
-}
-
-static int pl061_get_value(struct gpio_chip *gc, unsigned offset)
-{
- struct pl061 *pl061 = gpiochip_get_data(gc);
-
- return regmap_test_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- BIT(offset));
-}
-
-static int pl061_set_value(struct gpio_chip *gc, unsigned int offset, int value)
-{
- struct pl061 *pl061 = gpiochip_get_data(gc);
-
- return regmap_assign_bits(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- BIT(offset), value);
-}
-
static int pl061_irq_type(struct irq_data *d, unsigned trigger)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct pl061 *pl061 = gpiochip_get_data(gc);
- struct device *parent = gc->parent;
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ struct device *parent = regmap_get_device(pl061->regmap);
+ const struct pl061_regs *regs = pl061->data->regs;
int offset = irqd_to_hwirq(d);
unsigned int bit = BIT(offset);
unsigned int gpioiev = 0;
unsigned int gpioibe = 0;
unsigned int gpiois = 0;
- if (offset < 0 || offset >= PL061_GPIO_NR)
+ if (offset < 0 || offset >= pl061->data->ngpio)
return -EINVAL;
if ((trigger & IRQ_TYPE_LEVEL_MASK) && (trigger & IRQ_TYPE_EDGE_BOTH)) {
@@ -176,9 +146,9 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
guard(raw_spinlock_irqsave)(&pl061->lock);
- regmap_read(pl061->regmap, GPIOIEV, &gpioiev);
- regmap_read(pl061->regmap, GPIOIS, &gpiois);
- regmap_read(pl061->regmap, GPIOIBE, &gpioibe);
+ regmap_read(pl061->regmap, regs->iev, &gpioiev);
+ regmap_read(pl061->regmap, regs->is, &gpiois);
+ regmap_read(pl061->regmap, regs->ibe, &gpioibe);
if (trigger & IRQ_TYPE_LEVEL_MASK) {
bool polarity = trigger & IRQ_TYPE_LEVEL_HIGH;
@@ -230,54 +200,47 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
offset);
}
- regmap_write(pl061->regmap, GPIOIS, gpiois);
- regmap_write(pl061->regmap, GPIOIBE, gpioibe);
- regmap_write(pl061->regmap, GPIOIEV, gpioiev);
+ regmap_write(pl061->regmap, regs->is, gpiois);
+ regmap_write(pl061->regmap, regs->ibe, gpioibe);
+ regmap_write(pl061->regmap, regs->iev, gpioiev);
+ if (pl061->data->clear_irq_on_type)
+ regmap_write(pl061->regmap, regs->ic, bit);
return 0;
}
static void pl061_irq_handler(struct irq_desc *desc)
{
- struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+ struct pl061 *pl061 = irq_desc_get_handler_data(desc);
struct irq_chip *irqchip = irq_desc_get_chip(desc);
- struct pl061 *pl061 = gpiochip_get_data(gc);
unsigned long pending;
unsigned int mis = 0;
int offset;
chained_irq_enter(irqchip, desc);
- regmap_read(pl061->regmap, GPIOMIS, &mis);
+ regmap_read(pl061->regmap, pl061->data->regs->mis, &mis);
pending = mis;
- for_each_set_bit(offset, &pending, PL061_GPIO_NR)
- generic_handle_domain_irq(gc->irq.domain, offset);
+ for_each_set_bit(offset, &pending, pl061->data->ngpio)
+ generic_handle_domain_irq(pl061->irq_domain, offset);
chained_irq_exit(irqchip, desc);
}
static void pl061_irq_mask(struct irq_data *d)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct pl061 *pl061 = gpiochip_get_data(gc);
- u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ unsigned int mask = pl061_line_mask(pl061, irqd_to_hwirq(d));
- scoped_guard(raw_spinlock, &pl061->lock)
- regmap_clear_bits(pl061->regmap, GPIOIE, mask);
-
- gpiochip_disable_irq(gc, d->hwirq);
+ regmap_clear_bits(pl061->regmap, pl061->data->regs->ie, mask);
}
static void pl061_irq_unmask(struct irq_data *d)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct pl061 *pl061 = gpiochip_get_data(gc);
- u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
-
- gpiochip_enable_irq(gc, d->hwirq);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ unsigned int mask = pl061_line_mask(pl061, irqd_to_hwirq(d));
- scoped_guard(raw_spinlock, &pl061->lock)
- regmap_set_bits(pl061->regmap, GPIOIE, mask);
+ regmap_set_bits(pl061->regmap, pl061->data->regs->ie, mask);
}
/**
@@ -290,25 +253,23 @@ static void pl061_irq_unmask(struct irq_data *d)
*/
static void pl061_irq_ack(struct irq_data *d)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct pl061 *pl061 = gpiochip_get_data(gc);
- u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ unsigned int mask = pl061_line_mask(pl061, irqd_to_hwirq(d));
- regmap_write(pl061->regmap, GPIOIC, mask);
+ regmap_write(pl061->regmap, pl061->data->regs->ic, mask);
}
static int pl061_irq_set_wake(struct irq_data *d, unsigned int state)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
- struct pl061 *pl061 = gpiochip_get_data(gc);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
return irq_set_irq_wake(pl061->parent_irq, state);
}
static void pl061_irq_print_chip(struct irq_data *data, struct seq_file *p)
{
- struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
- struct device *parent = gc->parent;
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(data);
+ struct device *parent = regmap_get_device(pl061->regmap);
seq_puts(p, dev_name(parent));
}
@@ -321,67 +282,165 @@ static const struct irq_chip pl061_irq_chip = {
.irq_set_wake = pl061_irq_set_wake,
.irq_print_chip = pl061_irq_print_chip,
.flags = IRQCHIP_IMMUTABLE,
- GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static struct lock_class_key pl061_irq_lock_class;
+static struct lock_class_key pl061_irq_request_class;
+
+static int pl061_irq_domain_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hwirq)
+{
+ struct pl061 *pl061 = d->host_data;
+
+ irq_set_chip_data(virq, pl061);
+ irq_set_lockdep_class(virq, &pl061_irq_lock_class,
+ &pl061_irq_request_class);
+ irq_set_chip_and_handler(virq, &pl061_irq_chip, handle_bad_irq);
+ irq_set_noprobe(virq);
+ irq_set_parent(virq, pl061->parent_irq);
+
+ return 0;
+}
+
+static void pl061_irq_domain_unmap(struct irq_domain *d, unsigned int virq)
+{
+ irq_set_chip_and_handler(virq, NULL, NULL);
+ irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops pl061_irq_domain_ops = {
+ .map = pl061_irq_domain_map,
+ .unmap = pl061_irq_domain_unmap,
+ .xlate = irq_domain_xlate_twothreecell,
+};
+
+static void pl061_remove_irq(void *data)
+{
+ struct pl061 *pl061 = data;
+
+ if (pl061->irq_handler_installed)
+ irq_set_chained_handler_and_data(pl061->parent_irq, NULL, NULL);
+
+ for (unsigned int i = 0; i < pl061->data->ngpio; i++)
+ irq_dispose_mapping(irq_find_mapping(pl061->irq_domain, i));
+}
+
+static const struct pl061_regs pl061_arm_regs = {
+ .dir = GPIODIR,
+ .is = GPIOIS,
+ .ibe = GPIOIBE,
+ .iev = GPIOIEV,
+ .ie = GPIOIE,
+ .mis = GPIOMIS,
+ .ic = GPIOIC,
+};
+
+static const struct pl061_drvdata pl061_arm = {
+ .regmap_config = &pl061_arm_regmap_config,
+ .regs = &pl061_arm_regs,
+ .ngpio = PL061_GPIO_NR,
+ .quirks = GPIO_REGMAP_QUIRK_SET_AFTER_DIR,
+ .pm = true,
+ .reg_mask_xlate = pl061_arm_reg_mask_xlate,
};
static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
{
+ struct gpio_regmap_config config = {};
+ const struct pl061_drvdata *data;
struct device *dev = &adev->dev;
- struct gpio_irq_chip *girq;
+ struct gpio_regmap *gpio_regmap;
+ const struct pl061_regs *regs;
struct pl061 *pl061;
void __iomem *base;
- int ret, irq;
+ int ret;
+ int irq;
+
+ data = id->data;
+ if (!data)
+ return -ENODATA;
+
+ regs = data->regs;
pl061 = devm_kzalloc(dev, sizeof(*pl061), GFP_KERNEL);
if (pl061 == NULL)
return -ENOMEM;
+ pl061->data = data;
+
base = devm_ioremap_resource(dev, &adev->res);
if (IS_ERR(base))
return PTR_ERR(base);
- pl061->regmap = devm_regmap_init_mmio(dev, base, &pl061_arm_regmap_config);
+ pl061->regmap = devm_regmap_init_mmio(dev, base, data->regmap_config);
if (IS_ERR(pl061->regmap))
return PTR_ERR(pl061->regmap);
raw_spin_lock_init(&pl061->lock);
- pl061->gc.request = gpiochip_generic_request;
- pl061->gc.free = gpiochip_generic_free;
- pl061->gc.base = -1;
- pl061->gc.get_direction = pl061_get_direction;
- pl061->gc.direction_input = pl061_direction_input;
- pl061->gc.direction_output = pl061_direction_output;
- pl061->gc.get = pl061_get_value;
- pl061->gc.set = pl061_set_value;
- pl061->gc.ngpio = PL061_GPIO_NR;
- pl061->gc.label = dev_name(dev);
- pl061->gc.parent = dev;
- pl061->gc.owner = THIS_MODULE;
+
+ if (regs->enable) {
+ ret = regmap_write(pl061->regmap, regs->enable, ~0U);
+ if (ret)
+ return ret;
+ }
+ if (regs->mask) {
+ ret = regmap_write(pl061->regmap, regs->mask, ~0U);
+ if (ret)
+ return ret;
+ }
/*
* irq_chip support
*/
- regmap_write(pl061->regmap, GPIOIE, 0); /* disable irqs */
+ regmap_write(pl061->regmap, regs->ie, 0); /* disable irqs */
+
irq = adev->irq[0];
if (!irq)
- dev_warn(&adev->dev, "IRQ support disabled\n");
+ dev_warn(dev, "IRQ support disabled\n");
pl061->parent_irq = irq;
- girq = &pl061->gc.irq;
- gpio_irq_chip_set_chip(girq, &pl061_irq_chip);
- girq->parent_handler = pl061_irq_handler;
- girq->num_parents = 1;
- girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
- GFP_KERNEL);
- if (!girq->parents)
- return -ENOMEM;
- girq->parents[0] = irq;
- girq->default_type = IRQ_TYPE_NONE;
- girq->handler = handle_bad_irq;
+ if (irq) {
+ const struct irq_domain_info info = {
+ .fwnode = dev_fwnode(dev),
+ .size = data->ngpio,
+ .hwirq_max = data->ngpio,
+ .ops = &pl061_irq_domain_ops,
+ .host_data = pl061,
+ };
+
+ pl061->irq_domain = devm_irq_domain_instantiate(dev, &info);
+ if (IS_ERR(pl061->irq_domain))
+ return PTR_ERR(pl061->irq_domain);
+
+ /*
+ * Registered before the gpiochip so unbind runs it after
+ * gpiochip_free_remaining_irqs(), while the domain is still
+ * alive. devm removes the domain after this action.
+ */
+ ret = devm_add_action_or_reset(dev, pl061_remove_irq, pl061);
+ if (ret)
+ return ret;
+ }
- ret = devm_gpiochip_add_data(dev, &pl061->gc, pl061);
- if (ret)
- return ret;
+ config.parent = dev;
+ config.regmap = pl061->regmap;
+ config.ngpio = data->ngpio;
+ config.reg_dat_base = GPIO_REGMAP_ADDR(regs->dat);
+ config.reg_set_base = GPIO_REGMAP_ADDR(regs->dat);
+ config.reg_dir_out_base = GPIO_REGMAP_ADDR(regs->dir);
+ config.reg_mask_xlate = data->reg_mask_xlate;
+ config.quirks = data->quirks;
+ config.irq_domain = pl061->irq_domain;
+ config.drvdata = pl061;
+
+ gpio_regmap = devm_gpio_regmap_register(dev, &config);
+ if (IS_ERR(gpio_regmap))
+ return PTR_ERR(gpio_regmap);
+
+ if (irq) {
+ irq_set_chained_handler_and_data(irq, pl061_irq_handler, pl061);
+ pl061->irq_handler_installed = true;
+ }
amba_set_drvdata(adev, pl061);
dev_info(dev, "PL061 GPIO chip registered\n");
@@ -389,28 +448,39 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
return 0;
}
+static void pl061_cache_ctrl_regs(struct pl061 *pl061, unsigned int *dir)
+{
+ const struct pl061_regs *regs = pl061->data->regs;
+ unsigned int val;
+
+ regmap_read(pl061->regmap, regs->dir, dir);
+ regmap_read(pl061->regmap, regs->is, &val);
+ regmap_read(pl061->regmap, regs->ibe, &val);
+ regmap_read(pl061->regmap, regs->iev, &val);
+ regmap_read(pl061->regmap, regs->ie, &val);
+}
+
static int pl061_suspend(struct device *dev)
{
struct pl061 *pl061 = dev_get_drvdata(dev);
- unsigned int val = 0;
- int offset;
-
- pl061->csave_regs.gpio_data = 0;
- regmap_read(pl061->regmap, GPIODIR, &val);
- pl061->csave_regs.gpio_dir = val;
- regmap_read(pl061->regmap, GPIOIS, &val);
- pl061->csave_regs.gpio_is = val;
- regmap_read(pl061->regmap, GPIOIBE, &val);
- pl061->csave_regs.gpio_ibe = val;
- regmap_read(pl061->regmap, GPIOIEV, &val);
- pl061->csave_regs.gpio_iev = val;
- regmap_read(pl061->regmap, GPIOIE, &val);
- pl061->csave_regs.gpio_ie = val;
-
- for (offset = 0; offset < PL061_GPIO_NR; offset++) {
- if (pl061->csave_regs.gpio_dir & (BIT(offset)))
- pl061->csave_regs.gpio_data |=
- pl061_get_value(&pl061->gc, offset) << offset;
+ unsigned long dir_mask;
+ unsigned int dir = 0;
+ unsigned int offset;
+ unsigned int val;
+
+ if (!pl061->data->pm)
+ return 0;
+
+ pl061_cache_ctrl_regs(pl061, &dir);
+
+ pl061->saved_dat = 0;
+ dir_mask = dir;
+ for_each_set_bit(offset, &dir_mask, pl061->data->ngpio) {
+ val = 0;
+ regmap_read_bypassed(pl061->regmap,
+ BIT(offset + PL061_DATA_OFFSET),
+ &val);
+ pl061->saved_dat |= val;
}
return 0;
@@ -419,23 +489,27 @@ static int pl061_suspend(struct device *dev)
static int pl061_resume(struct device *dev)
{
struct pl061 *pl061 = dev_get_drvdata(dev);
- int offset;
+ const struct pl061_regs *regs;
+ unsigned long dir_mask;
+ unsigned int dir = 0;
+ unsigned int offset;
- for (offset = 0; offset < PL061_GPIO_NR; offset++) {
- if (pl061->csave_regs.gpio_dir & (BIT(offset)))
- pl061_direction_output(&pl061->gc, offset,
- pl061->csave_regs.gpio_data &
- (BIT(offset)));
- else
- pl061_direction_input(&pl061->gc, offset);
- }
+ if (!pl061->data->pm)
+ return 0;
- regmap_write(pl061->regmap, GPIOIS, pl061->csave_regs.gpio_is);
- regmap_write(pl061->regmap, GPIOIBE, pl061->csave_regs.gpio_ibe);
- regmap_write(pl061->regmap, GPIOIEV, pl061->csave_regs.gpio_iev);
- regmap_write(pl061->regmap, GPIOIE, pl061->csave_regs.gpio_ie);
+ regs = pl061->data->regs;
- return 0;
+ regmap_read(pl061->regmap, regs->dir, &dir);
+
+ regcache_mark_dirty(pl061->regmap);
+ regcache_sync_region(pl061->regmap, regs->dir, regs->dir);
+
+ dir_mask = dir;
+ for_each_set_bit(offset, &dir_mask, pl061->data->ngpio)
+ regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ pl061->saved_dat & BIT(offset));
+
+ return regcache_sync_region(pl061->regmap, regs->is, regs->ie);
}
static DEFINE_SIMPLE_DEV_PM_OPS(pl061_dev_pm_ops, pl061_suspend, pl061_resume);
@@ -444,6 +518,7 @@ static const struct amba_id pl061_ids[] = {
{
.id = 0x00041061,
.mask = 0x000fffff,
+ .data = (void *)&pl061_arm,
},
{ 0, 0 },
};
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 10/11] gpio: pl061: add Ambarella register-layout variant
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (8 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging Long Zhao via B4 Relay
10 siblings, 0 replies; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Add a 32-bit, 32-line layout with DAT/DIR/IS/IBE/IEV/IE/MIS/IC plus
MASK and ENABLE. Probe unmasks the bank and enables the controller.
IRQ type changes also clear the pending bit. This derivative does not
need write-after-direction or system suspend save/restore.
Cache DAT (REGCACHE_FLAT_S, not volatile) so gpio_set() updates one
bit from the output shadow instead of read-modify-writing the pin
level across the bank.
Match the part with designer AMBA_VENDOR_AMBARELLA (JEP106 identity
0x68, bit 7 set) and part number 0x061.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/amba/bus.h | 1 +
2 files changed, 59 insertions(+)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index b715c3a2326e..0fb42c531451 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -39,6 +39,18 @@
#define PL061_GPIO_NR 8
#define PL061_DATA_OFFSET 2
+#define AMBARELLA_GPIO_NR 32
+#define AMBARELLA_DAT 0x00
+#define AMBARELLA_DIR 0x04
+#define AMBARELLA_IS 0x08
+#define AMBARELLA_IBE 0x0c
+#define AMBARELLA_IEV 0x10
+#define AMBARELLA_IE 0x14
+#define AMBARELLA_MIS 0x20
+#define AMBARELLA_IC 0x24
+#define AMBARELLA_MASK 0x28
+#define AMBARELLA_ENABLE 0x2c
+
struct pl061_regs {
unsigned int dat;
unsigned int dir;
@@ -94,6 +106,17 @@ static bool pl061_arm_volatile_reg(struct device *dev, unsigned int reg)
}
}
+static bool pl061_ambarella_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case AMBARELLA_MIS:
+ case AMBARELLA_IC:
+ return true;
+ default:
+ return false;
+ }
+}
+
static const struct regmap_config pl061_arm_regmap_config = {
.reg_bits = 16,
.val_bits = 8,
@@ -104,6 +127,16 @@ static const struct regmap_config pl061_arm_regmap_config = {
.use_raw_spinlock = true,
};
+static const struct regmap_config pl061_ambarella_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = AMBARELLA_ENABLE,
+ .volatile_reg = pl061_ambarella_volatile_reg,
+ .cache_type = REGCACHE_FLAT_S,
+ .use_raw_spinlock = true,
+};
+
static unsigned int pl061_line_mask(struct pl061 *pl061, irq_hw_number_t hwirq)
{
return BIT(hwirq % pl061->data->ngpio);
@@ -335,6 +368,19 @@ static const struct pl061_regs pl061_arm_regs = {
.ic = GPIOIC,
};
+static const struct pl061_regs pl061_ambarella_regs = {
+ .dat = AMBARELLA_DAT,
+ .dir = AMBARELLA_DIR,
+ .is = AMBARELLA_IS,
+ .ibe = AMBARELLA_IBE,
+ .iev = AMBARELLA_IEV,
+ .ie = AMBARELLA_IE,
+ .mis = AMBARELLA_MIS,
+ .ic = AMBARELLA_IC,
+ .mask = AMBARELLA_MASK,
+ .enable = AMBARELLA_ENABLE,
+};
+
static const struct pl061_drvdata pl061_arm = {
.regmap_config = &pl061_arm_regmap_config,
.regs = &pl061_arm_regs,
@@ -344,6 +390,13 @@ static const struct pl061_drvdata pl061_arm = {
.reg_mask_xlate = pl061_arm_reg_mask_xlate,
};
+static const struct pl061_drvdata pl061_ambarella = {
+ .regmap_config = &pl061_ambarella_regmap_config,
+ .regs = &pl061_ambarella_regs,
+ .ngpio = AMBARELLA_GPIO_NR,
+ .clear_irq_on_type = true,
+};
+
static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
{
struct gpio_regmap_config config = {};
@@ -520,6 +573,11 @@ static const struct amba_id pl061_ids[] = {
.mask = 0x000fffff,
.data = (void *)&pl061_arm,
},
+ {
+ .id = (AMBA_VENDOR_AMBARELLA << 12) | 0x061,
+ .mask = 0x000fffff,
+ .data = (void *)&pl061_ambarella,
+ },
{ 0, 0 },
};
MODULE_DEVICE_TABLE(amba, pl061_ids);
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 80a74cd2da7e..411533bfa61d 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -98,6 +98,7 @@ enum amba_vendor {
AMBA_VENDOR_ST = 0x80,
AMBA_VENDOR_QCOM = 0x51,
AMBA_VENDOR_LSI = 0xb6,
+ AMBA_VENDOR_AMBARELLA = 0xe8,
};
extern const struct bus_type amba_bustype;
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (9 preceding siblings ...)
2026-10-27 4:18 ` [PATCH v12 10/11] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
@ 2026-10-27 4:18 ` Long Zhao via B4 Relay
2026-10-27 8:08 ` Andy Shevchenko
10 siblings, 1 reply; 18+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-27 4:18 UTC (permalink / raw)
To: Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, devicetree, Long Zhao, Long Zhao, linux-kernel
From: Long Zhao <longzhao@ambarella.com>
Join the level-and-edge dev_err() into one string literal so a grep
for "edge detection" finds it.
Use str_high_low() for the level debug line. str_rising_falling()
is not in linux-next, so the edge debug line keeps its own text.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 0fb42c531451..1d77afd8c409 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -26,6 +26,7 @@
#include <linux/regmap.h>
#include <linux/seq_file.h>
#include <linux/spinlock.h>
+#include <linux/string_choices.h>
#define GPIODIR 0x400
#define GPIOIS 0x404
@@ -171,8 +172,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
if ((trigger & IRQ_TYPE_LEVEL_MASK) && (trigger & IRQ_TYPE_EDGE_BOTH)) {
dev_err(parent,
- "trying to configure line %d for both level and edge "
- "detection, choose one!\n",
+ "trying to configure line %d for both level and edge detection, choose one!\n",
offset);
return -EINVAL;
}
@@ -198,7 +198,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
irq_set_handler_locked(d, handle_level_irq);
dev_dbg(parent, "line %d: IRQ on %s level\n",
offset,
- polarity ? "HIGH" : "LOW");
+ str_high_low(polarity));
} else if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
/* Disable level detection */
gpiois &= ~bit;
--
2.34.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains
2026-10-27 4:18 ` [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains Long Zhao via B4 Relay
@ 2026-10-27 5:29 ` Mika Westerberg
0 siblings, 0 replies; 18+ messages in thread
From: Mika Westerberg @ 2026-10-27 5:29 UTC (permalink / raw)
To: longzhao
Cc: Bartosz Golaszewski, linux-gpio, Linus Walleij, Michael Walle,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, Long Zhao, linux-kernel, Mika Westerberg, linux-acpi
On Tue, Oct 27, 2026 at 12:18:55PM +0800, Long Zhao via B4 Relay wrote:
> From: Long Zhao <longzhao@ambarella.com>
>
> gpiochip_add_irqchip() requests GPIO-signaled ACPI events after
> installing the irqchip. A gpiochip that gets its IRQ domain through
> gpiochip_irqchip_add_domain() never goes through that path, so _AEI
> events, such as the power button on ARM64 ACPI platforms, are not
> requested.
>
> Request them here as well. gpiochip_remove() already calls
> acpi_gpiochip_free_interrupts().
>
> Cc: Mika Westerberg <westeri@kernel.org>
One tiny comment below, other than that looks good to me,
Acked-by: Mika Westerberg (Intel) <westeri@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Long Zhao <longzhao@ambarella.com>
> ---
> drivers/gpio/gpiolib.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e5fb60111151..63c51d9ee0bf 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2353,7 +2353,15 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gc)
> int gpiochip_irqchip_add_domain(struct gpio_chip *gc,
> struct irq_domain *domain)
> {
> - return gpiochip_irqchip_add_allocated_domain(gc, domain, true);
> + int ret;
> +
> + ret = gpiochip_irqchip_add_allocated_domain(gc, domain, true);
> + if (ret)
> + return ret;
> +
> + acpi_gpiochip_request_interrupts(gc);
> +
Looks better if you drop this empty line.
> + return 0;
> }
> EXPORT_SYMBOL_GPL(gpiochip_irqchip_add_domain);
>
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 07/11] gpio: pl061: keep the parent device in a local
2026-10-27 4:18 ` [PATCH v12 07/11] gpio: pl061: keep the parent device in a local Long Zhao via B4 Relay
@ 2026-10-27 7:42 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-10-27 7:42 UTC (permalink / raw)
To: longzhao
Cc: Bartosz Golaszewski, linux-gpio, Linus Walleij, Michael Walle,
Rob Herring, Krzysztof Kozlowski, Krzysztof Kozlowski,
Krzysztof Kozlowski, Conor Dooley, devicetree, Long Zhao,
linux-kernel
On Tue, Oct 27, 2026 at 12:18:54PM +0800, Long Zhao via B4 Relay wrote:
> IRQ logging uses gc->parent. Keep that device in a local so the
> gpio-regmap switch can change where it comes from without rewriting
> every log line.
...
> - dev_warn(gc->parent, "no trigger selected for line %d\n",
> + dev_warn(parent, "no trigger selected for line %d\n",
> offset);
Make it a single line now.
dev_warn(parent, "no trigger selected for line %d\n", offset);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap
2026-10-27 4:18 ` [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
@ 2026-10-27 8:06 ` Andy Shevchenko
2026-10-27 10:32 ` zl020895
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2026-10-27 8:06 UTC (permalink / raw)
To: longzhao
Cc: Bartosz Golaszewski, linux-gpio, Linus Walleij, Michael Walle,
Rob Herring, Krzysztof Kozlowski, Krzysztof Kozlowski,
Krzysztof Kozlowski, Conor Dooley, devicetree, Long Zhao,
linux-kernel
On Tue, Oct 27, 2026 at 12:18:56PM +0800, Long Zhao via B4 Relay wrote:
>
> Move get, set and direction onto gpio-regmap. ARM PL061 sets
> GPIO_REGMAP_QUIRK_SET_AFTER_DIR so the output value is written
> after the direction change.
>
> Keep a custom irqchip and irq domain. Create the domain with
> the PL061 private data and let gpio-regmap attach it.
>
> Leave the chained handler's regmap_read() unchecked. mis stays 0
> on failure, so the pending loop does nothing, and
> chained_irq_exit() still runs. irq_type still ignores MMIO write
> returns.
>
> The irqchip uses its own child IRQ lockdep class and clears the
> chained handler only after it was installed, checked with lockdep
> on the CV75 board. irq_set_parent() nests
> the GPIO irq_desc under the AMBA parent for irq_set_wake(). The
> gpiochip IRQ resource helpers cannot be kept here: the gpio_chip is
> not reachable through gpio-regmap's external irq_domain path, so
> GPIOD_FLAG_USED_AS_IRQ is never tracked. This matches the other
> users of that path (gpio-idio-16, gpio-pcie-idio-24,
> gpio-104-idi-48).
Something happened to the text formatting, it's quite unbalanced in terms of
occupied room on each of the lines. Make each of the lines to occupy ~72
characters.
...
> +struct pl061_drvdata {
> + const struct regmap_config *regmap_config;
> + const struct pl061_regs *regs;
> + unsigned int ngpio;
Is here any gap (of 4 bytes) in accordance with `pahole`? If so, you better
relocate 'quirks' somewhere to make int and booleans be together.
> + unsigned long quirks;
> + bool clear_irq_on_type;
> + bool pm;
> + int (*reg_mask_xlate)(struct gpio_regmap *gpio,
> + unsigned int base, unsigned int offset,
> + unsigned int *reg, unsigned int *mask);
> };
...
> +static int pl061_irq_domain_map(struct irq_domain *d, unsigned int virq,
> + irq_hw_number_t hwirq)
> +{
> + struct pl061 *pl061 = d->host_data;
> +
> + irq_set_chip_data(virq, pl061);
> + irq_set_lockdep_class(virq, &pl061_irq_lock_class,
> + &pl061_irq_request_class);
Make it a single line.
> + irq_set_chip_and_handler(virq, &pl061_irq_chip, handle_bad_irq);
> + irq_set_noprobe(virq);
> + irq_set_parent(virq, pl061->parent_irq);
> +
> + return 0;
> +}
...
> static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
> {
> + struct gpio_regmap_config config = {};
> + const struct pl061_drvdata *data;
> struct device *dev = &adev->dev;
> - struct gpio_irq_chip *girq;
> + struct gpio_regmap *gpio_regmap;
> + const struct pl061_regs *regs;
> struct pl061 *pl061;
> void __iomem *base;
> - int ret, irq;
> + int ret;
> + int irq;
Unneeded change.
...
> + if (irq) {
> + irq_set_chained_handler_and_data(irq, pl061_irq_handler, pl061);
> + pl061->irq_handler_installed = true;
Can't we remove the chained handler unconditionally?
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging
2026-10-27 4:18 ` [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging Long Zhao via B4 Relay
@ 2026-10-27 8:08 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-10-27 8:08 UTC (permalink / raw)
To: longzhao
Cc: Bartosz Golaszewski, linux-gpio, Linus Walleij, Michael Walle,
Rob Herring, Krzysztof Kozlowski, Krzysztof Kozlowski,
Krzysztof Kozlowski, Conor Dooley, devicetree, Long Zhao,
linux-kernel
On Tue, Oct 27, 2026 at 12:18:58PM +0800, Long Zhao via B4 Relay wrote:
>
> Join the level-and-edge dev_err() into one string literal so a grep
> for "edge detection" finds it.
This is fine change.
> Use str_high_low() for the level debug line. str_rising_falling()
> is not in linux-next, so the edge debug line keeps its own text.
This should be a separate change and it requires to have str_raising_falling()
to be added to string_choices.h for the consistency's sake.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re:Re: [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap
2026-10-27 8:06 ` Andy Shevchenko
@ 2026-10-27 10:32 ` zl020895
2026-10-27 10:50 ` Andy Shevchenko
0 siblings, 1 reply; 18+ messages in thread
From: zl020895 @ 2026-10-27 10:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: longzhao, Bartosz Golaszewski, linux-gpio, Linus Walleij,
Michael Walle, Rob Herring, Krzysztof Kozlowski,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, linux-kernel
At 2026-10-27 16:06:15, "Andy Shevchenko" <andriy.shevchenko@linux.intel.com> wrote:
>On Tue, Oct 27, 2026 at 12:18:56PM +0800, Long Zhao via B4 Relay wrote:
>
>> + if (irq) {
>> + irq_set_chained_handler_and_data(irq, pl061_irq_handler, pl061);
>
>> + pl061->irq_handler_installed = true;
>
>Can't we remove the chained handler unconditionally?
>
>> + }
>
>--
>With Best Regards,
>Andy Shevchenko
>
Hi Andy,
Thanks for the review.
On the chained-handler teardown: pl061_remove_irq() is registered
before devm_gpio_regmap_register(), so on unbind it runs after
gpiochip_remove(). gpiochip_free_remaining_irqs() therefore still
sees the irq domain and its mappings; the action then disposes
them. The chained handler is installed only after
devm_gpio_regmap_register() succeeds. If that call fails, the
action still runs although no handler was installed, which is
why irq_handler_installed exists.
Options for v13:
A) Keep irq_handler_installed and document this ordering.
B) Split into two devm actions. The early one, still registered
before the gpiochip, only calls irq_dispose_mapping(), so
unbind order is unchanged. A second action, registered right
after irq_set_chained_handler_and_data(), clears the handler
unconditionally, and the flag goes away.
Do you prefer A or B?
Best regards,
Long Zhao
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Re: [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap
2026-10-27 10:32 ` zl020895
@ 2026-10-27 10:50 ` Andy Shevchenko
0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2026-10-27 10:50 UTC (permalink / raw)
To: zl020895
Cc: longzhao, Bartosz Golaszewski, linux-gpio, Linus Walleij,
Michael Walle, Rob Herring, Krzysztof Kozlowski,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, linux-kernel
On Tue, Oct 27, 2026 at 06:32:04PM +0800, zl020895 wrote:
> At 2026-10-27 16:06:15, "Andy Shevchenko" <andriy.shevchenko@linux.intel.com> wrote:
> >On Tue, Oct 27, 2026 at 12:18:56PM +0800, Long Zhao via B4 Relay wrote:
...
> >> + if (irq) {
> >> + irq_set_chained_handler_and_data(irq, pl061_irq_handler, pl061);
> >
> >> + pl061->irq_handler_installed = true;
> >
> >Can't we remove the chained handler unconditionally?
> >
> >> + }
> On the chained-handler teardown: pl061_remove_irq() is registered
> before devm_gpio_regmap_register(), so on unbind it runs after
> gpiochip_remove(). gpiochip_free_remaining_irqs() therefore still
> sees the irq domain and its mappings; the action then disposes
> them. The chained handler is installed only after
> devm_gpio_regmap_register() succeeds. If that call fails, the
> action still runs although no handler was installed, which is
> why irq_handler_installed exists.
>
> Options for v13:
> A) Keep irq_handler_installed and document this ordering.
> B) Split into two devm actions. The early one, still registered
> before the gpiochip, only calls irq_dispose_mapping(), so
> unbind order is unchanged. A second action, registered right
> after irq_set_chained_handler_and_data(), clears the handler
> unconditionally, and the flag goes away.
>
> Do you prefer A or B?
Option B seems plausible.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-10-27 10:50 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 4:18 [PATCH v12 00/11] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 01/11] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 02/11] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 03/11] gpio: pl061: drop redundant pending check in the IRQ handler Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 04/11] gpio: pl061: convert register access to regmap Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 05/11] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 06/11] gpio: pl061: use cleanup helpers for locks Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 07/11] gpio: pl061: keep the parent device in a local Long Zhao via B4 Relay
2026-10-27 7:42 ` Andy Shevchenko
2026-10-27 4:18 ` [PATCH v12 08/11] gpiolib: request ACPI GPIO events for external IRQ domains Long Zhao via B4 Relay
2026-10-27 5:29 ` Mika Westerberg
2026-10-27 4:18 ` [PATCH v12 09/11] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
2026-10-27 8:06 ` Andy Shevchenko
2026-10-27 10:32 ` zl020895
2026-10-27 10:50 ` Andy Shevchenko
2026-10-27 4:18 ` [PATCH v12 10/11] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
2026-10-27 4:18 ` [PATCH v12 11/11] gpio: pl061: tidy up IRQ trigger logging Long Zhao via B4 Relay
2026-10-27 8:08 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®