* RE: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing
2024-07-25 7:30 [ndctl PATCH] cxl/list: add firmware_version to default memdev listing alison.schofield
@ 2024-07-25 8:34 ` Xingtao Yao (Fujitsu)
2024-07-25 19:22 ` fan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Xingtao Yao (Fujitsu) @ 2024-07-25 8:34 UTC (permalink / raw)
To: alison.schofield@intel.com, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org
Cc: Dan Williams
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing
2024-07-25 7:30 [ndctl PATCH] cxl/list: add firmware_version to default memdev listing alison.schofield
2024-07-25 8:34 ` Xingtao Yao (Fujitsu)
@ 2024-07-25 19:22 ` fan
2024-07-25 20:15 ` Dave Jiang
2024-08-09 22:56 ` Dan Williams
3 siblings, 0 replies; 5+ messages in thread
From: fan @ 2024-07-25 19:22 UTC (permalink / raw)
To: alison.schofield; +Cc: nvdimm, linux-cxl, Dan Williams
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing
2024-07-25 7:30 [ndctl PATCH] cxl/list: add firmware_version to default memdev listing alison.schofield
2024-07-25 8:34 ` Xingtao Yao (Fujitsu)
2024-07-25 19:22 ` fan
@ 2024-07-25 20:15 ` Dave Jiang
2024-08-09 22:56 ` Dan Williams
3 siblings, 0 replies; 5+ messages in thread
From: Dave Jiang @ 2024-07-25 20:15 UTC (permalink / raw)
To: alison.schofield, nvdimm, linux-cxl; +Cc: Dan Williams
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)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [ndctl PATCH] cxl/list: add firmware_version to default memdev listing
2024-07-25 7:30 [ndctl PATCH] cxl/list: add firmware_version to default memdev listing alison.schofield
` (2 preceding siblings ...)
2024-07-25 20:15 ` Dave Jiang
@ 2024-08-09 22:56 ` Dan Williams
3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2024-08-09 22:56 UTC (permalink / raw)
To: alison.schofield, nvdimm, linux-cxl; +Cc: Alison Schofield, Dan Williams
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>
^ permalink raw reply [flat|nested] 5+ messages in thread