[v7,0/4] riscv: spacemit: add gpio support for K1 SoC

Message

Yixun Lan Feb. 26, 2025, 12:41 a.m. UTC

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 ports, each consisting of 32 pins and
has indepedent register sets, while still sharing IRQ line and clocks.

The GPIO controller 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 merged.

Due to first three GPIO ports has interleave register settings, some
resources (IRQ, clock) are shared by all pins.

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

Note, this patch is rebased to v6.14-rc1.

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

This version should resolve DT related concern in V4, and register each bank as
indepedent gpio chip in driver, no more sub children gpio DT node needed.

One problem is still not resolved, the interrupt cells parsing isn't correct.
but it works if request gpio irq via gpiod_get() + gpiod_to_irq()

Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf [1]
Link: https://lore.kernel.org/all/[e-mail] [2]
Link: https://lore.kernel.org/all/[e-mail]/ [3]
Link: https://lore.kernel.org/all/[e-mail] [4]
Link: https://lore.kernel.org/all/[e-mail] [5]
Signed-off-by: Yixun Lan <[e-mail]>
---
Changes in v7:
- dt-binding: fix 80 column, drop unneeded dependencies
- tested with patch v3 of "gpiolib: of: Handle threecell gpios" [5]
- collect review tags
- Link to v6: https://lore.kernel.org/r/[e-mail]

Changes in v6:
- rebase to threecell gpio patch which proposed by LinusW at [4], 
  drop unneeded *xlate(), *add_pin_range() function
- add SPACEMIT prefix to macro
- adjust register comments
- drop 'index' member, instead calculate from offset
- add IRQCHIP_SKIP_SET_WAKE as gpio doesn't support irq wake up
- drop #ifdef CONFIG_OF_GPIO
- move interrupt mask disabling/enabling into irq_*mask()
- Link to v5: https://lore.kernel.org/r/[e-mail]

Changes in v5:
- export add_pin_range() from gpio core, support to add custom version
- change to 3 gpio cells, model to <bank number>, <bank offset>, <gpio flag>
- fold children DT nodes into parent
- Link to v4: https://lore.kernel.org/r/[e-mail]

Changes in v4:
- gpio: re-construct gpio as four independent ports, also leverage gpio mmio API
- gpio interrupt: convert to generic gpio irqchip
- Link to v3: https://lore.kernel.org/r/[e-mail]

Changes in v3:
- dt: drop ranges, interrupt-names property
- Link to v2: https://lore.kernel.org/r/[e-mail]

Changes in v2:
- address dt-bindings comments, simplify example
- rebase to 6.13-rc3 
- Link to v1: https://lore.kernel.org/r/[e-mail]

---
Yixun Lan (4):
      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
      riscv: dts: spacemit: add gpio LED for system heartbeat

 .../devicetree/bindings/gpio/spacemit,k1-gpio.yaml |  79 ++++++
 arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts    |  11 +
 arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi       |   3 +
 arch/riscv/boot/dts/spacemit/k1.dtsi               |  15 ++
 drivers/gpio/Kconfig                               |   8 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-spacemit-k1.c                    | 277 +++++++++++++++++++++
 7 files changed, 394 insertions(+)
---
base-commit: 3d72d603afa72082501e9076eed61e0531339ef8
change-id: 20240828-03-k1-gpio-61bf92f9032c
prerequisite-change-id: 20250217-gpio-ranges-fourcell-85888ad219da:v3
prerequisite-patch-id: 9d4c8b05cc56d25bfb93f3b06420ba6e93340d31
prerequisite-patch-id: 7949035abd05ec02a9426bb17819d9108e66e0d7

Best regards,

Comments

Yixun Lan Feb. 26, 2025, 1:01 a.m. UTC | #1

Hi Linus Walleij:

  I'm quite satisfied with this version, but there is still one problem
of irq parsing that haven't been resolved, although it probably is 
an independent patch that we can submit it later.

  For this, I'm not sure what's the approach we should proceed,
1) if we can get current version merged first then solve it later, or 
2) find a solution now and get it eventually fixed in this cycle

for the detail problem, see comments below

