All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yixun Lan <dlan@gentoo.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Conor Dooley <conor@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Alex Elder <elder@riscstar.com>, Yangyu Chen <cyy@cyyself.name>,
	Jisheng Zhang <jszhang@kernel.org>,
	Jesse Taube <mr.bossman075@gmail.com>,
	Inochi Amaoto <inochiama@outlook.com>,
	Icenowy Zheng <uwu@icenowy.me>,
	Meng Zhang <zhangmeng.kevin@linux.spacemit.com>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	spacemit@lists.linux.dev
Subject: Re: [PATCH v7 0/4] riscv: spacemit: add gpio support for K1 SoC
Date: Wed, 26 Feb 2025 11:59:57 +0000	[thread overview]
Message-ID: <20250226115957-GYA45508@gentoo> (raw)
In-Reply-To: <CACRpkdY7nzzu3-+FwpSYqmX+O559LoXHiqcvP2OxkhX+9f-3wg@mail.gmail.com>

Hi Linus Walleij:

On 11:24 Wed 26 Feb     , Linus Walleij wrote:
> On Wed, Feb 26, 2025 at 2:01 AM Yixun Lan <dlan@gentoo.org> 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 (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

WARNING: multiple messages have this Message-ID (diff)
From: Yixun Lan <dlan@gentoo.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Rob Herring <robh@kernel.org>, Conor Dooley <conor+dt@kernel.org>,
	Meng Zhang <zhangmeng.kevin@linux.spacemit.com>,
	Alex Elder <elder@riscstar.com>,
	linux-gpio@vger.kernel.org, Bartosz Golaszewski <brgl@bgdev.pl>,
	linux-kernel@vger.kernel.org, Conor Dooley <conor@kernel.org>,
	Yangyu Chen <cyy@cyyself.name>,
	devicetree@vger.kernel.org, Palmer Dabbelt <palmer@dabbelt.com>,
	Jesse Taube <mr.bossman075@gmail.com>,
	Jisheng Zhang <jszhang@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Inochi Amaoto <inochiama@outlook.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	spacemit@lists.linux.dev, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v7 0/4] riscv: spacemit: add gpio support for K1 SoC
Date: Wed, 26 Feb 2025 11:59:57 +0000	[thread overview]
Message-ID: <20250226115957-GYA45508@gentoo> (raw)
In-Reply-To: <CACRpkdY7nzzu3-+FwpSYqmX+O559LoXHiqcvP2OxkhX+9f-3wg@mail.gmail.com>

Hi Linus Walleij:

On 11:24 Wed 26 Feb     , Linus Walleij wrote:
> On Wed, Feb 26, 2025 at 2:01 AM Yixun Lan <dlan@gentoo.org> 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 (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

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

  reply	other threads:[~2025-02-26 12:00 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-26  0:41 [PATCH v7 0/4] riscv: spacemit: add gpio support for K1 SoC Yixun Lan
2025-02-26  0:41 ` Yixun Lan
2025-02-26  0:41 ` [PATCH v7 1/4] dt-bindings: gpio: spacemit: add " Yixun Lan
2025-02-26  0:41   ` Yixun Lan
2025-02-26 15:55   ` Rob Herring (Arm)
2025-02-26 15:55     ` Rob Herring (Arm)
2025-02-26  0:41 ` [PATCH v7 2/4] " Yixun Lan
2025-02-26  0:41   ` Yixun Lan
2025-03-25 10:00   ` Yixun Lan
2025-03-25 10:00     ` Yixun Lan
2025-02-26  0:41 ` [PATCH v7 3/4] riscv: dts: spacemit: add gpio " Yixun Lan
2025-02-26  0:41   ` Yixun Lan
2025-02-26  0:41 ` [PATCH v7 4/4] riscv: dts: spacemit: add gpio LED for system heartbeat Yixun Lan
2025-02-26  0:41   ` Yixun Lan
2025-02-26  1:01 ` [PATCH v7 0/4] riscv: spacemit: add gpio support for K1 SoC Yixun Lan
2025-02-26  1:01   ` Yixun Lan
2025-02-26 10:24   ` Linus Walleij
2025-02-26 10:24     ` Linus Walleij
2025-02-26 11:59     ` Yixun Lan [this message]
2025-02-26 11:59       ` Yixun Lan
2025-02-26 13:56       ` Yixun Lan
2025-02-26 13:56         ` Yixun Lan
2025-02-26 15:45         ` Thomas Gleixner
2025-02-26 15:45           ` Thomas Gleixner
2025-02-27  2:12           ` Yixun Lan
2025-02-27  2:12             ` Yixun Lan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250226115957-GYA45508@gentoo \
    --to=dlan@gentoo.org \
    --cc=brgl@bgdev.pl \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=cyy@cyyself.name \
    --cc=devicetree@vger.kernel.org \
    --cc=elder@riscstar.com \
    --cc=inochiama@outlook.com \
    --cc=jszhang@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mr.bossman075@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh@kernel.org \
    --cc=spacemit@lists.linux.dev \
    --cc=uwu@icenowy.me \
    --cc=zhangmeng.kevin@linux.spacemit.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.