* [PATCH 1/7] dt-bindings: gpio: pl061: add Ambarella CV75 variant
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 11:32 ` Krzysztof Kozlowski
2026-10-09 10:57 ` [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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>
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 related [flat|nested] 14+ messages in thread* Re: [PATCH 1/7] dt-bindings: gpio: pl061: add Ambarella CV75 variant
2026-10-09 10:57 ` [PATCH 1/7] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
@ 2026-10-09 11:32 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-09 11:32 UTC (permalink / raw)
To: longzhao, Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, Long Zhao, linux-kernel, Conor Dooley
On 09/10/2026 12:57, Long Zhao via B4 Relay wrote:
> 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>
> Signed-off-by: Long Zhao <longzhao@ambarella.com>
> ---
> .../devicetree/bindings/gpio/pl061-gpio.yaml | 25 +++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
This depends on vendor prefix patch at least from the linux-next point
of view.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 1/7] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 11:17 ` Michael Walle
2026-10-09 11:27 ` sashiko-bot
2026-10-09 10:57 ` [PATCH 3/7] gpio: pl061: convert register access to regmap Long Zhao via B4 Relay
` (5 subsequent siblings)
7 siblings, 2 replies; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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>
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 related [flat|nested] 14+ messages in thread* Re: [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
2026-10-09 10:57 ` [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
@ 2026-10-09 11:17 ` Michael Walle
2026-10-09 11:27 ` sashiko-bot
1 sibling, 0 replies; 14+ messages in thread
From: Michael Walle @ 2026-10-09 11:17 UTC (permalink / raw)
To: longzhao, Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, Long Zhao, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 502 bytes --]
On Fri Oct 9, 2026 at 12:57 PM CEST, Long Zhao via B4 Relay wrote:
> 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>
> Signed-off-by: Long Zhao <longzhao@ambarella.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
2026-10-09 10:57 ` [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
2026-10-09 11:17 ` Michael Walle
@ 2026-10-09 11:27 ` sashiko-bot
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-09 11:27 UTC (permalink / raw)
To: Long Zhao; +Cc: devicetree, conor+dt, robh, linux-gpio
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: regmap: missing return value check for gpio_regmap_set in direction_output
--
commit 4dfbff55c3fbea74424f90710c5b76efb22f738e
Author: Long Zhao <longzhao@ambarella.com>
gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR
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.
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 51b4d69b87403..0a44deae3d01e 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
[ ... ]
> @@ -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;
> }
[Severity: Medium]
Does this code silently ignore hardware or bus errors?
If gpio_regmap_set() fails due to an underlying I2C/SPI bus timeout or NACK,
it returns a negative error code. However, the return value is dropped in the
newly added GPIO_REGMAP_QUIRK_SET_AFTER_DIR path, just as it is in the
pre-existing path.
Could this falsely signal to the gpiolib framework that the output pin was
successfully configured, leaving the hardware in an undefined state and
masking the error from consumers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-cv75-gpio-v1-0-5836bd438423@ambarella.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/7] gpio: pl061: convert register access to regmap
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 1/7] dt-bindings: gpio: pl061: add Ambarella CV75 variant Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 2/7] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 4/7] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH Long Zhao via B4 Relay
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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>
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 | 135 ++++++++++++++++++++++++++++++----------------
2 files changed, 90 insertions(+), 46 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 919cf86fd590..2b1e23fdb7d7 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_is_data_reg(unsigned int reg)
+{
+ return is_power_of_2(reg) &&
+ reg >= BIT(PL061_DATA_OFFSET) &&
+ reg <= BIT(PL061_DATA_OFFSET + PL061_GPIO_NR - 1);
+}
+
+static bool pl061_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case GPIOMIS:
+ case GPIOIC:
+ return true;
+ default:
+ return pl061_is_data_reg(reg);
+ }
+}
+
+static const struct regmap_config pl061_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .reg_stride = 4,
+ .max_register = GPIOIC,
+ .volatile_reg = pl061_volatile_reg,
+ .fast_io = true,
+ .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;
@@ -86,19 +119,19 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
{
struct pl061 *pl061 = gpiochip_get_data(gc);
unsigned long flags;
- unsigned char gpiodir;
+ unsigned int mask = BIT(offset);
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_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ !!value << offset);
+ 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_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ !!value << offset);
raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
@@ -108,14 +141,17 @@ 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)));
+ regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
+ !!value << offset);
return 0;
}
@@ -126,8 +162,8 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
struct pl061 *pl061 = gpiochip_get_data(gc);
int offset = irqd_to_hwirq(d);
unsigned long flags;
- u8 gpiois, gpioibe, gpioiev;
- u8 bit = BIT(offset);
+ unsigned int gpiois = 0, gpioibe = 0, gpioiev = 0;
+ unsigned int bit = BIT(offset);
if (offset < 0 || offset >= PL061_GPIO_NR)
return -EINVAL;
@@ -145,9 +181,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 +235,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);
@@ -211,6 +247,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
static void pl061_irq_handler(struct irq_desc *desc)
{
unsigned long pending;
+ unsigned int mis = 0;
int offset;
struct gpio_chip *gc = irq_desc_get_handler_data(desc);
struct pl061 *pl061 = gpiochip_get_data(gc);
@@ -218,8 +255,9 @@ static void pl061_irq_handler(struct irq_desc *desc)
chained_irq_enter(irqchip, desc);
- pending = readb(pl061->base + GPIOMIS);
- if (pending) {
+ regmap_read(pl061->regmap, GPIOMIS, &mis);
+ if (mis) {
+ pending = mis;
for_each_set_bit(offset, &pending, PL061_GPIO_NR)
generic_handle_domain_irq(gc->irq.domain,
offset);
@@ -233,11 +271,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);
@@ -248,13 +284,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);
}
@@ -272,9 +306,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)
@@ -308,15 +340,20 @@ 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;
+ 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_regmap_config);
+ if (IS_ERR(pl061->regmap))
+ return PTR_ERR(pl061->regmap);
raw_spin_lock_init(&pl061->lock);
pl061->gc.request = gpiochip_generic_request;
@@ -335,7 +372,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");
@@ -366,14 +403,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)))
@@ -398,10 +441,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 related [flat|nested] 14+ messages in thread* [PATCH 4/7] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (2 preceding siblings ...)
2026-10-09 10:57 ` [PATCH 3/7] gpio: pl061: convert register access to regmap Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 5/7] gpio: pl061: use cleanup helpers for locks Long Zhao via B4 Relay
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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 2b1e23fdb7d7..35aabe4beff6 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -168,9 +168,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",
@@ -185,7 +183,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 related [flat|nested] 14+ messages in thread* [PATCH 5/7] gpio: pl061: use cleanup helpers for locks
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (3 preceding siblings ...)
2026-10-09 10:57 ` [PATCH 4/7] gpio: pl061: use IRQ_TYPE_LEVEL_MASK and IRQ_TYPE_EDGE_BOTH Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 10:57 ` [PATCH 6/7] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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>
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 35aabe4beff6..b7632b30a909 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,9 @@ 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;
}
@@ -118,10 +117,9 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
int value)
{
struct pl061 *pl061 = gpiochip_get_data(gc);
- unsigned long flags;
unsigned int mask = BIT(offset);
- raw_spin_lock_irqsave(&pl061->lock, flags);
+ guard(raw_spinlock_irqsave)(&pl061->lock);
regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
!!value << offset);
regmap_set_bits(pl061->regmap, GPIODIR, mask);
@@ -132,7 +130,6 @@ static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
*/
regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
!!value << offset);
- raw_spin_unlock_irqrestore(&pl061->lock, flags);
return 0;
}
@@ -161,7 +158,6 @@ 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 long flags;
unsigned int gpiois = 0, gpioibe = 0, gpioiev = 0;
unsigned int bit = BIT(offset);
@@ -177,7 +173,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
}
- 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);
@@ -237,7 +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;
}
@@ -270,9 +265,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);
+ guard(raw_spinlock)(&pl061->lock);
regmap_clear_bits(pl061->regmap, GPIOIE, mask);
- raw_spin_unlock(&pl061->lock);
gpiochip_disable_irq(gc, d->hwirq);
}
@@ -285,9 +279,8 @@ static void pl061_irq_unmask(struct irq_data *d)
gpiochip_enable_irq(gc, d->hwirq);
- raw_spin_lock(&pl061->lock);
+ guard(raw_spinlock)(&pl061->lock);
regmap_set_bits(pl061->regmap, GPIOIE, mask);
- raw_spin_unlock(&pl061->lock);
}
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/7] gpio: pl061: switch data path to gpio-regmap
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (4 preceding siblings ...)
2026-10-09 10:57 ` [PATCH 5/7] gpio: pl061: use cleanup helpers for locks Long Zhao via B4 Relay
@ 2026-10-09 10:57 ` Long Zhao via B4 Relay
2026-10-09 11:50 ` sashiko-bot
2026-10-09 10:58 ` [PATCH 7/7] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
2026-10-09 11:32 ` [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Krzysztof Kozlowski
7 siblings, 1 reply; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:57 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.
Check the chained-handler regmap_read() return and always run
chained_irq_exit(). irq_type still ignores MMIO write returns.
irq_set_parent() nests the GPIO irq_desc under the AMBA parent
for irq_set_wake() and lockdep; checked with PROVE_LOCKING on
CV75.
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-pl061.c | 506 ++++++++++++++++++++++++++--------------------
2 files changed, 288 insertions(+), 219 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 b7632b30a909..ba3350a8971c 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -12,172 +12,111 @@
#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
-#define GPIOIS 0x404
-#define GPIOIBE 0x408
-#define GPIOIEV 0x40C
-#define GPIOIE 0x410
-#define GPIORIS 0x414
-#define GPIOMIS 0x418
-#define GPIOIC 0x41C
-
#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;
+#define PL061_DIR 0x400
+#define PL061_IS 0x404
+#define PL061_IBE 0x408
+#define PL061_IEV 0x40c
+#define PL061_IE 0x410
+#define PL061_MIS 0x418
+#define PL061_IC 0x41c
+
+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 device *dev;
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;
};
-static bool pl061_is_data_reg(unsigned int reg)
-{
- return is_power_of_2(reg) &&
- reg >= BIT(PL061_DATA_OFFSET) &&
- reg <= BIT(PL061_DATA_OFFSET + PL061_GPIO_NR - 1);
-}
-
-static bool pl061_volatile_reg(struct device *dev, unsigned int reg)
-{
- switch (reg) {
- case GPIOMIS:
- case GPIOIC:
- return true;
- default:
- return pl061_is_data_reg(reg);
- }
-}
-
-static const struct regmap_config pl061_regmap_config = {
- .reg_bits = 16,
- .val_bits = 8,
- .reg_stride = 4,
- .max_register = GPIOIC,
- .volatile_reg = pl061_volatile_reg,
- .fast_io = true,
- .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 (ret)
- return GPIO_LINE_DIRECTION_OUT;
-
- return GPIO_LINE_DIRECTION_IN;
-}
-
-static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
-{
- struct pl061 *pl061 = gpiochip_get_data(gc);
-
- guard(raw_spinlock_irqsave)(&pl061->lock);
- regmap_clear_bits(pl061->regmap, GPIODIR, BIT(offset));
-
- return 0;
-}
-
-static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
- int value)
+static unsigned int pl061_line_mask(struct pl061 *pl061, irq_hw_number_t hwirq)
{
- struct pl061 *pl061 = gpiochip_get_data(gc);
- unsigned int mask = BIT(offset);
-
- guard(raw_spinlock_irqsave)(&pl061->lock);
- regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- !!value << offset);
- 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_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- !!value << offset);
-
- return 0;
+ return BIT(hwirq % pl061->data->ngpio);
}
-static int pl061_get_value(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);
-
- 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);
-
- regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
- !!value << offset);
+ if (base == 0)
+ *reg = BIT(offset + PL061_DATA_OFFSET);
+ else
+ *reg = base;
+ *mask = BIT(offset);
return 0;
}
-static int pl061_irq_type(struct irq_data *d, unsigned trigger)
+static int pl061_irq_type(struct irq_data *d, unsigned int trigger)
{
- 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);
+ const struct pl061_regs *regs = pl061->data->regs;
int offset = irqd_to_hwirq(d);
unsigned int gpiois = 0, gpioibe = 0, gpioiev = 0;
unsigned int bit = BIT(offset);
- 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)) {
- dev_err(gc->parent,
+ dev_err(pl061->dev,
"trying to configure line %d for both level and edge "
"detection, choose one!\n",
offset);
return -EINVAL;
}
-
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;
@@ -192,7 +131,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(pl061->dev, "line %d: IRQ on %s level\n",
offset,
polarity ? "HIGH" : "LOW");
} else if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
@@ -201,7 +140,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(pl061->dev, "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 +155,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(pl061->dev, "line %d: IRQ on %s edge\n",
offset,
rising ? "RISING" : "FALLING");
} else {
@@ -225,14 +164,15 @@ 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(pl061->dev, "no trigger selected for line %d\n",
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;
}
@@ -241,46 +181,40 @@ static void pl061_irq_handler(struct irq_desc *desc)
{
unsigned long pending;
unsigned int mis = 0;
- int offset;
- struct gpio_chip *gc = irq_desc_get_handler_data(desc);
- struct pl061 *pl061 = gpiochip_get_data(gc);
+ int offset, ret;
+ struct pl061 *pl061 = irq_desc_get_handler_data(desc);
struct irq_chip *irqchip = irq_desc_get_chip(desc);
chained_irq_enter(irqchip, desc);
- regmap_read(pl061->regmap, GPIOMIS, &mis);
+ ret = regmap_read(pl061->regmap, pl061->data->regs->mis, &mis);
+ if (ret)
+ goto out;
+
if (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);
}
+out:
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);
-
- guard(raw_spinlock)(&pl061->lock);
- regmap_clear_bits(pl061->regmap, GPIOIE, mask);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ unsigned int mask = pl061_line_mask(pl061, irqd_to_hwirq(d));
- 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);
+ struct pl061 *pl061 = irq_data_get_irq_chip_data(d);
+ unsigned int mask = pl061_line_mask(pl061, irqd_to_hwirq(d));
- gpiochip_enable_irq(gc, d->hwirq);
-
- guard(raw_spinlock)(&pl061->lock);
- regmap_set_bits(pl061->regmap, GPIOIE, mask);
+ regmap_set_bits(pl061->regmap, pl061->data->regs->ie, mask);
}
/**
@@ -293,29 +227,28 @@ 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 pl061 *pl061 = irq_data_get_irq_chip_data(data);
- seq_puts(p, dev_name(gc->parent));
+ seq_puts(p, dev_name(pl061->dev));
}
-static const struct irq_chip pl061_irq_chip = {
+static const struct irq_chip pl061_irqchip = {
+ .name = "pl061",
.irq_ack = pl061_irq_ack,
.irq_mask = pl061_irq_mask,
.irq_unmask = pl061_irq_unmask,
@@ -323,67 +256,187 @@ 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 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_chip_and_handler(virq, &pl061_irqchip, 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;
+
+ irq_set_chained_handler_and_data(pl061->parent_irq, NULL, NULL);
+
+ for (unsigned int i = 0; i < pl061->data->ngpio; i++) {
+ unsigned int virq;
+
+ virq = irq_find_mapping(pl061->irq_domain, i);
+ if (virq)
+ irq_dispose_mapping(virq);
+ }
+
+ irq_domain_remove(pl061->irq_domain);
+}
+
+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 PL061_MIS:
+ case PL061_IC:
+ 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 = PL061_IC,
+ .volatile_reg = pl061_arm_volatile_reg,
+ .cache_type = REGCACHE_FLAT_S,
+ .fast_io = true,
+ .use_raw_spinlock = true,
+};
+
+static const struct pl061_regs pl061_arm_regs = {
+ .dir = PL061_DIR,
+ .is = PL061_IS,
+ .ibe = PL061_IBE,
+ .iev = PL061_IEV,
+ .ie = PL061_IE,
+ .mis = PL061_MIS,
+ .ic = PL061_IC,
+};
+
+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 device *dev = &adev->dev;
+ const struct pl061_drvdata *data;
+ const struct pl061_regs *regs;
+ struct gpio_regmap_config config = {};
+ struct gpio_regmap *gpio_regmap;
struct pl061 *pl061;
- struct gpio_irq_chip *girq;
void __iomem *base;
int ret, 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->dev = dev;
+ 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_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) {
+ pl061->irq_domain = irq_domain_create_linear(dev_fwnode(dev),
+ data->ngpio,
+ &pl061_irq_domain_ops,
+ pl061);
+ if (!pl061->irq_domain)
+ return -ENOMEM;
+ }
- 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)) {
+ if (pl061->irq_domain)
+ irq_domain_remove(pl061->irq_domain);
+ return PTR_ERR(gpio_regmap);
+ }
+
+ if (irq) {
+ ret = devm_add_action_or_reset(dev, pl061_remove_irq, pl061);
+ if (ret)
+ return ret;
+
+ irq_set_chained_handler_and_data(irq, pl061_irq_handler, pl061);
+ }
amba_set_drvdata(adev, pl061);
dev_info(dev, "PL061 GPIO chip registered\n");
@@ -391,28 +444,38 @@ 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, val;
+ unsigned int offset;
+
+ 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;
@@ -421,23 +484,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, 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)) << 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);
@@ -446,6 +513,7 @@ static const struct amba_id pl061_ids[] = {
{
.id = 0x00041061,
.mask = 0x000fffff,
+ .data = (void *)&pl061_arm,
},
{ 0, 0 },
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/7] gpio: pl061: switch data path to gpio-regmap
2026-10-09 10:57 ` [PATCH 6/7] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
@ 2026-10-09 11:50 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-09 11:50 UTC (permalink / raw)
To: Long Zhao; +Cc: devicetree, robh, conor+dt, linux-gpio
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] gpio: UAF of irq_domain during devm teardown due to LIFO ordering
- [High] gpio: Missing GPIOCHIP_IRQ_RESOURCE_HELPERS allows GPIO/IRQ conflicts
- [High] gpio: Missing synchronize_irq() causes UAF on chained handler removal
--
commit b485edbbc0d9055aa12255ec0a09e3fe648808bb
Author: Long Zhao <longzhao@ambarella.com>
gpio: pl061: switch data path to gpio-regmap
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.
> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index b7632b30a9098..ba3350a8971c8 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
[ ... ]
> @@ -293,29 +227,28 @@ static void pl061_irq_unmask(struct irq_data *d)
[ ... ]
> -static const struct irq_chip pl061_irq_chip = {
> +static const struct irq_chip pl061_irqchip = {
> + .name = "pl061",
> .irq_ack = pl061_irq_ack,
> .irq_mask = pl061_irq_mask,
> .irq_unmask = pl061_irq_unmask,
> @@ -323,67 +256,187 @@ 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,
> -};
[Severity: High]
Does this code omit the GPIOCHIP_IRQ_RESOURCE_HELPERS macro?
Without the .irq_request_resources and .irq_release_resources callbacks it
provides, gpiolib won't be notified when an IRQ is requested, and
gpiochip_lock_as_irq() is skipped.
Could this allow the GPIO line to be concurrently configured as an output
while it is actively firing interrupts?
[ ... ]
> +static void pl061_remove_irq(void *data)
> +{
> + struct pl061 *pl061 = data;
> +
> + irq_set_chained_handler_and_data(pl061->parent_irq, NULL, NULL);
[Severity: High]
Can a concurrent interrupt handler still be executing here?
This replaces the flow handler but doesn't wait for currently executing
handlers to finish like synchronize_irq() would.
> +
> + for (unsigned int i = 0; i < pl061->data->ngpio; i++) {
> + unsigned int virq;
> +
> + virq = irq_find_mapping(pl061->irq_domain, i);
> + if (virq)
> + irq_dispose_mapping(virq);
> + }
> +
> + irq_domain_remove(pl061->irq_domain);
[Severity: High]
Could the lack of synchronization against concurrent interrupts lead to a
use-after-free here?
If pl061_irq_handler() is concurrently executing on another CPU, it might call
generic_handle_domain_irq(pl061->irq_domain, ...) and dereference the
domain after it has been freed.
> +}
[ ... ]
> + gpio_regmap = devm_gpio_regmap_register(dev, &config);
> + if (IS_ERR(gpio_regmap)) {
> + if (pl061->irq_domain)
> + irq_domain_remove(pl061->irq_domain);
> + return PTR_ERR(gpio_regmap);
> + }
> +
> + if (irq) {
> + ret = devm_add_action_or_reset(dev, pl061_remove_irq, pl061);
[Severity: High]
Because devm executes actions in reverse (LIFO) order, pl061_remove_irq() will
execute before the gpio_regmap is unregistered.
Will this destroy the irq_domain while the gpio_chip is still active?
If so, when the gpio_chip is later unregistered, gpiochip_remove() might
dereference the freed domain pointer when attempting to tear down requested
IRQs.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-cv75-gpio-v1-0-5836bd438423@ambarella.com?part=6
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 7/7] gpio: pl061: add Ambarella register-layout variant
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (5 preceding siblings ...)
2026-10-09 10:57 ` [PATCH 6/7] gpio: pl061: switch data path to gpio-regmap Long Zhao via B4 Relay
@ 2026-10-09 10:58 ` Long Zhao via B4 Relay
2026-10-09 12:03 ` sashiko-bot
2026-10-09 11:32 ` [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Krzysztof Kozlowski
7 siblings, 1 reply; 14+ messages in thread
From: Long Zhao via B4 Relay @ 2026-10-09 10:58 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.
Signed-off-by: Long Zhao <longzhao@ambarella.com>
---
drivers/gpio/gpio-pl061.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/amba/bus.h | 1 +
2 files changed, 60 insertions(+)
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index ba3350a8971c..ccadeb118305 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -38,6 +38,18 @@
#define PL061_MIS 0x418
#define PL061_IC 0x41c
+#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;
@@ -318,6 +330,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,
@@ -329,6 +352,17 @@ 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,
+ .fast_io = true,
+ .use_raw_spinlock = true,
+};
+
static const struct pl061_regs pl061_arm_regs = {
.dir = PL061_DIR,
.is = PL061_IS,
@@ -339,6 +373,19 @@ static const struct pl061_regs pl061_arm_regs = {
.ic = PL061_IC,
};
+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,
@@ -348,6 +395,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 device *dev = &adev->dev;
@@ -515,6 +569,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 related [flat|nested] 14+ messages in thread* Re: [PATCH 7/7] gpio: pl061: add Ambarella register-layout variant
2026-10-09 10:58 ` [PATCH 7/7] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
@ 2026-10-09 12:03 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-09 12:03 UTC (permalink / raw)
To: Long Zhao; +Cc: linux-gpio, conor+dt, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio: pl061: regmap cache seeded with physical pin levels on first write
- [High] gpio: UAF of irq_domain in gpiochip_remove() due to devres ordering
--
commit 35420eba05adeb99d5c7ba4b27a388ab6107e2d2
Author: Long Zhao <longzhao@ambarella.com>
Subject: gpio: pl061: add Ambarella register-layout variant
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.
> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index ba3350a8971c8..ccadeb1183058 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
[ ... ]
> @@ -318,6 +330,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_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,
> + .fast_io = true,
> + .use_raw_spinlock = true,
> +};
[Severity: High]
Does this configuration cause the regmap cache to be seeded with physical
pin levels on the first write?
Because AMBARELLA_DAT is not marked as volatile in
pl061_ambarella_volatile_reg() and the regmap_config lacks .reg_defaults,
the cache is completely empty at probe. On the first call to gpio_set(),
the regmap core experiences a cache miss in _regmap_update_bits() and
falls back to a physical MMIO read via _regmap_read().
This populates the regmap cache with the physical input levels of all pins
on the bank. The single requested bit is then modified, and the full 32-bit
register is written back to hardware.
If any output pin was heavily loaded or externally driven during this
initial read, could its false physical state be latched and permanently
driven as an output, directly causing the glitch the patch aimed to prevent?
[ ... ]
> @@ -348,6 +395,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)
> {
[Severity: High]
Does unbinding the driver trigger a use-after-free of the irq_domain in
gpiochip_remove() due to devres ordering?
Inside pl061_probe(), the devres action to remove the IRQ is registered
after the gpio_regmap:
gpio_regmap = devm_gpio_regmap_register(dev, &config);
...
if (irq) {
ret = devm_add_action_or_reset(dev, pl061_remove_irq, pl061);
...
If a client or userspace holds a GPIO IRQ, unbinding the driver triggers
devres teardown in LIFO order. pl061_remove_irq() will run first, calling
irq_domain_remove(), which kfrees the irq_domain.
Next, devm_gpio_regmap_unregister() runs and calls gpiochip_remove().
This invokes gpiochip_free_remaining_irqs(), which calls gpiod_free_irqs()
and eventually gpiochip_to_irq():
drivers/gpio/gpiolib.c:gpiochip_to_irq() {
struct irq_domain *domain = gc->irq.domain;
...
return irq_create_mapping(domain, offset);
}
Because gc->irq.domain was already freed by pl061_remove_irq(), this causes
a use-after-free.
Can the devres registration order be adjusted to ensure the gpiochip is
removed before the irq_domain is freed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-cv75-gpio-v1-0-5836bd438423@ambarella.com?part=7
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] gpio: add Ambarella CV75 PL061 support
2026-10-09 10:57 [PATCH 0/7] gpio: add Ambarella CV75 PL061 support Long Zhao via B4 Relay
` (6 preceding siblings ...)
2026-10-09 10:58 ` [PATCH 7/7] gpio: pl061: add Ambarella register-layout variant Long Zhao via B4 Relay
@ 2026-10-09 11:32 ` Krzysztof Kozlowski
7 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-09 11:32 UTC (permalink / raw)
To: longzhao, Bartosz Golaszewski, linux-gpio
Cc: Linus Walleij, Michael Walle, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Krzysztof Kozlowski, Conor Dooley,
devicetree, Long Zhao, linux-kernel, Conor Dooley, Chanho Min
On 09/10/2026 12:57, Long Zhao via B4 Relay wrote:
> Split from the combined CV75 bring-up series. These patches convert
> the ARM PL061 data path to regmap, then add the CV75 layout.
>
> Please take this series through the GPIO tree. The quirk, the PL061
> conversion and the Ambarella layout belong together.
>
Here - in the most visible place - you document the dependencies or
merging restrictions.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread