[3/5] irqchip: crossbar: Skip some irqs from getting mapped to crossbar

Commit Message

R Sricharan May 5, 2014, 2:18 p.m. UTC

From: Nishanth Menon <[e-mail]>

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 <[e-mail]>
Signed-off-by: Sricharan R <[e-mail]>
---
 drivers/irqchip/irq-crossbar.c |   47 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 4 deletions(-)

Comments

Joel Fernandes May 8, 2014, 7:24 p.m. UTC | #1

On 05/05/2014 09:18 AM, Sricharan R wrote:
> From: Nishanth Menon <[e-mail]>
> 
> 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.
> 

Just wondering, instead of hardcoding this data in the code, and
introducing additional flags (IRQ_SKIP), why not just put these GIC IRQs
in the ti,irq-reserved property in DTS for platforms where such IRQs are
not usable. That way you're skipping these IRQs anyway.

Also that would avoid adding more hard coded data for future SoCs into
the source for such IRQs that must be skipped, and also reduces LOC.

thanks,

-Joel


> Signed-off-by: Nishanth Menon <[e-mail]>
> Signed-off-by: Sricharan R <[e-mail]>
> ---
>  drivers/irqchip/irq-crossbar.c |   47 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
> index 51d4b87..847f6e3 100644
> --- a/drivers/irqchip/irq-crossbar.c
> +++ b/drivers/irqchip/irq-crossbar.c
> @@ -13,11 +13,13 @@
>  #include <linux/io.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/irqchip/arm-gic.h>
>  
>  #define IRQ_FREE	-1
>  #define IRQ_RESERVED	-2
> +#define IRQ_SKIP	-3
>  #define GIC_IRQ_START	32
>  
>  /*
> @@ -34,6 +36,16 @@ struct crossbar_device {
>  	void (*write) (int, int);
>  };
>  
> +/**
> + * struct crossbar_data: Platform specific data
> + * @irqs_unused: array of irqs that cannot be used because of hw erratas
> + * @size: size of the irqs_unused array
> + */
> +struct crossbar_data {
> +	const uint *irqs_unused;
> +	const uint size;
> +};
> +
>  static struct crossbar_device *cb;
>  
>  static inline void crossbar_writel(int irq_no, int cb_no)
> @@ -119,10 +131,12 @@ const struct irq_domain_ops routable_irq_domain_ops = {
>  	.xlate = crossbar_domain_xlate
>  };
>  
> -static int __init crossbar_of_init(struct device_node *node)
> +static int __init crossbar_of_init(struct device_node *node,
> +				   const struct crossbar_data *data)
>  {
>  	int i, size, max, reserved = 0, entry;
>  	const __be32 *irqsr;
> +	const int *irqsk = NULL;
>  
>  	cb = kzalloc(sizeof(*cb), GFP_KERNEL);
>  
> @@ -194,6 +208,22 @@ static int __init crossbar_of_init(struct device_node *node)
>  		reserved += size;
>  	}
>  
> +	/* Skip the ones marked as unused */
> +	if (data) {
> +		irqsk = data->irqs_unused;
> +		size = data->size;
> +
> +		for (i = 0; i < size; i++) {
> +			entry = irqsk[i];
> +
> +			if (entry > max) {
> +				pr_err("Invalid skip entry\n");
> +				goto err3;
> +			}
> +			cb->irq_map[entry] = IRQ_SKIP;
> +		}
> +	}
> +
>  	register_routable_domain_ops(&routable_irq_domain_ops);
>  	return 0;
>  
> @@ -208,18 +238,27 @@ err1:
>  	return -ENOMEM;
>  }
>  
> +/* irq number 10 cannot be used because of hw bug */
> +int dra_irqs_unused[] = { 10 };
> +struct crossbar_data cb_dra_data = { dra_irqs_unused,
> +				     ARRAY_SIZE(dra_irqs_unused) };
> +
>  static const struct of_device_id crossbar_match[] __initconst = {
> -	{ .compatible = "ti,irq-crossbar" },
> +	{ .compatible = "ti,irq-crossbar", .data = &cb_dra_data },
>  	{}
>  };
>  
>  int __init irqcrossbar_init(void)
>  {
>  	struct device_node *np;
> -	np = of_find_matching_node(NULL, crossbar_match);
> +	const struct of_device_id *of_id;
> +	const struct crossbar_data *cdata;
> +
> +	np = of_find_matching_node_and_match(NULL, crossbar_match, &of_id);
>  	if (!np)
>  		return -ENODEV;
>  
> -	crossbar_of_init(np);
> +	cdata = of_id->data;
> +	crossbar_of_init(np, cdata);
>  	return 0;
>  }
>

