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 <m...@linux.vnet.ibm.com>
>> ---
>>  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
m...@linux.vnet.ibm.com

Reply via email to