diff mbox series

[1/4] libndctl: Unify adding dimms for papr and nfit families

Message ID 20210328021001.2340251-1-santosh@fossix.org (mailing list archive)
State Superseded
Headers show
Series [1/4] libndctl: Unify adding dimms for papr and nfit families | expand

Commit Message

Santosh Sivaraj March 28, 2021, 2:09 a.m. UTC 
In preparation for enabling tests on non-nfit devices, unify both, already very
similar, functions into one. This will help in adding all attributes needed for
the unit tests. Since the function doesn't fail if some of the dimm attributes
are missing, this will work fine on PAPR platforms though only part of the DIMM
attributes are provided (This doesn't mean that all of the DIMM attributes can
be missing).

Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
 ndctl/lib/libndctl.c | 103 ++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 65 deletions(-)

Comments

Santosh Sivaraj March 28, 2021, 2:15 a.m. UTC | #1 
Sorry, I missed to provide the version in the subject, this is v4 of the series.

Santosh Sivaraj <santosh@fossix.org> writes:

> In preparation for enabling tests on non-nfit devices, unify both, already very
> similar, functions into one. This will help in adding all attributes needed for
> the unit tests. Since the function doesn't fail if some of the dimm attributes
> are missing, this will work fine on PAPR platforms though only part of the DIMM
> attributes are provided (This doesn't mean that all of the DIMM attributes can
> be missing).
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
>  ndctl/lib/libndctl.c | 103 ++++++++++++++++---------------------------
>  1 file changed, 38 insertions(+), 65 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 36fb6fe..26b9317 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1646,41 +1646,9 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
>  static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
>  static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
>  
> -static int add_papr_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> -{
> -	int rc = -ENODEV;
> -	char buf[SYSFS_ATTR_SIZE];
> -	struct ndctl_ctx *ctx = dimm->bus->ctx;
> -	char *path = calloc(1, strlen(dimm_base) + 100);
> -	const char * const devname = ndctl_dimm_get_devname(dimm);
> -
> -	dbg(ctx, "%s: Probing of_pmem dimm at %s\n", devname, dimm_base);
> -
> -	if (!path)
> -		return -ENOMEM;
> -
> -	/* construct path to the papr compatible dimm flags file */
> -	sprintf(path, "%s/papr/flags", dimm_base);
> -
> -	if (ndctl_bus_is_papr_scm(dimm->bus) &&
> -	    sysfs_read_attr(ctx, path, buf) == 0) {
> -
> -		dbg(ctx, "%s: Adding papr-scm dimm flags:\"%s\"\n", devname, buf);
> -		dimm->cmd_family = NVDIMM_FAMILY_PAPR;
> -
> -		/* Parse dimm flags */
> -		parse_papr_flags(dimm, buf);
> -
> -		/* Allocate monitor mode fd */
> -		dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
> -		rc = 0;
> -	}
> -
> -	free(path);
> -	return rc;
> -}
> -
> -static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> +static int populate_dimm_attributes(struct ndctl_dimm *dimm,
> +				    const char *dimm_base,
> +				    const char *bus_prefix)
>  {
>  	int i, rc = -1;
>  	char buf[SYSFS_ATTR_SIZE];
> @@ -1694,7 +1662,7 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>  	 * 'unique_id' may not be available on older kernels, so don't
>  	 * fail if the read fails.
>  	 */
> -	sprintf(path, "%s/nfit/id", dimm_base);
> +	sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0) {
>  		unsigned int b[9];
>  
> @@ -1709,68 +1677,74 @@ static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>  		}
>  	}
>  
> -	sprintf(path, "%s/nfit/handle", dimm_base);
> +	sprintf(path, "%s/%s/handle", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		goto err_read;
>  	dimm->handle = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/phys_id", dimm_base);
> +	sprintf(path, "%s/%s/phys_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		goto err_read;
>  	dimm->phys_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/serial", dimm_base);
> +	sprintf(path, "%s/%s/serial", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->serial = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/vendor", dimm_base);
> +	sprintf(path, "%s/%s/vendor", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->vendor_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/device", dimm_base);
> +	sprintf(path, "%s/%s/device", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->device_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/rev_id", dimm_base);
> +	sprintf(path, "%s/%s/rev_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->revision_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/dirty_shutdown", dimm_base);
> +	sprintf(path, "%s/%s/dirty_shutdown", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->dirty_shutdown = strtoll(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_vendor", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_vendor", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_device", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_device", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_device_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/subsystem_rev_id", dimm_base);
> +	sprintf(path, "%s/%s/subsystem_rev_id", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->subsystem_revision_id = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/family", dimm_base);
> +	sprintf(path, "%s/%s/family", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->cmd_family = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/dsm_mask", dimm_base);
> +	sprintf(path, "%s/%s/dsm_mask", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/format", dimm_base);
> +	sprintf(path, "%s/%s/format", dimm_base, bus_prefix);
>  	if (sysfs_read_attr(ctx, path, buf) == 0)
>  		dimm->format[0] = strtoul(buf, NULL, 0);
>  	for (i = 1; i < dimm->formats; i++) {
> -		sprintf(path, "%s/nfit/format%d", dimm_base, i);
> +		sprintf(path, "%s/%s/format%d", dimm_base, bus_prefix, i);
>  		if (sysfs_read_attr(ctx, path, buf) == 0)
>  			dimm->format[i] = strtoul(buf, NULL, 0);
>  	}
>  
> -	sprintf(path, "%s/nfit/flags", dimm_base);
> -	if (sysfs_read_attr(ctx, path, buf) == 0)
> -		parse_nfit_mem_flags(dimm, buf);
> +	sprintf(path, "%s/%s/flags", dimm_base, bus_prefix);
> +	if (sysfs_read_attr(ctx, path, buf) == 0) {
> +		if (ndctl_bus_has_nfit(dimm->bus))
> +			parse_nfit_mem_flags(dimm, buf);
> +		else if (ndctl_bus_is_papr_scm(dimm->bus)) {
> +			dimm->cmd_family = NVDIMM_FAMILY_PAPR;
> +			parse_papr_flags(dimm, buf);
> +		}
> +	}
>  
>  	dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
>  	rc = 0;
> @@ -1792,7 +1766,8 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
>  	if (!path)
>  		return NULL;
>  
> -	sprintf(path, "%s/nfit/formats", dimm_base);
> +	sprintf(path, "%s/%s/formats", dimm_base,
> +		ndctl_bus_has_nfit(bus) ? "nfit" : "papr");
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		formats = 1;
>  	else
> @@ -1866,13 +1841,12 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
>  	else
>  		dimm->fwa_result = fwa_result_to_result(buf);
>  
> +	dimm->formats = formats;
>  	/* Check if the given dimm supports nfit */
>  	if (ndctl_bus_has_nfit(bus)) {
> -		dimm->formats = formats;
> -		rc = add_nfit_dimm(dimm, dimm_base);
> -	} else if (ndctl_bus_has_of_node(bus)) {
> -		rc = add_papr_dimm(dimm, dimm_base);
> -	}
> +		rc = populate_dimm_attributes(dimm, dimm_base, "nfit");
> +	} else if (ndctl_bus_has_of_node(bus))
> +		rc = populate_dimm_attributes(dimm, dimm_base, "papr");
>  
>  	if (rc == -ENODEV) {
>  		/* Unprobed dimm with no family */
> @@ -2531,13 +2505,12 @@ static void *add_region(void *parent, int id, const char *region_base)
>  		goto err_read;
>  	region->num_mappings = strtoul(buf, NULL, 0);
>  
> -	sprintf(path, "%s/nfit/range_index", region_base);
> -	if (ndctl_bus_has_nfit(bus)) {
> -		if (sysfs_read_attr(ctx, path, buf) < 0)
> -			goto err_read;
> -		region->range_index = strtoul(buf, NULL, 0);
> -	} else
> +	sprintf(path, "%s/%s/range_index", region_base,
> +		ndctl_bus_has_nfit(bus) ? "nfit": "papr");
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		region->range_index = -1;
> +	else
> +		region->range_index = strtoul(buf, NULL, 0);
>  
>  	sprintf(path, "%s/read_only", region_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
> -- 
> 2.30.2
diff mbox series

Patch

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 36fb6fe..26b9317 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1646,41 +1646,9 @@  static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
 static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
 static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
 
-static int add_papr_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
-{
-	int rc = -ENODEV;
-	char buf[SYSFS_ATTR_SIZE];
-	struct ndctl_ctx *ctx = dimm->bus->ctx;
-	char *path = calloc(1, strlen(dimm_base) + 100);
-	const char * const devname = ndctl_dimm_get_devname(dimm);
-
-	dbg(ctx, "%s: Probing of_pmem dimm at %s\n", devname, dimm_base);
-
-	if (!path)
-		return -ENOMEM;
-
-	/* construct path to the papr compatible dimm flags file */
-	sprintf(path, "%s/papr/flags", dimm_base);
-
-	if (ndctl_bus_is_papr_scm(dimm->bus) &&
-	    sysfs_read_attr(ctx, path, buf) == 0) {
-
-		dbg(ctx, "%s: Adding papr-scm dimm flags:\"%s\"\n", devname, buf);
-		dimm->cmd_family = NVDIMM_FAMILY_PAPR;
-
-		/* Parse dimm flags */
-		parse_papr_flags(dimm, buf);
-
-		/* Allocate monitor mode fd */
-		dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
-		rc = 0;
-	}
-
-	free(path);
-	return rc;
-}
-
-static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
+static int populate_dimm_attributes(struct ndctl_dimm *dimm,
+				    const char *dimm_base,
+				    const char *bus_prefix)
 {
 	int i, rc = -1;
 	char buf[SYSFS_ATTR_SIZE];
@@ -1694,7 +1662,7 @@  static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
 	 * 'unique_id' may not be available on older kernels, so don't
 	 * fail if the read fails.
 	 */
-	sprintf(path, "%s/nfit/id", dimm_base);
+	sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0) {
 		unsigned int b[9];
 
@@ -1709,68 +1677,74 @@  static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
 		}
 	}
 
-	sprintf(path, "%s/nfit/handle", dimm_base);
+	sprintf(path, "%s/%s/handle", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
 	dimm->handle = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/phys_id", dimm_base);
+	sprintf(path, "%s/%s/phys_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
 	dimm->phys_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/serial", dimm_base);
+	sprintf(path, "%s/%s/serial", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->serial = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/vendor", dimm_base);
+	sprintf(path, "%s/%s/vendor", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->vendor_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/device", dimm_base);
+	sprintf(path, "%s/%s/device", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->device_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/rev_id", dimm_base);
+	sprintf(path, "%s/%s/rev_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->revision_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/dirty_shutdown", dimm_base);
+	sprintf(path, "%s/%s/dirty_shutdown", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->dirty_shutdown = strtoll(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_vendor", dimm_base);
+	sprintf(path, "%s/%s/subsystem_vendor", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_vendor_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_device", dimm_base);
+	sprintf(path, "%s/%s/subsystem_device", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_device_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/subsystem_rev_id", dimm_base);
+	sprintf(path, "%s/%s/subsystem_rev_id", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->subsystem_revision_id = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/family", dimm_base);
+	sprintf(path, "%s/%s/family", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->cmd_family = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/dsm_mask", dimm_base);
+	sprintf(path, "%s/%s/dsm_mask", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/format", dimm_base);
+	sprintf(path, "%s/%s/format", dimm_base, bus_prefix);
 	if (sysfs_read_attr(ctx, path, buf) == 0)
 		dimm->format[0] = strtoul(buf, NULL, 0);
 	for (i = 1; i < dimm->formats; i++) {
-		sprintf(path, "%s/nfit/format%d", dimm_base, i);
+		sprintf(path, "%s/%s/format%d", dimm_base, bus_prefix, i);
 		if (sysfs_read_attr(ctx, path, buf) == 0)
 			dimm->format[i] = strtoul(buf, NULL, 0);
 	}
 
-	sprintf(path, "%s/nfit/flags", dimm_base);
-	if (sysfs_read_attr(ctx, path, buf) == 0)
-		parse_nfit_mem_flags(dimm, buf);
+	sprintf(path, "%s/%s/flags", dimm_base, bus_prefix);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		if (ndctl_bus_has_nfit(dimm->bus))
+			parse_nfit_mem_flags(dimm, buf);
+		else if (ndctl_bus_is_papr_scm(dimm->bus)) {
+			dimm->cmd_family = NVDIMM_FAMILY_PAPR;
+			parse_papr_flags(dimm, buf);
+		}
+	}
 
 	dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
 	rc = 0;
@@ -1792,7 +1766,8 @@  static void *add_dimm(void *parent, int id, const char *dimm_base)
 	if (!path)
 		return NULL;
 
-	sprintf(path, "%s/nfit/formats", dimm_base);
+	sprintf(path, "%s/%s/formats", dimm_base,
+		ndctl_bus_has_nfit(bus) ? "nfit" : "papr");
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		formats = 1;
 	else
@@ -1866,13 +1841,12 @@  static void *add_dimm(void *parent, int id, const char *dimm_base)
 	else
 		dimm->fwa_result = fwa_result_to_result(buf);
 
+	dimm->formats = formats;
 	/* Check if the given dimm supports nfit */
 	if (ndctl_bus_has_nfit(bus)) {
-		dimm->formats = formats;
-		rc = add_nfit_dimm(dimm, dimm_base);
-	} else if (ndctl_bus_has_of_node(bus)) {
-		rc = add_papr_dimm(dimm, dimm_base);
-	}
+		rc = populate_dimm_attributes(dimm, dimm_base, "nfit");
+	} else if (ndctl_bus_has_of_node(bus))
+		rc = populate_dimm_attributes(dimm, dimm_base, "papr");
 
 	if (rc == -ENODEV) {
 		/* Unprobed dimm with no family */
@@ -2531,13 +2505,12 @@  static void *add_region(void *parent, int id, const char *region_base)
 		goto err_read;
 	region->num_mappings = strtoul(buf, NULL, 0);
 
-	sprintf(path, "%s/nfit/range_index", region_base);
-	if (ndctl_bus_has_nfit(bus)) {
-		if (sysfs_read_attr(ctx, path, buf) < 0)
-			goto err_read;
-		region->range_index = strtoul(buf, NULL, 0);
-	} else
+	sprintf(path, "%s/%s/range_index", region_base,
+		ndctl_bus_has_nfit(bus) ? "nfit": "papr");
+	if (sysfs_read_attr(ctx, path, buf) < 0)
 		region->range_index = -1;
+	else
+		region->range_index = strtoul(buf, NULL, 0);
 
 	sprintf(path, "%s/read_only", region_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)