Nishanth Menon May 8, 2014, 8:37 p.m. UTC | #2

On 14:24-20140508, Joel Fernandes wrote:
> On 05/05/2014 09:18 AM, Sricharan R wrote:
> > From: Nishanth Menon <[e-mail]>
> > 
> > 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.
> > 
> 
> Just wondering, instead of hardcoding this data in the code, and
> introducing additional flags (IRQ_SKIP), why not just put these GIC IRQs
> in the ti,irq-reserved property in DTS for platforms where such IRQs are
> not usable. That way you're skipping these IRQs anyway.
> 
> Also that would avoid adding more hard coded data for future SoCs into
> the source for such IRQs that must be skipped, and also reduces LOC.
> 

Good question - lets try to explain the hardware a little here ->
obviously a driver that cannot use the hardware is useless compared to
reducing LOC count ;).. and apologies about the long reply..

Basic understanding:
GIC has 160 SPIs and number of hardware block interrupt sources is around or
more than 400. So, in comes crossbar - which is basically a mapper by
allowing us to select an hardware block interrupt source (identified as
crossbar_number or cb_no in code). So all we have to do is to write to a
register in crossbar corresponding to GIC and viola, we now routed the
interrupt source to a GIC interrupt of our choice. At least the
Specification reads so.... until you drill down to the details.

A) You have 160 SPI GIC, and 152 crossbar registers. So, you have 8 GIC SPI
interrupts that are hardwired. the reserved mapping basically marks
these to indicate that we dont have registers. Example: 0 1 2 3 5
6 131 and 132
	- Limitation today - if you want to use PMU for CPU0, SPI
	interrupt is 131, then if you define, in dts:
	interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>
	driver assumes it is crossbar number 131(reserved), Similarly:
	GIC CS_CTI_MPU_C0_IRQ (SPI 1) is ELM_IRQ (crossbar 1)
	GIC CS_CTI_MPU_C1_IRQ (SPI 2) is EXT_SYS_IRQ_1 (crossbar 2)
	GIC MPU_CLUSTER_IRQ_AXIERR (SPI 3) is reserved (crossbar 3)
	GIC WD_TIMER_MPU_C0_IRQ_WARN (SPI 5) is L3_MAIN_IRQ_APP_ERR (crossbar 5)
	GIC WD_TIMER_MPU_C1_IRQ_WARN (SPI 6) is PRM_IRQ_MPU (crossbar 6)
	GIC MPU_CLUSTER_IRQ_PMU_C0 (SPI 131) is reserved (crossbar 131)
	GIC MPU_CLUSTER_IRQ_PMU_C1 (SPI 132) is reserved (crossbar 132)

	As of today, we cannot differentiate in DTS if it is one of
	these "direct map" interrupts we are requesting or crossbar
	number we are requesting.

B) among the 152 cross bar registers, you have three sets:
B.1) The ones like Crossbar register 1 which maps to SPI4 - no problem -
     you write the crossbar number you want to map, bingo, job done.
     - The driver works brilliantly here. and this is true for 148 GIC
     SPIs.
B.2) The ones like 10 139 140 - these are interesting, because we have
     crossbar registers corresponding to these, However writing anything
     to them has no impact - at least 10 is confirmed to have been
     hardwired to L3_APP_IRQ (but not documented), we are trying to get
     explanations for 139 and 140. - but there is strong indication
     based on testing performed that the registers are NOPs and GIC is
     hardwired in.

     I had originally discovered 10, but only a day or so back did we
     understand what is going on, others we dont know yet.
B.3) 133 is a variation to B.2 - There is an magical efuse register
    which controls if the GIC is hardwired or not. when the efuse bit is
    0, it behaves like B.1(program and it works), but almost all silicon
    have it set to "hardwired mode" :(

The following you wont find in any TRM, and is based on tests performed
during the last few days - primarily meant to illustrate this.

                      MPU Crossbar                  
                      152 registers                 
   +-------+         +------+                       
   |       |    +----+C1    |                       
   | PPI.. |    |    +------+                       
   | 0..32 |    | <--+C2    |                       
   |       |    |    +------+     +------------+    
   +-------+    | +--+C5    |     |            +---+
   |  SPI1 |    | |  +------+   <-+ L3 APP IRQ |   |
   |       |    | |  |      |     ++-------+---+   |
   +-------+    | |  |      |      +-------+       |
   |  SPI3 |    | |  +------+      | CPU0  |       |
   |       |    | |  |      |      | PMU   +----+  |
   +-------+    | |  +------+      +-------+    |  |
   | SPI4  | <--+ |  |      |                   |  |
   |       |      |  |      |                   |  |
   +-------+      |  |      |    +---------+    |  |
   | SPI10 | <----+  |      |    | External|    |  |
+> |       |         |      |    | NMI     |    |  |
|  +-------+         +------+    +-+-----+-++   |  |
|  | SPI131|         |      |      +-----+  |   |  |
|  |       | <+      +------+      | Efuse  |   |  |
|  +-------+  |      |C126  | <--+-----+-+  |   |  |
|  | SPI133| <---+   +------+    +-----+    |   |  |
|  +-------+  |  | +-+C132  |    |CPU0 |    |   |  |
|  | SPI139| <-----+ +------+    |WDT  |    |   |  |
|  +-------+  |  | | +------+    +--+--+    |   |  |
|  | ..... |  |  | |                |       |   |  |
|  +-------+  |  | +----------------+       |   |  |
|  | SPI159|  |  |                          |   |  |
|  +-------+  |  +--------------------------+   |  |
|             |                                 |  |
|    GIC      +---------------------------------+  |
|  160 SPI                                         |
|                                                  |
+--------------------------------------------------+


So, to answer your question - I hope this explains skip and reserved.
Now, we happily can handle case B.1 (148 SPI interrupts) - However,

The reason I requested this series to be blocked is:
a) We dont completely (yet) have explanation about hardware for B.2 139
   and 140.
b) we definitely need to be able to request the interrupts of A, B.2,
   B.3 - and our framework as it stands right now fails.

NOTE:
obviously we claim dra7 compatibility. dra742 and 744 seem similar - but
we dont have confirmation for the same yet. following device tree
maintainer recommendations of having dts compatibility closely match
with SoC behavior. yeah, we could make the driver too generic and move
everything to dts.. but that does not seem to be the way we do things with dt.

Joel A Fernandes May 8, 2014, 10:43 p.m. UTC | #3

On Thu, May 8, 2014 at 3:37 PM, Nishanth Menon <[e-mail]> wrote:
> On 14:24-20140508, Joel Fernandes wrote:
>> On 05/05/2014 09:18 AM, Sricharan R wrote:
>> > From: Nishanth Menon <[e-mail]>
>> >
>> > 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.
>> >
>>
>> Just wondering, instead of hardcoding this data in the code, and
>> introducing additional flags (IRQ_SKIP), why not just put these GIC IRQs
>> in the ti,irq-reserved property in DTS for platforms where such IRQs are
>> not usable. That way you're skipping these IRQs anyway.
>>
>> Also that would avoid adding more hard coded data for future SoCs into
>> the source for such IRQs that must be skipped, and also reduces LOC.
>>
>
> Good question - lets try to explain the hardware a little here ->
> obviously a driver that cannot use the hardware is useless compared to
> reducing LOC count ;).. and apologies about the long reply..
>
> Basic understanding:
> GIC has 160 SPIs and number of hardware block interrupt sources is around or
> more than 400. So, in comes crossbar - which is basically a mapper by
> allowing us to select an hardware block interrupt source (identified as
> crossbar_number or cb_no in code). So all we have to do is to write to a
> register in crossbar corresponding to GIC and viola, we now routed the
> interrupt source to a GIC interrupt of our choice. At least the
> Specification reads so.... until you drill down to the details.

Thanks for the long explanation and the diagrams!

Yes, I feel there is no other way and with so many HW bugs, I think it
makes sense to make it a real irqchip driver.

Further since not everything goes through the crossbar and some are
direct mapped like your diagram, the correct fix is probably making it
an irqchip and doing the interrupt controller parenting correctly in
DT.

That would take care of A), because users of such direct mapped
interrupts will go through the GIC interrupt controller directly.

It will also take care of B), because if writing to cross bar has no
effect for a particular IRQ, or if those IRQs are hard-wired to
something, as you said, then that something should go through the GIC
directly.

I can try to whip up something like this if it makes sense, let me know...

thanks,

-Joel


>
> A) You have 160 SPI GIC, and 152 crossbar registers. So, you have 8 GIC SPI
> interrupts that are hardwired. the reserved mapping basically marks
> these to indicate that we dont have registers. Example: 0 1 2 3 5
> 6 131 and 132
>         - Limitation today - if you want to use PMU for CPU0, SPI
>         interrupt is 131, then if you define, in dts:
>         interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>
>         driver assumes it is crossbar number 131(reserved), Similarly:
>         GIC CS_CTI_MPU_C0_IRQ (SPI 1) is ELM_IRQ (crossbar 1)
>         GIC CS_CTI_MPU_C1_IRQ (SPI 2) is EXT_SYS_IRQ_1 (crossbar 2)
>         GIC MPU_CLUSTER_IRQ_AXIERR (SPI 3) is reserved (crossbar 3)
>         GIC WD_TIMER_MPU_C0_IRQ_WARN (SPI 5) is L3_MAIN_IRQ_APP_ERR (crossbar 5)
>         GIC WD_TIMER_MPU_C1_IRQ_WARN (SPI 6) is PRM_IRQ_MPU (crossbar 6)
>         GIC MPU_CLUSTER_IRQ_PMU_C0 (SPI 131) is reserved (crossbar 131)
>         GIC MPU_CLUSTER_IRQ_PMU_C1 (SPI 132) is reserved (crossbar 132)
>
>         As of today, we cannot differentiate in DTS if it is one of
>         these "direct map" interrupts we are requesting or crossbar
>         number we are requesting.
>
> B) among the 152 cross bar registers, you have three sets:
> B.1) The ones like Crossbar register 1 which maps to SPI4 - no problem -
>      you write the crossbar number you want to map, bingo, job done.
>      - The driver works brilliantly here. and this is true for 148 GIC
>      SPIs.
> B.2) The ones like 10 139 140 - these are interesting, because we have
>      crossbar registers corresponding to these, However writing anything
>      to them has no impact - at least 10 is confirmed to have been
>      hardwired to L3_APP_IRQ (but not documented), we are trying to get
>      explanations for 139 and 140. - but there is strong indication
>      based on testing performed that the registers are NOPs and GIC is
>      hardwired in.
>
>      I had originally discovered 10, but only a day or so back did we
>      understand what is going on, others we dont know yet.
> B.3) 133 is a variation to B.2 - There is an magical efuse register
>     which controls if the GIC is hardwired or not. when the efuse bit is
>     0, it behaves like B.1(program and it works), but almost all silicon
>     have it set to "hardwired mode" :(
>
> The following you wont find in any TRM, and is based on tests performed
> during the last few days - primarily meant to illustrate this.
>
>                       MPU Crossbar
>                       152 registers
>    +-------+         +------+
>    |       |    +----+C1    |
>    | PPI.. |    |    +------+
>    | 0..32 |    | <--+C2    |
>    |       |    |    +------+     +------------+
>    +-------+    | +--+C5    |     |            +---+
>    |  SPI1 |    | |  +------+   <-+ L3 APP IRQ |   |
>    |       |    | |  |      |     ++-------+---+   |
>    +-------+    | |  |      |      +-------+       |
>    |  SPI3 |    | |  +------+      | CPU0  |       |
>    |       |    | |  |      |      | PMU   +----+  |
>    +-------+    | |  +------+      +-------+    |  |
>    | SPI4  | <--+ |  |      |                   |  |
>    |       |      |  |      |                   |  |
>    +-------+      |  |      |    +---------+    |  |
>    | SPI10 | <----+  |      |    | External|    |  |
> +> |       |         |      |    | NMI     |    |  |
> |  +-------+         +------+    +-+-----+-++   |  |
> |  | SPI131|         |      |      +-----+  |   |  |
> |  |       | <+      +------+      | Efuse  |   |  |
> |  +-------+  |      |C126  | <--+-----+-+  |   |  |
> |  | SPI133| <---+   +------+    +-----+    |   |  |
> |  +-------+  |  | +-+C132  |    |CPU0 |    |   |  |
> |  | SPI139| <-----+ +------+    |WDT  |    |   |  |
> |  +-------+  |  | | +------+    +--+--+    |   |  |
> |  | ..... |  |  | |                |       |   |  |
> |  +-------+  |  | +----------------+       |   |  |
> |  | SPI159|  |  |                          |   |  |
> |  +-------+  |  +--------------------------+   |  |
> |             |                                 |  |
> |    GIC      +---------------------------------+  |
> |  160 SPI                                         |
> |                                                  |
> +--------------------------------------------------+
>
>
> So, to answer your question - I hope this explains skip and reserved.
> Now, we happily can handle case B.1 (148 SPI interrupts) - However,
>
> The reason I requested this series to be blocked is:
> a) We dont completely (yet) have explanation about hardware for B.2 139
>    and 140.
> b) we definitely need to be able to request the interrupts of A, B.2,
>    B.3 - and our framework as it stands right now fails.
>
> NOTE:
> obviously we claim dra7 compatibility. dra742 and 744 seem similar - but
> we dont have confirmation for the same yet. following device tree
> maintainer recommendations of having dts compatibility closely match
> with SoC behavior. yeah, we could make the driver too generic and move
> everything to dts.. but that does not seem to be the way we do things with dt.
>
> --
> Regards,
> Nishanth Menon
>
> _______________________________________________
> linux-arm-kernel mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Santosh Shilimkar May 8, 2014, 11:05 p.m. UTC | #4

On Thursday 08 May 2014 06:43 PM, Joel Fernandes wrote:
> On Thu, May 8, 2014 at 3:37 PM, Nishanth Menon <[e-mail]> wrote:
>> On 14:24-20140508, Joel Fernandes wrote:
>>> On 05/05/2014 09:18 AM, Sricharan R wrote:
>>>> From: Nishanth Menon <[e-mail]>
>>>>
>>>> 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.
>>>>
>>>
>>> Just wondering, instead of hardcoding this data in the code, and
>>> introducing additional flags (IRQ_SKIP), why not just put these GIC IRQs
>>> in the ti,irq-reserved property in DTS for platforms where such IRQs are
>>> not usable. That way you're skipping these IRQs anyway.
>>>
>>> Also that would avoid adding more hard coded data for future SoCs into
>>> the source for such IRQs that must be skipped, and also reduces LOC.
>>>
>>
>> Good question - lets try to explain the hardware a little here ->
>> obviously a driver that cannot use the hardware is useless compared to
>> reducing LOC count ;).. and apologies about the long reply..
>>
>> Basic understanding:
>> GIC has 160 SPIs and number of hardware block interrupt sources is around or
>> more than 400. So, in comes crossbar - which is basically a mapper by
>> allowing us to select an hardware block interrupt source (identified as
>> crossbar_number or cb_no in code). So all we have to do is to write to a
>> register in crossbar corresponding to GIC and viola, we now routed the
>> interrupt source to a GIC interrupt of our choice. At least the
>> Specification reads so.... until you drill down to the details.
> 
> Thanks for the long explanation and the diagrams!
> 
> Yes, I feel there is no other way and with so many HW bugs, I think it
> makes sense to make it a real irqchip driver.
>
It doesn't because its not an irqchip. 
 
> Further since not everything goes through the crossbar and some are
> direct mapped like your diagram, the correct fix is probably making it
> an irqchip and doing the interrupt controller parenting correctly in
> DT.
> 
> That would take care of A), because users of such direct mapped
> interrupts will go through the GIC interrupt controller directly.
> 
> It will also take care of B), because if writing to cross bar has no
> effect for a particular IRQ, or if those IRQs are hard-wired to
> something, as you said, then that something should go through the GIC
> directly.
> 
> I can try to whip up something like this if it makes sense, let me know...
> 
I have been ignoring this series considering they were just fixes
but you comments are like re-inventing wheel. Please read all
the old threads and comments from Thomas and me on why we took
approach and why it is not an irqchip. There is no need to complicate
it further.


Regards,
Santosh

Joel A Fernandes May 9, 2014, 12:13 a.m. UTC | #5

On Thu, May 8, 2014 at 6:05 PM, Santosh Shilimkar
<[e-mail]> wrote:
[..]
>> Further since not everything goes through the crossbar and some are
>> direct mapped like your diagram, the correct fix is probably making it
>> an irqchip and doing the interrupt controller parenting correctly in
>> DT.
>>
>> That would take care of A), because users of such direct mapped
>> interrupts will go through the GIC interrupt controller directly.
>>
>> It will also take care of B), because if writing to cross bar has no
>> effect for a particular IRQ, or if those IRQs are hard-wired to
>> something, as you said, then that something should go through the GIC
>> directly.
>>
>> I can try to whip up something like this if it makes sense, let me know...
>>
> I have been ignoring this series considering they were just fixes
> but you comments are like re-inventing wheel. Please read all
> the old threads and comments from Thomas and me on why we took
> approach and why it is not an irqchip. There is no need to complicate
> it further.

Are you talking about the discussion on this thread?
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/194318.html

I didn't really get a sense that there was a common agreement that
irqchip is not the way to go there. I can stand corrected if there was
a common consensus that irqchip is not the right solution (with any
specific comments why). There was a concern in the thread that making
it irqchip doesn't help dma reuse the infrastructure, but that concern
seems moot now that the driver is proposed to live in drivers/irqchip.

Further, I don't think my comments are re-inventing anything because
the brand-new bugs that are being found now weren't found then and my
comments were more related to these bugs.

As for complexity, it appears there will be signficant hacks coming up
that are required to handle the corner cases by adding more lists and
dt-properties. So its already on a complicated path.. Even with such
hacks, Nishanth pointed out that the implementation can break. Its
already like a deck of cards in my opinion and complicates things for
everyone using it. You said you hadn't gone through the fixes in this
series, you should go over them and see the new lists added to skip
and reserve various things.

thanks,

  -Joel

Santosh Shilimkar May 9, 2014, 12:25 a.m. UTC | #6

On Thursday 08 May 2014 08:13 PM, Joel Fernandes wrote:
> On Thu, May 8, 2014 at 6:05 PM, Santosh Shilimkar
> <[e-mail]> wrote:
> [..]
>>> Further since not everything goes through the crossbar and some are
>>> direct mapped like your diagram, the correct fix is probably making it
>>> an irqchip and doing the interrupt controller parenting correctly in
>>> DT.
>>>
>>> That would take care of A), because users of such direct mapped
>>> interrupts will go through the GIC interrupt controller directly.
>>>
>>> It will also take care of B), because if writing to cross bar has no
>>> effect for a particular IRQ, or if those IRQs are hard-wired to
>>> something, as you said, then that something should go through the GIC
>>> directly.
>>>
>>> I can try to whip up something like this if it makes sense, let me know...
>>>
>> I have been ignoring this series considering they were just fixes
>> but you comments are like re-inventing wheel. Please read all
>> the old threads and comments from Thomas and me on why we took
>> approach and why it is not an irqchip. There is no need to complicate
>> it further.
> 
> Are you talking about the discussion on this thread?
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/194318.html
> 
> I didn't really get a sense that there was a common agreement that
> irqchip is not the way to go there. I can stand corrected if there was
> a common consensus that irqchip is not the right solution (with any
> specific comments why). There was a concern in the thread that making
> it irqchip doesn't help dma reuse the infrastructure, but that concern
> seems moot now that the driver is proposed to live in drivers/irqchip.
> 
Obviously you haven't read all the threads... Please read [1]. There
was a reason I said read *all* the threads. Because anyone who looks
at this hardware IP block thinks it can be irqchip. You are not the
first one who said that.

