See below. On 05/22/2018 04:39 PM, Nathan Fontenot wrote: > On 05/22/2018 11:37 AM, Michael Bringmann wrote: >> This patch extends the use of a common parse function for the >> ibm,drc-info property that can be modified by a callback function >> to the hotplug device processing. Candidate code is replaced by >> a call to the parser including a pointer to a local context-specific >> functions, and local data. >> >> In addition, several more opportunities to compress and reuse >> common code between the old and new property parsers were applied. >> >> Finally, a bug with the registration of slots was observed on some >> systems, and the code was rewritten to prevent its reoccurrence. >> >> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com> >> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info >> firmwar >> e feature" -- end of patch series applied to powerpc next) >> --- >> Changes in V4: >> -- Update code to account for latest kernel checkins. >> -- Fix bug searching for virtual device slots. >> -- Rebased to 4.17-rc5 kernel >> -- Patch cleanup >> --- >> drivers/pci/hotplug/rpaphp_core.c | 181 >> ++++++++++++++++++++++++++----------- >> 1 file changed, 126 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/pci/hotplug/rpaphp_core.c >> b/drivers/pci/hotplug/rpaphp_core.c >> index 435c1a0..dc4ec68 100644 >> --- a/drivers/pci/hotplug/rpaphp_core.c >> +++ b/drivers/pci/hotplug/rpaphp_core.c >> @@ -222,47 +222,51 @@ static int rpaphp_check_drc_props_v1(struct >> device_node *dn, char *drc_name, >> return -EINVAL; >> } >> >> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name, >> - char *drc_type, unsigned int my_index) >> +struct check_drc_props_v2_struct { >> + char *drc_name; >> + char *drc_type; >> + unsigned int my_index; >> +}; >> + >> +static int check_drc_props_v2_cb(struct of_drc_info *drc, void *idata, >> + void *not_used, int *ret_code) >> { >> - struct property *info; >> - unsigned int entries; >> - struct of_drc_info drc; >> - const __be32 *value; >> + struct check_drc_props_v2_struct *cdata = idata; >> 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; >> + (*ret_code) = -EINVAL; >> >> - value = info->value; >> - entries = of_read_number(value++, 1); >> - >> - 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; >> + if (cdata->my_index > drc->last_drc_index) >> + return 0; >> >> - fndit = 1; >> - break; >> + /* Found drc_index. Now match the rest. */ >> + sprintf(cell_drc_name, "%s%d", drc->drc_name_prefix, >> + cdata->my_index - drc->drc_index_start + >> + drc->drc_name_suffix_start); >> + >> + if (((cdata->drc_name == NULL) || >> + (cdata->drc_name && !strcmp(cdata->drc_name, cell_drc_name))) && >> + ((cdata->drc_type == NULL) || >> + (cdata->drc_type && !strcmp(cdata->drc_type, drc->drc_type)))) { >> + (*ret_code) = 0; >> + return 1; >> } >> - /* Found it */ >> >> - if (fndit) >> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix, >> - my_index); >> + return 0; >> +} >> >> - 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; >> +static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name, >> + char *drc_type, unsigned int my_index) >> +{ >> + struct device_node *root = dn; >> + struct check_drc_props_v2_struct cdata = { >> + drc_name, drc_type, be32_to_cpu(my_index) }; >> >> - return -EINVAL; >> + if (!drc_type || (drc_type && strcmp(drc_type, "SLOT"))) >> + root = dn->parent; >> + >> + return drc_info_parser(root, check_drc_props_v2_cb, >> + drc_type, &cdata); >> } >> >> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name, >> @@ -285,7 +289,6 @@ int rpaphp_check_drc_props(struct device_node *dn, char >> *drc_name, >> } >> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props); >> >> - >> static int is_php_type(char *drc_type) >> { >> unsigned long value; >> @@ -345,17 +348,41 @@ 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_common(struct device_node *dn, >> + u32 drc_index, char *drc_name, char *drc_type, >> + u32 drc_power_domain) >> { >> struct slot *slot; >> int retval = 0; >> - int i; >> + >> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain); >> + if (!slot) >> + return -ENOMEM; >> + >> + slot->type = simple_strtoul(drc_type, NULL, 10); >> + if (retval) >> + return -EINVAL; >> + >> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n", >> + drc_index, drc_name, drc_type); >> + >> + retval = rpaphp_enable_slot(slot); >> + if (!retval) >> + retval = rpaphp_register_slot(slot); >> + >> + if (retval) >> + dealloc_slot_struct(slot); >> + >> + return retval; >> +} >> + >> +static int rpaphp_add_slot_v1(struct device_node *dn) >> +{ >> + int i, retval = 0; >> 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; >> @@ -366,25 +393,12 @@ int rpaphp_add_slot(struct device_node *dn) >> name = (char *) &names[1]; >> type = (char *) &types[1]; >> for (i = 0; i < be32_to_cpu(indexes[0]); i++) { >> - int index; >> - >> - 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(type, NULL, 10); >> >> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n", >> - index, name, type); >> - >> - retval = rpaphp_enable_slot(slot); >> + retval = rpaphp_add_slot_common(dn, >> + be32_to_cpu(indexes[i + 1]), name, type, >> + be32_to_cpu(power_domains[i + 1])); >> if (!retval) >> - retval = rpaphp_register_slot(slot); >> - >> - if (retval) >> - dealloc_slot_struct(slot); >> + return retval; >> >> name += strlen(name) + 1; >> type += strlen(type) + 1; >> @@ -394,6 +408,63 @@ int rpaphp_add_slot(struct device_node *dn) >> /* XXX FIXME: reports a failure only if last entry in loop failed */ >> return retval; >> } >> + >> +struct rpaphp_add_slot_v2_struct { >> + struct device_node *dn; >> +}; >> + >> +static int rpaphp_add_slot_v2_cb(struct of_drc_info *drc, void *idata, >> + void *not_used, int *ret_code) >> +{ >> + struct rpaphp_add_slot_v2_struct *cdata = idata; >> + u32 drc_index; >> + char drc_name[MAX_DRC_NAME_LEN]; >> + int i, retval; >> + >> + (*ret_code) = -EINVAL; >> + >> + if (!is_php_type((char *) drc->drc_type)) { >> + (*ret_code) = 0; >> + return 1; >> + } >> + >> + for (i = 0, drc_index = drc->drc_index_start; >> + i < drc->num_sequential_elems; i++, drc_index++) { >> + >> + sprintf(drc_name, "%s%d", drc->drc_name_prefix, >> + drc_index - drc->drc_index_start + >> + drc->drc_name_suffix_start); >> + >> + retval = rpaphp_add_slot_common(cdata->dn, >> + drc_index, drc_name, drc->drc_type, >> + drc->drc_power_domain); >> + if (!retval) { >> + (*ret_code) = retval; >> + return 1; >> + } >> + } > > I may be reading this loop wrong, but it appears that it will add all drc > indexes > for all drc_info entries to the specified device node, minus drc-types that > don't match.
It is supposed to exit the function after the first successful call to rpaphp_add_slot_common(). That function is supposed to try to allocate/enable a slot, and return 0 when it succeeds. Will reconfirm this week. > > -Nathan Michael > >> + >> + (*ret_code) = retval; >> + return 0; >> +} >> + >> +static int rpaphp_add_slot_v2(struct device_node *dn) >> +{ >> + struct rpaphp_add_slot_v2_struct cdata = { dn }; >> + >> + return drc_info_parser(dn, rpaphp_add_slot_v2_cb, NULL, &cdata); >> +} >> + >> +int rpaphp_add_slot(struct device_node *dn) >> +{ >> + if (!dn->name || strcmp(dn->name, "pci")) >> + return 0; >> + >> + if (firmware_has_feature(FW_FEATURE_DRC_INFO)) >> + return rpaphp_add_slot_v2(dn); >> + else >> + return rpaphp_add_slot_v1(dn); >> +} >> EXPORT_SYMBOL_GPL(rpaphp_add_slot); >> >> static void __exit cleanup_slots(void) >> > > -- 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