Re: [PATCH 4/6] PM / devfreq: Set the freq_table of devfreq device

[MyungJoo Ham <myungjoo.ham@samsung.com>]

>  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.