shweta gulati <[email protected]> writes: > From: Shweta Gulati <[email protected]> > > This version of patch incorporates review comments which includes > shifting the code change in specific function 'omap3_iva_idle' and > removing iva_pwrdm from pwrst_list rather than checking all the pwrdms > in list to exclude iva_pwrdm.
Comments about the version history of a patch don't belong here. They belong after the '---' so they are not picked up in the git history. > The PM code should not set latency on IVA power state based on > the flag 'enable_off_mode'. Why? A changelog is about answering *both* 'what' and 'why' Also the SRF change should be a separate patch since SRF is PM branch only. The primary change here should be targetted for mainline. > This is taken care of in this version. > > Signed-off-by: Sripathy Vishwanath <[email protected]> > Signed-off-by: Shweta Gulati <[email protected]> > --- I *strongly* recommend using git-format-patch, then manually editing this part to add patch version history. > Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c > =================================================================== > --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c > +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c > @@ -786,6 +786,12 @@ static void __init omap3_iva_idle(void) > OMAP3430_RST2_IVA2 | > OMAP3430_RST3_IVA2, > OMAP3430_IVA2_MOD, OMAP2_RM_RSTCTRL); > + /* Put the IVA2 In Idle */ > + prm_rmw_mod_reg_bits(OMAP3430_LASTPOWERSTATEENTERED_MASK, 0, > + OMAP3430_IVA2_MOD, OMAP2_PM_PWSTCTRL); huh? this is confusing for multiple reasons 1. The comment is wrong. You're setting the nex powerstate to OFF. 2. LASTPOWERSTATEENTERED is a field in PM_PREPWSTST, not in PM_PWSTCTRL, so the field your changing is the POWERSTATE field of PM_PWRSTCTRL. 3. setting this state is already done in pwrdms_setup() > + /* Make Clock transition Automatic*/ nit: need a space before the '*/' > + cm_rmw_mod_reg_bits(OMAP3430_CLKTRCTRL_IVA2_MASK, 0x3, > + OMAP3430_IVA2_MOD, OMAP2_CM_CLKSTCTRL); > } > > static void __init omap3_d2d_idle(void) > @@ -1074,8 +1080,11 @@ static int __init pwrdms_setup(struct po > if (!pwrst) > return -ENOMEM; > pwrst->pwrdm = pwrdm; > - pwrst->next_state = PWRDM_POWER_RET; > - list_add(&pwrst->node, &pwrst_list); > + if (strcmp("iva2_pwrdm", pwrdm->name)) { > + pwrst->next_state = PWRDM_POWER_RET; > + list_add(&pwrst->node, &pwrst_list); > + } else > + pwrst->next_state = PWRDM_POWER_OFF; See Documentation/CodingStyle about use of {}. If one part of an 'if' has {}s, the other(s) should as well. > if (pwrdm_has_hdwr_sar(pwrdm)) > pwrdm_enable_hdwr_sar(pwrdm); > Index: linux-omap-pm/arch/arm/mach-omap2/resource34xx.c > =================================================================== > --- linux-omap-pm.orig/arch/arm/mach-omap2/resource34xx.c > +++ linux-omap-pm/arch/arm/mach-omap2/resource34xx.c > @@ -140,7 +140,8 @@ int set_pd_latency(struct shared_resourc > } > > if (!enable_off_mode && pd_lat_level == PD_LATENCY_OFF) > - pd_lat_level = PD_LATENCY_RET; > + if (strcmp("iva2_pwrdm", pwrdm->name)) > + pd_lat_level = PD_LATENCY_RET; > > resp->curr_level = pd_lat_level; > set_pwrdm_state(pwrdm, pd_lat_level); This change should be a separate patch including a changelog that answers "why?" Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
