subject:"RE\: \[PATCH v3 1\/3\] AM35x\: voltage\: Basic initialization"

RE: [PATCH v3 1/3] AM35x: voltage: Basic initialization

2011-09-23 Thread Koyamangalath, Abhilash

 = voltagedomains_omap3[i], voltdm; i++) {
 + if (cpu_is_omap3505() || cpu_is_omap3517())
 + voltdm-scalable = false;
   voltdm-sys_clk.name = sys_clk_name;
 + }

   voltdm_init(voltagedomains_omap3);
  };




From: Hilman, Kevin
Sent: Friday, September 23, 2011 4:00 AM
To: Koyamangalath, Abhilash
Cc: linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
linux-ker...@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; 
sa...@linux.intel.com; Shilimkar, Santosh; Premi, Sanjeev; 
david.woodho...@intel.com; christian.gmei...@gmail.com
Subject: Re: [PATCH v3 1/3] AM35x: voltage: Basic initialization

Hi Abhilash,

Abhilash K V abhilash...@ti.com writes:

 This patch adds the basic initialization of voltage layer
 for AM35x. Since AM35x doesn't support voltage scaling,

I must admit to still being confused by this series.

This patch says AM35x doesn't support voltage scaling, and the next
patch adds PMIC support, and registers it with the voltage layer.

However, with each voltdm-scalable flag set to false, none of the PMIC
values will ever be used (by the current voltage layer.)  Do you have
more patches on top of this that extend the voltage layer to directly
use the PMIC instead of using VC or VP?

I'm assuming we have some more assumptions in our current voltage layer
about the presence of VC/VP that are wrong and need to be fixed.  Now
that the big voltage layer cleanup is done, I am *very* interested in
getting rid of any more assumptions we have in that code about how
devices are hooked up with PMICs.

Can you summarize how these devices are using (or want to use) the
voltage layer?

Thanks,

Kevin

 Many functions have been defined to plug into existing
 voltage layer.

 Signed-off-by: Sanjeev Premi pr...@ti.com
 Signed-off-by: Abhilash K V abhilash...@ti.com
 ---
  arch/arm/mach-omap2/omap_opp_data.h   |1 +
  arch/arm/mach-omap2/opp3xxx_data.c|9 +
  arch/arm/mach-omap2/voltagedomains3xxx_data.c |   10 --
  3 files changed, 18 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-omap2/omap_opp_data.h 
 b/arch/arm/mach-omap2/omap_opp_data.h
 index c784c12..c7cedf3 100644
 --- a/arch/arm/mach-omap2/omap_opp_data.h
 +++ b/arch/arm/mach-omap2/omap_opp_data.h
 @@ -88,6 +88,7 @@ extern struct omap_volt_data omap34xx_vddmpu_volt_data[];
  extern struct omap_volt_data omap34xx_vddcore_volt_data[];
  extern struct omap_volt_data omap36xx_vddmpu_volt_data[];
  extern struct omap_volt_data omap36xx_vddcore_volt_data[];
 +extern struct omap_volt_data am35xx_vdd_volt_data[];

  extern struct omap_volt_data omap44xx_vdd_mpu_volt_data[];
  extern struct omap_volt_data omap44xx_vdd_iva_volt_data[];
 diff --git a/arch/arm/mach-omap2/opp3xxx_data.c 
 b/arch/arm/mach-omap2/opp3xxx_data.c
 index d95f3f9..e4a5ee6 100644
 --- a/arch/arm/mach-omap2/opp3xxx_data.c
 +++ b/arch/arm/mach-omap2/opp3xxx_data.c
 @@ -85,6 +85,15 @@ struct omap_volt_data omap36xx_vddcore_volt_data[] = {
   VOLT_DATA_DEFINE(0, 0, 0, 0),
  };

 +/* AM35x
 + *
 + * Fields related to SmartReflex and Voltage Processor are set to 0.
 + */

fix multi-line comment style (search for 'multi-line' in 
Documentation/CodingStyle)

 +struct omap_volt_data am35xx_vdd_volt_data[] = {
 + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP3_UV, 0x0, 0x0, 0x0),
 + VOLT_DATA_DEFINE(0, 0, 0, 0),
 +};
 +
  /* OPP data */

  static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 diff --git a/arch/arm/mach-omap2/voltagedomains3xxx_data.c 
 b/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 index 071101d..530082f 100644
 --- a/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 +++ b/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 @@ -85,7 +85,10 @@ void __init omap3xxx_voltagedomains_init(void)
* XXX Will depend on the process, validation, and binning
* for the currently-running IC
*/
 - if (cpu_is_omap3630()) {
 + if (cpu_is_omap3505() || cpu_is_omap3517()) {
 + omap3_voltdm_mpu.volt_data = am35xx_vdd_volt_data;
 + omap3_voltdm_core.volt_data = am35xx_vdd_volt_data;
 + } else if (cpu_is_omap3630()) {
   omap3_voltdm_mpu.volt_data = omap36xx_vddmpu_volt_data;
   omap3_voltdm_core.volt_data = omap36xx_vddcore_volt_data;
   } else {
 @@ -93,8 +96,11 @@ void __init omap3xxx_voltagedomains_init(void)
   omap3_voltdm_core.volt_data = omap34xx_vddcore_volt_data;
   }

 - for (i = 0; voltdm = voltagedomains_omap3[i], voltdm; i++)
 + for (i = 0; voltdm = voltagedomains_omap3[i], voltdm; i++) {
 + if (cpu_is_omap3505() || cpu_is_omap3517())
 + voltdm-scalable = false;
   voltdm-sys_clk.name = sys_clk_name;
 + }

   voltdm_init(voltagedomains_omap3);
  };
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord

Re: [PATCH v3 1/3] AM35x: voltage: Basic initialization

2011-09-23 Thread Kevin Hilman

Hi Abhilash,

Koyamangalath, Abhilash abhilash...@ti.com writes:

 On Fri, Sep 23, 2011 at 4:00 AM, Hilman, Kevin wrote:
 Hi Abhilash, Abhilash K V abhilash...@ti.com writes:
 This patch adds the basic initialization of voltage layer for
 AM35x. Since AM35x doesn't support voltage scaling,
 I must admit to still being confused by this series.  This patch
 says AM35x doesn't support voltage scaling, and the next patch adds
 PMIC support, and registers it with the voltage layer.  However,
 with each voltdm-scalable flag set to false, none of the PMIC
 values will ever be used (by the current voltage layer.)  Do you
 have more patches on top of this that extend the voltage layer to
 directly use the PMIC instead of using VC or VP?  I'm assuming we
 have some more assumptions in our current voltage layer about the
 presence of VC/VP that are wrong and need to be fixed.  Now that the
 big voltage layer cleanup is done, I am *very* interested in getting
 rid of any more assumptions we have in that code about how devices
 are hooked up with PMICs.  Can you summarize how these devices are
 using (or want to use) the voltage layer?
 [Abhilash K V] Your concerns are grave and am trying to address most,
 however these are the only points I can make outright:

 - AM35x has just one voltage domain, so I tried having only one entry
 in voltagedomains_omap3[ ] ( and calling it mpu_core, maybe or mpu
 or core ?).  

Based the TRM, it's called core.

 Either ways, some power-domain, say mpu_pwrdm would try
 looking for the mpu_iva volt-domain and return error, this happens
 for most powerdomains as their constituent volt-domains are hard-coded
 (and so unavailable on am35xx). Changing the code (which will be
 massive) there going against our initial premise that am35xx is still
 a type of omap3 SoC.

While the AM35x is similar to the OMAP3 in many ways, in terms of power,
there are some significant differences that we need to model properly.

The problem with the current approach is it's trying to trick the core
code into thinking an AM35x is like an OMAP34xx by creating voltage
domains that don't exist in hardware.  

The point of these voltage/power/clock domain data files is to represent
*exactly* what is in hardware.

Looking closer at SPRUGR0B, I don't think you should be directly using
the 34xx powerdomains as a starting point.  There are a few reasons:

- not all 34xx powerdomains exist on AM35x (at least cam, iva2, USB host
  are missing)
- AM35x powerdomains are in different voltage domains
- AM35x powerdomains do not support retention or off (only on and inactive
  according to SPRUGR0B)

 - TPS65023 PMIC code was originally included as a starting point to
 support a omap34xx (with SR disabled maybe) with power supplied by a
 TPS65023. Yes,I agree that since this looks more of like hypothetical
 scenario right now and so we can do without the addition of file
 pmic_tps65023.c for now as it doesn't provide any support for scaling.

I see now.

Adding support for that PMIC to the kernel is fine, I just don't think
it makes sense in context of this series for this device, since it does
not support voltage scaling, and AFAICT, this PMIC is for DVS uses.

Kevin

--
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 v3 1/3] AM35x: voltage: Basic initialization

2011-09-22 Thread Kevin Hilman

Hi Abhilash,

Abhilash K V abhilash...@ti.com writes:

 This patch adds the basic initialization of voltage layer
 for AM35x. Since AM35x doesn't support voltage scaling,

I must admit to still being confused by this series.  

This patch says AM35x doesn't support voltage scaling, and the next
patch adds PMIC support, and registers it with the voltage layer.