On 08:41 Wed 26 Feb     , Yixun Lan wrote:
> 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 ports, each consisting of 32 pins and
> has indepedent register sets, while still sharing IRQ line and clocks.
> 
> The GPIO controller 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 merged.
> 
> Due to first three GPIO ports has interleave register settings, some
> resources (IRQ, clock) are shared by all pins.
> 
> The GPIO docs of K1 SoC can be found here, chapter 16.4 GPIO [1]
> 
> Note, this patch is rebased to v6.14-rc1.
> 
> 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
> 
> This version should resolve DT related concern in V4, and register each bank as
> indepedent gpio chip in driver, no more sub children gpio DT node needed.
> 
> One problem is still not resolved, the interrupt cells parsing isn't correct.
> but it works if request gpio irq via gpiod_get() + gpiod_to_irq()
> 

Let me iterate a little bit more detail on this..

Current this v7 version work great with request irq from gpio, like:
	pin = devm_gpiod_get_optional(dev, "myirq", GPIOD_IN);
	irq = gpiod_to_irq(pin);
	devm_request_threaded_irq(dev, irq, ..)

but have problem if request irq via of_irq_get(), something like this:
DT part 
	mytst {
		..
		interrupt-parent = <&gpio>;
		interrupts = <1 28 IRQ_TYPE_EDGE_RISING>;
		interrupt-names = "wakeup";
	}

In source code
	irq = of_irq_get_byname(dev->of_node, "wakeup");

I've made an attempt to patch gpiolib to support three cells "interrupts"
syntax, but still fail, it always get last gpio irqchip of four, thus using
the wrong pin (e.g: will always get 3 from gpiochips 0, 1, 2, 3)


> Link: https://developer.spacemit.com/documentation?token=Rn9Kw3iFHirAMgkIpTAcV2Arnkf [1]
> Link: https://lore.kernel.org/all/[e-mail] [2]
> Link: https://lore.kernel.org/all/[e-mail]/ [3]
> Link: https://lore.kernel.org/all/[e-mail] [4]
> Link: https://lore.kernel.org/all/[e-mail] [5]
> Signed-off-by: Yixun Lan <[e-mail]>
> ---
> Changes in v7:
> - dt-binding: fix 80 column, drop unneeded dependencies
> - tested with patch v3 of "gpiolib: of: Handle threecell gpios" [5]
> - collect review tags
> - Link to v6: https://lore.kernel.org/r/[e-mail]
> 
> Changes in v6:
> - rebase to threecell gpio patch which proposed by LinusW at [4], 
>   drop unneeded *xlate(), *add_pin_range() function
> - add SPACEMIT prefix to macro
> - adjust register comments
> - drop 'index' member, instead calculate from offset
> - add IRQCHIP_SKIP_SET_WAKE as gpio doesn't support irq wake up
> - drop #ifdef CONFIG_OF_GPIO
> - move interrupt mask disabling/enabling into irq_*mask()
> - Link to v5: https://lore.kernel.org/r/[e-mail]
> 
> Changes in v5:
> - export add_pin_range() from gpio core, support to add custom version
> - change to 3 gpio cells, model to <bank number>, <bank offset>, <gpio flag>
> - fold children DT nodes into parent
> - Link to v4: https://lore.kernel.org/r/[e-mail]
> 
> Changes in v4:
> - gpio: re-construct gpio as four independent ports, also leverage gpio mmio API
> - gpio interrupt: convert to generic gpio irqchip
> - Link to v3: https://lore.kernel.org/r/[e-mail]
> 
> Changes in v3:
> - dt: drop ranges, interrupt-names property
> - Link to v2: https://lore.kernel.org/r/[e-mail]
> 
> Changes in v2:
> - address dt-bindings comments, simplify example
> - rebase to 6.13-rc3 
> - Link to v1: https://lore.kernel.org/r/[e-mail]
> 
> ---
> Yixun Lan (4):
>       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
>       riscv: dts: spacemit: add gpio LED for system heartbeat
> 
>  .../devicetree/bindings/gpio/spacemit,k1-gpio.yaml |  79 ++++++
>  arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts    |  11 +
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi       |   3 +
>  arch/riscv/boot/dts/spacemit/k1.dtsi               |  15 ++
>  drivers/gpio/Kconfig                               |   8 +
>  drivers/gpio/Makefile                              |   1 +
>  drivers/gpio/gpio-spacemit-k1.c                    | 277 +++++++++++++++++++++
>  7 files changed, 394 insertions(+)
> ---
> base-commit: 3d72d603afa72082501e9076eed61e0531339ef8
> change-id: 20240828-03-k1-gpio-61bf92f9032c
> prerequisite-change-id: 20250217-gpio-ranges-fourcell-85888ad219da:v3
> prerequisite-patch-id: 9d4c8b05cc56d25bfb93f3b06420ba6e93340d31
> prerequisite-patch-id: 7949035abd05ec02a9426bb17819d9108e66e0d7
> 
> Best regards,
> -- 
> Yixun Lan
>

