| Message ID | 1295618465-15234-11-git-send-email-vishwanath.bs@ti.com |
|---|---|
| State | New |
| Headers | show |
Hi Vishwa, Vishwanath BS <vishwanath.bs@ti.com> writes: > From: Thara Gopinath <thara@ti.com> > > In OMAP3, for perfomrance reasons when VDD1 is at voltage above > 1.075V, VDD2 should be at 1.15V for perfomrance reasons. This > patch introduce this cross VDD dependency for OMAP3 VDD1. > > Signed-off-by: Thara Gopinath <thara@ti.com> As you are now on the delivery path, your signoff should be here as well. Some initial tests on 34xx turned up errors when trying to scale voltage: omap_voltage_get_voltdata: Unable to match the current voltage with the voltagetable for vdd_core First, this error message wasn't very useful since I had no idea what the "current" voltage was. Also, "current voltage" probably isn't the right word since to me, current voltage means the one currently set. This is actually the target voltage that is being looked up. In any case, root cause below... > This patch has checkpatch warnings for line over 80 chars. It is not fixed for > code readability. OK, this can either be left out, or added below the '---' as it's not needed in the final git history. > --- > arch/arm/mach-omap2/voltage.c | 42 +++++++++++++++++++++++++++++++++++++++- > 1 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c > index 92fe20d..8881c0c 100644 > --- a/arch/arm/mach-omap2/voltage.c > +++ b/arch/arm/mach-omap2/voltage.c > @@ -191,6 +191,39 @@ static struct omap_volt_data omap44xx_vdd_core_volt_data[] = { > VOLT_DATA_DEFINE(0, 0, 0, 0), > }; > > +/* OMAP 3430 MPU Core VDD dependency table */ > +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = { > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, > +}; This 34xx table has 4430 OPP voltages for CORE, which are clearly not right. 34xx has 3 CORE voltages to pick from, so I'm not sure which ones are right Please update this with the correct 34xx voltages and also validate on 34xx also. Thanks, Kevin > +static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = { > + { > + .name = "core", > + .dep_table = omap34xx_vdd1_vdd2_data, > + }, > +}; > + > +/* OMAP 3630 MPU Core VDD dependency table */ > +static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = { > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV}, > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, > +}; > + > +static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = { > + { > + .name = "core", > + .dep_table = omap36xx_vdd1_vdd2_data, > + }, > +}; > + > static struct dentry *voltage_dir; > > /* Init function pointers */ > @@ -696,10 +729,15 @@ static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd) > } > > if (!strcmp(vdd->voltdm.name, "mpu")) { > - if (cpu_is_omap3630()) > + if (cpu_is_omap3630()) { > vdd->volt_data = omap36xx_vddmpu_volt_data; > - else > + vdd->dep_vdd_info = omap36xx_vdd1_dep_info; > + vdd->nr_dep_vdd = ARRAY_SIZE(omap36xx_vdd1_dep_info); > + } else { > vdd->volt_data = omap34xx_vddmpu_volt_data; > + vdd->dep_vdd_info = omap34xx_vdd1_dep_info; > + vdd->nr_dep_vdd = ARRAY_SIZE(omap34xx_vdd1_dep_info); > + } > > vdd->vp_reg.tranxdone_status = OMAP3430_VP1_TRANXDONE_ST_MASK; > vdd->vc_reg.cmdval_reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET;
Kevin, > -----Original Message----- > From: Kevin Hilman [mailto:khilman@ti.com] > Sent: Saturday, January 29, 2011 6:01 AM > To: Vishwanath BS > Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath > Subject: Re: [PATCH 10/13] OMAP3: Add voltage dependency table for > VDD1. > > Hi Vishwa, > > Vishwanath BS <vishwanath.bs@ti.com> writes: > > > From: Thara Gopinath <thara@ti.com> > > > > In OMAP3, for perfomrance reasons when VDD1 is at voltage above > > 1.075V, VDD2 should be at 1.15V for perfomrance reasons. This > > patch introduce this cross VDD dependency for OMAP3 VDD1. > > > > Signed-off-by: Thara Gopinath <thara@ti.com> > > As you are now on the delivery path, your signoff should be here as > well. OK > > Some initial tests on 34xx turned up errors when trying to scale > voltage: > > omap_voltage_get_voltdata: Unable to match the current voltage with > the voltagetable for vdd_core > > First, this error message wasn't very useful since I had no idea what > the "current" voltage was. Also, "current voltage" probably isn't the > right word since to me, current voltage means the one currently set. > This is actually the target voltage that is being looked up. Agreed. Will fix it in the next version. > > In any case, root cause below... > > > This patch has checkpatch warnings for line over 80 chars. It is not > fixed for > > code readability. > > OK, this can either be left out, or added below the '---' as it's not > needed in the final git history. OK > > > --- > > arch/arm/mach-omap2/voltage.c | 42 > +++++++++++++++++++++++++++++++++++++++- > > 1 files changed, 40 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach- > omap2/voltage.c > > index 92fe20d..8881c0c 100644 > > --- a/arch/arm/mach-omap2/voltage.c > > +++ b/arch/arm/mach-omap2/voltage.c > > @@ -191,6 +191,39 @@ static struct omap_volt_data > omap44xx_vdd_core_volt_data[] = { > > VOLT_DATA_DEFINE(0, 0, 0, 0), > > }; > > > > +/* OMAP 3430 MPU Core VDD dependency table */ > > +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = { > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, > .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, > .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, > .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, > .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, > .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, > > +}; > > This 34xx table has 4430 OPP voltages for CORE, which are clearly not > right. 34xx has 3 CORE voltages to pick from, so I'm not sure which > ones are right > > Please update this with the correct 34xx voltages and also validate on > 34xx also. Yes, will fix it. Regarding 34xx testing, I did try to test it on 3430 SDP. However image was not booting up when I used pm branch. I see that with latest pm-core branch image boots up where as with pm branch it does not. Is this a known issue? Vishwa > > Thanks, > > Kevin > > > > +static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = { > > + { > > + .name = "core", > > + .dep_table = omap34xx_vdd1_vdd2_data, > > + }, > > +}; > > + > > +/* OMAP 3630 MPU Core VDD dependency table */ > > +static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = { > > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV, > .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV}, > > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV, > .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV, > .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV, > .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, > > +}; > > + > > +static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = { > > + { > > + .name = "core", > > + .dep_table = omap36xx_vdd1_vdd2_data, > > + }, > > +}; > > + > > static struct dentry *voltage_dir; > > > > /* Init function pointers */ > > @@ -696,10 +729,15 @@ static int __init > omap3_vdd_data_configure(struct omap_vdd_info *vdd) > > } > > > > if (!strcmp(vdd->voltdm.name, "mpu")) { > > - if (cpu_is_omap3630()) > > + if (cpu_is_omap3630()) { > > vdd->volt_data = omap36xx_vddmpu_volt_data; > > - else > > + vdd->dep_vdd_info = omap36xx_vdd1_dep_info; > > + vdd->nr_dep_vdd = > ARRAY_SIZE(omap36xx_vdd1_dep_info); > > + } else { > > vdd->volt_data = omap34xx_vddmpu_volt_data; > > + vdd->dep_vdd_info = omap34xx_vdd1_dep_info; > > + vdd->nr_dep_vdd = > ARRAY_SIZE(omap34xx_vdd1_dep_info); > > + } > > > > vdd->vp_reg.tranxdone_status = > OMAP3430_VP1_TRANXDONE_ST_MASK; > > vdd->vc_reg.cmdval_reg = > OMAP3_PRM_VC_CMD_VAL_0_OFFSET;
Vishwanath Sripathy <vishwanath.bs@ti.com> writes: [...] > Regarding 34xx testing, I did try to test it on 3430 SDP. However > image was not booting up when I used pm branch. I see that with > latest pm-core branch image boots up where as with pm branch it does > not. Is this a known issue? I don't have a 3430SDP, but it boots for me on 3430/n900, 3530/omap3evm and 3630/zoom3. Kevin
On Fri, 28 Jan 2011 16:31:22 -0800 Kevin Hilman <khilman@ti.com> wrote: > Hi Vishwa, > > Vishwanath BS <vishwanath.bs@ti.com> writes: > > > +/* OMAP 3430 MPU Core VDD dependency table */ > > +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = { > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, > > + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, > > +}; > > This 34xx table has 4430 OPP voltages for CORE, which are clearly not > right. 34xx has 3 CORE voltages to pick from, so I'm not sure which > ones are right > > Please update this with the correct 34xx voltages and also validate on > 34xx also. > Is it known what subsystem dependencies there are? What I've noticed on N900 that the DSS will stop working if core voltage is scaled under OMAP3430_VDD_CORE_OPP3_UV. I.e. either if using OMAP4430_VDD_CORE_OPP50_UV above or OMAP3430_VDD_CORE_OPP2 or 1.
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c index 92fe20d..8881c0c 100644 --- a/arch/arm/mach-omap2/voltage.c +++ b/arch/arm/mach-omap2/voltage.c @@ -191,6 +191,39 @@ static struct omap_volt_data omap44xx_vdd_core_volt_data[] = { VOLT_DATA_DEFINE(0, 0, 0, 0), }; +/* OMAP 3430 MPU Core VDD dependency table */ +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = { + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV}, + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, +}; + +static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = { + { + .name = "core", + .dep_table = omap34xx_vdd1_vdd2_data, + }, +}; + +/* OMAP 3630 MPU Core VDD dependency table */ +static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = { + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV}, + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV}, + {.main_vdd_volt = 0, .dep_vdd_volt = 0}, +}; + +static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = { + { + .name = "core", + .dep_table = omap36xx_vdd1_vdd2_data, + }, +}; + static struct dentry *voltage_dir; /* Init function pointers */ @@ -696,10 +729,15 @@ static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd) } if (!strcmp(vdd->voltdm.name, "mpu")) { - if (cpu_is_omap3630()) + if (cpu_is_omap3630()) { vdd->volt_data = omap36xx_vddmpu_volt_data; - else + vdd->dep_vdd_info = omap36xx_vdd1_dep_info; + vdd->nr_dep_vdd = ARRAY_SIZE(omap36xx_vdd1_dep_info); + } else { vdd->volt_data = omap34xx_vddmpu_volt_data; + vdd->dep_vdd_info = omap34xx_vdd1_dep_info; + vdd->nr_dep_vdd = ARRAY_SIZE(omap34xx_vdd1_dep_info); + } vdd->vp_reg.tranxdone_status = OMAP3430_VP1_TRANXDONE_ST_MASK; vdd->vc_reg.cmdval_reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET;