The implementation of the pseries-specific drc info information feature
is currently implemented within and outside of the powerpc pseries
code. This patch set moves the parsing code for the pseries-specific
device-tree properties ibm,drc-indexes, ibm,drc-names, ibm,drc-types,
ibm,drc-power-domains, and the compressed ibm,drc-info into the
platform-specific code for the Pseries features.
Signed-off-by: Michael W. Bringmann <[email protected]>
Michael Bringmann (6):
powerpc:/drc Define interface to acquire arch-specific drc info
pseries/drcinfo: Fix bug parsing ibm,drc-info
pseries/drcinfo: Pseries impl of arch_find_drc_info
powerpc/pseries: Use common drcinfo parsing
powerpc/pci/hotplug: Use common drcinfo parsing
powerpc: Enable support for ibm,drc-info devtree property
Define interface to acquire arch-specific drc info to match against
hotpluggable devices. The current implementation exposes several
pseries-specific dynamic memory properties in generic kernel code.
This patch set provides an interface to pull that code out of the
generic kernel.
Signed-off-by: Michael Bringmann <[email protected]>
---
include/linux/topology.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/topology.h b/include/linux/topology.h
index cb0775e..df97f5f 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -44,6 +44,15 @@
int arch_update_cpu_topology(void);
+int arch_find_drc_match(struct device_node *dn,
+ bool (*usercb)(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data),
+ char *opt_drc_type, char *opt_drc_name,
+ bool match_drc_index, bool ck_php_type,
+ void *data);
+
/* Conform to ACPI 2.0 SLIT distance definitions */
#define LOCAL_DISTANCE 10
#define REMOTE_DISTANCE 20
Replace use of of_prop_next_u32() in when parsing 'ibm,drc-info'
structure to simplify and reduce parsing code.
Signed-off-by: Michael Bringmann <[email protected]>
Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmware feature" -- end of patch series applied to powerpc next)
---
arch/powerpc/platforms/pseries/of_helpers.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 6df192f..0185e50 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+
#include <linux/string.h>
#include <linux/err.h>
#include <linux/slab.h>
@@ -6,6 +7,9 @@
#include <asm/prom.h>
#include "of_helpers.h"
+#include "pseries.h"
+
+#define MAX_DRC_NAME_LEN 64
/**
* pseries_of_derive_parent - basically like dirname(1)
@@ -65,29 +69,19 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
/* 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;
+ data->drc_index_start = of_read_number(p2++, 1);
/* Get drc-name-suffix-start:encode-int */
- p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
- if (!p2)
- return -EINVAL;
+ data->drc_name_suffix_start = of_read_number(p2++, 1);
/* Get number-sequential-elements:encode-int */
- p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
- if (!p2)
- return -EINVAL;
+ data->num_sequential_elems = of_read_number(p2++, 1);
/* Get sequential-increment:encode-int */
- p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
- if (!p2)
- return -EINVAL;
+ data->sequential_inc = of_read_number(p2++, 1);
/* Get drc-power-domain:encode-int */
- p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
- if (!p2)
- return -EINVAL;
+ data->drc_power_domain = of_read_number(p2++, 1);
/* Should now know end of current entry */
(*curval) = (void *)p2;
This patch provides a common interface to parse ibm,drc-indexes,
ibm,drc-names, ibm,drc-types, ibm,drc-power-domains, or ibm,drc-info.
The generic interface arch_find_drc_match is provided which accepts
callback functions that may be applied to examine the data for each
entry.
Signed-off-by: Michael Bringmann <[email protected]>
---
arch/powerpc/include/asm/prom.h | 3
arch/powerpc/platforms/pseries/of_helpers.c | 299 +++++++++++++++++++++++++++
include/linux/topology.h | 2
3 files changed, 298 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index b04c5ce..910d1dc 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -91,9 +91,6 @@ struct of_drc_info {
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.
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 0185e50..11c90cd 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) "drc: " fmt
+
#include <linux/string.h>
#include <linux/err.h>
#include <linux/slab.h>
@@ -11,6 +13,12 @@
#define MAX_DRC_NAME_LEN 64
+static int drc_debug;
+#define dbg(args...) if (drc_debug) { printk(KERN_DEBUG args); }
+#define err(arg...) printk(KERN_ERR args);
+#define info(arg...) printk(KERN_INFO args);
+#define warn(arg...) printk(KERN_WARNING args);
+
/**
* pseries_of_derive_parent - basically like dirname(1)
* @path: the full_name of a node to be added to the tree
@@ -46,7 +54,8 @@ struct device_node *pseries_of_derive_parent(const char *path)
/* Helper Routines to convert between drc_index to cpu numbers */
-int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
+static int of_read_drc_info_cell(struct property **prop,
+ const __be32 **curval,
struct of_drc_info *data)
{
const char *p;
@@ -90,4 +99,290 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
return 0;
}
-EXPORT_SYMBOL(of_read_drc_info_cell);
+
+static int walk_drc_info(struct device_node *dn,
+ bool (*usercb)(struct of_drc_info *drc,
+ void *data,
+ int *ret_code),
+ char *opt_drc_type,
+ void *data)
+{
+ struct property *info;
+ unsigned int entries;
+ struct of_drc_info drc;
+ const __be32 *value;
+ int j, ret_code = -EINVAL;
+ bool done = false;
+
+ info = of_find_property(dn, "ibm,drc-info", NULL);
+ if (info == NULL)
+ return -EINVAL;
+
+ value = info->value;
+ entries = of_read_number(value++, 1);
+
+ for (j = 0, done = 0; (j < entries) && (!done); j++) {
+ of_read_drc_info_cell(&info, &value, &drc);
+
+ if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
+ continue;
+
+ done = usercb(&drc, data, &ret_code);
+ }
+
+ return ret_code;
+}
+
+static int get_children_props(struct device_node *dn, const int **drc_indexes,
+ const int **drc_names, const int **drc_types,
+ const int **drc_power_domains)
+{
+ const int *indexes, *names, *types, *domains;
+
+ indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
+ names = of_get_property(dn, "ibm,drc-names", NULL);
+ types = of_get_property(dn, "ibm,drc-types", NULL);
+ domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
+
+ if (!indexes || !names || !types || !domains) {
+ /* Slot does not have dynamically-removable children */
+ return -EINVAL;
+ }
+ if (drc_indexes)
+ *drc_indexes = indexes;
+ if (drc_names)
+ /* &drc_names[1] contains NULL terminated slot names */
+ *drc_names = names;
+ if (drc_types)
+ /* &drc_types[1] contains NULL terminated slot types */
+ *drc_types = types;
+ if (drc_power_domains)
+ *drc_power_domains = domains;
+
+ return 0;
+}
+
+static int is_php_type(char *drc_type)
+{
+ unsigned long value;
+ char *endptr;
+
+ /* PCI Hotplug nodes have an integer for drc_type */
+ value = simple_strtoul(drc_type, &endptr, 10);
+ if (endptr == drc_type)
+ return 0;
+
+ return 1;
+}
+
+/**
+ * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
+ * @dn: target &device_node
+ * @indexes: passed to get_children_props()
+ * @names: passed to get_children_props()
+ * @types: returned from get_children_props()
+ * @power_domains:
+ *
+ * This routine will return true only if the device node is
+ * a hotpluggable slot. This routine will return false
+ * for built-in pci slots (even when the built-in slots are
+ * dlparable.)
+ */
+static int is_php_dn(struct device_node *dn, const int **indexes,
+ const int **names, const int **types,
+ const int **power_domains)
+{
+ const int *drc_types;
+ int rc;
+
+ rc = get_children_props(dn, indexes, names, &drc_types,
+ power_domains);
+ if (rc < 0)
+ return 0;
+
+ if (!is_php_type((char *) &drc_types[1]))
+ return 0;
+
+ *types = drc_types;
+ return 1;
+}
+
+struct find_drc_match_cb_struct {
+ struct device_node *dn;
+ bool (*usercb)(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data);
+ char *drc_type;
+ char *drc_name;
+ u32 drc_index;
+ bool match_drc_index;
+ bool add_slot;
+ void *data;
+};
+
+static int find_drc_match_v1(struct device_node *dn, void *data)
+{
+ struct find_drc_match_cb_struct *cdata = data;
+ int i, retval = 0;
+ const int *indexes, *names, *types, *domains;
+ char *name, *type;
+ struct device_node *root = dn;
+
+ if (cdata->match_drc_index)
+ root = dn->parent;
+
+ if (cdata->add_slot) {
+ /* If this is not a hotplug slot, return without doing
+ * anything.
+ */
+ if (!is_php_dn(root, &indexes, &names, &types, &domains))
+ return 0;
+ } else {
+ if (get_children_props(root, &indexes, &names, &types,
+ &domains) < 0)
+ return 0;
+ }
+
+ dbg("Entry %s: dn=%pOF\n", __func__, dn);
+
+ name = (char *) &names[1];
+ type = (char *) &types[1];
+ for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
+
+ if (cdata->match_drc_index &&
+ ((unsigned int) indexes[i + 1] != cdata->drc_index)) {
+ name += strlen(name) + 1;
+ type += strlen(type) + 1;
+ continue;
+ }
+
+ if (((cdata->drc_name == NULL) ||
+ (cdata->drc_name && !strcmp(cdata->drc_name, name))) &&
+ ((cdata->drc_type == NULL) ||
+ (cdata->drc_type && !strcmp(cdata->drc_type, type)))) {
+
+ if (cdata->usercb) {
+ retval = cdata->usercb(dn,
+ be32_to_cpu(indexes[i + 1]),
+ name, type,
+ be32_to_cpu(domains[i + 1]),
+ cdata->data);
+ if (!retval)
+ return retval;
+ } else {
+ return 0;
+ }
+ }
+
+ name += strlen(name) + 1;
+ type += strlen(type) + 1;
+ }
+
+ dbg("%s - Exit: rc[%d]\n", __func__, retval);
+
+ /* XXX FIXME: reports a failure only if last entry in loop failed */
+ return retval;
+}
+
+static bool find_drc_match_v2_cb(struct of_drc_info *drc, void *data,
+ int *ret_code)
+{
+ struct find_drc_match_cb_struct *cdata = data;
+ u32 drc_index;
+ char drc_name[MAX_DRC_NAME_LEN];
+ int i, retval;
+
+ (*ret_code) = -EINVAL;
+
+ /* This set not a PHP type? */
+ if (cdata->add_slot) {
+ if (!is_php_type((char *) drc->drc_type)) {
+ return false;
+ }
+ }
+
+ /* Anything to use from this set? */
+ if (cdata->match_drc_index && (cdata->drc_index > drc->last_drc_index))
+ return false;
+ if ((cdata->drc_type && strcmp(cdata->drc_type, drc->drc_type))
+ return false;
+
+ /* Check the drc-index entries of this set */
+ for (i = 0, drc_index = drc->drc_index_start;
+ i < drc->num_sequential_elems; i++, drc_index++) {
+
+ if (cdata->match_drc_index && (cdata->drc_index != drc_index))
+ continue;
+
+ sprintf(drc_name, "%s%d", drc->drc_name_prefix,
+ drc_index - drc->drc_index_start +
+ drc->drc_name_suffix_start);
+
+ if ((cdata->drc_name == NULL) ||
+ (cdata->drc_name && !strcmp(cdata->drc_name, drc_name))) {
+
+ if (cdata->usercb) {
+ retval = cdata->usercb(cdata->dn,
+ drc_index, drc_name,
+ drc->drc_type,
+ drc->drc_power_domain,
+ cdata->data);
+ if (!retval) {
+ (*ret_code) = retval;
+ return true;
+ }
+ } else {
+ (*ret_code) = 0;
+ return true;
+ }
+ }
+ }
+
+ (*ret_code) = retval;
+ return false;
+}
+
+static int find_drc_match_v2(struct device_node *dn, void *data)
+{
+ struct find_drc_match_cb_struct *cdata = data;
+ struct device_node *root = cdata->dn;
+
+ if (!cdata->add_slot) {
+ if (!cdata->drc_type ||
+ (cdata->drc_type && strcmp(cdata->drc_type, "SLOT")))
+ root = dn->parent;
+ }
+
+ return walk_drc_info(root, find_drc_match_v2_cb, NULL, data);
+}
+
+int arch_find_drc_match(struct device_node *dn,
+ bool (*usercb)(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data),
+ char *opt_drc_type, char *opt_drc_name,
+ bool match_drc_index, bool add_slot,
+ void *data)
+{
+ struct find_drc_match_cb_struct cdata = { dn, usercb,
+ opt_drc_type, opt_drc_name, 0,
+ match_drc_index, add_slot, data };
+
+ if (match_drc_index) {
+ const int *my_index =
+ of_get_property(dn, "ibm,my-drc-index", NULL);
+ if (!my_index) {
+ /* Node isn't DLPAR/hotplug capable */
+ return -EINVAL;
+ }
+ cdata.drc_index = *my_index;
+ }
+
+ if (firmware_has_feature(FW_FEATURE_DRC_INFO))
+ return find_drc_match_v2(dn, &cdata);
+ else
+ return find_drc_match_v1(dn, &cdata);
+}
+EXPORT_SYMBOL(arch_find_drc_match);
diff --git a/include/linux/topology.h b/include/linux/topology.h
index df97f5f..c3dfa53 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -50,7 +50,7 @@ int arch_find_drc_match(struct device_node *dn,
char *drc_type, u32 drc_power_domain,
void *data),
char *opt_drc_type, char *opt_drc_name,
- bool match_drc_index, bool ck_php_type,
+ bool match_drc_index, bool add_slot,
void *data);
/* Conform to ACPI 2.0 SLIT distance definitions */
The implementation of the pseries-specific drc info properties
is currently implemented in pseries-specific and non-pseries-specific
files. This patch set uses a new implementation of the device-tree
parsing code for the properties.
This patch refactors parsing of the drc-info properties out of
pseries_energy.c and hotplug-cpu.c to use the common parser.
Changes include creating appropriate callback functions and
passing callback-specific data blocks into arch_find_drc_match.
Signed-off-by: Michael Bringmann <[email protected]>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 83 +++++++-----
arch/powerpc/platforms/pseries/pseries_energy.c | 157 ++++++++---------------
2 files changed, 107 insertions(+), 133 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 2f8e621..ee3028c 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -411,23 +411,29 @@ static bool dlpar_cpu_exists(struct device_node *parent, u32 drc_index)
return found;
}
-static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
+struct cpu_drc_index_struct {
+ u32 drc_index;
+};
+
+bool cpu_drc_index_cb(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data)
{
- bool found = false;
- int rc, index;
+ struct cpu_drc_index_struct *cdata = data;
- index = 0;
- while (!found) {
- u32 drc;
+ if (drc_index == cdata->drc_index)
+ return true;
+ return false;
+}
- rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
- index++, &drc);
- if (rc)
- break;
+static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
+{
+ struct cpu_drc_index_struct cdata = { drc_index };
+ bool found = false;
- if (drc == drc_index)
- found = true;
- }
+ found = arch_find_drc_match(parent, cpu_drc_index_cb,
+ "CPU", NULL, false, false, &cdata);
return found;
}
@@ -721,11 +727,34 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
return rc;
}
+struct cpus_to_add_cb_struct {
+ struct device_node *parent;
+ u32 *cpu_drcs;
+ u32 cpus_to_add;
+ u32 cpus_found;
+};
+
+static bool cpus_to_add_cb(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data)
+{
+ struct cpus_to_add_cb_struct *cdata = data;
+
+ if (cdata->cpus_found < cdata->cpus_to_add) {
+ if (!dlpar_cpu_exists(cdata->parent, drc_index))
+ cdata->cpu_drcs[cdata->cpus_found++] = drc_index;
+ }
+
+ return !(cdata->cpus_found < cdata->cpus_to_add);
+}
+
static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
{
struct device_node *parent;
- int cpus_found = 0;
- int index, rc;
+ struct cpus_to_add_cb_struct cdata = {
+ NULL, cpu_drcs, cpus_to_add, 0 };
+ int cpus_found;
parent = of_find_node_by_path("/cpus");
if (!parent) {
@@ -734,25 +763,13 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
return -1;
}
- /* Search the ibm,drc-indexes array for possible CPU drcs to
- * add. Note that the format of the ibm,drc-indexes array is
- * the number of entries in the array followed by the array
- * of drc values so we start looking at index = 1.
+ /* Search the appropriate property for possible CPU drcs
+ * to add.
*/
- index = 1;
- while (cpus_found < cpus_to_add) {
- u32 drc;
-
- rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
- index++, &drc);
- if (rc)
- break;
-
- if (dlpar_cpu_exists(parent, drc))
- continue;
-
- cpu_drcs[cpus_found++] = drc;
- }
+ cdata.parent = parent;
+ arch_find_drc_match(parent, cpus_to_add_cb, "CPU",
+ NULL, false, false, &cdata);
+ cpus_found = cdata.cpus_found;
of_node_put(parent);
return cpus_found;
diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index 6ed2212..f7b9d86 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -23,6 +23,7 @@
#include <asm/hvcall.h>
#include <asm/firmware.h>
#include <asm/prom.h>
+#include <asm/topology.h>
#define MODULE_VERS "1.0"
@@ -36,60 +37,43 @@
/* Helper Routines to convert between drc_index to cpu numbers */
+struct cpu_to_drc_index_struct {
+ u32 thread_index;
+ u32 drc_index;
+ int counter;
+};
+
+static bool cpu_to_drc_index_cb(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data)
+{
+ struct cpu_to_drc_index_struct *cdata = data;
+
+ if (cdata->thread_index == cdata->counter++) {
+ cdata->drc_index = drc_index;
+ return true;
+ }
+ return false;
+}
+
static u32 cpu_to_drc_index(int cpu)
{
struct device_node *dn = NULL;
- int thread_index;
+ struct cpu_to_drc_index_struct cdata = { 0, 0, 0 };
int rc = 1;
- u32 ret = 0;
dn = of_find_node_by_path("/cpus");
if (dn == NULL)
goto err;
/* Convert logical cpu number to core number */
- 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 = 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];
- }
+ cdata.thread_index = cpu_core_index_of_thread(cpu);
+
+ rc = arch_find_drc_match(dn, cpu_to_drc_index_cb,
+ "CPU", NULL, false, false, &cdata);
+ if (rc < 0)
+ goto err_of_node_put;
rc = 0;
@@ -98,78 +82,51 @@ static u32 cpu_to_drc_index(int cpu)
err:
if (rc)
printk(KERN_WARNING "cpu_to_drc_index(%d) failed", cpu);
- return ret;
+ return cdata.drc_index;
+}
+
+struct drc_index_to_cpu_struct {
+ u32 drc_index;
+ u32 thread_index;
+ int counter;
+};
+
+static bool drc_index_to_cpu_cb(struct device_node *dn,
+ u32 drc_index, char *drc_name,
+ char *drc_type, u32 drc_power_domain,
+ void *data)
+{
+ struct drc_index_to_cpu_struct *cdata = data;
+
+ if (cdata->drc_index == drc_index) {
+ cdata->thread_index = cpu_first_thread_of_core(cdata->counter);
+ return true;
+ }
+ cdata->counter++;
+
+ return false;
}
static int drc_index_to_cpu(u32 drc_index)
{
struct device_node *dn = NULL;
- const int *indexes;
- int thread_index = 0, cpu = 0;
+ struct drc_index_to_cpu_struct cdata = {
+ drc_index, 0, 0 };
int rc = 1;
dn = of_find_node_by_path("/cpus");
if (dn == NULL)
goto err;
- 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 = 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;
+ rc = arch_find_drc_match(dn, drc_index_to_cpu_cb,
+ "CPU", NULL, false, false, &cdata);
- 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;
- }
-
-err_of_node_put:
of_node_put(dn);
+
err:
if (rc)
printk(KERN_WARNING "drc_index_to_cpu(%d) failed", drc_index);
- return thread_index;
+ return cdata.thread_index;
}
/*
The implementation of the pseries-specific drc info properties
is currently implemented in pseries-specific and non-pseries-specific
files. This patch set uses a new implementation of the device-tree
parsing code for the properties.
This patch refactors parsing of the pseries-specific drc-info properties
out of rpaphp_core.c to use the common parser. In the case where an
architecture does not use these properties, an __weak copy of the
function is provided with dummy return values. Changes include creating
appropriate callback functions and passing callback-specific data
blocks into arch_find_drc_match. All functions that were used just
to support the previous parsing implementation have been moved.
Signed-off-by: Michael Bringmann <[email protected]>
---
drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
1 file changed, 28 insertions(+), 204 deletions(-)
diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index bcd5d35..9ad7384 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c
@@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
return speed;
}
-static int get_children_props(struct device_node *dn, const int **drc_indexes,
- const int **drc_names, const int **drc_types,
- const int **drc_power_domains)
-{
- const int *indexes, *names, *types, *domains;
-
- indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
- names = of_get_property(dn, "ibm,drc-names", NULL);
- types = of_get_property(dn, "ibm,drc-types", NULL);
- domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
-
- if (!indexes || !names || !types || !domains) {
- /* Slot does not have dynamically-removable children */
- return -EINVAL;
- }
- if (drc_indexes)
- *drc_indexes = indexes;
- if (drc_names)
- /* &drc_names[1] contains NULL terminated slot names */
- *drc_names = names;
- if (drc_types)
- /* &drc_types[1] contains NULL terminated slot types */
- *drc_types = types;
- if (drc_power_domains)
- *drc_power_domains = domains;
-
- return 0;
-}
-
-
/* Verify the existence of 'drc_name' and/or 'drc_type' within the
- * current node. First obtain it's my-drc-index property. Next,
- * obtain the DRC info from it's parent. Use the my-drc-index for
- * correlation, and obtain/validate the requested properties.
+ * current node.
*/
-static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
- char *drc_type, unsigned int my_index)
-{
- char *name_tmp, *type_tmp;
- const int *indexes, *names;
- const int *types, *domains;
- int i, rc;
-
- rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
- if (rc < 0) {
- return -EINVAL;
- }
-
- name_tmp = (char *) &names[1];
- type_tmp = (char *) &types[1];
-
- /* Iterate through parent properties, looking for my-drc-index */
- for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
- if ((unsigned int) indexes[i + 1] == my_index)
- break;
-
- name_tmp += (strlen(name_tmp) + 1);
- type_tmp += (strlen(type_tmp) + 1);
- }
-
- if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
- ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
- return 0;
-
- return -EINVAL;
-}
-
-static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
- char *drc_type, unsigned int my_index)
-{
- struct property *info;
- unsigned int entries;
- struct of_drc_info drc;
- const __be32 *value;
- char cell_drc_name[MAX_DRC_NAME_LEN];
- int j, fndit;
-
- info = of_find_property(dn->parent, "ibm,drc-info", NULL);
- if (info == NULL)
- return -EINVAL;
-
- value = of_prop_next_u32(info, NULL, &entries);
- if (!value)
- return -EINVAL;
-
- for (j = 0; j < entries; j++) {
- of_read_drc_info_cell(&info, &value, &drc);
-
- /* Should now know end of current entry */
-
- if (my_index > drc.last_drc_index)
- continue;
-
- fndit = 1;
- break;
- }
- /* Found it */
-
- if (fndit)
- sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
- my_index);
-
- if (((drc_name == NULL) ||
- (drc_name && !strcmp(drc_name, cell_drc_name))) &&
- ((drc_type == NULL) ||
- (drc_type && !strcmp(drc_type, drc.drc_type))))
- return 0;
-
- return -EINVAL;
-}
-
int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
char *drc_type)
{
- const unsigned int *my_index;
-
- my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
- if (!my_index) {
- /* Node isn't DLPAR/hotplug capable */
- return -EINVAL;
- }
-
- if (firmware_has_feature(FW_FEATURE_DRC_INFO))
- return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
- *my_index);
- else
- return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
- *my_index);
+ return arch_find_drc_match(dn, NULL, drc_type, drc_name,
+ true, false, NULL);
}
EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
-
-static int is_php_type(char *drc_type)
-{
- unsigned long value;
- char *endptr;
-
- /* PCI Hotplug nodes have an integer for drc_type */
- value = simple_strtoul(drc_type, &endptr, 10);
- if (endptr == drc_type)
- return 0;
-
- return 1;
-}
-
-/**
- * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
- * @dn: target &device_node
- * @indexes: passed to get_children_props()
- * @names: passed to get_children_props()
- * @types: returned from get_children_props()
- * @power_domains:
- *
- * This routine will return true only if the device node is
- * a hotpluggable slot. This routine will return false
- * for built-in pci slots (even when the built-in slots are
- * dlparable.)
- */
-static int is_php_dn(struct device_node *dn, const int **indexes,
- const int **names, const int **types, const int **power_domains)
-{
- const int *drc_types;
- int rc;
-
- rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
- if (rc < 0)
- return 0;
-
- if (!is_php_type((char *) &drc_types[1]))
- return 0;
-
- *types = drc_types;
- return 1;
-}
-
/**
* rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
* @dn: device node of slot
@@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
*
* To remove a slot, it suffices to call rpaphp_deregister_slot().
*/
-int rpaphp_add_slot(struct device_node *dn)
+
+static int rpaphp_add_slot_cb(struct device_node *dn,
+ u32 drc_index, char *drc_name, char *drc_type,
+ u32 drc_power_domain, void *data)
{
struct slot *slot;
int retval = 0;
- int i;
- const int *indexes, *names, *types, *power_domains;
- char *name, *type;
-
- if (!dn->name || strcmp(dn->name, "pci"))
- return 0;
-
- /* If this is not a hotplug slot, return without doing anything. */
- if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
- return 0;
-
- dbg("Entry %s: dn=%pOF\n", __func__, dn);
- /* register PCI devices */
- name = (char *) &names[1];
- type = (char *) &types[1];
- for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
- int index;
+ slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
+ if (!slot)
+ return -ENOMEM;
- index = be32_to_cpu(indexes[i + 1]);
- slot = alloc_slot_struct(dn, index, name,
- be32_to_cpu(power_domains[i + 1]));
- if (!slot)
- return -ENOMEM;
+ slot->type = simple_strtoul(drc_type, NULL, 10);
+ if (retval)
+ return -EINVAL;
- slot->type = simple_strtoul(type, NULL, 10);
+ dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
+ drc_index, drc_name, drc_type);
- dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
- index, name, type);
+ retval = rpaphp_enable_slot(slot);
+ if (!retval)
+ retval = rpaphp_register_slot(slot);
- retval = rpaphp_enable_slot(slot);
- if (!retval)
- retval = rpaphp_register_slot(slot);
+ if (retval)
+ dealloc_slot_struct(slot);
- if (retval)
- dealloc_slot_struct(slot);
+ return retval;
+}
- name += strlen(name) + 1;
- type += strlen(type) + 1;
- }
- dbg("%s - Exit: rc[%d]\n", __func__, retval);
+int rpaphp_add_slot(struct device_node *dn)
+{
+ if (!dn->name || strcmp(dn->name, "pci"))
+ return 0;
- /* XXX FIXME: reports a failure only if last entry in loop failed */
- return retval;
+ return arch_find_drc_match(dn, rpaphp_add_slot_cb,
+ NULL, NULL, false, true, NULL);
}
EXPORT_SYMBOL_GPL(rpaphp_add_slot);
On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
> The implementation of the pseries-specific drc info properties
> is currently implemented in pseries-specific and non-pseries-specific
> files. This patch set uses a new implementation of the device-tree
> parsing code for the properties.
>
> This patch refactors parsing of the pseries-specific drc-info properties
> out of rpaphp_core.c to use the common parser. In the case where an
> architecture does not use these properties, an __weak copy of the
> function is provided with dummy return values. Changes include creating
> appropriate callback functions and passing callback-specific data
> blocks into arch_find_drc_match. All functions that were used just
> to support the previous parsing implementation have been moved.
>
> Signed-off-by: Michael Bringmann <[email protected]>
This is fine with me. Any rpaphp_core.c maintainers want to comment?
Tyrel?
$ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
Tyrel Datwyler <[email protected]> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
Benjamin Herrenschmidt <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Paul Mackerras <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Michael Ellerman <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> ---
> drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
> 1 file changed, 28 insertions(+), 204 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index bcd5d35..9ad7384 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
> return speed;
> }
>
> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
> - const int **drc_names, const int **drc_types,
> - const int **drc_power_domains)
> -{
> - const int *indexes, *names, *types, *domains;
> -
> - indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> - names = of_get_property(dn, "ibm,drc-names", NULL);
> - types = of_get_property(dn, "ibm,drc-types", NULL);
> - domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
> -
> - if (!indexes || !names || !types || !domains) {
> - /* Slot does not have dynamically-removable children */
> - return -EINVAL;
> - }
> - if (drc_indexes)
> - *drc_indexes = indexes;
> - if (drc_names)
> - /* &drc_names[1] contains NULL terminated slot names */
> - *drc_names = names;
> - if (drc_types)
> - /* &drc_types[1] contains NULL terminated slot types */
> - *drc_types = types;
> - if (drc_power_domains)
> - *drc_power_domains = domains;
> -
> - return 0;
> -}
> -
> -
> /* Verify the existence of 'drc_name' and/or 'drc_type' within the
> - * current node. First obtain it's my-drc-index property. Next,
> - * obtain the DRC info from it's parent. Use the my-drc-index for
> - * correlation, and obtain/validate the requested properties.
> + * current node.
> */
>
> -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
> - char *drc_type, unsigned int my_index)
> -{
> - char *name_tmp, *type_tmp;
> - const int *indexes, *names;
> - const int *types, *domains;
> - int i, rc;
> -
> - rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
> - if (rc < 0) {
> - return -EINVAL;
> - }
> -
> - name_tmp = (char *) &names[1];
> - type_tmp = (char *) &types[1];
> -
> - /* Iterate through parent properties, looking for my-drc-index */
> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> - if ((unsigned int) indexes[i + 1] == my_index)
> - break;
> -
> - name_tmp += (strlen(name_tmp) + 1);
> - type_tmp += (strlen(type_tmp) + 1);
> - }
> -
> - if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
> - ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
> - return 0;
> -
> - return -EINVAL;
> -}
> -
> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> - char *drc_type, unsigned int my_index)
> -{
> - struct property *info;
> - unsigned int entries;
> - struct of_drc_info drc;
> - const __be32 *value;
> - char cell_drc_name[MAX_DRC_NAME_LEN];
> - int j, fndit;
> -
> - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> - if (info == NULL)
> - return -EINVAL;
> -
> - value = of_prop_next_u32(info, NULL, &entries);
> - if (!value)
> - return -EINVAL;
> -
> - for (j = 0; j < entries; j++) {
> - of_read_drc_info_cell(&info, &value, &drc);
> -
> - /* Should now know end of current entry */
> -
> - if (my_index > drc.last_drc_index)
> - continue;
> -
> - fndit = 1;
> - break;
> - }
> - /* Found it */
> -
> - if (fndit)
> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> - my_index);
> -
> - if (((drc_name == NULL) ||
> - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
> - ((drc_type == NULL) ||
> - (drc_type && !strcmp(drc_type, drc.drc_type))))
> - return 0;
> -
> - return -EINVAL;
> -}
> -
> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> char *drc_type)
> {
> - const unsigned int *my_index;
> -
> - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> - if (!my_index) {
> - /* Node isn't DLPAR/hotplug capable */
> - return -EINVAL;
> - }
> -
> - if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> - return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
> - *my_index);
> - else
> - return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
> - *my_index);
> + return arch_find_drc_match(dn, NULL, drc_type, drc_name,
> + true, false, NULL);
> }
> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>
> -
> -static int is_php_type(char *drc_type)
> -{
> - unsigned long value;
> - char *endptr;
> -
> - /* PCI Hotplug nodes have an integer for drc_type */
> - value = simple_strtoul(drc_type, &endptr, 10);
> - if (endptr == drc_type)
> - return 0;
> -
> - return 1;
> -}
> -
> -/**
> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
> - * @dn: target &device_node
> - * @indexes: passed to get_children_props()
> - * @names: passed to get_children_props()
> - * @types: returned from get_children_props()
> - * @power_domains:
> - *
> - * This routine will return true only if the device node is
> - * a hotpluggable slot. This routine will return false
> - * for built-in pci slots (even when the built-in slots are
> - * dlparable.)
> - */
> -static int is_php_dn(struct device_node *dn, const int **indexes,
> - const int **names, const int **types, const int **power_domains)
> -{
> - const int *drc_types;
> - int rc;
> -
> - rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
> - if (rc < 0)
> - return 0;
> -
> - if (!is_php_type((char *) &drc_types[1]))
> - return 0;
> -
> - *types = drc_types;
> - return 1;
> -}
> -
> /**
> * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
> * @dn: device node of slot
> @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
> *
> * To remove a slot, it suffices to call rpaphp_deregister_slot().
> */
> -int rpaphp_add_slot(struct device_node *dn)
> +
> +static int rpaphp_add_slot_cb(struct device_node *dn,
> + u32 drc_index, char *drc_name, char *drc_type,
> + u32 drc_power_domain, void *data)
> {
> struct slot *slot;
> int retval = 0;
> - int i;
> - const int *indexes, *names, *types, *power_domains;
> - char *name, *type;
> -
> - if (!dn->name || strcmp(dn->name, "pci"))
> - return 0;
> -
> - /* If this is not a hotplug slot, return without doing anything. */
> - if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
> - return 0;
> -
> - dbg("Entry %s: dn=%pOF\n", __func__, dn);
>
> - /* register PCI devices */
> - name = (char *) &names[1];
> - type = (char *) &types[1];
> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> - int index;
> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
> + if (!slot)
> + return -ENOMEM;
>
> - index = be32_to_cpu(indexes[i + 1]);
> - slot = alloc_slot_struct(dn, index, name,
> - be32_to_cpu(power_domains[i + 1]));
> - if (!slot)
> - return -ENOMEM;
> + slot->type = simple_strtoul(drc_type, NULL, 10);
> + if (retval)
> + return -EINVAL;
>
> - slot->type = simple_strtoul(type, NULL, 10);
> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> + drc_index, drc_name, drc_type);
>
> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> - index, name, type);
> + retval = rpaphp_enable_slot(slot);
> + if (!retval)
> + retval = rpaphp_register_slot(slot);
>
> - retval = rpaphp_enable_slot(slot);
> - if (!retval)
> - retval = rpaphp_register_slot(slot);
> + if (retval)
> + dealloc_slot_struct(slot);
>
> - if (retval)
> - dealloc_slot_struct(slot);
> + return retval;
> +}
>
> - name += strlen(name) + 1;
> - type += strlen(type) + 1;
> - }
> - dbg("%s - Exit: rc[%d]\n", __func__, retval);
> +int rpaphp_add_slot(struct device_node *dn)
> +{
> + if (!dn->name || strcmp(dn->name, "pci"))
> + return 0;
>
> - /* XXX FIXME: reports a failure only if last entry in loop failed */
> - return retval;
> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
> + NULL, NULL, false, true, NULL);
> }
> EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>
>
On Mon, Jan 14, 2019 at 06:28:57PM -0600, Bjorn Helgaas wrote:
> On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
> > The implementation of the pseries-specific drc info properties
> > is currently implemented in pseries-specific and non-pseries-specific
> > files. This patch set uses a new implementation of the device-tree
> > parsing code for the properties.
> >
> > This patch refactors parsing of the pseries-specific drc-info properties
> > out of rpaphp_core.c to use the common parser. In the case where an
> > architecture does not use these properties, an __weak copy of the
> > function is provided with dummy return values. Changes include creating
> > appropriate callback functions and passing callback-specific data
> > blocks into arch_find_drc_match. All functions that were used just
> > to support the previous parsing implementation have been moved.
> >
> > Signed-off-by: Michael Bringmann <[email protected]>
>
> This is fine with me. Any rpaphp_core.c maintainers want to comment?
> Tyrel?
>
> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
> Tyrel Datwyler <[email protected]> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
> Benjamin Herrenschmidt <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Michael Ellerman <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
This looks like part of a larger series, but I only got this patch. So I
assume you'll route this elsewhere along with the rest of the series.
Here's my ack if it's useful:
Acked-by: Bjorn Helgaas <[email protected]>
> > ---
> > drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
> > 1 file changed, 28 insertions(+), 204 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> > index bcd5d35..9ad7384 100644
> > --- a/drivers/pci/hotplug/rpaphp_core.c
> > +++ b/drivers/pci/hotplug/rpaphp_core.c
> > @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
> > return speed;
> > }
> >
> > -static int get_children_props(struct device_node *dn, const int **drc_indexes,
> > - const int **drc_names, const int **drc_types,
> > - const int **drc_power_domains)
> > -{
> > - const int *indexes, *names, *types, *domains;
> > -
> > - indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> > - names = of_get_property(dn, "ibm,drc-names", NULL);
> > - types = of_get_property(dn, "ibm,drc-types", NULL);
> > - domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
> > -
> > - if (!indexes || !names || !types || !domains) {
> > - /* Slot does not have dynamically-removable children */
> > - return -EINVAL;
> > - }
> > - if (drc_indexes)
> > - *drc_indexes = indexes;
> > - if (drc_names)
> > - /* &drc_names[1] contains NULL terminated slot names */
> > - *drc_names = names;
> > - if (drc_types)
> > - /* &drc_types[1] contains NULL terminated slot types */
> > - *drc_types = types;
> > - if (drc_power_domains)
> > - *drc_power_domains = domains;
> > -
> > - return 0;
> > -}
> > -
> > -
> > /* Verify the existence of 'drc_name' and/or 'drc_type' within the
> > - * current node. First obtain it's my-drc-index property. Next,
> > - * obtain the DRC info from it's parent. Use the my-drc-index for
> > - * correlation, and obtain/validate the requested properties.
> > + * current node.
> > */
> >
> > -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
> > - char *drc_type, unsigned int my_index)
> > -{
> > - char *name_tmp, *type_tmp;
> > - const int *indexes, *names;
> > - const int *types, *domains;
> > - int i, rc;
> > -
> > - rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
> > - if (rc < 0) {
> > - return -EINVAL;
> > - }
> > -
> > - name_tmp = (char *) &names[1];
> > - type_tmp = (char *) &types[1];
> > -
> > - /* Iterate through parent properties, looking for my-drc-index */
> > - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> > - if ((unsigned int) indexes[i + 1] == my_index)
> > - break;
> > -
> > - name_tmp += (strlen(name_tmp) + 1);
> > - type_tmp += (strlen(type_tmp) + 1);
> > - }
> > -
> > - if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
> > - ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
> > - return 0;
> > -
> > - return -EINVAL;
> > -}
> > -
> > -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> > - char *drc_type, unsigned int my_index)
> > -{
> > - struct property *info;
> > - unsigned int entries;
> > - struct of_drc_info drc;
> > - const __be32 *value;
> > - char cell_drc_name[MAX_DRC_NAME_LEN];
> > - int j, fndit;
> > -
> > - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> > - if (info == NULL)
> > - return -EINVAL;
> > -
> > - value = of_prop_next_u32(info, NULL, &entries);
> > - if (!value)
> > - return -EINVAL;
> > -
> > - for (j = 0; j < entries; j++) {
> > - of_read_drc_info_cell(&info, &value, &drc);
> > -
> > - /* Should now know end of current entry */
> > -
> > - if (my_index > drc.last_drc_index)
> > - continue;
> > -
> > - fndit = 1;
> > - break;
> > - }
> > - /* Found it */
> > -
> > - if (fndit)
> > - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> > - my_index);
> > -
> > - if (((drc_name == NULL) ||
> > - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
> > - ((drc_type == NULL) ||
> > - (drc_type && !strcmp(drc_type, drc.drc_type))))
> > - return 0;
> > -
> > - return -EINVAL;
> > -}
> > -
> > int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> > char *drc_type)
> > {
> > - const unsigned int *my_index;
> > -
> > - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> > - if (!my_index) {
> > - /* Node isn't DLPAR/hotplug capable */
> > - return -EINVAL;
> > - }
> > -
> > - if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> > - return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
> > - *my_index);
> > - else
> > - return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
> > - *my_index);
> > + return arch_find_drc_match(dn, NULL, drc_type, drc_name,
> > + true, false, NULL);
> > }
> > EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
> >
> > -
> > -static int is_php_type(char *drc_type)
> > -{
> > - unsigned long value;
> > - char *endptr;
> > -
> > - /* PCI Hotplug nodes have an integer for drc_type */
> > - value = simple_strtoul(drc_type, &endptr, 10);
> > - if (endptr == drc_type)
> > - return 0;
> > -
> > - return 1;
> > -}
> > -
> > -/**
> > - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
> > - * @dn: target &device_node
> > - * @indexes: passed to get_children_props()
> > - * @names: passed to get_children_props()
> > - * @types: returned from get_children_props()
> > - * @power_domains:
> > - *
> > - * This routine will return true only if the device node is
> > - * a hotpluggable slot. This routine will return false
> > - * for built-in pci slots (even when the built-in slots are
> > - * dlparable.)
> > - */
> > -static int is_php_dn(struct device_node *dn, const int **indexes,
> > - const int **names, const int **types, const int **power_domains)
> > -{
> > - const int *drc_types;
> > - int rc;
> > -
> > - rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
> > - if (rc < 0)
> > - return 0;
> > -
> > - if (!is_php_type((char *) &drc_types[1]))
> > - return 0;
> > -
> > - *types = drc_types;
> > - return 1;
> > -}
> > -
> > /**
> > * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
> > * @dn: device node of slot
> > @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
> > *
> > * To remove a slot, it suffices to call rpaphp_deregister_slot().
> > */
> > -int rpaphp_add_slot(struct device_node *dn)
> > +
> > +static int rpaphp_add_slot_cb(struct device_node *dn,
> > + u32 drc_index, char *drc_name, char *drc_type,
> > + u32 drc_power_domain, void *data)
> > {
> > struct slot *slot;
> > int retval = 0;
> > - int i;
> > - const int *indexes, *names, *types, *power_domains;
> > - char *name, *type;
> > -
> > - if (!dn->name || strcmp(dn->name, "pci"))
> > - return 0;
> > -
> > - /* If this is not a hotplug slot, return without doing anything. */
> > - if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
> > - return 0;
> > -
> > - dbg("Entry %s: dn=%pOF\n", __func__, dn);
> >
> > - /* register PCI devices */
> > - name = (char *) &names[1];
> > - type = (char *) &types[1];
> > - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> > - int index;
> > + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
> > + if (!slot)
> > + return -ENOMEM;
> >
> > - index = be32_to_cpu(indexes[i + 1]);
> > - slot = alloc_slot_struct(dn, index, name,
> > - be32_to_cpu(power_domains[i + 1]));
> > - if (!slot)
> > - return -ENOMEM;
> > + slot->type = simple_strtoul(drc_type, NULL, 10);
> > + if (retval)
> > + return -EINVAL;
> >
> > - slot->type = simple_strtoul(type, NULL, 10);
> > + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> > + drc_index, drc_name, drc_type);
> >
> > - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> > - index, name, type);
> > + retval = rpaphp_enable_slot(slot);
> > + if (!retval)
> > + retval = rpaphp_register_slot(slot);
> >
> > - retval = rpaphp_enable_slot(slot);
> > - if (!retval)
> > - retval = rpaphp_register_slot(slot);
> > + if (retval)
> > + dealloc_slot_struct(slot);
> >
> > - if (retval)
> > - dealloc_slot_struct(slot);
> > + return retval;
> > +}
> >
> > - name += strlen(name) + 1;
> > - type += strlen(type) + 1;
> > - }
> > - dbg("%s - Exit: rc[%d]\n", __func__, retval);
> > +int rpaphp_add_slot(struct device_node *dn)
> > +{
> > + if (!dn->name || strcmp(dn->name, "pci"))
> > + return 0;
> >
> > - /* XXX FIXME: reports a failure only if last entry in loop failed */
> > - return retval;
> > + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
> > + NULL, NULL, false, true, NULL);
> > }
> > EXPORT_SYMBOL_GPL(rpaphp_add_slot);
> >
> >
On 12/14/2018 12:50 PM, Michael Bringmann wrote:
> Define interface to acquire arch-specific drc info to match against
> hotpluggable devices. The current implementation exposes several
> pseries-specific dynamic memory properties in generic kernel code.
> This patch set provides an interface to pull that code out of the
> generic kernel.
>
> Signed-off-by: Michael Bringmann <[email protected]>
> ---
> include/linux/topology.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index cb0775e..df97f5f 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -44,6 +44,15 @@
As far as I know pseries is the only platform that uses DR connectors, and I
highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
that this is really generic enough to belong in topology.h. If anything I would
suggest putting this in an include in arch/powerpc/include/ named something like
drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
that want/need to use this functionality.
-Tyrel
>
> int arch_update_cpu_topology(void);
>
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool ck_php_type,
> + void *data);
> +
> /* Conform to ACPI 2.0 SLIT distance definitions */
> #define LOCAL_DISTANCE 10
> #define REMOTE_DISTANCE 20
>
On 12/14/2018 12:51 PM, Michael Bringmann wrote:
> This patch provides a common interface to parse ibm,drc-indexes,
> ibm,drc-names, ibm,drc-types, ibm,drc-power-domains, or ibm,drc-info.
> The generic interface arch_find_drc_match is provided which accepts
> callback functions that may be applied to examine the data for each
> entry.
>
The title of your patch is "pseries/drcinfo: Pseries impl of arch_find_drc_info"
but the name of the function you are ultimately implementing is
arch_find_drc_match if I'm not mistaken.
> Signed-off-by: Michael Bringmann <[email protected]>
> ---
> arch/powerpc/include/asm/prom.h | 3
> arch/powerpc/platforms/pseries/of_helpers.c | 299 +++++++++++++++++++++++++++
> include/linux/topology.h | 2
> 3 files changed, 298 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index b04c5ce..910d1dc 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -91,9 +91,6 @@ struct of_drc_info {
> 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.
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 0185e50..11c90cd 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#define pr_fmt(fmt) "drc: " fmt
> +
> #include <linux/string.h>
> #include <linux/err.h>
> #include <linux/slab.h>
> @@ -11,6 +13,12 @@
>
> #define MAX_DRC_NAME_LEN 64
>
> +static int drc_debug;
> +#define dbg(args...) if (drc_debug) { printk(KERN_DEBUG args); }
> +#define err(arg...) printk(KERN_ERR args);
> +#define info(arg...) printk(KERN_INFO args);
> +#define warn(arg...) printk(KERN_WARNING args);
Its pretty standard these days to use the pr_debug, pr_err, pr_info, pr_warn
variations over printk(LEVEL args).
> +
> /**
> * pseries_of_derive_parent - basically like dirname(1)
> * @path: the full_name of a node to be added to the tree
> @@ -46,7 +54,8 @@ struct device_node *pseries_of_derive_parent(const char *path)
>
> /* Helper Routines to convert between drc_index to cpu numbers */
>
> -int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
> +static int of_read_drc_info_cell(struct property **prop,
> + const __be32 **curval,
> struct of_drc_info *data)
> {
> const char *p;
> @@ -90,4 +99,290 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>
> return 0;
> }
> -EXPORT_SYMBOL(of_read_drc_info_cell);
> +
> +static int walk_drc_info(struct device_node *dn,
> + bool (*usercb)(struct of_drc_info *drc,
> + void *data,
> + int *ret_code),
> + char *opt_drc_type,
> + void *data)
> +{
> + struct property *info;
> + unsigned int entries;
> + struct of_drc_info drc;
> + const __be32 *value;
> + int j, ret_code = -EINVAL;
> + bool done = false;
> +
> + info = of_find_property(dn, "ibm,drc-info", NULL);
> + if (info == NULL)
> + return -EINVAL;
> +
> + value = info->value;
> + entries = of_read_number(value++, 1);
> +
> + for (j = 0, done = 0; (j < entries) && (!done); j++) {
> + of_read_drc_info_cell(&info, &value, &drc);
> +
> + if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
> + continue;
> +
> + done = usercb(&drc, data, &ret_code);
> + }
> +
> + return ret_code;
> +}
> +
> +static int get_children_props(struct device_node *dn, const int **drc_indexes,
> + const int **drc_names, const int **drc_types,
> + const int **drc_power_domains)
> +{
> + const int *indexes, *names, *types, *domains;
> +
> + indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> + names = of_get_property(dn, "ibm,drc-names", NULL);
> + types = of_get_property(dn, "ibm,drc-types", NULL);
> + domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
> +
> + if (!indexes || !names || !types || !domains) {
> + /* Slot does not have dynamically-removable children */
> + return -EINVAL;
> + }
> + if (drc_indexes)
> + *drc_indexes = indexes;
> + if (drc_names)
> + /* &drc_names[1] contains NULL terminated slot names */
> + *drc_names = names;
> + if (drc_types)
> + /* &drc_types[1] contains NULL terminated slot types */
> + *drc_types = types;
> + if (drc_power_domains)
> + *drc_power_domains = domains;
> +
> + return 0;
> +}
> +
> +static int is_php_type(char *drc_type)
> +{
> + unsigned long value;
> + char *endptr;
> +
> + /* PCI Hotplug nodes have an integer for drc_type */
> + value = simple_strtoul(drc_type, &endptr, 10);
> + if (endptr == drc_type)
> + return 0;
> +
> + return 1;
> +}
> +
> +/**
> + * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
> + * @dn: target &device_node
> + * @indexes: passed to get_children_props()
> + * @names: passed to get_children_props()
> + * @types: returned from get_children_props()
> + * @power_domains:
> + *
> + * This routine will return true only if the device node is
> + * a hotpluggable slot. This routine will return false
> + * for built-in pci slots (even when the built-in slots are
> + * dlparable.)
> + */
> +static int is_php_dn(struct device_node *dn, const int **indexes,
> + const int **names, const int **types,
> + const int **power_domains)
> +{
> + const int *drc_types;
> + int rc;
> +
> + rc = get_children_props(dn, indexes, names, &drc_types,
> + power_domains);
> + if (rc < 0)
> + return 0;
> +
> + if (!is_php_type((char *) &drc_types[1]))
> + return 0;
> +
> + *types = drc_types;
> + return 1;
> +}
> +
> +struct find_drc_match_cb_struct {
> + struct device_node *dn;
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data);
> + char *drc_type;
> + char *drc_name;
> + u32 drc_index;
> + bool match_drc_index;
> + bool add_slot;
> + void *data;
> +};
> +
> +static int find_drc_match_v1(struct device_node *dn, void *data)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + int i, retval = 0;
> + const int *indexes, *names, *types, *domains;
> + char *name, *type;
> + struct device_node *root = dn;
> +
> + if (cdata->match_drc_index)
> + root = dn->parent;
> +
> + if (cdata->add_slot) {
> + /* If this is not a hotplug slot, return without doing
> + * anything.
> + */
> + if (!is_php_dn(root, &indexes, &names, &types, &domains))
> + return 0;
> + } else {
> + if (get_children_props(root, &indexes, &names, &types,
> + &domains) < 0)
> + return 0;
> + }
> +
> + dbg("Entry %s: dn=%pOF\n", __func__, dn);
> +
> + name = (char *) &names[1];
> + type = (char *) &types[1];
> + for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> +
> + if (cdata->match_drc_index &&
> + ((unsigned int) indexes[i + 1] != cdata->drc_index)) {
> + name += strlen(name) + 1;
> + type += strlen(type) + 1;
> + continue;
> + }
> +
> + if (((cdata->drc_name == NULL) ||
> + (cdata->drc_name && !strcmp(cdata->drc_name, name))) &&
> + ((cdata->drc_type == NULL) ||
> + (cdata->drc_type && !strcmp(cdata->drc_type, type)))) {
> +
> + if (cdata->usercb) {
> + retval = cdata->usercb(dn,
> + be32_to_cpu(indexes[i + 1]),
> + name, type,
> + be32_to_cpu(domains[i + 1]),
> + cdata->data);
> + if (!retval)
> + return retval;
> + } else {
> + return 0;
> + }
> + }
> +
> + name += strlen(name) + 1;
> + type += strlen(type) + 1;
> + }
> +
> + dbg("%s - Exit: rc[%d]\n", __func__, retval);
> +
> + /* XXX FIXME: reports a failure only if last entry in loop failed */
> + return retval;
> +}
> +
> +static bool find_drc_match_v2_cb(struct of_drc_info *drc, void *data,
> + int *ret_code)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + u32 drc_index;
> + char drc_name[MAX_DRC_NAME_LEN];
> + int i, retval;
> +
> + (*ret_code) = -EINVAL;
> +
> + /* This set not a PHP type? */
> + if (cdata->add_slot) {
> + if (!is_php_type((char *) drc->drc_type)) {
> + return false;
> + }
> + }
> +
> + /* Anything to use from this set? */
> + if (cdata->match_drc_index && (cdata->drc_index > drc->last_drc_index))
> + return false;
> + if ((cdata->drc_type && strcmp(cdata->drc_type, drc->drc_type))
> + return false;
> +
> + /* Check the drc-index entries of this set */
> + for (i = 0, drc_index = drc->drc_index_start;
> + i < drc->num_sequential_elems; i++, drc_index++) {
> +
> + if (cdata->match_drc_index && (cdata->drc_index != drc_index))
> + continue;
> +
> + sprintf(drc_name, "%s%d", drc->drc_name_prefix,
> + drc_index - drc->drc_index_start +
> + drc->drc_name_suffix_start);
> +
> + if ((cdata->drc_name == NULL) ||
> + (cdata->drc_name && !strcmp(cdata->drc_name, drc_name))) {
> +
> + if (cdata->usercb) {
> + retval = cdata->usercb(cdata->dn,
> + drc_index, drc_name,
> + drc->drc_type,
> + drc->drc_power_domain,
> + cdata->data);
> + if (!retval) {
> + (*ret_code) = retval;
> + return true;
> + }
> + } else {
> + (*ret_code) = 0;
> + return true;
> + }
> + }
> + }
> +
> + (*ret_code) = retval;
> + return false;
> +}
> +
> +static int find_drc_match_v2(struct device_node *dn, void *data)
> +{
> + struct find_drc_match_cb_struct *cdata = data;
> + struct device_node *root = cdata->dn;
> +
> + if (!cdata->add_slot) {
> + if (!cdata->drc_type ||
> + (cdata->drc_type && strcmp(cdata->drc_type, "SLOT")))
> + root = dn->parent;
> + }
> +
> + return walk_drc_info(root, find_drc_match_v2_cb, NULL, data);
> +}
> +
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool add_slot,
> + void *data)
> +{
> + struct find_drc_match_cb_struct cdata = { dn, usercb,
> + opt_drc_type, opt_drc_name, 0,
> + match_drc_index, add_slot, data };
> +
> + if (match_drc_index) {
> + const int *my_index =
> + of_get_property(dn, "ibm,my-drc-index", NULL);
> + if (!my_index) {
> + /* Node isn't DLPAR/hotplug capable */
> + return -EINVAL;
> + }
> + cdata.drc_index = *my_index;
> + }
> +
> + if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> + return find_drc_match_v2(dn, &cdata);
> + else
> + return find_drc_match_v1(dn, &cdata);
> +}
> +EXPORT_SYMBOL(arch_find_drc_match);
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index df97f5f..c3dfa53 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -50,7 +50,7 @@ int arch_find_drc_match(struct device_node *dn,
> char *drc_type, u32 drc_power_domain,
> void *data),
> char *opt_drc_type, char *opt_drc_name,
> - bool match_drc_index, bool ck_php_type,
> + bool match_drc_index, bool add_slot,
> void *data);
Why are you making a change to the prototype here? You should just define it in
your first patch instead of touching it again here.
-Tyrel
>
> /* Conform to ACPI 2.0 SLIT distance definitions */
>
On 12/14/2018 12:50 PM, Michael Bringmann wrote:
> Define interface to acquire arch-specific drc info to match against
> hotpluggable devices. The current implementation exposes several
> pseries-specific dynamic memory properties in generic kernel code.
> This patch set provides an interface to pull that code out of the
> generic kernel.
>
> Signed-off-by: Michael Bringmann <[email protected]>
> ---
> include/linux/topology.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/topology.h b/include/linux/topology.h
> index cb0775e..df97f5f 100644
> --- a/include/linux/topology.h
> +++ b/include/linux/topology.h
> @@ -44,6 +44,15 @@
>
> int arch_update_cpu_topology(void);
On another note a kern doc comment for this function would also be nice.
-Tyrel
>
> +int arch_find_drc_match(struct device_node *dn,
> + bool (*usercb)(struct device_node *dn,
> + u32 drc_index, char *drc_name,
> + char *drc_type, u32 drc_power_domain,
> + void *data),
> + char *opt_drc_type, char *opt_drc_name,
> + bool match_drc_index, bool ck_php_type,
> + void *data);
> +
> /* Conform to ACPI 2.0 SLIT distance definitions */
> #define LOCAL_DISTANCE 10
> #define REMOTE_DISTANCE 20
>
On 01/14/2019 04:28 PM, Bjorn Helgaas wrote:
> On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
>> The implementation of the pseries-specific drc info properties
>> is currently implemented in pseries-specific and non-pseries-specific
>> files. This patch set uses a new implementation of the device-tree
>> parsing code for the properties.
>>
>> This patch refactors parsing of the pseries-specific drc-info properties
>> out of rpaphp_core.c to use the common parser. In the case where an
>> architecture does not use these properties, an __weak copy of the
>> function is provided with dummy return values. Changes include creating
>> appropriate callback functions and passing callback-specific data
>> blocks into arch_find_drc_match. All functions that were used just
>> to support the previous parsing implementation have been moved.
>>
>> Signed-off-by: Michael Bringmann <[email protected]>
>
> This is fine with me. Any rpaphp_core.c maintainers want to comment?
> Tyrel?
It greatly simplifies the code in rpaphp_core.c, and as far as I can tell the
refactoring maintains the existing functionality.
Acked-by: Tyrel Datwyler <[email protected]>
>
> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
> Tyrel Datwyler <[email protected]> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
> Benjamin Herrenschmidt <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Michael Ellerman <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>
>> ---
>> drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
>> 1 file changed, 28 insertions(+), 204 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>> index bcd5d35..9ad7384 100644
>> --- a/drivers/pci/hotplug/rpaphp_core.c
>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>> @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>> return speed;
>> }
>>
>> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
>> - const int **drc_names, const int **drc_types,
>> - const int **drc_power_domains)
>> -{
>> - const int *indexes, *names, *types, *domains;
>> -
>> - indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> - names = of_get_property(dn, "ibm,drc-names", NULL);
>> - types = of_get_property(dn, "ibm,drc-types", NULL);
>> - domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>> -
>> - if (!indexes || !names || !types || !domains) {
>> - /* Slot does not have dynamically-removable children */
>> - return -EINVAL;
>> - }
>> - if (drc_indexes)
>> - *drc_indexes = indexes;
>> - if (drc_names)
>> - /* &drc_names[1] contains NULL terminated slot names */
>> - *drc_names = names;
>> - if (drc_types)
>> - /* &drc_types[1] contains NULL terminated slot types */
>> - *drc_types = types;
>> - if (drc_power_domains)
>> - *drc_power_domains = domains;
>> -
>> - return 0;
>> -}
>> -
>> -
>> /* Verify the existence of 'drc_name' and/or 'drc_type' within the
>> - * current node. First obtain it's my-drc-index property. Next,
>> - * obtain the DRC info from it's parent. Use the my-drc-index for
>> - * correlation, and obtain/validate the requested properties.
>> + * current node.
>> */
>>
>> -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
>> - char *drc_type, unsigned int my_index)
>> -{
>> - char *name_tmp, *type_tmp;
>> - const int *indexes, *names;
>> - const int *types, *domains;
>> - int i, rc;
>> -
>> - rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>> - if (rc < 0) {
>> - return -EINVAL;
>> - }
>> -
>> - name_tmp = (char *) &names[1];
>> - type_tmp = (char *) &types[1];
>> -
>> - /* Iterate through parent properties, looking for my-drc-index */
>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> - if ((unsigned int) indexes[i + 1] == my_index)
>> - break;
>> -
>> - name_tmp += (strlen(name_tmp) + 1);
>> - type_tmp += (strlen(type_tmp) + 1);
>> - }
>> -
>> - if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
>> - ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
>> - return 0;
>> -
>> - return -EINVAL;
>> -}
>> -
>> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
>> - char *drc_type, unsigned int my_index)
>> -{
>> - struct property *info;
>> - unsigned int entries;
>> - struct of_drc_info drc;
>> - const __be32 *value;
>> - char cell_drc_name[MAX_DRC_NAME_LEN];
>> - int j, fndit;
>> -
>> - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>> - if (info == NULL)
>> - return -EINVAL;
>> -
>> - value = of_prop_next_u32(info, NULL, &entries);
>> - if (!value)
>> - return -EINVAL;
>> -
>> - for (j = 0; j < entries; j++) {
>> - of_read_drc_info_cell(&info, &value, &drc);
>> -
>> - /* Should now know end of current entry */
>> -
>> - if (my_index > drc.last_drc_index)
>> - continue;
>> -
>> - fndit = 1;
>> - break;
>> - }
>> - /* Found it */
>> -
>> - if (fndit)
>> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
>> - my_index);
>> -
>> - if (((drc_name == NULL) ||
>> - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
>> - ((drc_type == NULL) ||
>> - (drc_type && !strcmp(drc_type, drc.drc_type))))
>> - return 0;
>> -
>> - return -EINVAL;
>> -}
>> -
>> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
>> char *drc_type)
>> {
>> - const unsigned int *my_index;
>> -
>> - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> - if (!my_index) {
>> - /* Node isn't DLPAR/hotplug capable */
>> - return -EINVAL;
>> - }
>> -
>> - if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>> - return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
>> - *my_index);
>> - else
>> - return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
>> - *my_index);
>> + return arch_find_drc_match(dn, NULL, drc_type, drc_name,
>> + true, false, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>>
>> -
>> -static int is_php_type(char *drc_type)
>> -{
>> - unsigned long value;
>> - char *endptr;
>> -
>> - /* PCI Hotplug nodes have an integer for drc_type */
>> - value = simple_strtoul(drc_type, &endptr, 10);
>> - if (endptr == drc_type)
>> - return 0;
>> -
>> - return 1;
>> -}
>> -
>> -/**
>> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>> - * @dn: target &device_node
>> - * @indexes: passed to get_children_props()
>> - * @names: passed to get_children_props()
>> - * @types: returned from get_children_props()
>> - * @power_domains:
>> - *
>> - * This routine will return true only if the device node is
>> - * a hotpluggable slot. This routine will return false
>> - * for built-in pci slots (even when the built-in slots are
>> - * dlparable.)
>> - */
>> -static int is_php_dn(struct device_node *dn, const int **indexes,
>> - const int **names, const int **types, const int **power_domains)
>> -{
>> - const int *drc_types;
>> - int rc;
>> -
>> - rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>> - if (rc < 0)
>> - return 0;
>> -
>> - if (!is_php_type((char *) &drc_types[1]))
>> - return 0;
>> -
>> - *types = drc_types;
>> - return 1;
>> -}
>> -
>> /**
>> * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>> * @dn: device node of slot
>> @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
>> *
>> * To remove a slot, it suffices to call rpaphp_deregister_slot().
>> */
>> -int rpaphp_add_slot(struct device_node *dn)
>> +
>> +static int rpaphp_add_slot_cb(struct device_node *dn,
>> + u32 drc_index, char *drc_name, char *drc_type,
>> + u32 drc_power_domain, void *data)
>> {
>> struct slot *slot;
>> int retval = 0;
>> - int i;
>> - const int *indexes, *names, *types, *power_domains;
>> - char *name, *type;
>> -
>> - if (!dn->name || strcmp(dn->name, "pci"))
>> - return 0;
>> -
>> - /* If this is not a hotplug slot, return without doing anything. */
>> - if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
>> - return 0;
>> -
>> - dbg("Entry %s: dn=%pOF\n", __func__, dn);
>>
>> - /* register PCI devices */
>> - name = (char *) &names[1];
>> - type = (char *) &types[1];
>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> - int index;
>> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
>> + if (!slot)
>> + return -ENOMEM;
>>
>> - index = be32_to_cpu(indexes[i + 1]);
>> - slot = alloc_slot_struct(dn, index, name,
>> - be32_to_cpu(power_domains[i + 1]));
>> - if (!slot)
>> - return -ENOMEM;
>> + slot->type = simple_strtoul(drc_type, NULL, 10);
>> + if (retval)
>> + return -EINVAL;
>>
>> - slot->type = simple_strtoul(type, NULL, 10);
>> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>> + drc_index, drc_name, drc_type);
>>
>> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>> - index, name, type);
>> + retval = rpaphp_enable_slot(slot);
>> + if (!retval)
>> + retval = rpaphp_register_slot(slot);
>>
>> - retval = rpaphp_enable_slot(slot);
>> - if (!retval)
>> - retval = rpaphp_register_slot(slot);
>> + if (retval)
>> + dealloc_slot_struct(slot);
>>
>> - if (retval)
>> - dealloc_slot_struct(slot);
>> + return retval;
>> +}
>>
>> - name += strlen(name) + 1;
>> - type += strlen(type) + 1;
>> - }
>> - dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> +int rpaphp_add_slot(struct device_node *dn)
>> +{
>> + if (!dn->name || strcmp(dn->name, "pci"))
>> + return 0;
>>
>> - /* XXX FIXME: reports a failure only if last entry in loop failed */
>> - return retval;
>> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
>> + NULL, NULL, false, true, NULL);
>> }
>> EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>>
>>
Adding Nathan Lynch
On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices. The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>>
>> Signed-off-by: Michael Bringmann <[email protected]>
>> ---
>> include/linux/topology.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
>
> As far as I know pseries is the only platform that uses DR connectors, and I
> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
> that this is really generic enough to belong in topology.h. If anything I would
> suggest putting this in an include in arch/powerpc/include/ named something like
> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
> that want/need to use this functionality.
>
> -Tyrel
>
>>
>> int arch_update_cpu_topology(void);
>>
>> +int arch_find_drc_match(struct device_node *dn,
>> + bool (*usercb)(struct device_node *dn,
>> + u32 drc_index, char *drc_name,
>> + char *drc_type, u32 drc_power_domain,
>> + void *data),
>> + char *opt_drc_type, char *opt_drc_name,
>> + bool match_drc_index, bool ck_php_type,
>> + void *data);
>> +
>> /* Conform to ACPI 2.0 SLIT distance definitions */
>> #define LOCAL_DISTANCE 10
>> #define REMOTE_DISTANCE 20
>>
>
>
--
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]
Adding Nathan Lynch.
Yes. We can amend the title.
On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:51 PM, Michael Bringmann wrote:
>> This patch provides a common interface to parse ibm,drc-indexes,
>> ibm,drc-names, ibm,drc-types, ibm,drc-power-domains, or ibm,drc-info.
>> The generic interface arch_find_drc_match is provided which accepts
>> callback functions that may be applied to examine the data for each
>> entry.
>>
>
> The title of your patch is "pseries/drcinfo: Pseries impl of arch_find_drc_info"
> but the name of the function you are ultimately implementing is
> arch_find_drc_match if I'm not mistaken.
>
>> Signed-off-by: Michael Bringmann <[email protected]>
>> ---
>> arch/powerpc/include/asm/prom.h | 3
>> arch/powerpc/platforms/pseries/of_helpers.c | 299 +++++++++++++++++++++++++++
>> include/linux/topology.h | 2
>> 3 files changed, 298 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index b04c5ce..910d1dc 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -91,9 +91,6 @@ struct of_drc_info {
>> 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.
>> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
>> index 0185e50..11c90cd 100644
>> --- a/arch/powerpc/platforms/pseries/of_helpers.c
>> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
>> @@ -1,5 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0
>>
>> +#define pr_fmt(fmt) "drc: " fmt
>> +
>> #include <linux/string.h>
>> #include <linux/err.h>
>> #include <linux/slab.h>
>> @@ -11,6 +13,12 @@
>>
>> #define MAX_DRC_NAME_LEN 64
>>
>> +static int drc_debug;
>> +#define dbg(args...) if (drc_debug) { printk(KERN_DEBUG args); }
>> +#define err(arg...) printk(KERN_ERR args);
>> +#define info(arg...) printk(KERN_INFO args);
>> +#define warn(arg...) printk(KERN_WARNING args);
>
> Its pretty standard these days to use the pr_debug, pr_err, pr_info, pr_warn
> variations over printk(LEVEL args).
>
>> +
>> /**
>> * pseries_of_derive_parent - basically like dirname(1)
>> * @path: the full_name of a node to be added to the tree
>> @@ -46,7 +54,8 @@ struct device_node *pseries_of_derive_parent(const char *path)
>>
>> /* Helper Routines to convert between drc_index to cpu numbers */
>>
>> -int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>> +static int of_read_drc_info_cell(struct property **prop,
>> + const __be32 **curval,
>> struct of_drc_info *data)
>> {
>> const char *p;
>> @@ -90,4 +99,290 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL(of_read_drc_info_cell);
>> +
>> +static int walk_drc_info(struct device_node *dn,
>> + bool (*usercb)(struct of_drc_info *drc,
>> + void *data,
>> + int *ret_code),
>> + char *opt_drc_type,
>> + void *data)
>> +{
>> + struct property *info;
>> + unsigned int entries;
>> + struct of_drc_info drc;
>> + const __be32 *value;
>> + int j, ret_code = -EINVAL;
>> + bool done = false;
>> +
>> + info = of_find_property(dn, "ibm,drc-info", NULL);
>> + if (info == NULL)
>> + return -EINVAL;
>> +
>> + value = info->value;
>> + entries = of_read_number(value++, 1);
>> +
>> + for (j = 0, done = 0; (j < entries) && (!done); j++) {
>> + of_read_drc_info_cell(&info, &value, &drc);
>> +
>> + if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
>> + continue;
>> +
>> + done = usercb(&drc, data, &ret_code);
>> + }
>> +
>> + return ret_code;
>> +}
>> +
>> +static int get_children_props(struct device_node *dn, const int **drc_indexes,
>> + const int **drc_names, const int **drc_types,
>> + const int **drc_power_domains)
>> +{
>> + const int *indexes, *names, *types, *domains;
>> +
>> + indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> + names = of_get_property(dn, "ibm,drc-names", NULL);
>> + types = of_get_property(dn, "ibm,drc-types", NULL);
>> + domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>> +
>> + if (!indexes || !names || !types || !domains) {
>> + /* Slot does not have dynamically-removable children */
>> + return -EINVAL;
>> + }
>> + if (drc_indexes)
>> + *drc_indexes = indexes;
>> + if (drc_names)
>> + /* &drc_names[1] contains NULL terminated slot names */
>> + *drc_names = names;
>> + if (drc_types)
>> + /* &drc_types[1] contains NULL terminated slot types */
>> + *drc_types = types;
>> + if (drc_power_domains)
>> + *drc_power_domains = domains;
>> +
>> + return 0;
>> +}
>> +
>> +static int is_php_type(char *drc_type)
>> +{
>> + unsigned long value;
>> + char *endptr;
>> +
>> + /* PCI Hotplug nodes have an integer for drc_type */
>> + value = simple_strtoul(drc_type, &endptr, 10);
>> + if (endptr == drc_type)
>> + return 0;
>> +
>> + return 1;
>> +}
>> +
>> +/**
>> + * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>> + * @dn: target &device_node
>> + * @indexes: passed to get_children_props()
>> + * @names: passed to get_children_props()
>> + * @types: returned from get_children_props()
>> + * @power_domains:
>> + *
>> + * This routine will return true only if the device node is
>> + * a hotpluggable slot. This routine will return false
>> + * for built-in pci slots (even when the built-in slots are
>> + * dlparable.)
>> + */
>> +static int is_php_dn(struct device_node *dn, const int **indexes,
>> + const int **names, const int **types,
>> + const int **power_domains)
>> +{
>> + const int *drc_types;
>> + int rc;
>> +
>> + rc = get_children_props(dn, indexes, names, &drc_types,
>> + power_domains);
>> + if (rc < 0)
>> + return 0;
>> +
>> + if (!is_php_type((char *) &drc_types[1]))
>> + return 0;
>> +
>> + *types = drc_types;
>> + return 1;
>> +}
>> +
>> +struct find_drc_match_cb_struct {
>> + struct device_node *dn;
>> + bool (*usercb)(struct device_node *dn,
>> + u32 drc_index, char *drc_name,
>> + char *drc_type, u32 drc_power_domain,
>> + void *data);
>> + char *drc_type;
>> + char *drc_name;
>> + u32 drc_index;
>> + bool match_drc_index;
>> + bool add_slot;
>> + void *data;
>> +};
>> +
>> +static int find_drc_match_v1(struct device_node *dn, void *data)
>> +{
>> + struct find_drc_match_cb_struct *cdata = data;
>> + int i, retval = 0;
>> + const int *indexes, *names, *types, *domains;
>> + char *name, *type;
>> + struct device_node *root = dn;
>> +
>> + if (cdata->match_drc_index)
>> + root = dn->parent;
>> +
>> + if (cdata->add_slot) {
>> + /* If this is not a hotplug slot, return without doing
>> + * anything.
>> + */
>> + if (!is_php_dn(root, &indexes, &names, &types, &domains))
>> + return 0;
>> + } else {
>> + if (get_children_props(root, &indexes, &names, &types,
>> + &domains) < 0)
>> + return 0;
>> + }
>> +
>> + dbg("Entry %s: dn=%pOF\n", __func__, dn);
>> +
>> + name = (char *) &names[1];
>> + type = (char *) &types[1];
>> + for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> +
>> + if (cdata->match_drc_index &&
>> + ((unsigned int) indexes[i + 1] != cdata->drc_index)) {
>> + name += strlen(name) + 1;
>> + type += strlen(type) + 1;
>> + continue;
>> + }
>> +
>> + if (((cdata->drc_name == NULL) ||
>> + (cdata->drc_name && !strcmp(cdata->drc_name, name))) &&
>> + ((cdata->drc_type == NULL) ||
>> + (cdata->drc_type && !strcmp(cdata->drc_type, type)))) {
>> +
>> + if (cdata->usercb) {
>> + retval = cdata->usercb(dn,
>> + be32_to_cpu(indexes[i + 1]),
>> + name, type,
>> + be32_to_cpu(domains[i + 1]),
>> + cdata->data);
>> + if (!retval)
>> + return retval;
>> + } else {
>> + return 0;
>> + }
>> + }
>> +
>> + name += strlen(name) + 1;
>> + type += strlen(type) + 1;
>> + }
>> +
>> + dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> +
>> + /* XXX FIXME: reports a failure only if last entry in loop failed */
>> + return retval;
>> +}
>> +
>> +static bool find_drc_match_v2_cb(struct of_drc_info *drc, void *data,
>> + int *ret_code)
>> +{
>> + struct find_drc_match_cb_struct *cdata = data;
>> + u32 drc_index;
>> + char drc_name[MAX_DRC_NAME_LEN];
>> + int i, retval;
>> +
>> + (*ret_code) = -EINVAL;
>> +
>> + /* This set not a PHP type? */
>> + if (cdata->add_slot) {
>> + if (!is_php_type((char *) drc->drc_type)) {
>> + return false;
>> + }
>> + }
>> +
>> + /* Anything to use from this set? */
>> + if (cdata->match_drc_index && (cdata->drc_index > drc->last_drc_index))
>> + return false;
>> + if ((cdata->drc_type && strcmp(cdata->drc_type, drc->drc_type))
>> + return false;
>> +
>> + /* Check the drc-index entries of this set */
>> + for (i = 0, drc_index = drc->drc_index_start;
>> + i < drc->num_sequential_elems; i++, drc_index++) {
>> +
>> + if (cdata->match_drc_index && (cdata->drc_index != drc_index))
>> + continue;
>> +
>> + sprintf(drc_name, "%s%d", drc->drc_name_prefix,
>> + drc_index - drc->drc_index_start +
>> + drc->drc_name_suffix_start);
>> +
>> + if ((cdata->drc_name == NULL) ||
>> + (cdata->drc_name && !strcmp(cdata->drc_name, drc_name))) {
>> +
>> + if (cdata->usercb) {
>> + retval = cdata->usercb(cdata->dn,
>> + drc_index, drc_name,
>> + drc->drc_type,
>> + drc->drc_power_domain,
>> + cdata->data);
>> + if (!retval) {
>> + (*ret_code) = retval;
>> + return true;
>> + }
>> + } else {
>> + (*ret_code) = 0;
>> + return true;
>> + }
>> + }
>> + }
>> +
>> + (*ret_code) = retval;
>> + return false;
>> +}
>> +
>> +static int find_drc_match_v2(struct device_node *dn, void *data)
>> +{
>> + struct find_drc_match_cb_struct *cdata = data;
>> + struct device_node *root = cdata->dn;
>> +
>> + if (!cdata->add_slot) {
>> + if (!cdata->drc_type ||
>> + (cdata->drc_type && strcmp(cdata->drc_type, "SLOT")))
>> + root = dn->parent;
>> + }
>> +
>> + return walk_drc_info(root, find_drc_match_v2_cb, NULL, data);
>> +}
>> +
>> +int arch_find_drc_match(struct device_node *dn,
>> + bool (*usercb)(struct device_node *dn,
>> + u32 drc_index, char *drc_name,
>> + char *drc_type, u32 drc_power_domain,
>> + void *data),
>> + char *opt_drc_type, char *opt_drc_name,
>> + bool match_drc_index, bool add_slot,
>> + void *data)
>> +{
>> + struct find_drc_match_cb_struct cdata = { dn, usercb,
>> + opt_drc_type, opt_drc_name, 0,
>> + match_drc_index, add_slot, data };
>> +
>> + if (match_drc_index) {
>> + const int *my_index =
>> + of_get_property(dn, "ibm,my-drc-index", NULL);
>> + if (!my_index) {
>> + /* Node isn't DLPAR/hotplug capable */
>> + return -EINVAL;
>> + }
>> + cdata.drc_index = *my_index;
>> + }
>> +
>> + if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>> + return find_drc_match_v2(dn, &cdata);
>> + else
>> + return find_drc_match_v1(dn, &cdata);
>> +}
>> +EXPORT_SYMBOL(arch_find_drc_match);
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index df97f5f..c3dfa53 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -50,7 +50,7 @@ int arch_find_drc_match(struct device_node *dn,
>> char *drc_type, u32 drc_power_domain,
>> void *data),
>> char *opt_drc_type, char *opt_drc_name,
>> - bool match_drc_index, bool ck_php_type,
>> + bool match_drc_index, bool add_slot,
>> void *data);
>
> Why are you making a change to the prototype here? You should just define it in
> your first patch instead of touching it again here.
>
> -Tyrel
>
>>
>> /* Conform to ACPI 2.0 SLIT distance definitions */
>>
>
>
--
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]
Adding Nathan Lynch.
On 1/24/19 6:10 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices. The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>>
>> Signed-off-by: Michael Bringmann <[email protected]>
>> ---
>> include/linux/topology.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
>>
>> int arch_update_cpu_topology(void);
>
> On another note a kern doc comment for this function would also be nice.
>
> -Tyrel
>
>>
>> +int arch_find_drc_match(struct device_node *dn,
>> + bool (*usercb)(struct device_node *dn,
>> + u32 drc_index, char *drc_name,
>> + char *drc_type, u32 drc_power_domain,
>> + void *data),
>> + char *opt_drc_type, char *opt_drc_name,
>> + bool match_drc_index, bool ck_php_type,
>> + void *data);
>> +
>> /* Conform to ACPI 2.0 SLIT distance definitions */
>> #define LOCAL_DISTANCE 10
>> #define REMOTE_DISTANCE 20
>>
>
>
--
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]
Adding Nathan Lynch.
On 1/24/19 6:29 PM, Tyrel Datwyler wrote:
> On 01/14/2019 04:28 PM, Bjorn Helgaas wrote:
>> On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
>>> The implementation of the pseries-specific drc info properties
>>> is currently implemented in pseries-specific and non-pseries-specific
>>> files. This patch set uses a new implementation of the device-tree
>>> parsing code for the properties.
>>>
>>> This patch refactors parsing of the pseries-specific drc-info properties
>>> out of rpaphp_core.c to use the common parser. In the case where an
>>> architecture does not use these properties, an __weak copy of the
>>> function is provided with dummy return values. Changes include creating
>>> appropriate callback functions and passing callback-specific data
>>> blocks into arch_find_drc_match. All functions that were used just
>>> to support the previous parsing implementation have been moved.
>>>
>>> Signed-off-by: Michael Bringmann <[email protected]>
>>
>> This is fine with me. Any rpaphp_core.c maintainers want to comment?
>> Tyrel?
>
> It greatly simplifies the code in rpaphp_core.c, and as far as I can tell the
> refactoring maintains the existing functionality.
>
> Acked-by: Tyrel Datwyler <[email protected]>
>
>>
>> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
>> Tyrel Datwyler <[email protected]> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
>> Benjamin Herrenschmidt <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Paul Mackerras <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Michael Ellerman <[email protected]> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>>
>>> ---
>>> drivers/pci/hotplug/rpaphp_core.c | 232 ++++---------------------------------
>>> 1 file changed, 28 insertions(+), 204 deletions(-)
>>>
>>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>>> index bcd5d35..9ad7384 100644
>>> --- a/drivers/pci/hotplug/rpaphp_core.c
>>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>>> @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>>> return speed;
>>> }
>>>
>>> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
>>> - const int **drc_names, const int **drc_types,
>>> - const int **drc_power_domains)
>>> -{
>>> - const int *indexes, *names, *types, *domains;
>>> -
>>> - indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>>> - names = of_get_property(dn, "ibm,drc-names", NULL);
>>> - types = of_get_property(dn, "ibm,drc-types", NULL);
>>> - domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>>> -
>>> - if (!indexes || !names || !types || !domains) {
>>> - /* Slot does not have dynamically-removable children */
>>> - return -EINVAL;
>>> - }
>>> - if (drc_indexes)
>>> - *drc_indexes = indexes;
>>> - if (drc_names)
>>> - /* &drc_names[1] contains NULL terminated slot names */
>>> - *drc_names = names;
>>> - if (drc_types)
>>> - /* &drc_types[1] contains NULL terminated slot types */
>>> - *drc_types = types;
>>> - if (drc_power_domains)
>>> - *drc_power_domains = domains;
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -
>>> /* Verify the existence of 'drc_name' and/or 'drc_type' within the
>>> - * current node. First obtain it's my-drc-index property. Next,
>>> - * obtain the DRC info from it's parent. Use the my-drc-index for
>>> - * correlation, and obtain/validate the requested properties.
>>> + * current node.
>>> */
>>>
>>> -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
>>> - char *drc_type, unsigned int my_index)
>>> -{
>>> - char *name_tmp, *type_tmp;
>>> - const int *indexes, *names;
>>> - const int *types, *domains;
>>> - int i, rc;
>>> -
>>> - rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>>> - if (rc < 0) {
>>> - return -EINVAL;
>>> - }
>>> -
>>> - name_tmp = (char *) &names[1];
>>> - type_tmp = (char *) &types[1];
>>> -
>>> - /* Iterate through parent properties, looking for my-drc-index */
>>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>>> - if ((unsigned int) indexes[i + 1] == my_index)
>>> - break;
>>> -
>>> - name_tmp += (strlen(name_tmp) + 1);
>>> - type_tmp += (strlen(type_tmp) + 1);
>>> - }
>>> -
>>> - if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
>>> - ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
>>> - return 0;
>>> -
>>> - return -EINVAL;
>>> -}
>>> -
>>> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
>>> - char *drc_type, unsigned int my_index)
>>> -{
>>> - struct property *info;
>>> - unsigned int entries;
>>> - struct of_drc_info drc;
>>> - const __be32 *value;
>>> - char cell_drc_name[MAX_DRC_NAME_LEN];
>>> - int j, fndit;
>>> -
>>> - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>>> - if (info == NULL)
>>> - return -EINVAL;
>>> -
>>> - value = of_prop_next_u32(info, NULL, &entries);
>>> - if (!value)
>>> - return -EINVAL;
>>> -
>>> - for (j = 0; j < entries; j++) {
>>> - of_read_drc_info_cell(&info, &value, &drc);
>>> -
>>> - /* Should now know end of current entry */
>>> -
>>> - if (my_index > drc.last_drc_index)
>>> - continue;
>>> -
>>> - fndit = 1;
>>> - break;
>>> - }
>>> - /* Found it */
>>> -
>>> - if (fndit)
>>> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
>>> - my_index);
>>> -
>>> - if (((drc_name == NULL) ||
>>> - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
>>> - ((drc_type == NULL) ||
>>> - (drc_type && !strcmp(drc_type, drc.drc_type))))
>>> - return 0;
>>> -
>>> - return -EINVAL;
>>> -}
>>> -
>>> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
>>> char *drc_type)
>>> {
>>> - const unsigned int *my_index;
>>> -
>>> - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>>> - if (!my_index) {
>>> - /* Node isn't DLPAR/hotplug capable */
>>> - return -EINVAL;
>>> - }
>>> -
>>> - if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>>> - return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
>>> - *my_index);
>>> - else
>>> - return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
>>> - *my_index);
>>> + return arch_find_drc_match(dn, NULL, drc_type, drc_name,
>>> + true, false, NULL);
>>> }
>>> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>>>
>>> -
>>> -static int is_php_type(char *drc_type)
>>> -{
>>> - unsigned long value;
>>> - char *endptr;
>>> -
>>> - /* PCI Hotplug nodes have an integer for drc_type */
>>> - value = simple_strtoul(drc_type, &endptr, 10);
>>> - if (endptr == drc_type)
>>> - return 0;
>>> -
>>> - return 1;
>>> -}
>>> -
>>> -/**
>>> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>>> - * @dn: target &device_node
>>> - * @indexes: passed to get_children_props()
>>> - * @names: passed to get_children_props()
>>> - * @types: returned from get_children_props()
>>> - * @power_domains:
>>> - *
>>> - * This routine will return true only if the device node is
>>> - * a hotpluggable slot. This routine will return false
>>> - * for built-in pci slots (even when the built-in slots are
>>> - * dlparable.)
>>> - */
>>> -static int is_php_dn(struct device_node *dn, const int **indexes,
>>> - const int **names, const int **types, const int **power_domains)
>>> -{
>>> - const int *drc_types;
>>> - int rc;
>>> -
>>> - rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>>> - if (rc < 0)
>>> - return 0;
>>> -
>>> - if (!is_php_type((char *) &drc_types[1]))
>>> - return 0;
>>> -
>>> - *types = drc_types;
>>> - return 1;
>>> -}
>>> -
>>> /**
>>> * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>>> * @dn: device node of slot
>>> @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
>>> *
>>> * To remove a slot, it suffices to call rpaphp_deregister_slot().
>>> */
>>> -int rpaphp_add_slot(struct device_node *dn)
>>> +
>>> +static int rpaphp_add_slot_cb(struct device_node *dn,
>>> + u32 drc_index, char *drc_name, char *drc_type,
>>> + u32 drc_power_domain, void *data)
>>> {
>>> struct slot *slot;
>>> int retval = 0;
>>> - int i;
>>> - const int *indexes, *names, *types, *power_domains;
>>> - char *name, *type;
>>> -
>>> - if (!dn->name || strcmp(dn->name, "pci"))
>>> - return 0;
>>> -
>>> - /* If this is not a hotplug slot, return without doing anything. */
>>> - if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
>>> - return 0;
>>> -
>>> - dbg("Entry %s: dn=%pOF\n", __func__, dn);
>>>
>>> - /* register PCI devices */
>>> - name = (char *) &names[1];
>>> - type = (char *) &types[1];
>>> - for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>>> - int index;
>>> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
>>> + if (!slot)
>>> + return -ENOMEM;
>>>
>>> - index = be32_to_cpu(indexes[i + 1]);
>>> - slot = alloc_slot_struct(dn, index, name,
>>> - be32_to_cpu(power_domains[i + 1]));
>>> - if (!slot)
>>> - return -ENOMEM;
>>> + slot->type = simple_strtoul(drc_type, NULL, 10);
>>> + if (retval)
>>> + return -EINVAL;
>>>
>>> - slot->type = simple_strtoul(type, NULL, 10);
>>> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>>> + drc_index, drc_name, drc_type);
>>>
>>> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>>> - index, name, type);
>>> + retval = rpaphp_enable_slot(slot);
>>> + if (!retval)
>>> + retval = rpaphp_register_slot(slot);
>>>
>>> - retval = rpaphp_enable_slot(slot);
>>> - if (!retval)
>>> - retval = rpaphp_register_slot(slot);
>>> + if (retval)
>>> + dealloc_slot_struct(slot);
>>>
>>> - if (retval)
>>> - dealloc_slot_struct(slot);
>>> + return retval;
>>> +}
>>>
>>> - name += strlen(name) + 1;
>>> - type += strlen(type) + 1;
>>> - }
>>> - dbg("%s - Exit: rc[%d]\n", __func__, retval);
>>> +int rpaphp_add_slot(struct device_node *dn)
>>> +{
>>> + if (!dn->name || strcmp(dn->name, "pci"))
>>> + return 0;
>>>
>>> - /* XXX FIXME: reports a failure only if last entry in loop failed */
>>> - return retval;
>>> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
>>> + NULL, NULL, false, true, NULL);
>>> }
>>> EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>>>
>>>
>
>
--
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]
On 1/25/19 10:09 AM, Michael Bringmann wrote:
> Adding Nathan Lynch
>
> On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
>> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>>> Define interface to acquire arch-specific drc info to match against
>>> hotpluggable devices. The current implementation exposes several
>>> pseries-specific dynamic memory properties in generic kernel code.
>>> This patch set provides an interface to pull that code out of the
>>> generic kernel.
>>>
>>> Signed-off-by: Michael Bringmann <[email protected]>
>>> ---
>>> include/linux/topology.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>>> index cb0775e..df97f5f 100644
>>> --- a/include/linux/topology.h
>>> +++ b/include/linux/topology.h
>>> @@ -44,6 +44,15 @@
>>
>> As far as I know pseries is the only platform that uses DR connectors, and I
>> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
>> that this is really generic enough to belong in topology.h. If anything I would
>> suggest putting this in an include in arch/powerpc/include/ named something like
>> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
>> that want/need to use this functionality.
It looks like the 'rpaphp' and 'rpadlpar_io' modules are also dependent upon the
powerpc platform. Shouldn't the relevant source files be moved completely to the
powerpc-specific directories out of drivers/pci/hotplug as well?
drivers/pci/hotplug/Kconfig has:
config HOTPLUG_PCI_RPA
tristate "RPA PCI Hotplug driver"
depends on PPC_PSERIES && EEH
help
Say Y here if you have a RPA system that supports PCI Hotplug.
To compile this driver as a module, choose M here: the
module will be called rpaphp.
When in doubt, say N.
config HOTPLUG_PCI_RPA_DLPAR
tristate "RPA Dynamic Logical Partitioning for I/O slots"
depends on HOTPLUG_PCI_RPA
help
Say Y here if your system supports Dynamic Logical Partitioning
for I/O slots.
To compile this driver as a module, choose M here: the
module will be called rpadlpar_io.
When in doubt, say N.
Michael
>>
>> -Tyrel
>>
>>>
>>> int arch_update_cpu_topology(void);
>>>
>>> +int arch_find_drc_match(struct device_node *dn,
>>> + bool (*usercb)(struct device_node *dn,
>>> + u32 drc_index, char *drc_name,
>>> + char *drc_type, u32 drc_power_domain,
>>> + void *data),
>>> + char *opt_drc_type, char *opt_drc_name,
>>> + bool match_drc_index, bool ck_php_type,
>>> + void *data);
>>> +
>>> /* Conform to ACPI 2.0 SLIT distance definitions */
>>> #define LOCAL_DISTANCE 10
>>> #define REMOTE_DISTANCE 20
>>>
>>
>>
>
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]
Michael Bringmann <[email protected]> writes:
> On 1/25/19 10:09 AM, Michael Bringmann wrote:
>> Adding Nathan Lynch
>>
>> On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
>>> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>>>> Define interface to acquire arch-specific drc info to match against
>>>> hotpluggable devices. The current implementation exposes several
>>>> pseries-specific dynamic memory properties in generic kernel code.
>>>> This patch set provides an interface to pull that code out of the
>>>> generic kernel.
>>>>
>>>> Signed-off-by: Michael Bringmann <[email protected]>
>>>> ---
>>>> include/linux/topology.h | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>>>> index cb0775e..df97f5f 100644
>>>> --- a/include/linux/topology.h
>>>> +++ b/include/linux/topology.h
>>>> @@ -44,6 +44,15 @@
>>>
>>> As far as I know pseries is the only platform that uses DR connectors, and I
>>> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
>>> that this is really generic enough to belong in topology.h. If anything I would
>>> suggest putting this in an include in arch/powerpc/include/ named something like
>>> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
>>> that want/need to use this functionality.
>
> It looks like the 'rpaphp' and 'rpadlpar_io' modules are also dependent upon the
> powerpc platform.
Yes that's right.
> Shouldn't the relevant source files be moved completely to the
> powerpc-specific directories out of drivers/pci/hotplug as well?
I don't think so. They are PCI hotplug drivers, so they should sit with
the other PCI hotplug drivers.
It's true that PCI hotplug drivers are more platform specific than other
types of drivers, but still they have some things in common with other
PCI hotplug drivers.
cheers
Tyrel Datwyler <[email protected]> writes:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices. The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>>
>> Signed-off-by: Michael Bringmann <[email protected]>
>> ---
>> include/linux/topology.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
>
> As far as I know pseries is the only platform that uses DR connectors, and I
> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
> that this is really generic enough to belong in topology.h.
Right. This does not belong in include/linux.
> If anything I would
> suggest putting this in an include in arch/powerpc/include/ named something like
> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
> that want/need to use this functionality.
Yeah that would make more sense.
Using "arch" in the name is wrong, it's pseries specific so
pseries_find_drc_match() would be more appropriate.
>> +int arch_find_drc_match(struct device_node *dn,
>> + bool (*usercb)(struct device_node *dn,
>> + u32 drc_index, char *drc_name,
>> + char *drc_type, u32 drc_power_domain,
>> + void *data),
>> + char *opt_drc_type, char *opt_drc_name,
>> + bool match_drc_index, bool ck_php_type,
>> + void *data);
This function signature is kind of insane.
You end with calls like:
+ return arch_find_drc_match(dn, rpaphp_add_slot_cb,
+ NULL, NULL, false, true, NULL);
Which is impossible to parse.
I feel like maybe this isn't the right level of abstraction.
cheers
On 1/29/19 3:31 AM, Michael Ellerman wrote:
> Tyrel Datwyler <[email protected]> writes:
>> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>>> Define interface to acquire arch-specific drc info to match against
>>> hotpluggable devices. The current implementation exposes several
>>> pseries-specific dynamic memory properties in generic kernel code.
>>> This patch set provides an interface to pull that code out of the
>>> generic kernel.
>>>
>>> Signed-off-by: Michael Bringmann <[email protected]>
>>> ---
>>> include/linux/topology.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>>> index cb0775e..df97f5f 100644
>>> --- a/include/linux/topology.h
>>> +++ b/include/linux/topology.h
>>> @@ -44,6 +44,15 @@
>>
>> As far as I know pseries is the only platform that uses DR connectors, and I
>> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
>> that this is really generic enough to belong in topology.h.
>
> Right. This does not belong in include/linux.
>
>> If anything I would
>> suggest putting this in an include in arch/powerpc/include/ named something like
>> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
>> that want/need to use this functionality.
>
> Yeah that would make more sense.
If you see no objection to referencing a powerpc-specific function from
the code ...
>
> Using "arch" in the name is wrong, it's pseries specific so
> pseries_find_drc_match() would be more appropriate.
>
>>> +int arch_find_drc_match(struct device_node *dn,
>>> + bool (*usercb)(struct device_node *dn,
>>> + u32 drc_index, char *drc_name,
>>> + char *drc_type, u32 drc_power_domain,
>>> + void *data),
>>> + char *opt_drc_type, char *opt_drc_name,
>>> + bool match_drc_index, bool ck_php_type,
>>> + void *data);
>
> This function signature is kind of insane.
>
> You end with calls like:
>
> + return arch_find_drc_match(dn, rpaphp_add_slot_cb,
> + NULL, NULL, false, true, NULL);
>
> Which is impossible to parse.
>
> I feel like maybe this isn't the right level of abstraction.
...
I had already been considering simplifying the interface for these
calls to something like the following:
int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
char *drc_type)
{
return pseries_find_drc_match(dn, drc_type, drc_name);
}
...
int rpaphp_add_slot(struct device_node *dn)
{
if (!dn->name || strcmp(dn->name, "pci"))
return 0;
return pseries_add_drc_slot(dn, rpaphp_add_slot_cb);
}
...
Further details would be hidden within the pseries code.
>
> cheers
Regards
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
[email protected]