[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

+ Suman, lakml

Hi Roger

On Thu, 18 Dec 2014, Roger Quadros wrote:

> Fixing up Paul's email id.
> 
> cheers,
> -roger
> 
> On 18/12/14 17:49, Roger Quadros wrote:
> > There are quite a few hwmods that don't have sysconfig register and so
> > _find_mpu_rt_port(oh) will return NULL thus preventing ready state check
> > on those modules after the module is enabled.

Hmm.  Any IP block that exposes registers that are accessible by the MPU 
should have an MPU register target port, even if there's no SYSCONFIG 
register.  And if an IP block doesn't have registers that are accessible 
from the MPU, then there shouldn't be much point to waiting for the module 
to become ready.

Looks like the real problem is the test for oh->class->sysc before the 
call to _init_mpu_rt_base().  That was introduced by commit 6423d6df1440 
("ARM: OMAP2+: hwmod: check for module address space during init"). It's 
not clear to me why that test was added, since _init_mpu_rt_base() doesn't 
do anything with oh->class->sysc or SYSCONFIG registers.

Could you please test the following patch?
I don't have an AM437x-gp-evm.  


- Paul

---
 arch/arm/mach-omap2/omap_hwmod.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index cbb908dc5cf0..ce6d11f3eda7 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2451,14 +2451,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 				oh->name, np->name);
 	}
 
-	if (oh->class->sysc) {
-		r = _init_mpu_rt_base(oh, NULL, index, np);
-		if (r < 0) {
-			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
-			     oh->name);
-			return 0;
-		}
-	}
+	r = _init_mpu_rt_base(oh, NULL, index, np);
+	if (r < 0)
+		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
+			 oh->name);
 
 	r = _init_clocks(oh, NULL);
 	if (r < 0) {
-- 
2.1.4

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Lokesh Vutla]

Hi Paul,
On Saturday 03 January 2015 02:40 AM, Paul Walmsley wrote:
> + Suman, lakml
> 
> Hi Roger
> 
> On Thu, 18 Dec 2014, Roger Quadros wrote:
> 
>> Fixing up Paul's email id.
>>
>> cheers,
>> -roger
>>
>> On 18/12/14 17:49, Roger Quadros wrote:
>>> There are quite a few hwmods that don't have sysconfig register and so
>>> _find_mpu_rt_port(oh) will return NULL thus preventing ready state check
>>> on those modules after the module is enabled.
> 
> Hmm.  Any IP block that exposes registers that are accessible by the MPU 
> should have an MPU register target port, even if there's no SYSCONFIG 
> register.  And if an IP block doesn't have registers that are accessible 
> from the MPU, then there shouldn't be much point to waiting for the module 
> to become ready.
> 
> Looks like the real problem is the test for oh->class->sysc before the 
> call to _init_mpu_rt_base().  That was introduced by commit 6423d6df1440 
> ("ARM: OMAP2+: hwmod: check for module address space during init"). It's 
> not clear to me why that test was added, since _init_mpu_rt_base() doesn't 
> do anything with oh->class->sysc or SYSCONFIG registers.
This was introduced by commit 
97597b962529 (ARM: OMAP2+: hwmod: Don't call _init_mpu_rt_base if no sysc)
Patch description states that "there are few hwmod which doesn't have sysconfig registers and hence
no need to ioremap() them in early init code".
Isn't this correct? 
May be a dumb question: If IP doesn't have sysconfig, is there any case that hwmod does
access the register address space of that IP? Why do we need to enable MPU register target port?

Thanks and regards,
Lokesh

> 
> Could you please test the following patch?
> I don't have an AM437x-gp-evm.  
> 
> 
> - Paul
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index cbb908dc5cf0..ce6d11f3eda7 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2451,14 +2451,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>  				oh->name, np->name);
>  	}
>  
> -	if (oh->class->sysc) {
> -		r = _init_mpu_rt_base(oh, NULL, index, np);
> -		if (r < 0) {
> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> -			     oh->name);
> -			return 0;
> -		}
> -	}
> +	r = _init_mpu_rt_base(oh, NULL, index, np);
> +	if (r < 0)
> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
> +			 oh->name);
>  
>  	r = _init_clocks(oh, NULL);
>  	if (r < 0) {
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Suman Anna]

On 01/05/2015 02:35 AM, Lokesh Vutla wrote:
> Hi Paul,
> On Saturday 03 January 2015 02:40 AM, Paul Walmsley wrote:
>> + Suman, lakml
>>
>> Hi Roger
>>
>> On Thu, 18 Dec 2014, Roger Quadros wrote:
>>
>>> Fixing up Paul's email id.
>>>
>>> cheers,
>>> -roger
>>>
>>> On 18/12/14 17:49, Roger Quadros wrote:
>>>> There are quite a few hwmods that don't have sysconfig register and so
>>>> _find_mpu_rt_port(oh) will return NULL thus preventing ready state check
>>>> on those modules after the module is enabled.
>>
>> Hmm.  Any IP block that exposes registers that are accessible by the MPU 
>> should have an MPU register target port, even if there's no SYSCONFIG 
>> register.  And if an IP block doesn't have registers that are accessible 
>> from the MPU, then there shouldn't be much point to waiting for the module 
>> to become ready.
>>
>> Looks like the real problem is the test for oh->class->sysc before the 
>> call to _init_mpu_rt_base().  That was introduced by commit 6423d6df1440 
>> ("ARM: OMAP2+: hwmod: check for module address space during init"). It's 
>> not clear to me why that test was added, since _init_mpu_rt_base() doesn't 
>> do anything with oh->class->sysc or SYSCONFIG registers.
> This was introduced by commit 
> 97597b962529 (ARM: OMAP2+: hwmod: Don't call _init_mpu_rt_base if no sysc)

That's right, the test was present even before 6423d6df1440, I merely
made this a block.

> Patch description states that "there are few hwmod which doesn't have sysconfig registers and hence
> no need to ioremap() them in early init code".
> Isn't this correct? 
> May be a dumb question: If IP doesn't have sysconfig, is there any case that hwmod does
> access the register address space of that IP? Why do we need to enable MPU register target port?
> 
> Thanks and regards,
> Lokesh
> 
>>
>> Could you please test the following patch?
>> I don't have an AM437x-gp-evm.  
>>
>>
>> - Paul
>>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index cbb908dc5cf0..ce6d11f3eda7 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -2451,14 +2451,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>>  				oh->name, np->name);
>>  	}
>>  
>> -	if (oh->class->sysc) {
>> -		r = _init_mpu_rt_base(oh, NULL, index, np);
>> -		if (r < 0) {
>> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
>> -			     oh->name);
>> -			return 0;
>> -		}
>> -	}
>> +	r = _init_mpu_rt_base(oh, NULL, index, np);
>> +	if (r < 0)
>> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
>> +			 oh->name);
>>  
>>  	r = _init_clocks(oh, NULL);
>>  	if (r < 0) {
>>
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

+ Santosh

Hi Lokesh

On Mon, 5 Jan 2015, Lokesh Vutla wrote:
> On Saturday 03 January 2015 02:40 AM, Paul Walmsley wrote:
> > On Thu, 18 Dec 2014, Roger Quadros wrote:
> > 
> >> On 18/12/14 17:49, Roger Quadros wrote:
> >>> There are quite a few hwmods that don't have sysconfig register and so
> >>> _find_mpu_rt_port(oh) will return NULL thus preventing ready state check
> >>> on those modules after the module is enabled.
> > 
> > Hmm.  Any IP block that exposes registers that are accessible by the MPU 
> > should have an MPU register target port, even if there's no SYSCONFIG 
> > register.  And if an IP block doesn't have registers that are accessible 
> > from the MPU, then there shouldn't be much point to waiting for the module 
> > to become ready.
> > 
> > Looks like the real problem is the test for oh->class->sysc before the 
> > call to _init_mpu_rt_base().  That was introduced by commit 6423d6df1440 
> > ("ARM: OMAP2+: hwmod: check for module address space during init"). It's 
> > not clear to me why that test was added, since _init_mpu_rt_base() doesn't 
> > do anything with oh->class->sysc or SYSCONFIG registers.
> This was introduced by commit 
> 97597b962529 (ARM: OMAP2+: hwmod: Don't call _init_mpu_rt_base if no sysc)

Yes, you're right.  I misread commit 6423d6df1440.

> Patch description states that "there are few hwmod which doesn't have sysconfig registers and hence
> no need to ioremap() them in early init code".

The MPU register target port code doesn't only determine whether the 
hwmod code should map the IP block's address space.  It also determines 
whether or not the hwmod code needs to wait for the IP block to become 
ready after being enabled, so register accesses by non-hwmod code can 
succeed.

> Isn't this correct? 

That commit 97597b962529 is bogus.  If the goal was to skip the ioremap(), 
then the right fix would have been to just skip the ioremap().  The 
existing commit also skips the call to _save_mpu_port_index(), which 
caused the problem that Roger's patch is trying to fix.

> May be a dumb question: If IP doesn't have sysconfig, is there any case 
> that hwmod does access the register address space of that IP? Why do we 
> need to enable MPU register target port?

hwmod shouldn't touch the IP block registers if there are no SYSCONFIG, 
SYSSTATUS, etc. registers, and there's no IP block-specific reset code.  
But other code on the system (like the IP block's device driver) will, and 
the system needs to indicate that the IP block is ready before those 
accesses occur.


- Paul

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

On Mon, 5 Jan 2015, Suman Anna wrote:

> That's right, the test was present even before 6423d6df1440, I merely
> made this a block.

Indeed, I misread the commit.  Sorry about ihat.


- Paul

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[santosh.shilimkar at oracle.com]

On 1/5/15 2:19 PM, Paul Walmsley wrote:
> + Santosh
>
> Hi Lokesh
>
> On Mon, 5 Jan 2015, Lokesh Vutla wrote:
>> On Saturday 03 January 2015 02:40 AM, Paul Walmsley wrote:
>>> On Thu, 18 Dec 2014, Roger Quadros wrote:
>>>
>>>> On 18/12/14 17:49, Roger Quadros wrote:
>>>>> There are quite a few hwmods that don't have sysconfig register and so
>>>>> _find_mpu_rt_port(oh) will return NULL thus preventing ready state check
>>>>> on those modules after the module is enabled.
>>>
>>> Hmm.  Any IP block that exposes registers that are accessible by the MPU
>>> should have an MPU register target port, even if there's no SYSCONFIG
>>> register.  And if an IP block doesn't have registers that are accessible
>>> from the MPU, then there shouldn't be much point to waiting for the module
>>> to become ready.
>>>
>>> Looks like the real problem is the test for oh->class->sysc before the
>>> call to _init_mpu_rt_base().  That was introduced by commit 6423d6df1440
>>> ("ARM: OMAP2+: hwmod: check for module address space during init"). It's
>>> not clear to me why that test was added, since _init_mpu_rt_base() doesn't
>>> do anything with oh->class->sysc or SYSCONFIG registers.
>> This was introduced by commit
>> 97597b962529 (ARM: OMAP2+: hwmod: Don't call _init_mpu_rt_base if no sysc)
>
> Yes, you're right.  I misread commit 6423d6df1440.
>
>> Patch description states that "there are few hwmod which doesn't have sysconfig registers and hence
>> no need to ioremap() them in early init code".
>
> The MPU register target port code doesn't only determine whether the
> hwmod code should map the IP block's address space.  It also determines
> whether or not the hwmod code needs to wait for the IP block to become
> ready after being enabled, so register accesses by non-hwmod code can
> succeed.
>
IIRC, Yes the intention was to skip the ioremap o.w there were some 
crashes seen. I am fine if that check is actually moved closer
to iormap as it should have been first place.

Regards,
Santosh
seen

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

Roger, Lokesh, could you try this one instead?

It passes all the basic tests here except it does not boot on the 4460 
VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
works on your AM4372 boards, since I don't have that one.

Test logs are here:

http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/


- Paul


>From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 5 Jan 2015 15:49:57 -0700
Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
 registers. Experimental.

---
 arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index cbb908dc5cf0..03df8833d399 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
 	if (oh->class->reset) {
+		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
+		     oh->name);
 		r = oh->class->reset(oh);
 	} else {
 		if (oh->rst_lines_cnt > 0) {
@@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
 }
 
 /**
- * _init_mpu_rt_base - populate the virtual address for a hwmod
+ * _init_mpu_rt_base - populate the MPU port and virtual address
  * @oh: struct omap_hwmod * to locate the virtual address
  * @data: (unused, caller should pass NULL)
  * @index: index of the reg entry iospace in device tree
  * @np: struct device_node * of the IP block's device node in the DT data
  *
- * Cache the virtual address used by the MPU to access this IP block's
- * registers.  This address is needed early so the OCP registers that
- * are part of the device's address space can be ioremapped properly.
+ * Cache the interconnect target port and the virtual address used by
+ * the MPU to access this IP block's registers.  The address is needed
+ * early so the OCP registers that are part of the device's address
+ * space can be ioremapped properly.  The presence or absence of the
+ * interconnect target port also indicates whether the hwmod code
+ * should wait for the IP block to indicate readiness after it is
+ * enabled.
  *
  * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
  * -ENXIO on absent or invalid register target address space.
@@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
 	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
 		return -ENXIO;
 
+	/*
+	 * If there's no need for the hwmod code to read or write to
+	 * the IP block registers, bail out early before the ioremap()
+	 */
+	if (!oh->class->sysc)
+		return 0;
+
 	mem = _find_mpu_rt_addr_space(oh);
 	if (!mem) {
 		pr_debug("omap_hwmod: %s: no MPU register target found\n",
@@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 				oh->name, np->name);
 	}
 
-	if (oh->class->sysc) {
-		r = _init_mpu_rt_base(oh, NULL, index, np);
-		if (r < 0) {
-			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
-			     oh->name);
-			return 0;
-		}
-	}
+	r = _init_mpu_rt_base(oh, NULL, index, np);
+	if (r < 0)
+		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
+			 oh->name);
 
 	r = _init_clocks(oh, NULL);
 	if (r < 0) {
-- 
2.1.4

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Lokesh Vutla]

Hi Paul,
On Tuesday 06 January 2015 07:34 AM, Paul Walmsley wrote:
> 
> Roger, Lokesh, could you try this one instead?
Yep, the below patch works on AM437x.
Boot logs here: http://paste.ubuntu.com/9680938/

Thanks and regards,
Lokesh
> 
> It passes all the basic tests here except it does not boot on the 4460 
> VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
> works on your AM4372 boards, since I don't have that one.
> 
> Test logs are here:
> 
> http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/
> 
> 
> - Paul
> 
> 
> From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
> From: Paul Walmsley <paul@pwsan.com>
> Date: Mon, 5 Jan 2015 15:49:57 -0700
> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>  registers. Experimental.
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index cbb908dc5cf0..03df8833d399 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>  
>  	if (oh->class->reset) {
> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
> +		     oh->name);
>  		r = oh->class->reset(oh);
>  	} else {
>  		if (oh->rst_lines_cnt > 0) {
> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>  }
>  
>  /**
> - * _init_mpu_rt_base - populate the virtual address for a hwmod
> + * _init_mpu_rt_base - populate the MPU port and virtual address
>   * @oh: struct omap_hwmod * to locate the virtual address
>   * @data: (unused, caller should pass NULL)
>   * @index: index of the reg entry iospace in device tree
>   * @np: struct device_node * of the IP block's device node in the DT data
>   *
> - * Cache the virtual address used by the MPU to access this IP block's
> - * registers.  This address is needed early so the OCP registers that
> - * are part of the device's address space can be ioremapped properly.
> + * Cache the interconnect target port and the virtual address used by
> + * the MPU to access this IP block's registers.  The address is needed
> + * early so the OCP registers that are part of the device's address
> + * space can be ioremapped properly.  The presence or absence of the
> + * interconnect target port also indicates whether the hwmod code
> + * should wait for the IP block to indicate readiness after it is
> + * enabled.
>   *
>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>   * -ENXIO on absent or invalid register target address space.
> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>  		return -ENXIO;
>  
> +	/*
> +	 * If there's no need for the hwmod code to read or write to
> +	 * the IP block registers, bail out early before the ioremap()
> +	 */
> +	if (!oh->class->sysc)
> +		return 0;
> +
>  	mem = _find_mpu_rt_addr_space(oh);
>  	if (!mem) {
>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>  				oh->name, np->name);
>  	}
>  
> -	if (oh->class->sysc) {
> -		r = _init_mpu_rt_base(oh, NULL, index, np);
> -		if (r < 0) {
> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> -			     oh->name);
> -			return 0;
> -		}
> -	}
> +	r = _init_mpu_rt_base(oh, NULL, index, np);
> +	if (r < 0)
> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
> +			 oh->name);
>  
>  	r = _init_clocks(oh, NULL);
>  	if (r < 0) {
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Suman Anna]

Hi Paul,

On 01/06/2015 02:14 AM, Lokesh Vutla wrote:
> Hi Paul,
> On Tuesday 06 January 2015 07:34 AM, Paul Walmsley wrote:
>>
>> Roger, Lokesh, could you try this one instead?
> Yep, the below patch works on AM437x.
> Boot logs here: http://paste.ubuntu.com/9680938/
> 
> Thanks and regards,
> Lokesh
>>
>> It passes all the basic tests here except it does not boot on the 4460 
>> VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
>> works on your AM4372 boards, since I don't have that one.
>>
>> Test logs are here:
>>
>> http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/
>>
>>
>> - Paul
>>
>>
>> From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
>> From: Paul Walmsley <paul@pwsan.com>
>> Date: Mon, 5 Jan 2015 15:49:57 -0700
>> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>>  registers. Experimental.
>>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>>  1 file changed, 21 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index cbb908dc5cf0..03df8833d399 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>>  
>>  	if (oh->class->reset) {
>> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
>> +		     oh->name);
>>  		r = oh->class->reset(oh);
>>  	} else {
>>  		if (oh->rst_lines_cnt > 0) {
>> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>>  }
>>  
>>  /**
>> - * _init_mpu_rt_base - populate the virtual address for a hwmod
>> + * _init_mpu_rt_base - populate the MPU port and virtual address
>>   * @oh: struct omap_hwmod * to locate the virtual address
>>   * @data: (unused, caller should pass NULL)
>>   * @index: index of the reg entry iospace in device tree
>>   * @np: struct device_node * of the IP block's device node in the DT data
>>   *
>> - * Cache the virtual address used by the MPU to access this IP block's
>> - * registers.  This address is needed early so the OCP registers that
>> - * are part of the device's address space can be ioremapped properly.
>> + * Cache the interconnect target port and the virtual address used by
>> + * the MPU to access this IP block's registers.  The address is needed
>> + * early so the OCP registers that are part of the device's address
>> + * space can be ioremapped properly.  The presence or absence of the
>> + * interconnect target port also indicates whether the hwmod code
>> + * should wait for the IP block to indicate readiness after it is
>> + * enabled.
>>   *
>>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>>   * -ENXIO on absent or invalid register target address space.
>> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>>  		return -ENXIO;
>>  
>> +	/*
>> +	 * If there's no need for the hwmod code to read or write to
>> +	 * the IP block registers, bail out early before the ioremap()
>> +	 */
>> +	if (!oh->class->sysc)
>> +		return 0;
>> +
>>  	mem = _find_mpu_rt_addr_space(oh);
>>  	if (!mem) {
>>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
>> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>>  				oh->name, np->name);
>>  	}
>>  
>> -	if (oh->class->sysc) {
>> -		r = _init_mpu_rt_base(oh, NULL, index, np);
>> -		if (r < 0) {
>> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
>> -			     oh->name);
>> -			return 0;
>> -		}
>> -	}
>> +	r = _init_mpu_rt_base(oh, NULL, index, np);
>> +	if (r < 0)
>> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
>> +			 oh->name);

You have removed the return from the above block on failure. If any DT
entry doesn't have the reg property, this will hang the kernel boot.
Just remove the "reg" entry from any of the existing DT, and you will
run into the issue, this is what 6423d6df1440 ("ARM: OMAP2+: hwmod:
check for module address space during init") fixed. Also, are you sure
you want to turn the WARN into a pr_debug, it won't even show during the
kernel boot log if the reg base is missing.

regards
Suman

>>  
>>  	r = _init_clocks(oh, NULL);
>>  	if (r < 0) {
>>
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Suman Anna]

Hi Paul,

On 01/06/2015 11:14 AM, Suman Anna wrote:
> Hi Paul,
> 
> On 01/06/2015 02:14 AM, Lokesh Vutla wrote:
>> Hi Paul,
>> On Tuesday 06 January 2015 07:34 AM, Paul Walmsley wrote:
>>>
>>> Roger, Lokesh, could you try this one instead?
>> Yep, the below patch works on AM437x.
>> Boot logs here: http://paste.ubuntu.com/9680938/
>>
>> Thanks and regards,
>> Lokesh
>>>
>>> It passes all the basic tests here except it does not boot on the 4460 
>>> VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
>>> works on your AM4372 boards, since I don't have that one.

Looking at your rc3 log @
http://www.pwsan.com/omap/testlogs/test_v3.19-rc3/20150105224749/boot/4460varsomom/,
I do see a missing reg entry for mailbox, and that's the reason for your
hang because of the missing bail out in your new patch.

[    0.261932] ------------[ cut here ]------------
[    0.261962] WARNING: CPU: 0 PID: 1 at
arch/arm/mach-omap2/omap_hwmod.c:2458 _init+0x3a0/0x3f0()
[    0.261962] omap_hwmod: mailbox: doesn't have mpu register target base
...
...
[    0.262329] ---[ end trace a1be72591db4662e ]---

Now that said, this is weird, since the reg property for mailbox is
defined in the base omap4.dtsi and should be inherited by the 4460
VAR-SOM-OM, unless you are booting with an old dtb.

regards
Suman

>>>
>>> Test logs are here:
>>>
>>> http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/
>>>
>>>
>>> - Paul
>>>
>>>
>>> From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
>>> From: Paul Walmsley <paul@pwsan.com>
>>> Date: Mon, 5 Jan 2015 15:49:57 -0700
>>> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>>>  registers. Experimental.
>>>
>>> ---
>>>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>>>  1 file changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index cbb908dc5cf0..03df8833d399 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>>>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>>>  
>>>  	if (oh->class->reset) {
>>> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
>>> +		     oh->name);
>>>  		r = oh->class->reset(oh);
>>>  	} else {
>>>  		if (oh->rst_lines_cnt > 0) {
>>> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>>>  }
>>>  
>>>  /**
>>> - * _init_mpu_rt_base - populate the virtual address for a hwmod
>>> + * _init_mpu_rt_base - populate the MPU port and virtual address
>>>   * @oh: struct omap_hwmod * to locate the virtual address
>>>   * @data: (unused, caller should pass NULL)
>>>   * @index: index of the reg entry iospace in device tree
>>>   * @np: struct device_node * of the IP block's device node in the DT data
>>>   *
>>> - * Cache the virtual address used by the MPU to access this IP block's
>>> - * registers.  This address is needed early so the OCP registers that
>>> - * are part of the device's address space can be ioremapped properly.
>>> + * Cache the interconnect target port and the virtual address used by
>>> + * the MPU to access this IP block's registers.  The address is needed
>>> + * early so the OCP registers that are part of the device's address
>>> + * space can be ioremapped properly.  The presence or absence of the
>>> + * interconnect target port also indicates whether the hwmod code
>>> + * should wait for the IP block to indicate readiness after it is
>>> + * enabled.
>>>   *
>>>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>>>   * -ENXIO on absent or invalid register target address space.
>>> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>>>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>>>  		return -ENXIO;
>>>  
>>> +	/*
>>> +	 * If there's no need for the hwmod code to read or write to
>>> +	 * the IP block registers, bail out early before the ioremap()
>>> +	 */
>>> +	if (!oh->class->sysc)
>>> +		return 0;
>>> +
>>>  	mem = _find_mpu_rt_addr_space(oh);
>>>  	if (!mem) {
>>>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
>>> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>>>  				oh->name, np->name);
>>>  	}
>>>  
>>> -	if (oh->class->sysc) {
>>> -		r = _init_mpu_rt_base(oh, NULL, index, np);
>>> -		if (r < 0) {
>>> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
>>> -			     oh->name);
>>> -			return 0;
>>> -		}
>>> -	}
>>> +	r = _init_mpu_rt_base(oh, NULL, index, np);
>>> +	if (r < 0)
>>> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
>>> +			 oh->name);
> 
> You have removed the return from the above block on failure. If any DT
> entry doesn't have the reg property, this will hang the kernel boot.
> Just remove the "reg" entry from any of the existing DT, and you will
> run into the issue, this is what 6423d6df1440 ("ARM: OMAP2+: hwmod:
> check for module address space during init") fixed. Also, are you sure
> you want to turn the WARN into a pr_debug, it won't even show during the
> kernel boot log if the reg base is missing.
> 
> regards
> Suman
> 
>>>  
>>>  	r = _init_clocks(oh, NULL);
>>>  	if (r < 0) {
>>>
>>
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Suman Anna]

On 01/06/2015 11:27 AM, Suman Anna wrote:
> Hi Paul,
> 
> On 01/06/2015 11:14 AM, Suman Anna wrote:
>> Hi Paul,
>>
>> On 01/06/2015 02:14 AM, Lokesh Vutla wrote:
>>> Hi Paul,
>>> On Tuesday 06 January 2015 07:34 AM, Paul Walmsley wrote:
>>>>
>>>> Roger, Lokesh, could you try this one instead?
>>> Yep, the below patch works on AM437x.
>>> Boot logs here: http://paste.ubuntu.com/9680938/
>>>
>>> Thanks and regards,
>>> Lokesh
>>>>
>>>> It passes all the basic tests here except it does not boot on the 4460 
>>>> VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
>>>> works on your AM4372 boards, since I don't have that one.
> 
> Looking at your rc3 log @
> http://www.pwsan.com/omap/testlogs/test_v3.19-rc3/20150105224749/boot/4460varsomom/,
> I do see a missing reg entry for mailbox, and that's the reason for your
> hang because of the missing bail out in your new patch.
> 
> [    0.261932] ------------[ cut here ]------------
> [    0.261962] WARNING: CPU: 0 PID: 1 at
> arch/arm/mach-omap2/omap_hwmod.c:2458 _init+0x3a0/0x3f0()
> [    0.261962] omap_hwmod: mailbox: doesn't have mpu register target base
> ...
> ...
> [    0.262329] ---[ end trace a1be72591db4662e ]---
> 
> Now that said, this is weird, since the reg property for mailbox is
> defined in the base omap4.dtsi and should be inherited by the 4460
> VAR-SOM-OM, unless you are booting with an old dtb.
> 
> regards
> Suman
> 
>>>>
>>>> Test logs are here:
>>>>
>>>> http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/
>>>>
>>>>
>>>> - Paul
>>>>
>>>>
>>>> From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
>>>> From: Paul Walmsley <paul@pwsan.com>
>>>> Date: Mon, 5 Jan 2015 15:49:57 -0700
>>>> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>>>>  registers. Experimental.
>>>>
>>>> ---
>>>>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>>>>  1 file changed, 21 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>> index cbb908dc5cf0..03df8833d399 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>>>>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>>>>  
>>>>  	if (oh->class->reset) {
>>>> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
>>>> +		     oh->name);
>>>>  		r = oh->class->reset(oh);
>>>>  	} else {
>>>>  		if (oh->rst_lines_cnt > 0) {
>>>> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>>>>  }
>>>>  
>>>>  /**
>>>> - * _init_mpu_rt_base - populate the virtual address for a hwmod
>>>> + * _init_mpu_rt_base - populate the MPU port and virtual address
>>>>   * @oh: struct omap_hwmod * to locate the virtual address
>>>>   * @data: (unused, caller should pass NULL)
>>>>   * @index: index of the reg entry iospace in device tree
>>>>   * @np: struct device_node * of the IP block's device node in the DT data
>>>>   *
>>>> - * Cache the virtual address used by the MPU to access this IP block's
>>>> - * registers.  This address is needed early so the OCP registers that
>>>> - * are part of the device's address space can be ioremapped properly.
>>>> + * Cache the interconnect target port and the virtual address used by
>>>> + * the MPU to access this IP block's registers.  The address is needed
>>>> + * early so the OCP registers that are part of the device's address
>>>> + * space can be ioremapped properly.  The presence or absence of the
>>>> + * interconnect target port also indicates whether the hwmod code
>>>> + * should wait for the IP block to indicate readiness after it is
>>>> + * enabled.
>>>>   *
>>>>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>>>>   * -ENXIO on absent or invalid register target address space.
>>>> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>>>>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>>>>  		return -ENXIO;
>>>>  
>>>> +	/*
>>>> +	 * If there's no need for the hwmod code to read or write to
>>>> +	 * the IP block registers, bail out early before the ioremap()
>>>> +	 */
>>>> +	if (!oh->class->sysc)
>>>> +		return 0;
>>>> +

Also, I am getting some additional warnings [1] w.r.t MPU hwmod when I
changed the pr_debug on the check on _init_mpu_rt_base returned value to
a WARN and return like before. These warnings seem to be because -ENXIO
is returned as MPU hwmod will be setup with _HWMOD_NO_MPU_PORT. By
moving the !oh->class->sysc check prior to the _int_flags check and
after the _save_mpu_port_index(oh) call, I get the log similar to 3.19-rc3.

Below test log is on BeagleBone black, and I have removed mailbox reg
from am33xx.dtsi as well for testing the patch.

regards
Suman

[1] http://slexy.org/view/s2bWsGSV1Y

>>>>  	mem = _find_mpu_rt_addr_space(oh);
>>>>  	if (!mem) {
>>>>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
>>>> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>>>>  				oh->name, np->name);
>>>>  	}
>>>>  
>>>> -	if (oh->class->sysc) {
>>>> -		r = _init_mpu_rt_base(oh, NULL, index, np);
>>>> -		if (r < 0) {
>>>> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
>>>> -			     oh->name);
>>>> -			return 0;
>>>> -		}
>>>> -	}
>>>> +	r = _init_mpu_rt_base(oh, NULL, index, np);
>>>> +	if (r < 0)
>>>> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
>>>> +			 oh->name);
>>
>> You have removed the return from the above block on failure. If any DT
>> entry doesn't have the reg property, this will hang the kernel boot.
>> Just remove the "reg" entry from any of the existing DT, and you will
>> run into the issue, this is what 6423d6df1440 ("ARM: OMAP2+: hwmod:
>> check for module address space during init") fixed. Also, are you sure
>> you want to turn the WARN into a pr_debug, it won't even show during the
>> kernel boot log if the reg base is missing.
>>
>> regards
>> Suman
>>
>>>>  
>>>>  	r = _init_clocks(oh, NULL);
>>>>  	if (r < 0) {
>>>>
>>>
>>
> 

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Roger Quadros]

On 06/01/15 04:04, Paul Walmsley wrote:
> 
> Roger, Lokesh, could you try this one instead?
> 
> It passes all the basic tests here except it does not boot on the 4460 
> VAR-SOM-OM - unclear why at this point - but it would be good to see if it 
> works on your AM4372 boards, since I don't have that one.
> 
> Test logs are here:
> 
> http://www.pwsan.com/omap/testlogs/hwmod_skip_only_remap_v3.19-rc/20150105171744/
> 
> 
> - Paul
> 
> 
> From 4f2e13bd2181e0ebede3aabc484aa2339830748a Mon Sep 17 00:00:00 2001
> From: Paul Walmsley <paul@pwsan.com>
> Date: Mon, 5 Jan 2015 15:49:57 -0700
> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>  registers. Experimental.
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index cbb908dc5cf0..03df8833d399 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>  
>  	if (oh->class->reset) {
> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
> +		     oh->name);

Not part of $subject.

>  		r = oh->class->reset(oh);
>  	} else {
>  		if (oh->rst_lines_cnt > 0) {
> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>  }
>  
>  /**
> - * _init_mpu_rt_base - populate the virtual address for a hwmod
> + * _init_mpu_rt_base - populate the MPU port and virtual address
>   * @oh: struct omap_hwmod * to locate the virtual address
>   * @data: (unused, caller should pass NULL)
>   * @index: index of the reg entry iospace in device tree
>   * @np: struct device_node * of the IP block's device node in the DT data
>   *
> - * Cache the virtual address used by the MPU to access this IP block's
> - * registers.  This address is needed early so the OCP registers that
> - * are part of the device's address space can be ioremapped properly.
> + * Cache the interconnect target port and the virtual address used by
> + * the MPU to access this IP block's registers.  The address is needed
> + * early so the OCP registers that are part of the device's address
> + * space can be ioremapped properly.  The presence or absence of the
> + * interconnect target port also indicates whether the hwmod code
> + * should wait for the IP block to indicate readiness after it is
> + * enabled.
>   *
>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>   * -ENXIO on absent or invalid register target address space.
> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>  		return -ENXIO;
>  
> +	/*
> +	 * If there's no need for the hwmod code to read or write to
> +	 * the IP block registers, bail out early before the ioremap()
> +	 */
> +	if (!oh->class->sysc)
> +		return 0;
> +
>  	mem = _find_mpu_rt_addr_space(oh);
>  	if (!mem) {
>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>  				oh->name, np->name);
>  	}
>  
> -	if (oh->class->sysc) {
> -		r = _init_mpu_rt_base(oh, NULL, index, np);
> -		if (r < 0) {
> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> -			     oh->name);
> -			return 0;
> -		}
> -	}
> +	r = _init_mpu_rt_base(oh, NULL, index, np);
> +	if (r < 0)
> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
> +			 oh->name);

This is the real piece that fixes the issue.

>  
>  	r = _init_clocks(oh, NULL);
>  	if (r < 0) {
> 

I've tested this patch on am43x-gp-evm, and it seems to fix the issue although with some unpleasant warning messages.
So if we can get rid of the WARN() you can put my

Acked-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

Hi Suman,

thanks for pitching in on this!

On Tue, 6 Jan 2015, Suman Anna wrote:

> You have removed the return from the above block on failure. If any DT
> entry doesn't have the reg property, this will hang the kernel boot.
> Just remove the "reg" entry from any of the existing DT, and you will
> run into the issue, this is what 6423d6df1440 ("ARM: OMAP2+: hwmod:
> check for module address space during init") fixed. 

Seems like that's the problem that we need to track down, then.  If a 
hwmod has no MPU-accessible registers, it should still be possible to 
init the clocks for the device, etc. ...

> Also, are you sure you want to turn the WARN into a pr_debug, it won't 
> even show during the kernel boot log if the reg base is missing.

No, I'm not sure :-)  I guess it depends how many hwmods we'll have with 
no MPU-accessible registers.  We don't seem to have address ranges for the 
interconnects defined; we could fix that fairly easily. 


- Paul

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

Hi Suman

On Tue, 6 Jan 2015, Suman Anna wrote:

> Looking at your rc3 log @
> http://www.pwsan.com/omap/testlogs/test_v3.19-rc3/20150105224749/boot/4460varsomom/,
> I do see a missing reg entry for mailbox, and that's the reason for your
> hang because of the missing bail out in your new patch.
> 
> [    0.261932] ------------[ cut here ]------------
> [    0.261962] WARNING: CPU: 0 PID: 1 at
> arch/arm/mach-omap2/omap_hwmod.c:2458 _init+0x3a0/0x3f0()
> [    0.261962] omap_hwmod: mailbox: doesn't have mpu register target base
> ...
> ...
> [    0.262329] ---[ end trace a1be72591db4662e ]---
> 
> Now that said, this is weird, since the reg property for mailbox is
> defined in the base omap4.dtsi and should be inherited by the 4460
> VAR-SOM-OM, unless you are booting with an old dtb.

Yeah, it doesn't make sense.  I'm pretty sure I'm using the right DTB 
here, since that board is booting with a concatenated uImage + DTB.  
Well, at least there's a good test platform here.


- Paul

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Paul Walmsley]

On Wed, 7 Jan 2015, Roger Quadros wrote:

> > From: Paul Walmsley <paul@pwsan.com>
> > Date: Mon, 5 Jan 2015 15:49:57 -0700
> > Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
> >  registers. Experimental.
> > 
> > ---
> >  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
> >  1 file changed, 21 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> > index cbb908dc5cf0..03df8833d399 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
> >  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
> >  
> >  	if (oh->class->reset) {
> > +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
> > +		     oh->name);
> 
> Not part of $subject.

Hmm, how do you mean?  If we skip the ioremap(), wouldn't you like to know 
before some custom reset code gets called that pretty much always depends 
on the ioremap() succeeding? :-)

