Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
Paul, On 16/07/15 04:47, Paul Walmsley wrote: Hi Roger On Mon, 13 Jul 2015, 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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. e.g. this fixes the below DCAN bus access error on AM437x-gp-evm. Could you update this patch to align with the discussion we had the last time this was posted in December? e.g., http://www.spinics.net/lists/arm-kernel/msg388002.html You can ignore the problems with the VAR-SOM-OM that were discussed; those were indeed due to an old DT file in use, as Suman suspected. Let me know if you have any questions about it - My bad. I forgot that I had posted this earlier and totally missed that thread. :P cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
Hi Roger On Mon, 13 Jul 2015, 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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. e.g. this fixes the below DCAN bus access error on AM437x-gp-evm. Could you update this patch to align with the discussion we had the last time this was posted in December? e.g., http://www.spinics.net/lists/arm-kernel/msg388002.html You can ignore the problems with the VAR-SOM-OM that were discussed; those were indeed due to an old DT file in use, as Suman suspected. Let me know if you have any questions about it - - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
On 14/01/15 01:46, Paul Walmsley wrote:
On Wed, 7 Jan 2015, Roger Quadros wrote:
From: Paul Walmsley p...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
On Wed, 7 Jan 2015, Roger Quadros wrote:
From: Paul Walmsley p...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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 rog...@ti.com
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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) {
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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,
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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) {
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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) {
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
+ 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 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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) {
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
* Roger Quadros rog...@ti.com [141219 01:08]: Lokesh, On 19/12/14 07:21, Lokesh Vutla wrote: Hi Roger, On Thursday 18 December 2014 09:22 PM, 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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. Yes, makes sense. This patch looks good to me. Tested this on AM437x-gp-evm. Roger, if the modules don't have sysconfig registers, care to check if we actually really need hwmod for those modules then? I know hwmod is managing runtime PM gate clocks for devices with clkctrl_offs. But if that's all we need hwmod for in the non-sysc cases, then it might make sense to manage the gate clocks in the clock framework directly instead for those devices. Of course that's more of a long term project, but at least we should be aware of the dependencies here :) May be good idea to warn every time if enabling of module is failed? Unrelated to this patch though. Yes, failing to be ready is serious enough for a warning. Care to send a separate patch for that? Yeah that sounds like a separate patch. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
+ 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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
Lokesh, On 19/12/14 07:21, Lokesh Vutla wrote: Hi Roger, On Thursday 18 December 2014 09:22 PM, 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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. Yes, makes sense. This patch looks good to me. Tested this on AM437x-gp-evm. Tested-by: Lokesh Vutla lokeshvu...@ti.com Thanks. May be good idea to warn every time if enabling of module is failed? Unrelated to this patch though. Yes, failing to be ready is serious enough for a warning. Care to send a separate patch for that? cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. e.g. this fixes the below DCAN bus access error on AM437x-gp-evm. [ 16.672978] [ cut here ] [ 16.677885] WARNING: CPU: 0 PID: 1580 at drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x234/0x35c() [ 16.687946] 4400.ocp:L3 Custom Error: MASTER M2 (64-bit) TARGET L4_PER_0 (Read): Data Access in User mode during Functional access [ 16.700654] Modules linked in: xhci_hcd btwilink ti_vpfe dwc3 videobuf2_core ov2659 bluetooth v4l2_common videodev ti_am335x_adc kfifo_buf industrialio c_can_platform videobuf2_dma_contig media snd_soc_tlv320aic3x pixcir_i2c_ts c_can dc [ 16.731144] CPU: 0 PID: 1580 Comm: rpc.statd Not tainted 3.14.26-02561-gf733aa036398 #180 [ 16.739747] Backtrace: [ 16.742336] [c0011108] (dump_backtrace) from [c00112a4] (show_stack+0x18/0x1c) [ 16.750285] r6:0093 r5:0009 r4:eab5b8a8 r3: [ 16.756252] [c001128c] (show_stack) from [c05a4418] (dump_stack+0x20/0x28) [ 16.763870] [c05a43f8] (dump_stack) from [c0037120] (warn_slowpath_common+0x6c/0x8c) [ 16.772408] [c00370b4] (warn_slowpath_common) from [c00371e4] (warn_slowpath_fmt+0x38/0x40) [ 16.781550] r8:c05d1f90 r7:c0730844 r6:c0730448 r5:80080003 r4:ed0cd210 [ 16.788626] [c00371b0] (warn_slowpath_fmt) from [c027fa94] (l3_interrupt_handler+0x234/0x35c) [ 16.797968] r3:ed0cd480 r2:c0730508 [ 16.801747] [c027f860] (l3_interrupt_handler) from [c0063758] (handle_irq_event_percpu+0x54/0x1bc) [ 16.811533] r10:ed005600 r9:c084855b r8:002a r7: r6: r5:002a [ 16.819780] r4:ed0e6d80 [ 16.822453] [c0063704] (handle_irq_event_percpu) from [c00638f0] (handle_irq_event+0x30/0x40) [ 16.831789] r10:eb2b6938 r9:eb2b6960 r8:bf011420 r7:fa240100 r6: r5:002a [ 16.840052] r4:ed005600 [ 16.842744] [c00638c0] (handle_irq_event) from [c00661d8] (handle_fasteoi_irq+0x74/0x128) [ 16.851702] r4:ed005600 r3: [ 16.855479] [c0066164] (handle_fasteoi_irq) from [c0063068] (generic_handle_irq+0x28/0x38) [ 16.864523] r4:002a r3:c0066164 [ 16.868294] [c0063040] (generic_handle_irq) from [c000ef60] (handle_IRQ+0x38/0x8c) [ 16.876612] r4:c081c640 r3:0202 [ 16.880380] [c000ef28] (handle_IRQ) from [c00084f0] (gic_handle_irq+0x30/0x5c) [ 16.888328] r6:eab5ba38 r5:c0804460 r4:fa24010c r3:0100 [ 16.894303] [c00084c0] (gic_handle_irq) from [c05a8d80] (__irq_svc+0x40/0x50) [ 16.902193] Exception stack(0xeab5ba38 to 0xeab5ba80) [ 16.907499] ba20: 0006 [ 16.916108] ba40: fa1d fa1d0008 ed3d3000 eab5bab4 ed3d3460 c0842af4 bf011420 eb2b6960 [ 16.924716] ba60: eb2b6938 eab5ba8c eab5ba90 eab5ba80 bf035220 bf07702c 600f0013 [ 16.933317] r7:eab5ba6c r6: r5:600f0013 r4:bf07702c [ 16.939317] [bf077000] (c_can_plat_read_reg_aligned_to_16bit [c_can_platform]) from [bf035220] (c_can_get_berr_counter+0x38/0x64 [c_can]) [ 16.952696] [bf0351e8] (c_can_get_berr_counter [c_can]) from [bf010294] (can_fill_info+0x124/0x15c [can_dev]) [ 16.963480] r5:ec8c9740 r4:ed3d3000 [ 16.967253] [bf010170] (can_fill_info [can_dev]) from [c0502fa8] (rtnl_fill_ifinfo+0x58c/0x8fc) [ 16.976749] r6:ec8c9740 r5:ed3d3000 r4:eb2b6780 [ 16.981613] [c0502a1c] (rtnl_fill_ifinfo) from [c0503408] (rtnl_dump_ifinfo+0xf0/0x1dc) [ 16.990401] r10:ec8c9740 r9: r8: r7: r6:ebd4d1b4 r5:ed3d3000 [ 16.998671] r4: [ 17.001342] [c0503318] (rtnl_dump_ifinfo) from [c050e6e4] (netlink_dump+0xa8/0x1e0) [ 17.009772] r10: r9: r8:c0503318 r7:ebf3e6c0 r6:ebd4d1b4 r5:ec8c9740 [ 17.018050] r4:ebd4d000 [ 17.020714] [c050e63c] (netlink_dump) from [c050ec10] (__netlink_dump_start+0x104/0x154) [ 17.029591] r6:eab5bd34 r5:ec8c9980 r4:ebd4d000 [ 17.034454] [c050eb0c] (__netlink_dump_start) from [c0505604] (rtnetlink_rcv_msg+0x110/0x1f4) [ 17.043778] r7: r6:ec8c9980 r5:0f40 r4:ebf3e6c0 [ 17.049743] [c05054f4] (rtnetlink_rcv_msg) from [c05108e8] (netlink_rcv_skb+0xb4/0xc8) [ 17.058449] r8:eab5bdac r7:ec8c9980 r6:c05054f4 r5:ec8c9980 r4:ebf3e6c0 [ 17.065534] [c0510834] (netlink_rcv_skb) from [c0504134] (rtnetlink_rcv+0x24/0x2c) [ 17.073854] r6:ebd4d000
Re: [PATCH] ARM: OMAP2+: hwmod: Fix _wait_target_ready() for hwmods without sysc
Hi Roger, On Thursday 18 December 2014 09:22 PM, 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. This can potentially cause a bus access error if the module is accessed before the module is ready. Get rid of the redundant _find_mpu_rt_port() check from the _wait_target_ready() funcion for all the SoCs. The following PRCM register access that checks the module ready state has nothing to do with module's SYSCONFIG or mpu_rt_port. Yes, makes sense. This patch looks good to me. Tested this on AM437x-gp-evm. Tested-by: Lokesh Vutla lokeshvu...@ti.com May be good idea to warn every time if enabling of module is failed? Unrelated to this patch though. diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 4c6b7b2..db67b66 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2243,7 +2243,7 @@ static int _enable(struct omap_hwmod *oh) if (soc_ops.disable_module) soc_ops.disable_module(oh); _disable_clocks(oh); - pr_debug(omap_hwmod: %s: _wait_target_ready: %d\n, + pr_warn(omap_hwmod: %s: _wait_target_ready failed : %d\n, oh-name, r); if (oh-clkdm) Thanks and regards, Lokesh e.g. this fixes the below DCAN bus access error on AM437x-gp-evm. [ 16.672978] [ cut here ] [ 16.677885] WARNING: CPU: 0 PID: 1580 at drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x234/0x35c() [ 16.687946] 4400.ocp:L3 Custom Error: MASTER M2 (64-bit) TARGET L4_PER_0 (Read): Data Access in User mode during Functional access [ 16.700654] Modules linked in: xhci_hcd btwilink ti_vpfe dwc3 videobuf2_core ov2659 bluetooth v4l2_common videodev ti_am335x_adc kfifo_buf industrialio c_can_platform videobuf2_dma_contig media snd_soc_tlv320aic3x pixcir_i2c_ts c_can dc [ 16.731144] CPU: 0 PID: 1580 Comm: rpc.statd Not tainted 3.14.26-02561-gf733aa036398 #180 [ 16.739747] Backtrace: [ 16.742336] [c0011108] (dump_backtrace) from [c00112a4] (show_stack+0x18/0x1c) [ 16.750285] r6:0093 r5:0009 r4:eab5b8a8 r3: [ 16.756252] [c001128c] (show_stack) from [c05a4418] (dump_stack+0x20/0x28) [ 16.763870] [c05a43f8] (dump_stack) from [c0037120] (warn_slowpath_common+0x6c/0x8c) [ 16.772408] [c00370b4] (warn_slowpath_common) from [c00371e4] (warn_slowpath_fmt+0x38/0x40) [ 16.781550] r8:c05d1f90 r7:c0730844 r6:c0730448 r5:80080003 r4:ed0cd210 [ 16.788626] [c00371b0] (warn_slowpath_fmt) from [c027fa94] (l3_interrupt_handler+0x234/0x35c) [ 16.797968] r3:ed0cd480 r2:c0730508 [ 16.801747] [c027f860] (l3_interrupt_handler) from [c0063758] (handle_irq_event_percpu+0x54/0x1bc) [ 16.811533] r10:ed005600 r9:c084855b r8:002a r7: r6: r5:002a [ 16.819780] r4:ed0e6d80 [ 16.822453] [c0063704] (handle_irq_event_percpu) from [c00638f0] (handle_irq_event+0x30/0x40) [ 16.831789] r10:eb2b6938 r9:eb2b6960 r8:bf011420 r7:fa240100 r6: r5:002a [ 16.840052] r4:ed005600 [ 16.842744] [c00638c0] (handle_irq_event) from [c00661d8] (handle_fasteoi_irq+0x74/0x128) [ 16.851702] r4:ed005600 r3: [ 16.855479] [c0066164] (handle_fasteoi_irq) from [c0063068] (generic_handle_irq+0x28/0x38) [ 16.864523] r4:002a r3:c0066164 [ 16.868294] [c0063040] (generic_handle_irq) from [c000ef60] (handle_IRQ+0x38/0x8c) [ 16.876612] r4:c081c640 r3:0202 [ 16.880380] [c000ef28] (handle_IRQ) from [c00084f0] (gic_handle_irq+0x30/0x5c) [ 16.888328] r6:eab5ba38 r5:c0804460 r4:fa24010c r3:0100 [ 16.894303] [c00084c0] (gic_handle_irq) from [c05a8d80] (__irq_svc+0x40/0x50) [ 16.902193] Exception stack(0xeab5ba38 to 0xeab5ba80) [ 16.907499] ba20: 0006 [ 16.916108] ba40: fa1d fa1d0008 ed3d3000 eab5bab4 ed3d3460 c0842af4 bf011420 eb2b6960 [ 16.924716] ba60: eb2b6938 eab5ba8c eab5ba90 eab5ba80 bf035220 bf07702c 600f0013 [ 16.933317] r7:eab5ba6c r6: r5:600f0013 r4:bf07702c [ 16.939317] [bf077000] (c_can_plat_read_reg_aligned_to_16bit [c_can_platform]) from [bf035220] (c_can_get_berr_counter+0x38/0x64 [c_can]) [ 16.952696] [bf0351e8] (c_can_get_berr_counter [c_can]) from [bf010294] (can_fill_info+0x124/0x15c [can_dev]) [ 16.963480] r5:ec8c9740 r4:ed3d3000 [ 16.967253] [bf010170] (can_fill_info [can_dev]) from [c0502fa8] (rtnl_fill_ifinfo+0x58c/0x8fc) [ 16.976749] r6:ec8c9740 r5:ed3d3000 r4:eb2b6780 [ 16.981613] [c0502a1c] (rtnl_fill_ifinfo) from [c0503408] (rtnl_dump_ifinfo+0xf0/0x1dc) [ 16.990401]
