RE: [PATCH] ARM: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Thu, Mar 29, 2012 at 20:33:17, Hunter, Jon wrote: Hi Viabhav, On 3/29/2012 3:56, Hiremath, Vaibhav wrote: On Thu, Mar 29, 2012 at 11:42:34, Nayak, Rajendra wrote: On Wednesday 28 March 2012 12:02 PM, Hiremath, Vaibhav wrote: On Tue, Mar 27, 2012 at 15:28:31, Nayak, Rajendra wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Let me add some flavor here :) AM33xx, which has module level control, but falls under OMAP3 family of devices. cpu_is_omap34xx() is true for AM33xx device and we have to add check in all these functions. And I am sure we will have many of such devices in the future. Can we use some flag based option here, instead of cpu_is_xxx() check? The intent of this patch was to make the error handling uniform across all modules control functions in hwmod, and it atleast addresses one problem of having to update these checks every time a new OMAP gets added. The problem that you bring up with AM33xx is regardless of this patch (you would still need to go update every !cpu_is_omap44xx() check) Indeed, in any of cpu_is_xxx() check implementation, I have to add check for cpu_is_am33xx(). I hope we can avoid adding more cpu_is_am. That should be a last resort. Yes, indeed; adding cpu_is_xxx check would be my last option. Thanks, Vaibhav Jon -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Wednesday 28 March 2012 12:02 PM, Hiremath, Vaibhav wrote: On Tue, Mar 27, 2012 at 15:28:31, Nayak, Rajendra wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Let me add some flavor here :) AM33xx, which has module level control, but falls under OMAP3 family of devices. cpu_is_omap34xx() is true for AM33xx device and we have to add check in all these functions. And I am sure we will have many of such devices in the future. Can we use some flag based option here, instead of cpu_is_xxx() check? The intent of this patch was to make the error handling uniform across all modules control functions in hwmod, and it atleast addresses one problem of having to update these checks every time a new OMAP gets added. The problem that you bring up with AM33xx is regardless of this patch (you would still need to go update every !cpu_is_omap44xx() check) and IMHO should be handled separately, with a flag like you are suggesting or by some other means. If you already have patches on how this can be done, you should go ahead and post them out. -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Thu, Mar 29, 2012 at 11:42:34, Nayak, Rajendra wrote: On Wednesday 28 March 2012 12:02 PM, Hiremath, Vaibhav wrote: On Tue, Mar 27, 2012 at 15:28:31, Nayak, Rajendra wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Let me add some flavor here :) AM33xx, which has module level control, but falls under OMAP3 family of devices. cpu_is_omap34xx() is true for AM33xx device and we have to add check in all these functions. And I am sure we will have many of such devices in the future. Can we use some flag based option here, instead of cpu_is_xxx() check? The intent of this patch was to make the error handling uniform across all modules control functions in hwmod, and it atleast addresses one problem of having to update these checks every time a new OMAP gets added. The problem that you bring up with AM33xx is regardless of this patch (you would still need to go update every !cpu_is_omap44xx() check) Indeed, in any of cpu_is_xxx() check implementation, I have to add check for cpu_is_am33xx(). The point I was trying to make here was, cpu_is_xxx() check will become ugly, as more and more devices gets added to the list, am33xx being the first one in omap34xx family. and IMHO should be handled separately, with a flag like you are suggesting or by some other means. If you already have patches on how this can be done, you should go ahead and post them out. Nope. But I can work on it, once I finish my AM33xx baseport submission. Thanks, Vaibhav -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Thursday 29 March 2012 02:26 PM, Hiremath, Vaibhav wrote: The point I was trying to make here was, cpu_is_xxx() check will become ugly, as more and more devices gets added to the list, am33xx being the first one in omap34xx family. So are there more, other than AM33xx which are cortex A8 based but share the OMAP4 PRCM IP block? -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Thu, Mar 29, 2012 at 14:32:34, Nayak, Rajendra wrote: On Thursday 29 March 2012 02:26 PM, Hiremath, Vaibhav wrote: The point I was trying to make here was, cpu_is_xxx() check will become ugly, as more and more devices gets added to the list, am33xx being the first one in omap34xx family. So are there more, other than AM33xx which are cortex A8 based but share the OMAP4 PRCM IP block? I would say, probably yes. But, things are so dynamic and you never know how things will shape up by the time you freeze the design. Thanks, Vaibhav -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Hi Viabhav, On 3/29/2012 4:14, Hiremath, Vaibhav wrote: On Thu, Mar 29, 2012 at 14:32:34, Nayak, Rajendra wrote: On Thursday 29 March 2012 02:26 PM, Hiremath, Vaibhav wrote: The point I was trying to make here was, cpu_is_xxx() check will become ugly, as more and more devices gets added to the list, am33xx being the first one in omap34xx family. So are there more, other than AM33xx which are cortex A8 based but share the OMAP4 PRCM IP block? I would say, probably yes. But, things are so dynamic and you never know how things will shape up by the time you freeze the design. So this begs the question, why does AM33xx return true from cpu_is_omap34xx() is the architecture is based upon OMAP4 and not OMAP3? I understand it is a Cortex-A8 versus Corex-A9, but if the architecture is closer to OMAP4, then should it not be classed as OMAP4 and not OMAP3? Raises another question if we should really have arch_is_omap and not cpu_is_omap. Cheers Jon -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Hi Viabhav, On 3/29/2012 3:56, Hiremath, Vaibhav wrote: On Thu, Mar 29, 2012 at 11:42:34, Nayak, Rajendra wrote: On Wednesday 28 March 2012 12:02 PM, Hiremath, Vaibhav wrote: On Tue, Mar 27, 2012 at 15:28:31, Nayak, Rajendra wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Let me add some flavor here :) AM33xx, which has module level control, but falls under OMAP3 family of devices. cpu_is_omap34xx() is true for AM33xx device and we have to add check in all these functions. And I am sure we will have many of such devices in the future. Can we use some flag based option here, instead of cpu_is_xxx() check? The intent of this patch was to make the error handling uniform across all modules control functions in hwmod, and it atleast addresses one problem of having to update these checks every time a new OMAP gets added. The problem that you bring up with AM33xx is regardless of this patch (you would still need to go update every !cpu_is_omap44xx() check) Indeed, in any of cpu_is_xxx() check implementation, I have to add check for cpu_is_am33xx(). I hope we can avoid adding more cpu_is_am. That should be a last resort. Jon -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
On Tue, Mar 27, 2012 at 15:28:31, Nayak, Rajendra wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Let me add some flavor here :) AM33xx, which has module level control, but falls under OMAP3 family of devices. cpu_is_omap34xx() is true for AM33xx device and we have to add check in all these functions. And I am sure we will have many of such devices in the future. Can we use some flag based option here, instead of cpu_is_xxx() check? Thanks, Vaibhav Signed-off-by: Rajendra Nayak rna...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 8ac26f2..f2a9afa 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -808,7 +808,7 @@ static void _enable_module(struct omap_hwmod *oh) */ static int _omap4_wait_target_disable(struct omap_hwmod *oh) { - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return 0; if (!oh) @@ -838,7 +838,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh) int v; /* The module mode does not exist prior OMAP4 */ - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return -EINVAL; if (!oh-clkdm || !oh-prcm.omap4.modulemode) -- 1.7.1 -- 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 -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Hi Paul, On 3/27/2012 21:39, Paul Walmsley wrote: Hi Jon, On Tue, 27 Mar 2012, Jon Hunter wrote: On 3/27/2012 4:58, Rajendra Nayak wrote: diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 8ac26f2..f2a9afa 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -808,7 +808,7 @@ static void _enable_module(struct omap_hwmod *oh) */ static int _omap4_wait_target_disable(struct omap_hwmod *oh) { - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return 0; What about omap36xx? Unfortunately, cpu_is_omap34xx() also covers OMAP36xx :-( Thanks. Is that still the case when MULTI_OMAP2 is defined? I can see if it is not define then it will always return 1 for all OMAP3, but for MULTI_OMAP2 it did not seem to me that it would. May be I should test ... Cheers Jon -- 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
[PATCH] ARM: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Signed-off-by: Rajendra Nayak rna...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 8ac26f2..f2a9afa 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -808,7 +808,7 @@ static void _enable_module(struct omap_hwmod *oh) */ static int _omap4_wait_target_disable(struct omap_hwmod *oh) { - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return 0; if (!oh) @@ -838,7 +838,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh) int v; /* The module mode does not exist prior OMAP4 */ - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return -EINVAL; if (!oh-clkdm || !oh-prcm.omap4.modulemode) -- 1.7.1 -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Hi Rajendra, On 3/27/2012 4:58, Rajendra Nayak wrote: Some functions like _omap4_disable_module() and _omap4_wait_target_disable() are (will be) used on all OMAPs OMAP4 and beyond which support module level control. Fix the error checks in these functions to return if called on any platform pre OMAP4 (i.e OMAP2 and OMAP3) instead of checking for !cpu_is_omap44xx(). This avoids having to update the error check with a ' !cpu_is_omap54xx()' when OMAP5 is introduced and possibly similar updates when further OMAP generations are added. Signed-off-by: Rajendra Nayakrna...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 8ac26f2..f2a9afa 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -808,7 +808,7 @@ static void _enable_module(struct omap_hwmod *oh) */ static int _omap4_wait_target_disable(struct omap_hwmod *oh) { - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return 0; What about omap36xx? if (!oh) @@ -838,7 +838,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh) int v; /* The module mode does not exist prior OMAP4 */ - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return -EINVAL; Same here. if (!oh-clkdm || !oh-prcm.omap4.modulemode) Cheers Jon -- 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: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards
Hi Jon, On Tue, 27 Mar 2012, Jon Hunter wrote: On 3/27/2012 4:58, Rajendra Nayak wrote: diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 8ac26f2..f2a9afa 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -808,7 +808,7 @@ static void _enable_module(struct omap_hwmod *oh) */ static int _omap4_wait_target_disable(struct omap_hwmod *oh) { - if (!cpu_is_omap44xx()) + if (cpu_is_omap24xx() || cpu_is_omap34xx()) return 0; What about omap36xx? Unfortunately, cpu_is_omap34xx() also covers OMAP36xx :-( - 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