* Smartreflex on 'pm-wip/voltdm' Branch @ 2011-03-28 10:53 Gulati, Shweta 2011-03-28 13:40 ` Vishwanath Sripathy 0 siblings, 1 reply; 5+ messages in thread From: Gulati, Shweta @ 2011-03-28 10:53 UTC (permalink / raw) To: linux-omap, Kevin Hilman Kevin, I am testing Smartreflex on your Branch 'pm-wip/voltdm'. There seems an issue with reading VP registers. For OMAP3 and OMAP4 reading debugfs entries shows constant voltage. Logs are: OMAP3: # cat /debug/voltage/vdd_mpu_iva/curr_nominal_volt 1200000 # cat /debug/voltage/vdd_core/curr_nominal_volt 1200000 # echo 1 > /debug/voltage/vdd_mpu_iva/smartreflex/autocomp [ 311.680816] omap_device: smartreflex.0: new worst case activate latency 0: 91552 # echo 1 > /debug/voltage/vdd_core/smartreflex/autocomp # cat /debug/voltage/vdd_core/vp/curr_volt 900000 # cat /debug/voltage/vdd_mpu_iva/vp/curr_volt 900000 OMAP4: # cat /debug/voltage/vdd_core/curr_nominal_volt 1200000 # cat /debug/voltage/vdd_mpu/curr_nominal_volt 1375000 # cat /debug/voltage/vdd_iva/curr_nominal_volt 1188000 # # echo 1 > /debug/voltage/vdd_mpu/smartreflex/autocomp [ 61.514038] omap_device: smartreflex.2: new worst case activate latency 0: 30517 # echo 1 > /debug/voltage/vdd_core/smartreflex/autocomp # echo 1 > /debug/voltage/vdd_mpu/smartreflex/autocomp # # cat /debug/voltage/vdd_iva/curr_nominal_volt 1188000 # cat /debug/voltage/vdd_iva/vp/curr_volt 1200000 # # echo 1 > /debug/voltage/vdd_iva/smartreflex/autocomp # cat /debug/voltage/vdd_iva/vp/curr_volt 812500 # # cat /debug/voltage/vdd_mpu/vp/curr_volt 812500 # cat /debug/voltage/vdd_core/vp/curr_volt 812500 # This Issue is not reproduced on 'pm-core' branch, seems in the voltage Layer Clean up Patches somewhere some thing is goofed up. Thanks, Regards, Shweta ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: Smartreflex on 'pm-wip/voltdm' Branch 2011-03-28 10:53 Smartreflex on 'pm-wip/voltdm' Branch Gulati, Shweta @ 2011-03-28 13:40 ` Vishwanath Sripathy 2011-03-28 16:27 ` Kevin Hilman 0 siblings, 1 reply; 5+ messages in thread From: Vishwanath Sripathy @ 2011-03-28 13:40 UTC (permalink / raw) To: Shweta Gulati, linux-omap, Kevin Hilman [-- Attachment #1: Type: text/plain, Size: 2635 bytes --] > -----Original Message----- > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Gulati, Shweta > Sent: Monday, March 28, 2011 4:23 PM > To: linux-omap@vger.kernel.org; Kevin Hilman > Subject: Smartreflex on 'pm-wip/voltdm' Branch > > Kevin, > > I am testing Smartreflex on your Branch 'pm-wip/voltdm'. There seems > an issue with reading VP registers. > For OMAP3 and OMAP4 reading debugfs entries shows constant voltage. > Logs are: > > OMAP3: > # cat /debug/voltage/vdd_mpu_iva/curr_nominal_volt > 1200000 > # cat /debug/voltage/vdd_core/curr_nominal_volt > 1200000 > # echo 1 > /debug/voltage/vdd_mpu_iva/smartreflex/autocomp > [ 311.680816] omap_device: smartreflex.0: new worst case activate > latency 0: 91552 > # echo 1 > /debug/voltage/vdd_core/smartreflex/autocomp > # cat /debug/voltage/vdd_core/vp/curr_volt > 900000 > # cat /debug/voltage/vdd_mpu_iva/vp/curr_volt > 900000 > > > OMAP4: > # cat /debug/voltage/vdd_core/curr_nominal_volt > 1200000 > # cat /debug/voltage/vdd_mpu/curr_nominal_volt > 1375000 > # cat /debug/voltage/vdd_iva/curr_nominal_volt > 1188000 > # > # echo 1 > /debug/voltage/vdd_mpu/smartreflex/autocomp > [ 61.514038] omap_device: smartreflex.2: new worst case activate > latency 0: 30517 > # echo 1 > /debug/voltage/vdd_core/smartreflex/autocomp > # echo 1 > /debug/voltage/vdd_mpu/smartreflex/autocomp > # > # cat /debug/voltage/vdd_iva/curr_nominal_volt > 1188000 > # cat /debug/voltage/vdd_iva/vp/curr_volt > 1200000 > # > # echo 1 > /debug/voltage/vdd_iva/smartreflex/autocomp > # cat /debug/voltage/vdd_iva/vp/curr_volt > 812500 > # > # cat /debug/voltage/vdd_mpu/vp/curr_volt > 812500 > # cat /debug/voltage/vdd_core/vp/curr_volt > 812500 > # > > This Issue is not reproduced on 'pm-core' branch, seems in the voltage > Layer Clean up Patches somewhere some thing is goofed up. I did a quick debug on this and found that the root cause of the issue is in usage of ffs (because of this, i2c slave address was configured wrongly in vc). Basically ffs returns the position of the first (least significant) bit set in the word and the least significant bit is position 1 where as our bit operation assumes that least significant position is 0. I tested the attached patch on OMAP3 and it seems to work fine. Kevin, You may want to incorporate this change in your next version if this seems OK to you. Vishwa > > > Thanks, > Regards, > Shweta > -- > 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 [-- Attachment #2: vc_fix.patch --] [-- Type: application/octet-stream, Size: 2308 bytes --] diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c index 4d7fe1a..ea27c9c --- a/arch/arm/mach-omap2/prm2xxx_3xxx.c +++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c @@ -189,7 +189,7 @@ int omap3_prm_vc_set_i2c_slave_addr(u8 vc_id, u8 slave_addr) return -EINVAL; omap2_prm_rmw_mod_reg_bits(vc->smps_sa_mask, - slave_addr << ffs(vc->smps_sa_mask), + slave_addr << (ffs(vc->smps_sa_mask)-1), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_SA_OFFSET); @@ -213,13 +213,13 @@ static int omap3_prm_vc_set_pmic_reg_addrs(u8 vc_id, u8 volt_addr, u8 cmd_addr) struct omap3_prm_vc *vc = &vc_channels[vc_id]; omap2_prm_rmw_mod_reg_bits(vc->smps_vol_ra_mask, - volt_addr << ffs(vc->smps_vol_ra_mask), + volt_addr << (ffs(vc->smps_vol_ra_mask)-1), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_VOL_RA_OFFSET); if (cmd_addr) omap2_prm_rmw_mod_reg_bits(vc->smps_cmd_ra_mask, - cmd_addr << ffs(vc->smps_cmd_ra_mask), + cmd_addr << (ffs(vc->smps_cmd_ra_mask)-1), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_CMD_RA_OFFSET); diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c old mode 100644 new mode 100755 index 2758b75..76bf6d3 --- a/arch/arm/mach-omap2/prm44xx.c +++ b/arch/arm/mach-omap2/prm44xx.c @@ -234,7 +234,7 @@ int omap4_prm_vc_set_i2c_slave_addr(u8 vc_id, u8 slave_addr) return -EINVAL; omap4_prminst_rmw_inst_reg_bits(vc->smps_sa_mask, - slave_addr << ffs(vc->smps_sa_mask), + slave_addr << (ffs(vc->smps_sa_mask)-1), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_SMPS_SA_OFFSET); @@ -260,14 +260,14 @@ static int omap4_prm_vc_set_pmic_reg_addrs(u8 vc_id, struct omap4_prm_vc *vc = &vc_channels[vc_id]; omap4_prminst_rmw_inst_reg_bits(vc->smps_vol_ra_mask, - volt_addr << ffs(vc->smps_vol_ra_mask), + volt_addr << (ffs(vc->smps_vol_ra_mask)-1), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_VAL_SMPS_RA_VOL_OFFSET); if (cmd_addr) omap4_prminst_rmw_inst_reg_bits(vc->smps_cmd_ra_mask, - cmd_addr << ffs(vc->smps_cmd_ra_mask), + cmd_addr << (ffs(vc->smps_cmd_ra_mask)-1), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_VAL_SMPS_RA_CMD_OFFSET); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Smartreflex on 'pm-wip/voltdm' Branch 2011-03-28 13:40 ` Vishwanath Sripathy @ 2011-03-28 16:27 ` Kevin Hilman 2011-03-30 6:21 ` Gulati, Shweta 0 siblings, 1 reply; 5+ messages in thread From: Kevin Hilman @ 2011-03-28 16:27 UTC (permalink / raw) To: Vishwanath Sripathy; +Cc: Shweta Gulati, linux-omap Vishwa, Shweta, Vishwanath Sripathy <vishwanath.bs@ti.com> writes: [...] >> I am testing Smartreflex on your Branch 'pm-wip/voltdm'. There seems >> an issue with reading VP registers. >> For OMAP3 and OMAP4 reading debugfs entries shows constant voltage. Thanks for testing. [...] > I did a quick debug on this and found that the root cause of the issue is > in usage of ffs (because of this, i2c slave address was configured wrongly > in vc). > Basically ffs returns the position of the first (least significant) bit > set in the word and the least significant bit is position 1 where as our > bit operation assumes that least significant position is 0. Vishwa, Thanks for findingg this. Indeed, I had assumed ffs() was zero based, but it is 1 based. > I tested the attached patch on OMAP3 and it seems to work fine. > Kevin, > You may want to incorporate this change in your next version if this seems > OK to you. Yes, I will fix this in forthcoming versions. Thanks, Kevin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Smartreflex on 'pm-wip/voltdm' Branch 2011-03-28 16:27 ` Kevin Hilman @ 2011-03-30 6:21 ` Gulati, Shweta 2011-03-30 14:52 ` Kevin Hilman 0 siblings, 1 reply; 5+ messages in thread From: Gulati, Shweta @ 2011-03-30 6:21 UTC (permalink / raw) To: Kevin Hilman; +Cc: Vishwanath Sripathy, linux-omap [-- Attachment #1: Type: text/plain, Size: 1276 bytes --] Hi, On Mon, Mar 28, 2011 at 9:57 PM, Kevin Hilman <khilman@ti.com> wrote: > Vishwa, Shweta, > > Vishwanath Sripathy <vishwanath.bs@ti.com> writes: > > [...] > >>> I am testing Smartreflex on your Branch 'pm-wip/voltdm'. There seems >>> an issue with reading VP registers. >>> For OMAP3 and OMAP4 reading debugfs entries shows constant voltage. > > Thanks for testing. > > [...] > >> I did a quick debug on this and found that the root cause of the issue is >> in usage of ffs (because of this, i2c slave address was configured wrongly >> in vc). >> Basically ffs returns the position of the first (least significant) bit >> set in the word and the least significant bit is position 1 where as our >> bit operation assumes that least significant position is 0. > > Vishwa, Thanks for findingg this. Indeed, I had assumed ffs() was zero based, > but it is 1 based. > >> I tested the attached patch on OMAP3 and it seems to work fine. Attached, is Patch which fixes the Issue, It works Fine for OMAP3 and OMAP4 SDP boards. >> Kevin, >> You may want to incorporate this change in your next version if this seems >> OK to you. > > Yes, I will fix this in forthcoming versions. > > Thanks, > > Kevin > -- Thanks, Regards, Shweta [-- Attachment #2: 0001-OMAP2-PM-Ensure-Bitshifts-are-calculated-accurately.patch --] [-- Type: application/octet-stream, Size: 3050 bytes --] From e97d6eb6376cbcebd057879588151e9e56b854b3 Mon Sep 17 00:00:00 2001 From: Shweta Gulati <shweta.gulati@ti.com> Date: Wed, 30 Mar 2011 11:31:04 +0530 Subject: [PATCH] OMAP2+: PM: Ensure Bitshifts are calculated accurately To Set I2C Slave Address and set PMIC register address used by VC, the bit shifts are calculated by using 'ffs' function on bit masks.With 'ffs' least significant bit is 1 instead of 0. So addresses are wrongly set. Using '__ffs' instead of 'ffs' where 0 is least significant bit. It applies on top of branch 'pm-wip/voltdm' Signed-off-by: Shweta Gulati <shweta.gulati@ti.com> --- arch/arm/mach-omap2/prm2xxx_3xxx.c | 6 +++--- arch/arm/mach-omap2/prm44xx.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c index 4d7fe1a..ecb15ed 100644 --- a/arch/arm/mach-omap2/prm2xxx_3xxx.c +++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c @@ -189,7 +189,7 @@ int omap3_prm_vc_set_i2c_slave_addr(u8 vc_id, u8 slave_addr) return -EINVAL; omap2_prm_rmw_mod_reg_bits(vc->smps_sa_mask, - slave_addr << ffs(vc->smps_sa_mask), + slave_addr << __ffs(vc->smps_sa_mask), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_SA_OFFSET); @@ -213,13 +213,13 @@ static int omap3_prm_vc_set_pmic_reg_addrs(u8 vc_id, u8 volt_addr, u8 cmd_addr) struct omap3_prm_vc *vc = &vc_channels[vc_id]; omap2_prm_rmw_mod_reg_bits(vc->smps_vol_ra_mask, - volt_addr << ffs(vc->smps_vol_ra_mask), + volt_addr << __ffs(vc->smps_vol_ra_mask), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_VOL_RA_OFFSET); if (cmd_addr) omap2_prm_rmw_mod_reg_bits(vc->smps_cmd_ra_mask, - cmd_addr << ffs(vc->smps_cmd_ra_mask), + cmd_addr << __ffs(vc->smps_cmd_ra_mask), OMAP3430_GR_MOD, OMAP3_PRM_VC_SMPS_CMD_RA_OFFSET); diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c index 2758b75..c542c25 100644 --- a/arch/arm/mach-omap2/prm44xx.c +++ b/arch/arm/mach-omap2/prm44xx.c @@ -234,7 +234,7 @@ int omap4_prm_vc_set_i2c_slave_addr(u8 vc_id, u8 slave_addr) return -EINVAL; omap4_prminst_rmw_inst_reg_bits(vc->smps_sa_mask, - slave_addr << ffs(vc->smps_sa_mask), + slave_addr << __ffs(vc->smps_sa_mask), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_SMPS_SA_OFFSET); @@ -260,14 +260,14 @@ static int omap4_prm_vc_set_pmic_reg_addrs(u8 vc_id, struct omap4_prm_vc *vc = &vc_channels[vc_id]; omap4_prminst_rmw_inst_reg_bits(vc->smps_vol_ra_mask, - volt_addr << ffs(vc->smps_vol_ra_mask), + volt_addr << __ffs(vc->smps_vol_ra_mask), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_VAL_SMPS_RA_VOL_OFFSET); if (cmd_addr) omap4_prminst_rmw_inst_reg_bits(vc->smps_cmd_ra_mask, - cmd_addr << ffs(vc->smps_cmd_ra_mask), + cmd_addr << __ffs(vc->smps_cmd_ra_mask), OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_VC_VAL_SMPS_RA_CMD_OFFSET); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Smartreflex on 'pm-wip/voltdm' Branch 2011-03-30 6:21 ` Gulati, Shweta @ 2011-03-30 14:52 ` Kevin Hilman 0 siblings, 0 replies; 5+ messages in thread From: Kevin Hilman @ 2011-03-30 14:52 UTC (permalink / raw) To: Gulati, Shweta; +Cc: Vishwanath Sripathy, linux-omap Hi Shweta, "Gulati, Shweta" <shweta.gulati@ti.com> writes: [...] >>> I did a quick debug on this and found that the root cause of the issue is >>> in usage of ffs (because of this, i2c slave address was configured wrongly >>> in vc). >>> Basically ffs returns the position of the first (least significant) bit >>> set in the word and the least significant bit is position 1 where as our >>> bit operation assumes that least significant position is 0. >> >> Vishwa, Thanks for findingg this. Indeed, I had assumed ffs() was zero based, >> but it is 1 based. >> >>> I tested the attached patch on OMAP3 and it seems to work fine. > Attached, is Patch which fixes the Issue, It works Fine for OMAP3 and > OMAP4 SDP boards. Thanks for the patch, but my voltage cleanup series is work-in-progress, and a moving target at the moment. I've changed (again) the way I'm splitting up the code so this patch won't apply directly. I will however change to using __ffs instead of ffs. Thanks for pointing that out. I wasn't aware of the difference between __ffs() and ffs() Kevin -- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-30 14:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-28 10:53 Smartreflex on 'pm-wip/voltdm' Branch Gulati, Shweta 2011-03-28 13:40 ` Vishwanath Sripathy 2011-03-28 16:27 ` Kevin Hilman 2011-03-30 6:21 ` Gulati, Shweta 2011-03-30 14:52 ` Kevin Hilman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox