RE: [PATCH] ARM: OMAP: hwmod: Fix error handling in functions used OMAP4 onwards

2012-03-30 Thread Hiremath, Vaibhav
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

2012-03-29 Thread Rajendra Nayak

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

2012-03-29 Thread Hiremath, Vaibhav
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

2012-03-29 Thread Rajendra Nayak

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

2012-03-29 Thread Hiremath, Vaibhav
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

2012-03-29 Thread Jon Hunter

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

2012-03-29 Thread Jon Hunter

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

2012-03-28 Thread Hiremath, Vaibhav
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

2012-03-28 Thread Jon Hunter

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

2012-03-27 Thread Rajendra Nayak
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

2012-03-27 Thread Jon Hunter

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

2012-03-27 Thread Paul Walmsley
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