However, with each voltdm-scalable flag set to false, none of the PMIC
values will ever be used (by the current voltage layer.)  Do you have
more patches on top of this that extend the voltage layer to directly
use the PMIC instead of using VC or VP?

I'm assuming we have some more assumptions in our current voltage layer
about the presence of VC/VP that are wrong and need to be fixed.  Now
that the big voltage layer cleanup is done, I am *very* interested in
getting rid of any more assumptions we have in that code about how
devices are hooked up with PMICs.

Can you summarize how these devices are using (or want to use) the
voltage layer?

Thanks,

Kevin

 Many functions have been defined to plug into existing
 voltage layer.

 Signed-off-by: Sanjeev Premi pr...@ti.com
 Signed-off-by: Abhilash K V abhilash...@ti.com
 ---
  arch/arm/mach-omap2/omap_opp_data.h   |1 +
  arch/arm/mach-omap2/opp3xxx_data.c|9 +
  arch/arm/mach-omap2/voltagedomains3xxx_data.c |   10 --
  3 files changed, 18 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/mach-omap2/omap_opp_data.h 
 b/arch/arm/mach-omap2/omap_opp_data.h
 index c784c12..c7cedf3 100644
 --- a/arch/arm/mach-omap2/omap_opp_data.h
 +++ b/arch/arm/mach-omap2/omap_opp_data.h
 @@ -88,6 +88,7 @@ extern struct omap_volt_data omap34xx_vddmpu_volt_data[];
  extern struct omap_volt_data omap34xx_vddcore_volt_data[];
  extern struct omap_volt_data omap36xx_vddmpu_volt_data[];
  extern struct omap_volt_data omap36xx_vddcore_volt_data[];
 +extern struct omap_volt_data am35xx_vdd_volt_data[];
  
  extern struct omap_volt_data omap44xx_vdd_mpu_volt_data[];
  extern struct omap_volt_data omap44xx_vdd_iva_volt_data[];
 diff --git a/arch/arm/mach-omap2/opp3xxx_data.c 
 b/arch/arm/mach-omap2/opp3xxx_data.c
 index d95f3f9..e4a5ee6 100644
 --- a/arch/arm/mach-omap2/opp3xxx_data.c
 +++ b/arch/arm/mach-omap2/opp3xxx_data.c
 @@ -85,6 +85,15 @@ struct omap_volt_data omap36xx_vddcore_volt_data[] = {
   VOLT_DATA_DEFINE(0, 0, 0, 0),
  };
  
 +/* AM35x
 + *
 + * Fields related to SmartReflex and Voltage Processor are set to 0.
 + */

fix multi-line comment style (search for 'multi-line' in 
Documentation/CodingStyle)

 +struct omap_volt_data am35xx_vdd_volt_data[] = {
 + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP3_UV, 0x0, 0x0, 0x0),
 + VOLT_DATA_DEFINE(0, 0, 0, 0),
 +};
 +
  /* OPP data */
  
  static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 diff --git a/arch/arm/mach-omap2/voltagedomains3xxx_data.c 
 b/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 index 071101d..530082f 100644
 --- a/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 +++ b/arch/arm/mach-omap2/voltagedomains3xxx_data.c
 @@ -85,7 +85,10 @@ void __init omap3xxx_voltagedomains_init(void)
* XXX Will depend on the process, validation, and binning
* for the currently-running IC
*/
 - if (cpu_is_omap3630()) {
 + if (cpu_is_omap3505() || cpu_is_omap3517()) {
 + omap3_voltdm_mpu.volt_data = am35xx_vdd_volt_data;
 + omap3_voltdm_core.volt_data = am35xx_vdd_volt_data;
 + } else if (cpu_is_omap3630()) {
   omap3_voltdm_mpu.volt_data = omap36xx_vddmpu_volt_data;
   omap3_voltdm_core.volt_data = omap36xx_vddcore_volt_data;
   } else {
 @@ -93,8 +96,11 @@ void __init omap3xxx_voltagedomains_init(void)
   omap3_voltdm_core.volt_data = omap34xx_vddcore_volt_data;
   }
  
 - for (i = 0; voltdm = voltagedomains_omap3[i], voltdm; i++)
 + for (i = 0; voltdm = voltagedomains_omap3[i], voltdm; i++) {
 + if (cpu_is_omap3505() || cpu_is_omap3517())
 + voltdm-scalable = false;
   voltdm-sys_clk.name = sys_clk_name;
 + }
  
   voltdm_init(voltagedomains_omap3);
  };
--
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 v3 1/3] AM35x: voltage: Basic initialization

Re: [PATCH v3 1/3] AM35x: voltage: Basic initialization

Re: [PATCH v3 1/3] AM35x: voltage: Basic initialization

3 matches

Site Navigation

Mail list logo

Footer information