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

["Eugeny S. Mints" <eugeny.mints@gmail.com>]

Dominik Brodowski wrote:
> 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.
THis code is no longer anyhow relevant to the latest take of PowerOP. Please 
check to make sure that you are looking at the very latest PowerOP code 
submitted for discussion. Latest PowerOP take gets rid of all parsing. Latest 
patches do not contain x86 patch yet but omap PM core reference code is 
supplied.  So please use  omap code for the discussion. FYI, forward porting of 
x86 patches to the latest POwerOP is on the way.

Thanks,
	Eugeny
> 
> Thanks,
> 	Dominik
>