Re: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields
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 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
Re: [PATCHv5 02/14] arm: omap: voltage: renamed vp_vddmin and vp_vddmax fields
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 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
On Tue, Feb 21, 2012 at 08:40:22AM -0600, Menon, Nishanth wrote: > On Tue, Feb 21, 2012 at 08:04, Tero Kristo 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
On Tue, Feb 21, 2012 at 08:04, Tero Kristo 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 > --- > 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 --g