[ added Zhang, Eduardo, Ørjan and Javi to Cc: ] On 11/15/19 7:21 AM, Chanwoo Choi wrote: > Hi Bartlomiej, > > On 11/15/19 12:25 PM, Chanwoo Choi wrote: >> Hi Bartlomiej, >> >> On 11/15/19 3:01 AM, Bartlomiej Zolnierkiewicz wrote: >>> >>> Hi Chanwoo, >>> >>> On 11/14/19 2:52 AM, Chanwoo Choi wrote: >>>> Hi Kamil, >>>> >>>> The 'freq_table' and 'max_state' in the devfreq_dev_profile >>>> were used in the ARM Mali device driver[1][2][3]. Although ARM Mali >>>> device driver was posted to mainline kernel, they used >>>> them for a long time. It means that this patch break >>>> the compatibility. The ARM Mali drivers are very >>>> important devfreq device driver. >>> >>> This argument is not a a technical one and the official upstream >>> kernel policy is to not depend on out-of-tree drivers. >>> >>> Besides the ARM Mali drivers are full of code like: >>> >>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(x, y, z) >>> ... >>> #else >>> ... >>> #endif >>> >>> so few more instances of similar code won't do any harm.. ;-) >>> >>>> [1] https://protect2.fireeye.com/url?k=909caa5c-cd52abe8-909d2113-000babdfecba-812f16576c3614a3&u=https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/bifrost-kernel# >>>> [2] https://protect2.fireeye.com/url?k=33265f96-6ee85e22-3327d4d9-000babdfecba-44c2daec328712e6&u=https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/midgard-kernel >>>> [3] https://protect2.fireeye.com/url?k=69bdcab0-3473cb04-69bc41ff-000babdfecba-4b576facf85e0208&u=https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/utgard-kernel >>> >>> I took a look at ARM Mali drivers source code anyway and I fail to >>> see a rationale behind their behavior of doing 'freq_table' and >>> 'max_state' initialization in the driver itself (instead of leaving >>> it up to the devfreq core code, like all in-kernel drivers are doing >>> already). >>> >>> Could you please explain rationale behind ARM Mali drivers' special >>> needs? >>> >>> [ Both ARM Mali and devfreq core code are using generic PM OPP code >>> these days to do 'freq_table' and 'max_state' initialization, the >>> only difference seems to be that ARM Mali creates the frequency >>> table in the descending order (but there also seems to be no real >>> need for it). ] >>> >>> Maybe this is an opportunity to simplify also the ARM Mali driver? >> >> OK. I agree to simplify them on this time. >> For touching the 'freq_table' and 'max_state', need to fix >> the descending order of freq_table. >> >> The partition_enable_ops() in the drivers/thermal/devfreq_cooling.c >> requires the descending order of freq_table. Have to change it by using >> the ascending time or support both ascending and descending order for freq_table. >> >> 1. Move freq_table, max_state of devfreq_dev_profile to struct devfreq >> 2. Edit partition_enable_ops() in the drivers/thermal/devfreq_cooling.c >> by using ascending order instead of descending order. >> > > After changed about 'freq_table' and 'max_state', the build error > will happen on ARM mail driver because the initialization code of > 'freq_table' in ARM mali driver, didn't check the kernel version. > > The first devfreq patch provided the 'freq_table' as optional variable > in the 'struct devfreq_dev_profile'. Even if ARM mali driver is out of mainline tree, > this change seems that break the usage rule of 'freq_table' in 'struct devfreq_dev_profile'. > > So, if there are no any beneficial reason, I just keep the current status of 'freq_table' > in order to keep the previous usage rule of 'freq_table' in 'struct devfreq_dev_profile'. > > Frankly, I'm note sure that it is necessary. I don't want to make > the side-effect without any useful reason. > > But, > Separately, have to fix the ordering issue of partition_enable_ops() > in the drivers/thermal/devfreq_cooling.c. Hmmm.. fixing partition_enable_opps() should be trivial but I wonder why we are carrying devfreq_cooling.c code in upstream kernel at all? It has been merged in the following commit: commit a76caf55e5b356ba20a5a43ac4d9f7a04b20941d Author: Ørjan Eide <orjan.eide@xxxxxxx> Date: Thu Sep 10 18:09:30 2015 +0100 thermal: Add devfreq cooling Add a generic thermal cooling device for devfreq, that is similar to cpu_cooling. The device must use devfreq. In order to use the power extension of the cooling device, it must have registered its OPPs using the OPP library. Cc: Zhang Rui <rui.zhang@xxxxxxxxx> Cc: Eduardo Valentin <edubezval@xxxxxxxxx> Signed-off-by: Javi Merino <javi.merino@xxxxxxx> Signed-off-by: Ørjan Eide <orjan.eide@xxxxxxx> Signed-off-by: Eduardo Valentin <edubezval@xxxxxxxxx> ... but 4 years later there is still no single in-kernel user for this code? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics >>> >>>> Also, the devfreq device driver specifies their own >>>> information and data into devfreq_dev_profile structure >>>> before registering the devfreq device with devfreq_add_device(). >>>> This patch breaks the basic usage rule of devfreq_dev_profile structure. >>> >>> Well, 'struct devfreq_stats *stats' can be trivially moved out of >>> 'struct devfreq_profile' to 'struct devfreq' if you prefer it that >>> way.. >>> >>> Best regards, >>> -- >>> Bartlomiej Zolnierkiewicz >>> Samsung R&D Institute Poland >>> Samsung Electronics >>> >>>> So, I can't agree this patch. Not ack. >>>> >>>> Regards, >>>> Chanwoo Choi >>>> >>>> On 11/13/19 6:13 PM, Kamil Konieczny wrote: >>>>> Count time and transitions between devfreq frequencies in separate struct >>>>> for improved code readability and maintenance. >>>>> >>>>> Signed-off-by: Kamil Konieczny <k.konieczny@xxxxxxxxxxx> >>>>> --- >>>>> drivers/devfreq/devfreq.c | 156 ++++++++++++++++------------- >>>>> drivers/devfreq/exynos-bus.c | 6 +- >>>>> drivers/devfreq/governor_passive.c | 26 +++-- >>>>> include/linux/devfreq.h | 43 ++++---- >>>>> 4 files changed, 129 insertions(+), 102 deletions(-) >>>>> >>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>>>> index d79412b0de59..d85867a91230 100644 >>>>> --- a/drivers/devfreq/devfreq.c >>>>> +++ b/drivers/devfreq/devfreq.c >>>>> @@ -105,10 +105,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq) >>>>> */ >>>>> static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >>>>> { >>>>> + struct devfreq_stats *stats = devfreq->profile->stats; >>>>> int lev; >>>>> >>>>> - for (lev = 0; lev < devfreq->profile->max_state; lev++) >>>>> - if (freq == devfreq->profile->freq_table[lev]) >>>>> + for (lev = 0; lev < stats->max_state; lev++) >>>>> + if (freq == stats->freq_table[lev]) >>>>> return lev; >>>>> >>>>> return -EINVAL; >>>>> @@ -117,56 +118,64 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >>>>> static int set_freq_table(struct devfreq *devfreq) >>>>> { >>>>> struct devfreq_dev_profile *profile = devfreq->profile; >>>>> + struct devfreq_stats *stats; >>>>> struct dev_pm_opp *opp; >>>>> unsigned long freq; >>>>> - int i, count; >>>>> + int i, count, err = -ENOMEM; >>>>> >>>>> /* Initialize the freq_table from OPP table */ >>>>> count = dev_pm_opp_get_opp_count(devfreq->dev.parent); >>>>> if (count <= 0) >>>>> return -EINVAL; >>>>> >>>>> - profile->max_state = count; >>>>> - profile->freq_table = devm_kcalloc(devfreq->dev.parent, >>>>> - count, >>>>> - sizeof(*profile->freq_table), >>>>> - GFP_KERNEL); >>>>> - if (!profile->freq_table) { >>>>> - profile->max_state = 0; >>>>> + stats = devm_kzalloc(devfreq->dev.parent, >>>>> + sizeof(struct devfreq_stats), GFP_KERNEL); >>>>> + if (!stats) >>>>> return -ENOMEM; >>>>> - } >>>>> >>>>> - for (i = 0, freq = 0; i < profile->max_state; i++, freq++) { >>>>> + profile->stats = stats; >>>>> + stats->max_state = count; >>>>> + stats->freq_table = devm_kcalloc(devfreq->dev.parent, >>>>> + count, >>>>> + sizeof(*stats->freq_table), >>>>> + GFP_KERNEL); >>>>> + if (!stats->freq_table) >>>>> + goto err_no_mem; >>>>> + >>>>> + for (i = 0, freq = 0; i < count; i++, freq++) { >>>>> opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &freq); >>>>> if (IS_ERR(opp)) { >>>>> - devm_kfree(devfreq->dev.parent, profile->freq_table); >>>>> - profile->max_state = 0; >>>>> - return PTR_ERR(opp); >>>>> + devm_kfree(devfreq->dev.parent, stats->freq_table); >>>>> + stats->max_state = 0; >>>>> + err = PTR_ERR(opp); >>>>> + goto err_no_mem; >>>>> } >>>>> dev_pm_opp_put(opp); >>>>> - profile->freq_table[i] = freq; >>>>> + stats->freq_table[i] = freq; >>>>> } >>>>> >>>>> - profile->trans_table = devm_kzalloc(devfreq->dev.parent, >>>>> - array3_size(sizeof(unsigned int), >>>>> - count, count), >>>>> - GFP_KERNEL); >>>>> - if (!profile->trans_table) >>>>> + stats->trans_table = devm_kzalloc(devfreq->dev.parent, >>>>> + array3_size(sizeof(unsigned int), >>>>> + count, count), >>>>> + GFP_KERNEL); >>>>> + if (!stats->trans_table) >>>>> goto err_no_mem; >>>>> >>>>> - profile->time_in_state = devm_kcalloc(devfreq->dev.parent, count, >>>>> - sizeof(*profile->time_in_state), >>>>> - GFP_KERNEL); >>>>> - if (!profile->time_in_state) >>>>> + stats->time_in_state = devm_kcalloc(devfreq->dev.parent, count, >>>>> + sizeof(*stats->time_in_state), >>>>> + GFP_KERNEL); >>>>> + if (!stats->time_in_state) >>>>> goto err_no_mem; >>>>> >>>>> - profile->last_time = get_jiffies_64(); >>>>> - spin_lock_init(&profile->stats_lock); >>>>> + stats->last_time = get_jiffies_64(); >>>>> + spin_lock_init(&stats->stats_lock); >>>>> >>>>> return 0; >>>>> err_no_mem: >>>>> - profile->max_state = 0; >>>>> - return -ENOMEM; >>>>> + stats->max_state = 0; >>>>> + devm_kfree(devfreq->dev.parent, profile->stats); >>>>> + profile->stats = NULL; >>>>> + return err; >>>>> } >>>>> >>>>> /** >>>>> @@ -176,7 +185,7 @@ static int set_freq_table(struct devfreq *devfreq) >>>>> */ >>>>> int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>>>> { >>>>> - struct devfreq_dev_profile *profile = devfreq->profile; >>>>> + struct devfreq_stats *stats = devfreq->profile->stats; >>>>> unsigned long long cur_time; >>>>> int lev, prev_lev, ret = 0; >>>>> >>>>> @@ -184,22 +193,21 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>>>> >>>>> /* Immediately exit if previous_freq is not initialized yet. */ >>>>> if (!devfreq->previous_freq) { >>>>> - spin_lock(&profile->stats_lock); >>>>> - profile->last_time = cur_time; >>>>> - spin_unlock(&profile->stats_lock); >>>>> + spin_lock(&stats->stats_lock); >>>>> + stats->last_time = cur_time; >>>>> + spin_unlock(&stats->stats_lock); >>>>> return 0; >>>>> } >>>>> >>>>> prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq); >>>>> >>>>> - spin_lock(&profile->stats_lock); >>>>> + spin_lock(&stats->stats_lock); >>>>> if (prev_lev < 0) { >>>>> ret = prev_lev; >>>>> goto out; >>>>> } >>>>> >>>>> - profile->time_in_state[prev_lev] += >>>>> - cur_time - profile->last_time; >>>>> + stats->time_in_state[prev_lev] += cur_time - stats->last_time; >>>>> lev = devfreq_get_freq_level(devfreq, freq); >>>>> if (lev < 0) { >>>>> ret = lev; >>>>> @@ -207,14 +215,14 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>>>> } >>>>> >>>>> if (lev != prev_lev) { >>>>> - profile->trans_table[(prev_lev * >>>>> - profile->max_state) + lev]++; >>>>> - profile->total_trans++; >>>>> + stats->trans_table[(prev_lev * >>>>> + stats->max_state) + lev]++; >>>>> + stats->total_trans++; >>>>> } >>>>> >>>>> out: >>>>> - profile->last_time = cur_time; >>>>> - spin_unlock(&profile->stats_lock); >>>>> + stats->last_time = cur_time; >>>>> + spin_unlock(&stats->stats_lock); >>>>> return ret; >>>>> } >>>>> EXPORT_SYMBOL(devfreq_update_status); >>>>> @@ -504,9 +512,9 @@ void devfreq_monitor_resume(struct devfreq *devfreq) >>>>> queue_delayed_work(devfreq_wq, &devfreq->work, >>>>> msecs_to_jiffies(profile->polling_ms)); >>>>> >>>>> - spin_lock(&profile->stats_lock); >>>>> - profile->last_time = get_jiffies_64(); >>>>> - spin_unlock(&profile->stats_lock); >>>>> + spin_lock(&profile->stats->stats_lock); >>>>> + profile->stats->last_time = get_jiffies_64(); >>>>> + spin_unlock(&profile->stats->stats_lock); >>>>> devfreq->stop_polling = false; >>>>> >>>>> if (profile->get_cur_freq && >>>>> @@ -677,7 +685,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >>>>> devfreq->data = data; >>>>> devfreq->nb.notifier_call = devfreq_notifier_call; >>>>> >>>>> - if (!profile->max_state && !profile->freq_table) { >>>>> + if (!profile->stats) { >>>>> mutex_unlock(&devfreq->lock); >>>>> err = set_freq_table(devfreq); >>>>> if (err < 0) >>>>> @@ -1282,6 +1290,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, >>>>> const char *buf, size_t count) >>>>> { >>>>> struct devfreq *df = to_devfreq(dev); >>>>> + struct devfreq_stats *stats = df->profile->stats; >>>>> unsigned long value; >>>>> int ret; >>>>> >>>>> @@ -1297,13 +1306,13 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, >>>>> goto unlock; >>>>> } >>>>> } else { >>>>> - unsigned long *freq_table = df->profile->freq_table; >>>>> + unsigned long *freq_table = stats->freq_table; >>>>> >>>>> /* Get minimum frequency according to sorting order */ >>>>> - if (freq_table[0] < freq_table[df->profile->max_state - 1]) >>>>> + if (freq_table[0] < freq_table[stats->max_state - 1]) >>>>> value = freq_table[0]; >>>>> else >>>>> - value = freq_table[df->profile->max_state - 1]; >>>>> + value = freq_table[stats->max_state - 1]; >>>>> } >>>>> >>>>> df->min_freq = value; >>>>> @@ -1326,6 +1335,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, >>>>> const char *buf, size_t count) >>>>> { >>>>> struct devfreq *df = to_devfreq(dev); >>>>> + struct devfreq_stats *stats = df->profile->stats; >>>>> unsigned long value; >>>>> int ret; >>>>> >>>>> @@ -1341,11 +1351,11 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, >>>>> goto unlock; >>>>> } >>>>> } else { >>>>> - unsigned long *freq_table = df->profile->freq_table; >>>>> + unsigned long *freq_table = stats->freq_table; >>>>> >>>>> /* Get maximum frequency according to sorting order */ >>>>> - if (freq_table[0] < freq_table[df->profile->max_state - 1]) >>>>> - value = freq_table[df->profile->max_state - 1]; >>>>> + if (freq_table[0] < freq_table[stats->max_state - 1]) >>>>> + value = freq_table[stats->max_state - 1]; >>>>> else >>>>> value = freq_table[0]; >>>>> } >>>>> @@ -1373,14 +1383,15 @@ static ssize_t available_frequencies_show(struct device *d, >>>>> char *buf) >>>>> { >>>>> struct devfreq *df = to_devfreq(d); >>>>> + struct devfreq_stats *stats = df->profile->stats; >>>>> ssize_t count = 0; >>>>> int i; >>>>> >>>>> mutex_lock(&df->lock); >>>>> >>>>> - for (i = 0; i < df->profile->max_state; i++) >>>>> + for (i = 0; i < stats->max_state; i++) >>>>> count += scnprintf(&buf[count], (PAGE_SIZE - count - 2), >>>>> - "%lu ", df->profile->freq_table[i]); >>>>> + "%lu ", stats->freq_table[i]); >>>>> >>>>> mutex_unlock(&df->lock); >>>>> /* Truncate the trailing space */ >>>>> @@ -1398,9 +1409,10 @@ static ssize_t trans_stat_show(struct device *dev, >>>>> { >>>>> struct devfreq *devfreq = to_devfreq(dev); >>>>> struct devfreq_dev_profile *profile = devfreq->profile; >>>>> + struct devfreq_stats *stats = profile->stats; >>>>> + unsigned int max_state = stats->max_state; >>>>> ssize_t len; >>>>> int i, j; >>>>> - unsigned int max_state = profile->max_state; >>>>> >>>>> if (!devfreq->stop_polling && >>>>> devfreq_update_status(devfreq, devfreq->previous_freq)) >>>>> @@ -1411,45 +1423,45 @@ static ssize_t trans_stat_show(struct device *dev, >>>>> len = sprintf(buf, " From : To\n"); >>>>> len += sprintf(buf + len, " :"); >>>>> >>>>> - spin_lock(&profile->stats_lock); >>>>> + spin_lock(&stats->stats_lock); >>>>> for (i = 0; i < max_state; i++) >>>>> len += sprintf(buf + len, "%10lu", >>>>> - profile->freq_table[i]); >>>>> + stats->freq_table[i]); >>>>> >>>>> len += sprintf(buf + len, " time(ms)\n"); >>>>> >>>>> for (i = 0; i < max_state; i++) { >>>>> - if (profile->freq_table[i] == devfreq->previous_freq) >>>>> + if (stats->freq_table[i] == devfreq->previous_freq) >>>>> len += sprintf(buf + len, "*"); >>>>> else >>>>> len += sprintf(buf + len, " "); >>>>> >>>>> len += sprintf(buf + len, "%10lu:", >>>>> - profile->freq_table[i]); >>>>> + stats->freq_table[i]); >>>>> for (j = 0; j < max_state; j++) >>>>> len += sprintf(buf + len, "%10u", >>>>> - profile->trans_table[(i * max_state) + j]); >>>>> + stats->trans_table[(i * max_state) + j]); >>>>> len += sprintf(buf + len, "%10llu\n", (u64) >>>>> - jiffies64_to_msecs(profile->time_in_state[i])); >>>>> + jiffies64_to_msecs(stats->time_in_state[i])); >>>>> } >>>>> >>>>> len += sprintf(buf + len, "Total transition : %u\n", >>>>> - profile->total_trans); >>>>> - spin_unlock(&profile->stats_lock); >>>>> + stats->total_trans); >>>>> + spin_unlock(&stats->stats_lock); >>>>> return len; >>>>> } >>>>> static DEVICE_ATTR_RO(trans_stat); >>>>> >>>>> -static void defvreq_stats_clear_table(struct devfreq_dev_profile *profile) >>>>> +static void defvreq_stats_clear_table(struct devfreq_stats *stats) >>>>> { >>>>> - unsigned int count = profile->max_state; >>>>> - >>>>> - spin_lock(&profile->stats_lock); >>>>> - memset(profile->time_in_state, 0, count * sizeof(u64)); >>>>> - memset(profile->trans_table, 0, count * count * sizeof(int)); >>>>> - profile->last_time = get_jiffies_64(); >>>>> - profile->total_trans = 0; >>>>> - spin_unlock(&profile->stats_lock); >>>>> + unsigned int count = stats->max_state; >>>>> + >>>>> + spin_lock(&stats->stats_lock); >>>>> + memset(stats->time_in_state, 0, count * sizeof(u64)); >>>>> + memset(stats->trans_table, 0, count * count * sizeof(int)); >>>>> + stats->last_time = get_jiffies_64(); >>>>> + stats->total_trans = 0; >>>>> + spin_unlock(&stats->stats_lock); >>>>> } >>>>> >>>>> static ssize_t trans_reset_store(struct device *dev, >>>>> @@ -1459,7 +1471,7 @@ static ssize_t trans_reset_store(struct device *dev, >>>>> { >>>>> struct devfreq *devfreq = to_devfreq(dev); >>>>> >>>>> - defvreq_stats_clear_table(devfreq->profile); >>>>> + defvreq_stats_clear_table(devfreq->profile->stats); >>>>> >>>>> return count; >>>>> } >>>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>>>> index d9f377912c10..b212aae2bb3e 100644 >>>>> --- a/drivers/devfreq/exynos-bus.c >>>>> +++ b/drivers/devfreq/exynos-bus.c >>>>> @@ -496,9 +496,9 @@ static int exynos_bus_probe(struct platform_device *pdev) >>>>> } >>>>> >>>>> out: >>>>> - max_state = bus->devfreq->profile->max_state; >>>>> - min_freq = (bus->devfreq->profile->freq_table[0] / 1000); >>>>> - max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); >>>>> + max_state = profile->stats->max_state; >>>>> + min_freq = (profile->stats->freq_table[0] / 1000); >>>>> + max_freq = (profile->stats->freq_table[max_state - 1] / 1000); >>>>> pr_info("exynos-bus: new bus device registered: %s (%6ld KHz ~ %6ld KHz)\n", >>>>> dev_name(dev), min_freq, max_freq); >>>>> >>>>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c >>>>> index 58308948b863..b2d87a88335c 100644 >>>>> --- a/drivers/devfreq/governor_passive.c >>>>> +++ b/drivers/devfreq/governor_passive.c >>>>> @@ -18,6 +18,8 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>>>> struct devfreq_passive_data *p_data >>>>> = (struct devfreq_passive_data *)devfreq->data; >>>>> struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent; >>>>> + struct devfreq_stats *parent_stats = parent_devfreq->profile->stats; >>>>> + struct devfreq_stats *stats; >>>>> unsigned long child_freq = ULONG_MAX; >>>>> struct dev_pm_opp *opp; >>>>> int i, count, ret = 0; >>>>> @@ -47,10 +49,14 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>>>> * device. And then the index is used for getting the suitable >>>>> * new frequency for passive devfreq device. >>>>> */ >>>>> - if (!devfreq->profile || !devfreq->profile->freq_table >>>>> - || devfreq->profile->max_state <= 0) >>>>> + if (!devfreq->profile || !devfreq->profile->stats || >>>>> + devfreq->profile->stats->max_state <= 0 || >>>>> + !parent_devfreq->profile || !parent_devfreq->profile->stats || >>>>> + parent_devfreq->profile->stats->max_state <= 0) >>>>> return -EINVAL; >>>>> >>>>> + stats = devfreq->profile->stats; >>>>> + parent_stats = parent_devfreq->profile->stats; >>>>> /* >>>>> * The passive governor have to get the correct frequency from OPP >>>>> * list of parent device. Because in this case, *freq is temporary >>>>> @@ -68,21 +74,21 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>>>> * Get the OPP table's index of decided freqeuncy by governor >>>>> * of parent device. >>>>> */ >>>>> - for (i = 0; i < parent_devfreq->profile->max_state; i++) >>>>> - if (parent_devfreq->profile->freq_table[i] == *freq) >>>>> + for (i = 0; i < parent_stats->max_state; i++) >>>>> + if (parent_stats->freq_table[i] == *freq) >>>>> break; >>>>> >>>>> - if (i == parent_devfreq->profile->max_state) { >>>>> + if (i == parent_stats->max_state) { >>>>> ret = -EINVAL; >>>>> goto out; >>>>> } >>>>> >>>>> /* Get the suitable frequency by using index of parent device. */ >>>>> - if (i < devfreq->profile->max_state) { >>>>> - child_freq = devfreq->profile->freq_table[i]; >>>>> + if (i < stats->max_state) { >>>>> + child_freq = stats->freq_table[i]; >>>>> } else { >>>>> - count = devfreq->profile->max_state; >>>>> - child_freq = devfreq->profile->freq_table[count - 1]; >>>>> + count = stats->max_state; >>>>> + child_freq = stats->freq_table[count - 1]; >>>>> } >>>>> >>>>> /* Return the suitable frequency for passive device. */ >>>>> @@ -109,7 +115,7 @@ static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq) >>>>> if (ret < 0) >>>>> goto out; >>>>> >>>>> - if (devfreq->profile->freq_table >>>>> + if (devfreq->profile->stats >>>>> && (devfreq_update_status(devfreq, freq))) >>>>> dev_err(&devfreq->dev, >>>>> "Couldn't update frequency transition information.\n"); >>>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >>>>> index 4ceb2a517a9c..8459af1a1583 100644 >>>>> --- a/include/linux/devfreq.h >>>>> +++ b/include/linux/devfreq.h >>>>> @@ -64,6 +64,30 @@ struct devfreq_dev_status { >>>>> */ >>>>> #define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1 >>>>> >>>>> +/** >>>>> + * struct devfreq_stats - Devfreq's transitions stats counters >>>>> + * @freq_table: Optional list of frequencies to support statistics >>>>> + * and freq_table must be generated in ascending order. >>>>> + * @max_state: The size of freq_table. >>>>> + * @total_trans: Number of devfreq transitions >>>>> + * @trans_table: Statistics of devfreq transitions >>>>> + * @time_in_state: Statistics of devfreq states >>>>> + * @last_time: The last time stats were updated >>>>> + * @stats_lock: Lock protecting trans_table, time_in_state, >>>>> + * last_time and total_trans used for statistics >>>>> + */ >>>>> +struct devfreq_stats { >>>>> + unsigned long *freq_table; >>>>> + unsigned int max_state; >>>>> + >>>>> + /* information for device frequency transition */ >>>>> + unsigned int total_trans; >>>>> + unsigned int *trans_table; >>>>> + u64 *time_in_state; >>>>> + unsigned long long last_time; >>>>> + spinlock_t stats_lock; >>>>> +}; >>>>> + >>>>> /** >>>>> * struct devfreq_dev_profile - Devfreq's user device profile >>>>> * @initial_freq: The operating frequency when devfreq_add_device() is >>>>> @@ -88,15 +112,7 @@ struct devfreq_dev_status { >>>>> * from devfreq_remove_device() call. If the user >>>>> * has registered devfreq->nb at a notifier-head, >>>>> * this is the time to unregister it. >>>>> - * @freq_table: Optional list of frequencies to support statistics >>>>> - * and freq_table must be generated in ascending order. >>>>> - * @max_state: The size of freq_table. >>>>> - * @total_trans: Number of devfreq transitions >>>>> - * @trans_table: Statistics of devfreq transitions >>>>> - * @time_in_state: Statistics of devfreq states >>>>> - * @last_time: The last time stats were updated >>>>> - * @stats_lock: Lock protecting trans_table, time_in_state, >>>>> - * last_time and total_trans used for statistics >>>>> + * @stats: Statistics of devfreq states and state transitions >>>>> */ >>>>> struct devfreq_dev_profile { >>>>> unsigned long initial_freq; >>>>> @@ -108,14 +124,7 @@ struct devfreq_dev_profile { >>>>> int (*get_cur_freq)(struct device *dev, unsigned long *freq); >>>>> void (*exit)(struct device *dev); >>>>> >>>>> - unsigned long *freq_table; >>>>> - unsigned int max_state; >>>>> - /* information for device frequency transition */ >>>>> - unsigned int total_trans; >>>>> - unsigned int *trans_table; >>>>> - u64 *time_in_state; >>>>> - unsigned long long last_time; >>>>> - spinlock_t stats_lock; >>>>> + struct devfreq_stats *stats; >>>>> }; >>>>> >>>>> /** >>>>>
