Re: [PATCH v3 06/20] cxl: add functions for resource request/release by a driver

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

On 9/13/24 18:35, Jonathan Cameron wrote:
> On Sat, 7 Sep 2024 09:18:22 +0100
> alejandro.lucero-palau@amd.com wrote:
>
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> Create accessors for an accel driver requesting and
>> releaseing a resource.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/cxl/core/memdev.c          | 40 ++++++++++++++++++++++++++++++
>>   drivers/net/ethernet/sfc/efx_cxl.c |  7 ++++++
>>   include/linux/cxl/cxl.h            |  2 ++
>>   3 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
>> index 10c0a6990f9a..a7d8daf4a59b 100644
>> --- a/drivers/cxl/core/memdev.c
>> +++ b/drivers/cxl/core/memdev.c
>> @@ -744,6 +744,46 @@ int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res,
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL);
>>   
>> +int cxl_request_resource(struct cxl_dev_state *cxlds, enum cxl_resource type)
>> +{
>> +	int rc;
>> +
>> +	switch (type) {
>> +	case CXL_ACCEL_RES_RAM:
>> +		rc = request_resource(&cxlds->dpa_res, &cxlds->ram_res);
>> +		break;
>> +	case CXL_ACCEL_RES_PMEM:
>> +		rc = request_resource(&cxlds->dpa_res, &cxlds->pmem_res);
>> +		break;
> return request_resource()


Yes.


>> +	default:
>> +		dev_err(cxlds->dev, "unknown resource type (%u)\n", type);
> No unknown. We know exactly what it is (DPA) but we don't have it.
> Unexpected maybe?


Is this not the same case that you brought in previously? Should I keep 
the default?


>> +		return -EINVAL;
>> +	}
>> +
>> +	return rc;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_request_resource, CXL);
>> +
>> +int cxl_release_resource(struct cxl_dev_state *cxlds, enum cxl_resource type)
>> +{
>> +	int rc;
>> +
>> +	switch (type) {
>> +	case CXL_ACCEL_RES_RAM:
>> +		rc = release_resource(&cxlds->ram_res);
>> +		break;
>> +	case CXL_ACCEL_RES_PMEM:
>> +		rc = release_resource(&cxlds->pmem_res);
> return ..


Sure.

Thanks


>> +		break;
>> +	default:
>> +		dev_err(cxlds->dev, "unknown resource type (%u)\n", type);
> As above. Probably know what we got, it it unexpected not unknown.
>
>> +		return -EINVAL;
>> +	}
>> +
>> +	return rc;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_release_resource, CXL);
>> +
>>   static int cxl_memdev_release_file(struct inode *inode, struct file *file)
>>   {
>>   	struct cxl_memdev *cxlmd =
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index fee143e94c1f..80259c8317fd 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -72,6 +72,12 @@ int efx_cxl_init(struct efx_nic *efx)
>>   		goto err;
>>   	}
>>   
>> +	rc = cxl_request_resource(cxl->cxlds, CXL_ACCEL_RES_RAM);
>> +	if (rc) {
>> +		pci_err(pci_dev, "CXL request resource failed");
>> +		goto err;
>> +	}
>> +
>>   	return 0;
>>   err:
>>   	kfree(cxl->cxlds);
>> @@ -84,6 +90,7 @@ int efx_cxl_init(struct efx_nic *efx)
>>   void efx_cxl_exit(struct efx_nic *efx)
>>   {
>>   	if (efx->cxl) {
>> +		cxl_release_resource(efx->cxl->cxlds, CXL_ACCEL_RES_RAM);
>>   		kfree(efx->cxl->cxlds);
>>   		kfree(efx->cxl);
>>   	}
>> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h
>> index f2dcba6cdc22..22912b2d9bb2 100644
>> --- a/include/linux/cxl/cxl.h
>> +++ b/include/linux/cxl/cxl.h
>> @@ -52,4 +52,6 @@ int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res,
>>   bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, u32 expected_caps,
>>   			u32 *current_caps);
>>   int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds);
>> +int cxl_request_resource(struct cxl_dev_state *cxlds, enum cxl_resource type);
>> +int cxl_release_resource(struct cxl_dev_state *cxlds, enum cxl_resource type);
>>   #endif