[PATCH v2 0/2] gpio: add support for Axiado SGPIO controller

[PATCH v2 0/2] gpio: add support for Axiado SGPIO controller

[Petar Stepanovic]

SGPIO position provides one input GPIO and one output GPIO with fixed
directions, along with interrupt support for input GPIOs.

This series adds support for the Axiado SGPIO controller.

The series includes:
- Device tree binding documentation
- GPIO driver implementation

The driver integrates with the Linux GPIO subsystem and
registers the controller as a gpio_chip.

Tested on Axiado platforms.

---
Patch 1: dt-bindings: gpio: add Axiado SGPIO controller
Patch 2: gpio: axiado: add SGPIO controller support

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>

---
Changes in v2:
- Split the series into two patches: the binding and `MAINTAINERS`
  update, followed by the SGPIO driver.
- Added SoC-specific compatibles for AX3000 and AX3005 and selected
  hardware limits and register layouts through match data.
- Reworked clock handling to use one APB clock and calculate the SGPIO
  divider from `bus-frequency`.
- Removed the custom `design-variant`, `dout-init`, `apb-frequency`,
  and `bus-frequency` clock-source properties from the previous design.
- Added validation for the GPIO count, clock rates, and SGPIO divider
  calculation.
- Added correct GPIO direction handling: even offsets are inputs and odd
  offsets are outputs.
- Fixed child IRQ mapping for the interleaved input/output GPIO layout
  and
  restricted interrupts to input GPIOs.
- Reworked interrupt handling to support rising, falling, and both-edge
  detection through software filtering.
- Initialized the input cache before enabling interrupts to prevent
  false
  edge events during probe.
- Serialized output cache updates and register writes with a mutex to
  prevent lost concurrent updates.
- Updated GPIO IRQ handling to use GPIO IRQ resource helpers and
  threaded
  nested interrupts.
- Added managed hardware cleanup for probe failures and device removal.
- Updated the Kconfig regmap dependency for the MMIO-backed variant.
- Simplified resource management and error handling and performed
  general
  coding-style cleanup.
- Link to v1: https://lore.kernel.org/r/20260414-axiado-ax3000-sgpio-controller-v1-0-b5c7e4c2e69b@axiado.com

---
Petar Stepanovic (2):
      dt-bindings: gpio: Add Axiado SGPIO controller
      gpio: axiado: add SGPIO controller support

 .../devicetree/bindings/gpio/axiado,sgpio.yaml     | 118 +++
 MAINTAINERS                                        |   9 +
 drivers/gpio/Kconfig                               |  18 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-axiado-sgpio.c                   | 795 +++++++++++++++++++++
 5 files changed, 941 insertions(+)
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260320-axiado-ax3000-sgpio-controller-00f6e1db6ce9

Best regards,
-- 
Petar Stepanovic <pstepanovic@axiado.com>

[PATCH v2 1/2] dt-bindings: gpio: Add Axiado SGPIO controller

[Petar Stepanovic]

Add a Devicetree binding for the SGPIO controller found on the
Axiado AX3000 and AX3005 SoCs.

Each SGPIO position provides one fixed-direction input GPIO and one
fixed-direction output GPIO. Input GPIOs support edge-triggered
interrupts.

The controller serializes output values and samples input values over
dedicated data, clock, and latch signals.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 .../devicetree/bindings/gpio/axiado,sgpio.yaml     | 118 +++++++++++++++++++++
 MAINTAINERS                                        |   8 ++
 2 files changed, 126 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml b/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml
new file mode 100644
index 000000000000..448f01c367dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml
@@ -0,0 +1,118 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/axiado,sgpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Axiado SGPIO Controller
+
+maintainers:
+  - Petar Stepanovic <pstepanovic@axiado.com>
+  - SriNavmani A <srinavmani@axiado.com>
+  - Prasad Bolisetty <pbolisetty@axiado.com>
+
+description: |
+  The Axiado SGPIO controller provides a serial GPIO expansion interface
+  that multiplexes multiple GPIO signals over a small set of physical lines.
+
+  The controller operates as a shift-based interface, where output values
+  are serialized on a data output line and input values are sampled from a
+  data input line in synchronization with a shift clock. A latch signal is
+  used to update output states after each transfer cycle.
+
+  The interface uses data output (DOUT), data input (DIN), shift clock
+  (SCLK), and latch signal (LATCH).
+
+  Each SGPIO position provides one fixed-direction input GPIO and one
+  fixed-direction output GPIO. Input GPIOs support edge-triggered
+  interrupts.
+
+  GPIOs are organized in banks of 32 signals, with registers controlling
+  output values, input sampling, and interrupt status. The number of GPIO
+  lines depends on the hardware variant.
+
+  The controller detects changes on input GPIOs. Interrupt events are
+  aggregated and signaled through a single interrupt line to the parent
+  interrupt controller.
+
+  Register access is performed over the APB bus. The SGPIO shift clock is
+  derived from the APB clock using a programmable divider.
+
+properties:
+  compatible:
+    enum:
+      - axiado,ax3000-sgpio
+      - axiado,ax3005-sgpio
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description:
+      APB clock used for register access and SGPIO shift clock generation
+    maxItems: 1
+
+  bus-frequency:
+    description: SGPIO shift clock frequency in Hz
+    minimum: 1
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+  ngpios:
+    description:
+      Number of SGPIO positions. Each position provides one input GPIO and
+      one output GPIO.
+    enum: [128, 512]
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          const: axiado,ax3000-sgpio
+    then:
+      properties:
+        ngpios:
+          const: 128
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - bus-frequency
+  - gpio-controller
+  - '#gpio-cells'
+  - interrupt-controller
+  - '#interrupt-cells'
+  - interrupts
+  - ngpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    sgpio@a000 {
+        compatible = "axiado,ax3005-sgpio";
+        reg = <0xa000 0x800>;
+        clocks = <&apb_clk>;
+        bus-frequency = <1000000>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        interrupt-parent = <&gpio6>;
+        interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+        ngpios = <128>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a674e36529f7..fc613271f435 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4407,6 +4407,14 @@ S:	Orphan
 F:	Documentation/devicetree/bindings/sound/axentia,*
 F:	sound/soc/atmel/tse850-pcm5142.c
 
+AXIADO SGPIO DRIVER
+M:	Petar Stepanovic <pstepanovic@axiado.com>
+M:	SriNavmani A <srinavmani@axiado.com>
+M:	Prasad Bolisetty <pbolisetty@axiado.com>
+L:	linux-gpio@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml
+
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
 M:	Lars Persson <lars.persson@axis.com>

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: gpio: Add Axiado SGPIO controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Schema Validation Logic Bug: The `if` condition for `axiado,ax3000-sgpio` incorrectly uses `const` instead of `contains: const`.
--

--- Patch [1]: [PATCH v2 1/2] dt-bindings: gpio: Add Axiado SGPIO controller ---
commit f6cc693f3d06f8cf16f8579009218084ca514b3e
Author: Petar Stepanovic <pstepanovic@axiado.com>

dt-bindings: gpio: Add Axiado SGPIO controller

This commit adds a Devicetree binding for the SGPIO controller found on the
Axiado AX3000 and AX3005 SoCs. The controller operates as a shift-based
interface to multiplex multiple GPIO signals over a small set of physical lines.

> diff --git a/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml b/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml
> new file mode 100644
> index 0000000000000..448f01c367dc1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: axiado,ax3000-sgpio

[Severity: Medium]
Does this condition correctly match the compatible string?

Since the compatible property is evaluated as an array of strings in the
device tree, using const to match directly against a scalar string will always
evaluate to false. 

Because this condition always fails, it allows invalid device tree
configurations to silently pass validation since the ngpios constraint for the
axiado,ax3000-sgpio variant will not be enforced during a dt_binding_check.

Would it be better to use contains: const: axiado,ax3000-sgpio instead?

> +    then:
> +      properties:
> +        ngpios:
> +          const: 128

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-axiado-ax3000-sgpio-controller-v2-0-976f4ed11438@axiado.com?part=1

Re: [PATCH v2 1/2] dt-bindings: gpio: Add Axiado SGPIO controller

[Krzysztof Kozlowski]

On 29/07/2026 09:02, Petar Stepanovic wrote:
> +description: |
> +  The Axiado SGPIO controller provides a serial GPIO expansion interface
> +  that multiplexes multiple GPIO signals over a small set of physical lines.
> +
> +  The controller operates as a shift-based interface, where output values
> +  are serialized on a data output line and input values are sampled from a
> +  data input line in synchronization with a shift clock. A latch signal is
> +  used to update output states after each transfer cycle.
> +
> +  The interface uses data output (DOUT), data input (DIN), shift clock
> +  (SCLK), and latch signal (LATCH).
> +
> +  Each SGPIO position provides one fixed-direction input GPIO and one
> +  fixed-direction output GPIO. Input GPIOs support edge-triggered
> +  interrupts.
> +
> +  GPIOs are organized in banks of 32 signals, with registers controlling
> +  output values, input sampling, and interrupt status. The number of GPIO
> +  lines depends on the hardware variant.
> +
> +  The controller detects changes on input GPIOs. Interrupt events are
> +  aggregated and signaled through a single interrupt line to the parent
> +  interrupt controller.
> +
> +  Register access is performed over the APB bus. The SGPIO shift clock is
> +  derived from the APB clock using a programmable divider.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - axiado,ax3000-sgpio
> +      - axiado,ax3005-sgpio
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description:
> +      APB clock used for register access and SGPIO shift clock generation
> +    maxItems: 1
> +
> +  bus-frequency:
> +    description: SGPIO shift clock frequency in Hz
> +    minimum: 1

Such property is not allowed. Why you cannot calculate it automatically?

> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  ngpios:
> +    description:
> +      Number of SGPIO positions. Each position provides one input GPIO and
> +      one output GPIO.
> +    enum: [128, 512]
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: axiado,ax3000-sgpio
> +    then:
> +      properties:
> +        ngpios:
> +          const: 128

else what? 3005 has 512? Then these are implied by compatible, no?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - bus-frequency
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - interrupt-controller
> +  - '#interrupt-cells'
> +  - interrupts
> +  - ngpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    sgpio@a000 {

git grep sgpio@

No results (except well known poor coding style platform). I already
commented on this.

> +        compatible = "axiado,ax3005-sgpio";
> +        reg = <0xa000 0x800>;
> +        clocks = <&apb_clk>;
> +        bus-frequency = <1000000>;
> +        gpio-controller;
Best regards,
Krzysztof

[PATCH v2 2/2] gpio: axiado: add SGPIO controller support

[Petar Stepanovic]

Add support for the Axiado SGPIO controller.

Each SGPIO position provides one input GPIO and one output GPIO with
fixed directions. The driver registers the controller as a gpio_chip,
supports interrupts on input GPIOs, and uses regmap for register access.

Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
---
 MAINTAINERS                      |   1 +
 drivers/gpio/Kconfig             |  18 +
 drivers/gpio/Makefile            |   1 +
 drivers/gpio/gpio-axiado-sgpio.c | 795 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 815 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fc613271f435..dd556d9252bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4414,6 +4414,7 @@ M:	Prasad Bolisetty <pbolisetty@axiado.com>
 L:	linux-gpio@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/gpio/axiado,sgpio.yaml
+F:	drivers/gpio/gpio-axiado-sgpio.c
 
 AXIS ARTPEC ARM64 SoC SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 28cf6d2e83c2..80db12d908bb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -204,6 +204,24 @@ config GPIO_ATH79
 	  Select this option to enable GPIO driver for
 	  Atheros AR71XX/AR724X/AR913X SoC devices.
 
+config GPIO_AXIADO_SGPIO
+	bool "Axiado SGPIO support"
+	depends on OF_GPIO
+	depends on ARCH_AXIADO || COMPILE_TEST
+	select GPIOLIB_IRQCHIP
+	select REGMAP_MMIO
+	help
+	  Enable support for the Axiado Serial GPIO (SGPIO) controller.
+
+	  The SGPIO controller provides a serialized interface for
+	  controlling multiple GPIO signals over a limited number of
+	  physical lines. Each SGPIO position provides one fixed-direction
+	  input GPIO and one fixed-direction output GPIO. Interrupts are
+	  supported on input GPIOs.
+
+	  This driver integrates with the Linux GPIO subsystem and
+	  exposes the controller as a standard GPIO provider.
+
 config GPIO_RASPBERRYPI_EXP
 	tristate "Raspberry Pi 3 GPIO Expander"
 	default RASPBERRYPI_FIRMWARE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4d0e900402fc..6ae73dbe7102 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_GPIO_ARIZONA)		+= gpio-arizona.o
 obj-$(CONFIG_GPIO_ASPEED)		+= gpio-aspeed.o
 obj-$(CONFIG_GPIO_ASPEED_SGPIO)		+= gpio-aspeed-sgpio.o
 obj-$(CONFIG_GPIO_ATH79)		+= gpio-ath79.o
+obj-$(CONFIG_GPIO_AXIADO_SGPIO)		+= gpio-axiado-sgpio.o
 obj-$(CONFIG_GPIO_BCM_KONA)		+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BCM_XGS_IPROC)	+= gpio-xgs-iproc.o
 obj-$(CONFIG_GPIO_BD71815)		+= gpio-bd71815.o
