Re: [PATCH v2 04/10] cxl/pci: Use synchronous API for DOE

["Li, Ming" <ming4.li@intel.com>]

On 1/24/2023 8:52 AM, Ira Weiny wrote:
> Lukas Wunner wrote:
>> A synchronous API for DOE has just been introduced.  Convert CXL CDAT
>> retrieval over to it.
>>
>> Tested-by: Ira Weiny <ira.weiny@intel.com>
> 
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> 
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
>> ---
>>  drivers/cxl/core/pci.c | 62 ++++++++++++++----------------------------
>>  1 file changed, 20 insertions(+), 42 deletions(-)
>>

......

>>  static int cxl_cdat_get_length(struct device *dev,
>>  			       struct pci_doe_mb *cdat_doe,
>>  			       size_t *length)
>>  {
>> -	DECLARE_CDAT_DOE_TASK(CDAT_DOE_REQ(0), t);
>> +	u32 request = CDAT_DOE_REQ(0);
>> +	u32 response[32];
>>  	int rc;
>>  
>> -	rc = pci_doe_submit_task(cdat_doe, &t.task);
>> +	rc = pci_doe(cdat_doe, PCI_DVSEC_VENDOR_ID_CXL,
>> +		     CXL_DOE_PROTOCOL_TABLE_ACCESS,
>> +		     &request, sizeof(request),
>> +		     &response, sizeof(response));
>>  	if (rc < 0) {
>> -		dev_err(dev, "DOE submit failed: %d", rc);
>> +		dev_err(dev, "DOE failed: %d", rc);
>>  		return rc;
>>  	}
>> -	wait_for_completion(&t.c);
>> -	if (t.task.rv < sizeof(u32))
>> +	if (rc < sizeof(u32))
>>  		return -EIO;
>>  

Sorry, I didn't find the original patchset email, only can reply here.
Should this "if (rc < sizeof(u32))" be "if (rc < 2 * sizeof(u32))"?
Because below code used response[1] directly, that means we need unless two u32 in response payload.

Thanks
Ming 

>> -	*length = t.response_pl[1];
>> +	*length = response[1]>>  	dev_dbg(dev, "CDAT length %zu\n", *length);
>>  
>>  	return 0;
>> @@ -546,26 +521,29 @@ static int cxl_cdat_read_table(struct device *dev,
>>  	int entry_handle = 0;
>>  
>>  	do {
>> -		DECLARE_CDAT_DOE_TASK(CDAT_DOE_REQ(entry_handle), t);
>> +		u32 request = CDAT_DOE_REQ(entry_handle);
>> +		u32 response[32];
>>  		size_t entry_dw;
>>  		u32 *entry;
>>  		int rc;
>>  
>> -		rc = pci_doe_submit_task(cdat_doe, &t.task);
>> +		rc = pci_doe(cdat_doe, PCI_DVSEC_VENDOR_ID_CXL,
>> +			     CXL_DOE_PROTOCOL_TABLE_ACCESS,
>> +			     &request, sizeof(request),
>> +			     &response, sizeof(response));
>>  		if (rc < 0) {
>> -			dev_err(dev, "DOE submit failed: %d", rc);
>> +			dev_err(dev, "DOE failed: %d", rc);
>>  			return rc;
>>  		}
>> -		wait_for_completion(&t.c);
>>  		/* 1 DW header + 1 DW data min */
>> -		if (t.task.rv < (2 * sizeof(u32)))
>> +		if (rc < (2 * sizeof(u32)))
>>  			return -EIO;
>>  
>>  		/* Get the CXL table access header entry handle */
>>  		entry_handle = FIELD_GET(CXL_DOE_TABLE_ACCESS_ENTRY_HANDLE,
>> -					 t.response_pl[0]);
>> -		entry = t.response_pl + 1;
>> -		entry_dw = t.task.rv / sizeof(u32);
>> +					 response[0]);
>> +		entry = response + 1;
>> +		entry_dw = rc / sizeof(u32);
>>  		/* Skip Header */
>>  		entry_dw -= 1;
>>  		entry_dw = min(length / sizeof(u32), entry_dw);
>> -- 
>> 2.39.1
>>
> 
>