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.
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
