Hi
two comments on this one:
On Mon, 27 Jun 2011, Benoit Cousson wrote:
> It is mandatory to wait for a module to be in disabled state before
> potentially disabling source clock or re-asserting a reset.
>
> omap_hwmod_idle and omap_hwmod_shutdown does not wait for
> the module to be fully idle.
>
> Add a cm_xxx accessor to wait the clkctrl idle status to be disabled.
> Fix hwmod_[idle|shutdown] to use this API.
>
> Based on Rajendra's initial patch.
>
> Please note that most interconnects hwmod will return one timeout because
> it is impossible for them to be in idle since the processor is accessing
> the registers though the interconnect.
Should we have some flag in the data for this so the code does not waste
time waiting for those modules to go idle?
> Signed-off-by: Benoit Cousson <b-cousson@xxxxxx>
> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> ---
> arch/arm/mach-omap2/cminst44xx.c | 25 +++++++++++++++++++++++
> arch/arm/mach-omap2/cminst44xx.h | 1 +
> arch/arm/mach-omap2/omap_hwmod.c | 40 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
> index 1df740e..fa44ff5 100644
> --- a/arch/arm/mach-omap2/cminst44xx.c
> +++ b/arch/arm/mach-omap2/cminst44xx.c
> @@ -244,3 +244,28 @@ int omap4_cm_wait_module_ready(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs)
> return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
> }
>
> +/**
> + * omap4_cm_wait_module_idle - wait for a module to be in 'disabled'
> + * state
> + * @part: PRCM partition ID that the CM_CLKCTRL register exists in
> + * @inst: CM instance register offset (*_INST macro)
> + * @cdoffs: Clockdomain register offset (*_CDOFFS macro)
> + * @clkctrl_offs: Module clock control register offset (*_CLKCTRL macro)
> + *
> + * Wait for the module IDLEST to be disabled. Some PRCM transition,
> + * like reset assertion or parent clock de-activation must wait the
> + * module to be fully disabled.
> + */
> +int omap4_cm_wait_module_idle(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs)
> +{
> + int i = 0;
> +
> + if (!clkctrl_offs)
> + return 0;
> +
> + omap_test_timeout(
> + _clkctrl_idlest(part, inst, cdoffs, clkctrl_offs) == 0x3,
> + MAX_MODULE_READY_TIME, i);
> +
> + return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
> +}
> diff --git a/arch/arm/mach-omap2/cminst44xx.h b/arch/arm/mach-omap2/cminst44xx.h
> index 9d39c70..4c5da7d 100644
> --- a/arch/arm/mach-omap2/cminst44xx.h
> +++ b/arch/arm/mach-omap2/cminst44xx.h
> @@ -18,6 +18,7 @@ extern void omap4_cminst_clkdm_force_sleep(u8 part, s16 inst, u16 cdoffs);
> extern void omap4_cminst_clkdm_force_wakeup(u8 part, s16 inst, u16 cdoffs);
>
> extern int omap4_cm_wait_module_ready(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs);
> +extern int omap4_cm_wait_module_idle(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_offs);
>
> /*
> * In an ideal world, we would not export these low-level functions,
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index ea1c976..adbd4b8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1029,6 +1029,36 @@ static int _wait_target_ready(struct omap_hwmod *oh)
> }
>
> /**
> + * _wait_target_disable - wait for a module to be disabled
> + * @oh: struct omap_hwmod *
> + *
> + * Wait for a module @oh to leave slave idle. Returns 0 if the module
> + * does not have an IDLEST bit or if the module successfully leaves
> + * slave idle; otherwise, pass along the return value of the
> + * appropriate *_cm_wait_module_ready() function.
> + */
> +static int _wait_target_disable(struct omap_hwmod *oh)
> +{
> + if (!oh)
> + return -EINVAL;
> +
> + if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
> + return 0;
> +
> + if (oh->flags & HWMOD_NO_IDLEST)
> + return 0;
> +
> + /* TODO: For now just handle OMAP4+ */
> + if (cpu_is_omap24xx() || cpu_is_omap34xx())
> + return 0;
This is a pretty minor issue, but I'd suggest moving this up to the
top of this function so the compiler can optimize it out completely on
non-OMAP4 builds.
> +
> + return omap4_cm_wait_module_idle(oh->clkdm->prcm_partition,
> + oh->clkdm->cm_inst,
> + oh->clkdm->clkdm_offs,
> + oh->prcm.omap4.clkctrl_offs);
> +}
> +
> +/**
> * _lookup_hardreset - fill register bit info for this hwmod/reset line
> * @oh: struct omap_hwmod *
> * @name: name of the reset line in the context of this hwmod
> @@ -1335,6 +1365,8 @@ static int _enable(struct omap_hwmod *oh)
> */
> static int _idle(struct omap_hwmod *oh)
> {
> + int ret;
> +
> if (oh->_state != _HWMOD_STATE_ENABLED) {
> WARN(1, "omap_hwmod: %s: idle state can only be entered from "
> "enabled state\n", oh->name);
> @@ -1347,6 +1379,10 @@ static int _idle(struct omap_hwmod *oh)
> _idle_sysc(oh);
> _del_initiator_dep(oh, mpu_oh);
> _disable_clocks(oh);
> + ret = _wait_target_disable(oh);
> + if (ret)
> + pr_debug("omap_hwmod: %s: _wait_target_disable failed\n",
> + oh->name);
>
> /* Mux pins for device idle if populated */
> if (oh->mux && oh->mux->pads_dynamic)
> @@ -1439,6 +1475,10 @@ static int _shutdown(struct omap_hwmod *oh)
> _del_initiator_dep(oh, mpu_oh);
> /* XXX what about the other system initiators here? dma, dsp */
> _disable_clocks(oh);
> + ret = _wait_target_disable(oh);
> + if (ret)
> + pr_debug("omap_hwmod: %s: _wait_target_disable failed\n",
> + oh->name);
> }
> /* XXX Should this code also force-disable the optional clocks? */
>
> --
> 1.7.0.4
>
- Paul
--
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