diff --git a/drivers/gpio/gpio-axiado-sgpio.c b/drivers/gpio/gpio-axiado-sgpio.c
new file mode 100644
index 000000000000..91e63239c99d
--- /dev/null
+++ b/drivers/gpio/gpio-axiado-sgpio.c
@@ -0,0 +1,795 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022-2026 Axiado Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+struct sgpio_reg_offsets {
+	u32 mux_0;
+	u32 preset_0;
+	u32 count_0;
+	u32 pos_0;
+
+	u32 mux_1;
+	u32 ld;
+	u32 ld_ss;
+
+	u32 preset_1;
+	u32 count_1;
+	u32 pos_1;
+
+	u32 mux_2;
+	u32 dout;
+	u32 dout_ss;
+
+	u32 preset_2;
+	u32 count_2;
+	u32 pos_2;
+
+	u32 mux_3;
+	u32 preset_3;
+	u32 count_3;
+	u32 pos_3;
+
+	u32 mux_4;
+	u32 oe;
+	u32 oe_ss;
+
+	u32 preset_4;
+	u32 count_4;
+	u32 pos_4;
+
+	u32 mask;
+	u32 ctrl_en;
+	u32 ctrl_en_pos;
+
+	u32 din_ss;
+	u32 status;
+};
+
+static const struct sgpio_reg_offsets sgpio_offsets_512 = {
+	.mux_0 = 0x000,
+	.preset_0 = 0x1dc,
+	.count_0 = 0x1f0,
+	.pos_0 = 0x204,
+
+	.mux_1 = 0x004,
+	.ld = 0x014,
+	.ld_ss = 0x0d8,
+
+	.preset_1 = 0x1e0,
+	.count_1 = 0x1f4,
+	.pos_1 = 0x208,
+
+	.mux_2 = 0x008,
+	.dout = 0x054,
+	.dout_ss = 0x158,
+
+	.preset_2 = 0x1e4,
+	.count_2 = 0x1f8,
+	.pos_2 = 0x20c,
+
+	.mux_3 = 0x00c,
+	.preset_3 = 0x1e8,
+	.count_3 = 0x1fc,
+	.pos_3 = 0x210,
+
+	.mux_4 = 0x010,
+	.oe = 0x0d4,
+	.oe_ss = 0x1d8,
+
+	.preset_4 = 0x1ec,
+	.count_4 = 0x200,
+	.pos_4 = 0x214,
+
+	.mask = 0x224,
+	.ctrl_en = 0x218,
+	.ctrl_en_pos = 0x21c,
+
+	.din_ss = 0x198,
+	.status = 0x228,
+};
+
+static const struct sgpio_reg_offsets sgpio_offsets_128 = {
+	.mux_0 = 0x000,
+	.preset_0 = 0x08c,
+	.count_0 = 0x0a0,
+	.pos_0 = 0x0b4,
+
+	.mux_1 = 0x004,
+	.ld = 0x014,
+	.ld_ss = 0x048,
+
+	.preset_1 = 0x090,
+	.count_1 = 0x0a4,
+	.pos_1 = 0x0b8,
+
+	.mux_2 = 0x008,
+	.dout = 0x024,
+	.dout_ss = 0x068,
+
+	.preset_2 = 0x094,
+	.count_2 = 0x0a8,
+	.pos_2 = 0x0bc,
+
+	.mux_3 = 0x00c,
+	.preset_3 = 0x098,
+	.count_3 = 0x0ac,
+	.pos_3 = 0x0c0,
+
+	.mux_4 = 0x010,
+	.oe = 0x044,
+	.oe_ss = 0x088,
+
+	.preset_4 = 0x09c,
+	.count_4 = 0x0b0,
+	.pos_4 = 0x0c4,
+
+	.mask = 0x0d4,
+	.ctrl_en = 0x0c8,
+	.ctrl_en_pos = 0x0cc,
+
+	.din_ss = 0x078,
+	.status = 0x0d8,
+};
+
+#define MAX_SGPIO_PINS 512
+#define MAX_OFFSET_REG 16
+#define MAX_SLICE_COUNT 5
+
+struct ax3000_slice_info {
+	u32 out_mux;
+	u32 sgpio_mux;
+	u32 slice_mux;
+	u32 reg[MAX_OFFSET_REG];
+	u32 reg_ss[MAX_OFFSET_REG];
+	u32 preset;
+	u32 count;
+	u32 pos;
+};
+
+struct ax3000_sgpio {
+	u32 preset_value;
+	u32 count_value;
+	u32 pos_reg;
+	struct ax3000_slice_info
+		slices[MAX_SLICE_COUNT]; /* 0=clk,1=load,2=out,3=in,4=oe */
+	spinlock_t lock;
+	struct mutex output_lock;
+	u32 ngpios;
+	u32 max_sgpio_pins;
+	u32 max_offset_regs;
+	struct gpio_chip chip;
+	u32 irq_unmasked[MAX_SGPIO_PINS];
+	unsigned int irq_type[MAX_SGPIO_PINS];
+	struct regmap *regmap;
+	u32 regmap_base_offset;
+	const struct sgpio_reg_offsets *regs;
+};
+
+struct axiado_sgpio_soc_data {
+	const struct sgpio_reg_offsets *regs;
+	u32 max_sgpio_pins;
+	u32 max_offset_regs;
+};
+
+static const struct axiado_sgpio_soc_data ax3000_sgpio_data = {
+	.regs = &sgpio_offsets_128,
+	.max_sgpio_pins = 128,
+	.max_offset_regs = 4,
+};
+
+static const struct axiado_sgpio_soc_data ax3005_sgpio_data = {
+	.regs = &sgpio_offsets_512,
+	.max_sgpio_pins = 512,
+	.max_offset_regs = 16,
+};
+
+static int sgpio_set_irq_type(struct irq_data *d, unsigned int type);
+static void sgpio_mask_irq(struct irq_data *d);
+static void sgpio_unmask_irq(struct irq_data *d);
+static void sgpio_irq_shutdown(struct irq_data *d);
+
+static const struct irq_chip axiado_sgpio_irqchip = {
+	.name = "axiado-sgpio",
+	.irq_mask = sgpio_mask_irq,
+	.irq_unmask = sgpio_unmask_irq,
+	.irq_set_type = sgpio_set_irq_type,
+	.irq_shutdown = sgpio_irq_shutdown,
+	.flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int ax3000_sgpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
+	u32 position = (offset / 2) % 32;
+	u32 bank = (offset / 2) / 32;
+	u32 val;
+	int ret;
+
+	if (!(offset % 2))
+		return -EINVAL;
+
+	guard(mutex)(&sgpio->output_lock);
+
+	val = sgpio->slices[2].reg_ss[bank];
+
+	if (value)
+		val |= BIT(position);
+	else
+		val &= ~BIT(position);
+
+	ret = regmap_write(sgpio->regmap,
+			   sgpio->regmap_base_offset + sgpio->regs->dout_ss +
+			   bank * sizeof(u32), val);
+	if (ret)
+		return ret;
+
+	sgpio->slices[2].reg_ss[bank] = val;
+
+	return 0;
+}
+
+static int ax3000_sgpio_get_direction(struct gpio_chip *chip,
+				      unsigned int offset)
+{
+	if (!(offset % 2))
+		return GPIO_LINE_DIRECTION_IN;
+
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int ax3000_sgpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
+	u32 position = (offset / 2) % 32;
+	u32 bank = (offset / 2) / 32;
+	u32 val;
+
+	if (!(offset % 2)) {
+		guard(spinlock_irqsave)(&sgpio->lock);
+
+		val = sgpio->slices[3].reg_ss[bank];
+	} else {
+		guard(mutex)(&sgpio->output_lock);
+
+		val = sgpio->slices[2].reg_ss[bank];
+	}
+
+	return !!(val & BIT(position));
+}
+
+static int ax3000_sgpio_dir_in(struct gpio_chip *chip, unsigned int offset)
+{
+	if (!(offset % 2))
+		return 0;
+
+	return -EINVAL;
+}
+
+static int ax3000_sgpio_dir_out(struct gpio_chip *chip, unsigned int offset,
+				int value)
+{
+	/* Even offsets represent input GPIOs. */
+	if (!(offset % 2))
+		return -EINVAL;
+
+	return ax3000_sgpio_set(chip, offset, value);
+}
+
+static irqreturn_t sgpio_irq_handler(int irq, void *arg)
+{
+	struct ax3000_sgpio *sgpio = (struct ax3000_sgpio *)arg;
+	u32 status, new_value;
+	u32 changed_value;
+	int bit, reg_ptr;
+	int ret, i;
+
+	/* Read-on-clear (ACK) parent cause */
+	ret = regmap_read(sgpio->regmap,
+			  sgpio->regmap_base_offset + sgpio->regs->status,
+			  &status);
+	if (ret)
+		return IRQ_NONE;
+
+	status >>= 16;
+
+	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
+
+	reg_ptr = has_shifted_layout ? 16 - DIV_ROUND_UP(sgpio->ngpios, 32) : 0;
+
+	for (i = 0; i < DIV_ROUND_UP(sgpio->ngpios, 32); i++, reg_ptr++) {
+		if (status & BIT(reg_ptr)) {
+			ret = regmap_read(sgpio->regmap,
+					  sgpio->regmap_base_offset +
+					  sgpio->regs->din_ss +
+					  reg_ptr * sizeof(u32),
+					  &new_value);
+			if (ret)
+				continue;
+
+			{
+				guard(spinlock_irqsave)(&sgpio->lock);
+				changed_value = sgpio->slices[3].reg_ss[i] ^ new_value;
+				sgpio->slices[3].reg_ss[i] = new_value;
+			}
+
+			while (changed_value) {
+				irq_hw_number_t hwirq;
+				unsigned int position;
+				unsigned int type;
+				bool rising;
+
+				bit = __ffs(changed_value);
+				changed_value &= ~BIT(bit);
+
+				position = i * 32 + bit;
+				hwirq = position * 2;
+
+				rising = !!(new_value & BIT(bit));
+				type = READ_ONCE(sgpio->irq_type[position]);
+
+				if (rising &&
+				    !(type & IRQ_TYPE_EDGE_RISING))
+					continue;
+
+				if (!rising &&
+				    !(type & IRQ_TYPE_EDGE_FALLING))
+					continue;
+
+				if (READ_ONCE(sgpio->irq_unmasked[position])) {
+					unsigned int child_irq;
+
+					child_irq = irq_find_mapping(sgpio->chip.irq.domain,
+								     hwirq);
+					if (child_irq)
+						handle_nested_irq(child_irq);
+				}
+			}
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void sgpio_hw_init(struct ax3000_sgpio *sgpio)
+{
+	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
+	u32 position;
+	u32 bank;
+	int i;
+
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mask, 0);
+
+	/* slice A0, Clock Pin - 0 */
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mux_0, 0x306);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->preset_0,
+		     sgpio->preset_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->count_0,
+		     sgpio->count_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->pos_0, 0x1f001f);
+
+	/* Slice B1, Data Load Pin - 1 */
+	bank = (sgpio->ngpios - 1) / 32;
+	position = (sgpio->ngpios - 1) % 32;
+
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mux_1,
+		     has_shifted_layout ? 0x30c : 0x304);
+
+	for (i = 0; i < bank; i++) {
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->ld +
+				     (i * 4),
+			     0xffffffff);
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->ld_ss +
+				     (i * 4),
+			     0xffffffff);
+	}
+
+	if (position) {
+		u32 val;
+
+		val = sgpio->slices[1].reg_ss[i];
+		val |= GENMASK(position, 0);
+
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->ld +
+				     (i * 4),
+			     val);
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->ld_ss +
+				     (i * 4),
+			     val);
+	}
+
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->preset_1,
+		     sgpio->preset_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->count_1,
+		     sgpio->count_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->pos_1,
+		     sgpio->pos_reg);
+
+	/* Slice C2, Data Out Pin - 2 */
+	bank = sgpio->ngpios / 32;
+	position = sgpio->ngpios % 32;
+
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mux_2,
+		     has_shifted_layout ? 0x30c : 0x304);
+
+	for (i = 0; i < bank; i++) {
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->dout +
+				     (i * 4),
+			     sgpio->slices[2].reg_ss[i]);
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->dout_ss +
+				     (i * 4),
+			     sgpio->slices[2].reg_ss[i]);
+	}
+
+	if (position) {
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->dout +
+				     (i * 4),
+			     sgpio->slices[2].reg_ss[i]);
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->dout_ss +
+				     (i * 4),
+			     sgpio->slices[2].reg_ss[i]);
+	}
+
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->preset_2,
+		     sgpio->preset_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->count_2,
+		     sgpio->count_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->pos_2,
+		     sgpio->pos_reg);
+
+	/* Slice D3, Data In Pin - 3 */
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mux_3, 0x14C);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->preset_3,
+		     sgpio->preset_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->count_3,
+		     sgpio->count_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->pos_3,
+		     sgpio->pos_reg);
+
+	/* Slice E4, Output Enable for respective pins */
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->mux_4,
+		     has_shifted_layout ? 0x10c : 0x104);
+	regmap_write(sgpio->regmap, sgpio->regmap_base_offset + sgpio->regs->oe,
+		     0xffffffff);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->oe_ss,
+		     0xffffffff);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->preset_4,
+		     sgpio->preset_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->count_4,
+		     sgpio->count_value);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->pos_4, 0x1f001f);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->ctrl_en, 0xffff);
+	regmap_write(sgpio->regmap,
+		     sgpio->regmap_base_offset + sgpio->regs->ctrl_en_pos,
+		     0xffff);
+}
+
+static int axiado_sgpio_irq_enable(struct ax3000_sgpio *sgpio)
+{
+	return regmap_write(sgpio->regmap,
+			    sgpio->regmap_base_offset + sgpio->regs->mask,
+			    0xdfff);
+}
+
+static int sgpio_set_irq_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	unsigned int position;
+
+	/* Only even GPIO offsets represent SGPIO inputs. */
+	if (hwirq & 1)
+		return -EINVAL;
+
+	position = hwirq / 2;
+	if (position >= sgpio->ngpios)
+		return -EINVAL;
+
+	type &= IRQ_TYPE_SENSE_MASK;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_BOTH:
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_EDGE_FALLING:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	WRITE_ONCE(sgpio->irq_type[position], type);
+	irq_set_handler_locked(d, handle_edge_irq);
+
+	return 0;
+}
+
+static void sgpio_mask_irq(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+
+	WRITE_ONCE(sgpio->irq_unmasked[hwirq / 2], 0);
+	gpiochip_disable_irq(chip, hwirq);
+}
+
+static void sgpio_unmask_irq(struct irq_data *d)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+
+	gpiochip_enable_irq(chip, hwirq);
+	WRITE_ONCE(sgpio->irq_unmasked[hwirq / 2], 1);
+}
+
+static void sgpio_irq_shutdown(struct irq_data *d)
+{
+	sgpio_mask_irq(d);
+}
+
+static void axiado_sgpio_irq_init_valid_mask(struct gpio_chip *chip,
+					 unsigned long *valid_mask,
+					 unsigned int ngpios)
+{
+	unsigned int offset;
+
+	/* Only even offsets represent input GPIOs. */
+	for (offset = 1; offset < ngpios; offset += 2)
+		clear_bit(offset, valid_mask);
+}
+
+static int axiado_sgpio_init_input_cache(struct ax3000_sgpio *sgpio)
+{
+	unsigned int num_banks = DIV_ROUND_UP(sgpio->ngpios, 32);
+	unsigned int reg_ptr;
+	unsigned int i;
+	int ret;
+
+	reg_ptr = sgpio->max_offset_regs == MAX_OFFSET_REG ?
+		  MAX_OFFSET_REG - num_banks : 0;
+
+	for (i = 0; i < num_banks; i++, reg_ptr++) {
+		ret = regmap_read(sgpio->regmap,
+				  sgpio->regmap_base_offset +
+				  sgpio->regs->din_ss +
+				  reg_ptr * sizeof(u32),
+				  &sgpio->slices[3].reg_ss[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct regmap_config regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int sgpio_probe(struct platform_device *pdev)
+{
+	const struct axiado_sgpio_soc_data *soc_data;
+	struct gpio_irq_chip *girq;
+	struct ax3000_sgpio *sgpio;
+	unsigned long apb_freq;
+	struct clk *apb_clk;
+	void __iomem *base;
+	u32 sgpio_freq;
+	int irq, rc;
+	u32 pos;
+
+	sgpio = devm_kzalloc(&pdev->dev, sizeof(*sgpio), GFP_KERNEL);
+	if (!sgpio)
+		return -ENOMEM;
+
+	spin_lock_init(&sgpio->lock);
+	mutex_init(&sgpio->output_lock);
+
+	sgpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+
+	if (sgpio->regmap) {
+		rc = device_property_read_u32(&pdev->dev, "reg",
+					      &sgpio->regmap_base_offset);
+		if (rc)
+			return dev_err_probe(&pdev->dev, rc,
+					     "Failed to read reg property\n");
+
+		dev_info(&pdev->dev, "Using regmap with base offset: 0x%x\n",
+			 sgpio->regmap_base_offset);
+	} else {
+		base = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(base))
+			return PTR_ERR(base);
+
+		sgpio->regmap =
+			devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
+
+		if (IS_ERR(sgpio->regmap))
+			return PTR_ERR(sgpio->regmap);
+
+		sgpio->regmap_base_offset = 0;
+
+		dev_info(&pdev->dev, "Using MMIO regmap\n");
+	}
+
+	rc = device_property_read_u32(&pdev->dev, "ngpios", &sgpio->ngpios);
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc,
+				     "Failed to read ngpios property\n");
+
+	soc_data = device_get_match_data(&pdev->dev);
+	if (!soc_data)
+		return -EINVAL;
+
+	sgpio->regs = soc_data->regs;
+	sgpio->max_sgpio_pins = soc_data->max_sgpio_pins;
+	sgpio->max_offset_regs = soc_data->max_offset_regs;
+
+	if (!sgpio->ngpios ||
+	    (sgpio->ngpios != 128 && sgpio->ngpios != 512) ||
+	    sgpio->ngpios > sgpio->max_sgpio_pins)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "Invalid ngpios value: %u\n",
+				     sgpio->ngpios);
+
+	apb_clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	if (IS_ERR(apb_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(apb_clk),
+				     "Failed to get and enable APB clock\n");
+
+	rc = device_property_read_u32(&pdev->dev, "bus-frequency",
+				      &sgpio_freq);
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc,
+				     "Failed to read bus-frequency\n");
+
+	apb_freq = clk_get_rate(apb_clk);
+
+	if (!apb_freq || !sgpio_freq || sgpio_freq > apb_freq)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "Invalid SGPIO bus frequency\n");
+
+	sgpio->preset_value = (apb_freq / sgpio_freq) - 1;
+	sgpio->count_value = sgpio->preset_value;
+
+	pos = sgpio->ngpios - 1;
+	sgpio->pos_reg = FIELD_PREP(GENMASK(31, 16), pos) |
+			 FIELD_PREP(GENMASK(15, 0), pos);
+
+	sgpio_hw_init(sgpio);
+
+	rc = axiado_sgpio_init_input_cache(sgpio);
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc,
+				     "Failed to initialize input cache\n");
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	rc = devm_request_threaded_irq(&pdev->dev, irq, NULL, sgpio_irq_handler,
+				       IRQF_ONESHOT, dev_name(&pdev->dev), sgpio);
+
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc, "Failed to request IRQ\n");
+
+	sgpio->chip.parent = &pdev->dev;
+	sgpio->chip.ngpio = sgpio->ngpios * 2;
+	sgpio->chip.get_direction = ax3000_sgpio_get_direction;
+	sgpio->chip.direction_input = ax3000_sgpio_dir_in;
+	sgpio->chip.direction_output = ax3000_sgpio_dir_out;
+	sgpio->chip.get = ax3000_sgpio_get;
+	sgpio->chip.set = ax3000_sgpio_set;
+	sgpio->chip.can_sleep = true;
+	sgpio->chip.label = dev_name(&pdev->dev);
+	sgpio->chip.base = -1;
+
+	girq = &sgpio->chip.irq;
+
+	gpio_irq_chip_set_chip(girq, &axiado_sgpio_irqchip);
+	girq->handler = handle_bad_irq;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->init_valid_mask = axiado_sgpio_irq_init_valid_mask;
+	girq->threaded = true;
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+
+	rc = devm_gpiochip_add_data(&pdev->dev, &sgpio->chip, sgpio);
+	if (rc < 0)
+		return dev_err_probe(&pdev->dev, rc,
+				     "Could not register gpiochip\n");
+
+	rc = axiado_sgpio_irq_enable(sgpio);
+	if (rc)
+		return dev_err_probe(&pdev->dev, rc,
+				     "Failed to enable interrupts\n");
+
+	platform_set_drvdata(pdev, sgpio);
+
+	return 0;
+}
+
+static void sgpio_remove(struct platform_device *pdev)
+{
+	struct ax3000_sgpio *sgpio = platform_get_drvdata(pdev);
+
+	/* Disable interrupts in hardware */
+	if (sgpio->regs) {
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->mask,
+			     0x0);
+		regmap_write(sgpio->regmap,
+			     sgpio->regmap_base_offset + sgpio->regs->ctrl_en,
+			     0x0);
+	}
+}
+
+static const struct of_device_id axiado_sgpio_of_match[] = {
+	{ .compatible = "axiado,ax3000-sgpio", .data = &ax3000_sgpio_data },
+	{ .compatible = "axiado,ax3005-sgpio", .data = &ax3005_sgpio_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axiado_sgpio_of_match);
+
+static struct platform_driver sgpio_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = axiado_sgpio_of_match,
+	},
+	.probe = sgpio_probe,
+	.remove = sgpio_remove,
+};
+
+module_platform_driver(sgpio_driver);
+MODULE_DESCRIPTION("Axiado Serial GPIO Driver");
+MODULE_AUTHOR("Axiado Corporation");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 2/2] gpio: axiado: add SGPIO controller support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] ax3000_sgpio_get() returns a stale cached value for input GPIOs, which breaks polling loops and risks livelocks.
- [High] Edge interrupts are permanently lost if they occur while the child interrupt is masked.
- [High] Missing required `irq_ack` callback for `handle_edge_irq` causes a NULL pointer dereference.
- [High] Hardware configuration for Clock (Slice A0) and Output Enable (Slice E4) is hardcoded to 32 pins, truncating SGPIO operation.
- [High] Function pointer type mismatch for `sgpio->chip.set` causes Control-Flow Integrity (KCFI) panics.
- [High] Devres registration order causes a Use-After-Free of the IRQ domain when the device is unbound.
- [Medium] The threaded IRQ handler `sgpio_irq_handler` unconditionally returns `IRQ_HANDLED`, preventing the detection of spurious interrupts.
--

--- Patch [2]: [PATCH v2 2/2] gpio: axiado: add SGPIO controller support ---
commit f070fa50e06960bd582fe83c1386133514675e7d
Author: Petar Stepanovic <pstepanovic@axiado.com>

gpio: axiado: add SGPIO controller support
    
Add support for the Axiado SGPIO controller.

Each SGPIO position provides one input GPIO and one output GPIO with
fixed directions. The driver registers the controller as a gpio_chip,
supports interrupts on input GPIOs, and uses regmap for register access.

> diff --git a/drivers/gpio/gpio-axiado-sgpio.c b/drivers/gpio/gpio-axiado-sgpio.c
> new file mode 100644
> index 0000000000000..91e63239c99d4
> --- /dev/null
> +++ b/drivers/gpio/gpio-axiado-sgpio.c

[ ... ]

> +static const struct irq_chip axiado_sgpio_irqchip = {
> +	.name = "axiado-sgpio",
> +	.irq_mask = sgpio_mask_irq,
> +	.irq_unmask = sgpio_unmask_irq,
> +	.irq_set_type = sgpio_set_irq_type,
> +	.irq_shutdown = sgpio_irq_shutdown,
> +	.flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};

[Severity: High]
Is a .irq_ack callback required here?

The flow handler for edge interrupts is set to handle_edge_irq() below in
sgpio_set_irq_type(). The kernel's handle_edge_irq() unconditionally calls
the .irq_ack callback on the irq_chip.

