RE: Issue with _are_all_hardreset_lines_asserted()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 09, 2012 at 12:29:53, Paul Walmsley wrote:
> On Tue, 9 Oct 2012, Archit Taneja wrote:
> 
> > The patch looks fine to me. I tried it out for DSS(with an additional patch to
> > not represent DSS modulemode bits as a slave clock), and modulemode is getting
> > disabled correctly now.
> 
> Thanks, I added your Tested-by: and also updated the patch description 
> which was a little unclear.
> 
> 
> - Paul
> 
> From: Paul Walmsley <paul@xxxxxxxxx>
> Date: Mon, 8 Oct 2012 23:08:15 -0600
> Subject: [PATCH] ARM: OMAP4/AM335x: hwmod: fix disable_module regression in
>  hardreset handling
> 
> Commit eb05f691290e99ee0bd1672317d6add789523c1e ("ARM: OMAP: hwmod:
> partially un-reset hwmods might not be properly enabled") added code
> to skip the IP block disable sequence if all of the block's hardreset
> lines weren't asserted.  But this did not handle the case when no
> hardreset lines were associated with a module, which is the general
> case.  In that situation, the IP block disable would be skipped.  This
> is likely to cause PM regressions.
> 
> So, modify _omap4_disable_module() and _am33xx_disable_module() to
> only bail out early if there are any hardreset lines asserted.  And
> move the AM33xx test above the actual module disable code to ensure
> that the behavior is consistent.
> 

Paul,

I just tested it on Bone + one gpmc fix (will submit shortly) and it boots 
up fine for me. I have also tested for module disable, and it works.

Tested-by: Vaibhav Hiremath <hvaibhav@xxxxxx>
Acked-by: Vaibhav Hiremath <hvaibhav@xxxxxx>


Log for reference - 


[    0.749504] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 84, v - 2
[    0.749630] omap_hwmod: timer3: _am33xx_disable_module
[    0.749652] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 84, v - 30000
[    0.749819] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 88, v - 30002
[    0.749841] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 88, v - 2
[    0.749904] omap_hwmod: timer4: _am33xx_disable_module
[    0.749923] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 88, v - 30000
[    0.750131] _clkctrl_idlest:109 inst - 0, clkctrl_offs - ec, v - 10002
[    0.750152] _clkctrl_idlest:109 inst - 0, clkctrl_offs - ec, v - 2
[    0.750218] omap_hwmod: timer5: _am33xx_disable_module
[    0.750236] _clkctrl_idlest:109 inst - 0, clkctrl_offs - ec, v - 10000
[    0.750253] _clkctrl_idlest:109 inst - 0, clkctrl_offs - ec, v - 30000
[    0.750404] _clkctrl_idlest:109 inst - 0, clkctrl_offs - f0, v - 30002
[    0.750423] _clkctrl_idlest:109 inst - 0, clkctrl_offs - f0, v - 2
[    0.750485] omap_hwmod: timer6: _am33xx_disable_module
[    0.750504] _clkctrl_idlest:109 inst - 0, clkctrl_offs - f0, v - 10000
[    0.750520] _clkctrl_idlest:109 inst - 0, clkctrl_offs - f0, v - 30000
[    0.750666] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 7c, v - 30002
[    0.750685] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 7c, v - 2
[    0.750747] omap_hwmod: timer7: _am33xx_disable_module
[    0.750765] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 7c, v - 10000
[    0.750782] _clkctrl_idlest:109 inst - 0, clkctrl_offs - 7c, v - 30000


Thanks,
Vaibhav


> Reported-by: Archit Taneja <a0393947@xxxxxx>
> Tested-by: Archit Taneja <a0393947@xxxxxx> # DSS
> Cc: Omar Ramirez Luna <omar.luna@xxxxxxxxxx>
> Cc: Vaibhav Hiremath <hvaibhav@xxxxxx>
> Signed-off-by: Paul Walmsley <paul@xxxxxxxxx>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |   31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 299ca28..b969ab1 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1698,6 +1698,29 @@ static bool _are_all_hardreset_lines_asserted(struct omap_hwmod *oh)
>  }
>  
>  /**
> + * _are_any_hardreset_lines_asserted - return true if any part of @oh is
> + * hard-reset
> + * @oh: struct omap_hwmod *
> + *
> + * If any hardreset lines associated with @oh are asserted, then
> + * return true.  Otherwise, if no hardreset lines associated with @oh
> + * are asserted, or if @oh has no hardreset lines, then return false.
> + * This function is used to avoid executing some parts of the IP block
> + * enable/disable sequence if any hardreset line is set.
> + */
> +static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh)
> +{
> +	int rst_cnt = 0;
> +	int i;
> +
> +	for (i = 0; i < oh->rst_lines_cnt && rst_cnt == 0; i++)
> +		if (_read_hardreset(oh, oh->rst_lines[i].name) > 0)
> +			rst_cnt++;
> +
> +	return (rst_cnt) ? true : false;
> +}
> +
> +/**
>   * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4
>   * @oh: struct omap_hwmod *
>   *
> @@ -1715,7 +1738,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh)
>  	 * Since integration code might still be doing something, only
>  	 * disable if all lines are under hardreset.
>  	 */
> -	if (!_are_all_hardreset_lines_asserted(oh))
> +	if (_are_any_hardreset_lines_asserted(oh))
>  		return 0;
>  
>  	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
> @@ -1749,12 +1772,12 @@ static int _am33xx_disable_module(struct omap_hwmod *oh)
>  
>  	pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__);
>  
> +	if (_are_any_hardreset_lines_asserted(oh))
> +		return 0;
> +
>  	am33xx_cm_module_disable(oh->clkdm->cm_inst, oh->clkdm->clkdm_offs,
>  				 oh->prcm.omap4.clkctrl_offs);
>  
> -	if (_are_all_hardreset_lines_asserted(oh))
> -		return 0;
> -
>  	v = _am33xx_wait_target_disable(oh);
>  	if (v)
>  		pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",
> -- 
> 1.7.10.4
> 
> 

--
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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux