[PATCH 0/2] riscv: spacemit: add gpio support for K3 SoC

[PATCH 0/2] riscv: spacemit: add gpio support for K3 SoC

[Yixun Lan]

Introduce gpio support for SpacemiT K3 SoC, the gpio controller
changes its register layout and bank offset, while mostly shares
other IP logic, so adjust the driver to support this.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
Yixun Lan (2):
      dt-bindings: gpio: spacemit: add compatible name for K3 SoC
      gpio: spacemit: Add GPIO support for K3 SoC

 .../devicetree/bindings/gpio/spacemit,k1-gpio.yaml |   4 +-
 drivers/gpio/gpio-spacemit-k1.c                    | 150 +++++++++++++++------
 2 files changed, 109 insertions(+), 45 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251229-02-k3-gpio-89b6e95cdf65

Best regards,
-- 
Yixun Lan


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] dt-bindings: gpio: spacemit: add compatible name for K3 SoC

[Yixun Lan]

Add new compatible string for SpacemiT K3 SoC's GPIO controller.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml b/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml
index 83e0b2d14c9f..24d22d95665f 100644
--- a/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml
@@ -19,7 +19,9 @@ properties:
     pattern: "^gpio@[0-9a-f]+$"
 
   compatible:
-    const: spacemit,k1-gpio
+    enum:
+      - spacemit,k1-gpio
+      - spacemit,k3-gpio
 
   reg:
     maxItems: 1

-- 
2.52.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] dt-bindings: gpio: spacemit: add compatible name for K3 SoC

[Krzysztof Kozlowski]

On Mon, Dec 29, 2025 at 08:46:38PM +0800, Yixun Lan wrote:
> Add new compatible string for SpacemiT K3 SoC's GPIO controller.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Yixun Lan]

SpacemiT K3 SoC has changed gpio register layout while comparing
with previous generation, the register offset and bank offset
need to be adjusted, introduce a compatible data to extend the
driver to support this.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
 1 file changed, 106 insertions(+), 44 deletions(-)

diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
index eb66a15c002f..02cc5c11b617 100644
--- a/drivers/gpio/gpio-spacemit-k1.c
+++ b/drivers/gpio/gpio-spacemit-k1.c
@@ -15,28 +15,19 @@
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
 
-/* register offset */
-#define SPACEMIT_GPLR		0x00 /* port level - R */
-#define SPACEMIT_GPDR		0x0c /* port direction - R/W */
-#define SPACEMIT_GPSR		0x18 /* port set - W */
-#define SPACEMIT_GPCR		0x24 /* port clear - W */
-#define SPACEMIT_GRER		0x30 /* port rising edge R/W */
-#define SPACEMIT_GFER		0x3c /* port falling edge R/W */
-#define SPACEMIT_GEDR		0x48 /* edge detect status - R/W1C */
-#define SPACEMIT_GSDR		0x54 /* (set) direction - W */
-#define SPACEMIT_GCDR		0x60 /* (clear) direction - W */
-#define SPACEMIT_GSRER		0x6c /* (set) rising edge detect enable - W */
-#define SPACEMIT_GCRER		0x78 /* (clear) rising edge detect enable - W */
-#define SPACEMIT_GSFER		0x84 /* (set) falling edge detect enable - W */
-#define SPACEMIT_GCFER		0x90 /* (clear) falling edge detect enable - W */
-#define SPACEMIT_GAPMASK	0x9c /* interrupt mask , 0 disable, 1 enable - R/W */
-
 #define SPACEMIT_NR_BANKS		4
 #define SPACEMIT_NR_GPIOS_PER_BANK	32
 
 #define to_spacemit_gpio_bank(x) container_of((x), struct spacemit_gpio_bank, gc)
+#define to_spacemit_gpio_regs(sg) ((sg)->data->reg_offsets)
 
 struct spacemit_gpio;
