Messages in this thread | | | Date | Mon, 23 Nov 2015 06:44:08 +0000 (GMT) | From | MyungJoo Ham <> | Subject | Re: [PATCH 4/6] PM / devfreq: Set the freq_table of devfreq device |
| |
> This patch initialize the freq_table array of each devfreq device by using > the devfreq_set_freq_table(). If freq_table is NULL, the devfreq framework > is not able to support the frequency transtion information through sysfs. > > The OPP core uses the integer type for the number of opps in the opp list > and uses the 'unsigned long' type for each frequency. So, this patch modifies > the type of some variable as following: > - the type of freq_table : unsigned int -> unsigned long > - the type of max_state : unsigned int -> int
I have some comments on this patch described below.
I've created an 'updated' patch based on this at: https://git.kernel.org/cgit/linux/kernel/git/mzx/devfreq.git/commit/?h=for-rafael&id=9e35a6caf143cd77f14d5e62269ed00ea1775854
It will be applied as suggested on the link above unless you have strong reason to make max_state signed.
[] > +static void devfreq_set_freq_table(struct devfreq *devfreq) > +{ > + struct devfreq_dev_profile *profile = devfreq->profile; > + struct dev_pm_opp *opp; > + unsigned long freq; # - int i;
+ int i, count;
> + > + /* Initialize the freq_table from OPP table */ # - profile->max_state = dev_pm_opp_get_opp_count(devfreq->dev.parent); # - if (profile->max_state <= 0) # - return;
+ count = dev_pm_opp_get_opp_count(devfreq->dev.parent); + if (count < 0) + return;
If dev_pm_opp_get_opp_count() gives us an error (probably, this device does not support OPP and does not give freq_table for the statistics support), we do not need to store that error in profile->max_state. We just need to return. (no need to be signed.)
[] > - len += sprintf(buf + len, "%8u", > + len += sprintf(buf + len, "%8ld", > devfreq->profile->freq_table[i]); [] > - len += sprintf(buf + len, "%8u:", > + len += sprintf(buf + len, "%8ld:", > devfreq->profile->freq_table[i]);
freq_table is unsigned.
| |