* RE: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls [not found] <1321547303-21807-1-git-send-email-vaibhav.bedia@ti.com> @ 2011-11-25 6:29 ` Bedia, Vaibhav 2011-12-02 22:27 ` Kevin Hilman 0 siblings, 1 reply; 6+ messages in thread From: Bedia, Vaibhav @ 2011-11-25 6:29 UTC (permalink / raw) To: Hilman, Kevin, linux-omap@vger.kernel.org; +Cc: Kattekola, Ravikumar Hi, On Thu, Nov 17, 2011 at 21:58:23, Bedia, Vaibhav wrote: > TI processors in TI81x and AM33x family work with PMICs like > TPS65910/1 which are not part of the TWL series. These processors > also do not have a voltage controller/processor module. > > In order to invoke the normal regulator calls from the voltage > layer the following changes are done to struct voltagedomain > - Add a flag use_regulator for the SoC voltagedomain > code to indicate its intention of using a PMIC which > is not controlled by VC/VP > - Add a regulator_init callback which the platform code > can utilise for any custom init sequence before making > use of the regulator. Platform code is also expected > to set the voltdm->scale function in the init callback > > Signed-off-by: Ravikumar Kattekola <rk@ti.com> > Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com> > --- > arch/arm/mach-omap2/voltage.c | 5 +++++ > arch/arm/mach-omap2/voltage.h | 4 ++++ > 2 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c > index 1f8fdf7..ca4ef9e 100644 > --- a/arch/arm/mach-omap2/voltage.c > +++ b/arch/arm/mach-omap2/voltage.c > @@ -280,6 +280,11 @@ int __init omap_voltage_late_init(void) > voltdm->scale = omap_vp_forceupdate_scale; > omap_vp_init(voltdm); > } > + > + if (voltdm->use_regulator) { > + if(voltdm->regulator_init) > + voltdm->regulator_init(voltdm); > + } > } > > return 0; > diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h > index 16a1b09..21c810f 100644 > --- a/arch/arm/mach-omap2/voltage.h > +++ b/arch/arm/mach-omap2/voltage.h > @@ -88,6 +88,10 @@ struct voltagedomain { > > u32 nominal_volt; > struct omap_volt_data *volt_data; > + > + bool use_regulator; > + struct regulator *regulator; > + int (*regulator_init) (struct voltagedomain *voltdm); > }; > Any comments on this approach? This enables us to make use of generic regulators calls from the voltage layer. Since TI81xx and AM33xx do not have VC/VP we need something like this for implementing DVFS. Regards, Vaibhav ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls 2011-11-25 6:29 ` [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls Bedia, Vaibhav @ 2011-12-02 22:27 ` Kevin Hilman 2011-12-05 16:02 ` Bedia, Vaibhav 0 siblings, 1 reply; 6+ messages in thread From: Kevin Hilman @ 2011-12-02 22:27 UTC (permalink / raw) To: Bedia, Vaibhav; +Cc: linux-omap@vger.kernel.org, Kattekola, Ravikumar "Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes: > Hi, > > On Thu, Nov 17, 2011 at 21:58:23, Bedia, Vaibhav wrote: >> TI processors in TI81x and AM33x family work with PMICs like >> TPS65910/1 which are not part of the TWL series. These processors >> also do not have a voltage controller/processor module. >> >> In order to invoke the normal regulator calls from the voltage >> layer the following changes are done to struct voltagedomain >> - Add a flag use_regulator for the SoC voltagedomain >> code to indicate its intention of using a PMIC which >> is not controlled by VC/VP >> - Add a regulator_init callback which the platform code >> can utilise for any custom init sequence before making >> use of the regulator. Platform code is also expected >> to set the voltdm->scale function in the init callback >> >> Signed-off-by: Ravikumar Kattekola <rk@ti.com> >> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com> >> --- >> arch/arm/mach-omap2/voltage.c | 5 +++++ >> arch/arm/mach-omap2/voltage.h | 4 ++++ >> 2 files changed, 9 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c >> index 1f8fdf7..ca4ef9e 100644 >> --- a/arch/arm/mach-omap2/voltage.c >> +++ b/arch/arm/mach-omap2/voltage.c >> @@ -280,6 +280,11 @@ int __init omap_voltage_late_init(void) >> voltdm->scale = omap_vp_forceupdate_scale; >> omap_vp_init(voltdm); >> } >> + >> + if (voltdm->use_regulator) { >> + if(voltdm->regulator_init) >> + voltdm->regulator_init(voltdm); >> + } >> } >> >> return 0; >> diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h >> index 16a1b09..21c810f 100644 >> --- a/arch/arm/mach-omap2/voltage.h >> +++ b/arch/arm/mach-omap2/voltage.h >> @@ -88,6 +88,10 @@ struct voltagedomain { >> >> u32 nominal_volt; >> struct omap_volt_data *volt_data; >> + >> + bool use_regulator; >> + struct regulator *regulator; >> + int (*regulator_init) (struct voltagedomain *voltdm); >> }; >> > > Any comments on this approach? Sorry, I didn't see this patch before, and I don't see it in the linux-omap archives either. Not sure what happened there... > This enables us to make use of generic regulators calls from the > voltage layer. >From this patch, I don't see how the regulator API is being used from the voltage layer, so I don't fully understand what you're trying to achieve. IOW, why should the voltagedomain code be calling the regulator API? The voltage domain code was designed with the opposite goal: namely, that a regulator driver would be calling the voltage domain layer, not vice versa. > Since TI81xx and AM33xx do not have VC/VP we need something like this > for implementing DVFS. This patch (and changelog) does not make that very clear, since it only adds an init-time hook. Kevin ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls 2011-12-02 22:27 ` Kevin Hilman @ 2011-12-05 16:02 ` Bedia, Vaibhav 2011-12-05 19:07 ` Kevin Hilman 0 siblings, 1 reply; 6+ messages in thread From: Bedia, Vaibhav @ 2011-12-05 16:02 UTC (permalink / raw) To: Hilman, Kevin; +Cc: linux-omap@vger.kernel.org, Kattekola, Ravikumar Hi Kevin, On Sat, Dec 03, 2011 at 03:57:57, Hilman, Kevin wrote: [...] > > > > Any comments on this approach? > > Sorry, I didn't see this patch before, and I don't see it in the > linux-omap archives either. Not sure what happened there... > > > This enables us to make use of generic regulators calls from the > > voltage layer. > > From this patch, I don't see how the regulator API is being used from > the voltage layer, so I don't fully understand what you're trying to > achieve. > > IOW, why should the voltagedomain code be calling the regulator API? > > The voltage domain code was designed with the opposite goal: namely, > that a regulator driver would be calling the voltage domain layer, not > vice versa. Sorry, I should have posted more detail on how we are making use of the change proposed here and how it helps in getting cpufreq and DVFS working. We want to use the existing OMAP implementation of cpufreq (and DVFS) on the devices which do not have VC/VP. The current OMAP cpufreq code under drivers/ invokes clk_set_rate(). We had a look at the future DVFS implementation for OMAP[1] and merged in the relevant patches (hope this is close to what's planned). After this change, cpufreq invokes omap_device_scale(). The DVFS code makes use of the OPP layer and adds a request for voltage and frequency change. However, the voltage change request expects the scaling to be done in scaling functions defined in the voltage layer (vc->scale or vp->scale) On devices which do not have VC/VP, instead of just hacking the current implementation to get the voltage scaling done, we thought of adding a separate path. Taking the example of AM335x, the regulator used on the EVM is TPS65910 the driver for which is already present in the mainline kernel. So, when omap_device_scale() is invoked from cpufreq, we want the voltage scaling to finally trickle down to a regulator_set_voltage() for the MPU supply. The way we have currently done this is- === int am33x_mpu_voltdm_scale(struct voltagedomain *voltdm, unsigned long target_volt) { int ret = -EINVAL; if (!voltdm->regulator) return ret; ret = regulator_set_voltage(voltdm->regulator, target_volt, target_volt + TOLERANCE); if (ret) pr_debug("Voltage change failed, ret = %d\n", ret); else pr_debug("Voltage scaled to %d\n", regulator_get_voltage(voltdm->regulator)); return ret; } struct omap_vdd_dep_info am33xx_vddmpu_dep_info[] = { {.name = NULL, .dep_table = NULL, .nr_dep_entries = 0}, }; static struct omap_vdd_info am33xx_vdd1_info; int am33x_mpu_voltdm_init(struct voltagedomain *voltdm) { struct regulator *mpu_regulator; struct device *mpu_dev; mpu_dev = omap_device_get_by_hwmod_name("mpu"); if (!mpu_dev) { pr_warning("%s: unable to get the mpu device\n", __func__); return -EINVAL; } mpu_regulator = regulator_get(mpu_dev, voltdm->name); if (IS_ERR(mpu_regulator)) { pr_err("%s: Could not get regulator for %s\n", __func__, voltdm->name); return -ENODEV; } else { voltdm->regulator = mpu_regulator; voltdm->scale = &am33x_mpu_voltdm_scale; } return 0; } static struct voltagedomain am33xx_voltdm_mpu = { .name = "mpu", .scalable = true, .use_regulator = 1, .regulator_init = &am33x_mpu_voltdm_init, .vdd = &am33xx_vdd1_info, }; === In the init hook we set the voltdm->scale to a function which invokes regulator_set_voltage(). All the other pieces are already taken care of by the OPP layer and the DVFS implementation. We just had to add the OPP data and enable cpufreq for AM335x. > > > Since TI81xx and AM33xx do not have VC/VP we need something like this > > for implementing DVFS. > > This patch (and changelog) does not make that very clear, since it only > adds an init-time hook. > Does this approach sound reasonable? If you want to check the implementation, I could push the cpufreq implementation that we currently have to Arago. Regards, Vaibhav [1] git://gitorious.org/~kristo/omap-pm/omap-pm-work.git:linaro_omap_dvfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls 2011-12-05 16:02 ` Bedia, Vaibhav @ 2011-12-05 19:07 ` Kevin Hilman 2011-12-06 4:21 ` Bedia, Vaibhav 0 siblings, 1 reply; 6+ messages in thread From: Kevin Hilman @ 2011-12-05 19:07 UTC (permalink / raw) To: Bedia, Vaibhav; +Cc: linux-omap@vger.kernel.org, Kattekola, Ravikumar "Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes: > Hi Kevin, > > On Sat, Dec 03, 2011 at 03:57:57, Hilman, Kevin wrote: > [...] >> > >> > Any comments on this approach? >> >> Sorry, I didn't see this patch before, and I don't see it in the >> linux-omap archives either. Not sure what happened there... >> >> > This enables us to make use of generic regulators calls from the >> > voltage layer. >> >> From this patch, I don't see how the regulator API is being used from >> the voltage layer, so I don't fully understand what you're trying to >> achieve. >> >> IOW, why should the voltagedomain code be calling the regulator API? >> >> The voltage domain code was designed with the opposite goal: namely, >> that a regulator driver would be calling the voltage domain layer, not >> vice versa. > > Sorry, I should have posted more detail on how we are making use of the > change proposed here and how it helps in getting cpufreq and DVFS working. > > We want to use the existing OMAP implementation of cpufreq (and DVFS) on > the devices which do not have VC/VP. > > The current OMAP cpufreq code under drivers/ invokes clk_set_rate(). > > We had a look at the future DVFS implementation for OMAP[1] and merged in > the relevant patches (hope this is close to what's planned). Unfortunately, that implementation is not what's planned, and in fact was rejected some time ago. > After this change, cpufreq invokes omap_device_scale(). The DVFS code > makes use of the OPP layer and adds a request for voltage and frequency > change. However, the voltage change request expects the scaling to be done > in scaling functions defined in the voltage layer (vc->scale or vp->scale) > > On devices which do not have VC/VP, instead of just hacking the current > implementation to get the voltage scaling done, we thought of adding > a separate path. A much better path for SoCs without VC/VP is to simply modify the CPUfreq driver to use the regulator API to scale voltage before calling clk_set_rate() (if scaling up) and after scaling the frequency (if scaling down.) In fact, that is the direction we're going for DVFS, even for VC/VP platforms. There will be a regulator driver which will call the voltagedomain/VC/VP calls to scale the voltage when needed, so from a DVFS perspective, you use the clock API to change frequency, and the regulator API to change voltages. This should be a much simpler approach for you, and much easier to understand. Kevin ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls 2011-12-05 19:07 ` Kevin Hilman @ 2011-12-06 4:21 ` Bedia, Vaibhav 2011-12-06 19:47 ` Kevin Hilman 0 siblings, 1 reply; 6+ messages in thread From: Bedia, Vaibhav @ 2011-12-06 4:21 UTC (permalink / raw) To: Hilman, Kevin; +Cc: linux-omap@vger.kernel.org, Kattekola, Ravikumar On Tue, Dec 06, 2011 at 00:37:38, Hilman, Kevin wrote: [...] > > > > We want to use the existing OMAP implementation of cpufreq (and DVFS) on > > the devices which do not have VC/VP. > > > > The current OMAP cpufreq code under drivers/ invokes clk_set_rate(). > > > > We had a look at the future DVFS implementation for OMAP[1] and merged in > > the relevant patches (hope this is close to what's planned). > > Unfortunately, that implementation is not what's planned, and in fact > was rejected some time ago. That's really unfortunate :( > > > After this change, cpufreq invokes omap_device_scale(). The DVFS code > > makes use of the OPP layer and adds a request for voltage and frequency > > change. However, the voltage change request expects the scaling to be done > > in scaling functions defined in the voltage layer (vc->scale or vp->scale) > > > > On devices which do not have VC/VP, instead of just hacking the current > > implementation to get the voltage scaling done, we thought of adding > > a separate path. > > A much better path for SoCs without VC/VP is to simply modify the > CPUfreq driver to use the regulator API to scale voltage before calling > clk_set_rate() (if scaling up) and after scaling the frequency (if > scaling down.) I assume this needs to be done using the OPP library? > > In fact, that is the direction we're going for DVFS, even for VC/VP > platforms. There will be a regulator driver which will call the > voltagedomain/VC/VP calls to scale the voltage when needed, so from a > DVFS perspective, you use the clock API to change frequency, and the > regulator API to change voltages. Wouldn't this end up adding OMAP specific code into the various regulator drivers? If TPS65910 gets used on a variant that has VC/VP, won't this require making changes in the regulator drivers to being with? > > This should be a much simpler approach for you, and much easier to > understand. Yes, making calls to the regulator and clock frameworks is much simpler and desirable. Regards, Vaibhav ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls 2011-12-06 4:21 ` Bedia, Vaibhav @ 2011-12-06 19:47 ` Kevin Hilman 0 siblings, 0 replies; 6+ messages in thread From: Kevin Hilman @ 2011-12-06 19:47 UTC (permalink / raw) To: Bedia, Vaibhav; +Cc: linux-omap@vger.kernel.org, Kattekola, Ravikumar "Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes: > On Tue, Dec 06, 2011 at 00:37:38, Hilman, Kevin wrote: > [...] >> > >> > We want to use the existing OMAP implementation of cpufreq (and DVFS) on >> > the devices which do not have VC/VP. >> > >> > The current OMAP cpufreq code under drivers/ invokes clk_set_rate(). >> > >> > We had a look at the future DVFS implementation for OMAP[1] and merged in >> > the relevant patches (hope this is close to what's planned). >> >> Unfortunately, that implementation is not what's planned, and in fact >> was rejected some time ago. > > That's really unfortunate :( > >> >> > After this change, cpufreq invokes omap_device_scale(). The DVFS code >> > makes use of the OPP layer and adds a request for voltage and frequency >> > change. However, the voltage change request expects the scaling to be done >> > in scaling functions defined in the voltage layer (vc->scale or vp->scale) >> > >> > On devices which do not have VC/VP, instead of just hacking the current >> > implementation to get the voltage scaling done, we thought of adding >> > a separate path. >> >> A much better path for SoCs without VC/VP is to simply modify the >> CPUfreq driver to use the regulator API to scale voltage before calling >> clk_set_rate() (if scaling up) and after scaling the frequency (if >> scaling down.) > > I assume this needs to be done using the OPP library? Yes, the mainline cpufreq driver (queued and merging for v3.3, see my for_3.3/omap-cpufreq branch) already uses the OPP library to determine frequencies. From there, it's a small step to get the voltage associated with the OPP and use the regulator framework to scale the voltage. >> >> In fact, that is the direction we're going for DVFS, even for VC/VP >> platforms. There will be a regulator driver which will call the >> voltagedomain/VC/VP calls to scale the voltage when needed, so from a >> DVFS perspective, you use the clock API to change frequency, and the >> regulator API to change voltages. > > Wouldn't this end up adding OMAP specific code into the various regulator > drivers? If TPS65910 gets used on a variant that has VC/VP, won't this > require making changes in the regulator drivers to being with? Yes. Tero is working on this now for the TWL PMICs, and the additional code is rather small. >> >> This should be a much simpler approach for you, and much easier to >> understand. > > Yes, making calls to the regulator and clock frameworks is much simpler > and desirable. Great, I'm glad you agree. Kevin ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-06 19:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1321547303-21807-1-git-send-email-vaibhav.bedia@ti.com>
2011-11-25 6:29 ` [RFC][PATCH 1/1] OMAP: voltage: add a hook for normal regulator calls Bedia, Vaibhav
2011-12-02 22:27 ` Kevin Hilman
2011-12-05 16:02 ` Bedia, Vaibhav
2011-12-05 19:07 ` Kevin Hilman
2011-12-06 4:21 ` Bedia, Vaibhav
2011-12-06 19:47 ` Kevin Hilman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.