Re: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields

2012-02-21 Thread Menon, Nishanth
On Tue, Feb 21, 2012 at 08:04, Tero Kristo t-kri...@ti.com wrote:
 These are now called vddmin and vddmax, as these fields will be used
 globally for selecting voltage ranges for a pmic channel, and not
 only for voltage processor.

NAK. I think we need to setup voltage for SoC limits as well. the
programmed voltage to the VP register should be:
VP-vlimito-min = MAX(soc-vdd_min, pmic-vdd_min)
VP-vlimito-max = MIN(soc-vdd_max, pmic-vdd_max)

else you could be running the SoC beyond design voltage potentially
damaging the device.

Regards,
Nishanth Menon



 Signed-off-by: Tero Kristo t-kri...@ti.com
 ---
  arch/arm/mach-omap2/omap_twl.c |   27 ++-
  arch/arm/mach-omap2/voltage.h  |    4 ++--
  arch/arm/mach-omap2/vp.c       |    4 ++--
  3 files changed, 14 insertions(+), 21 deletions(-)

 diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-omap2/omap_twl.c
 index df4e7c3..5224fbd 100644
 --- a/arch/arm/mach-omap2/omap_twl.c
 +++ b/arch/arm/mach-omap2/omap_twl.c
 @@ -149,8 +149,8 @@ static struct omap_voltdm_pmic omap3_mpu_pmic = {
        .vp_erroroffset         = OMAP3_VP_CONFIG_ERROROFFSET,
        .vp_vstepmin            = OMAP3_VP_VSTEPMIN_VSTEPMIN,
        .vp_vstepmax            = OMAP3_VP_VSTEPMAX_VSTEPMAX,
 -       .vp_vddmin              = OMAP3430_VP1_VLIMITTO_VDDMIN,
 -       .vp_vddmax              = OMAP3430_VP1_VLIMITTO_VDDMAX,
 +       .vddmin                 = 60,
 +       .vddmax                 = 145,
        .vp_timeout_us          = OMAP3_VP_VLIMITTO_TIMEOUT_US,
        .i2c_slave_addr         = OMAP3_SRI2C_SLAVE_ADDR,
        .volt_reg_addr          = OMAP3_VDD_MPU_SR_CONTROL_REG,
 @@ -170,8 +170,8 @@ static struct omap_voltdm_pmic omap3_core_pmic = {
        .vp_erroroffset         = OMAP3_VP_CONFIG_ERROROFFSET,
        .vp_vstepmin            = OMAP3_VP_VSTEPMIN_VSTEPMIN,
        .vp_vstepmax            = OMAP3_VP_VSTEPMAX_VSTEPMAX,
 -       .vp_vddmin              = OMAP3430_VP2_VLIMITTO_VDDMIN,
 -       .vp_vddmax              = OMAP3430_VP2_VLIMITTO_VDDMAX,
 +       .vddmin                 = 60,
 +       .vddmax                 = 145,
        .vp_timeout_us          = OMAP3_VP_VLIMITTO_TIMEOUT_US,
        .i2c_slave_addr         = OMAP3_SRI2C_SLAVE_ADDR,
        .volt_reg_addr          = OMAP3_VDD_CORE_SR_CONTROL_REG,
 @@ -191,8 +191,8 @@ static struct omap_voltdm_pmic omap4_mpu_pmic = {
        .vp_erroroffset         = OMAP4_VP_CONFIG_ERROROFFSET,
        .vp_vstepmin            = OMAP4_VP_VSTEPMIN_VSTEPMIN,
        .vp_vstepmax            = OMAP4_VP_VSTEPMAX_VSTEPMAX,
 -       .vp_vddmin              = OMAP4_VP_MPU_VLIMITTO_VDDMIN,
 -       .vp_vddmax              = OMAP4_VP_MPU_VLIMITTO_VDDMAX,
 +       .vddmin                 = 0,
 +       .vddmax                 = 150,
        .vp_timeout_us          = OMAP4_VP_VLIMITTO_TIMEOUT_US,
        .i2c_slave_addr         = OMAP4_SRI2C_SLAVE_ADDR,
        .volt_reg_addr          = OMAP4_VDD_MPU_SR_VOLT_REG,
 @@ -213,8 +213,8 @@ static struct omap_voltdm_pmic omap4_iva_pmic = {
        .vp_erroroffset         = OMAP4_VP_CONFIG_ERROROFFSET,
        .vp_vstepmin            = OMAP4_VP_VSTEPMIN_VSTEPMIN,
        .vp_vstepmax            = OMAP4_VP_VSTEPMAX_VSTEPMAX,
 -       .vp_vddmin              = OMAP4_VP_IVA_VLIMITTO_VDDMIN,
 -       .vp_vddmax              = OMAP4_VP_IVA_VLIMITTO_VDDMAX,
 +       .vddmin                 = 0,
 +       .vddmax                 = 150,
        .vp_timeout_us          = OMAP4_VP_VLIMITTO_TIMEOUT_US,
        .i2c_slave_addr         = OMAP4_SRI2C_SLAVE_ADDR,
        .volt_reg_addr          = OMAP4_VDD_IVA_SR_VOLT_REG,
 @@ -235,8 +235,8 @@ static struct omap_voltdm_pmic omap4_core_pmic = {
        .vp_erroroffset         = OMAP4_VP_CONFIG_ERROROFFSET,
        .vp_vstepmin            = OMAP4_VP_VSTEPMIN_VSTEPMIN,
        .vp_vstepmax            = OMAP4_VP_VSTEPMAX_VSTEPMAX,
 -       .vp_vddmin              = OMAP4_VP_CORE_VLIMITTO_VDDMIN,
 -       .vp_vddmax              = OMAP4_VP_CORE_VLIMITTO_VDDMAX,
 +       .vddmin                 = 0,
 +       .vddmax                 = 150,
        .vp_timeout_us          = OMAP4_VP_VLIMITTO_TIMEOUT_US,
        .i2c_slave_addr         = OMAP4_SRI2C_SLAVE_ADDR,
        .volt_reg_addr          = OMAP4_VDD_CORE_SR_VOLT_REG,
 @@ -271,13 +271,6 @@ int __init omap3_twl_init(void)
        if (!cpu_is_omap34xx())
                return -ENODEV;

 -       if (cpu_is_omap3630()) {
 -               omap3_mpu_pmic.vp_vddmin = OMAP3630_VP1_VLIMITTO_VDDMIN;
 -               omap3_mpu_pmic.vp_vddmax = OMAP3630_VP1_VLIMITTO_VDDMAX;
 -               omap3_core_pmic.vp_vddmin = OMAP3630_VP2_VLIMITTO_VDDMIN;
 -               omap3_core_pmic.vp_vddmax = OMAP3630_VP2_VLIMITTO_VDDMAX;
 -       }
 -
        /*
         * The smartreflex bit on twl4030 specifies if the setting of voltage
         * is done over the I2C_SR path. Since this setting is independent of
 diff --git a/arch/arm/mach-omap2/voltage.h 

Re: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields

2012-02-21 Thread Russell King - ARM Linux
On Tue, Feb 21, 2012 at 08:40:22AM -0600, Menon, Nishanth wrote:
 On Tue, Feb 21, 2012 at 08:04, Tero Kristo t-kri...@ti.com wrote:
  These are now called vddmin and vddmax, as these fields will be used
  globally for selecting voltage ranges for a pmic channel, and not
  only for voltage processor.
 
 NAK. I think we need to setup voltage for SoC limits as well. the
 programmed voltage to the VP register should be:
 VP-vlimito-min = MAX(soc-vdd_min, pmic-vdd_min)
 VP-vlimito-max = MIN(soc-vdd_max, pmic-vdd_max)
 
 else you could be running the SoC beyond design voltage potentially
 damaging the device.

And if you're doing that kind of thing, you must also check that
the resulting min and max are sane.  In other words, the minimum is
less than the maximum.

Sure, it's something that should never happen (because it would be a
design error) but if it did happen...
--
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: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields

2012-02-21 Thread Tero Kristo
On Tue, 2012-02-21 at 14:53 +, Russell King - ARM Linux wrote:
 On Tue, Feb 21, 2012 at 08:40:22AM -0600, Menon, Nishanth wrote:
  On Tue, Feb 21, 2012 at 08:04, Tero Kristo t-kri...@ti.com wrote:
   These are now called vddmin and vddmax, as these fields will be used
   globally for selecting voltage ranges for a pmic channel, and not
   only for voltage processor.
  
  NAK. I think we need to setup voltage for SoC limits as well. the
  programmed voltage to the VP register should be:
  VP-vlimito-min = MAX(soc-vdd_min, pmic-vdd_min)
  VP-vlimito-max = MIN(soc-vdd_max, pmic-vdd_max)
  

This kind of code is actually introduced in patch #7 of this set. VP
part of the code calculates the voltage processor vlimitto values in
omap_vp_init. VC limits are handled in omap_vc_init_channel /
omap_vc_calc_vsel.

  else you could be running the SoC beyond design voltage potentially
  damaging the device.
 
 And if you're doing that kind of thing, you must also check that
 the resulting min and max are sane.  In other words, the minimum is
 less than the maximum.
 
 Sure, it's something that should never happen (because it would be a
 design error) but if it did happen...

This could be added yes, current code assumes the limits themselves are
at least somewhat sane, doesn't hurt to add a kern dump for this case I
think as it sounds rather fatal.

-Tero


--
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: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields

2012-02-21 Thread Nishanth Menon
On 17:32-20120221, Tero Kristo wrote:
 On Tue, 2012-02-21 at 14:53 +, Russell King - ARM Linux wrote:
  On Tue, Feb 21, 2012 at 08:40:22AM -0600, Menon, Nishanth wrote:
   On Tue, Feb 21, 2012 at 08:04, Tero Kristo t-kri...@ti.com wrote:
These are now called vddmin and vddmax, as these fields will be used
globally for selecting voltage ranges for a pmic channel, and not
only for voltage processor.
   
   NAK. I think we need to setup voltage for SoC limits as well. the
   programmed voltage to the VP register should be:
   VP-vlimito-min = MAX(soc-vdd_min, pmic-vdd_min)
   VP-vlimito-max = MIN(soc-vdd_max, pmic-vdd_max)
   
 
 This kind of code is actually introduced in patch #7 of this set. VP
 part of the code calculates the voltage processor vlimitto values in
 omap_vp_init. VC limits are handled in omap_vc_init_channel /
 omap_vc_calc_vsel.

Apologies , you are right #7 does indeed take this into consideration
probably belongs to #7, but, we also need to ensure that vp forceupdate
and vc_bypass actually keep to the requirement as well.

 
   else you could be running the SoC beyond design voltage potentially
   damaging the device.
  
  And if you're doing that kind of thing, you must also check that
  the resulting min and max are sane.  In other words, the minimum is
  less than the maximum.
  
  Sure, it's something that should never happen (because it would be a
  design error) but if it did happen...
 
 This could be added yes, current code assumes the limits themselves are
 at least somewhat sane, doesn't hurt to add a kern dump for this case I
 think as it sounds rather fatal.
I agree - it is indeed the case.

-- 
Regards,
Nishanth Menon
--
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