diff mbox series

[v1,4/4] gpiolib: Reuse device's fwnode to create IRQ domain

Message ID 20210302153451.50593-4-andriy.shevchenko@linux.intel.com
State Superseded
Headers show
Series [v1,1/4] gpiolib: Unify the checks on fwnode type | expand

Commit Message

Andy Shevchenko March 2, 2021, 3:34 p.m. UTC
When IRQ domain is created for an ACPI case, the name of it becomes unknown-%d
since for now it utilizes of_node member only and doesn't consider fwnode case.
Convert IRQ domain creation code to utilize fwnode instead.

Before/After the change on Intel Galileo Gen 2 with two GPIO (IRQ) controllers:

  unknown-1	==>	\_SB.PCI0.GIP0.GPO
  unknown-2	==>	\_SB.NIO3

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

Comments

Linus Walleij March 3, 2021, 9:22 a.m. UTC | #1
On Tue, Mar 2, 2021 at 4:35 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> When IRQ domain is created for an ACPI case, the name of it becomes unknown-%d
> since for now it utilizes of_node member only and doesn't consider fwnode case.
> Convert IRQ domain creation code to utilize fwnode instead.
>
> Before/After the change on Intel Galileo Gen 2 with two GPIO (IRQ) controllers:
>
>   unknown-1     ==>     \_SB.PCI0.GIP0.GPO
>   unknown-2     ==>     \_SB.NIO3
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This first part seems to do what you want,

> @@ -1457,9 +1457,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>                                 struct lock_class_key *lock_key,
>                                 struct lock_class_key *request_key)
>  {
> +       struct fwnode_handle *fwnode = dev_fwnode(&gc->gpiodev->dev);

(...)

But this:

> @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>                         return ret;
>         } else {
>                 /* Some drivers provide custom irqdomain ops */
> -               if (gc->irq.domain_ops)
> -                       ops = gc->irq.domain_ops;
> -
> -               if (!ops)
> -                       ops = &gpiochip_domain_ops;
> -               gc->irq.domain = irq_domain_add_simple(np,
> -                       gc->ngpio,
> -                       gc->irq.first,
> -                       ops, gc);
> +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> +               if (gc->irq.first)
> +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> +                                                                 gc->irq.first, 0,
> +                                                                 ops, gc);
> +               else
> +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> +                                                                 ops, gc);

This looks like a refactoring and reimplementation of irq_domain_add_simple()?

Why, and should it rather be a separate patch?

Yours,
Linus Walleij
Andy Shevchenko March 3, 2021, 9:35 a.m. UTC | #2
On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:
> On Tue, Mar 2, 2021 at 4:35 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > When IRQ domain is created for an ACPI case, the name of it becomes unknown-%d
> > since for now it utilizes of_node member only and doesn't consider fwnode case.
> > Convert IRQ domain creation code to utilize fwnode instead.
> >
> > Before/After the change on Intel Galileo Gen 2 with two GPIO (IRQ) controllers:
> >
> >   unknown-1     ==>     \_SB.PCI0.GIP0.GPO
> >   unknown-2     ==>     \_SB.NIO3
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This first part seems to do what you want,

...

> But this:
> 
> > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
> >                         return ret;
> >         } else {
> >                 /* Some drivers provide custom irqdomain ops */
> > -               if (gc->irq.domain_ops)
> > -                       ops = gc->irq.domain_ops;
> > -
> > -               if (!ops)
> > -                       ops = &gpiochip_domain_ops;
> > -               gc->irq.domain = irq_domain_add_simple(np,
> > -                       gc->ngpio,
> > -                       gc->irq.first,
> > -                       ops, gc);
> > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> > +               if (gc->irq.first)
> > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> > +                                                                 gc->irq.first, 0,
> > +                                                                 ops, gc);
> > +               else
> > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> > +                                                                 ops, gc);
> 
> This looks like a refactoring and reimplementation of irq_domain_add_simple()?

If you named it as irq_domain_create_simple(), then yes, but the problem is
that we don't have irq_domain_create_simple() API right now.

> Why, and should it rather be a separate patch?

Nope.
Linus Walleij March 4, 2021, 8:06 a.m. UTC | #3
On Wed, Mar 3, 2021 at 10:35 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:

> > But this:
> >
> > > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
> > >                         return ret;
> > >         } else {
> > >                 /* Some drivers provide custom irqdomain ops */
> > > -               if (gc->irq.domain_ops)
> > > -                       ops = gc->irq.domain_ops;
> > > -
> > > -               if (!ops)
> > > -                       ops = &gpiochip_domain_ops;
> > > -               gc->irq.domain = irq_domain_add_simple(np,
> > > -                       gc->ngpio,
> > > -                       gc->irq.first,
> > > -                       ops, gc);
> > > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> > > +               if (gc->irq.first)
> > > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> > > +                                                                 gc->irq.first, 0,
> > > +                                                                 ops, gc);
> > > +               else
> > > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> > > +                                                                 ops, gc);
> >
> > This looks like a refactoring and reimplementation of irq_domain_add_simple()?
>
> If you named it as irq_domain_create_simple(), then yes, but the problem is
> that we don't have irq_domain_create_simple() API right now.
>
> > Why, and should it rather be a separate patch?
>
> Nope.

OK I looked closer at irq_domain_add_simple(), and what it does different
is to call irq_alloc_descs() for all lines if using sparse IRQs and then
associate them. irq_domain_create_linear|legacy() does not allocate IRQ
descriptors because it assumes something like DT or ACPI will do that
on-demand when drivers request IRQs.

This may be dangerous because some old platforms do not resolve IRQs
at runtime and you will get NULL pointer exceptions.

We then need to make sure all callers do what is done in e.g.
drivers/gpio/gpio-omap.c in the #ifdef CONFIG_ARCH_OMAP1 clause:
they need to be augmented to call irq_alloc_descs() explicitly,
and I don't think all of them do it as nicely for us as OMAP1.

I might be overly cautious though, however that is why this code
uses irq_domain_add_simple(), came in commit
commit 2854d167cc545d0642277bf8b77f972a91146fc6

Yours,
Linus Walleij
Andy Shevchenko March 4, 2021, 12:23 p.m. UTC | #4
On Thu, Mar 04, 2021 at 09:06:08AM +0100, Linus Walleij wrote:
> On Wed, Mar 3, 2021 at 10:35 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:
> 
> > > But this:
> > >
> > > > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
> > > >                         return ret;
> > > >         } else {
> > > >                 /* Some drivers provide custom irqdomain ops */
> > > > -               if (gc->irq.domain_ops)
> > > > -                       ops = gc->irq.domain_ops;
> > > > -
> > > > -               if (!ops)
> > > > -                       ops = &gpiochip_domain_ops;
> > > > -               gc->irq.domain = irq_domain_add_simple(np,
> > > > -                       gc->ngpio,
> > > > -                       gc->irq.first,
> > > > -                       ops, gc);
> > > > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> > > > +               if (gc->irq.first)
> > > > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> > > > +                                                                 gc->irq.first, 0,
> > > > +                                                                 ops, gc);
> > > > +               else
> > > > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> > > > +                                                                 ops, gc);
> > >
> > > This looks like a refactoring and reimplementation of irq_domain_add_simple()?
> >
> > If you named it as irq_domain_create_simple(), then yes, but the problem is
> > that we don't have irq_domain_create_simple() API right now.
> >
> > > Why, and should it rather be a separate patch?
> >
> > Nope.
> 
> OK I looked closer at irq_domain_add_simple(), and what it does different
> is to call irq_alloc_descs() for all lines if using sparse IRQs and then
> associate them. irq_domain_create_linear|legacy() does not allocate IRQ
> descriptors because it assumes something like DT or ACPI will do that
> on-demand when drivers request IRQs.
> 
> This may be dangerous because some old platforms do not resolve IRQs
> at runtime and you will get NULL pointer exceptions.
> 
> We then need to make sure all callers do what is done in e.g.
> drivers/gpio/gpio-omap.c in the #ifdef CONFIG_ARCH_OMAP1 clause:
> they need to be augmented to call irq_alloc_descs() explicitly,
> and I don't think all of them do it as nicely for us as OMAP1.
> 
> I might be overly cautious though, however that is why this code
> uses irq_domain_add_simple(), came in commit
> commit 2854d167cc545d0642277bf8b77f972a91146fc6

