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

Message ID 1248d141-88dc-6db0-90eb-dd31fd55fe81@linux.vnet.ibm.com
State New
Headers show
Series
  • Resend: [PATCH V5 2/4] pseries/drc-info: Search DRC properties for CPU indexes
Related show

Commit Message

Michael Bringmann Nov. 28, 2017, 11:07 p.m.
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@linux.vnet.ibm.com>
---
Changes in V5:
  -- Simplify of_prop_next_u32 invocation
  -- Remove unnecessary WARN_ON() tests
---
 arch/powerpc/include/asm/prom.h                 |   15 +++
 arch/powerpc/platforms/pseries/of_helpers.c     |   60 +++++++++++
 arch/powerpc/platforms/pseries/pseries_energy.c |  126 ++++++++++++++++++-----
 3 files changed, 173 insertions(+), 28 deletions(-)

Comments

Nathan Fontenot Nov. 30, 2017, 7:28 p.m. | #1
On 11/28/2017 05:07 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@linux.vnet.ibm.com>
> ---
> Changes in V5:
>   -- Simplify of_prop_next_u32 invocation
>   -- Remove unnecessary WARN_ON() tests

I'm not sure the WARN_ON()'s that you removed are unnecessary, I had just asked
that they get moved to read_drc_info_cell(). If you think they are not needed
perhaps making them pr_debug() instead.
 
> ---
>  arch/powerpc/include/asm/prom.h                 |   15 +++
>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 +++++++++++
>  arch/powerpc/platforms/pseries/pseries_energy.c |  126 ++++++++++++++++++-----
>  3 files changed, 173 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 3243455..0ef41b1 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -96,6 +96,21 @@ struct of_drconf_cell {
>  #define DRCONF_MEM_AI_INVALID	0x00000040
>  #define DRCONF_MEM_RESERVED	0x00000080
> 
> +struct of_drc_info {
> +	char *drc_type;
> +	char *drc_name_prefix;
> +	u32 drc_index_start;
> +	u32 drc_name_suffix_start;
> +	u32 num_sequential_elems;
> +	u32 sequential_inc;
> +	u32 drc_power_domain;
> +	u32 last_drc_index;
> +};
> +
> +extern int of_read_drc_info_cell(struct property **prop,
> +			const __be32 **curval, struct of_drc_info *data);
> +
> +
>  /*
>   * There are two methods for telling firmware what our capabilities are.
>   * Newer machines have an "ibm,client-architecture-support" method on the
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 7e75101..6df192f 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -3,6 +3,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <asm/prom.h>
> 
>  #include "of_helpers.h"
> 
> @@ -37,3 +38,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
>  		kfree(parent_path);
>  	return parent ? parent : ERR_PTR(-EINVAL);
>  }
> +
> +
> +/* Helper Routines to convert between drc_index to cpu numbers */
> +
> +int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
> +			struct of_drc_info *data)
> +{
> +	const char *p;
> +	const __be32 *p2;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	/* Get drc-type:encode-string */
> +	p = data->drc_type = (char*) (*curval);
> +	p = of_prop_next_string(*prop, p);
> +	if (!p)
> +		return -EINVAL;
> +
> +	/* Get drc-name-prefix:encode-string */
> +	data->drc_name_prefix = (char *)p;
> +	p = of_prop_next_string(*prop, p);
> +	if (!p)
> +		return -EINVAL;
> +
> +	/* Get drc-index-start:encode-int */
> +	p2 = (const __be32 *)p;
> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get drc-name-suffix-start:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get number-sequential-elements:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get sequential-increment:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get drc-power-domain:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Should now know end of current entry */
> +	(*curval) = (void *)p2;
> +	data->last_drc_index = data->drc_index_start +
> +		((data->num_sequential_elems - 1) * data->sequential_inc);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(of_read_drc_info_cell);
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 35c891a..f96677b 100644
> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> @@ -22,6 +22,7 @@
>  #include <asm/page.h>
>  #include <asm/hvcall.h>
>  #include <asm/firmware.h>
> +#include <asm/prom.h>
> 
> 
>  #define MODULE_VERS "1.0"
> @@ -38,26 +39,58 @@
>  static u32 cpu_to_drc_index(int cpu)
>  {
>  	struct device_node *dn = NULL;
> -	const int *indexes;
> -	int i;
> +	int thread_index;
>  	int rc = 1;
>  	u32 ret = 0;
> 
>  	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;
> +
>  	/* Convert logical cpu number to core number */
> -	i = cpu_core_index_of_thread(cpu);
> -	/*
> -	 * The first element indexes[0] is the number of drc_indexes
> -	 * returned in the list.  Hence i+1 will get the drc_index
> -	 * corresponding to core number i.
> -	 */
> -	WARN_ON(i > indexes[0]);
> -	ret = indexes[i + 1];
> +	thread_index = cpu_core_index_of_thread(cpu);
> +
> +	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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);

Shouldn't need to cast the return value to void *.

> +		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;
> +		}
> +
> +		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.
> +		 */
> +		ret = indexes[thread_index + 1];
> +	}
> +
>  	rc = 0;
> 
>  err_of_node_put:
> @@ -72,34 +105,71 @@ 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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);

