Hi,

On Wed, Jun 29, 2011 at 12:04:55PM +0300, Tero Kristo wrote:
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 96a7624..89cf027 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -880,20 +824,35 @@ static int __init omap3_pm_init(void)
>       /* XXX prcm_setup_regs needs to be before enabling hw
>        * supervised mode for powerdomains */
>       prcm_setup_regs();
> +     ret = omap_prcm_irq_init();
> +     if (ret) {
> +             pr_err("omap_prcm_irq_init() failed with %d\n", ret);
> +             goto err_prcm_irq_init;
> +     }
> +
> +     prcm_wkup_irq = omap_prcm_event_to_irq("wkup");
> +     prcm_io_irq = omap_prcm_event_to_irq("io");
> +
> +     ret = request_irq(prcm_wkup_irq, _prcm_int_handle_wakeup,
> +                     IRQF_NO_SUSPEND | IRQF_DISABLED, "prcm_wkup", NULL);

does this _have_ to be all in hardirq context ?

> -     ret = request_irq(INT_34XX_PRCM_MPU_IRQ,
> -                       (irq_handler_t)prcm_interrupt_handler,
> -                       IRQF_DISABLED, "prcm", NULL);
>       if (ret) {
> -             printk(KERN_ERR "request_irq failed to register for 0x%x\n",
> -                    INT_34XX_PRCM_MPU_IRQ);
> -             goto err1;
> +             printk(KERN_ERR "Failed to request prcm_wkup irq\n");
> +             goto err_prcm_wkup;
> +     }
> +
> +     ret = request_irq(prcm_io_irq, _prcm_int_handle_wakeup,
> +                     IRQF_NO_SUSPEND | IRQF_DISABLED, "prcm_io", NULL);

same here...

> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index 6be1438..794e451 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -23,6 +23,8 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/slab.h>
>  
>  #include <mach/system.h>
>  #include <plat/common.h>
> @@ -45,6 +47,167 @@ void __iomem *cm2_base;
>  
>  #define MAX_MODULE_ENABLE_WAIT               100000
>  
> +/* Setup for the interrupt handling based on used platform */
> +static struct omap_prcm_irq_setup *irq_setup;

you can set this is irq_chip data, then you can:

> +static void prcm_irq_ack(struct irq_data *data)
> +{
        struct omap_prcm_irq_setup      *irq_setup = 
irq_data_get_irq_chip_data(data)
        unsigned int                    prcm_irq = data->irq - irq_setup->base;

        irq_setup->ack_event(prcm_irq);
> +}

ditto to all other operations.

> +static struct irq_chip_generic *prcm_irq_chips[OMAP_PRCM_MAX_NR_PENDING_REG];

can't you allocate this dynamically ???

> +/*
> + * PRCM Interrupt Handler
> + *
> + * The PRM_IRQSTATUS_MPU register indicates if there are any pending
> + * interrupts from the PRCM for the MPU. These bits must be cleared in
> + * order to clear the PRCM interrupt. The PRCM interrupt handler is
> + * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
> + * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
> + * register indicates that a wake-up event is pending for the MPU and
> + * this bit can only be cleared if the all the wake-up events latched
> + * in the various PM_WKST_x registers have been cleared. The interrupt
> + * handler is implemented using a do-while loop so that if a wake-up
> + * event occurred during the processing of the prcm interrupt handler
> + * (setting a bit in the corresponding PM_WKST_x register and thus
> + * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
> + * this would be handled.
> + */
> +static void prcm_irq_handler(unsigned int irq, struct irq_desc *desc)
> +{
> +     unsigned long pending[OMAP_PRCM_MAX_NR_PENDING_REG];
> +     struct irq_chip *chip = irq_desc_get_chip(desc);
> +
> +     /*
> +      * Loop until all pending irqs are handled, since
> +      * generic_handle_irq(), called by prcm_irq_handle_virtirqs()
> +      * can cause new irqs to come
> +      */
> +     while (1) {
> +             unsigned int virtirq;
> +
> +             chip->irq_ack(&desc->irq_data);
> +
> +             memset(pending, 0, sizeof(pending));
> +             irq_setup->pending_events(pending);
> +
> +             /* No bit set, then all IRQs are handled */
> +             if (find_first_bit(pending, OMAP_PRCM_NR_IRQS)
> +                 >= OMAP_PRCM_NR_IRQS) {
> +                     chip->irq_unmask(&desc->irq_data);
> +                     break;
> +             }
> +
> +             /*
> +              * Loop on all currently pending irqs so that new irqs
> +              * cannot starve previously pending irqs
> +              */
> +             for_each_set_bit(virtirq, pending, OMAP_PRCM_NR_IRQS)
> +                     generic_handle_irq(OMAP_PRCM_IRQ_BASE + virtirq);

if you use nested IRQ threads, you could be using
handle_nested_irq(irq);

> +             chip->irq_unmask(&desc->irq_data);
> +     }
> +}
> +
> +/*
> + * Given a PRCM event name, returns the corresponding IRQ on which the
> + * handler should be registered.
> + */
> +int omap_prcm_event_to_irq(const char *name)
> +{
> +     int i;
> +
> +     for (i = 0; i < irq_setup->num_irqs; i++)
> +             if (!strcmp(irq_setup->irqs[i].name, name))
> +                     return OMAP_PRCM_IRQ_BASE + irq_setup->irqs[i].offset;
> +
> +     return -ENOENT;
> +}
> +
> +/*
> + * Prepare the array of PRCM events corresponding to the current SoC,
> + * and set-up the chained interrupt handler mechanism.
> + */
> +int __init omap_prcm_irq_init(void)
> +{
> +     int i;
> +     struct irq_chip_generic *gc;
> +     struct irq_chip_type *ct;
> +     u32 mask[2] = { 0, 0 };
> +     int offset;
> +     int max_irq = 0;
> +
> +     for (i = 0; i < irq_setup->num_irqs; i++)

how about using irq_alloc_descs() ?? Then we can make use of Sparse IRQ
numbers and avoid passing irq_base/irq_end and adding that weird ifdef
hackery to get NR_IRQS right.

> diff --git a/arch/arm/plat-omap/include/plat/irqs.h 
> b/arch/arm/plat-omap/include/plat/irqs.h
> index 5a25098..23b9680 100644
> --- a/arch/arm/plat-omap/include/plat/irqs.h
> +++ b/arch/arm/plat-omap/include/plat/irqs.h
> @@ -366,7 +366,14 @@
>  #define OMAP_MAX_GPIO_LINES  192
>  #define IH_GPIO_BASE         (128 + IH2_BASE)
>  #define IH_MPUIO_BASE                (OMAP_MAX_GPIO_LINES + IH_GPIO_BASE)
> -#define OMAP_IRQ_END         (IH_MPUIO_BASE + 16)
> +#define OMAP_MPUIO_IRQ_END   (IH_MPUIO_BASE + 16)
> +
> +/* 64 IRQs for the PRCM (32 are needed on OMAP3, 64 on OMAP4) */
> +#define OMAP_PRCM_IRQ_BASE      (OMAP_MPUIO_IRQ_END)
> +#define OMAP_PRCM_NR_IRQS       64
> +#define OMAP_PRCM_IRQ_END       (OMAP_PRCM_IRQ_BASE + OMAP_PRCM_NR_IRQS)
> +
> +#define OMAP_IRQ_END            (OMAP_PRCM_IRQ_END)

this is unnecessary with Sparse IRQ numbers and IMHO we should aim for
that. See the very simple conversion that I sent for the very old retu
driver [1] and [2] and you'll see that with time, we could get rid of
NR_IRQS altogether.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to