Vishwanath BS <vishwanath.bs@xxxxxx> writes:
> This patch adds omap_device_scale API which can be used to generic
> device rate scaling.
I would've expected a new omap_device_* API to be part of the
omap_device layer, not added here.
> Based on original patch from Thara.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@xxxxxx>
> Cc: Thara Gopinath <thara@xxxxxx>
> ---
> arch/arm/mach-omap2/dvfs.c | 116 ++++++++++++++++++++++++++++++++
> arch/arm/plat-omap/include/plat/dvfs.h | 7 ++
> 2 files changed, 123 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> index c9d3894..05a9ce3 100755
> --- a/arch/arm/mach-omap2/dvfs.c
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -19,6 +19,7 @@
> #include <plat/common.h>
> #include <plat/voltage.h>
> #include <plat/omap_device.h>
> +#include <plat/smartreflex.h>
>
> /**
> * struct omap_dev_user_list - Structure maitain userlist per device
> @@ -585,6 +586,121 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
> }
>
> /**
> + * omap_device_scale() - Set a new rate at which the device is to operate
> + * @req_dev: pointer to the device requesting the scaling.
> + * @target_dev: pointer to the device that is to be scaled
> + * @rate: the rnew rate for the device.
> + *
> + * This API gets the device opp table associated with this device and
> + * tries putting the device to the requested rate and the voltage domain
> + * associated with the device to the voltage corresponding to the
> + * requested rate. Since multiple devices can be assocciated with a
> + * voltage domain this API finds out the possible voltage the
> + * voltage domain can enter and then decides on the final device
> + * rate. Return 0 on success else the error value
> + */
Here would be a good place to describe why both the requesting device
and the target device need to be tracked.
> +int omap_device_scale(struct device *req_dev, struct device *target_dev,
> + unsigned long rate)
> +{
> + struct opp *opp;
> + unsigned long volt, freq, min_freq, max_freq;
> + struct omap_vdd_dvfs_info *dvfs_info;
> + struct platform_device *pdev;
> + struct omap_device *od;
> + int ret = 0;
> +
> + pdev = container_of(target_dev, struct platform_device, dev);
> + od = container_of(pdev, struct omap_device, pdev);
> +
> + /*
> + * Figure out if the desired frquency lies between the
> + * maximum and minimum possible for the particular device
> + */
> + min_freq = 0;
> + if (IS_ERR(opp_find_freq_ceil(target_dev, &min_freq))) {
> + dev_err(target_dev, "%s: Unable to find lowest opp\n",
> + __func__);
> + return -ENODEV;
> + }
> +
> + max_freq = ULONG_MAX;
> + if (IS_ERR(opp_find_freq_floor(target_dev, &max_freq))) {
> + dev_err(target_dev, "%s: Unable to find highest opp\n",
> + __func__);
> + return -ENODEV;
> + }
> +
> + if (rate < min_freq)
> + freq = min_freq;
> + else if (rate > max_freq)
> + freq = max_freq;
> + else
> + freq = rate;
> +
OK, frome here on, I would expect the adjusted value 'freq' to be used
instead of 'rate', but that is not the case below.
> + opp = opp_find_freq_ceil(target_dev, &freq);
> + if (IS_ERR(opp)) {
> + dev_err(target_dev, "%s: Unable to find OPP for freq%ld\n",
> + __func__, rate);
> + return -ENODEV;
> + }
> +
> + /* Get the voltage corresponding to the requested frequency */
> + volt = opp_get_voltage(opp);
> +
> + /*
> + * Call into the voltage layer to get the final voltage possible
> + * for the voltage domain associated with the device.
> + */
This comment doesn't match the following code.
> + if (rate) {
Why is rate used here, and not freq?
> + dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> +
> + ret = omap_dvfs_add_freq_request(dvfs_info, req_dev,
> + target_dev, freq);
> + if (ret) {
> + dev_err(target_dev, "%s: Unable to add frequency request\n",
> + __func__);
> + return ret;
> + }
> +
> + ret = omap_dvfs_add_vdd_user(dvfs_info, req_dev, volt);
> + if (ret) {
> + dev_err(target_dev, "%s: Unable to add voltage request\n",
> + __func__);
> + omap_dvfs_remove_freq_request(dvfs_info, req_dev,
> + target_dev);
> + return ret;
> + }
> + } else {
The function comments don't describe this case. Namely, that if you
pass in rate = 0, it removes any previous requests for the requesting
device.
> + dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> +
> + ret = omap_dvfs_remove_freq_request(dvfs_info, req_dev,
> + target_dev);
> + if (ret) {
> + dev_err(target_dev, "%s: Unable to remove frequency request\n",
> + __func__);
> + return ret;
> + }
> +
> + ret = omap_dvfs_remove_vdd_user(dvfs_info, req_dev);
> + if (ret) {
> + dev_err(target_dev, "%s: Unable to remove voltage request\n",
> + __func__);
> + return ret;
> + }
> + }
> +
> + /* Do the actual scaling */
> + ret = omap_dvfs_voltage_scale(dvfs_info);
ok
> + if (!ret)
> + if (omap_device_get_rate(target_dev) >= rate)
> + return 0;
> +
but this bit needs some explanation. IIUC: if the _voltage_scale()
fails (which also scales the frequency) but the frequency was
sucessfully changed, then return success.
Also 'rate' is used here where 'freq' would be expected.
> + return ret;
> +}
> +EXPORT_SYMBOL(omap_device_scale);
> +
> +/**
> * omap_dvfs_init() - Initialize omap dvfs layer
> *
> * Initalizes omap dvfs layer. It basically allocates memory for
> diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-omap/include/plat/dvfs.h
> index 1302990..1be2b9d 100644
> --- a/arch/arm/plat-omap/include/plat/dvfs.h
> +++ b/arch/arm/plat-omap/include/plat/dvfs.h
> @@ -17,11 +17,18 @@
>
> #ifdef CONFIG_PM
> int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev);
> +int omap_device_scale(struct device *req_dev, struct device *dev,
> + unsigned long rate);
> #else
> static inline int omap_dvfs_register_device(struct voltagedomain *voltdm,
> struct device *dev)
> {
> return -EINVAL;
> }
> +static inline int omap_device_scale(struct device *req_dev, struct devices
> + *target_dev, unsigned long rate);
> +{
> + return -EINVAL;
> +}
> #endif
> #endif
Kevin
--
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
