On Tuesday 03 July 2012, Andrew Lunn wrote:
> Both IRQ and GPIO controllers can now be represented in DT.  The IRQ
> controllers are setup first, and then the GPIO controllers. Interrupts
> for GPIO lines are placed directly after the main interrupts in the
> interrupt space.

Overall looks very good.

> diff --git a/Documentation/devicetree/bindings/arm/mrvl/intc.txt 
> b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> index 80b9a94..8927e10 100644
> --- a/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> +++ b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> @@ -38,3 +38,22 @@ Example:
>               reg-names = "mux status", "mux mask";
>               mrvl,intc-nr-irqs = <2>;
>       };
> +
> +* Marvell Orion Interrupt controller
> +
> +Required properties
> +- compatible :  Should be "marvell,orion-intc".
> +- #interrupt-cells: Specifies the number of cells needed to encode an
> +  interrupt source. Supported value is <1>.
> +- interrupt-controller : Declare this node to be an interrupt controller.
> +- reg : Interrupt mask address.

I think you should clarify that the "reg" property can be multiple
4-byte ranges, because that is fairly unusual.

> +#ifdef CONFIG_OF
> +static int __init orion_add_irq_domain(struct device_node *np,
> +                                    struct device_node *interrupt_parent)
> +{
> +     int i = 0, irq_gpio;
> +     void __iomem *base;
> +
> +     do {
> +             base = of_iomap(np, i);
> +             if (base) {
> +                     orion_irq_init(i * 32, base);
> +                     i++;
> +             }
> +     } while (base);
> +
> +     irq_domain_add_legacy(np, i * 32, 0, 0,
> +                           &irq_domain_simple_ops, NULL);
> +
> +     irq_gpio = i * 32;
> +     orion_gpio_of_init(irq_gpio);
> +
> +     return 0;
> +}
> +
> +static const struct of_device_id orion_irq_match[] = {
> +     { .compatible = "marvell,orion-intc",
> +       .data = orion_add_irq_domain, },
> +     {},
> +};
> +
> +void __init orion_dt_init_irq(void)
> +{
> +     of_irq_init(orion_irq_match);
> +}
> +#endif

I'm wondering about this one.  The other platforms usually put the secondary
interrupt controllers into the same match table, while you call 
orion_gpio_of_init
from orion_add_irq_domain. Can you explain why you do this? Does it have
any disadvantages?

        Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to