| Message ID | 20220317234049.69323-3-dave@stgolabs.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/mbox: Robustify handling of mbox_cmd.return_code | expand |
On Thu, Mar 17, 2022 at 04:40:46PM -0700, Davidlohr Bueso wrote: > Also mention the need for the caller to check against any > errors from the hardware in return_code. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > drivers/cxl/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 8a7267d116b7..c77e06aff8dc 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -177,9 +177,9 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds, > mbox_cmd->return_code = > FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg); > > - if (mbox_cmd->return_code != 0) { > + if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) { > dev_dbg(dev, "Mailbox operation had an error\n"); > - return 0; > + return 0; /* completed but caller must check return_code */ > } > > /* #7 */ > > -- > 2.26.2 > Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
On Thu, Mar 17, 2022 at 4:41 PM Davidlohr Bueso <dave@stgolabs.net> wrote: > > Also mention the need for the caller to check against any > errors from the hardware in return_code. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > drivers/cxl/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index 8a7267d116b7..c77e06aff8dc 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -177,9 +177,9 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds, > mbox_cmd->return_code = > FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg); > > - if (mbox_cmd->return_code != 0) { > + if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) { > dev_dbg(dev, "Mailbox operation had an error\n"); > - return 0; > + return 0; /* completed but caller must check return_code */ Looks like a good clarification to me. Applied. ...but note it won't appear anywhere until after v5.18-rc1 is out.
On Tue, Mar 29, 2022 at 4:42 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Thu, Mar 17, 2022 at 4:41 PM Davidlohr Bueso <dave@stgolabs.net> wrote: > > > > Also mention the need for the caller to check against any > > errors from the hardware in return_code. > > > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > > --- > > drivers/cxl/pci.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > > index 8a7267d116b7..c77e06aff8dc 100644 > > --- a/drivers/cxl/pci.c > > +++ b/drivers/cxl/pci.c > > @@ -177,9 +177,9 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds, > > mbox_cmd->return_code = > > FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg); > > > > - if (mbox_cmd->return_code != 0) { > > + if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) { > > dev_dbg(dev, "Mailbox operation had an error\n"); > > - return 0; > > + return 0; /* completed but caller must check return_code */ > > Looks like a good clarification to me. > > Applied. > > ...but note it won't appear anywhere until after v5.18-rc1 is out. ...although given there are already feedback comments on patch 1 and 3 I also don't mind if you resend the full series for v2.
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 8a7267d116b7..c77e06aff8dc 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -177,9 +177,9 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds, mbox_cmd->return_code = FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg); - if (mbox_cmd->return_code != 0) { + if (mbox_cmd->return_code != CXL_MBOX_SUCCESS) { dev_dbg(dev, "Mailbox operation had an error\n"); - return 0; + return 0; /* completed but caller must check return_code */ } /* #7 */
Also mention the need for the caller to check against any errors from the hardware in return_code. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- drivers/cxl/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)