On Mon, May 21, 2018 at 02:05:41PM +0300, ilialin at codeaurora.org wrote:

You are right.
cpu_dev_silver != cpu_dev_gold, and I found this with my tests as well.
Thank you.

quoted

-----Original Message-----
From: Russell King - ARM Linux <linux@armlinux.org.uk>
Sent: Monday, May 21, 2018 13:54
To: Ilia Lin <redacted>
Cc: viresh.kumar at linaro.org; devicetree at vger.kernel.org; linux-
pm at vger.kernel.org; linux-arm-msm at vger.kernel.org; linux-
kernel at vger.kernel.org; linux-soc at vger.kernel.org; linux-
clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] cpufreq: Add Kryo CPU scaling driver

On Mon, May 21, 2018 at 01:31:30PM +0300, Ilia Lin wrote:

quoted

+#define SILVER_LEAD	0
+#define GOLD_LEAD	2

Okay, two different values here, but "GOLD_LEAD" appears unused.

quoted

+	cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_silver)
+		return -ENODEV;
+
+	cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+	if (NULL == cpu_dev_gold)
+		return -ENODEV;

get_cpu_device() takes the logical CPU number.  So the above gets CPU 0
each time, and so cpu_dev_silver == cpu_dev_gold here.  So what's the
point of the second get_cpu_device() ?  If it's supposed to be:

	cpu_dev_gold = get_cpu_device(GOLD_LEAD);

That would get CPU 2, but in terms of these defines, it doesn't make that
much sense.  What exactly does "silver lead" and "gold lead" refer to in

these

quoted

definitions?

I think you still need to explain this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up