Re: [PATCH 4/4] OMAP: Voltage: Adaptive Body-Bias handlers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Mar 1, 2011 at 11:00 PM, Turquette, Mike <mturquette@xxxxxx> wrote:
> On Mon, Feb 28, 2011 at 3:35 AM, Gulati, Shweta <shweta.gulati@xxxxxx> wrote:
>> Hi,
>>
>> On Tue, Feb 22, 2011 at 1:17 AM, Turquette, Mike <mturquette@xxxxxx> wrote:
>>> On Mon, Feb 21, 2011 at 4:31 AM, Gulati, Shweta <shweta.gulati@xxxxxx> wrote:
>>>> Hi,
>>>>
>>>> On Fri, Feb 18, 2011 at 2:50 PM, Mike Turquette <mturquette@xxxxxx> wrote:
>>>>> Introduce voltage transition notification handlers for Adaptive Body-Bias
>>>>> LDOs.  There is an ABB LDO for VDD_MPU on OMAP3630 and an ABB_LDO on VDD_MPU
>>>>> and VDD_IVA on OMAP4430.
>>>>>
>>>>> All of these LDOs are handled similary.  Initial configuration is to enable
>>>>> the possibility of going into Forward Body-Bias (which boosts voltage at
>>>>> high OPPs).  This feature was designed for weak silicon, and eFuse values
>>>>> exist to control whether or not this feature should be turned on.  However
>>>>> recommendations from hardware folks always say to leave it on regardless of
>>>>> eFuse values, so we don't bother checking them.  For all other OPPs the LDO
>>>>> is in bypass and will follow the voltage of it's corresponding VDD_xxx.
>>>>> Reverse Body-Bias exists but we never use this in practice (for saving power
>>>>> on strongly characterised silicon).
>>>> Would RBB be never enabled in future for any of the OPPs or any platforms
>>>> that way SLOW_OPP should also be added in OPP types
>>>
>>> Will do this in next version.  RBB is a possibility.
>>>
>>>>> Upon a DVFS transition the notifiers handle the sequencing of voltage
>>>>> scaling and ABB LDO transitions.  When moving to a higher OPP that needs
>>>>> FBB, raise voltage first and then enable FBB.  When moving down to an OPP
>>>>> that bypasses ABB, first bypass the LDO then lower voltage.
>>>>>
>>>>> Signed-off-by: Mike Turquette <mturquette@xxxxxx>
>>>>> ---
>>>>>  arch/arm/mach-omap2/voltage.c             |  379 ++++++++++++++++++++++++++++-
>>>>>  arch/arm/plat-omap/include/plat/voltage.h |    6 +-
>>>>>  2 files changed, 370 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
>>>>> index 6ede092..644a45f 100644
>>>>> --- a/arch/arm/mach-omap2/voltage.c
>>>>> +++ b/arch/arm/mach-omap2/voltage.c
>>>>> @@ -43,6 +43,16 @@
>>>>>  #define FAST_OPP               0x1
>>>>>  #define NOMINAL_OPP            0x0
>>>>>
>>>>> +/* prototypes used by ABB function pointers */
>>>>> +static int omap_abb_notify_voltage(struct notifier_block *nb,
>>>>> +               unsigned long val, void *data);
>>>>> +
>>>>> +static int omap3_abb_configure(struct omap_abb_info *abb);
>>>>> +static int omap3_abb_set_opp(struct omap_abb_info *abb, int opp_type);
>>>>> +
>>>>> +static int omap4_abb_configure(struct omap_abb_info *abb);
>>>>> +static int omap4_abb_set_opp(struct omap_abb_info *abb, int opp_type);
>>>>> +
>>>>>  static struct omap_vdd_info *vdd_info;
>>>>>  /*
>>>>>  * Number of scalable voltage domains.
>>>>> @@ -70,9 +80,11 @@ static struct omap_vdd_info omap3_vdd_info[] = {
>>>>>                                        = OMAP3_PRM_IRQSTATUS_MPU_OFFSET,
>>>>>                        .done_st_shift  = OMAP3630_ABB_LDO_TRANXDONE_ST_SHIFT,
>>>>>                        .done_st_mask   = OMAP3630_ABB_LDO_TRANXDONE_ST_MASK,
>>>>> -                       .configure      = NULL,
>>>>> -                       .nb_handler     = NULL,
>>>>> -                       .set_opp        = NULL,
>>>>> +                       .nb     = {
>>>>> +                               .notifier_call = omap_abb_notify_voltage,
>>>>> +                       },
>>>>> +                       .configure      = omap3_abb_configure,
>>>>> +                       .set_opp        = omap3_abb_set_opp,
>>>>>                },
>>>>>        },
>>>>>        {
>>>>> @@ -113,9 +125,11 @@ static struct omap_vdd_info omap4_vdd_info[] = {
>>>>>                                        = OMAP4_PRM_IRQSTATUS_MPU_2_OFFSET,
>>>>>                        .done_st_shift  = OMAP4430_ABB_MPU_DONE_ST_SHIFT,
>>>>>                        .done_st_mask   = OMAP4430_ABB_MPU_DONE_ST_MASK,
>>>>> -                       .configure      = NULL,
>>>>> -                       .nb_handler     = NULL,
>>>>> -                       .set_opp        = NULL,
>>>>> +                       .nb             = {
>>>>> +                               .notifier_call = omap_abb_notify_voltage,
>>>>> +                       },
>>>>> +                       .configure      = omap4_abb_configure,
>>>>> +                       .set_opp        = omap4_abb_set_opp,
>>>>>                },
>>>>>        },
>>>>>        {
>>>>> @@ -137,9 +151,11 @@ static struct omap_vdd_info omap4_vdd_info[] = {
>>>>>                                        = OMAP4_PRM_IRQSTATUS_MPU_OFFSET,
>>>>>                        .done_st_shift  = OMAP4430_ABB_IVA_DONE_ST_SHIFT,
>>>>>                        .done_st_mask   = OMAP4430_ABB_IVA_DONE_ST_MASK,
>>>>> -                       .configure      = NULL,
>>>>> -                       .nb_handler     = NULL,
>>>>> -                       .set_opp        = NULL,
>>>>> +                       .nb             = {
>>>>> +                               .notifier_call = omap_abb_notify_voltage,
>>>>> +                       },
>>>>> +                       .configure      = omap4_abb_configure,
>>>>> +                       .set_opp        = omap4_abb_set_opp,
>>>>>                },
>>>>>        },
>>>>>        {
>>>>> @@ -194,7 +210,7 @@ static struct omap_volt_data omap36xx_vddmpu_volt_data[] = {
>>>>>                        NOMINAL_OPP),
>>>>>        VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP100_UV,
>>>>>                        OMAP3630_CONTROL_FUSE_OPP100_VDD1, 0xf9, 0x16,
>>>>> -                       NOMINAL_OPP),
>>>>> +                       FAST_OPP),
>>>>>        VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP120_UV,
>>>>>                        OMAP3630_CONTROL_FUSE_OPP120_VDD1, 0xfa, 0x23,
>>>>>                        NOMINAL_OPP),
>>>>> @@ -493,6 +509,74 @@ static void __init vdd_debugfs_init(struct omap_vdd_info *vdd)
>>>>>                                &nom_volt_debug_fops);
>>>>>  }
>>>>>
>> What about OMAP4, there volt_data should also be updated.
>> One think more, for IVA ABB should not be enabled by default rather
>> there should be Kconfig
>> option to Enable/Disable it.
Comments on the point of adding Kconfig options for OMAP4 IVA ABB,
which should not be enabled by default.
> You caught a bug, but not the one you think ;-)
 Yes OMAP3630_VDD_MPU_OPP1Ghz_VDD1 should be FAST OPP
> The NOMINAL_OPP/FAST_OPP definitions are done in patch 3 of this
> series (including OMAP4).  However to test that this was done
> correctly on a scope I set OPP120 to use FAST_OPP since the higher
> OPPs were disabled.  This line is a bug and I'll remove in v2.
>
>>>>> +/* voltage transition notification handlers */
>>>>> +
>>>>> +/**
>>>>> + * omap_abb_notify_voltage - voltage change notifier handler for ABB
>>>>> + * @nb         : notifier block
>>>>> + * @val                : VOLTSCALE_PRECHANGE or VOLTSCALE_POSTCHANGE
>>>>> + * @data       : struct omap_volt_change_info for a given voltage domain
>>>>> + *
>>>>> + * Sets ABB LDO to either bypass or Forward Body-Bias whenever a voltage
>>>>> + * change notification is generated.  Voltages marked as FAST will result in
>>>>> + * FBB operation of ABB LDO and voltages marked as NOMINAL will bypass the
>>>>> + * LDO.  Does not handle Reverse Body-Bias since there is not benefit for it
>>>>> + * on any existing silicon.  Returns 0 upon success, negative error code
>>>>> + * otherwise.
>>>>> + */
>>>>> +static int omap_abb_notify_voltage(struct notifier_block *nb,
>>>>> +               unsigned long val, void *data)
>>>>> +{
>>>>> +       struct omap_volt_change_info *v_info;
>>>>> +       struct omap_vdd_info *vdd;
>>>>> +       struct omap_volt_data *curr_volt_data, *target_volt_data;
>>>>> +       int ret = 0;
>>>>> +
>>>>> +       if (!nb || IS_ERR(nb) || !data || IS_ERR(data)) {
>>>>> +               pr_warning("%s: invalid data specified\n", __func__);
>>>>> +               ret = -EINVAL;
>>>>> +               goto out;
>>>>> +       }
>>>>> +
>>>>> +       v_info = (struct omap_volt_change_info *)data;
>>>>> +       vdd = v_info->vdd;
>>>>> +
>>>>> +       /* get the voltdata structures for the current & target voltage */
>>>>> +       target_volt_data = omap_voltage_get_voltdata(&vdd->voltdm,
>>>>> +                       v_info->target_volt);
>>>>> +       curr_volt_data = omap_voltage_get_voltdata(&vdd->voltdm,
>>>>> +                       v_info->curr_volt);
>>>>> +
>>>>> +       /* nothing to do here */
>>>>> +       if (target_volt_data->abb_opp == curr_volt_data->abb_opp)
>>>>> +               goto out;
>>>>> +
>>>>> +       /*
>>>>> +        * When the VDD drops from a voltage requiring the ABB LDO to be in
>>>>> +        * FBB mode to a voltage requiring bypass mode, we must bypass the LDO
>>>>> +        * before the voltage transition.
>>>>> +        */
>>>>> +       if (val == VOLTSCALE_PRECHANGE &&
>>>>> +                       target_volt_data->abb_opp == NOMINAL_OPP) {
>>>>> +               ret = vdd->abb.set_opp(&vdd->abb, NOMINAL_OPP);
>> what I meant is,  at this place you can write into OPP_SEL bits of
>> SETUP register rather than passing argument of opp_type.
>
> This defeats the point of abstraction!  In my previous implementation
> of this for Android I did directly write into the registers here.
> However I only supported OMAP4 and when it came time to add OMAP3 the
> code had yet another ugly cpu_is_omapxxxx() check.
>
> The goal is to completely abstract away the version-specific bits from
> this generic notification handler which we rely on the function
> pointer to do for us.  I'll group the register addresses/offsets
> together in the ABB struct, but I won't be changing the set_opp
> function signature in v2.
>
>>> +
>>>>> +       /*
>>>>> +        * When moving from a voltage requiring the ABB LDO to be bypassed to
>>>>> +        * a voltage requiring FBB mode, we must change the LDO operation
>>>>> +        * after the voltage transition.
>>>>> +        */
>>>>> +       } else if (val == VOLTSCALE_POSTCHANGE &&
>>>>> +                       target_volt_data->abb_opp == FAST_OPP) {
>>>>> +               ret = vdd->abb.set_opp(&vdd->abb, FAST_OPP);
>>>>> +
>> Same
>
> Same
>
>>>>> +       /* invalid combination, bail out */
>>>>> +       } else
>>>>> +               ret = -EINVAL;
>>>>> +
>>>>> +out:
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>  /* Voltage scale and accessory APIs */
>>>>>  static int _pre_volt_scale(struct omap_vdd_info *vdd,
>>>>>                unsigned long target_volt, u8 *target_vsel, u8 *current_vsel)
>>>>> @@ -717,6 +801,142 @@ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
>>>>>        return 0;
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * omap3_abb_set_opp - program ABB LDO upon a voltage transition
>>>>> + *
>>>>> + * @abb                : ABB instance being programmed
>>>>> + * @opp_type   : flag for NOMINAL or FAST OPP
>>>>> + */
>>>>> +static int omap3_abb_set_opp(struct omap_abb_info *abb, int opp_type)
>>>>> +{
>>>>> +       int ret = 0;
>>>>> +       int timeout;
>>>>> +
>>>>> +       /* program for NOMINAL OPP or FAST OPP */
>>>>> +       omap2_prm_rmw_mod_reg_bits(OMAP3630_OPP_SEL_MASK,
>>>>> +                       (opp_type << OMAP3630_OPP_SEL_SHIFT),
>>>>> +                       OMAP3430_GR_MOD, abb->setup_offs);
>>>> Rather than passing a parameter 'opp_type' u can set OPP_SEL bit in
>>>> SETUP Register in API
>>>
>>> I'm not sure I follow.  The opp_type corresponds to the voltage we're
>>> transitioning to, so that data must be passed in to this generic
>>> set_opp function.
>>>
>>>> omap_abb_notify_voltage where if-else are used to check whether ABB
>>>> needs to transition or not.
>>>
>>> The notifier handles this... it will not execute call set_opp function
>>> if there is no need to change ABB registers.
>> Answered above.
>
> Again, I won't be changing this in v2.  I'll abstract the register
> offsets away so that OMAP3 and OMAP4 can use the same notification
> handler.
>
>>>>> +
>>>>> +       /* clear ABB ldo interrupt status */
>>>>> +       omap2_prm_clear_mod_reg_bits(abb->done_st_mask, OCP_MOD,
>>>>> +                       abb->irqstatus_mpu_offs);
>>>>> +
>>>>> +       /* enable ABB LDO OPP change */
>>>>> +       omap2_prm_set_mod_reg_bits(OMAP3630_OPP_CHANGE_MASK, OMAP3430_GR_MOD,
>>>>> +                       abb->setup_offs);
>>>>> +
>>>>> +       timeout = 0;
>>>>> +
>>>>> +       /* wait until OPP change completes */
>>>>> +       while ((timeout < ABB_TRANXDONE_TIMEOUT) &&
>>>>> +                       (!(omap2_prm_read_mod_reg(OCP_MOD,
>>>>> +                                                 abb->irqstatus_mpu_offs) &
>>>>> +                          abb->done_st_mask))) {
>>>>> +               udelay(1);
>>>>> +               timeout++;
>>>>> +       }
>>>>> +
>>>>> +       if (timeout == ABB_TRANXDONE_TIMEOUT)
>>>>> +               pr_warning("%s: TRANXDONE timed out waiting for OPP change\n",
>>>>> +                               __func__);
>>>>> +
>>>>> +       timeout = 0;
>>>>> +
>>>>> +       /* Clear all pending TRANXDONE interrupts/status */
>>>>> +       while (timeout < ABB_TRANXDONE_TIMEOUT) {
>>>>> +               omap2_prm_write_mod_reg((1 << abb->done_st_shift), OCP_MOD,
>>>>> +                               abb->irqstatus_mpu_offs);
>>>>> +
>>>>> +               if (!(omap2_prm_read_mod_reg(OCP_MOD, abb->irqstatus_mpu_offs)
>>>>> +                                       & abb->done_st_mask))
>>>>> +                       break;
>>>>> +
>>>>> +               udelay(1);
>>>>> +               timeout++;
>>>>> +       }
>>>>> +
>>>>> +       if (timeout == ABB_TRANXDONE_TIMEOUT) {
>>>>> +               pr_warning("%s: TRANXDONE timed out trying to clear status\n",
>>>>> +                               __func__);
>>>>> +               ret = -EBUSY;
>>>>> +       }
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * omap4_abb_set_opp - program ABB LDO upon a voltage transition
>>>>> + *
>>>>> + * @abb                : ABB instance being programmed
>>>>> + * @opp_type   : flag for NOMINAL or FAST OPP
>>>>> + */
>>>>> +static int omap4_abb_set_opp(struct omap_abb_info *abb, int opp_type)
>>>>> +{
>>>>> +       int ret = 0;
>>>>> +       int timeout;
>>>>> +
>>>>> +       /* program for NOMINAL OPP or FAST OPP */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_OPP_SEL_MASK,
>>>>> +                       (opp_type << OMAP4430_OPP_SEL_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->ctrl_offs);
>>>>> +
>>>>> +       /* clear ABB ldo interrupt status */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(abb->done_st_mask,
>>>>> +                       (0x0 << abb->done_st_shift), OMAP4430_PRM_PARTITION,
>>>>> +                       OMAP4430_PRM_DEVICE_INST, abb->ctrl_offs);
>>>>> +
>>>>> +       /* enable ABB LDO OPP change */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_OPP_CHANGE_MASK,
>>>>> +                       (0x1 << OMAP4430_OPP_CHANGE_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->ctrl_offs);
>>>>> +
>>>> A generic struct can be created which has Masks and shifts of all
>>>> register offsets  like OMAP4430_OPP_CHANGE_MASK
>>>> and OMAP3630_ACTIVE_FBB_SEL_MASK whose objects for OMAP3 and OMAP4 can be used.
>>>
>>> True.  I chose to have the struct differentiate between VDDs, not OMAP
>>> chips.  Do others have an opinion on this?  I guess it comes down to
>>> whether we should have OMAP-specific ABB set_opp functions, and that
>>> struct data should be per-VDD (like it is in my patch)... OR if we
>>> should have a generic ABB set_opp function only that works across all
>>> OMAPs and all data is in the struct whether it is VDD- or
>>> OMAP-specific.  My problem with the latter solution is that it assumes
>>> the programming model will not change from OMAP to OMAP...
>>>
>>>>> +       timeout = 0;
>>>>> +
>>>>> +       /* wait until OPP change completes */
>>>>> +       while ((timeout < ABB_TRANXDONE_TIMEOUT) &&
>>>>> +                       (!(omap4_prminst_read_inst_reg(OMAP4430_PRM_PARTITION,
>>>>> +                                                      OMAP4430_PRM_DEVICE_INST,
>>>>> +                                                      abb->irqstatus_mpu_offs)
>>>>> +                          & abb->done_st_mask))) {
>>>>> +               udelay(1);
>>>>> +               timeout++;
>>>>> +       }
>>>>> +
>>>>> +       if (timeout == ABB_TRANXDONE_TIMEOUT)
>>>>> +               pr_warning("%s: TRANXDONE timed out waiting for OPP change\n",
>>>>> +                               __func__);
>>>>> +
>>>>> +       timeout = 0;
>>>>> +
>>>>> +       /* Clear all pending TRANXDONE interrupts/status */
>>>>> +       while (timeout < ABB_TRANXDONE_TIMEOUT) {
>>>>> +               omap4_prminst_write_inst_reg((1 << abb->done_st_shift),
>>>>> +                               OMAP4430_PRM_PARTITION,
>>>>> +                               OMAP4430_PRM_DEVICE_INST,
>>>>> +                               abb->irqstatus_mpu_offs);
>>>>> +
>>>>> +               if (!(omap4_prminst_read_inst_reg(OMAP4430_PRM_PARTITION,
>>>>> +                                               OMAP4430_PRM_DEVICE_INST,
>>>>> +                                               abb->irqstatus_mpu_offs)
>>>>> +                                       & abb->done_st_mask)) {
>>>>> +                       break;
>>>>> +
>>>>> +                       udelay(1);
>>>>> +                       timeout++;
>>>>> +               }
>>>>> +       }
>>>>> +
>>>>> +       if (timeout == ABB_TRANXDONE_TIMEOUT) {
>>>>> +               pr_warning("%s: TRANXDONE timed out trying to clear status\n",
>>>>> +                               __func__);
>>>>> +               ret = -EBUSY;
>>>>> +       }
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>  /* OMAP3 specific voltage init functions */
>>>>>
>>>>>  /*
>>>>> @@ -789,6 +1009,64 @@ static void __init omap3_vc_init(struct omap_vdd_info *vdd)
>>>>>        is_initialized = true;
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * omap3_abb_configure - per-VDD configuration of ABB
>>>>> + *
>>>>> + * @abb                : abb instance being initialized
>>>>> + */
>>>>> +static int omap3_abb_configure(struct omap_abb_info *abb)
>>>>> +{
>>>>> +       int ret = 0;
>>>>> +       u32 sr2_wt_cnt_val;
>>>>> +       struct clk *sys_ck;
>>>>> +       struct omap_vdd_info *vdd;
>>>>> +
>>>>> +       if (!abb || IS_ERR(abb)) {
>>>>> +                       pr_warning("%s: invalid abb\n", __func__);
>>>>> +                       ret = -EINVAL;
>>>>> +                       goto out;
>>>>> +       }
>>>>> +
>>>>> +       sys_ck = clk_get(NULL, "sys_ck");
>>>>> +       if (IS_ERR(sys_ck)) {
>>>>> +               pr_warning("%s: unable to fetch SYS_CK\n", __func__);
>>>>> +               ret = -ENODEV;
>>>>> +               goto out;
>>>>> +       }
>>>>> +
>>>>> +       vdd = container_of(abb, struct omap_vdd_info, abb);
>>>>> +
>>>>> +       /* LDO settling time */
>>>>> +       sr2_wt_cnt_val = clk_get_rate(sys_ck);
>>>>> +       sr2_wt_cnt_val = sr2_wt_cnt_val / 1000000 / 16;
>>>>> +
>>>>> +       omap2_prm_rmw_mod_reg_bits(OMAP3630_SR2_WTCNT_VALUE_MASK,
>>>>> +                       (sr2_wt_cnt_val << OMAP3630_SR2_WTCNT_VALUE_SHIFT),
>>>>> +                       OMAP3430_GR_MOD, abb->setup_offs);
>>>>> +
>>>>> +       /* allow FBB operation */
>>>>> +       omap2_prm_set_mod_reg_bits(OMAP3630_ACTIVE_FBB_SEL_MASK,
>>>>> +                       OMAP3430_GR_MOD, abb->setup_offs);
>>>>> +
>>>>> +       /* do not allow ACTIVE RBB operation */
>>>>> +       omap2_prm_set_mod_reg_bits(OMAP3630_ACTIVE_RBB_SEL_MASK,
>>>>> +                       OMAP3430_GR_MOD, abb->setup_offs);
>>>>> +
>>>>> +       /* do not allow SLEEP RBB operation */
>>>>> +       omap2_prm_set_mod_reg_bits(OMAP3630_SLEEP_RBB_SEL_MASK,
>>>>> +                       OMAP3430_GR_MOD, abb->setup_offs);
>>>>> +
>>>>> +       /* enable ABB LDO */
>>>>> +       omap2_prm_set_mod_reg_bits(OMAP3630_SR2EN_MASK,
>>>>> +                       OMAP3430_GR_MOD, abb->ctrl_offs);
>>>> ABB should n't be enabled in this API rather it should be enabled only
>>>> when it goes to FBB/RBB mode
>>>
>>> I disagree.  I've verified with hw team that this is perfectly safe as
>>> it leaves ABB ldo in bypass and will follow VDD_xxx voltage.  No
>>> reason to complicate notifier code with some "if
>>> (!abb_already_enabled)" check.
>>>
>>>>> +
>>>>> +       /* register the notifier handler */
>>>>> +       omap_voltage_register_notifier(vdd, &abb->nb);
>>>>> +
>>>>> +out:
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>  /* Sets up all the VDD related info for OMAP3 */
>>>>>  static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>  {
>>>>> @@ -824,6 +1102,9 @@ static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>                vdd->vc_reg.smps_volra_mask = OMAP3430_VOLRA0_MASK;
>>>>>                vdd->vc_reg.voltsetup_shift = OMAP3430_SETUP_TIME1_SHIFT;
>>>>>                vdd->vc_reg.voltsetup_mask = OMAP3430_SETUP_TIME1_MASK;
>>>>> +
>>>>> +               /* configure ABB */
>>>>> +               vdd->abb.configure(&vdd->abb);
>>>>>        } else if (!strcmp(vdd->voltdm.name, "core")) {
>>>>>                if (cpu_is_omap3630())
>>>>>                        vdd->volt_data = omap36xx_vddcore_volt_data;
>>>>> @@ -975,6 +1256,73 @@ static void __init omap4_vc_init(struct omap_vdd_info *vdd)
>>>>>        is_initialized = true;
>>>>>  }
>>>>>
>>>> To remove code repetition rather than having 2 APIs for abb_configure,
>>>> I believe 1 API which has generic register offsets struct objects for
>>>> all platforms
>>>> (OMAP3 and OMAP4 for now) can be used.
>>>> This would be scalable for OMAP5+ also.
>>>
>>> This is the same as what I've stated above for the ABB set_opp
>>> function.  We can't be sure what the programming model will be for
>>> future OMAPs, so should we really do it as your suggest?  It is
>>> possible that we retain the same set_opp function pointer, and only
>>> implemenet it once since it can be used by both OMAP3 and OMAP4 using
>>> more verbose struct data.  OMAP5 might have a different programming
>>> model altogether, so we will need to retain the set_opp function
>>> pointer just to be safe.  Thoughts on this?
>> I have checked OMAP5 ABB Transition Sequences, they are same in sequence
>> of setting/clearing registers bits, so I think a structure of
>> registers offsets and masks generic
>> across VDDs and OMAPs should be used and 1 API should be there to
>> remove code repetition
>
> OK, thanks for checking on the OMAP5 programming model.  I'll add
> register bits to the abb struct and consolidate the notification
> handler into a single generic function (will keep "omap3" in the
> function name).
>
> Regards,
> Mike
>
>>>>
>>>>> +/**
>>>>> + * omap4_abb_configure - per-VDD configuration of ABB
>>>>> + *
>>>>> + * @abb                : abb instance being initialized
>>>>> + */
>>>>> +static int omap4_abb_configure(struct omap_abb_info *abb)
>>>>> +{
>>>>> +       int ret = 0;
>>>>> +       u32 sr2_wt_cnt_val;
>>>>> +       struct clk *sys_ck;
>>>>> +       struct omap_vdd_info *vdd;
>>>>> +
>>>>> +       if (!abb || IS_ERR(abb)) {
>>>>> +                       pr_warning("%s: invalid abb\n", __func__);
>>>>> +                       ret = -EINVAL;
>>>>> +                       goto out;
>>>>> +       }
>>>>> +
>>>>> +       sys_ck = clk_get(NULL, "sys_clkin_ck");
>>>>> +       if (IS_ERR(sys_ck)) {
>>>>> +               pr_warning("%s: unable to fetch SYS_CK", __func__);
>>>>> +               ret = -ENODEV;
>>>>> +               goto out;
>>>>> +       }
>>>>> +
>>>>> +       vdd = container_of(abb, struct omap_vdd_info, abb);
>>>>> +
>>>>> +       /* LDO settling time */
>>>>> +       sr2_wt_cnt_val = clk_get_rate(sys_ck);
>>>>> +       sr2_wt_cnt_val = sr2_wt_cnt_val / 1000000 / 16;
>>>>> +
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_SR2_WTCNT_VALUE_MASK,
>>>>> +                       (sr2_wt_cnt_val << OMAP4430_SR2_WTCNT_VALUE_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->setup_offs);
>>>>> +
>>>>> +       /* allow FBB operation */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_ACTIVE_FBB_SEL_MASK,
>>>>> +                       (1 << OMAP4430_ACTIVE_FBB_SEL_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->setup_offs);
>>>>> +
>>>>> +       /* do not allow ACTIVE RBB operation */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_ACTIVE_RBB_SEL_MASK,
>>>>> +                       (0 << OMAP4430_ACTIVE_RBB_SEL_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->setup_offs);
>>>>> +
>>>>> +       /* do not allow SLEEP RBB operation */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_SLEEP_RBB_SEL_MASK,
>>>>> +                       (0 << OMAP4430_SLEEP_RBB_SEL_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->setup_offs);
>>>>> +
>>>>> +       /* enable ABB LDO */
>>>>> +       omap4_prminst_rmw_inst_reg_bits(OMAP4430_SR2EN_MASK,
>>>>> +                       (1 << OMAP4430_SR2EN_SHIFT),
>>>>> +                       OMAP4430_PRM_PARTITION, OMAP4430_PRM_DEVICE_INST,
>>>>> +                       abb->setup_offs);
>>>> Same as above.
>>>
>>> Same as my above comment ;-)  This is safe and there is no reason to change it.
>>>
>>> Regards,
>>> Mike
>>>
>>>>> +       /* register the notifier handler */
>>>>> +       omap_voltage_register_notifier(vdd, &abb->nb);
>>>>> +
>>>>> +out:
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>  /* Sets up all the VDD related info for OMAP4 */
>>>>>  static int __init omap4_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>  {
>>>>> @@ -983,8 +1331,8 @@ static int __init omap4_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>
>>>>>        if (!vdd->pmic_info) {
>>>>>                pr_err("%s: PMIC info requried to configure vdd_%s not"
>>>>> -                       "populated.Hence cannot initialize vdd_%s\n",
>>>>> -                       __func__, vdd->voltdm.name, vdd->voltdm.name);
>>>>> +                               "populated.Hence cannot initialize vdd_%s\n",
>>>>> +                               __func__, vdd->voltdm.name, vdd->voltdm.name);
>>>>>                return -EINVAL;
>>>>>        }
>>>>>
>>>>> @@ -1005,6 +1353,9 @@ static int __init omap4_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>                vdd->vc_reg.voltsetup_reg =
>>>>>                                OMAP4_PRM_VOLTSETUP_MPU_RET_SLEEP_OFFSET;
>>>>>                vdd->prm_irqst_reg = OMAP4_PRM_IRQSTATUS_MPU_2_OFFSET;
>>>>> +
>>>>> +               /* configure ABB */
>>>>> +               vdd->abb.configure(&vdd->abb);
>>>>>        } else if (!strcmp(vdd->voltdm.name, "core")) {
>>>>>                vdd->volt_data = omap44xx_vdd_core_volt_data;
>>>>>                vdd->vp_reg.tranxdone_status =
>>>>> @@ -1032,6 +1383,9 @@ static int __init omap4_vdd_data_configure(struct omap_vdd_info *vdd)
>>>>>                vdd->vc_reg.voltsetup_reg =
>>>>>                                OMAP4_PRM_VOLTSETUP_IVA_RET_SLEEP_OFFSET;
>>>>>                vdd->prm_irqst_reg = OMAP4_PRM_IRQSTATUS_MPU_OFFSET;
>>>>> +
>>>>> +               /* configure ABB */
>>>>> +               vdd->abb.configure(&vdd->abb);
>>>>>        } else {
>>>>>                pr_warning("%s: vdd_%s does not exisit in OMAP4\n",
>>>>>                        __func__, vdd->voltdm.name);
>>>>> @@ -1299,6 +1653,7 @@ int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
>>>>>
>>>>>        /* load notifier chain data */
>>>>>        v_info.target_volt = target_volt;
>>>>> +       v_info.curr_volt = vdd->curr_volt;
>>>>>        v_info.vdd = vdd;
>>>>>
>>>>>        srcu_notifier_call_chain(&vdd->volt_change_notify_chain,
>>>>> diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
>>>>> index af790bf..07997f7 100644
>>>>> --- a/arch/arm/plat-omap/include/plat/voltage.h
>>>>> +++ b/arch/arm/plat-omap/include/plat/voltage.h
>>>>> @@ -235,8 +235,8 @@ struct omap_vdd_dep_info {
>>>>>  * @irqstatus_mpu_offs : PRM_IRQSTATUS_MPU* register offset
>>>>>  * @done_st_shift      : ABB_vdd_DONE_ST shift
>>>>>  * @done_st_mask       : ABB_vdd_DONE_ST bit mask
>>>>> + * @nb                 : voltage transition notifier block
>>>>>  * @configure          : boot-time configuration
>>>>> - * @nb_handler         : voltage transition notification handler
>>>>>  * @set_opp            : transition function called from nb_handler
>>>>>  */
>>>>>  struct omap_abb_info {
>>>>> @@ -245,9 +245,8 @@ struct omap_abb_info {
>>>>>        u8 irqstatus_mpu_offs;
>>>>>        u8 done_st_shift;
>>>>>        u32 done_st_mask;
>>>>> +       struct notifier_block nb;
>>>>>        int (*configure) (struct omap_abb_info *abb);
>>>>> -       int (*nb_handler) (struct notifier_block *nb, unsigned long val,
>>>>> -                       void *data);
>>>>>        int (*set_opp) (struct omap_abb_info *abb, int opp_type);
>>>>>  };
>>>>>
>>>>> @@ -302,6 +301,7 @@ struct omap_vdd_info {
>>>>>  */
>>>>>  struct omap_volt_change_info {
>>>>>        unsigned long target_volt;
>>>>> +       unsigned long curr_volt;
>>>>>        struct omap_vdd_info *vdd;
>>>>>  };
>>>>>
>>>>> --
>>>>> 1.7.1
>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Thanks,
>>>> Regards,
>>>> Shweta
>>>>
>>>
>>
>>
>>
>> --
>> Thanks,
>> Regards,
>> Shweta
>>
>



-- 
Thanks,
Regards,
Shweta
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux