2014-06-16 11:25:49

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 00/16] irqchip: crossbar: driver fixes

This series does some cleanups, fixes for handling two interrupts
getting mapped twice to same crossbar and provides support for
hardwired IRQ and crossbar definitions.

On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10,
131, 132, 133 are direct wired to hardware blocks bypassing
crossbar. This quirky implementation is *NOT* supposed to be the
expectation of crossbar hardware usage. This series adds support
to represent such hard-wired irqs through DT and avoid generic
allocation/programming of crossbar in the driver.

This way of supporting hard-wired irqs was a result of
the below discussions.
http://www.spinics.net/lists/arm-kernel/msg329946.html

Based on 3.15 mainline.

All the patches are available here
[email protected]:Sricharanti/sricharan.git crossbar_updates

The fixes series[1] earlier posted is merged in to this.
[1] http://www.spinics.net/lists/arm-kernel/msg328273.html

[V2] Merged the above series and rebased on 3.15 mainline

[V3] Modified patch#3 to get irqs-skip properties from DT,
merged path#8 for checkpatch warning to other relevant
patches and fixed comments for other patches.

Nishanth Menon (14):
irqchip: crossbar: dont use '0' to mark reserved interrupts
irqchip: crossbar: check for premapped crossbar before allocating
irqchip: crossbar: introduce ti,irqs-skip to skip
irqchip: crossbar: initialise the crossbar with a safe value
irqchip: crossbar: change allocation logic by reversing search for
free irqs
irqchip: crossbar: remove IS_ERR_VALUE check
irqchip: crossbar: fix sparse and checkpatch warnings
irqchip: crossbar: fix kerneldoc warning
irqchip: crossbar: return proper error value
irqchip: crossbar: change the goto naming
irqchip: crossbar: introduce ti,max-crossbar-sources to identify
valid crossbar mapping
irqchip: crossbar: introduce centralized check for crossbar write
documentation: dt: omap: crossbar: add description for interrupt
consumer
irqchip: crossbar: allow for quirky hardware with direct hardwiring
of GIC

Sricharan R (2):
irqchip: crossbar: set cb pointer to null in case of error
irqchip: crossbar: add kerneldoc for crossbar_domain_unmap callback

.../devicetree/bindings/arm/omap/crossbar.txt | 34 ++++
drivers/irqchip/irq-crossbar.c | 168 +++++++++++++++++---
2 files changed, 177 insertions(+), 25 deletions(-)

--
1.7.9.5


2014-06-16 11:26:01

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 02/16] irqchip: crossbar: check for premapped crossbar before allocating

From: Nishanth Menon <[email protected]>

If irq_of_parse_and_map is executed twice, the same crossbar is mapped to two
different GIC interrupts. This is completely undesirable. Instead, check
if the requested crossbar event is pre-allocated and provide that GIC
mapping back to caller if already allocated.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 20105bc..51d4b87 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -51,6 +51,17 @@ static inline void crossbar_writeb(int irq_no, int cb_no)
writeb(cb_no, cb->crossbar_base + cb->register_offsets[irq_no]);
}

+static inline int get_prev_map_irq(int cb_no)
+{
+ int i;
+
+ for (i = 0; i < cb->int_max; i++)
+ if (cb->irq_map[i] == cb_no)
+ return i;
+
+ return -ENODEV;
+}
+
static inline int allocate_free_irq(int cb_no)
{
int i;
@@ -88,11 +99,16 @@ static int crossbar_domain_xlate(struct irq_domain *d,
{
unsigned long ret;

+ ret = get_prev_map_irq(intspec[1]);
+ if (!IS_ERR_VALUE(ret))
+ goto found;
+
ret = allocate_free_irq(intspec[1]);

if (IS_ERR_VALUE(ret))
return ret;

+found:
*out_hwirq = ret + GIC_IRQ_START;
return 0;
}
--
1.7.9.5

2014-06-16 11:26:07

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 05/16] irqchip: crossbar: change allocation logic by reversing search for free irqs

From: Nishanth Menon <[email protected]>

Reverse the search algorithm to ensure that address mapping and IRQ
allocation logics are proper. This makes the below bugs visible sooner.

class 1. address space errors -> example:
reg = <a size_b>
ti,max-irqs = is a wrong parameter

class 2: irq-reserved list - which decides which entries in the
address space is not actually wired in

class 3: wrong list of routable-irqs.

In general allocating from max to min tends to have benefits in
ensuring the different issues that may be present in dts is easily
caught at definition time, rather than at a later point in time.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] Added more description to commit log.

drivers/irqchip/irq-crossbar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index d1f67f6..9528cf2 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -58,7 +58,7 @@ static inline int get_prev_map_irq(int cb_no)
{
int i;

- for (i = 0; i < cb->int_max; i++)
+ for (i = cb->int_max - 1; i >= 0; i--)
if (cb->irq_map[i] == cb_no)
return i;

@@ -69,7 +69,7 @@ static inline int allocate_free_irq(int cb_no)
{
int i;

- for (i = 0; i < cb->int_max; i++) {
+ for (i = cb->int_max - 1; i >= 0; i--) {
if (cb->irq_map[i] == IRQ_FREE) {
cb->irq_map[i] = cb_no;
return i;
--
1.7.9.5

2014-06-16 11:26:10

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 06/16] irqchip: crossbar: remove IS_ERR_VALUE check

From: Nishanth Menon <[email protected]>

IS_ERR_VALUE makes sense only *if* there could be valid values in
negative error range. But in the cases that we do use it, there is no
such case. Just remove the same.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 9528cf2..42ea49d 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -102,15 +102,15 @@ static int crossbar_domain_xlate(struct irq_domain *d,
unsigned long *out_hwirq,
unsigned int *out_type)
{
- unsigned long ret;
+ int ret;

ret = get_prev_map_irq(intspec[1]);
- if (!IS_ERR_VALUE(ret))
+ if (ret >= 0)
goto found;

ret = allocate_free_irq(intspec[1]);

- if (IS_ERR_VALUE(ret))
+ if (ret < 0)
return ret;

found:
--
1.7.9.5

2014-06-16 11:26:20

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 07/16] irqchip: crossbar: fix sparse and checkpatch warnings

From: Nishanth Menon <[email protected]>

There is absolutely no need for crossbar driver to expose functions and
variables into global namespace. So make them all static

Also fix a couple of checkpatch warnings.

Fixes sparse warnings:
drivers/irqchip/irq-crossbar.c:129:29: warning: symbol 'routable_irq_domain_ops' was not declared. Should it be static?
drivers/irqchip/irq-crossbar.c:261:12: warning: symbol 'irqcrossbar_init' was not declared. Should it be static?

Checkpatch warnings:
WARNING: Prefer kcalloc over kzalloc with multiply
+ cb->irq_map = kzalloc(max * sizeof(int), GFP_KERNEL);

WARNING: Prefer kcalloc over kzalloc with multiply
+ cb->register_offsets = kzalloc(max * sizeof(int), GFP_KERNEL);

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] Added checkpatch fixes as well to this.

drivers/irqchip/irq-crossbar.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 42ea49d..2d7fbdb 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -15,6 +15,7 @@
#include <linux/of_irq.h>
#include <linux/slab.h>
#include <linux/irqchip/arm-gic.h>
+#include <linux/irqchip/irq-crossbar.h>

#define IRQ_FREE -1
#define IRQ_RESERVED -2
@@ -118,7 +119,7 @@ found:
return 0;
}

-const struct irq_domain_ops routable_irq_domain_ops = {
+static const struct irq_domain_ops routable_irq_domain_ops = {
.map = crossbar_domain_map,
.unmap = crossbar_domain_unmap,
.xlate = crossbar_domain_xlate
@@ -139,7 +140,7 @@ static int __init crossbar_of_init(struct device_node *node)
goto err1;

of_property_read_u32(node, "ti,max-irqs", &max);
- cb->irq_map = kzalloc(max * sizeof(int), GFP_KERNEL);
+ cb->irq_map = kcalloc(max, sizeof(int), GFP_KERNEL);
if (!cb->irq_map)
goto err2;

@@ -184,7 +185,7 @@ static int __init crossbar_of_init(struct device_node *node)
}


- cb->register_offsets = kzalloc(max * sizeof(int), GFP_KERNEL);
+ cb->register_offsets = kcalloc(max, sizeof(int), GFP_KERNEL);
if (!cb->register_offsets)
goto err3;

--
1.7.9.5

2014-06-16 11:26:24

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 08/16] irqchip: crossbar: fix kerneldoc warning

From: Nishanth Menon <[email protected]>

Adding missing properties for kerneldoc (@write) and cleanup
of harmless warnings while we are here.

kerneldoc warnings:
Warning(drivers/irqchip/irq-crossbar.c:27): missing initial short description on line:
* struct crossbar_device: crossbar device description
Info(drivers/irqchip/irq-crossbar.c:27): Scanning doc for struct
Warning(drivers/irqchip/irq-crossbar.c:39): No description found for parameter 'write'
2 warnings

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] Reworded the commit log

drivers/irqchip/irq-crossbar.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 2d7fbdb..6eee61b 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -22,12 +22,14 @@
#define IRQ_SKIP -3
#define GIC_IRQ_START 32

-/*
+/**
+ * struct crossbar_device - crossbar device description
* @int_max: maximum number of supported interrupts
* @safe_map: safe default value to initialize the crossbar
* @irq_map: array of interrupts to crossbar number mapping
* @crossbar_base: crossbar base address
* @register_offsets: offsets for each irq number
+ * @write: register write function pointer
*/
struct crossbar_device {
uint int_max;
--
1.7.9.5

2014-06-16 11:26:35

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 10/16] irqchip: crossbar: change the goto naming

From: Nishanth Menon <[email protected]>

Using err1,2,3,4 etc makes it hard to ensure a new exit path in the
middle will not result in spurious changes, so rename the error paths
as per the function it does.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 5bcedc0..5bd7f3d 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -140,17 +140,17 @@ static int __init crossbar_of_init(struct device_node *node)

cb->crossbar_base = of_iomap(node, 0);
if (!cb->crossbar_base)
- goto err1;
+ goto err_cb;

of_property_read_u32(node, "ti,max-irqs", &max);
if (!max) {
pr_err("missing 'ti,max-irqs' property\n");
ret = -EINVAL;
- goto err2;
+ goto err_base;
}
cb->irq_map = kcalloc(max, sizeof(int), GFP_KERNEL);
if (!cb->irq_map)
- goto err2;
+ goto err_base;

cb->int_max = max;

@@ -169,7 +169,7 @@ static int __init crossbar_of_init(struct device_node *node)
if (entry > max) {
pr_err("Invalid reserved entry\n");
ret = -EINVAL;
- goto err3;
+ goto err_irq_map;
}
cb->irq_map[entry] = IRQ_RESERVED;
}
@@ -187,7 +187,7 @@ static int __init crossbar_of_init(struct device_node *node)
if (entry > max) {
pr_err("Invalid skip entry\n");
ret = -EINVAL;
- goto err3;
+ goto err_irq_map;
}
cb->irq_map[entry] = IRQ_SKIP;
}
@@ -196,7 +196,7 @@ static int __init crossbar_of_init(struct device_node *node)

cb->register_offsets = kcalloc(max, sizeof(int), GFP_KERNEL);
if (!cb->register_offsets)
- goto err3;
+ goto err_irq_map;

of_property_read_u32(node, "ti,reg-size", &size);

@@ -213,7 +213,7 @@ static int __init crossbar_of_init(struct device_node *node)
default:
pr_err("Invalid reg-size property\n");
ret = -EINVAL;
- goto err4;
+ goto err_reg_offset;
break;
}

@@ -230,7 +230,6 @@ static int __init crossbar_of_init(struct device_node *node)
}