Will omitting .irq_ack from this structure cause a NULL pointer dereference
when the handler fires?

[ ... ]

> +static int ax3000_sgpio_set(struct gpio_chip *chip, unsigned int offset,
> +			     int value)

[Severity: High]
Is it safe to define this function as returning an int?

The set callback in struct gpio_chip is expected to return void.

Could assigning an int-returning function to the chip.set pointer trigger a
Control-Flow Integrity (KCFI) panic when the callback is indirectly invoked?

[ ... ]

> +static int ax3000_sgpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct ax3000_sgpio *sgpio = gpiochip_get_data(chip);
> +	u32 position = (offset / 2) % 32;
> +	u32 bank = (offset / 2) / 32;
> +	u32 val;
> +
> +	if (!(offset % 2)) {
> +		guard(spinlock_irqsave)(&sgpio->lock);
> +
> +		val = sgpio->slices[3].reg_ss[bank];

[Severity: High]
Does this return a stale cached value for input GPIOs?

The slices[3].reg_ss cache is only updated during the threaded IRQ handler
sgpio_irq_handler().

If a consumer driver loops polling this GPIO state while preventing the
IRQ thread from running, could this cause a permanent livelock since the
cache will never be refreshed?

[ ... ]

> +static irqreturn_t sgpio_irq_handler(int irq, void *arg)
> +{
> +	struct ax3000_sgpio *sgpio = (struct ax3000_sgpio *)arg;
> +	u32 status, new_value;
> +	u32 changed_value;
> +	int bit, reg_ptr;
> +	int ret, i;
> +
> +	/* Read-on-clear (ACK) parent cause */
> +	ret = regmap_read(sgpio->regmap,
> +			  sgpio->regmap_base_offset + sgpio->regs->status,
> +			  &status);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	status >>= 16;
> +
> +	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
> +
> +	reg_ptr = has_shifted_layout ? 16 - DIV_ROUND_UP(sgpio->ngpios, 32) : 0;
> +
> +	for (i = 0; i < DIV_ROUND_UP(sgpio->ngpios, 32); i++, reg_ptr++) {
> +		if (status & BIT(reg_ptr)) {
> +			ret = regmap_read(sgpio->regmap,
> +					  sgpio->regmap_base_offset +
> +					  sgpio->regs->din_ss +
> +					  reg_ptr * sizeof(u32),
> +					  &new_value);
> +			if (ret)
> +				continue;
> +
> +			{
> +				guard(spinlock_irqsave)(&sgpio->lock);
> +				changed_value = sgpio->slices[3].reg_ss[i] ^ new_value;
> +				sgpio->slices[3].reg_ss[i] = new_value;
> +			}

[Severity: High]
Can this permanently drop edge interrupts if they occur while the child
interrupt is masked?

When an edge event triggers the parent handler, the software cache is updated
here regardless of the child IRQ's mask state.

If the child IRQ is currently masked below, the event isn't pended or
otherwise saved. Once the IRQ is unmasked, wouldn't the edge be completely
lost since the cache has already absorbed the state change?

> +
> +			while (changed_value) {
> +				irq_hw_number_t hwirq;
> +				unsigned int position;
> +				unsigned int type;
> +				bool rising;
> +
> +				bit = __ffs(changed_value);
> +				changed_value &= ~BIT(bit);
> +
> +				position = i * 32 + bit;
> +				hwirq = position * 2;
> +
> +				rising = !!(new_value & BIT(bit));
> +				type = READ_ONCE(sgpio->irq_type[position]);
> +
> +				if (rising &&
> +				    !(type & IRQ_TYPE_EDGE_RISING))
> +					continue;
> +
> +				if (!rising &&
> +				    !(type & IRQ_TYPE_EDGE_FALLING))
> +					continue;
> +
> +				if (READ_ONCE(sgpio->irq_unmasked[position])) {
> +					unsigned int child_irq;
> +
> +					child_irq = irq_find_mapping(sgpio->chip.irq.domain,
> +								     hwirq);
> +					if (child_irq)
> +						handle_nested_irq(child_irq);
> +				}
> +			}
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}

[Severity: Medium]
Should this return IRQ_HANDLED unconditionally?

If no bits are set in the status register that match the loop conditions,
no events are processed but the handler still claims the interrupt.

Could this prevent the kernel's spurious interrupt detector from noticing
an interrupt storm and disabling the misbehaving IRQ line?

[ ... ]

> +static void sgpio_hw_init(struct ax3000_sgpio *sgpio)
> +{
> +	bool has_shifted_layout = (sgpio->max_offset_regs == MAX_OFFSET_REG);
> +	u32 position;
> +	u32 bank;
> +	int i;
> +
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mask, 0);
> +
> +	/* slice A0, Clock Pin - 0 */
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_0, 0x306);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_0,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_0,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_0, 0x1f001f);

[Severity: High]
Are the Clock (Slice A0) and Output Enable (Slice E4) hardware configurations
hardcoded to only support 32 pins?

Writing 0x1f001f configures the hardware position for 32 bits. Later on,
only a single 32-bit register is initialized for the Output Enable (regs->oe).

If this driver is bound to a 128-pin or 512-pin controller, won't this
leave all pins beyond the first 32 completely disabled and unclocked?

[ ... ]

> +	/* Slice E4, Output Enable for respective pins */
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->mux_4,
> +		     has_shifted_layout ? 0x10c : 0x104);
> +	regmap_write(sgpio->regmap, sgpio->regmap_base_offset + sgpio->regs->oe,
> +		     0xffffffff);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->oe_ss,
> +		     0xffffffff);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->preset_4,
> +		     sgpio->preset_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->count_4,
> +		     sgpio->count_value);
> +	regmap_write(sgpio->regmap,
> +		     sgpio->regmap_base_offset + sgpio->regs->pos_4, 0x1f001f);

[ ... ]

> +static int sgpio_probe(struct platform_device *pdev)
> +{
...
> +	rc = devm_request_threaded_irq(&pdev->dev, irq, NULL, sgpio_irq_handler,
> +				       IRQF_ONESHOT, dev_name(&pdev->dev), sgpio);
> +
> +	if (rc)
> +		return dev_err_probe(&pdev->dev, rc, "Failed to request IRQ\n");

[Severity: High]
Does registering the IRQ before adding the gpiochip cause a use-after-free
on device unbind?

Devres manages resource cleanup in reverse order (LIFO).

If the IRQ is requested here, and then the gpiochip is added below:

> +
> +	sgpio->chip.parent = &pdev->dev;
> +	sgpio->chip.ngpio = sgpio->ngpios * 2;
> +	sgpio->chip.get_direction = ax3000_sgpio_get_direction;
> +	sgpio->chip.direction_input = ax3000_sgpio_dir_in;
> +	sgpio->chip.direction_output = ax3000_sgpio_dir_out;
> +	sgpio->chip.get = ax3000_sgpio_get;
> +	sgpio->chip.set = ax3000_sgpio_set;
> +	sgpio->chip.can_sleep = true;
> +	sgpio->chip.label = dev_name(&pdev->dev);
> +	sgpio->chip.base = -1;
> +
> +	girq = &sgpio->chip.irq;
> +
> +	gpio_irq_chip_set_chip(girq, &axiado_sgpio_irqchip);
> +	girq->handler = handle_bad_irq;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->init_valid_mask = axiado_sgpio_irq_init_valid_mask;
> +	girq->threaded = true;
> +	girq->parent_handler = NULL;
> +	girq->num_parents = 0;
> +	girq->parents = NULL;
> +
> +	rc = devm_gpiochip_add_data(&pdev->dev, &sgpio->chip, sgpio);

won't the gpiochip and its IRQ domain be torn down during removal while the
interrupt handler is still registered?

If the interrupt fires in this window, the handler calls irq_find_mapping()
on a freed IRQ domain.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-axiado-ax3000-sgpio-controller-v2-0-976f4ed11438@axiado.com?part=2