| Message ID | 20210311074652.2783560-1-santosh@fossix.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ndctl,v3,1/4] libndctl: test enablement for non-nfit devices | expand |
On Thu, 2021-03-11 at 13:16 +0530, Santosh Sivaraj wrote: > Unify adding dimms for papr and nfit families, this will help in adding Minor nit, but it seems like the subject line and the first sentence in the body should be swapped. The one-line description of what's happening in this patch is "Unify adding dimms for papr and nfit families", and the body can go into more detail about why - i.e. in preparation for enabling tests on non-nfit devices. > all attributes needed for the unit tests too. We don't fail adding a dimm > if some of the dimm attributes are missing, so this will work fine on PAPR > platforms where most dimm attributes are provided. Does this mean 'most - but not all'? Might be a good clarification to make here. > > Signed-off-by: Santosh Sivaraj <santosh@fossix.org> > --- > ndctl/lib/libndctl.c | 103 ++++++++++++++++--------------------------- > 1 file changed, 38 insertions(+), 65 deletions(-) > > v3: > * Drop patch which skips SMART tests, smart test enablement will be posted > soon. > > v2: > * Patch 2: Fix a bug, I skip erroring out if PAPR family, but condition had > INTEL family instead. That change was there to test the same code on x86, but > accidently committed. Now have a environment variable to force test PAPR > family on x86. > > * Patch 4: Remove stray code, artifact of refactoring in patch 1. > > 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)
"Verma, Vishal L" <vishal.l.verma@intel.com> writes: Hi Vishal, > On Thu, 2021-03-11 at 13:16 +0530, Santosh Sivaraj wrote: >> Unify adding dimms for papr and nfit families, this will help in adding > > Minor nit, but it seems like the subject line and the first sentence in > the body should be swapped. The one-line description of what's happening > in this patch is "Unify adding dimms for papr and nfit families", and > the body can go into more detail about why - i.e. in preparation for > enabling tests on non-nfit devices. > Agree. >> all attributes needed for the unit tests too. We don't fail adding a dimm >> if some of the dimm attributes are missing, so this will work fine on PAPR >> platforms where most dimm attributes are provided. > > Does this mean 'most - but not all'? Might be a good clarification to > make here. Yes, that's right. I will fix that up in the next version. Thanks, Santosh > >> >> Signed-off-by: Santosh Sivaraj <santosh@fossix.org> >> --- >> ndctl/lib/libndctl.c | 103 ++++++++++++++++--------------------------- >> 1 file changed, 38 insertions(+), 65 deletions(-) >> >> v3: >> * Drop patch which skips SMART tests, smart test enablement will be posted >> soon. >> >> v2: >> * Patch 2: Fix a bug, I skip erroring out if PAPR family, but condition had >> INTEL family instead. That change was there to test the same code on x86, but >> accidently committed. Now have a environment variable to force test PAPR >> family on x86. >> >> * Patch 4: Remove stray code, artifact of refactoring in patch 1. >> >> 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)
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)
Unify adding dimms for papr and nfit families, this will help in adding all attributes needed for the unit tests too. We don't fail adding a dimm if some of the dimm attributes are missing, so this will work fine on PAPR platforms where most dimm attributes are provided. Signed-off-by: Santosh Sivaraj <santosh@fossix.org> --- ndctl/lib/libndctl.c | 103 ++++++++++++++++--------------------------- 1 file changed, 38 insertions(+), 65 deletions(-) v3: * Drop patch which skips SMART tests, smart test enablement will be posted soon. v2: * Patch 2: Fix a bug, I skip erroring out if PAPR family, but condition had INTEL family instead. That change was there to test the same code on x86, but accidently committed. Now have a environment variable to force test PAPR family on x86. * Patch 4: Remove stray code, artifact of refactoring in patch 1.