Linus Walleij Feb. 26, 2025, 10:24 a.m. UTC | #2

On Wed, Feb 26, 2025 at 2:01 AM Yixun Lan <[e-mail]> wrote:

> Current this v7 version work great with request irq from gpio, like:
>         pin = devm_gpiod_get_optional(dev, "myirq", GPIOD_IN);
>         irq = gpiod_to_irq(pin);
>         devm_request_threaded_irq(dev, irq, ..)
>
> but have problem if request irq via of_irq_get(), something like this:
> DT part
>         mytst {
>                 ..
>                 interrupt-parent = <&gpio>;
>                 interrupts = <1 28 IRQ_TYPE_EDGE_RISING>;
>                 interrupt-names = "wakeup";
>         }
>
> In source code
>         irq = of_irq_get_byname(dev->of_node, "wakeup");
>
> I've made an attempt to patch gpiolib to support three cells "interrupts"
> syntax, but still fail, it always get last gpio irqchip of four, thus using
> the wrong pin (e.g: will always get 3 from gpiochips 0, 1, 2, 3)

Right, we need a proper patch to fix this.

Can you paste your patch so I can see if I can spot/fix
the problem?

I think the irq cell parser needs to call out to
of_node_instance_match() - or similar - as well.

Yours,
Linus Walleij

Yixun Lan Feb. 26, 2025, 11:59 a.m. UTC | #3

Hi Linus Walleij:

On 11:24 Wed 26 Feb     , Linus Walleij wrote:
> On Wed, Feb 26, 2025 at 2:01 AM Yixun Lan <[e-mail]> wrote:
> 
> > Current this v7 version work great with request irq from gpio, like:
> >         pin = devm_gpiod_get_optional(dev, "myirq", GPIOD_IN);
> >         irq = gpiod_to_irq(pin);
> >         devm_request_threaded_irq(dev, irq, ..)
> >
> > but have problem if request irq via of_irq_get(), something like this:
> > DT part
> >         mytst {
> >                 ..
> >                 interrupt-parent = <&gpio>;
> >                 interrupts = <1 28 IRQ_TYPE_EDGE_RISING>;
> >                 interrupt-names = "wakeup";
> >         }
> >
> > In source code
> >         irq = of_irq_get_byname(dev->of_node, "wakeup");
> >
> > I've made an attempt to patch gpiolib to support three cells "interrupts"
> > syntax, but still fail, it always get last gpio irqchip of four, thus using
> > the wrong pin (e.g: will always get 3 from gpiochips 0, 1, 2, 3)
> 
> Right, we need a proper patch to fix this.
> 
> Can you paste your patch so I can see if I can spot/fix
> the problem?
> 
> I think the irq cell parser needs to call out to
> of_node_instance_match() - or similar - as well.
do you have any suggestion where to implement this similar function?

I actually miss this logic, the patch here only support parsing
interrupts with 3 cells

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 679ed764cb14..9aa88c3fa485 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1454,6 +1454,10 @@ static int gpiochip_hierarchy_irq_domain_translate(struct irq_domain *d,
 		return irq_domain_translate_twocell(d, fwspec, hwirq, type);
 	}
 