> >  		r = oh->class->reset(oh);
> >  	} else {
> >  		if (oh->rst_lines_cnt > 0) {
> > @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
> >  }
> >  
> >  /**
> > - * _init_mpu_rt_base - populate the virtual address for a hwmod
> > + * _init_mpu_rt_base - populate the MPU port and virtual address
> >   * @oh: struct omap_hwmod * to locate the virtual address
> >   * @data: (unused, caller should pass NULL)
> >   * @index: index of the reg entry iospace in device tree
> >   * @np: struct device_node * of the IP block's device node in the DT data
> >   *
> > - * Cache the virtual address used by the MPU to access this IP block's
> > - * registers.  This address is needed early so the OCP registers that
> > - * are part of the device's address space can be ioremapped properly.
> > + * Cache the interconnect target port and the virtual address used by
> > + * the MPU to access this IP block's registers.  The address is needed
> > + * early so the OCP registers that are part of the device's address
> > + * space can be ioremapped properly.  The presence or absence of the
> > + * interconnect target port also indicates whether the hwmod code
> > + * should wait for the IP block to indicate readiness after it is
> > + * enabled.
> >   *
> >   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
> >   * -ENXIO on absent or invalid register target address space.
> > @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> >  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
> >  		return -ENXIO;
> >  
> > +	/*
> > +	 * If there's no need for the hwmod code to read or write to
> > +	 * the IP block registers, bail out early before the ioremap()
> > +	 */
> > +	if (!oh->class->sysc)
> > +		return 0;
> > +
> >  	mem = _find_mpu_rt_addr_space(oh);
> >  	if (!mem) {
> >  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
> > @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
> >  				oh->name, np->name);
> >  	}
> >  
> > -	if (oh->class->sysc) {
> > -		r = _init_mpu_rt_base(oh, NULL, index, np);
> > -		if (r < 0) {
> > -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> > -			     oh->name);
> > -			return 0;
> > -		}
> > -	}
> > +	r = _init_mpu_rt_base(oh, NULL, index, np);
> > +	if (r < 0)
> > +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
> > +			 oh->name);
> 
> This is the real piece that fixes the issue.
> 
> >  
> >  	r = _init_clocks(oh, NULL);
> >  	if (r < 0) {
> > 
> 
> I've tested this patch on am43x-gp-evm, and it seems to fix the issue 
> although with some unpleasant warning messages.

Could you paste those in?


- Paul

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Suman Anna]

