| Message ID | 20240725073050.219952-1-alison.schofield@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [ndctl] cxl/list: add firmware_version to default memdev listing | expand |
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 >
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 >
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)
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 --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)