Re: [PATCHv4 1/9] omap: prcm: switch to a chained IRQ handler mechanism

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux