See below.
On 11/30/2017 01:51 PM, Nathan Fontenot wrote:
>
>
> On 11/28/2017 05:07 PM, Michael Bringmann wrote:
>> rpadlpar_core.c: Provide parallel routines to search the older device-
>> tree properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
>> and "ibm,drc-power-domains"), or the new property "ibm,drc-info".
>>
>> The interface to examine the DRC information is changed from a "get"
>> function that returns values for local verification elsewhere, to a
>> "check" function that validates the 'name' and/or 'type' of a device
>> node. This update hides the format of the underlying device-tree
>> properties, and concentrates the value checks into a single function
>> without requiring the user to verify whether a search was successful.
>>
>> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
>> ---
>> Changes in V5:
>> -- Simplify of_prop_next_u32 invocation
>> -- Fix some spacing within arguments
>> ---
>> drivers/pci/hotplug/rpadlpar_core.c | 13 ++--
>> drivers/pci/hotplug/rpaphp.h | 4 +
>> drivers/pci/hotplug/rpaphp_core.c | 109
>> +++++++++++++++++++++++++++--------
>> 3 files changed, 91 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/rpadlpar_core.c
>> b/drivers/pci/hotplug/rpadlpar_core.c
>> index a3449d7..fc01d7d 100644
>> --- a/drivers/pci/hotplug/rpadlpar_core.c
>> +++ b/drivers/pci/hotplug/rpadlpar_core.c
>> @@ -27,6 +27,7 @@
>> #include <linux/mutex.h>
>> #include <asm/rtas.h>
>> #include <asm/vio.h>
>> +#include <linux/firmware.h>
>>
>> #include "../pci.h"
>> #include "rpaphp.h"
>> @@ -44,15 +45,14 @@ static struct device_node *find_vio_slot_node(char
>> *drc_name)
>> {
>> struct device_node *parent = of_find_node_by_name(NULL, "vdevice");
>> struct device_node *dn = NULL;
>> - char *name;
>> int rc;
>>
>> if (!parent)
>> return NULL;
>>
>> while ((dn = of_get_next_child(parent, dn))) {
>> - rc = rpaphp_get_drc_props(dn, NULL, &name, NULL, NULL);
>> - if ((rc == 0) && (!strcmp(drc_name, name)))
>> + rc = rpaphp_check_drc_props(dn, drc_name, NULL);
>> + if (rc == 0)
>> break;
>> }
>>
>> @@ -64,15 +64,12 @@ static struct device_node *find_php_slot_pci_node(char
>> *drc_name,
>> char *drc_type)
>> {
>> struct device_node *np = NULL;
>> - char *name;
>> - char *type;
>> int rc;
>>
>> while ((np = of_find_node_by_name(np, "pci"))) {
>> - rc = rpaphp_get_drc_props(np, NULL, &name, &type, NULL);
>> + rc = rpaphp_check_drc_props(np, drc_name, drc_type);
>> if (rc == 0)
>> - if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
>> - break;
>> + break;
>> }
>>
>> return np;
>> diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
>> index 7db024e..8db5f2e 100644
>> --- a/drivers/pci/hotplug/rpaphp.h
>> +++ b/drivers/pci/hotplug/rpaphp.h
>> @@ -91,8 +91,8 @@ struct slot {
>>
>> /* rpaphp_core.c */
>> int rpaphp_add_slot(struct device_node *dn);
>> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> - char **drc_name, char **drc_type, int *drc_power_domain);
>> +int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
>> + char *drc_type);
>>
>> /* rpaphp_slot.c */
>> void dealloc_slot_struct(struct slot *slot);
>> diff --git a/drivers/pci/hotplug/rpaphp_core.c
>> b/drivers/pci/hotplug/rpaphp_core.c
>> index 1e29aba..6da613a 100644
>> --- a/drivers/pci/hotplug/rpaphp_core.c
>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>> @@ -30,6 +30,7 @@
>> #include <linux/smp.h>
>> #include <linux/init.h>
>> #include <linux/vmalloc.h>
>> +#include <asm/firmware.h>
>> #include <asm/eeh.h> /* for eeh_add_device() */
>> #include <asm/rtas.h> /* rtas_call */
>> #include <asm/pci-bridge.h> /* for pci_controller */
>> @@ -196,25 +197,21 @@ static int get_children_props(struct device_node *dn,
>> const int **drc_indexes,
>> return 0;
>> }
>>
>> -/* To get the DRC props describing the current node, first obtain it's
>> - * my-drc-index property. Next obtain the DRC list from it's parent. Use
>> - * the my-drc-index for correlation, and obtain the requested properties.
>> +
>> +/* 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.
>> */
>> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
>> - char **drc_name, char **drc_type, int *drc_power_domain)
>> +
>> +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;
>> - const unsigned int *my_index;
>> - char *name_tmp, *type_tmp;
>> int i, rc;
>>
>> - my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>> - if (!my_index) {
>> - /* Node isn't DLPAR/hotplug capable */
>> - return -EINVAL;
>> - }
>> -
>> rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>> if (rc < 0) {
>> return -EINVAL;
>> @@ -225,24 +222,86 @@ int rpaphp_get_drc_props(struct device_node *dn, int
>> *drc_index,
>>
>> /* 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) {
>> - if (drc_name)
>> - *drc_name = name_tmp;
>> - if (drc_type)
>> - *drc_type = type_tmp;
>> - if (drc_index)
>> - *drc_index = be32_to_cpu(*my_index);
>> - if (drc_power_domain)
>> - *drc_power_domain = be32_to_cpu(domains[i+1]);
>> - return 0;
>> - }
>> + 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;
>> + int j;
>> +
>> + info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>> + if (info == NULL)
>> + return -EINVAL;
>> +
>> + value = (void *)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 */
>> +
>> + WARN_ON((my_index < drc.drc_index_start) ||
>> + (((my_index - drc.drc_index_start) %
>> + drc.sequential_inc) != 0));
>> +
>> + if (my_index > drc.last_drc_index)
>> + continue;
>> +
>> + break;
>> + }
>> + /* Found it */
>> +
>> + if (((drc_name == NULL) ||
>> + (drc_name && !strncmp(drc_name,
>> + drc.drc_name_prefix,
>> + strlen(drc.drc_name_prefix)))) &&
>
> I'm still not convinced this is correct. If I understand correctly
> the new drc-info property has a name prefix field, which for the
> entries we are looking up here are likely "PHB", and a name suffix
> start filed for the first number to use as the suffix to the name in
> the current set.
>
> The name we would be trying to compare to would be something like "PHB 34".
> What you have is just doing a compare on the prefix, or "PHB", part of
> the name and not the full name.
I reviewed the execution again, and I agree with you. Updates to correct
the check will be in the next submission.
>
> -Nathan
Michael
>
>> + ((drc_type == NULL) ||
>> + (drc_type && !strncmp(drc_type,
>> + drc.drc_type,
>> + strlen(drc.drc_type)))))
>> + return 0;
>> +
>> return -EINVAL;
>> }
>> -EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
>> +
>> +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);
>> +}
>> +EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>> +
>>
>> static int is_php_type(char *drc_type)
>> {
>>
>
>
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
m...@linux.vnet.ibm.com
