Re: [PATCH 7/7] devfreq: move statistics to separate struct
From: Lukasz Luba
Date: Tue Dec 17 2019 - 04:10:36 EST
Hi Chanwoo,
On 12/17/19 12:07 AM, Chanwoo Choi wrote:
Hi Lukaz,
On 12/16/19 10:01 PM, Lukasz Luba wrote:
Hi Bartek,
[added Dietmar, Robin, Andrzej (for upcoming DRM drm-misc-next)]
On 11/15/19 12:40 PM, Bartlomiej Zolnierkiewicz wrote:
[ 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?
Well, the devfreq_cooling.c is going to have a client in mainline:
the GPU driver - Panfrost.
It is already in DRM branch 'drm-misc-next':
https://protect2.fireeye.com/url?k=75a0e087-283b1ce4-75a16bc8-0cc47a31cdbc-4953aa9e0574f6dc&u=https://patchwork.freedesktop.org/patch/342848/
Regarding the devfreq_cooling.c code structure.
I am currently working on cleaning up the devfreq cooling code and
adding Energy Model instead for private freq, power tables. It will be
in similar fashion as it is done in cpufreq_cooling. The model will
be also simplified so hopefully more clients would come.
It is under internal review and will be posted shortly.
Good news about Energy Model. When you send the patch related to Energy model,
please add me to Cc list.
I will add you, thanks. More eyeballs in capturing bugs are more than
welcome.
Regards,
Lukasz
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?
There will be, via DRM tree.
Regards,
Lukasz
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;
 };
  /**
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
https://protect2.fireeye.com/url?k=856913bb-d8f2efd8-856898f4-0cc47a31cdbc-5cb40ce5f31ed8ed&u=http://lists.infradead.org/mailman/listinfo/linux-arm-kernel