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

[Dave Jiang]

On 1/15/26 8:07 PM, dan.j.williams@xxxxxxxxx wrote:
> Dave Jiang wrote:
>>
>>
>> On 1/14/26 11:20 AM, Terry Bowman wrote:
>>> From: Dan Williams <dan.j.williams@xxxxxxxxx>
>>>
>>> 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@xxxxxxxxx>
>>> Reviewed-by: Terry Bowman <terry.bowman@xxxxxxx>
>>
>> Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>>
>> 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.