[PATCH v3 0/3] riscv: spacemit: add gpio support for K1 SoC

[PATCH v3 0/3] riscv: spacemit: add gpio support for K1 SoC

[Yixun Lan]

The gpio controller of K1 support basic GPIO functions,
which capable of enabling as input, output. It can also be used
as GPIO interrupt which able to detect rising edge, falling edge,
or both. There are four GPIO banks, each consisting of 32 pins.

The GPIO driver request the clock source from APBC block,
In this series, I haven't added the clock support, but plan
to fix it after clock driver is implemented/merged.

The GPIO docs of K1 SoC can be found here, chapter 16.4 GPIO [1]

Note, this patch need two prerequisite series, basic dt[2] and pinctrl[3]

This patch series has been tested on Bananapi-F3 board,
with following GPIO cases passed:
 1) gpio input
 2) gpio output - set to high, low
 3) gpio interrupt - rising trigger, falling trigger, both edge trigger

Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf [1]
Link: https://lore.kernel.org/all/20240730-k1-01-basic-dt-v5-0-98263aae83be@gentoo.org [2]
Link: https://lore.kernel.org/all/20241016-02-k1-pinctrl-v5-0-03d395222e4f@gentoo.org/ [3]
Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
Changes in v3:
- dt: drop ranges, interrupt-names property
- Link to v2: https://lore.kernel.org/r/20241219-03-k1-gpio-v2-0-28444fd221cd@gentoo.org

Changes in v2:
- address dt-bindings comments, simplify example
- rebase to 6.13-rc3 
- Link to v1: https://lore.kernel.org/r/20240904-03-k1-gpio-v1-0-6072ebeecae0@gentoo.org

---
Yixun Lan (3):
      dt-bindings: gpio: spacemit: add support for K1 SoC
      gpio: spacemit: add support for K1 SoC
      riscv: dts: spacemit: add gpio support for K1 SoC

 .../devicetree/bindings/gpio/spacemit,k1-gpio.yaml |  69 ++++
 arch/riscv/boot/dts/spacemit/k1.dtsi               |  12 +
 drivers/gpio/Kconfig                               |   7 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-spacemit-k1.c                    | 454 +++++++++++++++++++++
 5 files changed, 543 insertions(+)
---
base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
change-id: 20240828-03-k1-gpio-61bf92f9032c
prerequisite-change-id: 20240626-k1-01-basic-dt-1aa31eeebcd2:v5
prerequisite-patch-id: 47dcf6861f7d434d25855b379e6d7ef4ce369c9c
prerequisite-patch-id: 77787fe82911923aff15ccf565e8fa451538c3a6
prerequisite-patch-id: b0bdb1742d96c5738f05262c3b0059102761390b
prerequisite-patch-id: 3927d39d8d77e35d5bfe53d9950da574ff8f2054
prerequisite-patch-id: a98039136a4796252a6029e474f03906f2541643
prerequisite-patch-id: c95f6dc0547a2a63a76e3cba0cf5c623b212b4e6
prerequisite-patch-id: 66e750e438ee959ddc2a6f0650814a2d8c989139
prerequisite-patch-id: 29a0fd8c36c1a4340f0d0b68a4c34d2b8abfb1ab
prerequisite-patch-id: 0bdfff661c33c380d1cf00a6c68688e05f88c0b3
prerequisite-patch-id: 99f15718e0bfbb7ed1a96dfa19f35841b004dae9
prerequisite-change-id: 20240708-02-k1-pinctrl-3a2b0ec13101:v5
prerequisite-patch-id: 47dcf6861f7d434d25855b379e6d7ef4ce369c9c
prerequisite-patch-id: 77787fe82911923aff15ccf565e8fa451538c3a6
prerequisite-patch-id: b0bdb1742d96c5738f05262c3b0059102761390b
prerequisite-patch-id: 3927d39d8d77e35d5bfe53d9950da574ff8f2054
prerequisite-patch-id: a98039136a4796252a6029e474f03906f2541643
prerequisite-patch-id: c95f6dc0547a2a63a76e3cba0cf5c623b212b4e6
prerequisite-patch-id: 66e750e438ee959ddc2a6f0650814a2d8c989139
prerequisite-patch-id: 29a0fd8c36c1a4340f0d0b68a4c34d2b8abfb1ab
prerequisite-patch-id: 0bdfff661c33c380d1cf00a6c68688e05f88c0b3
prerequisite-patch-id: 99f15718e0bfbb7ed1a96dfa19f35841b004dae9
prerequisite-patch-id: 238db182251f2f79a947d16f0112a958d0837878
prerequisite-patch-id: 8c2cff9ceb9d062c91a34a39add45555bff2d545
prerequisite-patch-id: b93bb35db82d199d4d30deefbc34b8413539cf32