+struct spacemit_gpio_reg_offsets;
+
+struct spacemit_gpio_data {
+	struct spacemit_gpio_reg_offsets *reg_offsets;
+	u32 bank_offsets[4];
+};
 
 struct spacemit_gpio_bank {
 	struct gpio_generic_chip chip;
@@ -49,9 +40,28 @@ struct spacemit_gpio_bank {
 
 struct spacemit_gpio {
 	struct device *dev;
+	const struct spacemit_gpio_data *data;
 	struct spacemit_gpio_bank sgb[SPACEMIT_NR_BANKS];
 };
 
+struct spacemit_gpio_reg_offsets {
+	u32 gplr;      /* port level - R */
+	u32 gpdr;      /* port direction - R/W */
+	u32 gpsr;      /* port set - W */
+	u32 gpcr;      /* port clear - W */
+	u32 grer;      /* port rising edge R/W */
+	u32 gfer;      /* port falling edge R/W */
+	u32 gedr;      /* edge detect status - R/W1C */
+	u32 gsdr;      /* (set) direction - W */
+	u32 gcdr;      /* (clear) direction - W */
+	u32 gsrer;     /* (set) rising edge detect enable - W */
+	u32 gcrer;     /* (clear) rising edge detect enable - W */
+	u32 gsfer;     /* (set) falling edge detect enable - W */
+	u32 gcfer;     /* (clear) falling edge detect enable - W */
+	u32 gapmask;   /* interrupt mask , 0 disable, 1 enable - R/W */
+	u32 gcpmask;   /* interrupt mask for K3 */
+};
+
 static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
 {
 	return (u32)(gb - gb->sg->sgb);
@@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
 static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
 {
 	struct spacemit_gpio_bank *gb = dev_id;
+	struct spacemit_gpio *sg = gb->sg;
 	unsigned long pending;
 	u32 n, gedr;
 
-	gedr = readl(gb->base + SPACEMIT_GEDR);
+	gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
 	if (!gedr)
 		return IRQ_NONE;
-	writel(gedr, gb->base + SPACEMIT_GEDR);
+	writel(gedr, gb->base + to_spacemit_gpio_regs(sg)->gedr);
 
 	pending = gedr & gb->irq_mask;
 	if (!pending)
@@ -81,60 +92,64 @@ static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
 static void spacemit_gpio_irq_ack(struct irq_data *d)
 {
 	struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
+	struct spacemit_gpio *sg = gb->sg;
 
-	writel(BIT(irqd_to_hwirq(d)), gb->base + SPACEMIT_GEDR);
+	writel(BIT(irqd_to_hwirq(d)), gb->base + to_spacemit_gpio_regs(sg)->gedr);
 }
 
 static void spacemit_gpio_irq_mask(struct irq_data *d)
 {
 	struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
+	struct spacemit_gpio *sg = gb->sg;
 	u32 bit = BIT(irqd_to_hwirq(d));
 
 	gb->irq_mask &= ~bit;
-	writel(gb->irq_mask, gb->base + SPACEMIT_GAPMASK);
+	writel(gb->irq_mask, gb->base + to_spacemit_gpio_regs(sg)->gapmask);
 
 	if (bit & gb->irq_rising_edge)
-		writel(bit, gb->base + SPACEMIT_GCRER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gcrer);
 
 	if (bit & gb->irq_falling_edge)
-		writel(bit, gb->base + SPACEMIT_GCFER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gcfer);
 }
 
 static void spacemit_gpio_irq_unmask(struct irq_data *d)
 {
 	struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
+	struct spacemit_gpio *sg = gb->sg;
 	u32 bit = BIT(irqd_to_hwirq(d));
 
 	gb->irq_mask |= bit;
 
 	if (bit & gb->irq_rising_edge)
-		writel(bit, gb->base + SPACEMIT_GSRER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gsrer);
 
 	if (bit & gb->irq_falling_edge)
-		writel(bit, gb->base + SPACEMIT_GSFER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gsfer);
 
-	writel(gb->irq_mask, gb->base + SPACEMIT_GAPMASK);
+	writel(gb->irq_mask, gb->base + to_spacemit_gpio_regs(sg)->gapmask);
 }
 
 static int spacemit_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	struct spacemit_gpio_bank *gb = irq_data_get_irq_chip_data(d);
+	struct spacemit_gpio *sg = gb->sg;
 	u32 bit = BIT(irqd_to_hwirq(d));
 
 	if (type & IRQ_TYPE_EDGE_RISING) {
 		gb->irq_rising_edge |= bit;
-		writel(bit, gb->base + SPACEMIT_GSRER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gsrer);
 	} else {
 		gb->irq_rising_edge &= ~bit;
-		writel(bit, gb->base + SPACEMIT_GCRER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gcrer);
 	}
 
 	if (type & IRQ_TYPE_EDGE_FALLING) {
 		gb->irq_falling_edge |= bit;
-		writel(bit, gb->base + SPACEMIT_GSFER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gsfer);
 	} else {
 		gb->irq_falling_edge &= ~bit;
-		writel(bit, gb->base + SPACEMIT_GCFER);
+		writel(bit, gb->base + to_spacemit_gpio_regs(sg)->gcfer);
 	}
 
 	return 0;
@@ -179,15 +194,15 @@ static int spacemit_gpio_add_bank(struct spacemit_gpio *sg,
 	struct device *dev = sg->dev;
 	struct gpio_irq_chip *girq;
 	void __iomem *dat, *set, *clr, *dirin, *dirout;
-	int ret, bank_base[] = { 0x0, 0x4, 0x8, 0x100 };
+	int ret;
 
-	gb->base = regs + bank_base[index];
+	gb->base = regs + sg->data->bank_offsets[index];
 
-	dat	= gb->base + SPACEMIT_GPLR;
-	set	= gb->base + SPACEMIT_GPSR;
-	clr	= gb->base + SPACEMIT_GPCR;
-	dirin	= gb->base + SPACEMIT_GCDR;
-	dirout	= gb->base + SPACEMIT_GSDR;
+	dat	= gb->base + to_spacemit_gpio_regs(sg)->gplr;
+	set	= gb->base + to_spacemit_gpio_regs(sg)->gpsr;
+	clr	= gb->base + to_spacemit_gpio_regs(sg)->gpcr;
+	dirin	= gb->base + to_spacemit_gpio_regs(sg)->gcdr;
+	dirout	= gb->base + to_spacemit_gpio_regs(sg)->gsdr;
 
 	config = (struct gpio_generic_chip_config) {
 		.dev = dev,
@@ -223,13 +238,13 @@ static int spacemit_gpio_add_bank(struct spacemit_gpio *sg,
 	gpio_irq_chip_set_chip(girq, &spacemit_gpio_chip);
 
 	/* Disable Interrupt */
-	writel(0, gb->base + SPACEMIT_GAPMASK);
+	writel(0, gb->base + to_spacemit_gpio_regs(sg)->gapmask);
 	/* Disable Edge Detection Settings */
-	writel(0x0, gb->base + SPACEMIT_GRER);
-	writel(0x0, gb->base + SPACEMIT_GFER);
+	writel(0x0, gb->base + to_spacemit_gpio_regs(sg)->grer);
+	writel(0x0, gb->base + to_spacemit_gpio_regs(sg)->gfer);
 	/* Clear Interrupt */
-	writel(0xffffffff, gb->base + SPACEMIT_GCRER);
-	writel(0xffffffff, gb->base + SPACEMIT_GCFER);
+	writel(0xffffffff, gb->base + to_spacemit_gpio_regs(sg)->gcrer);
+	writel(0xffffffff, gb->base + to_spacemit_gpio_regs(sg)->gcfer);
 
 	ret = devm_request_threaded_irq(dev, irq, NULL,
 					spacemit_gpio_irq_handler,
@@ -260,6 +275,10 @@ static int spacemit_gpio_probe(struct platform_device *pdev)
 	if (!sg)
 		return -ENOMEM;
 
+	sg->data = of_device_get_match_data(dev);
+	if (!sg->data)
+		return dev_err_probe(dev, -EINVAL, "No available compatible data.");
+
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
@@ -287,8 +306,51 @@ static int spacemit_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct spacemit_gpio_data k1_gpio_data = {
+	.reg_offsets = &(struct spacemit_gpio_reg_offsets) {
+		.gplr    = 0x00,
+		.gpdr    = 0x0c,
+		.gpsr    = 0x18,
+		.gpcr    = 0x24,
+		.grer    = 0x30,
+		.gfer    = 0x3c,
+		.gedr    = 0x48,
+		.gsdr    = 0x54,
+		.gcdr    = 0x60,
+		.gsrer   = 0x6c,
+		.gcrer   = 0x78,
+		.gsfer   = 0x84,
+		.gcfer   = 0x90,
+		.gapmask = 0x9c,
+		.gcpmask = 0xA8,
+	},
+	.bank_offsets = { 0x0, 0x4, 0x8, 0x100 },
+};
+
+static const struct spacemit_gpio_data k3_gpio_data = {
+	.reg_offsets = &(struct spacemit_gpio_reg_offsets) {
+		.gplr    = 0x0,
+		.gpdr    = 0x4,
+		.gpsr    = 0x8,
+		.gpcr    = 0xc,
+		.grer    = 0x10,
+		.gfer    = 0x14,
+		.gedr    = 0x18,
+		.gsdr    = 0x1c,
+		.gcdr    = 0x20,
+		.gsrer   = 0x24,
+		.gcrer   = 0x28,
+		.gsfer   = 0x2c,
+		.gcfer   = 0x30,
+		.gapmask = 0x34,
+		.gcpmask = 0x38,
+	},
+	.bank_offsets = { 0x0, 0x40, 0x80, 0x100 },
+};
+
 static const struct of_device_id spacemit_gpio_dt_ids[] = {
-	{ .compatible = "spacemit,k1-gpio" },
+	{ .compatible = "spacemit,k1-gpio", .data = &k1_gpio_data },
+	{ .compatible = "spacemit,k3-gpio", .data = &k3_gpio_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, spacemit_gpio_dt_ids);
@@ -296,12 +358,12 @@ MODULE_DEVICE_TABLE(of, spacemit_gpio_dt_ids);
 static struct platform_driver spacemit_gpio_driver = {
 	.probe		= spacemit_gpio_probe,
 	.driver		= {
-		.name	= "k1-gpio",
+		.name	= "spacemit-gpio",
 		.of_match_table = spacemit_gpio_dt_ids,
 	},
 };
 module_platform_driver(spacemit_gpio_driver);
 
 MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
-MODULE_DESCRIPTION("GPIO driver for SpacemiT K1 SoC");
+MODULE_DESCRIPTION("GPIO driver for SpacemiT K1/K3 SoC");
 MODULE_LICENSE("GPL");

-- 
2.52.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Bartosz Golaszewski]

On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
>
> SpacemiT K3 SoC has changed gpio register layout while comparing
> with previous generation, the register offset and bank offset
> need to be adjusted, introduce a compatible data to extend the
> driver to support this.
>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
>  1 file changed, 106 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> index eb66a15c002f..02cc5c11b617 100644
> --- a/drivers/gpio/gpio-spacemit-k1.c
> +++ b/drivers/gpio/gpio-spacemit-k1.c
> @@ -15,28 +15,19 @@
>  #include <linux/platform_device.h>
>  #include <linux/seq_file.h>
>
> -/* register offset */
> -#define SPACEMIT_GPLR          0x00 /* port level - R */
> -#define SPACEMIT_GPDR          0x0c /* port direction - R/W */
> -#define SPACEMIT_GPSR          0x18 /* port set - W */
> -#define SPACEMIT_GPCR          0x24 /* port clear - W */
> -#define SPACEMIT_GRER          0x30 /* port rising edge R/W */
> -#define SPACEMIT_GFER          0x3c /* port falling edge R/W */
> -#define SPACEMIT_GEDR          0x48 /* edge detect status - R/W1C */
> -#define SPACEMIT_GSDR          0x54 /* (set) direction - W */
> -#define SPACEMIT_GCDR          0x60 /* (clear) direction - W */
> -#define SPACEMIT_GSRER         0x6c /* (set) rising edge detect enable - W */
> -#define SPACEMIT_GCRER         0x78 /* (clear) rising edge detect enable - W */
> -#define SPACEMIT_GSFER         0x84 /* (set) falling edge detect enable - W */
> -#define SPACEMIT_GCFER         0x90 /* (clear) falling edge detect enable - W */
> -#define SPACEMIT_GAPMASK       0x9c /* interrupt mask , 0 disable, 1 enable - R/W */
> -
>  #define SPACEMIT_NR_BANKS              4
>  #define SPACEMIT_NR_GPIOS_PER_BANK     32
>
>  #define to_spacemit_gpio_bank(x) container_of((x), struct spacemit_gpio_bank, gc)
> +#define to_spacemit_gpio_regs(sg) ((sg)->data->reg_offsets)
>
>  struct spacemit_gpio;
> +struct spacemit_gpio_reg_offsets;

Why not move this structure here instead and avoid the forward declaration?

> +
> +struct spacemit_gpio_data {
> +       struct spacemit_gpio_reg_offsets *reg_offsets;
> +       u32 bank_offsets[4];
> +};
>
>  struct spacemit_gpio_bank {
>         struct gpio_generic_chip chip;
> @@ -49,9 +40,28 @@ struct spacemit_gpio_bank {
>
>  struct spacemit_gpio {
>         struct device *dev;
> +       const struct spacemit_gpio_data *data;
>         struct spacemit_gpio_bank sgb[SPACEMIT_NR_BANKS];
>  };
>
> +struct spacemit_gpio_reg_offsets {
> +       u32 gplr;      /* port level - R */
> +       u32 gpdr;      /* port direction - R/W */
> +       u32 gpsr;      /* port set - W */
> +       u32 gpcr;      /* port clear - W */
> +       u32 grer;      /* port rising edge R/W */
> +       u32 gfer;      /* port falling edge R/W */
> +       u32 gedr;      /* edge detect status - R/W1C */
> +       u32 gsdr;      /* (set) direction - W */
> +       u32 gcdr;      /* (clear) direction - W */
> +       u32 gsrer;     /* (set) rising edge detect enable - W */
> +       u32 gcrer;     /* (clear) rising edge detect enable - W */
> +       u32 gsfer;     /* (set) falling edge detect enable - W */
> +       u32 gcfer;     /* (clear) falling edge detect enable - W */
> +       u32 gapmask;   /* interrupt mask , 0 disable, 1 enable - R/W */
> +       u32 gcpmask;   /* interrupt mask for K3 */
> +};
> +
>  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>  {
>         return (u32)(gb - gb->sg->sgb);
> @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
>  {
>         struct spacemit_gpio_bank *gb = dev_id;
> +       struct spacemit_gpio *sg = gb->sg;
>         unsigned long pending;
>         u32 n, gedr;
>
> -       gedr = readl(gb->base + SPACEMIT_GEDR);
> +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);

Since you're already touching all these register accesses - can you
maybe provide dedicated wrapper functions around readl()/writel() and
avoid any file-wide changes in the future if anything requires further
modification?

Bart

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Yixun Lan]

Hi Bart,

On 12:10 Fri 02 Jan     , Bartosz Golaszewski wrote:
> On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
> >
> > SpacemiT K3 SoC has changed gpio register layout while comparing
> > with previous generation, the register offset and bank offset
> > need to be adjusted, introduce a compatible data to extend the
> > driver to support this.
> >
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > ---
> >  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
> >  1 file changed, 106 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> > index eb66a15c002f..02cc5c11b617 100644
> > --- a/drivers/gpio/gpio-spacemit-k1.c
> > +++ b/drivers/gpio/gpio-spacemit-k1.c
> > @@ -15,28 +15,19 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/seq_file.h>
> >
> > -/* register offset */
> > -#define SPACEMIT_GPLR          0x00 /* port level - R */
> > -#define SPACEMIT_GPDR          0x0c /* port direction - R/W */
> > -#define SPACEMIT_GPSR          0x18 /* port set - W */
> > -#define SPACEMIT_GPCR          0x24 /* port clear - W */
> > -#define SPACEMIT_GRER          0x30 /* port rising edge R/W */
> > -#define SPACEMIT_GFER          0x3c /* port falling edge R/W */
> > -#define SPACEMIT_GEDR          0x48 /* edge detect status - R/W1C */
> > -#define SPACEMIT_GSDR          0x54 /* (set) direction - W */
> > -#define SPACEMIT_GCDR          0x60 /* (clear) direction - W */
> > -#define SPACEMIT_GSRER         0x6c /* (set) rising edge detect enable - W */
> > -#define SPACEMIT_GCRER         0x78 /* (clear) rising edge detect enable - W */
> > -#define SPACEMIT_GSFER         0x84 /* (set) falling edge detect enable - W */
> > -#define SPACEMIT_GCFER         0x90 /* (clear) falling edge detect enable - W */
> > -#define SPACEMIT_GAPMASK       0x9c /* interrupt mask , 0 disable, 1 enable - R/W */
> > -
> >  #define SPACEMIT_NR_BANKS              4
> >  #define SPACEMIT_NR_GPIOS_PER_BANK     32
> >
> >  #define to_spacemit_gpio_bank(x) container_of((x), struct spacemit_gpio_bank, gc)
> > +#define to_spacemit_gpio_regs(sg) ((sg)->data->reg_offsets)
> >
> >  struct spacemit_gpio;
> > +struct spacemit_gpio_reg_offsets;
> 
> Why not move this structure here instead and avoid the forward declaration?
> 
sure, I will do

> > +
> > +struct spacemit_gpio_data {
> > +       struct spacemit_gpio_reg_offsets *reg_offsets;
> > +       u32 bank_offsets[4];
> > +};
> >
> >  struct spacemit_gpio_bank {
> >         struct gpio_generic_chip chip;
> > @@ -49,9 +40,28 @@ struct spacemit_gpio_bank {
> >
> >  struct spacemit_gpio {
> >         struct device *dev;
> > +       const struct spacemit_gpio_data *data;
> >         struct spacemit_gpio_bank sgb[SPACEMIT_NR_BANKS];
> >  };
> >
> > +struct spacemit_gpio_reg_offsets {
> > +       u32 gplr;      /* port level - R */
> > +       u32 gpdr;      /* port direction - R/W */
> > +       u32 gpsr;      /* port set - W */
> > +       u32 gpcr;      /* port clear - W */
> > +       u32 grer;      /* port rising edge R/W */
> > +       u32 gfer;      /* port falling edge R/W */
> > +       u32 gedr;      /* edge detect status - R/W1C */
> > +       u32 gsdr;      /* (set) direction - W */
> > +       u32 gcdr;      /* (clear) direction - W */
> > +       u32 gsrer;     /* (set) rising edge detect enable - W */
> > +       u32 gcrer;     /* (clear) rising edge detect enable - W */
> > +       u32 gsfer;     /* (set) falling edge detect enable - W */
> > +       u32 gcfer;     /* (clear) falling edge detect enable - W */
> > +       u32 gapmask;   /* interrupt mask , 0 disable, 1 enable - R/W */
> > +       u32 gcpmask;   /* interrupt mask for K3 */
> > +};
> > +
> >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >  {
> >         return (u32)(gb - gb->sg->sgb);
> > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
> >  {
> >         struct spacemit_gpio_bank *gb = dev_id;
> > +       struct spacemit_gpio *sg = gb->sg;
> >         unsigned long pending;
> >         u32 n, gedr;
> >
> > -       gedr = readl(gb->base + SPACEMIT_GEDR);
> > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
> 
> Since you're already touching all these register accesses - can you
> maybe provide dedicated wrapper functions around readl()/writel() and
> avoid any file-wide changes in the future if anything requires further
> modification?
> 
can you elaborate a bit further on this?
I don't get how a wrapper helper could help to avoid file-wide changes..

-- 
Yixun Lan (dlan)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Bartosz Golaszewski]

On Fri, 2 Jan 2026 12:36:43 +0100, Yixun Lan <dlan@gentoo.org> said:
>
>> >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>> >  {
>> >         return (u32)(gb - gb->sg->sgb);
>> > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>> >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
>> >  {
>> >         struct spacemit_gpio_bank *gb = dev_id;
>> > +       struct spacemit_gpio *sg = gb->sg;
>> >         unsigned long pending;
>> >         u32 n, gedr;
>> >
>> > -       gedr = readl(gb->base + SPACEMIT_GEDR);
>> > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
>>
>> Since you're already touching all these register accesses - can you
>> maybe provide dedicated wrapper functions around readl()/writel() and
>> avoid any file-wide changes in the future if anything requires further
>> modification?
>>
> can you elaborate a bit further on this?
> I don't get how a wrapper helper could help to avoid file-wide changes..
>

Just create functions called "spacemit_gpio_read/write()" that wrap the
readl()/writel() and its arguments.

Like:

static u32 spacemit_gpio_read(struct spacemit_gpio_bank *gb, unsigned long reg)
{
	return readl(gb->base + func_to_convert_reg_enum_to_offset(reg));
}

Looks cleaner at call sites even if it's a bit more complex. Just create an
array mapping the register enum to offset and assign it to platform data.

Bart

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Yixun Lan]

Hi bart,

On 19:36 Fri 02 Jan     , Yixun Lan wrote:
> Hi Bart,
> 
> On 12:10 Fri 02 Jan     , Bartosz Golaszewski wrote:
> > On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
> > >
> > > SpacemiT K3 SoC has changed gpio register layout while comparing
> > > with previous generation, the register offset and bank offset
> > > need to be adjusted, introduce a compatible data to extend the
> > > driver to support this.
> > >
> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > > ---
> > >  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
> > >  1 file changed, 106 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> > > index eb66a15c002f..02cc5c11b617 100644
> > > --- a/drivers/gpio/gpio-spacemit-k1.c
> > > +++ b/drivers/gpio/gpio-spacemit-k1.c
> > > @@ -15,28 +15,19 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/seq_file.h>
> > >
[snip]...
> > >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> > >  {
> > >         return (u32)(gb - gb->sg->sgb);
> > > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> > >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
> > >  {
> > >         struct spacemit_gpio_bank *gb = dev_id;
> > > +       struct spacemit_gpio *sg = gb->sg;
> > >         unsigned long pending;
> > >         u32 n, gedr;
> > >
> > > -       gedr = readl(gb->base + SPACEMIT_GEDR);
> > > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
> > 
> > Since you're already touching all these register accesses - can you
> > maybe provide dedicated wrapper functions around readl()/writel() and
> > avoid any file-wide changes in the future if anything requires further
> > modification?
> > 
> can you elaborate a bit further on this?
> I don't get how a wrapper helper could help to avoid file-wide changes..
> 
here is my attempt to solve this, define a macro to register address:

#define to_spacemit_gpio_regs(gb) ((gb)->sg->data->reg_offsets)

#define SPACEMIT_GEDR(gb)      ((gb)->base + to_spacemit_gpio_regs(gb)->gedr)

	gedr = readl(SPACEMIT_GEDR(gb));

please let me know if this follow your suggestion or not

-- 
Yixun Lan (dlan)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Bartosz Golaszewski]

On Fri, 2 Jan 2026 13:20:45 +0100, Yixun Lan <dlan@gentoo.org> said:
> Hi bart,
>
> On 19:36 Fri 02 Jan     , Yixun Lan wrote:
>> Hi Bart,
>>
>> On 12:10 Fri 02 Jan     , Bartosz Golaszewski wrote:
>> > On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
>> > >
>> > > SpacemiT K3 SoC has changed gpio register layout while comparing
>> > > with previous generation, the register offset and bank offset
>> > > need to be adjusted, introduce a compatible data to extend the
>> > > driver to support this.
>> > >
>> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
>> > > ---
>> > >  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
>> > >  1 file changed, 106 insertions(+), 44 deletions(-)
>> > >
>> > > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
>> > > index eb66a15c002f..02cc5c11b617 100644
>> > > --- a/drivers/gpio/gpio-spacemit-k1.c
>> > > +++ b/drivers/gpio/gpio-spacemit-k1.c
>> > > @@ -15,28 +15,19 @@
>> > >  #include <linux/platform_device.h>
>> > >  #include <linux/seq_file.h>
>> > >
> [snip]...
>> > >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>> > >  {
>> > >         return (u32)(gb - gb->sg->sgb);
>> > > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
>> > >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
>> > >  {
>> > >         struct spacemit_gpio_bank *gb = dev_id;
>> > > +       struct spacemit_gpio *sg = gb->sg;
>> > >         unsigned long pending;
>> > >         u32 n, gedr;
>> > >
>> > > -       gedr = readl(gb->base + SPACEMIT_GEDR);
>> > > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
>> >
>> > Since you're already touching all these register accesses - can you
>> > maybe provide dedicated wrapper functions around readl()/writel() and
>> > avoid any file-wide changes in the future if anything requires further
>> > modification?
>> >
>> can you elaborate a bit further on this?
>> I don't get how a wrapper helper could help to avoid file-wide changes..
>>
> here is my attempt to solve this, define a macro to register address:
>
> #define to_spacemit_gpio_regs(gb) ((gb)->sg->data->reg_offsets)
>
> #define SPACEMIT_GEDR(gb)      ((gb)->base + to_spacemit_gpio_regs(gb)->gedr)
>
> 	gedr = readl(SPACEMIT_GEDR(gb));
>
> please let me know if this follow your suggestion or not
>
> --
> Yixun Lan (dlan)
>

I was thinking more of something like this:

enum spacemit_gpio_registers {
	SPACEMIT_GPLR,
	SPACEMIT_GPDR,
	...
};

static const unsigned int spacemit_gpio_k1_offsets = {
	[SPACEMIT_GPLR] = 0x00,
	[SPACEMIT_GPDR] = 0x0c,
	...
};

static const unsigned int spacemit_gpio_k3_offsets = ...

struct spacemit_gpio_data {
	const unsigned int *offsets;
	u32 bank_offsets[4];
};

static void spacemit_gpio_write(struct spacemit_gpio_bank *gb,
				enum spacemit_gpio_registers reg, u32 val)
{
	writel(val, gb->base + gb->data->offsets[reg]);
}

Bart

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] gpio: spacemit: Add GPIO support for K3 SoC

[Yixun Lan]

Hi Bartosz,

On 08:38 Fri 02 Jan     , Bartosz Golaszewski wrote:
> On Fri, 2 Jan 2026 13:20:45 +0100, Yixun Lan <dlan@gentoo.org> said:
> > Hi bart,
> >
> > On 19:36 Fri 02 Jan     , Yixun Lan wrote:
> >> Hi Bart,
> >>
> >> On 12:10 Fri 02 Jan     , Bartosz Golaszewski wrote:
> >> > On Mon, Dec 29, 2025 at 1:47 PM Yixun Lan <dlan@gentoo.org> wrote:
> >> > >
> >> > > SpacemiT K3 SoC has changed gpio register layout while comparing
> >> > > with previous generation, the register offset and bank offset
> >> > > need to be adjusted, introduce a compatible data to extend the
> >> > > driver to support this.
> >> > >
> >> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> >> > > ---
> >> > >  drivers/gpio/gpio-spacemit-k1.c | 150 ++++++++++++++++++++++++++++------------
> >> > >  1 file changed, 106 insertions(+), 44 deletions(-)
> >> > >
> >> > > diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
> >> > > index eb66a15c002f..02cc5c11b617 100644
> >> > > --- a/drivers/gpio/gpio-spacemit-k1.c
> >> > > +++ b/drivers/gpio/gpio-spacemit-k1.c
> >> > > @@ -15,28 +15,19 @@
> >> > >  #include <linux/platform_device.h>
> >> > >  #include <linux/seq_file.h>
> >> > >
> > [snip]...
> >> > >  static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >> > >  {
> >> > >         return (u32)(gb - gb->sg->sgb);
> >> > > @@ -60,13 +70,14 @@ static u32 spacemit_gpio_bank_index(struct spacemit_gpio_bank *gb)
> >> > >  static irqreturn_t spacemit_gpio_irq_handler(int irq, void *dev_id)
> >> > >  {
> >> > >         struct spacemit_gpio_bank *gb = dev_id;
> >> > > +       struct spacemit_gpio *sg = gb->sg;
> >> > >         unsigned long pending;
> >> > >         u32 n, gedr;
> >> > >
> >> > > -       gedr = readl(gb->base + SPACEMIT_GEDR);
> >> > > +       gedr = readl(gb->base + to_spacemit_gpio_regs(sg)->gedr);
> >> >
> >> > Since you're already touching all these register accesses - can you
> >> > maybe provide dedicated wrapper functions around readl()/writel() and
> >> > avoid any file-wide changes in the future if anything requires further
> >> > modification?
> >> >
> >> can you elaborate a bit further on this?
> >> I don't get how a wrapper helper could help to avoid file-wide changes..
> >>
> > here is my attempt to solve this, define a macro to register address:
> >
> > #define to_spacemit_gpio_regs(gb) ((gb)->sg->data->reg_offsets)
> >
> > #define SPACEMIT_GEDR(gb)      ((gb)->base + to_spacemit_gpio_regs(gb)->gedr)
> >
> > 	gedr = readl(SPACEMIT_GEDR(gb));
> >
> > please let me know if this follow your suggestion or not
> >
> > --
> > Yixun Lan (dlan)
> >
> 
> I was thinking more of something like this:
> 
> enum spacemit_gpio_registers {
> 	SPACEMIT_GPLR,
> 	SPACEMIT_GPDR,
> 	...
> };
> 
> static const unsigned int spacemit_gpio_k1_offsets = {
> 	[SPACEMIT_GPLR] = 0x00,
> 	[SPACEMIT_GPDR] = 0x0c,
> 	...
> };
> 
> static const unsigned int spacemit_gpio_k3_offsets = ...
> 
> struct spacemit_gpio_data {
> 	const unsigned int *offsets;
> 	u32 bank_offsets[4];
> };
> 
> static void spacemit_gpio_write(struct spacemit_gpio_bank *gb,
> 				enum spacemit_gpio_registers reg, u32 val)
> {
> 	writel(val, gb->base + gb->data->offsets[reg]);
> }
> 
Ok, I will implement this, thanks for the detail prototype

-- 
Yixun Lan (dlan)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv