[PATCH V4 2/4] pseries/drc-info: Search DRC properties for CPU indexes

[Michael Bringmann]

See below.

On 11/20/2017 09:35 AM, Nathan Fontenot wrote:
> On 11/16/2017 02:11 PM, Michael Bringmann wrote:
>> pseries/drc-info: Provide parallel routines to convert between
>> drc_index and CPU numbers at runtime, using the older device-tree
>> properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
>> and "ibm,drc-power-domains"), or the new property "ibm,drc-info".
>>
>> Signed-off-by: Michael Bringmann <mwb at linux.vnet.ibm.com>
>> ---
>> Changes in V4:
>>   -- Rename of_one_drc_info to of_read_drc_info_cell
>>   -- Fix some spacing within expressions
>>   -- Make some style corrections
>> ---
>>  arch/powerpc/include/asm/prom.h                 |   15 +++
>>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 ++++++++++
>>  arch/powerpc/platforms/pseries/pseries_energy.c |  138 ++++++++++++++++++-----
>>  3 files changed, 185 insertions(+), 28 deletions(-)
>>

...

>> +
>> +		value = info->value;
>> +		value = (void *)of_prop_next_u32(info, value,
>> +						&num_set_entries);
> 
> Missed this the first time, but setting value twice seems a bit odd.
> This should be able to be collapsed into;
> 
> 		value = (void *)of_prop_next_u32(info, NULL, &num_set_entries);

Okay
> 
>> +		if (!value)
>> +			goto err_of_node_put;
>> +
>> +		for (j = 0; j < num_set_entries; j++) {
>> +
>> +			of_read_drc_info_cell(&info, &value, &drc);
>> +			if (strncmp(drc.drc_type, "CPU", 3))
>> +				goto err;
>> +
>> +			if (thread_index < drc.last_drc_index)
>> +				break;
>> +
>> +			WARN_ON(((thread_index - drc.drc_index_start) %
>> +					drc.sequential_inc) != 0);
> 
> The WARN_ON that you have here, and in the code below, really fell like they
> should be in of_read_drc_info_cell. These are checks on reading the info in
> the drc-info property.

Remove unnecessary WARN_ON() checks.

> 
>> +		}
>> +
>> +		ret = drc.drc_index_start + (thread_index * drc.sequential_inc);
>> +	} else {
>> +		const __be32 *indexes;
>> +
>> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> +		if (indexes == NULL)
>> +			goto err_of_node_put;
>> +
>> +		/*
>> +		 * The first element indexes[0] is the number of drc_indexes
>> +		 * returned in the list.  Hence thread_index+1 will get the
>> +		 * drc_index corresponding to core number thread_index.
>> +		 */
>> +		WARN_ON(thread_index > indexes[0]);
>> +		ret = indexes[thread_index + 1];
>> +	}
>> +
>>  	rc = 0;
>>
>>  err_of_node_put:
>> @@ -72,34 +111,77 @@ static int drc_index_to_cpu(u32 drc_index)
>>  {
>>  	struct device_node *dn = NULL;
>>  	const int *indexes;
>> -	int i, cpu = 0;
>> +	int thread_index = 0, cpu = 0;
>>  	int rc = 1;
>>
>>  	dn = of_find_node_by_path("/cpus");
>>  	if (dn == NULL)
>>  		goto err;
>> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> -	if (indexes == NULL)
>> -		goto err_of_node_put;
>> -	/*
>> -	 * First element in the array is the number of drc_indexes
>> -	 * returned.  Search through the list to find the matching
>> -	 * drc_index and get the core number
>> -	 */
>> -	for (i = 0; i < indexes[0]; i++) {
>> -		if (indexes[i + 1] == drc_index)
>> +
>> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
>> +		struct property *info = NULL;
>> +		struct of_drc_info drc;
>> +		int j;
>> +		u32 num_set_entries;
>> +		const __be32 *value;
>> +
>> +		info = of_find_property(dn, "ibm,drc-info", NULL);
>> +		if (info == NULL)
>> +			goto err_of_node_put;
>> +
>> +		value = info->value;
>> +		value = (void *)of_prop_next_u32(info, value,
>> +						&num_set_entries);

Okay.

> 
> Same here as mentioned above.
> 
> -Nathan
> 
>> +		if (!value)
>> +			goto err_of_node_put;
>> +
>> +		for (j = 0; j < num_set_entries; j++) {
>> +
>> +			of_read_drc_info_cell(&info, &value, &drc);
>> +			if (strncmp(drc.drc_type, "CPU", 3))
>> +				goto err;
>> +
>> +			WARN_ON(drc_index < drc.drc_index_start);
>> +			WARN_ON(((drc_index - drc.drc_index_start) %
>> +					drc.sequential_inc) != 0);
>> +
>> +			if (drc_index > drc.last_drc_index) {
>> +				cpu += drc.num_sequential_elems;
>> +				continue;
>> +			}
>> +			cpu += ((drc_index - drc.drc_index_start) /
>> +				drc.sequential_inc);
>> +
>> +			thread_index = cpu_first_thread_of_core(cpu);
>> +			rc = 0;
>>  			break;
>> +		}
>> +	} else {
>> +		unsigned long int i;
>> +
>> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> +		if (indexes == NULL)
>> +			goto err_of_node_put;
>> +		/*
>> +		 * First element in the array is the number of drc_indexes
>> +		 * returned.  Search through the list to find the matching
>> +		 * drc_index and get the core number
>> +		 */
>> +		for (i = 0; i < indexes[0]; i++) {
>> +			if (indexes[i + 1] == drc_index)
>> +				break;
>> +		}
>> +		/* Convert core number to logical cpu number */
>> +		thread_index = cpu_first_thread_of_core(i);
>> +		rc = 0;
>>  	}
>> -	/* Convert core number to logical cpu number */
>> -	cpu = cpu_first_thread_of_core(i);
>> -	rc = 0;
>>
>>  err_of_node_put:
>>  	of_node_put(dn);
>>  err:
>>  	if (rc)
>>  		printk(KERN_WARNING "drc_index_to_cpu(%d) failed", drc_index);
>> -	return cpu;
>> +	return thread_index;
>>  }
>>
>>  /*
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb at linux.vnet.ibm.com