lkml.org 
[lkml]   [2019]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC 5/6] powerpc/pci/hotplug: Use common drcinfo parsing
    From
    Date
    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);
    >>>
    >>>
    >
    >

    --
    Michael W. Bringmann
    Linux I/O, Networking and Security Development
    IBM Corporation
    Tie-Line 363-5196
    External: (512) 286-5196
    Cell: (512) 466-0650
    mwb@linux.vnet.ibm.com

    \
     
     \ /
      Last update: 2019-01-25 17:12    [W:4.192 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site