On Wed, 21 Sep 2022 08:32:57 -0700
Dave Jiang <dave.ji...@intel.com> wrote:

> Add an id group attribute for CXL based nvdimm object. The addition allows
> ndctl to display the "unique id" for the nvdimm. The serial number for the
> CXL memory device will be used for this id.
> 
> [
>   {
>       "dev":"nmem10",
>       "id":"0x4",
>       "security":"disabled"
>   },
> ]
> 
> The id attribute is needed by the ndctl security key management to setup a
> keyblob with a unique file name tied to the mem device.
> 
> Signed-off-by: Dave Jiang <dave.ji...@intel.com>

One comment inline, but feel free to ignore.
Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>

> ---
>  drivers/cxl/pmem.c |   29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> index 24bec4ca3866..9f34f8701b57 100644
> --- a/drivers/cxl/pmem.c
> +++ b/drivers/cxl/pmem.c
> @@ -48,6 +48,32 @@ static void unregister_nvdimm(void *nvdimm)
>       cxl_nvd->bridge = NULL;
>  }
>  
> +static ssize_t id_show(struct device *dev, struct device_attribute *attr, 
> char *buf)
> +{
> +     struct nvdimm *nvdimm = to_nvdimm(dev);
> +     struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(nvdimm);
> +     struct cxl_memdev *cxlmd = cxl_nvd->cxlmd;
> +     struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +     return sysfs_emit(buf, "%lld\n", cxlds->serial);
Given single uses I'd be tempted to not bother with the local variables
except when it gets really olong

        struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(to_nvdimm(dev));
        struct cxl_dev_state *cxlds = cxl_nvd->cxlmd->cxlds;
maybe.  Up to you on what style you prefer though.



> +}


Reply via email to