Same here.

-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;
> +
> +			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 Bringmann Dec. 1, 2017, 9:53 p.m. | #2
See below.

On 11/30/2017 01:28 PM, Nathan Fontenot wrote:
> On 11/28/2017 05:07 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@linux.vnet.ibm.com>
>> ---
>> Changes in V5:
>>   -- Simplify of_prop_next_u32 invocation
>>   -- Remove unnecessary WARN_ON() tests
> 
> I'm not sure the WARN_ON()'s that you removed are unnecessary, I had just asked
> that they get moved to read_drc_info_cell(). If you think they are not needed
> perhaps making them pr_debug() instead.

I never saw this error.  I put these checks in more for my own debugging
of my code during bringup.  I feel safe in removing them from cpu_to_drc_index(),
and drc_index_to_cpu().

> 
>> ---
>>  arch/powerpc/include/asm/prom.h                 |   15 +++
>>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 +++++++++++
>>  arch/powerpc/platforms/pseries/pseries_energy.c |  126 ++++++++++++++++++-----
>>  3 files changed, 173 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index 3243455..0ef41b1 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -96,6 +96,21 @@ struct of_drconf_cell {
>>  #define DRCONF_MEM_AI_INVALID	0x00000040
>>  #define DRCONF_MEM_RESERVED	0x00000080
>>
>> +struct of_drc_info {
>> +	char *drc_type;
>> +	char *drc_name_prefix;
>> +	u32 drc_index_start;
>> +	u32 drc_name_suffix_start;
>> +	u32 num_sequential_elems;
>> +	u32 sequential_inc;
>> +	u32 drc_power_domain;
>> +	u32 last_drc_index;
>> +};
>> +
>> +extern int of_read_drc_info_cell(struct property **prop,
>> +			const __be32 **curval, struct of_drc_info *data);
>> +
>> +
>>  /*
>>   * There are two methods for telling firmware what our capabilities are.
>>   * Newer machines have an "ibm,client-architecture-support" method on the
>> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
>> index 7e75101..6df192f 100644
>> --- a/arch/powerpc/platforms/pseries/of_helpers.c
>> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
>> @@ -3,6 +3,7 @@
>>  #include <linux/err.h>
>>  #include <linux/slab.h>
>>  #include <linux/of.h>
>> +#include <asm/prom.h>
>>
>>  #include "of_helpers.h"
>>
>> @@ -37,3 +38,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
>>  		kfree(parent_path);
>>  	return parent ? parent : ERR_PTR(-EINVAL);
>>  }
>> +
>> +
>> +/* Helper Routines to convert between drc_index to cpu numbers */
>> +
>> +int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>> +			struct of_drc_info *data)
>> +{
>> +	const char *p;
>> +	const __be32 *p2;
>> +
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-type:encode-string */
>> +	p = data->drc_type = (char*) (*curval);
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-name-prefix:encode-string */
>> +	data->drc_name_prefix = (char *)p;
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-index-start:encode-int */
>> +	p2 = (const __be32 *)p;
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-name-suffix-start:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get number-sequential-elements:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get sequential-increment:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-power-domain:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Should now know end of current entry */
>> +	(*curval) = (void *)p2;
>> +	data->last_drc_index = data->drc_index_start +
>> +		((data->num_sequential_elems - 1) * data->sequential_inc);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(of_read_drc_info_cell);
>> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
>> index 35c891a..f96677b 100644
>> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
>> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
>> @@ -22,6 +22,7 @@
>>  #include <asm/page.h>
>>  #include <asm/hvcall.h>
>>  #include <asm/firmware.h>
>> +#include <asm/prom.h>
>>
>>
>>  #define MODULE_VERS "1.0"
>> @@ -38,26 +39,58 @@
>>  static u32 cpu_to_drc_index(int cpu)
>>  {
>>  	struct device_node *dn = NULL;
>> -	const int *indexes;
>> -	int i;
>> +	int thread_index;
>>  	int rc = 1;
>>  	u32 ret = 0;
>>
>>  	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;
>> +
>>  	/* Convert logical cpu number to core number */
>> -	i = cpu_core_index_of_thread(cpu);
>> -	/*
>> -	 * The first element indexes[0] is the number of drc_indexes
>> -	 * returned in the list.  Hence i+1 will get the drc_index
>> -	 * corresponding to core number i.
>> -	 */
>> -	WARN_ON(i > indexes[0]);
>> -	ret = indexes[i + 1];
>> +	thread_index = cpu_core_index_of_thread(cpu);
>> +
>> +	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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
> 
> Shouldn't need to cast the return value to void *.

Yes -- change applied.

> 
>> +		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;
>> +		}
>> +
>> +		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.
>> +		 */
>> +		ret = indexes[thread_index + 1];
>> +	}
>> +
>>  	rc = 0;
>>
>>  err_of_node_put:
>> @@ -72,34 +105,71 @@ 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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
> 
> Same here.

