lkml.org 
[lkml]   [2011]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/8] MIPS: Octeon: Add irq_create_of_mapping() and GPIO interrupts.
On 11/11/2011 12:41 PM, Rob Herring wrote:
> 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@gmail.com wrote:
>>>> From: David Daney<david.daney@cavium.com>
>>>>
>>>> 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@cavium.com>
>>>> ---
>>>> 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.

It is true. irq_domain_add() iterates.

>
>> 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.

I have a patch. It should be ready in a couple of hours.

>
> 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.
>

Too ugly.

David Daney


\
 
 \ /
  Last update: 2011-11-11 21:47    [from the cache]
©2003-2011 Jasper Spaans