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<[email protected]>

---
  arch/arm/mach-omap2/omap_hwmod.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/omap_hwmod.c +0 -0

index 8ac26f2..f2a9afa 100644

arch/arm/mach-omap2/omap_hwmod.c static void _enable_module(struct omap_hwmod *oh) · line 812

Quoted 12 lines · Rajendra Nayak

arch/arm/mach-omap2/omap_hwmod.c +1 -1

index 8ac26f2..f2a9afa 100644
@@ -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?

Hi Jon,

Quoted 16 lines · Jon Hunter

arch/arm/mach-omap2/omap_hwmod.c static void _enable_module(struct omap_hwmod *oh) · line 812

Quoted 12 lines · Rajendra Nayak

arch/arm/mach-omap2/omap_hwmod.c +1 -1

index 8ac26f2..f2a9afa 100644
@@ -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 :-(

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 <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/omap_hwmod.c +2 -2

index 8ac26f2..f2a9afa 100644
@@ -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)

Signature · 6 lines

1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 8 lines · Rajendra

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?

Quoted 10 lines · Vaibhav

Quoted 8 lines · Rajendra

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.

Quoted 9 lines

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.

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.

Quoted 3 lines · Vaibhav

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?

Quoted 5 lines · Rajendra

Quoted 3 lines · Vaibhav

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.

Quoted 3 lines

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.

Quoted 26 lines · Rajendra

Quoted 10 lines · Vaibhav

Quoted 8 lines · Rajendra

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.

Quoted 9 lines

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.

Quoted 2 lines

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().

Hi Viabhav,

Quoted 27 lines · Vaibhav

Quoted 26 lines · Rajendra

Quoted 10 lines · Vaibhav

Quoted 8 lines · Rajendra

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.

Quoted 9 lines

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.

Quoted 5 lines

Quoted 2 lines

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_amXXXX. That should be a last resort.

Jon