Yes, change applied.

> 
> -Nathan

Thanks.
Michael

> 
>> +		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 (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 Bringmann Dec. 4, 2017, 7:45 p.m. | #3
See below.

On 11/30/2017 01:28 PM, Nathan Fontenot wrote:
> On 11/28/2017 05:07 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@linux.vnet.ibm.com>
>> ---
>> Changes in V5:
>>   -- Simplify of_prop_next_u32 invocation
>>   -- Remove unnecessary WARN_ON() tests
> 
> I'm not sure the WARN_ON()'s that you removed are unnecessary, I had just asked
> that they get moved to read_drc_info_cell(). If you think they are not needed
> perhaps making them pr_debug() instead.

I removed them again, and their primary purpose was for debugging.  If we really
need to check the output at the calls, more precise checks could be targeted at
what we are debugging.

> 
>> ---
>>  arch/powerpc/include/asm/prom.h                 |   15 +++
>>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 +++++++++++
>>  arch/powerpc/platforms/pseries/pseries_energy.c |  126 ++++++++++++++++++-----
>>  3 files changed, 173 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index 3243455..0ef41b1 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -96,6 +96,21 @@ struct of_drconf_cell {
>>  #define DRCONF_MEM_AI_INVALID	0x00000040
>>  #define DRCONF_MEM_RESERVED	0x00000080
>>
>> +struct of_drc_info {
>> +	char *drc_type;
>> +	char *drc_name_prefix;
>> +	u32 drc_index_start;
>> +	u32 drc_name_suffix_start;
>> +	u32 num_sequential_elems;
>> +	u32 sequential_inc;
>> +	u32 drc_power_domain;
>> +	u32 last_drc_index;
>> +};
>> +
>> +extern int of_read_drc_info_cell(struct property **prop,
>> +			const __be32 **curval, struct of_drc_info *data);
>> +
>> +
>>  /*
>>   * There are two methods for telling firmware what our capabilities are.
>>   * Newer machines have an "ibm,client-architecture-support" method on the
>> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
>> index 7e75101..6df192f 100644
>> --- a/arch/powerpc/platforms/pseries/of_helpers.c
>> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
>> @@ -3,6 +3,7 @@
>>  #include <linux/err.h>
>>  #include <linux/slab.h>
>>  #include <linux/of.h>
>> +#include <asm/prom.h>
>>
>>  #include "of_helpers.h"
>>
>> @@ -37,3 +38,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
>>  		kfree(parent_path);
>>  	return parent ? parent : ERR_PTR(-EINVAL);
>>  }
>> +
>> +
>> +/* Helper Routines to convert between drc_index to cpu numbers */
>> +
>> +int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>> +			struct of_drc_info *data)
>> +{
>> +	const char *p;
>> +	const __be32 *p2;
>> +
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-type:encode-string */
>> +	p = data->drc_type = (char*) (*curval);
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-name-prefix:encode-string */
>> +	data->drc_name_prefix = (char *)p;
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-index-start:encode-int */
>> +	p2 = (const __be32 *)p;
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-name-suffix-start:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get number-sequential-elements:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get sequential-increment:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-power-domain:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Should now know end of current entry */
>> +	(*curval) = (void *)p2;
>> +	data->last_drc_index = data->drc_index_start +
>> +		((data->num_sequential_elems - 1) * data->sequential_inc);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(of_read_drc_info_cell);
>> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
>> index 35c891a..f96677b 100644
>> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
>> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
>> @@ -22,6 +22,7 @@
>>  #include <asm/page.h>
>>  #include <asm/hvcall.h>
>>  #include <asm/firmware.h>
>> +#include <asm/prom.h>
>>
>>
>>  #define MODULE_VERS "1.0"
>> @@ -38,26 +39,58 @@
>>  static u32 cpu_to_drc_index(int cpu)
>>  {
>>  	struct device_node *dn = NULL;
>> -	const int *indexes;
>> -	int i;
>> +	int thread_index;
>>  	int rc = 1;
>>  	u32 ret = 0;
>>
>>  	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;
>> +
>>  	/* Convert logical cpu number to core number */
>> -	i = cpu_core_index_of_thread(cpu);
>> -	/*
>> -	 * The first element indexes[0] is the number of drc_indexes
>> -	 * returned in the list.  Hence i+1 will get the drc_index
>> -	 * corresponding to core number i.
>> -	 */
>> -	WARN_ON(i > indexes[0]);
>> -	ret = indexes[i + 1];
>> +	thread_index = cpu_core_index_of_thread(cpu);
>> +
>> +	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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
> 
> Shouldn't need to cast the return value to void *.

Thanks.  Fixed.

>> +		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;
>> +		}
>> +
>> +		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.
>> +		 */
>> +		ret = indexes[thread_index + 1];
>> +	}
>> +
>>  	rc = 0;
>>
>>  err_of_node_put:
>> @@ -72,34 +105,71 @@ 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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
> 
> Same here.

Thanks.  Already saw this.

> 
> -Nathan

Michael

>> +		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 (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;
>>  }
>>
>>  /*
>>
> 
>

Patch

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 3243455..0ef41b1 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -96,6 +96,21 @@  struct of_drconf_cell {
 #define DRCONF_MEM_AI_INVALID	0x00000040
 #define DRCONF_MEM_RESERVED	0x00000080
 
+struct of_drc_info {
+	char *drc_type;
+	char *drc_name_prefix;
+	u32 drc_index_start;
+	u32 drc_name_suffix_start;
+	u32 num_sequential_elems;
+	u32 sequential_inc;
+	u32 drc_power_domain;
+	u32 last_drc_index;
+};
+
+extern int of_read_drc_info_cell(struct property **prop,
+			const __be32 **curval, struct of_drc_info *data);
+
+
 /*
  * There are two methods for telling firmware what our capabilities are.
  * Newer machines have an "ibm,client-architecture-support" method on the
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 7e75101..6df192f 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -3,6 +3,7 @@ 
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <asm/prom.h>
 
 #include "of_helpers.h"
 
@@ -37,3 +38,62 @@  struct device_node *pseries_of_derive_parent(const char *path)
 		kfree(parent_path);
 	return parent ? parent : ERR_PTR(-EINVAL);
 }
+
+
+/* Helper Routines to convert between drc_index to cpu numbers */
+
+int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
+			struct of_drc_info *data)
+{
+	const char *p;
+	const __be32 *p2;
+
+	if (!data)
+		return -EINVAL;
+
+	/* Get drc-type:encode-string */
+	p = data->drc_type = (char*) (*curval);
+	p = of_prop_next_string(*prop, p);
+	if (!p)
+		return -EINVAL;
+
+	/* Get drc-name-prefix:encode-string */
+	data->drc_name_prefix = (char *)p;
+	p = of_prop_next_string(*prop, p);
+	if (!p)
+		return -EINVAL;
+
+	/* Get drc-index-start:encode-int */
+	p2 = (const __be32 *)p;
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get drc-name-suffix-start:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get number-sequential-elements:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get sequential-increment:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get drc-power-domain:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
+	if (!p2)
+		return -EINVAL;
+
+	/* Should now know end of current entry */
+	(*curval) = (void *)p2;
+	data->last_drc_index = data->drc_index_start +
+		((data->num_sequential_elems - 1) * data->sequential_inc);
+
+	return 0;
+}
+EXPORT_SYMBOL(of_read_drc_info_cell);
diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index 35c891a..f96677b 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -22,6 +22,7 @@ 
 #include <asm/page.h>
 #include <asm/hvcall.h>
 #include <asm/firmware.h>
+#include <asm/prom.h>
 
 
 #define MODULE_VERS "1.0"
@@ -38,26 +39,58 @@ 
 static u32 cpu_to_drc_index(int cpu)
 {
 	struct device_node *dn = NULL;
-	const int *indexes;
-	int i;
+	int thread_index;
 	int rc = 1;
 	u32 ret = 0;
 
 	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;
+
 	/* Convert logical cpu number to core number */
-	i = cpu_core_index_of_thread(cpu);
-	/*
-	 * The first element indexes[0] is the number of drc_indexes
-	 * returned in the list.  Hence i+1 will get the drc_index
-	 * corresponding to core number i.
-	 */
-	WARN_ON(i > indexes[0]);
-	ret = indexes[i + 1];
+	thread_index = cpu_core_index_of_thread(cpu);
+
+	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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
+		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;
+		}
+
+		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.
+		 */
+		ret = indexes[thread_index + 1];
+	}
+
 	rc = 0;
 
 err_of_node_put:
@@ -72,34 +105,71 @@  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 = (void *)of_prop_next_u32(info, NULL, &num_set_entries);
+		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 (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;
 }
 
 /*