Best regards,
-- 
Yixun Lan

[PATCH v3 1/3] dt-bindings: gpio: spacemit: add support for K1 SoC

[Yixun Lan]

The GPIO controller of K1 support basic functions as input/output,
all pins can be used as interrupt which route to one IRQ line,
trigger type can be select between rising edge, failing edge, or both.
There are four GPIO banks, each consisting of 32 pins.

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

diff --git a/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml b/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..6ce8f27bd4a5a85f730420e103ea51710200d301
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/spacemit,k1-gpio.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/spacemit,k1-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SpacemiT K1 GPIO controller
+
+maintainers:
+  - Yixun Lan <dlan@gentoo.org>
+
+description:
+  The controller's registers are organized as sets of eight 32-bit
+  registers with each set controlling a bank of up to 32 pins.  A single
+  interrupt is shared for all of the banks handled by the controller.
+
+properties:
+  compatible:
+    const: spacemit,k1-gpio
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  gpio-ranges: true
+
+  interrupts:
+    maxItems: 1
+    description:
+      The interrupt shared by all GPIO lines for this controller.
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      The first cell is the GPIO number, the second should specify interrupt
+      flag. The controller does not support level interrupts, flags of
+      IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_LEVEL_LOW should not be used.
+      Refer <dt-bindings/interrupt-controller/irq.h> for valid flags.
+
+  interrupt-controller: true
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - "#gpio-cells"
+  - interrupts
+  - interrupt-controller
+  - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio@d4019000 {
+        compatible = "spacemit,k1-gpio";
+        reg = <0xd4019000 0x800>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupts = <58>;
+        interrupt-parent = <&plic>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        gpio-ranges = <&pinctrl 0 0 128>;
+    };

-- 
2.47.1

[PATCH v3 2/3] gpio: spacemit: add support for K1 SoC

[Yixun Lan]

Implement GPIO functionality which capable of setting pin as
input, output. Also, each pin can be used as interrupt which
support rising/failing edge type trigger, or both.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 drivers/gpio/Kconfig            |   7 +
 drivers/gpio/Makefile           |   1 +
 drivers/gpio/gpio-spacemit-k1.c | 454 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 462 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 93ee3aa092f81cd9573fcf91aef82ede1c5d7130..b525a98a6c176a4e71d2b0f1bd217e7d89141e81 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -655,6 +655,13 @@ config GPIO_SNPS_CREG
 	  where only several fields in register belong to GPIO lines and
 	  each GPIO line owns a field with different length and on/off value.
 
+config GPIO_SPACEMIT_K1
+	bool "SPACEMIT K1 GPIO support"
+	depends on ARCH_SPACEMIT || COMPILE_TEST
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support the SpacemiT's K1 GPIO device.
+
 config GPIO_SPEAR_SPICS
 	bool "ST SPEAr13xx SPI Chip Select as GPIO support"
 	depends on PLAT_SPEAR
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index af3ba4d81b583842893ea69e677fbe2abf31bc7b..6709ce511a0cf10310a94521c85a2d382dcfa696 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
 obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
 obj-$(CONFIG_GPIO_SLOPPY_LOGIC_ANALYZER) += gpio-sloppy-logic-analyzer.o
 obj-$(CONFIG_GPIO_SODAVILLE)		+= gpio-sodaville.o
