Re: [ndctl PATCH v2 2/4] libndctl: record dsm family in add_dimm()

[Dan Williams <dan.j.williams@intel.com>]

On Thu, Sep 15, 2016 at 11:17 AM, Boylston, Brian
<brian.boylston@hpe.com> wrote:
> Dan Williams wrote on 2016-09-14:
>> On Tue, Sep 13, 2016 at 10:41 AM, Brian Boylston <brian.boylston@hpe.com> wrote:
>>> The recorded DSM family can be used to provide family-specific
>>> functionality.
>>>
>>> Cc: Dan Williams <dan.j.williams@intel.com>
>>> Signed-off-by: Brian Boylston <brian.boylston@hpe.com>
>>> ---
>>>  ndctl/lib/libndctl.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
>>> index 9a9dc74..c824d83 100644
>>> --- a/ndctl/lib/libndctl.c
>>> +++ b/ndctl/lib/libndctl.c
>>> @@ -136,6 +136,7 @@ struct ndctl_dimm {
>>>         unsigned short subsystem_revision_id;
>>>         unsigned short manufacturing_date;
>>>         unsigned char manufacturing_location;
>>> +       unsigned long dsm_family;
>>>         unsigned long dsm_mask;
>>>         char *unique_id;
>>>         char *dimm_path;
>>> @@ -1145,6 +1146,11 @@ static int add_dimm(void *parent, int id, const char *dimm_base)
>>>         if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
>>>                 goto err_read;
>>>
>>> +       sprintf(path, "%s/nfit/family", dimm_base);
>>> +       if (sysfs_read_attr(ctx, path, buf) < 0)
>>> +               goto err_read;
>>> +       dimm->dsm_family = strtoul(buf, NULL, 0);
>>
>>
>> Hmm I don't think it should it be fatal to add_dimm() for a failure to
>> have a family.  It's reasonable for a DIMM to exist in the NFIT, but
>> have no DSMs or a DSM implementation that is new to the kernel.  Can
>> we make this continue on and teach consumers of ->dsm_family that it
>> might be an error value?
>
> Sure.  Some of the other values collected in add_dimm() use -1 as an
> error or not available value; ok to use that for dsm_family?

Sounds good.

> Also, should I add a public accessor function such as
> ndctl_dimm_get_dsm_family()?  (although there wouldn't be a consumer yet)

I'm not opposed to it for informational purposes, but I like the
current direction where it doesn't really matter and the other ndctl
routines are taught how to multiplex for different families.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm