David,
On 11/11/2011 01:49 PM, David Daney wrote:
> On 11/11/2011 08:08 AM, Rob Herring wrote:
>> On 11/10/2011 08:21 PM, ddaney.cavm@xxxxxxxxx wrote:
>>> From: David Daney<david.daney@xxxxxxxxxx>
>>>
>>> This is needed for Octeon to use the Device Tree.
>>>
>>> The GPIO interrupts are configured based on Device Tree properties
>>>
>>> Signed-off-by: David Daney<david.daney@xxxxxxxxxx>
>>> ---
>>> arch/mips/cavium-octeon/octeon-irq.c | 188
>>> +++++++++++++++++++++++++++++++++-
>>> 1 files changed, 187 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/mips/cavium-octeon/octeon-irq.c
>>> b/arch/mips/cavium-octeon/octeon-irq.c
>>> index ffd4ae6..bb10546 100644
>>> --- a/arch/mips/cavium-octeon/octeon-irq.c
>>> +++ b/arch/mips/cavium-octeon/octeon-irq.c
>>> @@ -8,11 +8,14 @@
>>>
>>> #include<linux/interrupt.h>
>>> #include<linux/bitops.h>
>>> +#include<linux/module.h>
>>> #include<linux/percpu.h>
>>> +#include<linux/of_irq.h>
>>> #include<linux/irq.h>
>>> #include<linux/smp.h>
>>>
>>> #include<asm/octeon/octeon.h>
>>> +#include<asm/octeon/cvmx-gpio-defs.h>
>>>
>>> static DEFINE_RAW_SPINLOCK(octeon_irq_ciu0_lock);
>>> static DEFINE_RAW_SPINLOCK(octeon_irq_ciu1_lock);
>>> @@ -58,6 +61,95 @@ static void __init octeon_irq_set_ciu_mapping(int
>>> irq, int line, int bit,
>>> octeon_irq_ciu_to_irq[line][bit] = irq;
>>> }
>>>
>>> +static unsigned int octeon_irq_gpio_mapping(struct device_node
>>> *controller,
>>> + const u32 *intspec,
>>> + unsigned int intsize)
>>> +{
>>> + struct of_irq oirq;
>>> + int i;
>>> + unsigned int irq = 0;
>>> + unsigned int type;
>>> + unsigned int ciu = 0, bit = 0;
>>> + unsigned int pin = be32_to_cpup(intspec);
>>> + unsigned int trigger = be32_to_cpup(intspec + 1);
>>> + bool set_edge_handler = false;
>>> +
>>> + if (pin>= 16)
>>> + goto err;
>>> + i = of_irq_map_one(controller, 0,&oirq);
>>> + if (i)
>>> + goto err;
>>> + if (oirq.size != 2)
>>> + goto err_put;
>>> +
>>> + ciu = oirq.specifier[0];
>>> + bit = oirq.specifier[1] + pin;
>>> +
>>> + if (ciu>= 8 || bit>= 64)
>>> + goto err_put;
>>> +
>>> + irq = octeon_irq_ciu_to_irq[ciu][bit];
>>> + if (!irq)
>>> + goto err_put;
>>> +
>>> + switch (trigger& 0xf) {
>>> + case 1:
>>> + type = IRQ_TYPE_EDGE_RISING;
>>> + set_edge_handler = true;
>>> + break;
>>> + case 2:
>>> + type = IRQ_TYPE_EDGE_FALLING;
>>> + set_edge_handler = true;
>>> + break;
>>> + case 4:
>>> + type = IRQ_TYPE_LEVEL_HIGH;
>>> + break;
>>> + case 8:
>>> + type = IRQ_TYPE_LEVEL_LOW;
>>> + break;
>>> + default:
>>> + pr_err("Error: Invalid irq trigger specification: %x\n",
>>> + trigger);
>>> + type = IRQ_TYPE_LEVEL_LOW;
>>> + break;
>>> + }
>>> +
>>> + irq_set_irq_type(irq, type);
>>> +
>>> + if (set_edge_handler)
>>> + __irq_set_handler(irq, handle_edge_irq, 0, NULL);
>>> +
>>> +err_put:
>>> + of_node_put(oirq.controller);
>>> +err:
>>> + return irq;
>>> +}
>>> +
>>> +/*
>>> + * irq_create_of_mapping - Hook to resolve OF irq specifier into a
>>> Linux irq#
>>> + *
>>> + * Octeon irq maps are a pair of indexes. The first selects either
>>> + * ciu0 or ciu1, the second is the bit within the ciu register.
>>> + */
>>> +unsigned int irq_create_of_mapping(struct device_node *controller,
>>> + const u32 *intspec, unsigned int intsize)
>>> +{
>>> + unsigned int irq = 0;
>>> + unsigned int ciu, bit;
>>> +
>>> + if (of_device_is_compatible(controller, "cavium,octeon-3860-gpio"))
>>> + return octeon_irq_gpio_mapping(controller, intspec, intsize);
>>> +
>>> + ciu = be32_to_cpup(intspec);
>>> + bit = be32_to_cpup(intspec + 1);
>>> +
>>> + if (ciu< 8&& bit< 64)
>>> + irq = octeon_irq_ciu_to_irq[ciu][bit];
>>> +
>>> + return irq;
>>> +}
>>> +EXPORT_SYMBOL_GPL(irq_create_of_mapping);
>>
>> Have you looked at irq_domains (kernel/irq/irqdomain.c)? That is what
>> you should be using for your (gpio) interrupt controller and then use
>> the common irq_create_of_mapping.
>>
>
> Unfortunatly, although a good idea, kernel/irq/irqdomain.c makes a bunch
> of assumptions that don't hold for Octeon. We may be able to improve it
> so that it flexible enough to suit us.
>
>
> Here are the problems I see:
>
> 1) It is assumed that there is some sort of linear correspondence
> between 'hwirq' and 'irq', and that the range of valid values is
> contiguous.
That's not true if you implement .to_irq for your domain.
> 2) It is assumed that the concepts of nr_irq, irq_base and hwirq_base
> have easy to determin values and you can do iteration over their ranges
> by adding indexes to the bases.
That's true for hwirq numbering, but not for Linux irq numbering.
irq_base is only used if you don't provide .to_irq. hwirq_base is just
to allow a linear range that doesn't start with 0.
> I think we can fix this by adding iteration helper functions to struct
> irq_domain. If these are present, we would just ignore the irq_base,
> nr_irq and hwirq_base elements of the structure.
> irq_domain_for_each_hwirq() and irq_domain_for_each_irq() would be
> modified to use the iteration helper functions.
Expanding irqdomain is certainly the intention and right direction.
It seems to me you just need something different than an irq. controller
with a linear range of irq lines. Perhaps sparsely allocated hwirq's?
Can you describe the h/w?
Also, you could have a different domain per irqdesc if you really needed to.
Rob
