Messages in this thread | | | Date | Fri, 9 Sep 2022 14:35:32 -0700 | From | Ira Weiny <> | Subject | Re: [RFC PATCH 2/9] cxl/mem: Implement Clear Event Records command |
| |
On Wed, Aug 24, 2022 at 04:55:13PM +0100, Jonathan Cameron wrote: > On Fri, 12 Aug 2022 22:32:36 -0700 > ira.weiny@intel.com wrote: > > > From: Ira Weiny <ira.weiny@intel.com> > > > > CXL v3.0 section 8.2.9.2.3 defines the Clear Event Records mailbox > > command. After an event record is read it needs to be cleared from the > > event log. > > > > Implement cxl_clear_event_record() and call it for each record retrieved > > from the device. > > > > Each record is cleared individually. A clear all bit is specified but > > events could arrive between a get and the final clear all operation. > > Therefore each event is cleared specifically. > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > Trivial suggestions inline, but other than that LGTM > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Thanks!
> > > --- > > drivers/cxl/core/mbox.c | 31 ++++++++++++++++++++++++++++--- > > drivers/cxl/cxlmem.h | 15 +++++++++++++++ > > include/uapi/linux/cxl_mem.h | 1 + > > 3 files changed, 44 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > index 2cceed8608dc..493f5ceb5d1c 100644 > > --- a/drivers/cxl/core/mbox.c > > +++ b/drivers/cxl/core/mbox.c > > @@ -52,6 +52,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > #endif > > CXL_CMD(GET_SUPPORTED_LOGS, 0, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE), > > CXL_CMD(GET_EVENT_RECORD, 1, CXL_VARIABLE_PAYLOAD, 0), > > + CXL_CMD(CLEAR_EVENT_RECORD, CXL_VARIABLE_PAYLOAD, 0, 0), > > CXL_CMD(GET_FW_INFO, 0, 0x50, 0), > > CXL_CMD(GET_PARTITION_INFO, 0, 0x20, 0), > > CXL_CMD(GET_LSA, 0x8, CXL_VARIABLE_PAYLOAD, 0), > > @@ -708,6 +709,26 @@ int cxl_enumerate_cmds(struct cxl_dev_state *cxlds) > > } > > EXPORT_SYMBOL_NS_GPL(cxl_enumerate_cmds, CXL); > > > > +static int cxl_clear_event_record(struct cxl_dev_state *cxlds, > > + enum cxl_event_log_type log, > > + __le16 handle) > > +{ > > + struct cxl_mbox_clear_event_payload payload; > > + int rc; > > + > > + memset(&payload, 0, sizeof(payload)); > > Could just do payload = {}; > > Thouch as you are setting stuff, why not just do > > payload = { > .event_log = log, > .nr_recs = 1, > .handle = handle, > }; > and let the compiler zero anything else (I think there are no holes to complicate > things).
Yea! Done.
> > > + payload.event_log = log; > > + payload.nr_recs = 1; > > + payload.handle = handle; > > + > > + rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_CLEAR_EVENT_RECORD, > > + &payload, sizeof(payload), NULL, 0); > > return cxl_mbox_send_cmd() and drop rc definition.
And Done. I've also used the return value now! ;-)
Thanks again! Ira
| |