Re: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing

[Dave Jiang <dave.jiang@intel.com>]

On 7/25/24 12:30 AM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> cxl list users may discover the firmware revision of a memory
> device by using the -F option to cxl list. That option uses
> the CXL GET_FW_INFO command and emits this json:
> 
> "firmware":{
>       "num_slots":2,
>       "active_slot":1,
>       "staged_slot":1,
>       "online_activate_capable":false,
>       "slot_1_version":"BWFW VERSION 0",
>       "fw_update_in_progress":false
>     }
> 
> Since device support for GET_FW_INFO is optional, the above method
> is not guaranteed. However, the IDENTIFY command is mandatory and
> provides the current firmware revision.
> 
> Accessors already exist for retrieval from sysfs so simply add
> the new json member to the default memdev listing.
> 
> This means users of the -F option will get the same info twice if
> GET_FW_INFO is supported.
> 
> [
>   {
>     "memdev":"mem9",
>     "pmem_size":268435456,
>     "serial":0,
>     "host":"0000:c0:00.0"
>     "firmware_version":"BWFW VERSION 00",
>   }
> ]
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  cxl/json.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 0c27abaea0bd..0b0b186a2594 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -577,6 +577,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  	const char *devname = cxl_memdev_get_devname(memdev);
>  	struct json_object *jdev, *jobj;
>  	unsigned long long serial, size;
> +	const char *fw_version;
>  	int numa_node;
>  	int qos_class;
>  
> @@ -646,6 +647,13 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  	if (jobj)
>  		json_object_object_add(jdev, "host", jobj);
>  
> +	fw_version = cxl_memdev_get_firmware_version(memdev);
> +	if (fw_version) {
> +		jobj = json_object_new_string(fw_version);
> +		if (jobj)
> +			json_object_object_add(jdev, "firmware_version", jobj);
> +	}
> +
>  	if (!cxl_memdev_is_enabled(memdev)) {
>  		jobj = json_object_new_string("disabled");
>  		if (jobj)