+obj-$(CONFIG_GPIO_SPACEMIT_K1)		+= gpio-spacemit-k1.o
 obj-$(CONFIG_GPIO_SPEAR_SPICS)		+= gpio-spear-spics.o
 obj-$(CONFIG_GPIO_SPRD)			+= gpio-sprd.o
 obj-$(CONFIG_GPIO_STMPE)		+= gpio-stmpe.o
diff --git a/drivers/gpio/gpio-spacemit-k1.c b/drivers/gpio/gpio-spacemit-k1.c
new file mode 100644
index 0000000000000000000000000000000000000000..493ddc99f38c141187b05db6c292bc28ad5331b4
--- /dev/null
+++ b/drivers/gpio/gpio-spacemit-k1.c
@@ -0,0 +1,454 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2023-2024 SpacemiT (Hangzhou) Technology Co. Ltd
+ * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/irqdomain.h>
+
+/* register offset */
+#define GPLR		0x00
+#define GPDR		0x0c
+#define GPSR		0x18
+#define GPCR		0x24
+#define GRER		0x30
+#define GFER		0x3c
+#define GEDR		0x48
+#define GSDR		0x54
+#define GCDR		0x60
+#define GSRER		0x6c
+#define GCRER		0x78
+#define GSFER		0x84
+#define GCFER		0x90
+#define GAPMASK		0x9c
+#define GCPMASK		0xa8
+
+#define K1_BANK_GPIO_NUMBER	(32)
+#define BANK_GPIO_MASK		(K1_BANK_GPIO_NUMBER - 1)
+
+#define spacemit_gpio_to_bank_idx(gpio)		((gpio) / K1_BANK_GPIO_NUMBER)
+#define spacemit_gpio_to_bank_offset(gpio)	((gpio) & BANK_GPIO_MASK)
+#define spacemit_bank_to_gpio(idx, offset)	\
+	(((idx) * K1_BANK_GPIO_NUMBER) | ((offset) & BANK_GPIO_MASK))
+
+static u32 k1_gpio_bank_offset[] = { 0x0, 0x4, 0x8, 0x100 };
+
+struct spacemit_gpio_bank {
+	void __iomem			*reg_bank;
+	u32				irq_mask;
+	u32				irq_rising_edge;
+	u32				irq_falling_edge;
+};
+
+struct spacemit_gpio_chip {
+	struct gpio_chip		chip;
+	struct irq_domain		*domain;
+	struct spacemit_gpio_bank	*banks;
+	void __iomem			*reg_base;
+	unsigned int			ngpio;
+	unsigned int			nbank;
+	int				irq;
+};
+
+static struct spacemit_gpio_chip *to_spacemit_gpio_chip(struct gpio_chip *chip)
+{
+	return container_of(chip, struct spacemit_gpio_chip, chip);
+}
+
+static inline void spacemit_clear_edge_detection(struct spacemit_gpio_bank *bank,
+						 u32 bit)
+{
+	writel(bit, bank->reg_bank + GCRER);
+	writel(bit, bank->reg_bank + GCFER);
+}
+
+static inline void spacemit_set_edge_detection(struct spacemit_gpio_bank *bank,
+					       u32 bit)
+{
+	writel(bit & bank->irq_rising_edge, bank->reg_bank + GSRER);
+	writel(bit & bank->irq_falling_edge, bank->reg_bank + GSFER);
+}
+
+static void spacemit_reset_edge_detection(struct spacemit_gpio_chip *schip)
+{
+	struct spacemit_gpio_bank *bank;
+	unsigned int i;
+
+	for (i = 0; i < schip->nbank; i++) {
+		bank = &schip->banks[i];
+
+		writel(0xffffffff, bank->reg_bank + GCFER);
+		writel(0xffffffff, bank->reg_bank + GCRER);
+		writel(0xffffffff, bank->reg_bank + GAPMASK);
+	}
+}
+
+static int spacemit_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	struct spacemit_gpio_chip *schip = to_spacemit_gpio_chip(chip);
+
+	return irq_create_mapping(schip->domain, offset);
+}
+
+static struct spacemit_gpio_bank *
+spacemit_gpio_get_bank(struct spacemit_gpio_chip *schip,
+		       unsigned int offset)
+{
+	return &schip->banks[spacemit_gpio_to_bank_idx(offset)];
+}
+
+static int spacemit_gpio_direction_input(struct gpio_chip *chip,
+					 unsigned int offset)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	u32 bit;
+
+	schip =	to_spacemit_gpio_chip(chip);
+	bank = spacemit_gpio_get_bank(schip, offset);
+	bit = BIT(spacemit_gpio_to_bank_offset(offset));
+
+	writel(bit, bank->reg_bank + GCDR);
+
+	return 0;
+}
+
+static int spacemit_gpio_direction_output(struct gpio_chip *chip,
+					  unsigned int offset, int value)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	u32 bit;
+
+	schip =	to_spacemit_gpio_chip(chip);
+	bank = spacemit_gpio_get_bank(schip, offset);
+	bit = BIT(spacemit_gpio_to_bank_offset(offset));
+
+	writel(bit, bank->reg_bank + (value ? GPSR : GPCR));
+	writel(bit, bank->reg_bank + GSDR);
+
+	return 0;
+}
+
+static int spacemit_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	u32 bit, gplr;
+
+	schip =	to_spacemit_gpio_chip(chip);
+	bank = spacemit_gpio_get_bank(schip, offset);
+	bit = BIT(spacemit_gpio_to_bank_offset(offset));
+
+	gplr = readl(bank->reg_bank + GPLR);
+
+	return !!(gplr & bit);
+}
+
+static void spacemit_gpio_set(struct gpio_chip *chip,
+			      unsigned int offset, int value)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	u32 bit, gpdr;
+
+	schip =	to_spacemit_gpio_chip(chip);
+	bank = spacemit_gpio_get_bank(schip, offset);
+	bit = BIT(spacemit_gpio_to_bank_offset(offset));
+	gpdr = readl(bank->reg_bank + GPDR);
+
+	/* Is it configured as output? */
+	if (gpdr & bit)
+		writel(bit, bank->reg_bank + (value ? GPSR : GPCR));
+}
+
+#ifdef CONFIG_OF_GPIO
+static int spacemit_gpio_of_xlate(struct gpio_chip *chip,
+				  const struct of_phandle_args *gpiospec,
+				  u32 *flags)
+{
+	struct spacemit_gpio_chip *schip;
+
+	schip =	to_spacemit_gpio_chip(chip);
+	/* GPIO index start from 0. */
+	if (gpiospec->args[0] >= schip->ngpio)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[1];
+
+	return gpiospec->args[0];
+}
+#endif
+
+static int spacemit_gpio_irq_type(struct irq_data *d, unsigned int type)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	int gpio = irqd_to_hwirq(d);
+	u32 bit;
+
+	schip = irq_data_get_irq_chip_data(d);
+	bank = spacemit_gpio_get_bank(schip, gpio);
+	bit = BIT(spacemit_gpio_to_bank_offset(gpio));
+
+	if (type & IRQ_TYPE_EDGE_RISING) {
+		bank->irq_rising_edge |= bit;
+		writel(bit, bank->reg_bank + GSRER);
+	} else {
+		bank->irq_rising_edge &= ~bit;
+		writel(bit, bank->reg_bank + GCRER);
+	}
+
+	if (type & IRQ_TYPE_EDGE_FALLING) {
+		bank->irq_falling_edge |= bit;
+		writel(bit, bank->reg_bank + GSFER);
+	} else {
+		bank->irq_falling_edge &= ~bit;
+		writel(bit, bank->reg_bank + GCFER);
+	}
+
+	return 0;
+}
+
+static irqreturn_t spacemit_gpio_irq_handler(int irq, void *data)
+{
+	struct spacemit_gpio_chip *schip = data;
+	struct spacemit_gpio_bank *bank;
+	unsigned int irqs_handled = 0;
+	unsigned long pending = 0;
+	u32 gedr, girq;
+	int i, n;
+
+	for (i = 0; i < schip->nbank; i++) {
+		bank = &schip->banks[i];
+
+		gedr = readl(bank->reg_bank + GEDR);
+		if (!gedr)
+			continue;
+
+		writel(gedr, bank->reg_bank + GEDR);
+		gedr = gedr & bank->irq_mask;
+
+		if (!gedr)
+			continue;
+
+		pending = gedr;
+		for_each_set_bit(n, &pending, BITS_PER_LONG) {
+			girq = irq_find_mapping(schip->domain,
+						spacemit_bank_to_gpio(i, n));
+			handle_nested_irq(girq);
+		}
+
+		irqs_handled++;
+	}
+
+	return irqs_handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void spacemit_ack_muxed_gpio(struct irq_data *d)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	int gpio = irqd_to_hwirq(d);
+	u32 bit;
+
+	schip = irq_data_get_irq_chip_data(d);
+	bank = spacemit_gpio_get_bank(schip, gpio);
+	bit = BIT(spacemit_gpio_to_bank_offset(gpio));
+
+	writel(bit, bank->reg_bank + GEDR);
+}
+
+static void spacemit_mask_muxed_gpio(struct irq_data *d)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	int gpio = irqd_to_hwirq(d);
+	u32 bit;
+
+	schip = irq_data_get_irq_chip_data(d);
+	bank = spacemit_gpio_get_bank(schip, gpio);
+	bit = BIT(spacemit_gpio_to_bank_offset(gpio));
+
+	bank->irq_mask &= ~bit;
+
+	spacemit_clear_edge_detection(bank, bit);
+}
+
+static void spacemit_unmask_muxed_gpio(struct irq_data *d)
+{
+	struct spacemit_gpio_chip *schip;
+	struct spacemit_gpio_bank *bank;
+	int gpio = irqd_to_hwirq(d);
+	u32 bit;
+
+	schip = irq_data_get_irq_chip_data(d);
+	bank = spacemit_gpio_get_bank(schip, gpio);
+	bit = BIT(spacemit_gpio_to_bank_offset(gpio));
+
+	bank->irq_mask |= bit;
+
+	spacemit_set_edge_detection(bank, bit);
+}
+
+static struct irq_chip spacemit_muxed_gpio_chip = {
+	.name		= "k1-gpio-irqchip",
+	.irq_ack	= spacemit_ack_muxed_gpio,
+	.irq_mask	= spacemit_mask_muxed_gpio,
+	.irq_unmask	= spacemit_unmask_muxed_gpio,
+	.irq_set_type	= spacemit_gpio_irq_type,
+	.flags		= IRQCHIP_SKIP_SET_WAKE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int spacemit_irq_domain_map(struct irq_domain *d, unsigned int irq,
+				   irq_hw_number_t hw)
+{
+	irq_set_chip_data(irq, d->host_data);
+	irq_set_chip_and_handler(irq, &spacemit_muxed_gpio_chip,
+				 handle_edge_irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops spacemit_gpio_irq_domain_ops = {
+	.map	= spacemit_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int spacemit_gpio_probe_dt(struct platform_device *pdev,
+				  struct spacemit_gpio_chip *schip)
+{
+	void __iomem *reg;
+	int i, nbank;
+
+	nbank = ARRAY_SIZE(k1_gpio_bank_offset);
+
+	schip->banks = devm_kzalloc(&pdev->dev,
+				    sizeof(*schip->banks) * nbank,
+				    GFP_KERNEL);
+	if (!schip->banks)
+		return -ENOMEM;
+
+	for (i = 0; i < nbank; i++) {
+		reg = schip->reg_base + k1_gpio_bank_offset[i];
+		schip->banks[i].reg_bank = reg;
+	}
+
+	schip->nbank = nbank;
+	schip->ngpio = nbank * K1_BANK_GPIO_NUMBER;
+
+	return 0;
+}
+
+static void spacemit_gpio_remove_irq_domain(void *data)
+{
+	struct irq_domain *domain = data;
+
+	irq_domain_remove(domain);
+}
+
+static int spacemit_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np;
+	struct spacemit_gpio_chip *schip;
+	struct irq_domain *domain;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	np = pdev->dev.of_node;
+	if (!np)
+		return -EINVAL;
+
+	schip = devm_kzalloc(dev, sizeof(*schip), GFP_KERNEL);
+	if (!schip)
+		return -ENOMEM;
+
+	schip->irq = platform_get_irq(pdev, 0);
+	if (schip->irq < 0)
+		return schip->irq;
+
+	base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	schip->reg_base = base;
+
+	ret = spacemit_gpio_probe_dt(pdev, schip);
+	if (ret)
+		return dev_err_probe(dev, ret, "Fail to initialize gpio unit\n");
+
+	spacemit_reset_edge_detection(schip);
+
+	domain = irq_domain_add_linear(np, schip->ngpio,
+				       &spacemit_gpio_irq_domain_ops,
+				       schip);
+	if (domain == NULL)
+		return -EINVAL;
+
+	ret = devm_add_action_or_reset(dev, spacemit_gpio_remove_irq_domain,
+				       domain);
+	if (ret)
+		return ret;
+
+	schip->domain			= domain;
+	schip->chip.label		= "k1-gpio";
+	schip->chip.parent		= dev;
+	schip->chip.request		= gpiochip_generic_request;
+	schip->chip.free		= gpiochip_generic_free;
+	schip->chip.direction_input	= spacemit_gpio_direction_input;
+	schip->chip.direction_output	= spacemit_gpio_direction_output;
+	schip->chip.get			= spacemit_gpio_get;
+	schip->chip.set			= spacemit_gpio_set;
+	schip->chip.to_irq		= spacemit_gpio_to_irq;
+#ifdef CONFIG_OF_GPIO
+	schip->chip.of_xlate		= spacemit_gpio_of_xlate;
+	schip->chip.of_gpio_n_cells	= 2;
+#endif
+	schip->chip.ngpio		= schip->ngpio;
+	schip->chip.base		= -1;
+
+	ret = devm_request_threaded_irq(dev, schip->irq, NULL,
+					spacemit_gpio_irq_handler,
+					IRQF_ONESHOT,
+					schip->chip.label, schip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to request high IRQ\n");
+
+	ret = devm_gpiochip_add_data(dev, &schip->chip, schip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to add gpiochip\n");
+
+	return 0;
+}
+
+static const struct of_device_id spacemit_gpio_dt_ids[] = {
+	{ .compatible = "spacemit,k1-gpio" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver spacemit_gpio_driver = {
+	.probe		= spacemit_gpio_probe,
+	.driver		= {
+		.name	= "k1-gpio",
+		.of_match_table = spacemit_gpio_dt_ids,
+	},
+};
+module_platform_driver(spacemit_gpio_driver);
+
+MODULE_DESCRIPTION("GPIO driver for SpacemiT K1 SoC");
+MODULE_LICENSE("GPL");

-- 
2.47.1

[PATCH v3 3/3] riscv: dts: spacemit: add gpio support for K1 SoC

[Yixun Lan]

Populate the GPIO node in the device tree for K1 SoC.
Also, map all 128 pins as GPIO to the pinctrl controller.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 arch/riscv/boot/dts/spacemit/k1.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index a2d5f7d4a942af26b3ba991928f23b2d9943366a..bdd25584d67c9a9e41f8d8227fe84a1bdeed7b41 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -416,6 +416,18 @@ uart9: serial@d4017800 {
 			status = "disabled";
 		};
 
+		gpio: gpio@d4019000 {
+			compatible = "spacemit,k1-gpio";
+			reg = <0x0 0xd4019000 0x0 0x800>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupts = <58>;
+			interrupt-parent = <&plic>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 128>;
+		};
+
 		pinctrl: pinctrl@d401e000 {
 			compatible = "spacemit,k1-pinctrl";
 			reg = <0x0 0xd401e000 0x0 0x400>;

-- 
2.47.1

Re: [PATCH v3 1/3] dt-bindings: gpio: spacemit: add support for K1 SoC

[Linus Walleij]

On Wed, Dec 25, 2024 at 1:33 AM Yixun Lan <dlan@gentoo.org> wrote:

> The GPIO controller of K1 support basic functions as input/output,
> all pins can be used as interrupt which route to one IRQ line,
> trigger type can be select between rising edge, failing edge, or both.
> There are four GPIO banks, each consisting of 32 pins.
>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>

Looks good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 2/3] gpio: spacemit: add support for K1 SoC

[Linus Walleij]

Hi Yixun,

thanks for your patch!

Some comments below:

On Wed, Dec 25, 2024 at 1:33 AM Yixun Lan <dlan@gentoo.org> wrote:

> Implement GPIO functionality which capable of setting pin as
> input, output. Also, each pin can be used as interrupt which
> support rising/failing edge type trigger, or both.
>
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
(...)
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/clk.h>
> +#include <linux/gpio.h>

Please drop this legacy include. It should not be needed.

> +#include <linux/gpio/driver.h>

This should be enough for any driver.

> +/* register offset */
> +#define GPLR           0x00
> +#define GPDR           0x0c
> +#define GPSR           0x18
> +#define GPCR           0x24
> +#define GRER           0x30
> +#define GFER           0x3c
> +#define GEDR           0x48
> +#define GSDR           0x54
> +#define GCDR           0x60
> +#define GSRER          0x6c
> +#define GCRER          0x78
> +#define GSFER          0x84
> +#define GCFER          0x90
> +#define GAPMASK                0x9c
> +#define GCPMASK                0xa8

This looks like the archetype of a memory-mapped GPIO chip.

> +#define spacemit_gpio_to_bank_idx(gpio)                ((gpio) / K1_BANK_GPIO_NUMBER)
> +#define spacemit_gpio_to_bank_offset(gpio)     ((gpio) & BANK_GPIO_MASK)
> +#define spacemit_bank_to_gpio(idx, offset)     \
> +       (((idx) * K1_BANK_GPIO_NUMBER) | ((offset) & BANK_GPIO_MASK))
> +
> +static u32 k1_gpio_bank_offset[] = { 0x0, 0x4, 0x8, 0x100 };

Yet this is not registered on a per-bank basis, instead all four banks
are hammered into one single chip. Why?

Can you please split this into 4 instances, also in the device
tree. It makes everything much simpler.

> +struct spacemit_gpio_chip {
> +       struct gpio_chip                chip;
> +       struct irq_domain               *domain;

If you're using GPIOLIB_IRQCHIP you should not
also define youe own irq_domain, the gpiolib handles this.

> +       struct spacemit_gpio_bank       *banks;
> +       void __iomem                    *reg_base;
> +       unsigned int                    ngpio;

Is this ever used after initialization?

> +       unsigned int                    nbank;
> +       int                             irq;

Or this?

> +static int spacemit_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct spacemit_gpio_chip *schip = to_spacemit_gpio_chip(chip);
> +
> +       return irq_create_mapping(schip->domain, offset);
> +}

Should not be needed when using GPIOLIB_IRQCHIP.

> +       schip = to_spacemit_gpio_chip(chip);
> +       bank = spacemit_gpio_get_bank(schip, offset);
> +       bit = BIT(spacemit_gpio_to_bank_offset(offset));

#include <linux/bits.h> to use the BIT() macro.

This driver becomes unnecessarily complex due to collecting 4 GPIO chips
into one.

Please split it into 4 instances of the same driver, then look at other
drivers using select GPIO_GENERIC such as
drivers/gpio/gpio-pl061.c
drivers/gpio/gpio-ftgpio010.c
for examples of how to create a very compact and simple driver
reusing the generic GPIO helpers, this will also provide
get/set_multiple implementations and other useful things.

Yours,
Linus Walleij

Re: [PATCH v3 1/3] dt-bindings: gpio: spacemit: add support for K1 SoC

[Linus Walleij]

On Wed, Dec 25, 2024 at 1:33 AM Yixun Lan <dlan@gentoo.org> wrote:

> The GPIO controller of K1 support basic functions as input/output,
> all pins can be used as interrupt which route to one IRQ line,
> trigger type can be select between rising edge, failing edge, or both.
> There are four GPIO banks, each consisting of 32 pins.
(...)
> +description:
> +  The controller's registers are organized as sets of eight 32-bit
> +  registers with each set controlling a bank of up to 32 pins.  A single
> +  interrupt is shared for all of the banks handled by the controller.

I looked at the driver and came to the conclusion that it's better to use
4 different instances of the chip, one for each set of 32bit registers,
so these 4 GPIO controllers are instantiated separately.

The operating system can handle the shared interrupt, there is no
need to use a single device instance just because the interrupt is
shared.

DT bindings are operating system neutral, but for example in Linux
(if we pretend this is just one possible example) then a driver
handling a shared IRQ can be requested with the flag IRQF_SHARED
and the driver can just return IRQ_HANDLED if it handled an IRQ
or IRQ_NONE if it didn't handle the irq (so other instances can
handle it).

Yours,
Linus Walleij

Re: [PATCH v3 1/3] dt-bindings: gpio: spacemit: add support for K1 SoC

[Yixun Lan]

Hi Linus:

thanks for your review

On 17:34 Fri 27 Dec     , Linus Walleij wrote:
> On Wed, Dec 25, 2024 at 1:33 AM Yixun Lan <dlan@gentoo.org> wrote:
> 
> > The GPIO controller of K1 support basic functions as input/output,
> > all pins can be used as interrupt which route to one IRQ line,
> > trigger type can be select between rising edge, failing edge, or both.
> > There are four GPIO banks, each consisting of 32 pins.
> (...)
> > +description:
> > +  The controller's registers are organized as sets of eight 32-bit
> > +  registers with each set controlling a bank of up to 32 pins.  A single
> > +  interrupt is shared for all of the banks handled by the controller.
> 
> I looked at the driver and came to the conclusion that it's better to use
> 4 different instances of the chip, one for each set of 32bit registers,
> so these 4 GPIO controllers are instantiated separately.
> 
sounds good to me, I will work according to this in next version

> The operating system can handle the shared interrupt, there is no
> need to use a single device instance just because the interrupt is
> shared.
> 
> DT bindings are operating system neutral, but for example in Linux
> (if we pretend this is just one possible example) then a driver
> handling a shared IRQ can be requested with the flag IRQF_SHARED
> and the driver can just return IRQ_HANDLED if it handled an IRQ
> or IRQ_NONE if it didn't handle the irq (so other instances can
> handle it).
> 
agree, make sense

> Yours,
> Linus Walleij

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55