RE: Smartreflex on 'pm-wip/voltdm' Branch

[Vishwanath Sripathy <vishwanath.bs@ti.com>]

[-- 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);