| Message ID | 20241115170032.108445-1-dave@stgolabs.net |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] cxl/pci: Fix potential bogus return value upon successful probing | expand |
Davidlohr Bueso wrote: > If cxl_pci_ras_unmask() returns non-zero, cxl_pci_probe() will end up > returning that value, instead of zero. > > Fixes: 248529edc86 (cxl: add RAS status unmasking for CXL) > Reviewed-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> NIT below. But. Reviewed-by: Ira Weiny <ira.weiny@intel.com> > --- > Changes from v1: > - Picked up Fan's review tag. > - Continue to return rc (Alison). > > drivers/cxl/pci.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b2cb81f6d9e7..2012301fcfbf 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -1032,8 +1032,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > if (rc) > return rc; > > - rc = cxl_pci_ras_unmask(pdev); > - if (rc) Technically if we are here... > + if (cxl_pci_ras_unmask(pdev)) > dev_dbg(&pdev->dev, "No RAS reporting unmasked\n"); > > pci_save_state(pdev); ... Then a simple ... return 0; At the end would be more straight forward. But this patch is fine, Ira [snip]
On Fri, Nov 15, 2024 at 09:00:32AM -0800, Davidlohr Bueso wrote: > If cxl_pci_ras_unmask() returns non-zero, cxl_pci_probe() will end up > returning that value, instead of zero. > > Fixes: 248529edc86 (cxl: add RAS status unmasking for CXL) > Reviewed-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > Changes from v1: > - Picked up Fan's review tag. > - Continue to return rc (Alison). I thought I said opposite - "I agree that the return value should be 0 if we get this far, because all rc's that need to be returned, are returned immediately." Regardless - it fixes what it said it will fix, as did v1. > > drivers/cxl/pci.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b2cb81f6d9e7..2012301fcfbf 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -1032,8 +1032,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > if (rc) > return rc; > > - rc = cxl_pci_ras_unmask(pdev); > - if (rc) > + if (cxl_pci_ras_unmask(pdev)) > dev_dbg(&pdev->dev, "No RAS reporting unmasked\n"); > > pci_save_state(pdev); > -- > 2.47.0 >
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index b2cb81f6d9e7..2012301fcfbf 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -1032,8 +1032,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (rc) return rc; - rc = cxl_pci_ras_unmask(pdev); - if (rc) + if (cxl_pci_ras_unmask(pdev)) dev_dbg(&pdev->dev, "No RAS reporting unmasked\n"); pci_save_state(pdev);