Date:	Wed, 20 Jun 2012 10:43:34 +0200
From:	Vincent Guittot <vincent.guittot@...aro.org>
To:	Namhyung Kim <namhyung@...nel.org>
Cc:	Vincent Guittot 
	<public-vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@...ne.gmane.org>,
	public-a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw@...ne.gmane.org,
	public-linux-kernel-u79uwXL29TY76Z2rM5mHXA@...ne.gmane.org,
	public-rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@...ne.gmane.org,
	public-linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@...ne.gmane.org,
	public-linaro-dev-cunTk1MwBs8s++Sfvej+rw@...ne.gmane.org,
	public-devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@...ne.gmane.org,
	public-linux-lFZ/pmaqli7XmaaqVzeoHQ@...ne.gmane.org
Subject: Re: [PATCH v2 3/5] ARM: topology: Update cpu_power according to DT information



On 20 June 2012 03:15, Namhyung Kim <namhyung@...nel.org> wrote:
>
>
>
> On Tue, 19 Jun 2012 10:28:54 +0200, Vincent Guittot wrote:
>> Use cpu compatibility field and clock-frequency field of DT to
>> estimate the capacity of each core of the system and to update
>> the cpu_power field accordingly.
>> This patch enables to put more running tasks on big cores than
>> on LITTLE ones. But this patch doesn't ensure that long running
>> tasks will run on big cores and short ones on LITTLE cores.
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@...lic.gmane.org>
>> ---
> [snip]
>> +static void __init parse_dt_topology(void)
>> +{
>> +     struct cpu_efficiency *cpu_eff;
>> +     struct device_node *cn = NULL;
>> +     unsigned long min_capacity = (unsigned long)(-1);
>> +     unsigned long max_capacity = 0;
>> +     unsigned long capacity = 0;
>> +     int alloc_size, cpu = 0;
>> +
>> +     alloc_size = nr_cpu_ids * sizeof(struct cpu_capacity);
>> +     cpu_capacity = (struct cpu_capacity *)kzalloc(alloc_size, GFP_NOWAIT);
>> +
>> +     while ((cn = of_find_node_by_type(cn, "cpu"))) {
>> +             const u32 *rate, *reg;
>> +             char *compatible;
>> +             int len;
>> +
>> +             if (cpu >= num_possible_cpus())
>> +                     break;
>> +
>> +             compatible = of_get_property(cn, "compatible", &len);
>
> Why is this line needed?

not needed is the final version, should have been removed.

>
>
>> +
>> +             for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
>> +                     if (of_device_is_compatible(cn, cpu_eff->compatible))
>> +                             break;
>> +
>> +             if (cpu_eff->compatible == NULL)
>> +                     continue;
>> +
>> +             rate = of_get_property(cn, "clock-frequency", &len);
>> +             if (!rate || len != 4) {
>> +                     pr_err("%s missing clock-frequency property\n",
>> +                             cn->full_name);
>> +                     continue;
>> +             }
>> +
>> +             reg = of_get_property(cn, "reg", &len);
>> +             if (!reg || len != 4) {
>> +                     pr_err("%s missing reg property\n", cn->full_name);
>> +                     continue;
>> +             }
>> +
>> +             capacity = ((be32_to_cpup(rate)) >> 20)
>> +                     * cpu_eff->efficiency;
>
> Why did you break this line?

It was more than 80 chars large previously but no more the case.
I'm going to correct

Thanks
>
> Thanks,
> Namhyung
>
>
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@...ts.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists