Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Hi,
I have a few comments, mostly on the DT binding and parsing.
diff --git a/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
new file mode 100644
index 000..5d465cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
@@ -0,0 +1,39 @@
+* IRQ CROSSBAR
+
+Some socs have a large number of interrupts requests to service
+the needs of its many peripherals and subsystems. All of the
+interrupt lines from the subsystems are not needed at the same
+time, so they have to be muxed to the irq-controller appropriately.
+In such places a interrupt controllers are preceded by an CROSSBAR
+that provides flexibility in muxing the device requests to the controller
+inputs.
+
+Required properties:
+- compatible : Should be irq-crossbar
Missing vendor prefix, this should be something like ti,irq-crossbar.
Does this have a more specific name than CROSSBAR that can be used to
qualify it?
+- interrupt-parent: phandle to crossbar's interrupt parent.
+- interrupt-controller: Identifies the node as an interrupt controller.
+- interrupt-cells: Should be the same value as the interrupt parent.
That doesn't make sense. The crossbar driver is necessarily interpreting
these cells in a way the parent won't (as it supports more interrupts).
What are the meaning of these cells?
+- reg: Base address and the size of the crossbar registers.
+- max-crossbar-lines: Total number of input lines of the crossbar.
+- max-irqs: Total number of irqs available at the interrupt controller.
Is this the maximum number of interrupts targeting the parent interrupt
controller? Starting at what number, ending at what number? Can this
have gaps?
Is this a shortcut so in the GIC case you don't have to describe up to
160 interrupts? I can see why you don't want to, but there's a big loss
of generality here...
+- reg-size: size of the crossbar registers.
As in the size of all the registers (the size component of reg)?
Or is this the size of each individual register? Does that apply to all
registers or only a subset of them?
What units are these in, bytes?
What are valid sizes?
Is this really that configurable?
+- irqs-reserved: List of the reserved irq lines that are not muxed using
+crossbar. These interrupt lines are reserved in the soc,
+so crossbar bar driver should not consider them as free
+lines.
Are these reserved inputs lines, or outputs to the parent interrupt
controller?
What is the format of each entry in this list?
The example seems to be a different format to the parent interrupt
controller (which per your binding also defined the crossbar's interrupt
format). While 0 1 2 is a valid interrupt per the GIC binding (SPI 0
edge-triggered both ways), 3 5 6, 131 132 139, and 140 . . are
not.
+
+Examples:
+ crossbar_mpu: @4a02 {
+ compatible = irq-crossbar;
+ interrupt-parent = gic;
+ interrupt-controller;
+ #interrupt-cells = 3;
+ reg = 0x4a002a48 0x130;
+ max-crossbar-lines = 512;
+ max-irqs = 160;
+ reg-size = 2;
+ irqs-reserved = 0 1 2 3 5 6 131 132 139 140;
+ #address-cells = 1;
+ #size-cells = 1;
Why are there #address-cells and #size cells? This has no children, and
this affects any interrupt-map property (as the parent unit address now
must be a single cell, that isn't going to be used).
[...]
+static int crossbar_set_affinity(struct irq_data *d,
+const struct cpumask *mask_val,
+bool force)
+{
+ struct irq_chip *chip;
+ struct irq_data *data;
+ int ret = 0;
+
+ crossbar_to_irq_chip_data(d-hwirq, chip, data);
+
+ if (chip-irq_set_affinity)
+ ret = chip-irq_set_affinity(data, mask_val, force);
+
+ return ret;
So if our parent chip can't set affinity, we pretend it can?
irq_set_affinity in kernel/irq/manage.c returns -EINVAL if an irqchip
doesn't have irq_set_affinity.
+/*
+ * Request and free are already called in atomic contexts
+ */
+unsigned int crossbar_request_irq(struct irq_data *d)
+{
+ int cb_no = d-hwirq;
+ int virq = allocate_free_irq(cb_no);
+ void *irq = cb-crossbar_map[cb_no].hwirq;
+ int err;
+
+ err = request_threaded_irq(virq, crossbar_irq, NULL,
+ 0, CROSSBAR, irq);
+ if (err)
+ pr_err(\n request_irq failed for crossbar %d, cb_no);
Why does the print begin with a newline rather than ending with one?
+
+ return 0;
+}
[...]
+static int crossbar_domain_xlate(struct irq_domain *d,
+struct
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Hi Mark,
On Friday 20 September 2013 02:28 PM, Mark Rutland wrote:
Hi,
I have a few comments, mostly on the DT binding and parsing.
Thanks for the review. The idea of seeing the crossbar as a new IRQCHIP
itself did not go and the latest direction on this was to handle it inside the
GIC.
http://www.spinics.net/lists/linux-omap/msg97085.html
I am working on that now.
I would have agreed with most of the comments below, otherwise.
diff --git a/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
new file mode 100644
index 000..5d465cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
@@ -0,0 +1,39 @@
+* IRQ CROSSBAR
+
+Some socs have a large number of interrupts requests to service
+the needs of its many peripherals and subsystems. All of the
+interrupt lines from the subsystems are not needed at the same
+time, so they have to be muxed to the irq-controller appropriately.
+In such places a interrupt controllers are preceded by an CROSSBAR
+that provides flexibility in muxing the device requests to the controller
+inputs.
+
+Required properties:
+- compatible : Should be irq-crossbar
Missing vendor prefix, this should be something like ti,irq-crossbar.
Does this have a more specific name than CROSSBAR that can be used to
qualify it?
yes, ti,irq-crossbar. Not sure if it can be called as anything
generically apart from crossbar .
+- interrupt-parent: phandle to crossbar's interrupt parent.
+- interrupt-controller: Identifies the node as an interrupt controller.
+- interrupt-cells: Should be the same value as the interrupt parent.
That doesn't make sense. The crossbar driver is necessarily interpreting
these cells in a way the parent won't (as it supports more interrupts).
What are the meaning of these cells?
These properties were added so that the DT code identifies this as a
interrupt controller and map the children's irq in to this domain and
to map the free irqs allocated in this driver to its parent.
+- reg: Base address and the size of the crossbar registers.
+- max-crossbar-lines: Total number of input lines of the crossbar.
+- max-irqs: Total number of irqs available at the interrupt controller.
Is this the maximum number of interrupts targeting the parent interrupt
controller? Starting at what number, ending at what number? Can this
have gaps?
Is this a shortcut so in the GIC case you don't have to describe up to
160 interrupts? I can see why you don't want to, but there's a big loss
of generality here...
Yes, this was the maximum irqs available at the parent.
The gaps was not considered here because it was mentioned
used the below property irqs-reserved.
+- reg-size: size of the crossbar registers.
As in the size of all the registers (the size component of reg)?
Or is this the size of each individual register? Does that apply to all
registers or only a subset of them?
What units are these in, bytes?
What are valid sizes?
Is this really that configurable?
This was meant to describe the size a individual register and applied to
all. This was used to choose the API's to write. But yes some more
description could be made here.
+- irqs-reserved: List of the reserved irq lines that are not muxed using
+crossbar. These interrupt lines are reserved in the soc,
+so crossbar bar driver should not consider them as free
+lines.
Are these reserved inputs lines, or outputs to the parent interrupt
controller?
What is the format of each entry in this list?
The example seems to be a different format to the parent interrupt
controller (which per your binding also defined the crossbar's interrupt
format). While 0 1 2 is a valid interrupt per the GIC binding (SPI 0
edge-triggered both ways), 3 5 6, 131 132 139, and 140 . . are
not.
These were parent's input lines that were not muxed from crossbar
but directly connected from peripherals, so the driver should not
consider it as a free line while allocating a irq. This property was meant to
interpreted only in this driver.
+
+Examples:
+ crossbar_mpu: @4a02 {
+ compatible = irq-crossbar;
+ interrupt-parent = gic;
+ interrupt-controller;
+ #interrupt-cells = 3;
+ reg = 0x4a002a48 0x130;
+ max-crossbar-lines = 512;
+ max-irqs = 160;
+ reg-size = 2;
+ irqs-reserved = 0 1 2 3 5 6 131 132 139 140;
+ #address-cells = 1;
+ #size-cells = 1;
Why are there #address-cells and #size cells? This has no children, and
this affects any interrupt-map property (as the parent unit address now
must be a single cell, that isn't going to be used).
[...]
yes, they could have been
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Hi Thomas,
On Tuesday 17 September 2013 05:56 PM, Linus Walleij wrote:
On Fri, Sep 13, 2013 at 4:24 PM, Thomas Gleixner t...@linutronix.de wrote:
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
irq
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
This is obviously the right approach, it's exactly what .map should do
the only special thing here being that we have hardware to perform
the mapping ... bah why didn't I realize this :-(
Yours,
Linus Walleij
Thanks for the suggestion.
So as i understand this, this implies using the GIC domain itself and
add the support for dynamically routable irqs (like crossbar) with in the
GIC driver itself right ?
Regards,
Sricharan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Thomas,
On Friday 13 September 2013 10:55 AM, Santosh Shilimkar wrote:
On Friday 13 September 2013 10:24 AM, Thomas Gleixner wrote:
[...]
Before you dig into MSI, lets talk about irq domains first.
GIC implements a legacy irq domain, i.e. a linear domain of all
possible GIC interrupts with a 1:1 mapping.
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
Thoughts?
This sounds pretty good idea. We will explore above option.
Thanks Thomas.
After further looking into this, the irqdomain approach lets us
setup the map only once during the init. This is similar to
the earlier approach of cross-bar driver where at probe time
the router was setup. The whole debate started with the fact
that we shouldn't fix the irq mapping at probe and should
dynamically change the mapping based on [request/free]_irq()
to be able to maximize the use of the IP.
Since we have agreed now to move ahead with irdomain, i thought
of mentioning it here.
Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Wednesday 18 September 2013 07:22 PM, Sricharan R wrote:
Hi Thomas,
On Tuesday 17 September 2013 05:56 PM, Linus Walleij wrote:
On Fri, Sep 13, 2013 at 4:24 PM, Thomas Gleixner t...@linutronix.de wrote:
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
irq
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
This is obviously the right approach, it's exactly what .map should do
the only special thing here being that we have hardware to perform
the mapping ... bah why didn't I realize this :-(
Yours,
Linus Walleij
Thanks for the suggestion.
So as i understand this, this implies using the GIC domain itself and
add the support for dynamically routable irqs (like crossbar) with in the
GIC driver itself right ?
Please ignore this. So the question was more of how to implement the
call outs in the case of routable irqs from map/ unmap callbacks.
I will look more here and come back.
Regards,
Sricharan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Wed, 18 Sep 2013, Sricharan R wrote:
On Wednesday 18 September 2013 07:22 PM, Sricharan R wrote:
Hi Thomas,
On Tuesday 17 September 2013 05:56 PM, Linus Walleij wrote:
On Fri, Sep 13, 2013 at 4:24 PM, Thomas Gleixner t...@linutronix.de
wrote:
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
irq
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
This is obviously the right approach, it's exactly what .map should do
the only special thing here being that we have hardware to perform
the mapping ... bah why didn't I realize this :-(
Yours,
Linus Walleij
Thanks for the suggestion.
So as i understand this, this implies using the GIC domain itself and
add the support for dynamically routable irqs (like crossbar) with in the
GIC driver itself right ?
Please ignore this. So the question was more of how to implement the
call outs in the case of routable irqs from map/ unmap callbacks.
If you look closely at what I suggested, you'll notice that I added a
separate domain in the routed case. Go figure ...
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Wed, 18 Sep 2013, Santosh Shilimkar wrote:
On Friday 13 September 2013 10:55 AM, Santosh Shilimkar wrote:
On Friday 13 September 2013 10:24 AM, Thomas Gleixner wrote:
[...]
Before you dig into MSI, lets talk about irq domains first.
GIC implements a legacy irq domain, i.e. a linear domain of all
possible GIC interrupts with a 1:1 mapping.
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
Thoughts?
This sounds pretty good idea. We will explore above option.
Thanks Thomas.
After further looking into this, the irqdomain approach lets us
setup the map only once during the init. This is similar to
the earlier approach of cross-bar driver where at probe time
the router was setup. The whole debate started with the fact
that we shouldn't fix the irq mapping at probe and should
dynamically change the mapping based on [request/free]_irq()
to be able to maximize the use of the IP.
Well, that's what irq_of_parse_and_map() resp. irq_create_mapping and
irq_dispose_mapping() are for.
It requires changes to drivers, but you can't get that thing for free.
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Fri, Sep 13, 2013 at 4:24 PM, Thomas Gleixner t...@linutronix.de wrote:
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
This is obviously the right approach, it's exactly what .map should do
the only special thing here being that we have hardware to perform
the mapping ... bah why didn't I realize this :-(
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Santosh Shilimkar wrote:
On Thursday 12 September 2013 08:26 PM, Thomas Gleixner wrote:
Let me summarize:
- GIC supports up to 160 interrupts
- CROSSBAR supports up to 250 interrupts
- CROSSBAR routes up to 160 out of 250 interrupts to the GIC ones
- Drivers request a CROSSBAR interrupt number which must be mapped
to some arbitrary available GIC irq number
Correct.
So basically the CROSSBAR mechanism is pretty much the same as MSI[X]
just in a different flavour and with a different set of semantics and
limitations, i.e. poor mans MSI[X] with a new level of bogosity.
So if CROSSBAR is going to be the new fangled SoC MSI[X] long term
equivalent then you better provide some infrastructure for that and
make the drivers ready to use it. Maybe check with the PCI/MSI folks
to share some of the interfaces.
If that whole thing is another onetime HW designers wet dream, then
please go back to the limited but completely functional (Who is going
to use more than 160 peripheral interrupts) device tree model. I
really have no interest to support hardware designer brain farts.
Thanks for clear NAK for irqchip approach. I should have looped you
in the discussion where I was also suggesting against the irqchip
approach. We will try to look at MSI stuff but if its get too
complicated am going to fall-back to the initial probe based
approach to achieve the functionality.
Before you dig into MSI, lets talk about irq domains first.
GIC implements a legacy irq domain, i.e. a linear domain of all
possible GIC interrupts with a 1:1 mapping.
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
Thoughts?
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Friday 13 September 2013 10:24 AM, Thomas Gleixner wrote:
On Thu, 12 Sep 2013, Santosh Shilimkar wrote:
On Thursday 12 September 2013 08:26 PM, Thomas Gleixner wrote:
Let me summarize:
- GIC supports up to 160 interrupts
- CROSSBAR supports up to 250 interrupts
- CROSSBAR routes up to 160 out of 250 interrupts to the GIC ones
- Drivers request a CROSSBAR interrupt number which must be mapped
to some arbitrary available GIC irq number
Correct.
So basically the CROSSBAR mechanism is pretty much the same as MSI[X]
just in a different flavour and with a different set of semantics and
limitations, i.e. poor mans MSI[X] with a new level of bogosity.
So if CROSSBAR is going to be the new fangled SoC MSI[X] long term
equivalent then you better provide some infrastructure for that and
make the drivers ready to use it. Maybe check with the PCI/MSI folks
to share some of the interfaces.
If that whole thing is another onetime HW designers wet dream, then
please go back to the limited but completely functional (Who is going
to use more than 160 peripheral interrupts) device tree model. I
really have no interest to support hardware designer brain farts.
Thanks for clear NAK for irqchip approach. I should have looped you
in the discussion where I was also suggesting against the irqchip
approach. We will try to look at MSI stuff but if its get too
complicated am going to fall-back to the initial probe based
approach to achieve the functionality.
Before you dig into MSI, lets talk about irq domains first.
GIC implements a legacy irq domain, i.e. a linear domain of all
possible GIC interrupts with a 1:1 mapping.
So why can't you make use of irq domains and have the whole routing
business implemented sanely?
What's needed is in gic_init_bases():
if (of_property_read(node, routable_irqs, nr_routable_irqs) {
irq_domain_add_legacy(nr_gic_irqs);
} else {
irq_domain_add_legacy(nr_per_cpu_irqs);
irq_domain_add_linear(nr_routable_irqs);
}
Now that separate domain has an xlate function which grabs a free GIC
irq from a bitmap and returns the hardware irq number in the gic
space. The map/unmap callbacks take care of setting up / tearing down
the route in the crossbar.
Thoughts?
This sounds pretty good idea. We will explore above option.
Thanks Thomas.
Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Some socs have a large number of interrupts requests to service
the needs of its many peripherals and subsystems. All of the
interrupt lines from the subsystems are not needed at the same
time, so they have to be muxed to the irq-controller appropriately.
In such places a interrupt controllers are preceded by an CROSSBAR
that provides flexibility in muxing the device requests to the controller
inputs.
This models the crossbar IP as a cascaded irqchip controller.
The peripheral crossbar inputs are mapped on to the crossbar irq-domain.
The driver then allocates a 'free' irq line and maps that to the
actual interrupt controller's domain. So every external peripheral interrupt
is routed through the crossbar handler.
GIC - CROSSBAR - PERIPHERAL INTERRUPT LINES
peripheral's irq_of_parse_and_map()
|
|
crossbar_xlate()
|
|
saves the interrupt properties passed
peripheral's request_irq(crossbar_number)
|
|
crossbar_request_irq
|
|
allocates free irq and maps it to parent domain
|
|
request_irq(mapped interrupt number)
gic_interrupt_hanadler
|
|
crossbar_irq(interrupt number)
|
|
get crossbar number from interrupt number
|
|
handle_irq(crossbar_domain(crossbar number))
The irqchip callback hooks added here are just a redirection to the
parent irqchip.
This adds a extra translation in the fast path. The maximum increase in
the average interrupt latency due to the same was measured as around 1.63us
on a cpu running at 1GHZ.
cat /proc/interrupts looks like this, with both crossbar and interrupt number
CPU0 CPU1
45:267 0 GIC OMAP UART0
205:267 0 CROSSBAR OMAP UART0
Cc: Thomas Gleixner t...@linutronix.de
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Russell King li...@arm.linux.org.uk
Cc: Tony Lindgren t...@atomide.com
Cc: Rajendra Nayak rna...@ti.com
Signed-off-by: Sricharan R r.sricha...@ti.com
---
There is lockdep warning during the boot. This is because we try to
do one request_irq with in another and that results in kmalloc being
called from an atomic context, which generates the warning.
Any suggestions to overcome this will help.
WARNING: at kernel/lockdep.c:2740 lockdep_trace_alloc+0xe8/0x108()
DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
.../devicetree/bindings/arm/omap/irq-crossbar.txt | 39 ++
drivers/irqchip/Kconfig|9 +
drivers/irqchip/Makefile |1 +
drivers/irqchip/irq-crossbar.c | 407
4 files changed, 456 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
create mode 100644 drivers/irqchip/irq-crossbar.c
diff --git a/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
new file mode 100644
index 000..5d465cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/irq-crossbar.txt
@@ -0,0 +1,39 @@
+* IRQ CROSSBAR
+
+Some socs have a large number of interrupts requests to service
+the needs of its many peripherals and subsystems. All of the
+interrupt lines from the subsystems are not needed at the same
+time, so they have to be muxed to the irq-controller appropriately.
+In such places a interrupt controllers are preceded by an CROSSBAR
+that provides flexibility in muxing the device requests to the controller
+inputs.
+
+Required properties:
+- compatible : Should be irq-crossbar
+- interrupt-parent: phandle to crossbar's interrupt parent.
+- interrupt-controller: Identifies the node as an interrupt controller.
+- interrupt-cells: Should be the same value as the interrupt parent.
+- reg: Base address and the size of the crossbar registers.
+- max-crossbar-lines: Total number of input lines of the crossbar.
+- max-irqs: Total number of irqs available at the interrupt controller.
+- reg-size: size of the crossbar registers.
+- irqs-reserved: List of the reserved irq lines that are not muxed using
+crossbar. These interrupt lines are reserved in the soc,
+so crossbar bar driver should not consider them as free
+lines.
+
+Examples:
+ crossbar_mpu: @4a02 {
+ compatible = irq-crossbar;
+ interrupt-parent = gic;
+ interrupt-controller;
+ #interrupt-cells = 3;
+ reg = 0x4a002a48 0x130;
+ max-crossbar-lines = 512;
+ max-irqs = 160;
+ reg-size = 2;
+ irqs-reserved = 0 1 2 3 5 6 131 132 139 140;
+ #address-cells = 1;
+
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Sricharan R wrote: Signed-off-by: Sricharan R r.sricha...@ti.com --- There is lockdep warning during the boot. This is because we try to do one request_irq with in another and that results in kmalloc being called from an atomic context, which generates the warning. Any suggestions to overcome this will help. You can't be serious about that. You post a patch series with a serious bug in it instead of asking in the first place how to solve the problem at hand. So why do you actually need to call request_irq() from inside request_irq() and how do you actually manage to do that at all? I have a hard time to figure out how request_irq() might call itself recursive. And I have no intention to look at your patch to figure out that you abuse an irq chip callback to do so, simply because that would force me to use abusive language which is frowned upon nowadays. Can you please explain what you are trying to solve without referring to your existing broken implementation. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
Thomas, On Thursday 12 September 2013 04:18 PM, Thomas Gleixner wrote: On Thu, 12 Sep 2013, Sricharan R wrote: Signed-off-by: Sricharan R r.sricha...@ti.com --- There is lockdep warning during the boot. This is because we try to do one request_irq with in another and that results in kmalloc being called from an atomic context, which generates the warning. Any suggestions to overcome this will help. You can't be serious about that. You post a patch series with a serious bug in it instead of asking in the first place how to solve the problem at hand. So why do you actually need to call request_irq() from inside request_irq() and how do you actually manage to do that at all? I have a hard time to figure out how request_irq() might call itself recursive. And I have no intention to look at your patch to figure out that you abuse an irq chip callback to do so, simply because that would force me to use abusive language which is frowned upon nowadays. This is an outcome of the discussion on earlier patch set [1] which tried to add IRQ event router functionality. From beginning I was against the idea because I also felt that we are abusing the irqchip infrastructure and force fitting the cross-bar to be behave like an irqchip. But Linus W and few others strongly felt it to make it an irqchip implementation. Can you please explain what you are trying to solve without referring to your existing broken implementation. I will try to summaries the IP and its need here. On TI SOCs, we have an IP cross-bar which is essentially an even router(hardware). It can route the IRQ and DMA in existing implementation. Specifically for the IRQ case addressed here, the cross-bar IP sits between the interrupt controller and peripheral interrupts. CPU -- GIC - CROSSBAR - PERIPHERAL IRQs Just to expand it better, cross-bar input IRQ lines are more than what a GIC IRQ controller can support. e.q Total 250 peripheral IRQ lines out of which GIC support only 160 IRQ lines. So the idea here is to dynamically map the IRQ lines at cross-bar level to pick based on request_irq() so that one can optimize the use of limited IRQ lines at the GIC level. Strictly speaking the need is just establish the IRQ connection from peripheral to GIC and thats achieved at the request_irq() level. Earlier approach was to statically build this connections using the DT information in a separate driver probe but it had limitations of fixing the IRQ map and taking away flexibility what this IP provide. Hope this gives better picture to you behind the patch series. Just for others to know, use of IRQCHIP also forced to have all the irqchip handler redirection which is pure redirection including the irq-handler. And since it is *fast path* I asked Sricharan to measure the latency which is around ~2 uS( 1GHz CPU) overhead for every interrupt just because of redirections. Regards, Santosh [1] https://lkml.org/lkml/2013/7/18/362 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, Sep 12, 2013 at 09:09:08PM +0530, Sricharan R wrote:
+unsigned int crossbar_request_irq(struct irq_data *d)
+{
+ int cb_no = d-hwirq;
+ int virq = allocate_free_irq(cb_no);
+ void *irq = cb-crossbar_map[cb_no].hwirq;
+ int err;
+
+ err = request_threaded_irq(virq, crossbar_irq, NULL,
+0, CROSSBAR, irq);
this is wrong, why don't you just set crossbar up as a chained handler.
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Felipe Balbi wrote:
On Thu, Sep 12, 2013 at 09:09:08PM +0530, Sricharan R wrote:
+unsigned int crossbar_request_irq(struct irq_data *d)
+{
+ int cb_no = d-hwirq;
+ int virq = allocate_free_irq(cb_no);
+ void *irq = cb-crossbar_map[cb_no].hwirq;
+ int err;
+
+ err = request_threaded_irq(virq, crossbar_irq, NULL,
+ 0, CROSSBAR, irq);
this is wrong, why don't you just set crossbar up as a chained handler.
That's just a detail, which does not solve the underlying problem of
the crossbar - GIC mapping.
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Thomas Gleixner wrote:
On Thu, 12 Sep 2013, Felipe Balbi wrote:
On Thu, Sep 12, 2013 at 09:09:08PM +0530, Sricharan R wrote:
+unsigned int crossbar_request_irq(struct irq_data *d)
+{
+ int cb_no = d-hwirq;
+ int virq = allocate_free_irq(cb_no);
+ void *irq = cb-crossbar_map[cb_no].hwirq;
+ int err;
+
+ err = request_threaded_irq(virq, crossbar_irq, NULL,
+0, CROSSBAR, irq);
this is wrong, why don't you just set crossbar up as a chained handler.
That's just a detail, which does not solve the underlying problem of
the crossbar - GIC mapping.
And actually the thing is the other way round:
GIC distributes to crossbar, so the underlying GIC irq would be the
chained handler.
Still does not solve the setup/request_irq issue.
Thanks,
tglx
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Santosh Shilimkar wrote: Specifically for the IRQ case addressed here, the cross-bar IP sits between the interrupt controller and peripheral interrupts. CPU -- GIC - CROSSBAR - PERIPHERAL IRQs Just to expand it better, cross-bar input IRQ lines are more than what a GIC IRQ controller can support. e.q Total 250 peripheral IRQ lines out of which GIC support only 160 IRQ lines. So the idea here is to dynamically map the IRQ lines at cross-bar level to pick based on request_irq() so that one can optimize the use of limited IRQ lines at the GIC level. Strictly speaking the need is just establish the IRQ connection from peripheral to GIC and thats achieved at the request_irq() level. Earlier approach was to statically build this connections using the DT information in a separate driver probe but it had limitations of fixing the IRQ map and taking away flexibility what this IP provide. Hope this gives better picture to you behind the patch series. Yes. I halfways understand what you are trying to achieve. So CROSSBAR is a routing block between the peripheral and the GIC in order to expand the number of possible interrupts. Now the real question is, how that expansion mechanism is supposed to work. There are two possible scenarios: 1) Expand the number of handled interrupts beyond the GIC capacity: That requires a mechanism in CROSSBAR to map several CROSSBAR interrupts to a particular GIC interrupt and provide a demux mechanism to invoke the shared handlers. 2) Provide a mapping mechanism between possibly 250 interrupt numbers and a limitation of a total 160 active interrupts by the underlying GIC. What are you trying to achieve? Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thursday 12 September 2013 06:22 PM, Thomas Gleixner wrote: On Thu, 12 Sep 2013, Santosh Shilimkar wrote: Specifically for the IRQ case addressed here, the cross-bar IP sits between the interrupt controller and peripheral interrupts. CPU -- GIC - CROSSBAR - PERIPHERAL IRQs Just to expand it better, cross-bar input IRQ lines are more than what a GIC IRQ controller can support. e.q Total 250 peripheral IRQ lines out of which GIC support only 160 IRQ lines. So the idea here is to dynamically map the IRQ lines at cross-bar level to pick based on request_irq() so that one can optimize the use of limited IRQ lines at the GIC level. Strictly speaking the need is just establish the IRQ connection from peripheral to GIC and thats achieved at the request_irq() level. Earlier approach was to statically build this connections using the DT information in a separate driver probe but it had limitations of fixing the IRQ map and taking away flexibility what this IP provide. Hope this gives better picture to you behind the patch series. Yes. I halfways understand what you are trying to achieve. So CROSSBAR is a routing block between the peripheral and the GIC in order to expand the number of possible interrupts. Now the real question is, how that expansion mechanism is supposed to work. There are two possible scenarios: 1) Expand the number of handled interrupts beyond the GIC capacity: That requires a mechanism in CROSSBAR to map several CROSSBAR interrupts to a particular GIC interrupt and provide a demux mechanism to invoke the shared handlers. This is not possible in hardware and not supported. Hardware has no notion of muxing multiple IRQ's to generate 1 IRQ or ack etc functionality. Its a simple MUX to tie knots between input and output wires. 2) Provide a mapping mechanism between possibly 250 interrupt numbers and a limitation of a total 160 active interrupts by the underlying GIC. This is the need and problem we are trying to solve. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thu, 12 Sep 2013, Santosh Shilimkar wrote: On Thursday 12 September 2013 06:22 PM, Thomas Gleixner wrote: Now the real question is, how that expansion mechanism is supposed to work. There are two possible scenarios: 1) Expand the number of handled interrupts beyond the GIC capacity: That requires a mechanism in CROSSBAR to map several CROSSBAR interrupts to a particular GIC interrupt and provide a demux mechanism to invoke the shared handlers. This is not possible in hardware and not supported. Hardware has no notion of muxing multiple IRQ's to generate 1 IRQ or ack etc functionality. Its a simple MUX to tie knots between input and output wires. It's not a MUX. It's a ROUTING mechanism. That's similar to the mechanisms which are used by MSI[X]. We assign arbitrary interrupt numbers to a device and route them to some underlying limited hardware interrupt controller. 2) Provide a mapping mechanism between possibly 250 interrupt numbers and a limitation of a total 160 active interrupts by the underlying GIC. This is the need and problem we are trying to solve. Let me summarize: - GIC supports up to 160 interrupts - CROSSBAR supports up to 250 interrupts - CROSSBAR routes up to 160 out of 250 interrupts to the GIC ones - Drivers request a CROSSBAR interrupt number which must be mapped to some arbitrary available GIC irq number So basically the CROSSBAR mechanism is pretty much the same as MSI[X] just in a different flavour and with a different set of semantics and limitations, i.e. poor mans MSI[X] with a new level of bogosity. So if CROSSBAR is going to be the new fangled SoC MSI[X] long term equivalent then you better provide some infrastructure for that and make the drivers ready to use it. Maybe check with the PCI/MSI folks to share some of the interfaces. If that whole thing is another onetime HW designers wet dream, then please go back to the limited but completely functional (Who is going to use more than 160 peripheral interrupts) device tree model. I really have no interest to support hardware designer brain farts. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] DRIVERS: IRQCHIP: Add crossbar irqchip driver
On Thursday 12 September 2013 08:26 PM, Thomas Gleixner wrote: On Thu, 12 Sep 2013, Santosh Shilimkar wrote: On Thursday 12 September 2013 06:22 PM, Thomas Gleixner wrote: Now the real question is, how that expansion mechanism is supposed to work. There are two possible scenarios: 1) Expand the number of handled interrupts beyond the GIC capacity: That requires a mechanism in CROSSBAR to map several CROSSBAR interrupts to a particular GIC interrupt and provide a demux mechanism to invoke the shared handlers. This is not possible in hardware and not supported. Hardware has no notion of muxing multiple IRQ's to generate 1 IRQ or ack etc functionality. Its a simple MUX to tie knots between input and output wires. It's not a MUX. It's a ROUTING mechanism. That's similar to the mechanisms which are used by MSI[X]. We assign arbitrary interrupt numbers to a device and route them to some underlying limited hardware interrupt controller. 2) Provide a mapping mechanism between possibly 250 interrupt numbers and a limitation of a total 160 active interrupts by the underlying GIC. This is the need and problem we are trying to solve. Let me summarize: - GIC supports up to 160 interrupts - CROSSBAR supports up to 250 interrupts - CROSSBAR routes up to 160 out of 250 interrupts to the GIC ones - Drivers request a CROSSBAR interrupt number which must be mapped to some arbitrary available GIC irq number Correct. So basically the CROSSBAR mechanism is pretty much the same as MSI[X] just in a different flavour and with a different set of semantics and limitations, i.e. poor mans MSI[X] with a new level of bogosity. So if CROSSBAR is going to be the new fangled SoC MSI[X] long term equivalent then you better provide some infrastructure for that and make the drivers ready to use it. Maybe check with the PCI/MSI folks to share some of the interfaces. If that whole thing is another onetime HW designers wet dream, then please go back to the limited but completely functional (Who is going to use more than 160 peripheral interrupts) device tree model. I really have no interest to support hardware designer brain farts. Thanks for clear NAK for irqchip approach. I should have looped you in the discussion where I was also suggesting against the irqchip approach. We will try to look at MSI stuff but if its get too complicated am going to fall-back to the initial probe based approach to achieve the functionality. Thanks again for clear direction and useful discussion. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
