Re: [PATCH v24 08/11] cxl: Export function for unwinding cxl by accelerators

[Alejandro Lucero Palau <alucerop@amd.com>]

On 3/24/26 16:50, Jonathan Cameron wrote:
> On Mon, 23 Mar 2026 11:31:14 +0000
> alejandro.lucero-palau@amd.com wrote:
>
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Add cxl_unregister_region() to the accelerator driver API
>> for a clean exit.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> For this I'm seeing an outstanding question from Gregory on v23.


Yes, I did not forget about it but I forgot to comment back.


The kreview is obviously wrong but I think it points out to the missing 
invocation I need to add. I did not suffer those problems Gregory 
mentions, but I think that is due to how I configure my testing kernel. 
So, yes, I need to remove the release action from inside this function 
to avoid invoking ... this same function. That is another thing that 
puzzled me when I saw that review, and it implies I need to have 
different functions for accelerator driver specific invocation and for 
normal release action call. Or maybe just an extra param only removing 
the devm release action if invoked by accelerators. I think I will go 
with this last one. Any suggestion?


Thank you


>> ---
>>   drivers/cxl/core/region.c | 17 ++++++++++++-----
>>   include/cxl/cxl.h         |  1 +
>>   2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 9c3ba911d865..8bb53a095290 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -2468,9 +2468,8 @@ static struct cxl_region *to_cxl_region(struct device *dev)
>>   	return container_of(dev, struct cxl_region, dev);
>>   }
>>   
>> -static void unregister_region(void *_cxlr)
>> +void cxl_unregister_region(struct cxl_region *cxlr)
>>   {
>> -	struct cxl_region *cxlr = _cxlr;
>>   	struct cxl_region_params *p = &cxlr->params;
>>   	int i;
>>   
>> @@ -2489,6 +2488,14 @@ static void unregister_region(void *_cxlr)
>>   	cxl_region_iomem_release(cxlr);
>>   	put_device(&cxlr->dev);
>>   }
>> +EXPORT_SYMBOL_NS_GPL(cxl_unregister_region, "CXL");
>> +
>> +static void __unregister_region(void *_cxlr)
>> +{
>> +	struct cxl_region *cxlr = _cxlr;
>> +
>> +	return cxl_unregister_region(cxlr);
>> +}
>>   
>>   static struct lock_class_key cxl_region_key;
>>   
>> @@ -2642,7 +2649,7 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
>>   	if (rc)
>>   		goto err;
>>   
>> -	rc = devm_add_action_or_reset(port->uport_dev, unregister_region, cxlr);
>> +	rc = devm_add_action_or_reset(port->uport_dev, __unregister_region, cxlr);
>>   	if (rc)
>>   		return ERR_PTR(rc);
>>   
>> @@ -2797,7 +2804,7 @@ static ssize_t delete_region_store(struct device *dev,
>>   	if (IS_ERR(cxlr))
>>   		return PTR_ERR(cxlr);
>>   
>> -	devm_release_action(port->uport_dev, unregister_region, cxlr);
>> +	devm_release_action(port->uport_dev, __unregister_region, cxlr);
>>   	put_device(&cxlr->dev);
>>   
>>   	return len;
>> @@ -3963,7 +3970,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>>   
>>   	rc = __construct_region(cxlr, ctx);
>>   	if (rc) {
>> -		devm_release_action(port->uport_dev, unregister_region, cxlr);
>> +		devm_release_action(port->uport_dev, __unregister_region, cxlr);
>>   		return ERR_PTR(rc);
>>   	}
>>   
>> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
>> index 122b3331469d..b8cdc604480a 100644
>> --- a/include/cxl/cxl.h
>> +++ b/include/cxl/cxl.h
>> @@ -236,4 +236,5 @@ struct cxl_endpoint_decoder *cxl_get_committed_decoder(struct cxl_memdev *cxlmd,
>>   						       struct cxl_region **cxlr);
>>   struct range;
>>   int cxl_get_region_range(struct cxl_region *region, struct range *range);
>> +void cxl_unregister_region(struct cxl_region *cxlr);
>>   #endif /* __CXL_CXL_H__ */