Re: [RFC v4 4/4] hotplug/drcinfo: Code cleanup for devices
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
>> 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
Re: [RFC v4 4/4] hotplug/drcinfo: Code cleanup for devices
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
> 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
[RFC v4 4/4] hotplug/drcinfo: Code cleanup for devices
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
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_strtou
