| Message ID | 20241209185429.54054-8-alejandro.lucero-palau@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add type2 device basic support | expand |
On 09/12/2024 18:54, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Use cxl code for registers discovery and mapping. > > Validate capabilities found based on those registers against expected > capabilities. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com> > Reviewed-by: Zhi Wang <zhi@nvidia.com> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
On Mon, Dec 09, 2024 at 06:54:08PM +0000, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Use cxl code for registers discovery and mapping. > > Validate capabilities found based on those registers against expected > capabilities. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com> > Reviewed-by: Zhi Wang <zhi@nvidia.com> > --- > drivers/net/ethernet/sfc/efx_cxl.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > index 58a6f14565ff..3f15486f99e4 100644 > --- a/drivers/net/ethernet/sfc/efx_cxl.c > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -21,6 +21,8 @@ > int efx_cxl_init(struct efx_probe_data *probe_data) > { > struct efx_nic *efx = &probe_data->efx; > + DECLARE_BITMAP(expected, CXL_MAX_CAPS); > + DECLARE_BITMAP(found, CXL_MAX_CAPS); > struct pci_dev *pci_dev; > struct efx_cxl *cxl; > struct resource res; > @@ -65,6 +67,23 @@ int efx_cxl_init(struct efx_probe_data *probe_data) > goto err2; > } > > + rc = cxl_pci_accel_setup_regs(pci_dev, cxl->cxlds); > + if (rc) { > + pci_err(pci_dev, "CXL accel setup regs failed"); > + goto err2; > + } > + > + bitmap_clear(expected, 0, CXL_MAX_CAPS); > + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); > + bitmap_set(expected, CXL_DEV_CAP_RAS, 1); > + > + if (!cxl_pci_check_caps(cxl->cxlds, expected, found)) { > + pci_err(pci_dev, > + "CXL device capabilities found(%08lx) not as expected(%08lx)", > + *found, *expected); > + goto err2; Hi Alejandro, goto err2 will result in the function returning rc. However, rc is 0 here. Should it be set to a negative error value instead? Flagged by Smatch. > + } > + > probe_data->cxl = cxl; > > return 0; > -- > 2.17.1 > >
On 12/12/24 18:04, Simon Horman wrote: > On Mon, Dec 09, 2024 at 06:54:08PM +0000, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Use cxl code for registers discovery and mapping. >> >> Validate capabilities found based on those registers against expected >> capabilities. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com> >> Reviewed-by: Zhi Wang <zhi@nvidia.com> >> --- >> drivers/net/ethernet/sfc/efx_cxl.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c >> index 58a6f14565ff..3f15486f99e4 100644 >> --- a/drivers/net/ethernet/sfc/efx_cxl.c >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -21,6 +21,8 @@ >> int efx_cxl_init(struct efx_probe_data *probe_data) >> { >> struct efx_nic *efx = &probe_data->efx; >> + DECLARE_BITMAP(expected, CXL_MAX_CAPS); >> + DECLARE_BITMAP(found, CXL_MAX_CAPS); >> struct pci_dev *pci_dev; >> struct efx_cxl *cxl; >> struct resource res; >> @@ -65,6 +67,23 @@ int efx_cxl_init(struct efx_probe_data *probe_data) >> goto err2; >> } >> >> + rc = cxl_pci_accel_setup_regs(pci_dev, cxl->cxlds); >> + if (rc) { >> + pci_err(pci_dev, "CXL accel setup regs failed"); >> + goto err2; >> + } >> + >> + bitmap_clear(expected, 0, CXL_MAX_CAPS); >> + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); >> + bitmap_set(expected, CXL_DEV_CAP_RAS, 1); >> + >> + if (!cxl_pci_check_caps(cxl->cxlds, expected, found)) { >> + pci_err(pci_dev, >> + "CXL device capabilities found(%08lx) not as expected(%08lx)", >> + *found, *expected); >> + goto err2; > Hi Alejandro, > > goto err2 will result in the function returning rc. > However, rc is 0 here. Should it be set to a negative error value instead? Hi Simon, Right. The code is functionally correct because the driver's variable setting the CXL initialization successful is not happening when skipping with the goto, which is the key part. Returning an error will log a CXL initialization driver's error but it has no impact with the driver's initialization or the CXL usage by design. However, the error should be returned properly. I will fix it in v8. Thanks! > Flagged by Smatch. > >> + } >> + >> probe_data->cxl = cxl; >> >> return 0; >> -- >> 2.17.1 >> >>
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c index 58a6f14565ff..3f15486f99e4 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c +++ b/drivers/net/ethernet/sfc/efx_cxl.c @@ -21,6 +21,8 @@ int efx_cxl_init(struct efx_probe_data *probe_data) { struct efx_nic *efx = &probe_data->efx; + DECLARE_BITMAP(expected, CXL_MAX_CAPS); + DECLARE_BITMAP(found, CXL_MAX_CAPS); struct pci_dev *pci_dev; struct efx_cxl *cxl; struct resource res; @@ -65,6 +67,23 @@ int efx_cxl_init(struct efx_probe_data *probe_data) goto err2; } + rc = cxl_pci_accel_setup_regs(pci_dev, cxl->cxlds); + if (rc) { + pci_err(pci_dev, "CXL accel setup regs failed"); + goto err2; + } + + bitmap_clear(expected, 0, CXL_MAX_CAPS); + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); + bitmap_set(expected, CXL_DEV_CAP_RAS, 1); + + if (!cxl_pci_check_caps(cxl->cxlds, expected, found)) { + pci_err(pci_dev, + "CXL device capabilities found(%08lx) not as expected(%08lx)", + *found, *expected); + goto err2; + } + probe_data->cxl = cxl; return 0;