Re: [PATCH 3/3] omap3: pm: removes hardcoded VDD1/2 OPP values and make threshold generic

["Menon, Nishanth" <nm@ti.com>]

G.N, Vijayakumar said the following on 11/19/2009 05:26 AM:
> >From 202416dbdf3fce31bfd76f1e74dda89b382a78c3 Mon Sep 17 00:00:00 2001
> From: Vijay Kumar <vijaykumar.gn@ti.com>
> Date: Thu, 19 Nov 2009 15:07:38 +0530
> Subject: [PATCH 3/3] OMAP 3630: Update VDD2 values
>  The OMAP3630 VDD2 minimum value is 1 and Maximum value is 2. The VDD2
>  API's are updated to return proper VDD2 values.
>
>
> Signed-off-by: Charulatha V <charu@ti.com>
> Signed-off-by: Manjunath GK <manjugk@ti.com>
> Signed-off-by: Vijay Kumar <vijaykumar.gn@ti.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 1ed7f53..c240804 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -1080,7 +1080,7 @@ EXPORT_SYMBOL(omap3_get_min_vdd1_opp);
>  int omap3_get_max_vdd2_opp(void)
>  {
>  	if (cpu_is_omap3630())
> -		return VDD2_OPP3;
> +		return VDD2_OPP2;
>  	else /* Place holder for other 34xx (3430/3440) */
>  		return VDD2_OPP3;
>  
> @@ -1090,9 +1090,9 @@ EXPORT_SYMBOL(omap3_get_max_vdd2_opp);
>  int omap3_get_min_vdd2_opp(void)
>  {
>  	if (cpu_is_omap3630())
> -		return VDD2_OPP2;
> -	else /* Place holder for other 34xx (3430/3440) */
>  		return VDD2_OPP1;
> +	else /* Place holder for other 34xx (3430/3440) */
>   
this is redundant.
> +		return VDD2_OPP2;
>  
>  }
>  EXPORT_SYMBOL(omap3_get_min_vdd2_opp);
>   
I NAK this patch on the principle that these are redundant unashamedly
copies of each function without handling all scenarios:
e.g.
a) what if board x wants to disable vdd2 opp1? how can he get mainline
support with hacking on this?
b) if you have 3630-1000 device, how do you handle it cleanly?
c) you believe in OPP ID here, I DO NOT - I believe we should do things
based off frequencies.

Regards,
Nishanth Menon