Re: [RFC] CPUFreq PowerOP integration, Centrino PM Core and OPs registration 2/3

[Dominik Brodowski <linux@dominikbrodowski.net>]

Hi,

On Thu, Sep 14, 2006 at 06:50:02PM +0400, Eugeny S. Mints wrote:

> +static int 
> +process_pwr_param(struct pm_core_point *opt, int op, char *param_name,
> +		  int va_arg)
> +{
> +	int cpu = 0;
> +	char buf[8];
> +
> +	for (cpu = 0; cpu < NR_CPUS; cpu++)
> +	{
> +		sprintf(buf, "v%d", cpu);
> +
> +		if (strcmp(param_name, buf) == 0) {
> +			if (op == PWR_PARAM_SET)
> +				opt->opt[cpu].pwpr[_I386_PM_CORE_POINT_V] = 
> +									va_arg;
> +			else if (opt != NULL)
> +				*(int *)va_arg = 
> +				     opt->opt[cpu].pwpr[_I386_PM_CORE_POINT_V];
> +			else if ((*(int *)va_arg = get_vtg(cpu)) <= 0)
> +				return -EINVAL;
> +			return 0;
> +		}
> +
> +		sprintf(buf, "freq%d", cpu);
> +
> +		if (strcmp(param_name, buf) == 0) {
> +			if (op == PWR_PARAM_SET)
> +				opt->opt[cpu].pwpr[_I386_PM_CORE_POINT_FREQ] = 
> +									va_arg;
> +			else if (opt != NULL)
> +				*(int *)va_arg = 
> +				  opt->opt[cpu].pwpr[_I386_PM_CORE_POINT_FREQ];
> +			else if ((*(int *)va_arg = get_freq(cpu)) <= 0)
> +				return -EINVAL;
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}

Ouch. IIRC Pavel had some fine comments about such string parsing deep in
arch code... Other than that I see lots of indirection, lots of code being
added (~400 lines) for no gain in functionality for the x86 case.

Thanks,
	Dominik