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

Alejandro Lucero Palau
2024-09-16T12:34:39Z

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