Dan Williams <[email protected]> writes: > On Sun, Nov 8, 2020 at 4:21 AM Santosh Sivaraj <[email protected]> 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 <[email protected]> >> --- >> 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.
Thanks for review Dan. I will make changes accordingly and send newer version without -ENOCOMMITMESSAGE error. Since I posted this only for the comments on the approach I took, I think I got how to proceed in terms on ndctl changes atleast. I will send a v5 soon enough. Thanks, Santosh _______________________________________________ Linux-nvdimm mailing list -- [email protected] To unsubscribe send an email to [email protected]
