diff mbox

[RFC/PATCH,2/7] OMAP3: beagle: don't touch omap_device internals

Message ID 1311292338-11830-4-git-send-email-khilman@ti.com (mailing list archive)
State Superseded
Headers show

Commit Message

Kevin Hilman July 21, 2011, 11:52 p.m. UTC
Board code should not touch omap_device internals.  To get the MPU/IVA devices,
use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().

Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

Comments

Felipe Balbi July 22, 2011, 8:57 a.m. UTC | #1
Hi,

On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
> Board code should not touch omap_device internals.  To get the MPU/IVA devices,
> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
> 
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
>  1 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> index 32f5f89..3ae16b4 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>  
>  	/* Custom OPP enabled for all xM versions */
>  	if (cpu_is_omap3630()) {
> -		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
> -		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
> -		struct device *dev;
> +		struct device *mpu_dev, *iva_dev;
>  
> -		if (!mh || !dh) {
> +		mpu_dev = omap2_get_mpuss_device();
> +		iva_dev = omap2_get_iva_device();

out of curiosity again, nothing to do with this patch.

Maybe it would be nicer to have an api such as:

omap2_get_device(name);

there are already four devices to be gotten, if that number grows any
bigger, so will the number of helper functions.
Jean Pihet July 28, 2011, 8:36 a.m. UTC | #2
Hi,

On Fri, Jul 22, 2011 at 10:57 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
>> Board code should not touch omap_device internals.  To get the MPU/IVA devices,
>> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
>>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>>  arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
>>  1 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> index 32f5f89..3ae16b4 100644
>> --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>>
>>       /* Custom OPP enabled for all xM versions */
>>       if (cpu_is_omap3630()) {
>> -             struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
>> -             struct omap_hwmod *dh = omap_hwmod_lookup("iva");
>> -             struct device *dev;
>> +             struct device *mpu_dev, *iva_dev;
>>
>> -             if (!mh || !dh) {
>> +             mpu_dev = omap2_get_mpuss_device();
>> +             iva_dev = omap2_get_iva_device();
>
> out of curiosity again, nothing to do with this patch.
>
> Maybe it would be nicer to have an api such as:
>
> omap2_get_device(name);
>
> there are already four devices to be gotten, if that number grows any
> bigger, so will the number of helper functions.

I agree with this.
The API is also helpful for the PM QoS devices constraints API, which
requires the pointers to devices structs. At the present time only a
few devices can be used and it would be good to have a generic
implementation.

Regards,
Jean

>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean Pihet July 28, 2011, 8:40 a.m. UTC | #3
Hi,

On Fri, Jul 22, 2011 at 10:57 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jul 21, 2011 at 04:52:13PM -0700, Kevin Hilman wrote:
>> Board code should not touch omap_device internals.  To get the MPU/IVA devices,
>> use existing APIs: omap2_get_mpu_device(), omap2_get_iva_device().
>>
>> Signed-off-by: Kevin Hilman <khilman@ti.com>
>> ---
>>  arch/arm/mach-omap2/board-omap3beagle.c |   23 ++++++++++-------------
>>  1 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> index 32f5f89..3ae16b4 100644
>> --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>> @@ -491,23 +491,22 @@ static void __init beagle_opp_init(void)
>>
>>       /* Custom OPP enabled for all xM versions */
>>       if (cpu_is_omap3630()) {
>> -             struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
>> -             struct omap_hwmod *dh = omap_hwmod_lookup("iva");
>> -             struct device *dev;
>> +             struct device *mpu_dev, *iva_dev;
>>
>> -             if (!mh || !dh) {
>> +             mpu_dev = omap2_get_mpuss_device();
>> +             iva_dev = omap2_get_iva_device();
>
> out of curiosity again, nothing to do with this patch.
>
> Maybe it would be nicer to have an api such as:
>
> omap2_get_device(name);
>
> there are already four devices to be gotten, if that number grows any
> bigger, so will the number of helper functions.

I agree with this.
The API is also helpful for the PM QoS devices constraints API, which
requires the pointers to devices structs. At the present time only a
few devices can be used and it would be good to have a generic
implementation.

Regards,
Jean

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

Patch

diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 32f5f89..3ae16b4 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -491,23 +491,22 @@  static void __init beagle_opp_init(void)
 
 	/* Custom OPP enabled for all xM versions */
 	if (cpu_is_omap3630()) {
-		struct omap_hwmod *mh = omap_hwmod_lookup("mpu");
-		struct omap_hwmod *dh = omap_hwmod_lookup("iva");
-		struct device *dev;
+		struct device *mpu_dev, *iva_dev;
 
-		if (!mh || !dh) {
+		mpu_dev = omap2_get_mpuss_device();
+		iva_dev = omap2_get_iva_device();
+
+		if (!mpu_dev || !iva_dev) {
 			pr_err("%s: Aiee.. no mpu/dsp devices? %p %p\n",
-				__func__, mh, dh);
+				__func__, mpu_dev, iva_dev);
 			return;
 		}
 		/* Enable MPU 1GHz and lower opps */
-		dev = &mh->od->pdev.dev;
-		r = opp_enable(dev, 800000000);
+		r = opp_enable(mpu_dev, 800000000);
 		/* TODO: MPU 1GHz needs SR and ABB */
 
 		/* Enable IVA 800MHz and lower opps */
-		dev = &dh->od->pdev.dev;
-		r |= opp_enable(dev, 660000000);
+		r |= opp_enable(iva_dev, 660000000);
 		/* TODO: DSP 800MHz needs SR and ABB */
 		if (r) {
 			pr_err("%s: failed to enable higher opp %d\n",
@@ -516,10 +515,8 @@  static void __init beagle_opp_init(void)
 			 * Cleanup - disable the higher freqs - we dont care
 			 * about the results
 			 */
-			dev = &mh->od->pdev.dev;
-			opp_disable(dev, 800000000);
-			dev = &dh->od->pdev.dev;
-			opp_disable(dev, 660000000);
+			opp_disable(mpu_dev, 800000000);
+			opp_disable(iva_dev, 660000000);
 		}
 	}
 	return;