of_property_read_u32(node, "ti,irqs-safe-map", &cb->safe_map);
-
/* Initialize the crossbar with safe map to start with */
for (i = 0; i < max; i++) {
if (cb->irq_map[i] == IRQ_RESERVED ||
@@ -243,13 +242,13 @@ static int __init crossbar_of_init(struct device_node *node)
register_routable_domain_ops(&routable_irq_domain_ops);
return 0;

-err4:
+err_reg_offset:
kfree(cb->register_offsets);
-err3:
+err_irq_map:
kfree(cb->irq_map);
-err2:
+err_base:
iounmap(cb->crossbar_base);
-err1:
+err_cb:
kfree(cb);
return ret;
}
--
1.7.9.5

2014-06-16 11:26:39

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 09/16] irqchip: crossbar: return proper error value

From: Nishanth Menon <[email protected]>

crossbar_of_init always returns -ENOMEM in case of errors.
There can be other causes of failure like invalid data from
DT. So return a appropriate error value for that case.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] Changed commit log

drivers/irqchip/irq-crossbar.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 6eee61b..5bcedc0 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -129,19 +129,25 @@ static const struct irq_domain_ops routable_irq_domain_ops = {

static int __init crossbar_of_init(struct device_node *node)
{
- int i, size, max, reserved = 0, entry;
+ int i, size, max = 0, reserved = 0, entry;
const __be32 *irqsr;
+ int ret = -ENOMEM;

cb = kzalloc(sizeof(*cb), GFP_KERNEL);

if (!cb)
- return -ENOMEM;
+ return ret;

cb->crossbar_base = of_iomap(node, 0);
if (!cb->crossbar_base)
goto err1;

of_property_read_u32(node, "ti,max-irqs", &max);
+ if (!max) {
+ pr_err("missing 'ti,max-irqs' property\n");
+ ret = -EINVAL;
+ goto err2;
+ }
cb->irq_map = kcalloc(max, sizeof(int), GFP_KERNEL);
if (!cb->irq_map)
goto err2;
@@ -162,6 +168,7 @@ static int __init crossbar_of_init(struct device_node *node)
i, &entry);
if (entry > max) {
pr_err("Invalid reserved entry\n");
+ ret = -EINVAL;
goto err3;
}
cb->irq_map[entry] = IRQ_RESERVED;
@@ -205,6 +212,7 @@ static int __init crossbar_of_init(struct device_node *node)
break;
default:
pr_err("Invalid reg-size property\n");
+ ret = -EINVAL;
goto err4;
break;
}
@@ -243,7 +251,7 @@ err2:
iounmap(cb->crossbar_base);
err1:
kfree(cb);
- return -ENOMEM;
+ return ret;
}

static const struct of_device_id crossbar_match[] __initconst = {
--
1.7.9.5

2014-06-16 11:26:45

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 12/16] irqchip: crossbar: add kerneldoc for crossbar_domain_unmap callback

Adding kerneldoc for unmap callback function.

Signed-off-by: Sricharan R <[email protected]>
---
[V3] Reworded the kerneldoc

drivers/irqchip/irq-crossbar.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 9b4c0f1..df16ef8 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -89,6 +89,17 @@ static int crossbar_domain_map(struct irq_domain *d, unsigned int irq,
return 0;
}

+/**
+ * crossbar_domain_unmap - unmap a crossbar<->irq connection
+ * @d: domain of irq to unmap
+ * @irq: virq number
+ *
+ * We do not maintain a use count of total number of map/unmap
+ * calls for a particular irq to find out if a irq can be really
+ * unmapped. This is because unmap is called during irq_dispose_mapping(irq),
+ * after which irq is anyways unusable. So an explicit map has to be called
+ * after that.
+ */
static void crossbar_domain_unmap(struct irq_domain *d, unsigned int irq)
{
irq_hw_number_t hw = irq_get_irq_data(irq)->hwirq;
--
1.7.9.5

2014-06-16 11:27:06

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 15/16] documentation: dt: omap: crossbar: add description for interrupt consumer

From: Nishanth Menon <[email protected]>

The current crossbar description does not include the description
required for the consumer of the crossbar, a.k.a devices whoes events
pass through the crossbar into the GIC interrupt controller.

So, provide documentation for the same.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
.../devicetree/bindings/arm/omap/crossbar.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
index 41aef44..8210ea4 100644
--- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -34,3 +34,20 @@ Examples:
ti,reg-size = <2>;
ti,irqs-reserved = <0 1 2 3 5 6 131 132 139 140>;
};
+
+Consumer:
+========
+See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt and
+Documentation/devicetree/bindings/arm/gic.txt for further details.
+
+An interrupt consumer on an SoC using crossbar will use:
+ interrupts = <GIC_SPI request_number interrupt_level>
+request number shall be between 0 to that described by
+"ti,max-crossbar-sources"
+
+Example:
+ device_x@0x4a023000 {
+ /* Crossbar 8 used */
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ ...
+ };
--
1.7.9.5

2014-06-16 11:27:09

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 16/16] irqchip: crossbar: allow for quirky hardware with direct hardwiring of GIC

From: Nishanth Menon <[email protected]>

On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10, 131,
132, 133 are direct wired to hardware blocks bypassing crossbar.
This quirky implementation is *NOT* supposed to be the expectation
of crossbar hardware usage. However, these are already marked in our
description of the hardware with SKIP and RESERVED where appropriate.

Unfortunately, we need to be able to refer to these hardwired IRQs.
So, to request these, crossbar driver can use the existing information
from it's table that these SKIP/RESERVED maps are direct wired sources
and generic allocation/programming of crossbar should be avoided.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
.../devicetree/bindings/arm/omap/crossbar.txt | 12 ++++++++++--
drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++--
2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
index 8210ea4..438ccab 100644
--- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -42,8 +42,10 @@ Documentation/devicetree/bindings/arm/gic.txt for further details.

An interrupt consumer on an SoC using crossbar will use:
interrupts = <GIC_SPI request_number interrupt_level>
-request number shall be between 0 to that described by
-"ti,max-crossbar-sources"
+When the request number is between 0 to that described by
+"ti,max-crossbar-sources", it is assumed to be a crossbar mapping. If the
+request_number is greater than "ti,max-crossbar-sources", then it is mapped as a
+quirky hardware mapping direct to GIC.

Example:
device_x@0x4a023000 {
@@ -51,3 +53,9 @@ Example:
interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
...
};
+
+ device_y@0x4a033000 {
+ /* Direct mapped GIC SPI 1 used */
+ interrupts = <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>;
+ ...
+ };
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index ef613c4..fff6218 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -86,8 +86,13 @@ static inline int allocate_free_irq(int cb_no)

static inline bool needs_crossbar_write(irq_hw_number_t hw)
{
- if (hw > GIC_IRQ_START)
- return true;
+ int cb_no;
+
+ if (hw > GIC_IRQ_START) {
+ cb_no = cb->irq_map[hw - GIC_IRQ_START];
+ if (cb_no != IRQ_RESERVED && cb_no != IRQ_SKIP)
+ return true;
+ }

return false;
}
@@ -130,8 +135,19 @@ static int crossbar_domain_xlate(struct irq_domain *d,
{
int ret;
int req_num = intspec[1];
+ int direct_map_num;

if (req_num >= cb->max_crossbar_sources) {
+ direct_map_num = req_num - cb->max_crossbar_sources;
+ if (direct_map_num < cb->int_max) {
+ ret = cb->irq_map[direct_map_num];
+ if (ret == IRQ_RESERVED || ret == IRQ_SKIP) {
+ /* We use the interrupt num as h/w irq num */
+ ret = direct_map_num;
+ goto found;
+ }
+ }
+
pr_err("%s: requested crossbar number %d > max %d\n",
__func__, req_num, cb->max_crossbar_sources);
return -EINVAL;
--
1.7.9.5

2014-06-16 11:27:50

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 14/16] irqchip: crossbar: introduce centralized check for crossbar write

From: Nishanth Menon <[email protected]>

This is a basic check to ensure that crossbar register needs to be
written. This ensures that we have a common check which is used in
both map and unmap logic.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index c46e14b..ef613c4 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -84,10 +84,20 @@ static inline int allocate_free_irq(int cb_no)
return -ENODEV;
}

+static inline bool needs_crossbar_write(irq_hw_number_t hw)
+{
+ if (hw > GIC_IRQ_START)
+ return true;
+
+ return false;
+}
+
static int crossbar_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
- cb->write(hw - GIC_IRQ_START, cb->irq_map[hw - GIC_IRQ_START]);
+ if (needs_crossbar_write(hw))
+ cb->write(hw - GIC_IRQ_START, cb->irq_map[hw - GIC_IRQ_START]);
+
return 0;
}

@@ -106,7 +116,7 @@ static void crossbar_domain_unmap(struct irq_domain *d, unsigned int irq)
{
irq_hw_number_t hw = irq_get_irq_data(irq)->hwirq;

- if (hw > GIC_IRQ_START) {
+ if (needs_crossbar_write(hw)) {
cb->irq_map[hw - GIC_IRQ_START] = IRQ_FREE;
cb->write(hw - GIC_IRQ_START, cb->safe_map);
}
--
1.7.9.5

2014-06-16 11:28:09

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 13/16] irqchip: crossbar: introduce ti,max-crossbar-sources to identify valid crossbar mapping

From: Nishanth Menon <[email protected]>

Currently we attempt to map any crossbar value to an IRQ, however,
this is not correct from hardware perspective. There is a max crossbar
event number upto which hardware supports. So describe the same in
device tree using 'ti,max-crossbar-sources' property and use it to
validate requests.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
.../devicetree/bindings/arm/omap/crossbar.txt | 2 ++
drivers/irqchip/irq-crossbar.c | 21 ++++++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
index e54d1fb..41aef44 100644
--- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -10,6 +10,7 @@ Required properties:
- compatible : Should be "ti,irq-crossbar"
- reg: Base address and the size of the crossbar registers.
- ti,max-irqs: Total number of irqs available at the interrupt controller.
+- ti,max-crossbar-sources: Maximum number of crossbar sources that can be routed.
- ti,reg-size: Size of a individual register in bytes. Every individual
register is assumed to be of same size. Valid sizes are 1, 2, 4.
- ti,irqs-reserved: List of the reserved irq lines that are not muxed using
@@ -29,6 +30,7 @@ Examples:
compatible = "ti,irq-crossbar";
reg = <0x4a002a48 0x130>;
ti,max-irqs = <160>;
+ ti,max-crossbar-sources = <400>;
ti,reg-size = <2>;
ti,irqs-reserved = <0 1 2 3 5 6 131 132 139 140>;
};
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index df16ef8..c46e14b 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -26,6 +26,7 @@
* struct crossbar_device - crossbar device description
* @int_max: maximum number of supported interrupts
* @safe_map: safe default value to initialize the crossbar
+ * @max_crossbar_sources: Maximum number of crossbar sources
* @irq_map: array of interrupts to crossbar number mapping
* @crossbar_base: crossbar base address
* @register_offsets: offsets for each irq number
@@ -34,6 +35,7 @@
struct crossbar_device {
uint int_max;
uint safe_map;
+ uint max_crossbar_sources;
uint *irq_map;
void __iomem *crossbar_base;
int *register_offsets;
@@ -117,12 +119,19 @@ static int crossbar_domain_xlate(struct irq_domain *d,
unsigned int *out_type)
{
int ret;
+ int req_num = intspec[1];

- ret = get_prev_map_irq(intspec[1]);
+ if (req_num >= cb->max_crossbar_sources) {
+ pr_err("%s: requested crossbar number %d > max %d\n",
+ __func__, req_num, cb->max_crossbar_sources);
+ return -EINVAL;
+ }
+
+ ret = get_prev_map_irq(req_num);
if (ret >= 0)
goto found;

- ret = allocate_free_irq(intspec[1]);
+ ret = allocate_free_irq(req_num);

if (ret < 0)
return ret;
@@ -153,6 +162,14 @@ static int __init crossbar_of_init(struct device_node *node)
if (!cb->crossbar_base)
goto err_cb;

+ of_property_read_u32(node, "ti,max-crossbar-sources",
+ &cb->max_crossbar_sources);
+ if (!cb->max_crossbar_sources) {
+ pr_err("missing 'ti,max-crossbar-sources' property\n");
+ ret = -EINVAL;
+ goto err_base;
+ }
+
of_property_read_u32(node, "ti,max-irqs", &max);
if (!max) {
pr_err("missing 'ti,max-irqs' property\n");
--
1.7.9.5

2014-06-16 11:28:07

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 11/16] irqchip: crossbar: set cb pointer to null in case of error

If crossbar_of_init returns with a error, then set the cb pointer
to null.

Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 5bd7f3d..9b4c0f1 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -250,6 +250,8 @@ err_base:
iounmap(cb->crossbar_base);
err_cb:
kfree(cb);
+
+ cb = NULL;
return ret;
}

--
1.7.9.5

2014-06-16 11:25:59

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 03/16] irqchip: crossbar: introduce ti,irqs-skip to skip

From: Nishanth Menon <[email protected]>

When, in the system due to varied reasons, interrupts might be unusable
due to hardware behavior, but register maps do exist, then those interrupts
should be skipped while mapping irq to crossbars.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] introduced ti,irqs-skip dt property to list the
irqs to be skipped.

.../devicetree/bindings/arm/omap/crossbar.txt | 4 ++++
drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
index fb88585..cfcbd52 100644
--- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -17,6 +17,10 @@ Required properties:
so crossbar bar driver should not consider them as free
lines.

+Optional properties:
+- ti,irqs-skip: This is similar to "ti,irqs-reserved", but are irq mappings
+ which are not supposed to be used for errata or other reasons(virtualization).
+
Examples:
crossbar_mpu: @4a020000 {
compatible = "ti,irq-crossbar";
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 51d4b87..27049de 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -18,6 +18,7 @@

#define IRQ_FREE -1
#define IRQ_RESERVED -2
+#define IRQ_SKIP -3
#define GIC_IRQ_START 32

/*
@@ -160,6 +161,25 @@ static int __init crossbar_of_init(struct device_node *node)
}
}

+ /* Skip the ones marked as skip */
+ irqsr = of_get_property(node, "ti,irqs-skip", &size);
+ if (irqsr) {
+ size /= sizeof(__be32);
+
+ for (i = 0; i < size; i++) {
+ of_property_read_u32_index(node,
+ "ti,irqs-skip",
+ i, &entry);
+ if (entry > max) {
+ pr_err("Invalid skip entry\n");
+ ret = -EINVAL;
+ goto err3;
+ }
+ cb->irq_map[entry] = IRQ_SKIP;
+ }
+ }
+
+
cb->register_offsets = kzalloc(max * sizeof(int), GFP_KERNEL);
if (!cb->register_offsets)
goto err3;
--
1.7.9.5

2014-06-16 11:25:57

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 04/16] irqchip: crossbar: initialise the crossbar with a safe value

From: Nishanth Menon <[email protected]>

Since crossbar is s/w configurable, the initial settings of the
crossbar cannot be assumed to be sane. This implies that:
a) On initialization all un-reserved crossbars must be initialized to
a known 'safe' value.
b) When unmapping the interrupt, the safe value must be written to
ensure that the crossbar mapping matches with interrupt controller
usage.

So provide a safe value in the dt data to map if
'0' is not safe for the platform and use it during init and unmap

While at this, fix the below checkpatch warning.
Fixes checkpatch warning:
WARNING: Unnecessary space before function pointer arguments
#37: FILE: drivers/irqchip/irq-crossbar.c:37:
+ void (*write) (int, int);

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
[V3] introduced ti,irqs-safe-map which defines a safe value
to initialize the crossbar.

.../devicetree/bindings/arm/omap/crossbar.txt | 3 +++
drivers/irqchip/irq-crossbar.c | 19 +++++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
index cfcbd52..e54d1fb 100644
--- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
+++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
@@ -21,6 +21,9 @@ Optional properties:
- ti,irqs-skip: This is similar to "ti,irqs-reserved", but are irq mappings
which are not supposed to be used for errata or other reasons(virtualization).

+- ti,irqs-safe-map: integer which maps to a safe configuration to use
+ when the interrupt controller irq is unused (when not provided, default is 0)
+
Examples:
crossbar_mpu: @4a020000 {
compatible = "ti,irq-crossbar";
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 27049de..d1f67f6 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -23,16 +23,18 @@

/*
* @int_max: maximum number of supported interrupts
+ * @safe_map: safe default value to initialize the crossbar
* @irq_map: array of interrupts to crossbar number mapping
* @crossbar_base: crossbar base address
* @register_offsets: offsets for each irq number
*/
struct crossbar_device {
uint int_max;
+ uint safe_map;
uint *irq_map;
void __iomem *crossbar_base;
int *register_offsets;
- void (*write) (int, int);
+ void (*write)(int, int);
};

static struct crossbar_device *cb;
@@ -88,8 +90,10 @@ static void crossbar_domain_unmap(struct irq_domain *d, unsigned int irq)
{
irq_hw_number_t hw = irq_get_irq_data(irq)->hwirq;

- if (hw > GIC_IRQ_START)
+ if (hw > GIC_IRQ_START) {
cb->irq_map[hw - GIC_IRQ_START] = IRQ_FREE;
+ cb->write(hw - GIC_IRQ_START, cb->safe_map);
+ }
}

static int crossbar_domain_xlate(struct irq_domain *d,
@@ -214,6 +218,17 @@ static int __init crossbar_of_init(struct device_node *node)
reserved += size;
}

+ of_property_read_u32(node, "ti,irqs-safe-map", &cb->safe_map);
+
+ /* Initialize the crossbar with safe map to start with */
+ for (i = 0; i < max; i++) {
+ if (cb->irq_map[i] == IRQ_RESERVED ||
+ cb->irq_map[i] == IRQ_SKIP)
+ continue;
+
+ cb->write(i, cb->safe_map);
+ }
+
register_routable_domain_ops(&routable_irq_domain_ops);
return 0;

--
1.7.9.5

2014-06-16 11:31:17

by R, Sricharan

[permalink] [raw]
Subject: [PATCH V3 01/16] irqchip: crossbar: dont use '0' to mark reserved interrupts

From: Nishanth Menon <[email protected]>

Today '0' is actually reserved, but may not be the same in the future.

So, use a flag to mark the GIC interrupts that are reserved.

Signed-off-by: Nishanth Menon <[email protected]>
Signed-off-by: Sricharan R <[email protected]>
---
drivers/irqchip/irq-crossbar.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 3d15d16..20105bc 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -17,6 +17,7 @@
#include <linux/irqchip/arm-gic.h>

#define IRQ_FREE -1
+#define IRQ_RESERVED -2
#define GIC_IRQ_START 32

/*
@@ -139,7 +140,7 @@ static int __init crossbar_of_init(struct device_node *node)
pr_err("Invalid reserved entry\n");
goto err3;
}
- cb->irq_map[entry] = 0;
+ cb->irq_map[entry] = IRQ_RESERVED;
}
}

@@ -170,7 +171,7 @@ static int __init crossbar_of_init(struct device_node *node)
* reserved irqs. so find and store the offsets once.
*/
for (i = 0; i < max; i++) {
- if (!cb->irq_map[i])
+ if (cb->irq_map[i] == IRQ_RESERVED)
continue;

cb->register_offsets[i] = reserved;
--
1.7.9.5

2014-06-16 14:05:47

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [PATCH V3 00/16] irqchip: crossbar: driver fixes

Sricharan,

On Monday 16 June 2014 07:23 AM, Sricharan R wrote:
> This series does some cleanups, fixes for handling two interrupts
> getting mapped twice to same crossbar and provides support for
> hardwired IRQ and crossbar definitions.
>
> On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10,
> 131, 132, 133 are direct wired to hardware blocks bypassing
> crossbar. This quirky implementation is *NOT* supposed to be the
> expectation of crossbar hardware usage. This series adds support
> to represent such hard-wired irqs through DT and avoid generic
> allocation/programming of crossbar in the driver.
>
> This way of supporting hard-wired irqs was a result of
> the below discussions.
> http://www.spinics.net/lists/arm-kernel/msg329946.html
>
> Based on 3.15 mainline.
>
> All the patches are available here
> [email protected]:Sricharanti/sricharan.git crossbar_updates
>
> The fixes series[1] earlier posted is merged in to this.
> [1] http://www.spinics.net/lists/arm-kernel/msg328273.html
>
> [V2] Merged the above series and rebased on 3.15 mainline
>
> [V3] Modified patch#3 to get irqs-skip properties from DT,
> merged path#8 for checkpatch warning to other relevant
> patches and fixed comments for other patches.
>
I scanned entire series again including your updates on Jason's
comments. All look good to my eyes.

Hopefully after this series now, we can actually enable the crossbar
support on those machines.

FWIW,
Acked-by: Santosh Shilimkar <[email protected]>

2014-06-17 08:56:54

by R, Sricharan

[permalink] [raw]
Subject: Re: [PATCH V3 00/16] irqchip: crossbar: driver fixes

On Monday 16 June 2014 07:34 PM, Santosh Shilimkar wrote:
> Sricharan,
>
> On Monday 16 June 2014 07:23 AM, Sricharan R wrote:
>> This series does some cleanups, fixes for handling two interrupts
>> getting mapped twice to same crossbar and provides support for
>> hardwired IRQ and crossbar definitions.
>>
>> On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10,
>> 131, 132, 133 are direct wired to hardware blocks bypassing
>> crossbar. This quirky implementation is *NOT* supposed to be the
>> expectation of crossbar hardware usage. This series adds support
>> to represent such hard-wired irqs through DT and avoid generic
>> allocation/programming of crossbar in the driver.
>>
>> This way of supporting hard-wired irqs was a result of
>> the below discussions.
>> http://www.spinics.net/lists/arm-kernel/msg329946.html
>>
>> Based on 3.15 mainline.
>>
>> All the patches are available here
>> [email protected]:Sricharanti/sricharan.git crossbar_updates
>>
>> The fixes series[1] earlier posted is merged in to this.
>> [1] http://www.spinics.net/lists/arm-kernel/msg328273.html
>>
>> [V2] Merged the above series and rebased on 3.15 mainline
>>
>> [V3] Modified patch#3 to get irqs-skip properties from DT,
>> merged path#8 for checkpatch warning to other relevant
>> patches and fixed comments for other patches.
>>
> I scanned entire series again including your updates on Jason's
> comments. All look good to my eyes.
>
> Hopefully after this series now, we can actually enable the crossbar
> support on those machines.
>
> FWIW,
> Acked-by: Santosh Shilimkar <[email protected]>
Thanks Santosh.

Regards,
Sricharan

2014-06-21 02:33:17

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH V3 03/16] irqchip: crossbar: introduce ti,irqs-skip to skip

Sricharan,

Your subject line seems truncated:

"irqchip: crossbar: introduce ti,irqs-skip to skip"

maybe "... Introduce DT property to skip hardwired irqs" ?

Also note that you need to correct the subject line for *every* patch in
the series wrt capitalization.

I don't mind correcting it when I apply it, provided that:

- the patch is otherwise ready
- I only have to do it once or twice for the series
- I never had a chance to ask since you created a rockstar patch series
the first time out of the gate (except for capitalization).

Once I've looked over the whole series, please resend with the subject
lines corrected.

On Mon, Jun 16, 2014 at 04:53:03PM +0530, Sricharan R wrote:
> From: Nishanth Menon <[email protected]>
>
> When, in the system due to varied reasons, interrupts might be unusable
> due to hardware behavior, but register maps do exist, then those interrupts
> should be skipped while mapping irq to crossbars.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Sricharan R <[email protected]>
> ---
> [V3] introduced ti,irqs-skip dt property to list the
> irqs to be skipped.
>
> .../devicetree/bindings/arm/omap/crossbar.txt | 4 ++++
> drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> index fb88585..cfcbd52 100644
> --- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> @@ -17,6 +17,10 @@ Required properties:
> so crossbar bar driver should not consider them as free
> lines.
>
> +Optional properties:
> +- ti,irqs-skip: This is similar to "ti,irqs-reserved", but are irq mappings
> + which are not supposed to be used for errata or other reasons(virtualization).

I would specifically mention SoC-specific hard-wiring of irqs here.
Also the fact that the hardwiring unexpectedly bypasses the crossbar.

> +
> Examples:
> crossbar_mpu: @4a020000 {
> compatible = "ti,irq-crossbar";

Please include a ti,irqs-skip example here.

> diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> index 51d4b87..27049de 100644
> --- a/drivers/irqchip/irq-crossbar.c
> +++ b/drivers/irqchip/irq-crossbar.c
> @@ -18,6 +18,7 @@
>
> #define IRQ_FREE -1
> #define IRQ_RESERVED -2
> +#define IRQ_SKIP -3
> #define GIC_IRQ_START 32
>
> /*
> @@ -160,6 +161,25 @@ static int __init crossbar_of_init(struct device_node *node)
> }
> }
>
> + /* Skip the ones marked as skip */

This comment is redundant, perhaps "Skip irqs hardwired to bypass the
crossbar."?

> + irqsr = of_get_property(node, "ti,irqs-skip", &size);
> + if (irqsr) {
> + size /= sizeof(__be32);
> +
> + for (i = 0; i < size; i++) {
> + of_property_read_u32_index(node,
> + "ti,irqs-skip",
> + i, &entry);
> + if (entry > max) {
> + pr_err("Invalid skip entry\n");
> + ret = -EINVAL;
> + goto err3;
> + }
> + cb->irq_map[entry] = IRQ_SKIP;
> + }
> + }
> +
> +
> cb->register_offsets = kzalloc(max * sizeof(int), GFP_KERNEL);
> if (!cb->register_offsets)
> goto err3;

thx,

Jason.

2014-06-21 02:57:38

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH V3 16/16] irqchip: crossbar: allow for quirky hardware with direct hardwiring of GIC

On Mon, Jun 16, 2014 at 04:53:16PM +0530, Sricharan R wrote:
> From: Nishanth Menon <[email protected]>
>
> On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10, 131,
> 132, 133 are direct wired to hardware blocks bypassing crossbar.
> This quirky implementation is *NOT* supposed to be the expectation
> of crossbar hardware usage. However, these are already marked in our
> description of the hardware with SKIP and RESERVED where appropriate.
>
> Unfortunately, we need to be able to refer to these hardwired IRQs.
> So, to request these, crossbar driver can use the existing information
> from it's table that these SKIP/RESERVED maps are direct wired sources
> and generic allocation/programming of crossbar should be avoided.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> Signed-off-by: Sricharan R <[email protected]>
> ---
> .../devicetree/bindings/arm/omap/crossbar.txt | 12 ++++++++++--
> drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++--
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> index 8210ea4..438ccab 100644
> --- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
> @@ -42,8 +42,10 @@ Documentation/devicetree/bindings/arm/gic.txt for further details.
>
> An interrupt consumer on an SoC using crossbar will use:
> interrupts = <GIC_SPI request_number interrupt_level>
> -request number shall be between 0 to that described by
> -"ti,max-crossbar-sources"
> +When the request number is between 0 to that described by
> +"ti,max-crossbar-sources", it is assumed to be a crossbar mapping. If the
> +request_number is greater than "ti,max-crossbar-sources", then it is mapped as a
> +quirky hardware mapping direct to GIC.
>
> Example:
> device_x@0x4a023000 {
> @@ -51,3 +53,9 @@ Example:
> interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> ...
> };
> +
> + device_y@0x4a033000 {
> + /* Direct mapped GIC SPI 1 used */
> + interrupts = <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>;

Ideally, I'd like to see a macro here so that it's clear that we crossed
a magic threshold. eg:

#define MAX_SOURCES 400
#define DIRECT_IRQ(irq) (MAX_SOURCES + irq)
...
interrupts = <GIC_SPI DIRECT_IRQ(1) IRQ_TYPE_LEVEL_HIGH>;

and, then:

ti,max-crossbar-sources = <MAX_SOURCES>;

> + ...
> + };
> diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> index ef613c4..fff6218 100644
> --- a/drivers/irqchip/irq-crossbar.c
> +++ b/drivers/irqchip/irq-crossbar.c
> @@ -86,8 +86,13 @@ static inline int allocate_free_irq(int cb_no)
>
> static inline bool needs_crossbar_write(irq_hw_number_t hw)
> {
> - if (hw > GIC_IRQ_START)
> - return true;
> + int cb_no;
> +
> + if (hw > GIC_IRQ_START) {
> + cb_no = cb->irq_map[hw - GIC_IRQ_START];
> + if (cb_no != IRQ_RESERVED && cb_no != IRQ_SKIP)
> + return true;
> + }
>
> return false;
> }
> @@ -130,8 +135,19 @@ static int crossbar_domain_xlate(struct irq_domain *d,
> {
> int ret;
> int req_num = intspec[1];
> + int direct_map_num;
>
> if (req_num >= cb->max_crossbar_sources) {
> + direct_map_num = req_num - cb->max_crossbar_sources;
> + if (direct_map_num < cb->int_max) {
> + ret = cb->irq_map[direct_map_num];
> + if (ret == IRQ_RESERVED || ret == IRQ_SKIP) {
> + /* We use the interrupt num as h/w irq num */
> + ret = direct_map_num;
> + goto found;
> + }
> + }
> +
> pr_err("%s: requested crossbar number %d > max %d\n",
> __func__, req_num, cb->max_crossbar_sources);
> return -EINVAL;

thx,

Jason.

2014-06-23 07:23:14

by R, Sricharan

[permalink] [raw]
Subject: Re: [PATCH V3 03/16] irqchip: crossbar: introduce ti,irqs-skip to skip

On Saturday 21 June 2014 08:03 AM, Jason Cooper wrote:
> Sricharan,
>
> Your subject line seems truncated:
>
> "irqchip: crossbar: introduce ti,irqs-skip to skip"
>
> maybe "... Introduce DT property to skip hardwired irqs" ?
>
> Also note that you need to correct the subject line for *every* patch in
> the series wrt capitalization.
>
> I don't mind correcting it when I apply it, provided that:
>
ha, i think this got truncated unintentionally. Sorry will fix this.
> - the patch is otherwise ready
> - I only have to do it once or twice for the series
> - I never had a chance to ask since you created a rockstar patch series
> the first time out of the gate (except for capitalization).
>
> Once I've looked over the whole series, please resend with the subject
> lines corrected.
>
Ok. I will look for your comments on the rest of the patches and
resend with capitalization fix said above.

> On Mon, Jun 16, 2014 at 04:53:03PM +0530, Sricharan R wrote:
>> From: Nishanth Menon <[email protected]>
>>
>> When, in the system due to varied reasons, interrupts might be unusable
>> due to hardware behavior, but register maps do exist, then those interrupts
>> should be skipped while mapping irq to crossbars.
>>
>> Signed-off-by: Nishanth Menon <[email protected]>
>> Signed-off-by: Sricharan R <[email protected]>
>> ---
>> [V3] introduced ti,irqs-skip dt property to list the
>> irqs to be skipped.
>>
>> .../devicetree/bindings/arm/omap/crossbar.txt | 4 ++++
>> drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> index fb88585..cfcbd52 100644
>> --- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> @@ -17,6 +17,10 @@ Required properties:
>> so crossbar bar driver should not consider them as free
>> lines.
>>
>> +Optional properties:
>> +- ti,irqs-skip: This is similar to "ti,irqs-reserved", but are irq mappings
>> + which are not supposed to be used for errata or other reasons(virtualization).
>
> I would specifically mention SoC-specific hard-wiring of irqs here.
> Also the fact that the hardwiring unexpectedly bypasses the crossbar.
ok, that will be more easily understandable and will add that.
>
>> +
>> Examples:
>> crossbar_mpu: @4a020000 {
>> compatible = "ti,irq-crossbar";
>
> Please include a ti,irqs-skip example here.
>
ok.
>> diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
>> index 51d4b87..27049de 100644
>> --- a/drivers/irqchip/irq-crossbar.c
>> +++ b/drivers/irqchip/irq-crossbar.c
>> @@ -18,6 +18,7 @@
>>
>> #define IRQ_FREE -1
>> #define IRQ_RESERVED -2
>> +#define IRQ_SKIP -3
>> #define GIC_IRQ_START 32
>>
>> /*
>> @@ -160,6 +161,25 @@ static int __init crossbar_of_init(struct device_node *node)
>> }
>> }
>>
>> + /* Skip the ones marked as skip */
>
> This comment is redundant, perhaps "Skip irqs hardwired to bypass the
> crossbar."?
ok, will change this.

Regards,
Sricharan

2014-06-23 07:25:18

by R, Sricharan

[permalink] [raw]
Subject: Re: [PATCH V3 16/16] irqchip: crossbar: allow for quirky hardware with direct hardwiring of GIC

Hi Jason,

On Saturday 21 June 2014 08:27 AM, Jason Cooper wrote:
> On Mon, Jun 16, 2014 at 04:53:16PM +0530, Sricharan R wrote:
>> From: Nishanth Menon <[email protected]>
>>
>> On certain platforms such as DRA7, SPIs 0, 1, 2, 3, 5, 6, 10, 131,
>> 132, 133 are direct wired to hardware blocks bypassing crossbar.
>> This quirky implementation is *NOT* supposed to be the expectation
>> of crossbar hardware usage. However, these are already marked in our
>> description of the hardware with SKIP and RESERVED where appropriate.
>>
>> Unfortunately, we need to be able to refer to these hardwired IRQs.
>> So, to request these, crossbar driver can use the existing information
>> from it's table that these SKIP/RESERVED maps are direct wired sources
>> and generic allocation/programming of crossbar should be avoided.
>>
>> Signed-off-by: Nishanth Menon <[email protected]>
>> Signed-off-by: Sricharan R <[email protected]>
>> ---
>> .../devicetree/bindings/arm/omap/crossbar.txt | 12 ++++++++++--
>> drivers/irqchip/irq-crossbar.c | 20 ++++++++++++++++++--
>> 2 files changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/crossbar.txt b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> index 8210ea4..438ccab 100644
>> --- a/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> +++ b/Documentation/devicetree/bindings/arm/omap/crossbar.txt
>> @@ -42,8 +42,10 @@ Documentation/devicetree/bindings/arm/gic.txt for further details.
>>
>> An interrupt consumer on an SoC using crossbar will use:
>> interrupts = <GIC_SPI request_number interrupt_level>
>> -request number shall be between 0 to that described by
>> -"ti,max-crossbar-sources"
>> +When the request number is between 0 to that described by
>> +"ti,max-crossbar-sources", it is assumed to be a crossbar mapping. If the
>> +request_number is greater than "ti,max-crossbar-sources", then it is mapped as a
>> +quirky hardware mapping direct to GIC.
>>
>> Example:
>> device_x@0x4a023000 {
>> @@ -51,3 +53,9 @@ Example:
>> interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
>> ...
>> };
>> +
>> + device_y@0x4a033000 {
>> + /* Direct mapped GIC SPI 1 used */
>> + interrupts = <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>;
>
> Ideally, I'd like to see a macro here so that it's clear that we crossed
> a magic threshold. eg:
>
> #define MAX_SOURCES 400
> #define DIRECT_IRQ(irq) (MAX_SOURCES + irq)
> ...
> interrupts = <GIC_SPI DIRECT_IRQ(1) IRQ_TYPE_LEVEL_HIGH>;
>
> and, then:
>
> ti,max-crossbar-sources = <MAX_SOURCES>;
>
Ok, thats more good for readability. Will add that macro then.

Regards,
Sricharan