On Thu, 2011-06-23 at 10:19 +0200, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@xxxxxx> [110622 09:38]:
> > Introduce a chained interrupt handler mechanism for the PRCM
> > interrupt, so that individual PRCM event can cleanly be handled by
> > handlers in separate drivers. We do this by introducing PRCM event
> > names, which are then matched to the particular PRCM interrupt bit
> > depending on the specific OMAP SoC being used.
> >
> > arch/arm/mach-omap2/prcm.c implements the chained interrupt mechanism
> > itself, with individual PRCM events for OMAP3 and OMAP4 being
> > described in arch/arm/mach-omap2/prcm3xxx.c and
> > arch/arm/mach-omap2/prcm4xxx.c respectively. At initialization time,
> > the set of PRCM events is filtered against the SoC on which we are
> > running, keeping only the ones that are actually useful. All the logic
> > is written to be generic with regard to OMAP3/OMAP4, even though OMAP3
> > has single PRCM event registers and OMAP4 has two PRCM event
> > registers.
>
> Nice, this makes things more generic. Some comments below.
>
> > +int omap_prcm_irq_init(void)
> > +{
> > + int i, j;
> > + struct omap_prcm_irq *unfiltered_irqs;
> > + unsigned unfiltered_irqs_nr;
> > +
> > + if (cpu_is_omap34xx() || cpu_is_omap3630()) {
> > + unfiltered_irqs = omap_prcm_3xxx_irqs;
> > + unfiltered_irqs_nr = omap_prcm_3xxx_irqs_nr;
> > + omap_prcm_mask_event = omap3_prcm_mask_event;
> > + omap_prcm_unmask_event = omap3_prcm_unmask_event;
> > + omap_prcm_ack_event = omap3_prcm_ack_event;
> > + omap_prcm_pending_events = omap3_prcm_pending_events;
> > + irq_set_chained_handler(INT_34XX_PRCM_MPU_IRQ,
> > + prcm_irq_handler);
> > + } else if (cpu_is_omap44xx()) {
> > + unfiltered_irqs = omap_prcm_4xxx_irqs;
> > + unfiltered_irqs_nr = omap_prcm_4xxx_irqs_nr;
> > + omap_prcm_mask_event = omap4_prcm_mask_event;
> > + omap_prcm_unmask_event = omap4_prcm_unmask_event;
> > + omap_prcm_ack_event = omap4_prcm_ack_event;
> > + omap_prcm_pending_events = omap4_prcm_pending_events;
> > + irq_set_chained_handler(OMAP44XX_IRQ_PRCM, prcm_irq_handler);
> > + } else {
> > + return -ENODEV;
> > + }
> > +
> > + for (i = 0; i < unfiltered_irqs_nr; i++)
> > + if (omap_chip_is(unfiltered_irqs[i].omap_chip))
> > + omap_prcm_irqs_nr++;
> > +
> > + omap_prcm_irqs = kmalloc(omap_prcm_irqs_nr *
> > + sizeof(struct omap_prcm_irq),
> > + GFP_KERNEL);
> > + if (!omap_prcm_irqs)
> > + return -ENOMEM;
> > +
> > + for (i = 0, j = 0; i < unfiltered_irqs_nr; i++)
> > + if (omap_chip_is(unfiltered_irqs[i].omap_chip)) {
> > + memcpy(&omap_prcm_irqs[j], &unfiltered_irqs[i],
> > + sizeof(struct omap_prcm_irq));
> > + j++;
> > + }
> > +
> > + for (i = OMAP_PRCM_IRQ_BASE; i < OMAP_PRCM_IRQ_END; i++) {
> > + irq_set_chip(i, &prcm_irq_chip);
> > + irq_set_handler(i, handle_level_irq);
> > + set_irq_flags(i, IRQF_VALID);
> > + }
> > +
> > + return 0;
> > +}
>
> Please make omap_prcm_irq_init generic so you pass it the configuration.
> Otherwise you have to add more else if cpu_is_omap code for each new omap.
> Then you can add just an arch_initcall for each new omap to call
> omap_prcm_irq_init. This will also make it easier to add support for
> initializing things from device tree for omap_prcm_irq_init.
Yea, can do this.
>
> > +/*
> > + * Reverses memory allocated and other setups done by
> > + * omap_prcm_irq_init().
> > + */
> > +void omap_prcm_irq_cleanup(void)
> > +{
> > + int i;
> > +
> > + for (i = OMAP_PRCM_IRQ_BASE; i < OMAP_PRCM_IRQ_END; i++) {
> > + irq_set_chip(i, NULL);
> > + irq_set_handler(i, NULL);
> > + set_irq_flags(i, 0);
> > + }
> > +
> > + kfree(omap_prcm_irqs);
> > +
> > + if (cpu_is_omap34xx() || cpu_is_omap3630()) {
> > + irq_set_chained_handler(INT_34XX_PRCM_MPU_IRQ, NULL);
> > + } else {
> > + irq_set_chained_handler(OMAP44XX_IRQ_PRCM, NULL);
> > + }
> > +}
>
> Please get rid of the cpu_is_omap tests here too so prcm.c is
> generic for the new code added.
Same.
>
> > +struct omap_prcm_irq __initdata omap_prcm_3xxx_irqs[] = {
> > + OMAP_PRCM_IRQ("wkup", 0,
> > + CHIP_IS_OMAP3430 | CHIP_GE_OMAP3630ES1_1),
> > + OMAP_PRCM_IRQ("evgenon", 2,
> > + CHIP_IS_OMAP3430 | CHIP_GE_OMAP3630ES1_1),
> > + OMAP_PRCM_IRQ("evgenoff", 3,
> > + CHIP_IS_OMAP3430 | CHIP_GE_OMAP3630ES1_1),
> ...
>
> Please note consider that this data will be coming from device
> tree and will disappear from here. We won't be merging any new
> data after v3.1 unless it comes from device tree. So this too
> will need to be converted because we won't be able to add support
> for new omaps otherwise.
This part I am not too sure what you mean with this. Do you have some
info / examples about the device tree somewhere and how this data should
be converted?
>
> Also, please Cc linux-arm-kernel too.
>
> Regards,
>
> Tony
Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
