On Fri, 26 Sep 2025 10:47:27 +0100
Alejandro Lucero Palau [off-list ref] wrote:

On 9/18/25 16:08, Jonathan Cameron wrote:

quoted

A few trivial things inline.
 

quoted

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 47349c148c0c..7bc854e2d22a 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
index 85490afc7930..3dde59003cd9 100644
--- a/drivers/net/ethernet/sfc/efx_cxl.c
+++ b/drivers/net/ethernet/sfc/efx_cxl.c

@@ -11,16 +11,23 @@
  
  #include "net_driver.h"
  #include "efx_cxl.h"
+#include "efx.h"
  
  #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
  
  static void efx_release_cxl_region(void *priv_cxl)
  {
  	struct efx_probe_data *probe_data = priv_cxl;
+	struct efx_nic *efx = &probe_data->efx;
  	struct efx_cxl *cxl = probe_data->cxl;
  
+	/* Next avoid contention with efx_cxl_exit() */
  	probe_data->cxl_pio_initialised = false;
+
+	/* Next makes cxl-based piobus to no be used */
+	efx_ef10_disable_piobufs(efx);
  	iounmap(cxl->ctpio_cxl);
+  

Avoid extra white space changes. Perhaps push to earlier patch.  

I'll fix the spaces. Not sure what you mean with the second part of your 
comment, but if I understand it right, I think those changes should be 
added in this patch, just when the final functionality is added.

Just the white space.  If you want that move it to where that iounmap() is
added.  This is just a patch cleanliness thing.

FWIW, I have decided to drop this driver callback as Dan did not like 
it, and after realizing those Dan's patches this patchset relies on fix 
most of the problem this callback tried to address.

quoted

quoted

  	cxl_put_root_decoder(cxl->cxlrd);
  }
  

@@ -30,6 +37,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
  	struct pci_dev *pci_dev = efx->pci_dev;
  	resource_size_t max_size;
  	struct efx_cxl *cxl;
+	struct range range;
  	u16 dvsec;
  	int rc;
  

@@ -133,17 +141,34 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
  					    &probe_data);
  	if (IS_ERR(cxl->efx_region)) {
  		pci_err(pci_dev, "CXL accel create region failed");
-		cxl_dpa_free(cxl->cxled);
  		rc = PTR_ERR(cxl->efx_region);
-		goto err_decoder;
+		goto err_dpa;  

Why do we now need to call cxl_dpa_free() and didn't previously here? That
seems like a probably bug in earlier patch.  

I think you misread it. We were calling cxl_dpa_free already, just 
moving it to a goto label here.

Indeed.  Needed more coffee that day (or less, can't remember which ;)