Re: [RFC PATCH 03/13] cxl: export core function for type2 devices

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

On 6/12/24 05:50, Dan Williams wrote:
> alucerop@ wrote:
>> From: Alejandro Lucero <alucerop@amd.com>
>>
>> CXL initialization by type2 devices requires to use current CXL kernel
>> infrastructure only available to such core code. Type2 devices are by
>> definition owned by specific vendor drivers which need to use part of
>> that infrastructure for initialization.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
>> ---
>>   drivers/cxl/pci.c                   |  3 ++-
>>   include/linux/cxlpci.h              |  2 ++
>>   tools/testing/cxl/type2/pci_type2.c | 31 +++++++++++++++++++++++++++++
>>   3 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index ccde33ac9c1c..497276302017 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -500,7 +500,7 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
>>   	return 0;
>>   }
>>   
>> -static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>> +int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>>   			      struct cxl_register_map *map)
>>   {
>>   	int rc;
>> @@ -520,6 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>>   
>>   	return cxl_setup_regs(map);
>>   }
>> +EXPORT_SYMBOL_NS_GPL(cxl_pci_setup_regs, CXL);
> Any functionality in cxl_pci that you want to export to a 3rd party CXL
> driver needs to move to drivers/cxl/core/pci.c


OK.


>>   
>>   static int cxl_pci_ras_unmask(struct pci_dev *pdev)
>>   {
>> diff --git a/include/linux/cxlpci.h b/include/linux/cxlpci.h
>> index 93992a1c8eec..28fa4861a4f9 100644
>> --- a/include/linux/cxlpci.h
>> +++ b/include/linux/cxlpci.h
>> @@ -130,4 +130,6 @@ void read_cdat_data(struct cxl_port *port);
>>   void cxl_cor_error_detected(struct pci_dev *pdev);
>>   pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>>   				    pci_channel_state_t state);
>> +int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>> +		       struct cxl_register_map *map);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/tools/testing/cxl/type2/pci_type2.c b/tools/testing/cxl/type2/pci_type2.c
>> index 863ce7dc28ef..b12f13e676fb 100644
>> --- a/tools/testing/cxl/type2/pci_type2.c
>> +++ b/tools/testing/cxl/type2/pci_type2.c
>> @@ -12,7 +12,9 @@ static int type2_pci_probe(struct pci_dev *pci_dev,
>>   			   const struct pci_device_id *entry)
>>   
>>   {
>> +	struct cxl_register_map map;
>>   	u16 dvsec;
>> +	int rc;
>>   
>>   	dvsec = pci_find_dvsec_capability(pci_dev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE);
>>   
>> @@ -35,6 +37,35 @@ static int type2_pci_probe(struct pci_dev *pci_dev,
>>   	cxlds->dpa_res = DEFINE_RES_MEM(0, CXL_TYPE2_MEM_SIZE);
>>   	cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, CXL_TYPE2_MEM_SIZE, "ram");
>>   
>> +	rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_MEMDEV, &map);
>> +	if (rc)
>> +		return rc;
>> +
>> +	rc = cxl_map_device_regs(&map, &cxlds->regs.device_regs);
>> +	if (rc)
>> +		return rc;
>> +
>> +	rc = cxl_pci_setup_regs(pci_dev, CXL_REGLOC_RBI_COMPONENT,
>> +				&cxlds->reg_map);
>> +	if (rc)
>> +		dev_warn(&pci_dev->dev, "No component registers (%d)\n", rc);
>> +
>> +	rc = cxl_map_component_regs(&cxlds->reg_map, &cxlds->regs.component,
>> +				    BIT(CXL_CM_CAP_CAP_ID_RAS));
>> +	if (rc)
>> +		dev_dbg(&pci_dev->dev, "Failed to map RAS capability.\n");
>> +
>> +	pci_info(pci_dev, "requesting resource...");
> Setting aside whether this driver moves forward vs a cxl_test mock, if
> you want the driver to be chatty use pci_dbg() or dev_dbg() not pci_info().
>

It seems this driver will be removed in future versions or probably in a 
new patchset.

Thanks