linux-kernel - Re: [PATCH 4/6] PM / devfreq: Set the freq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-id: <693585704.122261448261048874.JavaMail.weblogic@epmlwas06a>
Date:	Mon, 23 Nov 2015 06:44:08 +0000 (GMT)
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	최찬우 <cw00.choi@...sung.com>,
	박경민 <kyungmin.park@...sung.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
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.