[PATCH 0/3] ARM: OMAP2+: hwmod: resolve some OMAP4 warnings

[PATCH 0/3] ARM: OMAP2+: hwmod: resolve some OMAP4 warnings

[Paul Walmsley]

The decision was made a few months ago to weaken the clockdomain
requirements for hwmods and clocks.  Convert some of these
warnings into debug messages.  In other cases, don't return an
error when a clockdomain isn't specified.  And while here,
appropriately mark some OMAP4 hwmods that don't have idle
reporting bits.

---

hwmod_omap4_warning_fixes_a_3.7
   text    data     bss     dec     hex filename
7333017  764092 5611596 13708705         d12da1 vmlinux.omap2plus_defconfig.orig
7332977  764068 5611596 13708641         d12d61 vmlinux.omap2plus_defconfig

Paul Walmsley (3):
      ARM: OMAP4: hwmod data: add missing HWMOD_NO_IDLEST flags to some PRCM IP blocks
      ARM: OMAP4+: hwmod code: remove clkdm requirement in _omap4_wait_target_*()
      ARM: OMAP2+: hwmod code: convert missing clockdomain warnings to debug messages


 arch/arm/mach-omap2/omap_hwmod.c           |   16 +++++++++-------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +++
 2 files changed, 12 insertions(+), 7 deletions(-)

[PATCH 1/3] ARM: OMAP4: hwmod data: add missing HWMOD_NO_IDLEST flags to some PRCM IP blocks

[Paul Walmsley]

Some struct omap_hwmod records belonging to PRCM IP blocks are missing
HWMOD_NO_IDLEST flags; add them.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index afb6091..f54b3e5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2537,18 +2537,21 @@ static struct omap_hwmod omap44xx_prcm_mpu_hwmod = {
 	.name		= "prcm_mpu",
 	.class		= &omap44xx_prcm_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
+	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* cm_core_aon */
 static struct omap_hwmod omap44xx_cm_core_aon_hwmod = {
 	.name		= "cm_core_aon",
 	.class		= &omap44xx_prcm_hwmod_class,
+	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* cm_core */
 static struct omap_hwmod omap44xx_cm_core_hwmod = {
 	.name		= "cm_core",
 	.class		= &omap44xx_prcm_hwmod_class,
+	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* prm */

[PATCH 2/3] ARM: OMAP4+: hwmod code: remove clkdm requirement in _omap4_wait_target_*()

[Paul Walmsley]

We're no longer requiring struct omap_hwmod records to contain a
clockdomain.  So we shouldn't return -EINVAL any more from
_omap4_wait_target_disable() or _omap4_wait_target_ready() if there's
no clockdomain defined, since that just gets passed back to the
caller.  This can result in pointless warnings under the relaxed data
format.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 37afbd1..80cea5b 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -878,10 +878,10 @@ static void _omap4_enable_module(struct omap_hwmod *oh)
  */
 static int _omap4_wait_target_disable(struct omap_hwmod *oh)
 {
-	if (!oh || !oh->clkdm)
+	if (!oh)
 		return -EINVAL;
 
-	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
+	if (oh->_int_flags & _HWMOD_NO_MPU_PORT || !oh->clkdm)
 		return 0;
 
 	if (oh->flags & HWMOD_NO_IDLEST)
@@ -2531,10 +2531,10 @@ static int _omap2_wait_target_ready(struct omap_hwmod *oh)
  */
 static int _omap4_wait_target_ready(struct omap_hwmod *oh)
 {
-	if (!oh || !oh->clkdm)
+	if (!oh)
 		return -EINVAL;
 
-	if (oh->flags & HWMOD_NO_IDLEST)
+	if (oh->flags & HWMOD_NO_IDLEST || !oh->clkdm)
 		return 0;
 
 	if (!_find_mpu_rt_port(oh))

[PATCH 3/3] ARM: OMAP2+: hwmod code: convert missing clockdomain warnings to debug messages

[Paul Walmsley]

The decision was made a few months ago to allow struct omap_hwmod
records and struct clk records to omit clockdomain information if the
clockdomain is not software-controllable.  See for example commit
868c157df9721675c19729eed2c96bac6c3f1d01 ("ARM: OMAP2+: hwmod: remove
prm_clkdm, cm_clkdm; allow hwmods to have no clockdomain").

So convert an existing pr_warning() to a pr_debug() (regarding missing
clockdomains in clocks), and add a pr_debug() for missing hwmod
clockdomains.  It's still useful to enable these messages for
debugging, since missing clockdomains can cause hard-to-debug problems
with power management; see for example commit
6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data:
Force a DPLL clkdm/pwrdm ON before a relock").

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 80cea5b..e05228a 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -685,8 +685,8 @@ static int _init_main_clk(struct omap_hwmod *oh)
 	}
 
 	if (!oh->_clk->clkdm)
-		pr_warning("omap_hwmod: %s: missing clockdomain for %s.\n",
-			   oh->main_clk, oh->_clk->name);
+		pr_debug("omap_hwmod: %s: missing clockdomain for %s.\n",
+			 oh->main_clk, oh->_clk->name);
 
 	return ret;
 }
@@ -1380,8 +1380,10 @@ static struct omap_hwmod *_lookup(const char *name)
  */
 static int _init_clkdm(struct omap_hwmod *oh)
 {
-	if (!oh->clkdm_name)
+	if (!oh->clkdm_name) {
+		pr_debug("omap_hwmod: %s: missing clockdomain\n", oh->name);
 		return 0;
+	}
 
 	oh->clkdm = clkdm_lookup(oh->clkdm_name);
 	if (!oh->clkdm) {

[PATCH 3/3] ARM: OMAP2+: hwmod code: convert missing clockdomain warnings to debug messages

[Cousson, Benoit]

Hi Paul,

On 9/18/2012 4:12 PM, Paul Walmsley wrote:
> The decision was made a few months ago to allow struct omap_hwmod
> records and struct clk records to omit clockdomain information if the
> clockdomain is not software-controllable.  See for example commit
> 868c157df9721675c19729eed2c96bac6c3f1d01 ("ARM: OMAP2+: hwmod: remove
> prm_clkdm, cm_clkdm; allow hwmods to have no clockdomain").
>
> So convert an existing pr_warning() to a pr_debug() (regarding missing
> clockdomains in clocks), and add a pr_debug() for missing hwmod
> clockdomains.

At some point I sent you a small RFC to still keep the message if 
neither oh->clkdm nor oh->_clk->clkdm was there. I've never had the time 
to test it, but that should then cover OMAP2, 3 & 4 cases.

Regards,
Benoit

>  It's still useful to enable these messages for
> debugging, since missing clockdomains can cause hard-to-debug problems
> with power management; see for example commit
> 6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data:
> Force a DPLL clkdm/pwrdm ON before a relock").
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Beno?t Cousson <b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c |    8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 80cea5b..e05228a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -685,8 +685,8 @@ static int _init_main_clk(struct omap_hwmod *oh)
>   	}
>
>   	if (!oh->_clk->clkdm)
> -		pr_warning("omap_hwmod: %s: missing clockdomain for %s.\n",
> -			   oh->main_clk, oh->_clk->name);
> +		pr_debug("omap_hwmod: %s: missing clockdomain for %s.\n",
> +			 oh->main_clk, oh->_clk->name);
>
>   	return ret;
>   }
> @@ -1380,8 +1380,10 @@ static struct omap_hwmod *_lookup(const char *name)
>    */
>   static int _init_clkdm(struct omap_hwmod *oh)
>   {
> -	if (!oh->clkdm_name)
> +	if (!oh->clkdm_name) {
> +		pr_debug("omap_hwmod: %s: missing clockdomain\n", oh->name);
>   		return 0;
> +	}
>
>   	oh->clkdm = clkdm_lookup(oh->clkdm_name);
>   	if (!oh->clkdm) {
>
>

[PATCH 3/3] ARM: OMAP2+: hwmod code: convert missing clockdomain warnings to debug messages

[Paul Walmsley]

Hi Beno?t,

On Tue, 18 Sep 2012, Cousson, Benoit wrote:

> On 9/18/2012 4:12 PM, Paul Walmsley wrote:
> > The decision was made a few months ago to allow struct omap_hwmod
> > records and struct clk records to omit clockdomain information if the
> > clockdomain is not software-controllable.  See for example commit
> > 868c157df9721675c19729eed2c96bac6c3f1d01 ("ARM: OMAP2+: hwmod: remove
> > prm_clkdm, cm_clkdm; allow hwmods to have no clockdomain").
> > 
> > So convert an existing pr_warning() to a pr_debug() (regarding missing
> > clockdomains in clocks), and add a pr_debug() for missing hwmod
> > clockdomains.
> 
> At some point I sent you a small RFC to still keep the message if neither
> oh->clkdm nor oh->_clk->clkdm was there. I've never had the time to test it,
> but that should then cover OMAP2, 3 & 4 cases.

OK let's plan to test and add something like that during the 3.8 time 
frame... probably it should also depend on the value of clkdm_control.  
It's not clear to me exactly what that conditional should look like...


- Paul

[PATCH 3/3] ARM: OMAP2+: hwmod code: convert missing clockdomain warnings to debug messages

[Cousson, Benoit]

Hi Paul,

On 9/19/2012 5:48 PM, Paul Walmsley wrote:
> Hi Beno?t,
>
> On Tue, 18 Sep 2012, Cousson, Benoit wrote:
>
>> On 9/18/2012 4:12 PM, Paul Walmsley wrote:
>>> The decision was made a few months ago to allow struct omap_hwmod
>>> records and struct clk records to omit clockdomain information if the
>>> clockdomain is not software-controllable.  See for example commit
>>> 868c157df9721675c19729eed2c96bac6c3f1d01 ("ARM: OMAP2+: hwmod: remove
>>> prm_clkdm, cm_clkdm; allow hwmods to have no clockdomain").
>>>
>>> So convert an existing pr_warning() to a pr_debug() (regarding missing
>>> clockdomains in clocks), and add a pr_debug() for missing hwmod
>>> clockdomains.
>>
>> At some point I sent you a small RFC to still keep the message if neither
>> oh->clkdm nor oh->_clk->clkdm was there. I've never had the time to test it,
>> but that should then cover OMAP2, 3 & 4 cases.
>
> OK let's plan to test and add something like that during the 3.8 time
> frame... probably it should also depend on the value of clkdm_control.
> It's not clear to me exactly what that conditional should look like...

OK, I was thinking of a more basic thing. If some more values have to be 
checked it might be a little bit more tricky.

Let's see that later.

Thanks,
Benoit