Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hello Tero, On Fri, 2 Sep 2011, Tero Kristo wrote: On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote: If it would help, I'd be happy to do a first draft of the OMAP3430 PRM hwmod data. If you can do this it would help, as you have much better understanding of the hwmod data than I do. It will probably drop a couple of review rounds away as the hwmod data would be close to what it should be from beginning. If you are busy with other things, I can see what I can craft myself. Not sure if you're still waiting on this, but here's a draft of a PRM hwmod for OMAP34xx for testing purposes. It needs a little more review and thought before being ready for merging, but I think it will work for your purposes. The OMAP2430 version should be quite similar. Comments welcome, - Paul From: Paul Walmsley p...@pwsan.com Date: Sun, 4 Sep 2011 18:26:58 -0600 Subject: [PATCH] OMAP3xxx: hwmod data: add PRM hwmod *** DRAFT *** --- arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 63 1 files changed, 63 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c index 25bf43b..4b3640e 100644 --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c @@ -161,6 +161,7 @@ static struct omap_hwmod omap3xxx_l3_main_hwmod = { }; static struct omap_hwmod omap3xxx_l4_wkup_hwmod; +static struct omap_hwmod omap3xxx_prm_hwmod; static struct omap_hwmod omap3xxx_uart1_hwmod; static struct omap_hwmod omap3xxx_uart2_hwmod; static struct omap_hwmod omap3xxx_uart3_hwmod; @@ -478,6 +479,29 @@ static struct omap_hwmod omap3xxx_l4_per_hwmod = { .flags = HWMOD_NO_IDLEST, }; +static struct omap_hwmod_addr_space omap3xxx_prm_addrs[] = { + { + .pa_start = 0x48306000, + .pa_end = 0x48306000 + SZ_8K + SZ_4K - 1, + .flags = ADDR_TYPE_RT + }, + { } +}; + +/* L4_WKUP - PRM interface */ +static struct omap_hwmod_ocp_if omap3xxx_l4_wkup__prm = { + .master = omap3xxx_l4_wkup_hwmod, + .slave = omap3xxx_prm_hwmod, + .clk= wkup_l4_ick, + .addr = omap3xxx_prm_addrs, + .user = OCP_USER_MPU, +}; + +/* Master interfaces on the L4_WKUP interconnect */ +static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_masters[] = { + omap3xxx_l4_wkup__prm, +}; + /* Slave interfaces on the L4_WKUP interconnect */ static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = { omap3xxx_l4_core__l4_wkup, @@ -487,12 +511,48 @@ static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = { static struct omap_hwmod omap3xxx_l4_wkup_hwmod = { .name = l4_wkup, .class = l4_hwmod_class, + .masters= omap3xxx_l4_wkup_masters, + .masters_cnt= ARRAY_SIZE(omap3xxx_l4_wkup_masters), .slaves = omap3xxx_l4_wkup_slaves, .slaves_cnt = ARRAY_SIZE(omap3xxx_l4_wkup_slaves), .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3430), .flags = HWMOD_NO_IDLEST, }; +/* Slave interfaces on the L4_WKUP interconnect */ +static struct omap_hwmod_ocp_if *omap3xxx_prm_slaves[] = { + omap3xxx_l4_wkup__prm, +}; + +static struct omap_hwmod_class_sysconfig omap3xxx_prm_sysc = { + .rev_offs = 0x0804, + .sysc_offs = 0x0814, + .sysc_flags = SYSC_HAS_AUTOIDLE, + .sysc_fields= omap_hwmod_sysc_type1, +}; + +static struct omap_hwmod_class omap3xxx_prm_hwmod_class = { + .name = prm, + .sysc = omap3xxx_prm_sysc, +}; + +static struct omap_hwmod_irq_info omap3xxx_prm_irqs[] = { + { .irq = 11 }, + { .irq = -1 } +}; + +/* PRM */ +static struct omap_hwmod omap3xxx_prm_hwmod = { + .name = prm, + .mpu_irqs = omap3xxx_prm_irqs, + .class = omap3xxx_prm_hwmod_class, + .main_clk = wkup_32k_fck, + .slaves = omap3xxx_prm_slaves, + .slaves_cnt = ARRAY_SIZE(omap3xxx_prm_slaves), + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3430), + .flags = HWMOD_NO_IDLEST, +}; + /* Master interfaces on the MPU device */ static struct omap_hwmod_ocp_if *omap3xxx_mpu_masters[] = { omap3xxx_mpu__l3_main, @@ -3201,6 +3261,9 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = { omap3xxx_l4_core_hwmod, omap3xxx_l4_per_hwmod, omap3xxx_l4_wkup_hwmod, + + omap3xxx_prm_hwmod, + omap3xxx_mmc1_hwmod, omap3xxx_mmc2_hwmod, omap3xxx_mmc3_hwmod, -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
On Wed, 2011-09-14 at 14:10 +0200, Paul Walmsley wrote: Hello Tero, On Fri, 2 Sep 2011, Tero Kristo wrote: On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote: If it would help, I'd be happy to do a first draft of the OMAP3430 PRM hwmod data. If you can do this it would help, as you have much better understanding of the hwmod data than I do. It will probably drop a couple of review rounds away as the hwmod data would be close to what it should be from beginning. If you are busy with other things, I can see what I can craft myself. Not sure if you're still waiting on this, but here's a draft of a PRM hwmod for OMAP34xx for testing purposes. It needs a little more review and thought before being ready for merging, but I think it will work for your purposes. The OMAP2430 version should be quite similar. Comments welcome, Thanks Paul, I'll start experimenting with this. - Paul From: Paul Walmsley p...@pwsan.com Date: Sun, 4 Sep 2011 18:26:58 -0600 Subject: [PATCH] OMAP3xxx: hwmod data: add PRM hwmod *** DRAFT *** --- arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 63 1 files changed, 63 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c index 25bf43b..4b3640e 100644 --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c @@ -161,6 +161,7 @@ static struct omap_hwmod omap3xxx_l3_main_hwmod = { }; static struct omap_hwmod omap3xxx_l4_wkup_hwmod; +static struct omap_hwmod omap3xxx_prm_hwmod; static struct omap_hwmod omap3xxx_uart1_hwmod; static struct omap_hwmod omap3xxx_uart2_hwmod; static struct omap_hwmod omap3xxx_uart3_hwmod; @@ -478,6 +479,29 @@ static struct omap_hwmod omap3xxx_l4_per_hwmod = { .flags = HWMOD_NO_IDLEST, }; +static struct omap_hwmod_addr_space omap3xxx_prm_addrs[] = { + { + .pa_start = 0x48306000, + .pa_end = 0x48306000 + SZ_8K + SZ_4K - 1, + .flags = ADDR_TYPE_RT + }, + { } +}; + +/* L4_WKUP - PRM interface */ +static struct omap_hwmod_ocp_if omap3xxx_l4_wkup__prm = { + .master = omap3xxx_l4_wkup_hwmod, + .slave = omap3xxx_prm_hwmod, + .clk= wkup_l4_ick, + .addr = omap3xxx_prm_addrs, + .user = OCP_USER_MPU, +}; + +/* Master interfaces on the L4_WKUP interconnect */ +static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_masters[] = { + omap3xxx_l4_wkup__prm, +}; + /* Slave interfaces on the L4_WKUP interconnect */ static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = { omap3xxx_l4_core__l4_wkup, @@ -487,12 +511,48 @@ static struct omap_hwmod_ocp_if *omap3xxx_l4_wkup_slaves[] = { static struct omap_hwmod omap3xxx_l4_wkup_hwmod = { .name = l4_wkup, .class = l4_hwmod_class, + .masters= omap3xxx_l4_wkup_masters, + .masters_cnt= ARRAY_SIZE(omap3xxx_l4_wkup_masters), .slaves = omap3xxx_l4_wkup_slaves, .slaves_cnt = ARRAY_SIZE(omap3xxx_l4_wkup_slaves), .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3430), .flags = HWMOD_NO_IDLEST, }; +/* Slave interfaces on the L4_WKUP interconnect */ +static struct omap_hwmod_ocp_if *omap3xxx_prm_slaves[] = { + omap3xxx_l4_wkup__prm, +}; + +static struct omap_hwmod_class_sysconfig omap3xxx_prm_sysc = { + .rev_offs = 0x0804, + .sysc_offs = 0x0814, + .sysc_flags = SYSC_HAS_AUTOIDLE, + .sysc_fields= omap_hwmod_sysc_type1, +}; + +static struct omap_hwmod_class omap3xxx_prm_hwmod_class = { + .name = prm, + .sysc = omap3xxx_prm_sysc, +}; + +static struct omap_hwmod_irq_info omap3xxx_prm_irqs[] = { + { .irq = 11 }, + { .irq = -1 } +}; + +/* PRM */ +static struct omap_hwmod omap3xxx_prm_hwmod = { + .name = prm, + .mpu_irqs = omap3xxx_prm_irqs, + .class = omap3xxx_prm_hwmod_class, + .main_clk = wkup_32k_fck, + .slaves = omap3xxx_prm_slaves, + .slaves_cnt = ARRAY_SIZE(omap3xxx_prm_slaves), + .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP3430), + .flags = HWMOD_NO_IDLEST, +}; + /* Master interfaces on the MPU device */ static struct omap_hwmod_ocp_if *omap3xxx_mpu_masters[] = { omap3xxx_mpu__l3_main, @@ -3201,6 +3261,9 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = { omap3xxx_l4_core_hwmod, omap3xxx_l4_per_hwmod, omap3xxx_l4_wkup_hwmod, + + omap3xxx_prm_hwmod, + omap3xxx_mmc1_hwmod, omap3xxx_mmc2_hwmod, omap3xxx_mmc3_hwmod, Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki -- To unsubscribe from this list: send
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hi Tero, On Thu, 1 Sep 2011, Tero Kristo wrote: I've been looking at this now and got one question below. Otherwise your comments look okay to me and I can work with those. Great. As you work on it, please let me know if there's something that doesn't make sense with this arrangement. I think this will work, and will move some code out of arch/arm/*omap*, but it's hard to tell, until someone tries it. On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote: What I'd suggest is to create a short series that: 1. adds PRM hwmod data for OMAP2430+ platforms How should this be done? It believe all the data in the hwmods should be autogenerated somehow... should I just make a temporary hack patch for one platform that could be then autogenerated by someone for all omap platforms? Only OMAP4 is autogenerated, currently. OMAP2 3 are still done by hand. I'd suggest starting with either OMAP4 (because the hwmod data should be autogeneratable) or OMAP3 (because we know that one pretty well and the hwmod data should not be too difficult to build). If it would help, I'd be happy to do a first draft of the OMAP3430 PRM hwmod data. regards, - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hi Paul, On Fri, 2011-09-02 at 11:20 +0200, Paul Walmsley wrote: Hi Tero, On Thu, 1 Sep 2011, Tero Kristo wrote: I've been looking at this now and got one question below. Otherwise your comments look okay to me and I can work with those. Great. As you work on it, please let me know if there's something that doesn't make sense with this arrangement. I think this will work, and will move some code out of arch/arm/*omap*, but it's hard to tell, until someone tries it. On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote: What I'd suggest is to create a short series that: 1. adds PRM hwmod data for OMAP2430+ platforms How should this be done? It believe all the data in the hwmods should be autogenerated somehow... should I just make a temporary hack patch for one platform that could be then autogenerated by someone for all omap platforms? Only OMAP4 is autogenerated, currently. OMAP2 3 are still done by hand. I'd suggest starting with either OMAP4 (because the hwmod data should be autogeneratable) or OMAP3 (because we know that one pretty well and the hwmod data should not be too difficult to build). If it would help, I'd be happy to do a first draft of the OMAP3430 PRM hwmod data. If you can do this it would help, as you have much better understanding of the hwmod data than I do. It will probably drop a couple of review rounds away as the hwmod data would be close to what it should be from beginning. If you are busy with other things, I can see what I can craft myself. regards, - Paul 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hey Paul, I've been looking at this now and got one question below. Otherwise your comments look okay to me and I can work with those. On Fri, 2011-08-26 at 11:12 +0200, Paul Walmsley wrote: Hello Tero, a few comments on this patch: On Mon, 25 Jul 2011, Tero Kristo wrote: 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 SoC specific support / init structure defined in arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.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. Looking over this patch, it seems that this functionality should be part of a PRM device driver. That would allow the separation of the SoC-specific data from the code, so there wouldn't be a need to embed the OMAP_PRCM_IRQ data in the driver code. Rather, that data could go into the dev_attr data for the PRM hwmod. That avoids putting SoC-specific data in driver code, allows the removal of omap[34]_prcm_irq_setup(), and should also remove the dependency on omap_chip. Similarly, OMAP_PRCM_MAX_NR_PENDING_REG and OMAP_PRCM_NR_IRQS should be defined somewhere SoC-specific. I'd suggest defining those in the hwmod dev_attr data. That way that file won't need to be patched if those constants need change in the future. Unfortunately, doing this in a clean way will probably mean that the variables that are allocated via these constants will need to be allocated and freed dynamically. What I'd suggest is to create a short series that: 1. adds PRM hwmod data for OMAP2430+ platforms How should this be done? It believe all the data in the hwmods should be autogenerated somehow... should I just make a temporary hack patch for one platform that could be then autogenerated by someone for all omap platforms? 2. adds a basic PRM device driver skeleton in a directory such as drivers/power -- (I'm not convinced that this is the right place, in the end; but seems like a good place to start) 3. creates the chained interrupt handler in the PRM device driver, and removes the old PRCM interrupt handler from pm34xx.c ... A few other relatively minor comments: - Probably omap_prcm_irq_cleanup() shouldn't be called from pm34xx.c, since other code outside of pm34xx.c might wish to use the PRCM interrupt, even in the (admittedly unlikely) circumstance that some of the code in pm34xx.c fails? - It would be good to document struct omap_prcm_irq via KernelDoc, rather than inline comments (Documentation/kernel-doc-nano-HOWTO.txt). It would be ideal if the patch's function documentation followed the same standard. - Paul 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hello Tero, a few comments on this patch: On Mon, 25 Jul 2011, Tero Kristo wrote: 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 SoC specific support / init structure defined in arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.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. Looking over this patch, it seems that this functionality should be part of a PRM device driver. That would allow the separation of the SoC-specific data from the code, so there wouldn't be a need to embed the OMAP_PRCM_IRQ data in the driver code. Rather, that data could go into the dev_attr data for the PRM hwmod. That avoids putting SoC-specific data in driver code, allows the removal of omap[34]_prcm_irq_setup(), and should also remove the dependency on omap_chip. Similarly, OMAP_PRCM_MAX_NR_PENDING_REG and OMAP_PRCM_NR_IRQS should be defined somewhere SoC-specific. I'd suggest defining those in the hwmod dev_attr data. That way that file won't need to be patched if those constants need change in the future. Unfortunately, doing this in a clean way will probably mean that the variables that are allocated via these constants will need to be allocated and freed dynamically. What I'd suggest is to create a short series that: 1. adds PRM hwmod data for OMAP2430+ platforms 2. adds a basic PRM device driver skeleton in a directory such as drivers/power -- (I'm not convinced that this is the right place, in the end; but seems like a good place to start) 3. creates the chained interrupt handler in the PRM device driver, and removes the old PRCM interrupt handler from pm34xx.c ... A few other relatively minor comments: - Probably omap_prcm_irq_cleanup() shouldn't be called from pm34xx.c, since other code outside of pm34xx.c might wish to use the PRCM interrupt, even in the (admittedly unlikely) circumstance that some of the code in pm34xx.c fails? - It would be good to document struct omap_prcm_irq via KernelDoc, rather than inline comments (Documentation/kernel-doc-nano-HOWTO.txt). It would be ideal if the patch's function documentation followed the same standard. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
On Mon, 2011-07-25 at 19:03 +0200, Balbi, Felipe wrote: Hi, On Mon, Jul 25, 2011 at 07:36:01PM +0300, Tero Kristo wrote: 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 which drivers ? Are those somehow children of the PRCM device ?? If that's the case, you shouldn't need to match against names as you could allocate a platform_device for your children and pass in your resources with correct IRQ numbers. I am not quite sure what you mean by children in this case. There are a couple of devices that might be interested in using these, e.g. SR and ABB come to my mind. They are closely related to PRCM yes. 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 SoC specific support / init structure defined in arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.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. Then if OMAP5 has 3, OMAP6 4 and OMAP7 5, OMAP3 will also have an array of 5 PRCM events even though it only needs one, another argument for dynamic allocation ? --- [snip] @@ -246,64 +249,7 @@ static int _prcm_int_handle_wakeup(void) c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1); } - return c; -} - -/* - * 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 irqreturn_t prcm_interrupt_handler (int irq, void *dev_id) -{ - u32 irqenable_mpu, irqstatus_mpu; - int c = 0; - - irqenable_mpu = omap2_prm_read_mod_reg(OCP_MOD, -OMAP3_PRM_IRQENABLE_MPU_OFFSET); - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD, -OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - irqstatus_mpu = irqenable_mpu; - - do { - if (irqstatus_mpu (OMAP3430_WKUP_ST_MASK | -OMAP3430_IO_ST_MASK)) { - c = _prcm_int_handle_wakeup(); - - /* -* Is the MPU PRCM interrupt handler racing with the -* IVA2 PRCM interrupt handler ? -*/ - WARN(c == 0, prcm: WARNING: PRCM indicated MPU wakeup -but no wakeup sources are marked\n); - } else { - /* XXX we need to expand our PRCM interrupt handler */ - WARN(1, prcm: WARNING: PRCM interrupt received, but -no code to handle it (%08x)\n, irqstatus_mpu); - } - - omap2_prm_write_mod_reg(irqstatus_mpu, OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - irqstatus_mpu = irqenable_mpu; - - } while (irqstatus_mpu); - - return IRQ_HANDLED; + return c ? IRQ_HANDLED : IRQ_NONE; } static void omap34xx_save_context(u32 *save) @@ -875,20 +821,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 = omap3_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,
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hi, On Tue, Jul 26, 2011 at 01:33:35PM +0300, Tero Kristo wrote: + 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(irq_setup-base_irq + virtirq); + + chip-irq_unmask(desc-irq_data); can't the IRQ subsystem handle this for you ? I was expecting it would call irq_ack() and irq_unmask() automatically and you wouldn't have to do it yourself. Maybe Thomas can clear this out ? Thomas, should we call -irq_ack() -irq_mask ourselves here ? These are needed, as we are using a handle and a level interrupt. If you leave out the ack, we will return to the handler immediately as nobody else acks it. If you leave out the unmask, we will only ever get 1 interrupt and no more. but that's the thing, I guess IRQ subsystem can handle this automatically. At least from what I remember. That's how I converted the retu driver, for instance, and I checked that IRQ subsystem calls -irq_ack() and -irq_mask/unmask() for me... Maybe the irq_gc stuff is different, dunno. if you make this a platform_driver, there would be no need for this trickery. You could pass this as driver data. Something like: struct omap_prcm_irq_setup omap3_prcm_irq_setup = { .ack= (u32)OMAP3430_PRM_IRQSTATUS_MPU, .mask = (u32)OMAP3430_PRM_IRQENABLE_MPU, .pending_events = omap3_prm_pending_events, .irq= INT_34XX_PRCM_MPU_IRQ, }; struct const struct platform_device_id prcm_id_table[] __devinitconst = { { .name = omap3-prcm, .driver_data= omap3_prcm_irq_setup, }, { .name = omap4-prcm, .driver_data= omap4_prcm_irq_setup, }, }; MODULE_DEVICE_TABLE(platform, prcm_id_table); static struct platform_driver prcm_driver = { .probe = prcm_probe, .remove = __devexit_p(prcm_remove), .driver = { .name = prcm, .pm = DEV_PM_OPS, }, .id_table = prcm_id_table, }; or something similar. Then on probe you can make a copy of irq_setup to your driver's context structure, or only use temporarily to initialize some fields and so on... Hmm, this might be useful, however you would still need a way to register the driver from somewhere, and you would essentially end up with omap_chip version check somewhere. The reason for all this trickery is that the omap chip version detection is heavily frowned upon right now... it would be much cleaner if there was an accepted way of doing this already in place. wouldn't hwmod be able to handle this for you ? I mean, it's just the driver name that has to change, rather than exporting a few functions. -- balbi signature.asc Description: Digital signature
Re: [PATCHv6 01/11] omap: prcm: switch to a chained IRQ handler mechanism
Hi, On Mon, Jul 25, 2011 at 07:36:01PM +0300, Tero Kristo wrote: 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 which drivers ? Are those somehow children of the PRCM device ?? If that's the case, you shouldn't need to match against names as you could allocate a platform_device for your children and pass in your resources with correct IRQ numbers. 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 SoC specific support / init structure defined in arch/arm/mach-omap2/prm2xxx_3xxx.c and arch/arm/mach-omap2/prm4xxx.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. Then if OMAP5 has 3, OMAP6 4 and OMAP7 5, OMAP3 will also have an array of 5 PRCM events even though it only needs one, another argument for dynamic allocation ? --- [snip] @@ -246,64 +249,7 @@ static int _prcm_int_handle_wakeup(void) c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1); } - return c; -} - -/* - * 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 irqreturn_t prcm_interrupt_handler (int irq, void *dev_id) -{ - u32 irqenable_mpu, irqstatus_mpu; - int c = 0; - - irqenable_mpu = omap2_prm_read_mod_reg(OCP_MOD, - OMAP3_PRM_IRQENABLE_MPU_OFFSET); - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - irqstatus_mpu = irqenable_mpu; - - do { - if (irqstatus_mpu (OMAP3430_WKUP_ST_MASK | - OMAP3430_IO_ST_MASK)) { - c = _prcm_int_handle_wakeup(); - - /* - * Is the MPU PRCM interrupt handler racing with the - * IVA2 PRCM interrupt handler ? - */ - WARN(c == 0, prcm: WARNING: PRCM indicated MPU wakeup - but no wakeup sources are marked\n); - } else { - /* XXX we need to expand our PRCM interrupt handler */ - WARN(1, prcm: WARNING: PRCM interrupt received, but - no code to handle it (%08x)\n, irqstatus_mpu); - } - - omap2_prm_write_mod_reg(irqstatus_mpu, OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - - irqstatus_mpu = omap2_prm_read_mod_reg(OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - irqstatus_mpu = irqenable_mpu; - - } while (irqstatus_mpu); - - return IRQ_HANDLED; + return c ? IRQ_HANDLED : IRQ_NONE; } static void omap34xx_save_context(u32 *save) @@ -875,20 +821,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 = omap3_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, prcm_wkup, NULL); - 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, -