Jon Hunter <[email protected]> writes: > On Mon, 2009-06-22 at 18:44 -0500, Kevin Hilman wrote: >> Jon Hunter <[email protected]> writes: >> >> > From: Jon Hunter <[email protected]> >> > >> > 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. >
Thanks, incorporating this into a set of PRCM interrupt fixes to be posted shortly. Kevin > > > > 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 <[email protected]> > --- > 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; > } > -- > 1.6.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
