[RFC,5/6] powerpc/pci/hotplug: Use common drcinfo parsing
diff mbox series

Message ID 20181214205120.16435.46952.stgit@powerkvm6.aus.stglabs.ibm.com
State Changes Requested
Headers show
Series
  • powerpc/pseries: Refactor code to centralize drcinfo parsing
Related show

Checks

Context Check Description
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 2 checks, 264 lines checked
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied

Commit Message

Michael Bringmann Dec. 14, 2018, 8:51 p.m. UTC
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 <mwb@linux.vnet.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c |  232 ++++---------------------------------
 1 file changed, 28 insertions(+), 204 deletions(-)

Comments

Bjorn Helgaas Jan. 15, 2019, 12:28 a.m. UTC | #1
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 <mwb@linux.vnet.ibm.com>

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 <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
Michael Ellerman <mpe@ellerman.id.au> (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);
>  
>
Bjorn Helgaas Jan. 22, 2019, 7:58 p.m. UTC | #2
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 <mwb@linux.vnet.ibm.com>
> 
> 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 <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Michael Ellerman <mpe@ellerman.id.au> (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 <bhelgaas@google.com>

> > ---
> >  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);
> >  
> >
Tyrel Datwyler Jan. 25, 2019, 12:29 a.m. UTC | #3
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 <mwb@linux.vnet.ibm.com>
> 
> 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 <tyreld@linux.vnet.ibm.com>

> 
> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
> Tyrel Datwyler <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
> Michael Ellerman <mpe@ellerman.id.au> (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 Bringmann Jan. 25, 2019, 4:12 p.m. UTC | #4
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 <mwb@linux.vnet.ibm.com>
>>
>> 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 <tyreld@linux.vnet.ibm.com>
> 
>>
>> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
>> Tyrel Datwyler <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Michael Ellerman <mpe@ellerman.id.au> (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);
>>>  
>>>
> 
>

Patch
diff mbox series

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);