[PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()

Terry Bowman posted 34 patches 3 weeks, 4 days ago
There is a newer version of this series
[PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
Posted by Terry Bowman 3 weeks, 4 days ago
From: Dan Williams <dan.j.williams@intel.com>

The convention for devm_ helpers in the CXL driver is that the first
argument is the @host for the operation (locked driver::probe() context).

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Terry Bowman <terry.bowman@amd.com>

---

Changes in v13 -> v14:
- New patch
---
 drivers/cxl/core/pmem.c | 13 +++++++------
 drivers/cxl/cxl.h       |  3 ++-
 drivers/cxl/mem.c       |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
index 8853415c106a..e7b1e6fa0ea0 100644
--- a/drivers/cxl/core/pmem.c
+++ b/drivers/cxl/core/pmem.c
@@ -237,12 +237,13 @@ static void cxlmd_release_nvdimm(void *_cxlmd)
 
 /**
  * devm_cxl_add_nvdimm() - add a bridge between a cxl_memdev and an nvdimm
- * @parent_port: parent port for the (to be added) @cxlmd endpoint port
- * @cxlmd: cxl_memdev instance that will perform LIBNVDIMM operations
+ * @host: host device for devm operations
+ * @port: any port in the CXL topology to find the nvdimm-bridge device
+ * @cxlmd: parent of the to be created cxl_nvdimm device
  *
  * Return: 0 on success negative error code on failure.
  */
-int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
+int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,
 			struct cxl_memdev *cxlmd)
 {
 	struct cxl_nvdimm_bridge *cxl_nvb;
@@ -250,7 +251,7 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
 	struct device *dev;
 	int rc;
 
-	cxl_nvb = cxl_find_nvdimm_bridge(parent_port);
+	cxl_nvb = cxl_find_nvdimm_bridge(port);
 	if (!cxl_nvb)
 		return -ENODEV;
 
@@ -270,10 +271,10 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
 	if (rc)
 		goto err;
 
-	dev_dbg(&cxlmd->dev, "register %s\n", dev_name(dev));
+	dev_dbg(host, "register %s\n", dev_name(dev));
 
 	/* @cxlmd carries a reference on @cxl_nvb until cxlmd_release_nvdimm */
-	return devm_add_action_or_reset(&cxlmd->dev, cxlmd_release_nvdimm, cxlmd);
+	return devm_add_action_or_reset(host, cxlmd_release_nvdimm, cxlmd);
 
 err:
 	put_device(dev);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 42a76a7a088f..6f3741a57932 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -887,7 +887,8 @@ struct cxl_nvdimm_bridge *devm_cxl_add_nvdimm_bridge(struct device *host,
 						     struct cxl_port *port);
 struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
 bool is_cxl_nvdimm(struct device *dev);
-int devm_cxl_add_nvdimm(struct cxl_port *parent_port, struct cxl_memdev *cxlmd);
+int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,
+			struct cxl_memdev *cxlmd);
 struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port);
 
 #ifdef CONFIG_CXL_REGION
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 6e6777b7bafb..c2ee7f7f6320 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -153,7 +153,7 @@ static int cxl_mem_probe(struct device *dev)
 	}
 
 	if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
-		rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
+		rc = devm_cxl_add_nvdimm(dev, parent_port, cxlmd);
 		if (rc) {
 			if (rc == -ENODEV)
 				dev_info(dev, "PMEM disabled by platform\n");
-- 
2.34.1
Re: [PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
Posted by Dave Jiang 3 weeks, 4 days ago

On 1/14/26 11:20 AM, Terry Bowman wrote:
> From: Dan Williams <dan.j.williams@intel.com>
> 
> The convention for devm_ helpers in the CXL driver is that the first
> argument is the @host for the operation (locked driver::probe() context).
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Terry Bowman <terry.bowman@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

A nit below

> 
> ---
> 
> Changes in v13 -> v14:
> - New patch
> ---
>  drivers/cxl/core/pmem.c | 13 +++++++------
>  drivers/cxl/cxl.h       |  3 ++-
>  drivers/cxl/mem.c       |  2 +-
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
> index 8853415c106a..e7b1e6fa0ea0 100644
> --- a/drivers/cxl/core/pmem.c
> +++ b/drivers/cxl/core/pmem.c
> @@ -237,12 +237,13 @@ static void cxlmd_release_nvdimm(void *_cxlmd)
>  
>  /**
>   * devm_cxl_add_nvdimm() - add a bridge between a cxl_memdev and an nvdimm
> - * @parent_port: parent port for the (to be added) @cxlmd endpoint port
> - * @cxlmd: cxl_memdev instance that will perform LIBNVDIMM operations
> + * @host: host device for devm operations
> + * @port: any port in the CXL topology to find the nvdimm-bridge device
> + * @cxlmd: parent of the to be created cxl_nvdimm device
>   *
>   * Return: 0 on success negative error code on failure.
>   */
> -int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
> +int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,

s/port/parent_port/ to maintain clarity of the port

DJ

>  			struct cxl_memdev *cxlmd)
>  {
>  	struct cxl_nvdimm_bridge *cxl_nvb;
> @@ -250,7 +251,7 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
>  	struct device *dev;
>  	int rc;
>  
> -	cxl_nvb = cxl_find_nvdimm_bridge(parent_port);
> +	cxl_nvb = cxl_find_nvdimm_bridge(port);
>  	if (!cxl_nvb)
>  		return -ENODEV;
>  
> @@ -270,10 +271,10 @@ int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
>  	if (rc)
>  		goto err;
>  
> -	dev_dbg(&cxlmd->dev, "register %s\n", dev_name(dev));
> +	dev_dbg(host, "register %s\n", dev_name(dev));
>  
>  	/* @cxlmd carries a reference on @cxl_nvb until cxlmd_release_nvdimm */
> -	return devm_add_action_or_reset(&cxlmd->dev, cxlmd_release_nvdimm, cxlmd);
> +	return devm_add_action_or_reset(host, cxlmd_release_nvdimm, cxlmd);
>  
>  err:
>  	put_device(dev);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 42a76a7a088f..6f3741a57932 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -887,7 +887,8 @@ struct cxl_nvdimm_bridge *devm_cxl_add_nvdimm_bridge(struct device *host,
>  						     struct cxl_port *port);
>  struct cxl_nvdimm *to_cxl_nvdimm(struct device *dev);
>  bool is_cxl_nvdimm(struct device *dev);
> -int devm_cxl_add_nvdimm(struct cxl_port *parent_port, struct cxl_memdev *cxlmd);
> +int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,
> +			struct cxl_memdev *cxlmd);
>  struct cxl_nvdimm_bridge *cxl_find_nvdimm_bridge(struct cxl_port *port);
>  
>  #ifdef CONFIG_CXL_REGION
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 6e6777b7bafb..c2ee7f7f6320 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -153,7 +153,7 @@ static int cxl_mem_probe(struct device *dev)
>  	}
>  
>  	if (cxl_pmem_size(cxlds) && IS_ENABLED(CONFIG_CXL_PMEM)) {
> -		rc = devm_cxl_add_nvdimm(parent_port, cxlmd);
> +		rc = devm_cxl_add_nvdimm(dev, parent_port, cxlmd);
>  		if (rc) {
>  			if (rc == -ENODEV)
>  				dev_info(dev, "PMEM disabled by platform\n");
Re: [PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
Posted by dan.j.williams@intel.com 3 weeks, 2 days ago
Dave Jiang wrote:
> 
> 
> On 1/14/26 11:20 AM, Terry Bowman wrote:
> > From: Dan Williams <dan.j.williams@intel.com>
> > 
> > The convention for devm_ helpers in the CXL driver is that the first
> > argument is the @host for the operation (locked driver::probe() context).
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > Reviewed-by: Terry Bowman <terry.bowman@amd.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> A nit below
> 
> > 
> > ---
> > 
> > Changes in v13 -> v14:
> > - New patch
> > ---
> >  drivers/cxl/core/pmem.c | 13 +++++++------
> >  drivers/cxl/cxl.h       |  3 ++-
> >  drivers/cxl/mem.c       |  2 +-
> >  3 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
> > index 8853415c106a..e7b1e6fa0ea0 100644
> > --- a/drivers/cxl/core/pmem.c
> > +++ b/drivers/cxl/core/pmem.c
> > @@ -237,12 +237,13 @@ static void cxlmd_release_nvdimm(void *_cxlmd)
> >  
> >  /**
> >   * devm_cxl_add_nvdimm() - add a bridge between a cxl_memdev and an nvdimm
> > - * @parent_port: parent port for the (to be added) @cxlmd endpoint port
> > - * @cxlmd: cxl_memdev instance that will perform LIBNVDIMM operations
> > + * @host: host device for devm operations
> > + * @port: any port in the CXL topology to find the nvdimm-bridge device
> > + * @cxlmd: parent of the to be created cxl_nvdimm device
> >   *
> >   * Return: 0 on success negative error code on failure.
> >   */
> > -int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
> > +int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,
> 
> s/port/parent_port/ to maintain clarity of the port

...but it is not used as a "parent" port in this function. Any port in
the topology will do. The reason a port argument is needed is
disambiguate when there are multiple CXL root devices. That currently
only happens when cxl_test is loaded.

However, after writing that, it may make more sense to make that
semantic explicit and just have the caller responsible for passing in an
@cxl_root argument.

A change for not this series.
Re: [PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
Posted by Dave Jiang 3 weeks, 2 days ago

On 1/15/26 8:07 PM, dan.j.williams@intel.com wrote:
> Dave Jiang wrote:
>>
>>
>> On 1/14/26 11:20 AM, Terry Bowman wrote:
>>> From: Dan Williams <dan.j.williams@intel.com>
>>>
>>> The convention for devm_ helpers in the CXL driver is that the first
>>> argument is the @host for the operation (locked driver::probe() context).
>>>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> Reviewed-by: Terry Bowman <terry.bowman@amd.com>
>>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>
>> A nit below
>>
>>>
>>> ---
>>>
>>> Changes in v13 -> v14:
>>> - New patch
>>> ---
>>>  drivers/cxl/core/pmem.c | 13 +++++++------
>>>  drivers/cxl/cxl.h       |  3 ++-
>>>  drivers/cxl/mem.c       |  2 +-
>>>  3 files changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
>>> index 8853415c106a..e7b1e6fa0ea0 100644
>>> --- a/drivers/cxl/core/pmem.c
>>> +++ b/drivers/cxl/core/pmem.c
>>> @@ -237,12 +237,13 @@ static void cxlmd_release_nvdimm(void *_cxlmd)
>>>  
>>>  /**
>>>   * devm_cxl_add_nvdimm() - add a bridge between a cxl_memdev and an nvdimm
>>> - * @parent_port: parent port for the (to be added) @cxlmd endpoint port
>>> - * @cxlmd: cxl_memdev instance that will perform LIBNVDIMM operations
>>> + * @host: host device for devm operations
>>> + * @port: any port in the CXL topology to find the nvdimm-bridge device
>>> + * @cxlmd: parent of the to be created cxl_nvdimm device
>>>   *
>>>   * Return: 0 on success negative error code on failure.
>>>   */
>>> -int devm_cxl_add_nvdimm(struct cxl_port *parent_port,
>>> +int devm_cxl_add_nvdimm(struct device *host, struct cxl_port *port,
>>
>> s/port/parent_port/ to maintain clarity of the port
> 
> ...but it is not used as a "parent" port in this function. Any port in
> the topology will do. The reason a port argument is needed is
> disambiguate when there are multiple CXL root devices. That currently
> only happens when cxl_test is loaded.
> 
> However, after writing that, it may make more sense to make that
> semantic explicit and just have the caller responsible for passing in an
> @cxl_root argument.
> 
> A change for not this series.

I'll make a note in the backlog.
Re: [PATCH v14 16/34] cxl/mem: Clarify @host for devm_cxl_add_nvdimm()
Posted by Jonathan Cameron 3 weeks, 4 days ago
On Wed, 14 Jan 2026 12:20:37 -0600
Terry Bowman <terry.bowman@amd.com> wrote:

> From: Dan Williams <dan.j.williams@intel.com>
> 
> The convention for devm_ helpers in the CXL driver is that the first
> argument is the @host for the operation (locked driver::probe() context).
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Terry Bowman <terry.bowman@amd.com>

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>