Hi Paul,

On 01/13/2015 05:29 PM, Paul Walmsley wrote:
> Hi Suman,
> 
> thanks for pitching in on this!
> 
> On Tue, 6 Jan 2015, Suman Anna wrote:
> 
>> You have removed the return from the above block on failure. If any DT
>> entry doesn't have the reg property, this will hang the kernel boot.
>> Just remove the "reg" entry from any of the existing DT, and you will
>> run into the issue, this is what 6423d6df1440 ("ARM: OMAP2+: hwmod:
>> check for module address space during init") fixed. 
> 
> Seems like that's the problem that we need to track down, then.  If a 
> hwmod has no MPU-accessible registers, it should still be possible to 
> init the clocks for the device, etc. ...

Yes true, and I should have rephrased above statement a little better -
its for modules with sysc but with no reg property to supply the base
for the module's SYSCONFIG or SYSSTATUS registers. The commit
6423d6df1440 has the explanation for the hang.

> 
>> Also, are you sure you want to turn the WARN into a pr_debug, it won't 
>> even show during the kernel boot log if the reg base is missing.
> 
> No, I'm not sure :-)  I guess it depends how many hwmods we'll have with 
> no MPU-accessible registers.  We don't seem to have address ranges for the 
> interconnects defined; we could fix that fairly easily.

The WARN_ON previously was to throw a eye-catchy print for the case
where hwmods have sysc but no address space defined (is an error
usually, but this is what we run into during the DT conversion of a
device as the hwmod and DTS changes come in through separate topic
branches).  I still think that the sysc check should be before the check
for _HWMOD_NO_MPU_PORT, a module with sysc mandates it has an MPU port.

regards
Suman

[PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc

[Roger Quadros]

On 14/01/15 01:46, Paul Walmsley wrote:
> On Wed, 7 Jan 2015, Roger Quadros wrote:
> 
>>> From: Paul Walmsley <paul@pwsan.com>
>>> Date: Mon, 5 Jan 2015 15:49:57 -0700
>>> Subject: [PATCH] Only skip ioremap() if IP block does not have OCP header
>>>  registers. Experimental.
>>>
>>> ---
>>>  arch/arm/mach-omap2/omap_hwmod.c | 33 +++++++++++++++++++++------------
>>>  1 file changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index cbb908dc5cf0..03df8833d399 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -1938,6 +1938,8 @@ static int _reset(struct omap_hwmod *oh)
>>>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>>>  
>>>  	if (oh->class->reset) {
>>> +		WARN(!oh->_mpu_rt_va, "Attempt to call custom reset with no MPU register target ioremapped: %s",
>>> +		     oh->name);
>>
>> Not part of $subject.
> 
> Hmm, how do you mean?  If we skip the ioremap(), wouldn't you like to know 
> before some custom reset code gets called that pretty much always depends 
> on the ioremap() succeeding? :-)

Ah yes. you are right.

> 
>>>  		r = oh->class->reset(oh);
>>>  	} else {
>>>  		if (oh->rst_lines_cnt > 0) {
>>> @@ -2358,15 +2360,19 @@ static int of_dev_hwmod_lookup(struct device_node *np,
>>>  }
>>>  
>>>  /**
>>> - * _init_mpu_rt_base - populate the virtual address for a hwmod
>>> + * _init_mpu_rt_base - populate the MPU port and virtual address
>>>   * @oh: struct omap_hwmod * to locate the virtual address
>>>   * @data: (unused, caller should pass NULL)
>>>   * @index: index of the reg entry iospace in device tree
>>>   * @np: struct device_node * of the IP block's device node in the DT data
>>>   *
>>> - * Cache the virtual address used by the MPU to access this IP block's
>>> - * registers.  This address is needed early so the OCP registers that
>>> - * are part of the device's address space can be ioremapped properly.
>>> + * Cache the interconnect target port and the virtual address used by
>>> + * the MPU to access this IP block's registers.  The address is needed
>>> + * early so the OCP registers that are part of the device's address
>>> + * space can be ioremapped properly.  The presence or absence of the
>>> + * interconnect target port also indicates whether the hwmod code
>>> + * should wait for the IP block to indicate readiness after it is
>>> + * enabled.
>>>   *
>>>   * Returns 0 on success, -EINVAL if an invalid hwmod is passed, and
>>>   * -ENXIO on absent or invalid register target address space.
>>> @@ -2385,6 +2391,13 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
>>>  	if (oh->_int_flags & _HWMOD_NO_MPU_PORT)
>>>  		return -ENXIO;
>>>  
>>> +	/*
>>> +	 * If there's no need for the hwmod code to read or write to
>>> +	 * the IP block registers, bail out early before the ioremap()
>>> +	 */
>>> +	if (!oh->class->sysc)
>>> +		return 0;
>>> +
>>>  	mem = _find_mpu_rt_addr_space(oh);
>>>  	if (!mem) {
>>>  		pr_debug("omap_hwmod: %s: no MPU register target found\n",
>>> @@ -2451,14 +2464,10 @@ static int __init _init(struct omap_hwmod *oh, void *data)
>>>  				oh->name, np->name);
>>>  	}
>>>  
>>> -	if (oh->class->sysc) {
>>> -		r = _init_mpu_rt_base(oh, NULL, index, np);
>>> -		if (r < 0) {
>>> -			WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
>>> -			     oh->name);
>>> -			return 0;
>>> -		}
>>> -	}
>>> +	r = _init_mpu_rt_base(oh, NULL, index, np);
>>> +	if (r < 0)
>>> +		pr_debug("omap_hwmod: %s: doesn't have mpu register target base\n",
>>> +			 oh->name);
>>
>> This is the real piece that fixes the issue.
>>
>>>  
>>>  	r = _init_clocks(oh, NULL);
>>>  	if (r < 0) {
>>>
>>
>> I've tested this patch on am43x-gp-evm, and it seems to fix the issue 
>> although with some unpleasant warning messages.
> 
> Could you paste those in?
> 
I can't reproduce what I saw earlier. Sorry for the noise.

cheers,
-roger