Hi Aneesh, Thanks for looking into this patch.
"Aneesh Kumar K.V" <[email protected]> writes: > Vaibhav Jain <[email protected]> writes: > >> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence >> this patch refactors this functionality into two functions namely >> add_dimm() and add_nfit_dimm(). Function add_dimm() performs >> allocation and common 'struct ndctl_dimm' initialization and depending >> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once >> the probe is completed based on the value of 'ndctl_dimm.cmd_family' >> appropriate dimm-ops are assigned to the dimm. >> >> In case dimm-bus is of unknown type or doesn't support NFIT the >> initialization still continues, with no dimm-ops assigned to the >> 'struct ndctl_dimm' there-by limiting the functionality available. >> >> This patch shouldn't introduce any behavioral change. >> >> Signed-off-by: Vaibhav Jain <[email protected]> >> --- >> Changelog: >> >> v1..v2: >> * None >> --- >> ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------ >> 1 file changed, 112 insertions(+), 81 deletions(-) >> >> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c >> index ee737cbbfe3e..d76dbf7e17de 100644 >> --- a/ndctl/lib/libndctl.c >> +++ b/ndctl/lib/libndctl.c >> @@ -1441,82 +1441,15 @@ 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 *add_dimm(void *parent, int id, const char *dimm_base) >> +static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base) >> { >> - int formats, i; >> - struct ndctl_dimm *dimm; >> + int i, rc = -1; >> char buf[SYSFS_ATTR_SIZE]; >> - struct ndctl_bus *bus = parent; >> - struct ndctl_ctx *ctx = bus->ctx; >> + struct ndctl_ctx *ctx = dimm->bus->ctx; >> char *path = calloc(1, strlen(dimm_base) + 100); >> >> if (!path) >> - return NULL; >> - >> - sprintf(path, "%s/nfit/formats", dimm_base); >> - if (sysfs_read_attr(ctx, path, buf) < 0) >> - formats = 1; >> - else > > .... >> + rc = 0; >> + err_read: >> + >> + free(path); >> + return rc; >> +} >> + >> +static void *add_dimm(void *parent, int id, const char *dimm_base) >> +{ >> + int formats, i, rc = -ENODEV; >> + struct ndctl_dimm *dimm = NULL; >> + char buf[SYSFS_ATTR_SIZE]; >> + struct ndctl_bus *bus = parent; >> + struct ndctl_ctx *ctx = bus->ctx; >> + char *path = calloc(1, strlen(dimm_base) + 100); >> + >> + if (!path) >> + return NULL; >> + >> + sprintf(path, "%s/nfit/formats", dimm_base); > > Witht that abstraction this should be part of add_nfit_dimm? This part is needed to calculate the size of 'struct ndctl_dimm' to be allocated which is based on number of nfit formats reported in sysfs. > >> + if (sysfs_read_attr(ctx, path, buf) < 0) >> + formats = 1; >> + else >> + formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL); >> + >> + dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats); >> + if (!dimm) >> + goto err_dimm; >> + dimm->bus = bus; >> + dimm->id = id; >> + > _______________________________________________ > Linux-nvdimm mailing list -- [email protected] > To unsubscribe send an email to [email protected] ~ Vaibhav _______________________________________________ Linux-nvdimm mailing list -- [email protected] To unsubscribe send an email to [email protected]
