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    [W:0.037 / U:29.644 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site