+	if (is_of_node(fwspec->fwnode) && fwspec->param_count == 3) {
+		return irq_domain_translate_threecell(d, fwspec, hwirq, type);
+	}
+
 	/* This is for board files and others not using DT */
 	if (is_fwnode_irqchip(fwspec->fwnode)) {
 		int ret;
@@ -1758,7 +1762,8 @@ static const struct irq_domain_ops gpiochip_domain_ops = {
 	.map	= gpiochip_irq_map,
 	.unmap	= gpiochip_irq_unmap,
 	/* Virtually all GPIO irqchips are twocell:ed */
-	.xlate	= irq_domain_xlate_twocell,
+	/* FIXME: force switch to three cells */
+	.xlate	= irq_domain_xlate_threecell,
 };
 
 static struct irq_domain *gpiochip_simple_create_domain(struct gpio_chip *gc)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index e432b6a12a32..69a9540ec253 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -568,10 +568,18 @@ int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
 int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
 			const u32 *intspec, unsigned int intsize,
 			irq_hw_number_t *out_hwirq, unsigned int *out_type);
+int irq_domain_xlate_threecell(struct irq_domain *d, struct device_node *ctrlr,
+			const u32 *intspec, unsigned int intsize,
+			irq_hw_number_t *out_hwirq, unsigned int *out_type);
 int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
 			const u32 *intspec, unsigned int intsize,
 			irq_hw_number_t *out_hwirq, unsigned int *out_type);
 
+int irq_domain_translate_threecell(struct irq_domain *d,
+				 struct irq_fwspec *fwspec,
+				 unsigned long *out_hwirq,
+				 unsigned int *out_type);
+
 int irq_domain_translate_twocell(struct irq_domain *d,
 				 struct irq_fwspec *fwspec,
 				 unsigned long *out_hwirq,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index ec6d8e72d980..995e5e0ec2db 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1132,6 +1132,17 @@ int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
 }
 EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
 
+int irq_domain_xlate_threecell(struct irq_domain *d, struct device_node *ctrlr,
+			const u32 *intspec, unsigned int intsize,
+			irq_hw_number_t *out_hwirq, unsigned int *out_type)
+{
+	struct irq_fwspec fwspec;
+
+	of_phandle_args_to_fwspec(ctrlr, intspec, intsize, &fwspec);
+	return irq_domain_translate_threecell(d, &fwspec, out_hwirq, out_type);
+}
+EXPORT_SYMBOL_GPL(irq_domain_xlate_threecell);
+
 /**
  * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
  * @d:		Interrupt domain involved in the translation
@@ -1216,6 +1227,19 @@ int irq_domain_translate_twocell(struct irq_domain *d,
 }
 EXPORT_SYMBOL_GPL(irq_domain_translate_twocell);
 
+int irq_domain_translate_threecell(struct irq_domain *d,
+				 struct irq_fwspec *fwspec,
+				 unsigned long *out_hwirq,
+				 unsigned int *out_type)
+{
+	if (WARN_ON(fwspec->param_count < 3))
+		return -EINVAL;
+	*out_hwirq = fwspec->param[1];
+	*out_type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(irq_domain_translate_threecell);
+
 int irq_domain_alloc_descs(int virq, unsigned int cnt, irq_hw_number_t hwirq,
 			   int node, const struct irq_affinity_desc *affinity)
 {

Yixun Lan Feb. 26, 2025, 1:56 p.m. UTC | #4

Hi Linus Walleij:

I went ahead and made further progress on this, and now
the 3 cell interrupts model work fine, although still few issues left

see patch below

On 12:00 Wed 26 Feb     , Yixun Lan wrote:
> Hi Linus Walleij:
> 
> On 11:24 Wed 26 Feb     , Linus Walleij wrote:
> > On Wed, Feb 26, 2025 at 2:01 AM Yixun Lan <[e-mail]> wrote:
> > 
> > > Current this v7 version work great with request irq from gpio, like:
> > >         pin = devm_gpiod_get_optional(dev, "myirq", GPIOD_IN);
> > >         irq = gpiod_to_irq(pin);
> > >         devm_request_threaded_irq(dev, irq, ..)
> > >
> > > but have problem if request irq via of_irq_get(), something like this:
> > > DT part
> > >         mytst {
> > >                 ..
> > >                 interrupt-parent = <&gpio>;
> > >                 interrupts = <1 28 IRQ_TYPE_EDGE_RISING>;
> > >                 interrupt-names = "wakeup";
> > >         }
> > >
> > > In source code
> > >         irq = of_irq_get_byname(dev->of_node, "wakeup");
> > >
> > > I've made an attempt to patch gpiolib to support three cells "interrupts"
> > > syntax, but still fail, it always get last gpio irqchip of four, thus using
> > > the wrong pin (e.g: will always get 3 from gpiochips 0, 1, 2, 3)
> > 
> > Right, we need a proper patch to fix this.
> > 
> > Can you paste your patch so I can see if I can spot/fix
> > the problem?
> > 
> > I think the irq cell parser needs to call out to
> > of_node_instance_match() - or similar - as well.
> do you have any suggestion where to implement this similar function?
> 
> I actually miss this logic, the patch here only support parsing
> interrupts with 3 cells
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 679ed764cb14..9aa88c3fa485 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1454,6 +1454,10 @@ static int gpiochip_hierarchy_irq_domain_translate(struct irq_domain *d,
>  		return irq_domain_translate_twocell(d, fwspec, hwirq, type);
>  	}
>  
> +	if (is_of_node(fwspec->fwnode) && fwspec->param_count == 3) {
> +		return irq_domain_translate_threecell(d, fwspec, hwirq, type);
> +	}
> +
>  	/* This is for board files and others not using DT */
>  	if (is_fwnode_irqchip(fwspec->fwnode)) {
>  		int ret;
> @@ -1758,7 +1762,8 @@ static const struct irq_domain_ops gpiochip_domain_ops = {
>  	.map	= gpiochip_irq_map,
>  	.unmap	= gpiochip_irq_unmap,
>  	/* Virtually all GPIO irqchips are twocell:ed */
> -	.xlate	= irq_domain_xlate_twocell,
> +	/* FIXME: force switch to three cells */
> +	.xlate	= irq_domain_xlate_threecell,
>  };
>  
>  static struct irq_domain *gpiochip_simple_create_domain(struct gpio_chip *gc)
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index e432b6a12a32..69a9540ec253 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -568,10 +568,18 @@ int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
>  int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
>  			const u32 *intspec, unsigned int intsize,
>  			irq_hw_number_t *out_hwirq, unsigned int *out_type);
> +int irq_domain_xlate_threecell(struct irq_domain *d, struct device_node *ctrlr,
> +			const u32 *intspec, unsigned int intsize,
> +			irq_hw_number_t *out_hwirq, unsigned int *out_type);
>  int irq_domain_xlate_onetwocell(struct irq_domain *d, struct device_node *ctrlr,
>  			const u32 *intspec, unsigned int intsize,
>  			irq_hw_number_t *out_hwirq, unsigned int *out_type);
>  
> +int irq_domain_translate_threecell(struct irq_domain *d,
> +				 struct irq_fwspec *fwspec,
> +				 unsigned long *out_hwirq,
> +				 unsigned int *out_type);
> +
>  int irq_domain_translate_twocell(struct irq_domain *d,
>  				 struct irq_fwspec *fwspec,
>  				 unsigned long *out_hwirq,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index ec6d8e72d980..995e5e0ec2db 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1132,6 +1132,17 @@ int irq_domain_xlate_twocell(struct irq_domain *d, struct device_node *ctrlr,
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_xlate_twocell);
>  
> +int irq_domain_xlate_threecell(struct irq_domain *d, struct device_node *ctrlr,
> +			const u32 *intspec, unsigned int intsize,
> +			irq_hw_number_t *out_hwirq, unsigned int *out_type)
> +{
> +	struct irq_fwspec fwspec;
> +
> +	of_phandle_args_to_fwspec(ctrlr, intspec, intsize, &fwspec);
> +	return irq_domain_translate_threecell(d, &fwspec, out_hwirq, out_type);
> +}
> +EXPORT_SYMBOL_GPL(irq_domain_xlate_threecell);
> +
>  /**
>   * irq_domain_xlate_onetwocell() - Generic xlate for one or two cell bindings
>   * @d:		Interrupt domain involved in the translation
> @@ -1216,6 +1227,19 @@ int irq_domain_translate_twocell(struct irq_domain *d,
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_translate_twocell);
>  
> +int irq_domain_translate_threecell(struct irq_domain *d,
> +				 struct irq_fwspec *fwspec,
> +				 unsigned long *out_hwirq,
> +				 unsigned int *out_type)
> +{
> +	if (WARN_ON(fwspec->param_count < 3))
> +		return -EINVAL;
> +	*out_hwirq = fwspec->param[1];
> +	*out_type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_domain_translate_threecell);
> +
>  int irq_domain_alloc_descs(int virq, unsigned int cnt, irq_hw_number_t hwirq,
>  			   int node, const struct irq_affinity_desc *affinity)
>  {
> 

sounds we need to implement .select() or .match() in irq_domain_ops,
then find the irq_domain.. here is a prototype version 

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 9aa88c3fa485..73caba47bd2d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1758,9 +1758,25 @@ static void gpiochip_irq_unmap(struct irq_domain *d, unsigned int irq)
 	irq_set_chip_data(irq, NULL);
 }
 
+static int gpiochip_irq_select(struct irq_domain *d, struct irq_fwspec *fwspec,
+			enum irq_domain_bus_token bus_token)
+{
+	struct fwnode_handle *fwnode = fwspec->fwnode;
+	struct gpio_chip *gc = d->host_data;
+	unsigned int index = fwspec->param[0];
+
+	if (gc->of_gpio_n_cells == 3 && gc->of_node_instance_match)
+		return gc->of_node_instance_match(gc, index);
+
+	return ((fwnode != NULL) && (d->fwnode == fwnode) &&
+		((bus_token == DOMAIN_BUS_ANY) ||
+		(d->bus_token == bus_token)));
+}
+
 static const struct irq_domain_ops gpiochip_domain_ops = {
 	.map	= gpiochip_irq_map,
 	.unmap	= gpiochip_irq_unmap,
+	.select	= gpiochip_irq_select,
 	/* Virtually all GPIO irqchips are twocell:ed */
 	/* FIXME: force switch to three cells */
 	.xlate	= irq_domain_xlate_threecell,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 995e5e0ec2db..c4d18267e86e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -553,7 +553,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
 	 */
 	mutex_lock(&irq_domain_mutex);
 	list_for_each_entry(h, &irq_domain_list, link) {
-		if (h->ops->select && bus_token != DOMAIN_BUS_ANY)
+		if (h->ops->select /* && bus_token != DOMAIN_BUS_ANY */)
 			rc = h->ops->select(h, fwspec, bus_token);
 		else if (h->ops->match)
 			rc = h->ops->match(h, to_of_node(fwnode), bus_token);

Thomas Gleixner Feb. 26, 2025, 3:45 p.m. UTC | #5

On Wed, Feb 26 2025 at 13:56, Yixun Lan wrote:
> sounds we need to implement .select() or .match() in irq_domain_ops,
> then find the irq_domain.. here is a prototype version 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 995e5e0ec2db..c4d18267e86e 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -553,7 +553,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
>  	 */
>  	mutex_lock(&irq_domain_mutex);
>  	list_for_each_entry(h, &irq_domain_list, link) {
> -		if (h->ops->select && bus_token != DOMAIN_BUS_ANY)
> +		if (h->ops->select /* && bus_token != DOMAIN_BUS_ANY */)

This breaks existing usage and reintroduces the regression, which was
fixed with the commit which added the bus token check....

Thanks,

        tglx

Yixun Lan Feb. 27, 2025, 2:12 a.m. UTC | #6

Hi Thomas Gleixner:

On 16:45 Wed 26 Feb     , Thomas Gleixner wrote:
> On Wed, Feb 26 2025 at 13:56, Yixun Lan wrote:
> > sounds we need to implement .select() or .match() in irq_domain_ops,
> > then find the irq_domain.. here is a prototype version 
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > index 995e5e0ec2db..c4d18267e86e 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -553,7 +553,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
> >  	 */
> >  	mutex_lock(&irq_domain_mutex);
> >  	list_for_each_entry(h, &irq_domain_list, link) {
> > -		if (h->ops->select && bus_token != DOMAIN_BUS_ANY)
> > +		if (h->ops->select /* && bus_token != DOMAIN_BUS_ANY */)
> 
> This breaks existing usage and reintroduces the regression, which was
> fixed with the commit which added the bus token check....
> 
right, I shouldn't change it.

would setting a bus token explicitly for spacemit gpio be a good idea?
in drivers/gpio/gpio-spacemit-k1.c, something like:

irq_domain_update_bus_token(girq->domain, DOMAIN_BUS_WIRED);