Re: [RFC PATCH v2 2/3] CPPC as a PID controller backend

[Dirk Brandewie <dirk.brandewie@gmail.com>]

On 10/08/2014 01:11 PM, Ashwin Chaugule wrote:

> +static int __init acpi_pid_init(void)
> +{
> +	int cpu, rc = 0;
> +

You should add a check here to not bind to Intel CPU. The CPPC interface
was created to provided an ACPI interface to to hardware controlled P states
(HWP) described in Volume 3 section 14.4 of the Intel SDM.
intel_pstate will be enabling HWP by controlling the MSRs directly and
not using CPPC.

Adding this check will keep us from having to fight load order since
this driver and intel_pstate are at the same init level.

> +	rc = acpi_cppc_init();
> +
> +	if (rc)
> +		return -ENODEV;
> +
> +	pr_info("ACPI PID driver initializing.\n");
> +
> +	all_cpu_data = vzalloc(sizeof(void *) * num_possible_cpus());
> +	if (!all_cpu_data)
> +		return -ENOMEM;
> +
> +	rc = cpufreq_register_driver(&acpi_pid_driver);
> +	if (rc)
> +		goto out;
> +
> +	acpi_pid_debug_expose_params();
> +	acpi_pid_sysfs_expose_params();
> +
> +	return rc;
> +out:
> +	get_online_cpus();
> +	for_each_online_cpu(cpu) {
> +		if (all_cpu_data[cpu]) {
> +			del_timer_sync(&all_cpu_data[cpu]->timer);
> +			kfree(all_cpu_data[cpu]);
> +		}
> +	}
> +
> +	put_online_cpus();
> +	vfree(all_cpu_data);
> +	return -ENODEV;
> +}
> +
> +late_initcall(acpi_pid_init);
>