The concern was really not where the code resides but what the actual
hardware is and how can it fit into Linux. The whole reason I was
actually against irqhcip from beginning of crossbar series was the
hardware is not irqchip rather just a router. Thomas the formally
NAKed that approach on thread [1]. If there are bugs, doesn't mean
we can make fit the hardware into some subsystem where it can't be
described.

Regards,
Santosh
[1] https://lkml.org/lkml/2013/9/13/413

Joel A Fernandes May 9, 2014, 4:22 a.m. UTC | #7

On Thu, May 8, 2014 at 7:25 PM, Santosh Shilimkar
<[e-mail]> wrote:
[..]
> The concern was really not where the code resides but what the actual
> hardware is and how can it fit into Linux. The whole reason I was
> actually against irqhcip from beginning of crossbar series was the
> hardware is not irqchip rather just a router. Thomas the formally
> NAKed that approach on thread [1]. If there are bugs, doesn't mean
> we can make fit the hardware into some subsystem where it can't be
> described.
>
> Regards,
> Santosh
> [1] https://lkml.org/lkml/2013/9/13/413
>

Ok, thanks for pointing to the post.

Patch

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 51d4b87..847f6e3 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -13,11 +13,13 @@ 
 #include <linux/io.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/irqchip/arm-gic.h>
 
 #define IRQ_FREE	-1
 #define IRQ_RESERVED	-2
+#define IRQ_SKIP	-3
 #define GIC_IRQ_START	32
 
 /*
@@ -34,6 +36,16 @@  struct crossbar_device {
 	void (*write) (int, int);
 };
 
+/**
+ * struct crossbar_data: Platform specific data
+ * @irqs_unused: array of irqs that cannot be used because of hw erratas
+ * @size: size of the irqs_unused array
+ */
+struct crossbar_data {
+	const uint *irqs_unused;
+	const uint size;
+};
+
 static struct crossbar_device *cb;
 
 static inline void crossbar_writel(int irq_no, int cb_no)
@@ -119,10 +131,12 @@  const struct irq_domain_ops routable_irq_domain_ops = {
 	.xlate = crossbar_domain_xlate
 };
 
-static int __init crossbar_of_init(struct device_node *node)
+static int __init crossbar_of_init(struct device_node *node,
+				   const struct crossbar_data *data)
 {
 	int i, size, max, reserved = 0, entry;
 	const __be32 *irqsr;
+	const int *irqsk = NULL;
 
 	cb = kzalloc(sizeof(*cb), GFP_KERNEL);
 
@@ -194,6 +208,22 @@  static int __init crossbar_of_init(struct device_node *node)
 		reserved += size;
 	}
 
+	/* Skip the ones marked as unused */
+	if (data) {
+		irqsk = data->irqs_unused;
+		size = data->size;
+
+		for (i = 0; i < size; i++) {
+			entry = irqsk[i];
+
+			if (entry > max) {
+				pr_err("Invalid skip entry\n");
+				goto err3;
+			}
+			cb->irq_map[entry] = IRQ_SKIP;
+		}
+	}
+
 	register_routable_domain_ops(&routable_irq_domain_ops);
 	return 0;
 
@@ -208,18 +238,27 @@  err1:
 	return -ENOMEM;
 }
 
+/* irq number 10 cannot be used because of hw bug */
+int dra_irqs_unused[] = { 10 };
+struct crossbar_data cb_dra_data = { dra_irqs_unused,
+				     ARRAY_SIZE(dra_irqs_unused) };
+
 static const struct of_device_id crossbar_match[] __initconst = {
-	{ .compatible = "ti,irq-crossbar" },
+	{ .compatible = "ti,irq-crossbar", .data = &cb_dra_data },
 	{}
 };
 
 int __init irqcrossbar_init(void)
 {
 	struct device_node *np;
-	np = of_find_matching_node(NULL, crossbar_match);
+	const struct of_device_id *of_id;
+	const struct crossbar_data *cdata;
+
+	np = of_find_matching_node_and_match(NULL, crossbar_match, &of_id);
 	if (!np)
 		return -ENODEV;
 
-	crossbar_of_init(np);
+	cdata = of_id->data;
+	crossbar_of_init(np, cdata);
 	return 0;
 }