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

Gulati, Shweta
2011-03-30T06:21:50Z

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