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

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

On 2/3/2023 4:53 PM, Li, Ming wrote:
> 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.

Sorry, at least(not 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
>>>
>>
>>
>