Hi Todd, On Thu, 7 Jul 2011, Todd Poynor wrote: > On Thu, Jul 07, 2011 at 02:25:23AM -0600, Paul Walmsley wrote: > > > -int omap4_cm_wait_module_ready(void __iomem *clkctrl_reg) > > +int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs) > > { > > int i = 0; > > > > - if (!clkctrl_reg) > > + if (!clkctrl_offs) > > return 0; > > > > omap_test_timeout(( > > - ((__raw_readl(clkctrl_reg) & OMAP4430_IDLEST_MASK) == 0) || > > - (((__raw_readl(clkctrl_reg) & OMAP4430_IDLEST_MASK) >> > > - OMAP4430_IDLEST_SHIFT) == 0x2)), > > + _clkctrl_idlest(part, inst, cdoffs, clkctrl_offs) == 0 || > > + _clkctrl_idlest(part, inst, cdoffs, clkctrl_offs) == 0x2), > > MAX_MODULE_READY_TIME, i); > > Suggest adding symbols for the constant IDLEST values, next to the 0x3 > value added for "[PATCH v2 04/13] OMAP: hwmod: Wait the idle status to > be disabled". Done. > Would be nice to call _clkctrl_idlest() once. Agreed. That's now implemented by creating a new static function, _is_module_ready(), that only does one read. > Similar vague questioning of the API names as for the above-mentioned > patch: this waits for the module slave to be ready, don't know if > anything similar is needed for module masters or if it's important to > keep this distinction. I think I know the answer to this one, but would rather not speculate without some hardware investigation. Let's review this issue when Benoît returns. Thanks for the comments, they are much appreciated. - Paul