From patchwork Sat Jun 27 05:07:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Hunter, Jon" X-Patchwork-Id: 32656 X-Patchwork-Delegate: khilman@deeprootsystems.com Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n5R57dmq016804 for ; Sat, 27 Jun 2009 05:07:39 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750932AbZF0FH3 (ORCPT ); Sat, 27 Jun 2009 01:07:29 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751060AbZF0FH3 (ORCPT ); Sat, 27 Jun 2009 01:07:29 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:56412 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750932AbZF0FH2 (ORCPT ); Sat, 27 Jun 2009 01:07:28 -0400 Received: from dlep34.itg.ti.com ([157.170.170.115]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id n5R57QjM013314; Sat, 27 Jun 2009 00:07:31 -0500 Received: from dlep20.itg.ti.com (localhost [127.0.0.1]) by dlep34.itg.ti.com (8.13.7/8.13.7) with ESMTP id n5R57PEh007538; Sat, 27 Jun 2009 00:07:25 -0500 (CDT) Received: from dlee73.ent.ti.com (localhost [127.0.0.1]) by dlep20.itg.ti.com (8.12.11/8.12.11) with ESMTP id n5R57Pri019764; Sat, 27 Jun 2009 00:07:25 -0500 (CDT) Received: from [192.157.144.138] (192.157.144.138) by dlee73.ent.ti.com (157.170.170.88) with Microsoft SMTP Server id 8.1.358.0; Sat, 27 Jun 2009 00:07:25 -0500 Subject: Re: [PATCH][RFC] OMAP3: PM: Prevent hang in prcm_interrupt_handler From: Jon Hunter To: Kevin Hilman CC: "linux-omap@vger.kernel.org" In-Reply-To: <87prcv25ro.fsf@deeprootsystems.com> References: <1245452921-24142-1-git-send-email-jon-hunter@ti.com> <87prcv25ro.fsf@deeprootsystems.com> Date: Sat, 27 Jun 2009 00:07:25 -0500 Message-ID: <1246079245.3336.47.camel@a0741266-laptop> MIME-Version: 1.0 X-Mailer: Evolution 2.26.1 Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org On Mon, 2009-06-22 at 18:44 -0500, Kevin Hilman wrote: > Jon Hunter writes: > > > From: Jon Hunter > > > > There are two scenarios where a race condition could result in a hang in the > > prcm_interrupt handler. These are: > > IIRC, the RX51 tree has a workaround for some hangs seen in this > interrupt handler as well, so this has definitely been seen. The fix > there though was just to no loop, I think your fix is more thorough > and fixes the root cause. Thanks. > > No comments on the funtional changes, but a couple > cosmetic/documentation changes. > > 1) Would it be possible to summarize the requirements in the function > itself as part of this patch. > > 2) With the extra indentation, this function is getting too indented. > Looking closer, an abstraction of the 'enable clocks, poll PM_WKST, > disable clocks loop' could be done an called with the various modules > and offsets. I've attached a hack/patch below to show what I mean. > You could just fold this into your patch. Hi Kevin, Please find the update patch below. Cheers Jon There are two scenarios where a race condition could result in a hang in the prcm_interrupt handler. These are: 1). Waiting for PRM_IRQSTATUS_MPU register to clear. Bit 0 of the PRM_IRQSTATUS_MPU register indicates that a wake-up event is pending for the MPU. This bit can only be cleared if the all the wake-up events latched in the various PM_WKST_x registers have been cleared. If a wake-up event occurred during the processing of the prcm interrupt handler, after the corresponding PM_WKST_x register was checked but before the PRM_IRQSTATUS_MPU was cleared, then the CPU would be stuck forever waiting for bit 0 in PRM_IRQSTATUS_MPU to be cleared. 2). Waiting for the PM_WKST_x register to clear. Some power domains have more than one wake-up source. The PM_WKST_x registers indicate the source of a wake-up event and need to be cleared after a wake-up event occurs. When the PM_WKST_x registers are read and before they are cleared, it is possible that another wake-up event could occur causing another bit to be set in one of the PM_WKST_x registers. If this did occur after reading a PM_WKST_x register then the CPU would miss this event and get stuck forever in a loop waiting for that PM_WKST_x register to clear. This patch address the above race conditions that would result in a hang. Signed-off-by: Jon Hunter --- arch/arm/mach-omap2/pm34xx.c | 148 ++++++++++++++++++------------------------ 1 files changed, 62 insertions(+), 86 deletions(-) } diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index 7a4a525..9ba6b53 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -194,97 +194,73 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state) } } -/* PRCM Interrupt Handler for wakeups */ -static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id) -{ - u32 wkst, irqstatus_mpu; - u32 fclk, iclk; - - /* WKUP */ - wkst = prm_read_mod_reg(WKUP_MOD, PM_WKST); - if (wkst) { - iclk = cm_read_mod_reg(WKUP_MOD, CM_ICLKEN); - fclk = cm_read_mod_reg(WKUP_MOD, CM_FCLKEN); - cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_ICLKEN); - cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_FCLKEN); - prm_write_mod_reg(wkst, WKUP_MOD, PM_WKST); - while (prm_read_mod_reg(WKUP_MOD, PM_WKST)) - cpu_relax(); - cm_write_mod_reg(iclk, WKUP_MOD, CM_ICLKEN); - cm_write_mod_reg(fclk, WKUP_MOD, CM_FCLKEN); - } - - /* CORE */ - wkst = prm_read_mod_reg(CORE_MOD, PM_WKST1); - if (wkst) { - iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN1); - fclk = cm_read_mod_reg(CORE_MOD, CM_FCLKEN1); - cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN1); - cm_set_mod_reg_bits(wkst, CORE_MOD, CM_FCLKEN1); - prm_write_mod_reg(wkst, CORE_MOD, PM_WKST1); - while (prm_read_mod_reg(CORE_MOD, PM_WKST1)) - cpu_relax(); - cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN1); - cm_write_mod_reg(fclk, CORE_MOD, CM_FCLKEN1); - } - wkst = prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3); - if (wkst) { - iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN3); - fclk = cm_read_mod_reg(CORE_MOD, OMAP3430ES2_CM_FCLKEN3); - cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN3); - cm_set_mod_reg_bits(wkst, CORE_MOD, OMAP3430ES2_CM_FCLKEN3); - prm_write_mod_reg(wkst, CORE_MOD, OMAP3430ES2_PM_WKST3); - while (prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3)) - cpu_relax(); - cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN3); - cm_write_mod_reg(fclk, CORE_MOD, OMAP3430ES2_CM_FCLKEN3); - } +/* + * PRCM Interrupt Handler Helper Function + * + * The purpose of this function is to clear any wake-up events latched + * in the PRCM PM_WKST_x registers. It is possible that a wake-up event + * may occur whilst attempting to clear a PM_WKST_x register and thus + * set another bit in this register. A while loop is used to ensure + * that any peripheral wake-up events occurring while attempting to + * clear the PM_WKST_x are detected and cleared. + */ +static void prcm_clear_mod_irqs(s16 module, u16 wkst_off, + u16 iclk_off, u16 fclk_off) { + u32 wkst, fclk, iclk; - /* PER */ - wkst = prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST); + wkst = prm_read_mod_reg(module, wkst_off); if (wkst) { - iclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_ICLKEN); - fclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_FCLKEN); - cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_ICLKEN); - cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_FCLKEN); - prm_write_mod_reg(wkst, OMAP3430_PER_MOD, PM_WKST); - while (prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST)) - cpu_relax(); - cm_write_mod_reg(iclk, OMAP3430_PER_MOD, CM_ICLKEN); - cm_write_mod_reg(fclk, OMAP3430_PER_MOD, CM_FCLKEN); - } - - if (omap_rev() > OMAP3430_REV_ES1_0) { - /* USBHOST */ - wkst = prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, PM_WKST); - if (wkst) { - iclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, - CM_ICLKEN); - fclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, - CM_FCLKEN); - cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD, - CM_ICLKEN); - cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD, - CM_FCLKEN); - prm_write_mod_reg(wkst, OMAP3430ES2_USBHOST_MOD, - PM_WKST); - while (prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, - PM_WKST)) - cpu_relax(); - cm_write_mod_reg(iclk, OMAP3430ES2_USBHOST_MOD, - CM_ICLKEN); - cm_write_mod_reg(fclk, OMAP3430ES2_USBHOST_MOD, - CM_FCLKEN); + iclk = cm_read_mod_reg(module, iclk_off); + fclk = cm_read_mod_reg(module, fclk_off); + while (wkst) { + cm_set_mod_reg_bits(wkst, module, iclk_off); + cm_set_mod_reg_bits(wkst, module, fclk_off); + prm_write_mod_reg(wkst, module, wkst_off); + wkst = prm_read_mod_reg(module, wkst_off); } + cm_write_mod_reg(iclk, module, iclk_off); + cm_write_mod_reg(fclk, module, fclk_off); } +} - irqstatus_mpu = prm_read_mod_reg(OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - prm_write_mod_reg(irqstatus_mpu, OCP_MOD, - OMAP3_PRM_IRQSTATUS_MPU_OFFSET); - - while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET)) - cpu_relax(); +/* + * 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 irqstatus_mpu; + + do { + prcm_clear_mod_irqs(WKUP_MOD, PM_WKST, CM_ICLKEN, CM_FCLKEN); + prcm_clear_mod_irqs(CORE_MOD, PM_WKST1, CM_ICLKEN1, CM_ICLKEN1); + prcm_clear_mod_irqs(CORE_MOD, OMAP3430ES2_PM_WKST3, + CM_ICLKEN3, OMAP3430ES2_CM_FCLKEN3); + prcm_clear_mod_irqs(OMAP3430_PER_MOD, PM_WKST, + CM_ICLKEN, CM_FCLKEN); + if (omap_rev() > OMAP3430_REV_ES1_0) + prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, PM_WKST, + CM_ICLKEN, CM_FCLKEN); + + irqstatus_mpu = prm_read_mod_reg(OCP_MOD, + OMAP3_PRM_IRQSTATUS_MPU_OFFSET); + prm_write_mod_reg(irqstatus_mpu, OCP_MOD, + OMAP3_PRM_IRQSTATUS_MPU_OFFSET); + + } while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET)); return IRQ_HANDLED;