diff mbox series

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

Message ID 20240725073050.219952-1-alison.schofield@intel.com (mailing list archive)
State Accepted
Commit f1dc47e1f1dd0e93d1fe4e1e4056f663d52e7d30
Headers show
Series [ndctl] cxl/list: add firmware_version to default memdev listing | expand

Commit Message

Alison Schofield July 25, 2024, 7:30 a.m. UTC
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>
---
 cxl/json.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Xingtao Yao (Fujitsu) July 25, 2024, 8:34 a.m. UTC | #1
Reviewed-by: Xingtao Yao <yaoxt.fnst@fujitsu.com>
Tested-by: Xingtao Yao <yaoxt.fnst@fujitsu.com>

> -----Original Message-----
> From: alison.schofield@intel.com <alison.schofield@intel.com>
> Sent: Thursday, July 25, 2024 3:31 PM
> To: nvdimm@lists.linux.dev; linux-cxl@vger.kernel.org
> Cc: Alison Schofield <alison.schofield@intel.com>; Dan Williams
> <dan.j.williams@intel.com>
> Subject: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing
> 
> 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>
> ---
>  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)
> --
> 2.37.3
>
Fan Ni July 25, 2024, 7:22 p.m. UTC | #2
On Thu, Jul 25, 2024 at 12:30:50AM -0700, 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: Fan Ni <fan.ni@samsung.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)
> -- 
> 2.37.3
>
Dave Jiang July 25, 2024, 8:15 p.m. UTC | #3
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)
Dan Williams Aug. 9, 2024, 10:56 p.m. UTC | #4
alison.schofield@ 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>

Looks good to me:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
diff mbox series

Patch

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)