Ah, thanks! I was puzzled how and why the approach above had been extended like
now. This explains it. Okay, I will introduce irq_domain_create_simple().
Rafael J. Wysocki March 4, 2021, 1:41 p.m. UTC | #5
On Thu, Mar 4, 2021 at 1:25 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>

> On Thu, Mar 04, 2021 at 09:06:08AM +0100, Linus Walleij wrote:

> > On Wed, Mar 3, 2021 at 10:35 AM Andy Shevchenko

> > <andriy.shevchenko@linux.intel.com> wrote:

> > > On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:

> >

> > > > But this:

> > > >

> > > > > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,

> > > > >                         return ret;

> > > > >         } else {

> > > > >                 /* Some drivers provide custom irqdomain ops */

> > > > > -               if (gc->irq.domain_ops)

> > > > > -                       ops = gc->irq.domain_ops;

> > > > > -

> > > > > -               if (!ops)

> > > > > -                       ops = &gpiochip_domain_ops;

> > > > > -               gc->irq.domain = irq_domain_add_simple(np,

> > > > > -                       gc->ngpio,

> > > > > -                       gc->irq.first,

> > > > > -                       ops, gc);

> > > > > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;

> > > > > +               if (gc->irq.first)

> > > > > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,

> > > > > +                                                                 gc->irq.first, 0,

> > > > > +                                                                 ops, gc);

> > > > > +               else

> > > > > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,

> > > > > +                                                                 ops, gc);

> > > >

> > > > This looks like a refactoring and reimplementation of irq_domain_add_simple()?

> > >

> > > If you named it as irq_domain_create_simple(), then yes, but the problem is

> > > that we don't have irq_domain_create_simple() API right now.

> > >

> > > > Why, and should it rather be a separate patch?

> > >

> > > Nope.

> >

> > OK I looked closer at irq_domain_add_simple(), and what it does different

> > is to call irq_alloc_descs() for all lines if using sparse IRQs and then

> > associate them. irq_domain_create_linear|legacy() does not allocate IRQ

> > descriptors because it assumes something like DT or ACPI will do that

> > on-demand when drivers request IRQs.

> >

> > This may be dangerous because some old platforms do not resolve IRQs

> > at runtime and you will get NULL pointer exceptions.

> >

> > We then need to make sure all callers do what is done in e.g.

> > drivers/gpio/gpio-omap.c in the #ifdef CONFIG_ARCH_OMAP1 clause:

> > they need to be augmented to call irq_alloc_descs() explicitly,

> > and I don't think all of them do it as nicely for us as OMAP1.

> >

> > I might be overly cautious though, however that is why this code

> > uses irq_domain_add_simple(), came in commit

> > commit 2854d167cc545d0642277bf8b77f972a91146fc6

>

> Ah, thanks! I was puzzled how and why the approach above had been extended like

> now. This explains it. Okay, I will introduce irq_domain_create_simple().


OK

So please resend the series with that done and with the R-bys from
Linus added.  I'll apply it from Patchwork.

Thanks!
Andy Shevchenko March 4, 2021, 3:40 p.m. UTC | #6
On Thu, Mar 04, 2021 at 02:41:24PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 4, 2021 at 1:25 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Mar 04, 2021 at 09:06:08AM +0100, Linus Walleij wrote:
> > > On Wed, Mar 3, 2021 at 10:35 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:
> > >
> > > > > But this:
> > > > >
> > > > > > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
> > > > > >                         return ret;
> > > > > >         } else {
> > > > > >                 /* Some drivers provide custom irqdomain ops */
> > > > > > -               if (gc->irq.domain_ops)
> > > > > > -                       ops = gc->irq.domain_ops;
> > > > > > -
> > > > > > -               if (!ops)
> > > > > > -                       ops = &gpiochip_domain_ops;
> > > > > > -               gc->irq.domain = irq_domain_add_simple(np,
> > > > > > -                       gc->ngpio,
> > > > > > -                       gc->irq.first,
> > > > > > -                       ops, gc);
> > > > > > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> > > > > > +               if (gc->irq.first)
> > > > > > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> > > > > > +                                                                 gc->irq.first, 0,
> > > > > > +                                                                 ops, gc);
> > > > > > +               else
> > > > > > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> > > > > > +                                                                 ops, gc);
> > > > >
> > > > > This looks like a refactoring and reimplementation of irq_domain_add_simple()?
> > > >
> > > > If you named it as irq_domain_create_simple(), then yes, but the problem is
> > > > that we don't have irq_domain_create_simple() API right now.
> > > >
> > > > > Why, and should it rather be a separate patch?
> > > >
> > > > Nope.
> > >
> > > OK I looked closer at irq_domain_add_simple(), and what it does different
> > > is to call irq_alloc_descs() for all lines if using sparse IRQs and then
> > > associate them. irq_domain_create_linear|legacy() does not allocate IRQ
> > > descriptors because it assumes something like DT or ACPI will do that
> > > on-demand when drivers request IRQs.
> > >
> > > This may be dangerous because some old platforms do not resolve IRQs
> > > at runtime and you will get NULL pointer exceptions.
> > >
> > > We then need to make sure all callers do what is done in e.g.
> > > drivers/gpio/gpio-omap.c in the #ifdef CONFIG_ARCH_OMAP1 clause:
> > > they need to be augmented to call irq_alloc_descs() explicitly,
> > > and I don't think all of them do it as nicely for us as OMAP1.
> > >
> > > I might be overly cautious though, however that is why this code
> > > uses irq_domain_add_simple(), came in commit
> > > commit 2854d167cc545d0642277bf8b77f972a91146fc6
> >
> > Ah, thanks! I was puzzled how and why the approach above had been extended like
> > now. This explains it. Okay, I will introduce irq_domain_create_simple().
> 
> OK
> 
> So please resend the series with that done and with the R-bys from
> Linus added.  I'll apply it from Patchwork.

Done!

https://lore.kernel.org/linux-gpio/20210304150215.80652-1-andriy.shevchenko@linux.intel.com/T/#u

P.S. you seems haven't switched yet to b4 :-)
Rafael J. Wysocki March 4, 2021, 3:54 p.m. UTC | #7
On Thu, Mar 4, 2021 at 4:40 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Mar 04, 2021 at 02:41:24PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Mar 4, 2021 at 1:25 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Thu, Mar 04, 2021 at 09:06:08AM +0100, Linus Walleij wrote:
> > > > On Wed, Mar 3, 2021 at 10:35 AM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Wed, Mar 03, 2021 at 10:22:02AM +0100, Linus Walleij wrote:
> > > >
> > > > > > But this:
> > > > > >
> > > > > > > @@ -1504,15 +1497,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
> > > > > > >                         return ret;
> > > > > > >         } else {
> > > > > > >                 /* Some drivers provide custom irqdomain ops */
> > > > > > > -               if (gc->irq.domain_ops)
> > > > > > > -                       ops = gc->irq.domain_ops;
> > > > > > > -
> > > > > > > -               if (!ops)
> > > > > > > -                       ops = &gpiochip_domain_ops;
> > > > > > > -               gc->irq.domain = irq_domain_add_simple(np,
> > > > > > > -                       gc->ngpio,
> > > > > > > -                       gc->irq.first,
> > > > > > > -                       ops, gc);
> > > > > > > +               ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
> > > > > > > +               if (gc->irq.first)
> > > > > > > +                       gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
> > > > > > > +                                                                 gc->irq.first, 0,
> > > > > > > +                                                                 ops, gc);
> > > > > > > +               else
> > > > > > > +                       gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
> > > > > > > +                                                                 ops, gc);
> > > > > >
> > > > > > This looks like a refactoring and reimplementation of irq_domain_add_simple()?
> > > > >
> > > > > If you named it as irq_domain_create_simple(), then yes, but the problem is
> > > > > that we don't have irq_domain_create_simple() API right now.
> > > > >
> > > > > > Why, and should it rather be a separate patch?
> > > > >
> > > > > Nope.
> > > >
> > > > OK I looked closer at irq_domain_add_simple(), and what it does different
> > > > is to call irq_alloc_descs() for all lines if using sparse IRQs and then
> > > > associate them. irq_domain_create_linear|legacy() does not allocate IRQ
> > > > descriptors because it assumes something like DT or ACPI will do that
> > > > on-demand when drivers request IRQs.
> > > >
> > > > This may be dangerous because some old platforms do not resolve IRQs
> > > > at runtime and you will get NULL pointer exceptions.
> > > >
> > > > We then need to make sure all callers do what is done in e.g.
> > > > drivers/gpio/gpio-omap.c in the #ifdef CONFIG_ARCH_OMAP1 clause:
> > > > they need to be augmented to call irq_alloc_descs() explicitly,
> > > > and I don't think all of them do it as nicely for us as OMAP1.
> > > >
> > > > I might be overly cautious though, however that is why this code
> > > > uses irq_domain_add_simple(), came in commit
> > > > commit 2854d167cc545d0642277bf8b77f972a91146fc6
> > >
> > > Ah, thanks! I was puzzled how and why the approach above had been extended like
> > > now. This explains it. Okay, I will introduce irq_domain_create_simple().
> >
> > OK
> >
> > So please resend the series with that done and with the R-bys from
> > Linus added.  I'll apply it from Patchwork.
>
> Done!

Thanks.

> https://lore.kernel.org/linux-gpio/20210304150215.80652-1-andriy.shevchenko@linux.intel.com/T/#u
>
> P.S. you seems haven't switched yet to b4 :-)

Nope.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6827736ba05c..54cfea4e4a03 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1457,9 +1457,9 @@  static int gpiochip_add_irqchip(struct gpio_chip *gc,
 				struct lock_class_key *lock_key,
 				struct lock_class_key *request_key)
 {
+	struct fwnode_handle *fwnode = dev_fwnode(&gc->gpiodev->dev);
 	struct irq_chip *irqchip = gc->irq.chip;
-	const struct irq_domain_ops *ops = NULL;
-	struct device_node *np;
+	const struct irq_domain_ops *ops;
 	unsigned int type;
 	unsigned int i;
 
@@ -1471,7 +1471,6 @@  static int gpiochip_add_irqchip(struct gpio_chip *gc,
 		return -EINVAL;
 	}
 
-	np = gc->gpiodev->dev.of_node;
 	type = gc->irq.default_type;
 
 	/*
@@ -1479,16 +1478,10 @@  static int gpiochip_add_irqchip(struct gpio_chip *gc,
 	 * used to configure the interrupts, as you may end up with
 	 * conflicting triggers. Tell the user, and reset to NONE.
 	 */
-	if (WARN(np && type != IRQ_TYPE_NONE,
-		 "%s: Ignoring %u default trigger\n", np->full_name, type))
+	if (WARN(fwnode && type != IRQ_TYPE_NONE,
+		 "%pfw: Ignoring %u default trigger\n", fwnode, type))
 		type = IRQ_TYPE_NONE;
 
-	if (has_acpi_companion(gc->parent) && type != IRQ_TYPE_NONE) {
-		acpi_handle_warn(ACPI_HANDLE(gc->parent),
-				 "Ignoring %u default trigger\n", type);
-		type = IRQ_TYPE_NONE;
-	}
-
 	if (gc->to_irq)
 		chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
 
@@ -1504,15 +1497,14 @@  static int gpiochip_add_irqchip(struct gpio_chip *gc,
 			return ret;
 	} else {
 		/* Some drivers provide custom irqdomain ops */
-		if (gc->irq.domain_ops)
-			ops = gc->irq.domain_ops;
-
-		if (!ops)
-			ops = &gpiochip_domain_ops;
-		gc->irq.domain = irq_domain_add_simple(np,
-			gc->ngpio,
-			gc->irq.first,
-			ops, gc);
+		ops = gc->irq.domain_ops ?: &gpiochip_domain_ops;
+		if (gc->irq.first)
+			gc->irq.domain = irq_domain_create_legacy(fwnode, gc->ngpio,
+								  gc->irq.first, 0,
+								  ops, gc);
+		else
+			gc->irq.domain = irq_domain_create_linear(fwnode, gc->ngpio,
+								  ops, gc);
 		if (!gc->irq.domain)
 			return -EINVAL;
 	}