On Sun, Nov 8, 2020 at 4:21 AM Santosh Sivaraj <sant...@fossix.org> wrote: > > Don't fail is nfit module is missing, this will happen in > platforms that don't have ACPI support. Add attributes to > PAPR dimm family that are independent of platforms like the > test dimms. > > Signed-off-by: Santosh Sivaraj <sant...@fossix.org> > --- > ndctl/lib/libndctl.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ > test/core.c | 6 +++++ > 2 files changed, 58 insertions(+) > > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c > index ad521d3..d1f8e4e 100644 > --- a/ndctl/lib/libndctl.c > +++ b/ndctl/lib/libndctl.c > @@ -1655,6 +1655,54 @@ 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 void populate_dimm_attributes(struct ndctl_dimm *dimm, > + const char *dimm_base) > +{ > + char buf[SYSFS_ATTR_SIZE]; > + struct ndctl_ctx *ctx = dimm->bus->ctx; > + char *path = calloc(1, strlen(dimm_base) + 100); > + > + sprintf(path, "%s/phys_id", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) < 0) > + goto err_read; > + dimm->phys_id = strtoul(buf, NULL, 0); > + > + sprintf(path, "%s/handle", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) < 0) > + goto err_read; > + dimm->handle = strtoul(buf, NULL, 0); > + > + sprintf(path, "%s/vendor", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) == 0) > + dimm->vendor_id = strtoul(buf, NULL, 0); > + > + sprintf(path, "%s/id", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) == 0) { > + unsigned int b[9]; > + > + dimm->unique_id = strdup(buf); > + if (!dimm->unique_id) > + goto err_read; > + if (sscanf(dimm->unique_id, > "%02x%02x-%02x-%02x%02x-%02x%02x%02x%02x", > + &b[0], &b[1], &b[2], &b[3], &b[4], > + &b[5], &b[6], &b[7], &b[8]) == 9) { > + dimm->manufacturing_date = b[3] << 8 | b[4]; > + dimm->manufacturing_location = b[2]; > + } > + } > + sprintf(path, "%s/subsystem_vendor", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) == 0) > + dimm->subsystem_vendor_id = strtoul(buf, NULL, 0); > + > + > + sprintf(path, "%s/dirty_shutdown", dimm_base); > + if (sysfs_read_attr(ctx, path, buf) == 0) > + dimm->dirty_shutdown = strtoll(buf, NULL, 0);
These are fairly similar to the nfit ones... how about refactoring this into a routine that takes a bus prefix and shares it between "nfit" and "papr"... We might also consider unifying them into a standard set of attributes that both the nfit-bus-provider and the papr-bus-provider export. I.e. that nfit was wrong to place them under nfit/ and they should have went somewhere generic from the beginning. The nfit compatibility can be done with symlinks to the new common location. > + > +err_read: > + free(path); > +} > + > static int add_papr_dimm(struct ndctl_dimm *dimm, const char *dimm_base) > { > int rc = -ENODEV; > @@ -1685,6 +1733,10 @@ static int add_papr_dimm(struct ndctl_dimm *dimm, > const char *dimm_base) > rc = 0; > } > > + /* add the available dimm attributes, the platform can override or add > + * additional attributes later */ > + populate_dimm_attributes(dimm, dimm_base); > + > free(path); > return rc; > } > diff --git a/test/core.c b/test/core.c > index 5118d86..0fd1011 100644 > --- a/test/core.c > +++ b/test/core.c > @@ -195,6 +195,12 @@ retry: > > path = kmod_module_get_path(*mod); > if (!path) { > + /* For non-nfit platforms it's ok if nfit module is > + * missing */ > + if (strcmp(name, "nfit") == 0 || > + strcmp(name, "nd_e820") == 0) > + continue; This breaks the safety it afforded on nfit platforms. Instead I think this needs a couple changes: - rename nfit_test_init to ndctl_test_init - add a parameter for whether the test is initializing for nfit_test.ko or ndtest.ko. _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org