RESEND [PATCH V3 2/4] pseries/drc-info: Search DRC properties for CPU indexes
Nathan Fontenot
nfont at linux.vnet.ibm.com
Fri Nov 17 04:34:45 AEDT 2017
On 11/15/2017 12:09 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 V3:
> -- Some code compression and use of data structures for value passing.
> ---
> arch/powerpc/include/asm/prom.h | 15 ++
> arch/powerpc/platforms/pseries/of_helpers.c | 60 ++++++++++
> arch/powerpc/platforms/pseries/pseries_energy.c | 139 ++++++++++++++++++-----
> 3 files changed, 186 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 3243455..007430a 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_one_drc_info(struct property **prop, void **curval,
> + struct of_drc_info *data);
I'm not sure if prom.h is where this really belongs but I also do
not see an existing header file that it really makes sense to put it in.
> +
> +
> /*
> * 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 2798933..62dc8e9 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -2,6 +2,7 @@
> #include <linux/err.h>
> #include <linux/slab.h>
> #include <linux/of.h>
> +#include <asm/prom.h>
>
> #include "of_helpers.h"
>
> @@ -36,3 +37,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_one_drc_info(struct property **prop, void **curval,
> + struct of_drc_info *data)
Small nit, this should probably be of_read_drc_info_cell.
> +{
> + const char *p;
> + const __be32 *p2;
> +
> + if (!data)
> + return -EINVAL;
> +
> + /* Get drc-type:encode-string */
> + p = data->drc_type = (*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/skip drc-name-suffix-start:encode-int */
You're getting the suffix, should probably drop 'skip' in the comment.
> + 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/skip drc-power-domain:encode-int */
Same here.
> + 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_one_drc_info);
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 35c891a..7160855 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,65 @@
> 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;
No need to set this to NULL here.
> + struct of_drc_info drc;
> + int j;
> + u32 num_set_entries;
> + void *value;
For device tree values, which are BE by design, this would be better as
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);
> + if (!value)
> + goto err_of_node_put;
> +
> + for (j = 0; j < num_set_entries; j++) {
> +
> + of_one_drc_info(&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)%
Spaces around the '-' sign please.
> + drc.sequential_inc) != 0);
> + }
> + WARN_ON((drc.num_sequential_elems == 0) ||
> + (drc.sequential_inc == 0));
This warning seems like it would fit better in the routine that reads the
drc-info property values.
> +
> + ret = drc.drc_index_start + (thread_index*drc.sequential_inc);
Spaces... '*'
> + } 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 +112,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;
> + void *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);
> + if (!value)
> + goto err_of_node_put;
> +
> + for (j = 0; j < num_set_entries; j++) {
> +
> + of_one_drc_info(&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;
> + } else {
Since you do a continue in the if() part above you shouldn't need to
put this in an else block.
-Nathan
> + 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;
> }
>
> /*
>
More information about the Linuxppc-dev
mailing list